All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Cassel <niklas.cassel@wdc.com>
To: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Cc: Hannes Reinecke <hare@suse.de>,
	linux-scsi@vger.kernel.org, Niklas Cassel <niklas.cassel@wdc.com>,
	linux-ide@vger.kernel.org
Subject: [PATCH 06/25] ata: libata: allow ata_scsi_set_sense() to not set CHECK_CONDITION
Date: Thu,  8 Dec 2022 11:59:22 +0100	[thread overview]
Message-ID: <20221208105947.2399894-7-niklas.cassel@wdc.com> (raw)
In-Reply-To: <20221208105947.2399894-1-niklas.cassel@wdc.com>

Current ata_scsi_set_sense() calls scsi_build_sense(), which,
in addition to setting the sense data, unconditionally sets the
scsicmd->result to SAM_STAT_CHECK_CONDITION.

For Command Duration Limits policy 0xD:
The device shall complete the command without error (SAM_STAT_GOOD)
with the additional sense code set to DATA CURRENTLY UNAVAILABLE.

It is perfectly fine to have sense data for a command that returned
completion without error.

In order to support for CDL policy 0xD, we have to remove this
assumption that having sense data means that the command failed
(SAM_STAT_CHECK_CONDITION).

Add a new parameter to ata_scsi_set_sense() to allow us to set
sense data without unconditionally setting SAM_STAT_CHECK_CONDITION.
This new parameter will be used in a follow-up patch.

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 drivers/ata/libata-eh.c   |  3 ++-
 drivers/ata/libata-sata.c |  4 ++--
 drivers/ata/libata-scsi.c | 38 ++++++++++++++++++++------------------
 drivers/ata/libata.h      |  4 ++--
 4 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 8cb250930c48..c278366370ab 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1429,7 +1429,8 @@ static void ata_eh_request_sense(struct ata_queued_cmd *qc)
 	/* Ignore err_mask; ATA_ERR might be set */
 	if (tf.status & ATA_SENSE) {
 		if (ata_scsi_sense_is_valid(tf.lbah, tf.lbam, tf.lbal)) {
-			ata_scsi_set_sense(dev, cmd, tf.lbah, tf.lbam, tf.lbal);
+			ata_scsi_set_sense(dev, cmd, true, tf.lbah, tf.lbam,
+					   tf.lbal);
 			qc->flags |= ATA_QCFLAG_SENSE_VALID;
 		}
 	} else {
diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index f3e7396e3191..414d7f8a95bf 100644
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -1471,8 +1471,8 @@ void ata_eh_analyze_ncq_error(struct ata_link *link)
 		asc = (qc->result_tf.auxiliary >> 8) & 0xff;
 		ascq = qc->result_tf.auxiliary & 0xff;
 		if (ata_scsi_sense_is_valid(sense_key, asc, ascq)) {
-			ata_scsi_set_sense(dev, qc->scsicmd, sense_key, asc,
-					   ascq);
+			ata_scsi_set_sense(dev, qc->scsicmd, true, sense_key,
+					   asc, ascq);
 			ata_scsi_set_sense_information(dev, qc->scsicmd,
 						       &qc->result_tf);
 			qc->flags |= ATA_QCFLAG_SENSE_VALID;
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index cbb3a7a50816..0e6684ca0315 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -205,14 +205,16 @@ bool ata_scsi_sense_is_valid(u8 sk, u8 asc, u8 ascq)
 }
 
 void ata_scsi_set_sense(struct ata_device *dev, struct scsi_cmnd *cmd,
-			u8 sk, u8 asc, u8 ascq)
+			bool check_condition, u8 sk, u8 asc, u8 ascq)
 {
 	bool d_sense = (dev->flags & ATA_DFLAG_D_SENSE);
 
 	if (!cmd)
 		return;
 
-	scsi_build_sense(cmd, d_sense, sk, asc, ascq);
+	scsi_build_sense_buffer(d_sense, cmd->sense_buffer, sk, asc, ascq);
+	if (check_condition)
+		set_status_byte(cmd, SAM_STAT_CHECK_CONDITION);
 }
 
 void ata_scsi_set_sense_information(struct ata_device *dev,
@@ -235,7 +237,7 @@ void ata_scsi_set_sense_information(struct ata_device *dev,
 static void ata_scsi_set_invalid_field(struct ata_device *dev,
 				       struct scsi_cmnd *cmd, u16 field, u8 bit)
 {
-	ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x24, 0x0);
+	ata_scsi_set_sense(dev, cmd, true, ILLEGAL_REQUEST, 0x24, 0x0);
 	/* "Invalid field in CDB" */
 	scsi_set_sense_field_pointer(cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE,
 				     field, bit, 1);
@@ -245,7 +247,7 @@ static void ata_scsi_set_invalid_parameter(struct ata_device *dev,
 					   struct scsi_cmnd *cmd, u16 field)
 {
 	/* "Invalid field in parameter list" */
-	ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x26, 0x0);
+	ata_scsi_set_sense(dev, cmd, true, ILLEGAL_REQUEST, 0x26, 0x0);
 	scsi_set_sense_field_pointer(cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE,
 				     field, 0xff, 0);
 }
@@ -914,7 +916,7 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
 	    tf->status & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ)) {
 		ata_to_sense_error(qc->ap->print_id, tf->status, tf->error,
 				   &sense_key, &asc, &ascq, verbose);
-		ata_scsi_set_sense(qc->dev, cmd, sense_key, asc, ascq);
+		ata_scsi_set_sense(qc->dev, cmd, true, sense_key, asc, ascq);
 	} else {
 		/*
 		 * ATA PASS-THROUGH INFORMATION AVAILABLE
@@ -1005,7 +1007,7 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
 	if (ata_dev_disabled(dev)) {
 		/* Device disabled after error recovery */
 		/* LOGICAL UNIT NOT READY, HARD RESET REQUIRED */
-		ata_scsi_set_sense(dev, cmd, NOT_READY, 0x04, 0x21);
+		ata_scsi_set_sense(dev, cmd, true, NOT_READY, 0x04, 0x21);
 		return;
 	}
 	/* Use ata_to_sense_error() to map status register bits
@@ -1015,12 +1017,12 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
 	    tf->status & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ)) {
 		ata_to_sense_error(qc->ap->print_id, tf->status, tf->error,
 				   &sense_key, &asc, &ascq, verbose);
-		ata_scsi_set_sense(dev, cmd, sense_key, asc, ascq);
+		ata_scsi_set_sense(dev, cmd, true, sense_key, asc, ascq);
 	} else {
 		/* Could not decode error */
 		ata_dev_warn(dev, "could not decode error status 0x%x err_mask 0x%x\n",
 			     tf->status, qc->err_mask);
