Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1648375 > unrolled thread

RE: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RE: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs "Deucher, Alexander" <Alexander.Deucher@amd.com> - 2017-05-23 22:00 +0200
    Re: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs Joerg Roedel <jroedel@suse.de> - 2017-05-24 10:50 +0200
      Re: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs David Woodhouse <dwmw2@infradead.org> - 2017-05-24 12:50 +0200
      RE: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs "Deucher, Alexander" <Alexander.Deucher@amd.com> - 2017-05-24 15:10 +0200
        Re: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs Samuel Sieb <samuel@sieb.net> - 2017-05-26 09:00 +0200
      RE: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs "Deucher, Alexander" <Alexander.Deucher@amd.com> - 2017-05-26 14:00 +0200
        Re: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs David Woodhouse <dwmw2@infradead.org> - 2017-05-26 15:00 +0200
          RE: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs "Deucher, Alexander" <Alexander.Deucher@amd.com> - 2017-05-26 18:10 +0200

#1648375 — RE: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs

> -----Original Message-----
> From: David Woodhouse [mailto:dwmw2@infradead.org]
> Sent: Thursday, May 04, 2017 6:22 AM
> To: Deucher, Alexander; 'Joerg Roedel'; Bjorn Helgaas
> Cc: linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org; Daniel Drake;
> Samuel Sieb; Joerg Roedel
> Subject: Re: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs
> 
> On Fri, 2017-04-07 at 16:46 +0000, Deucher, Alexander wrote:
> > >
> > > -----Original Message-----
> > > From: Joerg Roedel [mailto:joro@8bytes.org]
> > > Sent: Friday, April 07, 2017 10:32 AM
> > > To: Bjorn Helgaas
> > > Cc: linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org; Daniel
> Drake;
> > > Deucher, Alexander; Samuel Sieb; David Woodhouse; Joerg Roedel
> > > Subject: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs
> > >
> > > From: Joerg Roedel <jroedel@suse.de>
> > >
> > > ATS is broken on this hardware and causes IOMMU stalls and
> > > system failure. Disable ATS on these devices to make them
> > > usable again with IOMMU enabled.
> > >
> > > Note that the commit in the Fixes-tag is not buggy, it
> > > just uncovers the problem in the hardware by increasing
> > > the ATS-flush rate.
> > >
> > > Fixes: b1516a14657a ('iommu/amd: Implement flush queue')
> > > Signed-off-by: Joerg Roedel <jroedel@suse.de>
> > Acked-by: Alex Deucher <alexander.deucher@amd.com>
> 
> Alex, are you able to confirm that it is *only* the device with PCI ID
> 0x98e4 which has this problem, or (more likely) come up with an
> exhaustive list? Thanks.
> 
> We'll want the same blacklist in Xen too, won't we?

I finally got an answer from the hw team and we validated ATS on stoney as well so in theory this patch shouldn’t actually be needed.  I think we may actually be papering over some other issue.  The following patch seems to also fix this issue (and other issues):
https://www.spinics.net/lists/stable/msg172631.html

Alex

> 
> > >
> > > ---
> > >  drivers/pci/quirks.c | 19 +++++++++++++++++++
> > >  1 file changed, 19 insertions(+)
> > >
> > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > > index 6736836..7cbe316 100644
> > > --- a/drivers/pci/quirks.c
> > > +++ b/drivers/pci/quirks.c
> > > @@ -4634,3 +4634,22 @@ static void quirk_no_aersid(struct pci_dev
> *pdev)
> > >  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2031,
> > > quirk_no_aersid);
> > >  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2032,
> > > quirk_no_aersid);
> > >  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2033,
> > > quirk_no_aersid);
> > > +
> > > +#ifdef CONFIG_PCI_ATS
> > > +/*
> > > + * Some devices have a broken ATS implementation causing IOMMU
> stalls.
> > > + * Don't use ATS for those devices.
> > > + */
> > > +static void quirk_disable_ats(struct pci_dev *pdev)
> > > +{
> > > +	/*
> > > +	 * Set pdev->ats_cap = 0 to make pci_enable_ats() bail out
> > > +	 * early.
> > > +	 */
> > > +	dev_info(&pdev->dev, "QUIRK: Disabling ATS");
> > > +	pdev->ats_cap = 0;
> > > +}
> > > +
> > > +/* AMD Stoney platform GPU */
> > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x98e4,
> quirk_disable_ats);
> > > +#endif /* CONFIG_PCI_ATS */
> > > --
> > > 1.9.1

[toc] | [next] | [standalone]

#1649303

Hi Alexander,

On Tue, May 23, 2017 at 07:54:12PM +0000, Deucher, Alexander wrote:
> I finally got an answer from the hw team and we validated ATS on
> stoney as well so in theory this patch shouldn’t actually be needed.
> I think we may actually be papering over some other issue.  The
> following patch seems to also fix this issue (and other issues):
> https://www.spinics.net/lists/stable/msg172631.html

