Re: [RFC PATCH V2 8/8] x86/hvm: factor out vm_event related functions into separate file

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 01/22/2015 06:42 PM, Tamas K Lengyel wrote:
> On Thu, Jan 22, 2015 at 5:25 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 18.01.15 at 16:18, <tamas.lengyel@zentific.com> wrote:
>>> +void hvm_event_msr(unsigned long msr, unsigned long value)
>>> +{
>>> +    vm_event_request_t req = {
>>> +        .reason = VM_EVENT_REASON_MSR,
>>> +        .vcpu_id = current->vcpu_id,
>>> +        .msr_event.msr = msr,
>>> +        .msr_event.new_value = value,
>>> +    };
>>
>> The .msr_event sub-structure also has an old_value member - how
>> come this doesn't get filled (I realize the old code was this way,
>> but I now doubt earlier patches are all correct in the regard).
> 
> Razvan might have more information on this side as I haven't really
> touched MSR events. I vaguely recall some issues with having access to
> the old MSR value?

Indeed, getting the previous value would be a bit involved. Please see
xen/arch/x86/hvm/hvm.c, specifically hvm_msr_write_intercept(). At first
glance, you'd have to call hvm_msr_read_intercept() to get the previous
value, or create some sort of cache for previous values for all MSRs,
updated by hvm_msr_write_intercept().

Since this has not had any real value for my use-case, and since my code
has only needed to keep track of a handful of MSRs, I took the route of
caching previous values for them in the userspace application.

This is, of course, not to say that it can't be done at the HV level,
just that (at least IMHO) the HV-level costs have outweighed the benefits.


Thanks for the question,
Razvan