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

[BALATON Zoltan]

On Wed, 7 May 2014, Mark Cave-Ayland wrote:
> On 07/05/14 18:00, BALATON Zoltan wrote:
> > On Wed, 7 May 2014, Mark Cave-Ayland wrote:
> > > I'm not sure if your problem related to s->lba == -1 should be solved
> > > just for macio or higher up in the block layer, but the block people
> > > will be on qemu-devel and not qemu-ppc so you should definitely CC the
> > > main list to get their feedback on this.

Cc'd the devel list now, the original question can be found here:
http://lists.nongnu.org/archive/html/qemu-ppc/2014-05/msg00003.html

> > I think it's handled by the block layer but the translation in macio
> > breaks it and converts it to -4 which causes an error so probably it
> > should only be solved for macio. But I hope Alex can look at it and tell
> > for sure or maybe solve it.
>
> Okay. So in that case it could just be a macio bug.
>
> > > Those ISOs only boot for me with the default -M g3beige, but darwin is
> > > a huge culprit for these unaligned accesses. My point was that if you
> > > are making changes in this area, if you can still boot the darwin
> > > images then that would be a good test that any changes you make are
> > > working correctly :)
> >
> > Ah, OK. They boot for me without a -M parameter with my changes (to the
> > point saying "available for installation:" and then empty as I did not
> > specify a hard disk image) too, but I was testing with -M mac99 as that
> > was what I changed. I think they still "work" as before.
>
> But... the block patchset link I sent you has the potential to fix this 
> anyway, since if the unaligned accesses can be passed directly up to the 
> block layer then in theory this problem should go away as this whole chunk of 
> code should no longer be required.
>
> A good test for this would be to try reverting this patch: 
> https://github.com/agraf/qemu/commit/063a333911f1bb09fcc1a4925c8ebf5c88fc2b63 
> which is Alex's original alignment fix. If you find with that with this patch 
> reverted darwin boots as before, it means that the block alignment patches in 
> core are working correctly and hopefully you may find that it fixes your 
> MorphOS problem.
>
> But again, you really should get this on qemu-devel to get the definitive 
> answer from the block guys.

That patch would be 80fc95d8bdaf3392106b131a97ca701fd374489a in QEMU 
master. I've tried reverting it and Darwin still boots (without -M mac99) 
up to the point where it asks to install as before but I don't know how 
good a test is this as I'm not sure it does unaligned accesses up to this 
point. I see accesses like this:

pmac_ide_transfer(ATAPI) lba=ffffffff, buffer_index=30, len=930
pmac_ide_transfer(ATAPI) lba=0, buffer_index=0, len=930
pmac_ide_transfer(ATAPI) lba=0, buffer_index=0, len=800
pmac_ide_transfer(ATAPI) lba=1, buffer_index=0, len=800
pmac_ide_transfer(ATAPI) lba=1, buffer_index=0, len=800
pmac_ide_transfer(ATAPI) lba=2, buffer_index=0, len=800
pmac_ide_transfer(ATAPI) lba=2, buffer_index=0, len=800
pmac_ide_transfer(ATAPI) lba=0, buffer_index=0, len=2000
pmac_ide_transfer(ATAPI) lba=f2, buffer_index=0, len=800
pmac_ide_transfer(ATAPI) lba=f3, buffer_index=0, len=800
pmac_ide_transfer(ATAPI) lba=1ac, buffer_index=0, len=800
pmac_ide_transfer(ATAPI) lba=9ae, buffer_index=0, len=800
pmac_ide_transfer(ATAPI) lba=9af, buffer_index=0, len=400
pmac_ide_transfer(ATAPI) lba=9ae, buffer_index=0, len=400
pmac_ide_transfer(ATAPI) lba=9ae, buffer_index=400, len=400
pmac_ide_transfer(ATAPI) lba=9af, buffer_index=0, len=800
pmac_ide_transfer(ATAPI) lba=11ae, buffer_index=0, len=800
pmac_ide_transfer(ATAPI) lba=11af, buffer_index=0, len=1000
[...]
pmac_ide_transfer(ATAPI) lba=3858e, buffer_index=0, len=2000
pmac_ide_transfer(ATAPI) lba=3856e, buffer_index=0, len=2000
pmac_ide_transfer(ATAPI) lba=38572, buffer_index=0, len=4000
pmac_ide_transfer(ATAPI) lba=38590, buffer_index=0, len=4000
pmac_ide_transfer(ATAPI) lba=38584, buffer_index=0, len=1000
pmac_ide_transfer(ATAPI) lba=38586, buffer_index=0, len=3000
pmac_ide_transfer(ATAPI) lba=3857e, buffer_index=0, len=3000
pmac_ide_transfer(ATAPI) lba=38566, buffer_index=0, len=4000
pmac_ide_transfer(ATAPI) lba=38550, buffer_index=0, len=1000
pmac_ide_transfer(ATAPI) lba=38552, buffer_index=0, len=4000
pmac_ide_transfer(ATAPI) lba=38536, buffer_index=0, len=4000
pmac_ide_transfer(ATAPI) lba=3852c, buffer_index=0, len=5000
pmac_ide_transfer(ATAPI) lba=385a6, buffer_index=0, len=4000
pmac_ide_transfer(ATAPI) lba=a724, buffer_index=0, len=2000
pmac_ide_transfer(ATAPI) lba=3854a, buffer_index=0, len=3000
pmac_ide_transfer(ATAPI) lba=1654, buffer_index=0, len=1000
pmac_ide_transfer(ATAPI) lba=3b1cc, buffer_index=0, len=8000
pmac_ide_transfer(ATAPI) lba=3b3a8, buffer_index=0, len=8000
pmac_ide_transfer(ATAPI) lba=3b3a0, buffer_index=0, len=4000
pmac_ide_transfer(ATAPI) lba=3b3a8, buffer_index=0, len=2000
pmac_ide_transfer(ATAPI) lba=38546, buffer_index=0, len=2000
pmac_ide_transfer(ATAPI) lba=3854a, buffer_index=0, len=8000
pmac_ide_transfer(ATAPI) lba=3b3d8, buffer_index=0, len=8000
pmac_ide_transfer(ATAPI) lba=3b3e8, buffer_index=0, len=7000
pmac_ide_transfer(ATAPI) lba=3b3ca, buffer_index=0, len=7000
pmac_ide_transfer(ATAPI) lba=3b3d8, buffer_index=0, len=1000

