[PATCH 0/2] in-kernel APIC v3 (usermode side)

[PATCH 0/2] in-kernel APIC v3 (usermode side)

[Gregory Haskins]

This includes updates to the usermode patches to coincide with the v3 kernel
patch.  As before, this usermode code is really just for testing the level-1
features of the kernel code.  This usermode code needs to be polished up
before it can be considered for inclusion.  For example:

1) Support older kernels which do not have in-kernel APIC
2) Support the ability to dynamically select the apic emulation level from the
command line.

etc.

Notably missing is support to feed kernel-based interrupts up to userspace.
In practice this isnt a problem, since timer interrupts will also kick the
system out of a halt.  But its a hack to rely on that, and since there is no
guarantee an OS will use the timer this might not work in all cases.
Fortunately both XP and Linux use the timer, so HLTs never deadlock the
system.  We will add the support for signaling userspace for real hopefully on
the next drop.

If you want to test level-0 vs level-1 today, simply run an unpatched qemu on
a v3 patched kernel for level-0, and then apply this patch for level-1.


Regards,
-Greg 

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[PATCH 1/2] KVM: Updates for compiling in-kernel APIC support with external-modules

[Gregory Haskins]

Signed-off-by: Gregory Haskins <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>
---

 kernel/Kbuild |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/Kbuild b/kernel/Kbuild
index e9bcda7..103a179 100644
--- a/kernel/Kbuild
+++ b/kernel/Kbuild
@@ -1,5 +1,5 @@
 EXTRA_CFLAGS := -I$(src)/include -include $(src)/external-module-compat.h
 obj-m := kvm.o kvm-intel.o kvm-amd.o
-kvm-objs := kvm_main.o mmu.o x86_emulate.o
+kvm-objs := kvm_main.o mmu.o x86_emulate.o userint.o kernint.o lapic.o
 kvm-intel-objs := vmx.o vmx-debug.o
 kvm-amd-objs := svm.o


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[PATCH 2/2] KVM: in-kernel-apic modification to QEMU

[Gregory Haskins]

Signed-off-by: Gregory Haskins <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>
---

 qemu/hw/apic.c  |   22 ++++++++++++++++++++--
 qemu/hw/pc.c    |   13 ++++++++++++-
 qemu/qemu-kvm.c |   18 ++++++++++--------
 qemu/vl.h       |    2 +-
 user/kvmctl.c   |   25 +++++++++++++++++++++++--
 user/kvmctl.h   |    5 +++--
 user/main.c     |    3 +--
 7 files changed, 70 insertions(+), 18 deletions(-)

diff --git a/qemu/hw/apic.c b/qemu/hw/apic.c
index 5704224..8aa3ef1 100644
--- a/qemu/hw/apic.c
+++ b/qemu/hw/apic.c
@@ -19,6 +19,11 @@
  */
 #include "vl.h"
 
+#ifdef USE_KVM
+#include "kvmctl.h"
+extern kvm_context_t kvm_context;
+#endif
+
 //#define DEBUG_APIC
 //#define DEBUG_IOAPIC
 
@@ -87,6 +92,7 @@ typedef struct APICState {
 } APICState;
 
 struct IOAPICState {
+    CPUState *cpu_env;
     uint8_t id;
     uint8_t ioregsel;
 
@@ -895,10 +901,21 @@ static void ioapic_service(IOAPICState *s)
                     vector = pic_read_irq(isa_pic);
                 else
                     vector = entry & 0xff;
-                
+             
+#ifdef USE_KVM
+		if (kvm_allowed) {
+		    kvm_apic_bus_deliver(kvm_context, dest, trig_mode,
+					 dest_mode, delivery_mode, vector);
+		    cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD);
+		} else {
+#endif
                 apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode);
                 apic_bus_deliver(deliver_bitmask, delivery_mode, 
                                  vector, polarity, trig_mode);
+#ifdef USE_KVM
+		}
+#endif
+
             }
         }
     }
@@ -1052,7 +1069,7 @@ static CPUWriteMemoryFunc *ioapic_mem_write[3] = {
     ioapic_mem_writel,
 };
 
