Re: [Xen-devel] [PATCH V5 06/12] x86/hvm: factor out and rename vm_event related functions

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.02.15 at 17:33, <tamas.lengyel@xxxxxxxxxxxx> wrote:
> +static void hvm_event_cr(uint32_t reason, unsigned long value,
> +                                unsigned long old)
> +{
> +    vm_event_request_t req = {
> +        .reason = reason,
> +        .vcpu_id = current->vcpu_id,
> +        .u.mov_to_cr.new_value = value,
> +        .u.mov_to_cr.old_value = old
> +    };
> +    uint64_t parameters = 0;
> +
> +    switch(reason)

Coding style. Also I continue to think using switch() here rather than
having the caller pass both VM_EVENT_* and HVM_PARAM_* is ugly/
inefficient (even if the compiler may be able to sort this out for you).

> +    {
> +    case VM_EVENT_REASON_MOV_TO_CR0:
> +        parameters = current->domain->arch.hvm_domain
> +                      .params[HVM_PARAM_MEMORY_EVENT_CR0];
> +        break;
> +    case VM_EVENT_REASON_MOV_TO_CR3:
> +        parameters = current->domain->arch.hvm_domain
> +                      .params[HVM_PARAM_MEMORY_EVENT_CR3];
> +        break;
> +    case VM_EVENT_REASON_MOV_TO_CR4:
> +        parameters = current->domain->arch.hvm_domain
> +                      .params[HVM_PARAM_MEMORY_EVENT_CR4];
> +        break;
> +    };

In any event, if you stay with the current model, latch current
(used four times) into local variable.

> +void hvm_event_msr(unsigned int msr, uint64_t value)
> +{
> +    struct vcpu *curr = current;
> +    vm_event_request_t req = {
> +        .reason = VM_EVENT_REASON_MOV_TO_MSR,
> +        .vcpu_id = curr->vcpu_id,
> +        .u.mov_to_msr.msr = msr,
> +        .u.mov_to_msr.value = value,
> +    };
> +    uint64_t params = current->domain->arch.hvm_domain

Why "current" when you have "curr"?

Jan


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