[Qemu-devel] [PATCH] hook cpu running at a higher level.

[Qemu-devel] [PATCH] hook cpu running at a higher level.

[Glauber Costa]

This patch removes the kvm_enabled() check from cpu-exec.c.
This file is highly tcg-specific, and we'll probably want it
out when tcg is not compiled in (coming soon, in a theathe near you)

Instead, we hook at the main loop level. The amount of code
duplication introduced is at worst, acceptable, and I believe
it pays. The tcg mainloop is likely to be different from the
hypervisors ones, since tcg runs all cpus in lockstep. KVM
(and probably xen), will be able to span threads out for its
vcpus.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 cpu-exec.c |    5 -----
 kvm-all.c  |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 vl.c       |   16 ++++++++++------
 3 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index ed1545b..be8ceac 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -333,11 +333,6 @@ int cpu_exec(CPUState *env1)
             }
 #endif
 
-            if (kvm_enabled()) {
-                kvm_cpu_exec(env);
-                longjmp(env->jmp_env, 1);
-            }
-
             next_tb = 0; /* force lookup of first TB */
             for(;;) {
                 interrupt_request = env->interrupt_request;
diff --git a/kvm-all.c b/kvm-all.c
index 11034df..a279d6c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -663,3 +663,53 @@ int kvm_has_sync_mmu(void)
 
     return 0;
 }
+
+extern CPUState *cur_cpu;
+extern CPUState *next_cpu;
+
+extern int reset_requested;
+extern int shutdown_requested;
+extern int powerdown_requested;
+
+int kvm_main_loop(void)
+{
+
+    int ret, timeout;
+    CPUState *env;
+
+    cur_cpu = first_cpu;
+    next_cpu = first_cpu;
+    env = first_cpu;
+
+    for(;;) {
+        /* get next cpu */
+        cpu_single_env = env;
+        ret = kvm_cpu_exec(env);
+        cpu_single_env = NULL;
+
+        if (shutdown_requested)
+            break;
+        if (reset_requested) {
+            reset_requested = 0;
+            qemu_system_reset();
+            ret = EXCP_INTERRUPT;
+        }
+        if (powerdown_requested) {
+            powerdown_requested = 0;
+            qemu_system_powerdown();
+            ret = EXCP_INTERRUPT;
+        }
+
+        if (ret == EXCP_HALTED) {
+            timeout = 5000;
+        } else {
+            timeout = 0;
+        }
+        
+        main_loop_wait(timeout);
+    }        
+    cpu_disable_ticks();
+    return ret;
+}
+
+
diff --git a/vl.c b/vl.c
index 0a02151..bcaccc3 100644
--- a/vl.c
+++ b/vl.c
@@ -248,8 +248,8 @@ static struct drive_opt {
     char opt[1024];
 } drives_opt[MAX_DRIVES];
 
-static CPUState *cur_cpu;
-static CPUState *next_cpu;
+CPUState *cur_cpu;
+CPUState *next_cpu;
 static int event_pending = 1;
 /* Conversion factor from emulated instructions to virtual clock ticks.  */
 static int icount_time_shift;
@@ -3452,9 +3452,9 @@ typedef struct QEMUResetEntry {
 } QEMUResetEntry;
 
 static QEMUResetEntry *first_reset_entry;
-static int reset_requested;
-static int shutdown_requested;
-static int powerdown_requested;
+int reset_requested;
+int shutdown_requested;
+int powerdown_requested;
 
 int qemu_shutdown_requested(void)
 {
@@ -5535,7 +5535,11 @@ int main(int argc, char **argv, char **envp)
 	close(fd);
     }
 
-    main_loop();
+    if (kvm_enabled())
+        kvm_main_loop();
+    else
+        main_loop();
+
     quit_timers();
     net_cleanup();
 
-- 
1.5.6.5

[Qemu-devel] Re: [PATCH] hook cpu running at a higher level.

[Ian Jackson]

Glauber Costa writes ("[PATCH] hook cpu running at a higher level."):
> This patch removes the kvm_enabled() check from cpu-exec.c.
> This file is highly tcg-specific, and we'll probably want it
> out when tcg is not compiled in (coming soon, in a theathe near you)

That would be interesting, certainly.  We don't compile that file
anyway right now, and have our own version of exec.c too (sadly it's
somewhat clone-and-hack and it would be nice to make that neater in
due course).