This however does not fix MorphOS which fails as before. I've tried this 
patch too (on top of the previous revert) but that did not work either:

diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index d3395f4..bce14fc 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -56,6 +56,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int 
ret)
     DBDMA_io *io = opaque;
     MACIOIDEState *m = io->opaque;
     IDEState *s = idebus_active_if(&m->bus);
+    int64_t sector_num;

     if (ret < 0) {
         m->aiocb = NULL;
@@ -107,13 +108,16 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, 
int r
     qemu_sglist_add(&s->sg, io->addr, io->len);
     io->addr += io->len;
     io->len = 0;
-
-    MACIO_DPRINTF("sector_num=%d size=%d, cmd_cmd=%d\n",
-                  (s->lba << 2) + (s->io_buffer_index >> 9),
+    if (s->lba >= 0)
+        sector_num = (s->lba << 2) + (s->io_buffer_index >> 9);
+    else
+        sector_num = s->lba;
+    MACIO_DPRINTF("sector_num=%ld size=%d, cmd_cmd=%d\n",
+                  sector_num,
                   s->packet_transfer_size, s->dma_cmd);

     m->aiocb = dma_bdrv_read(s->bs, &s->sg,
-                             (int64_t)(s->lba << 2) + (s->io_buffer_index >> 9)
+                             sector_num,
                              pmac_ide_atapi_transfer_cb, io);
     return;

This still produces an error in MorphOS as before:

DBDMA: writel 0x0000000000000d0c <= 0x00e5ac80
DBDMA: channel 0x1a reg 0x3
DBDMA: dbdma_cmdptr_load 0x00e5ac80
ATAPI limit=0x8000 packet: 43 00 00 00 00 00 00 03 24 00 00 00

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 0x7f7dddff5ae0
    req_count 0x0324
    command 0x3000
    phy_addr 0x00e7dddc
    cmd_dep 0x00000000
    res_count 0x0000
    xfer_status 0x0000
DBDMA: start_input
DBDMA: addr 0xe7dddc key 0x0
pmac_ide_transfer(ATAPI) lba=ffffffff, buffer_index=0, len=324
io_buffer_size = 0
io->len = 0x324
sector_num=-1 size=20, cmd_cmd=0
atapi_cmd_error: sense=0x5 asc=0x21
done DMA
DBDMA: dbdma_end

Interestingly a similar read works for Darwin which does this:

ATAPI limit=0x0 packet: bb 00 ff ff 00 00 00 00 00 00 00 00
ATAPI limit=0xfffe packet: 43 02 00 00 00 00 00 ff fe 80 00 00
reply: tx_size=48 elem_tx_size=0 index=0
byte_count_limit=65534
status=0x58
reply: tx_size=0 elem_tx_size=0 index=48
status=0x50
ATAPI limit=0x30 packet: 43 02 00 00 00 00 00 00 30 80 00 00
reply: tx_size=48 elem_tx_size=0 index=0
byte_count_limit=48
status=0x58
reply: tx_size=0 elem_tx_size=0 index=48
status=0x50
DBDMA: readl 0x0000000000000d04 => 0x00000000
DBDMA: channel 0x1a reg 0x1
DBDMA: writel 0x0000000000000d08 <= 0x00000000
DBDMA: channel 0x1a reg 0x2
DBDMA: writel 0x0000000000000d0c <= 0x011f8010
DBDMA: channel 0x1a reg 0x3
DBDMA: dbdma_cmdptr_load 0x011f8010
DBDMA: writel 0x0000000000000d00 <= 0x90009000
DBDMA: channel 0x1a reg 0x0
DBDMA:     status 0x00009400
DBDMA: DBDMA_run_bh
DBDMA: channel_run
dbdma_cmd 0x7ffa05173c60
    req_count 0x0930
    command 0x2000
    phy_addr 0x017cc000
    cmd_dep 0x00000000
    res_count 0x0000
    xfer_status 0x0000
DBDMA: start_input
DBDMA: addr 0x17cc000 key 0x0
pmac_ide_transfer(ATAPI) lba=ffffffff, buffer_index=30, len=930
waiting for data (0x3 - 0x930 - 50)
ATAPI limit=0x930 packet: be 00 00 00 00 00 00 00 01 f8 00 00
read dma: LBA=0 nb_sectors=1

DBDMA: DBDMA_run_bh
DBDMA: channel_run
dbdma_cmd 0x7ffa05173c60
    req_count 0x0930
    command 0x2000
    phy_addr 0x017cc000
    cmd_dep 0x00000000
    res_count 0x0000
    xfer_status 0x0000
DBDMA: start_input
DBDMA: addr 0x17cc000 key 0x0
pmac_ide_transfer(ATAPI) lba=0, buffer_index=0, len=930
io_buffer_size = 0
io->len = 0x930
sector_num=0 size=2352, cmd_cmd=0
io_buffer_size = 0x930
end of transfer
end of DMA
done DMA
DBDMA: dbdma_end

If these make sense to anyone and can tell what could be the problem 
please share your thoughts.

Regards,
BALATON Zoltan