[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] mem_event: fix regression affecting CR3, CR4 memory events



On Sep 10, 2012, at 6:06 PM, Steven Maresca wrote:

> On Mon, Sep 10, 2012 at 5:39 PM, Keir Fraser <keir@xxxxxxx> wrote:
>> On 10/09/2012 22:20, "Steven Maresca" <steve@xxxxxxxxxxxx> wrote:
>> 
>>> Given the refactoring in the commit related to the regression
>>> http://xenbits.xen.org/hg/xen-unstable.hg/rev/1276926e3795, it seemed
>>> (to me anyway) that inserting calls as shown in the patch would be
>>> cleaner, but I can definitely come up with some drawbacks.  However, I
>>> wanted to get this fixed for 4.2 if at all possible, so I wanted to
>>> send regardless.
>>> 
>>> In terms of drawbacks, this will require some ifdefs for x86_64, for 
>>> example.
>>> 
>>> Any suggestions for the cleanest means of achieving the same in vmx.c?
>> 
>> I don't understand this at all. The original commit moved some CR handling
>> to common HVM code. Your patch adds some mem_event calls into that common
>> HVM code. What would need to be "achieved" in vmx.c? What ifdefs would be
>> required for x86_64?
>> 
>> -- Keir
>> 
>> 
> 
> Keir,
> 
> In the process of moving CR handling to common code, that commit
> removed calls to hvm_memory_event_cr3() and hvm_memory_event_cr4().
> Without some patch restoring the calls to those two functions, current
> xen-unstable and xen-4.2-testing only reference them as function
> prototypes and function definitions -- there are no calls whatsoever.
> 
> I only mentioned an ifdef relative to x86_64 because of the #ifdef
> __x86_64__  around the function definitions. ( I know in the hvm.h
> itself they're just empty inline functions if not __x86_64__. )
> 
> Regarding vmx.c: hvm_memory_event_cr0 is called within vmx_cr_access.
> The patch I sent restoring the calls to hvm_memory_event_cr4 and
> hvm_memory_event_cr3 could be treated similarly, that's all.

The patch looks ok to me.
Acked-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>

You don't need to worry about x86_32. Is on its way out. You don't need to push 
this down into vmx either. Once AMD gains support for mem-event, this will just 
work for svm as well.

Like Keir says, no need for us. Your patch is good as is. Best to cc Tim Deegan 
<tim@xxxxxxx>, the mm maintainer.

Cheers
Andres

> 
> Steve


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.