* [Qemu-devel] [PATCH RFC 0/4] vGICv3 support
@ 2015-05-22 10:58 Pavel Fedin
2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 1/4] Add virt-v3 machine that uses GIC-500 Pavel Fedin
` (4 more replies)
0 siblings, 5 replies; 21+ messages in thread
From: Pavel Fedin @ 2015-05-22 10:58 UTC (permalink / raw)
To: qemu-devel; +Cc: Shlomo Pongratz, Pavel Fedin, Ashok Kumar, Eric Auger
This is my alternative to Ashok's vGICv3 patch
(https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg03021.html), which
i am currently working on. It addresses vGIC capability verification issue
(kvm_irqchip_create() / kvm_arch_irqchip_create()), as well as offers better
code structure (v3 code separated from v2).
This patchset applies on top of this:
https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg00943.html. Note that
GIC type selection still relies on machine name (virt-v3 vs virt), and not on
machine option. Since libvirt has recently introduced support for extra options,
i have absolutely nothing against Ashok's approach. I just did not change this
yet because it would affect my testing environment. The aim of this RFC is to
focus on vGICv3 implementation and related changes. And yes, i agree that v2 and
v3 now have some copypasted code, and this is TBD.
Pavel Fedin (4):
Add virt-v3 machine that uses GIC-500
Set kernel_irqchip_type for other ARM boards which use GIC
First bits of vGICv3 support:
Initial implementation of vGICv3.
hw/arm/exynos4_boards.c | 1 +
hw/arm/realview.c | 1 +
hw/arm/vexpress.c | 1 +
hw/arm/virt.c | 148 ++++++++++++++++++++-----
hw/intc/Makefile.objs | 1 +
hw/intc/arm_gicv3_kvm.c | 283 ++++++++++++++++++++++++++++++++++++++++++++++++
include/hw/boards.h | 1 +
include/sysemu/kvm.h | 3 +-
kvm-all.c | 2 +-
stubs/kvm.c | 2 +-
target-arm/kvm.c | 8 +-
11 files changed, 419 insertions(+), 32 deletions(-)
create mode 100644 hw/intc/arm_gicv3_kvm.c
--
1.9.5.msysgit.0
^ permalink raw reply [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH RFC 1/4] Add virt-v3 machine that uses GIC-500 2015-05-22 10:58 [Qemu-devel] [PATCH RFC 0/4] vGICv3 support Pavel Fedin @ 2015-05-22 10:58 ` Pavel Fedin 2015-05-25 14:07 ` Eric Auger 2015-07-01 10:11 ` Christoffer Dall 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 2/4] Set kernel_irqchip_type for other ARM boards which use GIC Pavel Fedin ` (3 subsequent siblings) 4 siblings, 2 replies; 21+ messages in thread From: Pavel Fedin @ 2015-05-22 10:58 UTC (permalink / raw) To: qemu-devel; +Cc: Shlomo Pongratz, Pavel Fedin, Ashok Kumar, Eric Auger This patch introduces kernel_irqchip_type member in Machine class. Currently it it used only by virt machine for its internal purposes, however in future it is to be passed to KVM in kvm_irqchip_create(). The variable is defined as int in order to be architecture agnostic, for potential future users. Signed-off-by: Pavel Fedin <p.fedin@samsung.com> --- hw/arm/virt.c | 148 +++++++++++++++++++++++++++++++++++++++++++--------- include/hw/boards.h | 1 + 2 files changed, 123 insertions(+), 26 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index a1186c5..15724b2 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -66,6 +66,10 @@ enum { VIRT_CPUPERIPHS, VIRT_GIC_DIST, VIRT_GIC_CPU, + VIRT_GIC_DIST_SPI = VIRT_GIC_CPU, + VIRT_ITS_CONTROL, + VIRT_ITS_TRANSLATION, + VIRT_LPI, VIRT_UART, VIRT_MMIO, VIRT_RTC, @@ -107,6 +111,8 @@ typedef struct { #define VIRT_MACHINE_CLASS(klass) \ OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE) +#define TYPE_VIRTV3_MACHINE "virt-v3" + /* Addresses and sizes of our components. * 0..128MB is space for a flash device so we can run bootrom code such as UEFI. * 128MB..256MB is used for miscellaneous device I/O. @@ -121,25 +127,29 @@ typedef struct { */ static const MemMapEntry a15memmap[] = { /* Space up to 0x8000000 is reserved for a boot ROM */ - [VIRT_FLASH] = { 0, 0x08000000 }, - [VIRT_CPUPERIPHS] = { 0x08000000, 0x00020000 }, + [VIRT_FLASH] = { 0, 0x08000000 }, + [VIRT_CPUPERIPHS] = { 0x08000000, 0x00020000 }, /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */ - [VIRT_GIC_DIST] = { 0x08000000, 0x00010000 }, - [VIRT_GIC_CPU] = { 0x08010000, 0x00010000 }, - [VIRT_UART] = { 0x09000000, 0x00001000 }, - [VIRT_RTC] = { 0x09010000, 0x00001000 }, - [VIRT_FW_CFG] = { 0x09020000, 0x0000000a }, - [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, + [VIRT_GIC_DIST] = { 0x08000000, 0x00010000 }, + [VIRT_GIC_CPU] = { 0x08010000, 0x00010000 }, + /* On v3 VIRT_GIC_DIST_SPI takes place of VIRT_GIC_CPU */ + [VIRT_ITS_CONTROL] = { 0x08020000, 0x00010000 }, + [VIRT_ITS_TRANSLATION] = { 0x08030000, 0x00010000 }, + [VIRT_LPI] = { 0x08040000, 0x00800000 }, + [VIRT_UART] = { 0x09000000, 0x00001000 }, + [VIRT_RTC] = { 0x09010000, 0x00001000 }, + [VIRT_FW_CFG] = { 0x09020000, 0x0000000a }, + [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ /* * PCIE verbose map: * - * MMIO window { 0x10000000, 0x2eff0000 }, - * PIO window { 0x3eff0000, 0x00010000 }, - * ECAM { 0x3f000000, 0x01000000 }, + * MMIO window { 0x10000000, 0x2eff0000 }, + * PIO window { 0x3eff0000, 0x00010000 }, + * ECAM { 0x3f000000, 0x01000000 }, */ - [VIRT_PCIE] = { 0x10000000, 0x30000000 }, - [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 }, + [VIRT_PCIE] = { 0x10000000, 0x30000000 }, + [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 }, }; static const int a15irqmap[] = { @@ -273,9 +283,11 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi) */ ARMCPU *armcpu; uint32_t irqflags = GIC_FDT_IRQ_FLAGS_EDGE_LO_HI; + /* Argument is 32 bit but 8 bits are reserved for flags */ + uint32_t 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"); @@ -299,6 +311,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); @@ -326,7 +350,7 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi) } } -static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi) +static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi, int type) { uint32_t gic_phandle; @@ -334,35 +358,65 @@ static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi) qemu_fdt_setprop_cell(vbi->fdt, "/", "interrupt-parent", 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", + 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, +#if 0 /* Currently no need for SPI & ITS */ + 2, vbi->memmap[VIRT_GIC_DIST_SPI].base, + 2, vbi->memmap[VIRT_GIC_DIST_SPI].size, + 2, vbi->memmap[VIRT_ITS_CONTROL].base, + 2, vbi->memmap[VIRT_ITS_CONTROL].size, + 2, vbi->memmap[VIRT_ITS_TRANSLATION].base, + 2, vbi->memmap[VIRT_ITS_TRANSLATION].size, +#endif + 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", gic_phandle); return gic_phandle; } -static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic) +static uint32_t create_gic(const 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()) { + if (type == KVM_DEV_TYPE_ARM_VGIC_V3) { + gictype = "arm_gicv3"; + } else if (kvm_irqchip_in_kernel()) { gictype = "kvm-arm-gic"; + } else { + gictype = "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). @@ -372,6 +426,11 @@ static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic) 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, 2, vbi->memmap[VIRT_ITS_CONTROL].base); + sysbus_mmio_map(gicbusdev, 3, vbi->memmap[VIRT_ITS_TRANSLATION].base); + sysbus_mmio_map(gicbusdev, 4, vbi->memmap[VIRT_LPI].base); + } /* Wire the outputs from each CPU's generic timer to the * appropriate GIC PPI inputs, and the GIC's IRQ output to @@ -398,7 +457,7 @@ static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic) pic[i] = qdev_get_gpio_in(gicdev, i); } - return fdt_add_gic_node(vbi); + return fdt_add_gic_node(vbi, type); } static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic) @@ -817,7 +876,7 @@ static void machvirt_init(MachineState *machine) create_flash(vbi); - gic_phandle = create_gic(vbi, pic); + gic_phandle = create_gic(vbi, pic, machine->kernel_irqchip_type); create_uart(vbi, pic); @@ -859,7 +918,7 @@ static void virt_set_secure(Object *obj, bool value, Error **errp) vms->secure = value; } -static void virt_instance_init(Object *obj) +static void virt_instance_init_common(Object *obj) { VirtMachineState *vms = VIRT_MACHINE(obj); @@ -873,6 +932,14 @@ static void virt_instance_init(Object *obj) NULL); } +static void virt_instance_init(Object *obj) +{ + MachineState *ms = MACHINE(obj); + + ms->kernel_irqchip_type = KVM_DEV_TYPE_ARM_VGIC_V2; + virt_instance_init_common(obj); +} + static void virt_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); @@ -892,9 +959,38 @@ static const TypeInfo machvirt_info = { .class_init = virt_class_init, }; +static void virtv3_instance_init(Object *obj) +{ + MachineState *ms = MACHINE(obj); + + ms->kernel_irqchip_type = KVM_DEV_TYPE_ARM_VGIC_V3; + virt_instance_init_common(obj); +} + +static void virtv3_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + + mc->name = TYPE_VIRTV3_MACHINE; + mc->desc = "ARM Virtual Machine with GICv3", + mc->init = machvirt_init; + /* With gic3 full implementation (with bitops) rase the lmit to 128 */ + mc->max_cpus = 64; +} + +static const TypeInfo machvirtv3_info = { + .name = TYPE_VIRTV3_MACHINE, + .parent = TYPE_VIRT_MACHINE, + .instance_size = sizeof(VirtMachineState), + .instance_init = virtv3_instance_init, + .class_size = sizeof(VirtMachineClass), + .class_init = virtv3_class_init, +}; + static void machvirt_machine_init(void) { type_register_static(&machvirt_info); + type_register_static(&machvirtv3_info); } machine_init(machvirt_machine_init); diff --git a/include/hw/boards.h b/include/hw/boards.h index 1f11881..3eb32f2 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -138,6 +138,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; -- 1.9.5.msysgit.0 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/4] Add virt-v3 machine that uses GIC-500 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 1/4] Add virt-v3 machine that uses GIC-500 Pavel Fedin @ 2015-05-25 14:07 ` Eric Auger 2015-05-25 14:51 ` Pavel Fedin 2015-07-01 10:11 ` Christoffer Dall 1 sibling, 1 reply; 21+ messages in thread From: Eric Auger @ 2015-05-25 14:07 UTC (permalink / raw) To: Pavel Fedin, qemu-devel; +Cc: Shlomo Pongratz, Ashok Kumar Hi Pavel, On 05/22/2015 12:58 PM, Pavel Fedin wrote: > This patch introduces kernel_irqchip_type member in Machine class. Currently it it used only by virt machine for its internal purposes, however in future it is to be passed to KVM in kvm_irqchip_create(). The variable is defined as int in order to be architecture agnostic, for potential future users. Well your patch does much more than that. To me you put additional info in the commit message such as rationale behind introducing a new machine, what does it feature compared to legacy virt, ... > > Signed-off-by: Pavel Fedin <p.fedin@samsung.com> > > --- > hw/arm/virt.c | 148 +++++++++++++++++++++++++++++++++++++++++++--------- > include/hw/boards.h | 1 + > 2 files changed, 123 insertions(+), 26 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index a1186c5..15724b2 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -66,6 +66,10 @@ enum { > VIRT_CPUPERIPHS, > VIRT_GIC_DIST, > VIRT_GIC_CPU, > + VIRT_GIC_DIST_SPI = VIRT_GIC_CPU, I have the same comment as in Shlomo's patch about the naming of those regions. Maybe you use another doc than GICv3 archi spec? If confirmed it matches GICv2M compat region, why not VIRT_GIC_DIST_MBI? > + VIRT_ITS_CONTROL, > + VIRT_ITS_TRANSLATION, > + VIRT_LPI, VIRT_GIC_REDIST? > VIRT_UART, > VIRT_MMIO, > VIRT_RTC, > @@ -107,6 +111,8 @@ typedef struct { > #define VIRT_MACHINE_CLASS(klass) \ > OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE) > > +#define TYPE_VIRTV3_MACHINE "virt-v3" > + > /* Addresses and sizes of our components. > * 0..128MB is space for a flash device so we can run bootrom code such as UEFI. > * 128MB..256MB is used for miscellaneous device I/O. > @@ -121,25 +127,29 @@ typedef struct { > */ > static const MemMapEntry a15memmap[] = { > /* Space up to 0x8000000 is reserved for a boot ROM */ > - [VIRT_FLASH] = { 0, 0x08000000 }, > - [VIRT_CPUPERIPHS] = { 0x08000000, 0x00020000 }, > + [VIRT_FLASH] = { 0, 0x08000000 }, > + [VIRT_CPUPERIPHS] = { 0x08000000, 0x00020000 }, > /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */ > - [VIRT_GIC_DIST] = { 0x08000000, 0x00010000 }, > - [VIRT_GIC_CPU] = { 0x08010000, 0x00010000 }, > - [VIRT_UART] = { 0x09000000, 0x00001000 }, > - [VIRT_RTC] = { 0x09010000, 0x00001000 }, > - [VIRT_FW_CFG] = { 0x09020000, 0x0000000a }, > - [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, I was advised by Peter, in the past, to handle indent changes in a different patch to ease the review. > + [VIRT_GIC_DIST] = { 0x08000000, 0x00010000 }, > + [VIRT_GIC_CPU] = { 0x08010000, 0x00010000 }, > + /* On v3 VIRT_GIC_DIST_SPI takes place of VIRT_GIC_CPU */ With GICv3 VIRT_GIC_CPU region is not used since system registers are used instead and the memory map hole is reused for GIC DIST MBI region? > + [VIRT_ITS_CONTROL] = { 0x08020000, 0x00010000 }, > + [VIRT_ITS_TRANSLATION] = { 0x08030000, 0x00010000 }, > + [VIRT_LPI] = { 0x08040000, 0x00800000 }, > + [VIRT_UART] = { 0x09000000, 0x00001000 }, > + [VIRT_RTC] = { 0x09010000, 0x00001000 }, > + [VIRT_FW_CFG] = { 0x09020000, 0x0000000a }, > + [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, > /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ > /* > * PCIE verbose map: > * > - * MMIO window { 0x10000000, 0x2eff0000 }, > - * PIO window { 0x3eff0000, 0x00010000 }, > - * ECAM { 0x3f000000, 0x01000000 }, > + * MMIO window { 0x10000000, 0x2eff0000 }, > + * PIO window { 0x3eff0000, 0x00010000 }, > + * ECAM { 0x3f000000, 0x01000000 }, > */ > - [VIRT_PCIE] = { 0x10000000, 0x30000000 }, > - [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 }, > + [VIRT_PCIE] = { 0x10000000, 0x30000000 }, > + [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 }, > }; > > static const int a15irqmap[] = { > @@ -273,9 +283,11 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi) > */ > ARMCPU *armcpu; > uint32_t irqflags = GIC_FDT_IRQ_FLAGS_EDGE_LO_HI; > + /* Argument is 32 bit but 8 bits are reserved for flags */ argument of what? > + uint32_t 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"); > > @@ -299,6 +311,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); > @@ -326,7 +350,7 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi) > } > } > > -static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi) > +static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi, int type) > { > uint32_t gic_phandle; > > @@ -334,35 +358,65 @@ static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi) > qemu_fdt_setprop_cell(vbi->fdt, "/", "interrupt-parent", 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); Documentation/devicetree/bindings/arm/gic-v3.txt says "The main GIC node must contain the appropriate #address-cells, #size-cells and ranges properties for the reg property of all ITS nodes." or to be done when adding the ITS nodes later on ... > qemu_fdt_setprop(vbi->fdt, "/intc", "interrupt-controller", NULL, 0); > - qemu_fdt_setprop_sized_cells(vbi->fdt, "/intc", "reg", > + 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, > +#if 0 /* Currently no need for SPI & ITS */ s/SPI/MBI, dead code to be removed when moving to PATCH > + 2, vbi->memmap[VIRT_GIC_DIST_SPI].base, > + 2, vbi->memmap[VIRT_GIC_DIST_SPI].size, > + 2, vbi->memmap[VIRT_ITS_CONTROL].base, > + 2, vbi->memmap[VIRT_ITS_CONTROL].size, > + 2, vbi->memmap[VIRT_ITS_TRANSLATION].base, > + 2, vbi->memmap[VIRT_ITS_TRANSLATION].size, indeed to be moved in an ITS sub-node > +#endif > + 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", gic_phandle); > > return gic_phandle; > } > > -static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic) > +static uint32_t create_gic(const 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()) { > + if (type == KVM_DEV_TYPE_ARM_VGIC_V3) { > + gictype = "arm_gicv3"; > + } else if (kvm_irqchip_in_kernel()) { > gictype = "kvm-arm-gic"; > + } else { > + gictype = "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). > @@ -372,6 +426,11 @@ static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic) > 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, 2, vbi->memmap[VIRT_ITS_CONTROL].base); > + sysbus_mmio_map(gicbusdev, 3, vbi->memmap[VIRT_ITS_TRANSLATION].base); > + sysbus_mmio_map(gicbusdev, 4, vbi->memmap[VIRT_LPI].base); > + } > > /* Wire the outputs from each CPU's generic timer to the > * appropriate GIC PPI inputs, and the GIC's IRQ output to > @@ -398,7 +457,7 @@ static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic) > pic[i] = qdev_get_gpio_in(gicdev, i); > } > > - return fdt_add_gic_node(vbi); > + return fdt_add_gic_node(vbi, type); > } > > static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic) > @@ -817,7 +876,7 @@ static void machvirt_init(MachineState *machine) > > create_flash(vbi); > > - gic_phandle = create_gic(vbi, pic); > + gic_phandle = create_gic(vbi, pic, machine->kernel_irqchip_type); > > create_uart(vbi, pic); > > @@ -859,7 +918,7 @@ static void virt_set_secure(Object *obj, bool value, Error **errp) > vms->secure = value; > } > > -static void virt_instance_init(Object *obj) > +static void virt_instance_init_common(Object *obj) > { > VirtMachineState *vms = VIRT_MACHINE(obj); > > @@ -873,6 +932,14 @@ static void virt_instance_init(Object *obj) > NULL); > } > > +static void virt_instance_init(Object *obj) > +{ > + MachineState *ms = MACHINE(obj); > + > + ms->kernel_irqchip_type = KVM_DEV_TYPE_ARM_VGIC_V2; > + virt_instance_init_common(obj); > +} > + > static void virt_class_init(ObjectClass *oc, void *data) > { > MachineClass *mc = MACHINE_CLASS(oc); > @@ -892,9 +959,38 @@ static const TypeInfo machvirt_info = { > .class_init = virt_class_init, > }; > > +static void virtv3_instance_init(Object *obj) > +{ > + MachineState *ms = MACHINE(obj); > + > + ms->kernel_irqchip_type = KVM_DEV_TYPE_ARM_VGIC_V3; > + virt_instance_init_common(obj); > +} > + > +static void virtv3_class_init(ObjectClass *oc, void *data) > +{ > + MachineClass *mc = MACHINE_CLASS(oc); > + > + mc->name = TYPE_VIRTV3_MACHINE; > + mc->desc = "ARM Virtual Machine with GICv3", > + mc->init = machvirt_init; > + /* With gic3 full implementation (with bitops) rase the lmit to 128 */ raise, limit > + mc->max_cpus = 64; > +} > + > +static const TypeInfo machvirtv3_info = { > + .name = TYPE_VIRTV3_MACHINE, > + .parent = TYPE_VIRT_MACHINE, > + .instance_size = sizeof(VirtMachineState), > + .instance_init = virtv3_instance_init, > + .class_size = sizeof(VirtMachineClass), > + .class_init = virtv3_class_init, > +}; > + > static void machvirt_machine_init(void) > { > type_register_static(&machvirt_info); > + type_register_static(&machvirtv3_info); > } So about choice between using a new machine name/reusing legacy with prop and interrupt controller type in machine base class I let Peter give his guidance... Best Regards Eric > > machine_init(machvirt_machine_init); > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 1f11881..3eb32f2 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -138,6 +138,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; > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/4] Add virt-v3 machine that uses GIC-500 2015-05-25 14:07 ` Eric Auger @ 2015-05-25 14:51 ` Pavel Fedin 0 siblings, 0 replies; 21+ messages in thread From: Pavel Fedin @ 2015-05-25 14:51 UTC (permalink / raw) To: 'Eric Auger', qemu-devel Cc: 'Shlomo Pongratz', 'Ashok Kumar' > Documentation/devicetree/bindings/arm/gic-v3.txt says > "The main GIC node must contain the appropriate #address-cells, > #size-cells and ranges properties for the reg property of all ITS > nodes." or to be done when adding the ITS nodes later on ... To be done. Currently we don't have an ITS and i've just started working on one. > So about choice between using a new machine name/reusing legacy with > prop and interrupt controller type in machine base class I let Peter > give his guidance... I got messed up by lots of CC's to different people... I have recently sent a email about my findings regarding this and didn't cc: to you... https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg04842.html Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/4] Add virt-v3 machine that uses GIC-500 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 1/4] Add virt-v3 machine that uses GIC-500 Pavel Fedin 2015-05-25 14:07 ` Eric Auger @ 2015-07-01 10:11 ` Christoffer Dall 1 sibling, 0 replies; 21+ messages in thread From: Christoffer Dall @ 2015-07-01 10:11 UTC (permalink / raw) To: Pavel Fedin; +Cc: Shlomo Pongratz, qemu-devel, Ashok Kumar, Eric Auger On Fri, May 22, 2015 at 01:58:41PM +0300, Pavel Fedin wrote: > This patch introduces kernel_irqchip_type member in Machine class. Currently it it used only by virt machine for its internal purposes, however in future it is to be passed to KVM in kvm_irqchip_create(). The variable is defined as int in order to be architecture agnostic, for potential future users. Can you use a decent editor/mail agent and break your lines at 72 chars for commit messages please? This commit message is very code-specific and doesn't explain the overall change/purpose; for example I don't understand from reading this commit if the patch expects a new machine virt-v3 or using machine properties as discussed before. I think it's the former and I think you should re-spin these patches using a property, since Peter already clearly expressed how this should be done and it's no use reviewing something which we already know is not the right approach: https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg02204.html Thanks, -Christoffer > > Signed-off-by: Pavel Fedin <p.fedin@samsung.com> > > --- > hw/arm/virt.c | 148 +++++++++++++++++++++++++++++++++++++++++++--------- > include/hw/boards.h | 1 + > 2 files changed, 123 insertions(+), 26 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index a1186c5..15724b2 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -66,6 +66,10 @@ enum { > VIRT_CPUPERIPHS, > VIRT_GIC_DIST, > VIRT_GIC_CPU, > + VIRT_GIC_DIST_SPI = VIRT_GIC_CPU, > + VIRT_ITS_CONTROL, > + VIRT_ITS_TRANSLATION, > + VIRT_LPI, > VIRT_UART, > VIRT_MMIO, > VIRT_RTC, > @@ -107,6 +111,8 @@ typedef struct { > #define VIRT_MACHINE_CLASS(klass) \ > OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE) > > +#define TYPE_VIRTV3_MACHINE "virt-v3" > + > /* Addresses and sizes of our components. > * 0..128MB is space for a flash device so we can run bootrom code such as UEFI. > * 128MB..256MB is used for miscellaneous device I/O. > @@ -121,25 +127,29 @@ typedef struct { > */ > static const MemMapEntry a15memmap[] = { > /* Space up to 0x8000000 is reserved for a boot ROM */ > - [VIRT_FLASH] = { 0, 0x08000000 }, > - [VIRT_CPUPERIPHS] = { 0x08000000, 0x00020000 }, > + [VIRT_FLASH] = { 0, 0x08000000 }, > + [VIRT_CPUPERIPHS] = { 0x08000000, 0x00020000 }, > /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */ > - [VIRT_GIC_DIST] = { 0x08000000, 0x00010000 }, > - [VIRT_GIC_CPU] = { 0x08010000, 0x00010000 }, > - [VIRT_UART] = { 0x09000000, 0x00001000 }, > - [VIRT_RTC] = { 0x09010000, 0x00001000 }, > - [VIRT_FW_CFG] = { 0x09020000, 0x0000000a }, > - [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, > + [VIRT_GIC_DIST] = { 0x08000000, 0x00010000 }, > + [VIRT_GIC_CPU] = { 0x08010000, 0x00010000 }, > + /* On v3 VIRT_GIC_DIST_SPI takes place of VIRT_GIC_CPU */ > + [VIRT_ITS_CONTROL] = { 0x08020000, 0x00010000 }, > + [VIRT_ITS_TRANSLATION] = { 0x08030000, 0x00010000 }, > + [VIRT_LPI] = { 0x08040000, 0x00800000 }, > + [VIRT_UART] = { 0x09000000, 0x00001000 }, > + [VIRT_RTC] = { 0x09010000, 0x00001000 }, > + [VIRT_FW_CFG] = { 0x09020000, 0x0000000a }, > + [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, > /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ > /* > * PCIE verbose map: > * > - * MMIO window { 0x10000000, 0x2eff0000 }, > - * PIO window { 0x3eff0000, 0x00010000 }, > - * ECAM { 0x3f000000, 0x01000000 }, > + * MMIO window { 0x10000000, 0x2eff0000 }, > + * PIO window { 0x3eff0000, 0x00010000 }, > + * ECAM { 0x3f000000, 0x01000000 }, > */ > - [VIRT_PCIE] = { 0x10000000, 0x30000000 }, > - [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 }, > + [VIRT_PCIE] = { 0x10000000, 0x30000000 }, > + [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 }, > }; > > static const int a15irqmap[] = { > @@ -273,9 +283,11 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi) > */ > ARMCPU *armcpu; > uint32_t irqflags = GIC_FDT_IRQ_FLAGS_EDGE_LO_HI; > + /* Argument is 32 bit but 8 bits are reserved for flags */ > + uint32_t 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"); > > @@ -299,6 +311,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); > @@ -326,7 +350,7 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi) > } > } > > -static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi) > +static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi, int type) > { > uint32_t gic_phandle; > > @@ -334,35 +358,65 @@ static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi) > qemu_fdt_setprop_cell(vbi->fdt, "/", "interrupt-parent", 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", > + 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, > +#if 0 /* Currently no need for SPI & ITS */ > + 2, vbi->memmap[VIRT_GIC_DIST_SPI].base, > + 2, vbi->memmap[VIRT_GIC_DIST_SPI].size, > + 2, vbi->memmap[VIRT_ITS_CONTROL].base, > + 2, vbi->memmap[VIRT_ITS_CONTROL].size, > + 2, vbi->memmap[VIRT_ITS_TRANSLATION].base, > + 2, vbi->memmap[VIRT_ITS_TRANSLATION].size, > +#endif > + 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", gic_phandle); > > return gic_phandle; > } > > -static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic) > +static uint32_t create_gic(const 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()) { > + if (type == KVM_DEV_TYPE_ARM_VGIC_V3) { > + gictype = "arm_gicv3"; > + } else if (kvm_irqchip_in_kernel()) { > gictype = "kvm-arm-gic"; > + } else { > + gictype = "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). > @@ -372,6 +426,11 @@ static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic) > 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, 2, vbi->memmap[VIRT_ITS_CONTROL].base); > + sysbus_mmio_map(gicbusdev, 3, vbi->memmap[VIRT_ITS_TRANSLATION].base); > + sysbus_mmio_map(gicbusdev, 4, vbi->memmap[VIRT_LPI].base); > + } > > /* Wire the outputs from each CPU's generic timer to the > * appropriate GIC PPI inputs, and the GIC's IRQ output to > @@ -398,7 +457,7 @@ static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic) > pic[i] = qdev_get_gpio_in(gicdev, i); > } > > - return fdt_add_gic_node(vbi); > + return fdt_add_gic_node(vbi, type); > } > > static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic) > @@ -817,7 +876,7 @@ static void machvirt_init(MachineState *machine) > > create_flash(vbi); > > - gic_phandle = create_gic(vbi, pic); > + gic_phandle = create_gic(vbi, pic, machine->kernel_irqchip_type); > > create_uart(vbi, pic); > > @@ -859,7 +918,7 @@ static void virt_set_secure(Object *obj, bool value, Error **errp) > vms->secure = value; > } > > -static void virt_instance_init(Object *obj) > +static void virt_instance_init_common(Object *obj) > { > VirtMachineState *vms = VIRT_MACHINE(obj); > > @@ -873,6 +932,14 @@ static void virt_instance_init(Object *obj) > NULL); > } > > +static void virt_instance_init(Object *obj) > +{ > + MachineState *ms = MACHINE(obj); > + > + ms->kernel_irqchip_type = KVM_DEV_TYPE_ARM_VGIC_V2; > + virt_instance_init_common(obj); > +} > + > static void virt_class_init(ObjectClass *oc, void *data) > { > MachineClass *mc = MACHINE_CLASS(oc); > @@ -892,9 +959,38 @@ static const TypeInfo machvirt_info = { > .class_init = virt_class_init, > }; > > +static void virtv3_instance_init(Object *obj) > +{ > + MachineState *ms = MACHINE(obj); > + > + ms->kernel_irqchip_type = KVM_DEV_TYPE_ARM_VGIC_V3; > + virt_instance_init_common(obj); > +} > + > +static void virtv3_class_init(ObjectClass *oc, void *data) > +{ > + MachineClass *mc = MACHINE_CLASS(oc); > + > + mc->name = TYPE_VIRTV3_MACHINE; > + mc->desc = "ARM Virtual Machine with GICv3", > + mc->init = machvirt_init; > + /* With gic3 full implementation (with bitops) rase the lmit to 128 */ > + mc->max_cpus = 64; > +} > + > +static const TypeInfo machvirtv3_info = { > + .name = TYPE_VIRTV3_MACHINE, > + .parent = TYPE_VIRT_MACHINE, > + .instance_size = sizeof(VirtMachineState), > + .instance_init = virtv3_instance_init, > + .class_size = sizeof(VirtMachineClass), > + .class_init = virtv3_class_init, > +}; > + > static void machvirt_machine_init(void) > { > type_register_static(&machvirt_info); > + type_register_static(&machvirtv3_info); > } > > machine_init(machvirt_machine_init); > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 1f11881..3eb32f2 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -138,6 +138,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; > -- > 1.9.5.msysgit.0 > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH RFC 2/4] Set kernel_irqchip_type for other ARM boards which use GIC 2015-05-22 10:58 [Qemu-devel] [PATCH RFC 0/4] vGICv3 support Pavel Fedin 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 1/4] Add virt-v3 machine that uses GIC-500 Pavel Fedin @ 2015-05-22 10:58 ` Pavel Fedin 2015-05-25 14:07 ` Eric Auger 2015-07-01 10:11 ` Christoffer Dall 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 3/4] First bits of vGICv3 support: Pavel Fedin ` (2 subsequent siblings) 4 siblings, 2 replies; 21+ messages in thread From: Pavel Fedin @ 2015-05-22 10:58 UTC (permalink / raw) To: qemu-devel; +Cc: Shlomo Pongratz, Pavel Fedin, Ashok Kumar, Eric Auger Signed-off-by: Pavel Fedin <p.fedin@samsung.com> --- hw/arm/exynos4_boards.c | 1 + hw/arm/realview.c | 1 + hw/arm/vexpress.c | 1 + 3 files changed, 3 insertions(+) diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c index d644db1..d4136bc 100644 --- a/hw/arm/exynos4_boards.c +++ b/hw/arm/exynos4_boards.c @@ -104,6 +104,7 @@ static Exynos4210State *exynos4_boards_init_common(MachineState *machine, exynos4_machines[board_type].max_cpus); } + machine->kernel_irqchip_type = KVM_DEV_TYPE_ARM_VGIC_V2; exynos4_board_binfo.ram_size = exynos4_board_ram_size[board_type]; exynos4_board_binfo.board_id = exynos4_board_id[board_type]; exynos4_board_binfo.smp_bootreg_addr = diff --git a/hw/arm/realview.c b/hw/arm/realview.c index ef2788d..f670d9f 100644 --- a/hw/arm/realview.c +++ b/hw/arm/realview.c @@ -74,6 +74,7 @@ static void realview_init(MachineState *machine, ram_addr_t ram_size = machine->ram_size; hwaddr periphbase = 0; + machine->kernel_irqchip_type = KVM_DEV_TYPE_ARM_VGIC_V2; switch (board_type) { case BOARD_EB: break; diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c index 8f1a5ea..b0a29f1 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 = KVM_DEV_TYPE_ARM_VGIC_V2; daughterboard->init(vms, machine->ram_size, machine->cpu_model, pic); /* -- 1.9.5.msysgit.0 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 2/4] Set kernel_irqchip_type for other ARM boards which use GIC 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 2/4] Set kernel_irqchip_type for other ARM boards which use GIC Pavel Fedin @ 2015-05-25 14:07 ` Eric Auger 2015-05-25 14:43 ` Pavel Fedin 2015-07-01 10:11 ` Christoffer Dall 1 sibling, 1 reply; 21+ messages in thread From: Eric Auger @ 2015-05-25 14:07 UTC (permalink / raw) To: Pavel Fedin, qemu-devel; +Cc: Shlomo Pongratz, Ashok Kumar On 05/22/2015 12:58 PM, Pavel Fedin wrote: missing commit msg. > Signed-off-by: Pavel Fedin <p.fedin@samsung.com> > --- > hw/arm/exynos4_boards.c | 1 + > hw/arm/realview.c | 1 + > hw/arm/vexpress.c | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c > index d644db1..d4136bc 100644 > --- a/hw/arm/exynos4_boards.c > +++ b/hw/arm/exynos4_boards.c > @@ -104,6 +104,7 @@ static Exynos4210State *exynos4_boards_init_common(MachineState *machine, > exynos4_machines[board_type].max_cpus); > } > > + machine->kernel_irqchip_type = KVM_DEV_TYPE_ARM_VGIC_V2; Are you sure this machine is able to use in-kernel GICv2? > exynos4_board_binfo.ram_size = exynos4_board_ram_size[board_type]; > exynos4_board_binfo.board_id = exynos4_board_id[board_type]; > exynos4_board_binfo.smp_bootreg_addr = > diff --git a/hw/arm/realview.c b/hw/arm/realview.c > index ef2788d..f670d9f 100644 > --- a/hw/arm/realview.c > +++ b/hw/arm/realview.c > @@ -74,6 +74,7 @@ static void realview_init(MachineState *machine, > ram_addr_t ram_size = machine->ram_size; > hwaddr periphbase = 0; > > + machine->kernel_irqchip_type = KVM_DEV_TYPE_ARM_VGIC_V2; same as above > switch (board_type) { > case BOARD_EB: > break; > diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c > index 8f1a5ea..b0a29f1 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 = KVM_DEV_TYPE_ARM_VGIC_V2; only hw/cpu/a15mpcore.c seems to be able to instantiate "kvm-arm-gic" > daughterboard->init(vms, machine->ram_size, machine->cpu_model, pic); > > /* > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 2/4] Set kernel_irqchip_type for other ARM boards which use GIC 2015-05-25 14:07 ` Eric Auger @ 2015-05-25 14:43 ` Pavel Fedin 0 siblings, 0 replies; 21+ messages in thread From: Pavel Fedin @ 2015-05-25 14:43 UTC (permalink / raw) To: 'Eric Auger', qemu-devel Cc: 'Shlomo Pongratz', 'Ashok Kumar' Hello! > > +++ b/hw/arm/exynos4_boards.c > > @@ -104,6 +104,7 @@ static Exynos4210State > *exynos4_boards_init_common(MachineState *machine, > > exynos4_machines[board_type].max_cpus); > > } > > > > + machine->kernel_irqchip_type = KVM_DEV_TYPE_ARM_VGIC_V2; > Are you sure this machine is able to use in-kernel GICv2? I'm not. I just scanned all machine sources for "GIC" string and added this wherever it was found. Actually it is harmless. If the machine cannot use in-kernel VGIC, it will just not use it. Ok, i will know this for the future. Actually i would like to wait for Shlomo's patch integration, because otherwise i violate the main rule "the patchset should apply to current master", and i believe it's not good to specify that "this patches applies on top of YYY one". Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 2/4] Set kernel_irqchip_type for other ARM boards which use GIC 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 2/4] Set kernel_irqchip_type for other ARM boards which use GIC Pavel Fedin 2015-05-25 14:07 ` Eric Auger @ 2015-07-01 10:11 ` Christoffer Dall 1 sibling, 0 replies; 21+ messages in thread From: Christoffer Dall @ 2015-07-01 10:11 UTC (permalink / raw) To: Pavel Fedin; +Cc: Shlomo Pongratz, qemu-devel, Ashok Kumar, Eric Auger Missing commit text completely? On Fri, May 22, 2015 at 01:58:42PM +0300, Pavel Fedin wrote: > Signed-off-by: Pavel Fedin <p.fedin@samsung.com> > --- > hw/arm/exynos4_boards.c | 1 + > hw/arm/realview.c | 1 + > hw/arm/vexpress.c | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c > index d644db1..d4136bc 100644 > --- a/hw/arm/exynos4_boards.c > +++ b/hw/arm/exynos4_boards.c > @@ -104,6 +104,7 @@ static Exynos4210State *exynos4_boards_init_common(MachineState *machine, > exynos4_machines[board_type].max_cpus); > } > > + machine->kernel_irqchip_type = KVM_DEV_TYPE_ARM_VGIC_V2; > exynos4_board_binfo.ram_size = exynos4_board_ram_size[board_type]; > exynos4_board_binfo.board_id = exynos4_board_id[board_type]; > exynos4_board_binfo.smp_bootreg_addr = > diff --git a/hw/arm/realview.c b/hw/arm/realview.c > index ef2788d..f670d9f 100644 > --- a/hw/arm/realview.c > +++ b/hw/arm/realview.c > @@ -74,6 +74,7 @@ static void realview_init(MachineState *machine, > ram_addr_t ram_size = machine->ram_size; > hwaddr periphbase = 0; > > + machine->kernel_irqchip_type = KVM_DEV_TYPE_ARM_VGIC_V2; > switch (board_type) { > case BOARD_EB: > break; > diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c > index 8f1a5ea..b0a29f1 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 = KVM_DEV_TYPE_ARM_VGIC_V2; > daughterboard->init(vms, machine->ram_size, machine->cpu_model, pic); > > /* > -- > 1.9.5.msysgit.0 > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH RFC 3/4] First bits of vGICv3 support: 2015-05-22 10:58 [Qemu-devel] [PATCH RFC 0/4] vGICv3 support Pavel Fedin 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 1/4] Add virt-v3 machine that uses GIC-500 Pavel Fedin 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 2/4] Set kernel_irqchip_type for other ARM boards which use GIC Pavel Fedin @ 2015-05-22 10:58 ` Pavel Fedin 2015-05-25 14:07 ` Eric Auger 2015-07-01 10:13 ` Christoffer Dall 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 4/4] Initial implementation of vGICv3 Pavel Fedin 2015-07-01 10:21 ` [Qemu-devel] [PATCH RFC 0/4] vGICv3 support Christoffer Dall 4 siblings, 2 replies; 21+ messages in thread From: Pavel Fedin @ 2015-05-22 10:58 UTC (permalink / raw) To: qemu-devel; +Cc: Shlomo Pongratz, Pavel Fedin, Ashok Kumar, Eric Auger - Make use of kernel_irqchip_type in kvm_arch_irqchip_create() - Instantiate "kvm-arm-gicv3" class (not implemented yet) for GICv3 with KVM acceleration Signed-off-by: Pavel Fedin <p.fedin@samsung.com> --- hw/arm/virt.c | 6 ++---- include/sysemu/kvm.h | 3 ++- kvm-all.c | 2 +- stubs/kvm.c | 2 +- target-arm/kvm.c | 8 ++++++-- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 15724b2..1e42e59 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -400,11 +400,9 @@ static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic, int type) int i; if (type == KVM_DEV_TYPE_ARM_VGIC_V3) { - gictype = "arm_gicv3"; - } else if (kvm_irqchip_in_kernel()) { - gictype = "kvm-arm-gic"; + gictype = kvm_irqchip_in_kernel() ? "kvm-arm-gicv3" : "arm_gicv3"; } else { - gictype = "arm_gic"; + gictype = kvm_irqchip_in_kernel() ? "kvm-arm-gic" : "arm_gic"; } gicdev = qdev_create(NULL, gictype); diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index 4878959..5d90257 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -424,6 +424,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. * @@ -431,7 +432,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 17a3771..22e2621 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -1393,7 +1393,7 @@ static int 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) { return ret; } else if (ret == 0) { 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.c b/target-arm/kvm.c index 16abbf1..65794cf 100644 --- a/target-arm/kvm.c +++ b/target-arm/kvm.c @@ -579,7 +579,7 @@ 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; @@ -587,11 +587,15 @@ int kvm_arch_irqchip_create(KVMState *s) * 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] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 3/4] First bits of vGICv3 support: 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 3/4] First bits of vGICv3 support: Pavel Fedin @ 2015-05-25 14:07 ` Eric Auger 2015-07-01 10:13 ` Christoffer Dall 1 sibling, 0 replies; 21+ messages in thread From: Eric Auger @ 2015-05-25 14:07 UTC (permalink / raw) To: Pavel Fedin, qemu-devel; +Cc: Shlomo Pongratz, Ashok Kumar On 05/22/2015 12:58 PM, Pavel Fedin wrote: > - Make use of kernel_irqchip_type in kvm_arch_irqchip_create() > - Instantiate "kvm-arm-gicv3" class (not implemented yet) for GICv3 with KVM acceleration I think this patch file should rather be the last one of the series. > > Signed-off-by: Pavel Fedin <p.fedin@samsung.com> > --- > hw/arm/virt.c | 6 ++---- > include/sysemu/kvm.h | 3 ++- > kvm-all.c | 2 +- > stubs/kvm.c | 2 +- > target-arm/kvm.c | 8 ++++++-- > 5 files changed, 12 insertions(+), 9 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 15724b2..1e42e59 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -400,11 +400,9 @@ static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic, int type) > int i; > > if (type == KVM_DEV_TYPE_ARM_VGIC_V3) { > - gictype = "arm_gicv3"; > - } else if (kvm_irqchip_in_kernel()) { > - gictype = "kvm-arm-gic"; > + gictype = kvm_irqchip_in_kernel() ? "kvm-arm-gicv3" : "arm_gicv3"; > } else { > - gictype = "arm_gic"; > + gictype = kvm_irqchip_in_kernel() ? "kvm-arm-gic" : "arm_gic"; > } > > gicdev = qdev_create(NULL, gictype); > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > index 4878959..5d90257 100644 > --- a/include/sysemu/kvm.h > +++ b/include/sysemu/kvm.h > @@ -424,6 +424,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. > * > @@ -431,7 +432,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 17a3771..22e2621 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -1393,7 +1393,7 @@ static int 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) { > return ret; > } else if (ret == 0) { > 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.c b/target-arm/kvm.c > index 16abbf1..65794cf 100644 > --- a/target-arm/kvm.c > +++ b/target-arm/kvm.c > @@ -579,7 +579,7 @@ 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; > > @@ -587,11 +587,15 @@ int kvm_arch_irqchip_create(KVMState *s) > * 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 */ the comment should be above return 0; > + if (type != KVM_DEV_TYPE_ARM_VGIC_V2) { > + return ret; > + } > return 0; > } > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 3/4] First bits of vGICv3 support: 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 3/4] First bits of vGICv3 support: Pavel Fedin 2015-05-25 14:07 ` Eric Auger @ 2015-07-01 10:13 ` Christoffer Dall 1 sibling, 0 replies; 21+ messages in thread From: Christoffer Dall @ 2015-07-01 10:13 UTC (permalink / raw) To: Pavel Fedin; +Cc: Shlomo Pongratz, qemu-devel, Ashok Kumar, Eric Auger Hi Pavel, Please get rid of the trailing colon in the subject message. On Fri, May 22, 2015 at 01:58:43PM +0300, Pavel Fedin wrote: > - Make use of kernel_irqchip_type in kvm_arch_irqchip_create() > - Instantiate "kvm-arm-gicv3" class (not implemented yet) for GICv3 with KVM acceleration I feel like these bullets should be written as paragraphs with a proper explanation of what we are doing here from a high-level point of view. Thanks, -Christoffer > > Signed-off-by: Pavel Fedin <p.fedin@samsung.com> > --- > hw/arm/virt.c | 6 ++---- > include/sysemu/kvm.h | 3 ++- > kvm-all.c | 2 +- > stubs/kvm.c | 2 +- > target-arm/kvm.c | 8 ++++++-- > 5 files changed, 12 insertions(+), 9 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 15724b2..1e42e59 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -400,11 +400,9 @@ static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic, int type) > int i; > > if (type == KVM_DEV_TYPE_ARM_VGIC_V3) { > - gictype = "arm_gicv3"; > - } else if (kvm_irqchip_in_kernel()) { > - gictype = "kvm-arm-gic"; > + gictype = kvm_irqchip_in_kernel() ? "kvm-arm-gicv3" : "arm_gicv3"; > } else { > - gictype = "arm_gic"; > + gictype = kvm_irqchip_in_kernel() ? "kvm-arm-gic" : "arm_gic"; > } > > gicdev = qdev_create(NULL, gictype); > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > index 4878959..5d90257 100644 > --- a/include/sysemu/kvm.h > +++ b/include/sysemu/kvm.h > @@ -424,6 +424,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. > * > @@ -431,7 +432,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 17a3771..22e2621 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -1393,7 +1393,7 @@ static int 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) { > return ret; > } else if (ret == 0) { > 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.c b/target-arm/kvm.c > index 16abbf1..65794cf 100644 > --- a/target-arm/kvm.c > +++ b/target-arm/kvm.c > @@ -579,7 +579,7 @@ 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; > > @@ -587,11 +587,15 @@ int kvm_arch_irqchip_create(KVMState *s) > * 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 [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH RFC 4/4] Initial implementation of vGICv3. 2015-05-22 10:58 [Qemu-devel] [PATCH RFC 0/4] vGICv3 support Pavel Fedin ` (2 preceding siblings ...) 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 3/4] First bits of vGICv3 support: Pavel Fedin @ 2015-05-22 10:58 ` Pavel Fedin 2015-05-22 15:17 ` Eric Auger 2015-07-01 10:21 ` [Qemu-devel] [PATCH RFC 0/4] vGICv3 support Christoffer Dall 4 siblings, 1 reply; 21+ messages in thread From: Pavel Fedin @ 2015-05-22 10:58 UTC (permalink / raw) To: qemu-devel; +Cc: Shlomo Pongratz, Pavel Fedin, Ashok Kumar, 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 | 1 + hw/intc/arm_gicv3_kvm.c | 283 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 284 insertions(+) create mode 100644 hw/intc/arm_gicv3_kvm.c diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs index 41fe9ec..776f517 100644 --- a/hw/intc/Makefile.objs +++ b/hw/intc/Makefile.objs @@ -17,6 +17,7 @@ common-obj-$(CONFIG_OPENPIC) += openpic.o obj-$(CONFIG_APIC) += apic.o apic_common.o obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o +obj-$(CONFIG_ARM_GIC_KVM) += arm_gicv3_kvm.o 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..6f892ea --- /dev/null +++ b/hw/intc/arm_gicv3_kvm.c @@ -0,0 +1,283 @@ +/* + * ARM Generic Interrupt Controller using KVM in-kernel support + * + * Copyright (c) 2012 Linaro Limited + * Written by Peter Maydell + * Save/Restore logic added by Christoffer Dall. + * + * 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" + +//#define DEBUG_GICV3_KVM + +#ifdef DEBUG_GICV3_KVM +static const int debug_gicv3_kvm = 1; +#else +static const int debug_gicv3_kvm = 0; +#endif + +#define DPRINTF(fmt, ...) do { \ + if (debug_gicv3_kvm) { \ + printf("kvm_gicv3: " fmt , ## __VA_ARGS__); \ + } \ + } while (0) + +#define TYPE_KVM_ARM_GICV3 "kvm-arm-gicv3" +#define KVM_ARM_GICV3(obj) \ + OBJECT_CHECK(GICState, (obj), TYPE_KVM_ARM_GICV3) +#define KVM_ARM_GICV3_CLASS(klass) \ + OBJECT_CLASS_CHECK(KVMARMGICClass, (klass), TYPE_KVM_ARM_GICV3) +#define KVM_ARM_GICV3_GET_CLASS(obj) \ + OBJECT_GET_CLASS(KVMARMGICClass, (obj), TYPE_KVM_ARM_GICV3) + +typedef struct KVMARMGICClass { + ARMGICCommonClass parent_class; + DeviceRealize parent_realize; + void (*parent_reset)(DeviceState *dev); +} KVMARMGICClass; + +static void kvm_arm_gicv3_set_irq(void *opaque, int irq, int level) +{ + /* Meaning of the 'irq' parameter: + * [0..N-1] : external interrupts + * [N..N+31] : PPI (internal) interrupts for CPU 0 + * [N+32..N+63] : PPI (internal interrupts for CPU 1 + * ... + * Convert this to the kernel's desired encoding, which + * 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 - GICV3_INTERNAL)) { + /* External interrupt. The kernel numbers these like the GIC + * hardware, with external interrupt IDs starting after the + * internal ones. + */ + irqtype = KVM_ARM_IRQ_TYPE_SPI; + cpu = 0; + irq += GICV3_INTERNAL; + } else { + /* Internal interrupt: decode into (cpu, interrupt id) */ + irqtype = KVM_ARM_IRQ_TYPE_PPI; + irq -= (s->num_irq - GICV3_INTERNAL); + cpu = irq / GICV3_INTERNAL; + irq %= GICV3_INTERNAL; + } + kvm_irq = (irqtype << KVM_ARM_IRQ_TYPE_SHIFT) + | (cpu << KVM_ARM_IRQ_VCPU_SHIFT) | irq; + + kvm_set_irq(kvm_state, kvm_irq, !!level); +} + +static bool kvm_gic_supports_attr(GICState *s, int group, int attrnum) +{ + struct kvm_device_attr attr = { + .group = group, + .attr = attrnum, + .flags = 0, + }; + + if (s->dev_fd == -1) { + return false; + } + + return kvm_device_ioctl(s->dev_fd, KVM_HAS_DEVICE_ATTR, &attr) == 0; +} + +static void kvm_gic_access(GICState *s, int group, int offset, + int cpu, uint32_t *val, bool write) +{ + struct kvm_device_attr attr; + int type; + int err; + + cpu = cpu & 0xff; + + attr.flags = 0; + attr.group = group; + attr.attr = (((uint64_t)cpu << KVM_DEV_ARM_VGIC_CPUID_SHIFT) & + KVM_DEV_ARM_VGIC_CPUID_MASK) | + (((uint64_t)offset << KVM_DEV_ARM_VGIC_OFFSET_SHIFT) & + KVM_DEV_ARM_VGIC_OFFSET_MASK); + attr.addr = (uintptr_t)val; + + if (write) { + type = KVM_SET_DEVICE_ATTR; + } else { + type = KVM_GET_DEVICE_ATTR; + } + + err = kvm_device_ioctl(s->dev_fd, type, &attr); + if (err < 0) { + fprintf(stderr, "KVM_{SET/GET}_DEVICE_ATTR failed: %s\n", + strerror(-err)); + abort(); + } +} + +static void kvm_arm_gicv3_put(GICState *s) +{ + /* TODO */ + DPRINTF("Cannot put kernel gic state, no kernel interface\n"); +} + +static void kvm_arm_gicv3_get(GICState *s) +{ + /* TODO */ + DPRINTF("Cannot get kernel gic state, no kernel interface\n"); +} + +static void kvm_arm_gicv3_reset(DeviceState *dev) +{ + GICState *s = ARM_GIC_COMMON(dev); + KVMARMGICClass *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) +{ + int i; + GICState *s = KVM_ARM_GICV3(dev); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); + KVMARMGICClass *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; + } + + if (s->security_extn) { + error_setg(errp, "the in-kernel VGIC does not implement the " + "security extensions"); + return; + } + + i = s->num_irq - GICV3_INTERNAL; + /* For the GIC, also expose incoming GPIO lines for PPIs for each CPU. + * GPIO array layout is thus: + * [0..N-1] SPIs + * [N..N+31] PPIs for CPU 0 + * [N+32..N+63] PPIs for CPU 1 + * ... + */ + i += (GICV3_INTERNAL * s->num_cpu); + qdev_init_gpio_in(dev, kvm_arm_gicv3_set_irq, i); + /* We never use our outbound IRQ lines but provide them so that + * we maintain the same interface as the non-KVM GIC. + */ + 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]); + } + + /* 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, 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, 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, + KVM_DEV_ARM_VGIC_CTRL_INIT)) { + DPRINTF("KVM_DEV_ARM_VGIC_CTRL_INIT\n"); + kvm_gic_access(s, KVM_DEV_ARM_VGIC_GRP_CTRL, + KVM_DEV_ARM_VGIC_CTRL_INIT, 0, 0, 1); + } + + /* Distributor */ + memory_region_init_reservation(&s->iomem_dist, OBJECT(s), + "kvm-gicv3_dist", 0x10000); + sysbus_init_mmio(sbd, &s->iomem_dist); + kvm_arm_register_device(&s->iomem_dist, -1, + KVM_DEV_ARM_VGIC_GRP_ADDR, + KVM_VGIC_V3_ADDR_TYPE_DIST, + s->dev_fd); + + /* Nothing much to do with SPI and ITS */ + memory_region_init_reservation(&s->iomem_spi, OBJECT(s), + "kvm-gicv3_spi", 0x10000); + sysbus_init_mmio(sbd, &s->iomem_spi); + memory_region_init_reservation(&s->iomem_its_cntrl, OBJECT(s), + "kvm-gicv3_its_control", 0x10000); + sysbus_init_mmio(sbd, &s->iomem_its_cntrl); + memory_region_init_reservation(&s->iomem_its, OBJECT(s), + "kvm-gicv3_its_translation", 0x10000); + sysbus_init_mmio(sbd, &s->iomem_its); + + /* Redistributor */ + memory_region_init_reservation(&s->iomem_lpi, OBJECT(s), + "kvm-gicv3_lpi", 0x800000); + sysbus_init_mmio(sbd, &s->iomem_lpi); + 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); + ARMGICCommonClass *agcc = ARM_GIC_COMMON_CLASS(klass); + KVMARMGICClass *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(GICState), + .class_init = kvm_arm_gicv3_class_init, + .class_size = sizeof(KVMARMGICClass), +}; + +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] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 4/4] Initial implementation of vGICv3. 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 4/4] Initial implementation of vGICv3 Pavel Fedin @ 2015-05-22 15:17 ` Eric Auger 2015-05-22 16:57 ` Pavel Fedin 0 siblings, 1 reply; 21+ messages in thread From: Eric Auger @ 2015-05-22 15:17 UTC (permalink / raw) To: Pavel Fedin, qemu-devel; +Cc: Shlomo Pongratz, Ashok Kumar On 05/22/2015 12:58 PM, Pavel Fedin wrote: > Get/put routines are missing. Live migration is not possible. > > Signed-off-by: Pavel Fedin <p.fedin@samsung.com> > --- > hw/intc/Makefile.objs | 1 + > hw/intc/arm_gicv3_kvm.c | 283 ++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 284 insertions(+) > create mode 100644 hw/intc/arm_gicv3_kvm.c > > diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs > index 41fe9ec..776f517 100644 > --- a/hw/intc/Makefile.objs > +++ b/hw/intc/Makefile.objs > @@ -17,6 +17,7 @@ common-obj-$(CONFIG_OPENPIC) += openpic.o > > obj-$(CONFIG_APIC) += apic.o apic_common.o > obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o > +obj-$(CONFIG_ARM_GIC_KVM) += arm_gicv3_kvm.o > 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..6f892ea > --- /dev/null > +++ b/hw/intc/arm_gicv3_kvm.c > @@ -0,0 +1,283 @@ > +/* > + * ARM Generic Interrupt Controller using KVM in-kernel support > + * > + * Copyright (c) 2012 Linaro Limited > + * Written by Peter Maydell > + * Save/Restore logic added by Christoffer Dall. > + * > + * 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" > + > +//#define DEBUG_GICV3_KVM > + > +#ifdef DEBUG_GICV3_KVM > +static const int debug_gicv3_kvm = 1; > +#else > +static const int debug_gicv3_kvm = 0; > +#endif > + > +#define DPRINTF(fmt, ...) do { \ > + if (debug_gicv3_kvm) { \ > + printf("kvm_gicv3: " fmt , ## __VA_ARGS__); \ > + } \ > + } while (0) > + > +#define TYPE_KVM_ARM_GICV3 "kvm-arm-gicv3" > +#define KVM_ARM_GICV3(obj) \ > + OBJECT_CHECK(GICState, (obj), TYPE_KVM_ARM_GICV3) > +#define KVM_ARM_GICV3_CLASS(klass) \ > + OBJECT_CLASS_CHECK(KVMARMGICClass, (klass), TYPE_KVM_ARM_GICV3) > +#define KVM_ARM_GICV3_GET_CLASS(obj) \ > + OBJECT_GET_CLASS(KVMARMGICClass, (obj), TYPE_KVM_ARM_GICV3) > + > +typedef struct KVMARMGICClass { > + ARMGICCommonClass parent_class; > + DeviceRealize parent_realize; > + void (*parent_reset)(DeviceState *dev); > +} KVMARMGICClass; > + > +static void kvm_arm_gicv3_set_irq(void *opaque, int irq, int level) > +{ > + /* Meaning of the 'irq' parameter: > + * [0..N-1] : external interrupts > + * [N..N+31] : PPI (internal) interrupts for CPU 0 > + * [N+32..N+63] : PPI (internal interrupts for CPU 1 > + * ... > + * Convert this to the kernel's desired encoding, which > + * 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 - GICV3_INTERNAL)) { > + /* External interrupt. The kernel numbers these like the GIC > + * hardware, with external interrupt IDs starting after the > + * internal ones. > + */ > + irqtype = KVM_ARM_IRQ_TYPE_SPI; > + cpu = 0; > + irq += GICV3_INTERNAL; > + } else { > + /* Internal interrupt: decode into (cpu, interrupt id) */ > + irqtype = KVM_ARM_IRQ_TYPE_PPI; > + irq -= (s->num_irq - GICV3_INTERNAL); > + cpu = irq / GICV3_INTERNAL; > + irq %= GICV3_INTERNAL; > + } > + kvm_irq = (irqtype << KVM_ARM_IRQ_TYPE_SHIFT) > + | (cpu << KVM_ARM_IRQ_VCPU_SHIFT) | irq; > + > + kvm_set_irq(kvm_state, kvm_irq, !!level); > +} > + > +static bool kvm_gic_supports_attr(GICState *s, int group, int attrnum) > +{ > + struct kvm_device_attr attr = { > + .group = group, > + .attr = attrnum, > + .flags = 0, > + }; > + > + if (s->dev_fd == -1) { > + return false; > + } > + > + return kvm_device_ioctl(s->dev_fd, KVM_HAS_DEVICE_ATTR, &attr) == 0; > +} > + > +static void kvm_gic_access(GICState *s, int group, int offset, > + int cpu, uint32_t *val, bool write) > +{ > + struct kvm_device_attr attr; > + int type; > + int err; > + > + cpu = cpu & 0xff; > + > + attr.flags = 0; > + attr.group = group; > + attr.attr = (((uint64_t)cpu << KVM_DEV_ARM_VGIC_CPUID_SHIFT) & > + KVM_DEV_ARM_VGIC_CPUID_MASK) | > + (((uint64_t)offset << KVM_DEV_ARM_VGIC_OFFSET_SHIFT) & > + KVM_DEV_ARM_VGIC_OFFSET_MASK); > + attr.addr = (uintptr_t)val; > + > + if (write) { > + type = KVM_SET_DEVICE_ATTR; > + } else { > + type = KVM_GET_DEVICE_ATTR; > + } > + > + err = kvm_device_ioctl(s->dev_fd, type, &attr); > + if (err < 0) { > + fprintf(stderr, "KVM_{SET/GET}_DEVICE_ATTR failed: %s\n", > + strerror(-err)); > + abort(); > + } > +} > + > +static void kvm_arm_gicv3_put(GICState *s) > +{ > + /* TODO */ > + DPRINTF("Cannot put kernel gic state, no kernel interface\n"); > +} > + > +static void kvm_arm_gicv3_get(GICState *s) > +{ > + /* TODO */ > + DPRINTF("Cannot get kernel gic state, no kernel interface\n"); > +} > + > +static void kvm_arm_gicv3_reset(DeviceState *dev) > +{ > + GICState *s = ARM_GIC_COMMON(dev); > + KVMARMGICClass *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) > +{ > + int i; > + GICState *s = KVM_ARM_GICV3(dev); > + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > + KVMARMGICClass *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; > + } > + > + if (s->security_extn) Hi Pavel, Shlomo, Looks GICv3 common class currently miss this security_extn field + parent_fiq so it does not compile without changes. Or did I miss something? Best Regards Eric > + error_setg(errp, "the in-kernel VGIC does not implement the " > + "security extensions"); > + return; > + } > + > + i = s->num_irq - GICV3_INTERNAL; > + /* For the GIC, also expose incoming GPIO lines for PPIs for each CPU. > + * GPIO array layout is thus: > + * [0..N-1] SPIs > + * [N..N+31] PPIs for CPU 0 > + * [N+32..N+63] PPIs for CPU 1 > + * ... > + */ > + i += (GICV3_INTERNAL * s->num_cpu); > + qdev_init_gpio_in(dev, kvm_arm_gicv3_set_irq, i); > + /* We never use our outbound IRQ lines but provide them so that > + * we maintain the same interface as the non-KVM GIC. > + */ > + 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]); > + } > + > + /* 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, 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, 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, > + KVM_DEV_ARM_VGIC_CTRL_INIT)) { > + DPRINTF("KVM_DEV_ARM_VGIC_CTRL_INIT\n"); > + kvm_gic_access(s, KVM_DEV_ARM_VGIC_GRP_CTRL, > + KVM_DEV_ARM_VGIC_CTRL_INIT, 0, 0, 1); > + } > + > + /* Distributor */ > + memory_region_init_reservation(&s->iomem_dist, OBJECT(s), > + "kvm-gicv3_dist", 0x10000); > + sysbus_init_mmio(sbd, &s->iomem_dist); > + kvm_arm_register_device(&s->iomem_dist, -1, > + KVM_DEV_ARM_VGIC_GRP_ADDR, > + KVM_VGIC_V3_ADDR_TYPE_DIST, > + s->dev_fd); > + > + /* Nothing much to do with SPI and ITS */ > + memory_region_init_reservation(&s->iomem_spi, OBJECT(s), > + "kvm-gicv3_spi", 0x10000); > + sysbus_init_mmio(sbd, &s->iomem_spi); > + memory_region_init_reservation(&s->iomem_its_cntrl, OBJECT(s), > + "kvm-gicv3_its_control", 0x10000); > + sysbus_init_mmio(sbd, &s->iomem_its_cntrl); > + memory_region_init_reservation(&s->iomem_its, OBJECT(s), > + "kvm-gicv3_its_translation", 0x10000); > + sysbus_init_mmio(sbd, &s->iomem_its); > + > + /* Redistributor */ > + memory_region_init_reservation(&s->iomem_lpi, OBJECT(s), > + "kvm-gicv3_lpi", 0x800000); > + sysbus_init_mmio(sbd, &s->iomem_lpi); > + 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); > + ARMGICCommonClass *agcc = ARM_GIC_COMMON_CLASS(klass); > + KVMARMGICClass *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(GICState), > + .class_init = kvm_arm_gicv3_class_init, > + .class_size = sizeof(KVMARMGICClass), > +}; > + > +static void kvm_arm_gicv3_register_types(void) > +{ > + type_register_static(&kvm_arm_gicv3_info); > +} > + > +type_init(kvm_arm_gicv3_register_types) > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 4/4] Initial implementation of vGICv3. 2015-05-22 15:17 ` Eric Auger @ 2015-05-22 16:57 ` Pavel Fedin 2015-07-01 10:19 ` Christoffer Dall 0 siblings, 1 reply; 21+ messages in thread From: Pavel Fedin @ 2015-05-22 16:57 UTC (permalink / raw) To: 'Eric Auger', qemu-devel Cc: 'Shlomo Pongratz', 'Ashok Kumar' Hi! > Looks GICv3 common class currently miss this security_extn field + > parent_fiq so it does not compile without changes. Or did I miss something? Just throw this if(...) away. It's my fault. Actually i have rebased Shlomo's patches on yesterday's master, and during this i added parent_fiq[] to GICv3, because without filling these in the thing stopped working due to recent GICv2 code changes. I needed to fix the problem quickly, so i copied initializing parent_fiq together with security_extn field from v2 code. But, this makes no sense because security_extn is never initialized, it is not even assigned property name. So just remove this small fragment, it's not needed now. I have fixed this in my working tree 2 hours ago. parent_fiq is declared as: --- cut --- qemu_irq parent_fiq[GICV3_NCPU]; --- cut --- Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 4/4] Initial implementation of vGICv3. 2015-05-22 16:57 ` Pavel Fedin @ 2015-07-01 10:19 ` Christoffer Dall 0 siblings, 0 replies; 21+ messages in thread From: Christoffer Dall @ 2015-07-01 10:19 UTC (permalink / raw) To: Pavel Fedin Cc: 'Shlomo Pongratz', qemu-devel, 'Ashok Kumar', 'Eric Auger' Hi Pavel, On Fri, May 22, 2015 at 07:57:13PM +0300, Pavel Fedin wrote: > Hi! > > > Looks GICv3 common class currently miss this security_extn field + > > parent_fiq so it does not compile without changes. Or did I miss something? > > Just throw this if(...) away. It's my fault. Actually i have rebased Shlomo's patches on > yesterday's master, and during this i added parent_fiq[] to GICv3, because without filling > these in the thing stopped working due to recent GICv2 code changes. I needed to fix the > problem quickly, so i copied initializing parent_fiq together with security_extn field > from v2 code. But, this makes no sense because security_extn is never initialized, it is > not even assigned property name. So just remove this small fragment, it's not needed now. > I have fixed this in my working tree 2 hours ago. parent_fiq is declared as: > --- cut --- > qemu_irq parent_fiq[GICV3_NCPU]; > --- cut --- > I think based on this comment that I can safely suggest a bit of advise regarding sending patches to the QEMU and Linux lists: Be as careful as you can, properly test your code, review your own commit messages, run spell check on them, and try to look at what you send out through the eyes of someone who has never seen this code before. Review resources are really scarse in these projects and almost all maintainers are overloaded, so we all have to work together and make sure we don't flood the lists unwarranted. It is good to re-spin quickly so people don't loose context, but if you are fixing things up in a matter of hours and sending out new revisions, you are moving too fast. Turn-around time is days/weeks in the best cases, so better to take a few extra days to thoroughly test a patch series. The exception is to illustrate a general idea or design, in which case clearly marking the series as an RFC and noting what people should look at and what people should ignore is key. Hope this makes sense, -Christoffer ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 0/4] vGICv3 support 2015-05-22 10:58 [Qemu-devel] [PATCH RFC 0/4] vGICv3 support Pavel Fedin ` (3 preceding siblings ...) 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 4/4] Initial implementation of vGICv3 Pavel Fedin @ 2015-07-01 10:21 ` Christoffer Dall 2015-07-01 10:26 ` Daniel P. Berrange 2015-07-01 11:14 ` Pavel Fedin 4 siblings, 2 replies; 21+ messages in thread From: Christoffer Dall @ 2015-07-01 10:21 UTC (permalink / raw) To: Pavel Fedin; +Cc: Shlomo Pongratz, qemu-devel, Ashok Kumar, Eric Auger On Fri, May 22, 2015 at 01:58:40PM +0300, Pavel Fedin wrote: > This is my alternative to Ashok's vGICv3 patch > (https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg03021.html), which > i am currently working on. It addresses vGIC capability verification issue > (kvm_irqchip_create() / kvm_arch_irqchip_create()), as well as offers better > code structure (v3 code separated from v2). > This patchset applies on top of this: > https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg00943.html. Note that > GIC type selection still relies on machine name (virt-v3 vs virt), and not on > machine option. Since libvirt has recently introduced support for extra options, > i have absolutely nothing against Ashok's approach. I just did not change this > yet because it would affect my testing environment. The aim of this RFC is to > focus on vGICv3 implementation and related changes. And yes, i agree that v2 and > v3 now have some copypasted code, and this is TBD. This cover letter is not really helpful as it only describes the history and circumstances of how this patch came to be. It would be helpful if the beginning of this cover letter focuses on what the patch series does and which design decisions have been taken to shape the patches the way they are. I don't understand the whole background thing about libvirt and I don't at all understand the thing about copy-pasted code...?? A generally good approach to writing a cover letter is to follow this skeleton: --- This series implements...<what> We accomplish this by...<design decisions> [Optional] Patches 1-3 <do something preliminary>, patches 4-8 <do something else>... [Optional] Note <something special, possibly historical> Changes since vX: --- I think it would be good if you could re-spin this series based on Eric's comments on the code, my comments on the patch style, and Peter's advise on using machine properties for GICv3. Do you have cycles to continue working on this? -Christoffer > > Pavel Fedin (4): > Add virt-v3 machine that uses GIC-500 > Set kernel_irqchip_type for other ARM boards which use GIC > First bits of vGICv3 support: > Initial implementation of vGICv3. > > hw/arm/exynos4_boards.c | 1 + > hw/arm/realview.c | 1 + > hw/arm/vexpress.c | 1 + > hw/arm/virt.c | 148 ++++++++++++++++++++----- > hw/intc/Makefile.objs | 1 + > hw/intc/arm_gicv3_kvm.c | 283 ++++++++++++++++++++++++++++++++++++++++++++++++ > include/hw/boards.h | 1 + > include/sysemu/kvm.h | 3 +- > kvm-all.c | 2 +- > stubs/kvm.c | 2 +- > target-arm/kvm.c | 8 +- > 11 files changed, 419 insertions(+), 32 deletions(-) > create mode 100644 hw/intc/arm_gicv3_kvm.c > > -- > 1.9.5.msysgit.0 > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 0/4] vGICv3 support 2015-07-01 10:21 ` [Qemu-devel] [PATCH RFC 0/4] vGICv3 support Christoffer Dall @ 2015-07-01 10:26 ` Daniel P. Berrange 2015-07-01 11:14 ` Pavel Fedin 1 sibling, 0 replies; 21+ messages in thread From: Daniel P. Berrange @ 2015-07-01 10:26 UTC (permalink / raw) To: Christoffer Dall Cc: Shlomo Pongratz, Pavel Fedin, qemu-devel, Ashok Kumar, Eric Auger On Wed, Jul 01, 2015 at 12:21:12PM +0200, Christoffer Dall wrote: > On Fri, May 22, 2015 at 01:58:40PM +0300, Pavel Fedin wrote: > > This is my alternative to Ashok's vGICv3 patch > > (https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg03021.html), which > > i am currently working on. It addresses vGIC capability verification issue > > (kvm_irqchip_create() / kvm_arch_irqchip_create()), as well as offers better > > code structure (v3 code separated from v2). > > This patchset applies on top of this: > > https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg00943.html. Note that > > GIC type selection still relies on machine name (virt-v3 vs virt), and not on > > machine option. Since libvirt has recently introduced support for extra options, > > i have absolutely nothing against Ashok's approach. I just did not change this > > yet because it would affect my testing environment. The aim of this RFC is to > > focus on vGICv3 implementation and related changes. And yes, i agree that v2 and > > v3 now have some copypasted code, and this is TBD. > > This cover letter is not really helpful as it only describes the history > and circumstances of how this patch came to be. > > It would be helpful if the beginning of this cover letter focuses on > what the patch series does and which design decisions have been taken to > shape the patches the way they are. > > I don't understand the whole background thing about libvirt and I don't I replied to the earlier posting of the patch series that the quoted libvirt limitation does not exist any longer, so that really should not be mentioned as a problem/rationale for the machine type approach anymore. I agree with Peter GICv3 should be selected based on properties not new machine types. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 0/4] vGICv3 support 2015-07-01 10:21 ` [Qemu-devel] [PATCH RFC 0/4] vGICv3 support Christoffer Dall 2015-07-01 10:26 ` Daniel P. Berrange @ 2015-07-01 11:14 ` Pavel Fedin 2015-07-01 11:28 ` Christoffer Dall 1 sibling, 1 reply; 21+ messages in thread From: Pavel Fedin @ 2015-07-01 11:14 UTC (permalink / raw) To: 'Christoffer Dall' Cc: 'Peter Maydell', 'Shlomo Pongratz', qemu-devel, 'Ashok Kumar', 'Eric Auger' Hello! > I think it would be good if you could re-spin this series based on > Eric's comments on the code, my comments on the patch style, and Peter's > advise on using machine properties for GICv3. There was a discussion about this, and i tried to add a machine property instead. This gave bad results: https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg04842.html > Do you have cycles to continue working on this? Yes, i do. And i wish to work on this, since upstreaming this is a goal of my project. I know that descriptions were bad, and actually they were parts of ongoing discussions. Actually i am waiting for integration of Shlomo Pongratz' series: http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg01512.html. Posted as standalone, my series collide with Shlomo's series (it actually relies on some parts from there), so i prefer to respin on top of something clean. The first patch from the mentioned series is already in master; Peter told me that he doesn't want to add the complete GICv3 before qemu 2.4 is released: https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg04171.html So waiting... Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 0/4] vGICv3 support 2015-07-01 11:14 ` Pavel Fedin @ 2015-07-01 11:28 ` Christoffer Dall 2015-07-01 12:31 ` Pavel Fedin 0 siblings, 1 reply; 21+ messages in thread From: Christoffer Dall @ 2015-07-01 11:28 UTC (permalink / raw) To: Pavel Fedin Cc: 'Peter Maydell', 'Shlomo Pongratz', qemu-devel, 'Ashok Kumar', 'Eric Auger' On Wed, Jul 01, 2015 at 02:14:55PM +0300, Pavel Fedin wrote: > Hello! > > > I think it would be good if you could re-spin this series based on > > Eric's comments on the code, my comments on the patch style, and Peter's > > advise on using machine properties for GICv3. > > There was a discussion about this, and i tried to add a machine property instead. This gave bad > results: https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg04842.html > > > Do you have cycles to continue working on this? > > Yes, i do. And i wish to work on this, since upstreaming this is a goal of my project. > I know that descriptions were bad, and actually they were parts of ongoing discussions. Actually i > am waiting for integration of Shlomo Pongratz' series: > http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg01512.html. Posted as standalone, my > series collide with Shlomo's series (it actually relies on some parts from there), so i prefer to > respin on top of something clean. I thought the general sense here was that since emulating the full device is much more complicated than driving the KVM part, the integration with the virt board should go in via this series, and the emulation should build on top of that? Of course, if the full emulation series is almost ready to be merged that's a different story. It just felt like both patch series have stalled somehow, and I would like to see what we can do to get this stuff moving again. > The first patch from the mentioned series is already in master; Peter told me that he doesn't want > to add the complete GICv3 before qemu 2.4 is released: > https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg04171.html > So waiting... > Fair enough. -Christoffer ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 0/4] vGICv3 support 2015-07-01 11:28 ` Christoffer Dall @ 2015-07-01 12:31 ` Pavel Fedin 0 siblings, 0 replies; 21+ messages in thread From: Pavel Fedin @ 2015-07-01 12:31 UTC (permalink / raw) To: 'Christoffer Dall' Cc: 'Peter Maydell', 'Shlomo Pongratz', qemu-devel, 'Eric Auger' Hello! > I thought the general sense here was that since emulating the full > device is much more complicated than driving the KVM part, Yes, but still it actually shares 50% of the code with SW emulation. It reuses vGICv3 base class as well as new machine. > the integration with the virt board should go in via this series, and the > emulation should build on top of that? You know... I could rip parts of Shlomo's patches and use them as base for my series. But, does it worth efforts? It will actually be a reposting of the same code over and over again... > It just felt like both patch series have stalled somehow, and I would > like to see what we can do to get this stuff moving again. For this purpose you can talk to Peter i guess, because it was his decision. By the way, when does freeze period end? Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2015-07-01 12:31 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-05-22 10:58 [Qemu-devel] [PATCH RFC 0/4] vGICv3 support Pavel Fedin 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 1/4] Add virt-v3 machine that uses GIC-500 Pavel Fedin 2015-05-25 14:07 ` Eric Auger 2015-05-25 14:51 ` Pavel Fedin 2015-07-01 10:11 ` Christoffer Dall 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 2/4] Set kernel_irqchip_type for other ARM boards which use GIC Pavel Fedin 2015-05-25 14:07 ` Eric Auger 2015-05-25 14:43 ` Pavel Fedin 2015-07-01 10:11 ` Christoffer Dall 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 3/4] First bits of vGICv3 support: Pavel Fedin 2015-05-25 14:07 ` Eric Auger 2015-07-01 10:13 ` Christoffer Dall 2015-05-22 10:58 ` [Qemu-devel] [PATCH RFC 4/4] Initial implementation of vGICv3 Pavel Fedin 2015-05-22 15:17 ` Eric Auger 2015-05-22 16:57 ` Pavel Fedin 2015-07-01 10:19 ` Christoffer Dall 2015-07-01 10:21 ` [Qemu-devel] [PATCH RFC 0/4] vGICv3 support Christoffer Dall 2015-07-01 10:26 ` Daniel P. Berrange 2015-07-01 11:14 ` Pavel Fedin 2015-07-01 11:28 ` Christoffer Dall 2015-07-01 12:31 ` Pavel Fedin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).