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 <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
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
On November 3, 2022 2:07:48 PM PDT, Nick Desaulniers <[email protected]> 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 <[email protected]>
>Signed-off-by: Nick Desaulniers <[email protected]>
>---
>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.
On Fri, Nov 4, 2022 at 2:55 AM H. Peter Anvin <[email protected]> wrote:
>
> On November 3, 2022 2:07:48 PM PDT, Nick Desaulniers <[email protected]> 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 <[email protected]>
> >Signed-off-by: Nick Desaulniers <[email protected]>
> >---
> >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