[v2,0/9] libata cleanups and improvements

The first two patches of this series fix potential NULL pointer
dereference problems.
The following three patches cleanup libata-core code in the area of
device configuration (ata_dev_configure() function).
Patch r64 improves ata_read_log_page() to avoid unnecessary warning
messages and patch 7 adds an informational message on device scan to
advertize the features supported by a device.

Path 8 adds the new sysfs ahci device attribute ncq_prio_supported to
indicate that a disk supports NCQ priority. Patch 9 does the same for
the mpt3sas driver, adding the sas_ncq_prio_supported device attribute.

Changes from v1:
* Added patch 1 and 2 to fix problems reported by the kernel test robot
* Use strscpy() instead of strcpy in patch 4
* Use sysfs_emit in patch 8 and 9 as suggested by Bart
* Fix typos in comments of the new sas_ncq_prio_supported attribute in
  patch 9

Damien Le Moal (9):
  libata: fix ata_host_alloc_pinfo()
  libata: fix ata_host_start()
  libata: cleanup device sleep capability detection
  libata: cleanup ata_dev_configure()
  libata: cleanup NCQ priority handling
  libata: fix ata_read_log_page() warning
  libata: print feature list on device scan
  libahci: Introduce ncq_prio_supported sysfs sttribute
  scsi: mpt3sas: Introduce sas_ncq_prio_supported sysfs sttribute

 drivers/ata/libahci.c              |   1 +
 drivers/ata/libata-core.c          | 253 +++++++++++++++--------------
 drivers/ata/libata-sata.c          |  61 ++++---
 drivers/scsi/mpt3sas/mpt3sas_ctl.c |  19 +++
 include/linux/libata.h             |   5 +
 5 files changed, 192 insertions(+), 147 deletions(-)

On 8/6/21 9:42 AM, Damien Le Moal wrote:
> Avoid a potential NULL pointer dereference by testing that the ATA port
> info variable "pi".
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  drivers/ata/libata-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 61c762961ca8..ea8b91297f12 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -5458,7 +5458,7 @@ struct ata_host *ata_host_alloc_pinfo(struct device *dev,
>  		ap->link.flags |= pi->link_flags;
>  		ap->ops = pi->port_ops;
>  
> -		if (!host->ops && (pi->port_ops != &ata_dummy_port_ops))
> +		if (!host->ops && pi && pi->port_ops != &ata_dummy_port_ops)
>  			host->ops = pi->port_ops;
>  	}
>  
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes

On 8/6/21 9:42 AM, Damien Le Moal wrote:
> Move the code to retrieve the device sleep capability and timings out of
> ata_dev_configure() into the helper function ata_dev_config_devslp().
> While at it, mark the device as supporting the device sleep capability
> only if the sata settings page was retrieved successfully to ensure that
> the timing information is correctly initialized.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  drivers/ata/libata-core.c | 55 +++++++++++++++++++++++----------------
>  1 file changed, 32 insertions(+), 23 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes

On 2021/08/06 18:12, Hannes Reinecke wrote:
> On 8/6/21 9:42 AM, Damien Le Moal wrote:
>> Similarly to AHCI, introduce the device sysfs attribute
>> sas_ncq_prio_supported to advertize if a SATA device supports the NCQ
>> priority feature. Without this new attribute, the user can only
>> discover if a SATA device supports NCQ priority by trying to enable
>> the feature use with the sas_ncq_prio_enable sysfs device attribute,
>> which fails when the device does not support high priroity commands.
>>
>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
>> ---
>>  drivers/scsi/mpt3sas/mpt3sas_ctl.c | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
>> index b66140e4c370..f83d4d32d459 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
>> @@ -3918,6 +3918,24 @@ sas_device_handle_show(struct device *dev, struct device_attribute *attr,
>>  }
>>  static DEVICE_ATTR_RO(sas_device_handle);
>>  
>> +/**
>> + * sas_ncq_prio_supported_show - Indicate if device supports NCQ priority
>> + * @dev: pointer to embedded device
>> + * @attr: sas_ncq_prio_supported attribute descriptor
>> + * @buf: the buffer returned
>> + *
>> + * A sysfs 'read-only' sdev attribute, only works with SATA
>> + */
>> +static ssize_t
>> +sas_ncq_prio_supported_show(struct device *dev,
>> +			    struct device_attribute *attr, char *buf)
>> +{
>> +	struct scsi_device *sdev = to_scsi_device(dev);
>> +
>> +	return sysfs_emit(buf, "%d\n", scsih_ncq_prio_supp(sdev));
>> +}
>> +static DEVICE_ATTR_RO(sas_ncq_prio_supported);
>> +
>>  /**
>>   * sas_ncq_prio_enable_show - send prioritized io commands to device
>>   * @dev: pointer to embedded device
>> @@ -3960,6 +3978,7 @@ static DEVICE_ATTR_RW(sas_ncq_prio_enable);
>>  struct device_attribute *mpt3sas_dev_attrs[] = {
>>  	&dev_attr_sas_address,
>>  	&dev_attr_sas_device_handle,
>> +	&dev_attr_sas_ncq_prio_supported,
>>  	&dev_attr_sas_ncq_prio_enable,
>>  	NULL,
>>  };
>>
> One wonders where the relationship to the previous patches are, but hey:

Yes, hard to see... mpt3sas is the only SAS HBA that can actually carry the
"high prio" bit for the command to the HBA FW SATL using the prio bit of the SAS
frame for the SCSI command that is to be translated into a high prio SATA command.

This driver supports NCQ priority since the first support series in 4.10 kernel.
So I added the same change as for libahci to keep it in sync with the pure ATA
side of things.

But sure, it could be a patch on its own outside this series.

> 
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> 
> Cheers,
> 
> Hannes
>