Re: [PATCH v2] KVM: x86/intr: Explicitly check NMI from guest to eliminate false positives

[Sean Christopherson <seanjc@google.com>]

On Wed, Dec 13, 2023, Like Xu wrote:
> 
> 
> On 13/12/2023 3:28 pm, Dongli Zhang wrote:
> > Hi Like,
> > 
> > On 12/5/23 19:20, Like Xu wrote:
> > > From: Like Xu <likexu@tencent.com>
> > > 
> > > Explicitly checking the source of external interrupt is indeed NMI and not
> > > other types in the kvm_arch_pmi_in_guest(), which prevents perf-kvm false
> > > positive samples generated in perf/core NMI mode after vm-exit but before
> > > kvm_before_interrupt() from being incorrectly labelled as guest samples:
> > 
> > About the before kvm_before_interrupt() ...
> > 
> > > 
> > > # test: perf-record + cpu-cycles:HP (which collects host-only precise samples)
> > > # Symbol                                   Overhead       sys       usr  guest sys  guest usr
> > > # .......................................  ........  ........  ........  .........  .........
> > > #
> > > # Before:
> > >    [g] entry_SYSCALL_64                       24.63%     0.00%     0.00%     24.63%      0.00%
> > >    [g] syscall_return_via_sysret              23.23%     0.00%     0.00%     23.23%      0.00%
> > >    [g] files_lookup_fd_raw                     6.35%     0.00%     0.00%      6.35%      0.00%
> > > # After:
> > >    [k] perf_adjust_freq_unthr_context         57.23%    57.23%     0.00%      0.00%      0.00%
> > >    [k] __vmx_vcpu_run                          4.09%     4.09%     0.00%      0.00%      0.00%
> > >    [k] vmx_update_host_rsp                     3.17%     3.17%     0.00%      0.00%      0.00%
> > > 
> > > In the above case, perf records the samples labelled '[g]', the RIPs behind
> > > the weird samples are actually being queried by perf_instruction_pointer()
> > > after determining whether it's in GUEST state or not, and here's the issue:
> > > 
> > > If vm-exit is caused by a non-NMI interrupt (such as hrtimer_interrupt) and
> > > at least one PMU counter is enabled on host, the kvm_arch_pmi_in_guest()
> > > will remain true (KVM_HANDLING_IRQ is set) until kvm_before_interrupt().
> > 
> > ... and here.
> > 
> > Would you mind helping why kvm_arch_pmi_in_guest() remains true before
> > *kvm_before_interrupt()*.
> > 
> > According to the source code, the vcpu->arch.handling_intr_from_guest
> > is set to non-zero only at kvm_before_interrupt(), and cleared at
> > kvm_after_interrupt().
> > 
> > Or would you mean kvm_after_interrupt()?
> 
> Oops, it should refer to kvm_after_interrupt() as the code fixed. Thank you.

No need for another version if that's the only hiccup, I can fixup when applying.