Re: [PATCH v2 19/20] KVM: s390: arm64: Promote PTRAUTH capability

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing PTRAUTH traps configuration causes guest crashes.
- [Low] Macro redefinition of 'vcpu_has_ptrauth' causes a build failure.
--

commit cda854ae1be029b75a496434857cbf27f60d5cc0
Author: Steffen Eiden <seiden@linux.ibm.com>

KVM: s390: arm64: Promote PTRAUTH capability
    
Promote the pointer authentication capability if supported by hardware.

> diff --git a/arch/s390/include/asm/kvm_host_arm64.h b/arch/s390/include/asm/kvm_host_arm64.h
> index 73eb09e98848f..32facf0457049 100644
> --- a/arch/s390/include/asm/kvm_host_arm64.h
> +++ b/arch/s390/include/asm/kvm_host_arm64.h
> @@ -32,6 +32,8 @@ bool cpus_have_final_cap(unsigned int num);
>  #define KVM_HAVE_MMU_RWLOCK
>  #define KVM_MAX_VCPUS 1
>  #define KVM_S390_ARM64_IMPL_FEATURES (		\
> +	 BIT(KVM_ARM_VCPU_PTRAUTH_ADDRESS) |	\
> +	 BIT(KVM_ARM_VCPU_PTRAUTH_GENERIC) |	\
>  	 BIT(KVM_ARM_VCPU_SVE)			\
>  )

[Severity: High]
Does promoting these PTRAUTH capabilities lead to a guest crash?

When a guest executes Pointer Authentication instructions, the hardware checks
the Hypervisor Configuration Register (HCR_EL2) bits HCR_API and HCR_APK.
Because vcpu_reset_hcr() hardcodes vcpu->arch.hcr_elz and omits these bits:

arch/s390/kvm/arm64/reset.c:vcpu_reset_hcr() {
        vcpu->arch.hcr_elz = HCR_EL2_E2H | HCR_EL2_RW | HCR_EL2_AMO |
                             HCR_EL2_IMO | HCR_EL2_FMO | HCR_EL2_PTW;
        /* traps */
        vcpu->arch.hcr_elz |= HCR_EL2_TSC | HCR_EL2_TID1 | HCR_EL2_TID2 |
                              HCR_EL2_TID3 | HCR_EL2_TID4 | HCR_EL2_TID5 |
                              HCR_EL2_TIDCP | HCR_EL2_TLOR;
}

PTRAUTH instructions will unconditionally trap to the hypervisor with
ESR_ELx_EC_PAC. Since arm_exit_handlers[] lacks a handler for ESR_ELx_EC_PAC:

arch/s390/kvm/arm64/handle_exit.c:arm_exit_handlers[] = {
        [0 ... ESR_ELx_EC_MAX]  = kvm_handle_unknown_ec,
        [ESR_ELx_EC_HVC64]      = handle_hvc,
        [ESR_ELx_EC_SYS64]      = kvm_handle_sys_reg,
        [ESR_ELx_EC_IABT_LOW]   = kvm_handle_guest_abort,
        [ESR_ELx_EC_DABT_LOW]   = kvm_handle_guest_abort,
};

It seems the hypervisor will fall back to kvm_handle_unknown_ec(), injecting
an unexpected undefined instruction exception back to the guest and crashing
it. Should we configure the traps or add an exit handler before enabling this?

[ ... ]

> diff --git a/arch/s390/kvm/arm64/feature.h b/arch/s390/kvm/arm64/feature.h
> index 892cf8bcf3a12..c11ac0daa4287 100644
> --- a/arch/s390/kvm/arm64/feature.h
> +++ b/arch/s390/kvm/arm64/feature.h
> @@ -94,4 +94,9 @@ static inline u64 kvm_sae_irptc(void)
>  
>  bool system_supports_sve(void);
>  
> +#define vcpu_has_ptrauth(vcpu)						\
> +	 (vcpu_has_feature(vcpu, KVM_ARM_VCPU_PTRAUTH_ADDRESS) ||       \
> +	  vcpu_has_feature(vcpu, KVM_ARM_VCPU_PTRAUTH_GENERIC))

[Severity: Low]
Can this macro redefinition cause a build failure on s390?

Because the C files including this header (like arm.c and feature.c) also
include <linux/kvm_host.h>, they transitively include <arm64/kvm_host.h>.
Since CONFIG_ARM64_PTR_AUTH is not set on s390, <arm64/kvm_host.h> defines
vcpu_has_ptrauth(vcpu) as false.

Redefining it here without first using #undef vcpu_has_ptrauth might trigger
a -Wmacro-redefined warning, which breaks the build if -Werror is used.

> +bool system_has_full_ptr_auth(void);
> +
>  #endif /* ARCH_S390_KVM_FEATURE_H */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831145536.913567-1-seiden@linux.ibm.com?part=19