Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210429084410.783998-1-amistry@google.com> <20210429184101.RFC.v2.1.Iadd904c1764f3bfe260db30fe41bdb9b8f98533d@changeid>
 <878s4wdwyy.ffs@nanos.tec.linutronix.de>
In-Reply-To: <878s4wdwyy.ffs@nanos.tec.linutronix.de>
From:   "Anand K. Mistry" <amistry@google.com>
Date:   Tue, 11 May 2021 18:39:15 +1000
Message-ID: <CAATStaOmBw1nr9Y5pZw9MXK0Y4TMvyOe0M6-PZpy7RQnS0szyA@mail.gmail.com>
Subject: Re: [RFC PATCH v2 1/2] x86/speculation: Allow per-process control of
 when to issue IBPB
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     x86@kernel.org, Joel Fernandes <joelaf@google.com>,
        Anthony Steinhauser <asteinhauser@google.com>,
        Borislav Petkov <bp@alien8.de>,
        Andy Lutomirski <luto@kernel.org>,
        Ben Segall <bsegall@google.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        "Chang S. Bae" <chang.seok.bae@intel.com>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Fenghua Yu <fenghua.yu@intel.com>,
        Gabriel Krisman Bertazi <krisman@collabora.com>,
        "H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@redhat.com>,
        Jay Lang <jaytlang@mit.edu>, Jens Axboe <axboe@kernel.dk>,
        Juri Lelli <juri.lelli@redhat.com>,
        Kees Cook <keescook@chromium.org>,
        Lai Jiangshan <laijs@linux.alibaba.com>,
        Mel Gorman <mgorman@suse.de>, Mike Rapoport <rppt@kernel.org>,
        Oleg Nesterov <oleg@redhat.com>,
        Peter Collingbourne <pcc@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Tony Luck <tony.luck@intel.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, 3 May 2021 at 18:48, Thomas Gleixner <tglx@linutronix.de> 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
>
>