[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



At 23:56 -0400 on 10 Sep (1347321409), Steven Maresca wrote:
> 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.

I think it's too late to get any more code changes in to 4.2.0, so this
will have to wait for 4.2.1.  Ian?

The patch looks OK, but why didn't you put the memory_event calls back
in mov_to_cr(), which is where they were before that?

Cheers,

Tim.

_______________________________________________
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®.