diff mbox

[06/19] pc: acpi: move SSDT part of memhp into a custom table

Message ID 1445612242-79172-7-git-send-email-imammedo@redhat.com
State New
Headers show

Commit Message

Igor Mammedov Oct. 23, 2015, 2:57 p.m. UTC 
moves SSDT part to custom MHPT table, which is loaded
at runtime by OSPM if it supports ACPIv2 revision and
only if memory hotplug is enabled.
That should reduce ACPI tables blob size if memory
hotplug is not enabled (default case).

Checked for compatibility issues with:
 * Windows XPsp3, Windows Server 2003: they don't load
   the table as OSPM only reports revision 1 as supported.
   And we don't care about memhp for these guests as they
   do not support it anyway.
 * Windows Server 2008, Windows Server 2008R2:
   works as expected

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/acpi/Makefile.objs               |   2 +-
 hw/acpi/memory_hotplug_acpi_table.c | 138 ++++++++++++++++++++++++++++++++++++
 hw/i386/acpi-build.c                | 130 +++++----------------------------
 include/hw/acpi/memory_hotplug.h    |   3 +
 4 files changed, 159 insertions(+), 114 deletions(-)
 create mode 100644 hw/acpi/memory_hotplug_acpi_table.c

Comments

Michael S. Tsirkin Oct. 24, 2015, 5:41 p.m. UTC | #1 
On Fri, Oct 23, 2015 at 04:57:09PM +0200, Igor Mammedov wrote:
> +    /* copy AML table into ACPI tables blob and patch header there */
> +    g_array_append_vals(table_data, table->buf->data, table->buf->len);
> +    build_header(linker, table_data,
> +        (void *)(table_data->data + table_data->len - table->buf->len),
> +        "MHPT", table->buf->len, 2);
> +    free_aml_allocator();
> +}

Why not just put this in an SSDT?
And what guarantees that there won't be an MHPT table
in a future version of the spec, conflicting with this use?
Michael S. Tsirkin Oct. 24, 2015, 5:59 p.m. UTC | #2 
On Fri, Oct 23, 2015 at 04:57:09PM +0200, Igor Mammedov wrote:
> moves SSDT part to custom MHPT table, which is loaded
> at runtime by OSPM if it supports ACPIv2 revision and
> only if memory hotplug is enabled.
> That should reduce ACPI tables blob size if memory
> hotplug is not enabled (default case).

With seabios, it's still in reserved memory. How does it help?

And this trick likely breaks UEFI.

I just checked, the only legal way to specify OEM
specific tables seems to be using OEMX prefix.

But there's a decent chance this will conflict with
people using command line flags to load their own
tables.

This likely means this trick is off.



