[PATCH] ata: libahci: clear PxCLBU and PxFBU for AHCI_HFLAG_32BIT_ONLY

[PATCH] ata: libahci: clear PxCLBU and PxFBU for AHCI_HFLAG_32BIT_ONLY

[Niklas Cassel]

A user reported that commit 105c42566a55 ("ata: ahci: force 32-bit DMA for
JMicron JMB582/JMB585") made the JMicron JMB585 unusable on his board.

The failure is seen as soon as the ahci driver is probed, and booting with
iommu=off does not solve the problem.

Looking at the AHCI specification, PxCLBU and PxFBU are both read only '0'
for HBAs that do not support 64-bit addressing.

For HBAs that support 64-bit addressing, the registers are read write,
with a reset value that is Implementation Specific.

When using the AHCI_HFLAG_32BIT_ONLY flag, the HBA does support 64-bit
addressing, but we are simply setting a 32-bit DMA mask. Thus, in this
case, we need to explicitly clear the registers to 0.

Fixes: c7a42156d99b ("ahci: disable 64bit dma on sb600")
Reported-by: Roland Waltersson <roland.waltersson@netinsight.net>
Closes: https://lore.kernel.org/linux-ide/IA0PR17MB668730A4ECCD65F7A1DC3EDC9EB62@IA0PR17MB6687.namprd17.prod.outlook.com/
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/ata/libahci.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 6d72eb017b49..3a40ea926588 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -748,11 +748,25 @@ void ahci_start_fis_rx(struct ata_port *ap)
 	if (hpriv->cap & HOST_CAP_64)
 		writel((pp->cmd_slot_dma >> 16) >> 16,
 		       port_mmio + PORT_LST_ADDR_HI);
+	/*
+	 * On HBAs that do not support 64-bit addressing PxCLBU is read only,
+	 * however, when forcing a HBA that has CAP.S64A in 32-bit only mode,
+	 * the register is RW, and the reset value is Implementation Specific.
+	 */
+	else if (hpriv->flags & AHCI_HFLAG_32BIT_ONLY)
+		writel(0, port_mmio + PORT_LST_ADDR_HI);
 	writel(pp->cmd_slot_dma & 0xffffffff, port_mmio + PORT_LST_ADDR);
 
 	if (hpriv->cap & HOST_CAP_64)
 		writel((pp->rx_fis_dma >> 16) >> 16,
 		       port_mmio + PORT_FIS_ADDR_HI);
+	/*
+	 * On HBAs that do not support 64-bit addressing PxFBU is read only,
+	 * however, when forcing a HBA that has CAP.S64A in 32-bit only mode,
+	 * the register is RW, and the reset value is Implementation Specific.
+	 */
+	else if (hpriv->flags & AHCI_HFLAG_32BIT_ONLY)
+		writel(0, port_mmio + PORT_FIS_ADDR_HI);
 	writel(pp->rx_fis_dma & 0xffffffff, port_mmio + PORT_FIS_ADDR);
 
 	/* enable FIS reception */
-- 
2.55.0

Re: [PATCH] ata: libahci: clear PxCLBU and PxFBU for AHCI_HFLAG_32BIT_ONLY

[Damien Le Moal]

On 9/4/26 05:03, Niklas Cassel wrote:
> A user reported that commit 105c42566a55 ("ata: ahci: force 32-bit DMA for
> JMicron JMB582/JMB585") made the JMicron JMB585 unusable on his board.
> 
> The failure is seen as soon as the ahci driver is probed, and booting with
> iommu=off does not solve the problem.
> 
> Looking at the AHCI specification, PxCLBU and PxFBU are both read only '0'
> for HBAs that do not support 64-bit addressing.
> 
> For HBAs that support 64-bit addressing, the registers are read write,
> with a reset value that is Implementation Specific.
> 
> When using the AHCI_HFLAG_32BIT_ONLY flag, the HBA does support 64-bit
> addressing, but we are simply setting a 32-bit DMA mask. Thus, in this
> case, we need to explicitly clear the registers to 0.
> 
> Fixes: c7a42156d99b ("ahci: disable 64bit dma on sb600")
> Reported-by: Roland Waltersson <roland.waltersson@netinsight.net>
> Closes: https://lore.kernel.org/linux-ide/IA0PR17MB668730A4ECCD65F7A1DC3EDC9EB62@IA0PR17MB6687.namprd17.prod.outlook.com/
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  drivers/ata/libahci.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index 6d72eb017b49..3a40ea926588 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -748,11 +748,25 @@ void ahci_start_fis_rx(struct ata_port *ap)
>  	if (hpriv->cap & HOST_CAP_64)
>  		writel((pp->cmd_slot_dma >> 16) >> 16,
>  		       port_mmio + PORT_LST_ADDR_HI);
> +	/*
> +	 * On HBAs that do not support 64-bit addressing PxCLBU is read only,
> +	 * however, when forcing a HBA that has CAP.S64A in 32-bit only mode,
> +	 * the register is RW, and the reset value is Implementation Specific.
> +	 */
> +	else if (hpriv->flags & AHCI_HFLAG_32BIT_ONLY)
> +		writel(0, port_mmio + PORT_LST_ADDR_HI);

I find the comment very confusing as it is not directly describing what is being
done here. Furthermore, it says "when forcing a HBA that has CAP.S64A in 32-bit
only mode", but the added code is an "else" of "if (hpriv->cap & HOST_CAP_64))",
so that is for the case where the adapter is not 64-bits DMA capable. I am not
understanding something here...

Could you clarify please ? Also, please move the code comment above the "if" so
that it describes both the "if" and "else". Having the comment in the middle of
this if/else makes things hard to read.

>  	writel(pp->cmd_slot_dma & 0xffffffff, port_mmio + PORT_LST_ADDR);
>  
>  	if (hpriv->cap & HOST_CAP_64)
>  		writel((pp->rx_fis_dma >> 16) >> 16,
>  		       port_mmio + PORT_FIS_ADDR_HI);
> +	/*
> +	 * On HBAs that do not support 64-bit addressing PxFBU is read only,
> +	 * however, when forcing a HBA that has CAP.S64A in 32-bit only mode,
> +	 * the register is RW, and the reset value is Implementation Specific.
> +	 */
> +	else if (hpriv->flags & AHCI_HFLAG_32BIT_ONLY)
> +		writel(0, port_mmio + PORT_FIS_ADDR_HI);

Same thing here. Comment needs to move up and needs to be improved because (at
least I) find it not clear.

>  	writel(pp->rx_fis_dma & 0xffffffff, port_mmio + PORT_FIS_ADDR);
>  
>  	/* enable FIS reception */


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] ata: libahci: clear PxCLBU and PxFBU for AHCI_HFLAG_32BIT_ONLY

