Re: [PATCH] PCI/AER: Add a kernel parameter to skip the reset during AER recovery

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci/aer: interrupt storm when skipping AER reset
--

commit fe869440b3515f272e9e500d4db9fdb37085a98f
Author: Sunny Feng <sunnfeng@amazon.com>

PCI/AER: Add a kernel parameter to skip the reset during AER recovery

This patch introduces a "pci=error_reset_disable=" kernel parameter to skip
resetting specific PCI devices during AER recovery. This allows the devices
to remain in their errored state so diagnostics can be collected, while
still re-enabling Root Port interrupts.

> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index c376c3adbefd2..1f403f0f5d258 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c

[ ... ]

> @@ -1872,7 +1883,21 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
>  	if (host->native_aer && aer)
>  		aer_disable_irq(root);
>  
> -	if (type == PCI_EXP_TYPE_RC_EC || type == PCI_EXP_TYPE_RC_END) {
> +	/*
> +	 * Leave the devices in the state the error left them in if one of
> +	 * them is listed in the error_reset_disable parameter, so their
> +	 * diagnostics can be collected before they are recovered.
> +	 */
> +	if (dev->subordinate)
> +		pci_walk_bus(dev->subordinate, aer_check_error_reset_disabled,
> +			     &reset_disabled);
> +	else
> +		aer_check_error_reset_disabled(dev, &reset_disabled);
> +
> +	if (reset_disabled) {
> +		rc = -ECANCELED;
> +		pci_info(dev, "not reset, error_reset_disable matches a device below\n");

[Severity: High]
Could this cause an interrupt storm if a device is skipped from AER reset?

When a device is skipped via the error_reset_disable parameter, it is left
in a broken state with its Uncorrectable Error Status asserted.

Later in aer_root_reset(), the code unconditionally clears the Root Error
Status and re-enables Root Port AER interrupts via aer_enable_irq(root).

According to the PCIe Base Specification Rev 6.0, Section 6.2.4.2
(Multiple Error Handling), a device is allowed to generate additional Error
Messages for subsequent errors of the same type even if the status bit is
already set.

If the un-reset device continues generating errors (for example, due to a
degraded link or ongoing background DMA), the Root Port will continually
receive messages and fire interrupts.

Since the aer_irq() ISR clears the Root Error Status and returns even when
the event queue is full:

drivers/pci/pcie/aer.c:aer_irq() {
    ...
    pci_write_config_dword(rp, aer + PCI_ERR_ROOT_STATUS, e_src.status);

    if (!kfifo_put(&rpc->aer_fifo, e_src))
        return IRQ_HANDLED;
    ...
}

Will this lead to an infinite hard interrupt storm loop, potentially
causing a denial of service?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013201045.33806-1-sunnfeng@amazon.com?part=1