All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] uq/master: irqchip-in-kernel support
@ 2010-02-26 20:12 Glauber Costa
  2010-02-26 20:12 ` [PATCH 01/10] introduce VMSTATE_U64 Glauber Costa
                   ` (2 more replies)
  0 siblings, 3 replies; 31+ messages in thread
From: Glauber Costa @ 2010-02-26 20:12 UTC (permalink / raw)
  To: kvm; +Cc: mtosatti

Hi guys,

This is the same in-kernel irqchip support already posted to qemu-devel,
just rebased, retested, etc. It passes my basic tests, so it seem to be
still in good shape.

It is provided against uq/master as part of the integration efforts

thanks.

Glauber Costa (10):
  introduce VMSTATE_U64
  Provide ioapic-kvm
  provide apic_set_irq_delivered
  provide i8259-kvm
  Don't call apic functions directly from kvm code
  export kvm_put_mp_state
  provide apic-kvm
  Add -kvm option
  Initialize in-kernel irqchip
  Do GSI routing

 Makefile.target   |    2 +
 hw/apic-kvm.c     |  157 ++++++++++++++++++++++++++++++++++++++++
 hw/apic.c         |    5 ++
 hw/hw.h           |   24 ++++++
 hw/i8259-kvm.c    |  112 +++++++++++++++++++++++++++++
 hw/ioapic-kvm.c   |   89 +++++++++++++++++++++++
 hw/pc.c           |   21 +++++-
 hw/pc.h           |    6 ++
 kvm-all.c         |   51 +++++++++++++-
 kvm.h             |   17 +++++
 qemu-config.c     |   16 ++++
 qemu-config.h     |    1 +
 qemu-options.hx   |   11 +++-
 savevm.c          |   23 ++++++
 target-i386/cpu.h |    9 +++
 target-i386/kvm.c |  204 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 target-ppc/kvm.c  |    5 ++
 vl.c              |   11 +++
 18 files changed, 751 insertions(+), 13 deletions(-)
 create mode 100644 hw/apic-kvm.c
 create mode 100644 hw/i8259-kvm.c
 create mode 100644 hw/ioapic-kvm.c


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 01/10] introduce VMSTATE_U64
  2010-02-26 20:12 [PATCH 00/10] uq/master: irqchip-in-kernel support Glauber Costa
@ 2010-02-26 20:12 ` Glauber Costa
  2010-02-26 20:12   ` [PATCH 02/10] Provide ioapic-kvm Glauber Costa
  2010-02-27 10:28 ` [PATCH 00/10] uq/master: irqchip-in-kernel support Jan Kiszka
  2010-03-04 16:33 ` Jan Kiszka
  2 siblings, 1 reply; 31+ messages in thread
From: Glauber Costa @ 2010-02-26 20:12 UTC (permalink / raw)
  To: kvm; +Cc: mtosatti

Slightly modified version of a patch already included in qemu-kvm:

This is a patch actually written by Juan, which, according to him,
he plans on posting to qemu.git. Problem is that linux defines
u64 in a way that is type-uncompatible with uint64_t.

I am including it here, because it is a dependency to my patch series
that follows.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 hw/hw.h  |   24 ++++++++++++++++++++++++
 savevm.c |   23 +++++++++++++++++++++++
 2 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index 7b500f4..9e86b29 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -342,6 +342,11 @@ extern const VMStateInfo vmstate_info_uint16;
 extern const VMStateInfo vmstate_info_uint32;
 extern const VMStateInfo vmstate_info_uint64;
 
+#ifdef __linux__
+#include <linux/types.h>
+extern const VMStateInfo vmstate_info_u64;
+#endif
+
 extern const VMStateInfo vmstate_info_timer;
 extern const VMStateInfo vmstate_info_ptimer;
 extern const VMStateInfo vmstate_info_buffer;
@@ -398,6 +403,16 @@ extern const VMStateInfo vmstate_info_unused_buffer;
     .offset     = vmstate_offset_array(_state, _field, _type, _num), \
 }
 
+#define VMSTATE_ARRAY_UNSAFE(_field, _state, _num, _version, _info, _type) {\
+    .name       = (stringify(_field)),                               \
+    .version_id = (_version),                                        \
+    .num        = (_num),                                            \
+    .info       = &(_info),                                          \
+    .size       = sizeof(_type),                                     \
+    .flags      = VMS_ARRAY,                                         \
+    .offset     = offsetof(_state, _field)                           \
+}
+
 #define VMSTATE_ARRAY_TEST(_field, _state, _num, _test, _info, _type) {\
     .name         = (stringify(_field)),                              \
     .field_exists = (_test),                                          \
@@ -622,6 +637,15 @@ extern const VMStateDescription vmstate_i2c_slave;
 #define VMSTATE_UINT64(_f, _s)                                        \
     VMSTATE_UINT64_V(_f, _s, 0)
 
+/* This is needed because on linux __u64 is unsigned long long
+   and on glibc uint64_t is unsigned long on 64 bits */
+#ifdef __linux__
+#define VMSTATE_U64_V(_f, _s, _v)                                     \
+    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_u64, __u64)
+#define VMSTATE_U64(_f, _s)                                           \
+    VMSTATE_U64_V(_f, _s, 0)
+#endif
+
 #define VMSTATE_UINT8_EQUAL(_f, _s)                                   \
     VMSTATE_SINGLE(_f, _s, 0, vmstate_info_uint8_equal, uint8_t)
 
diff --git a/savevm.c b/savevm.c
index 4b58663..1deb49a 100644
--- a/savevm.c
+++ b/savevm.c
@@ -871,6 +871,29 @@ const VMStateInfo vmstate_info_uint64 = {
     .put  = put_uint64,
 };
 
+/* 64 bit linux kernel unsigned int */
+
+#ifdef __linux__
+static int get_u64(QEMUFile *f, void *pv, size_t size)
+{
+    __u64 *v = pv;
+    qemu_get_be64s(f, (uint64_t *)v);
+    return 0;
+}
+
+static void put_u64(QEMUFile *f, void *pv, size_t size)
+{
+    __u64 *v = pv;
+    qemu_put_be64s(f, (uint64_t *)v);
+}
+
+const VMStateInfo vmstate_info_u64 = {
+    .name = "__u64",
+    .get  = get_u64,
+    .put  = put_u64,
+};
+#endif /* __linux__ */
+
 /* 8 bit int. See that the received value is the same than the one
    in the field */
 
-- 
1.6.6


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 02/10] Provide ioapic-kvm
  2010-02-26 20:12 ` [PATCH 01/10] introduce VMSTATE_U64 Glauber Costa
@ 2010-02-26 20:12   ` Glauber Costa
  2010-02-26 20:12     ` [PATCH 03/10] provide apic_set_irq_delivered Glauber Costa
  2010-03-02  4:28     ` [PATCH 02/10] Provide ioapic-kvm Marcelo Tosatti
  0 siblings, 2 replies; 31+ messages in thread
From: Glauber Costa @ 2010-02-26 20:12 UTC (permalink / raw)
  To: kvm; +Cc: mtosatti

This patch provides the file ioapic-kvm.c, which implements a schim over
the kvm in-kernel IO APIC.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 Makefile.target |    2 +
 hw/ioapic-kvm.c |   89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/pc.c         |    7 ++++-
 hw/pc.h         |    2 +
 kvm-all.c       |   18 +++++++++++
 kvm.h           |    4 ++
 6 files changed, 121 insertions(+), 1 deletions(-)
 create mode 100644 hw/ioapic-kvm.c

diff --git a/Makefile.target b/Makefile.target
index 4c4d397..78aff3c 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -213,6 +213,8 @@ obj-i386-y += usb-uhci.o vmmouse.o vmport.o vmware_vga.o hpet.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += ne2000-isa.o debugcon.o multiboot.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 += ide/cmd646.o
diff --git a/hw/ioapic-kvm.c b/hw/ioapic-kvm.c
new file mode 100644
index 0000000..78e0984
--- /dev/null
+++ b/hw/ioapic-kvm.c
@@ -0,0 +1,89 @@
+#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 []) {
+        /* Linux does not define __u64 the same as uint64_t */
+        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)
+{
+/*
+    int pic_ret;
+
+    if (kvm_set_irq(vector, level, &pic_ret)) {
+        if (pic_ret != 0)
+            apic_set_irq_delivered();
+        return;
+    }
+*/
+}
+
+qemu_irq *kvm_ioapic_init(void)
+{
+    struct kvm_irqchip *s;
+
+    s = qemu_mallocz(sizeof(*s));
+
+    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);
+}
diff --git a/hw/pc.c b/hw/pc.c
index bdc297f..ce9e832 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -45,6 +45,7 @@
 #include "loader.h"
 #include "elf.h"
 #include "multiboot.h"
+#include "kvm.h"
 
 /* output Bochs bios info messages */
 //#define DEBUG_BIOS
@@ -949,7 +950,11 @@ static void pc_init1(ram_addr_t ram_size,
     register_ioport_write(0x92, 1, 1, ioport92_write, NULL);
 
     if (pci_enabled) {
-        isa_irq_state->ioapic = ioapic_init();
+        if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+            isa_irq_state->ioapic = kvm_ioapic_init();
+        } else {
+            isa_irq_state->ioapic = ioapic_init();
+        }
     }
     pit = pit_init(0x40, isa_reserve_irq(0));
     pcspk_init(pit);
diff --git a/hw/pc.h b/hw/pc.h
index 92f8563..8ccdf63 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -49,6 +49,8 @@ void ioapic_set_irq(void *opaque, int vector, int level);
 void apic_reset_irq_delivered(void);
 int apic_get_irq_delivered(void);
 
+qemu_irq *kvm_ioapic_init(void);
+
 /* i8254.c */
 
 #define PIT_FREQ 1193182
diff --git a/kvm-all.c b/kvm-all.c
index 1a02076..6417183 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -568,6 +568,24 @@ static CPUPhysMemoryClient kvm_cpu_phys_memory_client = {
 	.migration_log = kvm_client_migration_log,
 };
 
+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);
+}
+
 int kvm_init(int smp_cpus)
 {
     static const char upgrade_note[] =
diff --git a/kvm.h b/kvm.h
index a74dfcb..812856e 100644
--- a/kvm.h
+++ b/kvm.h
@@ -16,6 +16,7 @@
 
 #include "config.h"
 #include "qemu-queue.h"
+#include <linux/kvm.h>
 
 #ifdef CONFIG_KVM
 extern int kvm_allowed;
@@ -60,6 +61,9 @@ int kvm_set_signal_mask(CPUState *env, const sigset_t *sigset);
 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;
-- 
1.6.6


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 03/10] provide apic_set_irq_delivered
  2010-02-26 20:12   ` [PATCH 02/10] Provide ioapic-kvm Glauber Costa
@ 2010-02-26 20:12     ` Glauber Costa
  2010-02-26 20:12       ` [PATCH 04/10] provide i8259-kvm Glauber Costa
  2010-03-02  4:28     ` [PATCH 02/10] Provide ioapic-kvm Marcelo Tosatti
  1 sibling, 1 reply; 31+ messages in thread
From: Glauber Costa @ 2010-02-26 20:12 UTC (permalink / raw)
  To: kvm; +Cc: mtosatti

i8259 chip will use it, so provide it, and export it through pc.h

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 hw/apic.c |    5 +++++
 hw/pc.h   |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index 87e7dc0..482bb1e 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -388,6 +388,11 @@ void apic_reset_irq_delivered(void)
     apic_irq_delivered = 0;
 }
 
+void apic_set_irq_delivered(void)
+{
+    apic_irq_delivered = 1;
+}
+
 int apic_get_irq_delivered(void)
 {
     return apic_irq_delivered;
diff --git a/hw/pc.h b/hw/pc.h
index 8ccdf63..cad735a 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -47,6 +47,7 @@ int apic_get_interrupt(CPUState *env);
 qemu_irq *ioapic_init(void);
 void ioapic_set_irq(void *opaque, int vector, int level);
 void apic_reset_irq_delivered(void);
+void apic_set_irq_delivered(void);
 int apic_get_irq_delivered(void);
 
 qemu_irq *kvm_ioapic_init(void);
-- 
1.6.6


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 04/10] provide i8259-kvm
  2010-02-26 20:12     ` [PATCH 03/10] provide apic_set_irq_delivered Glauber Costa
@ 2010-02-26 20:12       ` Glauber Costa
  2010-02-26 20:12         ` [PATCH 05/10] Don't call apic functions directly from kvm code Glauber Costa
  0 siblings, 1 reply; 31+ messages in thread
From: Glauber Costa @ 2010-02-26 20:12 UTC (permalink / raw)
  To: kvm; +Cc: mtosatti

This patch provides the file i8259-kvm.c, which implements a schim over
the kvm in-kernel PIC.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 Makefile.target   |    2 +-
 hw/i8259-kvm.c    |  112 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/pc.c           |    8 +++-
 hw/pc.h           |    1 +
 kvm-all.c         |    5 ++
 kvm.h             |    4 ++
 target-i386/kvm.c |   24 +++++++++++
 target-ppc/kvm.c  |    5 ++
 8 files changed, 159 insertions(+), 2 deletions(-)
 create mode 100644 hw/i8259-kvm.c

diff --git a/Makefile.target b/Makefile.target
index 78aff3c..bc5263e 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -213,7 +213,7 @@ obj-i386-y += usb-uhci.o vmmouse.o vmport.o vmware_vga.o hpet.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += ne2000-isa.o debugcon.o multiboot.o
 
-obj-i386-$(CONFIG_KVM) += ioapic-kvm.o
+obj-i386-$(CONFIG_KVM) += ioapic-kvm.o i8259-kvm.o
 
 # shared objects
 obj-ppc-y = ppc.o ide/core.o ide/qdev.o ide/isa.o ide/pci.o ide/macio.o
diff --git a/hw/i8259-kvm.c b/hw/i8259-kvm.c
new file mode 100644
index 0000000..bd6387b
--- /dev/null
+++ b/hw/i8259-kvm.c
@@ -0,0 +1,112 @@
+#include "hw.h"
+#include "pc.h"
+#include "isa.h"
+#include "monitor.h"
+#include "qemu-timer.h"
+#include "kvm.h"
+
+static void kvm_i8259_set_irq(void *opaque, int irq, int level)
+{
+    int pic_ret;
+
+    if (kvm_set_irq(irq, level, &pic_ret)) {
+        if (pic_ret != 0)
+            /* In theory, we should not be using any apic state, but we need
+             * to warn devices such as the rtc about state of delivery. Since this
+             * one is just a marker, it is no big deal */
+            apic_set_irq_delivered();
+        return;
+    }
+}
+
+static void kvm_pic_reset(void *opaque)
+{
+    struct kvm_pic_state *s = opaque;
+    struct kvm_irqchip *chip;
+
+    s->last_irr = 0;
+    s->irr = 0;
+    s->imr = 0;
+    s->isr = 0;
+    s->priority_add = 0;
+    s->irq_base = 0;
+    s->read_reg_select = 0;
+    s->poll = 0;
+    s->special_mask = 0;
+    s->init_state = 0;
+    s->auto_eoi = 0;
+    s->rotate_on_auto_eoi = 0;
+    s->special_fully_nested_mode = 0;
+    s->init4 = 0;
+
+    chip = container_of(s, struct kvm_irqchip, chip.pic);
+    kvm_set_irqchip(chip);
+}
+
+static void pic_pre_save(void *opaque)
+{
+    struct kvm_pic_state *s = opaque;
+    struct kvm_irqchip *chip;
+
+    chip = container_of(s, struct kvm_irqchip, chip.pic);
+
+    kvm_get_irqchip(chip);
+}
+
+static int pic_post_load(void *opaque, int version_id)
+{
+    struct kvm_pic_state *s = opaque;
+    struct kvm_irqchip *chip;
+
+    chip = container_of(s, struct kvm_irqchip, chip.pic);
+
+    return kvm_set_irqchip(chip);
+}
+
+static const VMStateDescription vmstate_kvm_pic = {
+    .name = "i8259-kvm",
+    .version_id = 1,
+    .pre_save = pic_pre_save,
+    .post_load = pic_post_load,
+    .minimum_version_id = 1,
+    .fields      = (VMStateField []) {
+        VMSTATE_UINT8(last_irr, struct kvm_pic_state),
+        VMSTATE_UINT8(irr, struct kvm_pic_state),
+        VMSTATE_UINT8(imr, struct kvm_pic_state),
+        VMSTATE_UINT8(isr, struct kvm_pic_state),
+        VMSTATE_UINT8(priority_add, struct kvm_pic_state),
+        VMSTATE_UINT8(irq_base, struct kvm_pic_state),
+        VMSTATE_UINT8(read_reg_select, struct kvm_pic_state),
+        VMSTATE_UINT8(poll, struct kvm_pic_state),
+        VMSTATE_UINT8(special_mask, struct kvm_pic_state),
+        VMSTATE_UINT8(init_state, struct kvm_pic_state),
+        VMSTATE_UINT8(auto_eoi, struct kvm_pic_state),
+        VMSTATE_UINT8(rotate_on_auto_eoi, struct kvm_pic_state),
+        VMSTATE_UINT8(special_fully_nested_mode, struct kvm_pic_state),
+        VMSTATE_UINT8(init4, struct kvm_pic_state),
+        VMSTATE_UINT8(elcr, struct kvm_pic_state),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void kvm_pic_init1(int io_addr, struct kvm_pic_state *s)
+{
+    vmstate_register(io_addr, &vmstate_kvm_pic, s);
+    qemu_register_reset(kvm_pic_reset, s);
+}
+
+qemu_irq *kvm_i8259_init(qemu_irq parent_irq)
+{
+    struct kvm_irqchip *master, *slave;
+
+    master = qemu_mallocz(sizeof(*master));
+    slave = qemu_mallocz(sizeof(*slave));
+
+    master->chip_id = KVM_IRQCHIP_PIC_MASTER;
+    slave->chip_id = KVM_IRQCHIP_PIC_SLAVE;
+
+    kvm_pic_init1(0x20, &master->chip.pic);
+    kvm_pic_init1(0xa0, &slave->chip.pic);
+
+    return qemu_allocate_irqs(kvm_i8259_set_irq, master, 16);
+}
diff --git a/hw/pc.c b/hw/pc.c
index ce9e832..fb40b86 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -903,8 +903,14 @@ static void pc_init1(ram_addr_t ram_size,
     }
 
     cpu_irq = qemu_allocate_irqs(pic_irq_request, NULL, 1);
-    i8259 = i8259_init(cpu_irq[0]);
     isa_irq_state = qemu_mallocz(sizeof(*isa_irq_state));
+
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+        i8259 = kvm_i8259_init(cpu_irq[0]);
+    } else {
+        i8259 = i8259_init(cpu_irq[0]);
+    }
+
     isa_irq_state->i8259 = i8259;
     isa_irq = qemu_allocate_irqs(isa_irq_handler, isa_irq_state, 24);
 
diff --git a/hw/pc.h b/hw/pc.h
index cad735a..f727e30 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -34,6 +34,7 @@ uint32_t pic_intack_read(PicState2 *s);
 void pic_info(Monitor *mon);
 void irq_info(Monitor *mon);
 
+qemu_irq *kvm_i8259_init(qemu_irq parent_irq);
 /* APIC */
 typedef struct IOAPICState IOAPICState;
 void apic_deliver_irq(uint8_t dest, uint8_t dest_mode,
diff --git a/kvm-all.c b/kvm-all.c
index 6417183..00e7411 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -586,6 +586,11 @@ int kvm_get_irqchip(struct kvm_irqchip *chip)
     return kvm_vm_ioctl(kvm_state, KVM_GET_IRQCHIP, chip);
 }
 
+int kvm_set_irq(int irq, int level, int *status)
+{
+    return kvm_arch_set_irq(kvm_state, irq, level, status);
+}
+
 int kvm_init(int smp_cpus)
 {
     static const char upgrade_note[] =
diff --git a/kvm.h b/kvm.h
index 812856e..f5be35c 100644
--- a/kvm.h
+++ b/kvm.h
@@ -64,6 +64,8 @@ int kvm_irqchip_in_kernel(void);
 int kvm_set_irqchip(struct kvm_irqchip *chip);
 int kvm_get_irqchip(struct kvm_irqchip *chip);
 
+int kvm_set_irq(int irq, int level, int *status);
+
 /* internal API */
 
 struct KVMState;
@@ -93,6 +95,8 @@ int kvm_arch_init_vcpu(CPUState *env);
 
 void kvm_arch_reset_vcpu(CPUState *env);
 
+int kvm_arch_set_irq(KVMState *s, int irq, int level, int *status);
+
 struct kvm_guest_debug;
 struct kvm_debug_exit_arch;
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 6b741ba..1cb4aeb 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1157,3 +1157,27 @@ void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg)
     }
 }
 #endif /* KVM_CAP_SET_GUEST_DEBUG */
+
+int kvm_arch_set_irq(KVMState *s, int irq, int level, int *status)
+{
+    struct kvm_irq_level event;
+    int r;
+
+    if (!kvm_irqchip_in_kernel()) {
+        return 0;
+    }
+
+    event.level = level;
+    event.irq = irq;
+
+    r = kvm_vm_ioctl(s, KVM_IRQ_LINE_STATUS, &event);
+
+    if (r < 0)
+        return 0;
+
+    if (status) {
+        *status = event.status;
+    }
+
+    return 1;
+}
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 8ad0037..ea18843 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -73,6 +73,11 @@ void kvm_arch_reset_vcpu(CPUState *env)
 {
 }
 
+int kvm_arch_set_irq(KVMState *s, int irq, int level, int *status)
+{
+    return -ENOSYS;
+}
+
 int kvm_arch_put_registers(CPUState *env)
 {
     struct kvm_regs regs;
-- 
1.6.6


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 05/10] Don't call apic functions directly from kvm code
  2010-02-26 20:12       ` [PATCH 04/10] provide i8259-kvm Glauber Costa
@ 2010-02-26 20:12         ` Glauber Costa
  2010-02-26 20:12           ` [PATCH 06/10] export kvm_put_mp_state Glauber Costa
  2010-03-09 13:27           ` [PATCH 05/10] Don't call apic functions directly from kvm code Avi Kivity
  0 siblings, 2 replies; 31+ messages in thread
From: Glauber Costa @ 2010-02-26 20:12 UTC (permalink / raw)
  To: kvm; +Cc: mtosatti

It is actually not necessary to call a tpr function to save and load cr8,
as cr8 is part of the processor state, and thus, it is much easier
to just add it to CPUState.

As for apic base, wrap kvm usages, so we can call either the qemu device,
or the in kernel version.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 target-i386/cpu.h |    1 +
 target-i386/kvm.c |   39 +++++++++++++++++++++++++++++++++------
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index ef7d951..abc2eb3 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -606,6 +606,7 @@ typedef struct CPUX86State {
     SegmentCache idt; /* only base and limit are used */
 
     target_ulong cr[5]; /* NOTE: cr1 is unused */
+    target_ulong cr8;
     int32_t a20_mask;
 
     /* FPU state */
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 1cb4aeb..18a6d8d 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -419,6 +419,33 @@ static void get_seg(SegmentCache *lhs, const struct kvm_segment *rhs)
 	| (rhs->avl * DESC_AVL_MASK);
 }
 
+static void kvm_set_apic_base(CPUState *env, uint64_t val)
+{
+    if (!kvm_irqchip_in_kernel())
+        cpu_set_apic_base(env, val);
+}
+
+static uint64_t kvm_get_apic_base(CPUState *env)
+{
+    if (!kvm_irqchip_in_kernel())
+        return cpu_get_apic_base(env);
+    return 0;
+}
+
+static void kvm_set_apic_tpr(CPUState *env, uint8_t val)
+{
+    if (!kvm_irqchip_in_kernel())
+        cpu_set_apic_tpr(env, val);
+}
+
+static uint8_t kvm_get_apic_tpr(CPUState *env)
+{
+    if (!kvm_irqchip_in_kernel())
+        return cpu_get_apic_tpr(env);
+    return 0;
+}
+
+
 static void kvm_getput_reg(__u64 *kvm_reg, target_ulong *qemu_reg, int set)
 {
     if (set)
@@ -530,8 +557,8 @@ static int kvm_put_sregs(CPUState *env)
     sregs.cr3 = env->cr[3];
     sregs.cr4 = env->cr[4];
 
-    sregs.cr8 = cpu_get_apic_tpr(env);
-    sregs.apic_base = cpu_get_apic_base(env);
+    sregs.cr8 = kvm_get_apic_tpr(env);
+    sregs.apic_base = kvm_get_apic_base(env);
 
     sregs.efer = env->efer;
 
@@ -639,7 +666,7 @@ static int kvm_get_sregs(CPUState *env)
     env->cr[3] = sregs.cr3;
     env->cr[4] = sregs.cr4;
 
-    cpu_set_apic_base(env, sregs.apic_base);
+    kvm_set_apic_base(env, sregs.apic_base);
 
     env->efer = sregs.efer;
     //cpu_set_apic_tpr(env, sregs.cr8);
@@ -942,7 +969,7 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
         run->request_interrupt_window = 0;
 
     dprintf("setting tpr\n");
-    run->cr8 = cpu_get_apic_tpr(env);
+    run->cr8 = kvm_get_apic_tpr(env);
 
     return 0;
 }
@@ -954,8 +981,8 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
     else
         env->eflags &= ~IF_MASK;
     
-    cpu_set_apic_tpr(env, run->cr8);
-    cpu_set_apic_base(env, run->apic_base);
+    kvm_set_apic_tpr(env, run->cr8);
+    kvm_set_apic_base(env, run->apic_base);
 
     return 0;
 }
-- 
1.6.6


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 06/10] export kvm_put_mp_state
  2010-02-26 20:12         ` [PATCH 05/10] Don't call apic functions directly from kvm code Glauber Costa
@ 2010-02-26 20:12           ` Glauber Costa
  2010-02-26 20:12             ` [PATCH 07/10] provide apic-kvm Glauber Costa
  2010-03-09 13:27           ` [PATCH 05/10] Don't call apic functions directly from kvm code Avi Kivity
  1 sibling, 1 reply; 31+ messages in thread
From: Glauber Costa @ 2010-02-26 20:12 UTC (permalink / raw)
  To: kvm; +Cc: mtosatti

We'll use it from inside the in-kernel apic chip, so get it into a header
file.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 target-i386/cpu.h |    4 ++++
 target-i386/kvm.c |    2 +-
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index abc2eb3..78399a1 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -933,4 +933,8 @@ void apic_init_reset(CPUState *env);
 void apic_sipi(CPUState *env);
 void do_cpu_init(CPUState *env);
 void do_cpu_sipi(CPUState *env);
+
+/* KVM hooks */
+int kvm_put_mp_state(CPUState *env);
+
 #endif /* CPU_I386_H */
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 18a6d8d..825decd 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -788,7 +788,7 @@ static int kvm_get_msrs(CPUState *env)
     return 0;
 }
 
-static int kvm_put_mp_state(CPUState *env)
+int kvm_put_mp_state(CPUState *env)
 {
     struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
 
-- 
1.6.6


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 07/10] provide apic-kvm
  2010-02-26 20:12           ` [PATCH 06/10] export kvm_put_mp_state Glauber Costa
@ 2010-02-26 20:12             ` Glauber Costa
  2010-02-26 20:12               ` [PATCH 08/10] Add -kvm option Glauber Costa
  2010-03-04 16:49               ` [PATCH 07/10] provide apic-kvm Jan Kiszka
  0 siblings, 2 replies; 31+ messages in thread
From: Glauber Costa @ 2010-02-26 20:12 UTC (permalink / raw)
  To: kvm; +Cc: mtosatti

This patch provides the file apic-kvm.c, which implements a schim over
the kvm in-kernel APIC.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 Makefile.target   |    2 +-
 hw/apic-kvm.c     |  157 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/pc.c           |    6 ++-
 hw/pc.h           |    2 +
 kvm.h             |    5 ++
 target-i386/cpu.h |    4 ++
 target-i386/kvm.c |   25 ++++++++-
 7 files changed, 197 insertions(+), 4 deletions(-)
 create mode 100644 hw/apic-kvm.c

diff --git a/Makefile.target b/Makefile.target
index bc5263e..f00af07 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -213,7 +213,7 @@ obj-i386-y += usb-uhci.o vmmouse.o vmport.o vmware_vga.o hpet.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += ne2000-isa.o debugcon.o multiboot.o
 
-obj-i386-$(CONFIG_KVM) += ioapic-kvm.o i8259-kvm.o
+obj-i386-$(CONFIG_KVM) += ioapic-kvm.o i8259-kvm.o apic-kvm.o
 
 # shared objects
 obj-ppc-y = ppc.o ide/core.o ide/qdev.o ide/isa.o ide/pci.o ide/macio.o
diff --git a/hw/apic-kvm.c b/hw/apic-kvm.c
new file mode 100644
index 0000000..089fa45
--- /dev/null
+++ b/hw/apic-kvm.c
@@ -0,0 +1,157 @@
+#include "hw.h"
+#include "pc.h"
+#include "pci.h"
+#include "msix.h"
+#include "qemu-timer.h"
+#include "host-utils.h"
+#include "kvm.h"
+
+#define APIC_LVT_NB      6
+#define APIC_LVT_LINT0   3
+
+struct qemu_lapic_state {
+    uint32_t apicbase;
+    uint8_t id;
+    uint8_t arb_id;
+    uint8_t tpr;
+    uint32_t spurious_vec;
+    uint8_t log_dest;
+    uint8_t dest_mode;
+    uint32_t isr[8];  /* in service register */
+    uint32_t tmr[8];  /* trigger mode register */
+    uint32_t irr[8]; /* interrupt request register */
+    uint32_t lvt[APIC_LVT_NB];
+    uint32_t esr; /* error register */
+    uint32_t icr[2];
+
+    uint32_t divide_conf;
+    int count_shift;
+    uint32_t initial_count;
+    int64_t initial_count_load_time, next_time;
+    uint32_t idx;
+    int sipi_vector;
+    int wait_for_sipi;
+};
+
+typedef struct APICState {
+    CPUState *cpu_env;
+
+/* KVM lapic structure is just a big array of regs. But it is what kvm
+ * functions expect. So have both the fields separated, for easy access,
+ * and the kvm stucture, for ioctls communications */
+    union {
+        struct qemu_lapic_state dev;
+        struct kvm_lapic_state kvm_lapic_state;
+    };
+} APICState;
+
+void kvm_apic_set_base(CPUState *env, uint64_t val)
+{
+    APICState *s = env->apic_state;
+
+    if (!s)
+        return;
+
+    s->dev.apicbase = val;
+}
+
+uint64_t kvm_apic_get_base(CPUState *env)
+{
+    APICState *s = env->apic_state;
+
+    return s ? s->dev.apicbase : 0;
+}
+
+static void apic_pre_save(void *opaque)
+{
+    APICState *s = opaque;
+
+    kvm_get_lapic(s->cpu_env, &s->kvm_lapic_state);
+}
+
+static int apic_post_load(void *opaque, int version_id)
+{
+    APICState *s = opaque;
+
+    return kvm_set_lapic(s->cpu_env, &s->kvm_lapic_state);
+}
+
+static const VMStateDescription vmstate_apic = {
+    .name = "apic-kvm",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField []) {
+        VMSTATE_BUFFER_UNSAFE(kvm_lapic_state.regs, APICState, 1, KVM_APIC_REG_SIZE),
+        VMSTATE_END_OF_LIST()
+    },
+    .pre_save = apic_pre_save,
+    .post_load = apic_post_load,
+};
+
+static void kvm_apic_reset(void *opaque)
+{
+    APICState *s = opaque;
+    int bsp;
+    int i;
+
+    cpu_synchronize_state(s->cpu_env);
+
+    bsp = cpu_is_bsp(s->cpu_env);
+
+    s->dev.apicbase = 0xfee00000 |
+        (bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE;
+
+    cpu_reset(s->cpu_env);
+
+    s->dev.tpr = 0;
+    s->dev.spurious_vec = 0xff;
+    s->dev.log_dest = 0;
+    s->dev.dest_mode = 0xf;
+    memset(s->dev.isr, 0, sizeof(s->dev.isr));
+    memset(s->dev.tmr, 0, sizeof(s->dev.tmr));
+    memset(s->dev.irr, 0, sizeof(s->dev.irr));
+    for(i = 0; i < APIC_LVT_NB; i++)
+        s->dev.lvt[i] = 1 << 16; /* mask LVT */
+    s->dev.esr = 0;
+    memset(s->dev.icr, 0, sizeof(s->dev.icr));
+    s->dev.divide_conf = 0;
+    s->dev.count_shift = 0;
+    s->dev.initial_count = 0;
+    s->dev.initial_count_load_time = 0;
+    s->dev.next_time = 0;
+    s->dev.wait_for_sipi = 1;
+
+    s->cpu_env->halted = !(s->dev.apicbase & MSR_IA32_APICBASE_BSP);
+
+    s->cpu_env->mp_state
+            = s->cpu_env->halted ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STATE_RUNNABLE;
+
+    kvm_put_mp_state(s->cpu_env);
+
+    if (bsp) {
+        /*
+         * LINT0 delivery mode on CPU #0 is set to ExtInt at initialization
+         * time typically by BIOS, so PIC interrupt can be delivered to the
+         * processor when local APIC is enabled.
+         */
+        s->dev.lvt[APIC_LVT_LINT0] = 0x700;
+    }
+    kvm_set_lapic(s->cpu_env, &s->kvm_lapic_state);
+}
+
+int kvm_apic_init(CPUState *env)
+{
+    APICState *s;
+
+    s = qemu_mallocz(sizeof(*s));
+    env->apic_state = s;
+    s->cpu_env = env;
+
+    msix_supported = 1;
+
+    vmstate_register(s->dev.id, &vmstate_apic, s);
+    qemu_register_reset(kvm_apic_reset, s);
+
+    return 0;
+}
diff --git a/hw/pc.c b/hw/pc.c
index fb40b86..66a54c0 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -776,7 +776,11 @@ static CPUState *pc_new_cpu(const char *cpu_model)
     if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
         env->cpuid_apic_id = env->cpu_index;
         /* APIC reset callback resets cpu */
-        apic_init(env);
+        if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+            kvm_apic_init(env);
+        } else {
+            apic_init(env);
+        }
     } else {
         qemu_register_reset((QEMUResetHandler*)cpu_reset, env);
     }
diff --git a/hw/pc.h b/hw/pc.h
index f727e30..c76831d 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -42,6 +42,8 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode,
                              uint8_t vector_num, uint8_t polarity,
                              uint8_t trigger_mode);
 int apic_init(CPUState *env);
+int kvm_apic_init(CPUState *env);
+
 int apic_accept_pic_intr(CPUState *env);
 void apic_deliver_pic_intr(CPUState *env, int level);
 int apic_get_interrupt(CPUState *env);
diff --git a/kvm.h b/kvm.h
index f5be35c..7278874 100644
--- a/kvm.h
+++ b/kvm.h
@@ -95,6 +95,11 @@ int kvm_arch_init_vcpu(CPUState *env);
 
 void kvm_arch_reset_vcpu(CPUState *env);
 
+#ifdef TARGET_I386
+int kvm_set_lapic(CPUState *env, struct kvm_lapic_state *s);
+int kvm_get_lapic(CPUState *env, struct kvm_lapic_state *s);
+#endif
+
 int kvm_arch_set_irq(KVMState *s, int irq, int level, int *status);
 
 struct kvm_guest_debug;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 78399a1..6678ea1 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -865,6 +865,10 @@ void cpu_set_apic_tpr(CPUX86State *env, uint8_t val);
 uint8_t cpu_get_apic_tpr(CPUX86State *env);
 #endif
 
+/* hw/apic-kvm.c */
+void kvm_apic_set_base(CPUX86State *env, uint64_t val);
+uint64_t kvm_apic_get_base(CPUX86State *env);
+
 /* hw/pc.c */
 void cpu_smm_update(CPUX86State *env);
 uint64_t cpu_get_tsc(CPUX86State *env);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 825decd..e9a7585 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -423,26 +423,31 @@ static void kvm_set_apic_base(CPUState *env, uint64_t val)
 {
     if (!kvm_irqchip_in_kernel())
         cpu_set_apic_base(env, val);
+    else
+        kvm_apic_set_base(env, val);
 }
 
 static uint64_t kvm_get_apic_base(CPUState *env)
 {
     if (!kvm_irqchip_in_kernel())
         return cpu_get_apic_base(env);
-    return 0;
+    else
+        return kvm_apic_get_base(env);
 }
 
 static void kvm_set_apic_tpr(CPUState *env, uint8_t val)
 {
     if (!kvm_irqchip_in_kernel())
         cpu_set_apic_tpr(env, val);
+    else
+        env->cr8 = val;
 }
 
 static uint8_t kvm_get_apic_tpr(CPUState *env)
 {
     if (!kvm_irqchip_in_kernel())
         return cpu_get_apic_tpr(env);
-    return 0;
+    return env->cr8;
 }
 
 
@@ -1208,3 +1213,19 @@ int kvm_arch_set_irq(KVMState *s, int irq, int level, int *status)
 
     return 1;
 }
+
+int kvm_set_lapic(CPUState *env, struct kvm_lapic_state *s)
+{
+    if (!kvm_irqchip_in_kernel())
+        return 0;
+
+    return kvm_vcpu_ioctl(env, KVM_SET_LAPIC, s);
+}
+
+int kvm_get_lapic(CPUState *env, struct kvm_lapic_state *s)
+{
+    if (!kvm_irqchip_in_kernel())
+        return 0;
+
+    return kvm_vcpu_ioctl(env, KVM_GET_LAPIC, s);
+}
-- 
1.6.6


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 08/10] Add -kvm option
  2010-02-26 20:12             ` [PATCH 07/10] provide apic-kvm Glauber Costa
@ 2010-02-26 20:12               ` Glauber Costa
  2010-02-26 20:12                 ` [PATCH 09/10] Initialize in-kernel irqchip Glauber Costa
                                   ` (3 more replies)
  2010-03-04 16:49               ` [PATCH 07/10] provide apic-kvm Jan Kiszka
  1 sibling, 4 replies; 31+ messages in thread
From: Glauber Costa @ 2010-02-26 20:12 UTC (permalink / raw)
  To: kvm; +Cc: mtosatti

This option deprecates --enable-kvm. It is a more flexible option,
that makes use of qemu-opts, and allow us to pass on options to enable or
disable kernel irqchip, for example.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 kvm-all.c       |    1 +
 kvm.h           |    1 +
 qemu-config.c   |   16 ++++++++++++++++
 qemu-config.h   |    1 +
 qemu-options.hx |   11 +++++++++--
 vl.c            |   11 +++++++++++
 6 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 00e7411..0527e0f 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -52,6 +52,7 @@ typedef struct KVMSlot
 typedef struct kvm_dirty_log KVMDirtyLog;
 
 int kvm_allowed = 0;
+int kvm_use_kernel_chip = 1;
 
 struct KVMState
 {
diff --git a/kvm.h b/kvm.h
index 7278874..480e651 100644
--- a/kvm.h
+++ b/kvm.h
@@ -20,6 +20,7 @@
 
 #ifdef CONFIG_KVM
 extern int kvm_allowed;
+extern int kvm_use_kernel_chip;
 
 #define kvm_enabled() (kvm_allowed)
 #else
diff --git a/qemu-config.c b/qemu-config.c
index 246fae6..310838e 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -290,6 +290,21 @@ QemuOptsList qemu_cpudef_opts = {
     },
 };
 
+QemuOptsList qemu_kvm_opts = {
+    .name = "kvm",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_kvm_opts.head),
+    .desc = {
+        {
+            .name = "irqchip-in-kernel",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "enabled",
+            .type = QEMU_OPT_BOOL,
+        },
+        { /* end if list */ }
+    },
+};
+
 static QemuOptsList *lists[] = {
     &qemu_drive_opts,
     &qemu_chardev_opts,
@@ -300,6 +315,7 @@ static QemuOptsList *lists[] = {
     &qemu_global_opts,
     &qemu_mon_opts,
     &qemu_cpudef_opts,
+    &qemu_kvm_opts,
     NULL,
 };
 
diff --git a/qemu-config.h b/qemu-config.h
index b335c42..506e5fb 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -10,6 +10,7 @@ extern QemuOptsList qemu_rtc_opts;
 extern QemuOptsList qemu_global_opts;
 extern QemuOptsList qemu_mon_opts;
 extern QemuOptsList qemu_cpudef_opts;
+extern QemuOptsList qemu_kvm_opts;
 
 int qemu_set_option(const char *str);
 int qemu_global_option(const char *str);
diff --git a/qemu-options.hx b/qemu-options.hx
index 3f49b44..f8fd86d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1793,10 +1793,17 @@ Set the filename for the BIOS.
 ETEXI
 
 #ifdef CONFIG_KVM
-DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, \
-    "-enable-kvm     enable KVM full virtualization support\n")
+HXCOMM Options deprecated by -kvm
+DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, "")
+
+DEF("kvm", HAS_ARG, QEMU_OPTION_kvm, \
+    "-kvm enable=on|off,irqchip-in-kernel=on|off\n" \
+    "                enable KVM full virtualization support\n")
+
 #endif
 STEXI
+@item -kvm enable=[on|off][,irqchip-in-kernel=on|off]
+@findex -kvm
 @item -enable-kvm
 @findex -enable-kvm
 Enable KVM full virtualization support. This option is only available
diff --git a/vl.c b/vl.c
index 66e477a..8c94fee 100644
--- a/vl.c
+++ b/vl.c
@@ -5416,6 +5416,17 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_enable_kvm:
                 kvm_allowed = 1;
                 break;
+            case QEMU_OPTION_kvm:
+
+                opts = qemu_opts_parse(&qemu_kvm_opts, optarg, NULL);
+                if (!opts) {
+                    fprintf(stderr, "parse error: %s\n", optarg);
+                    exit(1);
+                }
+
+                kvm_allowed = qemu_opt_get_bool(opts, "enabled", 1);
+                kvm_use_kernel_chip = qemu_opt_get_bool(opts, "irqchip-in-kernel", 1);
+                break;
 #endif
             case QEMU_OPTION_usb:
                 usb_enabled = 1;
-- 
1.6.6


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 09/10] Initialize in-kernel irqchip
  2010-02-26 20:12               ` [PATCH 08/10] Add -kvm option Glauber Costa
@ 2010-02-26 20:12                 ` Glauber Costa
  2010-02-26 20:12                   ` [PATCH 10/10] Do GSI routing Glauber Costa
  2010-03-02  4:31                   ` [PATCH 09/10] Initialize in-kernel irqchip Marcelo Tosatti
  2010-02-27 10:35                 ` [PATCH 08/10] Add -kvm option Jan Kiszka
                                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 31+ messages in thread
From: Glauber Costa @ 2010-02-26 20:12 UTC (permalink / raw)
  To: kvm; +Cc: mtosatti

Now that we have all devices set up, this patch initializes the irqchip.
This is dependant on the io-thread, since we need someone to pull ourselves
out of the halted state.

I believe this should be the default when we are running over the io-thread.
Later on, I plan to post a patch that makes it optional.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 kvm-all.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 0527e0f..3038465 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -592,6 +592,25 @@ int kvm_set_irq(int irq, int level, int *status)
     return kvm_arch_set_irq(kvm_state, irq, level, status);
 }
 
+static int kvm_create_irqchip(KVMState *s)
+{
+    int ret = 0;
+
+    if (!kvm_use_kernel_chip)
+        return ret;
+#if defined(CONFIG_IOTHREAD)
+    if (!kvm_check_extension(s, KVM_CAP_IRQCHIP))
+        return -1;
+
+    ret = kvm_vm_ioctl(s, KVM_CREATE_IRQCHIP);
+    if (ret < 0)
+        return ret;
+
+    s->irqchip_in_kernel = 1;
+#endif
+    return ret;
+}
+
 int kvm_init(int smp_cpus)
 {
     static const char upgrade_note[] =
@@ -683,6 +702,10 @@ int kvm_init(int smp_cpus)
     s->vcpu_events = kvm_check_extension(s, KVM_CAP_VCPU_EVENTS);
 #endif
 
+    ret = kvm_create_irqchip(s);
+    if (ret < 0)
+        goto err;
+
     ret = kvm_arch_init(s, smp_cpus);
     if (ret < 0)
         goto err;
-- 
1.6.6


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 10/10] Do GSI routing
  2010-02-26 20:12                 ` [PATCH 09/10] Initialize in-kernel irqchip Glauber Costa
@ 2010-02-26 20:12                   ` Glauber Costa
  2010-03-02  4:31                   ` [PATCH 09/10] Initialize in-kernel irqchip Marcelo Tosatti
  1 sibling, 0 replies; 31+ messages in thread
From: Glauber Costa @ 2010-02-26 20:12 UTC (permalink / raw)
  To: kvm; +Cc: mtosatti

To follow correctly what our bios ACPI tables say, we have to be able to program
our irqchips with GSI routing mappings. This support is already in qemu-kvm

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 kvm-all.c         |    4 +-
 kvm.h             |    3 +
 target-i386/kvm.c |  118 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 124 insertions(+), 1 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 3038465..5c4151f 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -71,6 +71,8 @@ struct KVMState
 #endif
     int irqchip_in_kernel;
     int pit_in_kernel;
+
+    KVMArchState *arch_state;
 };
 
 static KVMState *kvm_state;
@@ -627,6 +629,7 @@ int kvm_init(int smp_cpus)
 
     s = qemu_mallocz(sizeof(KVMState));
 
+    kvm_state = s;
 #ifdef KVM_CAP_SET_GUEST_DEBUG
     QTAILQ_INIT(&s->kvm_sw_breakpoints);
 #endif
@@ -710,7 +713,6 @@ int kvm_init(int smp_cpus)
     if (ret < 0)
         goto err;
 
-    kvm_state = s;
     cpu_register_phys_memory_client(&kvm_cpu_phys_memory_client);
 
     return 0;
diff --git a/kvm.h b/kvm.h
index 480e651..14d6e91 100644
--- a/kvm.h
+++ b/kvm.h
@@ -70,7 +70,10 @@ int kvm_set_irq(int irq, int level, int *status);
 /* internal API */
 
 struct KVMState;
+struct KVMArchState;
+
 typedef struct KVMState KVMState;
+typedef struct KVMArchState KVMArchState;
 
 int kvm_ioctl(KVMState *s, int type, ...);
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index e9a7585..58b1551 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -45,6 +45,16 @@
 
 #ifdef KVM_CAP_EXT_CPUID
 
+struct KVMArchState
+{
+    struct kvm_irq_routing *irq_routes;
+    int nr_allocated_irq_routes;
+    void *used_gsi_bitmap;
+    int max_gsi;
+};
+
+static KVMArchState *kvm_arch_state;
+
 static struct kvm_cpuid2 *try_get_cpuid(KVMState *s, int max)
 {
     struct kvm_cpuid2 *cpuid;
@@ -340,10 +350,118 @@ static int kvm_has_msr_star(CPUState *env)
     return 0;
 }
 
+/*
+ * Setup x86 specific IRQ routing
+ */
+static inline void set_gsi(KVMArchState *s, unsigned int gsi)
+{
+    uint32_t *bitmap = s->used_gsi_bitmap;
+
+    if (gsi < s->max_gsi)
+        bitmap[gsi / 32] |= 1U << (gsi % 32);
+    else
+        fprintf(stderr, "Invalid GSI %d\n", gsi);
+}
+
+static int kvm_add_routing_entry(KVMArchState *s, struct kvm_irq_routing_entry *entry)
+{
+    struct kvm_irq_routing *z;
+    struct kvm_irq_routing_entry *new;
+    int n, size;
+
+    if (s->irq_routes->nr == s->nr_allocated_irq_routes) {
+        n = s->nr_allocated_irq_routes * 2;
+        if (n < 64)
+            n = 64;
+        size = sizeof(struct kvm_irq_routing);
+        size += n * sizeof(*new);
+        z = realloc(s->irq_routes, size);
+        if (!z)
+            return -ENOMEM;
+        s->nr_allocated_irq_routes = n;
+        s->irq_routes = z;
+    }
+    n = s->irq_routes->nr++;
+    new = &s->irq_routes->entries[n];
+    memset(new, 0, sizeof(*new));
+    new->gsi = entry->gsi;
+    new->type = entry->type;
+    new->flags = entry->flags;
+    new->u = entry->u;
+
+    set_gsi(s, entry->gsi);
+
+    return 0;
+}
+
+static int kvm_add_irq_route(KVMArchState *s, int gsi, int irqchip, int pin)
+{
+    struct kvm_irq_routing_entry e;
+
+    e.gsi = gsi;
+    e.type = KVM_IRQ_ROUTING_IRQCHIP;
+    e.flags = 0;
+    e.u.irqchip.irqchip = irqchip;
+    e.u.irqchip.pin = pin;
+    return kvm_add_routing_entry(s, &e);
+}
+
+static int kvm_init_irq_routing(KVMState *s)
+{
+    int i, r;
+    int gsi_count, gsi_bits;
+
+    gsi_count = kvm_check_extension(s, KVM_CAP_IRQ_ROUTING);
+    if (!kvm_irqchip_in_kernel() && (gsi_count > 0)) {
+        return 0;
+    }
+
+        /* Round up so we can search ints using ffs */
+    gsi_bits = ((gsi_count - 31) & ~31);
+    kvm_arch_state->used_gsi_bitmap = qemu_mallocz(gsi_bits / 8);
+    kvm_arch_state->max_gsi = gsi_bits;
+
+    /* Mark any over-allocated bits as already in use */
+    for (i = gsi_count; i < gsi_bits; i++) {
+        set_gsi(kvm_arch_state, i);
+    }
+
+    kvm_arch_state->irq_routes->nr = 0;
+
+    for (i = 0; i < 8; ++i) {
+        if (i == 2)
+            continue;
+        r = kvm_add_irq_route(kvm_arch_state, i, KVM_IRQCHIP_PIC_MASTER, i);
+        if (r < 0)
+            return r;
+    }
+    for (i = 8; i < 16; ++i) {
+        r = kvm_add_irq_route(kvm_arch_state, i, KVM_IRQCHIP_PIC_SLAVE, i - 8);
+        if (r < 0)
+            return r;
+    }
+    for (i = 0; i < 24; ++i) {
+        if (i == 0) {
+            r = kvm_add_irq_route(kvm_arch_state, i, KVM_IRQCHIP_IOAPIC, 2);
+        } else if (i != 2) {
+            r = kvm_add_irq_route(kvm_arch_state, i, KVM_IRQCHIP_IOAPIC, i);
+        }
+        if (r < 0)
+            return r;
+    }
+
+    kvm_arch_state->irq_routes->flags = 0;
+    return kvm_vm_ioctl(s, KVM_SET_GSI_ROUTING, kvm_arch_state->irq_routes);
+}
 int kvm_arch_init(KVMState *s, int smp_cpus)
 {
     int ret;
 
+    kvm_arch_state = qemu_mallocz(sizeof(*kvm_arch_state));
+    kvm_arch_state->irq_routes = qemu_mallocz(sizeof(*kvm_arch_state->irq_routes));
+
+    kvm_init_irq_routing(s);
+
     /* create vm86 tss.  KVM uses vm86 mode to emulate 16-bit code
      * directly.  In order to use vm86 mode, a TSS is needed.  Since this
      * must be part of guest physical memory, we need to allocate it.  Older
-- 
1.6.6


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 00/10] uq/master: irqchip-in-kernel support
  2010-02-26 20:12 [PATCH 00/10] uq/master: irqchip-in-kernel support Glauber Costa
  2010-02-26 20:12 ` [PATCH 01/10] introduce VMSTATE_U64 Glauber Costa
@ 2010-02-27 10:28 ` Jan Kiszka
  2010-03-01 15:19   ` Marcelo Tosatti
  2010-03-04 16:33 ` Jan Kiszka
  2 siblings, 1 reply; 31+ messages in thread
From: Jan Kiszka @ 2010-02-27 10:28 UTC (permalink / raw)
  To: mtosatti; +Cc: Glauber Costa, kvm

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

Glauber Costa wrote:
> Hi guys,
> 
> This is the same in-kernel irqchip support already posted to qemu-devel,
> just rebased, retested, etc. It passes my basic tests, so it seem to be
> still in good shape.
> 
> It is provided against uq/master as part of the integration efforts
> 

Marcelo, what do you plan to take first, this series or my uq/master
version of the writeback rework? I have neither a problem rebasing my
upstream patches on top of this or helping Glauber with doing it the
other way around. I just want to avoid doing useless rebases.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 08/10] Add -kvm option
  2010-02-26 20:12               ` [PATCH 08/10] Add -kvm option Glauber Costa
  2010-02-26 20:12                 ` [PATCH 09/10] Initialize in-kernel irqchip Glauber Costa
@ 2010-02-27 10:35                 ` Jan Kiszka
  2010-03-04 16:20                   ` Jan Kiszka
  2010-03-02  4:31                 ` Marcelo Tosatti
  2010-03-04 19:38                 ` Anthony Liguori
  3 siblings, 1 reply; 31+ messages in thread
From: Jan Kiszka @ 2010-02-27 10:35 UTC (permalink / raw)
  To: Glauber Costa; +Cc: kvm, mtosatti

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

Glauber Costa wrote:
> This option deprecates --enable-kvm. It is a more flexible option,
> that makes use of qemu-opts, and allow us to pass on options to enable or
> disable kernel irqchip, for example.
> 

...

> diff --git a/qemu-options.hx b/qemu-options.hx
> index 3f49b44..f8fd86d 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1793,10 +1793,17 @@ Set the filename for the BIOS.
>  ETEXI
>  
>  #ifdef CONFIG_KVM
> -DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, \
> -    "-enable-kvm     enable KVM full virtualization support\n")
> +HXCOMM Options deprecated by -kvm
> +DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, "")
> +
> +DEF("kvm", HAS_ARG, QEMU_OPTION_kvm, \
> +    "-kvm enable=on|off,irqchip-in-kernel=on|off\n" \
> +    "                enable KVM full virtualization support\n")
> +

I would prefer "irqchip=kernel|user" - shorter and even more verbose.

Forgot if that was discussed already: Do we want "pit=kernel|user" as well?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 00/10] uq/master: irqchip-in-kernel support
  2010-02-27 10:28 ` [PATCH 00/10] uq/master: irqchip-in-kernel support Jan Kiszka
@ 2010-03-01 15:19   ` Marcelo Tosatti
  0 siblings, 0 replies; 31+ messages in thread
From: Marcelo Tosatti @ 2010-03-01 15:19 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Glauber Costa, kvm

On Sat, Feb 27, 2010 at 11:28:04AM +0100, Jan Kiszka wrote:
> Glauber Costa wrote:
> > Hi guys,
> > 
> > This is the same in-kernel irqchip support already posted to qemu-devel,
> > just rebased, retested, etc. It passes my basic tests, so it seem to be
> > still in good shape.
> > 
> > It is provided against uq/master as part of the integration efforts
> > 
> 
> Marcelo, what do you plan to take first, this series or my uq/master
> version of the writeback rework? I have neither a problem rebasing my
> upstream patches on top of this or helping Glauber with doing it the
> other way around. I just want to avoid doing useless rebases.

Your's first.


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 02/10] Provide ioapic-kvm
  2010-02-26 20:12   ` [PATCH 02/10] Provide ioapic-kvm Glauber Costa
  2010-02-26 20:12     ` [PATCH 03/10] provide apic_set_irq_delivered Glauber Costa
@ 2010-03-02  4:28     ` Marcelo Tosatti
  2010-03-02 18:26       ` Glauber Costa
  1 sibling, 1 reply; 31+ messages in thread
From: Marcelo Tosatti @ 2010-03-02  4:28 UTC (permalink / raw)
  To: Glauber Costa; +Cc: kvm

On Fri, Feb 26, 2010 at 05:12:13PM -0300, Glauber Costa wrote:
> This patch provides the file ioapic-kvm.c, which implements a schim over
> the kvm in-kernel IO APIC.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  Makefile.target |    2 +
>  hw/ioapic-kvm.c |   89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/pc.c         |    7 ++++-
>  hw/pc.h         |    2 +
>  kvm-all.c       |   18 +++++++++++
>  kvm.h           |    4 ++
>  6 files changed, 121 insertions(+), 1 deletions(-)
>  create mode 100644 hw/ioapic-kvm.c
> 
> diff --git a/Makefile.target b/Makefile.target
> index 4c4d397..78aff3c 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -213,6 +213,8 @@ obj-i386-y += usb-uhci.o vmmouse.o vmport.o vmware_vga.o hpet.o
>  obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>  obj-i386-y += ne2000-isa.o debugcon.o multiboot.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 += ide/cmd646.o
> diff --git a/hw/ioapic-kvm.c b/hw/ioapic-kvm.c
> new file mode 100644
> index 0000000..78e0984
> --- /dev/null
> +++ b/hw/ioapic-kvm.c
> @@ -0,0 +1,89 @@
> +#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 []) {
> +        /* Linux does not define __u64 the same as uint64_t */
> +        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)
> +{
> +/*
> +    int pic_ret;
> +
> +    if (kvm_set_irq(vector, level, &pic_ret)) {
> +        if (pic_ret != 0)
> +            apic_set_irq_delivered();
> +        return;
> +    }
> +*/

Didnt you forget to uncomment this later?


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 08/10] Add -kvm option
  2010-02-26 20:12               ` [PATCH 08/10] Add -kvm option Glauber Costa
  2010-02-26 20:12                 ` [PATCH 09/10] Initialize in-kernel irqchip Glauber Costa
  2010-02-27 10:35                 ` [PATCH 08/10] Add -kvm option Jan Kiszka
@ 2010-03-02  4:31                 ` Marcelo Tosatti
  2010-03-02 18:25                   ` Glauber Costa
  2010-03-04 19:38                 ` Anthony Liguori
  3 siblings, 1 reply; 31+ messages in thread