-		ata_scsi_set_sense(dev, cmd, ABORTED_COMMAND, 0, 0);
+		ata_scsi_set_sense(dev, cmd, true, ABORTED_COMMAND, 0, 0);
 		return;
 	}
 
@@ -1496,7 +1498,7 @@ static unsigned int ata_scsi_verify_xlat(struct ata_queued_cmd *qc)
 	return 1;
 
 out_of_range:
-	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x21, 0x0);
+	ata_scsi_set_sense(qc->dev, scmd, true, ILLEGAL_REQUEST, 0x21, 0x0);
 	/* "Logical Block Address out of range" */
 	return 1;
 
@@ -1631,7 +1633,7 @@ static unsigned int ata_scsi_rw_xlat(struct ata_queued_cmd *qc)
 	return 1;
 
 out_of_range:
-	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x21, 0x0);
+	ata_scsi_set_sense(qc->dev, scmd, true, ILLEGAL_REQUEST, 0x21, 0x0);
 	/* "Logical Block Address out of range" */
 	return 1;
 
@@ -2380,7 +2382,7 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
 	return 1;
 
 saving_not_supp:
-	ata_scsi_set_sense(dev, args->cmd, ILLEGAL_REQUEST, 0x39, 0x0);
+	ata_scsi_set_sense(dev, args->cmd, true, ILLEGAL_REQUEST, 0x39, 0x0);
 	 /* "Saving parameters not supported" */
 	return 1;
 }
