Re: [Qemu-devel] [PATCHv2] macio: handle non-block ATAPI DMA transfers the same as block DMA transfers

[Mark Cave-Ayland]

On 01/08/15 19:33, Aurelien Jarno wrote:

> On 2015-08-01 17:54, Mark Cave-Ayland wrote:
>> The existing code incorrectly changes the dma_active flag when a non-block
>> transfer has completed leading to a hang on newer versions of Linux because 
>> the
>> IDE and DMA engines deadlock waiting for each other.
>>
>> Instead copy the buffer directly to RAM, set the remaining transfer size to 
>> 0 and
>> then invoke the ATAPI callback manually once again to correctly finish the
>> transfer in an identical manner to a block transfer.
>>
>> Signed-off-by: Mark Cave-Ayland <address@hidden>
>> ---
> 
> Thanks for the patch, it improves things here. I don't get messages
> anymore, and I get less messages when mounting the CD-ROM, though I
> still get one:
> 
>   [  307.258463] pata-macio 0.00021000:ata-4: timeout flushing DMA
>   [  307.262856] ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
>   [  307.262919] ata2.00: BMDMA stat 0x6
>   [  307.263048] sr 1:0:0:0: CDB: Get configuration: 46 00 00 28 00 00 00 00 
> 10 00
>   [  307.263289] ata2.00: cmd a0/01:00:00:10:00/00:00:00:00:00/a0 tag 0 dma 
> 16400 in
>   [  307.263297]          res 41/50:03:00:10:00/00:00:00:00:00/a0 Emask 0x20 
> (host bus error)
>   [  307.263407] ata2.00: status: { DRDY ERR }
>   [  307.271251] ata2.00: configured for MWDMA2
>   [  307.271824] ata2: EH complete
> 
> The CD-ROM is fully functional though.

Apologies for the delay in getting back to you on this one, however I
finally found some time today to try and work out what is happening.
Attached is a v3 patch that works here for your lenny and wheezy
standard images - please can you test and confirm it works for you?
Debugging is enabled just in case anything unusual shows up during your
testing.

I've made a couple of minor tweaks to v2: firstly I don't call
pmac_ide_atapi_transfer_cb() to go around again, since I was seeing the
dbdma_end() function being called more than once when lba == -1.
Secondly, I've moved the DBDMA flush out of the RUN status check section
so it is called unconditionally, and removed the second version which
was actually causing the problem above by not clearing the FLUSH bit
upon write.

FWIW with this patch applied I still see an error upon mount with your
squeeze image but I believe that this may be a bug in that particular
version of the Linux kernel. With MACIO and DBDMA logging enabled I see
the following when trying to mount the CDROM:


------------ IDE transfer
buffer_size: 14   buffer_index: 0
lba: ffffffff    size: 14
-------------------------
DBDMA: DBDMA_run_bh
DBDMA: writel 0x0000000000000d00 <= 0x80008000
DBDMA: channel 0x1a reg 0x0
DBDMA:     status 0x00008400
DBDMA: readl 0x0000000000000d00 => 0x80008000
DBDMA: channel 0x1a reg 0x0
DBDMA: DBDMA_run_bh
DBDMA: channel_run
dbdma_cmd 0x252c328
    req_count 0x0020
    command 0x3000
    phy_addr 0x0670e000
    cmd_dep 0x00000000
    res_count 0x0000
    xfer_status 0x0000
DBDMA: start_input
DBDMA: addr 0x670e000 key 0x0

pmac_ide_atapi_transfer_cb
DBDMA: dbdma_end
DBDMA: conditional_wait
DBDMA: dbdma_cmdptr_save 0x05ef2000
DBDMA: xfer_status 0x00008400 res_count 0x0000
DBDMA: conditional_interrupt
DBDMA: conditional_branch
DBDMA: dbdma_cmdptr_load 0x05ef2010
DBDMA: channel_run
dbdma_cmd 0x252c328
    req_count 0x0000
    command 0x7000
    phy_addr 0x00000000
    cmd_dep 0x00000000
    res_count 0x0000
    xfer_status 0x0000
DBDMA: readl 0x0000000000000d04 => 0x00008000
DBDMA: channel 0x1a reg 0x1
DBDMA: readl 0x0000000000000d04 => 0x00008000
DBDMA: channel 0x1a reg 0x1
DBDMA: writel 0x0000000000000d00 <= 0x98000000
DBDMA: channel 0x1a reg 0x0
DBDMA:     status 0x00000000
DBDMA: writel 0x0000000000000d00 <= 0xf8000000
DBDMA: channel 0x1a reg 0x0
DBDMA:     status 0x00000000
DBDMA: readl 0x0000000000000d04 => 0x00000000
DBDMA: channel 0x1a reg 0x1
DBDMA: writel 0x0000000000000d0c <= 0x05ef2000
DBDMA: channel 0x1a reg 0x3
DBDMA: dbdma_cmdptr_load 0x05ef2000


