On Sat, 24 Oct 2015 20:59:07 +0300
"Michael S. Tsirkin" <m...@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 <imamm...@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
