| Message ID | 20220122182444.724087-4-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm_gicv3_its: Implement MOVI and MOVALL commands | expand |
On 1/23/22 05:24, Peter Maydell wrote: > In our implementation, all ITSes connected to a GIC share a single > AddressSpace, which we keep in the GICv3State::dma_as field and > initialized based on the GIC's 'sysmem' property. The right place > to set it up by calling address_space_init() is therefore in the > GIC's realize method, not the ITS's realize. > > This fixes a theoretical bug where QEMU hangs on startup if the board > model creates two ITSes connected to the same GIC -- we would call > address_space_init() twice on the same AddressSpace*, which creates > an infinite loop in the QTAILQ that softmmu/memory.c uses to store > its list of AddressSpaces and causes any subsequent attempt to > iterate through that list to loop forever. There aren't any board > models like that in the tree at the moment, though. > > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > hw/intc/arm_gicv3_common.c | 5 +++++ > hw/intc/arm_gicv3_its.c | 3 --- > 2 files changed, 5 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c index 9884d2e39b9..579aa0cb9ed 100644 --- a/hw/intc/arm_gicv3_common.c +++ b/hw/intc/arm_gicv3_common.c @@ -357,6 +357,11 @@ static void arm_gicv3_common_realize(DeviceState *dev, Error **errp) return; } + if (s->lpi_enable) { + address_space_init(&s->dma_as, s->dma, + "gicv3-its-sysmem"); + } + s->cpu = g_new0(GICv3CPUState, s->num_cpu); for (i = 0; i < s->num_cpu; i++) { diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index 6d2549e64b1..67f12d98af3 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -1194,9 +1194,6 @@ static void gicv3_arm_its_realize(DeviceState *dev, Error **errp) gicv3_its_init_mmio(s, &gicv3_its_control_ops, &gicv3_its_translation_ops); - address_space_init(&s->gicv3->dma_as, s->gicv3->dma, - "gicv3-its-sysmem"); - /* set the ITS default features supported */ s->typer = FIELD_DP64(s->typer, GITS_TYPER, PHYSICAL, 1); s->typer = FIELD_DP64(s->typer, GITS_TYPER, ITT_ENTRY_SIZE,
In our implementation, all ITSes connected to a GIC share a single AddressSpace, which we keep in the GICv3State::dma_as field and initialized based on the GIC's 'sysmem' property. The right place to set it up by calling address_space_init() is therefore in the GIC's realize method, not the ITS's realize. This fixes a theoretical bug where QEMU hangs on startup if the board model creates two ITSes connected to the same GIC -- we would call address_space_init() twice on the same AddressSpace*, which creates an infinite loop in the QTAILQ that softmmu/memory.c uses to store its list of AddressSpaces and causes any subsequent attempt to iterate through that list to loop forever. There aren't any board models like that in the tree at the moment, though. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gicv3_common.c | 5 +++++ hw/intc/arm_gicv3_its.c | 3 --- 2 files changed, 5 insertions(+), 3 deletions(-)