From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH] svm: support VMCB cleanbits Date: Thu, 16 Dec 2010 08:47:04 +0000 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: "Huang2, Wei" , "Egger, Christoph" , Tim Deegan Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On 15/12/2010 23:04, "Huang2, Wei" wrote: > Hi Keir, > > Thanks for putting up this patch. I think the comments you made are correct > after reading the spec again. Christoph and I misread some APM content. :-( No problem then. It would be good to know that the applied patch works and with the same performance win as you saw with your original patch. -- Keir > Thanks again, > -Wei > > -----Original Message----- > From: xen-devel-bounces@lists.xensource.com > [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser > Sent: Wednesday, December 15, 2010 10:56 AM > To: Egger, Christoph; Tim Deegan > Cc: xen-devel@lists.xensource.com > Subject: Re: [Xen-devel] [PATCH] svm: support VMCB cleanbits > > On 15/12/2010 12:36, "Christoph Egger" wrote: > >> On Wednesday 15 December 2010 12:27:51 Tim Deegan wrote: >>> This seems to change the logic so it doesn't clear the intercepts if >>> debug_state == 0. Is that OK? >> >> No, that's not ok. I fixed that in the new patch. >> >>> More generally, I'm not sure I like having all the VMCB accessor >>> functions in files called "cleanbits" -- wouldn't it make sense to have >>> all that in the vmcb files so people will see them and know to use them? >>> You could rename the actual vmcb fields as well to catch anyone writing >>> them directly, e.g. in forward-ported patches. >> >> I renamed the 'svmcleanbits.[ch]' files to 'vmc_funcs.[ch]' >> >> Thanks for your review. > > I went through this patch quite brutally when I applied it (c/s 22546). In > particular I made the VMCB field accessor functions more consistent in name > and semantics, and pulled out their implementations into a common macro to > make the code clearer. > > There should be no significant changes compared with your patch *EXCEPT*: > 1. Updates to the MSR and I/O bitmaps do not affect clear bits > 2. Updates to lbr_control.enable do not affect clear bits > 3. Updates to debugctlmsr *do* affect clear bits > > In the above I am following what is described in AMD Volume 2 Section > 15.15.3 "VMCB Clean Field". > > I note that the MSRPM_BASE and IOPM_BASE fields are listed as cacheable, but > *no* mention is made of caching the bitmap contents. > > Also, bit 10 (LBR) has debugctlmsr listed as cacheable, but again *no* > mention is made of the lbr_control.enable bit flag. > > If any of the above is wrong, then: (a) the reference manual should be > fixed; (b) I would accept a fixup patch, with a patch description > explaininbg why behaviour is deviating from cleanbits behaviour describved > in the latest version of the AMD reference manuals. > > -- Keir > >> Signed-off-by: Wei Huang >> Signed-off-by: Christoph Egger >> >> Christoph >> > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel > >