[PATCH 00/14] libata: SATL update

[PATCH 00/14] libata: SATL update

[Hannes Reinecke]

Hi all,

here's a patchset to update the SCSI-to-ATA translation layer.
It is a resubmission of an earlier patchset for ATA autosense handling,
which got reverted as Tejun pointed out some issues with libata EH.

This patchset now implements proper autosense handling, where the
retrieved sense code is evaluated to modify libata EH decisions.
Additionally, it implements the control mode page handling to allow
switching between fixed and descriptor sense formats.
Finally I've updated the sense code generation to set the 'information'
field where possible.

This is the first part of a larger patchset for ZAC/ZBC support;
it requires the scsi trace fixes queued for in mkp/4.7/scsi-queue.
The full patchset can be found at:

git.kernel.org/hare/scsi-devel/h/zbc.v3

As usual, comments and reviews are welcome.

Hannes Reinecke (14):
  libata: Implement NCQ autosense
  libata: Implement support for sense data reporting
  libata-scsi: sanitize ata_gen_ata_sense()
  libata: sanitize ata_tf_read_block()
  libata-scsi: use scsi_set_sense_information()
  libata-eh: Set 'information' field for autosense
  libata-scsi: use ata_scsi_set_sense()
  libata: evaluate SCSI sense code
  libata-scsi: generate correct ATA pass-through sense
  libata: Implement control mode page to select sense format
  scsi: add scsi_set_sense_field_pointer()
  libata-scsi: Set field pointer in sense code
  libata-scsi: set bit pointer for sense code information
  libata-scsi: Set information sense field for invalid parameter

 drivers/ata/libata-core.c  |  24 ++-
 drivers/ata/libata-eh.c    | 109 +++++++++--
 drivers/ata/libata-scsi.c  | 458 ++++++++++++++++++++++++++++++++-------------
 drivers/ata/libata.h       |   8 +-
 drivers/scsi/scsi_common.c |  53 ++++++
 drivers/scsi/scsi_error.c  |   3 +-
 include/linux/ata.h        |  18 ++
 include/linux/libata.h     |   1 +
 include/scsi/scsi_common.h |   1 +
 include/scsi/scsi_eh.h     |   1 +
 10 files changed, 531 insertions(+), 145 deletions(-)

-- 
1.8.5.6

[PATCH 01/14] libata: Implement NCQ autosense

[Hannes Reinecke]

Some newer devices support NCQ autosense (cf ACS-4), so we should
be using it to retrieve the sense code and speed up recovery.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-eh.c   | 12 ++++++++++++
 drivers/ata/libata-scsi.c |  7 ++++++-
 drivers/ata/libata.h      |  1 +
 include/linux/ata.h       |  2 ++
 4 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 961acc7..8c8355f 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1600,6 +1600,8 @@ static int ata_eh_read_log_10h(struct ata_device *dev,
 	tf->hob_lbah = buf[10];
 	tf->nsect = buf[12];
 	tf->hob_nsect = buf[13];
+	if (ata_id_has_ncq_autosense(dev->id))
+		tf->auxiliary = buf[14] << 16 | buf[15] << 8 | buf[16];
 
 	return 0;
 }
@@ -1797,6 +1799,16 @@ void ata_eh_analyze_ncq_error(struct ata_link *link)
 	memcpy(&qc->result_tf, &tf, sizeof(tf));
 	qc->result_tf.flags = ATA_TFLAG_ISADDR | ATA_TFLAG_LBA | ATA_TFLAG_LBA48;
 	qc->err_mask |= AC_ERR_DEV | AC_ERR_NCQ;
+	if (qc->result_tf.auxiliary) {
+		char sense_key, asc, ascq;
+
+		sense_key = (qc->result_tf.auxiliary >> 16) & 0xff;
+		asc = (qc->result_tf.auxiliary >> 8) & 0xff;
+		ascq = qc->result_tf.auxiliary & 0xff;
+		ata_scsi_set_sense(qc->scsicmd, sense_key, asc, ascq);
+		qc->flags |= ATA_QCFLAG_SENSE_VALID;
+	}
+
 	ehc->i.err_mask &= ~AC_ERR_DEV;
 }
 
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 567859c..6dc2fad 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -270,8 +270,11 @@ DEVICE_ATTR(unload_heads, S_IRUGO | S_IWUSR,
 	    ata_scsi_park_show, ata_scsi_park_store);
 EXPORT_SYMBOL_GPL(dev_attr_unload_heads);
 
-static void ata_scsi_set_sense(struct scsi_cmnd *cmd, u8 sk, u8 asc, u8 ascq)
+void ata_scsi_set_sense(struct scsi_cmnd *cmd, u8 sk, u8 asc, u8 ascq)
 {
+	if (!cmd)
+		return;
+
 	cmd->result = (DRIVER_SENSE << 24) | SAM_STAT_CHECK_CONDITION;
 
 	scsi_build_sense_buffer(0, cmd->sense_buffer, sk, asc, ascq);
@@ -1784,6 +1787,8 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
 	if (((cdb[0] == ATA_16) || (cdb[0] == ATA_12)) &&
 	    ((cdb[2] & 0x20) || need_sense))
 		ata_gen_passthru_sense(qc);
+	else if (qc->flags & ATA_QCFLAG_SENSE_VALID)
+		cmd->result = SAM_STAT_CHECK_CONDITION;
 	else if (need_sense)
 		ata_gen_ata_sense(qc);
 	else
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index f840ca1..8cfdd96 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -137,6 +137,7 @@ extern int ata_scsi_add_hosts(struct ata_host *host,
 			      struct scsi_host_template *sht);
 extern void ata_scsi_scan_host(struct ata_port *ap, int sync);
 extern int ata_scsi_offline_dev(struct ata_device *dev);
+extern void ata_scsi_set_sense(struct scsi_cmnd *cmd, u8 sk, u8 asc, u8 ascq);
 extern void ata_scsi_media_change_notify(struct ata_device *dev);
 extern void ata_scsi_hotplug(struct work_struct *work);
 extern void ata_schedule_scsi_eh(struct Scsi_Host *shost);
diff --git a/include/linux/ata.h b/include/linux/ata.h
index c1a2f34..e797e1b53 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -528,6 +528,8 @@ struct ata_bmdma_prd {
 #define ata_id_cdb_intr(id)	(((id)[ATA_ID_CONFIG] & 0x60) == 0x20)
 #define ata_id_has_da(id)	((id)[ATA_ID_SATA_CAPABILITY_2] & (1 << 4))
 #define ata_id_has_devslp(id)	((id)[ATA_ID_FEATURE_SUPP] & (1 << 8))
+#define ata_id_has_ncq_autosense(id) \
+				((id)[ATA_ID_FEATURE_SUPP] & (1 << 7))
 
 static inline bool ata_id_has_hipm(const u16 *id)
 {
-- 
1.8.5.6

[PATCH 02/14] libata: Implement support for sense data reporting

[Hannes Reinecke]

ACS-4 defines a sense data reporting feature set.
This patch implements support for it.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-core.c | 20 +++++++++++++-
 drivers/ata/libata-eh.c   | 67 ++++++++++++++++++++++++++++++++++++++++++++---
 include/linux/ata.h       | 16 +++++++++++
 3 files changed, 98 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 55e257c..f991f78 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2148,6 +2148,24 @@ static int ata_dev_config_ncq(struct ata_device *dev,
 	return 0;
 }
 
+static void ata_dev_config_sense_reporting(struct ata_device *dev)
+{
+	unsigned int err_mask;
+
+	if (!ata_id_has_sense_reporting(dev->id))
+		return;
+
+	if (ata_id_sense_reporting_enabled(dev->id))
+		return;
+
+	err_mask = ata_dev_set_feature(dev, SETFEATURE_SENSE_DATA, 0x1);
+	if (err_mask) {
+		ata_dev_dbg(dev,
+			    "failed to enable Sense Data Reporting, Emask 0x%x\n",
+			    err_mask);
+	}
+}
+
 /**
  *	ata_dev_configure - Configure the specified ATA/ATAPI device
  *	@dev: Target device to configure
@@ -2370,7 +2388,7 @@ int ata_dev_configure(struct ata_device *dev)
 					dev->devslp_timing[i] = sata_setting[j];
 				}
 		}
-
+		ata_dev_config_sense_reporting(dev);
 		dev->cdb_len = 16;
 	}
 
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 8c8355f..1a009d7 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1638,6 +1638,56 @@ unsigned int atapi_eh_tur(struct ata_device *dev, u8 *r_sense_key)
 }
 
 /**
+ *	ata_eh_request_sense - perform REQUEST_SENSE_DATA_EXT
+ *	@dev: device to perform REQUEST_SENSE_SENSE_DATA_EXT to
+ *	@cmd: scsi command for which the sense code should be set
+ *
+ *	Perform REQUEST_SENSE_DATA_EXT after the device reported CHECK
+ *	SENSE.  This function is an EH helper.
+*
+ *	LOCKING:
+ *	Kernel thread context (may sleep).
+ */
+static void ata_eh_request_sense(struct ata_queued_cmd *qc,
+				 struct scsi_cmnd *cmd)
+{
+	struct ata_device *dev = qc->dev;
+	struct ata_taskfile tf;
+	unsigned int err_mask;
+
+	if (qc->ap->pflags & ATA_PFLAG_FROZEN) {
+		ata_dev_warn(dev, "sense data available but port frozen\n");
+		return;
+	}
+
+	if (!cmd)
+		return;
+
+	if (!ata_id_sense_reporting_enabled(dev->id)) {
+		ata_dev_warn(qc->dev, "sense data reporting disabled\n");
+		return;
+	}
+
+	DPRINTK("ATA request sense\n");
+
+	ata_tf_init(dev, &tf);
+	tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+	tf.flags |= ATA_TFLAG_LBA | ATA_TFLAG_LBA48;
+	tf.command = ATA_CMD_REQ_SENSE_DATA;
+	tf.protocol = ATA_PROT_NODATA;
+
+	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0);
+	/* Ignore err_mask; ATA_ERR might be set */
+	if (tf.command & ATA_SENSE) {
+		ata_scsi_set_sense(cmd, tf.lbah, tf.lbam, tf.lbal);
+		qc->flags |= ATA_QCFLAG_SENSE_VALID;
+	} else {
+		ata_dev_warn(dev, "request sense failed stat %02x emask %x\n",
+			     tf.command, err_mask);
+	}
+}
+
+/**
  *	atapi_eh_request_sense - perform ATAPI REQUEST_SENSE
  *	@dev: device to perform REQUEST_SENSE to
  *	@sense_buf: result sense data buffer (SCSI_SENSE_BUFFERSIZE bytes long)
@@ -1838,14 +1888,22 @@ static unsigned int ata_eh_analyze_tf(struct ata_queued_cmd *qc,
 		return ATA_EH_RESET;
 	}
 
-	if (stat & (ATA_ERR | ATA_DF))
+	if (stat & (ATA_ERR | ATA_DF)) {
 		qc->err_mask |= AC_ERR_DEV;
-	else
+		/*
+		 * Sense data reporting does not work if the
+		 * device fault bit is set.
+		 */
+		if (stat & ATA_DF)
+			stat &= ~ATA_SENSE;
+	} else
 		return 0;
 
 	switch (qc->dev->class) {
 	case ATA_DEV_ATA:
 	case ATA_DEV_ZAC:
+		if (stat & ATA_SENSE)
+			ata_eh_request_sense(qc, qc->scsicmd);
 		if (err & ATA_ICRC)
 			qc->err_mask |= AC_ERR_ATA_BUS;
 		if (err & (ATA_UNC | ATA_AMNF))
@@ -2581,14 +2639,15 @@ static void ata_eh_link_report(struct ata_link *link)
 
 #ifdef CONFIG_ATA_VERBOSE_ERROR
 		if (res->command & (ATA_BUSY | ATA_DRDY | ATA_DF | ATA_DRQ |
-				    ATA_ERR)) {
+				    ATA_SENSE | ATA_ERR)) {
 			if (res->command & ATA_BUSY)
 				ata_dev_err(qc->dev, "status: { Busy }\n");
 			else
-				ata_dev_err(qc->dev, "status: { %s%s%s%s}\n",
+				ata_dev_err(qc->dev, "status: { %s%s%s%s%s}\n",
 				  res->command & ATA_DRDY ? "DRDY " : "",
 				  res->command & ATA_DF ? "DF " : "",
 				  res->command & ATA_DRQ ? "DRQ " : "",
+				  res->command & ATA_SENSE ? "SENSE " : "",
 				  res->command & ATA_ERR ? "ERR " : "");
 		}
 
diff --git a/include/linux/ata.h b/include/linux/ata.h
index e797e1b53..e402cb3 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -385,6 +385,8 @@ enum {
 	SATA_SSP		= 0x06,	/* Software Settings Preservation */
 	SATA_DEVSLP		= 0x09,	/* Device Sleep */
 
+	SETFEATURE_SENSE_DATA = 0xC3, /* Sense Data Reporting feature */
+
 	/* feature values for SET_MAX */
 	ATA_SET_MAX_ADDR	= 0x00,
 	ATA_SET_MAX_PASSWD	= 0x01,
@@ -718,6 +720,20 @@ static inline bool ata_id_has_read_log_dma_ext(const u16 *id)
 	return false;
 }
 
+static inline bool ata_id_has_sense_reporting(const u16 *id)
+{
+	if (!(id[ATA_ID_CFS_ENABLE_2] & (1 << 15)))
+		return false;
+	return id[ATA_ID_COMMAND_SET_3] & (1 << 6);
+}
+
+static inline bool ata_id_sense_reporting_enabled(const u16 *id)
+{
+	if (!(id[ATA_ID_CFS_ENABLE_2] & (1 << 15)))
+		return false;
+	return id[ATA_ID_COMMAND_SET_4] & (1 << 6);
+}
+
 /**
  *	ata_id_major_version	-	get ATA level of drive
  *	@id: Identify data
-- 
1.8.5.6

[PATCH 03/14] libata-scsi: sanitize ata_gen_ata_sense()

[Hannes Reinecke]

ata_to_sense_error() is called conditionally, so we should be
generating a default sense if the condition is not met.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/ata/libata-scsi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 6dc2fad..e331077 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1074,6 +1074,12 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
 		ata_to_sense_error(qc->ap->print_id, tf->command, tf->feature,
 				   &sb[1], &sb[2], &sb[3], verbose);
 		sb[1] &= 0x0f;
+	} else {
+		/* Could not decode error */
+		ata_dev_warn(dev, "could not decode error status 0x%x err_mask 0x%x\n",
+			     tf->command, qc->err_mask);
+		ata_scsi_set_sense(cmd, ABORTED_COMMAND, 0, 0);
+		return;
 	}
 
 	block = ata_tf_read_block(&qc->result_tf, dev);
-- 
1.8.5.6

[PATCH 04/14] libata: sanitize ata_tf_read_block()

[Hannes Reinecke]

Return (u64)-1 if ata_tf_read_block() could not decode the LBA
address, and do not set the information sense descriptor in
ata_gen_ata_sense() in these cases.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/ata/libata-core.c | 4 ++--
 drivers/ata/libata-scsi.c | 2 ++
 drivers/ata/libata.h      | 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index f991f78..a8d6336 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -695,7 +695,7 @@ static int ata_rwcmd_protocol(struct ata_taskfile *tf, struct ata_device *dev)
  *	RETURNS:
  *	Block address read from @tf.
  */
-u64 ata_tf_read_block(struct ata_taskfile *tf, struct ata_device *dev)
+u64 ata_tf_read_block(const struct ata_taskfile *tf, struct ata_device *dev)
 {
 	u64 block = 0;
 
@@ -720,7 +720,7 @@ u64 ata_tf_read_block(struct ata_taskfile *tf, struct ata_device *dev)
 		if (!sect) {
 			ata_dev_warn(dev,
 				     "device reported invalid CHS sector 0\n");
-			sect = 1; /* oh well */
+			return (u64)-1;
 		}
 
 		block = (cyl * dev->heads + head) * dev->sectors + sect - 1;
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index e331077..410ce14 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1083,6 +1083,8 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
 	}
 
 	block = ata_tf_read_block(&qc->result_tf, dev);
+	if (block == (u64)-1)
+		return;
 
 	/* information sense data descriptor */
 	sb[7] = 12;
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 8cfdd96..507c22f 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -67,7 +67,8 @@ extern struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev, int tag);
 extern int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,
 			   u64 block, u32 n_block, unsigned int tf_flags,
 			   unsigned int tag);
