Re: [PATCH v1] KVM: arm64: Tidying up PAuth code in KVM

[Marc Zyngier <maz@kernel.org>]

Hi Fuad,

On Mon, 22 Jul 2024 13:37:40 +0100,
Fuad Tabba <tabba@google.com> wrote:
> 
> Tidy up some of the PAuth trapping code to clear up some comments
> and avoid clang/checkpatch warnings. Also, do not bother setting
> the PAuth HCR_EL2 bits for protected VMs in pKVM, since that is
> handled by the hypervisor.
>
> Fixes: 814ad8f96e92 ("KVM: arm64: Drop trapping of PAuth instructions/keys")

nit: AFAICT, this doesn't really fix anything. It has no material
impact on the guest or the hypervisor.

> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  arch/arm64/include/asm/kvm_ptrauth.h    | 2 +-
>  arch/arm64/kvm/arm.c                    | 7 ++++---
>  arch/arm64/kvm/hyp/include/hyp/switch.h | 1 -
>  arch/arm64/kvm/hyp/nvhe/switch.c        | 5 ++---
>  4 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_ptrauth.h b/arch/arm64/include/asm/kvm_ptrauth.h
> index d81bac256abc..6199c9f7ec6e 100644
> --- a/arch/arm64/include/asm/kvm_ptrauth.h
> +++ b/arch/arm64/include/asm/kvm_ptrauth.h
> @@ -104,7 +104,7 @@ alternative_else_nop_endif
>  
>  #define __ptrauth_save_key(ctxt, key)					\
>  	do {								\
> -		u64 __val;                                              \
> +		u64 __val;						\
>  		__val = read_sysreg_s(SYS_ ## key ## KEYLO_EL1);	\
>  		ctxt_sys_reg(ctxt, key ## KEYLO_EL1) = __val;		\
>  		__val = read_sysreg_s(SYS_ ## key ## KEYHI_EL1);	\
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 59716789fe0f..6516348024ba 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -510,10 +510,10 @@ void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)
>  
>  static void vcpu_set_pauth_traps(struct kvm_vcpu *vcpu)
>  {
> -	if (vcpu_has_ptrauth(vcpu)) {
> +	if (vcpu_has_ptrauth(vcpu) && !vcpu_is_protected(vcpu)) {

I think this isn't quite correct.

Non-protected VMs in protected mode are still subjected to pKVM's own
handling of the HCR_EL2 configuration, and the whole thing should be
skipped altogether in that case, irrespective of the pauth status of
the vcpu.

What pKVM should evaluate is that status and decide for itself whether
it must enable it or not. You can then hoist the check for protected
mode early and skip the whole function unconditionally, irrespective
of the protected status of the vcpu.

Thanks,

	M.

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