Re: [PATCH v2 00/15] hw/southbridge: Extract ICH9 QOM container model

[Bernhard Beschow <shentey@gmail.com>]

Am 26. Februar 2024 11:13:59 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>Since v1 [1]:
>- Rebased on top of Bernhard patches
>- Rename files with 'ich9_' prefix (Bernhard)
>
>Hi,
>
>I have a long standing southbridge QOM rework branches. Since
>Bernhard is actively working on the PIIX, I'll try to refresh
>and post. This is also motivated by the Dynamic Machine work
>where we are trying to figure the ideal DSL for QEMU, so having
>complex models well designed help.
>
>Here we introduce the ICH9 'southbridge' as a QOM container.
>Since the chipset comes as a whole, we shouldn't instantiate
>its components separately. However in order to maintain old
>code we expose some properties to configure the container and
>not introduce any change for the Q35 machine. There is no
>migration change, only QOM objects moved around.

I really like the simplicity of the machine code and that the ICH9 southbridge becomes a proper device rather than being scattered around in machine code. I've made some reviews in form of a branch: https://github.com/shentok/qemu/commits/philmd/ich9_qom-v2/

>
>More work remain in the LPC function (more code to remove from
>Q35). Maybe worth doing in parallel with the PIIX to clean both
>PC machines.

Would be nice if the pattern could then also be applied to the VIA southbridges, otherwise this could break my via-apollo-pro-133t branch: https://github.com/shentok/qemu/tree/via-apollo-pro-133t

Best regards,
Bernhard

>
>Also we'd need to decouple the cpu_interrupt() calls between hw/
>and target/.
>
>Note that GSI is currently broken [2]. Once the LPC/ISA part is
>done, it might be easier to fix it.
>
>[1] https://lore.kernel.org/qemu-devel/20240219163855.87326-1-philmd@linaro.org/
>[2] https://lore.kernel.org/qemu-devel/cd0e13c6-c03d-411f-83a5-1d4d28ea4345@linaro.org/
>
>Philippe Mathieu-Daudé (15):
>  MAINTAINERS: Add 'ICH9 South Bridge' section
>  hw/i386/q35: Add local 'lpc_obj' variable
>  hw/acpi/ich9: Restrict definitions from 'hw/southbridge/ich9.h'
>  hw/acpi/ich9_tco: Include 'ich9' in names
>  hw/acpi/ich9_tco: Restrict ich9_generate_smi() declaration
>  hw/ide: Rename ich.c -> ich9_ahci.c
>  hw/i2c/smbus: Extract QOM ICH9 definitions to 'ich9_smbus.h'
>  hw/pci-bridge: Extract QOM ICH definitions to 'ich9_dmi.h'
>  hw/southbridge/ich9: Introduce TYPE_ICH9_SOUTHBRIDGE stub
>  hw/southbridge/ich9: Add the DMI-to-PCI bridge
>  hw/southbridge/ich9: Add a AHCI function
>  hw/southbridge/ich9: Add the SMBus function
>  hw/southbridge/ich9: Add the USB EHCI/UHCI functions
>  hw/southbridge/ich9: Extract LPC definitions to 'hw/isa/ich9_lpc.h'
>  hw/southbridge/ich9: Add the LPC / ISA bridge function
>
> MAINTAINERS                               |  21 +-
> include/hw/acpi/ich9.h                    |  15 ++
> include/hw/acpi/ich9_tco.h                |   6 +-
> include/hw/i2c/ich9_smbus.h               |  25 +++
> include/hw/isa/ich9_lpc.h                 | 166 +++++++++++++++
> include/hw/pci-bridge/ich9_dmi.h          |  20 ++
> include/hw/southbridge/ich9.h             | 235 +---------------------
> hw/acpi/ich9.c                            |   9 +-
> hw/acpi/ich9_tco.c                        |   5 +-
> hw/i2c/{smbus_ich9.c => ich9_smbus.c}     |  36 +++-
> hw/i386/acpi-build.c                      |   1 +
> hw/i386/pc_q35.c                          | 126 +++---------
> hw/ide/{ich.c => ich9_ahci.c}             |   0
> hw/isa/{lpc_ich9.c => ich9_lpc.c}         |  37 +++-
> hw/pci-bridge/{i82801b11.c => ich9_dmi.c} |  11 +-
> hw/southbridge/ich9.c                     | 213 ++++++++++++++++++++
> tests/qtest/tco-test.c                    |   2 +-
> hw/Kconfig                                |   1 +
> hw/i2c/meson.build                        |   2 +-
> hw/i386/Kconfig                           |   3 +-
> hw/ide/meson.build                        |   2 +-
> hw/isa/meson.build                        |   2 +-
> hw/meson.build                            |   1 +
> hw/pci-bridge/meson.build                 |   2 +-
> hw/southbridge/Kconfig                    |  11 +
> hw/southbridge/meson.build                |   3 +
> 26 files changed, 587 insertions(+), 368 deletions(-)
> create mode 100644 include/hw/i2c/ich9_smbus.h
> create mode 100644 include/hw/isa/ich9_lpc.h
> create mode 100644 include/hw/pci-bridge/ich9_dmi.h
> rename hw/i2c/{smbus_ich9.c => ich9_smbus.c} (77%)
> rename hw/ide/{ich.c => ich9_ahci.c} (100%)
> rename hw/isa/{lpc_ich9.c => ich9_lpc.c} (95%)
> rename hw/pci-bridge/{i82801b11.c => ich9_dmi.c} (95%)
> create mode 100644 hw/southbridge/ich9.c
> create mode 100644 hw/southbridge/Kconfig
> create mode 100644 hw/southbridge/meson.build
>