@@ -3241,11 +3243,11 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
 	return 1;
 invalid_param_len:
 	/* "Parameter list length error" */
-	ata_scsi_set_sense(dev, scmd, ILLEGAL_REQUEST, 0x1a, 0x0);
+	ata_scsi_set_sense(dev, scmd, true, ILLEGAL_REQUEST, 0x1a, 0x0);
 	return 1;
 invalid_opcode:
 	/* "Invalid command operation code" */
-	ata_scsi_set_sense(dev, scmd, ILLEGAL_REQUEST, 0x20, 0x0);
+	ata_scsi_set_sense(dev, scmd, true, ILLEGAL_REQUEST, 0x20, 0x0);
 	return 1;
 }
 
@@ -3471,7 +3473,7 @@ static unsigned int ata_scsi_zbc_in_xlat(struct ata_queued_cmd *qc)
 
 invalid_param_len:
 	/* "Parameter list length error" */
-	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x1a, 0x0);
+	ata_scsi_set_sense(qc->dev, scmd, true, ILLEGAL_REQUEST, 0x1a, 0x0);
 	return 1;
 }
 
@@ -3549,7 +3551,7 @@ static unsigned int ata_scsi_zbc_out_xlat(struct ata_queued_cmd *qc)
 	return 1;
 invalid_param_len:
 	/* "Parameter list length error" */
-	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x1a, 0x0);
+	ata_scsi_set_sense(qc->dev, scmd, true, ILLEGAL_REQUEST, 0x1a, 0x0);
 	return 1;
 }
 
@@ -3810,7 +3812,7 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc)
 
  invalid_param_len:
 	/* "Parameter list length error" */
-	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x1a, 0x0);
+	ata_scsi_set_sense(qc->dev, scmd, true, ILLEGAL_REQUEST, 0x1a, 0x0);
 	return 1;
 
  skip:
@@ -4166,7 +4168,7 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 		break;
 
 	case REQUEST_SENSE:
-		ata_scsi_set_sense(dev, cmd, 0, 0, 0);
+		ata_scsi_set_sense(dev, cmd, true, 0, 0, 0);
 		break;
 
 	/* if we reach this, then writeback caching is disabled,
@@ -4198,7 +4200,7 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 
 	/* all other commands */
 	default:
-		ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x20, 0x0);
+		ata_scsi_set_sense(dev, cmd, true, ILLEGAL_REQUEST, 0x20, 0x0);
 		/* "Invalid command operation code" */
 		break;
 	}
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 2cd6124a01e8..5481d29bb273 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -115,8 +115,8 @@ extern int ata_scsi_add_hosts(struct ata_host *host,
 extern void ata_scsi_scan_host(struct ata_port *ap, int sync);
 extern int ata_scsi_offline_dev(struct ata_device *dev);
 extern bool ata_scsi_sense_is_valid(u8 sk, u8 asc, u8 ascq);
-extern void ata_scsi_set_sense(struct ata_device *dev,
-			       struct scsi_cmnd *cmd, u8 sk, u8 asc, u8 ascq);
+extern void ata_scsi_set_sense(struct ata_device *dev, struct scsi_cmnd *cmd,
+			       bool check_condition, u8 sk, u8 asc, u8 ascq);
 extern void ata_scsi_set_sense_information(struct ata_device *dev,
 					   struct scsi_cmnd *cmd,
 					   const struct ata_taskfile *tf);
-- 
2.38.1


  parent reply	other threads:[~2022-12-08 11:03 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-08 10:59 [PATCH 00/25] Add Command Duration Limits support Niklas Cassel
2022-12-08 10:59 ` [PATCH 01/25] ata: scsi: rename flag ATA_QCFLAG_FAILED to ATA_QCFLAG_EH Niklas Cassel
2022-12-21 11:47   ` John Garry
2022-12-08 10:59 ` [PATCH 02/25] ata: libata: move NCQ related ATA_DFLAGs Niklas Cassel
2022-12-08 10:59 ` [PATCH 03/25] ata: libata: simplify qc_fill_rtf port operation interface Niklas Cassel
2022-12-21 11:48   ` John Garry
2022-12-08 10:59 ` [PATCH 04/25] ata: libata: fix broken NCQ command status handling Niklas Cassel
2022-12-08 10:59 ` [PATCH 05/25] ata: libata: respect successfully completed commands during errors Niklas Cassel
2022-12-08 10:59 ` Niklas Cassel [this message]
2022-12-08 10:59 ` [PATCH 07/25] ata: libata: allow ata_eh_request_sense() to not set CHECK_CONDITION Niklas Cassel
2022-12-08 10:59 ` [PATCH 08/25] ata: libata-scsi: do not overwrite SCSI ML and status bytes Niklas Cassel
2022-12-08 10:59 ` [PATCH 09/25] ata: libata-scsi: improve ata_scsiop_maint_in() Niklas Cassel
2022-12-08 10:59 ` [PATCH 10/25] scsi: core: allow libata to complete successful commands via EH Niklas Cassel
2022-12-08 10:59 ` [PATCH 11/25] scsi: move get_scsi_ml_byte() to scsi_priv.h Niklas Cassel
2022-12-08 23:58   ` Mike Christie
2022-12-28 20:41     ` Niklas Cassel
2022-12-29 18:55       ` Mike Christie
2022-12-29 20:19         ` Niklas Cassel
2022-12-08 10:59 ` [PATCH 12/25] scsi: support retrieving sub-pages of mode pages Niklas Cassel
2022-12-08 10:59 ` [PATCH 13/25] scsi: support service action in scsi_report_opcode() Niklas Cassel
2022-12-08 10:59 ` [PATCH 14/25] block: introduce duration-limits priority class Niklas Cassel
2022-12-08 10:59 ` [PATCH 15/25] block: introduce BLK_STS_DURATION_LIMIT Niklas Cassel
2022-12-08 10:59 ` [PATCH 16/25] ata: libata: detect support for command duration limits Niklas Cassel
2022-12-08 10:59 ` [PATCH 17/25] ata: libata-scsi: handle CDL bits in ata_scsiop_maint_in() Niklas Cassel
2022-12-08 10:59 ` [PATCH 18/25] ata: libata-scsi: add support for CDL pages mode sense Niklas Cassel
2022-12-08 10:59 ` [PATCH 19/25] ata: libata: add ATA feature control sub-page translation Niklas Cassel
2022-12-08 10:59 ` [PATCH 20/25] ata: libata: set read/write commands CDL index Niklas Cassel
2022-12-08 10:59 ` [PATCH 21/25] scsi: sd: detect support for command duration limits Niklas Cassel
2022-12-08 10:59 ` [PATCH 22/25] scsi: sd: set read/write commands CDL index Niklas Cassel
2022-12-08 10:59 ` [PATCH 23/25] scsi: sd: handle read/write CDL timeout failures Niklas Cassel
2022-12-09  0:13   ` Mike Christie
2022-12-09  0:26     ` Damien Le Moal
2022-12-08 10:59 ` [PATCH 24/25] ata: libata: handle completion of CDL commands using policy 0xD Niklas Cassel
2022-12-08 10:59 ` [PATCH 25/25] Documentation: sysfs-block-device: document command duration limits Niklas Cassel
2022-12-09  3:22   ` Bagas Sanjaya
2022-12-09  3:31     ` Damien Le Moal
2022-12-08 18:18 ` [PATCH 00/25] Add Command Duration Limits support Chaitanya Kulkarni
2022-12-09  0:29   ` 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=20221208105947.2399894-7-niklas.cassel@wdc.com \
    --to=niklas.cassel@wdc.com \
    --cc=damien.lemoal@opensource.wdc.com \
    --cc=hare@suse.de \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-scsi@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.