-extern u64 ata_tf_read_block(struct ata_taskfile *tf, struct ata_device *dev);
+extern u64 ata_tf_read_block(const struct ata_taskfile *tf,
+			     struct ata_device *dev);
 extern unsigned ata_exec_internal(struct ata_device *dev,
 				  struct ata_taskfile *tf, const u8 *cdb,
 				  int dma_dir, void *buf, unsigned int buflen,
-- 
1.8.5.6

[PATCH 05/14] libata-scsi: use scsi_set_sense_information()

[Hannes Reinecke]

Use scsi_set_sense_information() instead of hand-crafted function.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/ata/libata-scsi.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 410ce14..de593d8 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1055,7 +1055,6 @@ static void ata_gen_ata_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;
 	int verbose = qc->ap->ops->error_handler == NULL;
 	u64 block;
 
@@ -1086,18 +1085,7 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
 	if (block == (u64)-1)
 		return;
 
-	/* information sense data descriptor */
-	sb[7] = 12;
-	desc[0] = 0x00;
-	desc[1] = 10;
-
-	desc[2] |= 0x80;	/* valid */
-	desc[6] = block >> 40;
-	desc[7] = block >> 32;
-	desc[8] = block >> 24;
-	desc[9] = block >> 16;
-	desc[10] = block >> 8;
-	desc[11] = block;
+	scsi_set_sense_information(sb, SCSI_SENSE_BUFFERSIZE, block);
 }
 
 static void ata_scsi_sdev_config(struct scsi_device *sdev)
-- 
1.8.5.6

[PATCH 06/14] libata-eh: Set 'information' field for autosense

[Hannes Reinecke]

If NCQ autosense or the sense data reporting feature is enabled
the LBA of the offending command should be stored in the sense
data 'information' field.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-eh.c   |  2 ++
 drivers/ata/libata-scsi.c | 17 +++++++++++++++++
 drivers/ata/libata.h      |  3 +++
 3 files changed, 22 insertions(+)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 1a009d7..d33e7b8 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1856,6 +1856,8 @@ void ata_eh_analyze_ncq_error(struct ata_link *link)
 		asc = (qc->result_tf.auxiliary >> 8) & 0xff;
 		ascq = qc->result_tf.auxiliary & 0xff;
 		ata_scsi_set_sense(qc->scsicmd, 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 de593d8..e43ca29 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -280,6 +280,23 @@ void ata_scsi_set_sense(struct scsi_cmnd *cmd, u8 sk, u8 asc, u8 ascq)
 	scsi_build_sense_buffer(0, cmd->sense_buffer, sk, asc, ascq);
 }
 
+void ata_scsi_set_sense_information(struct ata_device *dev,
+				    struct scsi_cmnd *cmd,
+				    const struct ata_taskfile *tf)
+{
+	u64 information;
+
+	if (!cmd)
+		return;
+
+	information = ata_tf_read_block(tf, dev);
+	if (information == (u64)-1)
+		return;
+
+	scsi_set_sense_information(cmd->sense_buffer,
+				   SCSI_SENSE_BUFFERSIZE, information);
+}
+
 static ssize_t
 ata_scsi_em_message_store(struct device *dev, struct device_attribute *attr,
 			  const char *buf, size_t count)
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 507c22f..dbc6760 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -139,6 +139,9 @@ 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 void ata_scsi_set_sense(struct scsi_cmnd *cmd, 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);
 extern void ata_scsi_media_change_notify(struct ata_device *dev);
 extern void ata_scsi_hotplug(struct work_struct *work);
 extern void ata_schedule_scsi_eh(struct Scsi_Host *shost);
-- 
1.8.5.6

[PATCH 07/14] libata-scsi: use ata_scsi_set_sense()

[Hannes Reinecke]

Use ata_scsi_set_sense() throughout to ensure the sense code
format is consistent.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/ata/libata-scsi.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index e43ca29..c69186b 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1000,6 +1000,7 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
 	unsigned char *sb = cmd->sense_buffer;
 	unsigned char *desc = sb + 8;
 	int verbose = qc->ap->ops->error_handler == NULL;
+	u8 sense_key, asc, ascq;
 
 	memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
 
