Re: [PATCH 12/24] Add VMCS fields to the vmcs12

[Avi Kivity <avi@redhat.com>]

On 06/13/2010 03:28 PM, Nadav Har'El wrote:
> In this patch we add to vmcs12 (the VMCS that L1 keeps for L2) all the
> standard VMCS fields. These fields are encapsulated in a struct shadow_vmcs.
>
> Later patches will enable L1 to read and write these fields using VMREAD/
> VMWRITE, and they will be used during a VMLAUNCH/VMRESUME in preparing a real
> VMCS for running L2.
>
>
> +/* shadow_vmcs is a structure used in nested VMX for holding a copy of all
> + * standard VMCS fields. It is used for emulating a VMCS for L1 (see vmcs12),
> + * and also for easier access to VMCS data (see l1_shadow_vmcs).
> + */
> +struct __attribute__ ((__packed__)) shadow_vmcs {
>    


> +	u32 host_ia32_sysenter_cs;
> +	unsigned long cr0_guest_host_mask;
>    

I think I counted an odd number of u32 fields, which mean the ulong 
fields will be unaligned.  Please add padding to preserve natural alignment.

Have you considered placing often used fields together to reduce cache 
misses?  I'm not sure whether it's worth the effort.

>
>   /*
> @@ -139,6 +269,8 @@ struct __attribute__ ((__packed__)) vmcs
>   	u32 revision_id;
>   	u32 abort;
>
> +	struct shadow_vmcs shadow_vmcs;
> +
>   	bool launch_state; /* set to 0 by VMCLEAR, to 1 by VMLAUNCH */
>    

That will make it difficult to expand shadow_vmcs in the future.  I 
suggest putting it at the end, and reserving some space in the middle.

>
> +#define OFFSET(x) offsetof(struct shadow_vmcs, x)
> +
> +static unsigned short vmcs_field_to_offset_table[HOST_RIP+1] = {
>    

Can leave the size unspecified (and use ARRAY_SIZE() later on).

The encoding of the indexes is a very sparse, so the table will be very 
large.  No need to deal with that now though.

-- 
error compiling committee.c: too many arguments to function