[Qemu-devel] [PATCH 0/7] Update qemu accel

[Glauber Costa]

Hi guys,

I have the following patches that I'm about to commit to the QemuAccel tree,
in case you have no further comments.

Most of them are small fixes. I particularly don't like one of them:
   * check wether kqemu is enabled in open code

this patch should not exist, but I think this is the simplest solution right 
now.

I'm reverting the patch that moves ram allocation to before accel start, since 
it broke qemu.
This is theorectically needed for kvm, but I think we can rid of this need in 
KVM. We're
working on it.

The only new thing in here is the inclusion of a accel_opaque into the CPUState 
field.
both kqemu and kvm (and possibly others too) have code that fits well inside 
the CPUState,
and it makes sense to have an opaque to account for them.

Comments welcome.

======================================================
Glauber Costa (7):
      protect accel.h from multiple inclusion
      use accel_trace_io in calling sites.
      use break_loop accel
      give noaccel a name
      check wether kqemu is enabled in open code
      provide an opaque for accelerator in cpu state
      Revert "don't allocate ram before accel starts"

 accel.c                 |    2 +-
 accel.h                 |    7 ++++++-
 cpu-defs.h              |    1 +
 cpu-exec.c              |   11 +++--------
 exec-all.h              |   10 ++++++++--
 kqemu.c                 |   36 +++++++++++++++++++++++++++++++-----
 kqemu.h                 |    6 ++++++
 softmmu_template.h      |   10 ++++------
 target-i386/cpu.h       |    5 -----
 target-i386/op_helper.c |    2 +-
 vl.c                    |   11 ++++++-----
 11 files changed, 67 insertions(+), 34 deletions(-)
