[v4] AML Housekeeping

[PATCH v4 0/7] AML Housekeeping

Posted by Bernhard Beschow 1 year, 2 months ago

This series factors out AcpiCpuAmlIfClass::madt_cpu from AcpiDeviceIfClass.
By letting the (x86) CPUs implement the new interface, AML generation is
delegated to the CPUs, freeing the ACPI controllers from worrying about x86 CPU
specifics. The delegation to the CPUs is especially interesting for the PIIX4 PM
since it is also used in MIPS only contexts where no ACPI bios is available.

Furthermore, the series introduces qbus_build_aml() which replaces
isa_build_aml() and resolves some open coding.

v4:
- Squash qbus_build_aml() patches into one (Igor)
- Don't use a bare function pointer for AcpiDeviceIfClass::madt_cpu (Igor)

Testing done:
* `make check`
* `qemu-system-x86_64 -M pc -m 2G -cdrom manjaro-kde-21.2.6-220416-linux515.iso`
* `qemu-system-x86_64 -M q35 -m 2G -cdrom \
   manjaro-kde-21.2.6-220416-linux515.iso`

v3:
- Clean up includes in AcpiDeviceIfClass::madt_cpu sub series last (Markus)
- Restructure qbus_build_aml() sub series (Phil, me)

v2:
- Don't inline qbus_build_aml() (Phil)
- Add 'hw/acpi/piix4: No need to #include "hw/southbridge/piix.h"'

Bernhard Beschow (7):
  hw/i386/acpi-build: Remove unused attributes
  hw/isa/isa-bus: Turn isa_build_aml() into qbus_build_aml()
  hw/acpi/piix4: No need to #include "hw/southbridge/piix.h"
  hw/acpi/acpi_dev_interface: Remove unused parameter from
    AcpiDeviceIfClass::madt_cpu
  hw/acpi/acpi_dev_interface: Factor out TYPE_ACPI_CPU_AML_IF
  hw/acpi/acpi_dev_interface: Remove now unused #include "hw/boards.h"
  hw/i386/pc: Unexport pc_madt_cpu_entry()

 hw/acpi/hmat.h                       |  3 +-
 hw/i386/acpi-common.h                |  3 +-
 include/hw/acpi/acpi_aml_interface.h |  3 ++
 include/hw/acpi/acpi_cpu_interface.h | 26 ++++++++++++++++
 include/hw/acpi/acpi_dev_interface.h |  4 ---
 include/hw/i386/pc.h                 |  6 ----
 include/hw/isa/isa.h                 |  1 -
 hw/acpi/acpi-x86-stub.c              |  7 -----
 hw/acpi/acpi_interface.c             | 18 ++++++++++-
 hw/acpi/cpu.c                        | 13 ++++----
 hw/acpi/hmat.c                       |  1 +
 hw/acpi/memory_hotplug.c             |  1 +
 hw/acpi/piix4.c                      |  3 --
 hw/i2c/smbus_ich9.c                  |  5 +--
 hw/i386/acpi-build.c                 |  5 +--
 hw/i386/acpi-common.c                | 42 +++----------------------
 hw/i386/acpi-microvm.c               |  6 ++--
 hw/i386/generic_event_device_x86.c   |  9 ------
 hw/isa/isa-bus.c                     | 10 ------
 hw/isa/lpc_ich9.c                    |  6 +---
 hw/isa/piix3.c                       |  5 +--
 monitor/qmp-cmds.c                   |  1 +
 target/i386/cpu.c                    | 46 ++++++++++++++++++++++++++++
 23 files changed, 117 insertions(+), 107 deletions(-)
 create mode 100644 include/hw/acpi/acpi_cpu_interface.h

-- 
2.39.1

Re: [PATCH v4 0/7] AML Housekeeping

Posted by Igor Mammedov 1 year, 2 months ago

On Sat, 21 Jan 2023 16:19:34 +0100
Bernhard Beschow <shentey@gmail.com> wrote:

> This series factors out AcpiCpuAmlIfClass::madt_cpu from AcpiDeviceIfClass.
> By letting the (x86) CPUs implement the new interface, AML generation is
> delegated to the CPUs, freeing the ACPI controllers from worrying about x86 CPU
> specifics. The delegation to the CPUs is especially interesting for the PIIX4 PM
> since it is also used in MIPS only contexts where no ACPI bios is available.
> 
> Furthermore, the series introduces qbus_build_aml() which replaces
> isa_build_aml() and resolves some open coding.

