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?

> +             /*
> +              * 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);
> +
> +/**
> + * 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.

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

Thanks,
Oliver

Reply via email to