Yeah, but it still looks to me like that the hardware got into some
weird state with the storm of ATS invalidations sent to it.

The Completion-Wait loop timeouts seen in the original bug report
indicate that the IOMMU is waiting for a response that never comes. And
this is probably the ATS flush completion response from the GPU, as
disabling ATS on the GPU makes the issue disappear.

Regards,

	Joerg

[toc] | [prev] | [next] | [standalone]

#1649453

[Multipart message — attachments visible in raw view] — view raw

On Wed, 2017-05-24 at 10:44 +0200, Joerg Roedel wrote:
> Hi Alexander,
> 
> On Tue, May 23, 2017 at 07:54:12PM +0000, Deucher, Alexander wrote:
> > 
> > I finally got an answer from the hw team and we validated ATS on
> > stoney as well so in theory this patch shouldn’t actually be needed.
> > I think we may actually be papering over some other issue.  The
> > following patch seems to also fix this issue (and other issues):
> > https://www.spinics.net/lists/stable/msg172631.html
>
> Yeah, but it still looks to me like that the hardware got into some
> weird state with the storm of ATS invalidations sent to it.
> 
> The Completion-Wait loop timeouts seen in the original bug report
> indicate that the IOMMU is waiting for a response that never comes. And
> this is probably the ATS flush completion response from the GPU, as
> disabling ATS on the GPU makes the issue disappear.

The above patch doesn't actually fix any spec violation which could the
GPU an *excuse* to crash and stop responding to invalidations, does it?
It just seems to reduce the invalidation load a little, and thus paper
over the problem that the card tends to crash under load. Absent a more
coherent explanation, it still seems like the correct answer is to
blacklist these devices for ATS because they're broken.

[toc] | [prev] | [next] | [standalone]

#1649604

> -----Original Message-----
> From: Joerg Roedel [mailto:jroedel@suse.de]
> Sent: Wednesday, May 24, 2017 4:45 AM
> To: Deucher, Alexander
> Cc: 'David Woodhouse'; 'Joerg Roedel'; Bjorn Helgaas; linux-
> pci@vger.kernel.org; linux-kernel@vger.kernel.org; Daniel Drake; Samuel
> Sieb
> Subject: Re: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs
> 
> Hi Alexander,
> 
> On Tue, May 23, 2017 at 07:54:12PM +0000, Deucher, Alexander wrote:
> > I finally got an answer from the hw team and we validated ATS on
> > stoney as well so in theory this patch shouldn’t actually be needed.
> > I think we may actually be papering over some other issue.  The
> > following patch seems to also fix this issue (and other issues):
> > https://www.spinics.net/lists/stable/msg172631.html
> 
> Yeah, but it still looks to me like that the hardware got into some
> weird state with the storm of ATS invalidations sent to it.
> 
> The Completion-Wait loop timeouts seen in the original bug report
> indicate that the IOMMU is waiting for a response that never comes. And
> this is probably the ATS flush completion response from the GPU, as
> disabling ATS on the GPU makes the issue disappear.

Yeah, it's weird.  My ack on the patch still stands.  Just adding some additional data.

Alex

[toc] | [prev] | [next] | [standalone]

#1651114

On 05/24/2017 05:56 AM, Deucher, Alexander wrote:
>> -----Original Message-----
>> From: Joerg Roedel [mailto:jroedel@suse.de]
>> Sent: Wednesday, May 24, 2017 4:45 AM
>> To: Deucher, Alexander
>> Cc: 'David Woodhouse'; 'Joerg Roedel'; Bjorn Helgaas; linux-
>> pci@vger.kernel.org; linux-kernel@vger.kernel.org; Daniel Drake; Samuel
>> Sieb
>> Subject: Re: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs
>>
>> Hi Alexander,
>>
>> On Tue, May 23, 2017 at 07:54:12PM +0000, Deucher, Alexander wrote:
>>> I finally got an answer from the hw team and we validated ATS on
>>> stoney as well so in theory this patch shouldn’t actually be needed.
>>> I think we may actually be papering over some other issue.  The
>>> following patch seems to also fix this issue (and other issues):
>>> https://www.spinics.net/lists/stable/msg172631.html
>>
>> Yeah, but it still looks to me like that the hardware got into some
>> weird state with the storm of ATS invalidations sent to it.
>>
>> The Completion-Wait loop timeouts seen in the original bug report
>> indicate that the IOMMU is waiting for a response that never comes. And
>> this is probably the ATS flush completion response from the GPU, as
>> disabling ATS on the GPU makes the issue disappear.
> 
> Yeah, it's weird.  My ack on the patch still stands.  Just adding some additional data.
> 

