Re: [kvm-ppc-devel] [PATCH] Split kvm_vcpu to support new archs.

[Hollis Blanchard <hollisb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>]

On Thu, 2007-10-18 at 16:31 -0500, Anthony Liguori wrote:
> Hollis Blanchard wrote:
> > On Thu, 2007-10-18 at 16:04 -0500, Anthony Liguori wrote:
> >   
> >> Hollis Blanchard wrote:
> >>     
> >>> What is the problem with
> >>> embedding an architecture-specific sub-structure, i.e.
> >>>         struct kvm_vcpu {
> >>>                 ...
> >>>                 struct arch_kvm_vcpu arch_vcpu;
> >>>         };
> >>>   
> >>>       
> >> I think you want the opposite direction of nesting. 
> >>     
> > ...
> >   
> >> We should move to:
> >>
> >> struct kvm_vcpu {
> >>  /* stuff common to x86/ppc/ia64 */
> >> };
> >>
> >> struct vcpu_x86 {
> >>   struct kvm_vcpu vcpu;
> >>   /* stuff common to vt/svm */
> >> }
> >>
> >> struct vcpu_svm {
> >>   struct vcpu_x86 vcpu;
> >>   /* svm specific stuff  */
> >> };
> >>     
> >
> > Why?
> >   
> 
> It provides better encapsulation.  If you have a kvm_vcpu, unless you do 
> container_of(), you can't access the arch_vcpu.  It helps make sure that 
> architecture common code remains common.

I must be misunderstanding, because this seems completely backwards to
me. With your nesting, any time architecture code wants to access
architecture state (which is almost all the time), you'd *need*
container_of:

        void arch_func(struct kvm_vcpu *vcpu) {
                struct arch_vcpu *arch = container_of(vcpu, arch_vcpu,
                arch);
                arch->gpr[3] = 0;
        }

In contrast, my nesting proposal would look like this:

        void arch_func(struct kvm_vcpu *vcpu) {
                vcpu->arch.gpr[3] = 0;
        }

> It also leaves open the possibility of supporting multiple architectures 
> at the same time.  I don't know why you would want to do that :-)

That's true, though this could also be accomplished by keeping arch_vcpu
as the last member of kvm_vcpu.

-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/