Messages in this thread Patch in this message | | | Date | Wed, 29 Nov 2023 09:38:27 -0500 | Subject | Re: [PATCH] KVM: x86/pmu: Prevent any host user from enabling PEBS for profiling guest | From | "Liang, Kan" <> |
| |
On 2023-11-29 4:50 a.m., Like Xu wrote: > From: Like Xu <likexu@tencent.com> > > Stop using PEBS counters on host to profiling guest. Limit the range of > enabled PEBS counters to only those counters enabled from the guest PEBS > emulation perspective. > > If there is a perf-record agent on host that uses perf-tools events like > "cpu-cycles:GP" (G for attr.exclude_host, P for max precise event counter) > to capture guest performance events, then the guest will be hanged. This is > because Intel DS-based PEBS buffer is addressed using the 64-bit linear > address of the current {p/v}CPU context based on MSR_IA32_DS_AREA. > > Any perf user using PEBS counters to profile guest on host is, in perf/core > implementation details, trying to set bits on cpuc->intel_ctrl_guest_mask > and arr[pebs_enable].guest, much like the guest PEBS emulation behaviour. > But the subsequent PEBS memory write, regardless of whether guest PEBS is > enabled, can overshoot guest entry and corrupt guest memory. > > Profiling guest via PEBS-DS buffer on host is not supported at this time. > Fix this by filtering the real configured value of arr[pebs_enable].guest > with the emulated state of guest enabled PEBS counters, under the condition > of none cross-mapped PEBS counters.
So the counter will be silently disabled. The user never knows why nothing is sampled. Since we don't support the case, profiling guest via PEBS-DS buffer on host. Maybe we should error out when creating the event. For example (not tested),
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c index 3871267d3237..24b90c70737f 100644 --- a/arch/x86/events/intel/core.c +++ b/arch/x86/events/intel/core.c @@ -3958,6 +3958,10 @@ static int intel_pmu_hw_config(struct perf_event *event) if ((event->attr.config & INTEL_ARCH_EVENT_MASK) == INTEL_FIXED_VLBR_EVENT) return -EINVAL;
+ /* Profiling guest via PEBS-DS buffer on host is not supported. */ + if (event->attr.exclude_host) + return -EINVAL; + if (!(event->attr.freq || (event->attr.wakeup_events && !event->attr.watermark))) { event->hw.flags |= PERF_X86_EVENT_AUTO_RELOAD; if (!(event->attr.sample_type &
Thanks, Kan
> > Cc: Peter Zijlstra (Intel) <peterz@infradead.org> > Fixes: c59a1f106f5c ("KVM: x86/pmu: Add IA32_PEBS_ENABLE MSR emulation for extended PEBS") > Signed-off-by: Like Xu <likexu@tencent.com> > --- > arch/x86/events/intel/core.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c > index a08f794a0e79..17afd504c35b 100644 > --- a/arch/x86/events/intel/core.c > +++ b/arch/x86/events/intel/core.c > @@ -4103,13 +4103,19 @@ static struct perf_guest_switch_msr *intel_guest_get_msrs(int *nr, void *data) > .guest = pebs_mask & ~cpuc->intel_ctrl_host_mask, > }; > > + /* In any case, clear guest PEBS bits first. */ > + arr[global_ctrl].guest &= ~arr[pebs_enable].guest; > + > if (arr[pebs_enable].host) { > /* Disable guest PEBS if host PEBS is enabled. */ > arr[pebs_enable].guest = 0; > } else { > /* Disable guest PEBS thoroughly for cross-mapped PEBS counters. */ > arr[pebs_enable].guest &= ~kvm_pmu->host_cross_mapped_mask; > - arr[global_ctrl].guest &= ~kvm_pmu->host_cross_mapped_mask; > + > + /* Prevent any host user from enabling PEBS for profiling guest. */ > + arr[pebs_enable].guest &= (kvm_pmu->pebs_enable & kvm_pmu->global_ctrl); > + > /* Set hw GLOBAL_CTRL bits for PEBS counter when it runs for guest */ > arr[global_ctrl].guest |= arr[pebs_enable].guest; > } > > base-commit: 6803fb00772cc50cd59a66bd8caaee5c84b13fcf
| |