Presenting accelerators for better abstraction

PATCH In review Glauber Costa 3 participants 9 messages May 2, 2008 KVM

lore

Share & tools

lore

Glauber Costa

author

2008-05-02 17:49 UTC #0 lore raw

cover Presenting accelerators for better abstraction

Hey guys,

This was already discussed, mostly on kvm-devel (but qemu list always copied, if you remember), and here is the pre work I've done. I don't feel it to be mergeable yet, but with your comments, I expect to get it in a good shape soon. This is not extensibly tested (but it will).

These patches applies ontop of raw qemu, so they are not kvm-specific _at all_. I've choosen to start by abstracting kqemu first, since this is what we have already in the tree.
The abstraction of kqemu is also, not complete. It cover mostly areas that are common to kvm, i.e., provide functions that kvm could implement as-is.

Comments are welcome, rants not so.
Have fun.

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Glauber Costa

author

2008-05-02 17:49 UTC #1 lore raw

PATCH 1/4 introduce QEMUAccel and fill it with interrupt specific driver

This patch introduces QEMUAccel, a placeholder for function pointers that aims at helping qemu to abstract accelerators such as kqemu and kvm (actually, the 'accelerator' name was proposed by avi kivity, since he loves referring to kvm that way).

To begin with, the accelerator is given the opportunity to register a cpu_interrupt function, to be called after the raw cpu_interrupt.
This has the side effect of, for the kqemu accelerator, calling kqemu_cpu_interrupt everytime, which didn't use to happen. But looking at the code, this seems safe to me.

This patch applies on raw qemu.

Signed-off-by Glauber Costa <[email protected]>

---
 block-raw-posix.c |    5 -----
 exec-all.h        |   18 +++++++++++++++++-
 exec.c            |    2 ++
 kqemu.c           |   26 ++++++++++++++++----------
 vl.c              |    6 +-----
 5 files changed, 36 insertions(+), 21 deletions(-)

block-raw-posix.c +0 -5

index 6b0009e..61c23ba 100644
@@ -250,11 +250,6 @@ static void aio_signal_handler(int signum)
     if (env) {
         /* stop the currently executing cpu because a timer occured */
         cpu_interrupt(env, CPU_INTERRUPT_EXIT);
-#ifdef USE_KQEMU
-        if (env->kqemu_enabled) {
-            kqemu_cpu_interrupt(env);
-        }
-#endif
     }
 #endif
 }

exec-all.h +17 -1

index 3ce8242..5162307 100644
@@ -574,6 +574,23 @@ static inline target_ulong get_phys_addr_code(CPUState *env, target_ulong addr)
 }
 #endif
 
+typedef struct QEMUAccel {
+    void (*cpu_interrupt)(CPUState *env);
+} QEMUAccel;
+
+extern QEMUAccel *current_accel;
+
+static inline void register_qemu_accel(QEMUAccel *accel)
+{
+    current_accel = accel;
+}
+
+static inline void accel_cpu_interrupt(CPUState *env)
+{
+    if (current_accel && current_accel->cpu_interrupt)
+        current_accel->cpu_interrupt(env);
+}
+
 #ifdef USE_KQEMU
 #define KQEMU_MODIFY_PAGE_MASK (0xff & ~(VGA_DIRTY_FLAG | CODE_DIRTY_FLAG))

@@ -583,7 +600,6 @@ void kqemu_flush_page(CPUState *env, target_ulong addr);
 void kqemu_flush(CPUState *env, int global);
 void kqemu_set_notdirty(CPUState *env, ram_addr_t ram_addr);
 void kqemu_modify_page(CPUState *env, ram_addr_t ram_addr);
-void kqemu_cpu_interrupt(CPUState *env);
 void kqemu_record_dump(void);
 
 static inline int kqemu_is_ok(CPUState *env)

exec.c +2 -0

index 48dabd6..93d9b01 100644
@@ -1226,6 +1226,8 @@ void cpu_interrupt(CPUState *env, int mask)
         tb_reset_jump_recursive(tb);
         resetlock(&interrupt_lock);
     }
