Re: [PATCH v7 07/20] KVM: arm64: Set up FGT for Partitioned PMU

[Oliver Upton <oupton@kernel.org>]

On Mon, May 04, 2026 at 09:18:00PM +0000, Colton Lewis wrote:
> +static void __compute_hdfgrtr(struct kvm_vcpu *vcpu)
> +{
> +	__compute_fgt(vcpu, HDFGRTR_EL2);
> +
> +	*vcpu_fgt(vcpu, HDFGRTR_EL2) |=
> +		HDFGRTR_EL2_PMOVS
> +		| HDFGRTR_EL2_PMCCFILTR_EL0
> +		| HDFGRTR_EL2_PMEVTYPERn_EL0
> +		| HDFGRTR_EL2_PMCEIDn_EL0
> +		| HDFGRTR_EL2_PMMIR_EL1;
> +}
> +

I've given this feedback at least twice already...

Operators go on the preceding line in the case of line continuations.

> +
> +/**
> + * kvm_pmu_is_partitioned() - Determine if given PMU is partitioned
> + * @pmu: Pointer to arm_pmu struct
> + *
> + * Determine if given PMU is partitioned by looking at hpmn field. The
> + * PMU is partitioned if this field is less than the number of
> + * counters in the system.
> + *
> + * Return: True if the PMU is partitioned, false otherwise
> + */
> +bool kvm_pmu_is_partitioned(struct arm_pmu *pmu)
> +{
> +	if (!pmu)
> +		return false;
> +
> +	return pmu->max_guest_counters >= 0 &&
> +		pmu->max_guest_counters <= *host_data_ptr(nr_event_counters);
> +}
> +
> +/**
> + * kvm_vcpu_pmu_is_partitioned() - Determine if given VCPU has a partitioned PMU
> + * @vcpu: Pointer to kvm_vcpu struct
> + *
> + * Determine if given VCPU has a partitioned PMU by extracting that
> + * field and passing it to :c:func:`kvm_pmu_is_partitioned`
> + *
> + * Return: True if the VCPU PMU is partitioned, false otherwise
> + */
> +bool kvm_vcpu_pmu_is_partitioned(struct kvm_vcpu *vcpu)
> +{
> +	return kvm_pmu_is_partitioned(vcpu->kvm->arch.arm_pmu) &&
> +		false;
> +}

Ok, I'm thoroughly confused about these predicates.

Whether or not a vCPU is using a partitioned PMU is a per-VM property.
This is separate from whether or not the backing arm_pmu has a range of
available counters for the guest to use.

It is entirely possible that a VM *isn't* using the partitioned PMU
feature (i.e. backed with perf events) yet the supporting arm_pmu has a
guest counter range.

 
> +#if !defined(__KVM_NVHE_HYPERVISOR__)
> +bool kvm_vcpu_pmu_is_partitioned(struct kvm_vcpu *vcpu);
> +bool kvm_vcpu_pmu_use_fgt(struct kvm_vcpu *vcpu);
> +#else
> +static inline bool kvm_vcpu_pmu_is_partitioned(struct kvm_vcpu *vcpu)
> +{
> +	return false;
> +}
> +
> +static inline bool kvm_vcpu_pmu_use_fgt(struct kvm_vcpu *vcpu)
> +{
> +	return false;
> +}
> +#endif
> +

Don't use ifdeffery for this. Aim to have a single definition and rely
on has_vhe() to do the rest of the work.
 
Thanks,
Oliver