Re: [PATCH v2 5/8] KVM: arm64: Refactor CPTR trap deactivation

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@arm.com Cc: Catalin Marinas catalin.marinas@arm.com Cc: Fuad Tabba tabba@google.com Cc: Marc Zyngier maz@kernel.org Cc: Mark Brown broonie@kernel.org Cc: Oliver Upton oliver.upton@linux.dev Cc: Will Deacon will@kernel.org

[...]

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@kernel.org

Will