* [PATCH] arm64: fix alternatives with LLVM's integrated assembler
@ 2019-10-07 21:14 Sami Tolvanen
2019-10-07 21:34 ` Nick Desaulniers
0 siblings, 1 reply; 7+ messages in thread
From: Sami Tolvanen @ 2019-10-07 21:14 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Marc Zyngier
Cc: clang-built-linux, Sami Tolvanen, Kees Cook, linux-arm-kernel,
linux-kernel
LLVM's integrated assembler fails with the following error when
building KVM:
<inline asm>:12:6: error: expected absolute expression
.if kvm_update_va_mask == 0
^
<inline asm>:21:6: error: expected absolute expression
.if kvm_update_va_mask == 0
^
<inline asm>:24:2: error: unrecognized instruction mnemonic
NOT_AN_INSTRUCTION
^
LLVM ERROR: Error parsing inline asm
These errors come from ALTERNATIVE_CB and __ALTERNATIVE_CFG,
which test for the existence of the callback parameter in inline
assembly using the following expression:
" .if " __stringify(cb) " == 0\n"
This works with GNU as, but isn't supported by LLVM. This change
splits __ALTERNATIVE_CFG and ALTINSTR_ENTRY into separate macros
to fix the LLVM build.
Link: https://github.com/ClangBuiltLinux/linux/issues/472
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
arch/arm64/include/asm/alternative.h | 32 ++++++++++++++++++----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index b9f8d787eea9..324e7d5ab37e 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -35,13 +35,16 @@ void apply_alternatives_module(void *start, size_t length);
static inline void apply_alternatives_module(void *start, size_t length) { }
#endif
-#define ALTINSTR_ENTRY(feature,cb) \
+#define ALTINSTR_ENTRY(feature) \
" .word 661b - .\n" /* label */ \
- " .if " __stringify(cb) " == 0\n" \
" .word 663f - .\n" /* new instruction */ \
- " .else\n" \
+ " .hword " __stringify(feature) "\n" /* feature bit */ \
+ " .byte 662b-661b\n" /* source len */ \
+ " .byte 664f-663f\n" /* replacement len */
+
+#define ALTINSTR_ENTRY_CB(feature, cb) \
+ " .word 661b - .\n" /* label */ \
" .word " __stringify(cb) "- .\n" /* callback */ \
- " .endif\n" \
" .hword " __stringify(feature) "\n" /* feature bit */ \
" .byte 662b-661b\n" /* source len */ \
" .byte 664f-663f\n" /* replacement len */
@@ -62,15 +65,14 @@ static inline void apply_alternatives_module(void *start, size_t length) { }
*
* Alternatives with callbacks do not generate replacement instructions.
*/
-#define __ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg_enabled, cb) \
+#define __ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg_enabled) \
".if "__stringify(cfg_enabled)" == 1\n" \
"661:\n\t" \
oldinstr "\n" \
"662:\n" \
".pushsection .altinstructions,\"a\"\n" \
- ALTINSTR_ENTRY(feature,cb) \
+ ALTINSTR_ENTRY(feature) \
".popsection\n" \
- " .if " __stringify(cb) " == 0\n" \
".pushsection .altinstr_replacement, \"a\"\n" \
"663:\n\t" \
newinstr "\n" \
@@ -78,17 +80,25 @@ static inline void apply_alternatives_module(void *start, size_t length) { }
".popsection\n\t" \
".org . - (664b-663b) + (662b-661b)\n\t" \
".org . - (662b-661b) + (664b-663b)\n" \
- ".else\n\t" \
+ ".endif\n"
+
+#define __ALTERNATIVE_CFG_CB(oldinstr, feature, cfg_enabled, cb) \
+ ".if "__stringify(cfg_enabled)" == 1\n" \
+ "661:\n\t" \
+ oldinstr "\n" \
+ "662:\n" \
+ ".pushsection .altinstructions,\"a\"\n" \
+ ALTINSTR_ENTRY_CB(feature, cb) \
+ ".popsection\n" \
"663:\n\t" \
"664:\n\t" \
- ".endif\n" \
".endif\n"
#define _ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg, ...) \
- __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg), 0)
+ __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg))
#define ALTERNATIVE_CB(oldinstr, cb) \
- __ALTERNATIVE_CFG(oldinstr, "NOT_AN_INSTRUCTION", ARM64_CB_PATCH, 1, cb)
+ __ALTERNATIVE_CFG_CB(oldinstr, ARM64_CB_PATCH, 1, cb)
#else
#include <asm/assembler.h>
--
2.23.0.581.g78d2f28ef7-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: fix alternatives with LLVM's integrated assembler
2019-10-07 21:14 [PATCH] arm64: fix alternatives with LLVM's integrated assembler Sami Tolvanen
@ 2019-10-07 21:34 ` Nick Desaulniers
2019-10-07 23:47 ` Sami Tolvanen
0 siblings, 1 reply; 7+ messages in thread
From: Nick Desaulniers @ 2019-10-07 21:34 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Kees Cook, Catalin Marinas, LKML, clang-built-linux,
Marc Zyngier, Will Deacon, Linux ARM
On Mon, Oct 7, 2019 at 2:14 PM 'Sami Tolvanen' via Clang Built Linux
<clang-built-linux@googlegroups.com> wrote:
>
> LLVM's integrated assembler fails with the following error when
> building KVM:
>
> <inline asm>:12:6: error: expected absolute expression
> .if kvm_update_va_mask == 0
> ^
> <inline asm>:21:6: error: expected absolute expression
> .if kvm_update_va_mask == 0
> ^
> <inline asm>:24:2: error: unrecognized instruction mnemonic
> NOT_AN_INSTRUCTION
> ^
> LLVM ERROR: Error parsing inline asm
>
> These errors come from ALTERNATIVE_CB and __ALTERNATIVE_CFG,
> which test for the existence of the callback parameter in inline
> assembly using the following expression:
>
> " .if " __stringify(cb) " == 0\n"
>
> This works with GNU as, but isn't supported by LLVM. This change
> splits __ALTERNATIVE_CFG and ALTINSTR_ENTRY into separate macros
> to fix the LLVM build.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/472
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> ---
> arch/arm64/include/asm/alternative.h | 32 ++++++++++++++++++----------
> 1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
> index b9f8d787eea9..324e7d5ab37e 100644
> --- a/arch/arm64/include/asm/alternative.h
> +++ b/arch/arm64/include/asm/alternative.h
> @@ -35,13 +35,16 @@ void apply_alternatives_module(void *start, size_t length);
> static inline void apply_alternatives_module(void *start, size_t length) { }
> #endif
>
> -#define ALTINSTR_ENTRY(feature,cb) \
> +#define ALTINSTR_ENTRY(feature) \
> " .word 661b - .\n" /* label */ \
> - " .if " __stringify(cb) " == 0\n" \
> " .word 663f - .\n" /* new instruction */ \
> - " .else\n" \
> + " .hword " __stringify(feature) "\n" /* feature bit */ \
> + " .byte 662b-661b\n" /* source len */ \
> + " .byte 664f-663f\n" /* replacement len */
> +
> +#define ALTINSTR_ENTRY_CB(feature, cb) \
> + " .word 661b - .\n" /* label */ \
> " .word " __stringify(cb) "- .\n" /* callback */ \
> - " .endif\n" \
> " .hword " __stringify(feature) "\n" /* feature bit */ \
> " .byte 662b-661b\n" /* source len */ \
> " .byte 664f-663f\n" /* replacement len */
> @@ -62,15 +65,14 @@ static inline void apply_alternatives_module(void *start, size_t length) { }
> *
> * Alternatives with callbacks do not generate replacement instructions.
> */
> -#define __ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg_enabled, cb) \
> +#define __ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg_enabled) \
> ".if "__stringify(cfg_enabled)" == 1\n" \
> "661:\n\t" \
> oldinstr "\n" \
> "662:\n" \
> ".pushsection .altinstructions,\"a\"\n" \
> - ALTINSTR_ENTRY(feature,cb) \
> + ALTINSTR_ENTRY(feature) \
> ".popsection\n" \
> - " .if " __stringify(cb) " == 0\n" \
> ".pushsection .altinstr_replacement, \"a\"\n" \
> "663:\n\t" \
> newinstr "\n" \
> @@ -78,17 +80,25 @@ static inline void apply_alternatives_module(void *start, size_t length) { }
> ".popsection\n\t" \
> ".org . - (664b-663b) + (662b-661b)\n\t" \
> ".org . - (662b-661b) + (664b-663b)\n" \
> - ".else\n\t" \
> + ".endif\n"
> +
> +#define __ALTERNATIVE_CFG_CB(oldinstr, feature, cfg_enabled, cb) \
> + ".if "__stringify(cfg_enabled)" == 1\n" \
> + "661:\n\t" \
> + oldinstr "\n" \
> + "662:\n" \
> + ".pushsection .altinstructions,\"a\"\n" \
> + ALTINSTR_ENTRY_CB(feature, cb) \
> + ".popsection\n" \
> "663:\n\t" \
> "664:\n\t" \
> - ".endif\n" \
> ".endif\n"
>
> #define _ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg, ...) \
> - __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg), 0)
> + __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg))
>
> #define ALTERNATIVE_CB(oldinstr, cb) \
> - __ALTERNATIVE_CFG(oldinstr, "NOT_AN_INSTRUCTION", ARM64_CB_PATCH, 1, cb)
> + __ALTERNATIVE_CFG_CB(oldinstr, ARM64_CB_PATCH, 1, cb)
> #else
>
> #include <asm/assembler.h>
Should the definition of the ALTERNATIVE macro
(arch/arm64/include/asm/alternative.h#L295) also be updated in this
patch to not pass `1` as the final parameter? Otherwise with this
patch and your LSE one
(https://lore.kernel.org/lkml/20191007201452.208067-1-samitolvanen@google.com/T/#u)
I get one error on linux-next that looks related:
$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang AS=clang
-j71 arch/arm64/kvm/
...
arch/arm64/kvm/hyp/entry.S:109:87: error: too many positional arguments
alternative_insn nop, .inst (0xd500401f | ((0) << 16 | (4) << 5) |
((!!1) << 8)), 4, 1
^
Since __ALTERNATIVE_CFG now takes one less arg, with your patch?
--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: fix alternatives with LLVM's integrated assembler
2019-10-07 21:34 ` Nick Desaulniers
@ 2019-10-07 23:47 ` Sami Tolvanen
2019-10-15 0:00 ` Will Deacon
2019-10-15 20:31 ` Nick Desaulniers
0 siblings, 2 replies; 7+ messages in thread
From: Sami Tolvanen @ 2019-10-07 23:47 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Kees Cook, Catalin Marinas, LKML, clang-built-linux,
Marc Zyngier, Will Deacon, Linux ARM
On Mon, Oct 7, 2019 at 2:34 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> Should the definition of the ALTERNATIVE macro
> (arch/arm64/include/asm/alternative.h#L295) also be updated in this
> patch to not pass `1` as the final parameter?
No, that's the default value for cfg in case the caller omits the
parameter, and it's still needed.
> I get one error on linux-next that looks related:
> $ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang AS=clang
> -j71 arch/arm64/kvm/
> ...
This patch only touches the inline assembly version (i.e. when
compiling without -no-integrated-as), while with AS=clang you are
using clang also for stand-alone assembly code. I believe some
additional work is needed before we can do that.
> arch/arm64/kvm/hyp/entry.S:109:87: error: too many positional arguments
> alternative_insn nop, .inst (0xd500401f | ((0) << 16 | (4) << 5) |
> ((!!1) << 8)), 4, 1
>
> ^
>
> Since __ALTERNATIVE_CFG now takes one less arg, with your patch?
__ALTERNATIVE_CFG (with two underscores) is only defined for C code,
and this patch doesn't change the definition of _ALTERNATIVE_CFG (with
one underscore) that's used for stand-alone assembly.
Sami
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: fix alternatives with LLVM's integrated assembler
2019-10-07 23:47 ` Sami Tolvanen
@ 2019-10-15 0:00 ` Will Deacon
2019-10-15 0:28 ` Sami Tolvanen
2019-10-15 20:30 ` Nick Desaulniers
2019-10-15 20:31 ` Nick Desaulniers
1 sibling, 2 replies; 7+ messages in thread
From: Will Deacon @ 2019-10-15 0:00 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Kees Cook, Catalin Marinas, Nick Desaulniers, LKML,
clang-built-linux, Marc Zyngier, Linux ARM
On Mon, Oct 07, 2019 at 04:47:20PM -0700, Sami Tolvanen wrote:
> On Mon, Oct 7, 2019 at 2:34 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> > Should the definition of the ALTERNATIVE macro
> > (arch/arm64/include/asm/alternative.h#L295) also be updated in this
> > patch to not pass `1` as the final parameter?
>
> No, that's the default value for cfg in case the caller omits the
> parameter, and it's still needed.
>
> > I get one error on linux-next that looks related:
> > $ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang AS=clang
> > -j71 arch/arm64/kvm/
> > ...
>
> This patch only touches the inline assembly version (i.e. when
> compiling without -no-integrated-as), while with AS=clang you are
> using clang also for stand-alone assembly code. I believe some
> additional work is needed before we can do that.
Is there any benefit from supporting '-no-integrated-as' but not 'AS=clang'?
afaict, you have to hack the top-level Makefile for that.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: fix alternatives with LLVM's integrated assembler
2019-10-15 0:00 ` Will Deacon
@ 2019-10-15 0:28 ` Sami Tolvanen
2019-10-15 20:30 ` Nick Desaulniers
1 sibling, 0 replies; 7+ messages in thread
From: Sami Tolvanen @ 2019-10-15 0:28 UTC (permalink / raw)
To: Will Deacon
Cc: Kees Cook, Catalin Marinas, Nick Desaulniers, LKML,
clang-built-linux, Marc Zyngier, Linux ARM
On Mon, Oct 14, 2019 at 5:00 PM Will Deacon <will@kernel.org> wrote:
> Is there any benefit from supporting '-no-integrated-as' but not 'AS=clang'?
> afaict, you have to hack the top-level Makefile for that.
The goal is to eventually support AS=clang and this patch gets us one
step closer to that. However, with this patch (and the LSE one), we
can already use Clang's integrated assembler for inline assembly,
which is a requirement for compiling the kernel with LTO. Google has
shipped LTO kernels on Pixel devices for a couple of generations now.
Sami
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: fix alternatives with LLVM's integrated assembler
2019-10-15 0:00 ` Will Deacon
2019-10-15 0:28 ` Sami Tolvanen
@ 2019-10-15 20:30 ` Nick Desaulniers
1 sibling, 0 replies; 7+ messages in thread
From: Nick Desaulniers @ 2019-10-15 20:30 UTC (permalink / raw)
To: Will Deacon
Cc: Kees Cook, Catalin Marinas, LKML, clang-built-linux,
Sami Tolvanen, Marc Zyngier, Linux ARM
On Mon, Oct 14, 2019 at 5:00 PM Will Deacon <will@kernel.org> wrote:
>
> On Mon, Oct 07, 2019 at 04:47:20PM -0700, Sami Tolvanen wrote:
> > On Mon, Oct 7, 2019 at 2:34 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> > > Should the definition of the ALTERNATIVE macro
> > > (arch/arm64/include/asm/alternative.h#L295) also be updated in this
> > > patch to not pass `1` as the final parameter?
> >
> > No, that's the default value for cfg in case the caller omits the
> > parameter, and it's still needed.
> >
> > > I get one error on linux-next that looks related:
> > > $ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang AS=clang
> > > -j71 arch/arm64/kvm/
> > > ...
> >
> > This patch only touches the inline assembly version (i.e. when
> > compiling without -no-integrated-as), while with AS=clang you are
> > using clang also for stand-alone assembly code. I believe some
> > additional work is needed before we can do that.
>
> Is there any benefit from supporting '-no-integrated-as' but not 'AS=clang'?
I don't think so.
> afaict, you have to hack the top-level Makefile for that.
That's right.
$ make CC=clang
sets `-no-integrated-as` in the top level Makefile, unless `AS=clang`
was specified. So today it's either Clang for inline+out of line, or
GAS for both, but we don't support mixing and matching (ie. GAS for
inline, Clang for out of line, or vice versa).
But older LTS kernels probably don't have the patch that ties the two
together, so Sami's patch addresses the removal of `-no-integrated-as`
for inline assembly (IIUC).
--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] arm64: fix alternatives with LLVM's integrated assembler
2019-10-07 23:47 ` Sami Tolvanen
2019-10-15 0:00 ` Will Deacon
@ 2019-10-15 20:31 ` Nick Desaulniers
1 sibling, 0 replies; 7+ messages in thread
From: Nick Desaulniers @ 2019-10-15 20:31 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Kees Cook, Catalin Marinas, LKML, clang-built-linux,
Marc Zyngier, Will Deacon, Linux ARM
On Mon, Oct 7, 2019 at 4:47 PM Sami Tolvanen <samitolvanen@google.com> wrote:
>
> On Mon, Oct 7, 2019 at 2:34 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> > Should the definition of the ALTERNATIVE macro
> > (arch/arm64/include/asm/alternative.h#L295) also be updated in this
> > patch to not pass `1` as the final parameter?
>
> No, that's the default value for cfg in case the caller omits the
> parameter, and it's still needed.
>
> > I get one error on linux-next that looks related:
> > $ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang AS=clang
> > -j71 arch/arm64/kvm/
> > ...
>
> This patch only touches the inline assembly version (i.e. when
> compiling without -no-integrated-as), while with AS=clang you are
> using clang also for stand-alone assembly code. I believe some
> additional work is needed before we can do that.
Got it, thanks.
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
>
> > arch/arm64/kvm/hyp/entry.S:109:87: error: too many positional arguments
> > alternative_insn nop, .inst (0xd500401f | ((0) << 16 | (4) << 5) |
> > ((!!1) << 8)), 4, 1
> >
> > ^
> >
> > Since __ALTERNATIVE_CFG now takes one less arg, with your patch?
>
> __ALTERNATIVE_CFG (with two underscores) is only defined for C code,
> and this patch doesn't change the definition of _ALTERNATIVE_CFG (with
> one underscore) that's used for stand-alone assembly.
>
> Sami
--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-10-15 20:31 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-07 21:14 [PATCH] arm64: fix alternatives with LLVM's integrated assembler Sami Tolvanen
2019-10-07 21:34 ` Nick Desaulniers
2019-10-07 23:47 ` Sami Tolvanen
2019-10-15 0:00 ` Will Deacon
2019-10-15 0:28 ` Sami Tolvanen
2019-10-15 20:30 ` Nick Desaulniers
2019-10-15 20:31 ` 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).