From mboxrd@z Thu Jan  1 00:00:00 1970
From: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
Subject: Re: [PATCH V2 3/5] KVM: x86/vPMU: Create vPMU interface for VMX and
 SVM
Date: Tue, 21 Apr 2015 11:33:49 +0200
Message-ID: <20150421093345.GD26478@potion.brq.redhat.com>
References: <1428509905-32352-1-git-send-email-wei@redhat.com>
 <1428509905-32352-4-git-send-email-wei@redhat.com>
 <20150409194344.GC9729@potion.brq.redhat.com>
 <5535466A.5000808@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
	gleb@kernel.org
To: Wei Huang <wei@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:44740 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1750867AbbDUJd4 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 21 Apr 2015 05:33:56 -0400
Content-Disposition: inline
In-Reply-To: <5535466A.5000808@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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.