Patchwork [12/15] qtest: add support for target-i386 -M pc

login
register
mail settings
Submitter Anthony Liguori
Date Jan. 10, 2012, 7:10 p.m.
Message ID <1326222656-26588-12-git-send-email-aliguori@us.ibm.com>
Download mbox | patch
Permalink /patch/135287/
State New
Headers show

Comments

Anthony Liguori - Jan. 10, 2012, 7:10 p.m.
This involves forcing the CPU into the halted state if qtest is enabled and
replacing the local APIC with the qtest interrupt controller.

It should be pretty straight forward to do the same for other machine types on
other architectures.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/pc.c      |    7 ++++++-
 hw/pc_piix.c |    9 ++++++---
 2 files changed, 12 insertions(+), 4 deletions(-)
Paolo Bonzini - Jan. 10, 2012, 7:56 p.m.
On 01/10/2012 08:10 PM, Anthony Liguori wrote:
> This involves forcing the CPU into the halted state if qtest is enabled and
> replacing the local APIC with the qtest interrupt controller.
>
> It should be pretty straight forward to do the same for other machine types on
> other architectures.
>
> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
> ---
>   hw/pc.c      |    7 ++++++-
>   hw/pc_piix.c |    9 ++++++---
>   2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 85304cf..fac5098 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -43,6 +43,7 @@
>   #include "ui/qemu-spice.h"
>   #include "memory.h"
>   #include "exec-memory.h"
> +#include "qtest.h"
>
>   /* output Bochs bios info messages */
>   //#define DEBUG_BIOS
> @@ -926,7 +927,11 @@ static void pc_cpu_reset(void *opaque)
>       CPUState *env = opaque;
>
>       cpu_reset(env);
> -    env->halted = !cpu_is_bsp(env);
> +    if (qtest_enabled()) {
> +        env->halted = 1;
> +    } else {
> +        env->halted = !cpu_is_bsp(env);
> +    }

This is wrong.  qtest and Xen should simply not create the CPU threads 
at all.

Paolo
Anthony Liguori - Jan. 11, 2012, 7:44 p.m.
On 01/10/2012 01:56 PM, Paolo Bonzini wrote:
> On 01/10/2012 08:10 PM, Anthony Liguori wrote:
>> This involves forcing the CPU into the halted state if qtest is enabled and
>> replacing the local APIC with the qtest interrupt controller.
>>
>> It should be pretty straight forward to do the same for other machine types on
>> other architectures.
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>> ---
>> hw/pc.c | 7 ++++++-
>> hw/pc_piix.c | 9 ++++++---
>> 2 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 85304cf..fac5098 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -43,6 +43,7 @@
>> #include "ui/qemu-spice.h"
>> #include "memory.h"
>> #include "exec-memory.h"
>> +#include "qtest.h"
>>
>> /* output Bochs bios info messages */
>> //#define DEBUG_BIOS
>> @@ -926,7 +927,11 @@ static void pc_cpu_reset(void *opaque)
>> CPUState *env = opaque;
>>
>> cpu_reset(env);
>> - env->halted = !cpu_is_bsp(env);
>> + if (qtest_enabled()) {
>> + env->halted = 1;
>> + } else {
>> + env->halted = !cpu_is_bsp(env);
>> + }
>
> This is wrong. qtest and Xen should simply not create the CPU threads at all.

This is easier said than done.  I started down this road and there's a huge 
amount of code that assumes that first_cpu != NULL.

I agree it's where we want to go though.

Regards,

Anthony Liguori

>
> Paolo
>
>
>
Paolo Bonzini - Jan. 12, 2012, 9:25 a.m.
On 01/11/2012 08:44 PM, Anthony Liguori wrote:
> This is easier said than done.  I started down this road and there's a
> huge amount of code that assumes that first_cpu != NULL.

That's why I said do not create the CPU _threads_. :)  But that wouldn't 
be a big step forward from halted = 1; for example, it would prevent 
using per-CPU work items.  Currently they're only used internally by 
KVM, but you never know.

So you can also create a CPU thread that does nothing.  Here is how it 
could look like, based on the KVM implementation:

static void *qemu_qtest_cpu_thread_fn(void *arg)
{
     CPUState *env = arg;
     int r;

     qemu_mutex_lock(&qemu_global_mutex);
     qemu_thread_get_self(env->thread);
     env->thread_id = qemu_get_thread_id();

     sigset_t waitset;
     sigemptyset(&waitset);
     sigaddset(&waitset, SIG_IPI);

     /* signal CPU creation */
     env->created = 1;
     qemu_cond_signal(&qemu_cpu_cond);

     cpu_single_env = env;
     while (1) {
         cpu_single_env = NULL;
         qemu_mutex_unlock_iothread();
         do {
             int sig;
             r = sigwait(&waitset, &sig);
         } while (r == -1 && (errno == EAGAIN || errno == EINTR));
         if (r == -1) {
             perror("sigtimedwait");
             exit(1);
         }
         qemu_mutex_lock_iothread();
         cpu_single_env = env;
         qemu_wait_io_event_common(env);
     }

     return NULL;
}

Paolo

Patch

diff --git a/hw/pc.c b/hw/pc.c
index 85304cf..fac5098 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -43,6 +43,7 @@ 
 #include "ui/qemu-spice.h"
 #include "memory.h"
 #include "exec-memory.h"
+#include "qtest.h"
 
 /* output Bochs bios info messages */
 //#define DEBUG_BIOS
@@ -926,7 +927,11 @@  static void pc_cpu_reset(void *opaque)
     CPUState *env = opaque;
 
     cpu_reset(env);
-    env->halted = !cpu_is_bsp(env);
+    if (qtest_enabled()) {
+        env->halted = 1;
+    } else {
+        env->halted = !cpu_is_bsp(env);
+    }
 }
 
 static CPUState *pc_new_cpu(const char *cpu_model)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index b70431f..2aba89c 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -46,6 +46,7 @@ 
 #ifdef CONFIG_XEN
 #  include <xen/hvm/hvm_info_table.h>
 #endif
+#include "qtest.h"
 
 #define MAX_IDE_BUS 2
 
@@ -154,11 +155,13 @@  static void pc_init1(MemoryRegion *system_memory,
     }
     isa_bus_irqs(isa_bus, gsi);
 
-    if (!xen_enabled()) {
+    if (xen_enabled()) {
+        i8259 = xen_interrupt_controller_init();
+    } else if (qtest_enabled()) {
+        i8259 = qtest_interrupt_controller_init();
+    } else {
         cpu_irq = pc_allocate_cpu_irq();
         i8259 = i8259_init(isa_bus, cpu_irq[0]);
-    } else {
-        i8259 = xen_interrupt_controller_init();
     }
 
     for (i = 0; i < ISA_NUM_IRQS; i++) {