-IOAPICState *ioapic_init(void)
+IOAPICState *ioapic_init(CPUState *env)
 {
     IOAPICState *s;
     int io_memory;
@@ -1061,6 +1078,7 @@ IOAPICState *ioapic_init(void)
     if (!s)
         return NULL;
     ioapic_reset(s);
+    s->cpu_env = env;
     s->id = last_apic_id++;
 
     io_memory = cpu_register_io_memory(0, ioapic_mem_read, 
diff --git a/qemu/hw/pc.c b/qemu/hw/pc.c
index eda49cf..3862a8f 100644
--- a/qemu/hw/pc.c
+++ b/qemu/hw/pc.c
@@ -91,6 +91,9 @@ int cpu_get_pic_interrupt(CPUState *env)
 {
     int intno;
 
+#ifdef USE_KVM
+    if (!kvm_allowed) {
+#endif
     intno = apic_get_interrupt(env);
     if (intno >= 0) {
         /* set irq request if a PIC irq is still pending */
@@ -98,10 +101,15 @@ int cpu_get_pic_interrupt(CPUState *env)
         pic_update_irq(isa_pic); 
         return intno;
     }
+
     /* read the irq from the PIC */
     if (!apic_accept_pic_intr(env))
         return -1;
 
+#ifdef USE_KVM
+    }
+#endif
+
     intno = pic_read_irq(isa_pic);
     return intno;
 }
@@ -483,6 +491,9 @@ static void pc_init1(int ram_size, int vga_ram_size, int boot_device,
         }
         register_savevm("cpu", i, 4, cpu_save, cpu_load, env);
         qemu_register_reset(main_cpu_reset, env);
+#ifdef USE_KVM
+	if (!kvm_allowed)
+#endif
         if (pci_enabled) {
             apic_init(env);
         }
@@ -671,7 +682,7 @@ static void pc_init1(int ram_size, int vga_ram_size, int boot_device,
     register_ioport_write(0x92, 1, 1, ioport92_write, NULL);
 
     if (pci_enabled) {
-        ioapic = ioapic_init();
+        ioapic = ioapic_init(env);
     }
     isa_pic = pic_init(pic_irq_request, first_cpu);
     pit = pit_init(0x40, 0);
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index 1849997..d7679bc 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -227,9 +227,11 @@ static void load_regs(CPUState *env)
     sregs.cr3 = env->cr[3];
     sregs.cr4 = env->cr[4];
 
-    sregs.apic_base = cpu_get_apic_base(env);
+    /* These two are no longer used once the in-kernel APIC is enabled */
+    sregs.apic_base = 0;
+    sregs.cr8 = 0;
+
     sregs.efer = env->efer;
-    sregs.cr8 = cpu_get_apic_tpr(env);
 
     kvm_set_sregs(kvm_context, 0, &sregs);
 
@@ -321,7 +323,7 @@ static void save_regs(CPUState *env)
     env->cr[3] = sregs.cr3;
     env->cr[4] = sregs.cr4;
 
-    cpu_set_apic_base(env, sregs.apic_base);
+    //cpu_set_apic_base(env, sregs.apic_base);
 
     env->efer = sregs.efer;
     //cpu_set_apic_tpr(env, sregs.cr8);
@@ -410,9 +412,12 @@ static int try_push_interrupts(void *opaque)
     if (env->ready_for_interrupt_injection &&
         (env->interrupt_request & CPU_INTERRUPT_HARD) &&
         (env->eflags & IF_MASK)) {
+	    int irq = cpu_get_pic_interrupt(env);
+
+	    if (irq != -1)
+		    kvm_inject_isa_irq(kvm_context, irq);
+
             env->interrupt_request &= ~CPU_INTERRUPT_HARD;
-            // for now using cpu 0
-            kvm_inject_irq(kvm_context, 0, cpu_get_pic_interrupt(env));
     }
 
     return (env->interrupt_request & CPU_INTERRUPT_HARD) != 0;
@@ -427,8 +432,6 @@ static void post_kvm_run(void *opaque, int vcpu)
 	? env->eflags | IF_MASK : env->eflags & ~IF_MASK;
     env->ready_for_interrupt_injection
 	= kvm_is_ready_for_interrupt_injection(kvm_context, vcpu);
-    //cpu_set_apic_tpr(env, kvm_run->cr8);
-    cpu_set_apic_base(env, kvm_get_apic_base(kvm_context, vcpu));
 }
 
 static void pre_kvm_run(void *opaque, int vcpu)
@@ -436,7 +439,6 @@ static void pre_kvm_run(void *opaque, int vcpu)
     CPUState **envs = opaque, *env;
     env = envs[0];
 
-    kvm_set_cr8(kvm_context, vcpu, cpu_get_apic_tpr(env));
 }
 
 void kvm_load_registers(CPUState *env)
diff --git a/qemu/vl.h b/qemu/vl.h
index debd17c..dc72cd8 100644
--- a/qemu/vl.h
+++ b/qemu/vl.h
@@ -1059,7 +1059,7 @@ typedef struct IOAPICState IOAPICState;
 int apic_init(CPUState *env);
 int apic_get_interrupt(CPUState *env);
 int apic_accept_pic_intr(CPUState *env);
-IOAPICState *ioapic_init(void);
+IOAPICState *ioapic_init(CPUState *env);
 void ioapic_set_irq(void *opaque, int vector, int level);
 
 /* i8254.c */
diff --git a/user/kvmctl.c b/user/kvmctl.c
index 6b1778b..5d7be80 100644
--- a/user/kvmctl.c
+++ b/user/kvmctl.c
@@ -210,6 +210,7 @@ int kvm_create(kvm_context_t kvm, unsigned long memory, void **vm_mem)
 	int fd = kvm->fd;
 	int zfd;
 	int r;
+	const int apic_level = 1;
 	struct kvm_memory_region low_memory = {
 		.slot = 3,
 		.memory_size = memory  < dosmem ? memory : dosmem,
@@ -259,6 +260,12 @@ int kvm_create(kvm_context_t kvm, unsigned long memory, void **vm_mem)
 	     MAP_PRIVATE|MAP_FIXED, zfd, 0);
 	close(zfd);
 
+	r = ioctl(fd, KVM_ENABLE_KERNEL_PIC, &apic_level);
+	if (r == -1) {
+		fprintf(stderr, "kvm_enable_kernel_pic: %m\n");
+		return -1;
+	}
+
 	r = ioctl(fd, KVM_CREATE_VCPU, 0);
 	if (r == -1) {
 		fprintf(stderr, "kvm_create_vcpu: %m\n");
@@ -959,12 +966,26 @@ more:
 	return r;
 }
 
-int kvm_inject_irq(kvm_context_t kvm, int vcpu, unsigned irq)
+int kvm_inject_isa_irq(kvm_context_t kvm, unsigned irq)
 {
 	struct kvm_interrupt intr;
 
 	intr.irq = irq;
-	return ioctl(kvm->vcpu_fd[vcpu], KVM_INTERRUPT, &intr);
+	return ioctl(kvm->vm_fd, KVM_ISA_INTERRUPT, &intr);
+}
+
+int kvm_apic_bus_deliver(kvm_context_t kvm, int dest, int trig_mode,
+			 int dest_mode, int delivery_mode, int vector)
+{
+	struct kvm_apic_msg msg;
+
+	msg.dest          = dest;
+	msg.trig_mode     = trig_mode;
+	msg.dest_mode     = dest_mode;
+	msg.delivery_mode = delivery_mode;
+	msg.vector        = vector;
+
+	return ioctl(kvm->vm_fd, KVM_APIC_MSG, &msg);
 }
 
 int kvm_guest_debug(kvm_context_t kvm, int vcpu, struct kvm_debug_guest *dbg)
diff --git a/user/kvmctl.h b/user/kvmctl.h
index b14cb45..7203570 100644
--- a/user/kvmctl.h
+++ b/user/kvmctl.h
@@ -280,11 +280,12 @@ int kvm_set_msrs(kvm_context_t, int vcpu, struct kvm_msr_entry *msrs, int n);
  * This allows you to simulate an external vectored interrupt.
  *
  * \param kvm Pointer to the current kvm_context
- * \param vcpu Which virtual CPU should get dumped
  * \param irq Vector number
  * \return 0 on success
  */
-int kvm_inject_irq(kvm_context_t kvm, int vcpu, unsigned irq);
+int kvm_inject_isa_irq(kvm_context_t kvm, unsigned irq);
+int kvm_apic_bus_deliver(kvm_context_t kvm, int dest, int trig_mode,
+			 int dest_mode, int delivery_mode, int vector);
 int kvm_guest_debug(kvm_context_t, int vcpu, struct kvm_debug_guest *dbg);
 
 /*!
diff --git a/user/main.c b/user/main.c
index 9333da5..3d81d29 100644
--- a/user/main.c
+++ b/user/main.c
@@ -49,7 +49,7 @@ static int test_outb(void *opaque, uint16_t addr, uint8_t value)
     switch (addr) {
     case 0xff: // irq injector
 	printf("injecting interrupt 0x%x\n", value);
-	kvm_inject_irq(kvm, 0, value);
+	// kvm_inject_irq(kvm, 0, value);
 	break;
     case 0xf1: // serial
 	if (newline)
@@ -116,7 +116,6 @@ static struct kvm_callbacks test_callbacks = {
     .debug       = test_debug,
     .halt        = test_halt,
     .io_window = test_io_window,
-    .try_push_interrupts = test_try_push_interrupts,
     .post_kvm_run = test_post_kvm_run,
     .pre_kvm_run = test_pre_kvm_run,
 };


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH 2/2] KVM: in-kernel-apic modification to QEMU

[Anthony Liguori]

Gregory Haskins wrote:
> Signed-off-by: Gregory Haskins <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>
> ---
>
>  qemu/hw/apic.c  |   22 ++++++++++++++++++++--
>  qemu/hw/pc.c    |   13 ++++++++++++-
>  qemu/qemu-kvm.c |   18 ++++++++++--------
>  qemu/vl.h       |    2 +-
>  user/kvmctl.c   |   25 +++++++++++++++++++++++--
>  user/kvmctl.h   |    5 +++--
>  user/main.c     |    3 +--
>  7 files changed, 70 insertions(+), 18 deletions(-)
>
> diff --git a/qemu/hw/apic.c b/qemu/hw/apic.c
> index 5704224..8aa3ef1 100644
> --- a/qemu/hw/apic.c
> +++ b/qemu/hw/apic.c
> @@ -19,6 +19,11 @@
>   */
>  #include "vl.h"
>  
> +#ifdef USE_KVM
> +#include "kvmctl.h"
> +extern kvm_context_t kvm_context;
> +#endif
>   

extern's are usually a red-herring that something isn't right.

>  //#define DEBUG_APIC
>  //#define DEBUG_IOAPIC
>  
> @@ -87,6 +92,7 @@ typedef struct APICState {
>  } APICState;
>  
>  struct IOAPICState {
> +    CPUState *cpu_env;
>      uint8_t id;
>      uint8_t ioregsel;
>  
> @@ -895,10 +901,21 @@ static void ioapic_service(IOAPICState *s)
>                      vector = pic_read_irq(isa_pic);
>                  else
>                      vector = entry & 0xff;
> -                
> +             
> +#ifdef USE_KVM
> +		if (kvm_allowed) {
> +		    kvm_apic_bus_deliver(kvm_context, dest, trig_mode,
> +					 dest_mode, delivery_mode, vector);
>   

Perhaps a qemu-kvm.c wrapper that doesn't take a kvm_context is needed 
here.  That would eliminate the extern.

> +		    cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD);
> +		} else {
> +#endif
>                  apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode);
>                  apic_bus_deliver(deliver_bitmask, delivery_mode, 
>                                   vector, polarity, trig_mode);
> +#ifdef USE_KVM
> +		}
> +#endif
> +
>              }
>          }
>      }
> @@ -1052,7 +1069,7 @@ static CPUWriteMemoryFunc *ioapic_mem_write[3] = {
>      ioapic_mem_writel,
>  };
>  
> -IOAPICState *ioapic_init(void)
> +IOAPICState *ioapic_init(CPUState *env)
>  {
>      IOAPICState *s;
>      int io_memory;
> @@ -1061,6 +1078,7 @@ IOAPICState *ioapic_init(void)
>      if (!s)
>          return NULL;
>      ioapic_reset(s);
> +    s->cpu_env = env;
>      s->id = last_apic_id++;
>  
>      io_memory = cpu_register_io_memory(0, ioapic_mem_read, 
> diff --git a/qemu/hw/pc.c b/qemu/hw/pc.c
> index eda49cf..3862a8f 100644
> --- a/qemu/hw/pc.c
> +++ b/qemu/hw/pc.c
> @@ -91,6 +91,9 @@ int cpu_get_pic_interrupt(CPUState *env)
>  {
>      int intno;
>  
> +#ifdef USE_KVM
> +    if (!kvm_allowed) {
> +#endif
>      intno = apic_get_interrupt(env);
>      if (intno >= 0) {
>   

Indent the inner block please :-)

>          /* set irq request if a PIC irq is still pending */
> @@ -98,10 +101,15 @@ int cpu_get_pic_interrupt(CPUState *env)
>          pic_update_irq(isa_pic); 
>          return intno;
>      }
> +
>      /* read the irq from the PIC */
>      if (!apic_accept_pic_intr(env))
>          return -1;
>  
> +#ifdef USE_KVM
> +    }
> +#endif
> +
>      intno = pic_read_irq(isa_pic);
>      return intno;
>  }
> @@ -483,6 +491,9 @@ static void pc_init1(int ram_size, int vga_ram_size, int boot_device,
>          }
>          register_savevm("cpu", i, 4, cpu_save, cpu_load, env);
>          qemu_register_reset(main_cpu_reset, env);
> +#ifdef USE_KVM
> +	if (!kvm_allowed)
> +#endif
>          if (pci_enabled) {
>              apic_init(env);
>          }
>   

Same here.  Perhaps we should always define kvm_allowed and set it to 
zero when !defined(USE_KVM).  We can probably just #if stuff out that's 
performance critical.

>  
> -    sregs.apic_base = cpu_get_apic_base(env);
> +    /* These two are no longer used once the in-kernel APIC is enabled */
> +    sregs.apic_base = 0;
> +    sregs.cr8 = 0;
> +
>      sregs.efer = env->efer;
> -    sregs.cr8 = cpu_get_apic_tpr(env);
>  
>      kvm_set_sregs(kvm_context, 0, &sregs);
>  
> @@ -321,7 +323,7 @@ static void save_regs(CPUState *env)
>      env->cr[3] = sregs.cr3;
>      env->cr[4] = sregs.cr4;
>  
> -    cpu_set_apic_base(env, sregs.apic_base);
> +    //cpu_set_apic_base(env, sregs.apic_base);

I take it this code cannot support an in-QEMU APIC?  So it breaks -no-kvm?

Regards,

Anthony Liguori

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH 2/2] KVM: in-kernel-apic modification to QEMU

