mbox series

[0/5] fetch sense data for ATA devices supporting sense reporting

Message ID 20220926205257.601750-1-Niklas.Cassel@wdc.com
Headers show
Series fetch sense data for ATA devices supporting sense reporting | expand

Message

Niklas Cassel Sept. 26, 2022, 8:53 p.m. UTC
From: Niklas Cassel <niklas.cassel@wdc.com>

NOTE TO MAINTAINERS:
Perhaps the SCSI patch could go into both the scsi tree and the libata
tree to avoid any merge conflicts during the merge window.


Hello,

Currently, the sense data reporting feature set is enabled for all ATA
devices which supports the feature set (ata_id_has_sense_reporting()),
see ata_dev_config_sense_reporting().

However, even if sense data reporting is enabled, and the device
indicates that sense data is available, the sense data is only fetched
for ATA ZAC devices. For regular ATA devices, the available sense data
is never fetched, it is simply ignored. Instead, libata will use the
ERROR + STATUS fields and map them to a very generic and reduced set
of sense data, see ata_gen_ata_sense() and ata_to_sense_error().

When sense data reporting was first implemented, regular ATA devices
did fetch the sense data from the device. However, this was restricted
to only ATA ZAC devices in commit ca156e006add ("libata: don't request
sense data on !ZAC ATA devices").

With the recent fixes surrounding sense data and NCQ autosense:
https://lore.kernel.org/linux-ide/20220916122838.1190628-1-niklas.cassel@wdc.com/T/#u
together with the patches in this series, we want to, once again,
fetch the sense data for all ATA devices supporting sense reporting.
ata_gen_ata_sense() should only be used for devices that don't support
the sense data reporting feature set.

If we encounter a device that is misbehaving because the sense data is
actually fetched, then that device should be quirked such that it
never enables the sense data reporting feature set in the first place,
since such a device is obviously not compliant with the specification.


Kind regards,
Niklas

Damien Le Moal (1):
  scsi: Define the COMPLETED sense key

Niklas Cassel (4):
  ata: libata: fix NCQ autosense logic
  ata: libata: clarify when ata_eh_request_sense() will be called
  ata: libata: only set sense valid flag if sense data is valid
  ata: libata: fetch sense data for ATA devices supporting sense
    reporting

 drivers/ata/libata-eh.c   | 18 +++++++++++++-----
 drivers/ata/libata-sata.c | 21 ++++++++++++++-------
 drivers/ata/libata-scsi.c | 16 ++++++++++++++++
 drivers/ata/libata.h      |  1 +
 include/scsi/scsi_proto.h |  4 ++--
 5 files changed, 46 insertions(+), 14 deletions(-)

Comments

Damien Le Moal Oct. 17, 2022, 2:39 a.m. UTC | #1
On 9/27/22 05:53, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> NOTE TO MAINTAINERS:
> Perhaps the SCSI patch could go into both the scsi tree and the libata
> tree to avoid any merge conflicts during the merge window.
> 
> 
> Hello,
> 
> Currently, the sense data reporting feature set is enabled for all ATA
> devices which supports the feature set (ata_id_has_sense_reporting()),
> see ata_dev_config_sense_reporting().
> 
> However, even if sense data reporting is enabled, and the device
> indicates that sense data is available, the sense data is only fetched
> for ATA ZAC devices. For regular ATA devices, the available sense data
> is never fetched, it is simply ignored. Instead, libata will use the
> ERROR + STATUS fields and map them to a very generic and reduced set
> of sense data, see ata_gen_ata_sense() and ata_to_sense_error().
> 
> When sense data reporting was first implemented, regular ATA devices
> did fetch the sense data from the device. However, this was restricted
> to only ATA ZAC devices in commit ca156e006add ("libata: don't request
> sense data on !ZAC ATA devices").
> 
> With the recent fixes surrounding sense data and NCQ autosense:
> https://lore.kernel.org/linux-ide/20220916122838.1190628-1-niklas.cassel@wdc.com/T/#u
> together with the patches in this series, we want to, once again,
> fetch the sense data for all ATA devices supporting sense reporting.
> ata_gen_ata_sense() should only be used for devices that don't support
> the sense data reporting feature set.
> 
> If we encounter a device that is misbehaving because the sense data is
> actually fetched, then that device should be quirked such that it
> never enables the sense data reporting feature set in the first place,
> since such a device is obviously not compliant with the specification.

Applied to for-6.2. Thanks !

> 
> 
> Kind regards,
> Niklas
> 
> Damien Le Moal (1):
>   scsi: Define the COMPLETED sense key
> 
> Niklas Cassel (4):
>   ata: libata: fix NCQ autosense logic
>   ata: libata: clarify when ata_eh_request_sense() will be called
>   ata: libata: only set sense valid flag if sense data is valid
>   ata: libata: fetch sense data for ATA devices supporting sense
>     reporting
> 
>  drivers/ata/libata-eh.c   | 18 +++++++++++++-----
>  drivers/ata/libata-sata.c | 21 ++++++++++++++-------
>  drivers/ata/libata-scsi.c | 16 ++++++++++++++++
>  drivers/ata/libata.h      |  1 +
>  include/scsi/scsi_proto.h |  4 ++--
>  5 files changed, 46 insertions(+), 14 deletions(-)
>