Re: [PATCH 1/2] x86/VMX: introduce vmx_find_guest_msr()

["Jan Beulich" <JBeulich@suse.com>]

>>> On 25.01.17 at 18:26, <sergey.dyasli@citrix.com> wrote:
> @@ -1283,19 +1284,36 @@ static int construct_vmcs(struct vcpu *v)
>      return 0;
>  }
>  
> -int vmx_read_guest_msr(u32 msr, u64 *val)
> +static int vmx_msr_entry_key_cmp(const void *key, const void *elt)
>  {
> -    struct vcpu *curr = current;
> -    unsigned int i, msr_count = curr->arch.hvm_vmx.msr_count;
> +    const u32 *msr = key;
> +    const struct vmx_msr_entry *entry = elt;
> +
> +    if ( *msr > entry->index )
> +        return 1;
> +    if ( *msr < entry->index )
> +        return -1;
> +    return 0;
> +}
> +
> +struct vmx_msr_entry *vmx_find_guest_msr(const u32 msr)
> +{
> +    const struct vcpu *curr = current;
> +    const unsigned int msr_count = curr->arch.hvm_vmx.msr_count;
>      const struct vmx_msr_entry *msr_area = curr->arch.hvm_vmx.msr_area;

I'm not convinced the latter two local variables are particularly
useful. I'm also not convinced constifying non-pointed-to types
is all that useful (which also applies to the function parameter).
But I'll leave the decision whether to accept this to the VM
maintainers of course.

> +static int vmx_msr_entry_cmp(const void *a, const void *b)
> +{
> +    const struct vmx_msr_entry *l = a, *r = b;
> +
> +    if ( l->index > r->index )
> +        return 1;
> +    if ( l->index < r->index )
> +        return -1;
> +    return 0;
> +}

I'd prefer if there was just a single compare function, even if that
requires widening the key for the bsearch() invocation. Alternatively
...

> @@ -1369,6 +1410,9 @@ int vmx_add_msr(u32 msr, int type)
>          msr_area_elem->data = 0;
>          __vmwrite(VM_EXIT_MSR_STORE_COUNT, *msr_count);
>          __vmwrite(VM_ENTRY_MSR_LOAD_COUNT, *msr_count);
> +
> +        sort(*msr_area, *msr_count, sizeof(struct vmx_msr_entry),
> +             vmx_msr_entry_cmp, vmx_msr_entry_swap);

... how about avoiding the sort() here altogether, by simply
going through the list linearly (which, being O(n), is still faster
than sort())? The more that there is a linear scan already
anyway. At which point it may then be beneficial to also keep
the host MSR array sorted.

Jan


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