Re: [PATCH V2 3/5] KVM: x86/vPMU: Create vPMU interface for VMX and SVM

Radim Krčmář
2015-04-21T09:33:56Z

2015-04-20 13:33-0500, Wei Huang:
> <snip>
> >>+/* check if msr_idx is a valid index to access PMU */
> >>+inline int kvm_pmu_check_msr_idx(struct kvm_vcpu *vcpu, unsigned msr_idx)
> >
> >If we really want it inline, it's better done in header.
> >(I think GCC would inline this in-module anyway, but other modules still
> > have to call it.)
> >
> >>+{
> >>+ return kvm_pmu_ops->check_msr_idx(vcpu, msr_idx);
> >>+}
> >>+
> >| [...]
> >>+bool kvm_pmu_is_msr(struct kvm_vcpu *vcpu, u32 msr)
> >
> >(Might make sense to inline these trivial wrappers.)
> Hi Radim,
>
> I forgot to mention that I didn't create inline for these functions in V3.
> For an inline to work on across source files, I have to explicitly use
> "extern"; so I decided not to touch it in V3 yet.

(No need for the "extern inline" magic -- you can simply define them as
"static inline" in pmu.h.)

> If you insist, I will
> change it.

I don't, it has minimal impact.

> Another solution is to replace the functions with kvm_opmu_ops->blah_blah().
> But this looks less appealing to me.

Agreed, wrappers were a good idea.

Will review today, thanks for the explanation.