Re: [PATCH v3 12/16] KVM: Move x86's perf guest info callbacks to generic KVM

[Marc Zyngier <maz@xxxxxxxxxx>]

On Mon, 11 Oct 2021 15:46:25 +0100,
Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> 
> On Mon, Oct 11, 2021, Marc Zyngier wrote:
> > On Wed, 22 Sep 2021 01:05:29 +0100, Sean Christopherson <seanjc@xxxxxxxxxx> 
> > wrote:
> > > diff --git a/arch/arm64/include/asm/kvm_host.h 
> > > b/arch/arm64/include/asm/kvm_host.h
> > > index ed940aec89e0..828b6eaa2c56 100644
> > > --- a/arch/arm64/include/asm/kvm_host.h
> > > +++ b/arch/arm64/include/asm/kvm_host.h
> > > @@ -673,6 +673,14 @@ int io_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t 
> > > fault_ipa);
> > >  void kvm_perf_init(void);
> > >  void kvm_perf_teardown(void);
> > >  
> > > +#ifdef CONFIG_GUEST_PERF_EVENTS
> > > +static inline bool kvm_arch_pmi_in_guest(struct kvm_vcpu *vcpu)
> > 
> > Pardon my x86 ignorance, what is PMI? PMU Interrupt?
> 
> Ya, Performance Monitoring Interrupt.  I didn't realize the term wasn't
> common perf terminology.  Maybe kvm_arch_perf_events_in_guest() to be
> less x86-centric?

Up to you. I would be happy with just a comment.

> 
> > > +{
> > > + /* Any callback while a vCPU is loaded is considered to be in guest. */
> > > + return !!vcpu;
> > > +}
> > > +#endif
> > 
> > Do you really need this #ifdef?
> 
> Nope, should compile fine without it, though simply dropping the #ifdef
> would make make the semantics of the function wrong, even if nothing
> consumes it.  Tweak it to use IS_ENABLED()?
> 
>       return IS_ENABLED(CONFIG_GUEST_PERF_EVENTS) && !!vcpu;

LGTM.

        M.

-- 
Without deviation from the norm, progress is not possible.