On Mon, May 06, 2024 at 05:29:38AM +0000, Mingwei Zhang wrote:
> @@ -1749,6 +1749,23 @@ perf_event_nmi_handler(unsigned int cmd, struct pt_regs *regs)
> u64 finish_clock;
> int ret;
>
> + /*
> + * When guest pmu context is loaded this handler should be forbidden from
> + * running, the reasons are:
> + * 1. After x86_perf_guest_enter() is called, and before cpu enter into
> + * non-root mode, NMI could happen, but x86_pmu_handle_irq() restore PMU
> + * to use NMI vector, which destroy KVM PMI vector setting.
> + * 2. When VM is running, host NMI other than PMI causes VM exit, KVM will
> + * call host NMI handler (vmx_vcpu_enter_exit()) first before KVM save
> + * guest PMU context (kvm_pmu_save_pmu_context()), as x86_pmu_handle_irq()
> + * clear global_status MSR which has guest status now, then this destroy
> + * guest PMU status.
> + * 3. After VM exit, but before KVM save guest PMU context, host NMI other
> + * than PMI could happen, x86_pmu_handle_irq() clear global_status MSR
> + * which has guest status now, then this destroy guest PMU status.
> + */
> + if (perf_is_guest_context_loaded())
> + return 0;
A function call makes sense because? Also, isn't this naming at least a
very little misleading? Specifically this is about passthrough, not
guest context per se.
> /*
> * All PMUs/events that share this PMI handler should make sure to
> * increment active_events for their events.
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index acf16676401a..5da7de42954e 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -1736,6 +1736,7 @@ extern int perf_get_mediated_pmu(void);
> extern void perf_put_mediated_pmu(void);
> void perf_guest_enter(void);
> void perf_guest_exit(void);
> +bool perf_is_guest_context_loaded(void);
> #else /* !CONFIG_PERF_EVENTS: */
> static inline void *
> perf_aux_output_begin(struct perf_output_handle *handle,
> @@ -1830,6 +1831,10 @@ static inline int perf_get_mediated_pmu(void)
> static inline void perf_put_mediated_pmu(void) { }
> static inline void perf_guest_enter(void) { }
> static inline void perf_guest_exit(void) { }
> +static inline bool perf_is_guest_context_loaded(void)
> +{
> + return false;
> +}
> #endif
>
> #if defined(CONFIG_PERF_EVENTS) && defined(CONFIG_CPU_SUP_INTEL)
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 4c6daf5cc923..184d06c23391 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -5895,6 +5895,11 @@ void perf_guest_exit(void)
> perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
> }
>
> +bool perf_is_guest_context_loaded(void)
> +{
> + return __this_cpu_read(perf_in_guest);
> +}
> +
> /*
> * Holding the top-level event's child_mutex means that any
> * descendant process that has inherited this event will block
> --
> 2.45.0.rc1.225.g2a3ae87e7f-goog
>
