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

[Peter Collingbourne]

On Fri, Jun 18, 2021 at 8:10 AM Catalin Marinas <catalin.marinas at arm.com> wrote:
>
> On Thu, Jun 17, 2021 at 10:58:30PM +0100, Will Deacon wrote:
> > On Tue, Jun 15, 2021 at 01:38:07PM -0700, Peter Collingbourne wrote:
> > > +Upgrading to stricter tag checking modes
> > > +----------------------------------------
> > > +
> > > +On some CPUs the performance of MTE in stricter tag checking modes
> > > +is similar to that of less strict tag checking modes. This makes it
> > > +worthwhile to enable stricter checks on those CPUs when a less strict
> > > +checking mode is requested, in order to gain the error detection
> > > +benefits of the stricter checks without the performance downsides. To
> > > +opt into upgrading to a stricter checking mode on those CPUs, the user
> > > +can set the ``PR_MTE_DYNAMIC_TCF`` flag bit in the ``flags`` argument
> > > +to the ``prctl(PR_SET_TAGGED_ADDR_CTRL, flags, 0, 0, 0)`` system call.
> > > +
> > > +This feature is currently only supported for upgrading from
> > > +asynchronous mode. To configure a CPU to upgrade from asynchronous mode
> > > +to synchronous mode, a privileged user may write the value ``1`` to
> > > +``/sys/devices/system/cpu/cpu<N>/mte_upgrade_async``, and to disable
> > > +upgrading they may write the value ``0``. By default the feature is
> > > +disabled on all CPUs.
> > > +
> > >  Initial process state
> > >  ---------------------
> > >
> > > @@ -128,6 +147,7 @@ On ``execve()``, the new process has the following configuration:
> > >  - ``PR_TAGGED_ADDR_ENABLE`` set to 0 (disabled)
> > >  - Tag checking mode set to ``PR_MTE_TCF_NONE``
> > >  - ``PR_MTE_TAG_MASK`` set to 0 (all tags excluded)
> > > +- ``PR_MTE_DYNAMIC_TCF`` set to 0 (disabled)
> > >  - ``PSTATE.TCO`` set to 0
> > >  - ``PROT_MTE`` not set on any of the initial memory maps
> >
> > Something about this doesn't sit right with me, as we're mixing a per-task
> > interface with a per-cpu interface for selecting async/sync MTE and the
> > priorities are somewhat confusing.
> >
> > I think a better interface would be for the sysfs entry for each CPU to
> > allow selection between:
> >
> >       task  : Honour the prctl() (current behaviour)
> >       async : Force async for tasks using MTE
> >       sync  : Force sync for tasks using MTE
> >       none  : MTE disabled
> >
> > i.e. the per-cpu setting is an override.
>
> As Peter mentioned, forcing it is a potential ABI break, so such feature
> would need backporting to 5.10. There's also a minor use-case that came
> up in the early discussions - an app may want to use async mode only for
> reporting but forcing it to sync would break such application (since
> sync mode prevents the faulty access from taking place).
>
> So I'd rather leave it up to the user task to decide whether its choice
> can be changed. Peter introduced a new PR_MTE_DYNAMIC_TCF for this
> purpose (or a different name if you have a better suggestion).
>
> I think the other important question is whether we go for an override
> style or an upgrade one. Peter chose the latter, though I think an
> override is simpler to understand.
>
> BTW, I like the idea of using strings in the sysfs interface than
> numbers.

Agreed on the strings in the sysfs interface; done in v6.

Peter