John Snow <[email protected]> writes: > On 10/27/2014 05:32 AM, Markus Armbruster wrote: >> John Snow <[email protected]> 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.
Makes sense to me, thanks. Since you already got competent review, I won't review further, unless you ask for it.