> Checked for compatibility issues with:
>  * Windows XPsp3, Windows Server 2003: they don't load
>    the table as OSPM only reports revision 1 as supported.
>    And we don't care about memhp for these guests as they
>    do not support it anyway.
>  * Windows Server 2008, Windows Server 2008R2:
>    works as expected
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/acpi/Makefile.objs               |   2 +-
>  hw/acpi/memory_hotplug_acpi_table.c | 138 ++++++++++++++++++++++++++++++++++++
>  hw/i386/acpi-build.c                | 130 +++++----------------------------
>  include/hw/acpi/memory_hotplug.h    |   3 +
>  4 files changed, 159 insertions(+), 114 deletions(-)
>  create mode 100644 hw/acpi/memory_hotplug_acpi_table.c
> 
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index 7d3230c..c04064e 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -1,7 +1,7 @@
>  common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
>  common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
>  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
> -common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> +common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
>  common-obj-$(CONFIG_ACPI) += acpi_interface.o
>  common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
>  common-obj-$(CONFIG_ACPI) += aml-build.o
> diff --git a/hw/acpi/memory_hotplug_acpi_table.c b/hw/acpi/memory_hotplug_acpi_table.c
> new file mode 100644
> index 0000000..28da13c
> --- /dev/null
> +++ b/hw/acpi/memory_hotplug_acpi_table.c
> @@ -0,0 +1,138 @@
> +#include <stdbool.h>
> +#include "hw/acpi/aml-build.h"
> +#include "hw/acpi/memory_hotplug.h"
> +#include "include/hw/acpi/pc-hotplug.h"
> +#include "hw/boards.h"
> +
> +#define BASEPATH "\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE) "."
> +
> +void build_mhpt(GArray *table_data, GArray *linker, uint32_t nr_mem,
> +                uint16_t io_base, uint16_t io_len)
> +{
> +    int i;
> +    Aml *table, *sb_scope, *dev, *method, *ifctx, *ctrl_dev;
> +
> +    table = init_aml_allocator();
> +    acpi_data_push(table->buf, sizeof(AcpiTableHeader));
> +
> +    /* scope for memory hotplug controller device node */
> +    assert(nr_mem <= ACPI_MAX_RAM_SLOTS);
> +    ctrl_dev = aml_scope("\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE));
> +    {
> +        Aml *crs, *field;
> +
> +        aml_append(ctrl_dev,
> +            aml_name_decl(stringify(MEMORY_SLOTS_NUMBER), aml_int(nr_mem))
> +        );
> +
> +        crs = aml_resource_template();
> +        aml_append(crs, aml_io(AML_DECODE16, io_base, io_base, 0, io_len));
> +        aml_append(ctrl_dev, aml_name_decl("_CRS", crs));
> +
> +        aml_append(ctrl_dev, aml_operation_region(
> +            stringify(MEMORY_HOTPLUG_IO_REGION), AML_SYSTEM_IO,
> +            io_base, io_len)
> +        );
> +
> +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
> +                          AML_PRESERVE);
> +        aml_append(field, /* read only */
> +            aml_named_field(stringify(MEMORY_SLOT_ADDR_LOW), 32));
> +        aml_append(field, /* read only */
> +            aml_named_field(stringify(MEMORY_SLOT_ADDR_HIGH), 32));
> +        aml_append(field, /* read only */
> +            aml_named_field(stringify(MEMORY_SLOT_SIZE_LOW), 32));
> +        aml_append(field, /* read only */
> +            aml_named_field(stringify(MEMORY_SLOT_SIZE_HIGH), 32));
> +        aml_append(field, /* read only */
> +            aml_named_field(stringify(MEMORY_SLOT_PROXIMITY), 32));
> +        aml_append(ctrl_dev, field);
> +
> +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_BYTE_ACC,
> +                          AML_WRITE_AS_ZEROS);
> +        aml_append(field, aml_reserved_field(160 /* bits, Offset(20) */));
> +        aml_append(field, /* 1 if enabled, read only */
> +            aml_named_field(stringify(MEMORY_SLOT_ENABLED), 1));
> +        aml_append(field,
> +            /*(read) 1 if has a insert event. (write) 1 to clear event */
> +            aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
> +        aml_append(field,
> +            /* (read) 1 if has a remove event. (write) 1 to clear event */
> +            aml_named_field(stringify(MEMORY_SLOT_REMOVE_EVENT), 1));
> +        aml_append(field,
> +            /* initiates device eject, write only */
> +            aml_named_field(stringify(MEMORY_SLOT_EJECT), 1));
> +        aml_append(ctrl_dev, field);
> +
> +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
> +                          AML_PRESERVE);
> +        aml_append(field, /* DIMM selector, write only */
> +            aml_named_field(stringify(MEMORY_SLOT_SLECTOR), 32));
> +        aml_append(field, /* _OST event code, write only */
> +            aml_named_field(stringify(MEMORY_SLOT_OST_EVENT), 32));
> +        aml_append(field, /* _OST status code, write only */
> +            aml_named_field(stringify(MEMORY_SLOT_OST_STATUS), 32));
> +        aml_append(ctrl_dev, field);
> +    }
> +    aml_append(table, ctrl_dev);
> +
> +    sb_scope = aml_scope("\\_SB");
> +    for (i = 0; i < nr_mem; i++) {
> +        const char *s;
> +
> +        dev = aml_device("MP%02X", i);
> +        aml_append(dev, aml_name_decl("_UID", aml_string("0x%02X", i)));
> +        aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C80")));
> +
> +        method = aml_method("_CRS", 0);
> +        s = BASEPATH stringify(MEMORY_SLOT_CRS_METHOD);
> +        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> +        aml_append(dev, method);
> +
> +        method = aml_method("_STA", 0);
> +        s = BASEPATH stringify(MEMORY_SLOT_STATUS_METHOD);
> +        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> +        aml_append(dev, method);
> +
> +        method = aml_method("_PXM", 0);
> +        s = BASEPATH stringify(MEMORY_SLOT_PROXIMITY_METHOD);
> +        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> +        aml_append(dev, method);
> +
> +        method = aml_method("_OST", 3);
> +        s = BASEPATH stringify(MEMORY_SLOT_OST_METHOD);
> +        aml_append(method, aml_return(aml_call4(
> +            s, aml_name("_UID"), aml_arg(0), aml_arg(1), aml_arg(2)
> +        )));
> +        aml_append(dev, method);
> +
> +        method = aml_method("_EJ0", 1);
> +        s = BASEPATH stringify(MEMORY_SLOT_EJECT_METHOD);
> +        aml_append(method, aml_return(aml_call2(
> +                   s, aml_name("_UID"), aml_arg(0))));
> +        aml_append(dev, method);
> +
> +        aml_append(sb_scope, dev);
> +    }
> +
> +    /* build Method(MEMORY_SLOT_NOTIFY_METHOD, 2) {
> +     *     If (LEqual(Arg0, 0x00)) {Notify(MP00, Arg1)} ... }
> +     */
> +    method = aml_method(stringify(MEMORY_SLOT_NOTIFY_METHOD), 2);
> +    for (i = 0; i < nr_mem; i++) {
> +        ifctx = aml_if(aml_equal(aml_arg(0), aml_int(i)));
> +        aml_append(ifctx,
> +            aml_notify(aml_name("MP%.02X", i), aml_arg(1))
> +        );
> +        aml_append(method, ifctx);
> +    }
> +    aml_append(sb_scope, method);
> +    aml_append(table, sb_scope);
> +
> +    /* copy AML table into ACPI tables blob and patch header there */
> +    g_array_append_vals(table_data, table->buf->data, table->buf->len);
> +    build_header(linker, table_data,
> +        (void *)(table_data->data + table_data->len - table->buf->len),
> +        "MHPT", table->buf->len, 2);
> +    free_aml_allocator();
> +}
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 95e0c65..8add4d9 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -925,6 +925,16 @@ build_ssdt(GArray *table_data, GArray *linker,
>      /* Reserve space for header */
>      acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
>  
> +    sb_scope = aml_scope("\\_SB");
> +    method = aml_method("_INI", 0);
> +    ifctx = aml_if(aml_lgreater_equal(aml_name("_REV"), aml_int(2)));
> +    if (nr_mem) {
> +        aml_append(ifctx, aml_load_table("MHPT"));
> +    }
> +    aml_append(method, ifctx);
> +    aml_append(sb_scope, method);
> +    aml_append(ssdt, sb_scope);
> +
>      /* Extra PCI root buses are implemented  only for i440fx */
>      bus = find_i440fx();
>      if (bus) {
> @@ -1200,119 +1210,6 @@ build_ssdt(GArray *table_data, GArray *linker,
>          }
>          aml_append(sb_scope, aml_name_decl("CPON", pkg));
>  
> -        /* build memory devices */
> -        assert(nr_mem <= ACPI_MAX_RAM_SLOTS);
> -        scope = aml_scope("\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE));
> -        aml_append(scope,
> -            aml_name_decl(stringify(MEMORY_SLOTS_NUMBER), aml_int(nr_mem))
> -        );
> -
> -        crs = aml_resource_template();
> -        aml_append(crs,
> -            aml_io(AML_DECODE16, pm->mem_hp_io_base, pm->mem_hp_io_base, 0,
> -                   pm->mem_hp_io_len)
> -        );
> -        aml_append(scope, aml_name_decl("_CRS", crs));
> -
> -        aml_append(scope, aml_operation_region(
> -            stringify(MEMORY_HOTPLUG_IO_REGION), AML_SYSTEM_IO,
> -            pm->mem_hp_io_base, pm->mem_hp_io_len)
> -        );
> -
> -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
> -                          AML_PRESERVE);
> -        aml_append(field, /* read only */
> -            aml_named_field(stringify(MEMORY_SLOT_ADDR_LOW), 32));
> -        aml_append(field, /* read only */
> -            aml_named_field(stringify(MEMORY_SLOT_ADDR_HIGH), 32));
> -        aml_append(field, /* read only */
> -            aml_named_field(stringify(MEMORY_SLOT_SIZE_LOW), 32));
> -        aml_append(field, /* read only */
> -            aml_named_field(stringify(MEMORY_SLOT_SIZE_HIGH), 32));
> -        aml_append(field, /* read only */
> -            aml_named_field(stringify(MEMORY_SLOT_PROXIMITY), 32));
> -        aml_append(scope, field);
> -
> -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_BYTE_ACC,
> -                          AML_WRITE_AS_ZEROS);
> -        aml_append(field, aml_reserved_field(160 /* bits, Offset(20) */));
> -        aml_append(field, /* 1 if enabled, read only */
> -            aml_named_field(stringify(MEMORY_SLOT_ENABLED), 1));
> -        aml_append(field,
> -            /*(read) 1 if has a insert event. (write) 1 to clear event */
> -            aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
> -        aml_append(field,
> -            /* (read) 1 if has a remove event. (write) 1 to clear event */
> -            aml_named_field(stringify(MEMORY_SLOT_REMOVE_EVENT), 1));
> -        aml_append(field,
> -            /* initiates device eject, write only */
> -            aml_named_field(stringify(MEMORY_SLOT_EJECT), 1));
> -        aml_append(scope, field);
> -
> -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
> -                          AML_PRESERVE);
> -        aml_append(field, /* DIMM selector, write only */
> -            aml_named_field(stringify(MEMORY_SLOT_SLECTOR), 32));
> -        aml_append(field, /* _OST event code, write only */
> -            aml_named_field(stringify(MEMORY_SLOT_OST_EVENT), 32));
> -        aml_append(field, /* _OST status code, write only */
> -            aml_named_field(stringify(MEMORY_SLOT_OST_STATUS), 32));
> -        aml_append(scope, field);
> -
> -        aml_append(sb_scope, scope);
> -
> -        for (i = 0; i < nr_mem; i++) {
> -            #define BASEPATH "\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE) "."
> -            const char *s;
> -
> -            dev = aml_device("MP%02X", i);
> -            aml_append(dev, aml_name_decl("_UID", aml_string("0x%02X", i)));
> -            aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C80")));
> -
> -            method = aml_method("_CRS", 0);
> -            s = BASEPATH stringify(MEMORY_SLOT_CRS_METHOD);
> -            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> -            aml_append(dev, method);
> -
> -            method = aml_method("_STA", 0);
> -            s = BASEPATH stringify(MEMORY_SLOT_STATUS_METHOD);
> -            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> -            aml_append(dev, method);
> -
> -            method = aml_method("_PXM", 0);
> -            s = BASEPATH stringify(MEMORY_SLOT_PROXIMITY_METHOD);
> -            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> -            aml_append(dev, method);
> -
> -            method = aml_method("_OST", 3);
> -            s = BASEPATH stringify(MEMORY_SLOT_OST_METHOD);
> -            aml_append(method, aml_return(aml_call4(
> -                s, aml_name("_UID"), aml_arg(0), aml_arg(1), aml_arg(2)
> -            )));
> -            aml_append(dev, method);
> -
> -            method = aml_method("_EJ0", 1);
> -            s = BASEPATH stringify(MEMORY_SLOT_EJECT_METHOD);
> -            aml_append(method, aml_return(aml_call2(
> -                       s, aml_name("_UID"), aml_arg(0))));
> -            aml_append(dev, method);
> -
> -            aml_append(sb_scope, dev);
> -        }
> -
> -        /* build Method(MEMORY_SLOT_NOTIFY_METHOD, 2) {
> -         *     If (LEqual(Arg0, 0x00)) {Notify(MP00, Arg1)} ... }
> -         */
> -        method = aml_method(stringify(MEMORY_SLOT_NOTIFY_METHOD), 2);
> -        for (i = 0; i < nr_mem; i++) {
> -            ifctx = aml_if(aml_equal(aml_arg(0), aml_int(i)));
> -            aml_append(ifctx,
> -                aml_notify(aml_name("MP%.02X", i), aml_arg(1))
> -            );
> -            aml_append(method, ifctx);
> -        }
> -        aml_append(sb_scope, method);
> -
>          {
>              Object *pci_host;
>              PCIBus *bus = NULL;
> @@ -1671,6 +1568,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>      uint8_t *u;
>      size_t aml_len = 0;
>      GArray *tables_blob = tables->table_data;
> +    MachineState *machine = MACHINE(qdev_get_machine());
>  
>      acpi_get_cpu_info(&cpu);
>      acpi_get_pm_info(&pm);
> @@ -1742,6 +1640,12 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>          build_dmar_q35(tables_blob, tables->linker);
>      }
>  
> +    if (machine->ram_slots) {
> +        acpi_add_table(table_offsets, tables_blob);
> +        build_mhpt(tables_blob, tables->linker, machine->ram_slots,
> +                   pm.mem_hp_io_base, pm.mem_hp_io_len);
> +    }
> +
>      /* Add tables supplied by user (if any) */
>      for (u = acpi_table_first(); u; u = acpi_table_next(u)) {
>          unsigned len = acpi_table_len(u);
> diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
> index 1342adb..5fd2854 100644
> --- a/include/hw/acpi/memory_hotplug.h
> +++ b/include/hw/acpi/memory_hotplug.h
> @@ -45,4 +45,7 @@ extern const VMStateDescription vmstate_memory_hotplug;
>                     vmstate_memory_hotplug, MemHotplugState)
>  
>  void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList ***list);
> +
> +void build_mhpt(GArray *table_data, GArray *linker, uint32_t nr_mem,
> +                uint16_t io_base, uint16_t io_len);
>  #endif
> -- 
> 1.8.3.1
Michael S. Tsirkin Oct. 24, 2015, 7:37 p.m. UTC | #3 
On Fri, Oct 23, 2015 at 04:57:09PM +0200, Igor Mammedov wrote:
> moves SSDT part to custom MHPT table, which is loaded
> at runtime by OSPM if it supports ACPIv2 revision and
> only if memory hotplug is enabled.
> That should reduce ACPI tables blob size if memory
> hotplug is not enabled (default case).
> 
> Checked for compatibility issues with:
>  * Windows XPsp3, Windows Server 2003: they don't load
>    the table as OSPM only reports revision 1 as supported.
>    And we don't care about memhp for these guests as they
>    do not support it anyway.
>  * Windows Server 2008, Windows Server 2008R2:
>    works as expected
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/acpi/Makefile.objs               |   2 +-
>  hw/acpi/memory_hotplug_acpi_table.c | 138 ++++++++++++++++++++++++++++++++++++
>  hw/i386/acpi-build.c                | 130 +++++----------------------------
>  include/hw/acpi/memory_hotplug.h    |   3 +
>  4 files changed, 159 insertions(+), 114 deletions(-)
>  create mode 100644 hw/acpi/memory_hotplug_acpi_table.c
> 
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index 7d3230c..c04064e 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -1,7 +1,7 @@
>  common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
>  common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
>  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
> -common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> +common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
>  common-obj-$(CONFIG_ACPI) += acpi_interface.o
>  common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
>  common-obj-$(CONFIG_ACPI) += aml-build.o
> diff --git a/hw/acpi/memory_hotplug_acpi_table.c b/hw/acpi/memory_hotplug_acpi_table.c
> new file mode 100644
> index 0000000..28da13c

This diff is pretty hard to read. Isn't there some way
you could refactor this to make the diff smaller?
E.g. move the part that isn't changed into
a function first, then call it from a different place.

