2020-10-29 09:02:15

by Anand K. Mistry

[permalink] [raw]
Subject: [PATCH 0/1] x86/speculation: Allow IBPB to be conditionally enabled on CPUs with always-on STIBP

When attempting to do some performance testing of IBPB on and AMD
platform, I noticed the IBPB instruction was never being issued, even
though it was conditionally on and various seccomp protected processes
were force enabling it. Turns out, on those AMD CPUs, STIBP is set to
always-on and this was causing an early-out on the prctl() which turns
off IB speculation. Here is my attempt to fix it.

I'm hoping someone that understands this better than me can explain why
I'm wrong.


Anand K Mistry (1):
x86/speculation: Allow IBPB to be conditionally enabled on CPUs with
always-on STIBP

arch/x86/kernel/cpu/bugs.c | 41 +++++++++++++++++++++-----------------
1 file changed, 23 insertions(+), 18 deletions(-)

--
2.29.1.341.ge80a0c044ae-goog


2020-10-31 14:52:21

by Tom Lendacky

[permalink] [raw]
Subject: Re: [PATCH 0/1] x86/speculation: Allow IBPB to be conditionally enabled on CPUs with always-on STIBP

On 10/29/20 1:51 AM, Anand K Mistry wrote:
> When attempting to do some performance testing of IBPB on and AMD
> platform, I noticed the IBPB instruction was never being issued, even
> though it was conditionally on and various seccomp protected processes
> were force enabling it. Turns out, on those AMD CPUs, STIBP is set to
> always-on and this was causing an early-out on the prctl() which turns
> off IB speculation. Here is my attempt to fix it.
>
> I'm hoping someone that understands this better than me can explain why
> I'm wrong.

It all looks reasonable to me (some comments in the patch to follow). The
thing that makes this tough is the command line option of being able to
force IBPB using the "prctl,ibpb" or "seccomp,ibpb" while STIBP is prctl
or seccomp controlled. There's an inherent quality that is assumed that if
STIBP is forced then IBPB must be forced and it looks like 21998a351512
("x86/speculation: Avoid force-disabling IBPB based on STIBP and enhanced
IBRS.") used that. However, with the STIBP always on support, that doesn't
hold true.

Thanks,
Tom

>
>
> Anand K Mistry (1):
> x86/speculation: Allow IBPB to be conditionally enabled on CPUs with
> always-on STIBP
>
> arch/x86/kernel/cpu/bugs.c | 41 +++++++++++++++++++++-----------------
> 1 file changed, 23 insertions(+), 18 deletions(-)
>

2020-11-02 00:00:58

by Anand K. Mistry

[permalink] [raw]
Subject: Re: [PATCH 0/1] x86/speculation: Allow IBPB to be conditionally enabled on CPUs with always-on STIBP

On Sun, 1 Nov 2020 at 01:50, Tom Lendacky <[email protected]> wrote:
>
> On 10/29/20 1:51 AM, Anand K Mistry wrote:
> > When attempting to do some performance testing of IBPB on and AMD
> > platform, I noticed the IBPB instruction was never being issued, even
> > though it was conditionally on and various seccomp protected processes
> > were force enabling it. Turns out, on those AMD CPUs, STIBP is set to
> > always-on and this was causing an early-out on the prctl() which turns
> > off IB speculation. Here is my attempt to fix it.
> >
> > I'm hoping someone that understands this better than me can explain why
> > I'm wrong.
>
> It all looks reasonable to me (some comments in the patch to follow). The
> thing that makes this tough is the command line option of being able to
> force IBPB using the "prctl,ibpb" or "seccomp,ibpb" while STIBP is prctl
> or seccomp controlled. There's an inherent quality that is assumed that if
> STIBP is forced then IBPB must be forced and it looks like 21998a351512
> ("x86/speculation: Avoid force-disabling IBPB based on STIBP and enhanced
> IBRS.") used that. However, with the STIBP always on support, that doesn't
> hold true.

Yeah, and this is what I found confusing. With that commit, the number
of combinations
of IBPB and STIBP is 25, but only a small subset is possible. For example:
- (spectre_v2_user_ibpb == SPECTRE_V2_USER_STRICT &&
spectre_v2_user_stibp == SPECTRE_V2_USER_STRICT)
- (spectre_v2_user_ibpb == SPECTRE_V2_USER_STRICT &&
spectre_v2_user_stibp == SPECTRE_V2_USER_NONE)
are the only possible combinations of STRICT.

But also, if 'spectre_v2_user=seccomp,ibpb' (or prctl,ibpb), then
spectre_v2_user_ibpb == SPECTRE_V2_USER_SECCOMP even though it is
logically SPECTRE_V2_USER_STRICT.

It took a bit of time to wrap my head around this, hence I'm a bit
hesitant about this change (even though I think it's right).

>
> Thanks,
> Tom
>
> >
> >
> > Anand K Mistry (1):
> > x86/speculation: Allow IBPB to be conditionally enabled on CPUs with
> > always-on STIBP
> >
> > arch/x86/kernel/cpu/bugs.c | 41 +++++++++++++++++++++-----------------
> > 1 file changed, 23 insertions(+), 18 deletions(-)
> >