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

[BALATON Zoltan]

On Mon, 5 May 2014, Mark Cave-Ayland wrote:
> On 04/05/14 01:05, BALATON Zoltan wrote:
> > Hello,
> >
> > I think I've found a bug in the macio ide emulation that causes MorphOS
> > to fail when trying to mount its boot cd. But I'd need some help on how
> > to fix it because I could not completely understand the workings of
> > dbdma and how is it talking to the atapi emulation. Here are some debug
> > logs from QEMU to try to demonstrate the problem:
> >
> > DBDMA: writel 0x0000000000000d0c <= 0x00e5a760
> > DBDMA: channel 0x1a reg 0x3
> > DBDMA: dbdma_cmdptr_load 0x00e5a760
> > 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 0x7f7919f19c70
> >      req_count 0x0324
> >      command 0x3000
> >      phy_addr 0x00e7f6a4
> >      cmd_dep 0x00000000
> >      res_count 0x0000
> >      xfer_status 0x0000
> > DBDMA: start_input
> > DBDMA: addr 0xe7f6a4 key 0x0
> > pmac_ide_transfer(ATAPI) lba=ffffffff, buffer_index=0, len=324
> > io_buffer_size = 0
> > remainder: 0 io->len: 804 size: 20
> > precopying unaligned 292 bytes to 0xe7f8a4
> > io->len = 0x200
> > set remainder to: 0
> > sector_num=-4 size=20, cmd_cmd=0
> > atapi_cmd_error: sense=0x5 asc=0x21
> > done DMA
> > DBDMA: dbdma_end
> > DBDMA: conditional_wait
> > DBDMA: dbdma_cmdptr_save 0x00e5a760
> > DBDMA: xfer_status 0x00008400 res_count 0x0000
> >
> > What is happening is that a Read TOC command is sent and I think it
> > tries to get the reply with a read command with lba set to -1 which
> > according to comments in atapi.c should return results without reading a
> > sector but this is erroneously converted to sector -4 in
> > macio.c:pmac_ide_atapi_transfer_cb and causes it to fail. My question to
> > those who undestand this better if this analysis is correct and how
> > could it be fixed?
>
> I had a poke around that code last year when I was experiencing disk I/O 
> problems with Alex's original macio patches so I may be able to shed some 
> light on this.
>
> DBDMA is like a separate processor where you give it the address of a program 
> to run, toggle an execute bit and off it goes in the background. In the 
> output above, the current command in the program is "start_input" which reads 
> in a block of data from the disk subsystem and writes it to memory. The 
> "start_input" command calls the macio pmac_ide_transfer() function which sets 
> up the start of the transfer and then invokes pmac_ide_transfer_cb() for 
> standard IDE devices and pmac_ide_atapi_transfer_cb() for ATAPI devices.
>
> Both the pmac_ide_*_transfer_cb() functions work by building up a 
> scatter-gather list (list of disk blocks to memory buffer mappings) from the 
> DBDMA_io parameters obtained from "start_input" and then use the asynchronous 
> dma_bdrv_read() function to get the block layer to do the work in a separate 
> thread and invoke the callback when complete.
>
> Now this is the part where it gets tricky; the block layer works in 512 byte 
> sectors while DBDMA works in bytes, and would you believe it that there OSs 
> out there (like PPC OS X) that don't request block-aligned DMA transfers? 
> Hence you see reference in the pmac_ide_*_transfer_cb() functions to 
> "unaligned" accesses which are those where the IDE interface requests 0x200 
> bytes, but the DBDMA program is setup to perform the DMA with arbitrary 
> values such as two consecutive "start_input" commands with 0x198 and 0x2 
> bytes(!).
>
> The majority of the complexity of this code is to do with calculating non 
> block-aligned DMA requests with regard to IDE transfers and making sure that 
> unaligned requests (i.e. those that are not a multiple of the 512 byte block 
> size) are correctly copied to RAM between transfers.

Thanks for the explanation but I still don't see what should be the fix 
for the problem I have. Where is this translation coming from:

sector_num = (s->lba << 2) + (s->io_buffer_index >> 9)

and why is it not correct for lba=-1? Where is lba set to -1 and where 
should the lba=-1 case handled?

> Also if you want to test changes to this code, I highly recommend sourcing a 
> copy of the old darwinppc-602.iso file as both that and PPC OS X generate 
> huge sets of unaligned accesses in this way.

I've downloaded this and the latest 802 version of these images and tried 
it with my OpenBIOS modifications. They seem to get to the same point as 
with the stock QEMU version. Neither of them give me a login prompt though 
with or without modifications to OpenBIOS. Are these supposed to work or 
working as much as before is enough of a test?

> P.S. I'm not sure but I think recently Kevin posted patches to qemu-devel 
> which may remove the requirement for block-aligned accesses, so if these 
> patches have been applied then it might be possible to simplify this code 
> quite significantly.

I could not identify such patches in the commit log but I'm not sure what 
to look for.

Regards,
BALATON Zoltan