> --- /dev/null
> +++ b/hw/acpi/memory_hotplug_acpi_table.c
> @@ -0,0 +1,138 @@
> +#include <stdbool.h>
> +#include "hw/acpi/aml-build.h"
> +#include "hw/acpi/memory_hotplug.h"
> +#include "include/hw/acpi/pc-hotplug.h"
> +#include "hw/boards.h"
> +
> +#define BASEPATH "\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE) "."
> +
> +void build_mhpt(GArray *table_data, GArray *linker, uint32_t nr_mem,
> +                uint16_t io_base, uint16_t io_len)
> +{
> +    int i;
> +    Aml *table, *sb_scope, *dev, *method, *ifctx, *ctrl_dev;
> +
> +    table = init_aml_allocator();
> +    acpi_data_push(table->buf, sizeof(AcpiTableHeader));
> +
> +    /* scope for memory hotplug controller device node */
> +    assert(nr_mem <= ACPI_MAX_RAM_SLOTS);
> +    ctrl_dev = aml_scope("\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE));
> +    {
> +        Aml *crs, *field;
> +
> +        aml_append(ctrl_dev,
> +            aml_name_decl(stringify(MEMORY_SLOTS_NUMBER), aml_int(nr_mem))
> +        );
> +
> +        crs = aml_resource_template();
> +        aml_append(crs, aml_io(AML_DECODE16, io_base, io_base, 0, io_len));
> +        aml_append(ctrl_dev, aml_name_decl("_CRS", crs));
> +
> +        aml_append(ctrl_dev, aml_operation_region(
> +            stringify(MEMORY_HOTPLUG_IO_REGION), AML_SYSTEM_IO,
> +            io_base, io_len)
> +        );
> +
> +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
> +                          AML_PRESERVE);
> +        aml_append(field, /* read only */
> +            aml_named_field(stringify(MEMORY_SLOT_ADDR_LOW), 32));
> +        aml_append(field, /* read only */
> +            aml_named_field(stringify(MEMORY_SLOT_ADDR_HIGH), 32));
> +        aml_append(field, /* read only */
> +            aml_named_field(stringify(MEMORY_SLOT_SIZE_LOW), 32));
> +        aml_append(field, /* read only */
> +            aml_named_field(stringify(MEMORY_SLOT_SIZE_HIGH), 32));
> +        aml_append(field, /* read only */
> +            aml_named_field(stringify(MEMORY_SLOT_PROXIMITY), 32));
> +        aml_append(ctrl_dev, field);
> +
> +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_BYTE_ACC,
> +                          AML_WRITE_AS_ZEROS);
> +        aml_append(field, aml_reserved_field(160 /* bits, Offset(20) */));
> +        aml_append(field, /* 1 if enabled, read only */
> +            aml_named_field(stringify(MEMORY_SLOT_ENABLED), 1));
> +        aml_append(field,
> +            /*(read) 1 if has a insert event. (write) 1 to clear event */
> +            aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
> +        aml_append(field,
> +            /* (read) 1 if has a remove event. (write) 1 to clear event */
> +            aml_named_field(stringify(MEMORY_SLOT_REMOVE_EVENT), 1));
> +        aml_append(field,
> +            /* initiates device eject, write only */
> +            aml_named_field(stringify(MEMORY_SLOT_EJECT), 1));
> +        aml_append(ctrl_dev, field);
> +
> +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
> +                          AML_PRESERVE);
> +        aml_append(field, /* DIMM selector, write only */
> +            aml_named_field(stringify(MEMORY_SLOT_SLECTOR), 32));
> +        aml_append(field, /* _OST event code, write only */
> +            aml_named_field(stringify(MEMORY_SLOT_OST_EVENT), 32));
> +        aml_append(field, /* _OST status code, write only */
> +            aml_named_field(stringify(MEMORY_SLOT_OST_STATUS), 32));
> +        aml_append(ctrl_dev, field);
> +    }
> +    aml_append(table, ctrl_dev);
> +
> +    sb_scope = aml_scope("\\_SB");
> +    for (i = 0; i < nr_mem; i++) {
> +        const char *s;
> +
> +        dev = aml_device("MP%02X", i);
> +        aml_append(dev, aml_name_decl("_UID", aml_string("0x%02X", i)));
> +        aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C80")));
> +
> +        method = aml_method("_CRS", 0);
> +        s = BASEPATH stringify(MEMORY_SLOT_CRS_METHOD);
> +        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> +        aml_append(dev, method);
> +
> +        method = aml_method("_STA", 0);
> +        s = BASEPATH stringify(MEMORY_SLOT_STATUS_METHOD);
> +        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> +        aml_append(dev, method);
> +
> +        method = aml_method("_PXM", 0);
> +        s = BASEPATH stringify(MEMORY_SLOT_PROXIMITY_METHOD);
> +        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> +        aml_append(dev, method);
> +
> +        method = aml_method("_OST", 3);
> +        s = BASEPATH stringify(MEMORY_SLOT_OST_METHOD);
> +        aml_append(method, aml_return(aml_call4(
> +            s, aml_name("_UID"), aml_arg(0), aml_arg(1), aml_arg(2)
> +        )));
> +        aml_append(dev, method);
> +
> +        method = aml_method("_EJ0", 1);
> +        s = BASEPATH stringify(MEMORY_SLOT_EJECT_METHOD);
> +        aml_append(method, aml_return(aml_call2(
> +                   s, aml_name("_UID"), aml_arg(0))));
> +        aml_append(dev, method);
> +
> +        aml_append(sb_scope, dev);
> +    }
> +
> +    /* build Method(MEMORY_SLOT_NOTIFY_METHOD, 2) {
> +     *     If (LEqual(Arg0, 0x00)) {Notify(MP00, Arg1)} ... }
> +     */
> +    method = aml_method(stringify(MEMORY_SLOT_NOTIFY_METHOD), 2);
> +    for (i = 0; i < nr_mem; i++) {
> +        ifctx = aml_if(aml_equal(aml_arg(0), aml_int(i)));
> +        aml_append(ifctx,
> +            aml_notify(aml_name("MP%.02X", i), aml_arg(1))
> +        );
> +        aml_append(method, ifctx);
> +    }
> +    aml_append(sb_scope, method);
> +    aml_append(table, sb_scope);
> +
> +    /* copy AML table into ACPI tables blob and patch header there */
> +    g_array_append_vals(table_data, table->buf->data, table->buf->len);
> +    build_header(linker, table_data,
> +        (void *)(table_data->data + table_data->len - table->buf->len),
> +        "MHPT", table->buf->len, 2);
> +    free_aml_allocator();
> +}
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 95e0c65..8add4d9 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -925,6 +925,16 @@ build_ssdt(GArray *table_data, GArray *linker,
>      /* Reserve space for header */
>      acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
>  
> +    sb_scope = aml_scope("\\_SB");
> +    method = aml_method("_INI", 0);
> +    ifctx = aml_if(aml_lgreater_equal(aml_name("_REV"), aml_int(2)));
> +    if (nr_mem) {
> +        aml_append(ifctx, aml_load_table("MHPT"));
> +    }
> +    aml_append(method, ifctx);
> +    aml_append(sb_scope, method);
> +    aml_append(ssdt, sb_scope);
> +
>      /* Extra PCI root buses are implemented  only for i440fx */
>      bus = find_i440fx();
>      if (bus) {
> @@ -1200,119 +1210,6 @@ build_ssdt(GArray *table_data, GArray *linker,
>          }
>          aml_append(sb_scope, aml_name_decl("CPON", pkg));
>  
> -        /* build memory devices */
> -        assert(nr_mem <= ACPI_MAX_RAM_SLOTS);
> -        scope = aml_scope("\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE));
> -        aml_append(scope,
> -            aml_name_decl(stringify(MEMORY_SLOTS_NUMBER), aml_int(nr_mem))
> -        );
> -
> -        crs = aml_resource_template();
> -        aml_append(crs,
> -            aml_io(AML_DECODE16, pm->mem_hp_io_base, pm->mem_hp_io_base, 0,
> -                   pm->mem_hp_io_len)
> -        );
> -        aml_append(scope, aml_name_decl("_CRS", crs));
> -
> -        aml_append(scope, aml_operation_region(
> -            stringify(MEMORY_HOTPLUG_IO_REGION), AML_SYSTEM_IO,
> -            pm->mem_hp_io_base, pm->mem_hp_io_len)
> -        );
> -
> -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
> -                          AML_PRESERVE);
> -        aml_append(field, /* read only */
> -            aml_named_field(stringify(MEMORY_SLOT_ADDR_LOW), 32));
> -        aml_append(field, /* read only */
> -            aml_named_field(stringify(MEMORY_SLOT_ADDR_HIGH), 32));
> -        aml_append(field, /* read only */
> -            aml_named_field(stringify(MEMORY_SLOT_SIZE_LOW), 32));
> -        aml_append(field, /* read only */
> -            aml_named_field(stringify(MEMORY_SLOT_SIZE_HIGH), 32));
> -        aml_append(field, /* read only */
> -            aml_named_field(stringify(MEMORY_SLOT_PROXIMITY), 32));
> -        aml_append(scope, field);
> -
> -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_BYTE_ACC,
> -                          AML_WRITE_AS_ZEROS);
> -        aml_append(field, aml_reserved_field(160 /* bits, Offset(20) */));
> -        aml_append(field, /* 1 if enabled, read only */
> -            aml_named_field(stringify(MEMORY_SLOT_ENABLED), 1));
> -        aml_append(field,
> -            /*(read) 1 if has a insert event. (write) 1 to clear event */
> -            aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
> -        aml_append(field,
> -            /* (read) 1 if has a remove event. (write) 1 to clear event */
> -            aml_named_field(stringify(MEMORY_SLOT_REMOVE_EVENT), 1));
> -        aml_append(field,
> -            /* initiates device eject, write only */
> -            aml_named_field(stringify(MEMORY_SLOT_EJECT), 1));
> -        aml_append(scope, field);
> -
> -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
> -                          AML_PRESERVE);
> -        aml_append(field, /* DIMM selector, write only */
> -            aml_named_field(stringify(MEMORY_SLOT_SLECTOR), 32));
> -        aml_append(field, /* _OST event code, write only */
> -            aml_named_field(stringify(MEMORY_SLOT_OST_EVENT), 32));
> -        aml_append(field, /* _OST status code, write only */
> -            aml_named_field(stringify(MEMORY_SLOT_OST_STATUS), 32));
> -        aml_append(scope, field);
> -
> -        aml_append(sb_scope, scope);
> -
> -        for (i = 0; i < nr_mem; i++) {
> -            #define BASEPATH "\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE) "."
> -            const char *s;
> -
> -            dev = aml_device("MP%02X", i);
> -            aml_append(dev, aml_name_decl("_UID", aml_string("0x%02X", i)));
> -            aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C80")));
> -
> -            method = aml_method("_CRS", 0);
> -            s = BASEPATH stringify(MEMORY_SLOT_CRS_METHOD);
> -            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> -            aml_append(dev, method);
> -
> -            method = aml_method("_STA", 0);
> -            s = BASEPATH stringify(MEMORY_SLOT_STATUS_METHOD);
> -            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> -            aml_append(dev, method);
> -
> -            method = aml_method("_PXM", 0);
> -            s = BASEPATH stringify(MEMORY_SLOT_PROXIMITY_METHOD);
> -            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> -            aml_append(dev, method);
> -
> -            method = aml_method("_OST", 3);
> -            s = BASEPATH stringify(MEMORY_SLOT_OST_METHOD);
> -            aml_append(method, aml_return(aml_call4(
> -                s, aml_name("_UID"), aml_arg(0), aml_arg(1), aml_arg(2)
> -            )));
> -            aml_append(dev, method);
> -
> -            method = aml_method("_EJ0", 1);
> -            s = BASEPATH stringify(MEMORY_SLOT_EJECT_METHOD);
> -            aml_append(method, aml_return(aml_call2(
> -                       s, aml_name("_UID"), aml_arg(0))));
> -            aml_append(dev, method);
> -
> -            aml_append(sb_scope, dev);
> -        }
> -
> -        /* build Method(MEMORY_SLOT_NOTIFY_METHOD, 2) {
> -         *     If (LEqual(Arg0, 0x00)) {Notify(MP00, Arg1)} ... }
> -         */
> -        method = aml_method(stringify(MEMORY_SLOT_NOTIFY_METHOD), 2);
> -        for (i = 0; i < nr_mem; i++) {
> -            ifctx = aml_if(aml_equal(aml_arg(0), aml_int(i)));
> -            aml_append(ifctx,
> -                aml_notify(aml_name("MP%.02X", i), aml_arg(1))
> -            );
> -            aml_append(method, ifctx);
> -        }
> -        aml_append(sb_scope, method);
> -
>          {
>              Object *pci_host;
>              PCIBus *bus = NULL;
> @@ -1671,6 +1568,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>      uint8_t *u;
>      size_t aml_len = 0;
>      GArray *tables_blob = tables->table_data;
> +    MachineState *machine = MACHINE(qdev_get_machine());
>  
>      acpi_get_cpu_info(&cpu);
>      acpi_get_pm_info(&pm);
> @@ -1742,6 +1640,12 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>          build_dmar_q35(tables_blob, tables->linker);
>      }
>  
> +    if (machine->ram_slots) {
> +        acpi_add_table(table_offsets, tables_blob);
> +        build_mhpt(tables_blob, tables->linker, machine->ram_slots,
> +                   pm.mem_hp_io_base, pm.mem_hp_io_len);
> +    }
> +
>      /* Add tables supplied by user (if any) */
>      for (u = acpi_table_first(); u; u = acpi_table_next(u)) {
>          unsigned len = acpi_table_len(u);
> diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
> index 1342adb..5fd2854 100644
> --- a/include/hw/acpi/memory_hotplug.h
> +++ b/include/hw/acpi/memory_hotplug.h
> @@ -45,4 +45,7 @@ extern const VMStateDescription vmstate_memory_hotplug;
>                     vmstate_memory_hotplug, MemHotplugState)
>  
>  void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList ***list);
> +
> +void build_mhpt(GArray *table_data, GArray *linker, uint32_t nr_mem,
> +                uint16_t io_base, uint16_t io_len);
>  #endif
> -- 
> 1.8.3.1
Laszlo Ersek Oct. 25, 2015, 1:33 p.m. UTC | #4 
On 10/24/15 19:59, Michael S. Tsirkin wrote:
> On Fri, Oct 23, 2015 at 04:57:09PM +0200, Igor Mammedov wrote:
>> moves SSDT part to custom MHPT table, which is loaded
>> at runtime by OSPM if it supports ACPIv2 revision and
>> only if memory hotplug is enabled.
>> That should reduce ACPI tables blob size if memory
>> hotplug is not enabled (default case).
> 
> With seabios, it's still in reserved memory. How does it help?
> 
> And this trick likely breaks UEFI.

It does; see my response to the blurb.

In any case, I might send an email to the USWG list because I think
better cooperation is needed between the ACPI and UEFI specs for
supporting LoadTable and DataTableRegion. In particular,
EFI_ACPI_TABLE2_PROTOCOL.InstallTable() should take the EFI memory type
to install the table in. Luckily these two specs belong to the same
organization now.

Thanks
Laszlo

> I just checked, the only legal way to specify OEM
> specific tables seems to be using OEMX prefix.
> 
> But there's a decent chance this will conflict with
> people using command line flags to load their own
> tables.
> 
> This likely means this trick is off.
> 
> 
> 
>> Checked for compatibility issues with:
>>  * Windows XPsp3, Windows Server 2003: they don't load
>>    the table as OSPM only reports revision 1 as supported.
>>    And we don't care about memhp for these guests as they
>>    do not support it anyway.
>>  * Windows Server 2008, Windows Server 2008R2:
>>    works as expected
>>
>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> ---
>>  hw/acpi/Makefile.objs               |   2 +-
>>  hw/acpi/memory_hotplug_acpi_table.c | 138 ++++++++++++++++++++++++++++++++++++
>>  hw/i386/acpi-build.c                | 130 +++++----------------------------
>>  include/hw/acpi/memory_hotplug.h    |   3 +
>>  4 files changed, 159 insertions(+), 114 deletions(-)
>>  create mode 100644 hw/acpi/memory_hotplug_acpi_table.c
>>
>> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
>> index 7d3230c..c04064e 100644
>> --- a/hw/acpi/Makefile.objs
>> +++ b/hw/acpi/Makefile.objs
>> @@ -1,7 +1,7 @@
>>  common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
>>  common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
>>  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
>> -common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
>> +common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
>>  common-obj-$(CONFIG_ACPI) += acpi_interface.o
>>  common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
>>  common-obj-$(CONFIG_ACPI) += aml-build.o
>> diff --git a/hw/acpi/memory_hotplug_acpi_table.c b/hw/acpi/memory_hotplug_acpi_table.c
>> new file mode 100644
>> index 0000000..28da13c
>> --- /dev/null
>> +++ b/hw/acpi/memory_hotplug_acpi_table.c
>> @@ -0,0 +1,138 @@
>> +#include <stdbool.h>
>> +#include "hw/acpi/aml-build.h"
>> +#include "hw/acpi/memory_hotplug.h"
>> +#include "include/hw/acpi/pc-hotplug.h"
>> +#include "hw/boards.h"
>> +
>> +#define BASEPATH "\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE) "."
>> +
>> +void build_mhpt(GArray *table_data, GArray *linker, uint32_t nr_mem,
>> +                uint16_t io_base, uint16_t io_len)
>> +{
>> +    int i;
>> +    Aml *table, *sb_scope, *dev, *method, *ifctx, *ctrl_dev;
>> +
>> +    table = init_aml_allocator();
>> +    acpi_data_push(table->buf, sizeof(AcpiTableHeader));
>> +
>> +    /* scope for memory hotplug controller device node */
>> +    assert(nr_mem <= ACPI_MAX_RAM_SLOTS);
>> +    ctrl_dev = aml_scope("\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE));
>> +    {
>> +        Aml *crs, *field;
>> +
>> +        aml_append(ctrl_dev,
>> +            aml_name_decl(stringify(MEMORY_SLOTS_NUMBER), aml_int(nr_mem))
>> +        );
>> +
>> +        crs = aml_resource_template();
>> +        aml_append(crs, aml_io(AML_DECODE16, io_base, io_base, 0, io_len));
>> +        aml_append(ctrl_dev, aml_name_decl("_CRS", crs));
>> +
>> +        aml_append(ctrl_dev, aml_operation_region(
>> +            stringify(MEMORY_HOTPLUG_IO_REGION), AML_SYSTEM_IO,
>> +            io_base, io_len)
>> +        );
>> +
>> +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
>> +                          AML_PRESERVE);
>> +        aml_append(field, /* read only */
>> +            aml_named_field(stringify(MEMORY_SLOT_ADDR_LOW), 32));
>> +        aml_append(field, /* read only */
>> +            aml_named_field(stringify(MEMORY_SLOT_ADDR_HIGH), 32));
>> +        aml_append(field, /* read only */
>> +            aml_named_field(stringify(MEMORY_SLOT_SIZE_LOW), 32));
>> +        aml_append(field, /* read only */
>> +            aml_named_field(stringify(MEMORY_SLOT_SIZE_HIGH), 32));
>> +        aml_append(field, /* read only */
>> +            aml_named_field(stringify(MEMORY_SLOT_PROXIMITY), 32));
>> +        aml_append(ctrl_dev, field);
>> +
>> +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_BYTE_ACC,
>> +                          AML_WRITE_AS_ZEROS);
>> +        aml_append(field, aml_reserved_field(160 /* bits, Offset(20) */));
>> +        aml_append(field, /* 1 if enabled, read only */
>> +            aml_named_field(stringify(MEMORY_SLOT_ENABLED), 1));
>> +        aml_append(field,
>> +            /*(read) 1 if has a insert event. (write) 1 to clear event */
>> +            aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
>> +        aml_append(field,
>> +            /* (read) 1 if has a remove event. (write) 1 to clear event */
>> +            aml_named_field(stringify(MEMORY_SLOT_REMOVE_EVENT), 1));
>> +        aml_append(field,
>> +            /* initiates device eject, write only */
>> +            aml_named_field(stringify(MEMORY_SLOT_EJECT), 1));
>> +        aml_append(ctrl_dev, field);
>> +
>> +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
>> +                          AML_PRESERVE);
>> +        aml_append(field, /* DIMM selector, write only */
>> +            aml_named_field(stringify(MEMORY_SLOT_SLECTOR), 32));
>> +        aml_append(field, /* _OST event code, write only */
>> +            aml_named_field(stringify(MEMORY_SLOT_OST_EVENT), 32));
>> +        aml_append(field, /* _OST status code, write only */
>> +            aml_named_field(stringify(MEMORY_SLOT_OST_STATUS), 32));
>> +        aml_append(ctrl_dev, field);
>> +    }
>> +    aml_append(table, ctrl_dev);
>> +
>> +    sb_scope = aml_scope("\\_SB");
>> +    for (i = 0; i < nr_mem; i++) {
>> +        const char *s;
>> +
>> +        dev = aml_device("MP%02X", i);
>> +        aml_append(dev, aml_name_decl("_UID", aml_string("0x%02X", i)));
>> +        aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C80")));
>> +
>> +        method = aml_method("_CRS", 0);
>> +        s = BASEPATH stringify(MEMORY_SLOT_CRS_METHOD);
>> +        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
>> +        aml_append(dev, method);
>> +
>> +        method = aml_method("_STA", 0);
>> +        s = BASEPATH stringify(MEMORY_SLOT_STATUS_METHOD);
>> +        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
>> +        aml_append(dev, method);
>> +
>> +        method = aml_method("_PXM", 0);
>> +        s = BASEPATH stringify(MEMORY_SLOT_PROXIMITY_METHOD);
>> +        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
>> +        aml_append(dev, method);
>> +
>> +        method = aml_method("_OST", 3);
>> +        s = BASEPATH stringify(MEMORY_SLOT_OST_METHOD);
>> +        aml_append(method, aml_return(aml_call4(
>> +            s, aml_name("_UID"), aml_arg(0), aml_arg(1), aml_arg(2)
>> +        )));
>> +        aml_append(dev, method);
>> +
>> +        method = aml_method("_EJ0", 1);
>> +        s = BASEPATH stringify(MEMORY_SLOT_EJECT_METHOD);
>> +        aml_append(method, aml_return(aml_call2(
>> +                   s, aml_name("_UID"), aml_arg(0))));
>> +        aml_append(dev, method);
>> +
>> +        aml_append(sb_scope, dev);
>> +    }
>> +
>> +    /* build Method(MEMORY_SLOT_NOTIFY_METHOD, 2) {
>> +     *     If (LEqual(Arg0, 0x00)) {Notify(MP00, Arg1)} ... }
>> +     */
>> +    method = aml_method(stringify(MEMORY_SLOT_NOTIFY_METHOD), 2);
>> +    for (i = 0; i < nr_mem; i++) {
>> +        ifctx = aml_if(aml_equal(aml_arg(0), aml_int(i)));
>> +        aml_append(ifctx,
>> +            aml_notify(aml_name("MP%.02X", i), aml_arg(1))
>> +        );
>> +        aml_append(method, ifctx);
>> +    }
>> +    aml_append(sb_scope, method);
>> +    aml_append(table, sb_scope);
>> +
>> +    /* copy AML table into ACPI tables blob and patch header there */
>> +    g_array_append_vals(table_data, table->buf->data, table->buf->len);
>> +    build_header(linker, table_data,
>> +        (void *)(table_data->data + table_data->len - table->buf->len),
>> +        "MHPT", table->buf->len, 2);
>> +    free_aml_allocator();
>> +}
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 95e0c65..8add4d9 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -925,6 +925,16 @@ build_ssdt(GArray *table_data, GArray *linker,
>>      /* Reserve space for header */
>>      acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
>>  
>> +    sb_scope = aml_scope("\\_SB");
>> +    method = aml_method("_INI", 0);
>> +    ifctx = aml_if(aml_lgreater_equal(aml_name("_REV"), aml_int(2)));
>> +    if (nr_mem) {
>> +        aml_append(ifctx, aml_load_table("MHPT"));
>> +    }
>> +    aml_append(method, ifctx);
>> +    aml_append(sb_scope, method);
>> +    aml_append(ssdt, sb_scope);
>> +
>>      /* Extra PCI root buses are implemented  only for i440fx */
>>      bus = find_i440fx();
>>      if (bus) {
>> @@ -1200,119 +1210,6 @@ build_ssdt(GArray *table_data, GArray *linker,
>>          }
>>          aml_append(sb_scope, aml_name_decl("CPON", pkg));
>>  
>> -        /* build memory devices */
>> -        assert(nr_mem <= ACPI_MAX_RAM_SLOTS);
>> -        scope = aml_scope("\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE));
>> -        aml_append(scope,
>> -            aml_name_decl(stringify(MEMORY_SLOTS_NUMBER), aml_int(nr_mem))
>> -        );
>> -
>> -        crs = aml_resource_template();
>> -        aml_append(crs,
>> -            aml_io(AML_DECODE16, pm->mem_hp_io_base, pm->mem_hp_io_base, 0,
>> -                   pm->mem_hp_io_len)
>> -        );
>> -        aml_append(scope, aml_name_decl("_CRS", crs));
>> -
>> -        aml_append(scope, aml_operation_region(
>> -            stringify(MEMORY_HOTPLUG_IO_REGION), AML_SYSTEM_IO,
>> -            pm->mem_hp_io_base, pm->mem_hp_io_len)
>> -        );
>> -
>> -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
>> -                          AML_PRESERVE);
>> -        aml_append(field, /* read only */
>> -            aml_named_field(stringify(MEMORY_SLOT_ADDR_LOW), 32));
>> -        aml_append(field, /* read only */
>> -            aml_named_field(stringify(MEMORY_SLOT_ADDR_HIGH), 32));
>> -        aml_append(field, /* read only */
>> -            aml_named_field(stringify(MEMORY_SLOT_SIZE_LOW), 32));
>> -        aml_append(field, /* read only */
>> -            aml_named_field(stringify(MEMORY_SLOT_SIZE_HIGH), 32));
>> -        aml_append(field, /* read only */
>> -            aml_named_field(stringify(MEMORY_SLOT_PROXIMITY), 32));
>> -        aml_append(scope, field);
>> -
>> -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_BYTE_ACC,
>> -                          AML_WRITE_AS_ZEROS);
>> -        aml_append(field, aml_reserved_field(160 /* bits, Offset(20) */));
>> -        aml_append(field, /* 1 if enabled, read only */
>> -            aml_named_field(stringify(MEMORY_SLOT_ENABLED), 1));
>> -        aml_append(field,
>> -            /*(read) 1 if has a insert event. (write) 1 to clear event */
>> -            aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
>> -        aml_append(field,
>> -            /* (read) 1 if has a remove event. (write) 1 to clear event */
>> -            aml_named_field(stringify(MEMORY_SLOT_REMOVE_EVENT), 1));
>> -        aml_append(field,
>> -            /* initiates device eject, write only */
>> -            aml_named_field(stringify(MEMORY_SLOT_EJECT), 1));
>> -        aml_append(scope, field);
>> -
>> -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
>> -                          AML_PRESERVE);
>> -        aml_append(field, /* DIMM selector, write only */
>> -            aml_named_field(stringify(MEMORY_SLOT_SLECTOR), 32));
>> -        aml_append(field, /* _OST event code, write only */
>> -            aml_named_field(stringify(MEMORY_SLOT_OST_EVENT), 32));
>> -        aml_append(field, /* _OST status code, write only */
>> -            aml_named_field(stringify(MEMORY_SLOT_OST_STATUS), 32));
>> -        aml_append(scope, field);
>> -
>> -        aml_append(sb_scope, scope);
>> -
>> -        for (i = 0; i < nr_mem; i++) {
>> -            #define BASEPATH "\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE) "."
>> -            const char *s;
>> -
>> -            dev = aml_device("MP%02X", i);
>> -            aml_append(dev, aml_name_decl("_UID", aml_string("0x%02X", i)));
>> -            aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C80")));
>> -
>> -            method = aml_method("_CRS", 0);
>> -            s = BASEPATH stringify(MEMORY_SLOT_CRS_METHOD);
>> -            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
>> -            aml_append(dev, method);
>> -
>> -            method = aml_method("_STA", 0);
>> -            s = BASEPATH stringify(MEMORY_SLOT_STATUS_METHOD);
>> -            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
>> -            aml_append(dev, method);
>> -
>> -            method = aml_method("_PXM", 0);
>> -            s = BASEPATH stringify(MEMORY_SLOT_PROXIMITY_METHOD);
>> -            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
>> -            aml_append(dev, method);
>> -
>> -            method = aml_method("_OST", 3);
>> -            s = BASEPATH stringify(MEMORY_SLOT_OST_METHOD);
>> -            aml_append(method, aml_return(aml_call4(
>> -                s, aml_name("_UID"), aml_arg(0), aml_arg(1), aml_arg(2)
>> -            )));
>> -            aml_append(dev, method);
>> -
>> -            method = aml_method("_EJ0", 1);
>> -            s = BASEPATH stringify(MEMORY_SLOT_EJECT_METHOD);
>> -            aml_append(method, aml_return(aml_call2(
>> -                       s, aml_name("_UID"), aml_arg(0))));
>> -            aml_append(dev, method);
>> -
>> -            aml_append(sb_scope, dev);
>> -        }
>> -
>> -        /* build Method(MEMORY_SLOT_NOTIFY_METHOD, 2) {
>> -         *     If (LEqual(Arg0, 0x00)) {Notify(MP00, Arg1)} ... }
>> -         */
>> -        method = aml_method(stringify(MEMORY_SLOT_NOTIFY_METHOD), 2);
>> -        for (i = 0; i < nr_mem; i++) {
>> -            ifctx = aml_if(aml_equal(aml_arg(0), aml_int(i)));
>> -            aml_append(ifctx,
>> -                aml_notify(aml_name("MP%.02X", i), aml_arg(1))
>> -            );
>> -            aml_append(method, ifctx);
>> -        }
>> -        aml_append(sb_scope, method);
>> -
>>          {
>>              Object *pci_host;
>>              PCIBus *bus = NULL;
>> @@ -1671,6 +1568,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>>      uint8_t *u;
>>      size_t aml_len = 0;
>>      GArray *tables_blob = tables->table_data;
>> +    MachineState *machine = MACHINE(qdev_get_machine());
>>  
>>      acpi_get_cpu_info(&cpu);
>>      acpi_get_pm_info(&pm);
>> @@ -1742,6 +1640,12 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>>          build_dmar_q35(tables_blob, tables->linker);
>>      }
>>  
>> +    if (machine->ram_slots) {
>> +        acpi_add_table(table_offsets, tables_blob);
>> +        build_mhpt(tables_blob, tables->linker, machine->ram_slots,
>> +                   pm.mem_hp_io_base, pm.mem_hp_io_len);
>> +    }
>> +
>>      /* Add tables supplied by user (if any) */
>>      for (u = acpi_table_first(); u; u = acpi_table_next(u)) {
>>          unsigned len = acpi_table_len(u);
>> diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
>> index 1342adb..5fd2854 100644
>> --- a/include/hw/acpi/memory_hotplug.h
>> +++ b/include/hw/acpi/memory_hotplug.h
>> @@ -45,4 +45,7 @@ extern const VMStateDescription vmstate_memory_hotplug;
>>                     vmstate_memory_hotplug, MemHotplugState)
>>  
>>  void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList ***list);
>> +
>> +void build_mhpt(GArray *table_data, GArray *linker, uint32_t nr_mem,
>> +                uint16_t io_base, uint16_t io_len);
>>  #endif
>> -- 
>> 1.8.3.1
Igor Mammedov Oct. 26, 2015, 1:36 p.m. UTC | #5 
On Sat, 24 Oct 2015 20:59:07 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Fri, Oct 23, 2015 at 04:57:09PM +0200, Igor Mammedov wrote:
> > moves SSDT part to custom MHPT table, which is loaded
> > at runtime by OSPM if it supports ACPIv2 revision and
> > only if memory hotplug is enabled.
> > That should reduce ACPI tables blob size if memory
> > hotplug is not enabled (default case).
> 
> With seabios, it's still in reserved memory. How does it help?
> 
> And this trick likely breaks UEFI.
> 
> I just checked, the only legal way to specify OEM
> specific tables seems to be using OEMX prefix.
> 
> But there's a decent chance this will conflict with
> people using command line flags to load their own
> tables.
> 
> This likely means this trick is off.
Ok, I'll drop LoadTable and keep it in SSDT, there is no
hard req to use a separate table since all memhp code is
rev1.0 compatible for now and even when it's switched to
64-bit math it shouldn't affect XP unless someone would
try hotplug memory on it.

