Re: [PATCH v7 09/20] KVM: arm64: Set up MDCR_EL2 to handle a Partitioned PMU

[Colton Lewis]

Oliver Upton <oup...@kernel.org> writes:

On Mon, May 04, 2026 at 09:18:02PM +0000, Colton Lewis wrote:

diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index 3ad6b7c6e4ba7..0ab89c91e19cb 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -36,20 +36,43 @@ static int cpu_has_spe(u64 dfr0)
   */
  static void kvm_arm_setup_mdcr_el2(struct kvm_vcpu *vcpu)
  {
+       int hpmn = kvm_pmu_hpmn(vcpu);
+
        preempt_disable();

        /*
         * This also clears MDCR_EL2_E2PB_MASK and MDCR_EL2_E2TB_MASK
         * to disable guest access to the profiling and trace buffers
         */
-       vcpu->arch.mdcr_el2 = FIELD_PREP(MDCR_EL2_HPMN,
-                                        *host_data_ptr(nr_event_counters));
+
+       vcpu->arch.mdcr_el2 = FIELD_PREP(MDCR_EL2_HPMN, hpmn);
        vcpu->arch.mdcr_el2 |= (MDCR_EL2_TPM |
                                MDCR_EL2_TPMS |
                                MDCR_EL2_TTRF |
                                MDCR_EL2_TPMCR |
                                MDCR_EL2_TDRA |
-                               MDCR_EL2_TDOSA);
+                               MDCR_EL2_TDOSA |
+                               MDCR_EL2_HPME);
+
+       if (kvm_vcpu_pmu_is_partitioned(vcpu)) {
+               /*
+                * Filtering these should be redundant because we trap
+                * all the TYPER and FILTR registers anyway and ensure
+                * they filter EL2, but set the bits if they are here.
+                */
+               if (is_pmuv3p1(read_pmuver()))
+                       vcpu->arch.mdcr_el2 |= MDCR_EL2_HPMD;
+               if (is_pmuv3p5(read_pmuver()))
+                       vcpu->arch.mdcr_el2 |= MDCR_EL2_HCCD;

Neither of these controls are of any consequence on unsupported
hardware (RES0). Set them unconditionally?

Sure.

+               /*
+                * Take out the coarse grain traps if we are using
+                * fine grain traps.
+                */
+               if (kvm_vcpu_pmu_use_fgt(vcpu))

I think open coding the check here would actually improve readability.

                if (cpus_have_final_cap(ARM64_HAS_FGT) &&
                    (cpus_have_final_cap(ARM64_HAS_HPMN0) ||
                     vcpu->kvm->arch.nr_pmu_counters != 0))
                        vcpu->arch.mdcr_el2 &= ~(MDCR_EL2_TPM | MDCR_EL2_TPMCR);

I disagree but I'll do it.

+
+/**
+ * kvm_pmu_hpmn() - Calculate HPMN field value
+ * @vcpu: Pointer to struct kvm_vcpu
+ *
+ * Calculate the appropriate value to set for MDCR_EL2.HPMN. If
+ * partitioned, this is the number of counters set for the guest if

+ * supported, falling back to max_guest_counters if needed. If we are not

+ * partitioned or can't set the implied HPMN value, fall back to the
+ * host value.
+ *
+ * Return: A valid HPMN value
+ */
+u8 kvm_pmu_hpmn(struct kvm_vcpu *vcpu)
+{
+       u8 nr_guest_cntr = vcpu->kvm->arch.nr_pmu_counters;
+
+       if (kvm_vcpu_pmu_is_partitioned(vcpu)
+           && !vcpu_on_unsupported_cpu(vcpu)
+           && (cpus_have_final_cap(ARM64_HAS_HPMN0) || nr_guest_cntr > 0))
+               return nr_guest_cntr;
+
+       return *host_data_ptr(nr_event_counters);
+}

This helper isn't helpful. Just open code it in the place where we are
computing MDCR_EL2.

I disagree but I'll do it.

@@ -542,6 +542,13 @@ u8 kvm_arm_pmu_get_max_counters(struct kvm *kvm)
        if (cpus_have_final_cap(ARM64_WORKAROUND_PMUV3_IMPDEF_TRAPS))
                return 1;

+       /*
+        * If partitioned then we are limited by the max counters in
+        * the guest partition.
+        */
+       if (kvm_pmu_is_partitioned(arm_pmu))
+               return arm_pmu->max_guest_counters;
+

Ok, this is exactly what I was getting at earlier. What about a VM with
an emulated PMU? It should use cntr_mask calculation, not the guest
range.

True. I should use something different here.

Thanks,
Oliver