LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Will Deacon <will.deacon@arm.com>
To: Rob Clark <robdclark@gmail.com>
Cc: "list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>,"
	<iommu@lists.linux-foundation.org>,
	Robin Murphy <robin.murphy@arm.com>,
	"moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
	<linux-arm-kernel@lists.infradead.org>,
	freedreno <freedreno@lists.freedesktop.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	"list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>," <joro@8bytes.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] iommu: arm-smmu: Set SCTLR.HUPCF bit
Date: Tue, 13 Nov 2018 06:32:26 +0000	[thread overview]
Message-ID: <20181113063225.GA3109@brain-police> (raw)
In-Reply-To: <CAF6AEGtgC0V6ii2=p2HGmqvHFKNJhaOLi8873SGPDsrg70xGRg@mail.gmail.com>

On Fri, Nov 09, 2018 at 01:01:55PM -0500, Rob Clark wrote:
> On Mon, Oct 29, 2018 at 3:09 PM Will Deacon <will.deacon@arm.com> wrote:
> > On Thu, Sep 27, 2018 at 06:46:07PM -0400, Rob Clark wrote:
> > > We seem to need to set either this or CFCFG (stall), otherwise gpu
> > > faults trigger problems with other in-flight transactions from the
> > > GPU causing CP errors, etc.
> > >
> > > In the ARM SMMU spec, the 'Hit under previous context fault' bit is
> > > described as:
> > >
> > >  '0' - Stall or terminate subsequent transactions in the presence
> > >        of an outstanding context fault
> > >  '1' - Process all subsequent transactions independently of any
> > >        outstanding context fault.
> > >
> > > Since we don't enable CFCFG (stall) the behavior of terminating
> > > other transactions makes sense.  And is probably not what we want
> > > (and definately not what we want for GPU).
> > >
> > > Signed-off-by: Rob Clark <robdclark@gmail.com>
> > > ---
> > > So I hit this issue a long time back on 820 (msm8996) and at the
> > > time I solved it with a patch that enabled CFCFG.  And it resurfaced
> > > more recently on sdm845.  But at the time CFCFG was rejected, iirc
> > > because of concern that it would cause problems on other non-qcom
> > > arm smmu implementations.  And I think I forgot to send this version
> > > of the solution.
> > >
> > > If enabling HUPCF is anticipated to cause problems on other ARM
> > > SMMU implementations, I think I can come up with a variant of this
> > > patch which conditionally enables it for snapdragon.
> > >
> > > Either way, I'd really like to get some variant of this fix merged
> > > (and probably it would be a good idea for stable kernel branches
> > > too), since current behaviour with the GPU means faults turn into
> > > a fantastic cascade of fail.
> >
> > Can you describe how this fantastic cascade of fail improves with this
> > patch, please? If you're getting context faults then something has already
> > gone horribly wrong, so I'm trying to work out how this improves things.
> >
> 
> There are plenty of cases where getting iommu faults with a GPU is
> "normal", or at least not something the kernel or even GL driver can
> control.

Such as? All the mainline driver does is print a diagnostic and clear the
fault, which doesn't seem generally useful.

> With this patch, you still get the iommu fault, but it doesn't cause
> the gpu to crash.  But without it, other memory accesses in flight
> while the fault occurs, like the GPU command-processor reading further
> ahead in the cmdstream to setup next draw, would return zero's,
> causing the GPU to crash or get into a bad state.

I get that part, but I don't understand why we're seeing faults in the first
place and I worry that this patch is just the tip of the iceberg. It's also
not clear that processing subsequent transactions is always the right thing
to do in a world where we actually want to report (and handle) synchronous
faults from devices.

Will

  reply	other threads:[~2018-11-13  6:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-27 22:46 Rob Clark
2018-10-29 19:10 ` Will Deacon
2018-11-09 18:01   ` Rob Clark
2018-11-13  6:32     ` Will Deacon [this message]
2018-11-13 13:12       ` Rob Clark
2018-11-26 19:31         ` Will Deacon
2018-11-26 20:38           ` Jordan Crouse
2018-11-26 20:56           ` Rob Clark
2019-05-24 18:38           ` Rob Clark

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181113063225.GA3109@brain-police \
    --to=will.deacon@arm.com \
    --cc=freedreno@lists.freedesktop.org \
    --cc=iommu@lists.linux-foundation.org \
    --cc=joro@8bytes.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robdclark@gmail.com \
    --cc=robin.murphy@arm.com \
    --subject='Re: [PATCH] iommu: arm-smmu: Set SCTLR.HUPCF bit' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).