[PATCH] ata: libata-core: Fix NO_FUA quirks shadowed by earlier quirk entries

[PATCH] ata: libata-core: Fix NO_FUA quirks shadowed by earlier quirk entries

[Niklas Cassel]

ata_dev_quirks() uses the first entry in __ata_dev_quirks[] that
matches the device and ignores all later entries. Commit 38d431229bfb
("ata: libata: exclude FUA support for known buggy drives") appended
three ATA_QUIRK_NO_FUA entries to the end of the table, with model
patterns meant to cover whole product lines. Earlier entries already
match some or all of these drives, so NO_FUA is not always applied:

- "INTEL*SSDSC2CT*" never takes effect. Every model it matches also
  matches the earlier "INTEL*SSD*" entry, so these drives only get
  ATA_QUIRK_ZERO_AFTER_TRIM.

- "WDC*WD2500J*" does not take effect for "WDC WD2500JD-*" drives,
  which match the earlier ATA_QUIRK_WD_BROKEN_LPM entry.

- "OCZ-VERTEX*" does not take effect for "OCZ-VERTEX" drives with
  firmware "1.30", which match the earlier ATA_QUIRK_BROKEN_HPA entry.

Move the Intel entry in front of "INTEL*SSD*" and keep
ATA_QUIRK_ZERO_AFTER_TRIM in it, so these drives stay on the
ZERO_AFTER_TRIM whitelist. Add ATA_QUIRK_NO_FUA to the WD2500JD and
OCZ Vertex 1.30 entries. The quirks of all other drives are unchanged.

libata only uses FUA when it is enabled with libata.fua=1, so systems
using the default setting are not affected.

Fixes: 38d431229bfb ("ata: libata: exclude FUA support for known buggy drives")
Assisted-by: LLM
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/ata/libata-core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 92233fb25051..cff4ededafad 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4323,7 +4323,8 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
 	{ "MAXTOR 6L080L4",	"A93.0500",	ATA_QUIRK_BROKEN_HPA },
 
 	/* this one allows HPA unlocking but fails IOs on the area */
-	{ "OCZ-VERTEX",		    "1.30",	ATA_QUIRK_BROKEN_HPA },
+	{ "OCZ-VERTEX",		    "1.30",	ATA_QUIRK_BROKEN_HPA |
+						ATA_QUIRK_NO_FUA },
 
 	/* Devices which report 1 sector over size HPA */
 	{ "ST340823A",		NULL,		ATA_QUIRK_HPA_SIZE },
@@ -4459,6 +4460,8 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
 	{ "Crucial*",			NULL,	ATA_QUIRK_ZERO_AFTER_TRIM },
 	{ "INTEL SSDSC2KG480G8", "XCV10120",	ATA_QUIRK_ZERO_AFTER_TRIM |
 						ATA_QUIRK_MAX_SEC },
+	{ "INTEL*SSDSC2CT*",		NULL,	ATA_QUIRK_ZERO_AFTER_TRIM |
+						ATA_QUIRK_NO_FUA },
 	{ "INTEL*SSD*",			NULL,	ATA_QUIRK_ZERO_AFTER_TRIM },
 	{ "SSD*INTEL*",			NULL,	ATA_QUIRK_ZERO_AFTER_TRIM },
 	{ "Samsung*SSD*",		NULL,	ATA_QUIRK_ZERO_AFTER_TRIM },
