| Message ID | a199e550-4576-3a03-a7c1-8c21bb502cf5@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/05/2018 10:00 AM, Jan Kiszka wrote: > On 2018-07-05 08:51, Jan Kiszka wrote: >> On 2018-06-29 15:29, Luc Michel wrote: >>> Add support for GICv2 virtualization extensions by mapping the necessary >>> I/O regions and connecting the maintenance IRQ lines. >>> >>> Declare those additions in the device tree and in the ACPI tables. >>> >>> Signed-off-by: Luc Michel <luc.michel@greensocs.com> >>> --- >>> hw/arm/virt-acpi-build.c | 4 ++++ >>> hw/arm/virt.c | 50 +++++++++++++++++++++++++++++++++------- >>> include/hw/arm/virt.h | 3 +++ >>> 3 files changed, 49 insertions(+), 8 deletions(-) >>> >>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c >>> index 6ea47e2588..3b74bf0372 100644 >>> --- a/hw/arm/virt-acpi-build.c >>> +++ b/hw/arm/virt-acpi-build.c >>> @@ -659,6 +659,8 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) >>> gicc->length = sizeof(*gicc); >>> if (vms->gic_version == 2) { >>> gicc->base_address = cpu_to_le64(memmap[VIRT_GIC_CPU].base); >>> + gicc->gich_base_address = cpu_to_le64(memmap[VIRT_GIC_HYP].base); >>> + gicc->gicv_base_address = cpu_to_le64(memmap[VIRT_GIC_VCPU].base); >>> } >>> gicc->cpu_interface_number = cpu_to_le32(i); >>> gicc->arm_mpidr = cpu_to_le64(armcpu->mp_affinity); >>> @@ -670,6 +672,8 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) >>> } >>> if (vms->virt && vms->gic_version == 3) { >>> gicc->vgic_interrupt = cpu_to_le32(PPI(ARCH_GICV3_MAINT_IRQ)); >>> + } else if (vms->virt && vms->gic_version == 2) { >>> + gicc->vgic_interrupt = cpu_to_le32(PPI(ARCH_GICV2_MAINT_IRQ)); >>> } >>> } >>> >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >>> index 742f68afca..e45b9de3be 100644 >>> --- a/hw/arm/virt.c >>> +++ b/hw/arm/virt.c >>> @@ -131,6 +131,8 @@ static const MemMapEntry a15memmap[] = { >>> [VIRT_GIC_DIST] = { 0x08000000, 0x00010000 }, >>> [VIRT_GIC_CPU] = { 0x08010000, 0x00010000 }, >>> [VIRT_GIC_V2M] = { 0x08020000, 0x00001000 }, >>> + [VIRT_GIC_HYP] = { 0x08030000, 0x00001000 }, >>> + [VIRT_GIC_VCPU] = { 0x08040000, 0x00001000 }, >>> /* The space in between here is reserved for GICv3 CPU/vCPU/HYP */ >>> [VIRT_GIC_ITS] = { 0x08080000, 0x00020000 }, >>> /* This redistributor space allows up to 2*64kB*123 CPUs */ >>> @@ -438,11 +440,26 @@ static void fdt_add_gic_node(VirtMachineState *vms) >>> /* 'cortex-a15-gic' means 'GIC v2' */ >>> qemu_fdt_setprop_string(vms->fdt, "/intc", "compatible", >>> "arm,cortex-a15-gic"); >>> - qemu_fdt_setprop_sized_cells(vms->fdt, "/intc", "reg", >>> - 2, vms->memmap[VIRT_GIC_DIST].base, >>> - 2, vms->memmap[VIRT_GIC_DIST].size, >>> - 2, vms->memmap[VIRT_GIC_CPU].base, >>> - 2, vms->memmap[VIRT_GIC_CPU].size); >>> + if (!vms->virt) { >>> + qemu_fdt_setprop_sized_cells(vms->fdt, "/intc", "reg", >>> + 2, vms->memmap[VIRT_GIC_DIST].base, >>> + 2, vms->memmap[VIRT_GIC_DIST].size, >>> + 2, vms->memmap[VIRT_GIC_CPU].base, >>> + 2, vms->memmap[VIRT_GIC_CPU].size); >>> + } else { >>> + qemu_fdt_setprop_sized_cells(vms->fdt, "/intc", "reg", >>> + 2, vms->memmap[VIRT_GIC_DIST].base, >>> + 2, vms->memmap[VIRT_GIC_DIST].size, >>> + 2, vms->memmap[VIRT_GIC_CPU].base, >>> + 2, vms->memmap[VIRT_GIC_CPU].size, >>> + 2, vms->memmap[VIRT_GIC_HYP].base, >>> + 2, vms->memmap[VIRT_GIC_HYP].size, >>> + 2, vms->memmap[VIRT_GIC_VCPU].base, >>> + 2, vms->memmap[VIRT_GIC_VCPU].size); >>> + qemu_fdt_setprop_cells(vms->fdt, "/intc", "interrupts", >>> + GIC_FDT_IRQ_TYPE_PPI, ARCH_GICV2_MAINT_IRQ, >>> + GIC_FDT_IRQ_FLAGS_LEVEL_HI); >>> + } >>> } >>> >>> qemu_fdt_setprop_cell(vms->fdt, "/intc", "phandle", vms->gic_phandle); >>> @@ -563,6 +580,11 @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) >>> qdev_prop_set_uint32(gicdev, "redist-region-count[1]", >>> MIN(smp_cpus - redist0_count, redist1_capacity)); >>> } >>> + } else { >>> + if (!kvm_irqchip_in_kernel()) { >>> + qdev_prop_set_bit(gicdev, "has-virtualization-extensions", >>> + vms->virt); >>> + } >>> } >>> qdev_init_nofail(gicdev); >>> gicbusdev = SYS_BUS_DEVICE(gicdev); >>> @@ -574,6 +596,10 @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) >>> } >>> } else { >>> sysbus_mmio_map(gicbusdev, 1, vms->memmap[VIRT_GIC_CPU].base); >>> + if (vms->virt) { >>> + sysbus_mmio_map(gicbusdev, 2, vms->memmap[VIRT_GIC_HYP].base); >>> + sysbus_mmio_map(gicbusdev, 3, vms->memmap[VIRT_GIC_VCPU].base); >>> + } >>> } >>> >>> /* Wire the outputs from each CPU's generic timer and the GICv3 >>> @@ -600,9 +626,17 @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) >>> ppibase + timer_irq[irq])); >>> } >>> >>> - qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0, >>> - qdev_get_gpio_in(gicdev, ppibase >>> - + ARCH_GICV3_MAINT_IRQ)); >>> + if (type == 3) { >>> + qemu_irq irq = qdev_get_gpio_in(gicdev, >>> + ppibase + ARCH_GICV3_MAINT_IRQ); >>> + qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", >>> + 0, irq); >>> + } else if (vms->virt) { >>> + qemu_irq irq = qdev_get_gpio_in(gicdev, >>> + ppibase + ARCH_GICV2_MAINT_IRQ); >>> + sysbus_connect_irq(gicbusdev, i + 4 * smp_cpus, irq); >>> + } >>> + >>> qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0, >>> qdev_get_gpio_in(gicdev, ppibase >>> + VIRTUAL_PMU_IRQ)); >>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h >>> index 9a870ccb6a..9e2f33f2d1 100644 >>> --- a/include/hw/arm/virt.h >>> +++ b/include/hw/arm/virt.h >>> @@ -42,6 +42,7 @@ >>> #define NUM_VIRTIO_TRANSPORTS 32 >>> #define NUM_SMMU_IRQS 4 >>> >>> +#define ARCH_GICV2_MAINT_IRQ 9 >>> #define ARCH_GICV3_MAINT_IRQ 9 >>> >>> #define ARCH_TIMER_VIRT_IRQ 11 >>> @@ -60,6 +61,8 @@ enum { >>> VIRT_GIC_DIST, >>> VIRT_GIC_CPU, >>> VIRT_GIC_V2M, >>> + VIRT_GIC_HYP, >>> + VIRT_GIC_VCPU, >>> VIRT_GIC_ITS, >>> VIRT_GIC_REDIST, >>> VIRT_GIC_REDIST2, >>> >> >> This one apparently requires rebasing over master. Did this manually. >> >> But now I'm running into troubles with reading back GICD ITARGETSR. >> Maybe we are emulating an "early implementation" here? >> >> [from the related Jailhouse code [1]] >> /* >> * Get the CPU interface ID for this cpu. It can be discovered by >> * reading the banked value of the PPI and IPI TARGET registers >> * Patch 2bb3135 in Linux explains why the probe may need to scans the >> * first 8 registers: some early implementation returned 0 for the first >> * ITARGETSR registers. >> * Since those didn't have virtualization extensions, we can safely >> * ignore that case. >> */ >> >> But maybe I'm just off with the configuration, checking... >> > > As suspected, it's a bug in QEMU, this resolves it, and I can run Linux > as root cell and a bare metal non-root cell: > > diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c > index 7d24348d96..199f953ddb 100644 > --- a/hw/intc/arm_gic.c > +++ b/hw/intc/arm_gic.c > @@ -965,7 +965,11 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs) > if (irq >= 29 && irq <= 31) { > res = cm; > } else { > - res = GIC_DIST_TARGET(irq); > + if (irq < GIC_INTERNAL) { > + res = 1 << gic_get_current_cpu(s); > + } else { > + res = GIC_DIST_TARGET(irq); > + } > } > } > } else if (offset < 0xf00) { > > Didn't test Linux as non-root cell (secondary guest) yet, but that > should work as well. I'm seeing issues in an error shutdown path, but > that might be the same on real hw, needs cross-checking.Hi Jan, thanks for your feedback! Yes I encountered the same issue with Xen in SMP (see my cover letter). Re-reading the GICv2 specs, it's now clear to me that a read to ITARGETSR0 to ITARGETSR7 should return "the number of the processor performing the read". Reading the message of commit 2bb3135 in Linux, it seems that older versions of the GIC exposed this value in IRQs 29, 30, 31, hence the if (irq >= 29 && irq <= 31) { res = cm; } in the current QEMU implementation. I should probably add a patch to fix that. I'll have to dig in specs of older GIC revisions to see when this behaviour actually appeared. Maybe I wait for some reviews before sending a new re-roll? Peter, any thoughts? Thanks. Luc > > Jan >
On 5 July 2018 at 09:46, Luc Michel <luc.michel@greensocs.com> wrote: > Yes I encountered the same issue with Xen in SMP (see my cover letter). > Re-reading the GICv2 specs, it's now clear to me that a read to > ITARGETSR0 to ITARGETSR7 should return "the number of the processor > performing the read". Reading the message of commit 2bb3135 in Linux, it > seems that older versions of the GIC exposed this value in IRQs 29, 30, > 31, hence the > if (irq >= 29 && irq <= 31) { res = cm; } > in the current QEMU implementation. > > I should probably add a patch to fix that. I'll have to dig in specs of > older GIC revisions to see when this behaviour actually appeared. > > Maybe I wait for some reviews before sending a new re-roll? > Peter, any thoughts? I'm probably not going to have time to look at any of this GICv2 stuff for a bit (due to softfreeze and other for-3.0 work), so don't wait for my responses if you think a reroll makes sense. thanks -- PMM
On 2018-07-05 10:00, Jan Kiszka wrote: > On 2018-07-05 08:51, Jan Kiszka wrote: >> On 2018-06-29 15:29, Luc Michel wrote: >>> Add support for GICv2 virtualization extensions by mapping the necessary >>> I/O regions and connecting the maintenance IRQ lines. >>> >>> Declare those additions in the device tree and in the ACPI tables. >>> >>> Signed-off-by: Luc Michel <luc.michel@greensocs.com> >>> --- >>> hw/arm/virt-acpi-build.c | 4 ++++ >>> hw/arm/virt.c | 50 +++++++++++++++++++++++++++++++++------- >>> include/hw/arm/virt.h | 3 +++ >>> 3 files changed, 49 insertions(+), 8 deletions(-) >>> >>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c >>> index 6ea47e2588..3b74bf0372 100644 >>> --- a/hw/arm/virt-acpi-build.c >>> +++ b/hw/arm/virt-acpi-build.c >>> @@ -659,6 +659,8 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) >>> gicc->length = sizeof(*gicc); >>> if (vms->gic_version == 2) { >>> gicc->base_address = cpu_to_le64(memmap[VIRT_GIC_CPU].base); >>> + gicc->gich_base_address = cpu_to_le64(memmap[VIRT_GIC_HYP].base); >>> + gicc->gicv_base_address = cpu_to_le64(memmap[VIRT_GIC_VCPU].base); >>> } >>> gicc->cpu_interface_number = cpu_to_le32(i); >>> gicc->arm_mpidr = cpu_to_le64(armcpu->mp_affinity); >>> @@ -670,6 +672,8 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) >>> } >>> if (vms->virt && vms->gic_version == 3) { >>> gicc->vgic_interrupt = cpu_to_le32(PPI(ARCH_GICV3_MAINT_IRQ)); >>> + } else if (vms->virt && vms->gic_version == 2) { >>> + gicc->vgic_interrupt = cpu_to_le32(PPI(ARCH_GICV2_MAINT_IRQ)); >>> } >>> } >>> >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >>> index 742f68afca..e45b9de3be 100644 >>> --- a/hw/arm/virt.c >>> +++ b/hw/arm/virt.c >>> @@ -131,6 +131,8 @@ static const MemMapEntry a15memmap[] = { >>> [VIRT_GIC_DIST] = { 0x08000000, 0x00010000 }, >>> [VIRT_GIC_CPU] = { 0x08010000, 0x00010000 }, >>> [VIRT_GIC_V2M] = { 0x08020000, 0x00001000 }, >>> + [VIRT_GIC_HYP] = { 0x08030000, 0x00001000 }, >>> + [VIRT_GIC_VCPU] = { 0x08040000, 0x00001000 }, >>> /* The space in between here is reserved for GICv3 CPU/vCPU/HYP */ >>> [VIRT_GIC_ITS] = { 0x08080000, 0x00020000 }, >>> /* This redistributor space allows up to 2*64kB*123 CPUs */ >>> @@ -438,11 +440,26 @@ static void fdt_add_gic_node(VirtMachineState *vms) >>> /* 'cortex-a15-gic' means 'GIC v2' */ >>> qemu_fdt_setprop_string(vms->fdt, "/intc", "compatible", >>> "arm,cortex-a15-gic"); >>> - qemu_fdt_setprop_sized_cells(vms->fdt, "/intc", "reg", >>> - 2, vms->memmap[VIRT_GIC_DIST].base, >>> - 2, vms->memmap[VIRT_GIC_DIST].size, >>> - 2, vms->memmap[VIRT_GIC_CPU].base, >>> - 2, vms->memmap[VIRT_GIC_CPU].size); >>> + if (!vms->virt) { >>> + qemu_fdt_setprop_sized_cells(vms->fdt, "/intc", "reg", >>> + 2, vms->memmap[VIRT_GIC_DIST].base, >>> + 2, vms->memmap[VIRT_GIC_DIST].size, >>> + 2, vms->memmap[VIRT_GIC_CPU].base, >>> + 2, vms->memmap[VIRT_GIC_CPU].size); >>> + } else { >>> + qemu_fdt_setprop_sized_cells(vms->fdt, "/intc", "reg", >>> + 2, vms->memmap[VIRT_GIC_DIST].base, >>> + 2, vms->memmap[VIRT_GIC_DIST].size, >>> + 2, vms->memmap[VIRT_GIC_CPU].base, >>> + 2, vms->memmap[VIRT_GIC_CPU].size, >>> + 2, vms->memmap[VIRT_GIC_HYP].base, >>> + 2, vms->memmap[VIRT_GIC_HYP].size, >>> + 2, vms->memmap[VIRT_GIC_VCPU].base, >>> + 2, vms->memmap[VIRT_GIC_VCPU].size); >>> + qemu_fdt_setprop_cells(vms->fdt, "/intc", "interrupts", >>> + GIC_FDT_IRQ_TYPE_PPI, ARCH_GICV2_MAINT_IRQ, >>> + GIC_FDT_IRQ_FLAGS_LEVEL_HI); >>> + } >>> } >>> >>> qemu_fdt_setprop_cell(vms->fdt, "/intc", "phandle", vms->gic_phandle); >>> @@ -563,6 +580,11 @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) >>> qdev_prop_set_uint32(gicdev, "redist-region-count[1]", >>> MIN(smp_cpus - redist0_count, redist1_capacity)); >>> } >>> + } else { >>> + if (!kvm_irqchip_in_kernel()) { >>> + qdev_prop_set_bit(gicdev, "has-virtualization-extensions", >>> + vms->virt); >>> + } >>> } >>> qdev_init_nofail(gicdev); >>> gicbusdev = SYS_BUS_DEVICE(gicdev); >>> @@ -574,6 +596,10 @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) >>> } >>> } else { >>> sysbus_mmio_map(gicbusdev, 1, vms->memmap[VIRT_GIC_CPU].base); >>> + if (vms->virt) { >>> + sysbus_mmio_map(gicbusdev, 2, vms->memmap[VIRT_GIC_HYP].base); >>> + sysbus_mmio_map(gicbusdev, 3, vms->memmap[VIRT_GIC_VCPU].base); >>> + } >>> } >>> >>> /* Wire the outputs from each CPU's generic timer and the GICv3 >>> @@ -600,9 +626,17 @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) >>> ppibase + timer_irq[irq])); >>> } >>> >>> - qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0, >>> - qdev_get_gpio_in(gicdev, ppibase >>> - + ARCH_GICV3_MAINT_IRQ)); >>> + if (type == 3) { >>> + qemu_irq irq = qdev_get_gpio_in(gicdev, >>> + ppibase + ARCH_GICV3_MAINT_IRQ); >>> + qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", >>> + 0, irq); >>> + } else if (vms->virt) { >>> + qemu_irq irq = qdev_get_gpio_in(gicdev, >>> + ppibase + ARCH_GICV2_MAINT_IRQ); >>> + sysbus_connect_irq(gicbusdev, i + 4 * smp_cpus, irq); >>> + } >>> + >>> qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0, >>> qdev_get_gpio_in(gicdev, ppibase >>> + VIRTUAL_PMU_IRQ)); >>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h >>> index 9a870ccb6a..9e2f33f2d1 100644 >>> --- a/include/hw/arm/virt.h >>> +++ b/include/hw/arm/virt.h >>> @@ -42,6 +42,7 @@ >>> #define NUM_VIRTIO_TRANSPORTS 32 >>> #define NUM_SMMU_IRQS 4 >>> >>> +#define ARCH_GICV2_MAINT_IRQ 9 >>> #define ARCH_GICV3_MAINT_IRQ 9 >>> >>> #define ARCH_TIMER_VIRT_IRQ 11 >>> @@ -60,6 +61,8 @@ enum { >>> VIRT_GIC_DIST, >>> VIRT_GIC_CPU, >>> VIRT_GIC_V2M, >>> + VIRT_GIC_HYP, >>> + VIRT_GIC_VCPU, >>> VIRT_GIC_ITS, >>> VIRT_GIC_REDIST, >>> VIRT_GIC_REDIST2, >>> >> >> This one apparently requires rebasing over master. Did this manually. >> >> But now I'm running into troubles with reading back GICD ITARGETSR. >> Maybe we are emulating an "early implementation" here? >> >> [from the related Jailhouse code [1]] >> /* >> * Get the CPU interface ID for this cpu. It can be discovered by >> * reading the banked value of the PPI and IPI TARGET registers >> * Patch 2bb3135 in Linux explains why the probe may need to scans the >> * first 8 registers: some early implementation returned 0 for the first >> * ITARGETSR registers. >> * Since those didn't have virtualization extensions, we can safely >> * ignore that case. >> */ >> >> But maybe I'm just off with the configuration, checking... >> > > As suspected, it's a bug in QEMU, this resolves it, and I can run Linux > as root cell and a bare metal non-root cell: > > diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c > index 7d24348d96..199f953ddb 100644 > --- a/hw/intc/arm_gic.c > +++ b/hw/intc/arm_gic.c > @@ -965,7 +965,11 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs) > if (irq >= 29 && irq <= 31) { > res = cm; > } else { > - res = GIC_DIST_TARGET(irq); > + if (irq < GIC_INTERNAL) { > + res = 1 << gic_get_current_cpu(s); > + } else { > + res = GIC_DIST_TARGET(irq); > + } > } > } > } else if (offset < 0xf00) { > > Didn't test Linux as non-root cell (secondary guest) yet, but that > should work as well. I'm seeing issues in an error shutdown path, but > that might be the same on real hw, needs cross-checking. The shutdown issue actually turned out to be a Jailhouse bug. Fixed now as well, and we run smoothly in GICv2 mode over QEMU, also with the secondary Linux guest! Jan
On 5 July 2018 at 09:46, Luc Michel <luc.michel@greensocs.com> wrote: > On 07/05/2018 10:00 AM, Jan Kiszka wrote: >> On 2018-07-05 08:51, Jan Kiszka wrote: >>> But now I'm running into troubles with reading back GICD ITARGETSR. >>> Maybe we are emulating an "early implementation" here? >>> >>> [from the related Jailhouse code [1]] >>> /* >>> * Get the CPU interface ID for this cpu. It can be discovered by >>> * reading the banked value of the PPI and IPI TARGET registers >>> * Patch 2bb3135 in Linux explains why the probe may need to scans the >>> * first 8 registers: some early implementation returned 0 for the first >>> * ITARGETSR registers. >>> * Since those didn't have virtualization extensions, we can safely >>> * ignore that case. >>> */ >>> >>> But maybe I'm just off with the configuration, checking... >>> >> >> As suspected, it's a bug in QEMU, this resolves it, and I can run Linux >> as root cell and a bare metal non-root cell: >> >> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c >> index 7d24348d96..199f953ddb 100644 >> --- a/hw/intc/arm_gic.c >> +++ b/hw/intc/arm_gic.c >> @@ -965,7 +965,11 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs) >> if (irq >= 29 && irq <= 31) { >> res = cm; >> } else { >> - res = GIC_DIST_TARGET(irq); >> + if (irq < GIC_INTERNAL) { >> + res = 1 << gic_get_current_cpu(s); We already have the CPU number of the current cpu in the 'cpu' local variable, so we don't need to call gic_get_current_cpu() again, and we have 1 << cpu in "cm". >> + } else { >> + res = GIC_DIST_TARGET(irq); >> + } >> } >> } >> } else if (offset < 0xf00) { >> >> Didn't test Linux as non-root cell (secondary guest) yet, but that >> should work as well. I'm seeing issues in an error shutdown path, but >> that might be the same on real hw, needs cross-checking.Hi Jan, thanks for your feedback! > > Yes I encountered the same issue with Xen in SMP (see my cover letter). > Re-reading the GICv2 specs, it's now clear to me that a read to > ITARGETSR0 to ITARGETSR7 should return "the number of the processor > performing the read". Reading the message of commit 2bb3135 in Linux, it > seems that older versions of the GIC exposed this value in IRQs 29, 30, > 31, hence the > if (irq >= 29 && irq <= 31) { res = cm; } > in the current QEMU implementation. > > I should probably add a patch to fix that. I'll have to dig in specs of > older GIC revisions to see when this behaviour actually appeared. The "29..31 give the current CPU and others are zero" behaviour is specific to the 11MPCore: http://arminfo.emea.arm.com/help/topic/com.arm.doc.ddi0360f/CCHBHJFH.html The GICv1 spec matches the GICv2 here. So what we want is probably to refactor this to pull the 11MPCore code out as the top level special case (since it's weird for uniprocessor setups too). I'll send a patch in a bit... thanks -- PMM
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 7d24348d96..199f953ddb 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -965,7 +965,11 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs) if (irq >= 29 && irq <= 31) { res = cm; } else { - res = GIC_DIST_TARGET(irq); + if (irq < GIC_INTERNAL) { + res = 1 << gic_get_current_cpu(s); + } else { + res = GIC_DIST_TARGET(irq); + } } } } else if (offset < 0xf00) {