| Message ID | a77513013ba74812a17ce2653660eb8fee556c3f.1432291291.git.p.fedin@samsung.com |
|---|---|
| State | New |
| Headers | show |
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; >
> 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
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 > >
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;
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(-)