+
+    accel_cpu_interrupt(env);
 }
 
 void cpu_reset_interrupt(CPUState *env, int mask)

kqemu.c +16 ~~-10~~

index 148a52f..c46698c 100644
@@ -158,6 +158,19 @@ static void kqemu_update_cpuid(CPUState *env)
        accelerated code */
 }
 
+void kqemu_cpu_interrupt(CPUState *env)
+{
+#if defined(_WIN32) && KQEMU_VERSION >= 0x010101
+    /* cancelling the I/O request causes KQEMU to finish executing the
+       current block and successfully returning. */
+    CancelIo(kqemu_fd);
+#endif
+}
+
+QEMUAccel kqemu_accel = {
+    .cpu_interrupt = kqemu_cpu_interrupt,
+};
+
 int kqemu_init(CPUState *env)
 {
     struct kqemu_init init;

@@ -239,6 +252,9 @@ int kqemu_init(CPUState *env)
     }
     kqemu_update_cpuid(env);
     env->kqemu_enabled = kqemu_allowed;
+    if (env->kqemu_enabled)
+        register_qemu_accel(&kqemu_accel);
+
     nb_pages_to_flush = 0;
     nb_ram_pages_to_update = 0;
     return 0;

@@ -901,14 +917,4 @@ int kqemu_cpu_exec(CPUState *env)
     }
     return 0;
 }
-
-void kqemu_cpu_interrupt(CPUState *env)
-{
-#if defined(_WIN32) && KQEMU_VERSION >= 0x010101
-    /* cancelling the I/O request causes KQEMU to finish executing the
-       current block and successfully returning. */
-    CancelIo(kqemu_fd);
-#endif
-}
-
 #endif

vl.c +1 -5

index 9289982..9272541 100644
@@ -240,6 +240,7 @@ struct drive_opt {
 static CPUState *cur_cpu;
 static CPUState *next_cpu;
 static int event_pending = 1;
+QEMUAccel *current_accel;
 
 #define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR)

@@ -1200,11 +1201,6 @@ static void host_alarm_handler(int host_signum)
         if (env) {
             /* stop the currently executing cpu because a timer occured */
             cpu_interrupt(env, CPU_INTERRUPT_EXIT);
-#ifdef USE_KQEMU
-            if (env->kqemu_enabled) {
-                kqemu_cpu_interrupt(env);
-            }
-#endif
         }
         event_pending = 1;
     }

Signature · 8 lines

1.5.0.6


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Glauber Costa

author

2008-05-02 17:49 UTC #2 lore raw

PATCH 2/4 exec interrupt made abstract by accel_yyy()

---
 cpu-exec.c |   29 ++++++++++++++++++-----------
 exec-all.h |   10 ++++++++++
 kqemu.c    |    3 +++
 3 files changed, 31 insertions(+), 11 deletions(-)

cpu-exec.c +18 ~~-11~~

index 3246264..79da619 100644
@@ -35,6 +35,22 @@
 #include <sys/ucontext.h>
 #endif
 
+/* FIXME: This does not belong here */
+int kqemu_exec_interrupt(CPUState *env)
+{
+    int ret = 0;
+    if (kqemu_is_ok(env) && env->interrupt_request == 0) {
+        env->eflags = env->eflags | cc_table[CC_OP].compute_all() | (DF & DF_MASK);
+        ret = kqemu_cpu_exec(env);
+        /* put eflags in CPU temporary format */
+        CC_SRC = env->eflags & (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
+        DF = 1 - (2 * ((env->eflags >> 10) & 1));
+        CC_OP = CC_OP_EFLAGS;
+        env->eflags &= ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
+    }
+    return ret;
+}
+
 int tb_invalidated_flag;
 
 //#define DEBUG_EXEC

@@ -388,16 +404,8 @@ int cpu_exec(CPUState *env1)
                 }
                 env->exception_index = -1;
             }
