Re: [RFCv2 03/10] x86: Introduce userspace API to handle per-thread features

[Edgecombe, Rick P]

On Sun, 2022-05-15 at 21:38 +0200, Thomas Gleixner wrote:
> On Sun, May 15 2022 at 18:24, Edgecombe, Rick P wrote:
> > On Sun, 2022-05-15 at 11:02 +0200, Thomas Gleixner wrote:
> > > If it really turns out to be something which matters, then you
> > > can
> > > provide a batch interface later on if it makes sense to do so,
> > > but
> > > see
> > > above.
> > 
> > Thanks, sounds good to me.
> > 
> > Kirill, so I guess we can just change ARCH_THREAD_FEATURE_ENABLE/
> > ARCH_THREAD_FEATURE_DISABLE to return EINVAL if more than one bit
> > is
> > set. It returns 0 on success and whatever error code on failure.
> > Userspace can do whatever rollback logic it wants. What do you
> > think?
> 
> Why having this feature bit interface in the first place?

The idea was that we should not have duplicate interfaces if we can
avoid it. It of course grew out of the "elf feature bit" stuff, but we
considered splitting them after moving away from that. LAM and CET's
enabling needs seemed close enough to avoid having two interfaces.

> 
> It's going to be a demultiplex mechanism with incompatible
> arguments. Just look at LAM. What's really architecture specific
> about
> it?
> 
> The mechanism per se is architecture independent: pointer tagging.
> 
> What's architecture specific is whether it's supported, the address
> mask
> and the enable/disable mechanism.
> 
> So having e.g.
> 
>    prctl(POINTER_TAGGING_GET_MASK, &mask);
> 
> works on all architectures which support this. Ditto
> 
>    prctl(POINTER_TAGGING_ENABLE, &mask);
> 
> is architecture agnostic. Both need to be backed by an architecture
> specific implementation of course.
> 
> This makes it future proof because new CPUs could define the mask to
> be
> bit 57-61 and use bit 62 for something else. So from a user space
> perspective the mask retrival is useful because it's obvious and
> trivial
> to use and does not need code changes when the hardware
> implementation
> provides a different mask.

The lack of ability to pass extra arguments is a good point.

> 
> See?

Regarding making it arch specific or not, if the LAM interface can be
arch agnostic, then that makes sense to me. I guess some CPU features
(virtual memory, etc) are similar enough that the kernel can hide them
beyond common interfaces. Some aren't (cpuid, gs register, etc). If LAM
can be one of the former, then sharing an interface with other
architectures does seem much better.

I'm thinking CET is different enough from other similar features that
leaving it as an arch thing is probably appropriate. BTI is probably
the closest (to IBT). It uses it's own BTI specific elf header bit, and
requires special PROT on memory, unlike IBT.

> 
> The thread.features bitmap could still be used as an internal storage
> for enabled features, but having this as the primary programming
> interface is cumbersome and unflexible for anything which is not
> binary
> on/off.
> 
> Thanks,
> 
>         tglx
> 
>