Anthony Liguori wrote:
> 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.
>
Generally I'd agree, but in this case the function is oft-used and
limited in scope to just one file. So maybe:
struct kvm_svm_data *svm = __svm(vcpu);
An advantage of embedding vcpu into svm/vmx is that the offsetoff macros
used to feed inline assembly still work.
--
error compiling committee.c: too many arguments to function
-------------------------------------------------------------------------
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/
_______________________________________________
kvm-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel
