Re: [Xen-devel] [PATCH v8 05/19] vmx: Merge MSR management routines

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.07.14 at 16:37, <boris.ostrovsky@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -1176,117 +1176,107 @@ int vmx_write_guest_msr(u32 msr, u64 val)
>      return -ESRCH;
>  }
>  
> -int vmx_add_guest_msr(u32 msr)
> +int vmx_add_msr(u32 msr, u8 type)
>  {
>      struct vcpu *curr = current;
> -    unsigned int i, msr_count = curr->arch.hvm_vmx.msr_count;
> -    struct vmx_msr_entry *msr_area = curr->arch.hvm_vmx.msr_area;
> +    unsigned int idx, *msr_count;
> +    struct vmx_msr_entry **msr_area;
> +
> +    ASSERT((type == VMX_GUEST_MSR) || (type == VMX_HOST_MSR));
> +
> +    if ( type == VMX_GUEST_MSR )

Better (smaller code) to move the ASSERT() into the else branch in
cases like this.

> -    msr_area[msr_count].index = msr;
> -    msr_area[msr_count].mbz   = 0;
> -    msr_area[msr_count].data  = 0;
> -    curr->arch.hvm_vmx.msr_count = ++msr_count;
> -    __vmwrite(VM_EXIT_MSR_STORE_COUNT, msr_count);
> -    __vmwrite(VM_ENTRY_MSR_LOAD_COUNT, msr_count);
> +    (*msr_area)[*msr_count].index = msr;
> +    (*msr_area)[*msr_count].mbz   = 0;
> +    (*msr_count)++;
> +    if ( type == VMX_GUEST_MSR )
> +    {
> +        (*msr_area)[*msr_count - 1].data  = 0;
> +        __vmwrite(VM_EXIT_MSR_STORE_COUNT, *msr_count);
> +        __vmwrite(VM_ENTRY_MSR_LOAD_COUNT, *msr_count);
> +    }
> +    else
> +    {
> +        rdmsrl(msr, (*msr_area)[*msr_count - 1].data);
> +        __vmwrite(VM_EXIT_MSR_LOAD_COUNT, *msr_count);
> +    }

For better readability perhaps worth having a local variable set to
*msr_area + *msr_count, at once allowing to avoid the slightly ugly
*msr_count - 1 in both the if and else branches.

> +    --(*msr_count);
> +    memmove(&(*msr_area)[idx], &(*msr_area)[idx + 1],
> +            sizeof(struct vmx_msr_entry) * (*msr_count - idx));
> +    msr_area[(*msr_count)]->index = 0;

Pointless pairs of parentheses in the first and last quoted lines above.

Jan


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