[Qemu-devel] [RFC] in-kernel irqchip : split devices

[Qemu-devel] [RFC] in-kernel irqchip : split devices

[Glauber Costa]

[-- Attachment #1: Type: text/plain, Size: 401 bytes --]

Hello people,

As I promised, I am sending a very brief PoC wrt split devices and in-kernel irqchip.
In this mail, I am including only the ioapic version for apreciation. I also have i8259,
and apic will take me a little bit more. This is just to try to bind the discussion to real
code.

Note that we end up with a very slim representation of the device, and the code is much less
confusing, IMHO.



[-- Attachment #2: ioapic-kvm.patch --]
[-- Type: text/plain, Size: 4365 bytes --]

Index: qemu/Makefile.target
===================================================================
--- qemu.orig/Makefile.target
+++ qemu/Makefile.target
@@ -197,6 +197,8 @@ obj-i386-y += usb-uhci.o vmmouse.o vmpor
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += ne2000-isa.o
 
+obj-i386-$(CONFIG_KVM) += ioapic-kvm.o
+
 # shared objects
 obj-ppc-y = ppc.o ide/core.o ide/qdev.o ide/isa.o ide/pci.o ide/macio.o
 obj-ppc-y += vga.o vga-pci.o $(sound-obj-y) dma.o openpic.o
Index: qemu/hw/ioapic-kvm.c
===================================================================
--- /dev/null
+++ qemu/hw/ioapic-kvm.c
@@ -0,0 +1,81 @@
+#include "hw.h"
+#include "pc.h"
+#include "qemu-timer.h"
+#include "host-utils.h"
+#include "kvm.h"
+
+#define IOAPIC_NUM_PINS			0x18
+#define IOAPIC_DEFAULT_BASE_ADDRESS  0xfec00000
+
+static void ioapic_reset(void *opaque)
+{
+    struct kvm_ioapic_state *s = opaque;
+    struct kvm_irqchip *chip;
+    int i;
+
+    chip = container_of(s, struct kvm_irqchip, chip.ioapic);
+
+    chip->chip_id = KVM_IRQCHIP_IOAPIC;
+
+    memset(s, 0, sizeof(*s));
+    s->base_address = IOAPIC_DEFAULT_BASE_ADDRESS;
+    for(i = 0; i < IOAPIC_NUM_PINS; i++)
+        s->redirtbl[i].bits = 1 << 16; /* mask LVT */
+
+    kvm_set_irqchip(chip);
+}
+
+static void ioapic_pre_save(void *opaque)
+{
+    struct kvm_ioapic_state *s = opaque;
+    struct kvm_irqchip *chip;
+
+    chip = container_of(s, struct kvm_irqchip, chip.ioapic);
+
+    kvm_get_irqchip(chip);
+}
+
+static int ioapic_post_load(void *opaque, int version_id)
+{
+    struct kvm_ioapic_state *s = opaque;
+    struct kvm_irqchip *chip;
+
+    chip = container_of(s, struct kvm_irqchip, chip.ioapic);
+
+    return kvm_set_irqchip(chip);
+}
+
+static const VMStateDescription vmstate_kvm_ioapic = {
+    .name = "ioapic-kvm",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load = ioapic_post_load,
+    .pre_save = ioapic_pre_save,
+    .fields      = (VMStateField []) {
+        VMSTATE_U64(base_address, struct kvm_ioapic_state),
+        VMSTATE_UINT32(id, struct kvm_ioapic_state),
+        VMSTATE_UINT32(ioregsel, struct kvm_ioapic_state),
+        VMSTATE_UINT32(irr, struct kvm_ioapic_state),
+        VMSTATE_ARRAY_UNSAFE(redirtbl, struct kvm_ioapic_state, IOAPIC_NUM_PINS, 0, vmstate_info_u64, __u64),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+
+static void kvm_ioapic_set_irq(void *opaque, int vector, int level)
+{
+}
+
+qemu_irq *kvm_ioapic_init(void)
+{
+    struct kvm_irqchip *s;
+
+    s = qemu_mallocz(sizeof(*s));
+
+    ioapic_reset(&s->chip.ioapic);
+
+    vmstate_register(0, &vmstate_kvm_ioapic, &s->chip.ioapic);
+    qemu_register_reset(ioapic_reset, &s->chip.ioapic);
+
+    return qemu_allocate_irqs(kvm_ioapic_set_irq, &s->chip.ioapic, IOAPIC_NUM_PINS);
+}
Index: qemu/hw/pc.h
===================================================================
--- qemu.orig/hw/pc.h
+++ qemu/hw/pc.h
@@ -48,6 +48,8 @@ void ioapic_set_irq(void *opaque, int ve
 void apic_reset_irq_delivered(void);
 int apic_get_irq_delivered(void);
 
+qemu_irq *kvm_ioapic_init(void);
+
 /* i8254.c */
 
 #define PIT_FREQ 1193182
Index: qemu/kvm-all.c
===================================================================
--- qemu.orig/kvm-all.c
+++ qemu/kvm-all.c
@@ -411,6 +411,26 @@ int kvm_check_extension(KVMState *s, uns
     return ret;
 }
 
+#ifdef KVM_CAP_IRQCHIP
+int kvm_set_irqchip(struct kvm_irqchip *chip)
+{
+    if (!kvm_state->irqchip_in_kernel) {
+        return 0;
+    }
+
+    return kvm_vm_ioctl(kvm_state, KVM_SET_IRQCHIP, chip);
+}
+
+int kvm_get_irqchip(struct kvm_irqchip *chip)
+{
+    if (!kvm_state->irqchip_in_kernel) {
+        return 0;
+    }
+
+    return kvm_vm_ioctl(kvm_state, KVM_GET_IRQCHIP, chip);
+}
+#endif
+
 int kvm_init(int smp_cpus)
 {
     static const char upgrade_note[] =
Index: qemu/kvm.h
===================================================================
--- qemu.orig/kvm.h
+++ qemu/kvm.h
@@ -16,6 +16,7 @@
 
 #include "config.h"
 #include "qemu-queue.h"
+#include <linux/kvm.h>
 
 #ifdef CONFIG_KVM
 extern int kvm_allowed;
@@ -63,6 +64,9 @@ int kvm_update_guest_debug(CPUState *env
 int kvm_pit_in_kernel(void);
 int kvm_irqchip_in_kernel(void);
 
+int kvm_set_irqchip(struct kvm_irqchip *chip);
+int kvm_get_irqchip(struct kvm_irqchip *chip);
+
 /* internal API */
 
 struct KVMState;

Re: [Qemu-devel] [RFC] in-kernel irqchip : split devices

[Glauber Costa]

Specially Avi,

comments on this one?

2009/10/14 Glauber Costa <glommer@redhat.com>:
> Hello people,
>
> As I promised, I am sending a very brief PoC wrt split devices and in-kernel irqchip.
> In this mail, I am including only the ioapic version for apreciation. I also have i8259,
> and apic will take me a little bit more. This is just to try to bind the discussion to real
> code.
>
> Note that we end up with a very slim representation of the device, and the code is much less
> confusing, IMHO.
>
>
>
> Index: qemu/Makefile.target
> ===================================================================
> --- qemu.orig/Makefile.target
> +++ qemu/Makefile.target
> @@ -197,6 +197,8 @@ obj-i386-y += usb-uhci.o vmmouse.o vmpor
>  obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>  obj-i386-y += ne2000-isa.o
>
> +obj-i386-$(CONFIG_KVM) += ioapic-kvm.o
> +
>  # shared objects
>  obj-ppc-y = ppc.o ide/core.o ide/qdev.o ide/isa.o ide/pci.o ide/macio.o
>  obj-ppc-y += vga.o vga-pci.o $(sound-obj-y) dma.o openpic.o
> Index: qemu/hw/ioapic-kvm.c
> ===================================================================
> --- /dev/null
> +++ qemu/hw/ioapic-kvm.c
> @@ -0,0 +1,81 @@
> +#include "hw.h"
> +#include "pc.h"
> +#include "qemu-timer.h"
> +#include "host-utils.h"
> +#include "kvm.h"
> +
> +#define IOAPIC_NUM_PINS                        0x18
> +#define IOAPIC_DEFAULT_BASE_ADDRESS  0xfec00000
> +
> +static void ioapic_reset(void *opaque)
> +{
> +    struct kvm_ioapic_state *s = opaque;
> +    struct kvm_irqchip *chip;
> +    int i;
> +
> +    chip = container_of(s, struct kvm_irqchip, chip.ioapic);
> +
> +    chip->chip_id = KVM_IRQCHIP_IOAPIC;
> +
> +    memset(s, 0, sizeof(*s));
> +    s->base_address = IOAPIC_DEFAULT_BASE_ADDRESS;
> +    for(i = 0; i < IOAPIC_NUM_PINS; i++)
> +        s->redirtbl[i].bits = 1 << 16; /* mask LVT */
> +
> +    kvm_set_irqchip(chip);
> +}
> +
> +static void ioapic_pre_save(void *opaque)
> +{
> +    struct kvm_ioapic_state *s = opaque;
> +    struct kvm_irqchip *chip;
> +
> +    chip = container_of(s, struct kvm_irqchip, chip.ioapic);
> +
> +    kvm_get_irqchip(chip);
> +}
> +
> +static int ioapic_post_load(void *opaque, int version_id)
> +{
> +    struct kvm_ioapic_state *s = opaque;
> +    struct kvm_irqchip *chip;
> +
> +    chip = container_of(s, struct kvm_irqchip, chip.ioapic);
> +
> +    return kvm_set_irqchip(chip);
> +}
> +
> +static const VMStateDescription vmstate_kvm_ioapic = {
> +    .name = "ioapic-kvm",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .post_load = ioapic_post_load,
> +    .pre_save = ioapic_pre_save,
> +    .fields      = (VMStateField []) {
> +        VMSTATE_U64(base_address, struct kvm_ioapic_state),
> +        VMSTATE_UINT32(id, struct kvm_ioapic_state),
> +        VMSTATE_UINT32(ioregsel, struct kvm_ioapic_state),
> +        VMSTATE_UINT32(irr, struct kvm_ioapic_state),
> +        VMSTATE_ARRAY_UNSAFE(redirtbl, struct kvm_ioapic_state, IOAPIC_NUM_PINS, 0, vmstate_info_u64, __u64),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +
> +static void kvm_ioapic_set_irq(void *opaque, int vector, int level)
> +{
> +}
> +
> +qemu_irq *kvm_ioapic_init(void)
> +{
> +    struct kvm_irqchip *s;
> +
> +    s = qemu_mallocz(sizeof(*s));
> +
> +    ioapic_reset(&s->chip.ioapic);
> +
> +    vmstate_register(0, &vmstate_kvm_ioapic, &s->chip.ioapic);
> +    qemu_register_reset(ioapic_reset, &s->chip.ioapic);
> +
> +    return qemu_allocate_irqs(kvm_ioapic_set_irq, &s->chip.ioapic, IOAPIC_NUM_PINS);
> +}
> Index: qemu/hw/pc.h
> ===================================================================
> --- qemu.orig/hw/pc.h
> +++ qemu/hw/pc.h
> @@ -48,6 +48,8 @@ void ioapic_set_irq(void *opaque, int ve
>  void apic_reset_irq_delivered(void);
>  int apic_get_irq_delivered(void);
>
> +qemu_irq *kvm_ioapic_init(void);
> +
>  /* i8254.c */
>
>  #define PIT_FREQ 1193182
> Index: qemu/kvm-all.c
> ===================================================================
> --- qemu.orig/kvm-all.c
> +++ qemu/kvm-all.c
> @@ -411,6 +411,26 @@ int kvm_check_extension(KVMState *s, uns
>     return ret;
>  }
>
> +#ifdef KVM_CAP_IRQCHIP
> +int kvm_set_irqchip(struct kvm_irqchip *chip)
> +{
> +    if (!kvm_state->irqchip_in_kernel) {
> +        return 0;
> +    }
> +
> +    return kvm_vm_ioctl(kvm_state, KVM_SET_IRQCHIP, chip);
> +}
> +
> +int kvm_get_irqchip(struct kvm_irqchip *chip)
> +{
> +    if (!kvm_state->irqchip_in_kernel) {
> +        return 0;
> +    }
> +
> +    return kvm_vm_ioctl(kvm_state, KVM_GET_IRQCHIP, chip);
> +}
> +#endif
> +
>  int kvm_init(int smp_cpus)
>  {
>     static const char upgrade_note[] =
> Index: qemu/kvm.h
> ===================================================================
> --- qemu.orig/kvm.h
> +++ qemu/kvm.h
> @@ -16,6 +16,7 @@
>
>  #include "config.h"
>  #include "qemu-queue.h"
> +#include <linux/kvm.h>
>
>  #ifdef CONFIG_KVM
>  extern int kvm_allowed;
> @@ -63,6 +64,9 @@ int kvm_update_guest_debug(CPUState *env
>  int kvm_pit_in_kernel(void);
>  int kvm_irqchip_in_kernel(void);
>
> +int kvm_set_irqchip(struct kvm_irqchip *chip);
> +int kvm_get_irqchip(struct kvm_irqchip *chip);
> +
>  /* internal API */
>
>  struct KVMState;
>
>



-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

Re: [Qemu-devel] [RFC] in-kernel irqchip : split devices

[Marcelo Tosatti]

On Wed, Oct 14, 2009 at 11:30:43AM -0300, Glauber Costa wrote:
> Hello people,
> 
> As I promised, I am sending a very brief PoC wrt split devices and in-kernel irqchip.
> In this mail, I am including only the ioapic version for apreciation. I also have i8259,
> and apic will take me a little bit more. This is just to try to bind the discussion to real
> code.
> 
> Note that we end up with a very slim representation of the device, and the code is much less
> confusing, IMHO.

Looks good to me.

> Index: qemu/Makefile.target
> ===================================================================
> --- qemu.orig/Makefile.target
> +++ qemu/Makefile.target
> @@ -197,6 +197,8 @@ obj-i386-y += usb-uhci.o vmmouse.o vmpor
>  obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>  obj-i386-y += ne2000-isa.o
>  
> +obj-i386-$(CONFIG_KVM) += ioapic-kvm.o
> +
>  # shared objects
>  obj-ppc-y = ppc.o ide/core.o ide/qdev.o ide/isa.o ide/pci.o ide/macio.o
>  obj-ppc-y += vga.o vga-pci.o $(sound-obj-y) dma.o openpic.o
> Index: qemu/hw/ioapic-kvm.c
> ===================================================================
> --- /dev/null
> +++ qemu/hw/ioapic-kvm.c
> @@ -0,0 +1,81 @@
> +#include "hw.h"
> +#include "pc.h"
> +#include "qemu-timer.h"
> +#include "host-utils.h"
> +#include "kvm.h"
> +
> +#define IOAPIC_NUM_PINS			0x18
> +#define IOAPIC_DEFAULT_BASE_ADDRESS  0xfec00000
> +
> +static void ioapic_reset(void *opaque)
> +{
> +    struct kvm_ioapic_state *s = opaque;
> +    struct kvm_irqchip *chip;
> +    int i;
> +
> +    chip = container_of(s, struct kvm_irqchip, chip.ioapic);
> +
> +    chip->chip_id = KVM_IRQCHIP_IOAPIC;
> +
> +    memset(s, 0, sizeof(*s));
> +    s->base_address = IOAPIC_DEFAULT_BASE_ADDRESS;
> +    for(i = 0; i < IOAPIC_NUM_PINS; i++)
> +        s->redirtbl[i].bits = 1 << 16; /* mask LVT */
> +
> +    kvm_set_irqchip(chip);
> +}
> +
> +static void ioapic_pre_save(void *opaque)
> +{
> +    struct kvm_ioapic_state *s = opaque;
> +    struct kvm_irqchip *chip;
> +
> +    chip = container_of(s, struct kvm_irqchip, chip.ioapic);
> +
> +    kvm_get_irqchip(chip);
> +}
> +
> +static int ioapic_post_load(void *opaque, int version_id)
> +{
> +    struct kvm_ioapic_state *s = opaque;
> +    struct kvm_irqchip *chip;
> +
> +    chip = container_of(s, struct kvm_irqchip, chip.ioapic);
> +
> +    return kvm_set_irqchip(chip);
> +}
> +
> +static const VMStateDescription vmstate_kvm_ioapic = {
> +    .name = "ioapic-kvm",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .post_load = ioapic_post_load,
> +    .pre_save = ioapic_pre_save,
> +    .fields      = (VMStateField []) {
> +        VMSTATE_U64(base_address, struct kvm_ioapic_state),
> +        VMSTATE_UINT32(id, struct kvm_ioapic_state),
> +        VMSTATE_UINT32(ioregsel, struct kvm_ioapic_state),
> +        VMSTATE_UINT32(irr, struct kvm_ioapic_state),
> +        VMSTATE_ARRAY_UNSAFE(redirtbl, struct kvm_ioapic_state, IOAPIC_NUM_PINS, 0, vmstate_info_u64, __u64),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +
> +static void kvm_ioapic_set_irq(void *opaque, int vector, int level)
> +{
> +}
> +
> +qemu_irq *kvm_ioapic_init(void)
> +{
> +    struct kvm_irqchip *s;
> +
> +    s = qemu_mallocz(sizeof(*s));
> +
> +    ioapic_reset(&s->chip.ioapic);
> +
> +    vmstate_register(0, &vmstate_kvm_ioapic, &s->chip.ioapic);
> +    qemu_register_reset(ioapic_reset, &s->chip.ioapic);
> +
> +    return qemu_allocate_irqs(kvm_ioapic_set_irq, &s->chip.ioapic, IOAPIC_NUM_PINS);
> +}
> Index: qemu/hw/pc.h
> ===================================================================
> --- qemu.orig/hw/pc.h
> +++ qemu/hw/pc.h
> @@ -48,6 +48,8 @@ void ioapic_set_irq(void *opaque, int ve
>  void apic_reset_irq_delivered(void);
>  int apic_get_irq_delivered(void);
>  
> +qemu_irq *kvm_ioapic_init(void);
> +
>  /* i8254.c */
>  
>  #define PIT_FREQ 1193182
> Index: qemu/kvm-all.c
> ===================================================================
> --- qemu.orig/kvm-all.c
> +++ qemu/kvm-all.c
> @@ -411,6 +411,26 @@ int kvm_check_extension(KVMState *s, uns
>      return ret;
>  }
>  
> +#ifdef KVM_CAP_IRQCHIP
> +int kvm_set_irqchip(struct kvm_irqchip *chip)
> +{
> +    if (!kvm_state->irqchip_in_kernel) {
> +        return 0;
> +    }
> +
> +    return kvm_vm_ioctl(kvm_state, KVM_SET_IRQCHIP, chip);
> +}
> +
> +int kvm_get_irqchip(struct kvm_irqchip *chip)
> +{
> +    if (!kvm_state->irqchip_in_kernel) {
> +        return 0;
> +    }
> +
> +    return kvm_vm_ioctl(kvm_state, KVM_GET_IRQCHIP, chip);
> +}
> +#endif
> +
>  int kvm_init(int smp_cpus)
>  {
>      static const char upgrade_note[] =
> Index: qemu/kvm.h
> ===================================================================
> --- qemu.orig/kvm.h
> +++ qemu/kvm.h
> @@ -16,6 +16,7 @@
>  
>  #include "config.h"
>  #include "qemu-queue.h"
> +#include <linux/kvm.h>
>  
>  #ifdef CONFIG_KVM
>  extern int kvm_allowed;
> @@ -63,6 +64,9 @@ int kvm_update_guest_debug(CPUState *env
>  int kvm_pit_in_kernel(void);
>  int kvm_irqchip_in_kernel(void);
>  
> +int kvm_set_irqchip(struct kvm_irqchip *chip);
> +int kvm_get_irqchip(struct kvm_irqchip *chip);
> +
>  /* internal API */
>  
>  struct KVMState;

Re: [Qemu-devel] [RFC] in-kernel irqchip : split devices

[Avi Kivity]

On 10/14/2009 04:30 PM, Glauber Costa wrote:
> Hello people,
>
> As I promised, I am sending a very brief PoC wrt split devices and in-kernel irqchip.
> In this mail, I am including only the ioapic version for apreciation. I also have i8259,
> and apic will take me a little bit more. This is just to try to bind the discussion to real
> code.
>
>    

I still can't say I like it.  The reset function is duplicated, the 
state representation (which is an ABI) is gratuitously forked.

You can't save/restore in-kernel irqchip and userspace irqchip, even 
though where the code is located is an implementation detail.  While we 
may not care much for the ioapic, it sets a bad precedent for vhost-net, 
where we'd like to migrate from non-vhost-net hosts to vhost-net hosts 
without the user noticing anything.

> Note that we end up with a very slim representation of the device, and the code is much less
> confusing, IMHO.
>    

You can always remove if statements by duplicating the code and pushing 
the if one level upwards.  In total, there is more code, and it is more 
confusing (since you need to deal with implementation details at a 
higher level).

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

Re: [Qemu-devel] [RFC] in-kernel irqchip : split devices

[Glauber Costa]

On Sun, Oct 25, 2009 at 12:26:51PM +0200, Avi Kivity wrote:
> On 10/14/2009 04:30 PM, Glauber Costa wrote:
>> Hello people,
>>
>> As I promised, I am sending a very brief PoC wrt split devices and in-kernel irqchip.
>> In this mail, I am including only the ioapic version for apreciation. I also have i8259,
>> and apic will take me a little bit more. This is just to try to bind the discussion to real
>> code.
>>
>>    
>
> I still can't say I like it.  The reset function is duplicated, the  
> state representation (which is an ABI) is gratuitously forked.
>
> You can't save/restore in-kernel irqchip and userspace irqchip, even  
> though where the code is located is an implementation detail.  While we  
> may not care much for the ioapic, it sets a bad precedent for vhost-net,  
> where we'd like to migrate from non-vhost-net hosts to vhost-net hosts  
> without the user noticing anything.
>
>> Note that we end up with a very slim representation of the device, and the code is much less
>> confusing, IMHO.
>>    
>
> You can always remove if statements by duplicating the code and pushing  
> the if one level upwards.  In total, there is more code, and it is more  
> confusing (since you need to deal with implementation details at a  
> higher level).
>

It pretty much depends on your definition of confusing.
Larger? yes, probably. It has a separate file, and doesn't matter how hard
we fight duplicates, some will persist.

But just the other day, I was on IRC with anthony, trying to draw some
conclusions about the behaviour of our ioapic. We went through it,
and it took us quite a while to determine what pieces of code were being
used, and what were not. This is pretty much what I mean by confusing.

With the approach we are proposing, things get much more straightforward
> -- 
> error compiling committee.c: too many arguments to function
>

Re: [Qemu-devel] [RFC] in-kernel irqchip : split devices

[Anthony Liguori]

Avi Kivity wrote:
> On 10/14/2009 04:30 PM, Glauber Costa wrote:
>> Hello people,
>>
>> As I promised, I am sending a very brief PoC wrt split devices and 
>> in-kernel irqchip.
>> In this mail, I am including only the ioapic version for apreciation. 
>> I also have i8259,
>> and apic will take me a little bit more. This is just to try to bind 
>> the discussion to real
>> code.
>>
>>    
>
> I still can't say I like it.  The reset function is duplicated, the 
> state representation (which is an ABI) is gratuitously forked.
>
> You can't save/restore in-kernel irqchip and userspace irqchip, even 
> though where the code is located is an implementation detail.  While 
> we may not care much for the ioapic, it sets a bad precedent for 
> vhost-net, where we'd like to migrate from non-vhost-net hosts to 
> vhost-net hosts without the user noticing anything.
>
>> Note that we end up with a very slim representation of the device, 
>> and the code is much less
>> confusing, IMHO.
>>    
>
> You can always remove if statements by duplicating the code and 
> pushing the if one level upwards.  In total, there is more code, and 
> it is more confusing (since you need to deal with implementation 
> details at a higher level).

I'm surprised you feel this way.  Maybe this is an issue of having the 
model in your head vs. not having it because the current in-kernel code 
is extremely confusing IMHO.

When you look at ioapic.c in qemu-kvm, the first question I ask is, 
"what parts of this code is used when using in-kernel apic?".  The 
answer is not at all obvious.

To understand it, you have to first search for kvm_enabled() and you'll 
see that during save/restore the state is synced with the in-kernel 
state.  However, it's not clear whether pio/mmio operations still get 
processed and certainly not clear whether ioapic_set_irq() is not called 
anymore.

In fact, I think you to start with the assumption that it is which leads 
you to wonder why it doesn't do kvm_set_irq().

The answers are all subtle and have to do with weird things about how 
the isa irqs are allocated.  It's extremely confusing to someone who 
doesn't know exactly what's going on.

OTOH, the split model makes this all very obvious.  Sure there's some 
duplication but at the end of the day, you don't have to understand very 
much to see what's going on.  We just use userspace for device 
save/restore and reset support.  Code readability wins in my mind over 
reducing a couple dozen lines of code.

Regards,

Anthony Liguori