| Message ID | 20250302220112.17653-11-dongli.zhang@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/i386/kvm/pmu: PMU Enhancement, Bugfix and Cleanup | expand |
On 3/3/2025 6:00 AM, Dongli Zhang wrote: > 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. Suppose this works for upcoming mediated vPMU as well? If so, please mention it here. Thanks. > > 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(-) > > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c > index c5911baef0..4902694129 100644 > --- a/target/i386/kvm/kvm.c > +++ b/target/i386/kvm/kvm.c > @@ -4160,13 +4160,6 @@ static int kvm_put_msrs(X86CPU *cpu, int level) > } > > if (IS_INTEL_CPU(env) && has_pmu_version > 0) { > - if (has_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]); > @@ -4182,8 +4175,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,
Hi Dapeng, On 3/4/25 11:35 PM, Mi, Dapeng wrote: > > On 3/3/2025 6:00 AM, Dongli Zhang wrote: >> 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. > > Suppose this works for upcoming mediated vPMU as well? If so, please > mention it here. Thanks. TBH I am not sure if it works for mediated vPMU. The entire patchset is based the current implementation in mainline linux kernel. Otherwise, it is also required to modify the AMD's implementation ... that is, to stop AMD general PMCs or global registers (PerfMonV2). How about only consider the case without mediated vPMU so far? 1. For user without PerfMonV2 servers, they only need the patchset to reset general PMCs. 2. For user with PerfMonV2 servers, they need extra patch to reset global registers. 3. For mediated vPMU, we may add extra patch in the future. Thank you very much! Dongli Zhang > > >> >> 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(-) >> >> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c >> index c5911baef0..4902694129 100644 >> --- a/target/i386/kvm/kvm.c >> +++ b/target/i386/kvm/kvm.c >> @@ -4160,13 +4160,6 @@ static int kvm_put_msrs(X86CPU *cpu, int level) >> } >> >> if (IS_INTEL_CPU(env) && has_pmu_version > 0) { >> - if (has_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]); >> @@ -4182,8 +4175,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,
On 3/6/2025 3:00 AM, dongli.zhang@oracle.com wrote: > Hi Dapeng, > > On 3/4/25 11:35 PM, Mi, Dapeng wrote: >> On 3/3/2025 6:00 AM, Dongli Zhang wrote: >>> 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. >> Suppose this works for upcoming mediated vPMU as well? If so, please >> mention it here. Thanks. > TBH I am not sure if it works for mediated vPMU. The entire patchset is > based the current implementation in mainline linux kernel. > > Otherwise, it is also required to modify the AMD's implementation ... that > is, to stop AMD general PMCs or global registers (PerfMonV2). > > How about only consider the case without mediated vPMU so far? > > 1. For user without PerfMonV2 servers, they only need the patchset to reset > general PMCs. > > 2. For user with PerfMonV2 servers, they need extra patch to reset global > registers. > > 3. For mediated vPMU, we may add extra patch in the future. I suppose it should be fine for mediated vPMU but I have no bandwidth to thoroughly test it. Ok, I think it's fine not to consider mediated vPMU in this patch series, we can look at it later. Thanks. > > Thank you very much! > > Dongli Zhang > >> >>> 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(-) >>> >>> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c >>> index c5911baef0..4902694129 100644 >>> --- a/target/i386/kvm/kvm.c >>> +++ b/target/i386/kvm/kvm.c >>> @@ -4160,13 +4160,6 @@ static int kvm_put_msrs(X86CPU *cpu, int level) >>> } >>> >>> if (IS_INTEL_CPU(env) && has_pmu_version > 0) { >>> - if (has_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]); >>> @@ -4182,8 +4175,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,
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index c5911baef0..4902694129 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -4160,13 +4160,6 @@ static int kvm_put_msrs(X86CPU *cpu, int level) } if (IS_INTEL_CPU(env) && has_pmu_version > 0) { - if (has_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]); @@ -4182,8 +4175,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(-)