------------ IDE transfer
buffer_size: 1000   buffer_index: 0
lba: 21260    size: 1000
-------------------------

..etc..


Tracing through the kernel source I can see that the DMA read error is
being caused by pmac_ide_dma_end() returning a non-zero value:


static int
pmac_ide_dma_end (ide_drive_t *drive)
{
        ide_hwif_t *hwif = drive->hwif;
        pmac_ide_hwif_t *pmif =
                (pmac_ide_hwif_t *)dev_get_drvdata(hwif->gendev.parent);
        volatile struct dbdma_regs __iomem *dma = pmif->dma_regs;
        u32 dstat;

        dstat = readl(&dma->status);
        writel(((RUN|WAKE|DEAD) << 16), &dma->control);

        /* verify good dma status. we don't check for ACTIVE beeing 0.
We should...
         * in theory, but with ATAPI decices doing buffer underruns,
that would
         * cause us to disable DMA, which isn't what we want
         */
        return (dstat & (RUN|DEAD)) != RUN;
}


Right at the very end of the CDROM code the problem was being caused by
the very last request (very similar to the initial request above) that
looked like this:


------------ IDE transfer
buffer_size: 14   buffer_index: 0
lba: ffffffff    size: 14
-------------------------
DBDMA: writel 0x0000000000000d00 <= 0x80008000
DBDMA: channel 0x1a reg 0x0
DBDMA:     status 0x00008400
DBDMA: readl 0x0000000000000d00 => 0x80008000
DBDMA: channel 0x1a reg 0x0
DBDMA: DBDMA_run_bh
DBDMA: channel_run
dbdma_cmd 0x252c328
    req_count 0x0020
    command 0x3000
    phy_addr 0x05e10000
    cmd_dep 0x00000000
    res_count 0x0000
    xfer_status 0x0000
DBDMA: start_input
DBDMA: addr 0x5e10000 key 0x0

pmac_ide_atapi_transfer_cb
DBDMA: dbdma_end
DBDMA: conditional_wait
DBDMA: dbdma_cmdptr_save 0x05ef2000
DBDMA: xfer_status 0x00008400 res_count 0x0000
DBDMA: conditional_interrupt
DBDMA: conditional_branch
DBDMA: dbdma_cmdptr_load 0x05ef2010
DBDMA: channel_run
dbdma_cmd 0x252c328
    req_count 0x0000
    command 0x7000
    phy_addr 0x00000000
    cmd_dep 0x00000000
    res_count 0x0000
    xfer_status 0x0000
DBDMA: readl 0x0000000000000d04 => 0x00008000
DBDMA: channel 0x1a reg 0x1
DBDMA: readl 0x0000000000000d04 => 0x00008000
DBDMA: channel 0x1a reg 0x1
DBDMA: writel 0x0000000000000d00 <= 0x98000000
DBDMA: channel 0x1a reg 0x0
DBDMA:     status 0x00000000
DBDMA: writel 0x0000000000000d00 <= 0xf8000000
DBDMA: channel 0x1a reg 0x0
DBDMA:     status 0x00000000
DBDMA: readl 0x0000000000000d04 => 0x00000000
DBDMA: channel 0x1a reg 0x1
DBDMA: writel 0x0000000000000d0c <= 0x05ef2000
DBDMA: channel 0x1a reg 0x3
DBDMA: dbdma_cmdptr_load 0x05ef2000
DBDMA: readl 0x0000000000000d04 => 0x00000000
DBDMA: channel 0x1a reg 0x1

DBDMA: readl 0x0000000000000d04 => 0x00000000
                                   ^^^^^^^^^^
DBDMA: channel 0x1a reg 0x1
DBDMA: writel 0x0000000000000d00 <= 0x98000000


DBDMA: channel 0x1a reg 0x0
DBDMA:     status 0x00000000
DBDMA: writel 0x0000000000000b00 <= 0xf8000000
DBDMA: channel 0x16 reg 0x0
DBDMA:     status 0x00000000
DBDMA: readl 0x0000000000000b04 => 0x00000000
DBDMA: channel 0x16 reg 0x1
DBDMA: writel 0x0000000000000b0c <= 0x05eea000
DBDMA: channel 0x16 reg 0x3
DBDMA: dbdma_cmdptr_load 0x05eea000


For some reason pmac_ide_dma_end() was being called without an active
DMA request and so the RUN bit was clear on entry which was causing the
error flag to be set. Given that this doesn't occur on lenny and wheezy,
I'm inclined to believe that this is a bug in the squeeze kernel.

Anyhow if this survives your testing then I'll repost as 2 separate
patches with a CC to qemu-stable so that this also gets picked up in the
next 2.4 release.


ATB,

Mark.

macio-dbdma.patch
Description: Text Data