On Thu, Feb 06, 2025 at 02:10:59PM +0000, Mark Rutland wrote:
> For historical reasons, the VHE and nVHE/hVHE implementations of
> __activate_cptr_traps() pair with a common implementation of
> __kvm_reset_cptr_el2(), which ideally would be named
> __deactivate_cptr_traps().
>
> Rename __kvm_reset_cptr_el2() to __deactivate_cptr_traps(), and split it
> into separate VHE and nVHE/hVHE variants so that each can be paired with
> its corresponding implementation of __activate_cptr_traps().
>
> At the same time, fold kvm_write_cptr_el2() into its callers. This
> makes it clear in-context whether a write is made to the CPACR_EL1
> encoding or the CPTR_EL2 encoding, and removes the possibility of
> confusion as to whether kvm_write_cptr_el2() reformats the sysreg fields
> as cpacr_clear_set() does.
>
> In the nVHE/hVHE implementation of __activate_cptr_traps(), placing the
> sysreg writes within the if-else blocks requires that the call to
> __activate_traps_fpsimd32() is moved earlier, but as this was always
> called before writing to CPTR_EL2/CPACR_EL1, this should not result in a
> functional change.
>
> Signed-off-by: Mark Rutland <mark.rutland@xxxxxxx>
> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> Cc: Fuad Tabba <tabba@xxxxxxxxxx>
> Cc: Marc Zyngier <maz@xxxxxxxxxx>
> Cc: Mark Brown <broonie@xxxxxxxxxx>
> Cc: Oliver Upton <oliver.upton@xxxxxxxxx>
> Cc: Will Deacon <will@xxxxxxxxxx>
[...]
> diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
> index 7a2d189176249..5d79f63a4f861 100644
> --- a/arch/arm64/kvm/hyp/nvhe/switch.c
> +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
> @@ -39,6 +39,9 @@ static void __activate_cptr_traps(struct kvm_vcpu *vcpu)
> {
> u64 val = CPTR_EL2_TAM; /* Same bit irrespective of E2H */
>
> + if (!guest_owns_fp_regs())
> + __activate_traps_fpsimd32(vcpu);
> +
> if (has_hvhe()) {
> val |= CPACR_EL1_TTA;
>
> @@ -47,6 +50,8 @@ static void __activate_cptr_traps(struct kvm_vcpu *vcpu)
> if (vcpu_has_sve(vcpu))
> val |= CPACR_EL1_ZEN;
> }
> +
> + write_sysreg(val, cpacr_el1);
> } else {
> val |= CPTR_EL2_TTA | CPTR_NVHE_EL2_RES1;
>
> @@ -61,12 +66,34 @@ static void __activate_cptr_traps(struct kvm_vcpu *vcpu)
>
> if (!guest_owns_fp_regs())
> val |= CPTR_EL2_TFP;
> +
> + write_sysreg(val, cptr_el2);
> }
> +}
>
> - if (!guest_owns_fp_regs())
> - __activate_traps_fpsimd32(vcpu);
> +static void __deactivate_cptr_traps(struct kvm_vcpu *vcpu)
> +{
> + struct kvm *kvm = kern_hyp_va(vcpu->kvm);
nit: You could lose the local if you used vcpu_has_sve(vcpu) instead.
However, given that this gets removed _anyway_ when we eagerly switch
ZCR later on:
Acked-by: Will Deacon <will@xxxxxxxxxx>
Will
