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: Christoph Egger <Christoph.Egger@xxxxxxx>
Subject: Re: [Xen-devel] [PATCH 03/14] Nested Virtualization: data structure
From: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Date: Tue, 17 Aug 2010 12:02:41 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, "Dong, Eddie" <eddie.dong@xxxxxxxxx>
Delivery-date: Tue, 17 Aug 2010 04:06:01 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <201008171248.37216.Christoph.Egger@xxxxxxx>
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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Acs9+dJWNq1ouExVTSSAUxEm6PYfoQAAeOvc
Thread-topic: [Xen-devel] [PATCH 03/14] Nested Virtualization: data structure
User-agent: Microsoft-Entourage/
On 17/08/2010 11:48, "Christoph Egger" <Christoph.Egger@xxxxxxx> wrote:

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

Hm, I see. Well that's okay I guess. Two further comments then, by the by:
(1) Not sure why you hide it behind macro VCPU_NESTEDHVM(). That seems quite
pointless and you may as well reference v->...nh_arch directly. Apart from
that I don't like capitalised macros much anyway. If you must keep a macro
(why?) then at least don't capitalise it.
(2) No text speak in function names please. I get fed up enough with
blah2blah for conversion functions. Don't bring the numeral 4 into the fray
as well. This function would be just as clear with the '4' changed to '_'.

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

Yeah, it's pointless then. Noone's going to want to copy gobs of anonymous
arcbitrary vcpu-specific data.

 -- Keir

> It is currently nowhere needed. Once it turns out
> it is neither needed for VMX it can go away.

Xen-devel mailing list