[Niklas Cassel]

On Fri, Sep 04, 2026 at 08:54:37AM +0900, Damien Le Moal wrote:
> On 9/4/26 05:03, Niklas Cassel wrote:
> > A user reported that commit 105c42566a55 ("ata: ahci: force 32-bit DMA for
> > JMicron JMB582/JMB585") made the JMicron JMB585 unusable on his board.
> > 
> > The failure is seen as soon as the ahci driver is probed, and booting with
> > iommu=off does not solve the problem.
> > 
> > Looking at the AHCI specification, PxCLBU and PxFBU are both read only '0'
> > for HBAs that do not support 64-bit addressing.
> > 
> > For HBAs that support 64-bit addressing, the registers are read write,
> > with a reset value that is Implementation Specific.
> > 
> > When using the AHCI_HFLAG_32BIT_ONLY flag, the HBA does support 64-bit
> > addressing, but we are simply setting a 32-bit DMA mask. Thus, in this
> > case, we need to explicitly clear the registers to 0.
> > 
> > Fixes: c7a42156d99b ("ahci: disable 64bit dma on sb600")
> > Reported-by: Roland Waltersson <roland.waltersson@netinsight.net>
> > Closes: https://lore.kernel.org/linux-ide/IA0PR17MB668730A4ECCD65F7A1DC3EDC9EB62@IA0PR17MB6687.namprd17.prod.outlook.com/
> > Signed-off-by: Niklas Cassel <cassel@kernel.org>
> > ---
> >  drivers/ata/libahci.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> > index 6d72eb017b49..3a40ea926588 100644
> > --- a/drivers/ata/libahci.c
> > +++ b/drivers/ata/libahci.c
> > @@ -748,11 +748,25 @@ void ahci_start_fis_rx(struct ata_port *ap)
> >  	if (hpriv->cap & HOST_CAP_64)
> >  		writel((pp->cmd_slot_dma >> 16) >> 16,
> >  		       port_mmio + PORT_LST_ADDR_HI);
> > +	/*
> > +	 * On HBAs that do not support 64-bit addressing PxCLBU is read only,
> > +	 * however, when forcing a HBA that has CAP.S64A in 32-bit only mode,
> > +	 * the register is RW, and the reset value is Implementation Specific.
> > +	 */
> > +	else if (hpriv->flags & AHCI_HFLAG_32BIT_ONLY)
> > +		writel(0, port_mmio + PORT_LST_ADDR_HI);
> 
> I find the comment very confusing as it is not directly describing what is being
> done here. Furthermore, it says "when forcing a HBA that has CAP.S64A in 32-bit
> only mode", but the added code is an "else" of "if (hpriv->cap & HOST_CAP_64))",
> so that is for the case where the adapter is not 64-bits DMA capable. I am not
> understanding something here...

I think this will clarify things:
https://github.com/torvalds/linux/blob/v7.3-rc1/drivers/ata/libahci.c#L481-L485

libahci clears CAP.S64A from hpriv->cap for broken controllers.

If you do a:

$ git grep -C 4 HOST_CAP_64 drivers/ata

you will see that there are a bunch of drivers that use libahci.c, that relies
on "hpriv->cap & HOST_CAP_64" when setting the DMA mask.

So, while my initial idea was to just change so that the quirk does NOT clear
CAP.S64A from hpriv->cap, that would also mean that we would need to grow an

if (hpriv->flags & AHCI_HFLAG_32BIT_ONLY) in all these drivers that set the
dma mask:
drivers/ata/acard-ahci.c
drivers/ata/ahci.c
drivers/ata/libahci_platform.c
drivers/ata/sata_highbank.c

So I opted for letting the quirk continue doing what it is doing, make all
libahci.c based drivers set the DMA mask to 64 or 32, only based on
hpriv->cap & HOST_CAP_64.

> 
> Could you clarify please ? Also, please move the code comment above the "if" so
> that it describes both the "if" and "else". Having the comment in the middle of
> this if/else makes things hard to read.

Sure, will send a v2.


Kind regards,
Niklas