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

Marc Zyngier
2021-10-11T15:33:40Z

On Mon, 11 Oct 2021 15:46:25 +0100,
Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Oct 11, 2021, Marc Zyngier wrote:
> > On Wed, 22 Sep 2021 01:05:29 +0100, Sean Christopherson <seanjc@google.com> 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.