On Thu, Apr 10, 2025 at 10:00:29AM -0700, Igor Pylypiv wrote: > On Thu, Apr 10, 2025 at 04:04:29PM +0200, Niklas Cassel wrote: > > On Thu, Apr 10, 2025 at 03:02:33PM +0200, Niklas Cassel wrote: > > > > > > I guess now when we do have access to the information, the most consistent > > > thing would be to fill all field we can in qc->result_tf... but, to do this > > > for every IO might slow things down. > > > > > > So is there perhaps some logic to only filling LBA (in addition to STATUS > > > and ERROR, which are filled for all NCQ commands), since that is the only > > > field that can change, as per the specs. > > > > Looking at this more closely: > > https://github.com/torvalds/linux/blob/v6.15-rc1/include/linux/libata.h#L574-L577 > > > > FEATURE is a union with ERROR, so we cannot save it in qc->result_tf. > > > > COMMAND is a union with STATUS, so we cannot save it in qc->result_tf. > > > > > > The sense data descriptor does not provide AUXILIARY, nor DEVICE, > > so we cannot save these. > > > > Successful Sense Data descriptor provides AUXILIARY field as well: > > ACS-6 (revision 10) > > Table 339 — Successful Sense Data descriptor format > +--------+------+-----------------------------------------------------+ > | Offset | Type | Description | > +--------+------+-----------------------------------------------------+ > | 0 | Byte | SENSE KEY field (see 9.29.3.2) | > | 1 | Byte | ADDITIONAL SENSE CODE field (see 9.29.3.2) | > | 2 | Byte | ADDITIONAL SENSE CODE QUALIFIER field (see 9.29.3.2)| > | 3 | Byte | COMMAND field (see 9.29.3.3) | > | 4 | Byte | FEATURE field (7:0) (see 9.29.3.3) | > | 5 | Byte | FEATURE field (15:8) (see 9.29.3.3) | > | 6 | Byte | COUNT field (7:0) (see 9.29.3.3) | > | 7 | Byte | COUNT field (15:8) (see 9.29.3.3) | > | 8 | Byte | LBA field (7:0) (see 9.29.3.4) | > | 9 | Byte | LBA field (15:8) (see 9.29.3.4) | > | 10 | Byte | LBA field (23:16) (see 9.29.3.4) | > | 11 | Byte | LBA field (31:24) (see 9.29.3.4) | > | 12 | Byte | LBA field (39:32) (see 9.29.3.4) | > | 13 | Byte | LBA field (47:40) (see 9.29.3.4) | > | 14 | Byte | INFORMATION field (7:0) (see 9.29.3.5) | > | 15 | Byte | INFORMATION field (15:8) (see 9.29.3.5) | > | 16 | Byte | AUXILIARY field (7:0) (see 9.29.3.6) | > | 17 | Byte | AUXILIARY field (15:8) (see 9.29.3.6) | > | 18 | Byte | AUXILIARY field (23:16) (see 9.29.3.6) | > | 19 | Byte | AUXILIARY field (31:24) (see 9.29.3.6) | > | 20 | Byte | ICC field (7:0) (see 9.29.3.6) | > | 21..23 | Bytes| Reserved | > +--------+------+-----------------------------------------------------+ > > The data in AUXILIARY and ICC is only valid when the AUXILIARY AND ICC FIELDS > VALID bit is set. Similarly to other fields, AUXILIARY contains a copy of > command inputs. I was working on my laptop, and was not looking at the latest ACS-6 draft. ACS-6 r2 has bytes 16 to 13 marked as Reserved. But indeed, in the latest draft we do also have the AUX field, thank you for noticing! Kind regards, Niklas
