Re: [PATCH v15 4/4] PCI: Centralize pci_aer_available() checking

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pcie: stale host->native_aer enables AER after MSI is disabled
--

commit 905020873a7ae5dea6e2ab6198bde259d6bd9411
Author: Bjorn Helgaas <bhelgaas@google.com>

PCI: Centralize pci_aer_available() checking

This commit centralizes the pci_aer_available() check by evaluating it once
during host bridge initialization and storing the result in host->native_aer.
This aims to simplify callers by having them only check host->native_aer.

> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
> index 32fc623dd4108..9f8c6dd434c55 100644
> --- a/drivers/pci/pcie/portdrv.c
> +++ b/drivers/pci/pcie/portdrv.c
> @@ -239,8 +239,7 @@ static int get_port_device_capability(struct pci_dev *dev)
>  #ifdef CONFIG_PCIEAER
>  	if ((pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
>               pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC) &&
> -	    dev->aer_cap && pci_aer_available() &&
> -	    host->native_aer)
> +	    dev->aer_cap && host->native_aer)

[Severity: Medium]
Could this regression of removing the dynamic pci_aer_available() check
here lead to AER being erroneously enabled if MSI is disabled later during
enumeration?

Since pci_aer_available() requires MSI to be enabled, this code used to
prevent AER from starting if MSI was turned off dynamically. By relying
only on the statically cached host->native_aer, we might bypass that
requirement.

>  		services |= PCIE_PORT_SERVICE_AER;
>  #endif

> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 1e487a213eb0f..e1ca8096bcd58 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -670,7 +670,7 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge)

[ ... ]

> -	bridge->native_aer = port_services;
> +	bridge->native_aer = pci_aer_available();

[Severity: Medium]
Does this regression cache a stale value for pci_aer_available() if a
hardware quirk disables MSI after the host bridge is initialized?

For example, during device enumeration, quirk_disable_all_msi() might
be called for a specific device, disabling MSI globally:

drivers/pci/quirks.c:quirk_disable_all_msi() {
        ...
        pci_no_msi();
        ...
}

If this happens after pci_init_host_bridge() has already run,
bridge->native_aer will remain true even though pci_aer_available() would
now return false.

The AER device could then bind to the already-registered driver and
erroneously enable AER without MSI support, potentially causing interrupt
storms or unhandled errors.

>  	bridge->native_pcie_hotplug = port_services;
>  	bridge->native_shpc_hotplug = 1;
>  	bridge->native_pme = port_services;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002211828.1293177-1-sathyanarayanan.kuppuswamy@linux.intel.com?part=4