diff --git a/accel.c b/accel.c
index f9618b2..bf3cb73 100644
--- a/accel.c
+++ b/accel.c
@@ -21,7 +21,7 @@ CPUState *noaccel_get_env(void)
 
 /* Accelerator wrapper for the no-accel (raw qemu) case */
 QEMUAccel noaccel = {
-    .name = NULL,
+    .name = "noaccel",
     .cpu_interrupt = accel_nop,
     .init_env = accel_nop,
     .get_env = noaccel_get_env,
diff --git a/accel.h b/accel.h
index ae72e23..9a309ba 100644
--- a/accel.h
+++ b/accel.h
@@ -1,3 +1,6 @@
+#ifndef _ACCEL_H_
+#define _ACCEL_H_
+
 typedef struct QEMUAccel {
     char *name;
     void (*cpu_interrupt)(CPUState *env);
@@ -35,6 +38,8 @@ extern int _accel_nop(void);
 extern int noaccel_info(CPUState *env, char *buf);
 extern CPUState *noaccel_get_env(void);
 
+#define accel_opaque_field(env, type, field) ((type *)env->accel_opaque)->field
+
 #ifdef USE_KQEMU
 extern QEMUAccel kqemu_accel;
 #endif
@@ -144,4 +149,4 @@ static inline int accel_break_loop(CPUState *env)
 }
 
 extern void accel_add(const char *name);
-
+#endif
diff --git a/cpu-defs.h b/cpu-defs.h
index 5f6fcbb..6b58316 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -197,6 +197,7 @@ typedef struct icount_decr_u16 {
     int thread_id;                                                      \
     /* user data */                                                     \
     void *opaque;                                                       \
+    void *accel_opaque;                                                 \
                                                                         \
     const char *cpu_model_str;
 
diff --git a/cpu-exec.c b/cpu-exec.c
index 8637e2a..ed06de0 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -36,6 +36,7 @@
 #include <signal.h>
 #include <sys/ucontext.h>
 #endif
+#include "accel.h"
 
 #if defined(__sparc__) && !defined(HOST_SOLARIS)
 // Work around ugly bugs in glibc that mangle global register contents
@@ -605,7 +606,7 @@ int cpu_exec(CPUState *env1)
                 {
                     if (next_tb != 0 &&
 #ifdef USE_KQEMU
-                        (env->kqemu_enabled != 2) &&
+                        (!kqemu_kernel_enabled(env)) && 
 #endif
                         tb->page_addr[1] == -1) {
                     tb_add_jump((TranslationBlock *)(next_tb & ~3), next_tb & 
3, tb);
@@ -653,13 +654,7 @@ int cpu_exec(CPUState *env1)
                 }
                 /* reset soft MMU for next block (it can currently
                    only be set by a memory fault) */
-#if defined(USE_KQEMU)
-#define MIN_CYCLE_BEFORE_SWITCH (100 * 1000)
-                if (kqemu_is_ok(env) &&
-                    (cpu_get_time_fast() - env->last_io_time) >= 
MIN_CYCLE_BEFORE_SWITCH) {
-                    cpu_loop_exit();
-                }
-#endif
+                accel_break_loop(env);
             } /* for(;;) */
         } else {
             env_to_regs();
diff --git a/exec-all.h b/exec-all.h
index 0b51f23..c2191ec 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -370,16 +370,22 @@ void kqemu_set_phys_mem(uint64_t start_addr, ram_addr_t 
size,
 void kqemu_cpu_interrupt(CPUState *env);
 void kqemu_record_dump(void);
 
+int kqemu_is_enabled(CPUState *env);
+int kqemu_kernel_enabled(CPUState *env);
+
 extern uint32_t kqemu_comm_base;
 
+#include "accel.h"
+#include <kqemu.h>
+
 static inline int kqemu_is_ok(CPUState *env)
 {
-    return(env->kqemu_enabled &&
+    return(kqemu_is_enabled(env) &&
            (env->cr[0] & CR0_PE_MASK) &&
            !(env->hflags & HF_INHIBIT_IRQ_MASK) &&
            (env->eflags & IF_MASK) &&
            !(env->eflags & VM_MASK) &&
-           (env->kqemu_enabled == 2 ||
+           (kqemu_kernel_enabled(env) ||
             ((env->hflags & HF_CPL_MASK) == 3 &&
              (env->eflags & IOPL_MASK) != IOPL_MASK)));
 }
diff --git a/kqemu.c b/kqemu.c
index d727b1b..f38872f 100644
--- a/kqemu.c
+++ b/kqemu.c
@@ -62,6 +62,8 @@
 #define KQEMU_DEVICE "/dev/kqemu"
 #endif
 
+struct kqemu_cpu_opaque kqemu_opaque;
+
 static void qpi_init(void);
 
 #ifdef _WIN32
@@ -247,7 +249,31 @@ int kqemu_start(void)
 void kqemu_init_env(CPUState *env)
 {
     kqemu_update_cpuid(env);
-    env->kqemu_enabled = kqemu_allowed;
+    /* SMP currently not supported, so this is okay */
+    kqemu_opaque.kqemu_enabled = kqemu_allowed;
+    env->accel_opaque = &kqemu_opaque;
+}
+
+/* FIXME: Should not be needed, since ideally, QEMUAccel would avoid all kqemu 
tests
+ * altogether
+ */
+int kqemu_is_enabled(CPUState *env)
+{
+    if (strcasecmp(current_accel->name, "kqemu")) {
+        return 0;
+    }
+
+    return kqemu_opaque_field(env, kqemu_enabled);
+    
+}
+
+int kqemu_kernel_enabled(CPUState *env)
+{
+    if (strcasecmp(current_accel->name, "kqemu")) {
+        return 0;
+    }
+
+    return kqemu_opaque_field(env, kqemu_enabled) == 2;
 }
 
 void kqemu_flush_page(CPUState *env, target_ulong addr)
@@ -277,7 +303,7 @@ int kqemu_info(CPUState *env, char *buf)
 {
     int val, len;
     val = 0;
-    val = env->kqemu_enabled;
+    val = kqemu_opaque_field(env, kqemu_enabled);
     len = sprintf(buf, "kqemu support: ");
     buf += len;
 
@@ -436,14 +462,14 @@ void kqemu_set_phys_mem(uint64_t start_addr, ram_addr_t 
size,
 void kqemu_trace_io(CPUState *env)
 {
     if (env)
-        env->last_io_time = cpu_get_time_fast();
+        kqemu_opaque_field(env, last_io_time) = cpu_get_time_fast();
 }
 
 int kqemu_break_loop(CPUState *env)
 {
 #define MIN_CYCLE_BEFORE_SWITCH (100 * 1000)
     if (kqemu_is_ok(env) &&
-        (cpu_get_time_fast() - env->last_io_time) >= MIN_CYCLE_BEFORE_SWITCH) {
+        (cpu_get_time_fast() -  kqemu_opaque_field(env, last_io_time)) >= 
MIN_CYCLE_BEFORE_SWITCH) {
         return 1; 
     }
     return 0;
@@ -854,7 +880,7 @@ int kqemu_cpu_exec(CPUState *env)
     cpl = (env->hflags & HF_CPL_MASK);
     kenv->cpl = cpl;
     kenv->nb_pages_to_flush = nb_pages_to_flush;
-    kenv->user_only = (env->kqemu_enabled == 1);
+    kenv->user_only = (kqemu_opaque_field(env, kqemu_enabled) == 1);
     kenv->nb_ram_pages_to_update = nb_ram_pages_to_update;
     nb_ram_pages_to_update = 0;
     kenv->nb_modified_ram_pages = nb_modified_ram_pages;
diff --git a/kqemu.h b/kqemu.h
index 1c7e024..e121494 100644
--- a/kqemu.h
+++ b/kqemu.h
@@ -38,6 +38,12 @@ extern int64_t kqemu_ret_int_count;
 extern int64_t kqemu_ret_excp_count;
 extern int64_t kqemu_ret_intr_count;
 
+struct kqemu_cpu_opaque {
+    int kqemu_enabled;
+    int last_io_time;
+};
+#define kqemu_opaque_field(env, field) accel_opaque_field(env, struct 
kqemu_cpu_opaque, field)
+
 struct kqemu_segment_cache {
     uint16_t selector;
     uint16_t padding1;
diff --git a/softmmu_template.h b/softmmu_template.h
index 98dd378..4945352 100644
--- a/softmmu_template.h
+++ b/softmmu_template.h
@@ -47,6 +47,8 @@
 #define ADDR_READ addr_read
 #endif
 
+#include "accel.h"
+
 static DATA_TYPE glue(glue(slow_ld, SUFFIX), MMUSUFFIX)(target_ulong addr,
                                                         int mmu_idx,
                                                         void *retaddr);
@@ -75,9 +77,7 @@ static inline DATA_TYPE glue(io_read, 
SUFFIX)(target_phys_addr_t physaddr,
     res |= (uint64_t)io_mem_read[index][2](io_mem_opaque[index], physaddr + 4) 
<< 32;
 #endif
 #endif /* SHIFT > 2 */
-#ifdef USE_KQEMU
-    env->last_io_time = cpu_get_time_fast();
-#endif
+    accel_trace_io(env);
     return res;
 }
 
@@ -220,9 +220,7 @@ static inline void glue(io_write, 
SUFFIX)(target_phys_addr_t physaddr,
     io_mem_write[index][2](io_mem_opaque[index], physaddr + 4, val >> 32);
 #endif
 #endif /* SHIFT > 2 */
-#ifdef USE_KQEMU
-    env->last_io_time = cpu_get_time_fast();
-#endif
+    accel_trace_io(env);
 }
 
 void REGPARM glue(glue(__st, SUFFIX), MMUSUFFIX)(target_ulong addr,
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 9d8bda9..73d2378 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -590,11 +590,6 @@ typedef struct CPUX86State {
     uint32_t cpuid_ext3_features;
     uint32_t cpuid_apic_id;
 
-#ifdef USE_KQEMU
-    int kqemu_enabled;
-    int last_io_time;
-#endif
-
     /* in order to simplify APIC support, we leave this pointer to the
        user */
     struct APICState *apic_state;
diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
index 23f3080..6d338a3 100644
--- a/target-i386/op_helper.c
+++ b/target-i386/op_helper.c
@@ -3168,7 +3168,7 @@ void helper_rdmsr(void)
 #endif
 #ifdef USE_KQEMU
     case MSR_QPI_COMMBASE:
-        if (env->kqemu_enabled) {
+        if (kqemu_opaque_field(env, kqemu_enabled)) {
             val = kqemu_comm_base;
         } else {
             val = 0;
diff --git a/vl.c b/vl.c
index f5503bb..ede259f 100644
--- a/vl.c
+++ b/vl.c
@@ -9050,6 +9050,12 @@ int main(int argc, char **argv)
         phys_ram_size += ram_size;
     }
 
+    phys_ram_base = qemu_vmalloc(phys_ram_size);
+    if (!phys_ram_base) {
+        fprintf(stderr, "Could not allocate physical memory\n");
+        exit(1);
+    }
+
     /* init the dynamic translator */
     cpu_exec_init_all(tb_size * 1024 * 1024);
 
@@ -9058,11 +9064,6 @@ int main(int argc, char **argv)
            exit(1);
     }
 
-    phys_ram_base = qemu_vmalloc(phys_ram_size);
-    if (!phys_ram_base) {
-        fprintf(stderr, "Could not allocate physical memory\n");
-        exit(1);
-    }
 
     bdrv_init();