Re: [RFC PATCH v2 1/2] x86/speculation: Allow per-process control of when to issue IBPB

[Anand K. Mistry]

On Mon, 3 May 2021 at 18:48, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>
> Anand,
>
> On Thu, Apr 29 2021 at 18:44, Anand K. Mistry wrote:
> >
> > -static inline unsigned long mm_mangle_tif_spec_ib(struct task_struct *next)
> > +static inline unsigned long mm_mangle_tif_spec_ib_on_leave(
> > +     struct task_struct *next)
> >  {
> >       unsigned long next_tif = task_thread_info(next)->flags;
> > -     unsigned long ibpb = (next_tif >> TIF_SPEC_IB) & LAST_USER_MM_IBPB;
> > +     unsigned long spec_disabled =
> > +             (next_tif >> TIF_SPEC_IB) & ~(next_tif >> TIF_NO_IBPB_LEAVE);
> > +     unsigned long ibpb = spec_disabled & LAST_USER_MM_IBPB;
> >
> >       return (unsigned long)next->mm | ibpb;
> >  }
> >
> > +static inline bool ibpb_on_entry(struct task_struct *next)
> > +{
> > +     unsigned long next_tif = task_thread_info(next)->flags;
> > +     unsigned long spec_disabled =
> > +             (next_tif >> TIF_SPEC_IB) & ~(next_tif >> TIF_NO_IBPB_ENTRY);
> > +     return spec_disabled & 1;
> > +}
>
> Why exactly do we need _three_ TIF bits and this non-intuitive inverted
> logic?

The inverted logic is to avoid any side effects of flag bits
defaulting to 0, which should keep the existing behaviour of doing an
IBPB on both entry and exit.

>
> The existing mode is: Issue IBPB when scheduling in and when scheduling out.
>
> Ergo the obvious approach for making it more finegrained is to split the
> existing TIF_SPEC_IB into TIF_SPEC_IB_IN and TIF_SPEC_IB_OUT and have
> the existing mode both bits set.

Agreed. The problem is the existing bit doesn't just control IBPB
behaviour. It also controls STIBP, which you can kinda control
separately on the command line when booting with the spectre_v2_user
flag. The code, however, uses the same flag for both (see
arch/x86/include/asm/spec-ctrl.h).

Maybe this is another cleanup I should do prior to this change?
Separating IBPB and STIBP flags in the implementation.

>
> Thanks,
>
>         tglx
>
>