Re: [PATCH] ata: libata-sata: Use LBA from sense data descriptor

[Damien Le Moal <dlemoal@kernel.org>]

On 4/8/25 6:28 PM, Niklas Cassel wrote:
> The definition of the LBA field in the sense data descriptor is:
> 
> "If definition of the sense data to be returned when a command completes
> without an error includes an LBA value, then the LBA field contains the
> defined value. Otherwise, the LBA field contains a copy of the LBA field
> in the command inputs for the command that completed without an error
> and returned sense data."
> 
> Thus, the LBA field in the sense data descriptor can contain a LBA value
> that is different from the LBA field in the command input.
> 
> Therefore, just like how ata_eh_read_log_10h() overrides qc->result_tf
> with the LBA in the NCQ Command Error log, override qc->result_tf with
> the LBA in the Successful NCQ Commands log.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Looks OK to me. Do we want a fixes tag on this one though ? Technically, this
is a bug, but no-one has stumbled on it...

> ---
>  drivers/ata/libata-sata.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
> index ba300cc0a3a3..c21fdacd0777 100644
> --- a/drivers/ata/libata-sata.c
> +++ b/drivers/ata/libata-sata.c
> @@ -1556,6 +1556,14 @@ int ata_eh_get_ncq_success_sense(struct ata_link *link)
>  			continue;
>  		}
>  
> +		/* LBA in sense data desc can be different from LBA in qc->tf */
> +		qc->result_tf.lbal = sense[8];
> +		qc->result_tf.lbam = sense[9];
> +		qc->result_tf.lbah = sense[10];
> +		qc->result_tf.hob_lbal = sense[11];
> +		qc->result_tf.hob_lbam = sense[12];
> +		qc->result_tf.hob_lbah = sense[13];
> +
>  		/* Set sense without also setting scsicmd->result */
>  		scsi_build_sense_buffer(dev->flags & ATA_DFLAG_D_SENSE,
>  					qc->scsicmd->sense_buffer, sk,


-- 
Damien Le Moal
Western Digital Research