Re: [PATCH 13/22] i8259.c: introduce TYPE_I8259_PIC device

[Peter Maydell <peter.maydell@xxxxxxxxxx>]

On Wed, 16 Sept 2026 at 11:50, 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>
> ---
>  hw/intc/i8259.c | 133 ++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 118 insertions(+), 15 deletions(-)
>
> diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
> index af33b71591..fcc94e12f8 100644
> --- a/hw/intc/i8259.c
> +++ b/hw/intc/i8259.c
> @@ -25,7 +25,9 @@
>  #include "qemu/osdep.h"
>  #include "hw/intc/i8259.h"
>  #include "hw/core/irq.h"
> +#include "hw/core/qdev-properties.h"
>  #include "hw/isa/isa.h"
> +#include "qapi/error.h"
>  #include "qemu/timer.h"
>  #include "qemu/log.h"
>  #include "hw/isa/i8259_internal.h"
> @@ -36,6 +38,22 @@
>
>  #define TYPE_I8259 "isa-i8259"
>
> +#define TYPE_I8259_PIC "isa-i8259-pic"
> +
> +struct I8259PICState {
> +    DeviceState parent_obj;
> +
> +    qemu_irq pic_out_irq;
> +    qemu_irq pass_irqs[ISA_NUM_IRQS];
> +
> +    ISABus *isabus;
> +    IRQState i8259_primary_out_irq;
> +    I8259CommonState i8259[2];

This allocates only enough space for the common base class's
instance struct, but later we do an object_initialize_child()
of the child class. If the child class's state struct is
ever bigger than the common class, this will break.

>  static const TypeInfo i8259_type_infos[] = {
>      {
>          .name       = TYPE_I8259,
>          .parent     = TYPE_I8259_COMMON,
>          .class_init = i8259_class_init,
>      },
> +    {
> +        .name          = TYPE_I8259_PIC,
> +        .parent        = TYPE_DEVICE,

Devices that directly inherit from TYPE_DEVICE are awkward,
because they never get reset. Conviently this device doesn't
currently need a reset method, but still, is this a good idea?

> +        .class_init    = i8259_pic_class_init,
> +        .instance_init = i8259_pic_init,
> +        .instance_size = sizeof(I8259PICState),
> +    },
>  };
>
>  DEFINE_TYPES(i8259_type_infos)

-- PMM