From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KqDRf-00078o-H8 for qemu-devel@nongnu.org; Wed, 15 Oct 2008 16:58:15 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KqDRe-00077P-Gl for qemu-devel@nongnu.org; Wed, 15 Oct 2008 16:58:15 -0400 Received: from [199.232.76.173] (port=56403 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KqDRe-000778-Bh for qemu-devel@nongnu.org; Wed, 15 Oct 2008 16:58:14 -0400 Received: from yw-out-1718.google.com ([74.125.46.158]:20770) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KqDRd-0003z4-K0 for qemu-devel@nongnu.org; Wed, 15 Oct 2008 16:58:13 -0400 Received: by yw-out-1718.google.com with SMTP id 6so608919ywa.82 for ; Wed, 15 Oct 2008 13:58:12 -0700 (PDT) Message-ID: <5d6222a80810151358l16c8a4a9o97f468f65a1f38ab@mail.gmail.com> Date: Wed, 15 Oct 2008 18:58:11 -0200 From: "Glauber Costa" Subject: Re: [Qemu-devel] Re: [PATCH 02/21] introduce QEMUAccel and fill it with interrupt specific driver In-Reply-To: <48F65079.1040607@us.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <1224107718-19128-1-git-send-email-glommer@redhat.com> <1224107718-19128-3-git-send-email-glommer@redhat.com> <48F65079.1040607@us.ibm.com> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: jan.kiszka@siemens.com, Glauber Costa , jes@sgi.com, avi@qumranet.com, Glauber Costa , dmitry.baryshkov@siemens.com On Wed, Oct 15, 2008 at 6:20 PM, Anthony Liguori wrote: > Glauber Costa wrote: >> >> From: 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). >> >> 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 >> Signed-off-by: Dmitry Baryshkov >> --- >> Makefile.target | 2 +- >> accel.c | 17 +++++++++++++++++ >> accel.h | 20 ++++++++++++++++++++ >> exec.c | 3 +++ >> kqemu.c | 9 +++++++++ >> vl.c | 17 +++++------------ >> 6 files changed, 55 insertions(+), 13 deletions(-) >> create mode 100644 accel.c >> create mode 100644 accel.h >> >> diff --git a/Makefile.target b/Makefile.target >> index e2edf9d..623ecd8 100644 >> --- a/Makefile.target >> +++ b/Makefile.target >> @@ -188,7 +188,7 @@ all: $(PROGS) >> ######################################################### >> # cpu emulator library >> LIBOBJS=exec.o kqemu.o translate-all.o cpu-exec.o\ >> - translate.o host-utils.o >> + translate.o host-utils.o accel.o >> ifdef CONFIG_DYNGEN_OP >> exec.o: dyngen-opc.h >> LIBOBJS+=op.o >> diff --git a/accel.c b/accel.c >> new file mode 100644 >> index 0000000..d30460d >> --- /dev/null >> +++ b/accel.c >> @@ -0,0 +1,17 @@ >> +#include "hw/hw.h" >> +#include "accel.h" >> + >> +QEMUAccel *current_accel; >> + >> +int _accel_nop(void) >> +{ >> + return 0; >> +} >> + >> +#define accel_nop ((void *)_accel_nop) >> + >> +/* Accelerator wrapper for the no-accel (raw qemu) case */ >> +QEMUAccel noaccel = { >> + .cpu_interrupt = accel_nop, >> +}; >> + >> diff --git a/accel.h b/accel.h >> new file mode 100644 >> index 0000000..8e5ddc6 >> --- /dev/null >> +++ b/accel.h >> @@ -0,0 +1,20 @@ >> +#ifndef _ACCEL_H_ >> +#define _ACCEL_H_ >> + >> +typedef struct QEMUAccel { >> + void (*cpu_interrupt)(CPUState *env); >> +} QEMUAccel; >> + >> +extern QEMUAccel *current_accel; >> +extern QEMUAccel noaccel; >> + >> +static inline void register_qemu_accel(QEMUAccel *accel) >> +{ >> + current_accel = accel; >> +} >> + >> +static inline void accel_cpu_interrupt(CPUState *env) >> +{ >> + current_accel->cpu_interrupt(env); >> +} >> +#endif >> diff --git a/exec.c b/exec.c >> index 1cad0be..21253cc 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -43,6 +43,8 @@ >> #include >> #endif >> >> +#include "accel.h" >> + >> //#define DEBUG_TB_INVALIDATE >> //#define DEBUG_FLUSH >> //#define DEBUG_TLB >> @@ -1430,6 +1432,7 @@ void cpu_single_step(CPUState *env, int enabled) >> tb_flush(env); >> } >> #endif >> + accel_cpu_interrupt(env); >> } >> >> /* enable or disable low levels log */ >> diff --git a/kqemu.c b/kqemu.c >> index 9b52237..87c06cd 100644 >> --- a/kqemu.c >> +++ b/kqemu.c >> @@ -50,6 +50,7 @@ >> #include >> #include >> #include "kqemu.h" >> +#include "accel.h" >> >> #ifdef _WIN32 >> #define KQEMU_DEVICE "\\\\.\\kqemu" >> @@ -150,6 +151,8 @@ static void kqemu_update_cpuid(CPUState *env) >> accelerated code */ >> } >> >> +QEMUAccel kqemu_accel; >> + >> int kqemu_start(void) >> { >> struct kqemu_init kinit; >> @@ -232,6 +235,7 @@ int kqemu_start(void) >> } >> nb_pages_to_flush = 0; >> nb_ram_pages_to_update = 0; >> + register_qemu_accel(&kqemu_accel); >> >> qpi_init(); >> return 0; >> @@ -243,6 +247,11 @@ void kqemu_init_env(CPUState *env) >> env->kqemu_enabled = kqemu_allowed; >> } >> >> +QEMUAccel kqemu_accel = { >> + .cpu_interrupt = kqemu_cpu_interrupt, >> +}; >> > > The hook kqemu uses is for CPU_INTERRUPT_EXIT which is basically intended to > allow IO to run. The fact that it's done via cpu_interrupt is somewhat > hackish. > > I think a better hook would be something like run_io_handlers. Since this > is called from signal handlers, we have to be clear that this code needs to > be signal safe. This is compared to say a generic cpu_interrupt hook which > doesn't necessarily need to be signal safe. KVM has code in cpu interrupt. So even if we move kqemu elsewhere, this one might still be useful. > Regards, > > Anthony Liguori > > > -- Glauber Costa. "Free as in Freedom" http://glommer.net "The less confident you are, the more serious you have to act."