Re: [PATCH v2 0/8] ACPI: X86 AML generation and GPE tracing cleanup

[Michael S. Tsirkin]

On Tue, Sep 19, 2023 at 07:47:09PM +0000, Bernhard Beschow wrote:
> 
> 
> Am 8. September 2023 08:42:26 UTC schrieb Bernhard Beschow 
> <shentey@gmail.com>:
> >This series contains changes from my effort to bring the VIA south bridges to
> >
> >the PC machine [1]. The first part of the series resolves the
> >
> >AcpiCpuAmlIfClass::madt_cpu virtual method which frees ACPI controllers from
> >
> >worrying about CPU AML generation. The second part minimizes an 
> >Intel-specific
> >
> >assumption in AML generation to just one place. The third part contains two
> >
> >ACPI tracing patches which have been reviewed a long time ago but weren't 
> >merged
> >
> >yet.
> >
> >
> >
> >The removal of AcpiCpuAmlIfClass::madt_cpu is essentially a respin of [2] 
> >with
> >
> >a different approach. Igor wasn't generally against it but wasn't convinced
> >
> >either [3]. The new approach causes much less churn and instead allows to
> >
> >remove code. So I think it's worth to be reconsidered.
> >
> >
> >
> >The motivation for removing this virtual method didn't change: It frees the 
> >ACPI
> >
> >controllers in general and PIIX4 PM in particular from generating X86 CPU 
> >AML.
> >
> >The latter is also used in MPIS context where X86 CPU AML generation is
> >
> >stubbed out. This indicates a design issue where a problem was solved at the
> >
> >wrong place. Moreover, it turned out that TYPE_ACPI_GED_X86 could be removed 
> >as
> >
> >well, further supporting this claim.
> >
> >
> >
> >The second part of this series limits SMI command port determination during 
> >AML
> >
> >generation to just one place. Currently the ACPI_PORT_SMI_CMD constant is 
> >used
> >
> >multiple times which has an Intel-specific value. In order to make the code a
> >
> >microscopic bit more compatible with our VIA south bridge models its usage 
> >gets
> >
> >limited to one place, allowing the constant to be turned into a device model
> >
> >property in the future.
> >
> >
> >
> >The third part improves the tracing experience for ACPI general purpose 
> >events.
> >
> >It originates from an old series: [4].
> >
> >
> >
> >Testing done:
> >
> >* `make check`
> >
> >* `make check-avocado`
> >
> >
> >
> >v2:
> >
> >* Trace ACPI GPE values with "0x%02" (Phil)
> >
> 
> Ping
> 
> All patches reviewed. Michael, are you the one going to queue it?
> 
> Thanks,
> Bernhard

yes, thanks!

> >
> >
> >[1] https://github.com/shentok/qemu/tree/pc-via
> >
> >[2] 
> >https://lore.kernel.org/qemu-devel/20230121151941.24120-1-shentey@gmail.com/
> >
> >[3] 
> >https://lore.kernel.org/qemu-devel/20230125174842.395fda5d@imammedo.users.ipa.redhat.com/
> >
> >[4] https://patchew.org/QEMU/20230122170724.21868-1-shentey@gmail.com/
> >
> >
> >
> >Bernhard Beschow (8):
> >
> >  hw/i386/acpi-build: Use pc_madt_cpu_entry() directly
> >
> >  hw/acpi/cpu: Have build_cpus_aml() take a build_madt_cpu_fn callback
> >
> >  hw/acpi/acpi_dev_interface: Remove now unused madt_cpu virtual method
> >
> >  hw/acpi/acpi_dev_interface: Remove now unused #include "hw/boards.h"
> >
> >  hw/i386: Remove now redundant TYPE_ACPI_GED_X86
> >
> >  hw/i386/acpi-build: Determine SMI command port just once
> >
> >  hw/acpi: Trace GPE access in all device models, not just PIIX4
> >
> >  hw/acpi/core: Trace enable and status registers of GPE separately
> >
> >
> >
> > hw/acpi/hmat.h                         |  3 ++-
> >
> > hw/i386/acpi-common.h                  |  3 +--
> >
> > include/hw/acpi/acpi_dev_interface.h   |  3 ---
> >
> > include/hw/acpi/cpu.h                  |  6 ++++-
> >
> > include/hw/acpi/generic_event_device.h |  2 --
> >
> > hw/acpi/acpi-x86-stub.c                |  6 -----
> >
> > hw/acpi/core.c                         |  9 +++++++
> >
> > hw/acpi/cpu.c                          |  9 +++----
> >
> > hw/acpi/hmat.c                         |  1 +
> >
> > hw/acpi/memory_hotplug.c               |  1 +
> >
> > hw/acpi/piix4.c                        |  5 ----
> >
> > hw/i386/acpi-build.c                   | 13 +++++-----
> >
> > hw/i386/acpi-common.c                  |  5 ++--
> >
> > hw/i386/acpi-microvm.c                 |  3 +--
> >
> > hw/i386/generic_event_device_x86.c     | 36 --------------------------
> >
> > hw/i386/microvm.c                      |  2 +-
> >
> > hw/isa/lpc_ich9.c                      |  1 -
> >
> > hw/acpi/trace-events                   | 10 ++++---
> >
> > hw/i386/meson.build                    |  1 -
> >
> > 19 files changed, 38 insertions(+), 81 deletions(-)
> >
> > delete mode 100644 hw/i386/generic_event_device_x86.c
> >
> >
> >
> >-- >
> >2.42.0
> >
> >
> >