-#ifdef USE_KQEMU
-            if (kqemu_is_ok(env) && env->interrupt_request == 0) {
-                int ret;
-                env->eflags = env->eflags | cc_table[CC_OP].compute_all() | (DF & DF_MASK);
-                ret = kqemu_cpu_exec(env);
-                /* put eflags in CPU temporary format */
-                CC_SRC = env->eflags & (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
-                DF = 1 - (2 * ((env->eflags >> 10) & 1));
-                CC_OP = CC_OP_EFLAGS;
-                env->eflags &= ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
+
+            if (accel_exec_interrupt(env, &ret)) {
                 if (ret == 1) {
                     /* exception */
                     longjmp(env->jmp_env, 1);

@@ -412,7 +420,6 @@ int cpu_exec(CPUState *env1)
                     }
                 }
             }
-#endif
 
             T0 = 0; /* force lookup of first TB */
             for(;;) {

exec-all.h +10 -0

index 5162307..621e1ca 100644
@@ -576,6 +576,7 @@ static inline target_ulong get_phys_addr_code(CPUState *env, target_ulong addr)
 
 typedef struct QEMUAccel {
     void (*cpu_interrupt)(CPUState *env);
+    int (*exec_interrupt)(CPUState *env);
 } QEMUAccel;
 
 extern QEMUAccel *current_accel;

@@ -591,6 +592,15 @@ static inline void accel_cpu_interrupt(CPUState *env)
         current_accel->cpu_interrupt(env);
 }
 
+static inline int accel_exec_interrupt(CPUState *env, int *ret)
+{
+    if (current_accel && current_accel->exec_interrupt) {
+        *ret = current_accel->exec_interrupt(env);
+        return 1;
+    }
+    return 0;
+}
+
 #ifdef USE_KQEMU
 #define KQEMU_MODIFY_PAGE_MASK (0xff & ~(VGA_DIRTY_FLAG | CODE_DIRTY_FLAG))

kqemu.c +3 -0

index c46698c..08622ad 100644
@@ -167,8 +167,11 @@ void kqemu_cpu_interrupt(CPUState *env)
 #endif
 }
 
+extern int kqemu_exec_interrupt(CPUState *env);
+
 QEMUAccel kqemu_accel = {
     .cpu_interrupt = kqemu_cpu_interrupt,
+    .exec_interrupt = kqemu_exec_interrupt,
 };
 
 int kqemu_init(CPUState *env)

Signature · 8 lines

1.5.0.6


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Glauber Costa

author

2008-05-02 17:49 UTC #3 lore raw

PATCH 3/4 init env made accel driver

---
 exec-all.h            |    8 +++++++-
 kqemu.c               |    1 +
 target-i386/helper2.c |    4 +---
 3 files changed, 9 insertions(+), 4 deletions(-)

exec-all.h +7 -1

index 621e1ca..784d0ac 100644
@@ -577,6 +577,7 @@ static inline target_ulong get_phys_addr_code(CPUState *env, target_ulong addr)
 typedef struct QEMUAccel {
     void (*cpu_interrupt)(CPUState *env);
     int (*exec_interrupt)(CPUState *env);
+    void (*init_env)(CPUState *env);
 } QEMUAccel;
 
 extern QEMUAccel *current_accel;

@@ -601,10 +602,15 @@ static inline int accel_exec_interrupt(CPUState *env, int *ret)
     return 0;
 }
 
+static inline void accel_init_env(CPUState *env)
+{
+    if (current_accel && current_accel->init_env)
+        current_accel->init_env(env);
+}
+
 #ifdef USE_KQEMU
 #define KQEMU_MODIFY_PAGE_MASK (0xff & ~(VGA_DIRTY_FLAG | CODE_DIRTY_FLAG))
 
-int kqemu_init(CPUState *env);
 int kqemu_cpu_exec(CPUState *env);
 void kqemu_flush_page(CPUState *env, target_ulong addr);
 void kqemu_flush(CPUState *env, int global);

kqemu.c +1 -0

