On Wed, Jul 30, 2025 at 09:24:41AM +0900, Damien Le Moal wrote:
> ata_gen_ata_sense() is always called for a failed qc missing sense data
> so that a sense key, code and code qualifier can be generated using
> ata_to_sense_error() from the qc status and error fields of its result
> task file. However, if the qc does not have its result task file filled,
> ata_gen_ata_sense() returns early without setting a sense key.
>
> Improve this by defaulting to returning ABORTED COMMAND without any
> additional sense code, since we do not know the reason for the failure.
> The same fix is also applied in ata_gen_passthru_sense() with the
> additional check that the qc failed (qc->err_mask is set).
>
> Fixes: 816be86c7993 ("ata: libata-scsi: Check ATA_QCFLAG_RTF_FILLED before using result_tf")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
> ---
> drivers/ata/libata-scsi.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 9b16c0f553e0..57f674f51b0c 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -938,6 +938,8 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
> if (!(qc->flags & ATA_QCFLAG_RTF_FILLED)) {
> ata_dev_dbg(dev,
> "missing result TF: can't generate ATA PT sense data\n");
> + if (qc->err_mask)
> + ata_scsi_set_sense(dev, cmd, ABORTED_COMMAND, 0, 0);
> return;
> }
>
> @@ -992,8 +994,8 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
>
> if (!(qc->flags & ATA_QCFLAG_RTF_FILLED)) {
> ata_dev_dbg(dev,
> - "missing result TF: can't generate sense data\n");
> - return;
> + "Missing result TF: reporting aborted command\n");
> + goto aborted;
> }
>
> /* Use ata_to_sense_error() to map status register bits
> @@ -1004,13 +1006,15 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
There is a redundant check in ata_gen_ata_sense(). qc->err_mask (is_error) is
already checked in ata_scsi_qc_complete() before it calls ata_gen_ata_sense().
if (qc->err_mask ||
tf->status & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ)) {
The function will be much cleaner once we remove this check.
> ata_to_sense_error(tf->status, tf->error,
> &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->status, qc->err_mask);
> - ata_scsi_set_sense(dev, cmd, ABORTED_COMMAND, 0, 0);
> return;
> }
> +
> + /* Could not decode error */
> + ata_dev_warn(dev,
> + "Could not decode error 0x%x, status 0x%x (err_mask=0x%x)\n",
> + tf->error, tf->status, qc->err_mask);
> +aborted:
> + ata_scsi_set_sense(dev, cmd, ABORTED_COMMAND, 0, 0);
> }
>
> void ata_scsi_sdev_config(struct scsi_device *sdev)
> --
> 2.50.1
>
>
