Re: [Qemu-devel] [PATCH 00/19] pc: acpi: move memory hotplug out of DSDT/SSDT into custom table

[Laszlo Ersek <lersek@redhat.com>]

Igor,

On 10/23/15 16:57, Igor Mammedov wrote:
> As part of moving to dynamic DSDT and dropping
> ASL templates althogether this series moves out
> ASL part of memory hotplug from DSDT into a custom
> ACPI table. Beside of reducing ASL codebase (DSDT)
> series tries to generalize and consolidate ACPI
> part of memory hotplug code so it could be easier
> to reuse it with outher targets (I plan to do it
> for ARM later).
> 
> New table uses 64-bit integers for simplification
> of code that are available only since ACPI 2.0.
> To avoid breaking Windows XP based (rev1) guests
> that table is loaded only if guest supports 2.0
> revision Using LoadTable() ASL method.

Please consider CC'ing me on future ACPI patches -- it's risky to hope
that (or not to care if) I notice them automatically.

In particular, it's great that Windows supports LoadTable(), *but* the
following requirement of the ACPI spec:

    Any table referenced by LoadTable must be in memory marked by
    AddressRangeReserved or AddressRangeNVS.

needs extra care if you'd like this to work under OVMF.

In fact this topic shares a whole bunch of details with our pending
DataTableRegion / VMGenID design:

http://thread.gmane.org/gmane.comp.emulators.qemu/357940/focus=361701

and for OVMF's linker/loader (and for edk2's generic
EFI_ACPI_TABLE_PROTOCOL) to work with this, you'd have to jump through
almost the same hoops.

In particular, the new ACPI table should have signature "UEFI" (to make
sure EFI_ACPI_TABLE_PROTOCOL allocates it in AcpiNVS). In addition, that
signature dictates a data table structure (with a few extra fixed
headers), not a definition block, according to the UEFI spec Appendix O.

So, I'm not sure this can be made work with OVMF at all, if we'd like to
respect all relevant specs (and existing code).

... I wonder when we'll *finally* bite the bullet, and say, "if you want
XP to work, then simply don't specify this feature on the QEMU command
line, otherwise XP will crash".

We could simply create a text file under docs/, and collect all the
features / cmdline switches that are *expressly* incompatible with
Windows XP. (CC'ing Peter.)

For this memory hotplug feature, the direct consequence would be that
you could separate out the MHPT stuff into a new *SSDT* instead, and
everything would just work automatically with OVMF as well.

Of course I can also accept if the consensus is to ignore OVMF...

Thanks
Laszlo

> 
> As a side effect of code consolidation/simplification
> series reduces ACPI tables size on:
>  - 879 bytes without memory hotplug
>  - 1216 bytes with 256 hotpluggable memory slots
> 
> More detailed breakdown on max table sizes in default
> and hotplug enabled cases:
>      new+HP  new    old   old+HP
> MHPT 50808   -       -    -
> SSDT 2425    2368   2486  53688
> DSDT 2267    2267   3028  3028
> 
> Tested with following guests:
>  - RHEL7.2,
>  - Windows XPsp3, Windows Server 2003EE,
>  - Windows Server 2008DC, Windows Server 2012R2DC
> 
> Since memory hotpulg doesn't work on XP based guests,
> I've just tested that it boots just fine with/without
> memory hotplug enabled on QEMU side.
> The rest of the guests work as expected.
> 
> git tree for testing:
> https://github.com/imammedo/qemu/commits/mhpt_table_v1
> 
> Igor Mammedov (18):
>   acpi: aml: add aml_lgreater_equal() and aml_load_table()
>   acpi: aml: add aml_create_qword_field()
>   acpi: aml: add aml_decrement() and aml_subtract()
>   acpi: aml: add aml_call0() helper
>   pc: acpi: move SSDT part of memhp into a custom table
>   pc: acpi: memhp: move MHPD._STA method into MHPT table
>   pc: acpi: memhp: move MHPD.MLCK mutex into NHPT table
>   pc: acpi: memhp: move MHPD.MSCN method into MHPT table
>   pc: acpi: make memory device's _UID integer
>   pc: acpi: memhp: move MHPD.MRST method into MHPT table
>   pc: acpi: memhp: move MHPD.MPXM method into MHPT table
>   pc: acpi: memhp: move MHPD.MOST method into MHPT table
>   pc: acpi: memhp: move MHPD.MEJ0 method into MHPT table
>   pc: acpi: bump DSDT revision compliance to v2
>   pc: acpi: memhp: move MHPD.MCRS method into MHPT table
>   pc: acpi: memhp: move MHPD Device along with _UID/_HID into MHPT table
>   pc: acpi: memhp: remove acpi-dsdt-mem-hotplug.dsl and move \_GPE._E03
>     into SSDT
>   pc: acpi: memhp: cleanup MEMORY_HOTPLUG_IO_REGION usage
> 
> Xiao Guangrong (1):
>   acpi: add aml_mutex(), aml_acquire(), aml_release()
> 
>  hw/acpi/Makefile.objs               |   2 +-
>  hw/acpi/aml-build.c                 | 115 ++++++++++++++-
>  hw/acpi/memory_hotplug_acpi_table.c | 286 ++++++++++++++++++++++++++++++++++++
>  hw/i386/acpi-build.c                | 140 ++++--------------
>  hw/i386/acpi-dsdt-mem-hotplug.dsl   | 171 ---------------------
>  hw/i386/acpi-dsdt.dsl               |   7 +-
>  hw/i386/q35-acpi-dsdt.dsl           |   7 +-
>  include/hw/acpi/aml-build.h         |   9 ++
>  include/hw/acpi/memory_hotplug.h    |   8 +
>  include/hw/acpi/pc-hotplug.h        |  24 ---
>  10 files changed, 443 insertions(+), 326 deletions(-)
>  create mode 100644 hw/acpi/memory_hotplug_acpi_table.c
>  delete mode 100644 hw/i386/acpi-dsdt-mem-hotplug.dsl
>