[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 35/74] pc: acpi: factor out memhp code from buil
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 35/74] pc: acpi: factor out memhp code from build_ssdt() into separate function |
Date: |
Sat, 19 Dec 2015 22:18:14 +0200 |
On Thu, Dec 10, 2015 at 12:41:29AM +0100, Igor Mammedov wrote:
> before consolidating memhp code in memory_hotplug_acpi_table.c
> and for simplifying review, first factor out memhp code into
> new function build_memory_devices() in i386/acpi-build.c
>
> Signed-off-by: Igor Mammedov <address@hidden>
> ----
> PS:
> no functional change, only code movement.
> ---
> hw/i386/acpi-build.c | 239
> +++++++++++++++++++++++++++------------------------
> 1 file changed, 126 insertions(+), 113 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 1b609e6..765fccd 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -903,6 +903,130 @@ static Aml *build_crs(PCIHostState *host,
> return crs;
> }
>
> +static void build_memory_devices(Aml *sb_scope, int nr_mem,
> + uint16_t io_base, uint16_t io_len)
> +{
> + int i;
> + Aml *scope;
> + Aml *crs;
> + Aml *field;
> + Aml *dev;
> + Aml *method;
> + Aml *ifctx;
> +
> + /* 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, io_base, io_base, 0, io_len)
> + );
> + aml_append(scope, aml_name_decl("_CRS", crs));
> +
> + aml_append(scope, 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_NOLOCK, 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_NOLOCK, 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_NOLOCK, 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) "."
Again we'll have to clean this up down the line.
All #defines should start at the 1st column.
And we should not pollute global namespace like this.
Would ACPI_MEMHP_BASEPATH be all that bad? I think not.
Existing code so can be a separate patch on top.
> + 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, AML_NOTSERIALIZED);
> + 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, AML_NOTSERIALIZED);
> + 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, AML_NOTSERIALIZED);
> + 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, AML_NOTSERIALIZED);
> + 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, AML_NOTSERIALIZED);
> + 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)} ... }
> + */
BTW this needs a cleanup. It does nothing to clarify what is
the method doing, which by now I have forgotten :(
ASL is generally barely readable, if one finds documenting
C using ASL one knows one is in trouble :)
> + method = aml_method(stringify(MEMORY_SLOT_NOTIFY_METHOD), 2,
> + AML_NOTSERIALIZED);
> + 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);
> +}
> +
> static void
> build_ssdt(GArray *table_data, GArray *linker,
> AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
> @@ -1206,119 +1330,8 @@ 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_NOLOCK, 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_NOLOCK, 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_NOLOCK, 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, AML_NOTSERIALIZED);
> - 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, AML_NOTSERIALIZED);
> - 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, AML_NOTSERIALIZED);
> - 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, AML_NOTSERIALIZED);
> - 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, AML_NOTSERIALIZED);
> - 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,
> - AML_NOTSERIALIZED);
> - 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);
> + build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base,
> + pm->mem_hp_io_len);
>
> {
> Object *pci_host;
> --
> 1.8.3.1
>
- Re: [Qemu-devel] [PATCH 10/74] acpi: add aml_call0() helper, (continued)
- [Qemu-devel] [PATCH 14/74] acpi: add aml_sleep(), Igor Mammedov, 2015/12/09
- [Qemu-devel] [PATCH 34/74] pc: acpi: memhp: move MHPD Device into SSDT, Igor Mammedov, 2015/12/09
- [Qemu-devel] [PATCH 13/74] acpi: add aml_alias(), Igor Mammedov, 2015/12/09
- [Qemu-devel] [PATCH 22/74] acpi: extend aml_or() to accept target argument, Igor Mammedov, 2015/12/09
- [Qemu-devel] [PATCH 33/74] pc: acpi: memhp: move MHPD.MCRS method into SSDT, Igor Mammedov, 2015/12/09
- [Qemu-devel] [PATCH 35/74] pc: acpi: factor out memhp code from build_ssdt() into separate function, Igor Mammedov, 2015/12/09
- Re: [Qemu-devel] [PATCH 35/74] pc: acpi: factor out memhp code from build_ssdt() into separate function,
Michael S. Tsirkin <=
- [Qemu-devel] [PATCH 38/74] pc: acpi: drop unused CPU_STATUS_LEN from DSDT, Igor Mammedov, 2015/12/09
- [Qemu-devel] [PATCH 40/74] pc: acpi: cpuhp: move CPMA() method into SSDT, Igor Mammedov, 2015/12/09
- [Qemu-devel] [PATCH 41/74] pc: acpi: cpuhp: move CPST() method into SSDT, Igor Mammedov, 2015/12/09
- [Qemu-devel] [PATCH 48/74] pc: acpi: move KBD device from DSDT to SSDT, Igor Mammedov, 2015/12/09
- [Qemu-devel] [PATCH 53/74] pc: acpi: move PIIX4 isa-bridge and pm devices into SSDT, Igor Mammedov, 2015/12/09
- [Qemu-devel] [PATCH 17/74] acpi: add aml_lgreater(), Igor Mammedov, 2015/12/09
- [Qemu-devel] [PATCH 19/74] acpi: add aml_to_hexstring(), Igor Mammedov, 2015/12/09