Re: [PATCH 0/3] Architecture independence layer - v0

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Christian Ehrhardt wrote:
> Basics I used:
> - Since architecture will not change at runtime I do not use a arch_ops 
> structure
> - Instead I used a hidden Kconfig value specifying the kvm architecture 
> which then
>   influence the make file to link together the generic and the arch 
> specific part
>   to one kvm module
>   

Isn't there already such a config variable (e.g. CONFIG_X86, 
CONFIG_X86_64, CONFIG_S390)?

> - I split the code into kvm_main.c (generic) and kvm_x86.h/kvm_x86.c 
> (arch portion)
>   

Just x86.[ch], please.  Similarly for all other new files.

> - A new header kvm_arch.h specifies the interface the architecture has 
> to implement
> - The ioctl interfaces are implemented by the generic kvm_main.c in 
> three ways
> a) a completely generic function is covered by kvm_main.c completely
> b) a completely arch dependent function is not covered in kvm_main.c - 
> at the end
>    each ioctl goes to a architecture mapping doing the rest of the ioctl 
> not handled
>    in the generic part e.g. in kvm_vcpu_ioctl:
>         default:
>                r = kvm_arch_vcpu_ioctl(filp, ioctl, arg);
>         }
> c) functions that have a significant amount of shared code are 
> implemented by
>    kvm_main.c using kvm_arch_* to implement the arch specific part. 
> Either by just
>    mapping the old *_ioctl_whatever to *_arch_whatever or by moving some 
> more of the
>    code out of the arch function up to the generic portion e.g.:
>           case KVM_RUN:
>                 r = -EINVAL;
>                 if (arg)
>                         goto out;
>                 r = kvm_arch_vcpu_run(vcpu, vcpu->run);
>                 break;
> - there is still a lot of potential to move code up and down along these 
> split, but
>   thats it so far. I strive to improve that patch series until we are 
> happy with it
>   

I agree with all these points.  In addition, I'd like to point out that 
in some cases an API is generic, but some of its implementation isn't, 
and its arguments are not generic.  The example I have in mind is 
KVM_GET_REGS/KVM_SET_REGS.  In this case, the argument structures should 
be defined in arch dependent headers.

> The patch compiles and works without issues on my Opteron based machine 
> here,
> unfortunately I have no Intel base HW to test.
> I look forward to every comment. Even if this approach may not be what 
> we want in
> the end, at least it starts the discussion how to do it :-)
>
> Grüsse / Regards
> Christian Ehrhardt
>
> P.S. Still a todo and not (yet) covered in this patch proposal is the 
> arch splitting of structures like vcpu to a generic and an arch part.
>   

Yes.  I don't think this will be difficult.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
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/