Re: [PATCH v2 08/13] KVM: arm64: nv: Handle HCR_EL2.{API,APK} independently

[Joey Gouly <joey.gouly@arm.com>]

On Mon, Feb 26, 2024 at 10:05:56AM +0000, Marc Zyngier wrote:
> Although KVM couples API and APK for simplicity, the architecture
> makes no such requirement, and the two can be independently set or
> cleared.
> 
> Check for which of the two possible reasons we have trapped here,
> and if the corresponding L1 control bit isn't set, delegate the
> handling for forwarding.
> 
> Otherwise, set this exact bit in HCR_EL2 and resume the guest.
> Of course, in the non-NV case, we keep setting both bits and
> be done with it. Note that the entry core already saves/restores
> the keys should any of the two control bits be set.
> 
> This results in a bit of rework, and the removal of the (trivial)
> vcpu_ptrauth_enable() helper.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/include/asm/kvm_emulate.h    |  5 ----
>  arch/arm64/kvm/hyp/include/hyp/switch.h | 32 +++++++++++++++++++++----
>  2 files changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index debc3753d2ef..d2177bc77844 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -125,11 +125,6 @@ static inline void vcpu_set_wfx_traps(struct kvm_vcpu *vcpu)
>  	vcpu->arch.hcr_el2 |= HCR_TWI;
>  }
>  
> -static inline void vcpu_ptrauth_enable(struct kvm_vcpu *vcpu)
> -{
> -	vcpu->arch.hcr_el2 |= (HCR_API | HCR_APK);
> -}
> -
>  static inline void vcpu_ptrauth_disable(struct kvm_vcpu *vcpu)
>  {
>  	vcpu->arch.hcr_el2 &= ~(HCR_API | HCR_APK);
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index f5f701f309a9..a0908d7a8f56 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -480,11 +480,35 @@ DECLARE_PER_CPU(struct kvm_cpu_context, kvm_hyp_ctxt);
>  static bool kvm_hyp_handle_ptrauth(struct kvm_vcpu *vcpu, u64 *exit_code)
>  {
>  	struct kvm_cpu_context *ctxt;
> -	u64 val;
> +	u64 enable = 0;
>  
>  	if (!vcpu_has_ptrauth(vcpu))
>  		return false;
>  
> +	/*
> +	 * NV requires us to handle API and APK independently, just in
> +	 * case the hypervisor is totally nuts. Please barf >here<.
> +	 */
> +	if (vcpu_has_nv(vcpu) && !is_hyp_ctxt(vcpu)) {
> +		switch (ESR_ELx_EC(kvm_vcpu_get_esr(vcpu))) {
> +		case ESR_ELx_EC_PAC:
> +			if (!(__vcpu_sys_reg(vcpu, HCR_EL2) & HCR_API))
> +				return false;
> +
> +			enable |= HCR_API;
> +			break;
> +
> +		case ESR_ELx_EC_SYS64:
> +			if (!(__vcpu_sys_reg(vcpu, HCR_EL2) & HCR_APK))
> +				return false;
> +
> +			enable |= HCR_APK;
> +			break;
> +		}
> +	} else {
> +		enable = HCR_API | HCR_APK;
> +	}
> +
>  	ctxt = this_cpu_ptr(&kvm_hyp_ctxt);
>  	__ptrauth_save_key(ctxt, APIA);
>  	__ptrauth_save_key(ctxt, APIB);
> @@ -492,11 +516,9 @@ static bool kvm_hyp_handle_ptrauth(struct kvm_vcpu *vcpu, u64 *exit_code)
>  	__ptrauth_save_key(ctxt, APDB);
>  	__ptrauth_save_key(ctxt, APGA);
>  
> -	vcpu_ptrauth_enable(vcpu);
>  
> -	val = read_sysreg(hcr_el2);
> -	val |= (HCR_API | HCR_APK);
> -	write_sysreg(val, hcr_el2);
> +	vcpu->arch.hcr_el2 |= enable;
> +	sysreg_clear_set(hcr_el2, 0, enable);
>  
>  	return true;
>  }

A bit of sleuthing tells me you plan to delete kvm_hyp_handle_ptrauth() anyway,
so presumably it makes some sense to put that patch before this to avoid
modifying the code just to delete it!

Thanks,
Joey

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel