Commit 11533932 authored by Igor Pylypiv's avatar Igor Pylypiv Committed by Damien Le Moal
Browse files

ata: libata-scsi: Do not set the INFORMATION field twice for ATA PT 



For ATA PASS-THROUGH + fixed format sense data + NCQ autosense
the INFORMATION sense data field is being written twice:

- 1st write: (redundant)
scsi_set_sense_information() sets the INFORMATION field to ATA LBA.
This is incorrect for ATA PASS-THROUGH.

- 2nd write: (correct)
ata_scsi_set_passthru_sense_fields() sets the INFORMATION field
to ATA ERROR/STATUS/DEVICE/COUNT(7:0) as per SAT spec.

There is no user-visible issue because second write overwrites
the incorrect data from the first write.

This patch eliminates the reduntant write by moving the INFORMATION
sense data field population logic to ata_scsi_qc_complete().

Signed-off-by: default avatarIgor Pylypiv <ipylypiv@google.com>
Reviewed-by: default avatarNiklas Cassel <cassel@kernel.org>
Signed-off-by: default avatarDamien Le Moal <dlemoal@kernel.org>
parent 23a8e0df

drivers/ata/libata-sata.c

+0 −2
Original line number Diff line number Diff line
@@ -1659,8 +1659,6 @@ void ata_eh_analyze_ncq_error(struct ata_link *link) 
		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_information(dev, qc->scsicmd,

						       &qc->result_tf);

			qc->flags |= ATA_QCFLAG_SENSE_VALID;

		}

	}

drivers/ata/libata-scsi.c

+14 −17
Original line number Diff line number Diff line
@@ -216,17 +216,21 @@ void ata_scsi_set_sense(struct ata_device *dev, struct scsi_cmnd *cmd, 
	scsi_build_sense(cmd, d_sense, sk, asc, ascq);

}



void ata_scsi_set_sense_information(struct ata_device *dev,

				    struct scsi_cmnd *cmd,

				    const struct ata_taskfile *tf)

static void ata_scsi_set_sense_information(struct ata_queued_cmd *qc)

{

	u64 information;



	information = ata_tf_read_block(tf, dev);

	if (!(qc->flags & ATA_QCFLAG_RTF_FILLED)) {

		ata_dev_dbg(qc->dev,

			    "missing result TF: can't set INFORMATION sense field\n");

		return;

	}



	information = ata_tf_read_block(&qc->result_tf, qc->dev);

	if (information == U64_MAX)

		return;



	scsi_set_sense_information(cmd->sense_buffer,

	scsi_set_sense_information(qc->scsicmd->sense_buffer,

				   SCSI_SENSE_BUFFERSIZE, information);

}
@@ -971,8 +975,7 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc) 
 *	ata_gen_ata_sense - generate a SCSI fixed sense block

 *	@qc: Command that we are erroring out

 *

 *	Generate sense block for a failed ATA command @qc.  Descriptor

 *	format is used to accommodate LBA48 block address.

 *	Generate sense block for a failed ATA command @qc.

 *

 *	LOCKING:

 *	None.
@@ -982,8 +985,6 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc) 
	struct ata_device *dev = qc->dev;

	struct scsi_cmnd *cmd = qc->scsicmd;

	struct ata_taskfile *tf = &qc->result_tf;

	unsigned char *sb = cmd->sense_buffer;

	u64 block;

	u8 sense_key, asc, ascq;



	if (ata_dev_disabled(dev)) {
@@ -1014,12 +1015,6 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc) 
		ata_scsi_set_sense(dev, cmd, ABORTED_COMMAND, 0, 0);

		return;

	}



	block = ata_tf_read_block(&qc->result_tf, dev);

	if (block == U64_MAX)

		return;



	scsi_set_sense_information(sb, SCSI_SENSE_BUFFERSIZE, block);

}



void ata_scsi_sdev_config(struct scsi_device *sdev)
@@ -1679,8 +1674,10 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd *qc) 
		ata_scsi_set_passthru_sense_fields(qc);

		if (is_ck_cond_request)

			set_status_byte(qc->scsicmd, SAM_STAT_CHECK_CONDITION);

	} else if (is_error && !have_sense) {

	} else if (is_error) {

		if (!have_sense)

			ata_gen_ata_sense(qc);

		ata_scsi_set_sense_information(qc);

	}



	ata_qc_done(qc);

drivers/ata/libata.h

+0 −3
Original line number Diff line number Diff line
@@ -141,9 +141,6 @@ 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_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_scsi_dev_rescan(struct work_struct *work);