* [0.14?][PATCH 0/4] IOAPIC fixes
@ 2011-02-03 14:55 ` Jan Kiszka
0 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 14:55 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: kvm, Alexander Graf, Gleb Natapov, Avi Kivity, Marcelo Tosatti
This series fixes the re-injection of level-triggered IRQs that are
still raised on APIC EOI, adds a must-have field to the vmstate of the
IOAPIC, and also aligns that vmstate with qemu-kvm.
I would recommend the whole series for 0.14, but at least patch 1 should
be applied.
Jan Kiszka (4):
ioapic: Implement EOI handling for level-triggered IRQs
ioapic: Save/restore irr
ioapic: Prepare for base address relocation
ioapic: Style & magics cleanup
hw/apic.c | 9 ++-
hw/ioapic.c | 244 +++++++++++++++++++++++++++++++++++++++++-----------------
hw/ioapic.h | 24 ++++++
hw/pc_piix.c | 5 +-
4 files changed, 206 insertions(+), 76 deletions(-)
create mode 100644 hw/ioapic.h
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [0.14?][PATCH 0/4] IOAPIC fixes
@ 2011-02-03 14:55 ` Jan Kiszka
0 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 14:55 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Gleb Natapov, Marcelo Tosatti, Alexander Graf, kvm, Avi Kivity
This series fixes the re-injection of level-triggered IRQs that are
still raised on APIC EOI, adds a must-have field to the vmstate of the
IOAPIC, and also aligns that vmstate with qemu-kvm.
I would recommend the whole series for 0.14, but at least patch 1 should
be applied.
Jan Kiszka (4):
ioapic: Implement EOI handling for level-triggered IRQs
ioapic: Save/restore irr
ioapic: Prepare for base address relocation
ioapic: Style & magics cleanup
hw/apic.c | 9 ++-
hw/ioapic.c | 244 +++++++++++++++++++++++++++++++++++++++++-----------------
hw/ioapic.h | 24 ++++++
hw/pc_piix.c | 5 +-
4 files changed, 206 insertions(+), 76 deletions(-)
create mode 100644 hw/ioapic.h
^ permalink raw reply [flat|nested] 36+ messages in thread
* [0.14?][PATCH 1/4] ioapic: Implement EOI handling for level-triggered IRQs
2011-02-03 14:55 ` [Qemu-devel] " Jan Kiszka
@ 2011-02-03 14:55 ` Jan Kiszka
-1 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 14:55 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: kvm, Alexander Graf, Gleb Natapov, Avi Kivity, Marcelo Tosatti
Add the missing EOI broadcast from local APIC to the IOAPICs on
completion of level-triggered IRQs. This ensures that a still asserted
IRQ source properly re-triggers an APIC IRQ.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/apic.c | 9 ++++++---
hw/ioapic.c | 43 +++++++++++++++++++++++++++++++++++++++++--
hw/ioapic.h | 20 ++++++++++++++++++++
3 files changed, 67 insertions(+), 5 deletions(-)
create mode 100644 hw/ioapic.h
diff --git a/hw/apic.c b/hw/apic.c
index ff581f0..05a115f 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -18,6 +18,7 @@
*/
#include "hw.h"
#include "apic.h"
+#include "ioapic.h"
#include "qemu-timer.h"
#include "host-utils.h"
#include "sysbus.h"
@@ -57,7 +58,8 @@
#define ESR_ILLEGAL_ADDRESS (1 << 7)
-#define APIC_SV_ENABLE (1 << 8)
+#define APIC_SV_DIRECTED_IO (1<<12)
+#define APIC_SV_ENABLE (1<<8)
#define MAX_APICS 255
#define MAX_APIC_WORDS 8
@@ -420,8 +422,9 @@ static void apic_eoi(APICState *s)
if (isrv < 0)
return;
reset_bit(s->isr, isrv);
- /* XXX: send the EOI packet to the APIC bus to allow the I/O APIC to
- set the remote IRR bit for level triggered interrupts. */
+ if (!(s->spurious_vec & APIC_SV_DIRECTED_IO) && get_bit(s->tmr, isrv)) {
+ ioapic_eio_broadcast(isrv);
+ }
apic_update_irq(s);
}
diff --git a/hw/ioapic.c b/hw/ioapic.c
index 2109568..443c579 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -23,6 +23,7 @@
#include "hw.h"
#include "pc.h"
#include "apic.h"
+#include "ioapic.h"
#include "qemu-timer.h"
#include "host-utils.h"
#include "sysbus.h"
@@ -36,7 +37,10 @@
#define DPRINTF(fmt, ...)
#endif
-#define IOAPIC_LVT_MASKED (1<<16)
+#define MAX_IOAPICS 1
+
+#define IOAPIC_LVT_MASKED (1 << 16)
+#define IOAPIC_LVT_REMOTE_IRR (1 << 14)
#define IOAPIC_TRIGGER_EDGE 0
#define IOAPIC_TRIGGER_LEVEL 1
@@ -61,6 +65,8 @@ struct IOAPICState {
uint64_t ioredtbl[IOAPIC_NUM_PINS];
};
+static IOAPICState *ioapics[MAX_IOAPICS];
+
static void ioapic_service(IOAPICState *s)
{
uint8_t i;
@@ -83,8 +89,11 @@ static void ioapic_service(IOAPICState *s)
dest_mode = (entry >> 11) & 1;
delivery_mode = (entry >> 8) & 7;
polarity = (entry >> 13) & 1;
- if (trig_mode == IOAPIC_TRIGGER_EDGE)
+ if (trig_mode == IOAPIC_TRIGGER_EDGE) {
s->irr &= ~mask;
+ } else {
+ s->ioredtbl[i] |= IOAPIC_LVT_REMOTE_IRR;
+ }
if (delivery_mode == IOAPIC_DM_EXTINT)
vector = pic_read_irq(isa_pic);
else
@@ -131,6 +140,29 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
}
}
+void ioapic_eio_broadcast(int vector)
+{
+ IOAPICState *s;
+ uint64_t entry;
+ int i, n;
+
+ for (i = 0; i < MAX_IOAPICS; i++) {
+ s = ioapics[i];
+ if (!s) {
+ continue;
+ }
+ for (n = 0; n < IOAPIC_NUM_PINS; n++) {
+ entry = s->ioredtbl[n];
+ if ((entry & IOAPIC_LVT_REMOTE_IRR) && (entry & 0xff) == vector) {
+ s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
+ if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) {
+ ioapic_service(s);
+ }
+ }
+ }
+ }
+}
+
static uint32_t ioapic_mem_readl(void *opaque, target_phys_addr_t addr)
{
IOAPICState *s = opaque;
@@ -240,6 +272,11 @@ static int ioapic_init1(SysBusDevice *dev)
{
IOAPICState *s = FROM_SYSBUS(IOAPICState, dev);
int io_memory;
+ static int ioapic_no;
+
+ if (ioapic_no >= MAX_IOAPICS) {
+ return -1;
+ }
io_memory = cpu_register_io_memory(ioapic_mem_read,
ioapic_mem_write, s,
@@ -248,6 +285,8 @@ static int ioapic_init1(SysBusDevice *dev)
qdev_init_gpio_in(&dev->qdev, ioapic_set_irq, IOAPIC_NUM_PINS);
+ ioapics[ioapic_no++] = s;
+
return 0;
}
diff --git a/hw/ioapic.h b/hw/ioapic.h
new file mode 100644
index 0000000..c441567
--- /dev/null
+++ b/hw/ioapic.h
@@ -0,0 +1,20 @@
+/*
+ * ioapic.c IOAPIC emulation logic
+ *
+ * Copyright (c) 2011 Jan Kiszka, Siemens AG
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+void ioapic_eio_broadcast(int vector);
--
1.7.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [0.14?][PATCH 1/4] ioapic: Implement EOI handling for level-triggered IRQs
@ 2011-02-03 14:55 ` Jan Kiszka
0 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 14:55 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Gleb Natapov, Marcelo Tosatti, Alexander Graf, kvm, Avi Kivity
Add the missing EOI broadcast from local APIC to the IOAPICs on
completion of level-triggered IRQs. This ensures that a still asserted
IRQ source properly re-triggers an APIC IRQ.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/apic.c | 9 ++++++---
hw/ioapic.c | 43 +++++++++++++++++++++++++++++++++++++++++--
hw/ioapic.h | 20 ++++++++++++++++++++
3 files changed, 67 insertions(+), 5 deletions(-)
create mode 100644 hw/ioapic.h
diff --git a/hw/apic.c b/hw/apic.c
index ff581f0..05a115f 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -18,6 +18,7 @@
*/
#include "hw.h"
#include "apic.h"
+#include "ioapic.h"
#include "qemu-timer.h"
#include "host-utils.h"
#include "sysbus.h"
@@ -57,7 +58,8 @@
#define ESR_ILLEGAL_ADDRESS (1 << 7)
-#define APIC_SV_ENABLE (1 << 8)
+#define APIC_SV_DIRECTED_IO (1<<12)
+#define APIC_SV_ENABLE (1<<8)
#define MAX_APICS 255
#define MAX_APIC_WORDS 8
@@ -420,8 +422,9 @@ static void apic_eoi(APICState *s)
if (isrv < 0)
return;
reset_bit(s->isr, isrv);
- /* XXX: send the EOI packet to the APIC bus to allow the I/O APIC to
- set the remote IRR bit for level triggered interrupts. */
+ if (!(s->spurious_vec & APIC_SV_DIRECTED_IO) && get_bit(s->tmr, isrv)) {
+ ioapic_eio_broadcast(isrv);
+ }
apic_update_irq(s);
}
diff --git a/hw/ioapic.c b/hw/ioapic.c
index 2109568..443c579 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -23,6 +23,7 @@
#include "hw.h"
#include "pc.h"
#include "apic.h"
+#include "ioapic.h"
#include "qemu-timer.h"
#include "host-utils.h"
#include "sysbus.h"
@@ -36,7 +37,10 @@
#define DPRINTF(fmt, ...)
#endif
-#define IOAPIC_LVT_MASKED (1<<16)
+#define MAX_IOAPICS 1
+
+#define IOAPIC_LVT_MASKED (1 << 16)
+#define IOAPIC_LVT_REMOTE_IRR (1 << 14)
#define IOAPIC_TRIGGER_EDGE 0
#define IOAPIC_TRIGGER_LEVEL 1
@@ -61,6 +65,8 @@ struct IOAPICState {
uint64_t ioredtbl[IOAPIC_NUM_PINS];
};
+static IOAPICState *ioapics[MAX_IOAPICS];
+
static void ioapic_service(IOAPICState *s)
{
uint8_t i;
@@ -83,8 +89,11 @@ static void ioapic_service(IOAPICState *s)
dest_mode = (entry >> 11) & 1;
delivery_mode = (entry >> 8) & 7;
polarity = (entry >> 13) & 1;
- if (trig_mode == IOAPIC_TRIGGER_EDGE)
+ if (trig_mode == IOAPIC_TRIGGER_EDGE) {
s->irr &= ~mask;
+ } else {
+ s->ioredtbl[i] |= IOAPIC_LVT_REMOTE_IRR;
+ }
if (delivery_mode == IOAPIC_DM_EXTINT)
vector = pic_read_irq(isa_pic);
else
@@ -131,6 +140,29 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
}
}
+void ioapic_eio_broadcast(int vector)
+{
+ IOAPICState *s;
+ uint64_t entry;
+ int i, n;
+
+ for (i = 0; i < MAX_IOAPICS; i++) {
+ s = ioapics[i];
+ if (!s) {
+ continue;
+ }
+ for (n = 0; n < IOAPIC_NUM_PINS; n++) {
+ entry = s->ioredtbl[n];
+ if ((entry & IOAPIC_LVT_REMOTE_IRR) && (entry & 0xff) == vector) {
+ s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
+ if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) {
+ ioapic_service(s);
+ }
+ }
+ }
+ }
+}
+
static uint32_t ioapic_mem_readl(void *opaque, target_phys_addr_t addr)
{
IOAPICState *s = opaque;
@@ -240,6 +272,11 @@ static int ioapic_init1(SysBusDevice *dev)
{
IOAPICState *s = FROM_SYSBUS(IOAPICState, dev);
int io_memory;
+ static int ioapic_no;
+
+ if (ioapic_no >= MAX_IOAPICS) {
+ return -1;
+ }
io_memory = cpu_register_io_memory(ioapic_mem_read,
ioapic_mem_write, s,
@@ -248,6 +285,8 @@ static int ioapic_init1(SysBusDevice *dev)
qdev_init_gpio_in(&dev->qdev, ioapic_set_irq, IOAPIC_NUM_PINS);
+ ioapics[ioapic_no++] = s;
+
return 0;
}
diff --git a/hw/ioapic.h b/hw/ioapic.h
new file mode 100644
index 0000000..c441567
--- /dev/null
+++ b/hw/ioapic.h
@@ -0,0 +1,20 @@
+/*
+ * ioapic.c IOAPIC emulation logic
+ *
+ * Copyright (c) 2011 Jan Kiszka, Siemens AG
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+void ioapic_eio_broadcast(int vector);
--
1.7.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [0.14?][PATCH 2/4] ioapic: Save/restore irr
2011-02-03 14:55 ` [Qemu-devel] " Jan Kiszka
@ 2011-02-03 14:55 ` Jan Kiszka
-1 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 14:55 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: kvm, Alexander Graf, Gleb Natapov, Avi Kivity, Marcelo Tosatti
This is a guest modifiable state that must be saved/restored properly.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/ioapic.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/hw/ioapic.c b/hw/ioapic.c
index 443c579..c7019f5 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -231,14 +231,25 @@ static void ioapic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t va
}
}
+static int ioapic_pre_load(void *opaque)
+{
+ IOAPICState *s = opaque;
+
+ /* in case we are loading version 1, set these to sane values */
+ s->irr = 0;
+ return 0;
+}
+
static const VMStateDescription vmstate_ioapic = {
.name = "ioapic",
- .version_id = 1,
+ .version_id = 2,
+ .pre_load = ioapic_pre_load,
.minimum_version_id = 1,
.minimum_version_id_old = 1,
.fields = (VMStateField []) {
VMSTATE_UINT8(id, IOAPICState),
VMSTATE_UINT8(ioregsel, IOAPICState),
+ VMSTATE_UINT32_V(irr, IOAPICState, 2),
VMSTATE_UINT64_ARRAY(ioredtbl, IOAPICState, IOAPIC_NUM_PINS),
VMSTATE_END_OF_LIST()
}
--
1.7.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [0.14?][PATCH 2/4] ioapic: Save/restore irr
@ 2011-02-03 14:55 ` Jan Kiszka
0 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 14:55 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Gleb Natapov, Marcelo Tosatti, Alexander Graf, kvm, Avi Kivity
This is a guest modifiable state that must be saved/restored properly.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/ioapic.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/hw/ioapic.c b/hw/ioapic.c
index 443c579..c7019f5 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -231,14 +231,25 @@ static void ioapic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t va
}
}
+static int ioapic_pre_load(void *opaque)
+{
+ IOAPICState *s = opaque;
+
+ /* in case we are loading version 1, set these to sane values */
+ s->irr = 0;
+ return 0;
+}
+
static const VMStateDescription vmstate_ioapic = {
.name = "ioapic",
- .version_id = 1,
+ .version_id = 2,
+ .pre_load = ioapic_pre_load,
.minimum_version_id = 1,
.minimum_version_id_old = 1,
.fields = (VMStateField []) {
VMSTATE_UINT8(id, IOAPICState),
VMSTATE_UINT8(ioregsel, IOAPICState),
+ VMSTATE_UINT32_V(irr, IOAPICState, 2),
VMSTATE_UINT64_ARRAY(ioredtbl, IOAPICState, IOAPIC_NUM_PINS),
VMSTATE_END_OF_LIST()
}
--
1.7.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
2011-02-03 14:55 ` [Qemu-devel] " Jan Kiszka
@ 2011-02-03 14:55 ` Jan Kiszka
-1 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 14:55 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: kvm, Alexander Graf, Gleb Natapov, Avi Kivity, Marcelo Tosatti
The registers of real IOAPICs can be relocated during runtime (via
chipset registers). We don't support this yet, but qemu-kvm carries the
current base address in its version 2 vmstate.
To align both implementations for migratability, add the proper
infrastructure to accept initial as well as updated base addresses and
include the current address in the vmstate. This is done in a way that
will also allow multiple IOAPICs in the future.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/ioapic.c | 17 +++++++++++++++++
hw/ioapic.h | 4 ++++
hw/pc_piix.c | 5 ++---
3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/hw/ioapic.c b/hw/ioapic.c
index c7019f5..b53d436 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -63,6 +63,8 @@ struct IOAPICState {
uint32_t irr;
uint64_t ioredtbl[IOAPIC_NUM_PINS];
+ uint64_t default_base_address;
+ uint64_t current_base_address;
};
static IOAPICState *ioapics[MAX_IOAPICS];
@@ -237,6 +239,7 @@ static int ioapic_pre_load(void *opaque)
/* in case we are loading version 1, set these to sane values */
s->irr = 0;
+ s->current_base_address = s->default_base_address;
return 0;
}
@@ -249,6 +252,7 @@ static const VMStateDescription vmstate_ioapic = {
.fields = (VMStateField []) {
VMSTATE_UINT8(id, IOAPICState),
VMSTATE_UINT8(ioregsel, IOAPICState),
+ VMSTATE_UINT64_V(current_base_address, IOAPICState, 2),
VMSTATE_UINT32_V(irr, IOAPICState, 2),
VMSTATE_UINT64_ARRAY(ioredtbl, IOAPICState, IOAPIC_NUM_PINS),
VMSTATE_END_OF_LIST()
@@ -260,6 +264,8 @@ static void ioapic_reset(DeviceState *d)
IOAPICState *s = DO_UPCAST(IOAPICState, busdev.qdev, d);
int i;
+ s->current_base_address = s->default_base_address;
+ sysbus_mmio_map(&s->busdev, 0, s->current_base_address);
s->id = 0;
s->ioregsel = 0;
s->irr = 0;
@@ -279,6 +285,17 @@ static CPUWriteMemoryFunc * const ioapic_mem_write[3] = {
ioapic_mem_writel,
};
+void ioapic_set_base_address(DeviceState *d, target_phys_addr_t addr)
+{
+ IOAPICState *s = DO_UPCAST(IOAPICState, busdev.qdev, d);
+
+ s->current_base_address = addr;
+ if (!s->default_base_address) {
+ s->default_base_address = addr;
+ }
+ sysbus_mmio_map(&s->busdev, 0, addr);
+}
+
static int ioapic_init1(SysBusDevice *dev)
{
IOAPICState *s = FROM_SYSBUS(IOAPICState, dev);
diff --git a/hw/ioapic.h b/hw/ioapic.h
index c441567..1130d60 100644
--- a/hw/ioapic.h
+++ b/hw/ioapic.h
@@ -17,4 +17,8 @@
* License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/
+#define IOAPIC_DEFAULT_BASE_ADDRESS 0xfec00000
+
void ioapic_eio_broadcast(int vector);
+
+void ioapic_set_base_address(DeviceState *d, target_phys_addr_t addr);
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 7b74473..479334f 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -25,6 +25,7 @@
#include "hw.h"
#include "pc.h"
#include "apic.h"
+#include "ioapic.h"
#include "pci.h"
#include "usb-uhci.h"
#include "usb-ohci.h"
@@ -46,13 +47,11 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
static void ioapic_init(IsaIrqState *isa_irq_state)
{
DeviceState *dev;
- SysBusDevice *d;
unsigned int i;
dev = qdev_create(NULL, "ioapic");
qdev_init_nofail(dev);
- d = sysbus_from_qdev(dev);
- sysbus_mmio_map(d, 0, 0xfec00000);
+ ioapic_set_base_address(dev, IOAPIC_DEFAULT_BASE_ADDRESS);
for (i = 0; i < IOAPIC_NUM_PINS; i++) {
isa_irq_state->ioapic[i] = qdev_get_gpio_in(dev, i);
--
1.7.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
@ 2011-02-03 14:55 ` Jan Kiszka
0 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 14:55 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Gleb Natapov, Marcelo Tosatti, Alexander Graf, kvm, Avi Kivity
The registers of real IOAPICs can be relocated during runtime (via
chipset registers). We don't support this yet, but qemu-kvm carries the
current base address in its version 2 vmstate.
To align both implementations for migratability, add the proper
infrastructure to accept initial as well as updated base addresses and
include the current address in the vmstate. This is done in a way that
will also allow multiple IOAPICs in the future.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/ioapic.c | 17 +++++++++++++++++
hw/ioapic.h | 4 ++++
hw/pc_piix.c | 5 ++---
3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/hw/ioapic.c b/hw/ioapic.c
index c7019f5..b53d436 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -63,6 +63,8 @@ struct IOAPICState {
uint32_t irr;
uint64_t ioredtbl[IOAPIC_NUM_PINS];
+ uint64_t default_base_address;
+ uint64_t current_base_address;
};
static IOAPICState *ioapics[MAX_IOAPICS];
@@ -237,6 +239,7 @@ static int ioapic_pre_load(void *opaque)
/* in case we are loading version 1, set these to sane values */
s->irr = 0;
+ s->current_base_address = s->default_base_address;
return 0;
}
@@ -249,6 +252,7 @@ static const VMStateDescription vmstate_ioapic = {
.fields = (VMStateField []) {
VMSTATE_UINT8(id, IOAPICState),
VMSTATE_UINT8(ioregsel, IOAPICState),
+ VMSTATE_UINT64_V(current_base_address, IOAPICState, 2),
VMSTATE_UINT32_V(irr, IOAPICState, 2),
VMSTATE_UINT64_ARRAY(ioredtbl, IOAPICState, IOAPIC_NUM_PINS),
VMSTATE_END_OF_LIST()
@@ -260,6 +264,8 @@ static void ioapic_reset(DeviceState *d)
IOAPICState *s = DO_UPCAST(IOAPICState, busdev.qdev, d);
int i;
+ s->current_base_address = s->default_base_address;
+ sysbus_mmio_map(&s->busdev, 0, s->current_base_address);
s->id = 0;
s->ioregsel = 0;
s->irr = 0;
@@ -279,6 +285,17 @@ static CPUWriteMemoryFunc * const ioapic_mem_write[3] = {
ioapic_mem_writel,
};
+void ioapic_set_base_address(DeviceState *d, target_phys_addr_t addr)
+{
+ IOAPICState *s = DO_UPCAST(IOAPICState, busdev.qdev, d);
+
+ s->current_base_address = addr;
+ if (!s->default_base_address) {
+ s->default_base_address = addr;
+ }
+ sysbus_mmio_map(&s->busdev, 0, addr);
+}
+
static int ioapic_init1(SysBusDevice *dev)
{
IOAPICState *s = FROM_SYSBUS(IOAPICState, dev);
diff --git a/hw/ioapic.h b/hw/ioapic.h
index c441567..1130d60 100644
--- a/hw/ioapic.h
+++ b/hw/ioapic.h
@@ -17,4 +17,8 @@
* License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/
+#define IOAPIC_DEFAULT_BASE_ADDRESS 0xfec00000
+
void ioapic_eio_broadcast(int vector);
+
+void ioapic_set_base_address(DeviceState *d, target_phys_addr_t addr);
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 7b74473..479334f 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -25,6 +25,7 @@
#include "hw.h"
#include "pc.h"
#include "apic.h"
+#include "ioapic.h"
#include "pci.h"
#include "usb-uhci.h"
#include "usb-ohci.h"
@@ -46,13 +47,11 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
static void ioapic_init(IsaIrqState *isa_irq_state)
{
DeviceState *dev;
- SysBusDevice *d;
unsigned int i;
dev = qdev_create(NULL, "ioapic");
qdev_init_nofail(dev);
- d = sysbus_from_qdev(dev);
- sysbus_mmio_map(d, 0, 0xfec00000);
+ ioapic_set_base_address(dev, IOAPIC_DEFAULT_BASE_ADDRESS);
for (i = 0; i < IOAPIC_NUM_PINS; i++) {
isa_irq_state->ioapic[i] = qdev_get_gpio_in(dev, i);
--
1.7.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [0.14?][PATCH 4/4] ioapic: Style & magics cleanup
2011-02-03 14:55 ` [Qemu-devel] " Jan Kiszka
@ 2011-02-03 14:55 ` Jan Kiszka
-1 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 14:55 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: kvm, Alexander Graf, Gleb Natapov, Avi Kivity, Marcelo Tosatti
Fix a few style issues and convert magic numbers into prober symbolic
constants, also fixing the wrong but unused IOAPIC_DM_SIPI value.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/ioapic.c | 177 +++++++++++++++++++++++++++++++++++-----------------------
1 files changed, 107 insertions(+), 70 deletions(-)
diff --git a/hw/ioapic.c b/hw/ioapic.c
index b53d436..a9d7fcd 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -39,20 +39,48 @@
#define MAX_IOAPICS 1
-#define IOAPIC_LVT_MASKED (1 << 16)
-#define IOAPIC_LVT_REMOTE_IRR (1 << 14)
+#define IOAPIC_VERSION 0x11
-#define IOAPIC_TRIGGER_EDGE 0
-#define IOAPIC_TRIGGER_LEVEL 1
+#define IOAPIC_LVT_DEST_SHIFT 56
+#define IOAPIC_LVT_MASKED_SHIFT 16
+#define IOAPIC_LVT_TRIGGER_MODE_SHIFT 15
+#define IOAPIC_LVT_REMOTE_IRR_SHIFT 14
+#define IOAPIC_LVT_POLARITY_SHIFT 13
+#define IOAPIC_LVT_DELIV_STATUS_SHIFT 12
+#define IOAPIC_LVT_DEST_MODE_SHIFT 11
+#define IOAPIC_LVT_DELIV_MODE_SHIFT 8
+
+#define IOAPIC_LVT_MASKED (1 << IOAPIC_LVT_MASKED_SHIFT)
+#define IOAPIC_LVT_REMOTE_IRR (1 << IOAPIC_LVT_REMOTE_IRR_SHIFT)
+
+#define IOAPIC_TRIGGER_EDGE 0
+#define IOAPIC_TRIGGER_LEVEL 1
/*io{apic,sapic} delivery mode*/
-#define IOAPIC_DM_FIXED 0x0
-#define IOAPIC_DM_LOWEST_PRIORITY 0x1
-#define IOAPIC_DM_PMI 0x2
-#define IOAPIC_DM_NMI 0x4
-#define IOAPIC_DM_INIT 0x5
-#define IOAPIC_DM_SIPI 0x5
-#define IOAPIC_DM_EXTINT 0x7
+#define IOAPIC_DM_FIXED 0x0
+#define IOAPIC_DM_LOWEST_PRIORITY 0x1
+#define IOAPIC_DM_PMI 0x2
+#define IOAPIC_DM_NMI 0x4
+#define IOAPIC_DM_INIT 0x5
+#define IOAPIC_DM_SIPI 0x6
+#define IOAPIC_DM_EXTINT 0x7
+#define IOAPIC_DM_MASK 0x7
+
+#define IOAPIC_VECTOR_MASK 0xff
+
+#define IOAPIC_IOREGSEL 0x00
+#define IOAPIC_IOWIN 0x10
+
+#define IOAPIC_REG_ID 0x00
+#define IOAPIC_REG_VER 0x01
+#define IOAPIC_REG_ARB 0x02
+#define IOAPIC_REG_REDTBL_BASE 0x10
+#define IOAPIC_ID 0x00
+
+#define IOAPIC_ID_SHIFT 24
+#define IOAPIC_ID_MASK 0xf
+
+#define IOAPIC_VER_ENTRIES_SHIFT 16
typedef struct IOAPICState IOAPICState;
@@ -60,7 +88,6 @@ struct IOAPICState {
SysBusDevice busdev;
uint8_t id;
uint8_t ioregsel;
-
uint32_t irr;
uint64_t ioredtbl[IOAPIC_NUM_PINS];
uint64_t default_base_address;
@@ -86,21 +113,22 @@ static void ioapic_service(IOAPICState *s)
if (s->irr & mask) {
entry = s->ioredtbl[i];
if (!(entry & IOAPIC_LVT_MASKED)) {
- trig_mode = ((entry >> 15) & 1);
- dest = entry >> 56;
- dest_mode = (entry >> 11) & 1;
- delivery_mode = (entry >> 8) & 7;
- polarity = (entry >> 13) & 1;
+ trig_mode = ((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1);
+ dest = entry >> IOAPIC_LVT_DEST_SHIFT;
+ dest_mode = (entry >> IOAPIC_LVT_DEST_MODE_SHIFT) & 1;
+ delivery_mode =
+ (entry >> IOAPIC_LVT_DELIV_MODE_SHIFT) & IOAPIC_DM_MASK;
+ polarity = (entry >> IOAPIC_LVT_POLARITY_SHIFT) & 1;
if (trig_mode == IOAPIC_TRIGGER_EDGE) {
s->irr &= ~mask;
} else {
s->ioredtbl[i] |= IOAPIC_LVT_REMOTE_IRR;
}
- if (delivery_mode == IOAPIC_DM_EXTINT)
+ if (delivery_mode == IOAPIC_DM_EXTINT) {
vector = pic_read_irq(isa_pic);
- else
- vector = entry & 0xff;
-
+ } else {
+ vector = entry & IOAPIC_VECTOR_MASK;
+ }
apic_deliver_irq(dest, dest_mode, delivery_mode,
vector, polarity, trig_mode);
}
@@ -116,15 +144,16 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
* to GSI 2. GSI maps to ioapic 1-1. This is not
* the cleanest way of doing it but it should work. */
- DPRINTF("%s: %s vec %x\n", __func__, level? "raise" : "lower", vector);
- if (vector == 0)
+ DPRINTF("%s: %s vec %x\n", __func__, level ? "raise" : "lower", vector);
+ if (vector == 0) {
vector = 2;
-
+ }
if (vector >= 0 && vector < IOAPIC_NUM_PINS) {
uint32_t mask = 1 << vector;
uint64_t entry = s->ioredtbl[vector];
- if ((entry >> 15) & 1) {
+ if (((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) ==
+ IOAPIC_TRIGGER_LEVEL) {
/* level triggered */
if (level) {
s->irr |= mask;
@@ -155,7 +184,8 @@ void ioapic_eio_broadcast(int vector)
}
for (n = 0; n < IOAPIC_NUM_PINS; n++) {
entry = s->ioredtbl[n];
- if ((entry & IOAPIC_LVT_REMOTE_IRR) && (entry & 0xff) == vector) {
+ if ((entry & IOAPIC_LVT_REMOTE_IRR)
+ && (entry & IOAPIC_VECTOR_MASK) == vector) {
s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) {
ioapic_service(s);
@@ -171,65 +201,71 @@ static uint32_t ioapic_mem_readl(void *opaque, target_phys_addr_t addr)
int index;
uint32_t val = 0;
- addr &= 0xff;
- if (addr == 0x00) {
+ switch (addr & 0xff) {
+ case IOAPIC_IOREGSEL:
val = s->ioregsel;
- } else if (addr == 0x10) {
+ break;
+ case IOAPIC_IOWIN:
switch (s->ioregsel) {
- case 0x00:
- val = s->id << 24;
- break;
- case 0x01:
- val = 0x11 | ((IOAPIC_NUM_PINS - 1) << 16); /* version 0x11 */
- break;
- case 0x02:
- val = 0;
- break;
- default:
- index = (s->ioregsel - 0x10) >> 1;
- if (index >= 0 && index < IOAPIC_NUM_PINS) {
- if (s->ioregsel & 1)
- val = s->ioredtbl[index] >> 32;
- else
- val = s->ioredtbl[index] & 0xffffffff;
+ case IOAPIC_REG_ID:
+ val = s->id << IOAPIC_ID_SHIFT;
+ break;
+ case IOAPIC_REG_VER:
+ val = IOAPIC_VERSION |
+ ((IOAPIC_NUM_PINS - 1) << IOAPIC_VER_ENTRIES_SHIFT);
+ break;
+ case IOAPIC_REG_ARB:
+ val = 0;
+ break;
+ default:
+ index = (s->ioregsel - IOAPIC_REG_REDTBL_BASE) >> 1;
+ if (index >= 0 && index < IOAPIC_NUM_PINS) {
+ if (s->ioregsel & 1) {
+ val = s->ioredtbl[index] >> 32;
+ } else {
+ val = s->ioredtbl[index] & 0xffffffff;
}
+ }
}
DPRINTF("read: %08x = %08x\n", s->ioregsel, val);
+ break;
}
return val;
}
-static void ioapic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
+static void
+ioapic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
{
IOAPICState *s = opaque;
int index;
- addr &= 0xff;
- if (addr == 0x00) {
+ switch (addr & 0xff) {
+ case IOAPIC_IOREGSEL:
s->ioregsel = val;
- return;
- } else if (addr == 0x10) {
+ break;
+ case IOAPIC_IOWIN:
DPRINTF("write: %08x = %08x\n", s->ioregsel, val);
switch (s->ioregsel) {
- case 0x00:
- s->id = (val >> 24) & 0xff;
- return;
- case 0x01:
- case 0x02:
- return;
- default:
- index = (s->ioregsel - 0x10) >> 1;
- if (index >= 0 && index < IOAPIC_NUM_PINS) {
- if (s->ioregsel & 1) {
- s->ioredtbl[index] &= 0xffffffff;
- s->ioredtbl[index] |= (uint64_t)val << 32;
- } else {
- s->ioredtbl[index] &= ~0xffffffffULL;
- s->ioredtbl[index] |= val;
- }
- ioapic_service(s);
+ case IOAPIC_REG_ID:
+ s->id = (val >> IOAPIC_ID_SHIFT) & IOAPIC_ID_MASK;
+ break;
+ case IOAPIC_REG_VER:
+ case IOAPIC_REG_ARB:
+ break;
+ default:
+ index = (s->ioregsel - IOAPIC_REG_REDTBL_BASE) >> 1;
+ if (index >= 0 && index < IOAPIC_NUM_PINS) {
+ if (s->ioregsel & 1) {
+ s->ioredtbl[index] &= 0xffffffff;
+ s->ioredtbl[index] |= (uint64_t)val << 32;
+ } else {
+ s->ioredtbl[index] &= ~0xffffffffULL;
+ s->ioredtbl[index] |= val;
}
+ ioapic_service(s);
+ }
}
+ break;
}
}
@@ -249,7 +285,7 @@ static const VMStateDescription vmstate_ioapic = {
.pre_load = ioapic_pre_load,
.minimum_version_id = 1,
.minimum_version_id_old = 1,
- .fields = (VMStateField []) {
+ .fields = (VMStateField[]) {
VMSTATE_UINT8(id, IOAPICState),
VMSTATE_UINT8(ioregsel, IOAPICState),
VMSTATE_UINT64_V(current_base_address, IOAPICState, 2),
@@ -269,8 +305,9 @@ static void ioapic_reset(DeviceState *d)
s->id = 0;
s->ioregsel = 0;
s->irr = 0;
- for(i = 0; i < IOAPIC_NUM_PINS; i++)
- s->ioredtbl[i] = 1 << 16; /* mask LVT */
+ for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+ s->ioredtbl[i] = 1 << IOAPIC_LVT_MASKED_SHIFT;
+ }
}
static CPUReadMemoryFunc * const ioapic_mem_read[3] = {
--
1.7.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [0.14?][PATCH 4/4] ioapic: Style & magics cleanup
@ 2011-02-03 14:55 ` Jan Kiszka
0 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 14:55 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Gleb Natapov, Marcelo Tosatti, Alexander Graf, kvm, Avi Kivity
Fix a few style issues and convert magic numbers into prober symbolic
constants, also fixing the wrong but unused IOAPIC_DM_SIPI value.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/ioapic.c | 177 +++++++++++++++++++++++++++++++++++-----------------------
1 files changed, 107 insertions(+), 70 deletions(-)
diff --git a/hw/ioapic.c b/hw/ioapic.c
index b53d436..a9d7fcd 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -39,20 +39,48 @@
#define MAX_IOAPICS 1
-#define IOAPIC_LVT_MASKED (1 << 16)
-#define IOAPIC_LVT_REMOTE_IRR (1 << 14)
+#define IOAPIC_VERSION 0x11
-#define IOAPIC_TRIGGER_EDGE 0
-#define IOAPIC_TRIGGER_LEVEL 1
+#define IOAPIC_LVT_DEST_SHIFT 56
+#define IOAPIC_LVT_MASKED_SHIFT 16
+#define IOAPIC_LVT_TRIGGER_MODE_SHIFT 15
+#define IOAPIC_LVT_REMOTE_IRR_SHIFT 14
+#define IOAPIC_LVT_POLARITY_SHIFT 13
+#define IOAPIC_LVT_DELIV_STATUS_SHIFT 12
+#define IOAPIC_LVT_DEST_MODE_SHIFT 11
+#define IOAPIC_LVT_DELIV_MODE_SHIFT 8
+
+#define IOAPIC_LVT_MASKED (1 << IOAPIC_LVT_MASKED_SHIFT)
+#define IOAPIC_LVT_REMOTE_IRR (1 << IOAPIC_LVT_REMOTE_IRR_SHIFT)
+
+#define IOAPIC_TRIGGER_EDGE 0
+#define IOAPIC_TRIGGER_LEVEL 1
/*io{apic,sapic} delivery mode*/
-#define IOAPIC_DM_FIXED 0x0
-#define IOAPIC_DM_LOWEST_PRIORITY 0x1
-#define IOAPIC_DM_PMI 0x2
-#define IOAPIC_DM_NMI 0x4
-#define IOAPIC_DM_INIT 0x5
-#define IOAPIC_DM_SIPI 0x5
-#define IOAPIC_DM_EXTINT 0x7
+#define IOAPIC_DM_FIXED 0x0
+#define IOAPIC_DM_LOWEST_PRIORITY 0x1
+#define IOAPIC_DM_PMI 0x2
+#define IOAPIC_DM_NMI 0x4
+#define IOAPIC_DM_INIT 0x5
+#define IOAPIC_DM_SIPI 0x6
+#define IOAPIC_DM_EXTINT 0x7
+#define IOAPIC_DM_MASK 0x7
+
+#define IOAPIC_VECTOR_MASK 0xff
+
+#define IOAPIC_IOREGSEL 0x00
+#define IOAPIC_IOWIN 0x10
+
+#define IOAPIC_REG_ID 0x00
+#define IOAPIC_REG_VER 0x01
+#define IOAPIC_REG_ARB 0x02
+#define IOAPIC_REG_REDTBL_BASE 0x10
+#define IOAPIC_ID 0x00
+
+#define IOAPIC_ID_SHIFT 24
+#define IOAPIC_ID_MASK 0xf
+
+#define IOAPIC_VER_ENTRIES_SHIFT 16
typedef struct IOAPICState IOAPICState;
@@ -60,7 +88,6 @@ struct IOAPICState {
SysBusDevice busdev;
uint8_t id;
uint8_t ioregsel;
-
uint32_t irr;
uint64_t ioredtbl[IOAPIC_NUM_PINS];
uint64_t default_base_address;
@@ -86,21 +113,22 @@ static void ioapic_service(IOAPICState *s)
if (s->irr & mask) {
entry = s->ioredtbl[i];
if (!(entry & IOAPIC_LVT_MASKED)) {
- trig_mode = ((entry >> 15) & 1);
- dest = entry >> 56;
- dest_mode = (entry >> 11) & 1;
- delivery_mode = (entry >> 8) & 7;
- polarity = (entry >> 13) & 1;
+ trig_mode = ((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1);
+ dest = entry >> IOAPIC_LVT_DEST_SHIFT;
+ dest_mode = (entry >> IOAPIC_LVT_DEST_MODE_SHIFT) & 1;
+ delivery_mode =
+ (entry >> IOAPIC_LVT_DELIV_MODE_SHIFT) & IOAPIC_DM_MASK;
+ polarity = (entry >> IOAPIC_LVT_POLARITY_SHIFT) & 1;
if (trig_mode == IOAPIC_TRIGGER_EDGE) {
s->irr &= ~mask;
} else {
s->ioredtbl[i] |= IOAPIC_LVT_REMOTE_IRR;
}
- if (delivery_mode == IOAPIC_DM_EXTINT)
+ if (delivery_mode == IOAPIC_DM_EXTINT) {
vector = pic_read_irq(isa_pic);
- else
- vector = entry & 0xff;
-
+ } else {
+ vector = entry & IOAPIC_VECTOR_MASK;
+ }
apic_deliver_irq(dest, dest_mode, delivery_mode,
vector, polarity, trig_mode);
}
@@ -116,15 +144,16 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
* to GSI 2. GSI maps to ioapic 1-1. This is not
* the cleanest way of doing it but it should work. */
- DPRINTF("%s: %s vec %x\n", __func__, level? "raise" : "lower", vector);
- if (vector == 0)
+ DPRINTF("%s: %s vec %x\n", __func__, level ? "raise" : "lower", vector);
+ if (vector == 0) {
vector = 2;
-
+ }
if (vector >= 0 && vector < IOAPIC_NUM_PINS) {
uint32_t mask = 1 << vector;
uint64_t entry = s->ioredtbl[vector];
- if ((entry >> 15) & 1) {
+ if (((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) ==
+ IOAPIC_TRIGGER_LEVEL) {
/* level triggered */
if (level) {
s->irr |= mask;
@@ -155,7 +184,8 @@ void ioapic_eio_broadcast(int vector)
}
for (n = 0; n < IOAPIC_NUM_PINS; n++) {
entry = s->ioredtbl[n];
- if ((entry & IOAPIC_LVT_REMOTE_IRR) && (entry & 0xff) == vector) {
+ if ((entry & IOAPIC_LVT_REMOTE_IRR)
+ && (entry & IOAPIC_VECTOR_MASK) == vector) {
s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) {
ioapic_service(s);
@@ -171,65 +201,71 @@ static uint32_t ioapic_mem_readl(void *opaque, target_phys_addr_t addr)
int index;
uint32_t val = 0;
- addr &= 0xff;
- if (addr == 0x00) {
+ switch (addr & 0xff) {
+ case IOAPIC_IOREGSEL:
val = s->ioregsel;
- } else if (addr == 0x10) {
+ break;
+ case IOAPIC_IOWIN:
switch (s->ioregsel) {
- case 0x00:
- val = s->id << 24;
- break;
- case 0x01:
- val = 0x11 | ((IOAPIC_NUM_PINS - 1) << 16); /* version 0x11 */
- break;
- case 0x02:
- val = 0;
- break;
- default:
- index = (s->ioregsel - 0x10) >> 1;
- if (index >= 0 && index < IOAPIC_NUM_PINS) {
- if (s->ioregsel & 1)
- val = s->ioredtbl[index] >> 32;
- else
- val = s->ioredtbl[index] & 0xffffffff;
+ case IOAPIC_REG_ID:
+ val = s->id << IOAPIC_ID_SHIFT;
+ break;
+ case IOAPIC_REG_VER:
+ val = IOAPIC_VERSION |
+ ((IOAPIC_NUM_PINS - 1) << IOAPIC_VER_ENTRIES_SHIFT);
+ break;
+ case IOAPIC_REG_ARB:
+ val = 0;
+ break;
+ default:
+ index = (s->ioregsel - IOAPIC_REG_REDTBL_BASE) >> 1;
+ if (index >= 0 && index < IOAPIC_NUM_PINS) {
+ if (s->ioregsel & 1) {
+ val = s->ioredtbl[index] >> 32;
+ } else {
+ val = s->ioredtbl[index] & 0xffffffff;
}
+ }
}
DPRINTF("read: %08x = %08x\n", s->ioregsel, val);
+ break;
}
return val;
}
-static void ioapic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
+static void
+ioapic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
{
IOAPICState *s = opaque;
int index;
- addr &= 0xff;
- if (addr == 0x00) {
+ switch (addr & 0xff) {
+ case IOAPIC_IOREGSEL:
s->ioregsel = val;
- return;
- } else if (addr == 0x10) {
+ break;
+ case IOAPIC_IOWIN:
DPRINTF("write: %08x = %08x\n", s->ioregsel, val);
switch (s->ioregsel) {
- case 0x00:
- s->id = (val >> 24) & 0xff;
- return;
- case 0x01:
- case 0x02:
- return;
- default:
- index = (s->ioregsel - 0x10) >> 1;
- if (index >= 0 && index < IOAPIC_NUM_PINS) {
- if (s->ioregsel & 1) {
- s->ioredtbl[index] &= 0xffffffff;
- s->ioredtbl[index] |= (uint64_t)val << 32;
- } else {
- s->ioredtbl[index] &= ~0xffffffffULL;
- s->ioredtbl[index] |= val;
- }
- ioapic_service(s);
+ case IOAPIC_REG_ID:
+ s->id = (val >> IOAPIC_ID_SHIFT) & IOAPIC_ID_MASK;
+ break;
+ case IOAPIC_REG_VER:
+ case IOAPIC_REG_ARB:
+ break;
+ default:
+ index = (s->ioregsel - IOAPIC_REG_REDTBL_BASE) >> 1;
+ if (index >= 0 && index < IOAPIC_NUM_PINS) {
+ if (s->ioregsel & 1) {
+ s->ioredtbl[index] &= 0xffffffff;
+ s->ioredtbl[index] |= (uint64_t)val << 32;
+ } else {
+ s->ioredtbl[index] &= ~0xffffffffULL;
+ s->ioredtbl[index] |= val;
}
+ ioapic_service(s);
+ }
}
+ break;
}
}
@@ -249,7 +285,7 @@ static const VMStateDescription vmstate_ioapic = {
.pre_load = ioapic_pre_load,
.minimum_version_id = 1,
.minimum_version_id_old = 1,
- .fields = (VMStateField []) {
+ .fields = (VMStateField[]) {
VMSTATE_UINT8(id, IOAPICState),
VMSTATE_UINT8(ioregsel, IOAPICState),
VMSTATE_UINT64_V(current_base_address, IOAPICState, 2),
@@ -269,8 +305,9 @@ static void ioapic_reset(DeviceState *d)
s->id = 0;
s->ioregsel = 0;
s->irr = 0;
- for(i = 0; i < IOAPIC_NUM_PINS; i++)
- s->ioredtbl[i] = 1 << 16; /* mask LVT */
+ for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+ s->ioredtbl[i] = 1 << IOAPIC_LVT_MASKED_SHIFT;
+ }
}
static CPUReadMemoryFunc * const ioapic_mem_read[3] = {
--
1.7.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
2011-02-03 14:55 ` [Qemu-devel] " Jan Kiszka
@ 2011-02-03 17:03 ` Blue Swirl
-1 siblings, 0 replies; 36+ messages in thread
From: Blue Swirl @ 2011-02-03 17:03 UTC (permalink / raw)
To: Jan Kiszka
Cc: qemu-devel, Anthony Liguori, Gleb Natapov, Marcelo Tosatti,
Alexander Graf, kvm, Avi Kivity
On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> The registers of real IOAPICs can be relocated during runtime (via
> chipset registers). We don't support this yet, but qemu-kvm carries the
> current base address in its version 2 vmstate.
>
> To align both implementations for migratability, add the proper
> infrastructure to accept initial as well as updated base addresses and
> include the current address in the vmstate. This is done in a way that
> will also allow multiple IOAPICs in the future.
Nack, the addresses should be device properties.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
@ 2011-02-03 17:03 ` Blue Swirl
0 siblings, 0 replies; 36+ messages in thread
From: Blue Swirl @ 2011-02-03 17:03 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Gleb Natapov, kvm, Marcelo Tosatti, qemu-devel,
Alexander Graf, Avi Kivity
On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> The registers of real IOAPICs can be relocated during runtime (via
> chipset registers). We don't support this yet, but qemu-kvm carries the
> current base address in its version 2 vmstate.
>
> To align both implementations for migratability, add the proper
> infrastructure to accept initial as well as updated base addresses and
> include the current address in the vmstate. This is done in a way that
> will also allow multiple IOAPICs in the future.
Nack, the addresses should be device properties.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
2011-02-03 17:03 ` Blue Swirl
@ 2011-02-03 17:18 ` Jan Kiszka
-1 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 17:18 UTC (permalink / raw)
To: Blue Swirl
Cc: qemu-devel, Anthony Liguori, Gleb Natapov, Marcelo Tosatti,
Alexander Graf, kvm, Avi Kivity
On 2011-02-03 18:03, Blue Swirl wrote:
> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> The registers of real IOAPICs can be relocated during runtime (via
>> chipset registers). We don't support this yet, but qemu-kvm carries the
>> current base address in its version 2 vmstate.
>>
>> To align both implementations for migratability, add the proper
>> infrastructure to accept initial as well as updated base addresses and
>> include the current address in the vmstate. This is done in a way that
>> will also allow multiple IOAPICs in the future.
>
> Nack, the addresses should be device properties.
Hmm.... we could make default_base_address a property. Will change that.
But current_base_address is just the same as apicbase and can't be a
property.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
@ 2011-02-03 17:18 ` Jan Kiszka
0 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 17:18 UTC (permalink / raw)
To: Blue Swirl
Cc: Anthony Liguori, Gleb Natapov, kvm, Marcelo Tosatti, qemu-devel,
Alexander Graf, Avi Kivity
On 2011-02-03 18:03, Blue Swirl wrote:
> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> The registers of real IOAPICs can be relocated during runtime (via
>> chipset registers). We don't support this yet, but qemu-kvm carries the
>> current base address in its version 2 vmstate.
>>
>> To align both implementations for migratability, add the proper
>> infrastructure to accept initial as well as updated base addresses and
>> include the current address in the vmstate. This is done in a way that
>> will also allow multiple IOAPICs in the future.
>
> Nack, the addresses should be device properties.
Hmm.... we could make default_base_address a property. Will change that.
But current_base_address is just the same as apicbase and can't be a
property.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
2011-02-03 17:18 ` Jan Kiszka
@ 2011-02-03 17:36 ` Blue Swirl
-1 siblings, 0 replies; 36+ messages in thread
From: Blue Swirl @ 2011-02-03 17:36 UTC (permalink / raw)
To: Jan Kiszka
Cc: qemu-devel, Anthony Liguori, Gleb Natapov, Marcelo Tosatti,
Alexander Graf, kvm, Avi Kivity
On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-02-03 18:03, Blue Swirl wrote:
>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> The registers of real IOAPICs can be relocated during runtime (via
>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>> current base address in its version 2 vmstate.
>>>
>>> To align both implementations for migratability, add the proper
>>> infrastructure to accept initial as well as updated base addresses and
>>> include the current address in the vmstate. This is done in a way that
>>> will also allow multiple IOAPICs in the future.
>>
>> Nack, the addresses should be device properties.
>
> Hmm.... we could make default_base_address a property. Will change that.
> But current_base_address is just the same as apicbase and can't be a
> property.
Oh, right. What will current_base_address used for? Why can't board
just unmap IOAPIC from current address and remap it at the new
address? Then the device would not need to know its base address.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
@ 2011-02-03 17:36 ` Blue Swirl
0 siblings, 0 replies; 36+ messages in thread
From: Blue Swirl @ 2011-02-03 17:36 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Gleb Natapov, kvm, Marcelo Tosatti, qemu-devel,
Alexander Graf, Avi Kivity
On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-02-03 18:03, Blue Swirl wrote:
>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> The registers of real IOAPICs can be relocated during runtime (via
>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>> current base address in its version 2 vmstate.
>>>
>>> To align both implementations for migratability, add the proper
>>> infrastructure to accept initial as well as updated base addresses and
>>> include the current address in the vmstate. This is done in a way that
>>> will also allow multiple IOAPICs in the future.
>>
>> Nack, the addresses should be device properties.
>
> Hmm.... we could make default_base_address a property. Will change that.
> But current_base_address is just the same as apicbase and can't be a
> property.
Oh, right. What will current_base_address used for? Why can't board
just unmap IOAPIC from current address and remap it at the new
address? Then the device would not need to know its base address.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
2011-02-03 17:36 ` Blue Swirl
@ 2011-02-03 17:43 ` Jan Kiszka
-1 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 17:43 UTC (permalink / raw)
To: Blue Swirl
Cc: qemu-devel, Anthony Liguori, Gleb Natapov, Marcelo Tosatti,
Alexander Graf, kvm, Avi Kivity
On 2011-02-03 18:36, Blue Swirl wrote:
> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2011-02-03 18:03, Blue Swirl wrote:
>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>> current base address in its version 2 vmstate.
>>>>
>>>> To align both implementations for migratability, add the proper
>>>> infrastructure to accept initial as well as updated base addresses and
>>>> include the current address in the vmstate. This is done in a way that
>>>> will also allow multiple IOAPICs in the future.
>>>
>>> Nack, the addresses should be device properties.
>>
>> Hmm.... we could make default_base_address a property. Will change that.
>> But current_base_address is just the same as apicbase and can't be a
>> property.
>
> Oh, right. What will current_base_address used for? Why can't board
> just unmap IOAPIC from current address and remap it at the new
> address? Then the device would not need to know its base address.
The board could do this. The question is where we put this service, in
the context if the IOAPIC as ioapic_set_base_address (compare to
cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
each and every board code. In the latter case, the boards would also be
responsible for saving/restoring the address.
Well, unless we have a good reason, I would prefer to keep the split as
suggested, also to avoid that qemu-kvm needs to break its vmstate.
Jan
PS: Another bug in the APIC emulation: cpu_set_apic_base lacks a call to
sysbus_mmio_map to update its mapping.
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
@ 2011-02-03 17:43 ` Jan Kiszka
0 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 17:43 UTC (permalink / raw)
To: Blue Swirl
Cc: Anthony Liguori, Gleb Natapov, kvm, Marcelo Tosatti, qemu-devel,
Alexander Graf, Avi Kivity
On 2011-02-03 18:36, Blue Swirl wrote:
> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2011-02-03 18:03, Blue Swirl wrote:
>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>> current base address in its version 2 vmstate.
>>>>
>>>> To align both implementations for migratability, add the proper
>>>> infrastructure to accept initial as well as updated base addresses and
>>>> include the current address in the vmstate. This is done in a way that
>>>> will also allow multiple IOAPICs in the future.
>>>
>>> Nack, the addresses should be device properties.
>>
>> Hmm.... we could make default_base_address a property. Will change that.
>> But current_base_address is just the same as apicbase and can't be a
>> property.
>
> Oh, right. What will current_base_address used for? Why can't board
> just unmap IOAPIC from current address and remap it at the new
> address? Then the device would not need to know its base address.
The board could do this. The question is where we put this service, in
the context if the IOAPIC as ioapic_set_base_address (compare to
cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
each and every board code. In the latter case, the boards would also be
responsible for saving/restoring the address.
Well, unless we have a good reason, I would prefer to keep the split as
suggested, also to avoid that qemu-kvm needs to break its vmstate.
Jan
PS: Another bug in the APIC emulation: cpu_set_apic_base lacks a call to
sysbus_mmio_map to update its mapping.
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
2011-02-03 17:43 ` Jan Kiszka
@ 2011-02-03 17:54 ` Blue Swirl
-1 siblings, 0 replies; 36+ messages in thread
From: Blue Swirl @ 2011-02-03 17:54 UTC (permalink / raw)
To: Jan Kiszka
Cc: qemu-devel, Anthony Liguori, Gleb Natapov, Marcelo Tosatti,
Alexander Graf, kvm, Avi Kivity
On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-02-03 18:36, Blue Swirl wrote:
>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>> current base address in its version 2 vmstate.
>>>>>
>>>>> To align both implementations for migratability, add the proper
>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>> include the current address in the vmstate. This is done in a way that
>>>>> will also allow multiple IOAPICs in the future.
>>>>
>>>> Nack, the addresses should be device properties.
>>>
>>> Hmm.... we could make default_base_address a property. Will change that.
>>> But current_base_address is just the same as apicbase and can't be a
>>> property.
>>
>> Oh, right. What will current_base_address used for? Why can't board
>> just unmap IOAPIC from current address and remap it at the new
>> address? Then the device would not need to know its base address.
>
> The board could do this. The question is where we put this service, in
> the context if the IOAPIC as ioapic_set_base_address (compare to
> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
> each and every board code. In the latter case, the boards would also be
> responsible for saving/restoring the address.
How is the device relocated? Where are the chipset registers you mention?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
@ 2011-02-03 17:54 ` Blue Swirl
0 siblings, 0 replies; 36+ messages in thread
From: Blue Swirl @ 2011-02-03 17:54 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Gleb Natapov, kvm, Marcelo Tosatti, qemu-devel,
Alexander Graf, Avi Kivity
On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-02-03 18:36, Blue Swirl wrote:
>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>> current base address in its version 2 vmstate.
>>>>>
>>>>> To align both implementations for migratability, add the proper
>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>> include the current address in the vmstate. This is done in a way that
>>>>> will also allow multiple IOAPICs in the future.
>>>>
>>>> Nack, the addresses should be device properties.
>>>
>>> Hmm.... we could make default_base_address a property. Will change that.
>>> But current_base_address is just the same as apicbase and can't be a
>>> property.
>>
>> Oh, right. What will current_base_address used for? Why can't board
>> just unmap IOAPIC from current address and remap it at the new
>> address? Then the device would not need to know its base address.
>
> The board could do this. The question is where we put this service, in
> the context if the IOAPIC as ioapic_set_base_address (compare to
> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
> each and every board code. In the latter case, the boards would also be
> responsible for saving/restoring the address.
How is the device relocated? Where are the chipset registers you mention?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
2011-02-03 17:54 ` Blue Swirl
@ 2011-02-03 18:01 ` Jan Kiszka
-1 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 18:01 UTC (permalink / raw)
To: Blue Swirl
Cc: qemu-devel, Anthony Liguori, Gleb Natapov, Marcelo Tosatti,
Alexander Graf, kvm, Avi Kivity
On 2011-02-03 18:54, Blue Swirl wrote:
> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2011-02-03 18:36, Blue Swirl wrote:
>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>> current base address in its version 2 vmstate.
>>>>>>
>>>>>> To align both implementations for migratability, add the proper
>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>> will also allow multiple IOAPICs in the future.
>>>>>
>>>>> Nack, the addresses should be device properties.
>>>>
>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>> But current_base_address is just the same as apicbase and can't be a
>>>> property.
>>>
>>> Oh, right. What will current_base_address used for? Why can't board
>>> just unmap IOAPIC from current address and remap it at the new
>>> address? Then the device would not need to know its base address.
>>
>> The board could do this. The question is where we put this service, in
>> the context if the IOAPIC as ioapic_set_base_address (compare to
>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>> each and every board code. In the latter case, the boards would also be
>> responsible for saving/restoring the address.
>
> How is the device relocated? Where are the chipset registers you mention?
Intel's PIIX chipsets contain a register called APICBASE (but it means
the IOAPIC), and that defines the location. The analogy in the APIC
world is the MSR_IA32_APICBASE which we maintain via the APIC state.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
@ 2011-02-03 18:01 ` Jan Kiszka
0 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 18:01 UTC (permalink / raw)
To: Blue Swirl
Cc: Anthony Liguori, Gleb Natapov, kvm, Marcelo Tosatti, qemu-devel,
Alexander Graf, Avi Kivity
On 2011-02-03 18:54, Blue Swirl wrote:
> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2011-02-03 18:36, Blue Swirl wrote:
>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>> current base address in its version 2 vmstate.
>>>>>>
>>>>>> To align both implementations for migratability, add the proper
>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>> will also allow multiple IOAPICs in the future.
>>>>>
>>>>> Nack, the addresses should be device properties.
>>>>
>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>> But current_base_address is just the same as apicbase and can't be a
>>>> property.
>>>
>>> Oh, right. What will current_base_address used for? Why can't board
>>> just unmap IOAPIC from current address and remap it at the new
>>> address? Then the device would not need to know its base address.
>>
>> The board could do this. The question is where we put this service, in
>> the context if the IOAPIC as ioapic_set_base_address (compare to
>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>> each and every board code. In the latter case, the boards would also be
>> responsible for saving/restoring the address.
>
> How is the device relocated? Where are the chipset registers you mention?
Intel's PIIX chipsets contain a register called APICBASE (but it means
the IOAPIC), and that defines the location. The analogy in the APIC
world is the MSR_IA32_APICBASE which we maintain via the APIC state.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
2011-02-03 18:01 ` Jan Kiszka
@ 2011-02-03 19:01 ` Blue Swirl
-1 siblings, 0 replies; 36+ messages in thread
From: Blue Swirl @ 2011-02-03 19:01 UTC (permalink / raw)
To: Jan Kiszka
Cc: qemu-devel, Anthony Liguori, Gleb Natapov, Marcelo Tosatti,
Alexander Graf, kvm, Avi Kivity
On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-02-03 18:54, Blue Swirl wrote:
>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2011-02-03 18:36, Blue Swirl wrote:
>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>>> current base address in its version 2 vmstate.
>>>>>>>
>>>>>>> To align both implementations for migratability, add the proper
>>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>>> will also allow multiple IOAPICs in the future.
>>>>>>
>>>>>> Nack, the addresses should be device properties.
>>>>>
>>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>>> But current_base_address is just the same as apicbase and can't be a
>>>>> property.
>>>>
>>>> Oh, right. What will current_base_address used for? Why can't board
>>>> just unmap IOAPIC from current address and remap it at the new
>>>> address? Then the device would not need to know its base address.
>>>
>>> The board could do this. The question is where we put this service, in
>>> the context if the IOAPIC as ioapic_set_base_address (compare to
>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>>> each and every board code. In the latter case, the boards would also be
>>> responsible for saving/restoring the address.
>>
>> How is the device relocated? Where are the chipset registers you mention?
>
> Intel's PIIX chipsets contain a register called APICBASE (but it means
> the IOAPIC), and that defines the location. The analogy in the APIC
> world is the MSR_IA32_APICBASE which we maintain via the APIC state.
In ICH10 the register is called OIC—Other Interrupt Control Register
and the interesting bits APIC Range Select (ASEL).
So actually PIIX should manage IOAPIC mapping, not board level.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
@ 2011-02-03 19:01 ` Blue Swirl
0 siblings, 0 replies; 36+ messages in thread
From: Blue Swirl @ 2011-02-03 19:01 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Gleb Natapov, kvm, Marcelo Tosatti, qemu-devel,
Alexander Graf, Avi Kivity
On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-02-03 18:54, Blue Swirl wrote:
>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2011-02-03 18:36, Blue Swirl wrote:
>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>>> current base address in its version 2 vmstate.
>>>>>>>
>>>>>>> To align both implementations for migratability, add the proper
>>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>>> will also allow multiple IOAPICs in the future.
>>>>>>
>>>>>> Nack, the addresses should be device properties.
>>>>>
>>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>>> But current_base_address is just the same as apicbase and can't be a
>>>>> property.
>>>>
>>>> Oh, right. What will current_base_address used for? Why can't board
>>>> just unmap IOAPIC from current address and remap it at the new
>>>> address? Then the device would not need to know its base address.
>>>
>>> The board could do this. The question is where we put this service, in
>>> the context if the IOAPIC as ioapic_set_base_address (compare to
>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>>> each and every board code. In the latter case, the boards would also be
>>> responsible for saving/restoring the address.
>>
>> How is the device relocated? Where are the chipset registers you mention?
>
> Intel's PIIX chipsets contain a register called APICBASE (but it means
> the IOAPIC), and that defines the location. The analogy in the APIC
> world is the MSR_IA32_APICBASE which we maintain via the APIC state.
In ICH10 the register is called OIC—Other Interrupt Control Register
and the interesting bits APIC Range Select (ASEL).
So actually PIIX should manage IOAPIC mapping, not board level.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
2011-02-03 19:01 ` Blue Swirl
@ 2011-02-03 19:06 ` Jan Kiszka
-1 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 19:06 UTC (permalink / raw)
To: Blue Swirl
Cc: Anthony Liguori, Gleb Natapov, kvm, Marcelo Tosatti, qemu-devel,
Alexander Graf, Avi Kivity
On 2011-02-03 20:01, Blue Swirl wrote:
> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2011-02-03 18:54, Blue Swirl wrote:
>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2011-02-03 18:36, Blue Swirl wrote:
>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>>>> current base address in its version 2 vmstate.
>>>>>>>>
>>>>>>>> To align both implementations for migratability, add the proper
>>>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>>>> will also allow multiple IOAPICs in the future.
>>>>>>>
>>>>>>> Nack, the addresses should be device properties.
>>>>>>
>>>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>>>> But current_base_address is just the same as apicbase and can't be a
>>>>>> property.
>>>>>
>>>>> Oh, right. What will current_base_address used for? Why can't board
>>>>> just unmap IOAPIC from current address and remap it at the new
>>>>> address? Then the device would not need to know its base address.
>>>>
>>>> The board could do this. The question is where we put this service, in
>>>> the context if the IOAPIC as ioapic_set_base_address (compare to
>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>>>> each and every board code. In the latter case, the boards would also be
>>>> responsible for saving/restoring the address.
>>>
>>> How is the device relocated? Where are the chipset registers you mention?
>>
>> Intel's PIIX chipsets contain a register called APICBASE (but it means
>> the IOAPIC), and that defines the location. The analogy in the APIC
>> world is the MSR_IA32_APICBASE which we maintain via the APIC state.
>
> In ICH10 the register is called OIC—Other Interrupt Control Register
> and the interesting bits APIC Range Select (ASEL).
>
> So actually PIIX should manage IOAPIC mapping, not board level.
The point is we need ioapic_set_base_address logic in multiple places
(once chipsets start to implement it). Better push it to a central place
from the beginning. Also the bit keeping. There is no difference to
apicbase.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
@ 2011-02-03 19:06 ` Jan Kiszka
0 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 19:06 UTC (permalink / raw)
To: Blue Swirl
Cc: Anthony Liguori, Gleb Natapov, kvm, Marcelo Tosatti, qemu-devel,
Alexander Graf, Avi Kivity
On 2011-02-03 20:01, Blue Swirl wrote:
> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2011-02-03 18:54, Blue Swirl wrote:
>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2011-02-03 18:36, Blue Swirl wrote:
>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>>>> current base address in its version 2 vmstate.
>>>>>>>>
>>>>>>>> To align both implementations for migratability, add the proper
>>>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>>>> will also allow multiple IOAPICs in the future.
>>>>>>>
>>>>>>> Nack, the addresses should be device properties.
>>>>>>
>>>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>>>> But current_base_address is just the same as apicbase and can't be a
>>>>>> property.
>>>>>
>>>>> Oh, right. What will current_base_address used for? Why can't board
>>>>> just unmap IOAPIC from current address and remap it at the new
>>>>> address? Then the device would not need to know its base address.
>>>>
>>>> The board could do this. The question is where we put this service, in
>>>> the context if the IOAPIC as ioapic_set_base_address (compare to
>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>>>> each and every board code. In the latter case, the boards would also be
>>>> responsible for saving/restoring the address.
>>>
>>> How is the device relocated? Where are the chipset registers you mention?
>>
>> Intel's PIIX chipsets contain a register called APICBASE (but it means
>> the IOAPIC), and that defines the location. The analogy in the APIC
>> world is the MSR_IA32_APICBASE which we maintain via the APIC state.
>
> In ICH10 the register is called OIC—Other Interrupt Control Register
> and the interesting bits APIC Range Select (ASEL).
>
> So actually PIIX should manage IOAPIC mapping, not board level.
The point is we need ioapic_set_base_address logic in multiple places
(once chipsets start to implement it). Better push it to a central place
from the beginning. Also the bit keeping. There is no difference to
apicbase.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
2011-02-03 19:06 ` [Qemu-devel] " Jan Kiszka
@ 2011-02-03 19:11 ` Blue Swirl
-1 siblings, 0 replies; 36+ messages in thread
From: Blue Swirl @ 2011-02-03 19:11 UTC (permalink / raw)
To: Jan Kiszka
Cc: qemu-devel, Anthony Liguori, Gleb Natapov, Marcelo Tosatti,
Alexander Graf, kvm, Avi Kivity
On Thu, Feb 3, 2011 at 7:06 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-02-03 20:01, Blue Swirl wrote:
>> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2011-02-03 18:54, Blue Swirl wrote:
>>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> On 2011-02-03 18:36, Blue Swirl wrote:
>>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>>>>> current base address in its version 2 vmstate.
>>>>>>>>>
>>>>>>>>> To align both implementations for migratability, add the proper
>>>>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>>>>> will also allow multiple IOAPICs in the future.
>>>>>>>>
>>>>>>>> Nack, the addresses should be device properties.
>>>>>>>
>>>>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>>>>> But current_base_address is just the same as apicbase and can't be a
>>>>>>> property.
>>>>>>
>>>>>> Oh, right. What will current_base_address used for? Why can't board
>>>>>> just unmap IOAPIC from current address and remap it at the new
>>>>>> address? Then the device would not need to know its base address.
>>>>>
>>>>> The board could do this. The question is where we put this service, in
>>>>> the context if the IOAPIC as ioapic_set_base_address (compare to
>>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>>>>> each and every board code. In the latter case, the boards would also be
>>>>> responsible for saving/restoring the address.
>>>>
>>>> How is the device relocated? Where are the chipset registers you mention?
>>>
>>> Intel's PIIX chipsets contain a register called APICBASE (but it means
>>> the IOAPIC), and that defines the location. The analogy in the APIC
>>> world is the MSR_IA32_APICBASE which we maintain via the APIC state.
>>
>> In ICH10 the register is called OIC—Other Interrupt Control Register
>> and the interesting bits APIC Range Select (ASEL).
>>
>> So actually PIIX should manage IOAPIC mapping, not board level.
>
> The point is we need ioapic_set_base_address logic in multiple places
> (once chipsets start to implement it). Better push it to a central place
> from the beginning. Also the bit keeping. There is no difference to
> apicbase.
In that case, the function should be made inline version in ioapic.h.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
@ 2011-02-03 19:11 ` Blue Swirl
0 siblings, 0 replies; 36+ messages in thread
From: Blue Swirl @ 2011-02-03 19:11 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Gleb Natapov, kvm, Marcelo Tosatti, qemu-devel,
Alexander Graf, Avi Kivity
On Thu, Feb 3, 2011 at 7:06 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-02-03 20:01, Blue Swirl wrote:
>> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2011-02-03 18:54, Blue Swirl wrote:
>>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> On 2011-02-03 18:36, Blue Swirl wrote:
>>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>>>>> current base address in its version 2 vmstate.
>>>>>>>>>
>>>>>>>>> To align both implementations for migratability, add the proper
>>>>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>>>>> will also allow multiple IOAPICs in the future.
>>>>>>>>
>>>>>>>> Nack, the addresses should be device properties.
>>>>>>>
>>>>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>>>>> But current_base_address is just the same as apicbase and can't be a
>>>>>>> property.
>>>>>>
>>>>>> Oh, right. What will current_base_address used for? Why can't board
>>>>>> just unmap IOAPIC from current address and remap it at the new
>>>>>> address? Then the device would not need to know its base address.
>>>>>
>>>>> The board could do this. The question is where we put this service, in
>>>>> the context if the IOAPIC as ioapic_set_base_address (compare to
>>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>>>>> each and every board code. In the latter case, the boards would also be
>>>>> responsible for saving/restoring the address.
>>>>
>>>> How is the device relocated? Where are the chipset registers you mention?
>>>
>>> Intel's PIIX chipsets contain a register called APICBASE (but it means
>>> the IOAPIC), and that defines the location. The analogy in the APIC
>>> world is the MSR_IA32_APICBASE which we maintain via the APIC state.
>>
>> In ICH10 the register is called OIC—Other Interrupt Control Register
>> and the interesting bits APIC Range Select (ASEL).
>>
>> So actually PIIX should manage IOAPIC mapping, not board level.
>
> The point is we need ioapic_set_base_address logic in multiple places
> (once chipsets start to implement it). Better push it to a central place
> from the beginning. Also the bit keeping. There is no difference to
> apicbase.
In that case, the function should be made inline version in ioapic.h.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
2011-02-03 19:11 ` Blue Swirl
@ 2011-02-03 19:25 ` Jan Kiszka
-1 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 19:25 UTC (permalink / raw)
To: Blue Swirl
Cc: qemu-devel, Anthony Liguori, Gleb Natapov, Marcelo Tosatti,
Alexander Graf, kvm, Avi Kivity
On 2011-02-03 20:11, Blue Swirl wrote:
> On Thu, Feb 3, 2011 at 7:06 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2011-02-03 20:01, Blue Swirl wrote:
>>> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2011-02-03 18:54, Blue Swirl wrote:
>>>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>> On 2011-02-03 18:36, Blue Swirl wrote:
>>>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>>>>>> current base address in its version 2 vmstate.
>>>>>>>>>>
>>>>>>>>>> To align both implementations for migratability, add the proper
>>>>>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>>>>>> will also allow multiple IOAPICs in the future.
>>>>>>>>>
>>>>>>>>> Nack, the addresses should be device properties.
>>>>>>>>
>>>>>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>>>>>> But current_base_address is just the same as apicbase and can't be a
>>>>>>>> property.
>>>>>>>
>>>>>>> Oh, right. What will current_base_address used for? Why can't board
>>>>>>> just unmap IOAPIC from current address and remap it at the new
>>>>>>> address? Then the device would not need to know its base address.
>>>>>>
>>>>>> The board could do this. The question is where we put this service, in
>>>>>> the context if the IOAPIC as ioapic_set_base_address (compare to
>>>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>>>>>> each and every board code. In the latter case, the boards would also be
>>>>>> responsible for saving/restoring the address.
>>>>>
>>>>> How is the device relocated? Where are the chipset registers you mention?
>>>>
>>>> Intel's PIIX chipsets contain a register called APICBASE (but it means
>>>> the IOAPIC), and that defines the location. The analogy in the APIC
>>>> world is the MSR_IA32_APICBASE which we maintain via the APIC state.
>>>
>>> In ICH10 the register is called OIC—Other Interrupt Control Register
>>> and the interesting bits APIC Range Select (ASEL).
>>>
>>> So actually PIIX should manage IOAPIC mapping, not board level.
>>
>> The point is we need ioapic_set_base_address logic in multiple places
>> (once chipsets start to implement it). Better push it to a central place
>> from the beginning. Also the bit keeping. There is no difference to
>> apicbase.
>
> In that case, the function should be made inline version in ioapic.h.
That still replicates the bit keeping.
I don't see the benefit of moving it over, even less when we want to
consolidate with a vmstate layout that is already in use.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
@ 2011-02-03 19:25 ` Jan Kiszka
0 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 19:25 UTC (permalink / raw)
To: Blue Swirl
Cc: Anthony Liguori, Gleb Natapov, kvm, Marcelo Tosatti, qemu-devel,
Alexander Graf, Avi Kivity
On 2011-02-03 20:11, Blue Swirl wrote:
> On Thu, Feb 3, 2011 at 7:06 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2011-02-03 20:01, Blue Swirl wrote:
>>> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2011-02-03 18:54, Blue Swirl wrote:
>>>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>> On 2011-02-03 18:36, Blue Swirl wrote:
>>>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>>>>>> current base address in its version 2 vmstate.
>>>>>>>>>>
>>>>>>>>>> To align both implementations for migratability, add the proper
>>>>>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>>>>>> will also allow multiple IOAPICs in the future.
>>>>>>>>>
>>>>>>>>> Nack, the addresses should be device properties.
>>>>>>>>
>>>>>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>>>>>> But current_base_address is just the same as apicbase and can't be a
>>>>>>>> property.
>>>>>>>
>>>>>>> Oh, right. What will current_base_address used for? Why can't board
>>>>>>> just unmap IOAPIC from current address and remap it at the new
>>>>>>> address? Then the device would not need to know its base address.
>>>>>>
>>>>>> The board could do this. The question is where we put this service, in
>>>>>> the context if the IOAPIC as ioapic_set_base_address (compare to
>>>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>>>>>> each and every board code. In the latter case, the boards would also be
>>>>>> responsible for saving/restoring the address.
>>>>>
>>>>> How is the device relocated? Where are the chipset registers you mention?
>>>>
>>>> Intel's PIIX chipsets contain a register called APICBASE (but it means
>>>> the IOAPIC), and that defines the location. The analogy in the APIC
>>>> world is the MSR_IA32_APICBASE which we maintain via the APIC state.
>>>
>>> In ICH10 the register is called OIC—Other Interrupt Control Register
>>> and the interesting bits APIC Range Select (ASEL).
>>>
>>> So actually PIIX should manage IOAPIC mapping, not board level.
>>
>> The point is we need ioapic_set_base_address logic in multiple places
>> (once chipsets start to implement it). Better push it to a central place
>> from the beginning. Also the bit keeping. There is no difference to
>> apicbase.
>
> In that case, the function should be made inline version in ioapic.h.
That still replicates the bit keeping.
I don't see the benefit of moving it over, even less when we want to
consolidate with a vmstate layout that is already in use.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
2011-02-03 19:25 ` Jan Kiszka
@ 2011-02-03 19:30 ` Blue Swirl
-1 siblings, 0 replies; 36+ messages in thread
From: Blue Swirl @ 2011-02-03 19:30 UTC (permalink / raw)
To: Jan Kiszka
Cc: qemu-devel, Anthony Liguori, Gleb Natapov, Marcelo Tosatti,
Alexander Graf, kvm, Avi Kivity
On Thu, Feb 3, 2011 at 7:25 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-02-03 20:11, Blue Swirl wrote:
>> On Thu, Feb 3, 2011 at 7:06 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2011-02-03 20:01, Blue Swirl wrote:
>>>> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> On 2011-02-03 18:54, Blue Swirl wrote:
>>>>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>> On 2011-02-03 18:36, Blue Swirl wrote:
>>>>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>>>>>>> current base address in its version 2 vmstate.
>>>>>>>>>>>
>>>>>>>>>>> To align both implementations for migratability, add the proper
>>>>>>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>>>>>>> will also allow multiple IOAPICs in the future.
>>>>>>>>>>
>>>>>>>>>> Nack, the addresses should be device properties.
>>>>>>>>>
>>>>>>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>>>>>>> But current_base_address is just the same as apicbase and can't be a
>>>>>>>>> property.
>>>>>>>>
>>>>>>>> Oh, right. What will current_base_address used for? Why can't board
>>>>>>>> just unmap IOAPIC from current address and remap it at the new
>>>>>>>> address? Then the device would not need to know its base address.
>>>>>>>
>>>>>>> The board could do this. The question is where we put this service, in
>>>>>>> the context if the IOAPIC as ioapic_set_base_address (compare to
>>>>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>>>>>>> each and every board code. In the latter case, the boards would also be
>>>>>>> responsible for saving/restoring the address.
>>>>>>
>>>>>> How is the device relocated? Where are the chipset registers you mention?
>>>>>
>>>>> Intel's PIIX chipsets contain a register called APICBASE (but it means
>>>>> the IOAPIC), and that defines the location. The analogy in the APIC
>>>>> world is the MSR_IA32_APICBASE which we maintain via the APIC state.
>>>>
>>>> In ICH10 the register is called OIC—Other Interrupt Control Register
>>>> and the interesting bits APIC Range Select (ASEL).
>>>>
>>>> So actually PIIX should manage IOAPIC mapping, not board level.
>>>
>>> The point is we need ioapic_set_base_address logic in multiple places
>>> (once chipsets start to implement it). Better push it to a central place
>>> from the beginning. Also the bit keeping. There is no difference to
>>> apicbase.
>>
>> In that case, the function should be made inline version in ioapic.h.
>
> That still replicates the bit keeping.
>
> I don't see the benefit of moving it over, even less when we want to
> consolidate with a vmstate layout that is already in use.
The benefit is that the device model is improved.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
@ 2011-02-03 19:30 ` Blue Swirl
0 siblings, 0 replies; 36+ messages in thread
From: Blue Swirl @ 2011-02-03 19:30 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Gleb Natapov, kvm, Marcelo Tosatti, qemu-devel,
Alexander Graf, Avi Kivity
On Thu, Feb 3, 2011 at 7:25 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-02-03 20:11, Blue Swirl wrote:
>> On Thu, Feb 3, 2011 at 7:06 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2011-02-03 20:01, Blue Swirl wrote:
>>>> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> On 2011-02-03 18:54, Blue Swirl wrote:
>>>>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>> On 2011-02-03 18:36, Blue Swirl wrote:
>>>>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>>>>>>> current base address in its version 2 vmstate.
>>>>>>>>>>>
>>>>>>>>>>> To align both implementations for migratability, add the proper
>>>>>>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>>>>>>> will also allow multiple IOAPICs in the future.
>>>>>>>>>>
>>>>>>>>>> Nack, the addresses should be device properties.
>>>>>>>>>
>>>>>>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>>>>>>> But current_base_address is just the same as apicbase and can't be a
>>>>>>>>> property.
>>>>>>>>
>>>>>>>> Oh, right. What will current_base_address used for? Why can't board
>>>>>>>> just unmap IOAPIC from current address and remap it at the new
>>>>>>>> address? Then the device would not need to know its base address.
>>>>>>>
>>>>>>> The board could do this. The question is where we put this service, in
>>>>>>> the context if the IOAPIC as ioapic_set_base_address (compare to
>>>>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>>>>>>> each and every board code. In the latter case, the boards would also be
>>>>>>> responsible for saving/restoring the address.
>>>>>>
>>>>>> How is the device relocated? Where are the chipset registers you mention?
>>>>>
>>>>> Intel's PIIX chipsets contain a register called APICBASE (but it means
>>>>> the IOAPIC), and that defines the location. The analogy in the APIC
>>>>> world is the MSR_IA32_APICBASE which we maintain via the APIC state.
>>>>
>>>> In ICH10 the register is called OIC—Other Interrupt Control Register
>>>> and the interesting bits APIC Range Select (ASEL).
>>>>
>>>> So actually PIIX should manage IOAPIC mapping, not board level.
>>>
>>> The point is we need ioapic_set_base_address logic in multiple places
>>> (once chipsets start to implement it). Better push it to a central place
>>> from the beginning. Also the bit keeping. There is no difference to
>>> apicbase.
>>
>> In that case, the function should be made inline version in ioapic.h.
>
> That still replicates the bit keeping.
>
> I don't see the benefit of moving it over, even less when we want to
> consolidate with a vmstate layout that is already in use.
The benefit is that the device model is improved.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
2011-02-03 19:30 ` Blue Swirl
@ 2011-02-03 19:42 ` Jan Kiszka
-1 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 19:42 UTC (permalink / raw)
To: Blue Swirl
Cc: qemu-devel, Anthony Liguori, Gleb Natapov, Marcelo Tosatti,
Alexander Graf, kvm, Avi Kivity
On 2011-02-03 20:30, Blue Swirl wrote:
> On Thu, Feb 3, 2011 at 7:25 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2011-02-03 20:11, Blue Swirl wrote:
>>> On Thu, Feb 3, 2011 at 7:06 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2011-02-03 20:01, Blue Swirl wrote:
>>>>> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>> On 2011-02-03 18:54, Blue Swirl wrote:
>>>>>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>> On 2011-02-03 18:36, Blue Swirl wrote:
>>>>>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>>>>>>>> current base address in its version 2 vmstate.
>>>>>>>>>>>>
>>>>>>>>>>>> To align both implementations for migratability, add the proper
>>>>>>>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>>>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>>>>>>>> will also allow multiple IOAPICs in the future.
>>>>>>>>>>>
>>>>>>>>>>> Nack, the addresses should be device properties.
>>>>>>>>>>
>>>>>>>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>>>>>>>> But current_base_address is just the same as apicbase and can't be a
>>>>>>>>>> property.
>>>>>>>>>
>>>>>>>>> Oh, right. What will current_base_address used for? Why can't board
>>>>>>>>> just unmap IOAPIC from current address and remap it at the new
>>>>>>>>> address? Then the device would not need to know its base address.
>>>>>>>>
>>>>>>>> The board could do this. The question is where we put this service, in
>>>>>>>> the context if the IOAPIC as ioapic_set_base_address (compare to
>>>>>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>>>>>>>> each and every board code. In the latter case, the boards would also be
>>>>>>>> responsible for saving/restoring the address.
>>>>>>>
>>>>>>> How is the device relocated? Where are the chipset registers you mention?
>>>>>>
>>>>>> Intel's PIIX chipsets contain a register called APICBASE (but it means
>>>>>> the IOAPIC), and that defines the location. The analogy in the APIC
>>>>>> world is the MSR_IA32_APICBASE which we maintain via the APIC state.
>>>>>
>>>>> In ICH10 the register is called OIC—Other Interrupt Control Register
>>>>> and the interesting bits APIC Range Select (ASEL).
>>>>>
>>>>> So actually PIIX should manage IOAPIC mapping, not board level.
>>>>
>>>> The point is we need ioapic_set_base_address logic in multiple places
>>>> (once chipsets start to implement it). Better push it to a central place
>>>> from the beginning. Also the bit keeping. There is no difference to
>>>> apicbase.
>>>
>>> In that case, the function should be made inline version in ioapic.h.
>>
>> That still replicates the bit keeping.
>>
>> I don't see the benefit of moving it over, even less when we want to
>> consolidate with a vmstate layout that is already in use.
>
> The benefit is that the device model is improved.
I disagree about the benefit, but I will simply drop this patch and
instead add a dummy field to a vmstate version 3 so that qemu-kvm can
remove the base_address evaluation logic when updating.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation
@ 2011-02-03 19:42 ` Jan Kiszka
0 siblings, 0 replies; 36+ messages in thread
From: Jan Kiszka @ 2011-02-03 19:42 UTC (permalink / raw)
To: Blue Swirl
Cc: Anthony Liguori, Gleb Natapov, kvm, Marcelo Tosatti, qemu-devel,
Alexander Graf, Avi Kivity
On 2011-02-03 20:30, Blue Swirl wrote:
> On Thu, Feb 3, 2011 at 7:25 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2011-02-03 20:11, Blue Swirl wrote:
>>> On Thu, Feb 3, 2011 at 7:06 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2011-02-03 20:01, Blue Swirl wrote:
>>>>> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>> On 2011-02-03 18:54, Blue Swirl wrote:
>>>>>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>> On 2011-02-03 18:36, Blue Swirl wrote:
>>>>>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm carries the
>>>>>>>>>>>> current base address in its version 2 vmstate.
>>>>>>>>>>>>
>>>>>>>>>>>> To align both implementations for migratability, add the proper
>>>>>>>>>>>> infrastructure to accept initial as well as updated base addresses and
>>>>>>>>>>>> include the current address in the vmstate. This is done in a way that
>>>>>>>>>>>> will also allow multiple IOAPICs in the future.
>>>>>>>>>>>
>>>>>>>>>>> Nack, the addresses should be device properties.
>>>>>>>>>>
>>>>>>>>>> Hmm.... we could make default_base_address a property. Will change that.
>>>>>>>>>> But current_base_address is just the same as apicbase and can't be a
>>>>>>>>>> property.
>>>>>>>>>
>>>>>>>>> Oh, right. What will current_base_address used for? Why can't board
>>>>>>>>> just unmap IOAPIC from current address and remap it at the new
>>>>>>>>> address? Then the device would not need to know its base address.
>>>>>>>>
>>>>>>>> The board could do this. The question is where we put this service, in
>>>>>>>> the context if the IOAPIC as ioapic_set_base_address (compare to
>>>>>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>>>>>>>> each and every board code. In the latter case, the boards would also be
>>>>>>>> responsible for saving/restoring the address.
>>>>>>>
>>>>>>> How is the device relocated? Where are the chipset registers you mention?
>>>>>>
>>>>>> Intel's PIIX chipsets contain a register called APICBASE (but it means
>>>>>> the IOAPIC), and that defines the location. The analogy in the APIC
>>>>>> world is the MSR_IA32_APICBASE which we maintain via the APIC state.
>>>>>
>>>>> In ICH10 the register is called OIC—Other Interrupt Control Register
>>>>> and the interesting bits APIC Range Select (ASEL).
>>>>>
>>>>> So actually PIIX should manage IOAPIC mapping, not board level.
>>>>
>>>> The point is we need ioapic_set_base_address logic in multiple places
>>>> (once chipsets start to implement it). Better push it to a central place
>>>> from the beginning. Also the bit keeping. There is no difference to
>>>> apicbase.
>>>
>>> In that case, the function should be made inline version in ioapic.h.
>>
>> That still replicates the bit keeping.
>>
>> I don't see the benefit of moving it over, even less when we want to
>> consolidate with a vmstate layout that is already in use.
>
> The benefit is that the device model is improved.
I disagree about the benefit, but I will simply drop this patch and
instead add a dummy field to a vmstate version 3 so that qemu-kvm can
remove the base_address evaluation logic when updating.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [0.14?][PATCH 1/4] ioapic: Implement EOI handling for level-triggered IRQs
2011-02-03 14:55 ` [Qemu-devel] " Jan Kiszka
@ 2011-02-03 19:59 ` Anthony Liguori
-1 siblings, 0 replies; 36+ messages in thread
From: Anthony Liguori @ 2011-02-03 19:59 UTC (permalink / raw)
To: Jan Kiszka
Cc: qemu-devel, Anthony Liguori, kvm, Alexander Graf, Gleb Natapov,
Avi Kivity, Marcelo Tosatti
On 02/03/2011 08:55 AM, Jan Kiszka wrote:
> Add the missing EOI broadcast from local APIC to the IOAPICs on
> completion of level-triggered IRQs. This ensures that a still asserted
> IRQ source properly re-triggers an APIC IRQ.
>
> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
> ---
> hw/apic.c | 9 ++++++---
> hw/ioapic.c | 43 +++++++++++++++++++++++++++++++++++++++++--
> hw/ioapic.h | 20 ++++++++++++++++++++
> 3 files changed, 67 insertions(+), 5 deletions(-)
> create mode 100644 hw/ioapic.h
>
> diff --git a/hw/apic.c b/hw/apic.c
> index ff581f0..05a115f 100644
> --- a/hw/apic.c
> +++ b/hw/apic.c
> @@ -18,6 +18,7 @@
> */
> #include "hw.h"
> #include "apic.h"
> +#include "ioapic.h"
> #include "qemu-timer.h"
> #include "host-utils.h"
> #include "sysbus.h"
> @@ -57,7 +58,8 @@
>
> #define ESR_ILLEGAL_ADDRESS (1<< 7)
>
> -#define APIC_SV_ENABLE (1<< 8)
> +#define APIC_SV_DIRECTED_IO (1<<12)
> +#define APIC_SV_ENABLE (1<<8)
>
> #define MAX_APICS 255
> #define MAX_APIC_WORDS 8
> @@ -420,8 +422,9 @@ static void apic_eoi(APICState *s)
> if (isrv< 0)
> return;
> reset_bit(s->isr, isrv);
> - /* XXX: send the EOI packet to the APIC bus to allow the I/O APIC to
> - set the remote IRR bit for level triggered interrupts. */
> + if (!(s->spurious_vec& APIC_SV_DIRECTED_IO)&& get_bit(s->tmr, isrv)) {
> + ioapic_eio_broadcast(isrv);
> + }
> apic_update_irq(s);
> }
>
> diff --git a/hw/ioapic.c b/hw/ioapic.c
> index 2109568..443c579 100644
> --- a/hw/ioapic.c
> +++ b/hw/ioapic.c
> @@ -23,6 +23,7 @@
> #include "hw.h"
> #include "pc.h"
> #include "apic.h"
> +#include "ioapic.h"
> #include "qemu-timer.h"
> #include "host-utils.h"
> #include "sysbus.h"
> @@ -36,7 +37,10 @@
> #define DPRINTF(fmt, ...)
> #endif
>
> -#define IOAPIC_LVT_MASKED (1<<16)
> +#define MAX_IOAPICS 1
> +
> +#define IOAPIC_LVT_MASKED (1<< 16)
> +#define IOAPIC_LVT_REMOTE_IRR (1<< 14)
>
> #define IOAPIC_TRIGGER_EDGE 0
> #define IOAPIC_TRIGGER_LEVEL 1
> @@ -61,6 +65,8 @@ struct IOAPICState {
> uint64_t ioredtbl[IOAPIC_NUM_PINS];
> };
>
> +static IOAPICState *ioapics[MAX_IOAPICS];
> +
> static void ioapic_service(IOAPICState *s)
> {
> uint8_t i;
> @@ -83,8 +89,11 @@ static void ioapic_service(IOAPICState *s)
> dest_mode = (entry>> 11)& 1;
> delivery_mode = (entry>> 8)& 7;
> polarity = (entry>> 13)& 1;
> - if (trig_mode == IOAPIC_TRIGGER_EDGE)
> + if (trig_mode == IOAPIC_TRIGGER_EDGE) {
> s->irr&= ~mask;
> + } else {
> + s->ioredtbl[i] |= IOAPIC_LVT_REMOTE_IRR;
> + }
> if (delivery_mode == IOAPIC_DM_EXTINT)
> vector = pic_read_irq(isa_pic);
> else
> @@ -131,6 +140,29 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
> }
> }
>
> +void ioapic_eio_broadcast(int vector)
>
You mean iopaic_eoi_broadcast(), no?
Regards,
Anthony Liguori
> +{
> + IOAPICState *s;
> + uint64_t entry;
> + int i, n;
> +
> + for (i = 0; i< MAX_IOAPICS; i++) {
> + s = ioapics[i];
> + if (!s) {
> + continue;
> + }
> + for (n = 0; n< IOAPIC_NUM_PINS; n++) {
> + entry = s->ioredtbl[n];
> + if ((entry& IOAPIC_LVT_REMOTE_IRR)&& (entry& 0xff) == vector) {
> + s->ioredtbl[n] = entry& ~IOAPIC_LVT_REMOTE_IRR;
> + if (!(entry& IOAPIC_LVT_MASKED)&& (s->irr& (1<< n))) {
> + ioapic_service(s);
> + }
> + }
> + }
> + }
> +}
> +
> static uint32_t ioapic_mem_readl(void *opaque, target_phys_addr_t addr)
> {
> IOAPICState *s = opaque;
> @@ -240,6 +272,11 @@ static int ioapic_init1(SysBusDevice *dev)
> {
> IOAPICState *s = FROM_SYSBUS(IOAPICState, dev);
> int io_memory;
> + static int ioapic_no;
> +
> + if (ioapic_no>= MAX_IOAPICS) {
> + return -1;
> + }
>
> io_memory = cpu_register_io_memory(ioapic_mem_read,
> ioapic_mem_write, s,
> @@ -248,6 +285,8 @@ static int ioapic_init1(SysBusDevice *dev)
>
> qdev_init_gpio_in(&dev->qdev, ioapic_set_irq, IOAPIC_NUM_PINS);
>
> + ioapics[ioapic_no++] = s;
> +
> return 0;
> }
>
> diff --git a/hw/ioapic.h b/hw/ioapic.h
> new file mode 100644
> index 0000000..c441567
> --- /dev/null
> +++ b/hw/ioapic.h
> @@ -0,0 +1,20 @@
> +/*
> + * ioapic.c IOAPIC emulation logic
> + *
> + * Copyright (c) 2011 Jan Kiszka, Siemens AG
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see<http://www.gnu.org/licenses/>.
> + */
> +
> +void ioapic_eio_broadcast(int vector);
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] Re: [0.14?][PATCH 1/4] ioapic: Implement EOI handling for level-triggered IRQs
@ 2011-02-03 19:59 ` Anthony Liguori
0 siblings, 0 replies; 36+ messages in thread
From: Anthony Liguori @ 2011-02-03 19:59 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, kvm, Gleb Natapov, Marcelo Tosatti, qemu-devel,
Alexander Graf, Avi Kivity
On 02/03/2011 08:55 AM, Jan Kiszka wrote:
> Add the missing EOI broadcast from local APIC to the IOAPICs on
> completion of level-triggered IRQs. This ensures that a still asserted
> IRQ source properly re-triggers an APIC IRQ.
>
> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
> ---
> hw/apic.c | 9 ++++++---
> hw/ioapic.c | 43 +++++++++++++++++++++++++++++++++++++++++--
> hw/ioapic.h | 20 ++++++++++++++++++++
> 3 files changed, 67 insertions(+), 5 deletions(-)
> create mode 100644 hw/ioapic.h
>
> diff --git a/hw/apic.c b/hw/apic.c
> index ff581f0..05a115f 100644
> --- a/hw/apic.c
> +++ b/hw/apic.c
> @@ -18,6 +18,7 @@
> */
> #include "hw.h"
> #include "apic.h"
> +#include "ioapic.h"
> #include "qemu-timer.h"
> #include "host-utils.h"
> #include "sysbus.h"
> @@ -57,7 +58,8 @@
>
> #define ESR_ILLEGAL_ADDRESS (1<< 7)
>
> -#define APIC_SV_ENABLE (1<< 8)
> +#define APIC_SV_DIRECTED_IO (1<<12)
> +#define APIC_SV_ENABLE (1<<8)
>
> #define MAX_APICS 255
> #define MAX_APIC_WORDS 8
> @@ -420,8 +422,9 @@ static void apic_eoi(APICState *s)
> if (isrv< 0)
> return;
> reset_bit(s->isr, isrv);
> - /* XXX: send the EOI packet to the APIC bus to allow the I/O APIC to
> - set the remote IRR bit for level triggered interrupts. */
> + if (!(s->spurious_vec& APIC_SV_DIRECTED_IO)&& get_bit(s->tmr, isrv)) {
> + ioapic_eio_broadcast(isrv);
> + }
> apic_update_irq(s);
> }
>
> diff --git a/hw/ioapic.c b/hw/ioapic.c
> index 2109568..443c579 100644
> --- a/hw/ioapic.c
> +++ b/hw/ioapic.c
> @@ -23,6 +23,7 @@
> #include "hw.h"
> #include "pc.h"
> #include "apic.h"
> +#include "ioapic.h"
> #include "qemu-timer.h"
> #include "host-utils.h"
> #include "sysbus.h"
> @@ -36,7 +37,10 @@
> #define DPRINTF(fmt, ...)
> #endif
>
> -#define IOAPIC_LVT_MASKED (1<<16)
> +#define MAX_IOAPICS 1
> +
> +#define IOAPIC_LVT_MASKED (1<< 16)
> +#define IOAPIC_LVT_REMOTE_IRR (1<< 14)
>
> #define IOAPIC_TRIGGER_EDGE 0
> #define IOAPIC_TRIGGER_LEVEL 1
> @@ -61,6 +65,8 @@ struct IOAPICState {
> uint64_t ioredtbl[IOAPIC_NUM_PINS];
> };
>
> +static IOAPICState *ioapics[MAX_IOAPICS];
> +
> static void ioapic_service(IOAPICState *s)
> {
> uint8_t i;
> @@ -83,8 +89,11 @@ static void ioapic_service(IOAPICState *s)
> dest_mode = (entry>> 11)& 1;
> delivery_mode = (entry>> 8)& 7;
> polarity = (entry>> 13)& 1;
> - if (trig_mode == IOAPIC_TRIGGER_EDGE)
> + if (trig_mode == IOAPIC_TRIGGER_EDGE) {
> s->irr&= ~mask;
> + } else {
> + s->ioredtbl[i] |= IOAPIC_LVT_REMOTE_IRR;
> + }
> if (delivery_mode == IOAPIC_DM_EXTINT)
> vector = pic_read_irq(isa_pic);
> else
> @@ -131,6 +140,29 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
> }
> }
>
> +void ioapic_eio_broadcast(int vector)
>
You mean iopaic_eoi_broadcast(), no?
Regards,
Anthony Liguori
> +{
> + IOAPICState *s;
> + uint64_t entry;
> + int i, n;
> +
> + for (i = 0; i< MAX_IOAPICS; i++) {
> + s = ioapics[i];
> + if (!s) {
> + continue;
> + }
> + for (n = 0; n< IOAPIC_NUM_PINS; n++) {
> + entry = s->ioredtbl[n];
> + if ((entry& IOAPIC_LVT_REMOTE_IRR)&& (entry& 0xff) == vector) {
> + s->ioredtbl[n] = entry& ~IOAPIC_LVT_REMOTE_IRR;
> + if (!(entry& IOAPIC_LVT_MASKED)&& (s->irr& (1<< n))) {
> + ioapic_service(s);
> + }
> + }
> + }
> + }
> +}
> +
> static uint32_t ioapic_mem_readl(void *opaque, target_phys_addr_t addr)
> {
> IOAPICState *s = opaque;
> @@ -240,6 +272,11 @@ static int ioapic_init1(SysBusDevice *dev)
> {
> IOAPICState *s = FROM_SYSBUS(IOAPICState, dev);
> int io_memory;
> + static int ioapic_no;
> +
> + if (ioapic_no>= MAX_IOAPICS) {
> + return -1;
> + }
>
> io_memory = cpu_register_io_memory(ioapic_mem_read,
> ioapic_mem_write, s,
> @@ -248,6 +285,8 @@ static int ioapic_init1(SysBusDevice *dev)
>
> qdev_init_gpio_in(&dev->qdev, ioapic_set_irq, IOAPIC_NUM_PINS);
>
> + ioapics[ioapic_no++] = s;
> +
> return 0;
> }
>
> diff --git a/hw/ioapic.h b/hw/ioapic.h
> new file mode 100644
> index 0000000..c441567
> --- /dev/null
> +++ b/hw/ioapic.h
> @@ -0,0 +1,20 @@
> +/*
> + * ioapic.c IOAPIC emulation logic
> + *
> + * Copyright (c) 2011 Jan Kiszka, Siemens AG
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see<http://www.gnu.org/licenses/>.
> + */
> +
> +void ioapic_eio_broadcast(int vector);
>
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2011-02-03 20:00 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-03 14:55 [0.14?][PATCH 0/4] IOAPIC fixes Jan Kiszka
2011-02-03 14:55 ` [Qemu-devel] " Jan Kiszka
2011-02-03 14:55 ` [0.14?][PATCH 1/4] ioapic: Implement EOI handling for level-triggered IRQs Jan Kiszka
2011-02-03 14:55 ` [Qemu-devel] " Jan Kiszka
2011-02-03 19:59 ` Anthony Liguori
2011-02-03 19:59 ` [Qemu-devel] " Anthony Liguori
2011-02-03 14:55 ` [0.14?][PATCH 2/4] ioapic: Save/restore irr Jan Kiszka
2011-02-03 14:55 ` [Qemu-devel] " Jan Kiszka
2011-02-03 14:55 ` [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation Jan Kiszka
2011-02-03 14:55 ` [Qemu-devel] " Jan Kiszka
2011-02-03 17:03 ` Blue Swirl
2011-02-03 17:03 ` Blue Swirl
2011-02-03 17:18 ` Jan Kiszka
2011-02-03 17:18 ` Jan Kiszka
2011-02-03 17:36 ` Blue Swirl
2011-02-03 17:36 ` Blue Swirl
2011-02-03 17:43 ` Jan Kiszka
2011-02-03 17:43 ` Jan Kiszka
2011-02-03 17:54 ` Blue Swirl
2011-02-03 17:54 ` Blue Swirl
2011-02-03 18:01 ` Jan Kiszka
2011-02-03 18:01 ` Jan Kiszka
2011-02-03 19:01 ` Blue Swirl
2011-02-03 19:01 ` Blue Swirl
2011-02-03 19:06 ` Jan Kiszka
2011-02-03 19:06 ` [Qemu-devel] " Jan Kiszka
2011-02-03 19:11 ` Blue Swirl
2011-02-03 19:11 ` Blue Swirl
2011-02-03 19:25 ` Jan Kiszka
2011-02-03 19:25 ` Jan Kiszka
2011-02-03 19:30 ` Blue Swirl
2011-02-03 19:30 ` Blue Swirl
2011-02-03 19:42 ` Jan Kiszka
2011-02-03 19:42 ` Jan Kiszka
2011-02-03 14:55 ` [0.14?][PATCH 4/4] ioapic: Style & magics cleanup Jan Kiszka
2011-02-03 14:55 ` [Qemu-devel] " Jan Kiszka
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.