Re: [PATCH 13/48] hw/arm/xlnx-versal: VersalMap: add support for OR'ed IRQs

[Peter Maydell <peter.maydell@linaro.org>]

On Wed, 16 Jul 2025 at 10:55, Luc Michel <luc.michel@amd.com> wrote:
>
> Improve the IRQ index in the VersalMap structure to turn it into a
> descriptor:
>    - the lower 16 bits still represent the IRQ index
>    - bit 18 is used to indicate a shared IRQ connected to a OR gate
>    - bits 19 to 22 indicate the index on the OR gate.
>
> This allows to share an IRQ among multiple devices. An OR gate is
> created to connect the devices to the actual IRQ pin.


> +/*
> + * When the R_VERSAL_IRQ_ORED flag is set on an IRQ descriptor, this function is
> + * used to return the corresponding or gate input IRQ. The or gate is created if
> + * not already existant.
> + *
> + * 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)
> +{
> +    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);
> +
> +    name = g_strdup_printf("irq[%d]", idx);
> +    dev = DEVICE(object_resolve_path_at(container, name));
> +
> +    if (dev == NULL) {
> +        dev = qdev_new(TYPE_OR_IRQ);

Here we create a device...

> +        object_property_add_child(container, name, OBJECT(dev));
> +        qdev_prop_set_uint16(dev, "num-lines", 1 << R_VERSAL_IRQ_OR_IDX_LENGTH);
> +        qdev_realize_and_unref(dev, NULL, &error_abort);
> +        qdev_connect_gpio_out(dev, 0, target_irq);
> +    }
> +
> +    return qdev_get_gpio_in(dev, or_idx);

...but then we don't save the pointer to it, so the only
thing still hanging onto it is the QOM tree.

If you want to change "embedded device struct" into
"allocate memory to create devices" that's fine, but the
SoC should still keep track of everything it's creating,
so that (at least in theory) it could clean it up on
unrealize.

thanks
-- PMM