Hi Peter,
On 13:24 Mon 04 Aug , Peter Maydell wrote:
> On Wed, 16 Jul 2025 at 10:55, Luc Michel <luc.mic...@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.
I'm not sure I fully understand your point:
- The OR gate device is parented to the "irq-or-gates" container,
which is itself parented to the SoC. So finalizing (freeing) the
SoC would trigger a cascade of free calls to the children,
including those objects right?
- Are you talking specifically about an unrealize call on the SoC
object, without a subsequent free? I guess we don't expect to see
such thing to happen in QEMU. The warning in the qdev_unrealize()
documentation seems to go in that direction.
For reference, the QOM tree is as follows:
/machine (amd-versal-virt-machine)
[...]
/xlnx-versal (xlnx-versal)
[...]
/irq-or-gates (container)
/gic-irq[121] (or-irq)
/unnamed-gpio-in[0] (irq)
/unnamed-gpio-in[10] (irq)
/unnamed-gpio-in[11] (irq)
/unnamed-gpio-in[12] (irq)
/unnamed-gpio-in[13] (irq)
/unnamed-gpio-in[14] (irq)
/unnamed-gpio-in[15] (irq)
/unnamed-gpio-in[1] (irq)
/unnamed-gpio-in[2] (irq)
/unnamed-gpio-in[3] (irq)
/unnamed-gpio-in[4] (irq)
/unnamed-gpio-in[5] (irq)
/unnamed-gpio-in[6] (irq)
/unnamed-gpio-in[7] (irq)
/unnamed-gpio-in[8] (irq)
/unnamed-gpio-in[9] (irq)
[...]
Thanks
Luc
