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

From: Peter Collingbourne <pcc@google.com>
To: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Szabolcs Nagy <szabolcs.nagy@arm.com>,
	 Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Evgenii Stepanov <eugenis@google.com>,
	 Linux ARM <linux-arm-kernel@lists.infradead.org>,
	 Tejas Belagod <Tejas.Belagod@arm.com>
Subject: Re: [PATCH v5] arm64: mte: allow async MTE to be upgraded to sync on a per-CPU basis
Date: Tue, 29 Jun 2021 12:11:17 -0700	[thread overview]
Message-ID: <CAMn1gO5_dWJtfpSkB2CGHb8owXJ3wOqep1F56rdnRpQPw9+FBg@mail.gmail.com> (raw)
In-Reply-To: <20210629104625.GA7168@willie-the-truck>

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.

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.

Peter

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