On 2026/04/20 2:19, Marc Zyngier wrote:
On Sat, 18 Apr 2026 09:14:25 +0100,
Akihiko Odaki <[email protected]> wrote:
On a heterogeneous arm64 system, KVM's PMU emulation is based on the
features of a single host PMU instance. When a vCPU is migrated to a
pCPU with an incompatible PMU, counters such as PMCCNTR_EL0 stop
incrementing.
Although this behavior is permitted by the architecture, Windows does
not handle it gracefully and may crash with a division-by-zero error.
The current workaround requires VMMs to pin vCPUs to a set of pCPUs
that share a compatible PMU. This is difficult to implement correctly in
QEMU/libvirt, where pinning occurs after vCPU initialization, and it
also restricts the guest to a subset of available pCPUs.
Introduce the KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY attribute to
create a "fixed-counters-only" PMU. When set, KVM exposes a PMU that is
compatible with all pCPUs but that does not support programmable
event counters which may have different feature sets on different PMUs.
This allows Windows guests to run reliably on heterogeneous systems
without crashing, even without vCPU pinning, and enables VMMs to
schedule vCPUs across all available pCPUs, making full use of the host
hardware.
Much like KVM_ARM_VCPU_PMU_V3_IRQ and other read-write attributes, this
attribute provides a getter that facilitates kernel and userspace
debugging/testing.
OK, so that's the sales pitch. But how is it implemented? I would like
to be able to read a high-level description of the implementation
trade-offs.
Implementation-wise it is very trivial. Essentially the following
addition in kvm_arm_pmu_v3_get_attr() is the entire implementation:
+ case KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY:
+ if (test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY,
&vcpu->kvm->arch.flags))
+ return 0;
Both its functionality and code complexity is trivial. So we can argue that:
- the functionality is too trivial to be useful or
- the interface/implementation complexity is so trivial that it does not
incur maintenance burden
In this case the selftest uses the getter so I was more inclined to have
it, but adding one just for the selftest sounds too ad-hoc, so here I
looked into other attributes to ensure that it was not introducing
inconsistency with existing interfaces.
As the result, I found there are other read-write attributes; in fact
there are more read-write attributes than write-only ones.
Signed-off-by: Akihiko Odaki <[email protected]>
---
Documentation/virt/kvm/devices/vcpu.rst | 29 ++++++
arch/arm64/include/asm/kvm_host.h | 2 +
arch/arm64/include/uapi/asm/kvm.h | 1 +
arch/arm64/kvm/arm.c | 1 +
arch/arm64/kvm/pmu-emul.c | 155 +++++++++++++++++++++++---------
include/kvm/arm_pmu.h | 2 +
6 files changed, 147 insertions(+), 43 deletions(-)
diff --git a/Documentation/virt/kvm/devices/vcpu.rst
b/Documentation/virt/kvm/devices/vcpu.rst
index 60bf205cb373..e0aeb1897d77 100644
--- a/Documentation/virt/kvm/devices/vcpu.rst
+++ b/Documentation/virt/kvm/devices/vcpu.rst
@@ -161,6 +161,35 @@ explicitly selected, or the number of counters is out of
range for the
selected PMU. Selecting a new PMU cancels the effect of setting this
attribute.
+1.6 ATTRIBUTE: KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY
+------------------------------------------------------
+
+:Parameters: no additional parameter in kvm_device_attr.addr
+
+:Returns:
+
+ ======= =====================================================
+ -EBUSY Attempted to set after initializing PMUv3 or running
+ VCPU, or attempted to set for the first time after
+ setting an event filter
+ -ENXIO Attempted to get before setting
+ -ENODEV Attempted to set while PMUv3 not supported
+ ======= =====================================================
+
+If set, PMUv3 will be emulated without programmable event counters. The VCPU
+will use any compatible hardware PMU. This attribute is particularly useful on
Not quite "any PMU". It will use *the* PMU of the physical CPU,
irrespective of the implementation.
I think:
- this comment
- one on the KVM_EXIT_FAIL_ENTRY_CPU_UNSUPPORTED note
- one on kvm_pmu_create_perf_event()
- and one on kvm_arm_pmu_v3_set_pmu_fixed_counters_only()
All boil down into one question: will it support all possible CPUs, or
will it support a subset? Let me answer here:
This patch is written to support a subset instead of all possible CPUs.
If a pCPU does not have a compatible PMU, the pCPU will not be supported
and cause KVM_EXIT_FAIL_ENTRY_CPU_UNSUPPORTED.
This patch does not enforce all possible CPUs are covered by the
compatible PMUs. Theoretically speaking, kvm_arm_pmu_get_pmuver_limit()
enables the PMU emulation when real PMUv3 hardware covers all possible
CPUs *or* the relevant registers can be trapped with IMPDEF, so some
pCPU may not have a compatible PMU and only provide the IMPDEF trapping.
Practically, I don't think any sane configuration will ever have such a
subset support, so we can explicitly enforce all possible CPUs are
covered by the compatible PMUs if desired.
+heterogeneous systems where different hardware PMUs cover different physical
+CPUs. The compatibility of hardware PMUs can be checked with
+KVM_ARM_VCPU_PMU_V3_SET_PMU. All VCPUs in a VM share this attribute. It isn't
+possible to set it for the first time if a PMU event filter is already present.
"for the first time" gives the impression that it will work if you try
again. I'd rather we say that "This feature is incompatible with the
existence of a PMU event filter".
The following sequence will work:
1. Set KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY
2. Set KVM_ARM_VCPU_PMU_V3_FILTER
3. Set KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY
This is to make the behavior conistent with KVM_ARM_VCPU_PMU_V3_SET_PMU.
Furthermore, the architecture currently describes *two* fixed-function
counters (cycles and instructions), while KVM only expose the cycle
counter. I'm all for the extra abstraction, but what does it mean for
migration if we enable FEAT_PMUv3_ICNTR?
I'll answe this at the end of this email.
+
+Note that KVM will not make any attempts to run the VCPU on the physical CPUs
+with compatible hardware PMUs. This is entirely left to userspace. However,
+attempting to run the VCPU on an unsupported CPU will fail and KVM_RUN will
+return with exit_reason = KVM_EXIT_FAIL_ENTRY and populate the fail_entry
struct
+by setting hardware_entry_failure_reason field to
+KVM_EXIT_FAIL_ENTRY_CPU_UNSUPPORTED and the cpu field to the processor id.
+
This is mostly a copy-paste of the previous section. How relevant is
this to the fixed-counters-only feature? If the whole point of this
stuff is to ensure compatibility across CPUs with different PMU
implementations, surely what you describe here is the opposite of what
you want.
Please see the earlier discussion of supported pCPUs.
My preference would be to move this to a separate patch in any case,
more on that below.
I will do so with the next version.
2. GROUP: KVM_ARM_VCPU_TIMER_CTRL
=================================
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 59f25b85be2b..b59e0182472c 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -353,6 +353,8 @@ struct kvm_arch {
#define KVM_ARCH_FLAG_WRITABLE_IMP_ID_REGS 10
/* Unhandled SEAs are taken to userspace */
#define KVM_ARCH_FLAG_EXIT_SEA 11
+ /* PMUv3 is emulated without progammable event counters */
+#define KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY 12
unsigned long flags;
/* VM-wide vCPU feature set */
diff --git a/arch/arm64/include/uapi/asm/kvm.h
b/arch/arm64/include/uapi/asm/kvm.h
index a792a599b9d6..474c84fa757f 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -436,6 +436,7 @@ enum {
#define KVM_ARM_VCPU_PMU_V3_FILTER 2
#define KVM_ARM_VCPU_PMU_V3_SET_PMU 3
#define KVM_ARM_VCPU_PMU_V3_SET_NR_COUNTERS 4
+#define KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY 5
#define KVM_ARM_VCPU_TIMER_CTRL 1
#define KVM_ARM_VCPU_TIMER_IRQ_VTIMER 0
#define KVM_ARM_VCPU_TIMER_IRQ_PTIMER 1
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 620a465248d1..dca16ca26d32 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -634,6 +634,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
if (has_vhe())
kvm_vcpu_load_vhe(vcpu);
kvm_arch_vcpu_load_fp(vcpu);
+ kvm_vcpu_load_pmu(vcpu);
kvm_vcpu_pmu_restore_guest(vcpu);
if (kvm_arm_is_pvtime_enabled(&vcpu->arch))
kvm_make_request(KVM_REQ_RECORD_STEAL, vcpu);
diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index ef5140bbfe28..d1009c144581 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -326,7 +326,10 @@ u64 kvm_pmu_implemented_counter_mask(struct kvm_vcpu *vcpu)
static void kvm_pmc_enable_perf_event(struct kvm_pmc *pmc)
{
- if (!pmc->perf_event) {
+ struct kvm_vcpu *vcpu = kvm_pmc_to_vcpu(pmc);
+
+ if (!pmc->perf_event ||
+ !cpumask_test_cpu(vcpu->cpu,
&to_arm_pmu(pmc->perf_event->pmu)->supported_cpus)) {
kvm_pmu_create_perf_event(pmc);
return;
}
@@ -667,10 +670,8 @@ static bool kvm_pmc_counts_at_el2(struct kvm_pmc *pmc)
return kvm_pmc_read_evtreg(pmc) & ARMV8_PMU_INCLUDE_EL2;
}
-static int kvm_map_pmu_event(struct kvm *kvm, unsigned int eventsel)
+static int kvm_map_pmu_event(struct arm_pmu *pmu, unsigned int eventsel)
{
- struct arm_pmu *pmu = kvm->arch.arm_pmu;
-
/*
* The CPU PMU likely isn't PMUv3; let the driver provide a mapping
* for the guest's PMUv3 event ID.
This refactor should be in its own patch. This sort of minor change is
adding noise to the mean of the patch, for no good reason.
I'll make that change with the next version too.
@@ -681,6 +682,23 @@ static int kvm_map_pmu_event(struct kvm *kvm, unsigned int
eventsel)
return eventsel;
}
+static struct arm_pmu *kvm_pmu_probe_armpmu(int cpu)
+{
+ struct arm_pmu_entry *entry;
+ struct arm_pmu *pmu;
+
+ guard(rcu)();
+
+ list_for_each_entry_rcu(entry, &arm_pmus, entry) {
+ pmu = entry->arm_pmu;
+
+ if (cpumask_test_cpu(cpu, &pmu->supported_cpus))
+ return pmu;
+ }
+
+ return NULL;
+}
+
/**
* kvm_pmu_create_perf_event - create a perf event for a counter
* @pmc: Counter context
@@ -694,6 +712,12 @@ static void kvm_pmu_create_perf_event(struct kvm_pmc *pmc)
int eventsel;
u64 evtreg;
+ if (test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY, &vcpu->kvm->arch.flags)) {
+ arm_pmu = kvm_pmu_probe_armpmu(vcpu->cpu);
+ if (!arm_pmu)
+ return;
How is it possible to not get a PMU here? We don't expose the PMU to a
guest at all if there are CPUs without PMUs, see the comment in
kvm_host_pmu_init(). So I'd expect this to never fail.
Please see the earlier comment.
+ }
+
evtreg = kvm_pmc_read_evtreg(pmc);
kvm_pmu_stop_counter(pmc);
@@ -722,7 +746,7 @@ static void kvm_pmu_create_perf_event(struct kvm_pmc *pmc)
* Don't create an event if we're running on hardware that requires
* PMUv3 event translation and we couldn't find a valid mapping.
*/
- eventsel = kvm_map_pmu_event(vcpu->kvm, eventsel);
+ eventsel = kvm_map_pmu_event(arm_pmu, eventsel);
if (eventsel < 0)
return;
@@ -810,42 +834,6 @@ void kvm_host_pmu_init(struct arm_pmu *pmu)
list_add_tail_rcu(&entry->entry, &arm_pmus);
}
-static struct arm_pmu *kvm_pmu_probe_armpmu(void)
-{
- struct arm_pmu_entry *entry;
- struct arm_pmu *pmu;
- int cpu;
-
- guard(rcu)();
-
- /*
- * It is safe to use a stale cpu to iterate the list of PMUs so long as
- * the same value is used for the entirety of the loop. Given this, and
- * the fact that no percpu data is used for the lookup there is no need
- * to disable preemption.
- *
- * It is still necessary to get a valid cpu, though, to probe for the
- * default PMU instance as userspace is not required to specify a PMU
- * type. In order to uphold the preexisting behavior KVM selects the
- * PMU instance for the core during vcpu init. A dependent use
- * case would be a user with disdain of all things big.LITTLE that
- * affines the VMM to a particular cluster of cores.
- *
- * In any case, userspace should just do the sane thing and use the UAPI
- * to select a PMU type directly. But, be wary of the baggage being
- * carried here.
- */
- cpu = raw_smp_processor_id();
- list_for_each_entry_rcu(entry, &arm_pmus, entry) {
- pmu = entry->arm_pmu;
-
- if (cpumask_test_cpu(cpu, &pmu->supported_cpus))
- return pmu;
- }
-
- return NULL;
-}
-
Same thing for the refactoring of this function. Moving it, changing
the signature and moving the comment somewhere else would be better
placed on its own.
This will be in a separate patch with the next version.
static u64 __compute_pmceid(struct arm_pmu *pmu, bool pmceid1)
{
u32 hi[2], lo[2];
@@ -888,6 +876,9 @@ u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu, bool pmceid1)
u64 val, mask = 0;
int base, i, nr_events;
+ if (test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY, &vcpu->kvm->arch.flags))
+ return 0;
+
if (!pmceid1) {
val = compute_pmceid0(cpu_pmu);
base = 0;
@@ -915,6 +906,26 @@ u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu, bool pmceid1)
return val & mask;
}
+void kvm_vcpu_load_pmu(struct kvm_vcpu *vcpu)
+{
+ unsigned long mask = kvm_pmu_enabled_counter_mask(vcpu);
+ struct kvm_pmc *pmc;
+ struct arm_pmu *cpu_pmu;
Move these to be inside the loop.
I followed the pattern of other functions, but I agree this new code can
follow a more modern style. It will be done with the next version.
+ int i;
+
+ for_each_set_bit(i, &mask, 32) {
+ pmc = kvm_vcpu_idx_to_pmc(vcpu, i);
+ if (!pmc->perf_event)
+ continue;
+
+ cpu_pmu = to_arm_pmu(pmc->perf_event->pmu);
+ if (!cpumask_test_cpu(vcpu->cpu, &cpu_pmu->supported_cpus)) {
+ kvm_make_request(KVM_REQ_RELOAD_PMU, vcpu);
+ break;
+ }
+ }
+}
+
Why do we need to inflict this on VMs that do not have the fixed
counter restriction?
This function is to re-create the perf_event in case the current
perf_event does not support the pCPU because e.g., the pCPU is a E-core
while the perf_event only covers the P-cores.
And even then, all you have to reconfigure is the cycle counter. So
why the loop? All we want to find out is whether the cycle counter is
instantiated on the PMU that matches the current CPU.
I just wanted to avoid hardcoding assumptions on the fixed counter(s).
FEAT_PMUv3_ICNTR will be naturaly handled with a loop, for example.
void kvm_vcpu_reload_pmu(struct kvm_vcpu *vcpu)
{
u64 mask = kvm_pmu_implemented_counter_mask(vcpu);
@@ -1016,6 +1027,9 @@ u8 kvm_arm_pmu_get_max_counters(struct kvm *kvm)
{
struct arm_pmu *arm_pmu = kvm->arch.arm_pmu;
+ if (test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY, &kvm->arch.flags))
+ return 0;
+
/*
* PMUv3 requires that all event counters are capable of counting any
* event, though the same may not be true of non-PMUv3 hardware.
@@ -1070,7 +1084,24 @@ static void kvm_arm_set_pmu(struct kvm *kvm, struct
arm_pmu *arm_pmu)
*/
int kvm_arm_set_default_pmu(struct kvm *kvm)
{
- struct arm_pmu *arm_pmu = kvm_pmu_probe_armpmu();
+ /*
+ * It is safe to use a stale cpu to iterate the list of PMUs so long as
+ * the same value is used for the entirety of the loop. Given this, and
+ * the fact that no percpu data is used for the lookup there is no need
+ * to disable preemption.
+ *
+ * It is still necessary to get a valid cpu, though, to probe for the
+ * default PMU instance as userspace is not required to specify a PMU
+ * type. In order to uphold the preexisting behavior KVM selects the
+ * PMU instance for the core during vcpu init. A dependent use
+ * case would be a user with disdain of all things big.LITTLE that
+ * affines the VMM to a particular cluster of cores.
+ *
+ * In any case, userspace should just do the sane thing and use the UAPI
+ * to select a PMU type directly. But, be wary of the baggage being
+ * carried here.
+ */
+ struct arm_pmu *arm_pmu = kvm_pmu_probe_armpmu(raw_smp_processor_id());
if (!arm_pmu)
return -ENODEV;
@@ -1098,6 +1129,7 @@ static int kvm_arm_pmu_v3_set_pmu(struct kvm_vcpu *vcpu,
int pmu_id)
break;
}
+ clear_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY, &kvm->arch.flags);
Why does this need to be cleared? I'd rather we make sure it is never
set the first place.
KVM_ARM_VCPU_PMU_V3_SET_PMU and KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY
can be set on the same VCPU. The last KVM_ARM_VCPU_PMU_V3_SET_PMU or
KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY setting will be effective.
A VMM may try set these attributes to check if the setting is supported.
For example, the RFC QEMU patch first uses KVM_ARM_VCPU_PMU_V3_SET_PMU
to find a compatible PMU that covers all pCPUs, and then falls back to
KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY. The order of such probing is up
to the VMM.
This rationale applies also to the next comment.
kvm_arm_set_pmu(kvm, arm_pmu);
cpumask_copy(kvm->arch.supported_cpus,
&arm_pmu->supported_cpus);
ret = 0;
@@ -1108,11 +1140,42 @@ static int kvm_arm_pmu_v3_set_pmu(struct kvm_vcpu
*vcpu, int pmu_id)
return ret;
}
+static int kvm_arm_pmu_v3_set_pmu_fixed_counters_only(struct kvm_vcpu *vcpu)
+{
+ struct kvm *kvm = vcpu->kvm;
+ struct arm_pmu_entry *entry;
+ struct arm_pmu *arm_pmu;
+ struct cpumask *supported_cpus = kvm->arch.supported_cpus;
+
+ lockdep_assert_held(&kvm->arch.config_lock);
+
+ if (kvm_vm_has_ran_once(kvm) ||
+ (kvm->arch.pmu_filter &&
+ !test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY,
&kvm->arch.flags)))
+ return -EBUSY;
+
+ set_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY, &kvm->arch.flags);
+ kvm_arm_set_nr_counters(kvm, 0);
+ cpumask_clear(supported_cpus);
What is the purpose of this cpumask_clear()? Under what conditions can
you have something else?
+
+ guard(rcu)();
+
+ list_for_each_entry_rcu(entry, &arm_pmus, entry) {
+ arm_pmu = entry->arm_pmu;
+ cpumask_or(supported_cpus, supported_cpus,
&arm_pmu->supported_cpus);
Why isn't supported_cpus directly set to possible_cpus? Isn't that the
base requirement that you can run on any CPU at all?
Please see the earlier discussion of supported pCPUs.
+ }
+
+ return 0;
+}
+
static int kvm_arm_pmu_v3_set_nr_counters(struct kvm_vcpu *vcpu, unsigned int
n)
{
struct kvm *kvm = vcpu->kvm;
- if (!kvm->arch.arm_pmu)
+ lockdep_assert_held(&kvm->arch.config_lock);
+
+ if (!kvm->arch.arm_pmu &&
+ !test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY,
&kvm->arch.flags))
return -EINVAL;
if (n > kvm_arm_pmu_get_max_counters(kvm))
@@ -1227,6 +1290,8 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct
kvm_device_attr *attr)
return kvm_arm_pmu_v3_set_nr_counters(vcpu, n);
}
+ case KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY:
+ return kvm_arm_pmu_v3_set_pmu_fixed_counters_only(vcpu);
case KVM_ARM_VCPU_PMU_V3_INIT:
return kvm_arm_pmu_v3_init(vcpu);
}
@@ -1253,6 +1318,9 @@ int kvm_arm_pmu_v3_get_attr(struct kvm_vcpu *vcpu, struct
kvm_device_attr *attr)
irq = vcpu->arch.pmu.irq_num;
return put_user(irq, uaddr);
}
+ case KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY:
+ if (test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY,
&vcpu->kvm->arch.flags))
With 6 occurrences of this test_bit(), it feels like it'd be valuable
to have a dedicate predicate to help with readability.
I'll add one with the next version.
+ return 0;
}
return -ENXIO;
@@ -1266,6 +1334,7 @@ int kvm_arm_pmu_v3_has_attr(struct kvm_vcpu *vcpu, struct
kvm_device_attr *attr)
case KVM_ARM_VCPU_PMU_V3_FILTER:
case KVM_ARM_VCPU_PMU_V3_SET_PMU:
case KVM_ARM_VCPU_PMU_V3_SET_NR_COUNTERS:
+ case KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY:
if (kvm_vcpu_has_pmu(vcpu))
return 0;
}
diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index 96754b51b411..1375cbaf97b2 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -56,6 +56,7 @@ void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64
val);
void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val);
void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,
u64 select_idx);
+void kvm_vcpu_load_pmu(struct kvm_vcpu *vcpu);
void kvm_vcpu_reload_pmu(struct kvm_vcpu *vcpu);
int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu,
struct kvm_device_attr *attr);
@@ -161,6 +162,7 @@ static inline u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu,
bool pmceid1)
static inline void kvm_pmu_update_vcpu_events(struct kvm_vcpu *vcpu) {}
static inline void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu) {}
static inline void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu) {}
+static inline void kvm_vcpu_load_pmu(struct kvm_vcpu *vcpu) {}
static inline void kvm_vcpu_reload_pmu(struct kvm_vcpu *vcpu) {}
static inline u8 kvm_arm_pmu_get_pmuver_limit(void)
{
In conclusion, I find this patch to be rather messy. For a start, it
needs to be split in at least 5 patches:
- at least two for the refactoring
- one for the PMU core changes
- one for the UAPI
- one for documentation
That clarifies the expected granurarity of patches. The next version
will be in that layout, perhaps with more patches if an additional
change. Thanks for the guidance.
I'd also like some clarification on how this is intended to work if we
enable FEAT_PMUv3_ICNTR, because the definition seems to be designed
to encompass all fixed-function counters, and I expect this to grow
over time.
Indeed the UAPI was designed to encompass all fixed-function counters as
suggested by Oliver.
To support the UAPI, the implementation avoids hardcoding the assumption
on the fixed counter(s). FEAT_PMUv3_INCTR will be naturaly supported
once the common code is properly updated (i.e., the size of the event
counter bitmask is grown the corresponding registers are wired up with a
proper check of the feature.)
I expect migration will be handled with the conventional register
getters and setters, but please share if you have a concern.
I'm also not planning to look at the selftest at this stage.
That is completely understandable; I'll focus on refining the design and
implementation for the next version first.
Regards,
Akihiko Odaki
