Re: [PATCH for-6.2 01/25] arm: Move M-profile RAS register block into its own device

[Philippe Mathieu-Daudé]

+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.maydell@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? I see the NVIC default
region is 1 MiB wide. Aren't smaller regions returned first when
multiple regions have same priority? This might be one of my
misunderstandings with QEMU MR/AS APIs. Without looking at the
code, assuming 2 MRs overlapping with the same priority, is there
some assumption which one will be hit first?

> +    }
> +
>      for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
>          if (s->enable_bitband) {
>              Object *obj = OBJECT(&s->bitband[i]);