From: Kees Cook <keescook@chromium.org>
To: Sami Tolvanen <samitolvanen@google.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
Nick Desaulniers <ndesaulniers@google.com>,
linux-kernel@vger.kernel.org, clang-built-linux@googlegroups.com,
Marc Zyngier <maz@kernel.org>, Will Deacon <will@kernel.org>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [RESEND PATCH] arm64: fix alternatives with LLVM's integrated assembler
Date: Thu, 31 Oct 2019 13:03:02 -0700 [thread overview]
Message-ID: <201910311303.2FBAA3E3@keescook> (raw)
In-Reply-To: <20191031194652.118427-1-samitolvanen@google.com>
On Thu, Oct 31, 2019 at 12:46:52PM -0700, Sami Tolvanen 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>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Tested-by: Nick Desaulniers <ndesaulniers@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.24.0.rc0.303.g954a862665-goog
>
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Kees Cook <keescook@chromium.org>
To: Sami Tolvanen <samitolvanen@google.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>, Marc Zyngier <maz@kernel.org>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, clang-built-linux@googlegroups.com,
Nick Desaulniers <ndesaulniers@google.com>
Subject: Re: [RESEND PATCH] arm64: fix alternatives with LLVM's integrated assembler
Date: Thu, 31 Oct 2019 13:03:02 -0700 [thread overview]
Message-ID: <201910311303.2FBAA3E3@keescook> (raw)
In-Reply-To: <20191031194652.118427-1-samitolvanen@google.com>
On Thu, Oct 31, 2019 at 12:46:52PM -0700, Sami Tolvanen 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>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Tested-by: Nick Desaulniers <ndesaulniers@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.24.0.rc0.303.g954a862665-goog
>
--
Kees Cook
next prev parent reply other threads:[~2019-10-31 20:03 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-31 19:46 [RESEND PATCH] arm64: fix alternatives with LLVM's integrated assembler Sami Tolvanen
2019-10-31 19:46 ` Sami Tolvanen
2019-10-31 20:03 ` Kees Cook [this message]
2019-10-31 20:03 ` Kees Cook
2019-11-14 16:55 ` Will Deacon
2019-11-14 16:55 ` Will Deacon
2019-11-18 18:31 ` Nick Desaulniers
2019-11-18 18:31 ` Nick Desaulniers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201910311303.2FBAA3E3@keescook \
--to=keescook@chromium.org \
--cc=catalin.marinas@arm.com \
--cc=clang-built-linux@googlegroups.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maz@kernel.org \
--cc=ndesaulniers@google.com \
--cc=samitolvanen@google.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.