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
