On 05/06/2018 01:27, John Snow wrote: > > > On 06/04/2018 11:50 AM, Paolo Bonzini wrote: >> On 02/06/2018 03:22, John Snow wrote: >>> - Status: Should be the status register after receiving the H2D Register >>> update FIS, but prior to the data transfer, I think. "New value of the >>> Status register of the Command Block for initiation of host data >>> transfer." >>> I think this is being set correctly after this patch. >>> >>> - Error: "Contains the new value of the Error register of the Command >>> Block at the conclusion of all subsequent Data to Device frames." >>> >>> This is why we were sending out post-hoc PIO Setup FIS frames before, >>> how would I know what the error register *will* be...? What? >> >> You don't, I guess. Zero? >> > > Yeah, I don't mean to hold it up based on other arcane stuff, I was just > briefly hoping that maybe you actually understood it so I could fix it > once and for all... > >>> - LBA: Presumably unimportant for the purposes of receiving a command >>> PACKET, as we won't be writing it to disk, but a buffer. The values >>> can be reported dutifully. >>> >>> - Device: Just report the register value dutifully. >>> >>> - Count: Likely just relays 0, as the H2D REG FIS should have updated >>> that to zero as part of the PACKET command, as per ATA8 ACS3 7.21.3. >>> In any case, just report the register value dutifully. >>> >>> - E_Status: "Contains the new value of the Status register of the >>> Command Block at the conclusion of the subsequent Data FIS." >>> >>> Again, how would I know...? >>> >>> - Transfer Count: Should be 12, as per what we specified in 0xA1 >>> IDENTIFY PACKET DEVICE, see core.c line 234. Your patch gets this >>> correct, as we'll actually report the PIO FIS for the packet itself. >>> >>> >>> What this patch does do, though, is change the context of the Status, >>> Error and E_Status registers to something different. Everything else >>> should be the same, but I'd feel better about taking this patch if I >>> understood what exactly this FIS packet was supposed to convey, but I don't. >> >> At least Status and Transfer Count are correct after this patch. :/ >> >> Paolo >> > > How about ... > > https://github.com/jnsnow/qemu/commit/0657390a2639b7cb533ca8b514c49c0edd3f4483
Yes, it's sane. Paolo
