Re: [PATCH 2/2] PCI: Disable ATS for specific Intel IPU E2000 devices

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Aug 16, 2023 at 12:50:28PM -0700, Sheena Mohan wrote:
> It is good to have the SW fix for backward compatibility rather than
> disabling ATS on previous revisions. Maybe adding a flag to toggle the
> feature will be useful.

FYI, I received this (and probably the other direct recipients did,
too), but I think the mailing lists rejected it, probably because it
was a multi-part message (see
http://vger.kernel.org/majordomo-info.html).

The list archive is https://lore.kernel.org/linux-pci/20230816172115.1375716-1-bartosz.pawlowski@intel.com/

> On Wed, Aug 16, 2023 at 10:39 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> 
> > On Wed, Aug 16, 2023 at 05:21:15PM +0000, Bartosz Pawlowski wrote:
> > > There is a HW issue in A and B steppings of Intel IPU E2000 that it
> > > expects wrong endianness in ATS invalidation message body. This problem
> > > can lead to outdated translations being returned as valid and finally
> > > cause system instability.
> > >
> > > In order to prevent such issues introduce quirk_intel_e2000_no_ats()
> > > function to disable ATS for vulnerable IPU E2000 devices.
> > >
> > > Signed-off-by: Bartosz Pawlowski <bartosz.pawlowski@intel.com>
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>
> >
> > Andy, Alexander, would you please reiterate your reviewed-by on the
> > mailing list?  I try to avoid relying on internal reviews collected by
> > the author.  Those are great and I'm glad they happen, but it's good
> > if they also appear as part of the public conversation on the mailing
> > list.
> >
> > > ---
> > >  drivers/pci/quirks.c | 22 ++++++++++++++++++++++
> > >  1 file changed, 22 insertions(+)
> > >
> > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > > index a900546d8d45..9aa1e0148ed2 100644
> > > --- a/drivers/pci/quirks.c
> > > +++ b/drivers/pci/quirks.c
> > > @@ -5550,6 +5550,28 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI,
> > 0x7347, quirk_amd_harvest_no_ats);
> > >  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x734f,
> > quirk_amd_harvest_no_ats);
> > >  /* AMD Raven platform iGPU */
> > >  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x15d8,
> > quirk_amd_harvest_no_ats);
> > > +
> > > +/*
> > > + * Intel IPU E2000 revisions before C0 implement incorrect endianness
> > > + * in ATS Invalidate Request message body. Although there is existing
> > software
> > > + * workaround for this issue, it is not functionally complete (no 5-lvl
> > paging
> > > + * support) and it requires changes in all IOMMU implementations
> > supporting
> > > + * ATS. Therefore, disabling ATS seems to be more reasonable.
> > > + */
> > > +static void quirk_intel_e2000_no_ats(struct pci_dev *pdev)
> > > +{
> > > +     if (pdev->revision < 0x20)
> > > +             quirk_no_ats(pdev);
> > > +}
> > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1451,
> > quirk_intel_e2000_no_ats);
> > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1452,
> > quirk_intel_e2000_no_ats);
> > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1453,
> > quirk_intel_e2000_no_ats);
> > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1454,
> > quirk_intel_e2000_no_ats);
> > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1455,
> > quirk_intel_e2000_no_ats);
> > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1457,
> > quirk_intel_e2000_no_ats);
> > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1459,
> > quirk_intel_e2000_no_ats);
> > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x145a,
> > quirk_intel_e2000_no_ats);
> > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x145c,
> > quirk_intel_e2000_no_ats);
> > >  #endif /* CONFIG_PCI_ATS */
> > >
> > >  /* Freescale PCIe doesn't support MSI in RC mode */
> > > --
> > > 2.41.0
> > >
> > > ---------------------------------------------------------------------
> > > Intel Technology Poland sp. z o.o.
> > > ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII
> > Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP
> > 957-07-52-316 | Kapital zakladowy 200.000 PLN.
> > > Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu
> > ustawy z dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w
> > transakcjach handlowych.
> > >
> > > Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego
> > adresata i moze zawierac informacje poufne. W razie przypadkowego
> > otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej
> > usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
> > > This e-mail and any attachments may contain confidential material for
> > the sole use of the intended recipient(s). If you are not the intended
> > recipient, please contact the sender and delete all copies; any review or
> > distribution by others is strictly prohibited.
> > >
> >