Re: [PATCH 0/6] QOM'ify PIIX southbridge creation

[Mark Cave-Ayland]

On 13/05/2022 18:54, Bernhard Beschow wrote:

> The piix3 and piix4 southbridge devices still rely on create() functions which
> are deprecated. This series resolves these functions piece by piece to
> modernize the code.
>
> Both devices are modified in lockstep where possible to provide more context.
>
> Testing done:
> * `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
> * `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda 
> debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`
>
> In both cases the system booted successfully and it was possible to shut down
> the system using the `poweroff` command.
>
> Bernhard Beschow (6):
>    include/hw: Move TYPE_PIIX4_PCI_DEVICE to southbridge/piix.h
>    hw/isa/piix{3,4}: Move pci_map_irq_fn's near pci_set_irq_fn's
>    hw/isa/piix{3,4}: QOM'ify PCI device creation and wiring
>    hw/isa/piix{3,4}: Factor out ISABus retrieval from create() functions
>    hw/isa/piix4: Factor out SM bus initialization from create() function
>    hw/isa/piix{3,4}: Inline and remove create() functions
>
>   hw/i386/pc_piix.c             |  7 ++-
>   hw/isa/piix3.c                | 98 +++++++++++++++++++----------------
>   hw/isa/piix4.c                | 91 +++++++++++++-------------------
>   hw/mips/malta.c               |  9 +++-
>   include/hw/isa/isa.h          |  2 -
>   include/hw/southbridge/piix.h |  6 +--
>   6 files changed, 105 insertions(+), 108 deletions(-)

I've just reviewed these, and other than a couple of minor issues these look good to 
me and definitely help to improve the code.

One thing reading over this patches has made me realise is that there is quite a 
model violation here in that the PIIX3 and PIIX4 devices (which are the PCI-ISA 
bridges) are actually setting the PCI host bridge IRQs(!). What should happen is that 
the PCI bus IRQs and routing should be done in the PCI host bridge, and gpios used in 
the PCI-ISA bridges to pass them up. But that's definitely something outside the 
scope of these improvements.


ATB,

Mark.