Re: [PATCH] Avoid FLR for AMD Starship/Matisse Cryptographic Coprocessor

[Alex Williamson <alex.williamson@redhat.com>]

On Tue, 28 Sep 2021 20:59:02 -0500
Bjorn Helgaas <helgaas@kernel.org> wrote:

> [+cc Alex, Krzysztof, AMD folks]
> 
> On Tue, Sep 28, 2021 at 05:16:49PM -0700, David Jaundrew wrote:
> > This patch fixes another FLR bug for the Starship/Matisse controller:
> > 
> > AMD Starship/Matisse Cryptogrpahic Coprocessor PSPCPP
> > 
> > This patch allows functions on the same Starship/Matisse device (such as
> > USB controller,sound card) to properly pass through to a guest OS using
> > vfio-pc. Without this patch, the virtual machine does not boot and
> > eventually times out.  
> 
> Apparently yet another AMD device that advertises FLR support, but it
> doesn't work?
> 
> I don't have a problem avoiding the FLR, but I *would* like some
> indication that:
> 
>   - This is a known erratum and AMD has some plan to fix this in
>     future devices so we don't have to trip over them all
>     individually, and
> 
>   - This is not a security issue.  Part of the reason VFIO resets
>     pass-through devices is to avoid leaking state from one guest to
>     another.  If reset doesn't work, that makes me wonder, especially
>     since this is a cryptographic coprocessor that sounds like it
>     might be full of secrets.  So I *assume* VFIO will use a different
>     type of reset instead of FLR, but I'm just double-checking.

It depends on what's available, chances are not good that we have
another means of function level reset, so this probably means it's
exposed as-is.  I agree, not great for a device managing something to
do with cryptography.  It's potentially a better security measure to
let the device wedge itself.  Thanks,

Alex
 
> > Excerpt from lspci -nn showing crypto function on same device as USB and
> > sound card (which are already listed in quirks.c):
> > 
> > 0e:00.1 Encryption controller [1080]: Advanced Micro Devices, Inc. [AMD]
> >   Starship/Matisse Cryptographic Coprocessor PSPCPP [1022:1486]
> > 0e:00.3 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD]
> >   Matisse USB 3.0 Host Controller [1022:149c]
> > 0e:00.4 Audio device [0403]: Advanced Micro Devices, Inc. [AMD]
> >   Starship/Matisse HD Audio Controller [1022:1487]
> > 
> > Fix tested successfully on an Asus ROG STRIX X570-E GAMING motherboard
> > with AMD Ryzen 9 3900X.
> > 
> > Signed-off-by: David Jaundrew <david@jaundrew.com>  
> 
> The patch below still doesn't apply.  Looks like maybe it was pasted
> into the email and the tabs got changed to space?  No worries, I can
> apply it manually if appropriate.
> 
> > ---
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 6d74386eadc2..0d19e7aa219a 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -5208,6 +5208,7 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443, quirk_intel_qat_vf_cap);
> >  /*
> >   * FLR may cause the following to devices to hang:
> >   *
> > + * AMD Starship/Matisse Cryptographic Coprocessor PSPCPP 0x1486
> >   * AMD Starship/Matisse HD Audio Controller 0x1487
> >   * AMD Starship USB 3.0 Host Controller 0x148c
> >   * AMD Matisse USB 3.0 Host Controller 0x149c
> > @@ -5219,6 +5220,7 @@ static void quirk_no_flr(struct pci_dev *dev)
> >  {
> >         dev->dev_flags |= PCI_DEV_FLAGS_NO_FLR_RESET;
> >  }
> > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_AMD, 0x1486, quirk_no_flr);
> >  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_AMD, 0x1487, quirk_no_flr);
> >  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_AMD, 0x148c, quirk_no_flr);
> >  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_AMD, 0x149c, quirk_no_flr);
> >   
>