| Message ID | 20250415073013.414987-6-cassel@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | Successful NCQ commands sense_valid cleanups/fixes | expand |
On 4/15/25 4:30 PM, Niklas Cassel wrote: > When filling the taskfile result for a successful NCQ command, we use > the SDB FIS from the FIS Receive Area, see e.g. ahci_qc_ncq_fill_rtf(). > > However, the SDB FIS only has fields STATUS and ERROR. > > For a successful NCQ command that has sense data, we will have a > successful sense data descriptor, in the Sense Data for Successful NCQ > Commands log. > > Since we have access to additional taskfile result fields, fill in these > additional fields in qc->result_tf. > > This matches how for failing/aborted NCQ commands, we will use e.g. > ahci_qc_fill_rtf() to fill in some fields, but then for the command that > actually caused the NCQ error, we will use ata_eh_read_log_10h(), which > provides additional fields, saving additional fields/overriding the > qc->result_tf that was fetched using ahci_qc_fill_rtf(). > > Fixes: 18bd7718b5c4 ("scsi: ata: libata: Handle completion of CDL commands using policy 0xD") > Signed-off-by: Niklas Cassel <cassel@kernel.org> > Reviewed-by: Igor Pylypiv <ipylypiv@google.com> > Reviewed-by: Hannes Reinecke <hare@suse.de> Applied to for-6.15-fixes. Thanks !
diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c index ba300cc0a3a3..2e4463d3a356 100644 --- a/drivers/ata/libata-sata.c +++ b/drivers/ata/libata-sata.c @@ -1510,6 +1510,8 @@ int ata_eh_get_ncq_success_sense(struct ata_link *link) unsigned int err_mask, tag; u8 *sense, sk = 0, asc = 0, ascq = 0; u64 sense_valid, val; + u16 extended_sense; + bool aux_icc_valid; int ret = 0; err_mask = ata_read_log_page(dev, ATA_LOG_SENSE_NCQ, 0, buf, 2); @@ -1529,6 +1531,8 @@ int ata_eh_get_ncq_success_sense(struct ata_link *link) sense_valid = (u64)buf[8] | ((u64)buf[9] << 8) | ((u64)buf[10] << 16) | ((u64)buf[11] << 24); + extended_sense = get_unaligned_le16(&buf[14]); + aux_icc_valid = extended_sense & BIT(15); ata_qc_for_each_raw(ap, qc, tag) { if (!(qc->flags & ATA_QCFLAG_EH) || @@ -1556,6 +1560,17 @@ int ata_eh_get_ncq_success_sense(struct ata_link *link) continue; } + qc->result_tf.nsect = sense[6]; + qc->result_tf.hob_nsect = sense[7]; + qc->result_tf.lbal = sense[8]; + qc->result_tf.lbam = sense[9]; + qc->result_tf.lbah = sense[10]; + qc->result_tf.hob_lbal = sense[11]; + qc->result_tf.hob_lbam = sense[12]; + qc->result_tf.hob_lbah = sense[13]; + if (aux_icc_valid) + qc->result_tf.auxiliary = get_unaligned_le32(&sense[16]); + /* Set sense without also setting scsicmd->result */ scsi_build_sense_buffer(dev->flags & ATA_DFLAG_D_SENSE, qc->scsicmd->sense_buffer, sk,
When filling the taskfile result for a successful NCQ command, we use the SDB FIS from the FIS Receive Area, see e.g. ahci_qc_ncq_fill_rtf(). However, the SDB FIS only has fields STATUS and ERROR. For a successful NCQ command that has sense data, we will have a successful sense data descriptor, in the Sense Data for Successful NCQ Commands log. Since we have access to additional taskfile result fields, fill in these additional fields in qc->result_tf. This matches how for failing/aborted NCQ commands, we will use e.g. ahci_qc_fill_rtf() to fill in some fields, but then for the command that actually caused the NCQ error, we will use ata_eh_read_log_10h(), which provides additional fields, saving additional fields/overriding the qc->result_tf that was fetched using ahci_qc_fill_rtf(). Fixes: 18bd7718b5c4 ("scsi: ata: libata: Handle completion of CDL commands using policy 0xD") Signed-off-by: Niklas Cassel <cassel@kernel.org> Reviewed-by: Igor Pylypiv <ipylypiv@google.com> Reviewed-by: Hannes Reinecke <hare@suse.de> --- drivers/ata/libata-sata.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)