I just tested this patch without the previous ATS disabling patch (I 
verified that ATS was enabled).  Doing a stress-test kernel build while 
running a 3D graphical application caused no disk corruption.  That was 
running for several hours.  If it's not the solution, it sure hides the 
problem really well.

[toc] | [prev] | [next] | [standalone]

#1651344

> -----Original Message-----
> From: Joerg Roedel [mailto:jroedel@suse.de]
> Sent: Wednesday, May 24, 2017 4:45 AM
> To: Deucher, Alexander
> Cc: 'David Woodhouse'; 'Joerg Roedel'; Bjorn Helgaas; linux-
> pci@vger.kernel.org; linux-kernel@vger.kernel.org; Daniel Drake; Samuel
> Sieb
> Subject: Re: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs
> 
> Hi Alexander,
> 
> On Tue, May 23, 2017 at 07:54:12PM +0000, Deucher, Alexander wrote:
> > I finally got an answer from the hw team and we validated ATS on
> > stoney as well so in theory this patch shouldn’t actually be needed.
> > I think we may actually be papering over some other issue.  The
> > following patch seems to also fix this issue (and other issues):
> > https://www.spinics.net/lists/stable/msg172631.html
> 
> Yeah, but it still looks to me like that the hardware got into some
> weird state with the storm of ATS invalidations sent to it.
> 
> The Completion-Wait loop timeouts seen in the original bug report
> indicate that the IOMMU is waiting for a response that never comes. And
> this is probably the ATS flush completion response from the GPU, as
> disabling ATS on the GPU makes the issue disappear.

FWIW, the GPU driver does not actually use ATS at the moment so I don't think we should see any ATS transactions.

Alex

[toc] | [prev] | [next] | [standalone]

#1651370

[Multipart message — attachments visible in raw view] — view raw

On Fri, 2017-05-26 at 11:57 +0000, Deucher, Alexander wrote:
> 
> FWIW, the GPU driver does not actually use ATS at the moment so I
> don't think we should see any ATS transactions.

That's a confusing sentence. The "GPU driver", if you mean software
running in the OS, wouldn't be expected to have anything to do with
ATS.

ATS is something that the CPU itself (or its DMA engine) would do.
Instead of just performing a DMA transaction to a given bus address,
and letting the IOMMU do the translation, the hardware might choose to
first perform an IOTLB lookup, and then later do the actual DMA
transaction to the pre-translated, raw physical address. Which kind of
makes a mockery of any kind of protection the IOMMU is supposed to give
you, but does shave a cycle or two of latency off the DMA when it
finally happens, since the translation can be done in advance.

[toc] | [prev] | [next] | [standalone]

#1651457

> -----Original Message-----
> From: David Woodhouse [mailto:dwmw2@infradead.org]
> Sent: Friday, May 26, 2017 8:55 AM
> To: Deucher, Alexander; 'Joerg Roedel'
> Cc: 'Joerg Roedel'; Bjorn Helgaas; linux-pci@vger.kernel.org; linux-
> kernel@vger.kernel.org; Daniel Drake; Samuel Sieb
> Subject: Re: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs
> 
> On Fri, 2017-05-26 at 11:57 +0000, Deucher, Alexander wrote:
> >
> > FWIW, the GPU driver does not actually use ATS at the moment so I
> > don't think we should see any ATS transactions.
> 
> That's a confusing sentence. The "GPU driver", if you mean software
> running in the OS, wouldn't be expected to have anything to do with
> ATS.
> 
> ATS is something that the CPU itself (or its DMA engine) would do.
> Instead of just performing a DMA transaction to a given bus address,
> and letting the IOMMU do the translation, the hardware might choose to
> first perform an IOTLB lookup, and then later do the actual DMA
> transaction to the pre-translated, raw physical address. Which kind of
> makes a mockery of any kind of protection the IOMMU is supposed to give
> you, but does shave a cycle or two of latency off the DMA when it
> finally happens, since the translation can be done in advance.

+ John, Suravee

Full disclosure, I'm not by any means an expert with ATS.  I guess I'm thinking of PRI support rather than ATS per se.  On the GPU side the GPU's memory controller has multiple paths to system memory, the non-ATS/PRI path and the ATS/PRI path.  The GPU has its own integrated MMU to virtualize the GPU's internal address space per GPU client.  The non-ATS/PRI path uses the GPU's MMU and is just "regular" dma to addresses potentially translated by the IOMMU just like any other device that may not have ATS support.  The system memory has to be resident because if the GPU faults, it can't retry the transaction.  For the ATS/PRI path, the GPU's MMU is bypassed and PASIDs need to be setup on the IOMMU for each client, but once done, transactions that use that interface support retries on GPU page faults (after the OS had paged the memory in and the IOMMU tables been updated) and other features.  I think only the ATS/PRI case uses the ATC on the end point.  John, Suravee, correct me if I'm wrong.

Alex

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web