* [PATCH] x86/msr-index: make SPEC_CTRL_IBRS assembler-portable
@ 2022-11-03 21:07 Nick Desaulniers
2022-11-04 9:55 ` H. Peter Anvin
0 siblings, 1 reply; 3+ messages in thread
From: Nick Desaulniers @ 2022-11-03 21:07 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Peter Zijlstra
Cc: x86, H. Peter Anvin, Nathan Chancellor, Tom Rix, Pawan Gupta,
Adrian Hunter, Josh Poimboeuf, Alexander Shishkin,
Daniel Sneddon, Sandipan Das, Huang Rui, linux-kernel, llvm,
kernel-team, Nick Desaulniers, Greg Kroah-Hartman
GNU binutils' assembler (GAS) didn't support L suffixes on immediates
until binutils 2.28 release. Building arch/x86/entry/entry_64.S with GAS
v2.27 will produce the following assembler errors:
arch/x86/entry/entry_64.S: Assembler messages:
arch/x86/entry/entry_64.S:308: Error: found 'L', expected: ')'
arch/x86/entry/entry_64.S:308: Error: found 'L', expected: ')'
arch/x86/entry/entry_64.S:308: Error: junk `L<<(0)))' after expression
arch/x86/entry/entry_64.S:596: Error: found 'L', expected: ')'
arch/x86/entry/entry_64.S:596: Error: found 'L', expected: ')'
arch/x86/entry/entry_64.S:596: Error: junk `L<<(0)))' after expression
These come from the use of the preprocessor defined SPEC_CTRL_IBRS in
the IBRS_ENTER and IBRS_EXIT assembler macros. SPEC_CTRL_IBRS was using
the BIT macros from include/linux/bits.h which are only portable between
C and assembler for assemblers such as GAS v2.28 (or newer) or clang
because they use the L suffixes for immediate operands, which older GAS
releases cannot parse. The kernel still supports GAS v2.23 and newer
(and older for branches of stable). Let's expand the value of
SPEC_CTRL_IBRS in place so that assemblers don't have issues parsing the
value.
Fixes: 2dbb887e875b ("x86/entry: Add kernel IBRS implementation")
Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Some other ideas considered:
* Use U64_C from include/asm-generic/int-ll64.h rather than BIT for the
value of SPEC_CTRL_IBRS.
* Do so for the entirety of arch/x86/include/asm/msr-index.h or just
SPEC_CTRL_IBRS? include/asm-generic/int-ll64.h doesn't define a UL
suffix; add one?
* Make include/linux/bits.h assembler-portable (for older assemblers)
via the use of include/asm-generic/int-ll64.h.
arch/x86/include/asm/msr-index.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 10ac52705892..0192d853136c 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -46,7 +46,7 @@
#define MSR_TEST_CTRL_SPLIT_LOCK_DETECT BIT(MSR_TEST_CTRL_SPLIT_LOCK_DETECT_BIT)
#define MSR_IA32_SPEC_CTRL 0x00000048 /* Speculation Control */
-#define SPEC_CTRL_IBRS BIT(0) /* Indirect Branch Restricted Speculation */
+#define SPEC_CTRL_IBRS 1 /* Indirect Branch Restricted Speculation */
#define SPEC_CTRL_STIBP_SHIFT 1 /* Single Thread Indirect Branch Predictor (STIBP) bit */
#define SPEC_CTRL_STIBP BIT(SPEC_CTRL_STIBP_SHIFT) /* STIBP mask */
#define SPEC_CTRL_SSBD_SHIFT 2 /* Speculative Store Bypass Disable bit */
--
2.38.1.431.g37b22c650d-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/msr-index: make SPEC_CTRL_IBRS assembler-portable
2022-11-03 21:07 [PATCH] x86/msr-index: make SPEC_CTRL_IBRS assembler-portable Nick Desaulniers
@ 2022-11-04 9:55 ` H. Peter Anvin
2022-11-04 18:07 ` Nick Desaulniers
0 siblings, 1 reply; 3+ messages in thread
From: H. Peter Anvin @ 2022-11-04 9:55 UTC (permalink / raw)
To: Nick Desaulniers, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, Peter Zijlstra
Cc: x86, Nathan Chancellor, Tom Rix, Pawan Gupta, Adrian Hunter,
Josh Poimboeuf, Alexander Shishkin, Daniel Sneddon, Sandipan Das,
Huang Rui, linux-kernel, llvm, kernel-team, Greg Kroah-Hartman
On November 3, 2022 2:07:48 PM PDT, Nick Desaulniers <ndesaulniers@google.com> wrote:
>GNU binutils' assembler (GAS) didn't support L suffixes on immediates
>until binutils 2.28 release. Building arch/x86/entry/entry_64.S with GAS
>v2.27 will produce the following assembler errors:
>
> arch/x86/entry/entry_64.S: Assembler messages:
> arch/x86/entry/entry_64.S:308: Error: found 'L', expected: ')'
> arch/x86/entry/entry_64.S:308: Error: found 'L', expected: ')'
> arch/x86/entry/entry_64.S:308: Error: junk `L<<(0)))' after expression
> arch/x86/entry/entry_64.S:596: Error: found 'L', expected: ')'
> arch/x86/entry/entry_64.S:596: Error: found 'L', expected: ')'
> arch/x86/entry/entry_64.S:596: Error: junk `L<<(0)))' after expression
>
>These come from the use of the preprocessor defined SPEC_CTRL_IBRS in
>the IBRS_ENTER and IBRS_EXIT assembler macros. SPEC_CTRL_IBRS was using
>the BIT macros from include/linux/bits.h which are only portable between
>C and assembler for assemblers such as GAS v2.28 (or newer) or clang
>because they use the L suffixes for immediate operands, which older GAS
>releases cannot parse. The kernel still supports GAS v2.23 and newer
>(and older for branches of stable). Let's expand the value of
>SPEC_CTRL_IBRS in place so that assemblers don't have issues parsing the
>value.
>
>Fixes: 2dbb887e875b ("x86/entry: Add kernel IBRS implementation")
>Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>---
>Some other ideas considered:
>* Use U64_C from include/asm-generic/int-ll64.h rather than BIT for the
> value of SPEC_CTRL_IBRS.
> * Do so for the entirety of arch/x86/include/asm/msr-index.h or just
> SPEC_CTRL_IBRS? include/asm-generic/int-ll64.h doesn't define a UL
> suffix; add one?
>* Make include/linux/bits.h assembler-portable (for older assemblers)
> via the use of include/asm-generic/int-ll64.h.
>
> arch/x86/include/asm/msr-index.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
>index 10ac52705892..0192d853136c 100644
>--- a/arch/x86/include/asm/msr-index.h
>+++ b/arch/x86/include/asm/msr-index.h
>@@ -46,7 +46,7 @@
> #define MSR_TEST_CTRL_SPLIT_LOCK_DETECT BIT(MSR_TEST_CTRL_SPLIT_LOCK_DETECT_BIT)
>
> #define MSR_IA32_SPEC_CTRL 0x00000048 /* Speculation Control */
>-#define SPEC_CTRL_IBRS BIT(0) /* Indirect Branch Restricted Speculation */
>+#define SPEC_CTRL_IBRS 1 /* Indirect Branch Restricted Speculation */
> #define SPEC_CTRL_STIBP_SHIFT 1 /* Single Thread Indirect Branch Predictor (STIBP) bit */
> #define SPEC_CTRL_STIBP BIT(SPEC_CTRL_STIBP_SHIFT) /* STIBP mask */
> #define SPEC_CTRL_SSBD_SHIFT 2 /* Speculative Store Bypass Disable bit */
Let's fix the macro instead.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/msr-index: make SPEC_CTRL_IBRS assembler-portable
2022-11-04 9:55 ` H. Peter Anvin
@ 2022-11-04 18:07 ` Nick Desaulniers
0 siblings, 0 replies; 3+ messages in thread
From: Nick Desaulniers @ 2022-11-04 18:07 UTC (permalink / raw)
To: H. Peter Anvin, Greg Kroah-Hartman
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
Peter Zijlstra, x86, Nathan Chancellor, Tom Rix, Pawan Gupta,
Adrian Hunter, Josh Poimboeuf, Alexander Shishkin,
Daniel Sneddon, Sandipan Das, Huang Rui, linux-kernel, llvm,
kernel-team, Masahiro Yamada
On Fri, Nov 4, 2022 at 2:55 AM H. Peter Anvin <hpa@zytor.com> wrote:
>
> On November 3, 2022 2:07:48 PM PDT, Nick Desaulniers <ndesaulniers@google.com> wrote:
> >GNU binutils' assembler (GAS) didn't support L suffixes on immediates
> >until binutils 2.28 release. Building arch/x86/entry/entry_64.S with GAS
> >v2.27 will produce the following assembler errors:
> >
> > arch/x86/entry/entry_64.S: Assembler messages:
> > arch/x86/entry/entry_64.S:308: Error: found 'L', expected: ')'
> > arch/x86/entry/entry_64.S:308: Error: found 'L', expected: ')'
> > arch/x86/entry/entry_64.S:308: Error: junk `L<<(0)))' after expression
> > arch/x86/entry/entry_64.S:596: Error: found 'L', expected: ')'
> > arch/x86/entry/entry_64.S:596: Error: found 'L', expected: ')'
> > arch/x86/entry/entry_64.S:596: Error: junk `L<<(0)))' after expression
> >
> >These come from the use of the preprocessor defined SPEC_CTRL_IBRS in
> >the IBRS_ENTER and IBRS_EXIT assembler macros. SPEC_CTRL_IBRS was using
> >the BIT macros from include/linux/bits.h which are only portable between
> >C and assembler for assemblers such as GAS v2.28 (or newer) or clang
> >because they use the L suffixes for immediate operands, which older GAS
> >releases cannot parse. The kernel still supports GAS v2.23 and newer
> >(and older for branches of stable). Let's expand the value of
> >SPEC_CTRL_IBRS in place so that assemblers don't have issues parsing the
> >value.
> >
> >Fixes: 2dbb887e875b ("x86/entry: Add kernel IBRS implementation")
> >Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> >---
> >Some other ideas considered:
> >* Use U64_C from include/asm-generic/int-ll64.h rather than BIT for the
> > value of SPEC_CTRL_IBRS.
> > * Do so for the entirety of arch/x86/include/asm/msr-index.h or just
> > SPEC_CTRL_IBRS? include/asm-generic/int-ll64.h doesn't define a UL
> > suffix; add one?
> >* Make include/linux/bits.h assembler-portable (for older assemblers)
> > via the use of include/asm-generic/int-ll64.h.
> >
> > arch/x86/include/asm/msr-index.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> >index 10ac52705892..0192d853136c 100644
> >--- a/arch/x86/include/asm/msr-index.h
> >+++ b/arch/x86/include/asm/msr-index.h
> >@@ -46,7 +46,7 @@
> > #define MSR_TEST_CTRL_SPLIT_LOCK_DETECT BIT(MSR_TEST_CTRL_SPLIT_LOCK_DETECT_BIT)
> >
> > #define MSR_IA32_SPEC_CTRL 0x00000048 /* Speculation Control */
> >-#define SPEC_CTRL_IBRS BIT(0) /* Indirect Branch Restricted Speculation */
> >+#define SPEC_CTRL_IBRS 1 /* Indirect Branch Restricted Speculation */
> > #define SPEC_CTRL_STIBP_SHIFT 1 /* Single Thread Indirect Branch Predictor (STIBP) bit */
> > #define SPEC_CTRL_STIBP BIT(SPEC_CTRL_STIBP_SHIFT) /* STIBP mask */
> > #define SPEC_CTRL_SSBD_SHIFT 2 /* Speculative Store Bypass Disable bit */
>
> Let's fix the macro instead.
Ah, I need to do more homework; this issue is specific to stable
backports of the retbleed mitigations.
commit 95b980d62d52 ("linux/bits.h: make BIT(), GENMASK(), and friends
available in assembly")
fixed this exact problem already. Ok, I'll work with stable to get
that backported as part of the series. Sorry for the noise.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-11-04 18:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-03 21:07 [PATCH] x86/msr-index: make SPEC_CTRL_IBRS assembler-portable Nick Desaulniers
2022-11-04 9:55 ` H. Peter Anvin
2022-11-04 18:07 ` Nick Desaulniers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).