Glauber Costa wrote:
@@ -93,7 +74,20 @@
CPUState *qemu_kvm_cpu_env(int index)
{
[snip]
}
This doesn't seem right. This function exists because we used to have
a vcpu and and env structs, that were separated but
should be tied together in some uses.
Now, there's absolutely nothing in here that is not qemu-specific.
This is just a function to return and env given a cpu number.
You'll lose the current_env optimization that probably matters a lot
in your case, but I'm afraid you will just have to live with that:
it sucks for qemu too, and when it is fixed, it will be fixed for both
(means getting rid of the ugly cpu_single_env)
Hi Glauber,
I see this function as a temporary transition type thing, it wasn't
my intention for it to stay permanently, but it's not really called from
a lot of performance critical places for now.
if (env) {
- if (!vcpu)
+ if (!current_env->vcpu_info.created)
signal = 1;
!vcpu is probably meant to catch the case in witch the vcpu tls
variable is not yet assigned. By dereferencing current_env here,
you are probably doing an invalid access. So unless you can prove this
is not an issue, should add another test.
Hmmm I think the test got lost because I moved this over in multiple
steps. It was a thread variable 'cpu_info' prior, but we have no
guarantee here that current_env is valid. Thanks.
- if (vcpu && env != vcpu->env &&
!vcpu_info[env->cpu_index].signalled)
+ /*
+ * Testing for vcpu_info.created here is really redundant
+ */
+ if (current_env->vcpu_info.created &&
+ env != current_env && env->vcpu_info.signalled)
should be !env->vcpu_info.signalled instead?
Good catch!
static void flush_queued_work(CPUState *env)
{
- struct vcpu_info *vi = &vcpu_info[env->cpu_index];
+ struct vcpu_info *vi = &env->vcpu_info;
struct qemu_kvm_work_item *wi;
"vi" is not a good name, since emacs users will likely complain.
vcpu_info is a much better name.
I'm an Emacs user! It's just a temporary variable, I really don't think
this matters. We could name it 'ed' just to be different, if you prefer.
Here's an updated version of that patch.
Cheers,
Jes
PS: I am gone through Wednesday, so should give you a few days to
review :)
Merge vcpu_info into CPUState.
Moves definition of vcpu related structs to new header qemu-kvm-vcpu.h
and declares this struct in i386/ia64/ppc CPUState structs if USE_KVM
is defined. In addition conver qemu-kvm.c to pull vcpu_info out of
CPUState.
This eliminates ugly static sized array of struct vcpu_info.
Signed-off-by: Jes Sorensen <jes@xxxxxxx>
---
qemu/qemu-kvm-vcpu.h | 34 +++++++++++
qemu/qemu-kvm.c | 143 ++++++++++++++++++++++++++-----------------------
qemu/target-i386/cpu.h | 4 +
qemu/target-ia64/cpu.h | 5 +
qemu/target-ppc/cpu.h | 5 +
5 files changed, 126 insertions(+), 65 deletions(-)
Index: kvm-userspace.git/qemu/qemu-kvm-vcpu.h
===================================================================
--- /dev/null
+++ kvm-userspace.git/qemu/qemu-kvm-vcpu.h
@@ -0,0 +1,34 @@
+/*
+ * qemu/kvm vcpu definitions
+ *
+ * Copyright (C) 2006-2008 Qumranet Technologies
+ *
+ * Licensed under the terms of the GNU GPL version 2 or higher.
+ */
+#ifndef QEMU_KVM_VCPU_H
+#define QEMU_KVM_VCPU_H
+
+#include <pthread.h>
+
+struct qemu_kvm_work_item {
+ struct qemu_kvm_work_item *next;
+ void (*func)(void *data);
+ void *data;
+ int done;
+};
+
+/*
+ * KVM vcpu struct
+ */
+struct vcpu_info {
+ int sipi_needed;
+ int init;
+ pthread_t thread;
+ int signalled;
+ int stop;
+ int stopped;
+ int created;
+ struct qemu_kvm_work_item *queued_work_first, *queued_work_last;
+};
+
+#endif
Index: kvm-userspace.git/qemu/qemu-kvm.c
===================================================================
--- kvm-userspace.git.orig/qemu/qemu-kvm.c
+++ kvm-userspace.git/qemu/qemu-kvm.c
@@ -22,13 +22,13 @@
#include "compatfd.h"
#include "qemu-kvm.h"
+#include "qemu-kvm-vcpu.h"
#include <libkvm.h>
#include <pthread.h>
#include <sys/utsname.h>
#include <sys/syscall.h>
#include <sys/mman.h>
-#define bool _Bool
#define false 0
#define true 1
@@ -43,31 +43,12 @@
pthread_cond_t qemu_system_cond = PTHREAD_COND_INITIALIZER;
pthread_cond_t qemu_pause_cond = PTHREAD_COND_INITIALIZER;
pthread_cond_t qemu_work_cond = PTHREAD_COND_INITIALIZER;
-__thread struct vcpu_info *vcpu;
+__thread struct CPUState *current_env;
static int qemu_system_ready;
#define SIG_IPI (SIGRTMIN+4)
-struct qemu_kvm_work_item {
- struct qemu_kvm_work_item *next;
- void (*func)(void *data);
- void *data;
- bool done;
-};
-
-struct vcpu_info {
- CPUState *env;
- int sipi_needed;
- int init;
- pthread_t thread;
- int signalled;
- int stop;
- int stopped;
- int created;
- struct qemu_kvm_work_item *queued_work_first, *queued_work_last;
-} vcpu_info[256];
-
pthread_t io_thread;
static int io_thread_fd = -1;
static int io_thread_sigfd = -1;
@@ -93,7 +74,20 @@
CPUState *qemu_kvm_cpu_env(int index)
{
- return vcpu_info[index].env;
+ CPUState *penv;
+
+ if (current_env->cpu_index == index)
+ return current_env;
+
+ penv = first_cpu;
+
+ while (penv) {
+ if (penv->cpu_index == index)
+ return penv;
+ penv = (CPUState *)penv->next_cpu;
+ }
+
+ return NULL;
}
static void sig_ipi_handler(int n)
@@ -102,10 +96,10 @@
static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
{
- struct vcpu_info *vi = &vcpu_info[env->cpu_index];
+ struct vcpu_info *vi = &env->vcpu_info;
struct qemu_kvm_work_item wi;
- if (vi == vcpu) {
+ if (env == current_env) {
func(data);
return;
}
@@ -127,7 +121,7 @@
static void inject_interrupt(void *data)
{
- cpu_interrupt(vcpu->env, (int)data);
+ cpu_interrupt(current_env, (int)data);
}
void kvm_inject_interrupt(CPUState *env, int mask)
@@ -140,29 +134,33 @@
int signal = 0;
if (env) {
- if (!vcpu)
+ if (current_env && !current_env->vcpu_info.created)
signal = 1;
- if (vcpu && env != vcpu->env && !vcpu_info[env->cpu_index].signalled)
+ /*
+ * Testing for vcpu_info.created here is really redundant
+ */
+ if (current_env->vcpu_info.created &&
+ env != current_env && !env->vcpu_info.signalled)
signal = 1;
if (signal) {
- vcpu_info[env->cpu_index].signalled = 1;
- if (vcpu_info[env->cpu_index].thread)
- pthread_kill(vcpu_info[env->cpu_index].thread, SIG_IPI);
+ env->vcpu_info.signalled = 1;
+ if (env->vcpu_info.thread)
+ pthread_kill(env->vcpu_info.thread, SIG_IPI);
}
}
}
void kvm_update_after_sipi(CPUState *env)
{
- vcpu_info[env->cpu_index].sipi_needed = 1;
+ env->vcpu_info.sipi_needed = 1;
kvm_update_interrupt_request(env);
}
void kvm_apic_init(CPUState *env)
{
if (env->cpu_index != 0)
- vcpu_info[env->cpu_index].init = 1;
+ env->vcpu_info.init = 1;
kvm_update_interrupt_request(env);
}
@@ -240,7 +238,7 @@
static int has_work(CPUState *env)
{
- if (!vm_running || (env && vcpu_info[env->cpu_index].stopped))
+ if (!vm_running || (env && env->vcpu_info.stopped))
return 0;
if (!env->halted)
return 1;
@@ -249,7 +247,7 @@
static void flush_queued_work(CPUState *env)
{
- struct vcpu_info *vi = &vcpu_info[env->cpu_index];
+ struct vcpu_info *vi = &env->vcpu_info;
struct qemu_kvm_work_item *wi;
if (!vi->queued_work_first)
@@ -266,6 +264,7 @@
static void kvm_main_loop_wait(CPUState *env, int timeout)
{
+ struct vcpu_info *vi = &env->vcpu_info;
struct timespec ts;
int r, e;
siginfo_t siginfo;
@@ -291,49 +290,55 @@
cpu_single_env = env;
flush_queued_work(env);
- if (vcpu_info[env->cpu_index].stop) {
- vcpu_info[env->cpu_index].stop = 0;
- vcpu_info[env->cpu_index].stopped = 1;
+ if (vi->stop) {
+ vi->stop = 0;
+ vi->stopped = 1;
pthread_cond_signal(&qemu_pause_cond);
}
- vcpu_info[env->cpu_index].signalled = 0;
+ vi->signalled = 0;
}
static int all_threads_paused(void)
{
- int i;
+ CPUState *penv = first_cpu;
+
+ while (penv) {
+ if (penv->vcpu_info.stop)
+ return 0;
+ penv = (CPUState *)penv->next_cpu;
+ }
- for (i = 0; i < smp_cpus; ++i)
- if (vcpu_info[i].stop)
- return 0;
return 1;
}
static void pause_all_threads(void)
{
- int i;
+ CPUState *penv = first_cpu;
assert(!cpu_single_env);
- for (i = 0; i < smp_cpus; ++i) {
- vcpu_info[i].stop = 1;
- pthread_kill(vcpu_info[i].thread, SIG_IPI);
+ while (penv) {
+ penv->vcpu_info.stop = 1;
+ pthread_kill(penv->vcpu_info.thread, SIG_IPI);
+ penv = (CPUState *)penv->next_cpu;
}
+
while (!all_threads_paused())
qemu_cond_wait(&qemu_pause_cond);
}
static void resume_all_threads(void)
{
- int i;
+ CPUState *penv = first_cpu;
assert(!cpu_single_env);
- for (i = 0; i < smp_cpus; ++i) {
- vcpu_info[i].stop = 0;
- vcpu_info[i].stopped = 0;
- pthread_kill(vcpu_info[i].thread, SIG_IPI);
+ while (penv) {
+ penv->vcpu_info.stop = 0;
+ penv->vcpu_info.stopped = 0;
+ pthread_kill(penv->vcpu_info.thread, SIG_IPI);
+ penv = (CPUState *)penv->next_cpu;
}
}
@@ -348,8 +353,8 @@
static void update_regs_for_sipi(CPUState *env)
{
kvm_arch_update_regs_for_sipi(env);
- vcpu_info[env->cpu_index].sipi_needed = 0;
- vcpu_info[env->cpu_index].init = 0;
+ env->vcpu_info.sipi_needed = 0;
+ env->vcpu_info.init = 0;
}
static void update_regs_for_init(CPUState *env)
@@ -376,21 +381,23 @@
void qemu_kvm_system_reset(void)
{
- int i;
+ CPUState *penv = first_cpu;
pause_all_threads();
qemu_system_reset();
- for (i = 0; i < smp_cpus; ++i)
- kvm_arch_cpu_reset(vcpu_info[i].env);
+ while (penv) {
+ kvm_arch_cpu_reset(penv);
+ penv = (CPUState *)penv->next_cpu;
+ }
resume_all_threads();
}
static int kvm_main_loop_cpu(CPUState *env)
{
- struct vcpu_info *info = &vcpu_info[env->cpu_index];
+ struct vcpu_info *info = &env->vcpu_info;
setup_kernel_sigmask(env);
@@ -429,9 +436,8 @@
CPUState *env = _env;
sigset_t signals;
- vcpu = &vcpu_info[env->cpu_index];
- vcpu->env = env;
- vcpu->env->thread_id = kvm_get_thread_id();
+ current_env = env;
+ env->thread_id = kvm_get_thread_id();
sigfillset(&signals);
sigprocmask(SIG_BLOCK, &signals, NULL);
kvm_create_vcpu(kvm_context, env->cpu_index);
@@ -439,7 +445,7 @@
/* signal VCPU creation */
pthread_mutex_lock(&qemu_mutex);
- vcpu->created = 1;
+ current_env->vcpu_info.created = 1;
pthread_cond_signal(&qemu_vcpu_cond);
/* and wait for machine initialization */
@@ -453,9 +459,9 @@
void kvm_init_new_ap(int cpu, CPUState *env)
{
- pthread_create(&vcpu_info[cpu].thread, NULL, ap_main_loop, env);
+ pthread_create(&env->vcpu_info.thread, NULL, ap_main_loop, env);
- while (vcpu_info[cpu].created == 0)
+ while (env->vcpu_info.created == 0)
qemu_cond_wait(&qemu_vcpu_cond);
}
@@ -613,8 +619,12 @@
static int kvm_debug(void *opaque, int vcpu)
{
+ CPUState *env;
+
+ env = qemu_kvm_cpu_env(vcpu);
+
kvm_debug_stop_requested = 1;
- vcpu_info[vcpu].stopped = 1;
+ env->vcpu_info.stopped = 1;
return 1;
}
@@ -710,8 +720,11 @@
static int kvm_shutdown(void *opaque, int vcpu)
{
+ CPUState *env;
+
+ env = qemu_kvm_cpu_env(cpu_single_env->cpu_index);
/* stop the current vcpu from going back to guest mode */
- vcpu_info[cpu_single_env->cpu_index].stopped = 1;
+ env->vcpu_info.stopped = 1;
qemu_system_reset_request();
return 1;
Index: kvm-userspace.git/qemu/target-i386/cpu.h
===================================================================
--- kvm-userspace.git.orig/qemu/target-i386/cpu.h
+++ kvm-userspace.git/qemu/target-i386/cpu.h
@@ -45,6 +45,7 @@
#include "cpu-defs.h"
#include "softfloat.h"
+#include "qemu-kvm-vcpu.h"
#define R_EAX 0
#define R_ECX 1
@@ -622,6 +623,9 @@
#define NR_IRQ_WORDS (256/ BITS_PER_LONG)
uint32_t kvm_interrupt_bitmap[NR_IRQ_WORDS];
+#ifdef USE_KVM
+ struct vcpu_info vcpu_info;
+#endif
/* in order to simplify APIC support, we leave this pointer to the
user */
struct APICState *apic_state;
Index: kvm-userspace.git/qemu/target-ia64/cpu.h
===================================================================
--- kvm-userspace.git.orig/qemu/target-ia64/cpu.h
+++ kvm-userspace.git/qemu/target-ia64/cpu.h
@@ -40,10 +40,15 @@
#include "cpu-defs.h"
#include "softfloat.h"
+#include "qemu-kvm-vcpu.h"
+
typedef struct CPUIA64State {
CPU_COMMON;
uint32_t hflags;
int mp_state;
+#ifdef USE_KVM
+ struct vcpu_info vcpu_info;
+#endif
} CPUIA64State;
#define CPUState CPUIA64State
Index: kvm-userspace.git/qemu/target-ppc/cpu.h
===================================================================
--- kvm-userspace.git.orig/qemu/target-ppc/cpu.h
+++ kvm-userspace.git/qemu/target-ppc/cpu.h
@@ -22,6 +22,7 @@
#include "config.h"
#include <inttypes.h>
+#include "qemu-kvm-vcpu.h"
//#define PPC_EMULATE_32BITS_HYPV
@@ -578,6 +579,10 @@
CPU_COMMON
+#ifdef USE_KVM
+ struct vcpu_info vcpu_info;
+#endif
+
int access_type; /* when a memory exception occurs, the access
type is stored here */
