Re: [PATCH 03/14] Nested Virtualization: data structure

[Keir Fraser <keir.fraser@eu.citrix.com>]

On 17/08/2010 11:01, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>> Yes, we should be strict on the layout of this structure. SVM/VMX-specific
>> stuff goes into a sub-structure in a union. Absolutely.
> 
> I have moved the SVM/VMX-specific pieces into the 'void *nh_arch' field above.
> It is initialized in the svm/vmx specific vcpu initialization.

I suggest to make this a union including SVM/VMX-specific struct pointers.
It will avoid unnecessary explicit casting, and you can use an anonymous
union if you want. Is using pointers better than actually including the
structures in the union, do you think?

So I mean something like: union {
    void *nh_arch;
    struct nestedsvm *nh_svm;
    struct nestedvmx *nh_vmx;
};

Or: union {
    struct nestedsvm nh_svm;
    struct nestedvmx nh_vmx;
};

What is the nh_arch_size field for? Well I can guess what it represents, but
why do you need such a thing?

> When you look into the svm specific patch, you will find a 'struct nestedsvm'
> in xen/include/asm-x86/hvm/svm/vmcb.h
> 
>> And you would only go peeking at the SVM sub-structure
>> if hvm_svm_enabled(v)==TRUE.
> 
> Correct. On a Intel CPU Xen should never allow the guest to
> set the EFER.SVME bit.
> 
>> And we'd have a similar predicate hvm_vmx_enabled(v)==TRUE, presumably.
>> And maybe a generic hvm_nestedvirt_enabled(v) too.
> 
> What you call hvm_nestedvirt_enabled() actually exists as nestedhvm_enabled().

Fine.

 -- Keir