public inbox for linux-ide@vger.kernel.org
 help / color / mirror / Atom feed
From: Lorenz Brun <lorenz@brun.one>
To: Damien Le Moal <dlemoal@kernel.org>
Cc: linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] ata: libata-scsi: fix bogus SCSI sense after abort
Date: Fri, 23 Jun 2023 20:19:07 +0200	[thread overview]
Message-ID: <20230623181908.2032764-1-lorenz@brun.one> (raw)

Since commit 058e55e120ca which fixed that commands without valid
error/status codes did not result in any sense error, the returned sense
errors were completely bogus as ata_to_sense_error did not have valid
inputs in the first place.

For example the following ATA error

exception Emask 0x10 SAct 0x20c000 SErr 0x280100 action 0x6 frozen
irq_stat 0x08000000, interface fatal error
SError: { UnrecovData 10B8B BadCRC }
failed command: READ FPDMA QUEUED
cmd 60/e0:70:20:0a:00/00:00:00:00:00/40 tag 14 ncq dma 114688 in
res 40/00:ac:20:5e:50/00:00:5d:01:00/40 Emask 0x10 (ATA bus error)
status: { DRDY }

got turned into the following nonsensical SCSI error

FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=0s
Sense Key : Illegal Request [current]
Add. Sense: Unaligned write command
CDB: Read(16) 88 00 00 00 00 00 00 00 0a 20 00 00 00 e0 00 00

This has nothing to do with an unaligned write command, but is due to an
ATA EH-triggered abort. But ata_to_sense_error only knows about
status and error, both of which aren't even valid here as the command
has been aborted.

Add an additional section to ata_gen_ata_sense which handles
errors not coming from the device first, before calling into
ata_to_sense_error.

According to the SAT-5 spec a reset should cause a Unit Attention event,
which the SCSI subsystem should handle to retry its commands but I
am not sure how much of that infra is present in Linux's SCSI layer, so
this is a simpler solution.

Signed-off-by: Lorenz Brun <lorenz@brun.one>
---
 drivers/ata/libata-scsi.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 551077cea4e4..61c6a4e8123a 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -13,6 +13,7 @@
  *  - http://www.t13.org/
  */
 
+#include "scsi/scsi_proto.h"
 #include <linux/compat.h>
 #include <linux/slab.h>
 #include <linux/kernel.h>
@@ -1013,6 +1014,21 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
 		ata_scsi_set_sense(dev, cmd, NOT_READY, 0x04, 0x21);
 		return;
 	}
+	if (qc->err_mask & (AC_ERR_HSM | AC_ERR_ATA_BUS | AC_ERR_HOST_BUS |
+		AC_ERR_SYSTEM | AC_ERR_OTHER)) {
+		/* Command aborted because of some issue with the ATA subsystem
+		 * Should technically cause unit attention, but this is better
+		 * than nothing, which results in nonsensical errors.
+		 * POWER ON, RESET, OR BUS DEVICE RESET OCCURRED
+		 */
+		ata_scsi_set_sense(dev, cmd, ABORTED_COMMAND, 0x29, 0x00);
+		return;
+	}
+	if (qc->err_mask & AC_ERR_TIMEOUT) {
+		/* COMMAND TIMEOUT DURING PROCESSING */
+		ata_scsi_set_sense(dev, cmd, ABORTED_COMMAND, 0x2e, 0x02);
+		return;
+	}
 	/* Use ata_to_sense_error() to map status register bits
 	 * onto sense key, asc & ascq.
 	 */
-- 
2.40.1


             reply	other threads:[~2023-06-23 18:36 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-23 18:19 Lorenz Brun [this message]
2023-06-26  7:29 ` [PATCH] ata: libata-scsi: fix bogus SCSI sense after abort Damien Le Moal
2023-06-26  7:46   ` Hannes Reinecke
2023-06-26  7:51     ` Damien Le Moal
2023-06-29 19:11       ` Lorenz Brun
2023-07-03  9:09         ` Damien Le Moal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230623181908.2032764-1-lorenz@brun.one \
    --to=lorenz@brun.one \
    --cc=dlemoal@kernel.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox