On 8/2/25 05:28, Igor Pylypiv wrote:
> 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.
Yep, we can remove the err_mask check.
--
Damien Le Moal
Western Digital Research