I'm done with this series review
(skipped 6-7/7, since they depend on 5/7 which seems unnecessary to me)

> 
> v4:
> - Squash qbus_build_aml() patches into one (Igor)
> - Don't use a bare function pointer for AcpiDeviceIfClass::madt_cpu (Igor)
> 
> Testing done:
> * `make check`
> * `qemu-system-x86_64 -M pc -m 2G -cdrom manjaro-kde-21.2.6-220416-linux515.iso`
> * `qemu-system-x86_64 -M q35 -m 2G -cdrom \
>    manjaro-kde-21.2.6-220416-linux515.iso`
> 
> v3:
> - Clean up includes in AcpiDeviceIfClass::madt_cpu sub series last (Markus)
> - Restructure qbus_build_aml() sub series (Phil, me)
> 
> v2:
> - Don't inline qbus_build_aml() (Phil)
> - Add 'hw/acpi/piix4: No need to #include "hw/southbridge/piix.h"'
> 
> Bernhard Beschow (7):
>   hw/i386/acpi-build: Remove unused attributes
>   hw/isa/isa-bus: Turn isa_build_aml() into qbus_build_aml()
>   hw/acpi/piix4: No need to #include "hw/southbridge/piix.h"
>   hw/acpi/acpi_dev_interface: Remove unused parameter from
>     AcpiDeviceIfClass::madt_cpu
>   hw/acpi/acpi_dev_interface: Factor out TYPE_ACPI_CPU_AML_IF
>   hw/acpi/acpi_dev_interface: Remove now unused #include "hw/boards.h"
>   hw/i386/pc: Unexport pc_madt_cpu_entry()
> 
>  hw/acpi/hmat.h                       |  3 +-
>  hw/i386/acpi-common.h                |  3 +-
>  include/hw/acpi/acpi_aml_interface.h |  3 ++
>  include/hw/acpi/acpi_cpu_interface.h | 26 ++++++++++++++++
>  include/hw/acpi/acpi_dev_interface.h |  4 ---
>  include/hw/i386/pc.h                 |  6 ----
>  include/hw/isa/isa.h                 |  1 -
>  hw/acpi/acpi-x86-stub.c              |  7 -----
>  hw/acpi/acpi_interface.c             | 18 ++++++++++-
>  hw/acpi/cpu.c                        | 13 ++++----
>  hw/acpi/hmat.c                       |  1 +
>  hw/acpi/memory_hotplug.c             |  1 +
>  hw/acpi/piix4.c                      |  3 --
>  hw/i2c/smbus_ich9.c                  |  5 +--
>  hw/i386/acpi-build.c                 |  5 +--
>  hw/i386/acpi-common.c                | 42 +++----------------------
>  hw/i386/acpi-microvm.c               |  6 ++--
>  hw/i386/generic_event_device_x86.c   |  9 ------
>  hw/isa/isa-bus.c                     | 10 ------
>  hw/isa/lpc_ich9.c                    |  6 +---
>  hw/isa/piix3.c                       |  5 +--
>  monitor/qmp-cmds.c                   |  1 +
>  target/i386/cpu.c                    | 46 ++++++++++++++++++++++++++++
>  23 files changed, 117 insertions(+), 107 deletions(-)
>  create mode 100644 include/hw/acpi/acpi_cpu_interface.h
>

Re: [PATCH v4 0/7] AML Housekeeping

Posted by Bernhard Beschow 1 year, 2 months ago


Am 25. Januar 2023 16:52:34 UTC schrieb Igor Mammedov <imammedo@redhat.com>:
>On Sat, 21 Jan 2023 16:19:34 +0100
>Bernhard Beschow <shentey@gmail.com> wrote:
>
>> This series factors out AcpiCpuAmlIfClass::madt_cpu from AcpiDeviceIfClass.
>> By letting the (x86) CPUs implement the new interface, AML generation is
>> delegated to the CPUs, freeing the ACPI controllers from worrying about x86 CPU
>> specifics. The delegation to the CPUs is especially interesting for the PIIX4 PM
>> since it is also used in MIPS only contexts where no ACPI bios is available.
>> 
>> Furthermore, the series introduces qbus_build_aml() which replaces
>> isa_build_aml() and resolves some open coding.
>
>I'm done with this series review
>(skipped 6-7/7, since they depend on 5/7 which seems unnecessary to me)

