Skip to contentSkip to conversation

enable smp > 1 and related fixes

PATCH Applied Marcelo Tosatti 3 participants 14 messages May 4, 2010 – May 6, 2010 KVM

lore
Share & tools
lore
author
#1 lore raw

PATCH 1/9 kvm: set cpu_single_env around KVM_RUN ioctl

Zero cpu_single_env before leaving global lock protection, and restore on return.

Signed-off-by Marcelo Tosatti <[email protected]>
kvm-all.c +2 -0
Index: qemu/kvm-all.c
===================================================================
@@ -846,9 +846,11 @@ int kvm_cpu_exec(CPUState *env)
         }
 
         kvm_arch_pre_run(env, run);
+        cpu_single_env = NULL;
         qemu_mutex_unlock_iothread();
         ret = kvm_vcpu_ioctl(env, KVM_RUN, 0);
         qemu_mutex_lock_iothread();
+        cpu_single_env = env;
         kvm_arch_post_run(env, run);
 
         if (ret == -EINTR || ret == -EAGAIN) {
author
#2 lore raw

PATCH 2/9 make SIG_IPI to tcg vcpu thread reliable

In reply to Marcelo Tosatti

Store tcg loop exit request on a global variable, and transfer it to per-CPUState exit_request after assignment of cpu_single_env.

This makes exit request signal from robust. Drop the timedlock hack.

Signed-off-by Marcelo Tosatti <[email protected]>
cpu-exec.c +7 -0
Index: qemu/cpu-exec.c
===================================================================
@@ -213,6 +213,8 @@ static void cpu_handle_debug_exception(C
 
 /* main execution loop */
 
+volatile sig_atomic_t exit_request;
+
 int cpu_exec(CPUState *env1)
 {
     volatile host_reg_t saved_env_reg;

@@ -234,6 +236,11 @@ int cpu_exec(CPUState *env1)
     asm("");
     env = env1;
 
+    if (exit_request) {
+        env->exit_request = 1;
+        exit_request = 0;
+    }
+
 #if defined(TARGET_I386)
     if (!kvm_enabled()) {
         /* put eflags in CPU temporary format */
cpus.c +9 -14
Index: qemu/cpus.c
===================================================================
@@ -472,6 +472,7 @@ static void cpu_signal(int sig)
 {
     if (cpu_single_env)
         cpu_exit(cpu_single_env);
+    exit_request = 1;
 }
 
 static void tcg_block_io_signals(void)

@@ -542,26 +543,20 @@ static void unblock_io_signals(void)
     pthread_sigmask(SIG_BLOCK, &set, NULL);
 }
 
-static void qemu_signal_lock(unsigned int msecs)
-{
-    qemu_mutex_lock(&qemu_fair_mutex);
-
-    while (qemu_mutex_trylock(&qemu_global_mutex)) {
-        qemu_thread_signal(tcg_cpu_thread, SIG_IPI);
-        if (!qemu_mutex_timedlock(&qemu_global_mutex, msecs))
-            break;
-    }
-    qemu_mutex_unlock(&qemu_fair_mutex);
-}
-
 void qemu_mutex_lock_iothread(void)
 {
     if (kvm_enabled()) {
         qemu_mutex_lock(&qemu_fair_mutex);
         qemu_mutex_lock(&qemu_global_mutex);
         qemu_mutex_unlock(&qemu_fair_mutex);
-    } else
-        qemu_signal_lock(100);
+    } else {
+        qemu_mutex_lock(&qemu_fair_mutex);
+        if (qemu_mutex_trylock(&qemu_global_mutex)) {
+            qemu_thread_signal(tcg_cpu_thread, SIG_IPI);
+            qemu_mutex_lock(&qemu_global_mutex);
+        }
+        qemu_mutex_unlock(&qemu_fair_mutex);
+    }
 }
 
 void qemu_mutex_unlock_iothread(void)
exec-all.h +3 -0
Index: qemu/exec-all.h
===================================================================
@@ -339,4 +339,7 @@ CPUDebugExcpHandler *cpu_set_debug_excp_
 /* vl.c */
 extern int singlestep;
 
+/* cpu-exec.c */
+extern volatile sig_atomic_t exit_request;
+
 #endif
author
#3 lore raw

PATCH 3/9 standardize on qemu_cpu_kick for signalling cpu thread(s)

In reply to Marcelo Tosatti
Signed-off-by Marcelo Tosatti <[email protected]>
cpus.c +2 -5
Index: qemu/cpus.c
===================================================================
@@ -454,8 +454,7 @@ void qemu_cpu_kick(void *_env)
 {
     CPUState *env = _env;
     qemu_cond_broadcast(env->halt_cond);
-    if (kvm_enabled())
-        qemu_thread_signal(env->thread, SIG_IPI);
+    qemu_thread_signal(env->thread, SIG_IPI);
 }
 
 int qemu_cpu_self(void *_env)

@@ -583,7 +582,6 @@ void pause_all_vcpus(void)
 
     while (penv) {
         penv->stop = 1;
-        qemu_thread_signal(penv->thread, SIG_IPI);
         qemu_cpu_kick(penv);
         penv = (CPUState *)penv->next_cpu;
     }

@@ -592,7 +590,7 @@ void pause_all_vcpus(void)
         qemu_cond_timedwait(&qemu_pause_cond, &qemu_global_mutex, 100);
         penv = first_cpu;
         while (penv) {
-            qemu_thread_signal(penv->thread, SIG_IPI);
+            qemu_cpu_kick(penv);
             penv = (CPUState *)penv->next_cpu;
         }
     }

@@ -605,7 +603,6 @@ void resume_all_vcpus(void)
     while (penv) {
         penv->stop = 0;
         penv->stopped = 0;
-        qemu_thread_signal(penv->thread, SIG_IPI);
         qemu_cpu_kick(penv);
         penv = (CPUState *)penv->next_cpu;
     }
author
#4 lore raw

PATCH 4/9 port qemu-kvm's on_vcpu code

In reply to Marcelo Tosatti

run_on_cpu allows to execute work on a given CPUState context.

Signed-off-by Marcelo Tosatti <[email protected]>
cpu-all.h +1 -0
Index: qemu/cpu-all.h
===================================================================
@@ -818,6 +818,7 @@ void cpu_watchpoint_remove_all(CPUState 
 
 void cpu_single_step(CPUState *env, int enabled);
 void cpu_reset(CPUState *s);
+void run_on_cpu(CPUState *env, void (*func)(void *data), void *data);
 
 #define CPU_LOG_TB_OUT_ASM (1 << 0)
 #define CPU_LOG_TB_IN_ASM  (1 << 1)
cpu-defs.h +2 -0
Index: qemu/cpu-defs.h
===================================================================
@@ -132,6 +132,7 @@ typedef struct icount_decr_u16 {
 
 struct kvm_run;
 struct KVMState;
+struct qemu_work_item;
 
 typedef struct CPUBreakpoint {
     target_ulong pc;

@@ -204,6 +205,7 @@ typedef struct CPUWatchpoint {
     uint32_t created;                                                   \
     struct QemuThread *thread;                                          \
     struct QemuCond *halt_cond;                                         \
+    struct qemu_work_item *queued_work_first, *queued_work_last;        \
     const char *cpu_model_str;                                          \
     struct KVMState *kvm_state;                                         \
     struct kvm_run *kvm_run;                                            \
cpus.c +53 -0

83 lines not on this page · raw message

qemu-common.h +8 -0
Index: qemu/qemu-common.h
===================================================================
@@ -249,6 +249,14 @@ void qemu_notify_event(void);
 void qemu_cpu_kick(void *env);
 int qemu_cpu_self(void *env);
 
+/* work queue */
+struct qemu_work_item {
+    struct qemu_work_item *next;
+    void (*func)(void *data);
+    void *data;
+    int done;
+};
+
 #ifdef CONFIG_USER_ONLY
 #define qemu_init_vcpu(env) do { } while (0)
 #else
author
#5 lore raw

PATCH 5/9 kvm: synchronize state from cpu context

In reply to Marcelo Tosatti

From: Jan Kiszka <[email protected]>

It is not safe to retrieve the KVM internal state of a given cpu while its potentially modifying it.

Queue the request to run on cpu context, similarly to qemu-kvm.

Signed-off-by Marcelo Tosatti <[email protected]>
kvm-all.c +9 -1
Index: qemu/kvm-all.c
===================================================================
@@ -796,14 +796,22 @@ void kvm_flush_coalesced_mmio_buffer(voi
 #endif
 }
 
-void kvm_cpu_synchronize_state(CPUState *env)
+static void do_kvm_cpu_synchronize_state(void *_env)
 {
+    CPUState *env = _env;
+
     if (!env->kvm_vcpu_dirty) {
         kvm_arch_get_registers(env);
         env->kvm_vcpu_dirty = 1;
     }
 }
 
+void kvm_cpu_synchronize_state(CPUState *env)
+{
+    if (!env->kvm_vcpu_dirty)
+        run_on_cpu(env, do_kvm_cpu_synchronize_state, env);
+}
+
 void kvm_cpu_synchronize_post_reset(CPUState *env)
 {
     kvm_arch_put_registers(env, KVM_PUT_RESET_STATE);
#6 lore raw

Re: [patch uq/master 5/9] kvm: synchronize state from cpu context

Quoted 7 lines · Marcelo Tosatti

From: Jan Kiszka<[email protected]>

It is not safe to retrieve the KVM internal state of a given cpu while its potentially modifying it.

Queue the request to run on cpu context, similarly to qemu-kvm.

Even better is to query the state in the cpu thread; but that can come later.

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#7 lore raw

PATCH 6/9 add cpu_is_stopped helper

In reply to Marcelo Tosatti
Signed-off-by Marcelo Tosatti <[email protected]>
cpu-all.h +1 -0
Index: qemu/cpu-all.h
===================================================================
@@ -818,6 +818,7 @@ void cpu_watchpoint_remove_all(CPUState 
 
 void cpu_single_step(CPUState *env, int enabled);
 void cpu_reset(CPUState *s);
+int cpu_is_stopped(CPUState *env);
 void run_on_cpu(CPUState *env, void (*func)(void *data), void *data);
 
 #define CPU_LOG_TB_OUT_ASM (1 << 0)
cpus.c +5 -0
Index: qemu/cpus.c
===================================================================
@@ -91,6 +91,11 @@ void cpu_synchronize_all_post_init(void)
     }
 }
 
+int cpu_is_stopped(CPUState *env)
+{
+    return !vm_running || env->stopped;
+}
+
 static void do_vm_stop(int reason)
 {
     if (vm_running) {
author
#8 lore raw

PATCH 7/9 move stop/stopped CPU_COMMON fields after area zeroed by reset

In reply to Marcelo Tosatti

cpu_reset zeroes CPUState upto breakpoints member. Contents of stop/stopped should not be zeroed on cpu_reset.

Signed-off-by Marcelo Tosatti <[email protected]>
cpu-defs.h +2 -2
Index: qemu/cpu-defs.h
===================================================================
@@ -159,8 +159,6 @@ typedef struct CPUWatchpoint {
     target_ulong mem_io_vaddr; /* target virtual addr at which the      \
                                      memory was accessed */             \
     uint32_t halted; /* Nonzero if the CPU is in suspend state */       \
-    uint32_t stop;   /* Stop request */                                 \
-    uint32_t stopped; /* Artificially stopped */                        \
     uint32_t interrupt_request;                                         \
     volatile sig_atomic_t exit_request;                                 \
     CPU_COMMON_TLB                                                      \

@@ -203,6 +201,8 @@ typedef struct CPUWatchpoint {
     void *opaque;                                                       \
                                                                         \
     uint32_t created;                                                   \
+    uint32_t stop;   /* Stop request */                                 \
+    uint32_t stopped; /* Artificially stopped */                        \
     struct QemuThread *thread;                                          \
     struct QemuCond *halt_cond;                                         \
     struct qemu_work_item *queued_work_first, *queued_work_last;        \
author
#9 lore raw

PATCH 8/9 kvm: validate context for kvm cpu get/put operations

In reply to Marcelo Tosatti

From: Jan Kiszka <[email protected]>

Validate that KVM vcpu state is only read/written from cpu thread itself or that cpu is stopped.

Signed-off-by Marcelo Tosatti <[email protected]>
target-i386/kvm.c +4 -0
Index: qemu/target-i386/kvm.c
===================================================================
@@ -949,6 +949,8 @@ int kvm_arch_put_registers(CPUState *env
 {
     int ret;
 
+    assert(cpu_is_stopped(env) || qemu_cpu_self(env));
+
     ret = kvm_getput_regs(env, 1);
     if (ret < 0)
         return ret;

@@ -991,6 +993,8 @@ int kvm_arch_get_registers(CPUState *env
 {
     int ret;
 
+    assert(cpu_is_stopped(env) || qemu_cpu_self(env));
+
     ret = kvm_getput_regs(env, 0);
     if (ret < 0)
         return ret;
author
#10 lore raw

PATCH 9/9 kvm: enable smp > 1

In reply to Marcelo Tosatti

Process INIT/SIPI requests and enable -smp > 1.

Signed-off-by Marcelo Tosatti <[email protected]>
kvm-all.c +5 -5
Index: qemu/kvm-all.c
===================================================================
@@ -593,11 +593,6 @@ int kvm_init(int smp_cpus)
     int ret;
     int i;
 
-    if (smp_cpus > 1) {
-        fprintf(stderr, "No SMP KVM support, use '-smp 1'\n");
-        return -EINVAL;
-    }
-
     s = qemu_mallocz(sizeof(KVMState));
 
 #ifdef KVM_CAP_SET_GUEST_DEBUG

@@ -840,6 +835,11 @@ int kvm_cpu_exec(CPUState *env)
         }
 #endif
 
+        if (kvm_arch_process_irqchip_events(env)) {
+            ret = 0;
+            break;
+        }
+
         if (env->kvm_vcpu_dirty) {
             kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE);
             env->kvm_vcpu_dirty = 0;
kvm.h +2 -0
Index: qemu/kvm.h
===================================================================
@@ -90,6 +90,8 @@ int kvm_arch_handle_exit(CPUState *env, 
 
 int kvm_arch_pre_run(CPUState *env, struct kvm_run *run);
 
+int kvm_arch_process_irqchip_events(CPUState *env);
+
 int kvm_arch_get_registers(CPUState *env);
 
 /* state subset only touched by the VCPU itself during runtime */
target-i386/kvm.c +16 -0
Index: qemu/target-i386/kvm.c
===================================================================
@@ -1073,6 +1073,22 @@ int kvm_arch_post_run(CPUState *env, str
     return 0;
 }
 
+int kvm_arch_process_irqchip_events(CPUState *env)
+{
+    if (env->interrupt_request & CPU_INTERRUPT_INIT) {
+        kvm_cpu_synchronize_state(env);
+        do_cpu_init(env);
+        env->exception_index = EXCP_HALTED;
+    }
+
+    if (env->interrupt_request & CPU_INTERRUPT_SIPI) {
+        kvm_cpu_synchronize_state(env);
+        do_cpu_sipi(env);
+    }
+
+    return env->halted;
+}
+
 static int kvm_handle_halt(CPUState *env)
 {
     if (!((env->interrupt_request & CPU_INTERRUPT_HARD) &&
target-ppc/kvm.c +5 -0
Index: qemu/target-ppc/kvm.c
===================================================================
@@ -224,6 +224,11 @@ int kvm_arch_post_run(CPUState *env, str
     return 0;
 }
 
+int kvm_arch_process_irqchip_events(CPUState *env)
+{
+    return 0;
+}
+
 static int kvmppc_handle_halt(CPUState *env)
 {
     if (!(env->interrupt_request & CPU_INTERRUPT_HARD) && (msr_ee)) {
target-s390x/kvm.c +5 -0
Index: qemu/target-s390x/kvm.c
===================================================================
@@ -175,6 +175,11 @@ int kvm_arch_post_run(CPUState *env, str
     return 0;
 }
 
+int kvm_arch_process_irqchip_events(CPUState *env)
+{
+    return 0;
+}
+
 static void kvm_s390_interrupt_internal(CPUState *env, int type, uint32_t parm,
                                         uint64_t parm64, int vm)
 {
#12 lore raw
Quoted 7 lines · Anthony Liguori
Quoted 4 lines · Marcelo Tosatti
Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

How does this work without an in-kernel apic (or does uq/master already have an in-kernel apic)?

An in-kernel apic isn't needed (qemu-kvm supports smp with userspace apic).

Signature · 1 line
error compiling committee.c: too many arguments to function