Re: [PATCH 04/65] hw/intc/arm_gicv5: Implement skeleton code for IRS register frames

[Jonathan Cameron via <qemu-arm@nongnu.org>]

On Mon, 23 Feb 2026 17:01:11 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> The GICv5 IRS has one mandatory register frame (the config frame) for
> each of up to four supported physical interrupt domains.  Implement
> the skeleton of the code needed to create these as sysbus MMIO
> regions.
> 
> The config frame has a mix of 32-bit and 64-bit registers, and it is
> valid to access the 64-bit registers with 32-bit accesses.  In a
> similar way to the various GICv3 devices, we turn the MemoryRegionOps
> read_with_attrs and write_with_attrs calls into calls on functions
> specifically to read 32 or 64 bit values.  (We can't trivially
> implement one in terms of the other because various registers have
> side effects on write which must only trigger when the "correct" half
> of the 64-bit register is written to.)
> 
> Unlike the GICv3, we choose to expose a sysbus MMIO region for each
> interrupt domain even if the config of the GICv5 means that it
> doesn't implement that domain.  This avoids having the config frame
> for a domain ending up at a different MMIO region index depending on
> the config of the GICv5.  (This matters more for GICv5 because it
> supports Realm, and so there are more possible valid configurations.)
> 
> gicv5_common_init_irqs_and_mmio() does not yet create any IRQs, but
> we name it this way to parallel the equivalent GICv3 function and to
> avoid having to rename it when we add the IRQ line creation in a
> subsequent commit.
> 
> The arm_gicv5_types.h header is a little undermotivated at this
> point, but the aim is to have somewhere to put definitions that we
> want in both the GIC proper and the CPU interface.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Really trivial stuff inline.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>


> +DEFINE_READ_WRITE_WRAPPERS(ns, GICV5_ID_NS)
> +DEFINE_READ_WRITE_WRAPPERS(realm, GICV5_ID_REALM)
> +DEFINE_READ_WRITE_WRAPPERS(secure, GICV5_ID_S)
> +DEFINE_READ_WRITE_WRAPPERS(el3, GICV5_ID_EL3)
> +
> +static const MemoryRegionOps config_frame_ops[NUM_GICV5_DOMAINS] = {
> +    [GICV5_ID_S] = {

Maybe it's worth a macro for filing in the MemoryRegionOps as well
if we don't expect them to diverge beyond the config_xx_read/write.

> +        .read_with_attrs = config_secure_read,
> +        .write_with_attrs = config_secure_write,
> +        .endianness = DEVICE_LITTLE_ENDIAN,
> +        .valid.min_access_size = 4,
> +        .valid.max_access_size = 8,
> +        .impl.min_access_size = 4,
> +        .impl.max_access_size = 8,
> +    },
> +    [GICV5_ID_NS] = {
> +        .read_with_attrs = config_ns_read,
> +        .write_with_attrs = config_ns_write,
> +        .endianness = DEVICE_LITTLE_ENDIAN,
> +        .valid.min_access_size = 4,
> +        .valid.max_access_size = 8,
> +        .impl.min_access_size = 4,
> +        .impl.max_access_size = 8,
> +    },
> +    [GICV5_ID_EL3] = {
> +        .read_with_attrs = config_el3_read,
> +        .write_with_attrs = config_el3_write,
> +        .endianness = DEVICE_LITTLE_ENDIAN,
> +        .valid.min_access_size = 4,
> +        .valid.max_access_size = 8,
> +        .impl.min_access_size = 4,
> +        .impl.max_access_size = 8,
> +    },
> +    [GICV5_ID_REALM] = {
> +        .read_with_attrs = config_realm_read,
> +        .write_with_attrs = config_realm_write,
> +        .endianness = DEVICE_LITTLE_ENDIAN,
> +        .valid.min_access_size = 4,
> +        .valid.max_access_size = 8,
> +        .impl.min_access_size = 4,
> +        .impl.max_access_size = 8,
> +    },
> +};

> diff --git a/include/hw/intc/arm_gicv5_types.h b/include/hw/intc/arm_gicv5_types.h
> new file mode 100644
> index 0000000000..143dcdec28
> --- /dev/null
> +++ b/include/hw/intc/arm_gicv5_types.h

> +/*
> + * The GICv5 has four physical Interrupt Domains. This numbering
> + * must match the encoding used in IRS_IDR0.INT_DOM.

Wrap seems a bit short but meh, maybe it's more readable like this.

> + */
> +typedef enum GICv5Domain {
> +    GICV5_ID_S = 0,
> +    GICV5_ID_NS = 1,
> +    GICV5_ID_EL3 = 2,
> +    GICV5_ID_REALM = 3,
> +} GICv5Domain;
> +
> +#define NUM_GICV5_DOMAINS 4
> +
> +#endif