On Sun, 15 Aug 2021 at 18:30, Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > > +Peter/David > > On 8/12/21 11:33 AM, Peter Maydell wrote: > > Currently we implement the RAS register block within the NVIC device. > > It isn't really very tightly coupled with the NVIC proper, so instead > > move it out into a sysbus device of its own and have the top level > > ARMv7M container create it and map it into memory at the right > > address. > > > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > > --- > > include/hw/arm/armv7m.h | 2 + > > include/hw/intc/armv7m_nvic.h | 1 - > > include/hw/misc/armv7m_ras.h | 37 ++++++++++++++ > > hw/arm/armv7m.c | 12 +++++ > > hw/intc/armv7m_nvic.c | 56 --------------------- > > hw/misc/armv7m_ras.c | 93 +++++++++++++++++++++++++++++++++++ > > MAINTAINERS | 2 + > > hw/misc/meson.build | 2 + > > 8 files changed, 148 insertions(+), 57 deletions(-) > > create mode 100644 include/hw/misc/armv7m_ras.h > > create mode 100644 hw/misc/armv7m_ras.c > > > diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c > > index 9ce5c30cd5c..8964730d153 100644 > > --- a/hw/arm/armv7m.c > > +++ b/hw/arm/armv7m.c > > @@ -231,6 +231,18 @@ static void armv7m_realize(DeviceState *dev, Error > > **errp) > > memory_region_add_subregion(&s->container, 0xe0000000, > > sysbus_mmio_get_region(sbd, 0)); > > > > + /* If the CPU has RAS support, create the RAS register block */ > > + if (cpu_isar_feature(aa32_ras, s->cpu)) { > > + object_initialize_child(OBJECT(dev), "armv7m-ras", > > + &s->ras, TYPE_ARMV7M_RAS); > > + sbd = SYS_BUS_DEVICE(&s->ras); > > + if (!sysbus_realize(sbd, errp)) { > > + return; > > + } > > + memory_region_add_subregion_overlap(&s->container, 0xe0005000, > > + sysbus_mmio_get_region(sbd, > > 0), 1); > > Just curious, is the overlap really needed?
Yes, because this block is currently in the middle of the PPB-area region provided by the NVIC, and needs to take priority over it. Once the refactoring is complete, the background-region will also be created in this armv7m realize function, but the RAS block still needs to go above it. > I see the NVIC default > region is 1 MiB wide. Aren't smaller regions returned first when > multiple regions have same priority? As David says, if you don't specify the priority then it's pot-luck which one you see. Having overlaps and not setting priorities is a QEMU bug. (We used to have some code to print a warning about unintentionally overlapping regions, but it was always disabled with #if 0 and we eventually deleted it in commit b61359781958. IIRC the reason we never enabled either a warning or an assertion was because for the PC machine's PCI devices in particular we thought it might be possible for the guest to map PCI devices at a silly address and generate overlaps, but I may well have the details wrong as it was years back.) -- PMM