Hi Oliver. Thanks for the review.

Oliver Upton <[email protected]> writes:

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.

I apologize for letting that slip through again.

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

Yes and I add that to this predicate in a later patch when I introduce
the flag. I can always reorder to introduce the flag before (or along
with) this predicate.

+#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.

Will do.


Thanks,
Oliver

Reply via email to