linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Rob Clark <robdclark@gmail.com>
To: Doug Anderson <dianders@chromium.org>
Cc: linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Will Deacon <will.deacon@arm.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Evan Green <evgreen@chromium.org>,
	"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>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] iommu/arm-smmu: Allow disabling bypass via kernel config
Date: Fri, 15 Feb 2019 17:37:41 -0500	[thread overview]
Message-ID: <CAF6AEGsGc8g--3W_xpR8VUt778WR19v_HOemhd7Jv=f4QBqVgw@mail.gmail.com> (raw)
In-Reply-To: <CAD=FV=WeBD0bqxWPZcV1dz5Sdn47Ap=FZU3Y+YPbxsVCQEUcLQ@mail.gmail.com>

On Thu, Feb 14, 2019 at 7:40 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Thu, Feb 14, 2019 at 1:32 PM Robin Murphy <robin.murphy@arm.com> wrote:
> >
> > Hi Doug,
> >
> > On 2019-02-14 8:44 pm, Douglas Anderson wrote:
> > > Right now the only way to disable the iommu bypass for the ARM SMMU is
> > > with the kernel command line parameter 'arm-smmu.disable_bypass'.
> > >
> > > In general kernel command line parameters make sense for things that
> > > someone would like to tweak without rebuilding the kernel or for very
> > > basic communication between the bootloader and the kernel, but are
> > > awkward for other things.  Specifically:
> > > * Human parsing of the kernel command line can be difficult since it's
> > >    just a big runon space separated line of text.
> > > * If every bit of the system was configured via the kernel command
> > >    line the kernel command line would get very large and even more
> > >    unwieldly.
> > > * Typically there are not easy ways in build systems to adjust the
> > >    kernel command line for config-like options.
> > >
> > > Let's introduce a new config option that allows us to disable the
> > > iommu bypass without affecting the existing default nor the existing
> > > ability to adjust the configuration via kernel command line.
> >
> > I say let's just flip the default - for a while now it's been one of
> > those "oh yeah, we should probably do that" things that gets instantly
> > forgotten again, so some 3rd-party demand is plenty to convince me :)
> >
> > There are few reasons to allow unmatched stream bypass, and even fewer
> > good ones, so I'd be happy to shift the command-line burden over to the
> > esoteric cases at this point, and consider the config option in future
> > if anyone from that camp pops up and screams hard enough.
>
> Sure, I can submit that patch if we want.  I presume I'll get lots of
> screaming but I'm used to that.  ;-)
>
> ...specifically I found that when I turned on "disably bypass" on my
> board (sdm845-cheza, which is not yet upstream) that a bunch of things
> that used to work broke.  That's a good thing because all the things
> that broke need to be fixed properly (by adding the IOMMUs) but
> presumably my board is not special in relying on the old insecure
> behavior.

So one niggly bit, beyond the drivers that aren't but should be using
iommu, is the case where bootloader lights up the display.  We
actually still have a lot of work to do for this (in that clk and
genpd drivers need to be aware of handover, in addition to just
iommu)..  But I'd rather not make a hard problem harder just yet.

(it gets worse, afaict so far on the windows-arm laptops since in that
case uefi/edk is actually taking the iommu out of bypass and things go
badly when arm-smmu disables clks after probe..)

But I might recommend making the default a kconfig option for now, so
in the distro kernel case, where display driver is a kernel module,
you aren't making a hard problem harder.  For cases where bootloader
isn't enabling display, things are much easier, and I think we could
switch the default sooner.  But I fear for cases where bootloader is
enabling display it is a much harder problem :-(

BR,
-R


>
> I'm about to head on vacation for a week so I'm not sure I'll get to
> re-post before then.  If not I'll post this sometime after I get back
> unless someone beats me to it.
>
> -Doug
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-02-15 22:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-14 20:44 [PATCH] iommu/arm-smmu: Allow disabling bypass via kernel config Douglas Anderson
2019-02-14 21:31 ` Robin Murphy
2019-02-15  0:40   ` Doug Anderson
2019-02-15 22:37     ` Rob Clark [this message]
2019-03-01 19:21       ` Doug Anderson

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='CAF6AEGsGc8g--3W_xpR8VUt778WR19v_HOemhd7Jv=f4QBqVgw@mail.gmail.com' \
    --to=robdclark@gmail.com \
    --cc=dianders@chromium.org \
    --cc=evgreen@chromium.org \
    --cc=iommu@lists.linux-foundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=will.deacon@arm.com \
    /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 NNTP newsgroup(s).