From patchwork Mon Feb 29 09:38:35 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 589800 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 37999140317 for ; Mon, 29 Feb 2016 20:39:05 +1100 (AEDT) Received: from localhost ([::1]:35390 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaKIB-0003CZ-R3 for incoming@patchwork.ozlabs.org; Mon, 29 Feb 2016 04:39:03 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52922) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaKHv-0002ux-N2 for qemu-devel@nongnu.org; Mon, 29 Feb 2016 04:38:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aaKHq-0003Tk-Nh for qemu-devel@nongnu.org; Mon, 29 Feb 2016 04:38:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59394) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaKHq-0003Tg-F4 for qemu-devel@nongnu.org; Mon, 29 Feb 2016 04:38:42 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id F330C8EA39; Mon, 29 Feb 2016 09:38:40 +0000 (UTC) Received: from redhat.com (vpn1-6-40.ams2.redhat.com [10.36.6.40]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with SMTP id u1T9cZSe010122; Mon, 29 Feb 2016 04:38:36 -0500 Date: Mon, 29 Feb 2016 11:38:35 +0200 From: "Michael S. Tsirkin" To: Xiao Guangrong Message-ID: <20160229112956-mutt-send-email-mst@redhat.com> References: <1455439865-75784-1-git-send-email-guangrong.xiao@linux.intel.com> <1455439865-75784-6-git-send-email-guangrong.xiao@linux.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1455439865-75784-6-git-send-email-guangrong.xiao@linux.intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: ehabkost@redhat.com, kvm@vger.kernel.org, gleb@kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, imammedo@redhat.com, pbonzini@redhat.com, dan.j.williams@intel.com, rth@twiddle.net Subject: Re: [Qemu-devel] [PATCH v3 5/8] nvdimm acpi: introduce patched dsm memory X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Sun, Feb 14, 2016 at 04:51:02PM +0800, Xiao Guangrong wrote: > The dsm memory is used to save the input parameters and store > the dsm result which is filled by QEMU. > > The address of dsm memory is decided by bios and patched into > int32 object named "MEMA" > > Signed-off-by: Xiao Guangrong This is a bit too hacky for my taste. First, I would prefer an explicit API to add a DWORD. Second, I would like to avoid offset math hacks and make API returning offsets. Pls see below for a suggestion. > --- > hw/acpi/nvdimm.c | 34 +++++++++++++++++++++++++++++++++- > 1 file changed, 33 insertions(+), 1 deletion(-) > > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c > index 8568b20..bca36ae 100644 > --- a/hw/acpi/nvdimm.c > +++ b/hw/acpi/nvdimm.c > @@ -29,6 +29,7 @@ > #include "qemu/osdep.h" > #include "hw/acpi/acpi.h" > #include "hw/acpi/aml-build.h" > +#include "hw/acpi/bios-linker-loader.h" > #include "hw/nvram/fw_cfg.h" > #include "hw/mem/nvdimm.h" > > @@ -406,6 +407,7 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io, > } > > #define NVDIMM_COMMON_DSM "NCAL" > +#define NVDIMM_ACPI_MEM_ADDR "MEMA" > > static void nvdimm_build_common_dsm(Aml *dev) > { > @@ -470,7 +472,9 @@ static void nvdimm_build_nvdimm_devices(GSList *device_list, Aml *root_dev) > static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets, > GArray *table_data, GArray *linker) > { > - Aml *ssdt, *sb_scope, *dev; > + Aml *ssdt, *sb_scope, *dev, *mem_addr; > + uint32_t zero_offset = 0; > + int offset; > > acpi_add_table(table_offsets, table_data); > > @@ -501,9 +505,37 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets, > > aml_append(sb_scope, dev); > > + /* > + * leave it at the end of ssdt so that we can conveniently get the > + * offset of int32 object which will be patched with the real address > + * of the dsm memory by BIOS. > + * > + * 0x32000000 is the magic number to let aml_int() create int32 object. > + * It will be zeroed later to make bios_linker_loader_add_pointer() > + * happy. > + */ > + mem_addr = aml_name_decl(NVDIMM_ACPI_MEM_ADDR, aml_int(0x32000000)); > + > + aml_append(sb_scope, mem_addr); > aml_append(ssdt, sb_scope); > /* copy AML table into ACPI tables blob and patch header there */ > g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len); > + > + offset = table_data->len - 4; > + > + /* > + * zero the last 4 bytes, i.e, it is the offset of > + * NVDIMM_ACPI_MEM_ADDR object. > + */ > + g_array_remove_range(table_data, offset, 4); > + g_array_append_vals(table_data, &zero_offset, 4); > + > + bios_linker_loader_alloc(linker, NVDIMM_DSM_MEM_FILE, TARGET_PAGE_SIZE, > + false /* high memory */); > + bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, > + NVDIMM_DSM_MEM_FILE, table_data, > + table_data->data + offset, > + sizeof(uint32_t)); > build_header(linker, table_data, > (void *)(table_data->data + table_data->len - ssdt->buf->len), > "SSDT", ssdt->buf->len, 1, NULL, "NVDIMM"); > -- > 1.8.3.1 ---> acpi: add build_append_named_dword, returning an offset in buffer This is a very limited form of support for runtime patching - similar in functionality to what we can do with ACPI_EXTRACT macros in python, but implemented in C. This is to allow ACPI code direct access to data tables - which is exactly what DataTableRegion is there for, except no known windows release so far implements DataTableRegion. Signed-off-by: Michael S. Tsirkin diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h index 1b632dc..f8998ea 100644 --- a/include/hw/acpi/aml-build.h +++ b/include/hw/acpi/aml-build.h @@ -286,4 +286,7 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre); void build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets); +int +build_append_named_dword(GArray *array, const char *name_format, ...); + #endif diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index 0d4b324..7f9fa65 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -262,6 +262,32 @@ static void build_append_int(GArray *table, uint64_t value) } } +/* Build NAME(XXXX, 0x00000000) where 0x00000000 is encoded as a dword, + * and return the offset to 0x00000000 for runtime patching. + * + * Warning: runtime patching is best avoided. Only use this as + * a replacement for DataTableRegion (for guests that don't + * support it). + */ +int +build_append_named_dword(GArray *array, const char *name_format, ...) +{ + int offset; + va_list ap; + + va_start(ap, name_format); + build_append_namestringv(array, name_format, ap); + va_end(ap); + + build_append_byte(array, 0x0C); /* DWordPrefix */ + + offset = array->len; + build_append_int_noprefix(array, 0x00000000, 4); + assert(array->len == offset + 4); + + return offset; +} + static GPtrArray *alloc_list; static Aml *aml_alloc(void)