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. Thanks, Igor > > I will send a v3 that does populate COUNT (7:0) and COUNT (15:8), > since it is only so few fields that we do have, we might as well > populate them properly. > > > Kind regards, > Niklas