Re: [PATCH 5/5] perf: Correct perf sampling with guest VMs

Mark Rutland [off-list ref] writes:

On Wed, Sep 04, 2024 at 08:41:33PM +0000, Colton Lewis wrote:

quoted

Previously any PMU overflow interrupt that fired while a VCPU was
loaded was recorded as a guest event whether it truly was or not. This
resulted in nonsense perf recordings that did not honor
perf_event_attr.exclude_guest and recorded guest IPs where it should
have recorded host IPs.

quoted

Reorganize that plumbing to record perf events correctly even when
VCPUs are loaded.

It'd be good if we could make that last bit a little more explicit,
e.g.

   Rework the sampling logic to only record guest samples for events with
   exclude_guest clear. This way any host-only events with exclude_guest
   set will never see unexpected guest samples. The behaviour of events
   with exclude_guest clear is unchanged.

[...]

Done

quoted

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 4384f6c49930..e1a66c9c3773 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c

@@ -6915,13 +6915,26 @@ void perf_unregister_guest_info_callbacks(struct  

perf_guest_info_callbacks *cbs)
  EXPORT_SYMBOL_GPL(perf_unregister_guest_info_callbacks);
  #endif

quoted

-unsigned long perf_misc_flags(unsigned long pt_regs *regs)
+static bool is_guest_event(struct perf_event *event)
  {
+	return !event->attr.exclude_guest && perf_guest_state();
+}

Could we name this something like "should_sample_guest()"? Calling this
"is_guest_event()" makes it should like it's checking a static property
of the event (and not other conditions like perf_guest_state()).

Otherwise this all looks reasonable to me, modulo Ingo's comments. I'll
happily test a v2 once those have been addressed.

Done

Mark.