From 01b5ab8cc08ab0afb6574f46a4d966725a00a1de Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 30 Sep 2021 16:08:40 +0100 Subject: hw/intc/arm_gicv3: Move checking of redist-region-count to arm_gicv3_common_realize The GICv3 devices have an array property redist-region-count. Currently we check this for errors (bad values) in gicv3_init_irqs_and_mmio(), just before we use it. Move this error checking to the arm_gicv3_common_realize() function, where we sanity-check all of the other base-class properties. (This will always be before gicv3_init_irqs_and_mmio() is called, because that function is called in the subclass realize methods, after they have called the parent-class realize.) The motivation for this refactor is: * we would like to use the redist_region_count[] values in arm_gicv3_common_realize() in a subsequent patch, so we need to have already done the sanity-checking first * this removes the only use of the Error** argument to gicv3_init_irqs_and_mmio(), so we can remove some error-handling boilerplate Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson --- hw/intc/arm_gicv3.c | 6 +----- hw/intc/arm_gicv3_common.c | 26 +++++++++++++------------- hw/intc/arm_gicv3_kvm.c | 6 +----- 3 files changed, 15 insertions(+), 23 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c index 3f24707..bcf54a5 100644 --- a/hw/intc/arm_gicv3.c +++ b/hw/intc/arm_gicv3.c @@ -393,11 +393,7 @@ static void arm_gic_realize(DeviceState *dev, Error **errp) return; } - gicv3_init_irqs_and_mmio(s, gicv3_set_irq, gic_ops, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } + gicv3_init_irqs_and_mmio(s, gicv3_set_irq, gic_ops); gicv3_init_cpuif(s); } diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c index 223db16..8e47809 100644 --- a/hw/intc/arm_gicv3_common.c +++ b/hw/intc/arm_gicv3_common.c @@ -250,22 +250,11 @@ static const VMStateDescription vmstate_gicv3 = { }; void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler, - const MemoryRegionOps *ops, Error **errp) + const MemoryRegionOps *ops) { SysBusDevice *sbd = SYS_BUS_DEVICE(s); - int rdist_capacity = 0; int i; - for (i = 0; i < s->nb_redist_regions; i++) { - rdist_capacity += s->redist_region_count[i]; - } - if (rdist_capacity < s->num_cpu) { - error_setg(errp, "Capacity of the redist regions(%d) " - "is less than number of vcpus(%d)", - rdist_capacity, s->num_cpu); - return; - } - /* For the GIC, also expose incoming GPIO lines for PPIs for each CPU. * GPIO array layout is thus: * [0..N-1] spi @@ -308,7 +297,7 @@ void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler, static void arm_gicv3_common_realize(DeviceState *dev, Error **errp) { GICv3State *s = ARM_GICV3_COMMON(dev); - int i; + int i, rdist_capacity; /* revision property is actually reserved and currently used only in order * to keep the interface compatible with GICv2 code, avoiding extra @@ -350,6 +339,17 @@ static void arm_gicv3_common_realize(DeviceState *dev, Error **errp) return; } + rdist_capacity = 0; + for (i = 0; i < s->nb_redist_regions; i++) { + rdist_capacity += s->redist_region_count[i]; + } + if (rdist_capacity < s->num_cpu) { + error_setg(errp, "Capacity of the redist regions(%d) " + "is less than number of vcpus(%d)", + rdist_capacity, s->num_cpu); + return; + } + s->cpu = g_new0(GICv3CPUState, s->num_cpu); for (i = 0; i < s->num_cpu; i++) { diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index 5c09f00..ab58c73 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -787,11 +787,7 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) return; } - gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } + gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL); for (i = 0; i < s->num_cpu; i++) { ARMCPU *cpu = ARM_CPU(qemu_get_cpu(i)); -- cgit v1.1 From 046164155abe7594ad1d8729dbe150e95badeaed Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 30 Sep 2021 16:08:41 +0100 Subject: hw/intc/arm_gicv3: Set GICR_TYPER.Last correctly when nb_redist_regions > 1 The 'Last' bit in the GICR_TYPER GICv3 redistributor register is supposed to be set to 1 if this is the last redistributor in a series of contiguous redistributor pages. Currently we set Last only for the redistributor for CPU (num_cpu - 1). This only works if there is a single redistributor region; if there are multiple redistributor regions then we need to set the Last bit for the last redistributor in each region. This doesn't cause any problems currently because only the KVM GICv3 supports multiple redistributor regions, and it ignores the value in GICv3State::gicr_typer. But we need to fix this before we can enable support for multiple regions in the emulated GICv3. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson --- hw/intc/arm_gicv3_common.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c index 8e47809..8de9205 100644 --- a/hw/intc/arm_gicv3_common.c +++ b/hw/intc/arm_gicv3_common.c @@ -297,7 +297,7 @@ void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler, static void arm_gicv3_common_realize(DeviceState *dev, Error **errp) { GICv3State *s = ARM_GICV3_COMMON(dev); - int i, rdist_capacity; + int i, rdist_capacity, cpuidx; /* revision property is actually reserved and currently used only in order * to keep the interface compatible with GICv2 code, avoiding extra @@ -355,7 +355,6 @@ static void arm_gicv3_common_realize(DeviceState *dev, Error **errp) for (i = 0; i < s->num_cpu; i++) { CPUState *cpu = qemu_get_cpu(i); uint64_t cpu_affid; - int last; s->cpu[i].cpu = cpu; s->cpu[i].gic = s; @@ -375,7 +374,6 @@ static void arm_gicv3_common_realize(DeviceState *dev, Error **errp) * PLPIS == 0 (physical LPIs not supported) */ cpu_affid = object_property_get_uint(OBJECT(cpu), "mp-affinity", NULL); - last = (i == s->num_cpu - 1); /* The CPU mp-affinity property is in MPIDR register format; squash * the affinity bytes into 32 bits as the GICR_TYPER has them. @@ -384,13 +382,22 @@ static void arm_gicv3_common_realize(DeviceState *dev, Error **errp) (cpu_affid & 0xFFFFFF); s->cpu[i].gicr_typer = (cpu_affid << 32) | (1 << 24) | - (i << 8) | - (last << 4); + (i << 8); if (s->lpi_enable) { s->cpu[i].gicr_typer |= GICR_TYPER_PLPIS; } } + + /* + * Now go through and set GICR_TYPER.Last for the final + * redistributor in each region. + */ + cpuidx = 0; + for (i = 0; i < s->nb_redist_regions; i++) { + cpuidx += s->redist_region_count[i]; + s->cpu[cpuidx - 1].gicr_typer |= GICR_TYPER_LAST; + } } static void arm_gicv3_finalize(Object *obj) -- cgit v1.1 From e5cba10ee16566a479cebec3890e91df91e33ab5 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 30 Sep 2021 16:08:42 +0100 Subject: hw/intc/arm_gicv3: Support multiple redistributor regions Our GICv3 QOM interface includes an array property redist-region-count which allows board models to specify that the registributor registers are not in a single contiguous range, but split into multiple pieces. We implemented this for KVM, but currently the TCG GICv3 model insists that there is only one region. You can see the limit being hit with a setup like: qemu-system-aarch64 -machine virt,gic-version=3 -smp 124 Add support for split regions to the TCG GICv3. To do this we switch from allocating a simple array of MemoryRegions to an array of GICv3RedistRegion structs so that we can use the GICv3RedistRegion as the opaque pointer in the MemoryRegion read/write callbacks. Each GICv3RedistRegion contains the MemoryRegion, a backpointer allowing the read/write callback to get hold of the GICv3State, and an index which allows us to calculate which CPU's redistributor is being accessed. Note that arm_gicv3_kvm always passes in NULL as the ops argument to gicv3_init_irqs_and_mmio(), so the only MemoryRegion read/write callbacks we need to update to handle this new scheme are the gicv3_redist_read/write functions used by the emulated GICv3. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson --- hw/intc/arm_gicv3.c | 6 ------ hw/intc/arm_gicv3_common.c | 15 +++++++++++---- hw/intc/arm_gicv3_kvm.c | 4 ++-- hw/intc/arm_gicv3_redist.c | 40 ++++++++++++++++++++++------------------ 4 files changed, 35 insertions(+), 30 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c index bcf54a5..c628298 100644 --- a/hw/intc/arm_gicv3.c +++ b/hw/intc/arm_gicv3.c @@ -387,12 +387,6 @@ static void arm_gic_realize(DeviceState *dev, Error **errp) return; } - if (s->nb_redist_regions != 1) { - error_setg(errp, "VGICv3 redist region number(%d) not equal to 1", - s->nb_redist_regions); - return; - } - gicv3_init_irqs_and_mmio(s, gicv3_set_irq, gic_ops); gicv3_init_cpuif(s); diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c index 8de9205..9884d2e 100644 --- a/hw/intc/arm_gicv3_common.c +++ b/hw/intc/arm_gicv3_common.c @@ -254,6 +254,7 @@ void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler, { SysBusDevice *sbd = SYS_BUS_DEVICE(s); int i; + int cpuidx; /* For the GIC, also expose incoming GPIO lines for PPIs for each CPU. * GPIO array layout is thus: @@ -282,14 +283,20 @@ void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler, "gicv3_dist", 0x10000); sysbus_init_mmio(sbd, &s->iomem_dist); - s->iomem_redist = g_new0(MemoryRegion, s->nb_redist_regions); + s->redist_regions = g_new0(GICv3RedistRegion, s->nb_redist_regions); + cpuidx = 0; for (i = 0; i < s->nb_redist_regions; i++) { char *name = g_strdup_printf("gicv3_redist_region[%d]", i); + GICv3RedistRegion *region = &s->redist_regions[i]; + + region->gic = s; + region->cpuidx = cpuidx; + cpuidx += s->redist_region_count[i]; - memory_region_init_io(&s->iomem_redist[i], OBJECT(s), - ops ? &ops[1] : NULL, s, name, + memory_region_init_io(®ion->iomem, OBJECT(s), + ops ? &ops[1] : NULL, region, name, s->redist_region_count[i] * GICV3_REDIST_SIZE); - sysbus_init_mmio(sbd, &s->iomem_redist[i]); + sysbus_init_mmio(sbd, ®ion->iomem); g_free(name); } } diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index ab58c73..5ec5ff9 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -825,7 +825,7 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) KVM_VGIC_V3_ADDR_TYPE_DIST, s->dev_fd, 0); if (!multiple_redist_region_allowed) { - kvm_arm_register_device(&s->iomem_redist[0], -1, + kvm_arm_register_device(&s->redist_regions[0].iomem, -1, KVM_DEV_ARM_VGIC_GRP_ADDR, KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd, 0); } else { @@ -838,7 +838,7 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) uint64_t addr_ormask = i | ((uint64_t)s->redist_region_count[i] << 52); - kvm_arm_register_device(&s->iomem_redist[i], -1, + kvm_arm_register_device(&s->redist_regions[i].iomem, -1, KVM_DEV_ARM_VGIC_GRP_ADDR, KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, s->dev_fd, addr_ormask); diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c index 7072bfc..424e7e2 100644 --- a/hw/intc/arm_gicv3_redist.c +++ b/hw/intc/arm_gicv3_redist.c @@ -425,22 +425,24 @@ static MemTxResult gicr_writell(GICv3CPUState *cs, hwaddr offset, MemTxResult gicv3_redist_read(void *opaque, hwaddr offset, uint64_t *data, unsigned size, MemTxAttrs attrs) { - GICv3State *s = opaque; + GICv3RedistRegion *region = opaque; + GICv3State *s = region->gic; GICv3CPUState *cs; MemTxResult r; int cpuidx; assert((offset & (size - 1)) == 0); - /* This region covers all the redistributor pages; there are - * (for GICv3) two 64K pages per CPU. At the moment they are - * all contiguous (ie in this one region), though we might later - * want to allow splitting of redistributor pages into several - * blocks so we can support more CPUs. + /* + * There are (for GICv3) two 64K redistributor pages per CPU. + * In some cases the redistributor pages for all CPUs are not + * contiguous (eg on the virt board they are split into two + * parts if there are too many CPUs to all fit in the same place + * in the memory map); if so then the GIC has multiple MemoryRegions + * for the redistributors. */ - cpuidx = offset / 0x20000; - offset %= 0x20000; - assert(cpuidx < s->num_cpu); + cpuidx = region->cpuidx + offset / GICV3_REDIST_SIZE; + offset %= GICV3_REDIST_SIZE; cs = &s->cpu[cpuidx]; @@ -482,22 +484,24 @@ MemTxResult gicv3_redist_read(void *opaque, hwaddr offset, uint64_t *data, MemTxResult gicv3_redist_write(void *opaque, hwaddr offset, uint64_t data, unsigned size, MemTxAttrs attrs) { - GICv3State *s = opaque; + GICv3RedistRegion *region = opaque; + GICv3State *s = region->gic; GICv3CPUState *cs; MemTxResult r; int cpuidx; assert((offset & (size - 1)) == 0); - /* This region covers all the redistributor pages; there are - * (for GICv3) two 64K pages per CPU. At the moment they are - * all contiguous (ie in this one region), though we might later - * want to allow splitting of redistributor pages into several - * blocks so we can support more CPUs. + /* + * There are (for GICv3) two 64K redistributor pages per CPU. + * In some cases the redistributor pages for all CPUs are not + * contiguous (eg on the virt board they are split into two + * parts if there are too many CPUs to all fit in the same place + * in the memory map); if so then the GIC has multiple MemoryRegions + * for the redistributors. */ - cpuidx = offset / 0x20000; - offset %= 0x20000; - assert(cpuidx < s->num_cpu); + cpuidx = region->cpuidx + offset / GICV3_REDIST_SIZE; + offset %= GICV3_REDIST_SIZE; cs = &s->cpu[cpuidx]; -- cgit v1.1