Thanks!

Okay, let's omit patches 5-7 for now. It makes sense to include them in a dedicated x86 cleanup series.

Michael, shall I respin a v5 with only the reviewed patches?

Best regards,
Bernhard
>
>> 
>> v4:
>> - Squash qbus_build_aml() patches into one (Igor)
>> - Don't use a bare function pointer for AcpiDeviceIfClass::madt_cpu (Igor)
>> 
>> Testing done:
>> * `make check`
>> * `qemu-system-x86_64 -M pc -m 2G -cdrom manjaro-kde-21.2.6-220416-linux515.iso`
>> * `qemu-system-x86_64 -M q35 -m 2G -cdrom \
>>    manjaro-kde-21.2.6-220416-linux515.iso`
>> 
>> v3:
>> - Clean up includes in AcpiDeviceIfClass::madt_cpu sub series last (Markus)
>> - Restructure qbus_build_aml() sub series (Phil, me)
>> 
>> v2:
>> - Don't inline qbus_build_aml() (Phil)
>> - Add 'hw/acpi/piix4: No need to #include "hw/southbridge/piix.h"'
>> 
>> Bernhard Beschow (7):
>>   hw/i386/acpi-build: Remove unused attributes
>>   hw/isa/isa-bus: Turn isa_build_aml() into qbus_build_aml()
>>   hw/acpi/piix4: No need to #include "hw/southbridge/piix.h"
>>   hw/acpi/acpi_dev_interface: Remove unused parameter from
>>     AcpiDeviceIfClass::madt_cpu
>>   hw/acpi/acpi_dev_interface: Factor out TYPE_ACPI_CPU_AML_IF
>>   hw/acpi/acpi_dev_interface: Remove now unused #include "hw/boards.h"
>>   hw/i386/pc: Unexport pc_madt_cpu_entry()
>> 
>>  hw/acpi/hmat.h                       |  3 +-
>>  hw/i386/acpi-common.h                |  3 +-
>>  include/hw/acpi/acpi_aml_interface.h |  3 ++
>>  include/hw/acpi/acpi_cpu_interface.h | 26 ++++++++++++++++
>>  include/hw/acpi/acpi_dev_interface.h |  4 ---
>>  include/hw/i386/pc.h                 |  6 ----
>>  include/hw/isa/isa.h                 |  1 -
>>  hw/acpi/acpi-x86-stub.c              |  7 -----
>>  hw/acpi/acpi_interface.c             | 18 ++++++++++-
>>  hw/acpi/cpu.c                        | 13 ++++----
>>  hw/acpi/hmat.c                       |  1 +
>>  hw/acpi/memory_hotplug.c             |  1 +
>>  hw/acpi/piix4.c                      |  3 --
>>  hw/i2c/smbus_ich9.c                  |  5 +--
>>  hw/i386/acpi-build.c                 |  5 +--
>>  hw/i386/acpi-common.c                | 42 +++----------------------
>>  hw/i386/acpi-microvm.c               |  6 ++--
>>  hw/i386/generic_event_device_x86.c   |  9 ------
>>  hw/isa/isa-bus.c                     | 10 ------
>>  hw/isa/lpc_ich9.c                    |  6 +---
>>  hw/isa/piix3.c                       |  5 +--
>>  monitor/qmp-cmds.c                   |  1 +
>>  target/i386/cpu.c                    | 46 ++++++++++++++++++++++++++++
>>  23 files changed, 117 insertions(+), 107 deletions(-)
>>  create mode 100644 include/hw/acpi/acpi_cpu_interface.h
>> 
>

Re: [PATCH v4 0/7] AML Housekeeping

Posted by Michael S. Tsirkin 1 year, 2 months ago

