Re: [PATCH 10/11] KVM: arm64: nv: Honor guest hypervisor's FP/SVE traps in CPTR_EL2

[Marc Zyngier <maz@kernel.org>]

On Sat, 01 Jun 2024 00:13:57 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> Start folding the guest hypervisor's FP/SVE traps into the value
> programmed in hardware. Note that as of writing this is dead code, since
> KVM does a full put() / load() for every nested exception boundary which
> saves + flushes the FP/SVE state.
> 
> However, this will become useful when we can keep the guest's FP/SVE
> state alive across a nested exception boundary and the host no longer
> needs to conservatively program traps.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/kvm/hyp/vhe/switch.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> index 697253673d7b..d07b4f4be5e5 100644
> --- a/arch/arm64/kvm/hyp/vhe/switch.c
> +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> @@ -85,6 +85,19 @@ static void __activate_cptr_traps(struct kvm_vcpu *vcpu)
>  		__activate_traps_fpsimd32(vcpu);
>  	}
>  
> +	/*
> +	 * Layer the guest hypervisor's trap configuration on top of our own if
> +	 * we're in a nested context.
> +	 */
> +	if (!vcpu_has_nv(vcpu) || is_hyp_ctxt(vcpu))
> +		goto write;
> +
> +	if (guest_hyp_fpsimd_traps_enabled(vcpu))
> +		val &= ~CPACR_ELx_FPEN;
> +	if (guest_hyp_sve_traps_enabled(vcpu))
> +		val &= ~CPACR_ELx_ZEN;

I'm afraid this isn't quite right. You are clearing both FPEN (resp
ZEN) bits based on any of the two bits being clear, while what we want
is to actually propagate the 0 bits (and only those).

What I have in my tree is something along the lines of:

	cptr = vcpu_sanitised_cptr_el2(vcpu);
	tmp = cptr & (CPACR_ELx_ZEN_MASK | CPACR_ELx_FPEN_MASK);
	val &= ~(tmp ^ (CPACR_ELx_ZEN_MASK | CPACR_ELx_FPEN_MASK));

which makes sure that we only clear the relevant bits.

Thanks,

	M.

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