2018-01-13 01:07:39

by Tom Lendacky

[permalink] [raw]
Subject: [PATCH v1] x86/retpoline: Use lfence in the retpoline/RSB filling RSB macros

The pause instruction is currently used in the retpoline and RSB filling
macros as a speculation trap. The use of pause was originally suggested
because it showed a very, very small difference in the amount of
cycles/time used to execute the retpoline as compared to lfence. On AMD,
the pause instruction is not a serializing instruction, so the pause/jmp
loop will use excess power as it is speculated over waiting for return
to mispredict to the correct target.

The RSB filling macro is applicable to AMD, and, if software is unable to
verify that lfence is serializing on AMD (possible when running under a
hypervisor), the generic retpoline support will be used and, so, is also
applicable to AMD. Change the use of pause to lfence.

Signed-off-by: Tom Lendacky <[email protected]>
---
arch/x86/include/asm/nospec-branch.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 402a11c..2c4a09a 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -11,7 +11,7 @@
* Fill the CPU return stack buffer.
*
* Each entry in the RSB, if used for a speculative 'ret', contains an
- * infinite 'pause; jmp' loop to capture speculative execution.
+ * infinite 'lfence; jmp' loop to capture speculative execution.
*
* This is required in various cases for retpoline and IBRS-based
* mitigations for the Spectre variant 2 vulnerability. Sometimes to
@@ -37,12 +37,12 @@
771: \
call 772f; \
773: /* speculation trap */ \
- pause; \
+ lfence; \
jmp 773b; \
772: \
call 774f; \
775: /* speculation trap */ \
- pause; \
+ lfence; \
jmp 775b; \
774: \
dec reg; \
@@ -72,7 +72,7 @@
.macro RETPOLINE_JMP reg:req
call .Ldo_rop_\@
.Lspec_trap_\@:
- pause
+ lfence
jmp .Lspec_trap_\@
.Ldo_rop_\@:
mov \reg, (%_ASM_SP)
@@ -164,7 +164,7 @@
" jmp 904f;\n" \
" .align 16\n" \
"901: call 903f;\n" \
- "902: pause;\n" \
+ "902: lfence;\n" \
" jmp 902b;\n" \
" .align 16\n" \
"903: addl $4, %%esp;\n" \


2018-01-13 01:53:37

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH v1] x86/retpoline: Use lfence in the retpoline/RSB filling RSB macros

On Fri, Jan 12, 2018 at 5:07 PM, Tom Lendacky <[email protected]> wrote:
> The pause instruction is currently used in the retpoline and RSB filling
> macros as a speculation trap. The use of pause was originally suggested
> because it showed a very, very small difference in the amount of
> cycles/time used to execute the retpoline as compared to lfence. On AMD,
> the pause instruction is not a serializing instruction, so the pause/jmp
> loop will use excess power as it is speculated over waiting for return
> to mispredict to the correct target.
>
> The RSB filling macro is applicable to AMD, and, if software is unable to
> verify that lfence is serializing on AMD (possible when running under a
> hypervisor), the generic retpoline support will be used and, so, is also
> applicable to AMD. Change the use of pause to lfence.

Should we use ASM_IFENCE for this?

2018-01-13 02:22:32

by Tom Lendacky

[permalink] [raw]
Subject: Re: [PATCH v1] x86/retpoline: Use lfence in the retpoline/RSB filling RSB macros

On 1/12/2018 7:53 PM, Dan Williams wrote:
> On Fri, Jan 12, 2018 at 5:07 PM, Tom Lendacky <[email protected]> wrote:
>> The pause instruction is currently used in the retpoline and RSB filling
>> macros as a speculation trap. The use of pause was originally suggested
>> because it showed a very, very small difference in the amount of
>> cycles/time used to execute the retpoline as compared to lfence. On AMD,
>> the pause instruction is not a serializing instruction, so the pause/jmp
>> loop will use excess power as it is speculated over waiting for return
>> to mispredict to the correct target.
>>
>> The RSB filling macro is applicable to AMD, and, if software is unable to
>> verify that lfence is serializing on AMD (possible when running under a
>> hypervisor), the generic retpoline support will be used and, so, is also
>> applicable to AMD. Change the use of pause to lfence.
>
> Should we use ASM_IFENCE for this?

I don't think we need to. On bare-metal this will be fine. When running
as a guest, the hypervisor should have made lfence serializing, and if it
hasn't, this is still better then pause.

