Re: [PATCH] KVM: arm64: nv: Work around lack of pauth support in old toolchains

[Marc Zyngier <maz@kernel.org>]

On Tue, 23 Apr 2024 13:37:09 +0100,
"Arnd Bergmann" <arnd@arndb.de> wrote:
> 
> On Tue, Apr 23, 2024, at 14:06, Marc Zyngier wrote:
> > On Tue, 23 Apr 2024 13:00:55 +0100,
> > "Aiqun Yu (Maria)" <quic_aiquny@quicinc.com> wrote:
> >> On 4/23/2024 4:24 PM, Arnd Bergmann wrote:
> >> > On Tue, Apr 23, 2024, at 00:48, Marc Zyngier wrote:
> >> >> We still support GCC 8.x, and it appears that this toolchain
> >> >> does not understand "pauth" as a valid architectural extension.
> >> >> After all, it's only been 8 years since ARMv8.3 was released...
> >> > 
> >> > Just to clarify: I'm fairly sure that all supported toolchains
> >> > support ARMv8.3 and PACGA, the problem with ".arch_extension pauth\n"
> >> > seems to be that it was retroactively made an optional
> >> > feature for earlier architecture versions a few years after
> >> > ARMv8.3, so most binutils versions we support understand
> >> > pacga as an armv8.3 feature but reject the pauth name for the
> >> > extension.
> >> Kind of agree with Arnd here.
> >> Shall the fix just remove the ".arch_extension pauth"?
> >> 
> >> I've tried gcc 7 failed with the pauth name for the extension.
> >> After I remove the ".arch_extension pauth" and use "pacga" instruction
> >> directly pass the gcc 7 compilation.
> 
> It really depends on the binutils version, not gcc of course.

Right. I'll amend the commit message to reflect that.

> 
> > And breaks with LLVM:
> >
> >   CC      arch/arm64/kvm/pauth.o
> > arch/arm64/kvm/pauth.c:40:9: error: instruction requires: pauth
> >                      "pacga %0, %1, %2" : "=r" (pac) : "r" (ptr), "r" (mod));
> >                       ^
> > <inline asm>:2:1: note: instantiated into assembly here
> > pacga x19, x1, x9
> > ^
> 
> It works when building with LLVM_IAS=0, which we obviously don't
> want to mandate here. The variant below works for both clang+ias
> (including all still supported versions) and gcc+binutils, but at
> that point it gets obscure enough that your .inst version is easier
> to understand.

Exactly. Either we have a good way to abstract this behind the scenes
(which I don't see right now), or we just assume control of the
instruction generation, which is what my patch does.

In general, I question the value of the ".arch_extension" requirement
for something like Linux, where we already have a pretty fine grained
control of what we want to see being output by the compiler. but that
ship has sailed long ago.

Thanks,

	M.

> 
>     arnd
> 
> --- a/arch/arm64/kvm/pauth.c
> +++ b/arch/arm64/kvm/pauth.c
> @@ -36,7 +36,12 @@ static u64 compute_pac(struct kvm_vcpu *vcpu, u64 ptr,
>         __ptrauth_key_install_nosync(APGA, ikey);
>         isb();
>  
> -       asm volatile(ARM64_ASM_PREAMBLE ".arch_extension pauth\n"
> +       asm volatile(ARM64_ASM_PREAMBLE
> +#ifdef CONFIG_AS_IS_LLVM
> +                    ".arch_extension pauth\n"
> +#else
> +                    ".arch armv8.3-a\n"
> +#endif
>                      "pacga %0, %1, %2" : "=r" (pac) : "r" (ptr), "r" (mod));
>         isb();
>  
> 

-- 
Without deviation from the norm, progress is not possible.