Re: [PATCH] arm64: Paper over ARM_SMCCC_ARCH_WORKAROUND_3 Clang issue

From: James Morse <james.morse@arm.com>
To: Nathan Chancellor <nathan@kernel.org>, Marc Zyngier <maz@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org, kernel-team@android.com,
	Nick Desaulniers <ndesaulniers@google.com>,
	Will Deacon <will@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	llvm@lists.linux.dev
Subject: Re: [PATCH] arm64: Paper over ARM_SMCCC_ARCH_WORKAROUND_3 Clang issue
Date: Wed, 9 Mar 2022 18:26:07 +0000	[thread overview]
Message-ID: <70b5df18-efd3-4fca-7f08-a2de0ebae060@arm.com> (raw)
In-Reply-To: <Yijo7Nmt3AiP1uLH@dev-arch.thelio-3990X>

Hi Nathan,

On 09/03/2022 17:50, Nathan Chancellor wrote:
> On Wed, Mar 09, 2022 at 10:11:18AM -0700, Nathan Chancellor wrote:
>> On Wed, Mar 09, 2022 at 03:57:16PM +0000, Marc Zyngier wrote:
>>> Compiling the arm64 kernel with the BHB workarounds and Clang+LTO
>>> results in a bunch of:
>>>
>>> <instantiation>:4:2: error: invalid fixup for movz/movk instruction
>>>  mov w0, #ARM_SMCCC_ARCH_WORKAROUND_3
>>>
>>> when compiling arch/arm64/kernel/entry.S, and makes no sense at all.
>>>
>>> As it turns out, moving a single include line around makes the
>>> problem disappear. Why, you'd ask? Well, I don't have the faintest
>>> idea, and I'm running out of patience. So make of that what you want.

[...]

> CONFIG_LTO=y causes the following include chain:
> 
> include/linux/arm-smccc.h
> include/linux/init.h
> include/linux/compiler.h

> arch/arm64/include/asm/rwonce.h

Gah, a header file that does something completely different under LTO!

> arch/arm64/include/asm/alternative-macros.h
> arch/arm64/include/asm/assembler.h
> 
> assembler.h has the use of ARM_SMCCC_ARCH_WORKAROUND_3 but the init.h
> include in arm-smccc.h happens before the definition, so it does not get
> expanded.

Thanks for getting to the bottom of this! The error message had me stumped.

...

> This diff seems like a somewhat proper solution, as __READ_ONCE() cannot
> be used in assembly, but I am open to other suggestions.
> 
> diff --git a/arch/arm64/include/asm/rwonce.h b/arch/arm64/include/asm/rwonce.h
> index 1bce62fa908a..56f7b1d4d54b 100644
> --- a/arch/arm64/include/asm/rwonce.h
> +++ b/arch/arm64/include/asm/rwonce.h
> @@ -5,7 +5,7 @@
>  #ifndef __ASM_RWONCE_H
>  #define __ASM_RWONCE_H
>  
> -#ifdef CONFIG_LTO
> +#if defined(CONFIG_LTO) && !defined(__ASSEMBLY__)
>  
>  #include <linux/compiler_types.h>
>  #include <asm/alternative-macros.h>
> @@ -66,7 +66,7 @@
>  })
>  
>  #endif	/* !BUILD_VDSO */
> -#endif	/* CONFIG_LTO */
> +#endif	/* CONFIG_LTO && !__ASSEMBLY__ */
>  
>  #include <asm-generic/rwonce.h>

This looks like the smallest/cleanest fix.

Acked-by: James Morse <james.morse@arm.com>

I guess ultimately we should split things like arm-smccc.h into multiple files...

Thanks,

James