Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]

On 15/05/14 21:28, BALATON Zoltan wrote:

> On Thu, 15 May 2014, BALATON Zoltan wrote:
>> On Thu, 15 May 2014, BALATON Zoltan wrote:
>>> confusing.) Do you think that replacing io->len in your patch with
>>> s->io_buffer_size would be the correct thing to do?

That looks reasonable, as the MIN() will help prevent excessive memory 
clobber.

>> Probably that's not enough. I've tried it and then it gets to here:
>
> I should've also included these lines too to make it more clear what did
> I change:

Yes, this is definitely helpful. It appears that cmd_read_toc_pma_atip() 
returns 0x20 bytes of data (can you confirm this?), while the DMA engine 
is configured to transfer 0x324 bytes of data - perhaps this is the 
maximum possible size of a TOC?. So the existing code determines there 
are still 0x324 - 0x20 = 0x304 bytes remaining for the DMA request and 
falls into the unaligned code which is definitely not what we want.

Perhaps we need to assume for a non-IO DMA request that the result will 
only be a single ATAPI reply packet? Attached is another version of the 
patch for you to experiment with which makes your s->io_buffer_size 
change but also moves the logic into pmac_ide_transfer() so that we 
don't inadvertently drop into the unaligned code.


ATB,

Mark.


[-- Attachment #2: qemu-macio-atapi-v2.patch --]
[-- Type: text/x-diff, Size: 1115 bytes --]

diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index da94580..0f68124 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -337,6 +337,24 @@ static void pmac_ide_transfer(DBDMA_io *io)
 
     s->io_buffer_size = 0;
     if (s->drive_kind == IDE_CD) {
+        
+        /* Handle non-IO DMA ATAPI transfers */
+        if (s->lba == -1) {
+            s->io_buffer_size = MIN(io->len, s->packet_transfer_size);
+            bdrv_acct_start(s->bs, &s->acct, s->io_buffer_size, BDRV_ACCT_READ);
+            MACIO_DPRINTF("non-IO ATAPI DMA transfer size: %d\n", s->io_buffer_size);
+
+            /* Copy ATAPI buffer directly to RAM and finish */
+            cpu_physical_memory_write(io->addr, s->io_buffer, s->io_buffer_size);
+            ide_atapi_cmd_ok(s);
+            m->dma_active = false;
+            
+            MACIO_DPRINTF("end of non-IO ATAPI DMA transfer\n");
+            bdrv_acct_done(s->bs, &s->acct);
+            io->dma_end(io);
+            return;
+        }
+
         bdrv_acct_start(s->bs, &s->acct, io->len, BDRV_ACCT_READ);
         pmac_ide_atapi_transfer_cb(io, 0);
         return;