[v2,00/43] Consolidate PIIX south bridges

Message

Bernhard Beschow Oct. 22, 2022, 3:04 p.m. UTC

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.

A challenge still remains with consolidating PCI interrupt handling. There are
still board-specific piix3_pci_slot_get_pirq() and piix4_pci_slot_get_pirq()
which are implemented in isa/piix.c. Any advice how to resolve these would be
highly appreaciated.

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
* make check-avocado
* 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`
* 'qemu-system-mips64el -M malta -kernel vmlinux-3.2.0-4-5kc-malta -hda debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`

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

v2:
- Introduce TYPE_ defines for IDE and USB device models (Mark)
- Omit unexporting of PIIXState (Mark)
- Improve commit message of patch 5 to mention reset triggering through PCI
  configuration space (Mark)
- Move reviewed patches w/o dependencies to the bottom of the series for early
  upstreaming -> @Michael?

Bernhard Beschow (43):
  hw/i386/pc: Create DMA controllers in south bridges
  hw/i386/pc_piix: Allow for setting properties before realizing PIIX3
    south bridge
  hw/isa/piix3: Remove extra ';' outside of functions
  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/piix4: Rename wrongly named method
  hw/ide/piix: Introduce TYPE_ macros for PIIX IDE controllers
  hw/usb/hcd-uhci: Introduce TYPE_ defines for device models
  hw/i386/pc: Create RTC controllers in south bridges
  hw/i386/pc: No need for rtc_state to be an out-parameter
  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 unused include
  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: 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: 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                        |  16 +-
 hw/ide/piix.c                           |   5 +-
 hw/intc/i8259.c                         |  27 ++
 hw/isa/Kconfig                          |  14 +-
 hw/isa/lpc_ich9.c                       |  11 +
 hw/isa/meson.build                      |   3 +-
 hw/isa/{piix3.c => piix.c}              | 337 ++++++++++++++++++------
 hw/isa/piix4.c                          | 325 -----------------------
 hw/mips/malta.c                         |  34 ++-
 hw/usb/hcd-uhci.c                       |  16 +-
 hw/usb/hcd-uhci.h                       |   9 +
 include/hw/i386/ich9.h                  |   2 +
 include/hw/i386/pc.h                    |   2 +-
 include/hw/ide/piix.h                   |   7 +
 include/hw/intc/i8259.h                 |  14 +
 include/hw/southbridge/piix.h           |  33 ++-
 22 files changed, 473 insertions(+), 489 deletions(-)
 rename hw/isa/{piix3.c => piix.c} (51%)
 delete mode 100644 hw/isa/piix4.c
 create mode 100644 include/hw/ide/piix.h

Comments

Philippe Mathieu-Daudé Oct. 23, 2022, 9:05 p.m. UTC | #1

On 22/10/22 17:04, Bernhard Beschow wrote:
> Ammends commit 988fb613215993dd0ce642b89ca8182c479d39dd.
> 
> Signed-off-by: Bernhard Beschow <[e-mail]>
> ---
>   hw/isa/piix3.c | 1 -
>   1 file changed, 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>

Philippe Mathieu-Daudé Oct. 24, 2022, 5:12 a.m. UTC | #2

On 22/10/22 17:04, Bernhard Beschow wrote:
> PIIX3 initializes the PIRQx route control registers to the default
> values as described in the 82371AB PCI-TO-ISA/IDE XCELERATOR (PIIX4)
> April 1997 manual. PIIX4, however, initializes the routes according to
> the Malta™ User’s Manual, ch 6.6, which are IRQs 10 and 11. In order to
> allow the reset methods to be consolidated, allow board code to specify
> the routes.
> 
> Signed-off-by: Bernhard Beschow <[e-mail]>
> ---
>   hw/isa/piix3.c                | 12 ++++++++----
>   include/hw/southbridge/piix.h |  1 +
>   2 files changed, 9 insertions(+), 4 deletions(-)

> diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
> index 1f22eb1444..df3e0084c5 100644
> --- a/include/hw/southbridge/piix.h
> +++ b/include/hw/southbridge/piix.h
> @@ -54,6 +54,7 @@ struct PIIXState {
>   
>       /* This member isn't used. Just for save/load compatibility */
>       int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
> +    uint8_t pci_irq_reset_mappings[PIIX_NUM_PIRQS];

pci_irq_reset_mappings[PCI_NUM_PINS]?

>   
>       ISAPICState pic;
>       RTCState rtc;