On Thu, Jan 26, 2023 at 10:42:31AM +0000, Bernhard Beschow wrote:
> 
> 
> Am 25. Januar 2023 16:52:34 UTC schrieb Igor Mammedov <imammedo@redhat.com>:
> >On Sat, 21 Jan 2023 16:19:34 +0100
> >Bernhard Beschow <shentey@gmail.com> wrote:
> >
> >> This series factors out AcpiCpuAmlIfClass::madt_cpu from AcpiDeviceIfClass.
> >> By letting the (x86) CPUs implement the new interface, AML generation is
> >> delegated to the CPUs, freeing the ACPI controllers from worrying about x86 CPU
> >> specifics. The delegation to the CPUs is especially interesting for the PIIX4 PM
> >> since it is also used in MIPS only contexts where no ACPI bios is available.
> >> 
> >> Furthermore, the series introduces qbus_build_aml() which replaces
> >> isa_build_aml() and resolves some open coding.
> >
> >I'm done with this series review
> >(skipped 6-7/7, since they depend on 5/7 which seems unnecessary to me)
> 
> Thanks!
> 
> Okay, let's omit patches 5-7 for now. It makes sense to include them in a dedicated x86 cleanup series.
> 
> Michael, shall I respin a v5 with only the reviewed patches?
> 
> Best regards,
> Bernhard

No need.

> >
> >> 
> >> v4:
> >> - Squash qbus_build_aml() patches into one (Igor)
> >> - Don't use a bare function pointer for AcpiDeviceIfClass::madt_cpu (Igor)
> >> 
> >> Testing done:
> >> * `make check`
> >> * `qemu-system-x86_64 -M pc -m 2G -cdrom manjaro-kde-21.2.6-220416-linux515.iso`
> >> * `qemu-system-x86_64 -M q35 -m 2G -cdrom \
> >>    manjaro-kde-21.2.6-220416-linux515.iso`
> >> 
> >> v3:
> >> - Clean up includes in AcpiDeviceIfClass::madt_cpu sub series last (Markus)
> >> - Restructure qbus_build_aml() sub series (Phil, me)
> >> 
> >> v2:
> >> - Don't inline qbus_build_aml() (Phil)
> >> - Add 'hw/acpi/piix4: No need to #include "hw/southbridge/piix.h"'
> >> 
> >> Bernhard Beschow (7):
> >>   hw/i386/acpi-build: Remove unused attributes
> >>   hw/isa/isa-bus: Turn isa_build_aml() into qbus_build_aml()
> >>   hw/acpi/piix4: No need to #include "hw/southbridge/piix.h"
> >>   hw/acpi/acpi_dev_interface: Remove unused parameter from
> >>     AcpiDeviceIfClass::madt_cpu
> >>   hw/acpi/acpi_dev_interface: Factor out TYPE_ACPI_CPU_AML_IF
> >>   hw/acpi/acpi_dev_interface: Remove now unused #include "hw/boards.h"
> >>   hw/i386/pc: Unexport pc_madt_cpu_entry()
> >> 
> >>  hw/acpi/hmat.h                       |  3 +-
> >>  hw/i386/acpi-common.h                |  3 +-
> >>  include/hw/acpi/acpi_aml_interface.h |  3 ++
> >>  include/hw/acpi/acpi_cpu_interface.h | 26 ++++++++++++++++
> >>  include/hw/acpi/acpi_dev_interface.h |  4 ---
> >>  include/hw/i386/pc.h                 |  6 ----
> >>  include/hw/isa/isa.h                 |  1 -
> >>  hw/acpi/acpi-x86-stub.c              |  7 -----
> >>  hw/acpi/acpi_interface.c             | 18 ++++++++++-
> >>  hw/acpi/cpu.c                        | 13 ++++----
> >>  hw/acpi/hmat.c                       |  1 +
> >>  hw/acpi/memory_hotplug.c             |  1 +
> >>  hw/acpi/piix4.c                      |  3 --
> >>  hw/i2c/smbus_ich9.c                  |  5 +--
> >>  hw/i386/acpi-build.c                 |  5 +--
> >>  hw/i386/acpi-common.c                | 42 +++----------------------
> >>  hw/i386/acpi-microvm.c               |  6 ++--
> >>  hw/i386/generic_event_device_x86.c   |  9 ------
> >>  hw/isa/isa-bus.c                     | 10 ------
> >>  hw/isa/lpc_ich9.c                    |  6 +---
> >>  hw/isa/piix3.c                       |  5 +--
> >>  monitor/qmp-cmds.c                   |  1 +
> >>  target/i386/cpu.c                    | 46 ++++++++++++++++++++++++++++
> >>  23 files changed, 117 insertions(+), 107 deletions(-)
> >>  create mode 100644 include/hw/acpi/acpi_cpu_interface.h
> >> 
> >