index 08622ad..95d8a94 100644
@@ -172,6 +172,7 @@ extern int kqemu_exec_interrupt(CPUState *env);
 QEMUAccel kqemu_accel = {
     .cpu_interrupt = kqemu_cpu_interrupt,
     .exec_interrupt = kqemu_exec_interrupt,
+    .init_env = kqemu_init,
 };
 
 int kqemu_init(CPUState *env)

target-i386/helper2.c +1 -3

index 551a0d8..b11bc22 100644
@@ -111,9 +111,7 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
         return NULL;
     }
     cpu_reset(env);
-#ifdef USE_KQEMU
-    kqemu_init(env);
-#endif
+    accel_init_env(env);
     return env;
 }

Signature · 8 lines

1.5.0.6


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Glauber Costa

author

2008-05-02 17:49 UTC #4 lore raw

PATCH 4/4 wrap cache flushing functions into accel drivers

---
 exec-all.h |   16 ++++++++++++++--
 exec.c     |   12 ++----------
 kqemu.c    |    2 ++
 3 files changed, 18 insertions(+), 12 deletions(-)

exec-all.h +14 -2

index 784d0ac..9925a80 100644
@@ -578,6 +578,8 @@ typedef struct QEMUAccel {
     void (*cpu_interrupt)(CPUState *env);
     int (*exec_interrupt)(CPUState *env);
     void (*init_env)(CPUState *env);
+    void (*flush_cache)(CPUState *env, int global);
+    void (*flush_page)(CPUState *env, target_ulong addr);
 } QEMUAccel;
 
 extern QEMUAccel *current_accel;

@@ -608,12 +610,22 @@ static inline void accel_init_env(CPUState *env)
         current_accel->init_env(env);
 }
 
+static inline void accel_flush_cache(CPUState *env, int global)
+{
+    if (current_accel && current_accel->flush_cache)
+        current_accel->flush_cache(env, global);
+}
+
+static inline void accel_flush_page(CPUState *env, target_ulong addr)
+{
+    if (current_accel && current_accel->flush_page)
+        current_accel->flush_page(env, addr);
+}
+
 #ifdef USE_KQEMU
 #define KQEMU_MODIFY_PAGE_MASK (0xff & ~(VGA_DIRTY_FLAG | CODE_DIRTY_FLAG))
 
 int kqemu_cpu_exec(CPUState *env);
-void kqemu_flush_page(CPUState *env, target_ulong addr);
-void kqemu_flush(CPUState *env, int global);
 void kqemu_set_notdirty(CPUState *env, ram_addr_t ram_addr);
 void kqemu_modify_page(CPUState *env, ram_addr_t ram_addr);
 void kqemu_record_dump(void);

exec.c +2 ~~-10~~

index 93d9b01..b0f50e3 100644
@@ -1393,11 +1393,7 @@ void tlb_flush(CPUState *env, int flush_global)
 #if !defined(CONFIG_SOFTMMU)
     munmap((void *)MMAP_AREA_START, MMAP_AREA_END - MMAP_AREA_START);
 #endif
-#ifdef USE_KQEMU
-    if (env->kqemu_enabled) {
-        kqemu_flush(env, flush_global);
-    }
-#endif
+    accel_flush_cache(env, flush_global);
     tlb_flush_count++;
 }

@@ -1450,11 +1446,7 @@ void tlb_flush_page(CPUState *env, target_ulong addr)
     if (addr < MMAP_AREA_END)
         munmap((void *)addr, TARGET_PAGE_SIZE);
 #endif
