| Message ID | 20250602192702.2125115-1-coltonlewis@google.com |
|---|---|
| Headers | show |
| Series | ARM64 PMU Partitioning | expand |
Hi Colton, On Mon, Jun 02, 2025 at 07:26:46PM +0000, Colton Lewis wrote: > Add a capability for FEAT_HPMN0, whether MDCR_EL2.HPMN can specify 0 > counters reserved for the guest. > > This required changing HPMN0 to an UnsignedEnum in tools/sysreg > because otherwise not all the appropriate macros are generated to add > it to arm64_cpu_capabilities_arm64_features. > > Signed-off-by: Colton Lewis <coltonlewis@google.com> > --- > arch/arm64/kernel/cpufeature.c | 8 ++++++++ > arch/arm64/tools/cpucaps | 1 + > arch/arm64/tools/sysreg | 6 +++--- > 3 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index a3da020f1d1c..578eea321a60 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -541,6 +541,7 @@ static const struct arm64_ftr_bits ftr_id_mmfr0[] = { > }; > > static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = { > + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_EL1_HPMN0_SHIFT, 4, 0), > S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_EL1_DoubleLock_SHIFT, 4, 0), > ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR0_EL1_PMSVer_SHIFT, 4, 0), > ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_EL1_CTX_CMPs_SHIFT, 4, 0), > @@ -2884,6 +2885,13 @@ static const struct arm64_cpu_capabilities arm64_features[] = { > .matches = has_cpuid_feature, > ARM64_CPUID_FIELDS(ID_AA64MMFR0_EL1, FGT, FGT2) > }, > + { > + .desc = "Hypervisor PMU Partitioning 0 Guest Counters", nit: just use the the FEAT_xxx name for the description (i.e. "HPMN0"). Thanks, Oliver
On Mon, Jun 02, 2025 at 07:26:51PM +0000, Colton Lewis wrote: > static void kvm_arm_setup_mdcr_el2(struct kvm_vcpu *vcpu) > { > + u8 hpmn = vcpu->kvm->arch.arm_pmu->hpmn; > + > 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 |= (MDCR_EL2_TPM | > + vcpu->arch.mdcr_el2 = FIELD_PREP(MDCR_EL2_HPMN, hpmn); > + vcpu->arch.mdcr_el2 |= (MDCR_EL2_HPMD | > + MDCR_EL2_TPM | This isn't safe, as there's no guarantee that kvm_arch::arm_pmu is pointing that the PMU for this CPU. KVM needs to derive HPMN from some per-CPU state, not anything tied to the VM/vCPU. > +/** > + * kvm_pmu_partition() - Partition the PMU > + * @pmu: Pointer to pmu being partitioned > + * @host_counters: Number of host counters to reserve > + * > + * Partition the given PMU by taking a number of host counters to > + * reserve and, if it is a valid reservation, recording the > + * corresponding HPMN value in the hpmn field of the PMU and clearing > + * the guest-reserved counters from the counter mask. > + * > + * Passing 0 for @host_counters has the effect of disabling partitioning. > + * > + * Return: 0 on success, -ERROR otherwise > + */ > +int kvm_pmu_partition(struct arm_pmu *pmu, u8 host_counters) > +{ > + u8 nr_counters; > + u8 hpmn; > + > + if (!kvm_pmu_reservation_is_valid(host_counters)) > + return -EINVAL; > + > + nr_counters = *host_data_ptr(nr_event_counters); > + hpmn = kvm_pmu_hpmn(host_counters); > + > + if (hpmn < nr_counters) { > + pmu->hpmn = hpmn; > + /* Inform host driver of available counters */ > + bitmap_clear(pmu->cntr_mask, 0, hpmn); > + bitmap_set(pmu->cntr_mask, hpmn, nr_counters); > + clear_bit(ARMV8_PMU_CYCLE_IDX, pmu->cntr_mask); > + if (pmuv3_has_icntr()) > + clear_bit(ARMV8_PMU_INSTR_IDX, pmu->cntr_mask); > + > + kvm_debug("Partitioned PMU with HPMN %u", hpmn); > + } else { > + pmu->hpmn = nr_counters; > + bitmap_set(pmu->cntr_mask, 0, nr_counters); > + set_bit(ARMV8_PMU_CYCLE_IDX, pmu->cntr_mask); > + if (pmuv3_has_icntr()) > + set_bit(ARMV8_PMU_INSTR_IDX, pmu->cntr_mask); > + > + kvm_debug("Unpartitioned PMU"); > + } > + > + return 0; > +} Hmm... Just in terms of code organization I'm not sure I like having KVM twiddling with *host* support for PMUv3. Feels like the ARM PMU driver should own partitioning and KVM just takes what it can get. > @@ -239,6 +245,13 @@ void kvm_host_pmu_init(struct arm_pmu *pmu) > if (!pmuv3_implemented(kvm_arm_pmu_get_pmuver_limit())) > return; > > + if (reserved_host_counters) { > + if (kvm_pmu_partition_supported()) > + WARN_ON(kvm_pmu_partition(pmu, reserved_host_counters)); > + else > + kvm_err("PMU Partition is not supported"); > + } > + Hasn't the ARM PMU been registered with perf at this point? Surely the driver wouldn't be very pleased with us ripping counters out from under its feet. Thanks, Oliver
On Mon, Jun 02, 2025 at 07:27:01PM +0000, Colton Lewis wrote: > + case KVM_ARM_PARTITION_PMU: { This should be a vCPU attribute similar to the other PMUv3 controls we already have. Ideally a single attribute where userspace tells us it wants paritioning and specifies the PMU ID to use. None of this can be changed after INIT'ing the PMU. > + struct arm_pmu *pmu; > + u8 host_counters; > + > + if (unlikely(!kvm_vcpu_initialized(vcpu))) > + return -ENOEXEC; > + > + if (!kvm_pmu_partition_supported()) > + return -EPERM; > + > + if (copy_from_user(&host_counters, argp, sizeof(host_counters))) > + return -EFAULT; > + > + pmu = vcpu->kvm->arch.arm_pmu; > + return kvm_pmu_partition(pmu, host_counters); Yeah, we really can't be changing the counters available to the ARM PMU driver at this point. What happens to host events already scheduled on the CPU? Either the partition of host / KVM-owned counters needs to be computed up front (prior to scheduling events) or KVM needs a way to direct perf to reschedule events on the PMU based on the new operating constraints. Thanks, Oliver
Overview: This series implements a new PMU scheme on ARM, a partitioned PMU that exists alongside the existing emulated PMU and may be enabled by the kernel command line kvm.reserved_host_counters or by the vcpu ioctl KVM_ARM_PARTITION_PMU. This is a continuation of the RFC posted earlier this year. [1] The high level overview and reason for the name is that this implementation takes advantage of recent CPU features to partition the PMU counters into a host-reserved set and a guest-reserved set. Guests are allowed untrapped hardware access to the most frequently used PMU registers and features for the guest-reserved counters only. This untrapped hardware access significantly reduces the overhead of using performance monitoring capabilities such as the `perf` tool inside a guest VM. Register accesses that aren't trapping to KVM mean less time spent in the host kernel and more time on the workloads guests care about. This optimization especially shines during high `perf` sample rates or large numbers of events that require multiplexing hardware counters. Performance: For example, the following tests were carried out on identical ARM machines with 10 general purpose counters with identical guest images run on QEMU, the only difference being my PMU implementation or the existing one. Some arguments have been simplified here to clarify the purpose of the test: 1) time perf record -e ${FIFTEEN_HW_EVENTS} -F 1000 -- \ gzip -c tmpfs/random.64M.img >/dev/null On emulated PMU this command took 4.143s real time with 0.159s system time. On partitioned PMU this command took 3.139s real time with 0.110s system time, runtime reductions of 24.23% and 30.82%. 2) time perf stat -dd -- \ automated_specint2017.sh On emulated PMU this benchmark completed in 3789.16s real time with 224.45s system time and a final benchmark score of 4.28. On partitioned PMU this benchmark completed in 3525.67s real time with 15.98s system time and a final benchmark score of 4.56. That is a 6.95% reduction in runtime, 92.88% reduction in system time, and 6.54% improvement in overall benchmark score. Seeing these improvements on something as lightweight as perf stat is remarkable and implies there would have been a much greater improvement with perf record. I did not test that because I was not confident it would even finish in a reasonable time on the emulated PMU Test 3 was slightly different, I ran the workload in a VM with a single VCPU pinned to a physical CPU and analyzed from the host where the physical CPU spent its time using mpstat. 3) perf record -e ${FIFTEEN_HW_EVENTS} -F 4000 -- \ stress-ng --cpu 0 --timeout 30 Over a period of 30s the cpu running with the emulated PMU spent 34.96% of the time in the host kernel and 55.85% of the time in the guest. The cpu running the partitioned PMU spent 0.97% of its time in the host kernel and 91.06% of its time in the guest. Taken together, these tests represent a remarkable performance improvement for anything perf related using this new PMU implementation. Caveats: Because the most consistent and performant thing to do was untrap PMCR_EL0, the number of counters visible to the guest via PMCR_EL0.N is always equal to the value KVM sets for MDCR_EL2.HPMN. Previously allowed writes to PMCR_EL0.N via {GET,SET}_ONE_REG no longer affect the guest. These improvements come at a cost to 7-35 new registers that must be swapped at every vcpu_load and vcpu_put if the feature is enabled. I have been informed KVM would like to avoid paying this cost when possible. One solution is to make the trapping changes and context swapping lazy such that the trapping changes and context swapping only take place after the guest has actually accessed the PMU so guests that never access the PMU never pay the cost. This is not done here because it is not crucial to the primary functionality and I thought review would be more productive as soon as I had something complete enough for reviewers to easily play with. However, this or any better ideas are on the table for inclusion in future re-rolls. [1] https://lore.kernel.org/kvmarm/20250213180317.3205285-1-coltonlewis@google.com/ Colton Lewis (16): arm64: cpufeature: Add cpucap for HPMN0 arm64: Generate sign macro for sysreg Enums arm64: cpufeature: Add cpucap for PMICNTR KVM: arm64: Reorganize PMU functions KVM: arm64: Introduce method to partition the PMU perf: arm_pmuv3: Generalize counter bitmasks perf: arm_pmuv3: Keep out of guest counter partition KVM: arm64: Set up FGT for Partitioned PMU KVM: arm64: Writethrough trapped PMEVTYPER register KVM: arm64: Use physical PMSELR for PMXEVTYPER if partitioned KVM: arm64: Writethrough trapped PMOVS register KVM: arm64: Context switch Partitioned PMU guest registers perf: pmuv3: Handle IRQs for Partitioned PMU guest counters KVM: arm64: Inject recorded guest interrupts KVM: arm64: Add ioctl to partition the PMU when supported KVM: arm64: selftests: Add test case for partitioned PMU Marc Zyngier (1): KVM: arm64: Cleanup PMU includes Documentation/virt/kvm/api.rst | 16 + arch/arm/include/asm/arm_pmuv3.h | 24 + arch/arm64/include/asm/arm_pmuv3.h | 36 +- arch/arm64/include/asm/kvm_host.h | 208 +++++- arch/arm64/include/asm/kvm_pmu.h | 82 +++ arch/arm64/kernel/cpufeature.c | 15 + arch/arm64/kvm/Makefile | 2 +- arch/arm64/kvm/arm.c | 24 +- arch/arm64/kvm/debug.c | 13 +- arch/arm64/kvm/hyp/include/hyp/switch.h | 65 +- arch/arm64/kvm/pmu-emul.c | 629 +---------------- arch/arm64/kvm/pmu-part.c | 358 ++++++++++ arch/arm64/kvm/pmu.c | 630 ++++++++++++++++++ arch/arm64/kvm/sys_regs.c | 54 +- arch/arm64/tools/cpucaps | 2 + arch/arm64/tools/gen-sysreg.awk | 1 + arch/arm64/tools/sysreg | 6 +- drivers/perf/arm_pmuv3.c | 55 +- include/kvm/arm_pmu.h | 199 ------ include/linux/perf/arm_pmu.h | 15 +- include/linux/perf/arm_pmuv3.h | 14 +- include/uapi/linux/kvm.h | 4 + tools/include/uapi/linux/kvm.h | 2 + .../selftests/kvm/arm64/vpmu_counter_access.c | 40 +- virt/kvm/kvm_main.c | 1 + 25 files changed, 1616 insertions(+), 879 deletions(-) create mode 100644 arch/arm64/include/asm/kvm_pmu.h create mode 100644 arch/arm64/kvm/pmu-part.c delete mode 100644 include/kvm/arm_pmu.h base-commit: 1b85d923ba8c9e6afaf19e26708411adde94fba8 -- 2.49.0.1204.g71687c7c1d-goog