Thanks,
Tom

>

Subject: [tip:x86/pti] x86/retpoline: Use LFENCE instead of PAUSE in the retpoline/RSB filling RSB macros

Commit-ID: 2eb9137c8744f9adf1670e9aa52850948a30112b
Gitweb: https://git.kernel.org/tip/2eb9137c8744f9adf1670e9aa52850948a30112b
Author: Tom Lendacky <[email protected]>
AuthorDate: Fri, 12 Jan 2018 19:07:28 -0600
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 13 Jan 2018 11:28:50 +0100

x86/retpoline: Use LFENCE instead of PAUSE in the retpoline/RSB filling RSB macros

The PAUSE instruction is currently used in the retpoline and RSB filling
macros as a speculation trap. The use of PAUSE was originally suggested
because it showed a very, very small difference in the amount of
cycles/time used to execute the retpoline as compared to LFENCE.

On AMD, the PAUSE instruction is not a serializing instruction, so the
PAUSE/JMP loop will use excess power as it is speculated over waiting
for return to mispredict to the correct target.

The RSB filling macro is applicable to AMD, and, if software is unable to
verify that LFENCE is serializing on AMD (possible when running under a
hypervisor), the generic retpoline support will be used and, so, is also
applicable to AMD. Change the use of PAUSE to LFENCE.

Signed-off-by: Tom Lendacky <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Kosina <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Paul Turner <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Rik van Riel <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Tim Chen <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/include/asm/nospec-branch.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 402a11c..2c4a09a 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -11,7 +11,7 @@
* Fill the CPU return stack buffer.
*
* Each entry in the RSB, if used for a speculative 'ret', contains an
- * infinite 'pause; jmp' loop to capture speculative execution.
+ * infinite 'lfence; jmp' loop to capture speculative execution.
*
* This is required in various cases for retpoline and IBRS-based
* mitigations for the Spectre variant 2 vulnerability. Sometimes to
@@ -37,12 +37,12 @@
771: \
call 772f; \
773: /* speculation trap */ \
- pause; \
+ lfence; \
jmp 773b; \
772: \
call 774f; \
775: /* speculation trap */ \
- pause; \
+ lfence; \
jmp 775b; \
774: \
dec reg; \
@@ -72,7 +72,7 @@
.macro RETPOLINE_JMP reg:req
call .Ldo_rop_\@
.Lspec_trap_\@:
- pause
+ lfence
jmp .Lspec_trap_\@
.Ldo_rop_\@:
mov \reg, (%_ASM_SP)
@@ -164,7 +164,7 @@
" jmp 904f;\n" \
" .align 16\n" \
"901: call 903f;\n" \
- "902: pause;\n" \
+ "902: lfence;\n" \
" jmp 902b;\n" \
" .align 16\n" \
"903: addl $4, %%esp;\n" \

2018-01-13 10:46:51

by Woodhouse, David

[permalink] [raw]
Subject: Re: [PATCH v1] x86/retpoline: Use lfence in the retpoline/RSB filling RSB macros

On Fri, 2018-01-12 at 19:07 -0600, Tom Lendacky wrote:
> The pause instruction is currently used in the retpoline and RSB filling
> macros as a speculation trap.  The use of pause was originally suggested
> because it showed a very, very small difference in the amount of
> cycles/time used to execute the retpoline as compared to lfence.  On AMD,
> the pause instruction is not a serializing instruction, so the pause/jmp
> loop will use excess power as it is speculated over waiting for return
> to mispredict to the correct target.
>
> The RSB filling macro is applicable to AMD, and, if software is unable to
> verify that lfence is serializing on AMD (possible when running under a
> hypervisor), the generic retpoline support will be used and, so, is also
> applicable to AMD.  Change the use of pause to lfence.
>
> Signed-off-by: Tom Lendacky <[email protected]>

Conditionally-Acked-by: David Woodhouse <[email protected]>

The condition being, as noted, that I'd really like to see it acked by
Arjan/Asit and Paul.



> ---
>  arch/x86/include/asm/nospec-branch.h |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> index 402a11c..2c4a09a 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -11,7 +11,7 @@
>   * Fill the CPU return stack buffer.
>   *
>   * Each entry in the RSB, if used for a speculative 'ret', contains an
> - * infinite 'pause; jmp' loop to capture speculative execution.
> + * infinite 'lfence; jmp' loop to capture speculative execution.
>   *
>   * This is required in various cases for retpoline and IBRS-based
>   * mitigations for the Spectre variant 2 vulnerability. Sometimes to
> @@ -37,12 +37,12 @@
>  771: \
>   call 772f; \
>  773: /* speculation trap */ \
> - pause; \
> + lfence; \
>   jmp 773b; \
>  772: \
>   call 774f; \
>  775: /* speculation trap */ \
> - pause; \
> + lfence; \
>   jmp 775b; \
>  774: \
>   dec reg; \
> @@ -72,7 +72,7 @@
>  .macro RETPOLINE_JMP reg:req
>   call .Ldo_rop_\@
>  .Lspec_trap_\@:
> - pause
> + lfence
>   jmp .Lspec_trap_\@
>  .Ldo_rop_\@:
>   mov \reg, (%_ASM_SP)
> @@ -164,7 +164,7 @@
>   "       jmp    904f;\n" \
>   "       .align 16\n" \
>   "901: call   903f;\n" \
> - "902: pause;\n" \
> + "902: lfence;\n" \
>   "       jmp    902b;\n" \
>   "       .align 16\n" \
>   "903: addl   $4, %%esp;\n" \
>
>


Attachments:
smime.p7s (5.09 kB)

2018-01-13 14:07:39

by Van De Ven, Arjan

[permalink] [raw]
Subject: RE: [PATCH v1] x86/retpoline: Use lfence in the retpoline/RSB filling RSB macros

> > The RSB filling macro is applicable to AMD, and, if software is unable to
> > verify that lfence is serializing on AMD (possible when running under a
> > hypervisor), the generic retpoline support will be used and, so, is also
> > applicable to AMD.  Change the use of pause to lfence.
> >
> > Signed-off-by: Tom Lendacky <[email protected]>
>
> Conditionally-Acked-by: David Woodhouse <[email protected]>


pause is technically the "save me power" instruction

how about a compromise where we do a double:

pause
lfence
jmp <up>

as sequence... that way if the branch recovery is fast, we get the performance of pause, but if it takes a while, on AMD you get the behavior of lfence?


2018-01-13 21:13:56

by Tom Lendacky

[permalink] [raw]
Subject: Re: [PATCH v1] x86/retpoline: Use lfence in the retpoline/RSB filling RSB macros

On 1/13/2018 8:07 AM, Van De Ven, Arjan wrote:
>>> The RSB filling macro is applicable to AMD, and, if software is unable to
>>> verify that lfence is serializing on AMD (possible when running under a
>>> hypervisor), the generic retpoline support will be used and, so, is also
>>> applicable to AMD.  Change the use of pause to lfence.
>>>
>>> Signed-off-by: Tom Lendacky <[email protected]>
>>
>> Conditionally-Acked-by: David Woodhouse <[email protected]>
>
>
> pause is technically the "save me power" instruction
>
> how about a compromise where we do a double:
>
> pause
> lfence
> jmp <up>
>
> as sequence... that way if the branch recovery is fast, we get the performance of pause, but if it takes a while, on AMD you get the behavior of lfence?

That should work on AMD.

Thanks,
Tom

>
>

2018-01-13 21:36:48

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v1] x86/retpoline: Use lfence in the retpoline/RSB filling RSB macros

On Sat, 13 Jan 2018, Tom Lendacky wrote:

> On 1/13/2018 8:07 AM, Van De Ven, Arjan wrote:
> >>> The RSB filling macro is applicable to AMD, and, if software is unable to
> >>> verify that lfence is serializing on AMD (possible when running under a
> >>> hypervisor), the generic retpoline support will be used and, so, is also
> >>> applicable to AMD.  Change the use of pause to lfence.
> >>>
> >>> Signed-off-by: Tom Lendacky <[email protected]>
> >>
> >> Conditionally-Acked-by: David Woodhouse <[email protected]>
> >
> >
> > pause is technically the "save me power" instruction
> >
> > how about a compromise where we do a double:
> >
> > pause
> > lfence
> > jmp <up>
> >
> > as sequence... that way if the branch recovery is fast, we get the performance of pause, but if it takes a while, on AMD you get the behavior of lfence?
>
> That should work on AMD.

I zapped the commit from tip for now until this discussion is resolved.

Thanks,

tglx