> Instead, we hook at the main loop level. The amount of code
> duplication introduced is at worst, acceptable, and I believe
> it pays. The tcg mainloop is likely to be different from the
> hypervisors ones, since tcg runs all cpus in lockstep. KVM
> (and probably xen), will be able to span threads out for its
> vcpus.

I don't have an opinion about this patch really, but I did want to
comment slightly on the way Xen does things just for future reference.

The Xen qemu process runs only in one thread which is fine because it
doesn't need to be involved with actual processor execution.  In
theory parallel execution (in different threads and thus on different
physical cpus) of IO emulations requested by different guest vcpus
might make some small performance difference but I doubt it would be
worth our while.  So I think the Xen setup will still from qemu's
point of view look like a single vcpu no matter how many vcpus the
guest aactually has.

> -    main_loop();
> +    if (kvm_enabled())
> +        kvm_main_loop();
> +    else
> +        main_loop();
> +

The way we have approached these problems in the Xen tree is to supply
an alternative implementation of (say) main_loop and arrange for the
standard one not to be compiled.  Is it the intent to make kvm a
run-time selectable option ?  It seems to me that that given that we
already have different qemu builds for all of the various target
(guest) cpu architectures, it might be simpler to continue that
approach.  With a bit of judicious movement of code into appropriate
files, this will avoid the need for ifs and ifdefs.

Regards,
Ian.

[Qemu-devel] Re: [PATCH] hook cpu running at a higher level.

[Avi Kivity]

Ian Jackson wrote:
> The way we have approached these problems in the Xen tree is to supply
> an alternative implementation of (say) main_loop and arrange for the
> standard one not to be compiled.  Is it the intent to make kvm a
> run-time selectable option ?  It seems to me that that given that we
> already have different qemu builds for all of the various target
> (guest) cpu architectures, it might be simpler to continue that
> approach.  With a bit of judicious movement of code into appropriate
> files, this will avoid the need for ifs and ifdefs.
>   

kvm is run-time selectable, both in upstream and in kvm-userspace.  If 
kvm is not detected (or the caller lacks sufficient privileges), we fall 
back to tcg (of course we'd also like the option of not compiling tcg 
where emulation is unacceptable like server deployments).

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

[Qemu-devel] Re: [PATCH] hook cpu running at a higher level.

[Ian Jackson]

Avi Kivity writes ("Re: [PATCH] hook cpu running at a higher level."):
> kvm is run-time selectable, both in upstream and in kvm-userspace.  If 
> kvm is not detected (or the caller lacks sufficient privileges), we fall 
> back to tcg (of course we'd also like the option of not compiling tcg 
> where emulation is unacceptable like server deployments).

Ah, right, well then the approach in the patch seems sensible.

Thanks,
Ian.

Re: [Qemu-devel] Re: [PATCH] hook cpu running at a higher level.

[Glauber Costa]

On Tue, Dec 30, 2008 at 8:24 AM, Ian Jackson > The Xen qemu process
runs only in one thread which is fine because it
> doesn't need to be involved with actual processor execution.  In
> theory parallel execution (in different threads and thus on different
> physical cpus) of IO emulations requested by different guest vcpus
> might make some small performance difference but I doubt it would be
> worth our while.  So I think the Xen setup will still from qemu's
> point of view look like a single vcpu no matter how many vcpus the
> guest aactually has.

Is it one vcpu or various in lockstep ? If you have only one vcpu,
your main loop can be even simpler than qemu's, which holds them all
in a for loop.

> The way we have approached these problems in the Xen tree is to supply
> an alternative implementation of (say) main_loop and arrange for the
> standard one not to be compiled.  Is it the intent to make kvm a
> run-time selectable option ?  It seems to me that that given that we
> already have different qemu builds for all of the various target
> (guest) cpu architectures, it might be simpler to continue that
> approach.  With a bit of judicious movement of code into appropriate
> files, this will avoid the need for ifs and ifdefs.
>

I believe the idea here is to bring up something close to the accel
patches (this one, plus the ones for memory_rw and registered I
already posted). So you would not see this, but simply
"cpu_main_loop()", that would call into the right thing, no matter
what it is. This allow for both runtime and compile time selection of
what you're running.

-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

Re: [Qemu-devel] Re: [PATCH] hook cpu running at a higher level.

[Ian Jackson]

