public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Architecture independence layer - v0
@ 2007-08-22  9:12 Christian Ehrhardt
       [not found] ` <46CBFDFD.3090501-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
  0 siblings, 1 reply; 2+ messages in thread
From: Christian Ehrhardt @ 2007-08-22  9:12 UTC (permalink / raw)
  To: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

Hello,
these three patches in v0 are be a base for discussion about an architecture
independence layer. It is part of an internal prototype we use to access 
the code
of our port as we develop it currently.
I know that a lot of things in the patches are still needing refining and
polishing, but it is enough to start the discussion about the basic 
mechanism
before I (or someone else) spend to much work working on such a layer.

The idea is, that x86 should always work however we split kvm internally to
generic/arch. The split in the current patches is done by gut feeling 
and will
improve by a) reviewing some functions more in detail, b) along the 
development
of our arch port and c) by your comments.

I know that the current kvm userspace is still very x86 centric and this 
will
(has to) change in the future, but we need an architecture mapping now 
to develop
our arch code. It should be adaptable to any userspace interface change 
that is coming.

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
- I split the code into kvm_main.c (generic) and kvm_x86.h/kvm_x86.c 
(arch portion)
- 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

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.

-- 
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization
+49 7031/16-3385
Ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
Ehrhardt-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen 
Geschäftsführung: Herbert Kircher 
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH 0/3] Architecture independence layer - v0
       [not found] ` <46CBFDFD.3090501-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
@ 2007-08-22  9:32   ` Avi Kivity
  0 siblings, 0 replies; 2+ messages in thread
From: Avi Kivity @ 2007-08-22  9:32 UTC (permalink / raw)
  To: Christian Ehrhardt
  Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@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/

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2007-08-22  9:32 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-08-22  9:12 [PATCH 0/3] Architecture independence layer - v0 Christian Ehrhardt
     [not found] ` <46CBFDFD.3090501-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2007-08-22  9:32   ` Avi Kivity

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox