Re: [PATCH 0 of 3] create kvm_x86

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

Hollis Blanchard wrote:
> On Fri, 2007-11-30 at 22:31 +0200, Avi Kivity wrote:
>   
>> Hollis Blanchard wrote:
>>     
>>> On Fri, 2007-11-30 at 11:04 +0200, Avi Kivity wrote:
>>>   
>>>       
>>>> Zhang, Xiantao wrote:
>>>>     
>>>>         
>>>>>>>       
>>>>>>>           
>>>>>>>               
>>>>>> The nicer one:
>>>>>>
>>>>>>    struct kvm {
>>>>>>         struct kvm_arch arch;
>>>>>>         // common fields
>>>>>>    }
>>>>>>     
>>>>>>         
>>>>>>             
>>>>> I prefer this one, seems it is more direct and readable. Same thinking
>>>>> about kvm_vcpu structure:)
>>>>>   
>>>>>       
>>>>>           
>>>> I agree, kvm_vcpu should use the same method.
>>>>     
>>>>         
>>> And we will convert vcpu_vmx/vcpu_svm as well?
>>>
>>>   
>>>       
>> These cannot use the same method, since we need to support both vmx and
>> svm in the same binary.  The arch specific members aren't the same size,
>> nor do the symbols they use have the same visibility.
>>     
>
> I have never understood this. Why on earth do you need to support VMX
> and SVM in the same binary? For example, when would you overwrite
> kvm_x86_ops after initialization? If you wouldn't, then why are you
> using function pointers instead of the linker?
>   

It's necessary for the distros to be able to ship both AMD and Intel 
support in a single binary.  We aren't talking, in general, about a 
single static binary but instead loadable modules.  There maybe some 
cases where it's useful to support both in a static kernel binary.

If you used the linker instead of function pointers, it would be 
impossible to build a static kernel binary that supported both.  Plus, 
depmod would get very confused because two modules would be providing 
the same symbols.  It can be made to work, but it's kind of funky.

> PowerPC will also need to support multiple processor types, and so I
> expect to have one kvm_arch structure for each. That also means struct
> kvm_arch must be the *last* member in struct kvm, which is not how it is
> shown above.
>   

Instead of having a kvm.ko and a kvm-ppc-440.ko, you probably should 
have a kvm.ko and a kvm-ppc.ko and then build the kvm-ppc.ko based on 
the board.  You would never build multiple kvm-ppc-XXX.ko modules in the 
same binary right?

Regards,

Anthony Liguori

>   



-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4