On Wed, Sep 06, 2023 at 03:13:29PM -0500, Bjorn Helgaas wrote:
> On Wed, Sep 06, 2023 at 11:00:19PM +0300, Andy Shevchenko wrote:
> > On Wed, Sep 06, 2023 at 10:59:20PM +0300, Andy Shevchenko wrote:
> > > On Wed, Sep 06, 2023 at 02:06:23PM -0500, Bjorn Helgaas wrote:
> > > > On Wed, Aug 16, 2023 at 05:21:15PM +0000, Bartosz Pawlowski wrote:
...
> > > > > +/*
> > > > > + * 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.
> > > >
> > > > Can we reference the commit that added the existing software
> > > > workaround?
> >
> > > See below.
> >
> > Oh, I meant the second question here, i.e.
> >
> > > > It sounds like systems that (a) don't require 5-level paging and (b)
> > > > use an IOMMU implementation that include the appropriate changes might
> > > > still be able to use ATS? Is there a way for them to do that?
> >
> > ^^^ this one.
>
> Sorry, I'm missing your point here.
>
> The comment mentions an existing partial software workaround.
> Presumably that was added by some commit, and it would be helpful to
> know which one.
>
> The comment also suggests that if the software workaround were
> completed (or if a system didn't require 5-level paging) and it had
> related changes to its IOMMU driver, we could still use ATS even on
> hardware with this defect.
>
> So I'm wondering if there's a way for an IOMMU driver that has the
> required changes and can tell that we're not using 5-level paging can
> override this quirk that disables ATS.
>
> Maybe we want to unconditionally disable ATS on these broken devices.
> In that case, I think we should just completely drop the comments
> about the software workaround and IOMMU driver changes because they
> wouldn't be relevant.
I see now what you are for. Yes, I agree that if we can't provide a workaround
then probably comment is a bit bogus.
> > > > > + */
> > > > > +static void quirk_intel_e2000_no_ats(struct pci_dev *pdev)
> > > > > +{
> > >
> > > > > + if (pdev->revision < 0x20)
> > >
> > > Isn't it the answer to your question?
> > >
> > > > > + quirk_no_ats(pdev);
> > > > > +}
--
With Best Regards,
Andy Shevchenko
