mbox series

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

Message ID 20240226111416.39217-1-philmd@linaro.org
Headers show
Series hw/southbridge: Extract ICH9 QOM container model | expand

Message

Philippe Mathieu-Daudé Feb. 26, 2024, 11:13 a.m. UTC
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.

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.

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

Comments

Philippe Mathieu-Daudé Feb. 26, 2024, 1:07 p.m. UTC | #1
On 26/2/24 12:13, Philippe Mathieu-Daudé wrote:

> 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.

> 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

Branch available here:
https://gitlab.com/philmd/qemu/-/commits/ich9_qom-v2/
Bernhard Beschow Feb. 26, 2024, 10:44 p.m. UTC | #2
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
>
Michael S. Tsirkin March 12, 2024, 4:49 p.m. UTC | #3
On Mon, Feb 26, 2024 at 12:13:59PM +0100, Philippe Mathieu-Daudé wrote:
> 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.
> 
> 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.

Bernhard had some comments so I hope you'll address them so
I can merge - after the release presumably.


> 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
> 
> -- 
> 2.41.0