| Message ID | 20220513175445.89616-1-shentey@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | QOM'ify PIIX southbridge creation | expand |
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.
On 21/5/22 10:48, Mark Cave-Ayland wrote: > On 13/05/2022 18:54, Bernhard Beschow wrote: >> 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 > 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. Yes. Do you mind opening a GitLab issue?
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(-)