Re: [PATCH v3 00/28] i8259: improve qdev PIC modelling (and misc fixes)

[Mark Cave-Ayland <mark.caveayland@xxxxxxxxxxx>]

On 19/10/2026 13:04, Mark Cave-Ayland wrote:

> This series attempts to improve the modelling of legacy x86 PICs within
> QEMU. The initial aim was to have a qdev device with one output gpio, and
> 16 input gpios representing the legacy IRQ lines, however as I went about
> trying to model this, I found a number of other inconsistencies/bugs: in
> particular there seemed to be some fuzziness around the functionality that
> belongs to an individual i8259 chip, and a cascaded i8259 configuration.
>
> For the purposes of this patch an i8259 device is considered to be a single
> i8259 chip whilst a PIC device is considered to be either a cascaded i8259
> configuration (emulated/KVM) or standalone device (Xen) representing the set
> of 16 legacy IRQs and a single output IRQ.
>
> Due to the updated device names, there is currently one visible change in
> the HMP monitor output for "info irq" where the isa-i8259 device now appears
> as isa-i8259-pic and the kvm-i8259 device appears as kvm-i8259-pic (see
> patch 25).
>
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@xxxxxxxxxxx>
>
> [Patches still requiring review: 18-21, 23]
>
> v3:
> - Rebase onto master
>
> - Add R-B tags from Peter
>
> - Introduce isa_realize() function in patch 18 and use it in patches 19 and
>    21 to fix refcounting for ISA devices within the PIC container device
>
> - Don't pass input IRQ 2 from the primary PIC when implementing the cascaded
>    PIC configuration in patch 20
>
> v2:
> - Rebase onto master
>
> - Add R-B tags from Peter
>
> - Remove FIXME from commit message and rename trace event in patch 13
>
> - Introduce a separate I8259State for TYPE_I8259 and KVMI8259State for
>    TYPE_KVM_I8259 as suggested by Peter
>
> - Keep but rename the associated classes for TYPE_I8259 and TYPE_KVM_I8259
>    and move the DeviceRealize function into each one, instead of keeping it
>    in the parent class
>
> - Add patches to make variable names consistent when accessing
>    I8259CommonState from child objects
>
> - Update the comment on patch 19 to reflect that implementing the
>    cascaded secondary is actually a bugfix
>
>
> Mark Cave-Ayland (28):
>    i8259_internal.h: rename PIC_COMMON to I8259_COMMON
>    i8259_internal.h: rename TYPE_I8259_COMMON to i8259-common
>    i8259_internal.h: rename PICCommonState to I8259CommonState
>    i8259_internal.h: rename PICCommonClass to I8259CommonClass
>    i8259.c: rename PICClass to I8259Class
>    i8259.c: introduce TYPE_I8259 device
>    i8259.c: always use cs for I8259CommonState variables
>    kvm/i8259.c: rename KVMPICClass to KVMI8259Class
>    kvm/i8259.c: introduce TYPE_KVM_I8259 device
>    kvm/i8259.c: always use cs for I8259CommonState variables
>    i8259_common.c: always use cs for I8259CommonState variables
>    i8259_common.c: use i8259/i8259_common prefix for functions
>    i8259.c: use i8259 prefix for functions
>    kvm/i8259.c: use kvm_i8259 prefix for functions
>    i8259_common.c: convert to use DEFINE_TYPES() macro
>    i8259.c: convert to use DEFINE_TYPES() macro
>    kvm/i8259.c: convert to use DEFINE_TYPES() macro
>    isa-bus.c: introduce isa_realize() function
>    i8259.c: introduce TYPE_I8259_PIC device
>    i8259.c: properly cascade secondary i8259 from primary i8259
>    kvm/i8259.c: introduce TYPE_KVM_I8259_PIC device
>    xen-hvm.c: rename xen_interrupt_controller_init() to xen_i8259_init()
>    xen/xen-hvm.c: introduce TYPE_XEN_I8259_PIC device
>    i8259_common.c: remove unused i8259_init_chip() function
>    i8259: move InterruptStatsProviderClass get_statistics() from i8259 to
>      PIC
>    i8259.c: switch isa_pic from I8259State to I8259PICState
>    i8259_common.c: remove static irq_count and irq_level variables
>    i8259_common.c: add missing 0x prefixes to "info pic" output
>
>   include/hw/intc/i8259.h         |  11 +-
>   include/hw/isa/i8259_internal.h |  19 +-
>   include/hw/isa/isa.h            |   1 +
>   include/hw/xen/xen.h            |   2 +-
>   hw/i386/kvm/i8259.c             | 250 ++++++++++++-----
>   hw/i386/pc.c                    |   2 +-
>   hw/i386/xen/xen-hvm.c           |  40 ++-
>   hw/intc/i8259.c                 | 470 ++++++++++++++++++++------------
>   hw/intc/i8259_common.c          | 225 +++++++--------
>   hw/isa/isa-bus.c                |   5 +
>   stubs/xen-hw-stub.c             |   2 +-
>   hw/intc/trace-events            |   8 +-
>   12 files changed, 648 insertions(+), 387 deletions(-)
Thanks for helping review this series, Peter. Now it is fully reviewed, 
I'll send a PR for this from my qom-qdev queue later today, unless I 
hear any last minute objections from any x86 maintainers.

ATB,

Mark.