On 02 February, 2022 - Damien Le Moal wrote:
> 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@xxxxxxxxxxxxxxx # v5.10+
> > Signed-off-by: Anton Lundin <glance@xxxxxxxxxx>
> > 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())
Non of those code paths are called for this device, probably due to some
other flag disqualifying them.
> 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.
That was my first thought but then I found ATA_HORKAGE_NO_ID_DEV_LOG
which was in the calling path that actually triggered this issue.
But, yes, I totally agree that's a more solid solution preventing this
kind of issue to crop up again.
> > +
> > /* End Marker */
> > { }
> > };
>
> Note: if you need this fix sent to linux-stable, add "Cc: stable@..."
> and a Fixes tag.
I'd think it's fitting to send it to linux-stable, because it prevents
those DOM's from working in v5.15.5+.
Ok. I must have missed that part when I read submitting-patches doc.
I'll rework and re-submit the patch.
//Anton
