Re: [PATCH] move structures to svm/vmx.c, use untyped arch_data in independent code

[Anthony Liguori <anthony-rdkfGonbjUSkNkDKm+mE6A@public.gmane.org>]

Paul Turner wrote:
>> >> > +     struct vmcs *vmcs;
>>
>> +     struct kvm_vcpu vcpu;
>>
>
> In this approach you might as well embed that at the start so that the
> arch independent code can still allocate/free, that or move the memory
> alloc/dealloc entirely to the arch specific code.  Although that
> should probably be done anyway with this approach otherwise it's not
> clear exactly what is occurring from the arch independent code path's
> pov.

If it's not too much churn, I'd think that allocating the vcpu structure 
in the arch dependent code would make the most sense since it's that 
code that knows how large the structure should be.

>> >> >   #define IOPM_ALLOC_ORDER 2
>> >> >   #define MSRPM_ALLOC_ORDER 1
>> >> >
>> >> > @@ -95,7 +115,7 @@ static inline u32 svm_has(u32 feat)
>> >> >
>> >> >   static unsigned get_addr_size(struct kvm_vcpu *vcpu)
>> >> >   {
>> >> > -     struct vmcb_save_area *sa = &vcpu->svm->vmcb->save;
>> >> > +     struct vmcb_save_area *sa = &svm(vcpu)->vmcb->save;
>>
>> I think this needs to be:
>>
>> struct kvm_svm_data *svm = kvm_vcpu_to_svm_data(vcpu);
>> struct vmcb_save_area *sa = &svm->vmcb->save;
>>
>> The rest should just follow the same style.
>
> Motivating reason?

Clarity on the part of the reader.  The problem with using a macro like 
this is that it's not very clear in this function what the type of 
svm(vcpu) is.  There's also a bit of magic as to how svm() gets the 
whatever the type is from a vcpu.  Not to mention the fact that svm() is 
a horrible function name :-)

By using the typeX_to_typeY idiom, it's very clear to the reader what's 
going on.  It's just another embedded structure who's conversion 
function is container_of() based.

Regards,

Anthony Liguori

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/