Re: [PATCH v6 38/47] hw/arm/xlnx-versal: add the target field in IRQ descriptor

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 26/9/25 09:07, Luc Michel wrote:
> Add the target field in the IRQ descriptor. This allows to target an IRQ
> to another IRQ controller than the GIC(s). Other supported targets are
> the PMC PPU1 CPU interrupt controller and the EAM (Error management)
> device. Those two devices are currently not implemented so IRQs
> targeting those will be left unconnected. This is in preparation for
> versal2.
> 
> Signed-off-by: Luc Michel <luc.michel@amd.com>
> Reviewed-by: Francisco Iglesias <francisco.iglesias@amd.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>   hw/arm/xlnx-versal.c | 41 +++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
> index 3d960ed2636..64744401182 100644
> --- a/hw/arm/xlnx-versal.c
> +++ b/hw/arm/xlnx-versal.c
> @@ -50,18 +50,30 @@
>   #include "hw/cpu/cluster.h"
>   #include "hw/arm/bsa.h"
>   
>   /*
>    * IRQ descriptor to catch the following cases:
> + *   - An IRQ can either connect to the GICs, to the PPU1 intc, or the the EAM
>    *   - Multiple devices can connect to the same IRQ. They are OR'ed together.
>    */
>   FIELD(VERSAL_IRQ, IRQ, 0, 16)
> +FIELD(VERSAL_IRQ, TARGET, 16, 2)
>   FIELD(VERSAL_IRQ, ORED, 18, 1)
>   FIELD(VERSAL_IRQ, OR_IDX, 19, 4) /* input index on the IRQ OR gate */
>   
> +typedef enum VersalIrqTarget {
> +    IRQ_TARGET_GIC,
> +    IRQ_TARGET_PPU1,
> +    IRQ_TARGET_EAM,

Maybe declare IRQ_TARGET_RSVD here,

> +} VersalIrqTarget;
> +
> +#define PPU1_IRQ(irq) ((IRQ_TARGET_PPU1 << R_VERSAL_IRQ_TARGET_SHIFT) | (irq))
> +#define EAM_IRQ(irq) ((IRQ_TARGET_EAM << R_VERSAL_IRQ_TARGET_SHIFT) | (irq))
>   #define OR_IRQ(irq, or_idx) \
>       (R_VERSAL_IRQ_ORED_MASK | ((or_idx) << R_VERSAL_IRQ_OR_IDX_SHIFT) | (irq))
> +#define PPU1_OR_IRQ(irq, or_idx) \
> +    ((IRQ_TARGET_PPU1 << R_VERSAL_IRQ_TARGET_SHIFT) | OR_IRQ(irq, or_idx))
>   
>   typedef struct VersalSimplePeriphMap {
>       uint64_t addr;
>       int irq;
>   } VersalSimplePeriphMap;
> @@ -412,19 +424,27 @@ static qemu_irq versal_get_gic_irq(Versal *s, int irq_idx)
>    * Or gates are placed under the /soc/irq-or-gates QOM container.
>    */
>   static qemu_irq versal_get_irq_or_gate_in(Versal *s, int irq_idx,
>                                             qemu_irq target_irq)
>   {
> +    static const char *TARGET_STR[] = {
> +        [IRQ_TARGET_GIC] = "gic",
> +        [IRQ_TARGET_PPU1] = "ppu1",
> +        [IRQ_TARGET_EAM] = "eam",
> +    };
> +
> +    VersalIrqTarget target;
>       Object *container = versal_get_child(s, "irq-or-gates");
>       DeviceState *dev;
>       g_autofree char *name;
>       int idx, or_idx;
>   
>       idx = FIELD_EX32(irq_idx, VERSAL_IRQ, IRQ);
>       or_idx = FIELD_EX32(irq_idx, VERSAL_IRQ, OR_IDX);
> +    target = FIELD_EX32(irq_idx, VERSAL_IRQ, TARGET);

and assert(target != IRQ_TARGET_RSVD) here?

>   
> -    name = g_strdup_printf("irq[%d]", idx);
> +    name = g_strdup_printf("%s-irq[%d]", TARGET_STR[target], idx);
>       dev = DEVICE(object_resolve_path_at(container, name));
>   
>       if (dev == NULL) {
>           dev = qdev_new(TYPE_OR_IRQ);
>           object_property_add_child(container, name, OBJECT(dev));
> @@ -436,16 +456,33 @@ static qemu_irq versal_get_irq_or_gate_in(Versal *s, int irq_idx,
>       return qdev_get_gpio_in(dev, or_idx);
>   }
>   
>   static qemu_irq versal_get_irq(Versal *s, int irq_idx)
>   {
> +    VersalIrqTarget target;
>       qemu_irq irq;
>       bool ored;
>   
> +    target = FIELD_EX32(irq_idx, VERSAL_IRQ, TARGET);
>       ored = FIELD_EX32(irq_idx, VERSAL_IRQ, ORED);
>   
> -    irq = versal_get_gic_irq(s, irq_idx);
> +    switch (target) {
> +    case IRQ_TARGET_EAM:
> +        /* EAM not implemented */
> +        return NULL;
> +
> +    case IRQ_TARGET_PPU1:
> +        /* PPU1 CPU not implemented */
> +        return NULL;
> +
> +    case IRQ_TARGET_GIC:
> +        irq = versal_get_gic_irq(s, irq_idx);
> +        break;
> +
> +    default:

And here 'case IRQ_TARGET_RSVD' instead.

> +        g_assert_not_reached();
> +    }
>   
>       if (ored) {
>           irq = versal_get_irq_or_gate_in(s, irq_idx, irq);
>       }
>