IOMMU Archive mirror
 help / color / mirror / Atom feed
From: Doug Anderson <dianders@chromium.org>
To: Will Deacon <will@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>,
	Joerg Roedel <joro@8bytes.org>,
	 Stephen Boyd <swboyd@chromium.org>,
	Chen Lin <chen45464546@163.com>,
	iommu@lists.linux.dev,  linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iommu/arm-smmu: Don't disable next-page prefetcher on devices it works on
Date: Fri, 17 May 2024 10:19:29 -0700	[thread overview]
Message-ID: <CAD=FV=UEXjD=w41Hj_gE--DXhkSjNdfPnkc7X=FrZJ5_90Jq0g@mail.gmail.com> (raw)
In-Reply-To: <20240517163742.GA525@willie-the-truck>

Hi,

On Fri, May 17, 2024 at 9:37 AM Will Deacon <will@kernel.org> wrote:
>
> Hi Doug,
>
> On Mon, May 13, 2024 at 04:13:47PM -0700, Douglas Anderson wrote:
> > On sc7180 trogdor devices we get a scary warning at bootup:
> >   arm-smmu 15000000.iommu:
> >   Failed to disable prefetcher [errata #841119 and #826419], check ACR.CACHE_LOCK
> >
> > We spent some time trying to figure out how we were going to fix these
> > errata and whether we needed to do a firmware update. Upon closer
> > inspection, however, we realized that the errata don't apply to us.
> > Specifically, the errata document says that for these errata:
> > * Found in: r0p0
> > * Fixed in: r2p2
> >
> > ...and trogdor devices appear to be running r2p4. That means that they
> > are unaffected despite the scary warning.
> >
> > The issue is that the kernel unconditionally tries to disable the
> > prefetcher even on unaffected devices and then warns when it's unable
> > to.
> >
> > Let's change the kernel to only disable the prefetcher on affected
> > devices, which will get rid of the scary warning on devices that are
> > unaffected. As per the comment the prefetcher is
> > "not-particularly-beneficial" but it shouldn't hurt to leave it on for
> > devices where it doesn't cause problems.
> >
> > Fixes: f87f6e5b4539 ("iommu/arm-smmu: Warn once when the perfetcher errata patch fails to apply")
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> >
> >  drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 21 +++++++++++++--------
> >  1 file changed, 13 insertions(+), 8 deletions(-)
>
>
> Just curious, but did you see any performance impact (good or bad) as a
> result of this patch? The next-page prefetcher has always looked a little
> naive to me and, with a tendency for tiny TLBs in some implementations,
> there's a possibility it could do more harm than good.

This patch actually makes no difference on trogdor today other than
getting rid of the scary warning. Specifically on trogdor the
ACR.CACHE_LOCK bit seems to be set so the kernel is unable to change
the setting anyway and has never been able to. We are working on
figuring out how to fix the firmware and then we have to get a
firmware spin before we can really see any changes. I'll keep an eye
out to see if performance numbers change when the firmware uprevs.

BTW: any idea how big of a deal these errata are? We're _just_
finishing a firmware uprev process and there is always pushback
against kicking off a new one unless the issue is important. Given
that we've been living with this issue since devices shipped I'm going
to assume we don't need to rush a firmware update, but if this is
really scary and needs to be addressed sooner we can figure that out.

-Doug

      reply	other threads:[~2024-05-17 17:19 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-13 23:13 [PATCH] iommu/arm-smmu: Don't disable next-page prefetcher on devices it works on Douglas Anderson
2024-05-14 17:14 ` Robin Murphy
2024-05-17 16:37 ` Will Deacon
2024-05-17 17:19   ` Doug Anderson [this message]

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='CAD=FV=UEXjD=w41Hj_gE--DXhkSjNdfPnkc7X=FrZJ5_90Jq0g@mail.gmail.com' \
    --to=dianders@chromium.org \
    --cc=chen45464546@163.com \
    --cc=iommu@lists.linux.dev \
    --cc=joro@8bytes.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=swboyd@chromium.org \
    --cc=will@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).