> 
> 
> 
> > Checked for compatibility issues with:
> >  * Windows XPsp3, Windows Server 2003: they don't load
> >    the table as OSPM only reports revision 1 as supported.
> >    And we don't care about memhp for these guests as they
> >    do not support it anyway.
> >  * Windows Server 2008, Windows Server 2008R2:
> >    works as expected
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  hw/acpi/Makefile.objs               |   2 +-
> >  hw/acpi/memory_hotplug_acpi_table.c | 138 ++++++++++++++++++++++++++++++++++++
> >  hw/i386/acpi-build.c                | 130 +++++----------------------------
> >  include/hw/acpi/memory_hotplug.h    |   3 +
> >  4 files changed, 159 insertions(+), 114 deletions(-)
> >  create mode 100644 hw/acpi/memory_hotplug_acpi_table.c
> > 
> > diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> > index 7d3230c..c04064e 100644
> > --- a/hw/acpi/Makefile.objs
> > +++ b/hw/acpi/Makefile.objs
> > @@ -1,7 +1,7 @@
> >  common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
> >  common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
> >  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
> > -common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> > +common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
> >  common-obj-$(CONFIG_ACPI) += acpi_interface.o
> >  common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
> >  common-obj-$(CONFIG_ACPI) += aml-build.o
> > diff --git a/hw/acpi/memory_hotplug_acpi_table.c b/hw/acpi/memory_hotplug_acpi_table.c
> > new file mode 100644
> > index 0000000..28da13c
> > --- /dev/null
> > +++ b/hw/acpi/memory_hotplug_acpi_table.c
> > @@ -0,0 +1,138 @@
> > +#include <stdbool.h>
> > +#include "hw/acpi/aml-build.h"
> > +#include "hw/acpi/memory_hotplug.h"
> > +#include "include/hw/acpi/pc-hotplug.h"
> > +#include "hw/boards.h"
> > +
> > +#define BASEPATH "\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE) "."
> > +
> > +void build_mhpt(GArray *table_data, GArray *linker, uint32_t nr_mem,
> > +                uint16_t io_base, uint16_t io_len)
> > +{
> > +    int i;
> > +    Aml *table, *sb_scope, *dev, *method, *ifctx, *ctrl_dev;
> > +
> > +    table = init_aml_allocator();
> > +    acpi_data_push(table->buf, sizeof(AcpiTableHeader));
> > +
> > +    /* scope for memory hotplug controller device node */
> > +    assert(nr_mem <= ACPI_MAX_RAM_SLOTS);
> > +    ctrl_dev = aml_scope("\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE));
> > +    {
> > +        Aml *crs, *field;
> > +
> > +        aml_append(ctrl_dev,
> > +            aml_name_decl(stringify(MEMORY_SLOTS_NUMBER), aml_int(nr_mem))
> > +        );
> > +
> > +        crs = aml_resource_template();
> > +        aml_append(crs, aml_io(AML_DECODE16, io_base, io_base, 0, io_len));
> > +        aml_append(ctrl_dev, aml_name_decl("_CRS", crs));
> > +
> > +        aml_append(ctrl_dev, aml_operation_region(
> > +            stringify(MEMORY_HOTPLUG_IO_REGION), AML_SYSTEM_IO,
> > +            io_base, io_len)
> > +        );
> > +
> > +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
> > +                          AML_PRESERVE);
> > +        aml_append(field, /* read only */
> > +            aml_named_field(stringify(MEMORY_SLOT_ADDR_LOW), 32));
> > +        aml_append(field, /* read only */
> > +            aml_named_field(stringify(MEMORY_SLOT_ADDR_HIGH), 32));
> > +        aml_append(field, /* read only */
> > +            aml_named_field(stringify(MEMORY_SLOT_SIZE_LOW), 32));
> > +        aml_append(field, /* read only */
> > +            aml_named_field(stringify(MEMORY_SLOT_SIZE_HIGH), 32));
> > +        aml_append(field, /* read only */
> > +            aml_named_field(stringify(MEMORY_SLOT_PROXIMITY), 32));
> > +        aml_append(ctrl_dev, field);
> > +
> > +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_BYTE_ACC,
> > +                          AML_WRITE_AS_ZEROS);
> > +        aml_append(field, aml_reserved_field(160 /* bits, Offset(20) */));
> > +        aml_append(field, /* 1 if enabled, read only */
> > +            aml_named_field(stringify(MEMORY_SLOT_ENABLED), 1));
> > +        aml_append(field,
> > +            /*(read) 1 if has a insert event. (write) 1 to clear event */
> > +            aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
> > +        aml_append(field,
> > +            /* (read) 1 if has a remove event. (write) 1 to clear event */
> > +            aml_named_field(stringify(MEMORY_SLOT_REMOVE_EVENT), 1));
> > +        aml_append(field,
> > +            /* initiates device eject, write only */
> > +            aml_named_field(stringify(MEMORY_SLOT_EJECT), 1));
> > +        aml_append(ctrl_dev, field);
> > +
> > +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
> > +                          AML_PRESERVE);
> > +        aml_append(field, /* DIMM selector, write only */
> > +            aml_named_field(stringify(MEMORY_SLOT_SLECTOR), 32));
> > +        aml_append(field, /* _OST event code, write only */
> > +            aml_named_field(stringify(MEMORY_SLOT_OST_EVENT), 32));
> > +        aml_append(field, /* _OST status code, write only */
> > +            aml_named_field(stringify(MEMORY_SLOT_OST_STATUS), 32));
> > +        aml_append(ctrl_dev, field);
> > +    }
> > +    aml_append(table, ctrl_dev);
> > +
> > +    sb_scope = aml_scope("\\_SB");
> > +    for (i = 0; i < nr_mem; i++) {
> > +        const char *s;
> > +
> > +        dev = aml_device("MP%02X", i);
> > +        aml_append(dev, aml_name_decl("_UID", aml_string("0x%02X", i)));
> > +        aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C80")));
> > +
> > +        method = aml_method("_CRS", 0);
> > +        s = BASEPATH stringify(MEMORY_SLOT_CRS_METHOD);
> > +        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> > +        aml_append(dev, method);
> > +
> > +        method = aml_method("_STA", 0);
> > +        s = BASEPATH stringify(MEMORY_SLOT_STATUS_METHOD);
> > +        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> > +        aml_append(dev, method);
> > +
> > +        method = aml_method("_PXM", 0);
> > +        s = BASEPATH stringify(MEMORY_SLOT_PROXIMITY_METHOD);
> > +        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> > +        aml_append(dev, method);
> > +
> > +        method = aml_method("_OST", 3);
> > +        s = BASEPATH stringify(MEMORY_SLOT_OST_METHOD);
> > +        aml_append(method, aml_return(aml_call4(
> > +            s, aml_name("_UID"), aml_arg(0), aml_arg(1), aml_arg(2)
> > +        )));
> > +        aml_append(dev, method);
> > +
> > +        method = aml_method("_EJ0", 1);
> > +        s = BASEPATH stringify(MEMORY_SLOT_EJECT_METHOD);
> > +        aml_append(method, aml_return(aml_call2(
> > +                   s, aml_name("_UID"), aml_arg(0))));
> > +        aml_append(dev, method);
> > +
> > +        aml_append(sb_scope, dev);
> > +    }
> > +
> > +    /* build Method(MEMORY_SLOT_NOTIFY_METHOD, 2) {
> > +     *     If (LEqual(Arg0, 0x00)) {Notify(MP00, Arg1)} ... }
> > +     */
> > +    method = aml_method(stringify(MEMORY_SLOT_NOTIFY_METHOD), 2);
> > +    for (i = 0; i < nr_mem; i++) {
> > +        ifctx = aml_if(aml_equal(aml_arg(0), aml_int(i)));
> > +        aml_append(ifctx,
> > +            aml_notify(aml_name("MP%.02X", i), aml_arg(1))
> > +        );
> > +        aml_append(method, ifctx);
> > +    }
> > +    aml_append(sb_scope, method);
> > +    aml_append(table, sb_scope);
> > +
> > +    /* copy AML table into ACPI tables blob and patch header there */
> > +    g_array_append_vals(table_data, table->buf->data, table->buf->len);
> > +    build_header(linker, table_data,
> > +        (void *)(table_data->data + table_data->len - table->buf->len),
> > +        "MHPT", table->buf->len, 2);
> > +    free_aml_allocator();
> > +}
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 95e0c65..8add4d9 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -925,6 +925,16 @@ build_ssdt(GArray *table_data, GArray *linker,
> >      /* Reserve space for header */
> >      acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
> >  
> > +    sb_scope = aml_scope("\\_SB");
> > +    method = aml_method("_INI", 0);
> > +    ifctx = aml_if(aml_lgreater_equal(aml_name("_REV"), aml_int(2)));
> > +    if (nr_mem) {
> > +        aml_append(ifctx, aml_load_table("MHPT"));
> > +    }
> > +    aml_append(method, ifctx);
> > +    aml_append(sb_scope, method);
> > +    aml_append(ssdt, sb_scope);
> > +
> >      /* Extra PCI root buses are implemented  only for i440fx */
> >      bus = find_i440fx();
> >      if (bus) {
> > @@ -1200,119 +1210,6 @@ build_ssdt(GArray *table_data, GArray *linker,
> >          }
> >          aml_append(sb_scope, aml_name_decl("CPON", pkg));
> >  
> > -        /* build memory devices */
> > -        assert(nr_mem <= ACPI_MAX_RAM_SLOTS);
> > -        scope = aml_scope("\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE));
> > -        aml_append(scope,
> > -            aml_name_decl(stringify(MEMORY_SLOTS_NUMBER), aml_int(nr_mem))
> > -        );
> > -
> > -        crs = aml_resource_template();
> > -        aml_append(crs,
> > -            aml_io(AML_DECODE16, pm->mem_hp_io_base, pm->mem_hp_io_base, 0,
> > -                   pm->mem_hp_io_len)
> > -        );
> > -        aml_append(scope, aml_name_decl("_CRS", crs));
> > -
> > -        aml_append(scope, aml_operation_region(
> > -            stringify(MEMORY_HOTPLUG_IO_REGION), AML_SYSTEM_IO,
> > -            pm->mem_hp_io_base, pm->mem_hp_io_len)
> > -        );
> > -
> > -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
> > -                          AML_PRESERVE);
> > -        aml_append(field, /* read only */
> > -            aml_named_field(stringify(MEMORY_SLOT_ADDR_LOW), 32));
> > -        aml_append(field, /* read only */
> > -            aml_named_field(stringify(MEMORY_SLOT_ADDR_HIGH), 32));
> > -        aml_append(field, /* read only */
> > -            aml_named_field(stringify(MEMORY_SLOT_SIZE_LOW), 32));
> > -        aml_append(field, /* read only */
> > -            aml_named_field(stringify(MEMORY_SLOT_SIZE_HIGH), 32));
> > -        aml_append(field, /* read only */
> > -            aml_named_field(stringify(MEMORY_SLOT_PROXIMITY), 32));
> > -        aml_append(scope, field);
> > -
> > -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_BYTE_ACC,
> > -                          AML_WRITE_AS_ZEROS);
> > -        aml_append(field, aml_reserved_field(160 /* bits, Offset(20) */));
> > -        aml_append(field, /* 1 if enabled, read only */
> > -            aml_named_field(stringify(MEMORY_SLOT_ENABLED), 1));
> > -        aml_append(field,
> > -            /*(read) 1 if has a insert event. (write) 1 to clear event */
> > -            aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
> > -        aml_append(field,
> > -            /* (read) 1 if has a remove event. (write) 1 to clear event */
> > -            aml_named_field(stringify(MEMORY_SLOT_REMOVE_EVENT), 1));
> > -        aml_append(field,
> > -            /* initiates device eject, write only */
> > -            aml_named_field(stringify(MEMORY_SLOT_EJECT), 1));
> > -        aml_append(scope, field);
> > -
> > -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
> > -                          AML_PRESERVE);
> > -        aml_append(field, /* DIMM selector, write only */
> > -            aml_named_field(stringify(MEMORY_SLOT_SLECTOR), 32));
> > -        aml_append(field, /* _OST event code, write only */
> > -            aml_named_field(stringify(MEMORY_SLOT_OST_EVENT), 32));
> > -        aml_append(field, /* _OST status code, write only */
> > -            aml_named_field(stringify(MEMORY_SLOT_OST_STATUS), 32));
> > -        aml_append(scope, field);
> > -
> > -        aml_append(sb_scope, scope);
> > -
> > -        for (i = 0; i < nr_mem; i++) {
> > -            #define BASEPATH "\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE) "."
> > -            const char *s;
> > -
> > -            dev = aml_device("MP%02X", i);
> > -            aml_append(dev, aml_name_decl("_UID", aml_string("0x%02X", i)));
> > -            aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C80")));
> > -
> > -            method = aml_method("_CRS", 0);
> > -            s = BASEPATH stringify(MEMORY_SLOT_CRS_METHOD);
> > -            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> > -            aml_append(dev, method);
> > -
> > -            method = aml_method("_STA", 0);
> > -            s = BASEPATH stringify(MEMORY_SLOT_STATUS_METHOD);
> > -            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> > -            aml_append(dev, method);
> > -
> > -            method = aml_method("_PXM", 0);
> > -            s = BASEPATH stringify(MEMORY_SLOT_PROXIMITY_METHOD);
> > -            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
> > -            aml_append(dev, method);
> > -
> > -            method = aml_method("_OST", 3);
> > -            s = BASEPATH stringify(MEMORY_SLOT_OST_METHOD);
> > -            aml_append(method, aml_return(aml_call4(
> > -                s, aml_name("_UID"), aml_arg(0), aml_arg(1), aml_arg(2)
> > -            )));
> > -            aml_append(dev, method);
> > -
> > -            method = aml_method("_EJ0", 1);
> > -            s = BASEPATH stringify(MEMORY_SLOT_EJECT_METHOD);
> > -            aml_append(method, aml_return(aml_call2(
> > -                       s, aml_name("_UID"), aml_arg(0))));
> > -            aml_append(dev, method);
> > -
> > -            aml_append(sb_scope, dev);
> > -        }
> > -
> > -        /* build Method(MEMORY_SLOT_NOTIFY_METHOD, 2) {
> > -         *     If (LEqual(Arg0, 0x00)) {Notify(MP00, Arg1)} ... }
> > -         */
> > -        method = aml_method(stringify(MEMORY_SLOT_NOTIFY_METHOD), 2);
> > -        for (i = 0; i < nr_mem; i++) {
> > -            ifctx = aml_if(aml_equal(aml_arg(0), aml_int(i)));
> > -            aml_append(ifctx,
> > -                aml_notify(aml_name("MP%.02X", i), aml_arg(1))
> > -            );
> > -            aml_append(method, ifctx);
> > -        }
> > -        aml_append(sb_scope, method);
> > -
> >          {
> >              Object *pci_host;
> >              PCIBus *bus = NULL;
> > @@ -1671,6 +1568,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
> >      uint8_t *u;
> >      size_t aml_len = 0;
> >      GArray *tables_blob = tables->table_data;
> > +    MachineState *machine = MACHINE(qdev_get_machine());
> >  
> >      acpi_get_cpu_info(&cpu);
> >      acpi_get_pm_info(&pm);
> > @@ -1742,6 +1640,12 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
> >          build_dmar_q35(tables_blob, tables->linker);
> >      }
> >  
> > +    if (machine->ram_slots) {
> > +        acpi_add_table(table_offsets, tables_blob);
> > +        build_mhpt(tables_blob, tables->linker, machine->ram_slots,
> > +                   pm.mem_hp_io_base, pm.mem_hp_io_len);
> > +    }
> > +
> >      /* Add tables supplied by user (if any) */
> >      for (u = acpi_table_first(); u; u = acpi_table_next(u)) {
> >          unsigned len = acpi_table_len(u);
> > diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
> > index 1342adb..5fd2854 100644
> > --- a/include/hw/acpi/memory_hotplug.h
> > +++ b/include/hw/acpi/memory_hotplug.h
> > @@ -45,4 +45,7 @@ extern const VMStateDescription vmstate_memory_hotplug;
> >                     vmstate_memory_hotplug, MemHotplugState)
> >  
> >  void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList ***list);
> > +
> > +void build_mhpt(GArray *table_data, GArray *linker, uint32_t nr_mem,
> > +                uint16_t io_base, uint16_t io_len);
> >  #endif
> > -- 
> > 1.8.3.1
diff mbox

