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

[Aiqun Yu (Maria)]

On 4/24/2024 12:15 AM, Marc Zyngier wrote:
> On Tue, 23 Apr 2024 13:37:09 +0100,
> "Arnd Bergmann" <arnd at 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 at 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.
Ack.
> 
> 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();
>>  
>>
> 

-- 
Thx and BRs,
Aiqun(Maria) Yu