On 12/29/2025 11:42 PM, Dongli Zhang wrote:
> PMU MSRs are set by QEMU only at levels >= KVM_PUT_RESET_STATE,
> excluding runtime. Therefore, updating these MSRs without stopping events
> should be acceptable.
It seems preferable to keep the existing logic. The sequence of
disabling -> setting new counters -> re-enabling is complete and
reasonable. Re-enabling the PMU implicitly tell KVM to do whatever
actions are needed to make the new counters take effect.
If the purpose of this patch to improve performance, given that this is
a non-critical path, trading this clear and robust logic for a minor
performance gain does not seem necessary.
> In addition, KVM creates kernel perf events with host mode excluded
> (exclude_host = 1). While the events remain active, they don't increment
> the counter during QEMU vCPU userspace mode.
>
> Finally, The kvm_put_msrs() sets the MSRs using KVM_SET_MSRS. The x86 KVM
> processes these MSRs one by one in a loop, only saving the config and
> triggering the KVM_REQ_PMU request. This approach does not immediately stop
> the event before updating PMC. This approach is true since Linux kernel
> commit 68fb4757e867 ("KVM: x86/pmu: Defer reprogram_counter() to
> kvm_pmu_handle_event"), that is, v6.2.
This seems to assume KVM's internal behavior. While that is true today
(and possibly in the future), it’s not necessary for QEMU to make such
assumptions, as that could unnecessarily limit KVM’s flexibility to
change its behavior later.
> No Fixed tag is going to be added for the commit 0d89436786b0 ("kvm:
> migrate vPMU state"), because this isn't a bugfix.
>
> Signed-off-by: Dongli Zhang <[email protected]>
> Reviewed-by: Zhao Liu <[email protected]>
> Reviewed-by: Dapeng Mi <[email protected]>
> ---
> Changed since v3:
> - Re-order reasons in commit messages.
> - Mention KVM's commit 68fb4757e867 (v6.2).
> - Keep Zhao's review as there isn't code change.
> Changed since v6:
> - Add Reviewed-by from Dapeng Mi.
>
> target/i386/kvm/kvm.c | 9 ---------
> 1 file changed, 9 deletions(-)
>
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 99837048b8..742dc6ac0d 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -4222,13 +4222,6 @@ static int kvm_put_msrs(X86CPU *cpu, KvmPutState level)
> }
>
> if ((IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env)) && pmu_version > 0) {
> - if (pmu_version > 1) {
> - /* Stop the counter. */
> - kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR_CTRL, 0);
> - kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_CTRL, 0);
> - }
> -
> - /* Set the counter values. */
> for (i = 0; i < num_pmu_fixed_counters; i++) {
> kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR0 + i,
> env->msr_fixed_counters[i]);
> @@ -4244,8 +4237,6 @@ static int kvm_put_msrs(X86CPU *cpu, KvmPutState level)
> env->msr_global_status);
> kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_OVF_CTRL,
> env->msr_global_ovf_ctrl);
> -
> - /* Now start the PMU. */
> kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR_CTRL,
> env->msr_fixed_ctr_ctrl);
> kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_CTRL,