Patch

diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index 7d3230c..c04064e 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -1,7 +1,7 @@ 
 common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
 common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
 common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
-common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
+common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
 common-obj-$(CONFIG_ACPI) += acpi_interface.o
 common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
 common-obj-$(CONFIG_ACPI) += aml-build.o
diff --git a/hw/acpi/memory_hotplug_acpi_table.c b/hw/acpi/memory_hotplug_acpi_table.c
new file mode 100644
index 0000000..28da13c
--- /dev/null
+++ b/hw/acpi/memory_hotplug_acpi_table.c
@@ -0,0 +1,138 @@ 
+#include <stdbool.h>
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/memory_hotplug.h"
+#include "include/hw/acpi/pc-hotplug.h"
+#include "hw/boards.h"
+
+#define BASEPATH "\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE) "."
+
+void build_mhpt(GArray *table_data, GArray *linker, uint32_t nr_mem,
+                uint16_t io_base, uint16_t io_len)
+{
+    int i;
+    Aml *table, *sb_scope, *dev, *method, *ifctx, *ctrl_dev;
+
+    table = init_aml_allocator();
+    acpi_data_push(table->buf, sizeof(AcpiTableHeader));
+
+    /* scope for memory hotplug controller device node */
+    assert(nr_mem <= ACPI_MAX_RAM_SLOTS);
+    ctrl_dev = aml_scope("\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE));
+    {
+        Aml *crs, *field;
+
+        aml_append(ctrl_dev,
+            aml_name_decl(stringify(MEMORY_SLOTS_NUMBER), aml_int(nr_mem))
+        );
+
+        crs = aml_resource_template();
+        aml_append(crs, aml_io(AML_DECODE16, io_base, io_base, 0, io_len));
+        aml_append(ctrl_dev, aml_name_decl("_CRS", crs));
+
+        aml_append(ctrl_dev, aml_operation_region(
+            stringify(MEMORY_HOTPLUG_IO_REGION), AML_SYSTEM_IO,
+            io_base, io_len)
+        );
+
+        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
+                          AML_PRESERVE);
+        aml_append(field, /* read only */
+            aml_named_field(stringify(MEMORY_SLOT_ADDR_LOW), 32));
+        aml_append(field, /* read only */
+            aml_named_field(stringify(MEMORY_SLOT_ADDR_HIGH), 32));
+        aml_append(field, /* read only */
+            aml_named_field(stringify(MEMORY_SLOT_SIZE_LOW), 32));
+        aml_append(field, /* read only */
+            aml_named_field(stringify(MEMORY_SLOT_SIZE_HIGH), 32));
+        aml_append(field, /* read only */
+            aml_named_field(stringify(MEMORY_SLOT_PROXIMITY), 32));
+        aml_append(ctrl_dev, field);
+
+        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_BYTE_ACC,
+                          AML_WRITE_AS_ZEROS);
+        aml_append(field, aml_reserved_field(160 /* bits, Offset(20) */));
+        aml_append(field, /* 1 if enabled, read only */
+            aml_named_field(stringify(MEMORY_SLOT_ENABLED), 1));
+        aml_append(field,
+            /*(read) 1 if has a insert event. (write) 1 to clear event */
+            aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
+        aml_append(field,
+            /* (read) 1 if has a remove event. (write) 1 to clear event */
+            aml_named_field(stringify(MEMORY_SLOT_REMOVE_EVENT), 1));
+        aml_append(field,
+            /* initiates device eject, write only */
+            aml_named_field(stringify(MEMORY_SLOT_EJECT), 1));
+        aml_append(ctrl_dev, field);
+
+        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
+                          AML_PRESERVE);
+        aml_append(field, /* DIMM selector, write only */
+            aml_named_field(stringify(MEMORY_SLOT_SLECTOR), 32));
+        aml_append(field, /* _OST event code, write only */
+            aml_named_field(stringify(MEMORY_SLOT_OST_EVENT), 32));
+        aml_append(field, /* _OST status code, write only */
+            aml_named_field(stringify(MEMORY_SLOT_OST_STATUS), 32));
+        aml_append(ctrl_dev, field);
+    }
+    aml_append(table, ctrl_dev);
+
+    sb_scope = aml_scope("\\_SB");
+    for (i = 0; i < nr_mem; i++) {
+        const char *s;
+
+        dev = aml_device("MP%02X", i);
+        aml_append(dev, aml_name_decl("_UID", aml_string("0x%02X", i)));
+        aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C80")));
+
+        method = aml_method("_CRS", 0);
+        s = BASEPATH stringify(MEMORY_SLOT_CRS_METHOD);
+        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
+        aml_append(dev, method);
+
+        method = aml_method("_STA", 0);
+        s = BASEPATH stringify(MEMORY_SLOT_STATUS_METHOD);
+        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
+        aml_append(dev, method);
+
+        method = aml_method("_PXM", 0);
+        s = BASEPATH stringify(MEMORY_SLOT_PROXIMITY_METHOD);
+        aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
+        aml_append(dev, method);
+
+        method = aml_method("_OST", 3);
+        s = BASEPATH stringify(MEMORY_SLOT_OST_METHOD);
+        aml_append(method, aml_return(aml_call4(
+            s, aml_name("_UID"), aml_arg(0), aml_arg(1), aml_arg(2)
+        )));
+        aml_append(dev, method);
+
+        method = aml_method("_EJ0", 1);
+        s = BASEPATH stringify(MEMORY_SLOT_EJECT_METHOD);
+        aml_append(method, aml_return(aml_call2(
+                   s, aml_name("_UID"), aml_arg(0))));
+        aml_append(dev, method);
+
+        aml_append(sb_scope, dev);
+    }
+
+    /* build Method(MEMORY_SLOT_NOTIFY_METHOD, 2) {
+     *     If (LEqual(Arg0, 0x00)) {Notify(MP00, Arg1)} ... }
+     */
+    method = aml_method(stringify(MEMORY_SLOT_NOTIFY_METHOD), 2);
+    for (i = 0; i < nr_mem; i++) {
+        ifctx = aml_if(aml_equal(aml_arg(0), aml_int(i)));
+        aml_append(ifctx,
+            aml_notify(aml_name("MP%.02X", i), aml_arg(1))
+        );
+        aml_append(method, ifctx);
+    }
+    aml_append(sb_scope, method);
+    aml_append(table, sb_scope);
+
+    /* copy AML table into ACPI tables blob and patch header there */
+    g_array_append_vals(table_data, table->buf->data, table->buf->len);
+    build_header(linker, table_data,
+        (void *)(table_data->data + table_data->len - table->buf->len),
+        "MHPT", table->buf->len, 2);
+    free_aml_allocator();
+}
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 95e0c65..8add4d9 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -925,6 +925,16 @@  build_ssdt(GArray *table_data, GArray *linker,
     /* Reserve space for header */
     acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
 
+    sb_scope = aml_scope("\\_SB");
+    method = aml_method("_INI", 0);
+    ifctx = aml_if(aml_lgreater_equal(aml_name("_REV"), aml_int(2)));
+    if (nr_mem) {
+        aml_append(ifctx, aml_load_table("MHPT"));
+    }
+    aml_append(method, ifctx);
+    aml_append(sb_scope, method);
+    aml_append(ssdt, sb_scope);
+
     /* Extra PCI root buses are implemented  only for i440fx */
     bus = find_i440fx();
     if (bus) {
@@ -1200,119 +1210,6 @@  build_ssdt(GArray *table_data, GArray *linker,
         }
         aml_append(sb_scope, aml_name_decl("CPON", pkg));
 
-        /* build memory devices */
-        assert(nr_mem <= ACPI_MAX_RAM_SLOTS);
-        scope = aml_scope("\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE));
-        aml_append(scope,
-            aml_name_decl(stringify(MEMORY_SLOTS_NUMBER), aml_int(nr_mem))
-        );
-
-        crs = aml_resource_template();
-        aml_append(crs,
-            aml_io(AML_DECODE16, pm->mem_hp_io_base, pm->mem_hp_io_base, 0,
-                   pm->mem_hp_io_len)
-        );
-        aml_append(scope, aml_name_decl("_CRS", crs));
-
-        aml_append(scope, aml_operation_region(
-            stringify(MEMORY_HOTPLUG_IO_REGION), AML_SYSTEM_IO,
-            pm->mem_hp_io_base, pm->mem_hp_io_len)
-        );
-
-        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
-                          AML_PRESERVE);
-        aml_append(field, /* read only */
-            aml_named_field(stringify(MEMORY_SLOT_ADDR_LOW), 32));
-        aml_append(field, /* read only */
-            aml_named_field(stringify(MEMORY_SLOT_ADDR_HIGH), 32));
-        aml_append(field, /* read only */
-            aml_named_field(stringify(MEMORY_SLOT_SIZE_LOW), 32));
-        aml_append(field, /* read only */
-            aml_named_field(stringify(MEMORY_SLOT_SIZE_HIGH), 32));
-        aml_append(field, /* read only */
-            aml_named_field(stringify(MEMORY_SLOT_PROXIMITY), 32));
-        aml_append(scope, field);
-
-        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_BYTE_ACC,
-                          AML_WRITE_AS_ZEROS);
-        aml_append(field, aml_reserved_field(160 /* bits, Offset(20) */));
-        aml_append(field, /* 1 if enabled, read only */
-            aml_named_field(stringify(MEMORY_SLOT_ENABLED), 1));
-        aml_append(field,
-            /*(read) 1 if has a insert event. (write) 1 to clear event */
-            aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
-        aml_append(field,
-            /* (read) 1 if has a remove event. (write) 1 to clear event */
-            aml_named_field(stringify(MEMORY_SLOT_REMOVE_EVENT), 1));
-        aml_append(field,
-            /* initiates device eject, write only */
-            aml_named_field(stringify(MEMORY_SLOT_EJECT), 1));
-        aml_append(scope, field);
-
-        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), AML_DWORD_ACC,
-                          AML_PRESERVE);
-        aml_append(field, /* DIMM selector, write only */
-            aml_named_field(stringify(MEMORY_SLOT_SLECTOR), 32));
-        aml_append(field, /* _OST event code, write only */
-            aml_named_field(stringify(MEMORY_SLOT_OST_EVENT), 32));
-        aml_append(field, /* _OST status code, write only */
-            aml_named_field(stringify(MEMORY_SLOT_OST_STATUS), 32));
-        aml_append(scope, field);
-
-        aml_append(sb_scope, scope);
-
-        for (i = 0; i < nr_mem; i++) {
-            #define BASEPATH "\\_SB.PCI0." stringify(MEMORY_HOTPLUG_DEVICE) "."
-            const char *s;
-
-            dev = aml_device("MP%02X", i);
-            aml_append(dev, aml_name_decl("_UID", aml_string("0x%02X", i)));
-            aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C80")));
-
-            method = aml_method("_CRS", 0);
-            s = BASEPATH stringify(MEMORY_SLOT_CRS_METHOD);
-            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
-            aml_append(dev, method);
-
-            method = aml_method("_STA", 0);
-            s = BASEPATH stringify(MEMORY_SLOT_STATUS_METHOD);
-            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
-            aml_append(dev, method);
-
-            method = aml_method("_PXM", 0);
-            s = BASEPATH stringify(MEMORY_SLOT_PROXIMITY_METHOD);
-            aml_append(method, aml_return(aml_call1(s, aml_name("_UID"))));
-            aml_append(dev, method);
-
-            method = aml_method("_OST", 3);
-            s = BASEPATH stringify(MEMORY_SLOT_OST_METHOD);
-            aml_append(method, aml_return(aml_call4(
-                s, aml_name("_UID"), aml_arg(0), aml_arg(1), aml_arg(2)
-            )));
-            aml_append(dev, method);
-
-            method = aml_method("_EJ0", 1);
-            s = BASEPATH stringify(MEMORY_SLOT_EJECT_METHOD);
-            aml_append(method, aml_return(aml_call2(
-                       s, aml_name("_UID"), aml_arg(0))));
-            aml_append(dev, method);
-
-            aml_append(sb_scope, dev);
-        }
-
-        /* build Method(MEMORY_SLOT_NOTIFY_METHOD, 2) {
-         *     If (LEqual(Arg0, 0x00)) {Notify(MP00, Arg1)} ... }
-         */
-        method = aml_method(stringify(MEMORY_SLOT_NOTIFY_METHOD), 2);
-        for (i = 0; i < nr_mem; i++) {
-            ifctx = aml_if(aml_equal(aml_arg(0), aml_int(i)));
-            aml_append(ifctx,
-                aml_notify(aml_name("MP%.02X", i), aml_arg(1))
-            );
-            aml_append(method, ifctx);
-        }
-        aml_append(sb_scope, method);
-
         {
             Object *pci_host;
             PCIBus *bus = NULL;
@@ -1671,6 +1568,7 @@  void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
     uint8_t *u;
     size_t aml_len = 0;
     GArray *tables_blob = tables->table_data;
+    MachineState *machine = MACHINE(qdev_get_machine());
 
     acpi_get_cpu_info(&cpu);
     acpi_get_pm_info(&pm);
@@ -1742,6 +1640,12 @@  void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
         build_dmar_q35(tables_blob, tables->linker);
     }
 
+    if (machine->ram_slots) {
+        acpi_add_table(table_offsets, tables_blob);
+        build_mhpt(tables_blob, tables->linker, machine->ram_slots,
+                   pm.mem_hp_io_base, pm.mem_hp_io_len);
+    }
+
     /* Add tables supplied by user (if any) */
     for (u = acpi_table_first(); u; u = acpi_table_next(u)) {
         unsigned len = acpi_table_len(u);
diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
index 1342adb..5fd2854 100644
--- a/include/hw/acpi/memory_hotplug.h
+++ b/include/hw/acpi/memory_hotplug.h
@@ -45,4 +45,7 @@  extern const VMStateDescription vmstate_memory_hotplug;
                    vmstate_memory_hotplug, MemHotplugState)
 
 void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList ***list);
+
+void build_mhpt(GArray *table_data, GArray *linker, uint32_t nr_mem,
+                uint16_t io_base, uint16_t io_len);
 #endif