This patch series is fixing a few ATA PASS-THROUGH issues:
1. Not reporting the sense data when ATA error happens and CK_COND is 0.
2. Generating "fake" sense data based on ATA status/error registers even
though a valid sense data was already fetched from a disk.
3. Fixed format sense data was using incorrect field offsets for ATA
PASS-THROUGH commands.
Igor Pylypiv (4):
ata: libata: Remove redundant sense_buffer memsets
ata: libata-scsi: Generate ATA PT sense data when ATA ERR/DF are set
ata: libata-scsi: Report valid sense data for ATA PT if present
ata: libata-scsi: Fix offsets for the fixed format sense data
drivers/ata/libata-eh.c | 2 --
drivers/ata/libata-scsi.c | 53 ++++++++++++++++++++-------------------
2 files changed, 27 insertions(+), 28 deletions(-)
--
2.45.2.627.g7a2c4fd464-goog
Correct the ATA PASS-THROUGH fixed format sense data offsets to conform
to SPC-6 and SAT-5 specifications. Additionally, set the VALID bit to
indicate that the INFORMATION field contains valid information.
INFORMATION
===========
SAT-5 Table 212 — "Fixed format sense data INFORMATION field for the ATA
PASS-THROUGH commands" defines the following format:
+------+------------+
| Byte | Field |
+------+------------+
| 0 | ERROR |
| 1 | STATUS |
| 2 | DEVICE |
| 3 | COUNT(7:0) |
+------+------------+
SPC-6 Table 48 - "Fixed format sense data" specifies that the INFORMATION
field starts at byte 3 in sense buffer resulting in the following offsets
for the ATA PASS-THROUGH commands:
+------------+-------------------------+
| Field | Offset in sense buffer |
+------------+-------------------------+
| ERROR | 3 |
| STATUS | 4 |
| DEVICE | 5 |
| COUNT(7:0) | 6 |
+------------+-------------------------+
COMMAND-SPECIFIC INFORMATION
============================
SAT-5 Table 213 - "Fixed format sense data COMMAND-SPECIFIC INFORMATION
field for ATA PASS-THROUGH" defines the following format:
+------+-------------------+
| Byte | Field |
+------+-------------------+
| 0 | FLAGS | LOG INDEX |
| 1 | LBA (7:0) |
| 2 | LBA (15:8) |
| 3 | LBA (23:16) |
+------+-------------------+
SPC-6 Table 48 - "Fixed format sense data" specifies that
the COMMAND-SPECIFIC-INFORMATION field starts at byte 8
in sense buffer resulting in the following offsets for
the ATA PASS-THROUGH commands:
Offsets of these fields in the fixed sense format are as follows:
+-------------------+-------------------------+
| Field | Offset in sense buffer |
+-------------------+-------------------------+
| FLAGS | LOG INDEX | 8 |
| LBA (7:0) | 9 |
| LBA (15:8) | 10 |
| LBA (23:16) | 11 |
+-------------------+-------------------------+
Reported-by: Akshat Jain <[email protected]>
Fixes: 11093cb1ef56 ("libata-scsi: generate correct ATA pass-through sense")
Signed-off-by: Igor Pylypiv <[email protected]>
---
drivers/ata/libata-scsi.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 4bfe47e7d266..8588512f5975 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -855,7 +855,6 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
struct scsi_cmnd *cmd = qc->scsicmd;
struct ata_taskfile *tf = &qc->result_tf;
unsigned char *sb = cmd->sense_buffer;
- unsigned char *desc = sb + 8;
u8 sense_key, asc, ascq;
if (qc->flags & ATA_QCFLAG_SENSE_VALID) {
@@ -880,8 +879,9 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
scsi_build_sense(cmd, 1, RECOVERED_ERROR, 0, 0x1D);
}
- if ((cmd->sense_buffer[0] & 0x7f) >= 0x72) {
+ if ((sb[0] & 0x7f) >= 0x72) {
u8 len;
+ unsigned char *desc;
/* descriptor format */
len = sb[7];
@@ -919,21 +919,21 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
}
} else {
/* Fixed sense format */
- desc[0] = tf->error;
- desc[1] = tf->status;
- desc[2] = tf->device;
- desc[3] = tf->nsect;
- desc[7] = 0;
+ sb[0] |= 0x80;
+ sb[3] = tf->error;
+ sb[4] = tf->status;
+ sb[5] = tf->device;
+ sb[6] = tf->nsect;
if (tf->flags & ATA_TFLAG_LBA48) {
- desc[8] |= 0x80;
+ sb[8] |= 0x80;
if (tf->hob_nsect)
- desc[8] |= 0x40;
+ sb[8] |= 0x40;
if (tf->hob_lbal || tf->hob_lbam || tf->hob_lbah)
- desc[8] |= 0x20;
+ sb[8] |= 0x20;
}
- desc[9] = tf->lbal;
- desc[10] = tf->lbam;
- desc[11] = tf->lbah;
+ sb[9] = tf->lbal;
+ sb[10] = tf->lbam;
+ sb[11] = tf->lbah;
}
}
--
2.45.2.627.g7a2c4fd464-goog
SCSI/ATA Translation-5 (SAT-5) Table 209 — "ATA command results"
specifies that SATL shall generate sense data for ATA PASS-THROUGH
commands when either CK_COND is set or when ATA_ERR or ATA_DF status
bits are set.
ata_eh_analyze_tf() sets AC_ERR_DEV bit in qc->err_mask when ATA_ERR
or ATA_DF bits are set. It looks like qc->err_mask can be used as
an error indicator but ata_eh_link_autopsy() clears AC_ERR_DEV bit
when ATA_QCFLAG_SENSE_VALID is set. This effectively clears the error
indication if no other bits were set in qc->err_mask.
ata_scsi_qc_complete() should not use qc->err_mask for ATA PASS-THROUGH
commands because qc->err_mask can be zero (i.e. "no error") even when
the corresponding command has failed with ATA_ERR/ATA_DF bits set.
Additionally, the presence of valid sense data (ATA_QCFLAG_SENSE_VALID)
should not prevent SATL from generating sense data for ATA PASS-THROUGH.
Signed-off-by: Igor Pylypiv <[email protected]>
---
drivers/ata/libata-scsi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 032cf11d0bcc..79e8103ef3a9 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1632,8 +1632,8 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
!(qc->flags & ATA_QCFLAG_SENSE_VALID);
/* For ATA pass thru (SAT) commands, generate a sense block if
- * user mandated it or if there's an error. Note that if we
- * generate because the user forced us to [CK_COND =1], a check
+ * user mandated it or if ATA_ERR or ATA_DF bits are set. Note that
+ * if we generate because the user forced us to [CK_COND=1], a check
* condition is generated and the ATA register values are returned
* whether the command completed successfully or not. If there
* was no error, we use the following sense data:
@@ -1641,7 +1641,7 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
* asc,ascq = ATA PASS-THROUGH INFORMATION AVAILABLE
*/
if (((cdb[0] == ATA_16) || (cdb[0] == ATA_12)) &&
- ((cdb[2] & 0x20) || need_sense))
+ ((cdb[2] & 0x20) || (qc->result_tf.status & (ATA_ERR | ATA_DF))))
ata_gen_passthru_sense(qc);
else if (need_sense)
ata_gen_ata_sense(qc);
--
2.45.2.627.g7a2c4fd464-goog
Do not generate sense data from ATA status/error registers
if valid sense data is already present.
Signed-off-by: Igor Pylypiv <[email protected]>
---
drivers/ata/libata-scsi.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 79e8103ef3a9..4bfe47e7d266 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -858,12 +858,17 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
unsigned char *desc = sb + 8;
u8 sense_key, asc, ascq;
- /*
- * Use ata_to_sense_error() to map status register bits
- * onto sense key, asc & ascq.
- */
- if (qc->err_mask ||
- tf->status & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ)) {
+ if (qc->flags & ATA_QCFLAG_SENSE_VALID) {
+ /*
+ * Do not generate sense data from ATA status/error
+ * registers if valid sense data is already present.
+ */
+ } else if (qc->err_mask ||
+ tf->status & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ)) {
+ /*
+ * Use ata_to_sense_error() to map status register bits
+ * onto sense key, asc & ascq.
+ */
ata_to_sense_error(qc->ap->print_id, tf->status, tf->error,
&sense_key, &asc, &ascq);
ata_scsi_set_sense(qc->dev, cmd, sense_key, asc, ascq);
--
2.45.2.627.g7a2c4fd464-goog
scsi_queue_rq() memsets sense_buffer before a command is dispatched.
Libata is not memsetting sense_buffer before setting sense data that
was obtained from a disk so there should be no reason to do a memset
for ATA PASS-THROUGH / ATAPI.
Memsetting the sense_buffer in ata_gen_passthru_sense() is erasing valid
sense data that was previously obtained from a disk. A follow-up patch
will modify ata_gen_passthru_sense() to stop generating sense data based
on ATA status register bits if a valid sense data is already present.
Signed-off-by: Igor Pylypiv <[email protected]>
---
drivers/ata/libata-eh.c | 2 --
drivers/ata/libata-scsi.c | 4 ----
2 files changed, 6 deletions(-)
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 214b935c2ced..b5e05efe73f6 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1479,8 +1479,6 @@ unsigned int atapi_eh_request_sense(struct ata_device *dev,
struct ata_port *ap = dev->link->ap;
struct ata_taskfile tf;
- memset(sense_buf, 0, SCSI_SENSE_BUFFERSIZE);
-
/* initialize sense_buf with the error register,
* for the case where they are -not- overwritten
*/
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index cdf29b178ddc..032cf11d0bcc 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -858,8 +858,6 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
unsigned char *desc = sb + 8;
u8 sense_key, asc, ascq;
- memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
-
/*
* Use ata_to_sense_error() to map status register bits
* onto sense key, asc & ascq.
@@ -953,8 +951,6 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
u64 block;
u8 sense_key, asc, ascq;
- memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
-
if (ata_dev_disabled(dev)) {
/* Device disabled after error recovery */
/* LOGICAL UNIT NOT READY, HARD RESET REQUIRED */
--
2.45.2.627.g7a2c4fd464-goog