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

[Tim Deegan <tim@xen.org>]

At 17:42 +0100 on 22 Jan (1421944966), 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:
> >> +static int hvm_event_traps(long parameters, vm_event_request_t *req)
> >> +{
> >> +    int rc;
> >> +    struct vcpu *v = current;
> >> +    struct domain *d = v->domain;
> >
> > Unless the intention is to have an exact 1:1 copy of the original,
> > please use curr and currd here respectively.
> 
> Ack.
> 
> >
> >> +void hvm_event_cr0(unsigned long value, unsigned long old)
> >> +{
> >> +    vm_event_request_t req = {
> >> +        .reason = VM_EVENT_REASON_CR0,
> >> +        .vcpu_id = current->vcpu_id,
> >> +        .cr_event.new_value = value,
> >> +        .cr_event.old_value = old
> >> +    };
> >> +
> >> +    long parameters = current->domain->arch.hvm_domain
> >> +                        .params[HVM_PARAM_MEMORY_EVENT_CR0];
> >
> > And latch current into a local variable curr here and below.
> 
> Ack.
> 
> >
> >> +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?

Yep - ISTR this is also why the MSR event doesn't respect
HVMPME_onchangeonly.

Tim.