Re: [Qemu-devel] [PATCH 1/6] ahci: Correct PIO/D2H FIS responses

[John Snow <jsnow@redhat.com>]

On 10/27/2014 05:32 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
>> Currently, the D2H FIS packets AHCI generates simply parrot back
>> the LBA that the guest sent to us in the cmd_fis. However, some
>> commands (like READ NATIVE MAX) modify the LBA registers as a
>> return value, through which the AHCI D2H FIS is the only response
>> mechanism. Thus, the D2H response should use the current register
>> values, not the initial ones.
>>
>> This patch adjusts the LBA and drive select register responses for
>> PIO Setup and D2H FIS response packets.
>>
>> Additionally, the PIO and D2H FIS responses copy too many bytes
>> from the command FIS that it is being generated from. Specifically,
>> byte 11 which is the Features(15:8) field for Register Host to
>> Device FIS packets, is instead reserved for the PIO Setup FIS and
>> should always be 0.
>
> Ignorant q: is this based on observation or some specification?  If
> specification: where can I find a copy?
>

Not ignorant. I do all kinds of crazy things. It's based off an 
interpretation of the specification and an observation.

Specification: The SATA specification defines the FIS responses to 
commands via the Register Device to Host FIS. See Sata 3.2 section 
10.5.6 "Register Device to Host FIS"

The fields of this packet are defined as things like:
"LBA(7:0) - Contains the *new value* of the LBA low register of the 
Shadow Register Block." (emphasis mine.) Many other registers are 
defined similarly. All six LBA registers, Device, Error, Count, and so on.

Implying that instead of returning back what the Host-To-Device FIS set, 
we are returning what it is currently (the "new value" after the command 
was run.)

The observation: READ_NATIVE_MAX_ADDRESS is a non-data command, so it 
performs no PIO or DMA actions on the IDE device. It will only return, 
in the case of the AHCI controller, an FIS packet. If the FIS packet 
returns (for example) the LBA registers that the user sent, there is no 
mechanism by which the host can actually /receive/ the native max address.

Therefore, I interpret the specification to imply that the response FIS 
after successful completion of a command should return the new, current 
values of the IDE registers, and not simply parrot back what the user 
sent. It's a synchronization mechanism between the device and the host.

For further specification... READ_NATIVE_MAX_ADDRESS is deprecated in 
AC3, but we can look at ATA8-ACS revision 4a, which is pretty clear 
about the line response to this command:

Section 7.33 READ NATIVE MAX ADDRESS - F8h, Non-data, subsection 7.33.4 
"Normal Outputs":
"See table 96. LBA contains the Native Max Address. Bits 47:28 of LBA 
shall be cleared to zero."

Table 96 - "HPA Normal Output" Specifies the line output for this 
command. Error, Count, LBA, Device, Status. These line outputs are for 
sure the information that is bundled up into the D2H FIS.

On this basis, then, is the reasoning behind this patch.