-#ifdef USE_KQEMU
-    if (env->kqemu_enabled) {
-        kqemu_flush_page(env, addr);
-    }
-#endif
+    accel_flush_page(env, addr);
 }
 
 /* update the TLBs so that writes to code in the virtual page 'addr'

kqemu.c +2 -0

index 95d8a94..f92b60d 100644
@@ -173,6 +173,8 @@ QEMUAccel kqemu_accel = {
     .cpu_interrupt = kqemu_cpu_interrupt,
     .exec_interrupt = kqemu_exec_interrupt,
     .init_env = kqemu_init,
+    .flush_cache = kqemu_flush,
+    .flush_page = kqemu_flush_page,
 };
 
 int kqemu_init(CPUState *env)

Signature · 8 lines

1.5.0.6


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Jamie Lokier

2008-05-02 18:16 UTC #5 lore raw

Re: [Qemu-devel] [PATCH 1/4] [PATCH] introduce QEMUAccel and fill it with interrupt specific driver

In reply to Glauber Costa

Quoted 4 lines · Glauber Costa

This patch introduces QEMUAccel, a placeholder for function pointers that aims at helping qemu to abstract accelerators such as kqemu and kvm (actually, the 'accelerator' name was proposed by avi kivity, since he loves referring to kvm that way).

Just a little thought...

Maybe 'VCPU' would be a clearer name? QEMU provides its own VCPU, and KQEMU+QEMU also provide one toegether. While KVM provides essentially one or more whole VCPUs by itself and uses QEMU's drivers only doesn't it?

-- Jamie

Glauber Costa

author

2008-05-02 19:49 UTC #6 lore raw

Re: [Qemu-devel] [PATCH 1/4] [PATCH] introduce QEMUAccel and fill it with interrupt specific driver

Quoted 6 lines · Jamie Lokier

Quoted 4 lines · Glauber Costa
This patch introduces QEMUAccel, a placeholder for function pointers that aims at helping qemu to abstract accelerators such as kqemu and kvm (actually, the 'accelerator' name was proposed by avi kivity, since he loves referring to kvm that way).

Quoted 8 lines

Just a little thought...
Maybe 'VCPU' would be a clearer name? QEMU provides its own VCPU, and KQEMU+QEMU also provide one toegether. While KVM provides essentially one or more whole VCPUs by itself and uses QEMU's drivers only doesn't it?
-- Jamie

VCPU is rather confusing with the vcpus themselves. KVM, for instance, has its own structures called "vcpu".

If it is preferred, however, we can name the structure VCPUOperations, and change the function names that involves accel_yyy to vcpu_op_yyy()

What do you think?

Paul Brook

2008-05-02 20:16 UTC #7 lore raw

Re: [Qemu-devel] [PATCH 1/4] [PATCH] introduce QEMUAccel and fill it with interrupt specific driver

Quoted 5 lines

Quoted 5 lines
Maybe 'VCPU' would be a clearer name? QEMU provides its own VCPU, and KQEMU+QEMU also provide one toegether. While KVM provides essentially one or more whole VCPUs by itself and uses QEMU's drivers only doesn't it?

Quoted 7 lines

Quoted 1 line
-- Jamie
VCPU is rather confusing with the vcpus themselves. KVM, for instance, has its own structures called "vcpu".
If it is preferred, however, we can name the structure VCPUOperations, and change the function names that involves accel_yyy to vcpu_op_yyy()

kvm wants to hook into more than just the CPU doesn't it?

Paul

Glauber Costa

author

2008-05-02 20:17 UTC #8 lore raw

Re: [Qemu-devel] [PATCH 1/4] [PATCH] introduce QEMUAccel and fill it with interrupt specific driver

Quoted 5 lines · Paul Brook

Quoted 5 lines
Quoted 5 lines
Maybe 'VCPU' would be a clearer name? QEMU provides its own VCPU, and KQEMU+QEMU also provide one toegether. While KVM provides essentially one or more whole VCPUs by itself and uses QEMU's drivers only doesn't it?

Quoted 8 lines

Quoted 6 lines
Quoted 1 line
-- Jamie
VCPU is rather confusing with the vcpus themselves. KVM, for instance, has its own structures called "vcpu".
If it is preferred, however, we can name the structure VCPUOperations, and change the function names that involves accel_yyy to vcpu_op_yyy()
kvm wants to hook into more than just the CPU doesn't it?

But so far, the structure I proposed only touches cpu-related things. We can have two structures for clarity. Like MEMOperations, and so far. We do it this way for paravirt_ops in linux, with good results in code clarity.