Re: [PATCH v5] arm64: mte: allow async MTE to be upgraded to sync on a per-CPU basis

[Will Deacon <will@kernel.org>]

On Fri, Jun 25, 2021 at 02:53:50PM +0100, Catalin Marinas wrote:
> On Fri, Jun 25, 2021 at 01:39:59PM +0100, Will Deacon wrote:
> > On Fri, Jun 25, 2021 at 01:01:37PM +0100, Catalin Marinas wrote:
> > > So we can document that the mode requested by the app is an indication,
> > > the system may change it to another value (and back-port documentation
> > > to 5.10). If we get a request from developers to honour a specific mode,
> > > we can add a new PR_MTE_TCF_EXACT bit or something but it's not
> > > essential we do it now.
> > > 
> > > So if we allow the kernel to change the user requested mode (via sysfs),
> > > I think we still have two more issues to clarify:
> > > 
> > > 1. Do we allow only "upgrade" (for some meaning of this) or sysfs can
> > >    downgrade to a less strict mode. I'd go for upgrade here to a
> > >    stricter check as in Peter's patch.
> > > 
> > > 2. Should the sysfs upgrade the PR_MTE_TCF_NONE? _MTAG_ENABLE does that,
> > >    so I'd say yes.
> > > 
> > > Any other thoughts are welcome.
> > 
> > As I mentioned before, I think the sysfs interface should offer:
> > 
> > 	"task"	: Honour whatever the task has asked for (default)
> > 	"async" : Force async on this CPU
> > 	"sync"  : Force sync on this CPU
> > 
> > I don't think we should upgrade PR_MTE_TCF_NONE unless we also have a "none"
> > option in here. I originally suggested that, but in hindsight it feels like
> > a bad idea because a task could SIGILL on migration. So what we're saying is
> > that PR_MTE_TCF_SYNC and PR_MTE_TCF_ASYNC will always enable MTE on success,
> > but the reporting mode is a hint.
> > 
> > I don't think upgrade/downgrade makes a lot of sense given that the sysfs
> > controls can be changed at any point in time. It should just be an override.
> 
> The problem with sysfs is that it's global, so it assumes that any
> process has the same needs. The _MTAG_ENABLE glibc tunable at least can
> be set per process.

Again, this seems to be an argument against doing this at all. We already
have a per-task interface to change the checking mode, but there is a need
to override this on a per-cpu basis to achieve acceptable performance.
Applications can't possibly be aware of that and so their "needs" cannot be
taken into account here.

> > This means that we can force async for CPUs where sync mode is horribly
> > slow, whilst honouring the task's request on CPUs which are better
> > implemented.
> 
> This may hamper debugging on, for example, a system where the root
> configured the modes for CPUs and a normal user wants to use MTE to
> identify access bugs. Another case is some service that wants tightened
> security from MTE irrespective of the performance.

I suppose the way I see this is similar to how I see CPU errata: essentially
sync mode is unusably slow on some CPUs, so we disable it (drop to async) on
a per-cpu basis. The only difference is that we provide the switch to
userspace, since there isn't a functional problem. However, when we
inevitably hit real errata, we could even force the mode in the kernel
rather than disable the feature entirely.

> The slight downside of the "upgrade" mode assumes that the user is aware
> that async is the fastest and asks for this unless it has specific
> needs. Of course, we can also extend the interface to "sync-force" or
> "sync-upgrade" etc. but I think it's over-engineered.

Another reason I dislike "upgrade" is because it means that the kernel embeds
an ordering of which mode upgrades to another mode and, as new modes get
added by the architecture in future, this feels more like policy to me and
would be better off handled in userspace.

Will

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