Re: [Xen-devel] [PATCH 3/4] nested vmx: optimize for bulk access of virtual VMCS

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.01.13 at 06:37, Dongxiao Xu <dongxiao.xu@xxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -829,6 +829,34 @@ static void vvmcs_to_shadow(void *vvmcs, unsigned int 
> field)
>      __vmwrite(field, value);
>  }
>  
> +static void vvmcs_to_shadow_bulk(void *vvmcs, int n, u16 *field)
> +{
> +    u64 *value = NULL;
> +    int i = 0;

Both 'n' and 'i' should be unsigned. And there again is a pointless
initializer here.

> +
> +    if ( cpu_has_vmx_vmcs_shadowing )
> +    {
> +        value = xzalloc_array(u64, n);
> +        if ( !value )
> +            goto fallback;
> +
> +        virtual_vmcs_enter(vvmcs);
> +        for ( i = 0; i < n; i++ )
> +            value[i] = __vmread(field[i]);
> +        virtual_vmcs_exit(vvmcs);
> +
> +        for ( i = 0; i < n; i++ )
> +            __vmwrite(field[i], value[i]);
> +
> +        xfree(value);
> +        return;
> +    }
> +
> +fallback:
> +    for ( i = 0; i < n; i++ )
> +        vvmcs_to_shadow(vvmcs, field[i]);

Putting the fallback code in a conditional and the "normal" code
outside would reduce overall amount of indentation.

> +}
> +
>  static void shadow_to_vvmcs(void *vvmcs, unsigned int field)
>  {
>      u64 value;
> @@ -862,13 +918,17 @@ static void load_shadow_guest_state(struct vcpu *v)
>  {
>      struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
>      void *vvmcs = nvcpu->nv_vvmcx;
> -    int i;
>      u32 control;
>      u64 cr_gh_mask, cr_read_shadow;
>  
> +    u16 vmentry_fields[] = {
> +        VM_ENTRY_INTR_INFO,
> +        VM_ENTRY_EXCEPTION_ERROR_CODE,
> +        VM_ENTRY_INSTRUCTION_LEN,
> +    };
> +
>      /* vvmcs.gstate to shadow vmcs.gstate */
> -    for ( i = 0; i < ARRAY_SIZE(vmcs_gstate_field); i++ )
> -        vvmcs_to_shadow(vvmcs, vmcs_gstate_field[i]);
> +    vvmcs_to_shadow_bulk(vvmcs, ARRAY_SIZE(vmcs_gstate_field), (u16 
> *)vmcs_gstate_field);

The cast should be dropped as being dangerous. Just const qualify
the function parameter.

Also - long line?

Jan


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