[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 1/6] ahci: move PIO Setup FIS befor
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 1/6] ahci: move PIO Setup FIS before transfer, fix it for ATAPI commands |
Date: |
Wed, 6 Jun 2018 15:44:21 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 |
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