Re: [Xen-devel] [PATCH V2] mem_event: Add support for MEM_EVENT_REASON_MSR

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-01-02 at 13:56 +0000, Razvan Cojocaru wrote:
> >> +#define MEM_EVENT_REASON_MSR         7    /* MSR was hit: gfn is MSR 
> >> value, gla is MSR type;
> >> +                                             does NOT honour 
> >> HVMPME_onchangeonly */ 
> > 
> > Why doesn't/shouldn't it support onchangeonly?
> 
> It doesn't support onchangeonly because it is unreasonably complicated
> to get the old value (which we need to compare to the new one, and thus
> establish if onchangeonly applies or not).
> 
> If you look at hvm_msr_write_intercept() (it's at line 2917 in
> xen/arch/x86/hvm/hvm.c), depending on the value of the "msr" parameter
> there are several ways of setting the value: hvm_set_efer() if msr is
> MSR_EFER, hvm_set_guest_tsc() if msr is MSR_IA32_TSC, and so on. So
> rather than having a getter call for the current value for each of these
> MSR types, I found it cleaner to just notify the event channel that a
> value is being written, regardless of whether it's different from the
> currently stored value or not, and regardless of how the actual write is
> being handle depending on the MSR type.

That seems reasonable, thanks. Might be nice to include some of this
description in the changelog.

> > HVM_PARAM_MEMORY_EVENT_INT3 and HVM_PARAM_MEMORY_EVENT_SINGLE_STEP seem
> > not to support it either but they reject attempts to use them with it
> > (in do_hvm_op). I think this new value should do the same.
> 
> Understood, I agree. Will add this new modification and submit V3 of the
> patch tomorrow (no access to my development machine now).

Thanks.

> > By "MSR type" do you mean the address?
> 
> I mean one of the following: MSR_EFER, MSR_IA32_TSC, etc.
> Would you prefer that I change the "MSR type" expression in the comment
> to something else? What would make more sense?

Those names are #defines for the MSR address (see
xen/include/asm-x86/msr-index.h).

"MSR address" or "MSR number" would be better than "MSR type" (which
implies to me membership of some sort of class of MSRs)`

Ian.


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