qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 00/42] Consolidate PIIX south bridges


From: Mark Cave-Ayland
Subject: Re: [PATCH 00/42] Consolidate PIIX south bridges
Date: Sun, 18 Sep 2022 21:22:55 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0

On 01/09/2022 17:25, Bernhard Beschow wrote:

This series consolidates the implementations of the PIIX3 and PIIX4 south
bridges and is an extended version of [1]. The motivation is to share as much
code as possible and to bring both device models to feature parity such that
perhaps PIIX4 can become a drop-in-replacement for PIIX3 in the pc machine. This
could resolve the "Frankenstein" PIIX4-PM problem in PIIX3 discussed on this
list before.

The series is structured as follows: First, PIIX3 is changed to instantiate
internal devices itself, like PIIX4 does already. Second, PIIX3 gets prepared
for the merge with PIIX4 which includes some fixes, cleanups, and renamings.
Third, the same is done for PIIX4. In step four the implementations are merged.
Since some consolidations could be done easier with merged implementations, the
consolidation continues in step five which concludes the series.

One particular challenge in this series was that the PIC of PIIX3 used to be
instantiated outside of the south bridge while some sub functions require a PIC
with populated qemu_irqs. This has been solved by introducing a proxy PIC which
furthermore allows PIIX3 to be agnostic towards the virtualization technology
used (KVM, TCG, Xen). Due to consolidation PIIX4 gained the PIC as well,
possibly allowing the Malta board to gain KVM capabilities in the future.

Another challenge was dealing with optional devices where Peter already gave
advice in [1] which this series implements.

An unsolved problem still is PCI interrupt handling. The first function
passed to pci_bus_irqs() is device-specific while the second one seems
board-specific. This causes both PIIX device models to be coupled to a
particular board. Any advice how to resolve this would be highly appreaciated.

Could you explain this in a bit more detail?

Last but not least there might be some opportunity to consolidate VM state
handling, probably by reusing the one from PIIX3. Since I'm not very familiar
with the requirements I didn't touch it so far.

Testing done:
* make check
* Boot live CD:
   * `qemu-system-x86_64 -M pc -m 2G -accel kvm -cpu host -cdrom
manjaro-kde-21.3.2-220704-linux515.iso`
   * `qemu-system-x86_64 -M q35 -m 2G -accel kvm -cpu host -cdrom
manjaro-kde-21.3.2-220704-linux515.iso`

[1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html

Bernhard Beschow (42):
   hw/i386/pc: Create DMA controllers in south bridges
   hw/i386/pc: Create RTC controllers in south bridges
   hw/i386/pc: No need for rtc_state to be an out-parameter
   hw/i386/pc_piix: Allow for setting properties before realizing PIIX3
     south bridge
   hw/isa/piix3: Create USB controller in host device
   hw/isa/piix3: Create power management controller in host device
   hw/intc/i8259: Introduce i8259 proxy "isa-pic"
   hw/isa/piix3: Create ISA PIC in host device
   hw/isa/piix3: Create IDE controller in host device
   hw/isa/piix3: Wire up ACPI interrupt internally
   hw/isa/piix3: Remove extra ';' outside of functions
   hw/isa/piix3: Remove unused include
   hw/isa/piix3: Add size constraints to rcr_ops
   hw/isa/piix3: Modernize reset handling
   hw/isa/piix3: Prefer pci_address_space() over get_system_memory()
   hw/isa/piix3: Allow board to provide PCI interrupt routes
   hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS
   hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4
   hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4
   hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"
   hw/isa/piix3: Rename typedef PIIX3State to PIIXState
   hw/mips/malta: Reuse dev variable
   meson: Fix dependencies of piix4 southbridge
   hw/isa/piix4: Add missing initialization
   hw/isa/piix4: Move pci_ide_create_devs() call to board code
   hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
   hw/isa/piix4: Allow board to provide PCI interrupt routes
   hw/isa/piix4: Remove unused code
   hw/isa/piix4: Use ISA PIC device
   hw/isa/piix4: Reuse struct PIIXState from PIIX3
   hw/isa/piix4: Rename reset control operations to match PIIX3
   hw/isa/piix4: Rename wrongly named method
   hw/isa/piix4: Prefix pci_slot_get_pirq() with "piix4_"
   hw/isa/piix3: Merge hw/isa/piix4.c
   hw/isa/piix: Harmonize names of reset control memory regions
   hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
   hw/isa/piix: Rename functions to be shared for interrupt triggering
   hw/isa/piix: Consolidate IRQ triggering
   hw/isa/piix: Unexport PIIXState
   hw/isa/piix: Share PIIX3 base class with PIIX4
   hw/isa/piix: Drop the "3" from the PIIX base class
   hw/i386/acpi-build: Resolve PIIX ISA bridge rather than ACPI
     controller

  MAINTAINERS                             |   6 +-
  configs/devices/mips-softmmu/common.mak |   3 +-
  hw/i386/Kconfig                         |   3 +-
  hw/i386/acpi-build.c                    |   4 +-
  hw/i386/pc.c                            |  19 +-
  hw/i386/pc_piix.c                       |  72 +--
  hw/i386/pc_q35.c                        |   3 +-
  hw/intc/i8259.c                         |  27 +
  hw/isa/Kconfig                          |  14 +-
  hw/isa/lpc_ich9.c                       |  11 +
  hw/isa/meson.build                      |   3 +-
  hw/isa/piix.c                           | 669 ++++++++++++++++++++++++
  hw/isa/piix3.c                          | 431 ---------------
  hw/isa/piix4.c                          | 325 ------------
  hw/mips/malta.c                         |  34 +-
  include/hw/i386/ich9.h                  |   2 +
  include/hw/i386/pc.h                    |   2 +-
  include/hw/intc/i8259.h                 |  14 +
  include/hw/southbridge/piix.h           |  41 +-
  19 files changed, 823 insertions(+), 860 deletions(-)
  create mode 100644 hw/isa/piix.c
  delete mode 100644 hw/isa/piix3.c
  delete mode 100644 hw/isa/piix4.c

I've had a quick skim over this series and commented on the parts that caught my eye, however I'm generally happy with the way this series is going and it seems like a nice tidy-up - thanks!


ATB,

Mark.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]