Re: [PATCH] PCI: Mark Intel bridge on SuperMicro Atom C3xxx motherboards to avoid bus reset

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Alex]

On Fri, May 24, 2019 at 05:31:18PM +0200, Maik Broemme wrote:
> The Intel PCI bridge on SuperMicro Atom C3xxx motherboards do not
> successfully complete a bus reset when used with certain child devices.
> After the reset, config accesses to the child may fail. If assigning
> such device via VFIO it will immediately fail with:
> 
>   vfio-pci 0000:01:00.0: Failed to return from FLR
>   vfio-pci 0000:01:00.0: timed out waiting for pending transaction;
>   performing function level reset anyway

I guess these messages are from v4.13 or earlier, since the "Failed to
return from FLR" text was removed by 821cdad5c46c ("PCI: Wait up to 60
seconds for device to become ready after FLR"), which appeared in
v4.14.

I suppose a current kernel would fail similarly, but could you try it?
I think a current kernel would give more informative messages like:

  not ready XXms after FLR, giving up
  not ready XXms after bus reset, giving up

I don't understand the connection here: the messages you quote are
related to FLR, but the quirk isn't related to FLR.  The quirk
prevents a secondary bus reset.  So is it the case that we try FLR
first, it fails, then we try a secondary bus reset (does this succeed?
you don't mention an error from it), and the device remains
unresponsive and VFIO assignment fails?

And with the quirk, I assume we still try FLR, and it still fails.
But we *don't* try a secondary bus reset, and the device magically
works?  That's confusing to me.

> Device will disappear from PCI device list:
> 
>   !!! Unknown header type 7f
>   Kernel driver in use: vfio-pci
>   Kernel modules: ddbridge
> 
> The attached patch will mark the root port as incapable of doing a
> bus level reset. After that all my tested devices survive a VFIO
> assignment and several VM reboot cycles.
> 
> Signed-off-by: Maik Broemme <mbroemme@libmpq.org>
> ---
>  drivers/pci/quirks.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 0f16acc323c6..86cd42872708 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3433,6 +3433,13 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATHEROS, 0x0034, quirk_no_bus_reset);
>   */
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CAVIUM, 0xa100, quirk_no_bus_reset);
>  
> +/*
> + * Root port on some SuperMicro Atom C3xxx motherboards do not successfully
> + * complete a bus reset when used with certain child devices. After the
> + * reset, config accesses to the child may fail.
> + */
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x19a4, quirk_no_bus_reset);
> +
>  static void quirk_no_pm_reset(struct pci_dev *dev)
>  {
>  	/*
> -- 
> 2.21.0