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

[Steven Maresca <steve@xxxxxxxxxxxx>]

On Mon, Sep 10, 2012 at 9:49 PM, Aravindh Puthiyaparambil
<aravindh@xxxxxxxxxxxx> wrote:
> On Mon, Sep 10, 2012 at 3:06 PM, Steven Maresca <steve@xxxxxxxxxxxx> 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.
>
> Looks good to me.
> Acked-by: Aravindh Puthiyaparambil <aravindh@xxxxxxxxxxxx>
>
> Cheers,
> Aravindh

Adding Tim Deegan as suggested.

Tim, this is a regression fix for CR3/CR4 memory events
unintentionally removed during some refactoring of HVM CR handling.
If at all possible, for 4.2 inclusion before we officially leave RC.
Acks received from Andres and Aravindh. Thanks to all for the quick
review.

Take care,
Steve

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