Re: [PATCH 1/1] powerpc/pseries/pci: quirks: Add pseries TG3 D3hot delay quirk for EEH stability

[Harsh Prateek Bora <harshpb@linux.ibm.com>]

Hi Narayana,

Few comments inline below ..

On 28/04/26 3:05 pm, Narayana Murty N wrote:
> On pseries platforms, binding Broadcom PCIe NIC devices to the vfio-pci
> driver frequently triggers an unintended EEH (Extended Error Handling)
> isolation event.
> 

Commit log title can be rephrased to say "Broadcom TG3" instead of 
"pseries TG3". Also, need to mention TG3 in commit log as well.

> This occurs because the device firmware violates PCIe specification
> recovery timings when transitioning from the D3hot to D0 power state
> during the binding process. The strict pseries PHB catches the
> resulting Unsupported Request during the subsequent configuration
> space read, assuming a device failure.
> 
> Add a pseries-specific PCI fixup quirk for Broadcom devices to

Not all broadcom devices, only TG3 specific, right ?

> explicitly extend the `d3hot_delay` to 200ms. This forces the PCI
> core to sleep long enough for the firmware to safely complete the
> D0 transition before attempting config reads, ensuring a clean VFIO
> passthrough initialization.

Do we need a "Fixes:" tag here to mention specific TG3 device support 
being tested/fixed ?

> 
> Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
> ---
>   arch/powerpc/platforms/pseries/pci.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
> index 84e4ffe957a8..5f3cf9a7bdd3 100644
> --- a/arch/powerpc/platforms/pseries/pci.c
> +++ b/arch/powerpc/platforms/pseries/pci.c
> @@ -291,3 +291,24 @@ int pseries_root_bridge_prepare(struct pci_host_bridge *bridge)
>   	bus->cur_bus_speed = prop_to_pci_speed(pcie_link_speed_stats[1]);
>   	return 0;
>   }
> +
> +/*
> + * Workaround for sluggish PCIe device firmware.

Mention Broadcom TG3 (specific device details) here as well, until the 
routine broadens its scope.

Also, which specific Broadcom device IDs exhibit this issue?
Are all TG3 devices affected, or only specific models? Please document.

> + *
> + * The device violates the PCIe spec recovery timing when transitioning
> + * from D3hot to D0. On standard architectures this is often ignored, but
> + * the strict PowerPC pseries PHB catches the Unsupported Request during
> + * the subsequent config read and triggers an EEH.
> + *
> + * We inject a longer delay to ensure the device is ready before the PCI
> + * core attempts to access configuration space.
> + */
> +static void quirk_pseries_d0_wake_delay(struct pci_dev *dev)
> +{
> +	dev->d3hot_delay = 200;

No justification provided for using this value. Is this minimum or
maximum from test observations? Please share test details in comments
and/or commit log. Also, I see Marvel sky2 driver using 300ms, not sure
if we may need to increase. Preferably, use a macro to define this
constant? Should it be a tg3 driver level fix during probe or be placed
in drivers/pci/quirks.c ? Also, is there an impact on system boot time?

> +	pci_info(dev, "pseries  Quirk:D3hot->D0 delay %d ms to prevent EEH\n",

Double space after "pseries", missing space after "Quirk:"
Also pci_dbg() may be more appropriate?

> +		 dev->d3hot_delay);
> +}
> +/* Blanket application to ALL Broadcom PCI devices */
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_BROADCOM,
> +			PCI_ANY_ID, quirk_pseries_d0_wake_delay);

Replace PCI_ANY_ID with specific device IDs that are known to have this 
issue, or provide strong justification for the broad scope as it may 
impact system boot time.