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

[Razvan Cojocaru <rzvncj@xxxxxxxxx>]

>> +#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.

> 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).

> 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?

Thanks,
Razvan Cojocaru

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