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

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

[dayou5941]

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)
+{
+	u32 cap = readl(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);
+
+	if (last_port_end > bar_size) {
+		dev_warn(&pdev->dev,
+			 "BAR5 too small for %u ports (last port ends at %u, BAR %llu)\n",
+			 max_ports, last_port_end,
+			 (unsigned long long)bar_size);
+		return -ENODEV;
+	}
+
+	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);
+	if (rc)
+		return rc;
+
 	/* detect remapped nvme devices */
 	ahci_remap_check(pdev, ahci_pci_bar, hpriv);
 
-- 
2.25.1

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

[Niklas Cassel]

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
>

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

[Damien Le Moal]

On 2026/04/27 21:14, Niklas Cassel wrote:
>> +			 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;

I do not agree here. We did not do any I/O. If anything, this should be EINVAL.
But I think that ENODEV is safer since we are in the probe context here and we
do not want to see that device show up.


-- 
Damien Le Moal
Western Digital Research

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

[Niklas Cassel]

On 27 April 2026 22:23:12 CEST, Damien Le Moal <dlemoal@kernel.org> wrote:
>On 2026/04/27 21:14, Niklas Cassel wrote:
>>> +			 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;
>
>I do not agree here. We did not do any I/O. If anything, this should be EINVAL.
>But I think that ENODEV is safer since we are in the probe context here and we
>do not want to see that device show up.


How about -ENXIO?

No such device or address


It is the only error code, except for -ENODEV
that is a valid error code to fail probe():
https://elixir.bootlin.com/linux/v7.0.1/source/drivers/base/dd.c#L653


Kind regards,
Nikla

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

[Damien Le Moal]

On 2026/04/28 5:32, Niklas Cassel wrote:
> On 27 April 2026 22:23:12 CEST, Damien Le Moal <dlemoal@kernel.org> wrote:
>> On 2026/04/27 21:14, Niklas Cassel wrote:
>>>> +			 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;
>>
>> I do not agree here. We did not do any I/O. If anything, this should be EINVAL.
>> But I think that ENODEV is safer since we are in the probe context here and we
>> do not want to see that device show up.
> 
> 
> How about -ENXIO?
> 
> No such device or address
> 
> 
> It is the only error code, except for -ENODEV
> that is a valid error code to fail probe():
> https://elixir.bootlin.com/linux/v7.0.1/source/drivers/base/dd.c#L653

That is my point, since we want to fail probe. So I would go with this ENODEV.


-- 
Damien Le Moal
Western Digital Research

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

[Niklas Cassel]

On 27 April 2026 23:47:41 CEST, Damien Le Moal <dlemoal@kernel.org> wrote:
>On 2026/04/28 5:32, Niklas Cassel wrote:
>> On 27 April 2026 22:23:12 CEST, Damien Le Moal <dlemoal@kernel.org> wrote:
>>> On 2026/04/27 21:14, Niklas Cassel wrote:
>>>>> +			 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;
>>>
>>> I do not agree here. We did not do any I/O. If anything, this should be EINVAL.
>>> But I think that ENODEV is safer since we are in the probe context here and we
>>> do not want to see that device show up.
>> 
>> 
>> How about -ENXIO?
>> 
>> No such device or address
>> 
>> 
>> It is the only error code, except for -ENODEV
>> that is a valid error code to fail probe():
>> https://elixir.bootlin.com/linux/v7.0.1/source/drivers/base/dd.c#L653
>
>That is my point, since we want to fail probe. So I would go with this ENODEV.


ENXIO and ENODEV are handled exactly the same way in dd.c

Anyway, looking at dd.c again, any negative error code will do. The only difference seems to be that the driver core will not print an error for ENXIO and ENODEV, but will do so for any other error code.

Looking at:
https://linux.kernel.narkive.com/NI9fvCoJ/device-driver-probe-return-codes

I don't think that my original suggestion of EIO is wrong.

Anyway, considering that all error codes will fail the probe, I'm fine with any error code (except for EPROBE_DEFER :P)


Kind regards,
Niklas