diff mbox

[Qemu-ppc] macio ide question/bug report

Message ID 5375F4F2.9030804@ilande.co.uk
State New
Headers show

Commit Message

Mark Cave-Ayland May 16, 2014, 11:22 a.m. UTC 
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.

Comments

BALATON Zoltan May 16, 2014, 8:31 p.m. UTC | #1 
On Fri, 16 May 2014, Mark Cave-Ayland wrote:
> 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.

This seems to have worked and now it mounts its boot cd and seems to boot 
afterwards. At least it generated a lot of logs that I'm currently looking 
at which end with an error about failing to open the display so it looks 
good so far. (I'm sure there are more problems ahead but hopefully I can 
now look at the display problem which is the next big thing.)

Thank you very much for helping with this. You have saved me a lot of time 
by providing this patch so I did not have to dig up all the pieces that 
are scattered in the qemu sources that are involved in this DMA transfer. 
Understanding it conceptually is one thing but finding all the details in 
the several files involved to get the details is more difficult.

Regards,
BALATON Zoltan
diff mbox

Patch

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;