| Message ID | 20250331013307.11937-11-dongli.zhang@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | target/i386/kvm/pmu: PMU Enhancement, Bugfix and Cleanup | expand |
On Sun, Mar 30, 2025 at 06:32:29PM -0700, Dongli Zhang wrote: > Date: Sun, 30 Mar 2025 18:32:29 -0700 > From: Dongli Zhang <dongli.zhang@oracle.com> > Subject: [PATCH v3 10/10] target/i386/kvm: don't stop Intel PMU counters > X-Mailer: git-send-email 2.43.5 > > 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 is ture after KVM's 68fb4757e867 (v6.2). QEMU even supports v4.5 (docs/system/target-i386.rst)... I'm not sure whether it is outdated, but it's better to mention the Linux version. > In additional, PMU MSRs are set only at levels >= KVM_PUT_RESET_STATE, > excluding runtime. Therefore, updating these MSRs without stopping events > should be acceptable. I agree. > Finally, 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. > > 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 <dongli.zhang@oracle.com> > --- > target/i386/kvm/kvm.c | 9 --------- > 1 file changed, 9 deletions(-) Fine for me, Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Hi Zhao, On 4/10/25 2:45 AM, Zhao Liu wrote: > On Sun, Mar 30, 2025 at 06:32:29PM -0700, Dongli Zhang wrote: >> Date: Sun, 30 Mar 2025 18:32:29 -0700 >> From: Dongli Zhang <dongli.zhang@oracle.com> >> Subject: [PATCH v3 10/10] target/i386/kvm: don't stop Intel PMU counters >> X-Mailer: git-send-email 2.43.5 >> >> 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 is ture after KVM's 68fb4757e867 (v6.2). QEMU even supports v4.5 > (docs/system/target-i386.rst)... I'm not sure whether it is outdated, > but it's better to mention the Linux version. Thank you very much for the reminder. I will: 1. Reorder the reasons and put the above at the end, because now "levels >= KVM_PUT_RESET_STATE" and "exclude_host = 1" (Intel uses atomic MSR autoload while looks AMD supports a special guest mode) are more convincing. 2. Add the commit id that you suggest to the last reason. 3. Add your Reviewed-by. Thank you very much! Dongli Zhang > >> In additional, PMU MSRs are set only at levels >= KVM_PUT_RESET_STATE, >> excluding runtime. Therefore, updating these MSRs without stopping events >> should be acceptable. > > I agree. > >> Finally, 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. >> >> 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 <dongli.zhang@oracle.com> >> --- >> target/i386/kvm/kvm.c | 9 --------- >> 1 file changed, 9 deletions(-) > > Fine for me, > > Reviewed-by: Zhao Liu <zhao1.liu@intel.com> >
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 4c3908e09e..d9c6c9905e 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -4170,13 +4170,6 @@ static int kvm_put_msrs(X86CPU *cpu, int 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]); @@ -4192,8 +4185,6 @@ static int kvm_put_msrs(X86CPU *cpu, int 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,
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. In additional, PMU MSRs are set only at levels >= KVM_PUT_RESET_STATE, excluding runtime. Therefore, updating these MSRs without stopping events should be acceptable. Finally, 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. 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 <dongli.zhang@oracle.com> --- target/i386/kvm/kvm.c | 9 --------- 1 file changed, 9 deletions(-)