From: Marcelo Tosatti @ 2010-03-02  4:31 UTC (permalink / raw)
  To: Glauber Costa; +Cc: kvm

On Fri, Feb 26, 2010 at 05:12:19PM -0300, Glauber Costa wrote:
> This option deprecates --enable-kvm. It is a more flexible option,
> that makes use of qemu-opts, and allow us to pass on options to enable or
> disable kernel irqchip, for example.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>

Really have to replace -enable-kvm? Can't you keep compatibility for it?


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 09/10] Initialize in-kernel irqchip
  2010-02-26 20:12                 ` [PATCH 09/10] Initialize in-kernel irqchip Glauber Costa
  2010-02-26 20:12                   ` [PATCH 10/10] Do GSI routing Glauber Costa
@ 2010-03-02  4:31                   ` Marcelo Tosatti
  2010-03-02 18:25                     ` Glauber Costa
  1 sibling, 1 reply; 31+ messages in thread
From: Marcelo Tosatti @ 2010-03-02  4:31 UTC (permalink / raw)
  To: Glauber Costa; +Cc: kvm

On Fri, Feb 26, 2010 at 05:12:20PM -0300, Glauber Costa wrote:
> Now that we have all devices set up, this patch initializes the irqchip.
> This is dependant on the io-thread, since we need someone to pull ourselves
> out of the halted state.

I don't understand why - it should work without iothread.


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 09/10] Initialize in-kernel irqchip
  2010-03-02  4:31                   ` [PATCH 09/10] Initialize in-kernel irqchip Marcelo Tosatti
@ 2010-03-02 18:25                     ` Glauber Costa
  2010-03-09 13:21                       ` Avi Kivity
  0 siblings, 1 reply; 31+ messages in thread
From: Glauber Costa @ 2010-03-02 18:25 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

On Tue, Mar 02, 2010 at 01:31:35AM -0300, Marcelo Tosatti wrote:
> On Fri, Feb 26, 2010 at 05:12:20PM -0300, Glauber Costa wrote:
> > Now that we have all devices set up, this patch initializes the irqchip.
> > This is dependant on the io-thread, since we need someone to pull ourselves
> > out of the halted state.
> 
> I don't understand why - it should work without iothread.
with irqchip in kernel, we have to handle halted state in the kernel too.

> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 08/10] Add -kvm option
  2010-03-02  4:31                 ` Marcelo Tosatti
@ 2010-03-02 18:25                   ` Glauber Costa
  2010-03-04 19:41                     ` Anthony Liguori
  0 siblings, 1 reply; 31+ messages in thread
From: Glauber Costa @ 2010-03-02 18:25 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

On Tue, Mar 02, 2010 at 01:31:05AM -0300, Marcelo Tosatti wrote:
> On Fri, Feb 26, 2010 at 05:12:19PM -0300, Glauber Costa wrote:
> > This option deprecates --enable-kvm. It is a more flexible option,
> > that makes use of qemu-opts, and allow us to pass on options to enable or
> > disable kernel irqchip, for example.
> > 
> > Signed-off-by: Glauber Costa <glommer@redhat.com>
> 
> Really have to replace -enable-kvm? Can't you keep compatibility for it?
We don't have to , but I'd rather deprecate it. I don't feel strongly, though.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 02/10] Provide ioapic-kvm
  2010-03-02  4:28     ` [PATCH 02/10] Provide ioapic-kvm Marcelo Tosatti
@ 2010-03-02 18:26       ` Glauber Costa
  0 siblings, 0 replies; 31+ messages in thread
From: Glauber Costa @ 2010-03-02 18:26 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

> 
> Didnt you forget to uncomment this later?
uh-ow, true...

> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 08/10] Add -kvm option
  2010-02-27 10:35                 ` [PATCH 08/10] Add -kvm option Jan Kiszka
@ 2010-03-04 16:20                   ` Jan Kiszka
  2010-03-04 19:39                     ` Glauber Costa
  0 siblings, 1 reply; 31+ messages in thread
From: Jan Kiszka @ 2010-03-04 16:20 UTC (permalink / raw)
  To: Glauber Costa; +Cc: kvm, mtosatti

Jan Kiszka wrote:
> Glauber Costa wrote:
>> This option deprecates --enable-kvm. It is a more flexible option,
>> that makes use of qemu-opts, and allow us to pass on options to enable or
>> disable kernel irqchip, for example.
>>
> 
> ...
> 
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 3f49b44..f8fd86d 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -1793,10 +1793,17 @@ Set the filename for the BIOS.
>>  ETEXI
>>  
>>  #ifdef CONFIG_KVM
>> -DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, \
>> -    "-enable-kvm     enable KVM full virtualization support\n")
>> +HXCOMM Options deprecated by -kvm
>> +DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, "")
>> +
>> +DEF("kvm", HAS_ARG, QEMU_OPTION_kvm, \
>> +    "-kvm enable=on|off,irqchip-in-kernel=on|off\n" \
>> +    "                enable KVM full virtualization support\n")
>> +

Argh, never trust documentation: The magic option is "enabled", not
"enable". :)

> 
> I would prefer "irqchip=kernel|user" - shorter and even more verbose.

And we should refuse to work if the user tries to enable in-kernel
support without having io-threads enabled. That obviously fails silently
so far.

"info kvm" should also be extended to report the configuration in force.

> 
> Forgot if that was discussed already: Do we want "pit=kernel|user" as well?
> 

No comments on this?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 00/10] uq/master: irqchip-in-kernel support
  2010-02-26 20:12 [PATCH 00/10] uq/master: irqchip-in-kernel support Glauber Costa
  2010-02-26 20:12 ` [PATCH 01/10] introduce VMSTATE_U64 Glauber Costa
  2010-02-27 10:28 ` [PATCH 00/10] uq/master: irqchip-in-kernel support Jan Kiszka
@ 2010-03-04 16:33 ` Jan Kiszka
  2010-03-08 20:56   ` Marcelo Tosatti
  2 siblings, 1 reply; 31+ messages in thread
From: Jan Kiszka @ 2010-03-04 16:33 UTC (permalink / raw)
  To: Glauber Costa; +Cc: kvm, mtosatti

Glauber Costa wrote:
> Hi guys,
> 
> This is the same in-kernel irqchip support already posted to qemu-devel,
> just rebased, retested, etc. It passes my basic tests, so it seem to be
> still in good shape.
> 
> It is provided against uq/master as part of the integration efforts

Just as another heads-up:

host->guest networking performance over slirp and non-virtio NICs
suffers with this irqchip support the same way as in qemu-kvm. It's not
a bug I expect to be directly related to these changes, but it is at
least triggered by them and should now really be addressed.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 07/10] provide apic-kvm
  2010-02-26 20:12             ` [PATCH 07/10] provide apic-kvm Glauber Costa
  2010-02-26 20:12               ` [PATCH 08/10] Add -kvm option Glauber Costa
@ 2010-03-04 16:49               ` Jan Kiszka
  1 sibling, 0 replies; 31+ messages in thread
From: Jan Kiszka @ 2010-03-04 16:49 UTC (permalink / raw)
  To: Glauber Costa; +Cc: kvm, mtosatti

Glauber Costa wrote:
> This patch provides the file apic-kvm.c, which implements a schim over
> the kvm in-kernel APIC.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  Makefile.target   |    2 +-
>  hw/apic-kvm.c     |  157 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/pc.c           |    6 ++-
>  hw/pc.h           |    2 +
>  kvm.h             |    5 ++
>  target-i386/cpu.h |    4 ++
>  target-i386/kvm.c |   25 ++++++++-
>  7 files changed, 197 insertions(+), 4 deletions(-)
>  create mode 100644 hw/apic-kvm.c
> 
> diff --git a/Makefile.target b/Makefile.target
> index bc5263e..f00af07 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -213,7 +213,7 @@ obj-i386-y += usb-uhci.o vmmouse.o vmport.o vmware_vga.o hpet.o
>  obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>  obj-i386-y += ne2000-isa.o debugcon.o multiboot.o
>  
> -obj-i386-$(CONFIG_KVM) += ioapic-kvm.o i8259-kvm.o
> +obj-i386-$(CONFIG_KVM) += ioapic-kvm.o i8259-kvm.o apic-kvm.o
>  
>  # shared objects
>  obj-ppc-y = ppc.o ide/core.o ide/qdev.o ide/isa.o ide/pci.o ide/macio.o
> diff --git a/hw/apic-kvm.c b/hw/apic-kvm.c
> new file mode 100644
> index 0000000..089fa45
> --- /dev/null
> +++ b/hw/apic-kvm.c
> @@ -0,0 +1,157 @@
> +#include "hw.h"
> +#include "pc.h"
> +#include "pci.h"
> +#include "msix.h"
> +#include "qemu-timer.h"
> +#include "host-utils.h"
> +#include "kvm.h"
> +
> +#define APIC_LVT_NB      6
> +#define APIC_LVT_LINT0   3
> +
> +struct qemu_lapic_state {
> +    uint32_t apicbase;
> +    uint8_t id;
> +    uint8_t arb_id;
> +    uint8_t tpr;
> +    uint32_t spurious_vec;
> +    uint8_t log_dest;
> +    uint8_t dest_mode;
> +    uint32_t isr[8];  /* in service register */
> +    uint32_t tmr[8];  /* trigger mode register */
> +    uint32_t irr[8]; /* interrupt request register */
> +    uint32_t lvt[APIC_LVT_NB];
> +    uint32_t esr; /* error register */
> +    uint32_t icr[2];
> +
> +    uint32_t divide_conf;
> +    int count_shift;
> +    uint32_t initial_count;
> +    int64_t initial_count_load_time, next_time;
> +    uint32_t idx;
> +    int sipi_vector;
> +    int wait_for_sipi;
> +};
> +
> +typedef struct APICState {
> +    CPUState *cpu_env;
> +
> +/* KVM lapic structure is just a big array of regs. But it is what kvm
> + * functions expect. So have both the fields separated, for easy access,
> + * and the kvm stucture, for ioctls communications */
> +    union {
> +        struct qemu_lapic_state dev;
> +        struct kvm_lapic_state kvm_lapic_state;

That looks fishy to me on second sight: Is, e.g., loading the
kvm_lapic_state from the kernel supposed to magically fill the (totally
unaligned) qemu_lapic_state structure? I'm missing the translations of
kvm_kernel_lapic_load_from_user/save_to_user here or some effort to
arrange qemu_lapic_state in a way that it robustly maps on the register
array passed to/from the kernel (if that is possible, haven't checked yet).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 08/10] Add -kvm option
  2010-02-26 20:12               ` [PATCH 08/10] Add -kvm option Glauber Costa
                                   ` (2 preceding siblings ...)
  2010-03-02  4:31                 ` Marcelo Tosatti
@ 2010-03-04 19:38                 ` Anthony Liguori
  3 siblings, 0 replies; 31+ messages in thread
From: Anthony Liguori @ 2010-03-04 19:38 UTC (permalink / raw)
  To: Glauber Costa; +Cc: kvm, mtosatti, Gerd Hoffmann

On 02/26/2010 02:12 PM, Glauber Costa wrote:
> This option deprecates --enable-kvm. It is a more flexible option,
> that makes use of qemu-opts, and allow us to pass on options to enable or
> disable kernel irqchip, for example.
>
> Signed-off-by: Glauber Costa<glommer@redhat.com>
>    

kernel vs. userspace irqchip shouldn't be a kvm option.

Ideally, it would be a -device thing but I think we've agreed that 
-device won't cover platform devices.  So what we probably should do is 
change the machine option to accept a qopts list,  IOW:

-M pc,irqchip=user|kernel,pit=user|kernel,...

That certainly makes a lot more sense for non-x86 KVM targets (like s390 
and ppc).  And certainly, there's nothing that says that every x86 KVM 
target is going to have an APIC...

Regards,

Anthony Liguori

> ---
>   kvm-all.c       |    1 +
>   kvm.h           |    1 +
>   qemu-config.c   |   16 ++++++++++++++++
>   qemu-config.h   |    1 +
>   qemu-options.hx |   11 +++++++++--
>   vl.c            |   11 +++++++++++
>   6 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index 00e7411..0527e0f 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -52,6 +52,7 @@ typedef struct KVMSlot
>   typedef struct kvm_dirty_log KVMDirtyLog;
>
>   int kvm_allowed = 0;
> +int kvm_use_kernel_chip = 1;
>
>   struct KVMState
>   {
> diff --git a/kvm.h b/kvm.h
> index 7278874..480e651 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -20,6 +20,7 @@
>
>   #ifdef CONFIG_KVM
>   extern int kvm_allowed;
> +extern int kvm_use_kernel_chip;
>
>   #define kvm_enabled() (kvm_allowed)
>   #else
> diff --git a/qemu-config.c b/qemu-config.c
> index 246fae6..310838e 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -290,6 +290,21 @@ QemuOptsList qemu_cpudef_opts = {
>       },
>   };
>
> +QemuOptsList qemu_kvm_opts = {
> +    .name = "kvm",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_kvm_opts.head),
> +    .desc = {
> +        {
> +            .name = "irqchip-in-kernel",
> +            .type = QEMU_OPT_BOOL,
> +        },{
> +            .name = "enabled",
> +            .type = QEMU_OPT_BOOL,
> +        },
> +        { /* end if list */ }
> +    },
> +};
> +
>   static QemuOptsList *lists[] = {
>       &qemu_drive_opts,
>       &qemu_chardev_opts,
> @@ -300,6 +315,7 @@ static QemuOptsList *lists[] = {
>       &qemu_global_opts,
>       &qemu_mon_opts,
>       &qemu_cpudef_opts,
> +&qemu_kvm_opts,
>       NULL,
>   };
>
> diff --git a/qemu-config.h b/qemu-config.h
> index b335c42..506e5fb 100644
> --- a/qemu-config.h
> +++ b/qemu-config.h
> @@ -10,6 +10,7 @@ extern QemuOptsList qemu_rtc_opts;
>   extern QemuOptsList qemu_global_opts;
>   extern QemuOptsList qemu_mon_opts;
>   extern QemuOptsList qemu_cpudef_opts;
> +extern QemuOptsList qemu_kvm_opts;
>
>   int qemu_set_option(const char *str);
>   int qemu_global_option(const char *str);
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 3f49b44..f8fd86d 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1793,10 +1793,17 @@ Set the filename for the BIOS.
>   ETEXI
>
>   #ifdef CONFIG_KVM
> -DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, \
> -    "-enable-kvm     enable KVM full virtualization support\n")
> +HXCOMM Options deprecated by -kvm
> +DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, "")
> +
> +DEF("kvm", HAS_ARG, QEMU_OPTION_kvm, \
> +    "-kvm enable=on|off,irqchip-in-kernel=on|off\n" \
> +    "                enable KVM full virtualization support\n")
> +
>   #endif
>   STEXI
> +@item -kvm enable=[on|off][,irqchip-in-kernel=on|off]
> +@findex -kvm
>   @item -enable-kvm
>   @findex -enable-kvm
>   Enable KVM full virtualization support. This option is only available
> diff --git a/vl.c b/vl.c
> index 66e477a..8c94fee 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -5416,6 +5416,17 @@ int main(int argc, char **argv, char **envp)
>               case QEMU_OPTION_enable_kvm:
>                   kvm_allowed = 1;
>                   break;
> +            case QEMU_OPTION_kvm:
> +
> +                opts = qemu_opts_parse(&qemu_kvm_opts, optarg, NULL);
> +                if (!opts) {
> +                    fprintf(stderr, "parse error: %s\n", optarg);
> +                    exit(1);
> +                }
> +
> +                kvm_allowed = qemu_opt_get_bool(opts, "enabled", 1);
> +                kvm_use_kernel_chip = qemu_opt_get_bool(opts, "irqchip-in-kernel", 1);
> +                break;
>   #endif
>               case QEMU_OPTION_usb:
>                   usb_enabled = 1;
>    


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 08/10] Add -kvm option
  2010-03-04 16:20                   ` Jan Kiszka
@ 2010-03-04 19:39                     ` Glauber Costa
  0 siblings, 0 replies; 31+ messages in thread
From: Glauber Costa @ 2010-03-04 19:39 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: kvm, mtosatti

On Thu, Mar 04, 2010 at 05:20:22PM +0100, Jan Kiszka wrote:
> Jan Kiszka wrote:
> > Glauber Costa wrote:
> >> This option deprecates --enable-kvm. It is a more flexible option,
> >> that makes use of qemu-opts, and allow us to pass on options to enable or
> >> disable kernel irqchip, for example.
> >>
> > 
> > ...
> > 
> >> diff --git a/qemu-options.hx b/qemu-options.hx
> >> index 3f49b44..f8fd86d 100644
> >> --- a/qemu-options.hx
> >> +++ b/qemu-options.hx
> >> @@ -1793,10 +1793,17 @@ Set the filename for the BIOS.
> >>  ETEXI
> >>  
> >>  #ifdef CONFIG_KVM
> >> -DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, \
> >> -    "-enable-kvm     enable KVM full virtualization support\n")
> >> +HXCOMM Options deprecated by -kvm
> >> +DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, "")
> >> +
> >> +DEF("kvm", HAS_ARG, QEMU_OPTION_kvm, \
> >> +    "-kvm enable=on|off,irqchip-in-kernel=on|off\n" \
> >> +    "                enable KVM full virtualization support\n")
> >> +
> 
> Argh, never trust documentation: The magic option is "enabled", not
> "enable". :)
> 
> > 
> > I would prefer "irqchip=kernel|user" - shorter and even more verbose.
> 
> And we should refuse to work if the user tries to enable in-kernel
> support without having io-threads enabled. That obviously fails silently
> so far.
> 
> "info kvm" should also be extended to report the configuration in force.
I am waiting for marcelo to apply your patches (if he hasn't done already), then
I'll redo this. Agreed with this point, so plan on changing.

> 
> > 
> > Forgot if that was discussed already: Do we want "pit=kernel|user" as well?
> > 
> 
I guess we do.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 08/10] Add -kvm option
  2010-03-02 18:25                   ` Glauber Costa
@ 2010-03-04 19:41                     ` Anthony Liguori
  0 siblings, 0 replies; 31+ messages in thread
