Re: [Qemu-devel] SATA CD-ROM booting failure with latest OVMF

[John Snow <jsnow@redhat.com>]

On 07/03/2015 10:15 AM, Hannes Reinecke wrote:
> Hi all,
> 
> I've updated OVMF to use the SATA controller, so that I could
> finally use the Q35 machine specification with OVMF.
> 
> However, upon doing so I found that the (SATA) CD-ROM wouldn't work
> anymore; directories on the CD-ROM can be read, but the file listing
> contained only garbage.
> 
> After some _very_ tedious debugging I've found this to be an issue
> with the ahci driver in qemu; ATAPI PIO transfers seem to be
> slightly buggy. Enabling 'atapi_dma' transfers in atapi driver (even
> for PIO mode) makes everything work again.
> 
> I still fail to see _why_ this is be the case; the logfiles
> in both cases are pretty identical. Originally I thought that
> somehow the big qemu lock plays a role here, but OVMF accesses
> the ahci HBA only ever _after_ all transfers are complete.
> And again, access patterns are identical in both cases.
> 
> So if anyone has another idea here ...
> 
> Cheers,
> 
> Hannes
> 

I wrote some quick and very hacky CDROM tests to help suss this out, and
the problem is that PIO transfers that go through the ATAPI layer don't
increment the sglist offset variable that AHCI uses. (s->io_buffer_offset.)

Instead of trying to increment it everywhere (like I was trying to do
previously) I figured I could consolidate it back in ahci_commit_buf.

Pro: All DMA, including DMA masquerading as PIO, now works.
Cons: It only works for AHCI, but nobody else apparently uses or cares
about io_buffer_offset.

If I make all of the PIO hooks in AHCI call back to to core.c's
dma_commit_buf I can probably route all DMA (and fake PIO style DMA) for
all HBAs through dma_commit_buf and roll the pointer forward there.

Anyway, Here's the branch with some fixes in it:

https://github.com/jnsnow/qemu/commits/ahci-sig-dev

I'll try to polish up the tests and the core fix before Friday so that
it's in a reviewable state and propose it for inclusion for 2.4 RC1.


Thanks for the report,
--js


PS: The signature fix for OVMF you wrote is now in master, too. Anything
else? :D