Re: [PATCH] libata: Don't issue ATA_LOG_DIRECTORY to SATADOM-ML 3ME

[Damien Le Moal <damien.lemoal@opensource.wdc.com>]

On 2/2/22 19:05, Anton Lundin wrote:
> Back in 06f6c4c6c3e8 ("ata: libata: add missing ata_identify_page_supported() calls")
> a read of ATA_LOG_DIRECTORY page was added. This caused the
> SATADOM-ML 3ME to lock up.
> 
> In 636f6e2af4fb ("libata: add horkage for missing Identify Device log")
> a flag was added to cache if a device supports this or not.
> 
> This adds a blacklist entry which flags that these devices doesn't
> support that call and shouldn't be issued that call.
> 
> Cc: stable@vger.kernel.org # v5.10+
> Signed-off-by: Anton Lundin <glance@acc.umu.se>
> Depends-on: 636f6e2af4fb ("libata: add horkage for missing Identify Device log")

I do not think so. See below.

> ---
>  drivers/ata/libata-core.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 87d36b29ca5f..e024af9f33d0 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -4070,6 +4070,13 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
>  	{ "WDC WD3000JD-*",		NULL,	ATA_HORKAGE_WD_BROKEN_LPM },
>  	{ "WDC WD3200JD-*",		NULL,	ATA_HORKAGE_WD_BROKEN_LPM },
>  
> +	/*
> +	 * This sata dom goes on a walkabout when it sees the
> +	 * ATA_LOG_DIRECTORY read request so ensure we don't issue such a
> +	 * request to these devices.
> +	 */
> +	{ "SATADOM-ML 3ME",		NULL,	ATA_HORKAGE_NO_ID_DEV_LOG },

This flag only disables trying to access the identify device log page,
it does *not* avoid access to the log directory log page in general. The
log directory will still be consulted for other log pages beside the
identify device log page, from any function that calls
ata_log_supported() (e.g. ata_dev_config_ncq_send_recv() and
ata_dev_config_ncq_non_data())

So it will be a lot more solid to define a ATA_HORKAGE_NO_LOG_DIR flag
and test for it in ata_log_supported(), completely preventing any access
to the log directory page for this drive type.

> +
>  	/* End Marker */
>  	{ }
>  };

Note: if you need this fix sent to linux-stable, add "Cc: stable@..."
and a Fixes tag.

-- 
Damien Le Moal
Western Digital Research