[Gregory Haskins]

Hehe..ya, your points are all valid.  Note that I never really intended this code to be reviewed quite yet.  Its included purely to enable testing of the kernel side.  I'll be the first to admit its a complete hack :).  I'll be working on polishing this up next.  Stay tuned.

-Greg

>>> On Thu, May 10, 2007 at 10:47 AM, in message <46433083.1080809-rdkfGonbjUSkNkDKm+mE6A@public.gmane.org>,
Anthony Liguori <anthony-rdkfGonbjUSkNkDKm+mE6A@public.gmane.org> wrote: 
> Gregory Haskins wrote:
>> Signed- off- by: Gregory Haskins <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>
>> ---
>>
>>  qemu/hw/apic.c  |   22 ++++++++++++++++++++--
>>  qemu/hw/pc.c    |   13 ++++++++++++-
>>  qemu/qemu- kvm.c |   18 ++++++++++--------
>>  qemu/vl.h       |    2 +-
>>  user/kvmctl.c   |   25 +++++++++++++++++++++++--
>>  user/kvmctl.h   |    5 +++--
>>  user/main.c     |    3 +--
>>  7 files changed, 70 insertions(+), 18 deletions(- )
>>
>> diff -- git a/qemu/hw/apic.c b/qemu/hw/apic.c
>> index 5704224..8aa3ef1 100644
>> ---  a/qemu/hw/apic.c
>> +++ b/qemu/hw/apic.c
>> @@ - 19,6 +19,11 @@
>>   */
>>  #include "vl.h"
>>  
>> +#ifdef USE_KVM
>> +#include "kvmctl.h"
>> +extern kvm_context_t kvm_context;
>> +#endif
>>   
> 
> extern's are usually a red- herring that something isn't right.
> 
>>  //#define DEBUG_APIC
>>  //#define DEBUG_IOAPIC
>>  
>> @@ - 87,6 +92,7 @@ typedef struct APICState {
>>  } APICState;
>>  
>>  struct IOAPICState {
>> +    CPUState *cpu_env;
>>      uint8_t id;
>>      uint8_t ioregsel;
>>  
>> @@ - 895,10 +901,21 @@ static void ioapic_service(IOAPICState *s)
>>                      vector = pic_read_irq(isa_pic);
>>                  else
>>                      vector = entry & 0xff;
>> -                 
>> +             
>> +#ifdef USE_KVM
>> +		if (kvm_allowed) {
>> +		    kvm_apic_bus_deliver(kvm_context, dest, trig_mode,
>> +					 dest_mode, delivery_mode, vector);
>>   
> 
> Perhaps a qemu- kvm.c wrapper that doesn't take a kvm_context is needed 
> here.  That would eliminate the extern.
> 
>> +		    cpu_interrupt(s- >cpu_env, CPU_INTERRUPT_HARD);
>> +		} else {
>> +#endif
>>                  apic_get_delivery_bitmask(deliver_bitmask, dest, 
> dest_mode);
>>                  apic_bus_deliver(deliver_bitmask, delivery_mode, 
>>                                   vector, polarity, trig_mode);
>> +#ifdef USE_KVM
>> +		}
>> +#endif
>> +
>>              }
>>          }
>>      }
>> @@ - 1052,7 +1069,7 @@ static CPUWriteMemoryFunc *ioapic_mem_write[3] = {
>>      ioapic_mem_writel,
>>  };
>>  
>> - IOAPICState *ioapic_init(void)
>> +IOAPICState *ioapic_init(CPUState *env)
>>  {
>>      IOAPICState *s;
>>      int io_memory;
>> @@ - 1061,6 +1078,7 @@ IOAPICState *ioapic_init(void)
>>      if (!s)
>>          return NULL;
>>      ioapic_reset(s);
>> +    s- >cpu_env = env;
>>      s- >id = last_apic_id++;
>>  
>>      io_memory = cpu_register_io_memory(0, ioapic_mem_read, 
>> diff -- git a/qemu/hw/pc.c b/qemu/hw/pc.c
>> index eda49cf..3862a8f 100644
>> ---  a/qemu/hw/pc.c
>> +++ b/qemu/hw/pc.c
>> @@ - 91,6 +91,9 @@ int cpu_get_pic_interrupt(CPUState *env)
>>  {
>>      int intno;
>>  
>> +#ifdef USE_KVM
>> +    if (!kvm_allowed) {
>> +#endif
>>      intno = apic_get_interrupt(env);
>>      if (intno >= 0) {
>>   
> 
> Indent the inner block please :- )
> 
>>          /* set irq request if a PIC irq is still pending */
>> @@ - 98,10 +101,15 @@ int cpu_get_pic_interrupt(CPUState *env)
>>          pic_update_irq(isa_pic); 
>>          return intno;
>>      }
>> +
>>      /* read the irq from the PIC */
>>      if (!apic_accept_pic_intr(env))
>>          return - 1;
>>  
>> +#ifdef USE_KVM
>> +    }
>> +#endif
>> +
>>      intno = pic_read_irq(isa_pic);
>>      return intno;
>>  }
>> @@ - 483,6 +491,9 @@ static void pc_init1(int ram_size, int vga_ram_size, int 
> boot_device,
>>          }
>>          register_savevm("cpu", i, 4, cpu_save, cpu_load, env);
>>          qemu_register_reset(main_cpu_reset, env);
>> +#ifdef USE_KVM
>> +	if (!kvm_allowed)
>> +#endif
>>          if (pci_enabled) {
>>              apic_init(env);
>>          }
>>   
> 
> Same here.  Perhaps we should always define kvm_allowed and set it to 
> zero when !defined(USE_KVM).  We can probably just #if stuff out that's 
> performance critical.
> 
>>  
>> -     sregs.apic_base = cpu_get_apic_base(env);
>> +    /* These two are no longer used once the in- kernel APIC is enabled */
>> +    sregs.apic_base = 0;
>> +    sregs.cr8 = 0;
>> +
>>      sregs.efer = env- >efer;
>> -     sregs.cr8 = cpu_get_apic_tpr(env);
>>  
>>      kvm_set_sregs(kvm_context, 0, &sregs);
>>  
>> @@ - 321,7 +323,7 @@ static void save_regs(CPUState *env)
>>      env- >cr[3] = sregs.cr3;
>>      env- >cr[4] = sregs.cr4;
>>  
>> -     cpu_set_apic_base(env, sregs.apic_base);
>> +    //cpu_set_apic_base(env, sregs.apic_base);
> 
> I take it this code cannot support an in- QEMU APIC?  So it breaks - no- kvm?
> 
> Regards,
> 
> Anthony Liguori



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH 2/2] KVM: in-kernel-apic modification to QEMU

[Avi Kivity]

Anthony Liguori wrote:
>>      int intno;
>>  
>> +#ifdef USE_KVM
>> +    if (!kvm_allowed) {
>> +#endif
>>      intno = apic_get_interrupt(env);
>>      if (intno >= 0) {
>>   
>>     
>
> Indent the inner block please :-)
>
>   

Indenting the inner code would cause merging to be very difficult, 
should qemu change something there.  Maybe you can reverse the sense of 
the test and add the code after th #endif instead.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/