@@ -4479,7 +4482,8 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
 	{ "WDC WD1200JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM },
 	{ "WDC WD1600JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM },
 	{ "WDC WD2000JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM },
-	{ "WDC WD2500JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM },
+	{ "WDC WD2500JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM |
+						ATA_QUIRK_NO_FUA },
 	{ "WDC WD3000JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM },
 	{ "WDC WD3200JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM },
 
@@ -4504,7 +4508,6 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
 	{ "Maxtor",		"BANC1G10",	ATA_QUIRK_NO_FUA },
 	{ "WDC*WD2500J*",	NULL,		ATA_QUIRK_NO_FUA },
 	{ "OCZ-VERTEX*",	NULL,		ATA_QUIRK_NO_FUA },
-	{ "INTEL*SSDSC2CT*",	NULL,		ATA_QUIRK_NO_FUA },
 
 	/* End Marker */
 	{ }
-- 
2.56.0

Re: [PATCH] ata: libata-core: Fix NO_FUA quirks shadowed by earlier quirk entries

[Damien Le Moal]

On 2026/10/13 21:39, Niklas Cassel wrote:
> ata_dev_quirks() uses the first entry in __ata_dev_quirks[] that
> matches the device and ignores all later entries. Commit 38d431229bfb
> ("ata: libata: exclude FUA support for known buggy drives") appended
> three ATA_QUIRK_NO_FUA entries to the end of the table, with model
> patterns meant to cover whole product lines. Earlier entries already
> match some or all of these drives, so NO_FUA is not always applied:
> 
> - "INTEL*SSDSC2CT*" never takes effect. Every model it matches also
>   matches the earlier "INTEL*SSD*" entry, so these drives only get
>   ATA_QUIRK_ZERO_AFTER_TRIM.
> 
> - "WDC*WD2500J*" does not take effect for "WDC WD2500JD-*" drives,
>   which match the earlier ATA_QUIRK_WD_BROKEN_LPM entry.
> 
> - "OCZ-VERTEX*" does not take effect for "OCZ-VERTEX" drives with
>   firmware "1.30", which match the earlier ATA_QUIRK_BROKEN_HPA entry.
> 
> Move the Intel entry in front of "INTEL*SSD*" and keep
> ATA_QUIRK_ZERO_AFTER_TRIM in it, so these drives stay on the
> ZERO_AFTER_TRIM whitelist. Add ATA_QUIRK_NO_FUA to the WD2500JD and
> OCZ Vertex 1.30 entries. The quirks of all other drives are unchanged.
> 
> libata only uses FUA when it is enabled with libata.fua=1, so systems
> using the default setting are not affected.
> 
> Fixes: 38d431229bfb ("ata: libata: exclude FUA support for known buggy drives")
> Assisted-by: LLM
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Your solution works, but I think it is very fragile. We will most likely again
make the same mistake with the oredering. So what about something like this:

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 92233fb25051..980a6a6256b1 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4515,6 +4515,7 @@ static u64 ata_dev_quirks(const struct ata_device *dev)
        unsigned char model_num[ATA_ID_PROD_LEN + 1];
        unsigned char model_rev[ATA_ID_FW_REV_LEN + 1];
        const struct ata_dev_quirks_entry *ad = __ata_dev_quirks;
+       u64 quirks = 0;

        /* dev->quirks is an u64. */
        BUILD_BUG_ON(__ATA_QUIRK_MAX > 64);
@@ -4527,11 +4528,11 @@ static u64 ata_dev_quirks(const struct ata_device *dev)
                    (!ad->model_rev || glob_match(ad->model_rev, model_rev))) {
                        ata_dev_print_quirks(dev, model_num, model_rev,
                                             ad->quirks);
-                       return ad->quirks;
+                       quirks |= ad->quirks;
                }
                ad++;
        }
-       return 0;
+       return quirks;
 }

 static u64 ata_dev_get_max_sec_quirk_value(struct ata_device *dev)

With this, all quirks that apply to a device will apply, even if there are
multiple matching entries. That will solve the issue and make things a lot more
solid.

> ---
>  drivers/ata/libata-core.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 92233fb25051..cff4ededafad 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -4323,7 +4323,8 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
>  	{ "MAXTOR 6L080L4",	"A93.0500",	ATA_QUIRK_BROKEN_HPA },
>  
>  	/* this one allows HPA unlocking but fails IOs on the area */
> -	{ "OCZ-VERTEX",		    "1.30",	ATA_QUIRK_BROKEN_HPA },
> +	{ "OCZ-VERTEX",		    "1.30",	ATA_QUIRK_BROKEN_HPA |
> +						ATA_QUIRK_NO_FUA },
>  
>  	/* Devices which report 1 sector over size HPA */
>  	{ "ST340823A",		NULL,		ATA_QUIRK_HPA_SIZE },
> @@ -4459,6 +4460,8 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
>  	{ "Crucial*",			NULL,	ATA_QUIRK_ZERO_AFTER_TRIM },
>  	{ "INTEL SSDSC2KG480G8", "XCV10120",	ATA_QUIRK_ZERO_AFTER_TRIM |
>  						ATA_QUIRK_MAX_SEC },
> +	{ "INTEL*SSDSC2CT*",		NULL,	ATA_QUIRK_ZERO_AFTER_TRIM |
> +						ATA_QUIRK_NO_FUA },
>  	{ "INTEL*SSD*",			NULL,	ATA_QUIRK_ZERO_AFTER_TRIM },
>  	{ "SSD*INTEL*",			NULL,	ATA_QUIRK_ZERO_AFTER_TRIM },
>  	{ "Samsung*SSD*",		NULL,	ATA_QUIRK_ZERO_AFTER_TRIM },
> @@ -4479,7 +4482,8 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
>  	{ "WDC WD1200JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM },
>  	{ "WDC WD1600JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM },
>  	{ "WDC WD2000JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM },
> -	{ "WDC WD2500JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM },
> +	{ "WDC WD2500JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM |
> +						ATA_QUIRK_NO_FUA },
>  	{ "WDC WD3000JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM },
>  	{ "WDC WD3200JD-*",		NULL,	ATA_QUIRK_WD_BROKEN_LPM },
>  
> @@ -4504,7 +4508,6 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
>  	{ "Maxtor",		"BANC1G10",	ATA_QUIRK_NO_FUA },
>  	{ "WDC*WD2500J*",	NULL,		ATA_QUIRK_NO_FUA },
>  	{ "OCZ-VERTEX*",	NULL,		ATA_QUIRK_NO_FUA },
> -	{ "INTEL*SSDSC2CT*",	NULL,		ATA_QUIRK_NO_FUA },
>  
>  	/* End Marker */
>  	{ }


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] ata: libata-core: Fix NO_FUA quirks shadowed by earlier quirk entries

[Niklas Cassel]

On Wed, Oct 14, 2026 at 04:34:55PM +0200, Damien Le Moal wrote:
> On 2026/10/13 21:39, Niklas Cassel wrote:
> > ata_dev_quirks() uses the first entry in __ata_dev_quirks[] that
> > matches the device and ignores all later entries. Commit 38d431229bfb
> > ("ata: libata: exclude FUA support for known buggy drives") appended
> > three ATA_QUIRK_NO_FUA entries to the end of the table, with model
> > patterns meant to cover whole product lines. Earlier entries already
> > match some or all of these drives, so NO_FUA is not always applied:
> > 
> > - "INTEL*SSDSC2CT*" never takes effect. Every model it matches also
> >   matches the earlier "INTEL*SSD*" entry, so these drives only get
> >   ATA_QUIRK_ZERO_AFTER_TRIM.
> > 
> > - "WDC*WD2500J*" does not take effect for "WDC WD2500JD-*" drives,
> >   which match the earlier ATA_QUIRK_WD_BROKEN_LPM entry.
> > 
> > - "OCZ-VERTEX*" does not take effect for "OCZ-VERTEX" drives with
> >   firmware "1.30", which match the earlier ATA_QUIRK_BROKEN_HPA entry.
> > 
> > Move the Intel entry in front of "INTEL*SSD*" and keep
> > ATA_QUIRK_ZERO_AFTER_TRIM in it, so these drives stay on the
> > ZERO_AFTER_TRIM whitelist. Add ATA_QUIRK_NO_FUA to the WD2500JD and
> > OCZ Vertex 1.30 entries. The quirks of all other drives are unchanged.
> > 
> > libata only uses FUA when it is enabled with libata.fua=1, so systems
> > using the default setting are not affected.
> > 
> > Fixes: 38d431229bfb ("ata: libata: exclude FUA support for known buggy drives")
> > Assisted-by: LLM
> > Signed-off-by: Niklas Cassel <cassel@kernel.org>
> 
> Your solution works, but I think it is very fragile. We will most likely again
> make the same mistake with the oredering. So what about something like this:
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 92233fb25051..980a6a6256b1 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -4515,6 +4515,7 @@ static u64 ata_dev_quirks(const struct ata_device *dev)
>         unsigned char model_num[ATA_ID_PROD_LEN + 1];
>         unsigned char model_rev[ATA_ID_FW_REV_LEN + 1];
>         const struct ata_dev_quirks_entry *ad = __ata_dev_quirks;
> +       u64 quirks = 0;
> 
>         /* dev->quirks is an u64. */
>         BUILD_BUG_ON(__ATA_QUIRK_MAX > 64);
> @@ -4527,11 +4528,11 @@ static u64 ata_dev_quirks(const struct ata_device *dev)
>                     (!ad->model_rev || glob_match(ad->model_rev, model_rev))) {
>                         ata_dev_print_quirks(dev, model_num, model_rev,
>                                              ad->quirks);
> -                       return ad->quirks;
> +                       quirks |= ad->quirks;
>                 }
>                 ad++;
>         }
> -       return 0;
> +       return quirks;
>  }

I like the idea.

Send it as a real patch :)


Kind regards,
Niklas

Re: [PATCH] ata: libata-core: Fix NO_FUA quirks shadowed by earlier quirk entries

[Damien Le Moal]

On 2026/10/13 21:39, Niklas Cassel wrote:
> ata_dev_quirks() uses the first entry in __ata_dev_quirks[] that
> matches the device and ignores all later entries. Commit 38d431229bfb
> ("ata: libata: exclude FUA support for known buggy drives") appended
> three ATA_QUIRK_NO_FUA entries to the end of the table, with model
> patterns meant to cover whole product lines. Earlier entries already
> match some or all of these drives, so NO_FUA is not always applied:
> 
> - "INTEL*SSDSC2CT*" never takes effect. Every model it matches also
>   matches the earlier "INTEL*SSD*" entry, so these drives only get
>   ATA_QUIRK_ZERO_AFTER_TRIM.
> 
> - "WDC*WD2500J*" does not take effect for "WDC WD2500JD-*" drives,
>   which match the earlier ATA_QUIRK_WD_BROKEN_LPM entry.
> 
> - "OCZ-VERTEX*" does not take effect for "OCZ-VERTEX" drives with
>   firmware "1.30", which match the earlier ATA_QUIRK_BROKEN_HPA entry.
> 
> Move the Intel entry in front of "INTEL*SSD*" and keep
> ATA_QUIRK_ZERO_AFTER_TRIM in it, so these drives stay on the
> ZERO_AFTER_TRIM whitelist. Add ATA_QUIRK_NO_FUA to the WD2500JD and
> OCZ Vertex 1.30 entries. The quirks of all other drives are unchanged.
> 
> libata only uses FUA when it is enabled with libata.fua=1, so systems
> using the default setting are not affected.
> 
> Fixes: 38d431229bfb ("ata: libata: exclude FUA support for known buggy drives")
> Assisted-by: LLM
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Unfortunately, my idea does not work. So:

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research