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

[Catalin Marinas <catalin.marinas@arm.com>]

On Tue, Jun 29, 2021 at 12:11:17PM -0700, Peter Collingbourne wrote:
> On Tue, Jun 29, 2021 at 3:46 AM Will Deacon <will@kernel.org> wrote:
> > On Mon, Jun 28, 2021 at 04:20:24PM +0100, Catalin Marinas wrote:
> > > Another option is a mapping table where async can be remapped to sync
> > > and sync to async (or even to "none" for both). That's not far from one
> > > of Peter's mte-upgrade-async proposal, we just add mte-map-async and
> > > mte-map-sync options. Most likely we'll just use mte-map-async for now
> > > to map it to sync on some CPUs but it wouldn't exclude other forced
> > > settings.
> >
> > Catalin and I discussed this offline and ended up with another option:
> > retrospectively change the prctl() ABI so that the 'flags' argument
> > accepts a bitmask of modes that the application is willing to accept. This
> > doesn't break any existing users, as we currently enforce that only one
> > mode is specified, but it would allow things like:
> >
> >         prctl(PR_SET_TAGGED_ADDR_CTRL,
> >               PR_MTE_TCF_SYNC | PR_MTE_TCF_ASYNC,
> >               0, 0, 0);
> >
> > which is actually very similar to Peter's PR_MTE_DYNAMIC_TCF proposal, with
> > the difference that I think this extends more naturally as new PR_MTR_TCF_*
> > flags are introduced.
> >
> > Then we expose a per-cpu file in sysfs (say "cpuX/mte_tcf_preferred")
> > which initially reads as "async". If the root user does, e.g.
> >
> >         # echo "sync" > cpu1/mte_tcf_preferred
> >
> > then a task which has successfully issued a PR_SET_TAGGED_ADDR_CTRL prctl()
> > request for PR_MTE_TCF_SYNC | PR_MTE_TCF_ASYNC will run in sync mode on
> > CPU1, but async mode on other CPUs (assuming they retain the default value).
> >
> > We'll need to special-case PR_MTE_TCF_NONE, as that's just a shorthand for
> > "no flags" so doing PR_MTE_TCF_NONE | PR_MTE_TCF_SYNC is just the same as
> > doing PR_MTE_TCF_SYNC (which I think is already the behaviour today). The
> > only values which the sysfs files would accept today are "sync" and "async".
> >
> > When faced with a situation where the prctl() flags for a task do not
> > intersect with the preferred mode for a CPU on which the task is going
> > to run, the lowest bit number flag is chosen from the mask set by the
> > prctl().
> >
> > Thoughts?
> 
> This all sounds great and I'm glad you were able to come to an
> agreement on this. I'll get started on implementing it.
> 
> Once we have ASYM support I'm not sure if we can rely on bit numbering
> for ordering, as we will want the ordering to be ASYNC < ASYM < SYNC
> and ASYNC is bit-adjacent to SYNC. So I think we will need to make
> ASYM a special case.

The bit position based order - SYNC < ASYNC < ASYM - is indeed arbitrary
but I think it's easier to follow. When we add ASYM, it will be the last
one rather than squeezing it in the middle of the current order. Any
other order somehow implies that one is better than the other but we
don't have a clear definition for "better".

> This would also allow NONE to be upgraded by allocating a bit position
> for NONE, but if we change the value of NONE it may break applications
> built against new headers running on old kernels, so maybe it should
> be made a separate constant. This doesn't need to be done immediately,
> though.

I think we should leave NONE as non-upgradable, the software doesn't
expect any fault when accessing with the wrong tag. We also can't change
the definition now as it's already in upstream glibc.

-- 
Catalin

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