This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


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

To: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 03/14] Nested Virtualization: data structure
From: Christoph Egger <Christoph.Egger@xxxxxxx>
Date: Tue, 17 Aug 2010 12:48:36 +0200
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, "Dong, Eddie" <eddie.dong@xxxxxxxxx>
Delivery-date: Tue, 17 Aug 2010 03:49:30 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <C89022D1.1E126%keir.fraser@xxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <C89022D1.1E126%keir.fraser@xxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: KMail/1.9.10
On Tuesday 17 August 2010 12:28:17 Keir Fraser wrote:
> On 17/08/2010 11:01, "Christoph Egger" <Christoph.Egger@xxxxxxx> 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;
> };

All of this is possible but a union is actually only needed if you want
to access svm or vmx specific data from common code which is
bad from the software engineering side.
The advantage of using a pointer is that gcc can point you to
such a mistake.

In svm/vmx code you don't need explicit casts with a pointer.
Have a look at the top of the nsvm_vmcb_prepare4vmrun() function
in the nh_svm patch. There you see how I access 'struct nestedsvm'
w/o a cast.

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

It's purpose is to allow to copy svm/vmx specific data to somewhere else
w/o knowing them. It is currently nowhere needed. Once it turns out
it is neither needed for VMX it can go away.

> > 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

---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Xen-devel mailing list