Re: [PATCH v2 18/27] i8259.c: introduce TYPE_I8259_PIC device

[Peter Maydell <peter.maydell@xxxxxxxxxx>]

On Fri, 16 Oct 2026 at 15:23, Mark Cave-Ayland
<mark.caveayland@xxxxxxxxxxx> wrote:
>
> This device represents a PIC for an x86 machine consisting of 2 cascaded i8259
> devices wired accordingly.
>
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@xxxxxxxxxxx>

> +static void i8259_pic_init(Object *obj)
> +{
> +    I8259PICState *s = I8259_PIC(obj);
> +
> +    object_initialize_child(obj, "primary", &s->i8259[0], TYPE_I8259);
> +    object_initialize_child(obj, "secondary", &s->i8259[1], TYPE_I8259);
> +
> +    qemu_init_irq(&s->i8259_primary_out_irq, i8259_primary_out_irq, s, 1);
> +
> +    qdev_init_gpio_in(DEVICE(obj), i8259_pic_set_irq, ISA_NUM_IRQS);
> +    qdev_init_gpio_out(DEVICE(obj), &s->pic_out_irq, 1);
> +}
> +
> +static void i8259_pic_realize(DeviceState *dev, Error **errp)
> +{
> +    I8259PICState *s = I8259_PIC(dev);
> +    DeviceState *pri_dev, *sec_dev;
> +    int i;
> +
> +    /* Primary */
> +    pri_dev = DEVICE(&s->i8259[0]);
> +    qdev_prop_set_uint32(pri_dev, "iobase", 0x20);
> +    qdev_prop_set_uint32(pri_dev, "elcr_addr", 0x4d0);
> +    qdev_prop_set_uint8(pri_dev, "elcr_mask", 0xf8);
> +    qdev_prop_set_bit(pri_dev, "master", 1);
> +    if (!isa_realize_and_unref(ISA_DEVICE(pri_dev), s->isabus, errp)) {
> +        return;
> +    }

I'm not sure the _and_unref version is what we want here. The
documentation for qdev_realize_and_unref says:

 * If you are embedding the device into some other QOM device and
 * initialized it via some variant on object_initialize_child() then
 * do not use this function, because that family of functions arrange
 * for the only reference to the child device to be held by the parent
 * via the child<> property, and so the reference-count-drop done here
 * would be incorrect. For that use case you want qdev_realize().

thanks
-- PMM