Re: [PATCH] Revert "ata: libata-scsi: Honor the D_SENSE bit for CK_COND=1 and no error"

Hannes Reinecke
2024-08-13T14:53:20Z

On 8/13/24 15:19, Niklas Cassel wrote:
> This reverts commit 28ab9769117ca944cb6eb537af5599aa436287a4.
>
> Sense data can be in either fixed format or descriptor format.
>
> SAT-6 revision 1, "10.4.6 Control mode page", defines the D_SENSE bit:
> "The SATL shall support this bit as defined in SPC-5 with the following
> exception: if the D_ SENSE bit is set to zero (i.e., fixed format sense
> data), then the SATL should return fixed format sense data for ATA
> PASS-THROUGH commands."
>
> The libata SATL has always kept D_SENSE set to zero by default. (It is
> however possible to change the value using a MODE SELECT SG_IO command.)
>
> Failed ATA PASS-THROUGH commands correctly respected the D_SENSE bit,
> however, successful ATA PASS-THROUGH commands incorrectly returned the
> sense data in descriptor format (regardless of the D_SENSE bit).
>
> Commit 28ab9769117c ("ata: libata-scsi: Honor the D_SENSE bit for
> CK_COND=1 and no error") fixed this bug for successful ATA PASS-THROUGH
> commands.
>
> However, after commit 28ab9769117c ("ata: libata-scsi: Honor the D_SENSE
> bit for CK_COND=1 and no error"), there were bug reports that hdparm,
> hddtemp, and udisks were no longer working as expected.
>
> These applications incorrectly assume the returned sense data is in
> descriptor format, without even looking at the RESPONSE CODE field in the
> returned sense data (to see which format the returned sense data is in).
>
> Considering that there will be broken versions of these applications around
> roughly forever, we are stuck with being bug compatible with older kernels.
>
> Cc: stable@vger.kernel.org # 4.19+
> Reported-by: Stephan Eisvogel <eisvogel@seitics.de>
> Reported-by: Christian Heusel <christian@heusel.eu>
> Closes: https://lore.kernel.org/linux-ide/0bf3f2f0-0fc6-4ba5-a420-c0874ef82d64@heusel.eu/
> Fixes: 28ab9769117c ("ata: libata-scsi: Honor the D_SENSE bit for CK_COND=1 and no error")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
> drivers/ata/libata-scsi.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Ivo Totev, Andrew McDonald,
Werner Knoblich