Glauber Costa writes ("Re: [Qemu-devel] Re: [PATCH] hook cpu running at a higher level."):
> On Tue, Dec 30, 2008 at 8:24 AM, Ian Jackson
> > The Xen qemu process runs only in one thread which is fine because it
> > doesn't need to be involved with actual processor execution.  In
> > theory parallel execution (in different threads and thus on different
> > physical cpus) of IO emulations requested by different guest vcpus
> > might make some small performance difference but I doubt it would be
> > worth our while.  So I think the Xen setup will still from qemu's
> > point of view look like a single vcpu no matter how many vcpus the
> > guest aactually has.
> 
> Is it one vcpu or various in lockstep ? If you have only one vcpu,
> your main loop can be even simpler than qemu's, which holds them all
> in a for loop.

Whether it's one vcpu or several depends how you think about it.  The
guest and Xen see several vcpus, which execute (without any
involvement from qemu) on the various physical cpus according to the
Xen hypervisor's scheduler.  qemu only sees one vcpu object which
appears to generate all of the IO requests.

Our main loop is really just
  read io request from magic fd
  call cpu_something_rw which eventually calls appropriate io emulation
  send response to io request (ack or any data `read') back to magic fd
so yes it is very simple.

Execution of the guest's vcpus happens asynchronously (from qemu's
point of view) until they need to do IO; then the hypervisor passes
the request to qemu and stops the vcpu (by not scheduling it) until it
gets a response from qemu.  While no IO requests are being processed,
qemu sleeps.

Ian.

[Qemu-devel] Re: [PATCH] hook cpu running at a higher level.

[Avi Kivity]

Ian Jackson wrote:
> The Xen qemu process runs only in one thread which is fine because it
> doesn't need to be involved with actual processor execution.  In
> theory parallel execution (in different threads and thus on different
> physical cpus) of IO emulations requested by different guest vcpus
> might make some small performance difference but I doubt it would be
> worth our while.  So I think the Xen setup will still from qemu's
> point of view look like a single vcpu no matter how many vcpus the
> guest aactually has.
>   

I thing it makes sense for you to have multiple vcpus -- that is 
multiple CPUState objects.  You can still have just one thread (which 
would be the iothread in kvm's terminology).

So:

  tcg - multiplexes all vcpus and io on one thread
  kvm - iothread + per-vcpu thread
  xen - iothread, vcpus scheduled by hypervisor

tcg also ought to move to an iothread model, so it can take advantage of 
multicore for things like display updates.  For x86-on-x86, it may even 
be possible to have vcpus running in parallel (with some kind of 
shared/exclusive lock on softmmu).

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

[Qemu-devel] Re: [PATCH] hook cpu running at a higher level.

[Ian Jackson]

Avi Kivity writes ("Re: [PATCH] hook cpu running at a higher level."):
> I thing it makes sense for you to have multiple vcpus -- that is 
> multiple CPUState objects.  You can still have just one thread (which 
> would be the iothread in kvm's terminology).

Xen does have multiple vcpus (of course) but the vcpus are modelled
entirely in the hypervisor.  No significant amount of state about
these vcpus is in qemu-dm.  Because of the way qemu is constructed we
have to have a CPUState object in qemu-dm but there is only one of it
and it's basically a dummy.  It doesn't contain any of the usual i386
registers.

This will all be a bit weird if you're used to thinking of qemu as a
whole system emulator (which is what it qemu, of course), but of
course Xen didn't need or want the whole system emulator.

>From the point of view of qemu's architecture, it's as if qemu were
emulating a machine consisting of the peripherals plus a (single)
pseudo-CPU which doesn't do anything resembling the execution of
machine instructions (at least as far as qemu is concerned).  This
pseudo-CPU actually consists of much of the rest of the machine,
including the hypervisor and the guest (including all of the guest's
RAM).  Since the state of this pseudo-CPU resides outside qemu, there
is no representation of it inside the qemu-dm process.

The the pseudo-CPU code in qemu-dm is mostly a stub which is
responsible for talking to the parts which actually run the pseudo-CPU
(ie, which actually run the guest's vcpus, maintain the guest's RAM,
and so forth).

> So:
>   tcg - multiplexes all vcpus and io on one thread
>   kvm - iothread + per-vcpu thread
>   xen - iothread, vcpus scheduled by hypervisor

So qemu-dm _is_ the iothread.  The vcpus are elsewhere.

Ian.