@@ -1012,12 +1013,11 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
 	if (qc->err_mask ||
 	    tf->command & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ)) {
 		ata_to_sense_error(qc->ap->print_id, tf->command, tf->feature,
-				   &sb[1], &sb[2], &sb[3], verbose);
-		sb[1] &= 0x0f;
+				   &sense_key, &asc, &ascq, verbose);
+		ata_scsi_set_sense(cmd, sense_key, asc, ascq);
 	} else {
-		sb[1] = RECOVERED_ERROR;
-		sb[2] = 0;
-		sb[3] = 0x1D;
+		/* ATA PASS-THROUGH INFORMATION AVAILABLE */
+		ata_scsi_set_sense(cmd, RECOVERED_ERROR, 0, 0x1D);
 	}
 
 	/*
@@ -1074,22 +1074,20 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
 	unsigned char *sb = cmd->sense_buffer;
 	int verbose = qc->ap->ops->error_handler == NULL;
 	u64 block;
+	u8 sense_key, asc, ascq;
 
 	memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
 
 	cmd->result = (DRIVER_SENSE << 24) | SAM_STAT_CHECK_CONDITION;
 
-	/* sense data is current and format is descriptor */
-	sb[0] = 0x72;
-
 	/* Use ata_to_sense_error() to map status register bits
 	 * onto sense key, asc & ascq.
 	 */
 	if (qc->err_mask ||
 	    tf->command & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ)) {
 		ata_to_sense_error(qc->ap->print_id, tf->command, tf->feature,
-				   &sb[1], &sb[2], &sb[3], verbose);
-		sb[1] &= 0x0f;
+				   &sense_key, &asc, &ascq, verbose);
+		ata_scsi_set_sense(cmd, 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",
-- 
1.8.5.6

[PATCH 08/14] libata: evaluate SCSI sense code

[Hannes Reinecke]

Whenever a sense code is set it would need to be evaluated to
update the error mask.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-eh.c   | 28 +++++++++++++++++++---------
 drivers/scsi/scsi_error.c |  3 ++-
 include/scsi/scsi_eh.h    |  1 +
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index d33e7b8..99bb9f9 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1919,20 +1919,30 @@ static unsigned int ata_eh_analyze_tf(struct ata_queued_cmd *qc,
 			tmp = atapi_eh_request_sense(qc->dev,
 						qc->scsicmd->sense_buffer,
 						qc->result_tf.feature >> 4);
-			if (!tmp) {
-				/* ATA_QCFLAG_SENSE_VALID is used to
-				 * tell atapi_qc_complete() that sense
-				 * data is already valid.
-				 *
-				 * TODO: interpret sense data and set
-				 * appropriate err_mask.
-				 */
+			if (!tmp)
 				qc->flags |= ATA_QCFLAG_SENSE_VALID;
-			} else
+			else
 				qc->err_mask |= tmp;
 		}
 	}
 
+	if (qc->flags & ATA_QCFLAG_SENSE_VALID) {
+		int ret = scsi_check_sense(qc->scsicmd);
+		/*
+		 * SUCCESS here means that the sense code could
+		 * evaluated and should be passed to the upper layers
+		 * for correct evaluation.
+		 * FAILED means the sense code could not interpreted
+		 * and the device would need to be reset.
+		 * NEEDS_RETRY and ADD_TO_MLQUEUE means that the
+		 * command would need to be retried.
+		 */
+		if (ret == NEEDS_RETRY || ret == ADD_TO_MLQUEUE) {
+			qc->flags |= ATA_QCFLAG_RETRY;
+			qc->err_mask |= AC_ERR_OTHER;
+		} else if (ret != SUCCESS)
+			qc->err_mask |= AC_ERR_HSM;
+	}
 	if (qc->err_mask & (AC_ERR_HSM | AC_ERR_TIMEOUT | AC_ERR_ATA_BUS))
 		action |= ATA_EH_RESET;
 
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 984ddcb..a8b610e 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -452,7 +452,7 @@ static void scsi_report_sense(struct scsi_device *sdev,
  *	When a deferred error is detected the current command has
  *	not been executed and needs retrying.
  */
-static int scsi_check_sense(struct scsi_cmnd *scmd)
+int scsi_check_sense(struct scsi_cmnd *scmd)
 {
 	struct scsi_device *sdev = scmd->device;
 	struct scsi_sense_hdr sshdr;
@@ -602,6 +602,7 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
 		return SUCCESS;
 	}
 }
+EXPORT_SYMBOL_GPL(scsi_check_sense);
 
 static void scsi_handle_queue_ramp_up(struct scsi_device *sdev)
 {
diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
index dbb8c64..98d366b 100644
--- a/include/scsi/scsi_eh.h
+++ b/include/scsi/scsi_eh.h
@@ -16,6 +16,7 @@ extern void scsi_report_device_reset(struct Scsi_Host *, int, int);
 extern int scsi_block_when_processing_errors(struct scsi_device *);
 extern bool scsi_command_normalize_sense(const struct scsi_cmnd *cmd,
 					 struct scsi_sense_hdr *sshdr);
+extern int scsi_check_sense(struct scsi_cmnd *);
 
 static inline bool scsi_sense_is_deferred(const struct scsi_sense_hdr *sshdr)
 {
-- 
1.8.5.6

[PATCH 09/14] libata-scsi: generate correct ATA pass-through sense

[Hannes Reinecke]

Generate ATA pass-through sense for both fixed and descriptor
format sense.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/ata/libata-scsi.c | 93 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 59 insertions(+), 34 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index c69186b..84c5e13 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1016,43 +1016,68 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
 				   &sense_key, &asc, &ascq, verbose);
 		ata_scsi_set_sense(cmd, sense_key, asc, ascq);
 	} else {
-		/* ATA PASS-THROUGH INFORMATION AVAILABLE */
-		ata_scsi_set_sense(cmd, RECOVERED_ERROR, 0, 0x1D);
+		/*
+		 * ATA PASS-THROUGH INFORMATION AVAILABLE
+		 * Always in descriptor format sense.
+		 */
+		scsi_build_sense_buffer(1, cmd->sense_buffer,
+					RECOVERED_ERROR, 0, 0x1D);
 	}
 
-	/*
-	 * Sense data is current and format is descriptor.
-	 */
-	sb[0] = 0x72;
-
-	desc[0] = 0x09;
-
-	/* set length of additional sense data */
-	sb[7] = 14;
-	desc[1] = 12;
-
-	/*
-	 * Copy registers into sense buffer.
-	 */
-	desc[2] = 0x00;
-	desc[3] = tf->feature;	/* == error reg */
-	desc[5] = tf->nsect;
-	desc[7] = tf->lbal;
-	desc[9] = tf->lbam;
-	desc[11] = tf->lbah;
-	desc[12] = tf->device;
-	desc[13] = tf->command; /* == status reg */
+	if ((cmd->sense_buffer[0] & 0x7f) >= 0x72) {
+		u8 len;
+
+		/* descriptor format */
+		len = sb[7];
+		desc = (char *)scsi_sense_desc_find(sb, len + 8, 9);
+		if (!desc) {
+			if (SCSI_SENSE_BUFFERSIZE < len + 14)
+				return;
+			sb[7] = len + 14;
+			desc = sb + 8 + len;
+		}
+		desc[0] = 9;
+		desc[1] = 12;
+		/*
+		 * Copy registers into sense buffer.
+		 */
+		desc[2] = 0x00;
+		desc[3] = tf->feature;	/* == error reg */
+		desc[5] = tf->nsect;
+		desc[7] = tf->lbal;
+		desc[9] = tf->lbam;
+		desc[11] = tf->lbah;
+		desc[12] = tf->device;
+		desc[13] = tf->command; /* == status reg */
 
-	/*
-	 * Fill in Extend bit, and the high order bytes
-	 * if applicable.
-	 */
-	if (tf->flags & ATA_TFLAG_LBA48) {
-		desc[2] |= 0x01;
-		desc[4] = tf->hob_nsect;
-		desc[6] = tf->hob_lbal;
-		desc[8] = tf->hob_lbam;
-		desc[10] = tf->hob_lbah;
+		/*
+		 * Fill in Extend bit, and the high order bytes
+		 * if applicable.
+		 */
+		if (tf->flags & ATA_TFLAG_LBA48) {
+			desc[2] |= 0x01;
+			desc[4] = tf->hob_nsect;
+			desc[6] = tf->hob_lbal;
+			desc[8] = tf->hob_lbam;
+			desc[10] = tf->hob_lbah;
+		}
+	} else {
+		/* Fixed sense format */
+		desc[0] = tf->feature;
+		desc[1] = tf->command; /* status */
+		desc[2] = tf->device;
+		desc[3] = tf->nsect;
+		desc[0] = 0;
+		if (tf->flags & ATA_TFLAG_LBA48)  {
+			desc[8] |= 0x80;
+			if (tf->hob_nsect)
+				desc[8] |= 0x40;
+			if (tf->hob_lbal || tf->hob_lbam || tf->hob_lbah)
+				desc[8] |= 0x20;
+		}
+		desc[9] = tf->lbal;
+		desc[10] = tf->lbam;
+		desc[11] = tf->lbah;
 	}
 }
 
-- 
1.8.5.6

[PATCH 10/14] libata: Implement control mode page to select sense format

[Hannes Reinecke]

Implement MODE SELECT for the control mode page to allow the OS
to switch to descriptor sense.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-eh.c   |   4 +-
 drivers/ata/libata-scsi.c | 119 +++++++++++++++++++++++++++++++++-------------
 drivers/ata/libata.h      |   3 +-
 include/linux/libata.h    |   1 +
 4 files changed, 92 insertions(+), 35 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 99bb9f9..e6b16f4 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1679,7 +1679,7 @@ static void ata_eh_request_sense(struct ata_queued_cmd *qc,
 	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0);
 	/* Ignore err_mask; ATA_ERR might be set */
 	if (tf.command & ATA_SENSE) {
-		ata_scsi_set_sense(cmd, tf.lbah, tf.lbam, tf.lbal);
+		ata_scsi_set_sense(dev, cmd, tf.lbah, tf.lbam, tf.lbal);
 		qc->flags |= ATA_QCFLAG_SENSE_VALID;
 	} else {
 		ata_dev_warn(dev, "request sense failed stat %02x emask %x\n",
@@ -1855,7 +1855,7 @@ void ata_eh_analyze_ncq_error(struct ata_link *link)
 		sense_key = (qc->result_tf.auxiliary >> 16) & 0xff;
 		asc = (qc->result_tf.auxiliary >> 8) & 0xff;
 		ascq = qc->result_tf.auxiliary & 0xff;
-		ata_scsi_set_sense(qc->scsicmd, sense_key, asc, ascq);
+		ata_scsi_set_sense(dev, qc->scsicmd, 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 84c5e13..80545fa 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -270,14 +270,17 @@ DEVICE_ATTR(unload_heads, S_IRUGO | S_IWUSR,
 	    ata_scsi_park_show, ata_scsi_park_store);
 EXPORT_SYMBOL_GPL(dev_attr_unload_heads);
 
-void ata_scsi_set_sense(struct scsi_cmnd *cmd, 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 d_sense = (dev->flags & ATA_DFLAG_D_SENSE);
+
 	if (!cmd)
 		return;
 
 	cmd->result = (DRIVER_SENSE << 24) | SAM_STAT_CHECK_CONDITION;
 
-	scsi_build_sense_buffer(0, cmd->sense_buffer, sk, asc, ascq);
+	scsi_build_sense_buffer(d_sense, cmd->sense_buffer, sk, asc, ascq);
 }
 
 void ata_scsi_set_sense_information(struct ata_device *dev,
@@ -384,9 +387,10 @@ struct device_attribute *ata_common_sdev_attrs[] = {
 };
 EXPORT_SYMBOL_GPL(ata_common_sdev_attrs);
 
-static void ata_scsi_invalid_field(struct scsi_cmnd *cmd)
+static void ata_scsi_invalid_field(struct ata_device *dev,
+				   struct scsi_cmnd *cmd)
 {
-	ata_scsi_set_sense(cmd, ILLEGAL_REQUEST, 0x24, 0x0);
+	ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x24, 0x0);
 	/* "Invalid field in cbd" */
 	cmd->scsi_done(cmd);
 }
@@ -1014,7 +1018,7 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
 	    tf->command & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ)) {
 		ata_to_sense_error(qc->ap->print_id, tf->command, tf->feature,
 				   &sense_key, &asc, &ascq, verbose);
-		ata_scsi_set_sense(cmd, sense_key, asc, ascq);
+		ata_scsi_set_sense(qc->dev, cmd, sense_key, asc, ascq);
 	} else {
 		/*
 		 * ATA PASS-THROUGH INFORMATION AVAILABLE
@@ -1112,20 +1116,20 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
 	    tf->command & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ)) {
 		ata_to_sense_error(qc->ap->print_id, tf->command, tf->feature,
 				   &sense_key, &asc, &ascq, verbose);
-		ata_scsi_set_sense(cmd, sense_key, asc, ascq);
+		ata_scsi_set_sense(dev, cmd, 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->command, qc->err_mask);
-		ata_scsi_set_sense(cmd, ABORTED_COMMAND, 0, 0);
+		ata_scsi_set_sense(dev, cmd, ABORTED_COMMAND, 0, 0);
 		return;
 	}
-
 	block = ata_tf_read_block(&qc->result_tf, dev);
 	if (block == (u64)-1)
 		return;
 
-	scsi_set_sense_information(sb, SCSI_SENSE_BUFFERSIZE, block);
+	scsi_set_sense_information(cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE,
+				   block);
 }
 
 static void ata_scsi_sdev_config(struct scsi_device *sdev)
@@ -1440,7 +1444,7 @@ static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc)
 	return 0;
 
  invalid_fld:
-	ata_scsi_set_sense(scmd, ILLEGAL_REQUEST, 0x24, 0x0);
+	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x24, 0x0);
 	/* "Invalid field in cbd" */
 	return 1;
  skip:
@@ -1679,12 +1683,12 @@ static unsigned int ata_scsi_verify_xlat(struct ata_queued_cmd *qc)
 	return 0;
 
 invalid_fld:
-	ata_scsi_set_sense(scmd, ILLEGAL_REQUEST, 0x24, 0x0);
+	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x24, 0x0);
 	/* "Invalid field in cbd" */
 	return 1;
 
 out_of_range:
-	ata_scsi_set_sense(scmd, ILLEGAL_REQUEST, 0x21, 0x0);
+	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x21, 0x0);
 	/* "Logical Block Address out of range" */
 	return 1;
 
@@ -1781,12 +1785,12 @@ static unsigned int ata_scsi_rw_xlat(struct ata_queued_cmd *qc)
 		goto out_of_range;
 	/* treat all other errors as -EINVAL, fall through */
 invalid_fld:
-	ata_scsi_set_sense(scmd, ILLEGAL_REQUEST, 0x24, 0x0);
+	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x24, 0x0);
 	/* "Invalid field in cbd" */
 	return 1;
 
 out_of_range:
-	ata_scsi_set_sense(scmd, ILLEGAL_REQUEST, 0x21, 0x0);
+	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x21, 0x0);
 	/* "Logical Block Address out of range" */
 	return 1;
 
@@ -2366,9 +2370,12 @@ static unsigned int ata_msense_caching(u16 *id, u8 *buf, bool changeable)
  *	LOCKING:
  *	None.
  */
-static unsigned int ata_msense_ctl_mode(u8 *buf, bool changeable)
+static unsigned int ata_msense_ctl_mode(struct ata_device *dev, u8 *buf,
+					bool changeable)
 {
 	modecpy(buf, def_control_mpage, sizeof(def_control_mpage), changeable);
+	if (changeable && (dev->flags & ATA_DFLAG_D_SENSE))
+		buf[2] |= (1 << 2);	/* Descriptor sense requested */
 	return sizeof(def_control_mpage);
 }
 
@@ -2482,13 +2489,13 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
 		break;
 
 	case CONTROL_MPAGE:
-		p += ata_msense_ctl_mode(p, page_control == 1);
+		p += ata_msense_ctl_mode(args->dev, p, page_control == 1);
 		break;
 
 	case ALL_MPAGES:
 		p += ata_msense_rw_recovery(p, page_control == 1);
 		p += ata_msense_caching(args->id, p, page_control == 1);
-		p += ata_msense_ctl_mode(p, page_control == 1);
+		p += ata_msense_ctl_mode(args->dev, p, page_control == 1);
 		break;
 
 	default:		/* invalid page code */
@@ -2521,12 +2528,12 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
 	return 0;
 
 invalid_fld:
-	ata_scsi_set_sense(args->cmd, ILLEGAL_REQUEST, 0x24, 0x0);
+	ata_scsi_set_sense(dev, args->cmd, ILLEGAL_REQUEST, 0x24, 0x0);
 	/* "Invalid field in cbd" */
 	return 1;
 
 saving_not_supp:
-	ata_scsi_set_sense(args->cmd, ILLEGAL_REQUEST, 0x39, 0x0);
+	ata_scsi_set_sense(dev, args->cmd, ILLEGAL_REQUEST, 0x39, 0x0);
 	 /* "Saving parameters not supported" */
 	return 1;
 }
@@ -3163,7 +3170,7 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
 	return 0;
 
  invalid_fld:
-	ata_scsi_set_sense(scmd, ILLEGAL_REQUEST, 0x24, 0x00);
+	ata_scsi_set_sense(dev, scmd, ILLEGAL_REQUEST, 0x24, 0x00);
 	/* "Invalid field in cdb" */
 	return 1;
 }
@@ -3228,7 +3235,7 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
 	return 0;
 
  invalid_fld:
-	ata_scsi_set_sense(scmd, ILLEGAL_REQUEST, 0x24, 0x00);
+	ata_scsi_set_sense(dev, scmd, ILLEGAL_REQUEST, 0x24, 0x00);
 	/* "Invalid field in cdb" */
 	return 1;
 }
@@ -3280,6 +3287,51 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc,
 }
 
 /**
+ *	ata_mselect_control - Simulate MODE SELECT for control page
+ *	@qc: Storage for translated ATA taskfile
+ *	@buf: input buffer
+ *	@len: number of valid bytes in the input buffer
+ *
+ *	Prepare a taskfile to modify caching information for the device.
+ *
+ *	LOCKING:
+ *	None.
+ */
+static int ata_mselect_control(struct ata_queued_cmd *qc,
+			       const u8 *buf, int len)
+{
+	struct ata_device *dev = qc->dev;
+	char mpage[CONTROL_MPAGE_LEN];
+	u8 d_sense;
+
+	/*
+	 * The first two bytes of def_control_mpage are a header, so offsets
+	 * in mpage are off by 2 compared to buf.  Same for len.
+	 */
+
+	if (len != CONTROL_MPAGE_LEN - 2)
+		return -EINVAL;
+
+	d_sense = buf[0] & (1 << 2);
+
+	/*
+	 * Check that read-only bits are not modified.
+	 */
+	ata_msense_ctl_mode(dev, mpage, false);
+	mpage[2] &= ~(1 << 2);
+	mpage[2] |= d_sense;
+	if (memcmp(mpage + 2, buf, CONTROL_MPAGE_LEN - 2) != 0)
+		return -EINVAL;
+	if (d_sense & (1 << 2))
+		dev->flags |= ATA_DFLAG_D_SENSE;
+	else
+		dev->flags &= ~ATA_DFLAG_D_SENSE;
+	qc->scsicmd->result = SAM_STAT_GOOD;
+	qc->scsicmd->scsi_done(qc->scsicmd);
+	return 0;
+}
+
+/**
  *	ata_scsiop_mode_select - Simulate MODE SELECT 6, 10 commands
  *	@qc: Storage for translated ATA taskfile
  *
@@ -3381,7 +3433,10 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc)
 		if (ata_mselect_caching(qc, p, pg_len) < 0)
 			goto invalid_param;
 		break;
-
+	case CONTROL_MPAGE:
+		if (ata_mselect_control(qc, p, pg_len) < 0)
+			goto invalid_param;
+		break;
 	default:		/* invalid page code */
 		goto invalid_param;
 	}
@@ -3397,17 +3452,17 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc)
 
  invalid_fld:
 	/* "Invalid field in CDB" */
-	ata_scsi_set_sense(scmd, ILLEGAL_REQUEST, 0x24, 0x0);
+	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x24, 0x0);
 	return 1;
 
  invalid_param:
 	/* "Invalid field in parameter list" */
-	ata_scsi_set_sense(scmd, ILLEGAL_REQUEST, 0x26, 0x0);
+	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x26, 0x0);
 	return 1;
 
  invalid_param_len:
 	/* "Parameter list length error" */
-	ata_scsi_set_sense(scmd, ILLEGAL_REQUEST, 0x1a, 0x0);
+	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x1a, 0x0);
 	return 1;
 
  skip:
@@ -3611,12 +3666,12 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 	switch(scsicmd[0]) {
 	/* TODO: worth improving? */
 	case FORMAT_UNIT:
-		ata_scsi_invalid_field(cmd);
+		ata_scsi_invalid_field(dev, cmd);
 		break;
 
 	case INQUIRY:
 		if (scsicmd[1] & 2)	           /* is CmdDt set?  */
-			ata_scsi_invalid_field(cmd);
+			ata_scsi_invalid_field(dev, cmd);
 		else if ((scsicmd[1] & 1) == 0)    /* is EVPD clear? */
 			ata_scsi_rbuf_fill(&args, ata_scsiop_inq_std);
 		else switch (scsicmd[2]) {
@@ -3642,7 +3697,7 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 			ata_scsi_rbuf_fill(&args, ata_scsiop_inq_b2);
 			break;
 		default:
-			ata_scsi_invalid_field(cmd);
+			ata_scsi_invalid_field(dev, cmd);
 			break;
 		}
 		break;
@@ -3660,7 +3715,7 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 		if ((scsicmd[1] & 0x1f) == SAI_READ_CAPACITY_16)
 			ata_scsi_rbuf_fill(&args, ata_scsiop_read_cap);
 		else
-			ata_scsi_invalid_field(cmd);
+			ata_scsi_invalid_field(dev, cmd);
 		break;
 
 	case REPORT_LUNS:
@@ -3668,7 +3723,7 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 		break;
 
 	case REQUEST_SENSE:
-		ata_scsi_set_sense(cmd, 0, 0, 0);
+		ata_scsi_set_sense(dev, cmd, 0, 0, 0);
 		cmd->result = (DRIVER_SENSE << 24);
 		cmd->scsi_done(cmd);
 		break;
@@ -3692,12 +3747,12 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 		if ((tmp8 == 0x4) && (!scsicmd[3]) && (!scsicmd[4]))
 			ata_scsi_rbuf_fill(&args, ata_scsiop_noop);
 		else
-			ata_scsi_invalid_field(cmd);
+			ata_scsi_invalid_field(dev, cmd);
 		break;
 
 	/* all other commands */
 	default:
-		ata_scsi_set_sense(cmd, ILLEGAL_REQUEST, 0x20, 0x0);
+		ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x20, 0x0);
 		/* "Invalid command operation code" */
 		cmd->scsi_done(cmd);
 		break;
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index dbc6760..3b301a4 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -138,7 +138,8 @@ extern int ata_scsi_add_hosts(struct ata_host *host,
 			      struct scsi_host_template *sht);
 extern void ata_scsi_scan_host(struct ata_port *ap, int sync);
 extern int ata_scsi_offline_dev(struct ata_device *dev);
-extern void ata_scsi_set_sense(struct scsi_cmnd *cmd, 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_information(struct ata_device *dev,
 					   struct scsi_cmnd *cmd,
 					   const struct ata_taskfile *tf);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 2c4ebef..a418bca 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -180,6 +180,7 @@ enum {
 	ATA_DFLAG_DA		= (1 << 26), /* device supports Device Attention */
 	ATA_DFLAG_DEVSLP	= (1 << 27), /* device supports Device Sleep */
 	ATA_DFLAG_ACPI_DISABLED = (1 << 28), /* ACPI for the device is disabled */
+	ATA_DFLAG_D_SENSE	= (1 << 29), /* Descriptor sense requested */
 
 	ATA_DEV_UNKNOWN		= 0,	/* unknown device */
 	ATA_DEV_ATA		= 1,	/* ATA device */
-- 
1.8.5.6

[PATCH 11/14] scsi: add scsi_set_sense_field_pointer()

[Hannes Reinecke]

Add a function to set the field pointer for SCSI sense codes.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/scsi_common.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++
 include/scsi/scsi_common.h |  1 +
 2 files changed, 54 insertions(+)

diff --git a/drivers/scsi/scsi_common.c b/drivers/scsi/scsi_common.c
index c126966..b51a1c0 100644
--- a/drivers/scsi/scsi_common.c
+++ b/drivers/scsi/scsi_common.c
@@ -285,3 +285,56 @@ int scsi_set_sense_information(u8 *buf, int buf_len, u64 info)
 	return 0;
 }
 EXPORT_SYMBOL(scsi_set_sense_information);
+
+/**
+ * scsi_set_sense_field_pointer - set the field pointer sense key
+ *		specific information in a formatted sense data buffer
+ * @buf:	Where to build sense data
+ * @buf_len:    buffer length
+ * @fp:		field pointer to be set
+ * @bp:		bit pointer to be set
+ * @cd:		command/data bit
+ *
+ * Return value:
+ *	0 on success or EINVAL for invalid sense buffer length
+ **/
+int scsi_set_sense_field_pointer(u8 *buf, int buf_len, u16 fp, u8 bp, bool cd)
+{
+	u8 *ucp, len;
+
+	if ((buf[0] & 0x7f) == 0x72) {
+		len = buf[7];
+		ucp = (char *)scsi_sense_desc_find(buf, len + 8, 2);
+		if (!ucp) {
+			buf[7] = len + 8;
+			ucp = buf + 8 + len;
+		}
+
+		if (buf_len < len + 8)
+			/* Not enough room for info */
+			return -EINVAL;
+
+		ucp[0] = 2;
+		ucp[1] = 6;
+		ucp[4] = 0x80; /* Valid bit */
+		if (cd)
+			ucp[4] |= 0x40;
+		if (bp < 0x8)
+			ucp[4] |= 0x8 | bp;
+		put_unaligned_be16(fp, &ucp[5]);
+	} else if ((buf[0] & 0x7f) == 0x70) {
+		len = buf[7];
+		if (len < 18)
+			buf[7] = 18;
+
+		buf[15] = 0x80;
+		if (cd)
+			buf[15] |= 0x40;
+		if (bp < 0x8)
+			buf[15] |= 0x8 | bp;
+		put_unaligned_be16(fp, &buf[16]);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(scsi_set_sense_field_pointer);
diff --git a/include/scsi/scsi_common.h b/include/scsi/scsi_common.h
index 11571b2..20bf7ea 100644
--- a/include/scsi/scsi_common.h
+++ b/include/scsi/scsi_common.h
@@ -63,6 +63,7 @@ extern bool scsi_normalize_sense(const u8 *sense_buffer, int sb_len,
 
 extern void scsi_build_sense_buffer(int desc, u8 *buf, u8 key, u8 asc, u8 ascq);
 int scsi_set_sense_information(u8 *buf, int buf_len, u64 info);
+int scsi_set_sense_field_pointer(u8 *buf, int buf_len, u16 fp, u8 bp, bool cd);
 extern const u8 * scsi_sense_desc_find(const u8 * sense_buffer, int sb_len,
 				       int desc_type);
 
-- 
1.8.5.6

[PATCH 12/14] libata-scsi: Set field pointer in sense code

[Hannes Reinecke]

If the sense code is 'Invalid field in CDB' we should be
setting the field pointer to the offending byte.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/ata/libata-scsi.c | 155 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 108 insertions(+), 47 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 80545fa..30ae9a8 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -300,6 +300,15 @@ void ata_scsi_set_sense_information(struct ata_device *dev,
 				   SCSI_SENSE_BUFFERSIZE, information);
 }
 
+static void ata_scsi_set_invalid_field(struct ata_device *dev,
+				       struct scsi_cmnd *cmd, u16 field)
+{
+	ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x24, 0x0);
+	/* "Invalid field in cbd" */
+	scsi_set_sense_field_pointer(cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE,
+				     field, 0xff, 1);
+}
+
 static ssize_t
 ata_scsi_em_message_store(struct device *dev, struct device_attribute *attr,
 			  const char *buf, size_t count)
@@ -388,10 +397,9 @@ struct device_attribute *ata_common_sdev_attrs[] = {
 EXPORT_SYMBOL_GPL(ata_common_sdev_attrs);
 
 static void ata_scsi_invalid_field(struct ata_device *dev,
-				   struct scsi_cmnd *cmd)
+				   struct scsi_cmnd *cmd, u16 field)
 {
-	ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x24, 0x0);
-	/* "Invalid field in cbd" */
+	ata_scsi_set_invalid_field(dev, cmd, field);
 	cmd->scsi_done(cmd);
 }
 
@@ -1386,19 +1394,26 @@ static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc)
 	struct scsi_cmnd *scmd = qc->scsicmd;
 	struct ata_taskfile *tf = &qc->tf;
 	const u8 *cdb = scmd->cmnd;
+	u16 fp;
 
-	if (scmd->cmd_len < 5)
+	if (scmd->cmd_len < 5) {
+		fp = 4;
 		goto invalid_fld;
+	}
 
 	tf->flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR;
 	tf->protocol = ATA_PROT_NODATA;
 	if (cdb[1] & 0x1) {
 		;	/* ignore IMMED bit, violates sat-r05 */
 	}
-	if (cdb[4] & 0x2)
+	if (cdb[4] & 0x2) {
+		fp = 4;
 		goto invalid_fld;       /* LOEJ bit set not supported */
-	if (((cdb[4] >> 4) & 0xf) != 0)
+	}
+	if (((cdb[4] >> 4) & 0xf) != 0) {
+		fp = 4;
 		goto invalid_fld;       /* power conditions not supported */
+	}
 
 	if (cdb[4] & 0x1) {
 		tf->nsect = 1;	/* 1 sector, lba=0 */
@@ -1444,8 +1459,7 @@ static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc)
 	return 0;
 
  invalid_fld:
-	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x24, 0x0);
-	/* "Invalid field in cbd" */
+	ata_scsi_set_invalid_field(qc->dev, scmd, fp);
 	return 1;
  skip:
 	scmd->result = SAM_STAT_GOOD;
@@ -1596,20 +1610,27 @@ static unsigned int ata_scsi_verify_xlat(struct ata_queued_cmd *qc)
 	const u8 *cdb = scmd->cmnd;
 	u64 block;
 	u32 n_block;
+	u16 fp;
 
 	tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
 	tf->protocol = ATA_PROT_NODATA;
 
 	if (cdb[0] == VERIFY) {
-		if (scmd->cmd_len < 10)
+		if (scmd->cmd_len < 10) {
+			fp = 9;
 			goto invalid_fld;
+		}
 		scsi_10_lba_len(cdb, &block, &n_block);
 	} else if (cdb[0] == VERIFY_16) {
-		if (scmd->cmd_len < 16)
+		if (scmd->cmd_len < 16) {
+			fp = 15;
 			goto invalid_fld;
+		}
 		scsi_16_lba_len(cdb, &block, &n_block);
-	} else
+	} else {
+		fp = 0;
 		goto invalid_fld;
+	}
 
 	if (!n_block)
 		goto nothing_to_do;
@@ -1683,8 +1704,7 @@ static unsigned int ata_scsi_verify_xlat(struct ata_queued_cmd *qc)
 	return 0;
 
 invalid_fld:
-	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x24, 0x0);
-	/* "Invalid field in cbd" */
+	ata_scsi_set_invalid_field(qc->dev, scmd, fp);
 	return 1;
 
 out_of_range:
@@ -1723,6 +1743,7 @@ static unsigned int ata_scsi_rw_xlat(struct ata_queued_cmd *qc)
 	u64 block;
 	u32 n_block;
 	int rc;
+	u16 fp = 0;
 
 	if (cdb[0] == WRITE_10 || cdb[0] == WRITE_6 || cdb[0] == WRITE_16)
 		tf_flags |= ATA_TFLAG_WRITE;
@@ -1731,16 +1752,20 @@ static unsigned int ata_scsi_rw_xlat(struct ata_queued_cmd *qc)
 	switch (cdb[0]) {
 	case READ_10:
 	case WRITE_10:
-		if (unlikely(scmd->cmd_len < 10))
+		if (unlikely(scmd->cmd_len < 10)) {
+			fp = 9;
 			goto invalid_fld;
+		}
 		scsi_10_lba_len(cdb, &block, &n_block);
 		if (cdb[1] & (1 << 3))
 			tf_flags |= ATA_TFLAG_FUA;
 		break;
 	case READ_6:
 	case WRITE_6:
-		if (unlikely(scmd->cmd_len < 6))
+		if (unlikely(scmd->cmd_len < 6)) {
+			fp = 5;
 			goto invalid_fld;
+		}
 		scsi_6_lba_len(cdb, &block, &n_block);
 
 		/* for 6-byte r/w commands, transfer length 0
@@ -1751,14 +1776,17 @@ static unsigned int ata_scsi_rw_xlat(struct ata_queued_cmd *qc)
 		break;
 	case READ_16:
 	case WRITE_16:
-		if (unlikely(scmd->cmd_len < 16))
+		if (unlikely(scmd->cmd_len < 16)) {
+			fp = 15;
 			goto invalid_fld;
+		}
 		scsi_16_lba_len(cdb, &block, &n_block);
 		if (cdb[1] & (1 << 3))
 			tf_flags |= ATA_TFLAG_FUA;
 		break;
 	default:
 		DPRINTK("no-byte command\n");
+		fp = 0;
 		goto invalid_fld;
 	}
 
@@ -1785,8 +1813,7 @@ static unsigned int ata_scsi_rw_xlat(struct ata_queued_cmd *qc)
 		goto out_of_range;
 	/* treat all other errors as -EINVAL, fall through */
 invalid_fld:
-	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x24, 0x0);
-	/* "Invalid field in cbd" */
+	ata_scsi_set_invalid_field(qc->dev, scmd, fp);
 	return 1;
 
 out_of_range:
@@ -2444,6 +2471,7 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
 	u8 pg, spg;
 	unsigned int ebd, page_control, six_byte;
 	u8 dpofua;
+	u16 fp;
 
 	VPRINTK("ENTER\n");
 
@@ -2462,6 +2490,7 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
 	case 3: /* saved */
 		goto saving_not_supp;
 	default:
+		fp = 2;
 		goto invalid_fld;
 	}
 
@@ -2476,8 +2505,10 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
 	 * No mode subpages supported (yet) but asking for _all_
 	 * subpages may be valid
 	 */
-	if (spg && (spg != ALL_SUB_MPAGES))
+	if (spg && (spg != ALL_SUB_MPAGES)) {
+		fp = 3;
 		goto invalid_fld;
+	}
 
 	switch(pg) {
 	case RW_RECOVERY_MPAGE:
@@ -2499,6 +2530,7 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
 		break;
 
 	default:		/* invalid page code */
+		fp = 2;
 		goto invalid_fld;
 	}
 
@@ -2528,8 +2560,7 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
 	return 0;
 
 invalid_fld:
-	ata_scsi_set_sense(dev, args->cmd, ILLEGAL_REQUEST, 0x24, 0x0);
-	/* "Invalid field in cbd" */
+	ata_scsi_set_invalid_field(dev, args->cmd, fp);
 	return 1;
 
 saving_not_supp:
@@ -2990,9 +3021,12 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
 	struct scsi_cmnd *scmd = qc->scsicmd;
 	struct ata_device *dev = qc->dev;
 	const u8 *cdb = scmd->cmnd;
+	u16 fp;
 
-	if ((tf->protocol = ata_scsi_map_proto(cdb[1])) == ATA_PROT_UNKNOWN)
+	if ((tf->protocol = ata_scsi_map_proto(cdb[1])) == ATA_PROT_UNKNOWN) {
+		fp = 1;
 		goto invalid_fld;
+	}
 
 	/* enable LBA */
 	tf->flags |= ATA_TFLAG_LBA;
@@ -3056,8 +3090,10 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
 	case ATA_CMD_READ_LONG_ONCE:
 	case ATA_CMD_WRITE_LONG:
 	case ATA_CMD_WRITE_LONG_ONCE:
-		if (tf->protocol != ATA_PROT_PIO || tf->nsect != 1)
+		if (tf->protocol != ATA_PROT_PIO || tf->nsect != 1) {
+			fp = 1;
 			goto invalid_fld;
+		}
 		qc->sect_size = scsi_bufflen(scmd);
 		break;
 
@@ -3120,12 +3156,16 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
 	ata_qc_set_pc_nbytes(qc);
 
 	/* We may not issue DMA commands if no DMA mode is set */
-	if (tf->protocol == ATA_PROT_DMA && dev->dma_mode == 0)
+	if (tf->protocol == ATA_PROT_DMA && dev->dma_mode == 0) {
+		fp = 1;
 		goto invalid_fld;
+	}
 
 	/* sanity check for pio multi commands */
-	if ((cdb[1] & 0xe0) && !is_multi_taskfile(tf))
+	if ((cdb[1] & 0xe0) && !is_multi_taskfile(tf)) {
+		fp = 1;
 		goto invalid_fld;
+	}
 
 	if (is_multi_taskfile(tf)) {
 		unsigned int multi_count = 1 << (cdb[1] >> 5);
@@ -3146,8 +3186,10 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
 	 * ->set_dmamode(), and ->post_set_mode() hooks).
 	 */
 	if (tf->command == ATA_CMD_SET_FEATURES &&
-	    tf->feature == SETFEATURES_XFER)
+	    tf->feature == SETFEATURES_XFER) {
+		fp = (cdb[0] == ATA_16) ? 4 : 3;
 		goto invalid_fld;
+	}
 
 	/*
 	 * Filter TPM commands by default. These provide an
@@ -3164,14 +3206,15 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
 	 * so that we comply with the TC consortium stated goal that the user
 	 * can turn off TC features of their system.
 	 */
-	if (tf->command >= 0x5C && tf->command <= 0x5F && !libata_allow_tpm)
+	if (tf->command >= 0x5C && tf->command <= 0x5F && !libata_allow_tpm) {
+		fp = (cdb[0] == ATA_16) ? 14 : 9;
 		goto invalid_fld;
+	}
 
 	return 0;
 
  invalid_fld:
-	ata_scsi_set_sense(dev, scmd, ILLEGAL_REQUEST, 0x24, 0x00);
-	/* "Invalid field in cdb" */
+	ata_scsi_set_invalid_field(dev, scmd, fp);
 	return 1;
 }
 
@@ -3185,25 +3228,30 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
 	u32 n_block;
 	u32 size;
 	void *buf;
+	u16 fp;
 
 	/* we may not issue DMA commands if no DMA mode is set */
 	if (unlikely(!dev->dma_mode))
-		goto invalid_fld;
+		goto invalid_opcode;
 
-	if (unlikely(scmd->cmd_len < 16))
+	if (unlikely(scmd->cmd_len < 16)) {
+		fp = 15;
 		goto invalid_fld;
+	}
 	scsi_16_lba_len(cdb, &block, &n_block);
 
 	/* for now we only support WRITE SAME with the unmap bit set */
-	if (unlikely(!(cdb[1] & 0x8)))
+	if (unlikely(!(cdb[1] & 0x8))) {
+		fp = 1;
 		goto invalid_fld;
+	}
 
 	/*
 	 * WRITE SAME always has a sector sized buffer as payload, this
 	 * should never be a multiple entry S/G list.
 	 */
 	if (!scsi_sg_count(scmd))
-		goto invalid_fld;
+		goto invalid_param_len;
 
 	buf = page_address(sg_page(scsi_sglist(scmd)));
 	size = ata_set_lba_range_entries(buf, 512, block, n_block);
@@ -3234,9 +3282,16 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
 
 	return 0;
 
- invalid_fld:
-	ata_scsi_set_sense(dev, scmd, ILLEGAL_REQUEST, 0x24, 0x00);
-	/* "Invalid field in cdb" */
+invalid_fld:
+	ata_scsi_set_invalid_field(dev, scmd, fp);
+	return 1;
+invalid_param_len:
+	/* "Parameter list length error" */
+	ata_scsi_set_sense(dev, scmd, ILLEGAL_REQUEST, 0x1a, 0x0);
+	return 1;
+invalid_opcode:
+	/* "Invalid command operation code" */
+	ata_scsi_set_sense(dev, scmd, ILLEGAL_REQUEST, 0x20, 0x0);
 	return 1;
 }
 
@@ -3350,27 +3405,34 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc)
 	u8 pg, spg;
 	unsigned six_byte, pg_len, hdr_len, bd_len;
 	int len;
+	u16 fp;
 
 	VPRINTK("ENTER\n");
 
 	six_byte = (cdb[0] == MODE_SELECT);
 	if (six_byte) {
-		if (scmd->cmd_len < 5)
+		if (scmd->cmd_len < 5) {
+			fp = 4;
 			goto invalid_fld;
+		}
 
 		len = cdb[4];
 		hdr_len = 4;
 	} else {
-		if (scmd->cmd_len < 9)
+		if (scmd->cmd_len < 9) {
+			fp = 8;
 			goto invalid_fld;
+		}
 
 		len = (cdb[7] << 8) + cdb[8];
 		hdr_len = 8;
 	}
 
 	/* We only support PF=1, SP=0.  */
-	if ((cdb[1] & 0x11) != 0x10)
+	if ((cdb[1] & 0x11) != 0x10) {
+		fp = 1;
 		goto invalid_fld;
+	}
 
 	/* Test early for possible overrun.  */
 	if (!scsi_sg_count(scmd) || scsi_sglist(scmd)->length < len)
@@ -3451,8 +3513,7 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc)
 	return 0;
 
  invalid_fld:
-	/* "Invalid field in CDB" */
-	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x24, 0x0);
+	ata_scsi_set_invalid_field(qc->dev, scmd, fp);
 	return 1;
 
  invalid_param:
@@ -3666,12 +3727,12 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 	switch(scsicmd[0]) {
 	/* TODO: worth improving? */
 	case FORMAT_UNIT:
-		ata_scsi_invalid_field(dev, cmd);
+		ata_scsi_invalid_field(dev, cmd, 0);
 		break;
 
 	case INQUIRY:
-		if (scsicmd[1] & 2)	           /* is CmdDt set?  */
-			ata_scsi_invalid_field(dev, cmd);
+		if (scsicmd[1] & 2)		   /* is CmdDt set?  */
+		    ata_scsi_invalid_field(dev, cmd, 1);
 		else if ((scsicmd[1] & 1) == 0)    /* is EVPD clear? */
 			ata_scsi_rbuf_fill(&args, ata_scsiop_inq_std);
 		else switch (scsicmd[2]) {
@@ -3697,7 +3758,7 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 			ata_scsi_rbuf_fill(&args, ata_scsiop_inq_b2);
 			break;
 		default:
-			ata_scsi_invalid_field(dev, cmd);
+			ata_scsi_invalid_field(dev, cmd, 2);
 			break;
 		}
 		break;
@@ -3715,7 +3776,7 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 		if ((scsicmd[1] & 0x1f) == SAI_READ_CAPACITY_16)
 			ata_scsi_rbuf_fill(&args, ata_scsiop_read_cap);
 		else
-			ata_scsi_invalid_field(dev, cmd);
+			ata_scsi_invalid_field(dev, cmd, 1);
 		break;
 
 	case REPORT_LUNS:
@@ -3747,7 +3808,7 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 		if ((tmp8 == 0x4) && (!scsicmd[3]) && (!scsicmd[4]))
 			ata_scsi_rbuf_fill(&args, ata_scsiop_noop);
 		else
-			ata_scsi_invalid_field(dev, cmd);
+			ata_scsi_invalid_field(dev, cmd, 1);
 		break;
 
 	/* all other commands */
-- 
1.8.5.6

[PATCH 13/14] libata-scsi: set bit pointer for sense code information

[Hannes Reinecke]

When generating a sense code of 'Invalid field in CDB' we
should be setting the bit pointer where appropriate.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/ata/libata-scsi.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 30ae9a8..d72d78d 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -301,12 +301,12 @@ 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)
+				       struct scsi_cmnd *cmd, u16 field, u8 bit)
 {
 	ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x24, 0x0);
 	/* "Invalid field in cbd" */
 	scsi_set_sense_field_pointer(cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE,
-				     field, 0xff, 1);
+				     field, bit, 1);
 }
 
 static ssize_t
@@ -399,7 +399,7 @@ EXPORT_SYMBOL_GPL(ata_common_sdev_attrs);
 static void ata_scsi_invalid_field(struct ata_device *dev,
 				   struct scsi_cmnd *cmd, u16 field)
 {
-	ata_scsi_set_invalid_field(dev, cmd, field);
+	ata_scsi_set_invalid_field(dev, cmd, field, 0xff);
 	cmd->scsi_done(cmd);
 }
 
@@ -1395,6 +1395,7 @@ static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc)
 	struct ata_taskfile *tf = &qc->tf;
 	const u8 *cdb = scmd->cmnd;
 	u16 fp;
+	u8 bp = 0xff;
 
 	if (scmd->cmd_len < 5) {
 		fp = 4;
@@ -1408,10 +1409,12 @@ static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc)
 	}
 	if (cdb[4] & 0x2) {
 		fp = 4;
+		bp = 1;
 		goto invalid_fld;       /* LOEJ bit set not supported */
 	}
 	if (((cdb[4] >> 4) & 0xf) != 0) {
 		fp = 4;
+		bp = 3;
 		goto invalid_fld;       /* power conditions not supported */
 	}
 
@@ -1459,7 +1462,7 @@ static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc)
 	return 0;
 
  invalid_fld:
-	ata_scsi_set_invalid_field(qc->dev, scmd, fp);
+	ata_scsi_set_invalid_field(qc->dev, scmd, fp, bp);
 	return 1;
  skip:
 	scmd->result = SAM_STAT_GOOD;
@@ -1704,7 +1707,7 @@ static unsigned int ata_scsi_verify_xlat(struct ata_queued_cmd *qc)
 	return 0;
 
 invalid_fld:
-	ata_scsi_set_invalid_field(qc->dev, scmd, fp);
+	ata_scsi_set_invalid_field(qc->dev, scmd, fp, 0xff);
 	return 1;
 
 out_of_range:
@@ -1813,7 +1816,7 @@ static unsigned int ata_scsi_rw_xlat(struct ata_queued_cmd *qc)
 		goto out_of_range;
 	/* treat all other errors as -EINVAL, fall through */
 invalid_fld:
-	ata_scsi_set_invalid_field(qc->dev, scmd, fp);
+	ata_scsi_set_invalid_field(qc->dev, scmd, fp, 0xff);
 	return 1;
 
 out_of_range:
@@ -2470,7 +2473,7 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
 	};
 	u8 pg, spg;
 	unsigned int ebd, page_control, six_byte;
-	u8 dpofua;
+	u8 dpofua, bp = 0xff;
 	u16 fp;
 
 	VPRINTK("ENTER\n");
@@ -2491,6 +2494,7 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
 		goto saving_not_supp;
 	default:
 		fp = 2;
+		bp = 6;
 		goto invalid_fld;
 	}
 
@@ -2560,7 +2564,7 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
 	return 0;
 
 invalid_fld:
-	ata_scsi_set_invalid_field(dev, args->cmd, fp);
+	ata_scsi_set_invalid_field(dev, args->cmd, fp, bp);
 	return 1;
 
 saving_not_supp:
@@ -3214,7 +3218,7 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
 	return 0;
 
  invalid_fld:
-	ata_scsi_set_invalid_field(dev, scmd, fp);
+	ata_scsi_set_invalid_field(dev, scmd, fp, 0xff);
 	return 1;
 }
 
@@ -3229,6 +3233,7 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
 	u32 size;
 	void *buf;
 	u16 fp;
+	u8 bp = 0xff;
 
 	/* we may not issue DMA commands if no DMA mode is set */
 	if (unlikely(!dev->dma_mode))
@@ -3243,6 +3248,7 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
 	/* for now we only support WRITE SAME with the unmap bit set */
 	if (unlikely(!(cdb[1] & 0x8))) {
 		fp = 1;
+		bp = 3;
 		goto invalid_fld;
 	}
 
@@ -3283,7 +3289,7 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
 	return 0;
 
 invalid_fld:
-	ata_scsi_set_invalid_field(dev, scmd, fp);
+	ata_scsi_set_invalid_field(dev, scmd, fp, bp);
 	return 1;
 invalid_param_len:
 	/* "Parameter list length error" */
@@ -3406,6 +3412,7 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc)
 	unsigned six_byte, pg_len, hdr_len, bd_len;
 	int len;
 	u16 fp;
+	u8 bp;
 
 	VPRINTK("ENTER\n");
 
@@ -3431,6 +3438,7 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc)
 	/* We only support PF=1, SP=0.  */
 	if ((cdb[1] & 0x11) != 0x10) {
 		fp = 1;
+		bp = (cdb[1] & 0x01) ? 1 : 5;
 		goto invalid_fld;
 	}
 
@@ -3513,7 +3521,7 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc)
 	return 0;
 
  invalid_fld:
-	ata_scsi_set_invalid_field(qc->dev, scmd, fp);
+	ata_scsi_set_invalid_field(qc->dev, scmd, fp, bp);
 	return 1;
 
  invalid_param:
-- 
1.8.5.6

[PATCH 14/14] libata-scsi: Set information sense field for invalid parameter

[Hannes Reinecke]

Whenever the sense key is set to 'invalid parameter' we should
be filling out the sense-key specific information field in the
sense buffer.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/ata/libata-scsi.c | 79 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 59 insertions(+), 20 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index d72d78d..d29832f 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -309,6 +309,15 @@ static void ata_scsi_set_invalid_field(struct ata_device *dev,
 				     field, bit, 1);
 }
 
+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);
+	scsi_set_sense_field_pointer(cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE,
+				     field, 0xff, 0);
+}
+
 static ssize_t
 ata_scsi_em_message_store(struct device *dev, struct device_attribute *attr,
 			  const char *buf, size_t count)
@@ -3313,20 +3322,26 @@ invalid_opcode:
  *	None.
  */
 static int ata_mselect_caching(struct ata_queued_cmd *qc,
-			       const u8 *buf, int len)
+			       const u8 *buf, int len, u16 *fp)
 {
 	struct ata_taskfile *tf = &qc->tf;
 	struct ata_device *dev = qc->dev;
 	char mpage[CACHE_MPAGE_LEN];
 	u8 wce;
+	int i;
 
 	/*
 	 * The first two bytes of def_cache_mpage are a header, so offsets
 	 * in mpage are off by 2 compared to buf.  Same for len.
 	 */
 
-	if (len != CACHE_MPAGE_LEN - 2)
+	if (len != CACHE_MPAGE_LEN - 2) {
+		if (len < CACHE_MPAGE_LEN - 2)
+			*fp = len;
+		else
+			*fp = CACHE_MPAGE_LEN - 2;
 		return -EINVAL;
+	}
 
 	wce = buf[0] & (1 << 2);
 
@@ -3334,10 +3349,14 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc,
 	 * Check that read-only bits are not modified.
 	 */
 	ata_msense_caching(dev->id, mpage, false);
-	mpage[2] &= ~(1 << 2);
-	mpage[2] |= wce;
-	if (memcmp(mpage + 2, buf, CACHE_MPAGE_LEN - 2) != 0)
-		return -EINVAL;
+	for (i = 0; i < CACHE_MPAGE_LEN - 2; i++) {
+		if (i == 0)
+			continue;
+		if (mpage[i + 2] != buf[i]) {
+			*fp = i;
+			return -EINVAL;
+		}
+	}
 
 	tf->flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR;
 	tf->protocol = ATA_PROT_NODATA;
@@ -3359,19 +3378,25 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc,
  *	None.
  */
 static int ata_mselect_control(struct ata_queued_cmd *qc,
-			       const u8 *buf, int len)
+			       const u8 *buf, int len, u16 *fp)
 {
 	struct ata_device *dev = qc->dev;
 	char mpage[CONTROL_MPAGE_LEN];
 	u8 d_sense;
+	int i;
 
 	/*
 	 * The first two bytes of def_control_mpage are a header, so offsets
 	 * in mpage are off by 2 compared to buf.  Same for len.
 	 */
 
-	if (len != CONTROL_MPAGE_LEN - 2)
+	if (len != CONTROL_MPAGE_LEN - 2) {
+		if (len < CONTROL_MPAGE_LEN - 2)
+			*fp = len;
+		else
+			*fp = CONTROL_MPAGE_LEN - 2;
 		return -EINVAL;
+	}
 
 	d_sense = buf[0] & (1 << 2);
 
@@ -3379,10 +3404,14 @@ static int ata_mselect_control(struct ata_queued_cmd *qc,
 	 * Check that read-only bits are not modified.
 	 */
 	ata_msense_ctl_mode(dev, mpage, false);
-	mpage[2] &= ~(1 << 2);
-	mpage[2] |= d_sense;
-	if (memcmp(mpage + 2, buf, CONTROL_MPAGE_LEN - 2) != 0)
-		return -EINVAL;
+	for (i = 0; i < CONTROL_MPAGE_LEN - 2; i++) {
+		if (i == 0)
+			continue;
+		if (mpage[2 + i] != buf[i]) {
+			*fp = i;
+			return -EINVAL;
+		}
+	}
 	if (d_sense & (1 << 2))
 		dev->flags |= ATA_DFLAG_D_SENSE;
 	else
@@ -3411,8 +3440,8 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc)
 	u8 pg, spg;
 	unsigned six_byte, pg_len, hdr_len, bd_len;
 	int len;
-	u16 fp;
-	u8 bp;
+	u16 fp = (u16)-1;
+	u8 bp = 0xff;
 
 	VPRINTK("ENTER\n");
 
@@ -3461,8 +3490,11 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc)
 	p += hdr_len;
 	if (len < bd_len)
 		goto invalid_param_len;
-	if (bd_len != 0 && bd_len != 8)
+	if (bd_len != 0 && bd_len != 8) {
+		fp = (six_byte) ? 3 : 6;
+		fp += bd_len + hdr_len;
 		goto invalid_param;
+	}
 
 	len -= bd_len;
 	p += bd_len;
@@ -3493,21 +3525,29 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc)
 	 * No mode subpages supported (yet) but asking for _all_
 	 * subpages may be valid
 	 */
-	if (spg && (spg != ALL_SUB_MPAGES))
+	if (spg && (spg != ALL_SUB_MPAGES)) {
+		fp = (p[0] & 0x40) ? 1 : 0;
+		fp += hdr_len + bd_len;
 		goto invalid_param;
+	}
 	if (pg_len > len)
 		goto invalid_param_len;
 
 	switch (pg) {
 	case CACHE_MPAGE:
-		if (ata_mselect_caching(qc, p, pg_len) < 0)
+		if (ata_mselect_caching(qc, p, pg_len, &fp) < 0) {
+			fp += hdr_len + bd_len;
 			goto invalid_param;
+		}
 		break;
 	case CONTROL_MPAGE:
-		if (ata_mselect_control(qc, p, pg_len) < 0)
+		if (ata_mselect_control(qc, p, pg_len, &fp) < 0) {
+			fp += hdr_len + bd_len;
 			goto invalid_param;
+		}
 		break;
 	default:		/* invalid page code */
+		fp = bd_len + hdr_len;
 		goto invalid_param;
 	}
 
@@ -3525,8 +3565,7 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc)
 	return 1;
 
  invalid_param:
-	/* "Invalid field in parameter list" */
-	ata_scsi_set_sense(qc->dev, scmd, ILLEGAL_REQUEST, 0x26, 0x0);
+	ata_scsi_set_invalid_parameter(qc->dev, scmd, fp);
 	return 1;
 
  invalid_param_len:
-- 
1.8.5.6

Re: [PATCH 08/14] libata: evaluate SCSI sense code

[Sergei Shtylyov]

On 4/4/2016 12:44 PM, Hannes Reinecke wrote:

> Whenever a sense code is set it would need to be evaluated to
> update the error mask.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   drivers/ata/libata-eh.c   | 28 +++++++++++++++++++---------
>   drivers/scsi/scsi_error.c |  3 ++-
>   include/scsi/scsi_eh.h    |  1 +
>   3 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
> index d33e7b8..99bb9f9 100644
> --- a/drivers/ata/libata-eh.c
> +++ b/drivers/ata/libata-eh.c
> @@ -1919,20 +1919,30 @@ static unsigned int ata_eh_analyze_tf(struct ata_queued_cmd *qc,
[...]
>
> +	if (qc->flags & ATA_QCFLAG_SENSE_VALID) {
> +		int ret = scsi_check_sense(qc->scsicmd);
> +		/*
> +		 * SUCCESS here means that the sense code could

    Could be?

> +		 * evaluated and should be passed to the upper layers
> +		 * for correct evaluation.
> +		 * FAILED means the sense code could not interpreted

    Could not be?

> +		 * and the device would need to be reset.
> +		 * NEEDS_RETRY and ADD_TO_MLQUEUE means that the
> +		 * command would need to be retried.
> +		 */
> +		if (ret == NEEDS_RETRY || ret == ADD_TO_MLQUEUE) {
> +			qc->flags |= ATA_QCFLAG_RETRY;
> +			qc->err_mask |= AC_ERR_OTHER;
> +		} else if (ret != SUCCESS)
> +			qc->err_mask |= AC_ERR_HSM;

    This is asking to be a *switch* statement instead.

[...]

MBR, Sergei

Re: [PATCH 03/14] libata-scsi: sanitize ata_gen_ata_sense()

[Sergei Shtylyov]

On 4/4/2016 12:43 PM, Hannes Reinecke wrote:

> ata_to_sense_error() is called conditionally, so we should be
> generating a default sense if the condition is not met.
>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>   drivers/ata/libata-scsi.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 6dc2fad..e331077 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1074,6 +1074,12 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
>   		ata_to_sense_error(qc->ap->print_id, tf->command, tf->feature,
>   				   &sb[1], &sb[2], &sb[3], verbose);
>   		sb[1] &= 0x0f;
> +	} else {
> +		/* Could not decode error */
> +		ata_dev_warn(dev, "could not decode error status 0x%x err_mask 0x%x\n",

    "%#x" is equivalent and takes up less space.

[...]

MBR, Sergei

Re: [PATCH 10/14] libata: Implement control mode page to select sense format

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3523 bytes --]

Hi Hannes,

[auto build test WARNING on tj-libata/for-next]
[also build test WARNING on v4.6-rc2 next-20160404]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Hannes-Reinecke/libata-SATL-update/20160404-174817
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tj/libata for-next
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

>> drivers/ata/libata-scsi.c:2375: warning: No description found for parameter 'dev'
>> drivers/ata/libata-scsi.c:2375: warning: No description found for parameter 'dev'

vim +/dev +2375 drivers/ata/libata-scsi.c

87340e98 drivers/ata/libata-scsi.c  Tejun Heo       2008-04-28  2359  		buf[12] |= (1 << 5);	/* disable read ahead */
87340e98 drivers/ata/libata-scsi.c  Tejun Heo       2008-04-28  2360  	return sizeof(def_cache_mpage);
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds  2005-04-16  2361  }
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds  2005-04-16  2362  
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds  2005-04-16  2363  /**
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds  2005-04-16  2364   *	ata_msense_ctl_mode - Simulate MODE SENSE control mode page
87340e98 drivers/ata/libata-scsi.c  Tejun Heo       2008-04-28  2365   *	@buf: output buffer
6ca8e794 drivers/ata/libata-scsi.c  Paolo Bonzini   2012-07-05  2366   *	@changeable: whether changeable parameters are requested
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds  2005-04-16  2367   *
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds  2005-04-16  2368   *	Generate a generic MODE SENSE control mode page.
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds  2005-04-16  2369   *
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds  2005-04-16  2370   *	LOCKING:
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds  2005-04-16  2371   *	None.
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds  2005-04-16  2372   */
50bb876e drivers/ata/libata-scsi.c  Hannes Reinecke 2016-04-04  2373  static unsigned int ata_msense_ctl_mode(struct ata_device *dev, u8 *buf,
50bb876e drivers/ata/libata-scsi.c  Hannes Reinecke 2016-04-04  2374  					bool changeable)
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds  2005-04-16 @2375  {
6ca8e794 drivers/ata/libata-scsi.c  Paolo Bonzini   2012-07-05  2376  	modecpy(buf, def_control_mpage, sizeof(def_control_mpage), changeable);
50bb876e drivers/ata/libata-scsi.c  Hannes Reinecke 2016-04-04  2377  	if (changeable && (dev->flags & ATA_DFLAG_D_SENSE))
50bb876e drivers/ata/libata-scsi.c  Hannes Reinecke 2016-04-04  2378  		buf[2] |= (1 << 2);	/* Descriptor sense requested */
00ac37f5 drivers/scsi/libata-scsi.c Douglas Gilbert 2005-10-28  2379  	return sizeof(def_control_mpage);
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds  2005-04-16  2380  }
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds  2005-04-16  2381  
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds  2005-04-16  2382  /**
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds  2005-04-16  2383   *	ata_msense_rw_recovery - Simulate MODE SENSE r/w error recovery page

:::::: The code at line 2375 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6288 bytes --]

Re: [PATCH 14/14] libata-scsi: Set information sense field for invalid parameter

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3808 bytes --]

Hi Hannes,

[auto build test WARNING on tj-libata/for-next]
[also build test WARNING on v4.6-rc2 next-20160404]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Hannes-Reinecke/libata-SATL-update/20160404-174817
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tj/libata for-next
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   drivers/ata/libata-scsi.c:2414: warning: No description found for parameter 'dev'
>> drivers/ata/libata-scsi.c:3326: warning: No description found for parameter 'fp'
   drivers/ata/libata-scsi.c:3382: warning: No description found for parameter 'fp'
   drivers/ata/libata-scsi.c:2414: warning: No description found for parameter 'dev'
>> drivers/ata/libata-scsi.c:3326: warning: No description found for parameter 'fp'
   drivers/ata/libata-scsi.c:3382: warning: No description found for parameter 'fp'

vim +/fp +3326 drivers/ata/libata-scsi.c

18f0f978 drivers/ata/libata-scsi.c  Christoph Hellwig 2009-11-17  3310  	return 1;
18f0f978 drivers/ata/libata-scsi.c  Christoph Hellwig 2009-11-17  3311  }
18f0f978 drivers/ata/libata-scsi.c  Christoph Hellwig 2009-11-17  3312  
^1da177e drivers/scsi/libata-scsi.c Linus Torvalds    2005-04-16  3313  /**
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3314   *	ata_mselect_caching - Simulate MODE SELECT for caching info page
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3315   *	@qc: Storage for translated ATA taskfile
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3316   *	@buf: input buffer
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3317   *	@len: number of valid bytes in the input buffer
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3318   *
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3319   *	Prepare a taskfile to modify caching information for the device.
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3320   *
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3321   *	LOCKING:
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3322   *	None.
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3323   */
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3324  static int ata_mselect_caching(struct ata_queued_cmd *qc,
c3f51c2f drivers/ata/libata-scsi.c  Hannes Reinecke   2016-04-04  3325  			       const u8 *buf, int len, u16 *fp)
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05 @3326  {
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3327  	struct ata_taskfile *tf = &qc->tf;
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3328  	struct ata_device *dev = qc->dev;
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3329  	char mpage[CACHE_MPAGE_LEN];
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3330  	u8 wce;
c3f51c2f drivers/ata/libata-scsi.c  Hannes Reinecke   2016-04-04  3331  	int i;
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3332  
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3333  	/*
1b26d29c drivers/ata/libata-scsi.c  Paolo Bonzini     2012-07-05  3334  	 * The first two bytes of def_cache_mpage are a header, so offsets

:::::: The code at line 3326 was first introduced by commit
:::::: 1b26d29ccd592ea585c7cc291384184c5568da92 [libata] scsi: implement MODE SELECT command

:::::: TO: Paolo Bonzini <pbonzini@redhat.com>
:::::: CC: Jeff Garzik <jgarzik@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6288 bytes --]

Re: [PATCH 00/14] libata: SATL update

[Tejun Heo]

Hello, Hannes.

Applied the series to libata/for-4.7-zac with cosmetic updates
((s64)-1 -> UINT64_MAX, kbuild warning fixes and other formatting
updates).

Thanks.

-- 
tejun

Re: [PATCH 03/14] libata-scsi: sanitize ata_gen_ata_sense()

[Tejun Heo]

On Mon, Apr 04, 2016 at 02:26:51PM +0300, Sergei Shtylyov wrote:
> >+	} else {
> >+		/* Could not decode error */
> >+		ata_dev_warn(dev, "could not decode error status 0x%x err_mask 0x%x\n",
> 
>    "%#x" is equivalent and takes up less space.

Oops, gmail for some reason put Sergei's messages into spam folder.
Hannes, can you please generate incremental patches for Sergei's
comments?

Thanks.

-- 
tejun

Re: [PATCH 03/14] libata-scsi: sanitize ata_gen_ata_sense()

[Sergei Shtylyov]

On 04/04/2016 07:22 PM, Tejun Heo wrote:

>>> +	} else {
>>> +		/* Could not decode error */
>>> +		ata_dev_warn(dev, "could not decode error status 0x%x err_mask 0x%x\n",
>>
>>     "%#x" is equivalent and takes up less space.

> Oops, gmail for some reason put Sergei's messages into spam folder.

    My (dynamic) IP was even blacklisted by the Freenode's IRC servers not so 
long ago. :-)

> Hannes, can you please generate incremental patches for Sergei's
> comments?

    Don't think it's worth it. Perhaps only the comment typos...

> Thanks.

MBR, Sergei

Re: [PATCH 00/14] libata: SATL update

[Hannes Reinecke]

On 04/04/2016 05:46 PM, Tejun Heo wrote:
> Hello, Hannes.
> 
> Applied the series to libata/for-4.7-zac with cosmetic updates
> ((s64)-1 -> UINT64_MAX, kbuild warning fixes and other formatting
> updates).
> 
Tejun, you are my hero :-)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html