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

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

On 17/08/2010 11:48, "Christoph Egger" <Christoph.Egger@amd.com> 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.