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

Groups > linux.kernel > #1684415 > 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 Bjorn Helgaas <helgaas@kernel.org> - 2017-07-10 19:00 +0200
    Re: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs Joerg Roedel <jroedel@suse.de> - 2017-07-11 14:00 +0200
      RE: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs "Deucher, Alexander" <Alexander.Deucher@amd.com> - 2017-07-11 21:10 +0200

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

On Fri, Jun 16, 2017 at 06:29:23PM +0200, Joerg Roedel wrote:
> Hi Bjorn,
> 
> On Thu, Jun 15, 2017 at 02:15:45PM -0500, Bjorn Helgaas wrote:
> > It was marked "superseded" in patchwork and thus off my radar.  I
> > don't remember if I did that or why.  I changed it back to "New" so I
> > won't forget about it.
> 
> Great!
> 
> > You mention (May 24) the original bug report.  Can you include the URL
> > for that?
> 
> I think there were multiple reports, here is one I could still find:
> 
> 	https://lists.linuxfoundation.org/pipermail/iommu/2017-March/020836.html
> 
> > I admit I just don't have warm fuzzies that the problem is well
> > understood.
> 
> The current understanding (without my ability to debug the hardware
> involved) is that the GPU in the Stoney systems gets into a weird state
> when ATS invalidations are sent too fast and stops responding to the
> iommu.
> 
> The iommu then can't complete the invalidation commands and the driver
> throws completion-wait loop timeout messages out.

I'm still confused.  Per Samuel
(6dd9dbac-9b65-bc7c-bb08-413a05d09fc8@sieb.net):

Samuel> The other patch seems to fix this issue without disabling ATS.
Samuel> Isn't that better?

and Alex
(BN6PR12MB1652DF4130FC792B71DD9974F7C00@BN6PR12MB1652.namprd12.prod.outlook.com):

Alex> I talked to our validation team and ATS was validated on Stoney,
Alex> so this patch is just working around something else.  The other
Alex> patch fixes it and is a valid optimization ...

I'm confused about what this "other patch" is and whether we want that
one, this one, or both.

Bjorn

[toc] | [next] | [standalone]

#1684994

Hi Bjorn,

On Mon, Jul 10, 2017 at 11:53:58AM -0500, Bjorn Helgaas wrote:
> I'm still confused.  Per Samuel
> (6dd9dbac-9b65-bc7c-bb08-413a05d09fc8@sieb.net):
> 
> Samuel> The other patch seems to fix this issue without disabling ATS.
> Samuel> Isn't that better?
> 
> and Alex
> (BN6PR12MB1652DF4130FC792B71DD9974F7C00@BN6PR12MB1652.namprd12.prod.outlook.com):
> 
> Alex> I talked to our validation team and ATS was validated on Stoney,
> Alex> so this patch is just working around something else.  The other
> Alex> patch fixes it and is a valid optimization ...
> 
> I'm confused about what this "other patch" is and whether we want that
> one, this one, or both.

The other patches floating around lowered the ATS flush-rate from the
AMD IOMMU driver, which makes the issue disappear as well. But the issue
only disappeared, it is not solved and could probably still be
reproduced with a GPU usage pattern that increases the ATS flush-rate.

So blacklisting the device for ATS is still the safest thing we could do
here.


Regards,

	Joerg

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

#1685302

> -----Original Message-----
> From: Joerg Roedel [mailto:jroedel@suse.de]
> Sent: Tuesday, July 11, 2017 7:50 AM
> To: Bjorn Helgaas
> Cc: Bjorn Helgaas; linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org;
> Daniel Drake; Deucher, Alexander; Samuel Sieb; David Woodhouse
> Subject: Re: [PATCH v2] PCI: Add ATS-disable quirk for AMD Stoney GPUs
> 
> Hi Bjorn,
> 
> On Mon, Jul 10, 2017 at 11:53:58AM -0500, Bjorn Helgaas wrote:
> > I'm still confused.  Per Samuel
> > (6dd9dbac-9b65-bc7c-bb08-413a05d09fc8@sieb.net):
> >
> > Samuel> The other patch seems to fix this issue without disabling ATS.
> > Samuel> Isn't that better?
> >
> > and Alex
> >
> (BN6PR12MB1652DF4130FC792B71DD9974F7C00@BN6PR12MB1652.namprd1
> 2.prod.outlook.com):
> >
> > Alex> I talked to our validation team and ATS was validated on Stoney,
> > Alex> so this patch is just working around something else.  The other
> > Alex> patch fixes it and is a valid optimization ...
> >
> > I'm confused about what this "other patch" is and whether we want that
> > one, this one, or both.

Here's the other patch:
https://lists.freedesktop.org/archives/amd-gfx/2017-May/009421.html

> 
> The other patches floating around lowered the ATS flush-rate from the
> AMD IOMMU driver, which makes the issue disappear as well. But the issue
> only disappeared, it is not solved and could probably still be
> reproduced with a GPU usage pattern that increases the ATS flush-rate.
> 
> So blacklisting the device for ATS is still the safest thing we could do
> here.

I don't have any objection per se, but I'd hate to add a quirk to disable it only to remove it again in the future if we needed ATS related functionality later.  We are in the process of upstreaming KFD support for Carrizo (which is a bigger version of Stoney) and that utilizes ATS related functionality to provide GPU access to pageable memory.  There are no immediate requirements for Stoney, but that may change.

Alex

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web