Re: [PATCH 05/22] i8259.c: move parent_realize from PICClass to I8259CommonClass

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

On 15/10/2026 17:22, Peter Maydell wrote:

> On Tue, 13 Oct 2026 at 16:54, Peter Maydell <peter.maydell@xxxxxxxxxx> wrote:
> > On Wed, 16 Sept 2026 at 11:49, Mark Cave-Ayland
> > <mark.caveayland@xxxxxxxxxxx> wrote:
> > > Both the emulated and KVM i8259 implementations store a reference to the
> > > parent_realize function in their respective classes. Start by moving
> > > parent_realize from PICClass to I8259CommonClass and removing PICClass since
> > > it is no longer required.
> > >
> > > Signed-off-by: Mark Cave-Ayland <mark.caveayland@xxxxxxxxxxx>
> > This is one of those things where I think we're not 100% consistent
> > about whether we think the parent_realize field should go in the
> > child class struct or the parent class struct. The name "parent_realize"
> > implies it ought to be in the child struct, though, and I think we
> > do that more often than we put it in the parent class.
> We do, although it's a bit closer than I thought: about 50 callsites
> put the parent_realize pointer in the child class struct; about 25
> put it in the parent class struct. So I think for new code we should
> prefer to follow the child-class-struct version.
>
> in child's class:
>
> hw/arm/smmu-common.c:    device_class_set_parent_realize(dc, smmu_base_realize,
> hw/arm/smmuv3.c:    device_class_set_parent_realize(dc, smmu_realize,
> hw/display/macfb.c:    device_class_set_parent_realize(dc, macfb_nubus_realize,
> hw/i386/kvm/i8254.c:    device_class_set_parent_realize(dc, kvm_pit_realizefn,
> hw/i386/kvm/i8259.c:    device_class_set_parent_realize(dc,
> kvm_pic_realize, &kpc->parent_realize);
> hw/input/adb-kbd.c:    device_class_set_parent_realize(dc, adb_kbd_realizefn,
> hw/input/adb-mouse.c:    device_class_set_parent_realize(dc,
> adb_mouse_realizefn,
> hw/intc/arm_gic.c:    device_class_set_parent_realize(dc,
> arm_gic_realize, &agc->parent_realize);
> hw/intc/arm_gic_kvm.c:    device_class_set_parent_realize(dc,
> kvm_arm_gic_realize,
> hw/intc/arm_gicv3.c:    device_class_set_parent_realize(dc,
> arm_gic_realize, &agc->parent_realize);
> hw/intc/arm_gicv3_hvf.c:    device_class_set_parent_realize(dc,
> hvf_gicv3_realize,
> hw/intc/arm_gicv3_kvm.c:    device_class_set_parent_realize(dc,
> kvm_arm_gicv3_realize,
> hw/intc/arm_gicv3_whpx.c:    device_class_set_parent_realize(dc,
> whpx_gicv3_realize,
> hw/intc/arm_gicv5.c:    device_class_set_parent_realize(dc,
> gicv5_realize, &gc->parent_realize);
> hw/intc/i8259.c:    device_class_set_parent_realize(dc, pic_realize,
> &k->parent_realize);
> hw/intc/loongarch_dintc.c:    device_class_set_parent_realize(dc,
> loongarch_dintc_realize,
> hw/intc/loongarch_extioi.c:    device_class_set_parent_realize(dc,
> loongarch_extioi_realize,
> hw/intc/loongarch_extioi_common.c:
> device_class_set_parent_realize(dc, loongarch_extioi_common_realize,
> hw/intc/loongarch_ipi.c:    device_class_set_parent_realize(dc,
> loongarch_ipi_realize,
> hw/intc/loongarch_pch_pic.c:    device_class_set_parent_realize(dc,
> loongarch_pic_realize,
> hw/intc/loongarch_pic_common.c:    device_class_set_parent_realize(dc,
> loongarch_pic_common_realize,
> hw/intc/loongson_ipi.c:    device_class_set_parent_realize(dc,
> loongson_ipi_realize,
> hw/intc/loongson_ipi_common.c:    device_class_set_parent_realize(dc,
> loongson_ipi_common_realize,
> hw/intc/pnv_xive.c:    device_class_set_parent_realize(dc,
> pnv_xive_realize, &pxc->parent_realize);
> hw/intc/pnv_xive2.c:    device_class_set_parent_realize(dc, pnv_xive2_realize,
> hw/intc/spapr_xive.c:    device_class_set_parent_realize(dc, spapr_xive_realize,
> hw/intc/xics_pnv.c:    device_class_set_parent_realize(dc, pnv_icp_realize,
> hw/intc/s390_flic_kvm.c:    device_class_set_parent_realize(dc,
> kvm_s390_flic_realize,
> hw/nubus/nubus-virtio-mmio.c:    device_class_set_parent_realize(dc,
> nubus_virtio_mmio_realize,
> hw/timer/i8254.c:    device_class_set_parent_realize(dc,
> pit_realizefn, &pc->parent_realize);
> hw/virtio/virtio-pci.c:    device_class_set_parent_realize(dc,
> virtio_pci_dc_realize,
> target/alpha/cpu.c:    device_class_set_parent_realize(dc, alpha_cpu_realizefn,
> target/arm/cpu.c:    device_class_set_parent_realize(dc, arm_cpu_realizefn,
> target/avr/cpu.c:    device_class_set_parent_realize(dc,
> avr_cpu_realizefn, &mcc->parent_realize);
> target/hexagon/cpu.c:    device_class_set_parent_realize(dc,
> hexagon_cpu_realize,
> target/hppa/cpu.c:    device_class_set_parent_realize(dc, hppa_cpu_realizefn,
> target/i386/cpu.c:    device_class_set_parent_realize(dc, x86_cpu_realizefn,
> target/loongarch/cpu.c:    device_class_set_parent_realize(dc,
> loongarch_cpu_realizefn,
> target/m68k/cpu.c:    device_class_set_parent_realize(dc, m68k_cpu_realizefn,
> target/microblaze/cpu.c:    device_class_set_parent_realize(dc,
> mb_cpu_realizefn,
> target/mips/cpu.c:    device_class_set_parent_realize(dc, mips_cpu_realizefn,
> target/or1k/cpu.c:    device_class_set_parent_realize(dc,
> openrisc_cpu_realizefn,
> target/ppc/cpu_init.c:    device_class_set_parent_realize(dc, ppc_cpu_realize,
> target/riscv/cpu.c:    device_class_set_parent_realize(dc, riscv_cpu_realize,
> target/rx/cpu.c:    device_class_set_parent_realize(dc, rx_cpu_realize,
> target/s390x/cpu.c:    device_class_set_parent_realize(dc, s390_cpu_realizefn,
> target/sh4/cpu.c:    device_class_set_parent_realize(dc, superh_cpu_realizefn,
> target/sparc/cpu.c:    device_class_set_parent_realize(dc, sparc_cpu_realizefn,
> target/tricore/cpu.c:    device_class_set_parent_realize(dc,
> tricore_cpu_realizefn,
> target/xtensa/cpu.c:    device_class_set_parent_realize(dc,
> xtensa_cpu_realizefn,
>
> in parent's class:
>
> hw/input/lasips2.c:    device_class_set_parent_realize(dc,
> lasips2_kbd_port_realize,
> hw/input/lasips2.c:    device_class_set_parent_realize(dc,
> lasips2_mouse_port_realize,
> hw/input/pl050.c:    device_class_set_parent_realize(dc, pl050_kbd_realize,
> hw/input/pl050.c:    device_class_set_parent_realize(dc, pl050_mouse_realize,
> hw/intc/xics_spapr.c:    device_class_set_parent_realize(dc, ics_spapr_realize,
> hw/isa/pc87312.c:    device_class_set_parent_realize(dc, pc87312_realize,
> hw/isa/vt82c686.c:    device_class_set_parent_realize(dc, via_superio_realize,
> hw/pci-bridge/cxl_root_port.c:    device_class_set_parent_realize(dc,
> cxl_rp_realize, &rpc->parent_realize);
> hw/pci-bridge/gen_pcie_root_port.c:
> device_class_set_parent_realize(dc, gen_rp_realize,
> &rpc->parent_realize);
> hw/pci-host/pnv_phb.c:    device_class_set_parent_realize(dc,
> pnv_phb_root_port_realize,
> hw/pci-host/pnv_phb3_msi.c:    device_class_set_parent_realize(dc,
> phb3_msi_realize,
> hw/ppc/pnv.c:    device_class_set_parent_realize(dc, pnv_chip_power8_realize,
> hw/ppc/pnv.c:    device_class_set_parent_realize(dc, pnv_chip_power9_realize,
> hw/ppc/pnv.c:    device_class_set_parent_realize(dc, pnv_chip_power10_realize,
> hw/ppc/pnv.c:    device_class_set_parent_realize(dc, pnv_chip_power11_realize,
> hw/ppc/pnv_lpc.c:    device_class_set_parent_realize(dc, pnv_lpc_power8_realize,
> hw/ppc/pnv_lpc.c:    device_class_set_parent_realize(dc, pnv_lpc_power9_realize,
> hw/s390x/tod-kvm.c:
> device_class_set_parent_realize(DEVICE_CLASS(oc),
> kvm_s390_tod_realize,
> hw/virtio/vhost-user-gpio.c:    device_class_set_parent_realize(dc,
> vgpio_realize,
> hw/virtio/vhost-user-i2c.c:    device_class_set_parent_realize(dc, vi2c_realize,
> hw/virtio/vhost-user-input.c:    device_class_set_parent_realize(dc,
> vinput_realize,
> hw/virtio/vhost-user-rng.c:    device_class_set_parent_realize(dc,
> vu_rng_base_realize,
> hw/virtio/vhost-user-rtc.c:    device_class_set_parent_realize(dc,
> vu_rtc_base_realize,
> hw/virtio/vhost-user-snd.c:    device_class_set_parent_realize(dc,
> vu_snd_base_realize,
> hw/virtio/vhost-user-spi.c:    device_class_set_parent_realize(dc, vspi_realize,
Thanks for the analysis. I agree that it makes sense to put 
parent_realize in the child class as opposed to the parent class, and 
I'll make that change to my current series.
I also realise from this and your previous response that I misunderstood 
exactly what you mean by storing it in the instance...

ATB,

Mark.