RE: [Xen-devel] [PATCH 04/16] vmx: nest: nested control structure

["Dong, Eddie" <eddie.dong@xxxxxxxxx>]

Christoph Egger wrote:
> On Wednesday 08 September 2010 17:22:12 Qing He wrote:
>> v->arch.hvm_vmx.nest as control structure
>> 
>> Signed-off-by: Qing He <qing.he@xxxxxxxxx>
>> Signed-off-by: Eddie Dong <eddie.dong@xxxxxxxxx>
>> 
>> ---
>> diff -r fc4de5eedd1d xen/include/asm-x86/hvm/vmx/nest.h
>> --- /dev/null        Thu Jan 01 00:00:00 1970 +0000
>> +++ b/xen/include/asm-x86/hvm/vmx/nest.h     Wed Sep 08 21:03:41 2010
>> +0800 @@ -0,0 +1,45 @@ +/*
>> + * nest.h: nested virtualization for VMX.
>> + *
>> + * Copyright (c) 2010, Intel Corporation.
>> + * Author: Qing He <qing.he@xxxxxxxxx>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> modify it + * under the terms and conditions of the GNU General
>> Public License, + * version 2, as published by the Free Software
>> Foundation. + * + * This program is distributed in the hope it will
>> be useful, but WITHOUT + * ANY WARRANTY; without even the implied
>> warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE.
>> See the GNU General Public License for + * more details. + *
>> + * You should have received a copy of the GNU General Public
>> License along with + * this program; if not, write to the Free
>> Software Foundation, Inc., 59 Temple + * Place - Suite 330, Boston,
>> MA 02111-1307 USA. + * + */
>> +#ifndef __ASM_X86_HVM_NEST_H__
>> +#define __ASM_X86_HVM_NEST_H__
>> +
>> +struct vmcs_struct;
>> +
>> +struct vmx_nest_struct {
> 
> Is it ok to name it 'struct nestedvmx' ?

Fine, renamed to nested_vmx.

> 
>> +    paddr_t              guest_vmxon_pa;
>> +
>> +    /* Saved host vmcs for vcpu itself */
>> +    struct vmcs_struct  *hvmcs;
>> +
>> +    /*
>> +     * Guest's `current vmcs' of vcpu
>> +     *  - gvmcs_pa: guest VMCS region physical address
>> +     *  - vvmcs:    (guest) virtual vmcs
>> +     *  - svmcs:    effective vmcs for the guest of this vcpu
>> +     *  - valid:    launch state: invalid on clear, valid on ld +  
>> */ +    paddr_t              gvmcs_pa;
>> +    void                *vvmcs;
>> +    struct vmcs_struct  *svmcs;
>> +    int                  vmcs_valid;
>> +};
>> +
>> +#endif /* __ASM_X86_HVM_NEST_H__ */
>> diff -r fc4de5eedd1d xen/include/asm-x86/hvm/vmx/vmcs.h
>> --- a/xen/include/asm-x86/hvm/vmx/vmcs.h     Wed Sep 08 21:00:00 2010
>> +0800 +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h       Wed Sep 08 21:03:41
>>  2010 +0800 @@ -22,6 +22,7 @@ #include <asm/config.h>
>>  #include <asm/hvm/io.h>
>>  #include <asm/hvm/vpmu.h>
>> +#include <asm/hvm/vmx/nest.h>
>> 
>>  extern void vmcs_dump_vcpu(struct vcpu *v);
>>  extern void setup_vmcs_dump(void);
>> @@ -99,6 +100,9 @@
>>      u32                  secondary_exec_control;
>>      u32                  exception_bitmap;
>> 
>> +    /* nested virtualization */
>> +    struct vmx_nest_struct nest;
>> +
>>  #ifdef __x86_64__
>>      struct vmx_msr_state msr_state;
>>      unsigned long        shadow_gs;
> 
> 
> I think, the structure should be allocated in the
> nestedhvm_vcpu_initialise() function hook and assigned to the nh_arch
> pointer in struct nestedhvm. 

Well, the structure itself is pretty small, so dynamic allocation is really not 
a good idea.
Instead, the internal field such as vvmcs/svmcs are big and thus we use a 
pointer, but they are allocated on demand.
This follows the style of arch_vmx_struct in vcpu data structure.

I am fine with your nestedhvm_vcpu_initialise "design", but VMX doesn't need to 
use the wrapper so far.

Thx, Eddie

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel