* [Qemu-devel] [PATCH v7 0/6] vGICv3 support
@ 2015-07-24 9:55 Pavel Fedin
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 1/6] Merge memory_region_init_reservation() into memory_region_init_io() Pavel Fedin
` (5 more replies)
0 siblings, 6 replies; 32+ messages in thread
From: Pavel Fedin @ 2015-07-24 9:55 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Shlomo Pongratz, Shlomo Pongratz,
Christoffer Dall, Eric Auger
This series introduces support for GICv3 by KVM. Software emulation is
currently not supported.
Differences from v6:
- Wrap own GIC type definitions on top of KVM ones. Fixed build on
non-ARM-Linux hosts
Differences from v5:
- Fixed various checkpatch.pl style warnings
- Removed TODO in gicv3_init_irqs_and_mmio(), relevant memory API patch
included
- gicv3_init_irqs_and_mmio() now takes 3 arguments instead of 4. It is more
convenient to pass MMIO descriptors as array
Differences from v4:
- Do not reintroduce several constants shared with GICv2, reuse them instead.
- Added gicv3_init_irqs_and_mmio() in base class, to be used by both software
emulation and KVM code. Avoids code duplication.
- Do not add NULL msi-parent phandle to PCI device in the FDT
- Removed a couple of stale things from virt.c
Differences from v3:
- Fixed stupid build breakage in patch 0002
- Rebased on top of current master, patch 0003 adjusted according to
kvm_irqchip_create() changes
- Added assertion against uninitialized kernel_irqchip_type
- Removed kernel_irqchip_type initialization from models which do not
use KVM vGIC
Differences from v2:
- Removed some unrelated and unnecessary changes from virt machine,
occasionally slipped in; some of them caused qemu to crash on ARM32.
- Fixed build for ARM32; vGICv3 code requires definitions which are
present only in ARM64 kernel
Differences from v1:
- Base class included, taken from the series by Shlomo Pongratz:
http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg01512.html
The code is refactored as little as possible in order to simplify
further addition of software emulation:
- Minor fixes in code style and comments, according to old reviews
- Removed REV_V3 definition because it's currently not used, and it does
not add any meaning to number 3.
- Removed reserved regions for MBI and ITS (except for 'virt' machine
memory map). These should go to separate classes when implemented.
- Improved commit messages
- vGIC patches restructured
- Use 'gicversion' option instead of virt-v3 machine
Pavel Fedin (5):
Merge memory_region_init_reservation() into memory_region_init_io()
Extract some reusable vGIC code
Introduce irqchip type specification for KVM
Initial implementation of vGICv3
Add gicversion option to virt machine
Shlomo Pongratz (1):
Implement GIC-500 base class
hw/arm/vexpress.c | 1 +
hw/arm/virt.c | 140 +++++++++++++++++----
hw/intc/Makefile.objs | 4 +
hw/intc/arm_gic_kvm.c | 84 ++++++-------
hw/intc/arm_gicv3_common.c | 249 +++++++++++++++++++++++++++++++++++++
hw/intc/arm_gicv3_kvm.c | 155 +++++++++++++++++++++++
hw/intc/gicv3_internal.h | 156 +++++++++++++++++++++++
hw/intc/vgic_common.h | 43 +++++++
include/exec/memory.h | 14 ++-
include/hw/arm/fdt.h | 2 +-
include/hw/arm/virt.h | 6 +-
include/hw/boards.h | 1 +
include/hw/intc/arm_gicv3_common.h | 112 +++++++++++++++++
include/sysemu/kvm.h | 3 +-
kvm-all.c | 2 +-
memory.c | 10 +-
stubs/kvm.c | 2 +-
target-arm/kvm-consts.h | 6 +
target-arm/kvm.c | 13 +-
19 files changed, 917 insertions(+), 86 deletions(-)
create mode 100644 hw/intc/arm_gicv3_common.c
create mode 100644 hw/intc/arm_gicv3_kvm.c
create mode 100644 hw/intc/gicv3_internal.h
create mode 100644 hw/intc/vgic_common.h
create mode 100644 include/hw/intc/arm_gicv3_common.h
--
1.9.5.msysgit.0
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v7 1/6] Merge memory_region_init_reservation() into memory_region_init_io()
2015-07-24 9:55 [Qemu-devel] [PATCH v7 0/6] vGICv3 support Pavel Fedin
@ 2015-07-24 9:55 ` Pavel Fedin
2015-07-24 16:20 ` Peter Maydell
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 2/6] Implement GIC-500 base class Pavel Fedin
` (4 subsequent siblings)
5 siblings, 1 reply; 32+ messages in thread
From: Pavel Fedin @ 2015-07-24 9:55 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Shlomo Pongratz, Shlomo Pongratz,
Christoffer Dall, Eric Auger
Just speficying ops = NULL in some cases can be more convenient than having
two functions. GICv3 code is going to use this. GICv2 code can be refactored
in a similar way, killing some code duplication.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/exec/memory.h | 14 +++++++++++---
memory.c | 10 +---------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1394715..3b5e44e 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -436,6 +436,9 @@ void memory_region_init_alias(MemoryRegion *mr,
* memory_region_init_rom_device: Initialize a ROM memory region. Writes are
* handled via callbacks.
*
+ * If NULL callbacks pointer is given, then I/O space is not supposed to be
+ * handled by QEMU itself. Any access via the memory API will cause an abort().
+ *
* @mr: the #MemoryRegion to be initialized.
* @owner: the object that tracks the region's reference count
* @ops: callbacks for write access handling.
@@ -458,16 +461,21 @@ void memory_region_init_rom_device(MemoryRegion *mr,
* A reservation region primariy serves debugging purposes. It claims I/O
* space that is not supposed to be handled by QEMU itself. Any access via
* the memory API will cause an abort().
+ * This function is deprecated. Use memory_region_init_io() with NULL
+ * callbacks instead.
*
* @mr: the #MemoryRegion to be initialized
* @owner: the object that tracks the region's reference count
* @name: used for debugging; not visible to the user or ABI
* @size: size of the region.
*/
-void memory_region_init_reservation(MemoryRegion *mr,
- struct Object *owner,
+static inline void memory_region_init_reservation(MemoryRegion *mr,
+ Object *owner,
const char *name,
- uint64_t size);
+ uint64_t size)
+{
+ memory_region_init_io(mr, owner, NULL, mr, name, size);
+}
/**
* memory_region_init_iommu: Initialize a memory region that translates
diff --git a/memory.c b/memory.c
index 0acebb1..b843952 100644
--- a/memory.c
+++ b/memory.c
@@ -1187,7 +1187,7 @@ void memory_region_init_io(MemoryRegion *mr,
uint64_t size)
{
memory_region_init(mr, owner, name, size);
- mr->ops = ops;
+ mr->ops = ops ? ops : &unassigned_mem_ops;
mr->opaque = opaque;
mr->terminates = true;
}
@@ -1307,14 +1307,6 @@ void memory_region_init_iommu(MemoryRegion *mr,
notifier_list_init(&mr->iommu_notify);
}
-void memory_region_init_reservation(MemoryRegion *mr,
- Object *owner,
- const char *name,
- uint64_t size)
-{
- memory_region_init_io(mr, owner, &unassigned_mem_ops, mr, name, size);
-}
-
static void memory_region_finalize(Object *obj)
{
MemoryRegion *mr = MEMORY_REGION(obj);
--
1.9.5.msysgit.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v7 2/6] Implement GIC-500 base class
2015-07-24 9:55 [Qemu-devel] [PATCH v7 0/6] vGICv3 support Pavel Fedin
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 1/6] Merge memory_region_init_reservation() into memory_region_init_io() Pavel Fedin
@ 2015-07-24 9:55 ` Pavel Fedin
2015-07-24 16:37 ` Peter Maydell
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 3/6] Extract some reusable vGIC code Pavel Fedin
` (3 subsequent siblings)
5 siblings, 1 reply; 32+ messages in thread
From: Pavel Fedin @ 2015-07-24 9:55 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Shlomo Pongratz, Shlomo Pongratz,
Christoffer Dall, Eric Auger
From: Shlomo Pongratz <shlomo.pongratz@huawei.com>
This class is to be used by both software and KVM implementations of GICv3
Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
hw/intc/Makefile.objs | 1 +
hw/intc/arm_gicv3_common.c | 249 +++++++++++++++++++++++++++++++++++++
hw/intc/gicv3_internal.h | 156 +++++++++++++++++++++++
include/hw/intc/arm_gicv3_common.h | 112 +++++++++++++++++
4 files changed, 518 insertions(+)
create mode 100644 hw/intc/arm_gicv3_common.c
create mode 100644 hw/intc/gicv3_internal.h
create mode 100644 include/hw/intc/arm_gicv3_common.h
diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index 092d8a8..1317e5a 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -12,6 +12,7 @@ common-obj-$(CONFIG_IOAPIC) += ioapic_common.o
common-obj-$(CONFIG_ARM_GIC) += arm_gic_common.o
common-obj-$(CONFIG_ARM_GIC) += arm_gic.o
common-obj-$(CONFIG_ARM_GIC) += arm_gicv2m.o
+common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_common.o
common-obj-$(CONFIG_OPENPIC) += openpic.o
obj-$(CONFIG_APIC) += apic.o apic_common.o
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
new file mode 100644
index 0000000..5a10784
--- /dev/null
+++ b/hw/intc/arm_gicv3_common.c
@@ -0,0 +1,249 @@
+/*
+ * ARM GICv3 support - common bits of emulated and KVM kernel model
+ *
+ * Copyright (c) 2012 Linaro Limited
+ * Copyright (c) 2015 Huawei.
+ * Written by Peter Maydell
+ * Extended to 64 cores by Shlomo Pongratz
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "gicv3_internal.h"
+
+static void gicv3_pre_save(void *opaque)
+{
+ GICv3State *s = (GICv3State *)opaque;
+ ARMGICv3CommonClass *c = ARM_GICV3_COMMON_GET_CLASS(s);
+
+ if (c->pre_save) {
+ c->pre_save(s);
+ }
+}
+
+static int gicv3_post_load(void *opaque, int version_id)
+{
+ GICv3State *s = (GICv3State *)opaque;
+ ARMGICv3CommonClass *c = ARM_GICV3_COMMON_GET_CLASS(s);
+
+ if (c->post_load) {
+ c->post_load(s);
+ }
+ return 0;
+}
+
+static const VMStateDescription vmstate_gicv3_irq_state = {
+ .name = "arm_gicv3_irq_state",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT64(pending, gicv3_irq_state),
+ VMSTATE_UINT64(active, gicv3_irq_state),
+ VMSTATE_UINT64(level, gicv3_irq_state),
+ VMSTATE_UINT64(group, gicv3_irq_state),
+ VMSTATE_BOOL(edge_trigger, gicv3_irq_state),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static const VMStateDescription vmstate_gicv3_sgi_state = {
+ .name = "arm_gicv3_sgi_state",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT64_ARRAY(pending, gicv3_sgi_state, GICV3_NCPU),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static const VMStateDescription vmstate_gicv3 = {
+ .name = "arm_gicv3",
+ .version_id = 7,
+ .minimum_version_id = 7,
+ .pre_save = gicv3_pre_save,
+ .post_load = gicv3_post_load,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(ctlr, GICv3State),
+ VMSTATE_UINT32_ARRAY(cpu_ctlr, GICv3State, GICV3_NCPU),
+ VMSTATE_STRUCT_ARRAY(irq_state, GICv3State, GICV3_MAXIRQ, 1,
+ vmstate_gicv3_irq_state, gicv3_irq_state),
+ VMSTATE_UINT64_ARRAY(irq_target, GICv3State, GICV3_MAXIRQ),
+ VMSTATE_UINT8_2DARRAY(priority1, GICv3State, GIC_INTERNAL,
+ GICV3_NCPU),
+ VMSTATE_UINT8_ARRAY(priority2, GICv3State,
+ GICV3_MAXIRQ - GIC_INTERNAL),
+ VMSTATE_UINT16_2DARRAY(last_active, GICv3State, GICV3_MAXIRQ,
+ GICV3_NCPU),
+ VMSTATE_STRUCT_ARRAY(sgi_state, GICv3State, GIC_NR_SGIS, 1,
+ vmstate_gicv3_sgi_state, gicv3_sgi_state),
+ VMSTATE_UINT16_ARRAY(priority_mask, GICv3State, GICV3_NCPU),
+ VMSTATE_UINT16_ARRAY(running_irq, GICv3State, GICV3_NCPU),
+ VMSTATE_UINT16_ARRAY(running_priority, GICv3State, GICV3_NCPU),
+ VMSTATE_UINT16_ARRAY(current_pending, GICv3State, GICV3_NCPU),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler,
+ const MemoryRegionOps *ops)
+{
+ SysBusDevice *sbd = SYS_BUS_DEVICE(s);
+ int i;
+
+ /* For the GIC, also expose incoming GPIO lines for PPIs for each CPU.
+ * GPIO array layout is thus:
+ * [0..N-1] spi
+ * [N..N+31] PPIs for CPU 0
+ * [N+32..N+63] PPIs for CPU 1
+ * ...
+ */
+ i = s->num_irq - GIC_INTERNAL + GIC_INTERNAL * s->num_cpu;
+ qdev_init_gpio_in(DEVICE(s), handler, i);
+
+ for (i = 0; i < s->num_cpu; i++) {
+ sysbus_init_irq(sbd, &s->parent_irq[i]);
+ }
+ for (i = 0; i < s->num_cpu; i++) {
+ sysbus_init_irq(sbd, &s->parent_fiq[i]);
+ }
+
+ memory_region_init_io(&s->iomem_dist, OBJECT(s), ops, s,
+ "gicv3_dist", 0x10000);
+ memory_region_init_io(&s->iomem_lpi, OBJECT(s), ops ? &ops[1] : NULL, s,
+ "gicv3_redist", 0x800000);
+
+ sysbus_init_mmio(sbd, &s->iomem_dist);
+ sysbus_init_mmio(sbd, &s->iomem_lpi);
+}
+
+static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
+{
+ GICv3State *s = ARM_GICV3_COMMON(dev);
+ int num_irq = s->num_irq;
+
+ if (s->num_cpu > GICV3_NCPU) {
+ error_setg(errp, "requested %u CPUs exceeds GIC maximum %d",
+ s->num_cpu, GICV3_NCPU);
+ return;
+ }
+ s->num_irq += GICV3_BASE_IRQ;
+ if (s->num_irq > GICV3_MAXIRQ) {
+ error_setg(errp,
+ "requested %u interrupt lines exceeds GIC maximum %d",
+ num_irq, GICV3_MAXIRQ);
+ return;
+ }
+ /* ITLinesNumber is represented as (N / 32) - 1 (see
+ * gic_dist_readb) so this is an implementation imposed
+ * restriction, not an architectural one:
+ */
+ if (s->num_irq < 32 || (s->num_irq % 32)) {
+ error_setg(errp,
+ "%d interrupt lines unsupported: not divisible by 32",
+ num_irq);
+ return;
+ }
+}
+
+static void arm_gicv3_common_reset(DeviceState *dev)
+{
+ GICv3State *s = ARM_GICV3_COMMON(dev);
+ int i;
+
+ /* Note num_cpu and num_irq are properties */
+
+ /* Don't reset anything assigned in arm_gic_realize or any property */
+
+ /* No GICv2 backwards computability support */
+ for (i = 0; i < s->num_cpu; i++) {
+ s->priority_mask[i] = 0;
+ s->current_pending[i] = 1023;
+ s->running_irq[i] = 1023;
+ s->running_priority[i] = 0x100;
+ s->cpu_ctlr[i] = 0;
+ }
+
+ memset(s->irq_state, 0, sizeof(s->irq_state));
+ /* GIC-500 comment 'j' SGI are always enabled */
+ for (i = 0; i < GIC_NR_SGIS; i++) {
+ GIC_SET_ENABLED(i, ALL_CPU_MASK);
+ GIC_SET_EDGE_TRIGGER(i);
+ }
+ memset(s->sgi_state, 0, sizeof(s->sgi_state));
+ memset(s->irq_target, 0, sizeof(s->irq_target));
+ if (s->num_cpu == 1) {
+ /* For uniprocessor GICs all interrupts always target the sole CPU */
+ for (i = 0; i < GICV3_MAXIRQ; i++) {
+ s->irq_target[i] = 1;
+ }
+ }
+ memset(s->priority1, 0, sizeof(s->priority1));
+ memset(s->priority2, 0, sizeof(s->priority2));
+ memset(s->last_active, 0, sizeof(s->last_active));
+
+ /* With all configuration we don't support GICv2 backwards computability */
+ if (s->security_levels > 1) {
+ /* GICv3 5.3.20 With two security So DS is RAZ/WI ARE_NS is RAO/WI
+ * and ARE_S is RAO/WI
+ */
+ s->ctlr = GICD_CTLR_ARE_S | GICD_CTLR_ARE_NS;
+ } else {
+ /* GICv3 5.3.20 With one security So DS is RAO/WI ARE is RAO/WI
+ */
+ s->ctlr = GICD_CTLR_DS | GICD_CTLR_ARE;
+ }
+ /* Workaround!
+ * Linux (drivers/irqchip/irq-gic-v3.c) is enabling only group one,
+ * in gic_cpu_sys_reg_init it calls gic_write_grpen1(1);
+ * but it doesn't conigure any interrupt to be in group one
+ */
+ for (i = 0; i < s->num_irq; i++) {
+ GIC_SET_GROUP(i, ALL_CPU_MASK);
+ }
+}
+
+static Property arm_gicv3_common_properties[] = {
+ DEFINE_PROP_UINT32("num-cpu", GICv3State, num_cpu, 1),
+ DEFINE_PROP_UINT32("num-irq", GICv3State, num_irq, 32),
+ DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
+ /* How many security levels are supported */
+ DEFINE_PROP_BOOL("security-levels", GICv3State, security_levels, 0),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void arm_gicv3_common_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->reset = arm_gicv3_common_reset;
+ dc->realize = arm_gicv3_common_realize;
+ dc->props = arm_gicv3_common_properties;
+ dc->vmsd = &vmstate_gicv3;
+}
+
+static const TypeInfo arm_gicv3_common_type = {
+ .name = TYPE_ARM_GICV3_COMMON,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(GICv3State),
+ .class_size = sizeof(ARMGICv3CommonClass),
+ .class_init = arm_gicv3_common_class_init,
+ .abstract = true,
+};
+
+static void register_types(void)
+{
+ type_register_static(&arm_gicv3_common_type);
+}
+
+type_init(register_types)
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
new file mode 100644
index 0000000..589baca
--- /dev/null
+++ b/hw/intc/gicv3_internal.h
@@ -0,0 +1,156 @@
+/*
+ * ARM GIC support - internal interfaces
+ *
+ * Copyright (c) 2012 Linaro Limited
+ * Copyright (c) 2015 Huawei.
+ * Written by Peter Maydell
+ * Extended to 64 cores by Shlomo Pongratz
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QEMU_ARM_GICV3_INTERNAL_H
+#define QEMU_ARM_GICV3_INTERNAL_H
+
+#include "hw/intc/arm_gicv3_common.h"
+
+#define ALL_CPU_MASK ((uint64_t) (0xffffffffffffffff))
+
+/* Keep this macro so it will be easy to compare the code to GICv2 code.
+ * The compiler will optimize any +/- operation involving this macro
+ */
+#define GICV3_BASE_IRQ (0)
+
+#define GIC_SET_ENABLED(irq, cm) s->irq_state[irq].enabled |= (cm)
+#define GIC_CLEAR_ENABLED(irq, cm) s->irq_state[irq].enabled &= ~(cm)
+#define GIC_TEST_ENABLED(irq, cm) ((s->irq_state[irq].enabled & (cm)) != 0)
+#define GIC_SET_PENDING(irq, cm) s->irq_state[irq].pending |= (cm)
+#define GIC_TEST_PENDING(irq, cm) ((s->irq_state[irq].pending & (cm)) != 0)
+#define GIC_CLEAR_PENDING(irq, cm) s->irq_state[irq].pending &= ~(cm)
+#define GIC_SET_ACTIVE(irq, cm) s->irq_state[irq].active |= (cm)
+#define GIC_CLEAR_ACTIVE(irq, cm) s->irq_state[irq].active &= ~(cm)
+#define GIC_TEST_ACTIVE(irq, cm) ((s->irq_state[irq].active & (cm)) != 0)
+#define GIC_SET_LEVEL(irq, cm) s->irq_state[irq].level |= (cm)
+#define GIC_CLEAR_LEVEL(irq, cm) s->irq_state[irq].level &= ~(cm)
+#define GIC_TEST_LEVEL(irq, cm) ((s->irq_state[irq].level & (cm)) != 0)
+#define GIC_SET_EDGE_TRIGGER(irq) s->irq_state[irq].edge_trigger = true
+#define GIC_CLEAR_EDGE_TRIGGER(irq) s->irq_state[irq].edge_trigger = false
+#define GIC_TEST_EDGE_TRIGGER(irq) (s->irq_state[irq].edge_trigger)
+#define GIC_GET_PRIORITY(irq, cpu) (((irq) < GIC_INTERNAL) ? \
+ s->priority1[irq][cpu] : \
+ s->priority2[(irq) - GIC_INTERNAL])
+#define GIC_TARGET(irq) s->irq_target[irq]
+#define GIC_CLEAR_GROUP(irq, cm) (s->irq_state[irq].group &= ~(cm))
+#define GIC_SET_GROUP(irq, cm) (s->irq_state[irq].group |= (cm))
+#define GIC_TEST_GROUP(irq, cm) ((s->irq_state[irq].group & (cm)) != 0)
+
+static inline bool gic_test_pending(GICv3State *s, int irq, uint64_t cm)
+{
+ /* Edge-triggered interrupts are marked pending on a rising edge, but
+ * level-triggered interrupts are either considered pending when the
+ * level is active or if software has explicitly written to
+ * GICD_ISPENDR to set the state pending.
+ */
+ return (s->irq_state[irq].pending & cm) ||
+ (!GIC_TEST_EDGE_TRIGGER(irq) && GIC_TEST_LEVEL(irq, cm));
+}
+
+#define GICD_CTLR 0x0000
+#define GICD_TYPER 0x0004
+#define GICD_IIDR 0x0008
+#define GICD_STATUSR 0x0010
+#define GICD_SETSPI_NSR 0x0040
+#define GICD_CLRSPI_NSR 0x0048
+#define GICD_SETSPI_SR 0x0050
+#define GICD_CLRSPI_SR 0x0058
+#define GICD_SEIR 0x0068
+#define GICD_ISENABLER 0x0100
+#define GICD_ICENABLER 0x0180
+#define GICD_ISPENDR 0x0200
+#define GICD_ICPENDR 0x0280
+#define GICD_ISACTIVER 0x0300
+#define GICD_ICACTIVER 0x0380
+#define GICD_IPRIORITYR 0x0400
+#define GICD_ICFGR 0x0C00
+#define GICD_IROUTER 0x6000
+#define GICD_PIDR2 0xFFE8
+
+/* GICD_CTLR fields */
+#define GICD_CTLR_EN_GRP0 (1U << 0)
+#define GICD_CTLR_EN_GRP1NS (1U << 1) /* GICv3 5.3.20 */
+#define GICD_CTLR_EN_GRP1S (1U << 2)
+#define GICD_CTLR_EN_GRP1_ALL (GICD_CTLR_EN_GRP1NS | GICD_CTLR_EN_GRP1S)
+#define GICD_CTLR_ARE (1U << 4)
+#define GICD_CTLR_ARE_S (1U << 4)
+#define GICD_CTLR_ARE_NS (1U << 5)
+#define GICD_CTLR_DS (1U << 6)
+#define GICD_CTLR_RWP (1U << 31)
+
+#define GICD_IROUTER_SPI_MODE_ONE (0U << 31)
+#define GICD_IROUTER_SPI_MODE_ANY (1U << 31)
+
+#define GIC_PIDR2_ARCH_MASK 0xf0
+#define GIC_PIDR2_ARCH_GICv3 0x30
+#define GIC_PIDR2_ARCH_GICv4 0x40
+
+/*
+ * Re-Distributor registers, offsets from RD_base
+ */
+#define GICR_CTLR GICD_CTLR
+#define GICR_IIDR 0x0004
+#define GICR_TYPER 0x0008
+#define GICR_STATUSR GICD_STATUSR
+#define GICR_WAKER 0x0014
+#define GICR_SETLPIR 0x0040
+#define GICR_CLRLPIR 0x0048
+#define GICR_SEIR GICD_SEIR
+#define GICR_PROPBASER 0x0070
+#define GICR_PENDBASER 0x0078
+#define GICR_INVLPIR 0x00A0
+#define GICR_INVALLR 0x00B0
+#define GICR_SYNCR 0x00C0
+#define GICR_MOVLPIR 0x0100
+#define GICR_MOVALLR 0x0110
+#define GICR_PIDR2 GICD_PIDR2
+
+#define GICR_WAKER_ProcessorSleep (1U << 1)
+#define GICR_WAKER_ChildrenAsleep (1U << 2)
+
+/*
+ * Re-Distributor registers, offsets from SGI_base
+ */
+#define GICR_ISENABLER0 GICD_ISENABLER
+#define GICR_ICENABLER0 GICD_ICENABLER
+#define GICR_ISPENDR0 GICD_ISPENDR
+#define GICR_ICPENDR0 GICD_ICPENDR
+#define GICR_ISACTIVER0 GICD_ISACTIVER
+#define GICR_ICACTIVER0 GICD_ICACTIVER
+#define GICR_IPRIORITYR0 GICD_IPRIORITYR
+#define GICR_ICFGR0 GICD_ICFGR
+
+#define GICR_TYPER_VLPIS (1U << 1)
+#define GICR_TYPER_LAST (1U << 4)
+
+/*
+ * Simulated used system registers
+ */
+#define GICC_CTLR_EN_GRP0 (1U << 0)
+#define GICC_CTLR_EN_GRP1 (1U << 1)
+#define GICC_CTLR_ACK_CTL (1U << 2)
+#define GICC_CTLR_FIQ_EN (1U << 3)
+#define GICC_CTLR_CBPR (1U << 4) /* GICv1: SBPR */
+#define GICC_CTLR_EOIMODE (1U << 9)
+#define GICC_CTLR_EOIMODE_NS (1U << 10)
+
+#endif /* !QEMU_ARM_GIC_INTERNAL_H */
diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
new file mode 100644
index 0000000..c8cc24e
--- /dev/null
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -0,0 +1,112 @@
+/*
+ * ARM GIC support
+ *
+ * Copyright (c) 2012 Linaro Limited
+ * Copyright (c) 2015 Huawei.
+ * Written by Peter Maydell
+ * Extended to 64 cores by Shlomo Pongratz
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef HW_ARM_GICV3_COMMON_H
+#define HW_ARM_GICV3_COMMON_H
+
+#include "hw/sysbus.h"
+#include "hw/intc/arm_gic_common.h"
+
+/* Maximum number of possible interrupts, determined by the GIC architecture */
+#define GICV3_MAXIRQ 1020
+#define GICV3_NCPU 64
+
+typedef struct gicv3_irq_state {
+ /* The enable bits are only banked for per-cpu interrupts. */
+ uint64_t enabled;
+ uint64_t pending;
+ uint64_t active;
+ uint64_t level;
+ uint64_t group;
+ bool edge_trigger; /* true: edge-triggered, false: level-triggered */
+} gicv3_irq_state;
+
+typedef struct gicv3_sgi_state {
+ uint64_t pending[GICV3_NCPU];
+} gicv3_sgi_state;
+
+typedef struct GICv3State {
+ /*< private >*/
+ SysBusDevice parent_obj;
+ /*< public >*/
+
+ qemu_irq parent_irq[GICV3_NCPU];
+ qemu_irq parent_fiq[GICV3_NCPU];
+ /* GICD_CTLR; for a GIC with the security extensions the NS banked version
+ * of this register is just an alias of bit 1 of the S banked version.
+ */
+ uint32_t ctlr;
+ /* Sim GICC_CTLR; again, the NS banked version is just aliases of bits of
+ * the S banked register, so our state only needs to store the S version.
+ */
+ uint32_t cpu_ctlr[GICV3_NCPU];
+ bool cpu_enabled[GICV3_NCPU];
+
+ gicv3_irq_state irq_state[GICV3_MAXIRQ];
+ uint64_t irq_target[GICV3_MAXIRQ];
+ uint8_t priority1[GIC_INTERNAL][GICV3_NCPU];
+ uint8_t priority2[GICV3_MAXIRQ - GIC_INTERNAL];
+ uint16_t last_active[GICV3_MAXIRQ][GICV3_NCPU];
+ /* For each SGI on the target CPU, we store 64 bits
+ * indicating which source CPUs have made this SGI
+ * pending on the target CPU. These correspond to
+ * the bytes in the GIC_SPENDSGIR* registers as
+ * read by the target CPU.
+ */
+ gicv3_sgi_state sgi_state[GIC_NR_SGIS];
+
+ uint16_t priority_mask[GICV3_NCPU];
+ uint16_t running_irq[GICV3_NCPU];
+ uint16_t running_priority[GICV3_NCPU];
+ uint16_t current_pending[GICV3_NCPU];
+
+ uint32_t cpu_mask; /* For redistributor */
+ uint32_t num_cpu;
+ MemoryRegion iomem_dist; /* Distributor */
+ MemoryRegion iomem_lpi; /* Redistributor */
+ uint32_t num_irq;
+ uint32_t revision;
+ bool security_levels;
+ int dev_fd; /* kvm device fd if backed by kvm vgic support */
+} GICv3State;
+
+#define TYPE_ARM_GICV3_COMMON "arm_gicv3_common"
+#define ARM_GICV3_COMMON(obj) \
+ OBJECT_CHECK(GICv3State, (obj), TYPE_ARM_GICV3_COMMON)
+#define ARM_GICV3_COMMON_CLASS(klass) \
+ OBJECT_CLASS_CHECK(ARMGICv3CommonClass, (klass), TYPE_ARM_GICV3_COMMON)
+#define ARM_GICV3_COMMON_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(ARMGICv3CommonClass, (obj), TYPE_ARM_GICV3_COMMON)
+
+typedef struct ARMGICv3CommonClass {
+ /*< private >*/
+ SysBusDeviceClass parent_class;
+ /*< public >*/
+
+ void (*pre_save)(GICv3State *s);
+ void (*post_load)(GICv3State *s);
+} ARMGICv3CommonClass;
+
+void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler,
+ const MemoryRegionOps *ops);
+
+#endif
--
1.9.5.msysgit.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v7 3/6] Extract some reusable vGIC code
2015-07-24 9:55 [Qemu-devel] [PATCH v7 0/6] vGICv3 support Pavel Fedin
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 1/6] Merge memory_region_init_reservation() into memory_region_init_io() Pavel Fedin
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 2/6] Implement GIC-500 base class Pavel Fedin
@ 2015-07-24 9:55 ` Pavel Fedin
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 4/6] Introduce irqchip type specification for KVM Pavel Fedin
` (2 subsequent siblings)
5 siblings, 0 replies; 32+ messages in thread
From: Pavel Fedin @ 2015-07-24 9:55 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Shlomo Pongratz, Shlomo Pongratz,
Christoffer Dall, Eric Auger
These functions are useful also for vGICv3 implementation. Make them accessible
from within other modules.
Actually kvm_dist_get() and kvm_dist_put() could also be made reusable, but
they would require two extra parameters (s->dev_fd and s->num_cpu) as well as
lots of typecasts of 's' to DeviceState * and back to GICState *. This makes
the code very ugly so i decided to stop at this point. I tried also an
approach with making a base class for all possible GICs, but it would contain
only three variables (dev_fd, cpu_num and irq_num), and accessing them through
the rest of the code would be again tedious (either ugly casts or qemu-style
separate object pointer). So i disliked it too.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
hw/intc/arm_gic_kvm.c | 84 ++++++++++++++++++++++++---------------------------
hw/intc/vgic_common.h | 43 ++++++++++++++++++++++++++
2 files changed, 82 insertions(+), 45 deletions(-)
create mode 100644 hw/intc/vgic_common.h
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index f56bff1..9b6d702 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -23,6 +23,7 @@
#include "sysemu/kvm.h"
#include "kvm_arm.h"
#include "gic_internal.h"
+#include "vgic_common.h"
//#define DEBUG_GIC_KVM
@@ -52,7 +53,7 @@ typedef struct KVMARMGICClass {
void (*parent_reset)(DeviceState *dev);
} KVMARMGICClass;
-static void kvm_arm_gic_set_irq(void *opaque, int irq, int level)
+void kvm_arm_gic_set_irq(uint32_t num_irq, int irq, int level)
{
/* Meaning of the 'irq' parameter:
* [0..N-1] : external interrupts
@@ -63,10 +64,9 @@ static void kvm_arm_gic_set_irq(void *opaque, int irq, int level)
* has separate fields in the irq number for type,
* CPU number and interrupt number.
*/
- GICState *s = (GICState *)opaque;
int kvm_irq, irqtype, cpu;
- if (irq < (s->num_irq - GIC_INTERNAL)) {
+ if (irq < (num_irq - GIC_INTERNAL)) {
/* External interrupt. The kernel numbers these like the GIC
* hardware, with external interrupt IDs starting after the
* internal ones.
@@ -77,7 +77,7 @@ static void kvm_arm_gic_set_irq(void *opaque, int irq, int level)
} else {
/* Internal interrupt: decode into (cpu, interrupt id) */
irqtype = KVM_ARM_IRQ_TYPE_PPI;
- irq -= (s->num_irq - GIC_INTERNAL);
+ irq -= (num_irq - GIC_INTERNAL);
cpu = irq / GIC_INTERNAL;
irq %= GIC_INTERNAL;
}
@@ -87,12 +87,19 @@ static void kvm_arm_gic_set_irq(void *opaque, int irq, int level)
kvm_set_irq(kvm_state, kvm_irq, !!level);
}
+static void kvm_arm_gicv2_set_irq(void *opaque, int irq, int level)
+{
+ GICState *s = (GICState *)opaque;
+
+ kvm_arm_gic_set_irq(s->num_irq, irq, level);
+}
+
static bool kvm_arm_gic_can_save_restore(GICState *s)
{
return s->dev_fd >= 0;
}
-static bool kvm_gic_supports_attr(GICState *s, int group, int attrnum)
+bool kvm_gic_supports_attr(int dev_fd, int group, int attrnum)
{
struct kvm_device_attr attr = {
.group = group,
@@ -100,14 +107,14 @@ static bool kvm_gic_supports_attr(GICState *s, int group, int attrnum)
.flags = 0,
};
- if (s->dev_fd == -1) {
+ if (dev_fd == -1) {
return false;
}
- return kvm_device_ioctl(s->dev_fd, KVM_HAS_DEVICE_ATTR, &attr) == 0;
+ return kvm_device_ioctl(dev_fd, KVM_HAS_DEVICE_ATTR, &attr) == 0;
}
-static void kvm_gic_access(GICState *s, int group, int offset,
+void kvm_gic_access(int dev_fd, int group, int offset,
int cpu, uint32_t *val, bool write)
{
struct kvm_device_attr attr;
@@ -130,7 +137,7 @@ static void kvm_gic_access(GICState *s, int group, int offset,
type = KVM_GET_DEVICE_ATTR;
}
- err = kvm_device_ioctl(s->dev_fd, type, &attr);
+ err = kvm_device_ioctl(dev_fd, type, &attr);
if (err < 0) {
fprintf(stderr, "KVM_{SET/GET}_DEVICE_ATTR failed: %s\n",
strerror(-err));
@@ -138,20 +145,6 @@ static void kvm_gic_access(GICState *s, int group, int offset,
}
}
-static void kvm_gicd_access(GICState *s, int offset, int cpu,
- uint32_t *val, bool write)
-{
- kvm_gic_access(s, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
- offset, cpu, val, write);
-}
-
-static void kvm_gicc_access(GICState *s, int offset, int cpu,
- uint32_t *val, bool write)
-{
- kvm_gic_access(s, KVM_DEV_ARM_VGIC_GRP_CPU_REGS,
- offset, cpu, val, write);
-}
-
#define for_each_irq_reg(_ctr, _max_irq, _field_width) \
for (_ctr = 0; _ctr < ((_max_irq) / (32 / (_field_width))); _ctr++)
@@ -291,7 +284,7 @@ static void kvm_dist_get(GICState *s, uint32_t offset, int width,
irq = i * regsz;
cpu = 0;
while ((cpu < s->num_cpu && irq < GIC_INTERNAL) || cpu == 0) {
- kvm_gicd_access(s, offset, cpu, ®, false);
+ kvm_gicd_access(s->dev_fd, offset, cpu, ®, false);
for (j = 0; j < regsz; j++) {
field = extract32(reg, j * width, width);
translate_fn(s, irq + j, cpu, &field, false);
@@ -324,7 +317,7 @@ static void kvm_dist_put(GICState *s, uint32_t offset, int width,
translate_fn(s, irq + j, cpu, &field, true);
reg = deposit32(reg, j * width, width, field);
}
- kvm_gicd_access(s, offset, cpu, ®, true);
+ kvm_gicd_access(s->dev_fd, offset, cpu, ®, true);
cpu++;
}
@@ -355,10 +348,10 @@ static void kvm_arm_gic_put(GICState *s)
/* s->ctlr -> GICD_CTLR */
reg = s->ctlr;
- kvm_gicd_access(s, 0x0, 0, ®, true);
+ kvm_gicd_access(s->dev_fd, 0x0, 0, ®, true);
/* Sanity checking on GICD_TYPER and s->num_irq, s->num_cpu */
- kvm_gicd_access(s, 0x4, 0, ®, false);
+ kvm_gicd_access(s->dev_fd, 0x4, 0, ®, false);
num_irq = ((reg & 0x1f) + 1) * 32;
num_cpu = ((reg & 0xe0) >> 5) + 1;
@@ -416,24 +409,24 @@ static void kvm_arm_gic_put(GICState *s)
for (cpu = 0; cpu < s->num_cpu; cpu++) {
/* s->cpu_ctlr[cpu] -> GICC_CTLR */
reg = s->cpu_ctlr[cpu];
- kvm_gicc_access(s, 0x00, cpu, ®, true);
+ kvm_gicc_access(s->dev_fd, 0x00, cpu, ®, true);
/* s->priority_mask[cpu] -> GICC_PMR */
reg = (s->priority_mask[cpu] & 0xff);
- kvm_gicc_access(s, 0x04, cpu, ®, true);
+ kvm_gicc_access(s->dev_fd, 0x04, cpu, ®, true);
/* s->bpr[cpu] -> GICC_BPR */
reg = (s->bpr[cpu] & 0x7);
- kvm_gicc_access(s, 0x08, cpu, ®, true);
+ kvm_gicc_access(s->dev_fd, 0x08, cpu, ®, true);
/* s->abpr[cpu] -> GICC_ABPR */
reg = (s->abpr[cpu] & 0x7);
- kvm_gicc_access(s, 0x1c, cpu, ®, true);
+ kvm_gicc_access(s->dev_fd, 0x1c, cpu, ®, true);
/* s->apr[n][cpu] -> GICC_APRn */
for (i = 0; i < 4; i++) {
reg = s->apr[i][cpu];
- kvm_gicc_access(s, 0xd0 + i * 4, cpu, ®, true);
+ kvm_gicc_access(s->dev_fd, 0xd0 + i * 4, cpu, ®, true);
}
}
}
@@ -454,11 +447,11 @@ static void kvm_arm_gic_get(GICState *s)
*/
/* GICD_CTLR -> s->ctlr */
- kvm_gicd_access(s, 0x0, 0, ®, false);
+ kvm_gicd_access(s->dev_fd, 0x0, 0, ®, false);
s->ctlr = reg;
/* Sanity checking on GICD_TYPER -> s->num_irq, s->num_cpu */
- kvm_gicd_access(s, 0x4, 0, ®, false);
+ kvm_gicd_access(s->dev_fd, 0x4, 0, ®, false);
s->num_irq = ((reg & 0x1f) + 1) * 32;
s->num_cpu = ((reg & 0xe0) >> 5) + 1;
@@ -469,7 +462,7 @@ static void kvm_arm_gic_get(GICState *s)
}
/* GICD_IIDR -> ? */
- kvm_gicd_access(s, 0x8, 0, ®, false);
+ kvm_gicd_access(s->dev_fd, 0x8, 0, ®, false);
/* Clear all the IRQ settings */
for (i = 0; i < s->num_irq; i++) {
@@ -507,24 +500,24 @@ static void kvm_arm_gic_get(GICState *s)
for (cpu = 0; cpu < s->num_cpu; cpu++) {
/* GICC_CTLR -> s->cpu_ctlr[cpu] */
- kvm_gicc_access(s, 0x00, cpu, ®, false);
+ kvm_gicc_access(s->dev_fd, 0x00, cpu, ®, false);
s->cpu_ctlr[cpu] = reg;
/* GICC_PMR -> s->priority_mask[cpu] */
- kvm_gicc_access(s, 0x04, cpu, ®, false);
+ kvm_gicc_access(s->dev_fd, 0x04, cpu, ®, false);
s->priority_mask[cpu] = (reg & 0xff);
/* GICC_BPR -> s->bpr[cpu] */
- kvm_gicc_access(s, 0x08, cpu, ®, false);
+ kvm_gicc_access(s->dev_fd, 0x08, cpu, ®, false);
s->bpr[cpu] = (reg & 0x7);
/* GICC_ABPR -> s->abpr[cpu] */
- kvm_gicc_access(s, 0x1c, cpu, ®, false);
+ kvm_gicc_access(s->dev_fd, 0x1c, cpu, ®, false);
s->abpr[cpu] = (reg & 0x7);
/* GICC_APRn -> s->apr[n][cpu] */
for (i = 0; i < 4; i++) {
- kvm_gicc_access(s, 0xd0 + i * 4, cpu, ®, false);
+ kvm_gicc_access(s->dev_fd, 0xd0 + i * 4, cpu, ®, false);
s->apr[i][cpu] = reg;
}
}
@@ -569,7 +562,7 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
* ...
*/
i += (GIC_INTERNAL * s->num_cpu);
- qdev_init_gpio_in(dev, kvm_arm_gic_set_irq, i);
+ qdev_init_gpio_in(dev, kvm_arm_gicv2_set_irq, i);
for (i = 0; i < s->num_irq - GIC_INTERNAL; i++) {
qemu_irq irq = qdev_get_gpio_in(dev, i);
@@ -596,15 +589,16 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
return;
}
- if (kvm_gic_supports_attr(s, KVM_DEV_ARM_VGIC_GRP_NR_IRQS, 0)) {
+ if (kvm_gic_supports_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS, 0)) {
uint32_t numirqs = s->num_irq;
- kvm_gic_access(s, KVM_DEV_ARM_VGIC_GRP_NR_IRQS, 0, 0, &numirqs, 1);
+ kvm_gic_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS, 0, 0,
+ &numirqs, 1);
}
/* Tell the kernel to complete VGIC initialization now */
- if (kvm_gic_supports_attr(s, KVM_DEV_ARM_VGIC_GRP_CTRL,
+ if (kvm_gic_supports_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
KVM_DEV_ARM_VGIC_CTRL_INIT)) {
- kvm_gic_access(s, KVM_DEV_ARM_VGIC_GRP_CTRL,
+ kvm_gic_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
KVM_DEV_ARM_VGIC_CTRL_INIT, 0, 0, 1);
}
diff --git a/hw/intc/vgic_common.h b/hw/intc/vgic_common.h
new file mode 100644
index 0000000..67241d3
--- /dev/null
+++ b/hw/intc/vgic_common.h
@@ -0,0 +1,43 @@
+/*
+ * ARM KVM vGIC utility functions
+ *
+ * Copyright (c) 2015 Samsung Electronics
+ * Written by Pavel Fedin
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QEMU_ARM_VGIC_COMMON_H
+#define QEMU_ARM_VGIC_COMMON_H
+
+void kvm_arm_gic_set_irq(uint32_t num_irq, int irq, int level);
+bool kvm_gic_supports_attr(int dev_fd, int group, int attrnum);
+void kvm_gic_access(int dev_fd, int group, int offset, int cpu,
+ uint32_t *val, bool write);
+
+static inline void kvm_gicd_access(int dev_fd, int offset, int cpu,
+ uint32_t *val, bool write)
+{
+ kvm_gic_access(dev_fd, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
+ offset, cpu, val, write);
+}
+
+static inline void kvm_gicc_access(int dev_fd, int offset, int cpu,
+ uint32_t *val, bool write)
+{
+ kvm_gic_access(dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_REGS,
+ offset, cpu, val, write);
+}
+
+#endif
--
1.9.5.msysgit.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v7 4/6] Introduce irqchip type specification for KVM
2015-07-24 9:55 [Qemu-devel] [PATCH v7 0/6] vGICv3 support Pavel Fedin
` (2 preceding siblings ...)
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 3/6] Extract some reusable vGIC code Pavel Fedin
@ 2015-07-24 9:55 ` Pavel Fedin
2015-07-24 16:42 ` Peter Maydell
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3 Pavel Fedin
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 6/6] Add gicversion option to virt machine Pavel Fedin
5 siblings, 1 reply; 32+ messages in thread
From: Pavel Fedin @ 2015-07-24 9:55 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Shlomo Pongratz, Shlomo Pongratz,
Christoffer Dall, Eric Auger
This patch introduces kernel_irqchip_type member in Machine class, which
is passed to kvm_arch_irqchip_create. Machine models which can use vGIC
now use it in order to supply correct GIC type for KVM capability
verification. The variable is defined as int in order to be
architecture-agnostic for potential future uses by other architectures.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
hw/arm/vexpress.c | 1 +
hw/arm/virt.c | 3 +++
include/hw/boards.h | 1 +
include/sysemu/kvm.h | 3 ++-
kvm-all.c | 2 +-
stubs/kvm.c | 2 +-
target-arm/kvm-consts.h | 6 ++++++
target-arm/kvm.c | 13 +++++++++++--
8 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index da21788..865e823 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -556,6 +556,7 @@ static void vexpress_common_init(MachineState *machine)
const hwaddr *map = daughterboard->motherboard_map;
int i;
+ machine->kernel_irqchip_type = QEMU_GIC_TYPE_V2;
daughterboard->init(vms, machine->ram_size, machine->cpu_model, pic);
/*
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 4846892..30d9ab8 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -945,6 +945,9 @@ static void virt_instance_init(Object *obj)
"Set on/off to enable/disable the ARM "
"Security Extensions (TrustZone)",
NULL);
+
+ /* Default GIC type is v2 */
+ vms->parent.kernel_irqchip_type = QEMU_GIC_TYPE_V2;
}
static void virt_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 2aec9cb..37eb767 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -127,6 +127,7 @@ struct MachineState {
char *accel;
bool kernel_irqchip_allowed;
bool kernel_irqchip_required;
+ int kernel_irqchip_type;
int kvm_shadow_mem;
char *dtb;
char *dumpdtb;
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 983e99e..8f4d485 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -434,6 +434,7 @@ void kvm_init_irq_routing(KVMState *s);
/**
* kvm_arch_irqchip_create:
* @KVMState: The KVMState pointer
+ * @type: irqchip type, architecture-specific
*
* Allow architectures to create an in-kernel irq chip themselves.
*
@@ -441,7 +442,7 @@ void kvm_init_irq_routing(KVMState *s);
* 0: irq chip was not created
* > 0: irq chip was created
*/
-int kvm_arch_irqchip_create(KVMState *s);
+int kvm_arch_irqchip_create(KVMState *s, int type);
/**
* kvm_set_one_reg - set a register value in KVM via KVM_SET_ONE_REG ioctl
diff --git a/kvm-all.c b/kvm-all.c
index 06e06f2..8df938d 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1395,7 +1395,7 @@ static void kvm_irqchip_create(MachineState *machine, KVMState *s)
/* First probe and see if there's a arch-specific hook to create the
* in-kernel irqchip for us */
- ret = kvm_arch_irqchip_create(s);
+ ret = kvm_arch_irqchip_create(s, machine->kernel_irqchip_type);
if (ret == 0) {
ret = kvm_vm_ioctl(s, KVM_CREATE_IRQCHIP);
}
diff --git a/stubs/kvm.c b/stubs/kvm.c
index e7c60b6..a8505ff 100644
--- a/stubs/kvm.c
+++ b/stubs/kvm.c
@@ -1,7 +1,7 @@
#include "qemu-common.h"
#include "sysemu/kvm.h"
-int kvm_arch_irqchip_create(KVMState *s)
+int kvm_arch_irqchip_create(KVMState *s, int type)
{
return 0;
}
diff --git a/target-arm/kvm-consts.h b/target-arm/kvm-consts.h
index 943bf89..0bc12b7 100644
--- a/target-arm/kvm-consts.h
+++ b/target-arm/kvm-consts.h
@@ -39,6 +39,12 @@ MISMATCH_CHECK(CP_REG_SIZE_U64, KVM_REG_SIZE_U64)
MISMATCH_CHECK(CP_REG_ARM, KVM_REG_ARM)
MISMATCH_CHECK(CP_REG_ARCH_MASK, KVM_REG_ARCH_MASK)
+#define QEMU_GIC_TYPE_V2 5
+#define QEMU_GIC_TYPE_V3 7
+
+MISMATCH_CHECK(QEMU_GIC_TYPE_V2, KVM_DEV_TYPE_ARM_VGIC_V2)
+MISMATCH_CHECK(QEMU_GIC_TYPE_V3, KVM_DEV_TYPE_ARM_VGIC_V3)
+
#define QEMU_PSCI_0_1_FN_BASE 0x95c1ba5e
#define QEMU_PSCI_0_1_FN(n) (QEMU_PSCI_0_1_FN_BASE + (n))
#define QEMU_PSCI_0_1_FN_CPU_SUSPEND QEMU_PSCI_0_1_FN(0)
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index b278542..180f75f 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -583,19 +583,28 @@ void kvm_arch_init_irq_routing(KVMState *s)
{
}
-int kvm_arch_irqchip_create(KVMState *s)
+int kvm_arch_irqchip_create(KVMState *s, int type)
{
int ret;
+ /* Failure here means forgotten initialization of
+ * machine->kernel_irqchip_type in model code
+ */
+ assert(type != 0);
+
/* If we can create the VGIC using the newer device control API, we
* let the device do this when it initializes itself, otherwise we
* fall back to the old API */
- ret = kvm_create_device(s, KVM_DEV_TYPE_ARM_VGIC_V2, true);
+ ret = kvm_create_device(s, type, true);
if (ret == 0) {
return 1;
}
+ /* Fallback will create VGIC v2 */
+ if (type != KVM_DEV_TYPE_ARM_VGIC_V2) {
+ return ret;
+ }
return 0;
}
--
1.9.5.msysgit.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3
2015-07-24 9:55 [Qemu-devel] [PATCH v7 0/6] vGICv3 support Pavel Fedin
` (3 preceding siblings ...)
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 4/6] Introduce irqchip type specification for KVM Pavel Fedin
@ 2015-07-24 9:55 ` Pavel Fedin
2015-07-30 17:00 ` Peter Maydell
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 6/6] Add gicversion option to virt machine Pavel Fedin
5 siblings, 1 reply; 32+ messages in thread
From: Pavel Fedin @ 2015-07-24 9:55 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Shlomo Pongratz, Shlomo Pongratz,
Christoffer Dall, Eric Auger
Get/put routines are missing, live migration is not possible.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
hw/intc/Makefile.objs | 3 +
hw/intc/arm_gicv3_kvm.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 158 insertions(+)
create mode 100644 hw/intc/arm_gicv3_kvm.c
diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index 1317e5a..e2525a8 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -17,6 +17,9 @@ common-obj-$(CONFIG_OPENPIC) += openpic.o
obj-$(CONFIG_APIC) += apic.o apic_common.o
obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o
+ifeq ($(ARCH), aarch64) # Only 64-bit KVM can use these
+obj-$(CONFIG_ARM_GIC_KVM) += arm_gicv3_kvm.o
+endif
obj-$(CONFIG_STELLARIS) += armv7m_nvic.o
obj-$(CONFIG_EXYNOS4) += exynos4210_gic.o exynos4210_combiner.o
obj-$(CONFIG_GRLIB) += grlib_irqmp.o
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
new file mode 100644
index 0000000..5cb06e8
--- /dev/null
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -0,0 +1,155 @@
+/*
+ * ARM Generic Interrupt Controller using KVM in-kernel support
+ *
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd.
+ * Written by Pavel Fedin
+ * Based on vGICv2 code by Peter Maydell
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "hw/sysbus.h"
+#include "sysemu/kvm.h"
+#include "kvm_arm.h"
+#include "gicv3_internal.h"
+#include "vgic_common.h"
+
+#ifdef DEBUG_GICV3_KVM
+#define DPRINTF(fmt, ...) \
+ do { fprintf(stderr, "kvm_gicv3: " fmt, ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) \
+ do { } while (0)
+#endif
+
+#define TYPE_KVM_ARM_GICV3 "kvm-arm-gicv3"
+#define KVM_ARM_GICV3(obj) \
+ OBJECT_CHECK(GICv3State, (obj), TYPE_KVM_ARM_GICV3)
+#define KVM_ARM_GICV3_CLASS(klass) \
+ OBJECT_CLASS_CHECK(KVMARMGICv3Class, (klass), TYPE_KVM_ARM_GICV3)
+#define KVM_ARM_GICV3_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(KVMARMGICv3Class, (obj), TYPE_KVM_ARM_GICV3)
+
+typedef struct KVMARMGICv3Class {
+ ARMGICv3CommonClass parent_class;
+ DeviceRealize parent_realize;
+ void (*parent_reset)(DeviceState *dev);
+} KVMARMGICv3Class;
+
+static void kvm_arm_gicv3_set_irq(void *opaque, int irq, int level)
+{
+ GICv3State *s = (GICv3State *)opaque;
+
+ kvm_arm_gic_set_irq(s->num_irq, irq, level);
+}
+
+static void kvm_arm_gicv3_put(GICv3State *s)
+{
+ /* TODO */
+ DPRINTF("Cannot put kernel gic state, no kernel interface\n");
+}
+
+static void kvm_arm_gicv3_get(GICv3State *s)
+{
+ /* TODO */
+ DPRINTF("Cannot get kernel gic state, no kernel interface\n");
+}
+
+static void kvm_arm_gicv3_reset(DeviceState *dev)
+{
+ GICv3State *s = ARM_GICV3_COMMON(dev);
+ KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
+
+ DPRINTF("Reset\n");
+
+ kgc->parent_reset(dev);
+ kvm_arm_gicv3_put(s);
+}
+
+static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
+{
+ GICv3State *s = KVM_ARM_GICV3(dev);
+ KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
+ Error *local_err = NULL;
+ int ret;
+
+ DPRINTF("kvm_arm_gicv3_realize\n");
+
+ kgc->parent_realize(dev, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL);
+
+ /* Try to create the device via the device control API */
+ s->dev_fd = -1;
+ ret = kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_V3, false);
+ if (ret >= 0) {
+ s->dev_fd = ret;
+ } else if (ret != -ENODEV && ret != -ENOTSUP) {
+ error_setg_errno(errp, -ret, "error creating in-kernel VGIC");
+ return;
+ }
+
+ if (kvm_gic_supports_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS, 0)) {
+ uint32_t numirqs = s->num_irq;
+ DPRINTF("KVM_DEV_ARM_VGIC_GRP_NR_IRQS = %u\n", numirqs);
+ kvm_gic_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS,
+ 0, 0, &numirqs, 1);
+ }
+
+ /* Tell the kernel to complete VGIC initialization now */
+ if (kvm_gic_supports_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+ KVM_DEV_ARM_VGIC_CTRL_INIT)) {
+ DPRINTF("KVM_DEV_ARM_VGIC_CTRL_INIT\n");
+ kvm_gic_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+ KVM_DEV_ARM_VGIC_CTRL_INIT, 0, 0, 1);
+ }
+
+ kvm_arm_register_device(&s->iomem_dist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
+ KVM_VGIC_V3_ADDR_TYPE_DIST, s->dev_fd);
+ kvm_arm_register_device(&s->iomem_lpi, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
+ KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd);
+}
+
+static void kvm_arm_gicv3_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ ARMGICv3CommonClass *agcc = ARM_GICV3_COMMON_CLASS(klass);
+ KVMARMGICv3Class *kgc = KVM_ARM_GICV3_CLASS(klass);
+
+ agcc->pre_save = kvm_arm_gicv3_get;
+ agcc->post_load = kvm_arm_gicv3_put;
+ kgc->parent_realize = dc->realize;
+ kgc->parent_reset = dc->reset;
+ dc->realize = kvm_arm_gicv3_realize;
+ dc->reset = kvm_arm_gicv3_reset;
+}
+
+static const TypeInfo kvm_arm_gicv3_info = {
+ .name = TYPE_KVM_ARM_GICV3,
+ .parent = TYPE_ARM_GICV3_COMMON,
+ .instance_size = sizeof(GICv3State),
+ .class_init = kvm_arm_gicv3_class_init,
+ .class_size = sizeof(KVMARMGICv3Class),
+};
+
+static void kvm_arm_gicv3_register_types(void)
+{
+ type_register_static(&kvm_arm_gicv3_info);
+}
+
+type_init(kvm_arm_gicv3_register_types)
--
1.9.5.msysgit.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v7 6/6] Add gicversion option to virt machine
2015-07-24 9:55 [Qemu-devel] [PATCH v7 0/6] vGICv3 support Pavel Fedin
` (4 preceding siblings ...)
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3 Pavel Fedin
@ 2015-07-24 9:55 ` Pavel Fedin
2015-07-31 15:44 ` Peter Maydell
5 siblings, 1 reply; 32+ messages in thread
From: Pavel Fedin @ 2015-07-24 9:55 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Shlomo Pongratz, Shlomo Pongratz,
Christoffer Dall, Eric Auger
Set kernel_irqchip_type according to value of the option and pass it
around where necessary. Instantiate devices and fdt nodes according
to the choice.
max_cpus for virt machine increased to 64. GICv2 compatibility check
happens inside arm_gic_common_realize().
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
hw/arm/virt.c | 137 ++++++++++++++++++++++++++++++++++++++++++--------
include/hw/arm/fdt.h | 2 +-
include/hw/arm/virt.h | 6 ++-
3 files changed, 121 insertions(+), 24 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 30d9ab8..6eb525d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -48,6 +48,7 @@
#include "hw/arm/sysbus-fdt.h"
#include "hw/platform-bus.h"
#include "hw/arm/fdt.h"
+#include "qapi/visitor.h"
/* Number of external interrupt lines to configure the GIC with */
#define NUM_IRQS 256
@@ -106,7 +107,12 @@ static const MemMapEntry a15memmap[] = {
/* GIC distributor and CPU interfaces sit inside the CPU peripheral space */
[VIRT_GIC_DIST] = { 0x08000000, 0x00010000 },
[VIRT_GIC_CPU] = { 0x08010000, 0x00010000 },
- [VIRT_GIC_V2M] = { 0x08020000, 0x00001000 },
+ [VIRT_GIC_V2M] = { 0x08020000, 0x00010000 },
+ /* On v3 VIRT_GIC_DIST_MBI and VIRT_ITS_CONTROL take place of
+ * VIRT_GIC_CPU and VIRT_GIC_V2M respectively
+ */
+ [VIRT_ITS_TRANSLATION] = { 0x08030000, 0x00010000 },
+ [VIRT_LPI] = { 0x08040000, 0x00800000 },
[VIRT_UART] = { 0x09000000, 0x00001000 },
[VIRT_RTC] = { 0x09010000, 0x00001000 },
[VIRT_FW_CFG] = { 0x09020000, 0x0000000a },
@@ -256,10 +262,13 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi)
* they are edge-triggered.
*/
ARMCPU *armcpu;
+ uint32_t max;
uint32_t irqflags = GIC_FDT_IRQ_FLAGS_EDGE_LO_HI;
+ /* Argument is 32 bit but 8 bits are reserved for flags */
+ max = (vbi->smp_cpus >= 24) ? 24 : vbi->smp_cpus;
irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
- GIC_FDT_IRQ_PPI_CPU_WIDTH, (1 << vbi->smp_cpus) - 1);
+ GIC_FDT_IRQ_PPI_CPU_WIDTH, (1 << max) - 1);
qemu_fdt_add_subnode(vbi->fdt, "/timer");
@@ -283,6 +292,18 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
{
int cpu;
+ /*
+ * From Documentation/devicetree/bindings/arm/cpus.txt
+ * On ARM v8 64-bit systems value should be set to 2,
+ * that corresponds to the MPIDR_EL1 register size.
+ * If MPIDR_EL1[63:32] value is equal to 0 on all CPUs
+ * in the system, #address-cells can be set to 1, since
+ * MPIDR_EL1[63:32] bits are not used for CPUs
+ * identification.
+ *
+ * Now GIC500 doesn't support affinities 2 & 3 so currently
+ * #address-cells can stay 1 until future GIC
+ */
qemu_fdt_add_subnode(vbi->fdt, "/cpus");
qemu_fdt_setprop_cell(vbi->fdt, "/cpus", "#address-cells", 0x1);
qemu_fdt_setprop_cell(vbi->fdt, "/cpus", "#size-cells", 0x0);
@@ -319,25 +340,36 @@ static void fdt_add_v2m_gic_node(VirtBoardInfo *vbi)
qemu_fdt_setprop_cell(vbi->fdt, "/intc/v2m", "phandle", vbi->v2m_phandle);
}
-static void fdt_add_gic_node(VirtBoardInfo *vbi)
+static void fdt_add_gic_node(VirtBoardInfo *vbi, int type)
{
vbi->gic_phandle = qemu_fdt_alloc_phandle(vbi->fdt);
qemu_fdt_setprop_cell(vbi->fdt, "/", "interrupt-parent", vbi->gic_phandle);
qemu_fdt_add_subnode(vbi->fdt, "/intc");
- /* 'cortex-a15-gic' means 'GIC v2' */
- qemu_fdt_setprop_string(vbi->fdt, "/intc", "compatible",
- "arm,cortex-a15-gic");
qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#interrupt-cells", 3);
qemu_fdt_setprop(vbi->fdt, "/intc", "interrupt-controller", NULL, 0);
- qemu_fdt_setprop_sized_cells(vbi->fdt, "/intc", "reg",
- 2, vbi->memmap[VIRT_GIC_DIST].base,
- 2, vbi->memmap[VIRT_GIC_DIST].size,
- 2, vbi->memmap[VIRT_GIC_CPU].base,
- 2, vbi->memmap[VIRT_GIC_CPU].size);
qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#address-cells", 0x2);
qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#size-cells", 0x2);
qemu_fdt_setprop(vbi->fdt, "/intc", "ranges", NULL, 0);
+ if (type == KVM_DEV_TYPE_ARM_VGIC_V3) {
+ qemu_fdt_setprop_string(vbi->fdt, "/intc", "compatible",
+ "arm,gic-v3");
+ qemu_fdt_setprop_sized_cells(vbi->fdt, "/intc", "reg",
+ 2, vbi->memmap[VIRT_GIC_DIST].base,
+ 2, vbi->memmap[VIRT_GIC_DIST].size,
+ 2, vbi->memmap[VIRT_LPI].base,
+ 2, vbi->memmap[VIRT_LPI].size);
+ } else {
+ /* 'cortex-a15-gic' means 'GIC v2' */
+ qemu_fdt_setprop_string(vbi->fdt, "/intc", "compatible",
+ "arm,cortex-a15-gic");
+ qemu_fdt_setprop_sized_cells(vbi->fdt, "/intc", "reg",
+ 2, vbi->memmap[VIRT_GIC_DIST].base,
+ 2, vbi->memmap[VIRT_GIC_DIST].size,
+ 2, vbi->memmap[VIRT_GIC_CPU].base,
+ 2, vbi->memmap[VIRT_GIC_CPU].size);
+ }
+
qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", vbi->gic_phandle);
}
@@ -360,20 +392,35 @@ static void create_v2m(VirtBoardInfo *vbi, qemu_irq *pic)
fdt_add_v2m_gic_node(vbi);
}
-static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
+static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type)
{
/* We create a standalone GIC v2 */
DeviceState *gicdev;
SysBusDevice *gicbusdev;
- const char *gictype = "arm_gic";
+ const char *gictype;
int i;
- if (kvm_irqchip_in_kernel()) {
- gictype = "kvm-arm-gic";
+ if (type == KVM_DEV_TYPE_ARM_VGIC_V3) {
+ /* TODO: Software emulation is not implemented yet */
+ if (!kvm_irqchip_in_kernel()) {
+ fprintf(stderr, "KVM is currently required for GICv3 emulation\n");
+ exit(1);
+ }
+ gictype = "kvm-arm-gicv3";
+ } else {
+ gictype = kvm_irqchip_in_kernel() ? "kvm-arm-gic" : "arm_gic";
}
gicdev = qdev_create(NULL, gictype);
- qdev_prop_set_uint32(gicdev, "revision", 2);
+
+ for (i = 0; i < vbi->smp_cpus; i++) {
+ CPUState *cpu = qemu_get_cpu(i);
+ CPUARMState *env = cpu->env_ptr;
+ env->nvic = gicdev;
+ }
+
+ qdev_prop_set_uint32(gicdev, "revision",
+ type == KVM_DEV_TYPE_ARM_VGIC_V3 ? 3 : 2);
qdev_prop_set_uint32(gicdev, "num-cpu", smp_cpus);
/* Note that the num-irq property counts both internal and external
* interrupts; there are always 32 of the former (mandated by GIC spec).
@@ -382,7 +429,11 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
qdev_init_nofail(gicdev);
gicbusdev = SYS_BUS_DEVICE(gicdev);
sysbus_mmio_map(gicbusdev, 0, vbi->memmap[VIRT_GIC_DIST].base);
- sysbus_mmio_map(gicbusdev, 1, vbi->memmap[VIRT_GIC_CPU].base);
+ if (type == KVM_DEV_TYPE_ARM_VGIC_V3) {
+ sysbus_mmio_map(gicbusdev, 1, vbi->memmap[VIRT_LPI].base);
+ } else {
+ sysbus_mmio_map(gicbusdev, 1, vbi->memmap[VIRT_GIC_CPU].base);
+ }
/* Wire the outputs from each CPU's generic timer to the
* appropriate GIC PPI inputs, and the GIC's IRQ output to
@@ -409,9 +460,11 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
pic[i] = qdev_get_gpio_in(gicdev, i);
}
- fdt_add_gic_node(vbi);
+ fdt_add_gic_node(vbi, type);
- create_v2m(vbi, pic);
+ if (type == KVM_DEV_TYPE_ARM_VGIC_V2) {
+ create_v2m(vbi, pic);
+ }
}
static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
@@ -715,7 +768,10 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic)
qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0,
nr_pcie_buses - 1);
- qemu_fdt_setprop_cells(vbi->fdt, nodename, "msi-parent", vbi->v2m_phandle);
+ if (vbi->v2m_phandle) {
+ qemu_fdt_setprop_cells(vbi->fdt, nodename, "msi-parent",
+ vbi->v2m_phandle);
+ }
qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
2, base_ecam, 2, size_ecam);
@@ -875,7 +931,7 @@ static void machvirt_init(MachineState *machine)
create_flash(vbi);
- create_gic(vbi, pic);
+ create_gic(vbi, pic, machine->kernel_irqchip_type);
create_uart(vbi, pic);
@@ -933,6 +989,37 @@ static void virt_set_secure(Object *obj, bool value, Error **errp)
vms->secure = value;
}
+static void virt_get_gic_version(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ MachineState *ms = MACHINE(obj);
+ int32_t value = (ms->kernel_irqchip_type == KVM_DEV_TYPE_ARM_VGIC_V3) ?
+ 3 : 2;
+
+ visit_type_int32(v, &value, name, errp);
+}
+
+static void virt_set_gic_version(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ MachineState *ms = MACHINE(obj);
+ int32_t value;
+
+ visit_type_int32(v, &value, name, errp);
+
+ switch (value) {
+ case 3:
+ ms->kernel_irqchip_type = QEMU_GIC_TYPE_V3;
+ break;
+ case 2:
+ ms->kernel_irqchip_type = QEMU_GIC_TYPE_V2;
+ break;
+ default:
+ error_report("Only GICv2 and GICv3 supported currently\n");
+ exit(1);
+ }
+}
+
static void virt_instance_init(Object *obj)
{
VirtMachineState *vms = VIRT_MACHINE(obj);
@@ -948,6 +1035,11 @@ static void virt_instance_init(Object *obj)
/* Default GIC type is v2 */
vms->parent.kernel_irqchip_type = QEMU_GIC_TYPE_V2;
+ object_property_add(obj, "gicversion", "int", virt_get_gic_version,
+ virt_set_gic_version, NULL, NULL, NULL);
+ object_property_set_description(obj, "gicversion",
+ "Set GIC version. "
+ "Valid values are 2 and 3", NULL);
}
static void virt_class_init(ObjectClass *oc, void *data)
@@ -957,7 +1049,8 @@ static void virt_class_init(ObjectClass *oc, void *data)
mc->name = TYPE_VIRT_MACHINE;
mc->desc = "ARM Virtual Machine",
mc->init = machvirt_init;
- mc->max_cpus = 8;
+ /* With gic3 full implementation (with bitops) rase the lmit to 128 */
+ mc->max_cpus = 64;
mc->has_dynamic_sysbus = true;
mc->block_default_type = IF_VIRTIO;
mc->no_cdrom = 1;
diff --git a/include/hw/arm/fdt.h b/include/hw/arm/fdt.h
index c3d5015..dd794dd 100644
--- a/include/hw/arm/fdt.h
+++ b/include/hw/arm/fdt.h
@@ -29,6 +29,6 @@
#define GIC_FDT_IRQ_FLAGS_LEVEL_LO 8
#define GIC_FDT_IRQ_PPI_CPU_START 8
-#define GIC_FDT_IRQ_PPI_CPU_WIDTH 8
+#define GIC_FDT_IRQ_PPI_CPU_WIDTH 24
#endif
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index d22fd8e..852efb9 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -46,6 +46,11 @@ enum {
VIRT_CPUPERIPHS,
VIRT_GIC_DIST,
VIRT_GIC_CPU,
+ VIRT_GIC_V2M,
+ VIRT_GIC_DIST_MBI = VIRT_GIC_CPU,
+ VIRT_ITS_CONTROL = VIRT_GIC_V2M,
+ VIRT_ITS_TRANSLATION,
+ VIRT_LPI,
VIRT_UART,
VIRT_MMIO,
VIRT_RTC,
@@ -54,7 +59,6 @@ enum {
VIRT_PCIE_MMIO,
VIRT_PCIE_PIO,
VIRT_PCIE_ECAM,
- VIRT_GIC_V2M,
VIRT_PLATFORM_BUS,
};
--
1.9.5.msysgit.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 1/6] Merge memory_region_init_reservation() into memory_region_init_io()
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 1/6] Merge memory_region_init_reservation() into memory_region_init_io() Pavel Fedin
@ 2015-07-24 16:20 ` Peter Maydell
0 siblings, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2015-07-24 16:20 UTC (permalink / raw)
To: Pavel Fedin
Cc: Eric Auger, Shlomo Pongratz, Shlomo Pongratz, QEMU Developers,
Paolo Bonzini, Christoffer Dall
On 24 July 2015 at 10:55, Pavel Fedin <p.fedin@samsung.com> wrote:
> Just speficying ops = NULL in some cases can be more convenient than having
"specifying"
> two functions. GICv3 code is going to use this. GICv2 code can be refactored
> in a similar way, killing some code duplication.
>
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> include/exec/memory.h | 14 +++++++++++---
> memory.c | 10 +---------
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 1394715..3b5e44e 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -436,6 +436,9 @@ void memory_region_init_alias(MemoryRegion *mr,
> * memory_region_init_rom_device: Initialize a ROM memory region. Writes are
> * handled via callbacks.
> *
> + * If NULL callbacks pointer is given, then I/O space is not supposed to be
> + * handled by QEMU itself. Any access via the memory API will cause an abort().
I know the memory_region_init_reservation() documentation says
this, but is it true? Looking at the memory.c and exec.c code
it seems like we'd return MEMTX_DECODE_ERROR for callers who
care about memory transaction statuses, behave as RAZ/WI
for the bulk of callers that don't, and maybe print some debug
logging, but I can't see a relevant call to abort()...
Paolo?
(For the avoidance of doubt, if this is an error in the doc text
I don't insist on it being fixed before this patch is applied.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 2/6] Implement GIC-500 base class
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 2/6] Implement GIC-500 base class Pavel Fedin
@ 2015-07-24 16:37 ` Peter Maydell
2015-07-26 13:52 ` Pavel Fedin
` (2 more replies)
0 siblings, 3 replies; 32+ messages in thread
From: Peter Maydell @ 2015-07-24 16:37 UTC (permalink / raw)
To: Pavel Fedin
Cc: Shlomo Pongratz, Shlomo Pongratz, QEMU Developers,
Christoffer Dall, Eric Auger
On 24 July 2015 at 10:55, Pavel Fedin <p.fedin@samsung.com> wrote:
> From: Shlomo Pongratz <shlomo.pongratz@huawei.com>
>
> This class is to be used by both software and KVM implementations of GICv3
>
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -0,0 +1,112 @@
> +/*
> + * ARM GIC support
> + *
> + * Copyright (c) 2012 Linaro Limited
> + * Copyright (c) 2015 Huawei.
> + * Written by Peter Maydell
> + * Extended to 64 cores by Shlomo Pongratz
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef HW_ARM_GICV3_COMMON_H
> +#define HW_ARM_GICV3_COMMON_H
> +
> +#include "hw/sysbus.h"
> +#include "hw/intc/arm_gic_common.h"
> +
> +/* Maximum number of possible interrupts, determined by the GIC architecture */
> +#define GICV3_MAXIRQ 1020
So how do LPIs work? They have IDs above 1023.
> +#define GICV3_NCPU 64
Where does '64' come from as a maximum limit?
> +
> +typedef struct gicv3_irq_state {
> + /* The enable bits are only banked for per-cpu interrupts. */
> + uint64_t enabled;
> + uint64_t pending;
> + uint64_t active;
> + uint64_t level;
> + uint64_t group;
Why are these uint64_t ?
> + bool edge_trigger; /* true: edge-triggered, false: level-triggered */
> +} gicv3_irq_state;
> +
> +typedef struct gicv3_sgi_state {
> + uint64_t pending[GICV3_NCPU];
> +} gicv3_sgi_state;
> +
> +typedef struct GICv3State {
> + /*< private >*/
> + SysBusDevice parent_obj;
> + /*< public >*/
> +
> + qemu_irq parent_irq[GICV3_NCPU];
> + qemu_irq parent_fiq[GICV3_NCPU];
> + /* GICD_CTLR; for a GIC with the security extensions the NS banked version
> + * of this register is just an alias of bit 1 of the S banked version.
> + */
> + uint32_t ctlr;
> + /* Sim GICC_CTLR; again, the NS banked version is just aliases of bits of
> + * the S banked register, so our state only needs to store the S version.
> + */
"Sim" ?
> + uint32_t cpu_ctlr[GICV3_NCPU];
> + bool cpu_enabled[GICV3_NCPU];
> +
> + gicv3_irq_state irq_state[GICV3_MAXIRQ];
> + uint64_t irq_target[GICV3_MAXIRQ];
> + uint8_t priority1[GIC_INTERNAL][GICV3_NCPU];
> + uint8_t priority2[GICV3_MAXIRQ - GIC_INTERNAL];
> + uint16_t last_active[GICV3_MAXIRQ][GICV3_NCPU];
> + /* For each SGI on the target CPU, we store 64 bits
> + * indicating which source CPUs have made this SGI
> + * pending on the target CPU. These correspond to
> + * the bytes in the GIC_SPENDSGIR* registers as
> + * read by the target CPU.
This comment doesn't make any sense. You can't fit
64 bits into a byte, and GIC_SPENDSGIR<n> only hold
8 set-pending bits per SGI.
> + */
> + gicv3_sgi_state sgi_state[GIC_NR_SGIS];
> +
> + uint16_t priority_mask[GICV3_NCPU];
> + uint16_t running_irq[GICV3_NCPU];
> + uint16_t running_priority[GICV3_NCPU];
> + uint16_t current_pending[GICV3_NCPU];
> +
> + uint32_t cpu_mask; /* For redistributor */
> + uint32_t num_cpu;
> + MemoryRegion iomem_dist; /* Distributor */
> + MemoryRegion iomem_lpi; /* Redistributor */
> + uint32_t num_irq;
> + uint32_t revision;
> + bool security_levels;
> + int dev_fd; /* kvm device fd if backed by kvm vgic support */
> +} GICv3State;
This whole struct reads like "we just took the GICv2 state
and changed it as little as possible beyond bumping the
NCPU define a bit". That doesn't make me very confident
that it's actually correct for GICv3...
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 4/6] Introduce irqchip type specification for KVM
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 4/6] Introduce irqchip type specification for KVM Pavel Fedin
@ 2015-07-24 16:42 ` Peter Maydell
2015-07-26 13:46 ` Pavel Fedin
0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2015-07-24 16:42 UTC (permalink / raw)
To: Pavel Fedin
Cc: Shlomo Pongratz, Shlomo Pongratz, QEMU Developers,
Christoffer Dall, Eric Auger
On 24 July 2015 at 10:55, Pavel Fedin <p.fedin@samsung.com> wrote:
> This patch introduces kernel_irqchip_type member in Machine class, which
> is passed to kvm_arch_irqchip_create. Machine models which can use vGIC
> now use it in order to supply correct GIC type for KVM capability
> verification. The variable is defined as int in order to be
> architecture-agnostic for potential future uses by other architectures.
This doesn't look right. The board model should just create
the GIC device that it wants, and it should be the job of
the GIC device itself to coordinate with the kvm.c code
if it has to do so to pass the right kind of irqchip type.
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 4/6] Introduce irqchip type specification for KVM
2015-07-24 16:42 ` Peter Maydell
@ 2015-07-26 13:46 ` Pavel Fedin
2015-07-28 17:37 ` Peter Maydell
0 siblings, 1 reply; 32+ messages in thread
From: Pavel Fedin @ 2015-07-26 13:46 UTC (permalink / raw)
To: 'Peter Maydell'
Cc: 'Shlomo Pongratz', 'Shlomo Pongratz',
'QEMU Developers', 'Christoffer Dall',
'Eric Auger'
Hello!
> > This patch introduces kernel_irqchip_type member in Machine class, which
> > is passed to kvm_arch_irqchip_create. Machine models which can use vGIC
> > now use it in order to supply correct GIC type for KVM capability
> > verification. The variable is defined as int in order to be
> > architecture-agnostic for potential future uses by other architectures.
>
> This doesn't look right. The board model should just create
> the GIC device that it wants, and it should be the job of
> the GIC device itself to coordinate with the kvm.c code
> if it has to do so to pass the right kind of irqchip type.
Unfortunately, this is how qemu is designed. kvm_irqchip_create() is called long before machine model's init code (machvirt_init() in our case) is called. So, if we want to check for the right thing, we have to know it before machine model actually starts to instantiate its components.
I don't see how to change this in a more or less simple way. This routine, except capability testing, does one more important thing on old kernels. There it actually creates vGICv2 using KVM_CREATE_IRQCHIP ioctl. And, on non-ARM architectures, this ioctl also works, and this is the only place where creation is done.
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 2/6] Implement GIC-500 base class
2015-07-24 16:37 ` Peter Maydell
@ 2015-07-26 13:52 ` Pavel Fedin
2015-07-26 19:10 ` Peter Maydell
2015-07-31 15:38 ` Peter Maydell
2015-07-26 14:03 ` Pavel Fedin
2015-07-26 14:45 ` Shlomo Pongratz
2 siblings, 2 replies; 32+ messages in thread
From: Pavel Fedin @ 2015-07-26 13:52 UTC (permalink / raw)
To: 'Peter Maydell'
Cc: 'Shlomo Pongratz', 'Shlomo Pongratz',
'QEMU Developers', 'Christoffer Dall',
'Eric Auger'
Hello!
> So how do LPIs work? They have IDs above 1023.
Currently we don't have LPIs. Shlomo's SW emulation did not include them.
> > +#define GICV3_NCPU 64
>
> Where does '64' come from as a maximum limit?
We don't use Aff2 field as far as i know. It's Shlomo's limitation, not my one.
> This whole struct reads like "we just took the GICv2 state
> and changed it as little as possible beyond bumping the
> NCPU define a bit". That doesn't make me very confident
> that it's actually correct for GICv3...
At least, with complete Shlomo's SW emulation code, it works.
And could you be not so strict on this file? It's actually Shlomo's work, not my one, i include it only because i need to base on something. He will fix up the rest of SW emulation stuff.
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 2/6] Implement GIC-500 base class
2015-07-24 16:37 ` Peter Maydell
2015-07-26 13:52 ` Pavel Fedin
@ 2015-07-26 14:03 ` Pavel Fedin
2015-07-26 14:45 ` Shlomo Pongratz
2 siblings, 0 replies; 32+ messages in thread
From: Pavel Fedin @ 2015-07-26 14:03 UTC (permalink / raw)
To: 'Peter Maydell'
Cc: 'Shlomo Pongratz', 'Shlomo Pongratz',
'QEMU Developers', 'Christoffer Dall',
'Eric Auger'
Hello!
> > +
> > +typedef struct gicv3_irq_state {
> > + /* The enable bits are only banked for per-cpu interrupts. */
> > + uint64_t enabled;
> > + uint64_t pending;
> > + uint64_t active;
> > + uint64_t level;
> > + uint64_t group;
>
> Why are these uint64_t ?
I studied the code for a little bit more. These are bitmasks, where bit represents a CPU. Take a look at gicv3_internal.h, GIC_SET/CLEAR/TEST_xxx macros. 'cm' stands for 'cpu mask'.
This is currently not used by my code, this is mainly for Shlomo's SW emulation. However, there is some usage in common class initialization code.
And i will fix up all badly understood comments.
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 2/6] Implement GIC-500 base class
2015-07-24 16:37 ` Peter Maydell
2015-07-26 13:52 ` Pavel Fedin
2015-07-26 14:03 ` Pavel Fedin
@ 2015-07-26 14:45 ` Shlomo Pongratz
2 siblings, 0 replies; 32+ messages in thread
From: Shlomo Pongratz @ 2015-07-26 14:45 UTC (permalink / raw)
To: Peter Maydell
Cc: Shlomo Pongratz, Pavel Fedin, QEMU Developers, Christoffer Dall,
Eric Auger
[-- Attachment #1: Type: text/plain, Size: 5464 bytes --]
Hi,
See inline.
On Friday, July 24, 2015, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 24 July 2015 at 10:55, Pavel Fedin <p.fedin@samsung.com <javascript:;>>
> wrote:
> > From: Shlomo Pongratz <shlomo.pongratz@huawei.com <javascript:;>>
> >
> > This class is to be used by both software and KVM implementations of
> GICv3
> >
>
> > +++ b/include/hw/intc/arm_gicv3_common.h
> > @@ -0,0 +1,112 @@
> > +/*
> > + * ARM GIC support
> > + *
> > + * Copyright (c) 2012 Linaro Limited
> > + * Copyright (c) 2015 Huawei.
> > + * Written by Peter Maydell
> > + * Extended to 64 cores by Shlomo Pongratz
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program 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 General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> along
> > + * with this program; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef HW_ARM_GICV3_COMMON_H
> > +#define HW_ARM_GICV3_COMMON_H
> > +
> > +#include "hw/sysbus.h"
> > +#include "hw/intc/arm_gic_common.h"
> > +
> > +/* Maximum number of possible interrupts, determined by the GIC
> architecture */
> > +#define GICV3_MAXIRQ 1020
>
> So how do LPIs work? They have IDs above 1023.
>
Currently I don't use LPI as the virt virtual machine doesn't use it. There
is no point to write something I can't test.
>
> > +#define GICV3_NCPU 64
>
> Where does '64' come from as a maximum limit?
>
The goal is 128 (the GIC500 limitation) but since the largest builtin is
uint64_t this will require the usage of bitfiled library. Therefore I
thought it should be delayed to future version after the logic part is
debugged.
>
> > +
> > +typedef struct gicv3_irq_state {
> > + /* The enable bits are only banked for per-cpu interrupts. */
> > + uint64_t enabled;
> > + uint64_t pending;
> > + uint64_t active;
> > + uint64_t level;
> > + uint64_t group;
>
> Why are these uint64_t ?
>
These are bitfields. which were enlarged from 8 (old limit) to 64 (new
limit)
>
> > + bool edge_trigger; /* true: edge-triggered, false: level-triggered
> */
> > +} gicv3_irq_state;
> > +
> > +typedef struct gicv3_sgi_state {
> > + uint64_t pending[GICV3_NCPU];
> > +} gicv3_sgi_state;
> > +
> > +typedef struct GICv3State {
> > + /*< private >*/
> > + SysBusDevice parent_obj;
> > + /*< public >*/
> > +
> > + qemu_irq parent_irq[GICV3_NCPU];
> > + qemu_irq parent_fiq[GICV3_NCPU];
> > + /* GICD_CTLR; for a GIC with the security extensions the NS banked
> version
> > + * of this register is just an alias of bit 1 of the S banked
> version.
> > + */
> > + uint32_t ctlr;
> > + /* Sim GICC_CTLR; again, the NS banked version is just aliases of
> bits of
> > + * the S banked register, so our state only needs to store the S
> version.
> > + */
>
> "Sim" ?
>
Should be simulate as I only support system registers and not memory mapped
registers, but keeping this variable makes the code more close to GICv2
code.
>
> > + uint32_t cpu_ctlr[GICV3_NCPU];
> > + bool cpu_enabled[GICV3_NCPU];
> > +
> > + gicv3_irq_state irq_state[GICV3_MAXIRQ];
> > + uint64_t irq_target[GICV3_MAXIRQ];
> > + uint8_t priority1[GIC_INTERNAL][GICV3_NCPU];
> > + uint8_t priority2[GICV3_MAXIRQ - GIC_INTERNAL];
> > + uint16_t last_active[GICV3_MAXIRQ][GICV3_NCPU];
> > + /* For each SGI on the target CPU, we store 64 bits
> > + * indicating which source CPUs have made this SGI
> > + * pending on the target CPU. These correspond to
> > + * the bytes in the GIC_SPENDSGIR* registers as
> > + * read by the target CPU.
>
> This comment doesn't make any sense. You can't fit
> 64 bits into a byte, and GIC_SPENDSGIR<n> only hold
> 8 set-pending bits per SGI.
>
Correct, the comment is wrong and should be removed as spi_mending was
replaced by a structure.
The usage of a structure and not by a uint64_t vector is because there is
no serialization macro for it.
>
> > + */
> > + gicv3_sgi_state sgi_state[GIC_NR_SGIS];
> > +
> > + uint16_t priority_mask[GICV3_NCPU];u
> > + uint16_t running_irq[GICV3_NCPU];
> > + uint16_t running_priority[GICV3_NCPU];
> > + uint16_t current_pending[GICV3_NCPU];
> > +
> > + uint32_t cpu_mask; /* For redistributor */
> > + uint32_t num_cpu;
> > + MemoryRegion iomem_dist; /* Distributor */
> > + MemoryRegion iomem_lpi; /* Redistributor */
> > + uint32_t num_irq;
> > + uint32_t revision;
> > + bool security_levels;
> > + int dev_fd; /* kvm device fd if backed by kvm vgic support */
> > +} GICv3State;
>
> This whole struct reads like "we just took the GICv2 state
> and changed it as little as possible beyond bumping the
> NCPU define a bit". That doesn't make me very confident
> that it's actually correct for GICv3...
>
I admit that the intention of the first is to make as little changes a
possible and yet to be able to boot 64 Linux kernel (which was achieved).
Note that the kernel identified and used the GICv3 driver.
>
> thanks
> -- PMM
>
[-- Attachment #2: Type: text/html, Size: 7539 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 2/6] Implement GIC-500 base class
2015-07-26 13:52 ` Pavel Fedin
@ 2015-07-26 19:10 ` Peter Maydell
2015-07-31 15:38 ` Peter Maydell
1 sibling, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2015-07-26 19:10 UTC (permalink / raw)
To: Pavel Fedin
Cc: Shlomo Pongratz, Shlomo Pongratz, QEMU Developers,
Christoffer Dall, Eric Auger
On 26 July 2015 at 14:52, Pavel Fedin <p.fedin@samsung.com> wrote:
>> This whole struct reads like "we just took the GICv2 state
>> and changed it as little as possible beyond bumping the
>> NCPU define a bit". That doesn't make me very confident
>> that it's actually correct for GICv3...
>
> At least, with complete Shlomo's SW emulation code, it works.
> And could you be not so strict on this file? It's actually Shlomo's
> work, not my one, i include it only because i need to base on
> something. He will fix up the rest of SW emulation stuff.
Data structures are important -- they're the foundation on
which everything else rests (and at some point they get
baked into VM migration formats, though we aren't at that
point yet). It's worth getting them right from the start.
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 4/6] Introduce irqchip type specification for KVM
2015-07-26 13:46 ` Pavel Fedin
@ 2015-07-28 17:37 ` Peter Maydell
0 siblings, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2015-07-28 17:37 UTC (permalink / raw)
To: Pavel Fedin
Cc: Shlomo Pongratz, Shlomo Pongratz, QEMU Developers,
Christoffer Dall, Eric Auger
On 26 July 2015 at 14:46, Pavel Fedin <p.fedin@samsung.com> wrote:
> Unfortunately, this is how qemu is designed. kvm_irqchip_create() is
> called long before machine model's init code (machvirt_init() in our
> case) is called. So, if we want to check for the right thing, we have
> to know it before machine model actually starts to instantiate its
> components.
>
> I don't see how to change this in a more or less simple way. This
> routine, except capability testing, does one more important thing
> on old kernels. There it actually creates vGICv2 using
> KVM_CREATE_IRQCHIP ioctl.
Right, but all we really need to do in kvm_irqchip_create()
is confirm whether we can use the new API later; we don't
actually need to care whether we will later be creating a
GICv2 or a GICv3 (or whether the kernel supports GICv3).
Rather than having kvm_irqchip_create() pile up a list of
every irqchip type we might want later, I think it's probably
better to just have it do the check for KVM_CAP_DEVICE_CTRL.
If that check fails, we are on an old kernel, and should
return 0 so we do the old-style KVM_CREATE_IRQCHIP. If it succeeds,
then we're on a new kernel, and can postpone doing any GIC
init until the GIC device object is created. Then the GIC
device object should just try to do the kvm_create_device()
for the relevant kind of GIC. If we're trying to create a
GICv3 on a kernel that only knows about a GICv2 then this
will fail at this point and we can display a suitable error
message.
Christoffer: was there a particular reason you wrote kvm_irqchip_create()
to do a test device-create as well as check the capability bit?
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3 Pavel Fedin
@ 2015-07-30 17:00 ` Peter Maydell
2015-07-31 9:32 ` Pavel Fedin
2015-08-03 15:30 ` Pavel Fedin
0 siblings, 2 replies; 32+ messages in thread
From: Peter Maydell @ 2015-07-30 17:00 UTC (permalink / raw)
To: Pavel Fedin
Cc: Shlomo Pongratz, Shlomo Pongratz, QEMU Developers,
Christoffer Dall, Eric Auger
Hi. I have been reading the GIC specs over the last few days and
I think I'm getting closer to understanding what we need to do
with this series to get it to a committable state (without tying
it up with GICv3 emulation). I'll send some mail about that
tomorrow probably, when I've figured out the details. In
the meantime, some review comments below (mostly minor stuff).
On 24 July 2015 at 10:55, Pavel Fedin <p.fedin@samsung.com> wrote:
> Get/put routines are missing, live migration is not possible.
This commit message could do with being made a bit less terse.
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> ---
> hw/intc/Makefile.objs | 3 +
> hw/intc/arm_gicv3_kvm.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 158 insertions(+)
> create mode 100644 hw/intc/arm_gicv3_kvm.c
>
> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
> index 1317e5a..e2525a8 100644
> --- a/hw/intc/Makefile.objs
> +++ b/hw/intc/Makefile.objs
> @@ -17,6 +17,9 @@ common-obj-$(CONFIG_OPENPIC) += openpic.o
>
> obj-$(CONFIG_APIC) += apic.o apic_common.o
> obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o
> +ifeq ($(ARCH), aarch64) # Only 64-bit KVM can use these
> +obj-$(CONFIG_ARM_GIC_KVM) += arm_gicv3_kvm.o
Does it actually fail to compile in a 32-bit KVM config?
Anyway, probably better to be consistent with how target-arm/Makefile.objs
enables kvm64.o:
obj-$(call land,$(CONFIG_ARM_GIC_KVM),$(TARGET_AARCH64)) += kvm64.o
> +static void kvm_arm_gicv3_reset(DeviceState *dev)
> +{
> + GICv3State *s = ARM_GICV3_COMMON(dev);
> + KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
> +
> + DPRINTF("Reset\n");
> +
> + kgc->parent_reset(dev);
> + kvm_arm_gicv3_put(s);
> +}
If we don't currently do anything in reset then does the GIC just
go wrong on a VM reset?
> +static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
> +{
> + GICv3State *s = KVM_ARM_GICV3(dev);
> + KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
> + Error *local_err = NULL;
> + int ret;
> +
> + DPRINTF("kvm_arm_gicv3_realize\n");
> +
> + kgc->parent_realize(dev, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> +
> + gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL);
> +
> + /* Try to create the device via the device control API */
> + s->dev_fd = -1;
> + ret = kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_V3, false);
> + if (ret >= 0) {
> + s->dev_fd = ret;
> + } else if (ret != -ENODEV && ret != -ENOTSUP) {
Why aren't ENODEV and ENOTSUP fatal errors like other errnos?
> + error_setg_errno(errp, -ret, "error creating in-kernel VGIC");
> + return;
> + }
> +
> + if (kvm_gic_supports_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS, 0)) {
Is there any kernel which supports GICv3 but does not support
this attribute? I would hope not, in which case we can skip the
conditional check for support.
> + uint32_t numirqs = s->num_irq;
> + DPRINTF("KVM_DEV_ARM_VGIC_GRP_NR_IRQS = %u\n", numirqs);
> + kvm_gic_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS,
> + 0, 0, &numirqs, 1);
> + }
> +
> + /* Tell the kernel to complete VGIC initialization now */
> + if (kvm_gic_supports_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
> + KVM_DEV_ARM_VGIC_CTRL_INIT)) {
Ditto.
> + DPRINTF("KVM_DEV_ARM_VGIC_CTRL_INIT\n");
> + kvm_gic_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
> + KVM_DEV_ARM_VGIC_CTRL_INIT, 0, 0, 1);
> + }
> +
> + kvm_arm_register_device(&s->iomem_dist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
> + KVM_VGIC_V3_ADDR_TYPE_DIST, s->dev_fd);
> + kvm_arm_register_device(&s->iomem_lpi, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
> + KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd);
> +}
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3
2015-07-30 17:00 ` Peter Maydell
@ 2015-07-31 9:32 ` Pavel Fedin
2015-07-31 10:08 ` Peter Maydell
2015-08-03 15:30 ` Pavel Fedin
1 sibling, 1 reply; 32+ messages in thread
From: Pavel Fedin @ 2015-07-31 9:32 UTC (permalink / raw)
To: 'Peter Maydell'
Cc: 'Shlomo Pongratz', 'Shlomo Pongratz',
'QEMU Developers', 'Christoffer Dall',
'Eric Auger'
Hello!
> On 24 July 2015 at 10:55, Pavel Fedin <p.fedin@samsung.com> wrote:
> > Get/put routines are missing, live migration is not possible.
>
> This commit message could do with being made a bit less terse.
Ok. Subject was self-explanatory, so i didn't have much to add.
> > +static void kvm_arm_gicv3_reset(DeviceState *dev)
> > +{
> > + GICv3State *s = ARM_GICV3_COMMON(dev);
> > + KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
> > +
> > + DPRINTF("Reset\n");
> > +
> > + kgc->parent_reset(dev);
> > + kvm_arm_gicv3_put(s);
> > +}
>
> If we don't currently do anything in reset then does the GIC just
> go wrong on a VM reset?
No it doesn't, reset works.
> > + /* Try to create the device via the device control API */
> > + s->dev_fd = -1;
> > + ret = kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_V3, false);
> > + if (ret >= 0) {
> > + s->dev_fd = ret;
> > + } else if (ret != -ENODEV && ret != -ENOTSUP) {
>
> Why aren't ENODEV and ENOTSUP fatal errors like other errnos?
Stupid leftover from vGICv2 code, copypasted. I have already removed it in my repo, just decided to delay the respin until i know the fate of this: https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg05778.html
>
> > + error_setg_errno(errp, -ret, "error creating in-kernel VGIC");
> > + return;
> > + }
> > +
> > + if (kvm_gic_supports_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS, 0)) {
>
> Is there any kernel which supports GICv3 but does not support
> this attribute? I would hope not, in which case we can skip the
> conditional check for support.
>
> > + uint32_t numirqs = s->num_irq;
> > + DPRINTF("KVM_DEV_ARM_VGIC_GRP_NR_IRQS = %u\n", numirqs);
> > + kvm_gic_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS,
> > + 0, 0, &numirqs, 1);
> > + }
> > +
> > + /* Tell the kernel to complete VGIC initialization now */
> > + if (kvm_gic_supports_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
> > + KVM_DEV_ARM_VGIC_CTRL_INIT)) {
>
> Ditto.
I intentionally put some tracing to these conditions. On my system KVM_DEV_ARM_VGIC_GRP_NR_IRQS is supported and KVM_DEV_ARM_VGIC_CTRL_INIT is not. So will it always be this way?
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3
2015-07-31 9:32 ` Pavel Fedin
@ 2015-07-31 10:08 ` Peter Maydell
2015-07-31 10:22 ` Peter Maydell
0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2015-07-31 10:08 UTC (permalink / raw)
To: Pavel Fedin
Cc: Shlomo Pongratz, Shlomo Pongratz, QEMU Developers,
Christoffer Dall, Eric Auger
On 31 July 2015 at 10:32, Pavel Fedin <p.fedin@samsung.com> wrote:
> Hello!
>
>> On 24 July 2015 at 10:55, Pavel Fedin <p.fedin@samsung.com> wrote:
>> > +static void kvm_arm_gicv3_reset(DeviceState *dev)
>> > +{
>> > + GICv3State *s = ARM_GICV3_COMMON(dev);
>> > + KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
>> > +
>> > + DPRINTF("Reset\n");
>> > +
>> > + kgc->parent_reset(dev);
>> > + kvm_arm_gicv3_put(s);
>> > +}
>>
>> If we don't currently do anything in reset then does the GIC just
>> go wrong on a VM reset?
>
> No it doesn't, reset works.
So who resets the KVM internal GIC state and how?
>> > + error_setg_errno(errp, -ret, "error creating in-kernel VGIC");
>> > + return;
>> > + }
>> > +
>> > + if (kvm_gic_supports_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS, 0)) {
>>
>> Is there any kernel which supports GICv3 but does not support
>> this attribute? I would hope not, in which case we can skip the
>> conditional check for support.
>>
>> > + uint32_t numirqs = s->num_irq;
>> > + DPRINTF("KVM_DEV_ARM_VGIC_GRP_NR_IRQS = %u\n", numirqs);
>> > + kvm_gic_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS,
>> > + 0, 0, &numirqs, 1);
>> > + }
>> > +
>> > + /* Tell the kernel to complete VGIC initialization now */
>> > + if (kvm_gic_supports_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
>> > + KVM_DEV_ARM_VGIC_CTRL_INIT)) {
>>
>> Ditto.
>
> I intentionally put some tracing to these conditions. On my
> system KVM_DEV_ARM_VGIC_GRP_NR_IRQS is supported and
> KVM_DEV_ARM_VGIC_CTRL_INIT is not. So will it always be this way?
That is weird. I thought that the kernel devs had agreed that all
new VGIC code should require explicit initialization (ie that
lazy init-on-first-time-it's-needed was only there for legacy
guests and the old GICv2). The lazy-init stuff was a big source
of bugs where things weren't inited at the point where you
expected, which is why it should have gone away...
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3
2015-07-31 10:08 ` Peter Maydell
@ 2015-07-31 10:22 ` Peter Maydell
2015-07-31 11:12 ` Pavel Fedin
0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2015-07-31 10:22 UTC (permalink / raw)
To: Pavel Fedin
Cc: Eric Auger, Marc Zyngier, Shlomo Pongratz, Shlomo Pongratz,
QEMU Developers, Christoffer Dall
On 31 July 2015 at 11:08, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 31 July 2015 at 10:32, Pavel Fedin <p.fedin@samsung.com> wrote:
>> I intentionally put some tracing to these conditions. On my
>> system KVM_DEV_ARM_VGIC_GRP_NR_IRQS is supported and
>> KVM_DEV_ARM_VGIC_CTRL_INIT is not. So will it always be this way?
>
> That is weird. I thought that the kernel devs had agreed that all
> new VGIC code should require explicit initialization (ie that
> lazy init-on-first-time-it's-needed was only there for legacy
> guests and the old GICv2). The lazy-init stuff was a big source
> of bugs where things weren't inited at the point where you
> expected, which is why it should have gone away...
I just checked with Marc, and he agreed that all kernels
with GICv3 support should support VGIC_CTRL_INIT. What kernel
are you running?
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3
2015-07-31 10:22 ` Peter Maydell
@ 2015-07-31 11:12 ` Pavel Fedin
2015-07-31 11:12 ` Peter Maydell
0 siblings, 1 reply; 32+ messages in thread
From: Pavel Fedin @ 2015-07-31 11:12 UTC (permalink / raw)
To: 'Peter Maydell'
Cc: 'Eric Auger', 'Marc Zyngier',
'Shlomo Pongratz', 'Shlomo Pongratz',
'QEMU Developers', 'Christoffer Dall'
Hello!
> >> I intentionally put some tracing to these conditions. On my
> >> system KVM_DEV_ARM_VGIC_GRP_NR_IRQS is supported and
> >> KVM_DEV_ARM_VGIC_CTRL_INIT is not. So will it always be this way?
> >
> > That is weird. I thought that the kernel devs had agreed that all
> > new VGIC code should require explicit initialization (ie that
> > lazy init-on-first-time-it's-needed was only there for legacy
> > guests and the old GICv2). The lazy-init stuff was a big source
> > of bugs where things weren't inited at the point where you
> > expected, which is why it should have gone away...
>
> I just checked with Marc, and he agreed that all kernels
> with GICv3 support should support VGIC_CTRL_INIT. What kernel
> are you running?
v3.18 with backported GICv3 support. It is a choice of our HW vendor and their current official kernel.
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3
2015-07-31 11:12 ` Pavel Fedin
@ 2015-07-31 11:12 ` Peter Maydell
2015-07-31 11:15 ` Pavel Fedin
0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2015-07-31 11:12 UTC (permalink / raw)
To: Pavel Fedin
Cc: Eric Auger, Marc Zyngier, Shlomo Pongratz, Shlomo Pongratz,
QEMU Developers, Christoffer Dall
On 31 July 2015 at 12:12, Pavel Fedin <p.fedin@samsung.com> wrote:
> Hello!
>
>> >> I intentionally put some tracing to these conditions. On my
>> >> system KVM_DEV_ARM_VGIC_GRP_NR_IRQS is supported and
>> >> KVM_DEV_ARM_VGIC_CTRL_INIT is not. So will it always be this way?
>> >
>> > That is weird. I thought that the kernel devs had agreed that all
>> > new VGIC code should require explicit initialization (ie that
>> > lazy init-on-first-time-it's-needed was only there for legacy
>> > guests and the old GICv2). The lazy-init stuff was a big source
>> > of bugs where things weren't inited at the point where you
>> > expected, which is why it should have gone away...
>>
>> I just checked with Marc, and he agreed that all kernels
>> with GICv3 support should support VGIC_CTRL_INIT. What kernel
>> are you running?
>
> v3.18 with backported GICv3 support. It is a choice of our
> HW vendor and their current official kernel.
Sounds like their backporting is buggy, then. They need to
fix it, I don't think we need to support broken branches
in QEMU.
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3
2015-07-31 11:12 ` Peter Maydell
@ 2015-07-31 11:15 ` Pavel Fedin
2015-07-31 11:24 ` Peter Maydell
0 siblings, 1 reply; 32+ messages in thread
From: Pavel Fedin @ 2015-07-31 11:15 UTC (permalink / raw)
To: 'Peter Maydell'
Cc: 'Eric Auger', 'Marc Zyngier',
'Shlomo Pongratz', 'Shlomo Pongratz',
'QEMU Developers', 'Christoffer Dall'
Hello!
> >> I just checked with Marc, and he agreed that all kernels
> >> with GICv3 support should support VGIC_CTRL_INIT. What kernel
> >> are you running?
> >
> > v3.18 with backported GICv3 support. It is a choice of our
> > HW vendor and their current official kernel.
>
> Sounds like their backporting is buggy, then. They need to
> fix it, I don't think we need to support broken branches
> in QEMU.
Ok, i will recheck it.
So, final conclusion: both conditions should be removed and their code should be executed unconditionally. Correct ?
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3
2015-07-31 11:15 ` Pavel Fedin
@ 2015-07-31 11:24 ` Peter Maydell
0 siblings, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2015-07-31 11:24 UTC (permalink / raw)
To: Pavel Fedin
Cc: Eric Auger, Marc Zyngier, Shlomo Pongratz, Shlomo Pongratz,
QEMU Developers, Christoffer Dall
On 31 July 2015 at 12:15, Pavel Fedin <p.fedin@samsung.com> wrote:
> Hello!
>
>> >> I just checked with Marc, and he agreed that all kernels
>> >> with GICv3 support should support VGIC_CTRL_INIT. What kernel
>> >> are you running?
>> >
>> > v3.18 with backported GICv3 support. It is a choice of our
>> > HW vendor and their current official kernel.
>>
>> Sounds like their backporting is buggy, then. They need to
>> fix it, I don't think we need to support broken branches
>> in QEMU.
>
> Ok, i will recheck it.
> So, final conclusion: both conditions should be removed and their
> code should be executed unconditionally. Correct ?
Yes.
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 2/6] Implement GIC-500 base class
2015-07-26 13:52 ` Pavel Fedin
2015-07-26 19:10 ` Peter Maydell
@ 2015-07-31 15:38 ` Peter Maydell
2015-08-03 12:18 ` Pavel Fedin
1 sibling, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2015-07-31 15:38 UTC (permalink / raw)
To: Pavel Fedin
Cc: Shlomo Pongratz, Shlomo Pongratz, QEMU Developers,
Christoffer Dall, Eric Auger
On 26 July 2015 at 14:52, Pavel Fedin <p.fedin@samsung.com> wrote:
> And could you be not so strict on this file?
OK, so having thought about this this week, this is what I
suggest.
For this patch series we should retain this class hierarchy,
but remove all the fields corresponding to state information
which isn't used by the KVM subclass (and all the accessor
macro definitions, and that GICV3_NCPU definition, and so on).
We should have a migration information struct which says
"not migratable".
That gives us the right structure into which we can
add the correct state fields when we get the design
right (and for whichever of emulation and KVM state
save/load gets there first).
We do need to make sure we get right:
* the class hierarchy
* the API to the rest of QEMU (meaning of GPIO pins,
memory regions, any qdev/qom properties)
(and in the other parts of the patchset, also that we get
the right memory map for the virt board with the GICv3
in it and that the mechanism for selecting GICv2 or GICv3
or "whatever you've got" is good.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 6/6] Add gicversion option to virt machine
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 6/6] Add gicversion option to virt machine Pavel Fedin
@ 2015-07-31 15:44 ` Peter Maydell
2015-08-03 7:19 ` Pavel Fedin
0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2015-07-31 15:44 UTC (permalink / raw)
To: Pavel Fedin
Cc: Shlomo Pongratz, Shlomo Pongratz, QEMU Developers,
Christoffer Dall, Eric Auger
On 24 July 2015 at 10:55, Pavel Fedin <p.fedin@samsung.com> wrote:
> Set kernel_irqchip_type according to value of the option and pass it
> around where necessary. Instantiate devices and fdt nodes according
> to the choice.
>
> max_cpus for virt machine increased to 64. GICv2 compatibility check
> happens inside arm_gic_common_realize().
>
> gicdev = qdev_create(NULL, gictype);
> - qdev_prop_set_uint32(gicdev, "revision", 2);
> +
> + for (i = 0; i < vbi->smp_cpus; i++) {
> + CPUState *cpu = qemu_get_cpu(i);
> + CPUARMState *env = cpu->env_ptr;
> + env->nvic = gicdev;
> + }
We definitely need to come up with a something cleaner
than this (which is ugly for two reasons -- firstly
for borrowing the nvic pointer and secondly because
we shouldn't just be putting random pointers between
the two like this, ideally; and we definitely don't want
the board code to have to do it at this level.)
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 6/6] Add gicversion option to virt machine
2015-07-31 15:44 ` Peter Maydell
@ 2015-08-03 7:19 ` Pavel Fedin
2015-08-03 7:57 ` Peter Maydell
0 siblings, 1 reply; 32+ messages in thread
From: Pavel Fedin @ 2015-08-03 7:19 UTC (permalink / raw)
To: 'Peter Maydell'
Cc: 'Shlomo Pongratz', 'Shlomo Pongratz',
'QEMU Developers', 'Christoffer Dall',
'Eric Auger'
Hello!
> > gicdev = qdev_create(NULL, gictype);
> > - qdev_prop_set_uint32(gicdev, "revision", 2);
> > +
> > + for (i = 0; i < vbi->smp_cpus; i++) {
> > + CPUState *cpu = qemu_get_cpu(i);
> > + CPUARMState *env = cpu->env_ptr;
> > + env->nvic = gicdev;
> > + }
>
> We definitely need to come up with a something cleaner
> than this (which is ugly for two reasons
This could be done:
a) as property
b) as global variable because 'gicdev' is a single of its kind.
But, actually, this is currently only for TCG, which needs it in order to forward system register accesses to GICv3 code. Would it be OK if i just omit this assignment ?
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 6/6] Add gicversion option to virt machine
2015-08-03 7:19 ` Pavel Fedin
@ 2015-08-03 7:57 ` Peter Maydell
2015-08-03 11:41 ` Shlomo Pongratz
0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2015-08-03 7:57 UTC (permalink / raw)
To: Pavel Fedin
Cc: Shlomo Pongratz, Shlomo Pongratz, QEMU Developers,
Christoffer Dall, Eric Auger
On 3 August 2015 at 08:19, Pavel Fedin <p.fedin@samsung.com> wrote:
> Hello!
>
>> > gicdev = qdev_create(NULL, gictype);
>> > - qdev_prop_set_uint32(gicdev, "revision", 2);
>> > +
>> > + for (i = 0; i < vbi->smp_cpus; i++) {
>> > + CPUState *cpu = qemu_get_cpu(i);
>> > + CPUARMState *env = cpu->env_ptr;
>> > + env->nvic = gicdev;
>> > + }
>>
>> We definitely need to come up with a something cleaner
>> than this (which is ugly for two reasons
>
> This could be done:
> a) as property
> b) as global variable because 'gicdev' is a single of its kind.
>
> But, actually, this is currently only for TCG, which needs it in
> order to forward system register accesses to GICv3 code. Would it
> be OK if i just omit this assignment ?
Yes, I think so.
I'm surprised we tell the CPU about the GIC pointer for the
system register stuff -- I was expecting that we'd give the
GIC a CPU pointer. (We could in theory implement some
equivalent of the architected protocol between the redistributors
and the CPU interfaces, but I think that would be overkill.)
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 6/6] Add gicversion option to virt machine
2015-08-03 7:57 ` Peter Maydell
@ 2015-08-03 11:41 ` Shlomo Pongratz
2015-08-03 12:01 ` Peter Maydell
0 siblings, 1 reply; 32+ messages in thread
From: Shlomo Pongratz @ 2015-08-03 11:41 UTC (permalink / raw)
To: Peter Maydell
Cc: Shlomo Pongratz, Pavel Fedin, QEMU Developers, Christoffer Dall,
Eric Auger
[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]
On Monday, August 3, 2015, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 3 August 2015 at 08:19, Pavel Fedin <p.fedin@samsung.com <javascript:;>>
> wrote:
> > Hello!
> >
> >> > gicdev = qdev_create(NULL, gictype);
> >> > - qdev_prop_set_uint32(gicdev, "revision", 2);
> >> > +
> >> > + for (i = 0; i < vbi->smp_cpus; i++) {
> >> > + CPUState *cpu = qemu_get_cpu(i);
> >> > + CPUARMState *env = cpu->env_ptr;
> >> > + env->nvic = gicdev;
> >> > + }
> >>
> >> We definitely need to come up with a something cleaner
> >> than this (which is ugly for two reasons
> >
> > This could be done:
> > a) as property
> > b) as global variable because 'gicdev' is a single of its kind.
> >
> > But, actually, this is currently only for TCG, which needs it in
> > order to forward system register accesses to GICv3 code. Would it
> > be OK if i just omit this assignment ?
>
> Yes, I think so.
>
> I'm surprised we tell the CPU about the GIC pointer for the
> system register stuff -- I was expecting that we'd give the
> GIC a CPU pointer. (We could in theory implement some
> equivalent of the architected protocol between the redistributors
> and the CPU interfaces, but I think that would be overkill.)
>
> -- PMM
>
The problem is that each function added as a system's instruction helper to
target-arm/cpu64.c has the signature of void set(CPUARMState *env,
ARMCPRegInfo *ri, uint64_t value) or uint64_t get(CPUARMState *env,
ARMCPRegInfo *ri)
I just mimicked the way armv7m_nvic_XXXX API works.
So in a sense the CPU must be familiar with the GIC (as an opaque object of
course).
Best regards,
S.P.
[-- Attachment #2: Type: text/html, Size: 2289 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 6/6] Add gicversion option to virt machine
2015-08-03 11:41 ` Shlomo Pongratz
@ 2015-08-03 12:01 ` Peter Maydell
0 siblings, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2015-08-03 12:01 UTC (permalink / raw)
To: Shlomo Pongratz
Cc: Shlomo Pongratz, Pavel Fedin, QEMU Developers, Christoffer Dall,
Eric Auger
On 3 August 2015 at 12:41, Shlomo Pongratz <shlomopongratz@gmail.com> wrote:
>
>
> On Monday, August 3, 2015, Peter Maydell <peter.maydell@linaro.org> wrote:
>> I'm surprised we tell the CPU about the GIC pointer for the
>> system register stuff -- I was expecting that we'd give the
>> GIC a CPU pointer. (We could in theory implement some
>> equivalent of the architected protocol between the redistributors
>> and the CPU interfaces, but I think that would be overkill.)
> The problem is that each function added as a system's instruction helper to
> target-arm/cpu64.c has the signature of void set(CPUARMState *env,
> ARMCPRegInfo *ri, uint64_t value) or uint64_t get(CPUARMState *env,
> ARMCPRegInfo *ri)
> I just mimicked the way armv7m_nvic_XXXX API works.
The v7M NVIC is a terrible example to copy -- it is ancient
code mostly written before QEMU acquired various useful
abstraction layers, and has received very little maintenance
since then.
> So in a sense the CPU must be familiar with the GIC (as an opaque object of
> course).
If the GIC just registers a set of system register information with
the CPU then the CPU doesn't need to know that it's the GIC
providing those system register functions. (You can use
define_arm_cp_regs_with_opaque() to define registers such
that your callbacks can fish an opaque data pointer back out
of ri->opaque, so you can get back to the GIC data structures.)
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 2/6] Implement GIC-500 base class
2015-07-31 15:38 ` Peter Maydell
@ 2015-08-03 12:18 ` Pavel Fedin
0 siblings, 0 replies; 32+ messages in thread
From: Pavel Fedin @ 2015-08-03 12:18 UTC (permalink / raw)
To: 'Peter Maydell'
Cc: 'Shlomo Pongratz', 'Shlomo Pongratz',
'QEMU Developers', 'Christoffer Dall',
'Eric Auger'
Hello!
> For this patch series we should retain this class hierarchy,
> but remove all the fields corresponding to state information
> which isn't used by the KVM subclass (and all the accessor
> macro definitions, and that GICV3_NCPU definition, and so on).
> We should have a migration information struct which says
> "not migratable".
Ok. I am wating for my GICv2 refactor review which includes memory API change.
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3
2015-07-30 17:00 ` Peter Maydell
2015-07-31 9:32 ` Pavel Fedin
@ 2015-08-03 15:30 ` Pavel Fedin
1 sibling, 0 replies; 32+ messages in thread
From: Pavel Fedin @ 2015-08-03 15:30 UTC (permalink / raw)
To: 'Peter Maydell'
Cc: 'Shlomo Pongratz', 'Shlomo Pongratz',
'QEMU Developers', 'Christoffer Dall',
'Eric Auger'
Hello!
> > obj-$(CONFIG_APIC) += apic.o apic_common.o
> > obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o
> > +ifeq ($(ARCH), aarch64) # Only 64-bit KVM can use these
> > +obj-$(CONFIG_ARM_GIC_KVM) += arm_gicv3_kvm.o
>
> Does it actually fail to compile in a 32-bit KVM config?
I forgot to reply to this. Yes, it does, because KVM_VGIC_V3_ADDR_TYPE_xxx definitions are available only for 64 bits.
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2015-08-03 15:30 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-24 9:55 [Qemu-devel] [PATCH v7 0/6] vGICv3 support Pavel Fedin
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 1/6] Merge memory_region_init_reservation() into memory_region_init_io() Pavel Fedin
2015-07-24 16:20 ` Peter Maydell
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 2/6] Implement GIC-500 base class Pavel Fedin
2015-07-24 16:37 ` Peter Maydell
2015-07-26 13:52 ` Pavel Fedin
2015-07-26 19:10 ` Peter Maydell
2015-07-31 15:38 ` Peter Maydell
2015-08-03 12:18 ` Pavel Fedin
2015-07-26 14:03 ` Pavel Fedin
2015-07-26 14:45 ` Shlomo Pongratz
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 3/6] Extract some reusable vGIC code Pavel Fedin
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 4/6] Introduce irqchip type specification for KVM Pavel Fedin
2015-07-24 16:42 ` Peter Maydell
2015-07-26 13:46 ` Pavel Fedin
2015-07-28 17:37 ` Peter Maydell
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3 Pavel Fedin
2015-07-30 17:00 ` Peter Maydell
2015-07-31 9:32 ` Pavel Fedin
2015-07-31 10:08 ` Peter Maydell
2015-07-31 10:22 ` Peter Maydell
2015-07-31 11:12 ` Pavel Fedin
2015-07-31 11:12 ` Peter Maydell
2015-07-31 11:15 ` Pavel Fedin
2015-07-31 11:24 ` Peter Maydell
2015-08-03 15:30 ` Pavel Fedin
2015-07-24 9:55 ` [Qemu-devel] [PATCH v7 6/6] Add gicversion option to virt machine Pavel Fedin
2015-07-31 15:44 ` Peter Maydell
2015-08-03 7:19 ` Pavel Fedin
2015-08-03 7:57 ` Peter Maydell
2015-08-03 11:41 ` Shlomo Pongratz
2015-08-03 12:01 ` Peter Maydell
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.