From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v10 06/20] vmx: Merge MSR management routines Date: Mon, 08 Sep 2014 13:28:11 -0400 Message-ID: <540DE72B.50705@oracle.com> References: <1409802080-6160-1-git-send-email-boris.ostrovsky@oracle.com> <1409802080-6160-7-git-send-email-boris.ostrovsky@oracle.com> <540DF06E02000078000322DF@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <540DF06E02000078000322DF@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , Dietmar Hahn Cc: tim@xen.org, kevin.tian@intel.com, keir@xen.org, suravee.suthikulpanit@amd.com, andrew.cooper3@citrix.com, eddie.dong@intel.com, xen-devel@lists.xen.org, Aravind.Gopalakrishnan@amd.com, jun.nakajima@intel.com List-Id: xen-devel@lists.xenproject.org On 09/08/2014 12:07 PM, Jan Beulich wrote: >>>> On 04.09.14 at 05:41, wrote: >> vmx_add_host_load_msr() and vmx_add_guest_msr() share fair amount of code. >> Merge >> them to simplify code maintenance. >> >> Signed-off-by: Boris Ostrovsky >> Acked-by: Kevin Tian > Considering the re-work, is this really valid? Probably not (although the difference between this and earlier version is removal of one of the two routines). This also brings up another question --- I have kept Dietmar's Tested-by tags for a while now and I believe he tested this quite a few revisions back. I am not sure whether the tags are valid any longer. I would like to keep them since it was very helpful work on his part but I don't know what the policy is. (And I noticed that I accidentally dropped him from recipients list) >> -int vmx_add_guest_msr(u32 msr) >> +int vmx_add_msr(u32 msr, u8 type) > Why u8? No reason. I'll change it to int. >> --- a/xen/arch/x86/hvm/vmx/vmx.c >> +++ b/xen/arch/x86/hvm/vmx/vmx.c >> @@ -2263,12 +2263,12 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content) >> >> for ( ; (rc == 0) && lbr->count; lbr++ ) >> for ( i = 0; (rc == 0) && (i < lbr->count); i++ ) >> - if ( (rc = vmx_add_guest_msr(lbr->base + i)) == 0 ) >> + if ( (rc = vmx_add_msr(lbr->base + i, VMX_GUEST_MSR)) == 0 ) > Perhaps better keep the old functions as inline wrappers? Yes. -boris