Re: [PATCH v4] ata: ahci: fail probe if BAR too small for claimed ports

[Niklas Cassel <cassel@kernel.org>]

On Mon, Apr 27, 2026 at 02:05:46PM +0800, dayou5941@163.com wrote:
> From: Youhong Li <liyouhong@kylinos.cn>
> 
> When an AHCI controller is disabled in BIOS, its HOST_CAP register may
> contain invalid values (e.g., 0xFFFFFFFF) indicating an impossibly large
> number of ports. If CAP.NP claims more ports than can physically fit
> within the mapped BAR region, accessing port registers beyond the BAR
> boundary causes a kernel panic.
> 
> Add validation in ahci_init_one() to check that the BAR size is
> sufficient for the number of ports claimed in CAP.NP. The check
> calculates the required MMIO size as:
> 
>   required_size = 0x100 (global registers) + max_ports * 0x80
> 
> If required_size exceeds the actual BAR size, the probe fails with
> -ENODEV, preventing the panic and providing a clear error message.
> 
> This solution follows the suggestion by Damien Le Moal and Niklas Cassel
> to detect and reject obviously broken controller configurations early.
> 
> Reported-by: liyouhong <liyouhong@kylinos.cn>
> Suggested-by: Damien Le Moal <dlemoal@kernel.org>
> Suggested-by: Niklas Cassel <cassel@kernel.org>
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: liyouhong <liyouhong@kylinos.cn>
> ---
> v2:
> - Complete rewrite based on community feedback
> - Move check from libahci.c to ahci.c
> - Fail probe early instead of attempting to work around invalid state
> - Implement BAR size validation as suggested
> 
> v3:
> - Fix patch format: add "---" separator and move changelog to correct location
> - Change dev_err to dev_warn as suggested
> 
> v4:
> - Break long lines as suggested by Damien
> - Keep complete changelog history
> 
> ---
>  drivers/ata/ahci.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 1d73a53370cf..c04bee682605 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -1888,6 +1888,25 @@ static ssize_t remapped_nvme_show(struct device *dev,
>  
>  static DEVICE_ATTR_RO(remapped_nvme);
>  
> +static int ahci_validate_bar_size(struct pci_dev *pdev, void __iomem *mmio)

static int ahci_validate_bar_size(struct pci_dev *pdev, int bar, struct ahci_host_priv *hpriv)


> +{
> +	u32 cap = readl(mmio + HOST_CAP);

readl(hpriv->mmio, HOST_CAP);


> +	unsigned int max_ports = ahci_nr_ports(cap);
> +	u32 last_port_end = 0x100 + (max_ports * 0x80);
> +	resource_size_t bar_size =
> +		pci_resource_len(pdev, AHCI_PCI_BAR_STANDARD);

pci_resource_len(pdev, bar);


> +
> +	if (last_port_end > bar_size) {
> +		dev_warn(&pdev->dev,
> +			 "BAR5 too small for %u ports (last port ends at %u, BAR %llu)\n",

"BAR%d too small for %u ports (last port ends at %#x, BAR %pa)\n", bar,


> +			 max_ports, last_port_end,
> +			 (unsigned long long)bar_size);

Print resource_size_t as %pa instead of casting to unsigned long long
and pass bar_size by reference (&bar_size):
https://docs.kernel.org/core-api/printk-formats.html#physical-address-types-phys-addr-t


> +		return -ENODEV;

return -EIO;


> +	}
> +
> +	return 0;
> +}
> +
>  static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  {
>  	unsigned int board_id = ent->driver_data;
> @@ -1988,6 +2007,10 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (!hpriv->mmio)
>  		return -ENOMEM;
>  
> +	rc = ahci_validate_bar_size(pdev, hpriv->mmio);

Please let this function be called with the arguments:

ahci_validate_bar_size(pdev, ahci_pci_bar, hpriv);

Such that it takes the same arguments as ahci_remap_check().


Kind regards,
Niklas


> +	if (rc)
> +		return rc;
> +
>  	/* detect remapped nvme devices */
>  	ahci_remap_check(pdev, ahci_pci_bar, hpriv);
>  
> -- 
> 2.25.1
>