From: Anthony Liguori @ 2010-03-04 19:41 UTC (permalink / raw)
  To: Glauber Costa, kvm-devel, Marcelo Tosatti

On 03/02/2010 12:25 PM, Glauber Costa wrote:
> On Tue, Mar 02, 2010 at 01:31:05AM -0300, Marcelo Tosatti wrote:
>    
>> On Fri, Feb 26, 2010 at 05:12:19PM -0300, Glauber Costa wrote:
>>      
>>> This option deprecates --enable-kvm. It is a more flexible option,
>>> that makes use of qemu-opts, and allow us to pass on options to enable or
>>> disable kernel irqchip, for example.
>>>
>>> Signed-off-by: Glauber Costa<glommer@redhat.com>
>>>        
>> Really have to replace -enable-kvm? Can't you keep compatibility for it?
>>      
> We don't have to , but I'd rather deprecate it. I don't feel strongly, though.
>    

It needs to stay.

For enabling/disabling, if you don't like -enable-kvm, I'd suggest 
thinking about modeling it through CPU.  For instance:

-cpu host,accel=kvm|tcg|kvm,tcg

Since we already specify CPU's in a global config, if you took this 
approach, it would make it possible to tweak the default kvm vs. tcg 
selection within the config file so a user could control whether vms 
were created with tcg or kvm by default or whether it tried kvm and then 
fell back to tcg.

Regards,

Anthony Liguori

> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>    


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 00/10] uq/master: irqchip-in-kernel support
  2010-03-04 16:33 ` Jan Kiszka
@ 2010-03-08 20:56   ` Marcelo Tosatti
  0 siblings, 0 replies; 31+ messages in thread
From: Marcelo Tosatti @ 2010-03-08 20:56 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Glauber Costa, kvm

On Thu, Mar 04, 2010 at 05:33:16PM +0100, Jan Kiszka wrote:
> Glauber Costa wrote:
> > Hi guys,
> > 
> > This is the same in-kernel irqchip support already posted to qemu-devel,
> > just rebased, retested, etc. It passes my basic tests, so it seem to be
> > still in good shape.
> > 
> > It is provided against uq/master as part of the integration efforts
> 
> Just as another heads-up:
> 
> host->guest networking performance over slirp and non-virtio NICs
> suffers with this irqchip support the same way as in qemu-kvm. It's not
> a bug I expect to be directly related to these changes, but it is at
> least triggered by them and should now really be addressed.

Isnt it triggered by enablement of the iothread (and if so irqchip
support is unrelated to the problem) ?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 09/10] Initialize in-kernel irqchip
  2010-03-02 18:25                     ` Glauber Costa
@ 2010-03-09 13:21                       ` Avi Kivity
  0 siblings, 0 replies; 31+ messages in thread
From: Avi Kivity @ 2010-03-09 13:21 UTC (permalink / raw)
  To: Glauber Costa; +Cc: Marcelo Tosatti, kvm

On 03/02/2010 08:25 PM, Glauber Costa wrote:
> On Tue, Mar 02, 2010 at 01:31:35AM -0300, Marcelo Tosatti wrote:
>    
>> On Fri, Feb 26, 2010 at 05:12:20PM -0300, Glauber Costa wrote:
>>      
>>> Now that we have all devices set up, this patch initializes the irqchip.
>>> This is dependant on the io-thread, since we need someone to pull ourselves
>>> out of the halted state.
>>>        
>> I don't understand why - it should work without iothread.
>>      
> with irqchip in kernel, we have to handle halted state in the kernel too.
>    

We still exit on signals, same as tcg w/o iothread.

qemu-kvm had irqchip before iothread, IIRC.

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


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 05/10] Don't call apic functions directly from kvm code
  2010-02-26 20:12         ` [PATCH 05/10] Don't call apic functions directly from kvm code Glauber Costa
  2010-02-26 20:12           ` [PATCH 06/10] export kvm_put_mp_state Glauber Costa
@ 2010-03-09 13:27           ` Avi Kivity
  2010-03-17 14:00             ` Glauber Costa
  1 sibling, 1 reply; 31+ messages in thread
From: Avi Kivity @ 2010-03-09 13:27 UTC (permalink / raw)
  To: Glauber Costa; +Cc: kvm, mtosatti

On 02/26/2010 10:12 PM, Glauber Costa wrote:
> It is actually not necessary to call a tpr function to save and load cr8,
> as cr8 is part of the processor state, and thus, it is much easier
> to just add it to CPUState.
>
> As for apic base, wrap kvm usages, so we can call either the qemu device,
> or the in kernel version.
>
>
>   }
>
> +static void kvm_set_apic_base(CPUState *env, uint64_t val)
> +{
> +    if (!kvm_irqchip_in_kernel())
> +        cpu_set_apic_base(env, val);
>    

What if it is in kernel?  Just ignored?  Doesn't seem right.

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


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 05/10] Don't call apic functions directly from kvm code
  2010-03-09 13:27           ` [PATCH 05/10] Don't call apic functions directly from kvm code Avi Kivity
@ 2010-03-17 14:00             ` Glauber Costa
  2010-03-17 16:29               ` Avi Kivity
  0 siblings, 1 reply; 31+ messages in thread
From: Glauber Costa @ 2010-03-17 14:00 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, mtosatti

On Tue, Mar 09, 2010 at 03:27:02PM +0200, Avi Kivity wrote:
> On 02/26/2010 10:12 PM, Glauber Costa wrote:
> >It is actually not necessary to call a tpr function to save and load cr8,
> >as cr8 is part of the processor state, and thus, it is much easier
> >to just add it to CPUState.
> >
> >As for apic base, wrap kvm usages, so we can call either the qemu device,
> >or the in kernel version.
> >
> >
> >  }
> >
> >+static void kvm_set_apic_base(CPUState *env, uint64_t val)
> >+{
> >+    if (!kvm_irqchip_in_kernel())
> >+        cpu_set_apic_base(env, val);
> 
> What if it is in kernel?  Just ignored?  Doesn't seem right.
At this point it is right, because there is no irqchip in kernel yet.

In a later patch, irqchip in kernel begins to exist, and this function
gets filled.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 05/10] Don't call apic functions directly from kvm code
  2010-03-17 14:00             ` Glauber Costa
@ 2010-03-17 16:29               ` Avi Kivity
  0 siblings, 0 replies; 31+ messages in thread
From: Avi Kivity @ 2010-03-17 16:29 UTC (permalink / raw)
  To: Glauber Costa; +Cc: kvm, mtosatti

On 03/17/2010 04:00 PM, Glauber Costa wrote:
> On Tue, Mar 09, 2010 at 03:27:02PM +0200, Avi Kivity wrote:
>    
>> On 02/26/2010 10:12 PM, Glauber Costa wrote:
>>      
>>> It is actually not necessary to call a tpr function to save and load cr8,
>>> as cr8 is part of the processor state, and thus, it is much easier
>>> to just add it to CPUState.
>>>
>>> As for apic base, wrap kvm usages, so we can call either the qemu device,
>>> or the in kernel version.
>>>
>>>
>>>   }
>>>
>>> +static void kvm_set_apic_base(CPUState *env, uint64_t val)
>>> +{
>>> +    if (!kvm_irqchip_in_kernel())
>>> +        cpu_set_apic_base(env, val);
>>>        
>> What if it is in kernel?  Just ignored?  Doesn't seem right.
>>      
> At this point it is right, because there is no irqchip in kernel yet.
>
> In a later patch, irqchip in kernel begins to exist, and this function
> gets filled.
>    

Ok.  In the future please code things like that without the if (), and 
add it when you introduce the other side.  Helps fend off nit-pickers.

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


^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2010-03-17 16:29 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-02-26 20:12 [PATCH 00/10] uq/master: irqchip-in-kernel support Glauber Costa
2010-02-26 20:12 ` [PATCH 01/10] introduce VMSTATE_U64 Glauber Costa
2010-02-26 20:12   ` [PATCH 02/10] Provide ioapic-kvm Glauber Costa
2010-02-26 20:12     ` [PATCH 03/10] provide apic_set_irq_delivered Glauber Costa
2010-02-26 20:12       ` [PATCH 04/10] provide i8259-kvm Glauber Costa
2010-02-26 20:12         ` [PATCH 05/10] Don't call apic functions directly from kvm code Glauber Costa
2010-02-26 20:12           ` [PATCH 06/10] export kvm_put_mp_state Glauber Costa
2010-02-26 20:12             ` [PATCH 07/10] provide apic-kvm Glauber Costa
2010-02-26 20:12               ` [PATCH 08/10] Add -kvm option Glauber Costa
2010-02-26 20:12                 ` [PATCH 09/10] Initialize in-kernel irqchip Glauber Costa
2010-02-26 20:12                   ` [PATCH 10/10] Do GSI routing Glauber Costa
2010-03-02  4:31                   ` [PATCH 09/10] Initialize in-kernel irqchip Marcelo Tosatti
2010-03-02 18:25                     ` Glauber Costa
2010-03-09 13:21                       ` Avi Kivity
2010-02-27 10:35                 ` [PATCH 08/10] Add -kvm option Jan Kiszka
2010-03-04 16:20                   ` Jan Kiszka
2010-03-04 19:39                     ` Glauber Costa
2010-03-02  4:31                 ` Marcelo Tosatti
2010-03-02 18:25                   ` Glauber Costa
2010-03-04 19:41                     ` Anthony Liguori
2010-03-04 19:38                 ` Anthony Liguori
2010-03-04 16:49               ` [PATCH 07/10] provide apic-kvm Jan Kiszka
2010-03-09 13:27           ` [PATCH 05/10] Don't call apic functions directly from kvm code Avi Kivity
2010-03-17 14:00             ` Glauber Costa
2010-03-17 16:29               ` Avi Kivity
2010-03-02  4:28     ` [PATCH 02/10] Provide ioapic-kvm Marcelo Tosatti
2010-03-02 18:26       ` Glauber Costa
2010-02-27 10:28 ` [PATCH 00/10] uq/master: irqchip-in-kernel support Jan Kiszka
2010-03-01 15:19   ` Marcelo Tosatti
2010-03-04 16:33 ` Jan Kiszka
2010-03-08 20:56   ` Marcelo Tosatti

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.