On Thu, Nov 21, 2024, Mingwei Zhang wrote: > Linux guests read IA32_APERF and IA32_MPERF on every scheduler tick > (250 Hz by default) to measure their effective CPU frequency. To avoid > the overhead of intercepting these frequent MSR reads, allow the guest > to read them directly by loading guest values into the hardware MSRs. > > These MSRs are continuously running counters whose values must be > carefully tracked during all vCPU state transitions: > - Guest IA32_APERF advances only during guest execution That's not what this series does though. Guest APERF advances while the vCPU is loaded by KVM_RUN, which is *very* different than letting APERF run freely only while the vCPU is actively executing in the guest. E.g. a vCPU that is memory oversubscribed via zswap will account a significant amount of CPU time in APERF when faulting in swapped memory, whereas traditional file-backed swap will not due to the task being scheduled out while waiting on I/O. In general, the "why" of this series is missing. What are the use cases you are targeting? What are the exact semantics you want to define? *Why* did are you proposed those exact semantics? E.g. emulated I/O that is handled in KVM will be accounted to APERF, but I/O that requires userspace exits will not. It's not necessarily wrong for heavy userspace I/O to cause observed frequency to drop, but it's not obviously correct either. The use cases matter a lot for APERF/MPERF, because trying to reason about what's desirable for an oversubscribed setup requires a lot more work than defining semantics for setups where all vCPUs are hard pinned 1:1 and memory is more or less just partitioned. Not to mention the complexity for trying to support all potential use cases is likely quite a bit higher. And if the use case is specifically for slice-of-hardware, hard pinned/partitioned VMs, does it matter if the host's view of APERF/MPERF is not accurately captured at all times? Outside of maybe a few CPUs running bookkeeping tasks, the only workloads running on CPUs should be vCPUs. It's not clear to me that observing the guest utilization is outright wrong in that case. One idea for supporting APERF/MPERF in KVM would be to add a kernel param to disable/hide APERF/MPERF from the host, and then let KVM virtualize/passthrough APERF/MPERF if and only if the feature is supported in hardware, but hidden from the kernel. I.e. let the system admin gift APERF/MPERF to KVM. > - Guest IA32_MPERF advances at the TSC frequency whenever the vCPU is > in C0 state, even when not actively running > - Host kernel access is redirected through get_host_[am]perf() which > adds per-CPU offsets to the hardware MSR values > - Remote MSR reads through /dev/cpu/*/msr also account for these > offsets > > Guest values persist in hardware while the vCPU is loaded and > running. Host MSR values are restored on vcpu_put (either at KVM_RUN > completion or when preempted) and when transitioning to halt state. > > Note that guest TSC scaling via KVM_SET_TSC_KHZ is not supported, as > it would require either intercepting MPERF reads on Intel (where MPERF > ticks at host rate regardless of guest TSC scaling) or significantly > complicating the cycle accounting on AMD. > > The host must have both CONSTANT_TSC and NONSTOP_TSC capabilities > since these ensure stable TSC frequency across C-states and P-states, > which is required for accurate background MPERF accounting. ... > arch/x86/include/asm/kvm_host.h | 11 ++ > arch/x86/include/asm/topology.h | 10 ++ > arch/x86/kernel/cpu/aperfmperf.c | 65 +++++++++++- > arch/x86/kvm/cpuid.c | 12 ++- > arch/x86/kvm/governed_features.h | 1 + > arch/x86/kvm/lapic.c | 5 +- > arch/x86/kvm/reverse_cpuid.h | 6 ++ > arch/x86/kvm/svm/nested.c | 2 +- > arch/x86/kvm/svm/svm.c | 7 ++ > arch/x86/kvm/svm/svm.h | 2 +- > arch/x86/kvm/vmx/nested.c | 2 +- > arch/x86/kvm/vmx/vmx.c | 7 ++ > arch/x86/kvm/vmx/vmx.h | 2 +- > arch/x86/kvm/x86.c | 171 ++++++++++++++++++++++++++++--- > arch/x86/lib/msr-smp.c | 11 ++ > drivers/cpufreq/amd-pstate.c | 4 +- > drivers/cpufreq/intel_pstate.c | 5 +- > 17 files changed, 295 insertions(+), 28 deletions(-) > > > base-commit: 0a9b9d17f3a781dea03baca01c835deaa07f7cc3 > -- > 2.47.0.371.ga323438b13-goog >
