From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55342) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNkyx-0003cm-IC for qemu-devel@nongnu.org; Tue, 17 Feb 2015 11:26:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YNkyp-0008Tn-RV for qemu-devel@nongnu.org; Tue, 17 Feb 2015 11:26:43 -0500 Received: from mail.kernel.org ([198.145.29.136]:35676) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNkyp-0008Px-Io for qemu-devel@nongnu.org; Tue, 17 Feb 2015 11:26:35 -0500 Date: Tue, 17 Feb 2015 17:26:16 +0100 From: "Michael S. Tsirkin" Message-ID: <20150217162616.GA23547@redhat.com> References: <1423479254-15342-1-git-send-email-imammedo@redhat.com> <1423479254-15342-2-git-send-email-imammedo@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1423479254-15342-2-git-send-email-imammedo@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 01/52] acpi: introduce AML composer aml_append() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: drjones@redhat.com, marcel.a@redhat.com, claudio.fontana@huawei.com, qemu-devel@nongnu.org, zhaoshenglong@huawei.com On Mon, Feb 09, 2015 at 10:53:23AM +0000, Igor Mammedov wrote: > Adds for dynamic AML creation, which will be used > for piecing ASL/AML primitives together and hiding > from user/caller details about how nested context > should be closed/packed leaving less space for > mistakes and necessity to know how AML should be > encoded, allowing user to concentrate on ASL > representation instead. > > For example it will allow to create AML like this: > > init_aml_allocator(); > ... > Aml *scope = aml_scope("PCI0") > Aml *dev = aml_device("PM") > aml_append(dev, aml_name_decl("_ADR", aml_int(addr))) > aml_append(scope, dev); > ... > free_aml_allocator(); > > Signed-off-by: Igor Mammedov > --- > hw/acpi/aml-build.c | 91 +++++++++++++++++++++++++++++++++++++++++++++ > hw/i386/acpi-build.c | 1 - > include/hw/acpi/aml-build.h | 61 ++++++++++++++++++++++++++++++ > 3 files changed, 152 insertions(+), 1 deletion(-) > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > index bcb288e..096f347 100644 > --- a/hw/acpi/aml-build.c > +++ b/hw/acpi/aml-build.c > @@ -25,6 +25,8 @@ > #include > #include > #include "hw/acpi/aml-build.h" > +#include "qemu/bswap.h" > +#include "hw/acpi/bios-linker-loader.h" > > GArray *build_alloc_array(void) > { > @@ -257,3 +259,92 @@ void build_append_int(GArray *table, uint32_t value) > build_append_value(table, value, 4); > } > } > + > +static GPtrArray *alloc_list; > + > +static Aml *aml_alloc(void) > +{ > + Aml *var = g_new0(typeof(*var), 1); > + > + g_ptr_array_add(alloc_list, var); > + var->block_flags = AML_HELPER; > + var->buf = build_alloc_array(); > + return var; > +} > + > +static void aml_free(gpointer data) > +{ > + Aml *var = data; > + build_free_array(var->buf); > +} > + > +Aml *init_aml_allocator(GArray *linker) > +{ > + Aml *var; > + > + assert(!alloc_list); > + alloc_list = g_ptr_array_new_with_free_func(aml_free); > + var = aml_alloc(); > + var->linker = linker; > + return var; > +} > + > +void free_aml_allocator(void) > +{ > + g_ptr_array_free(alloc_list, true); > + alloc_list = 0; > +} > + > +static void build_buffer(GArray *array, uint8_t op) > +{ > + GArray *data = build_alloc_array(); > + > + build_append_int(data, array->len); > + g_array_prepend_vals(array, data->data, data->len); > + build_free_array(data); > + build_package(array, op); > +} > + > +void aml_append(Aml *parent_ctx, Aml *child) > +{ > + switch (child->block_flags) { > + case AML_NON_BLOCK: > + build_append_byte(parent_ctx->buf, child->op); > + break; > + case AML_EXT_PACKAGE: > + build_extop_package(child->buf, child->op); > + break; > + case AML_PACKAGE: > + build_package(child->buf, child->op); > + break; > + case AML_RES_TEMPLATE: > + build_append_byte(child->buf, 0x79); /* EndTag */ > + /* > + * checksum operations is treated as succeeded if checksum > + * field is zero. [ACPI Spec 5.0, 6.4.2.9 End Tag] > + */ > + build_append_byte(child->buf, 0); > + /* fall through, to pack resources in buffer */ > + case AML_BUFFER: > + build_buffer(child->buf, child->op); > + break; > + case AML_DEF_BLOCK: { This one is inelegant. It's not a definition block, and the patching and manual offset calculation are scary. > + uint8_t *start = (uint8_t *)parent_ctx->buf->data + > + parent_ctx->buf->len; > + uint32_t le32_len = cpu_to_le32(child->buf->len); > + > + /* create linker entry for the DefinitionBlock */ > + bios_linker_loader_add_checksum(parent_ctx->linker, > + ACPI_BUILD_TABLE_FILE, Hard-coding file name here is very ugly. All this does not belong in this file, this is fw cfg interface. > + parent_ctx->buf->data, > + start, child->buf->len, start + 9 /* checksum offset */); > + > + /* set DefinitionBlock length at TableLength offset*/ > + memcpy(child->buf->data + 4, &le32_len, sizeof le32_len); This make no sense. In AML, there are no objects which are higher level than the table. So for now, please just leave this alone: drop linker completely, keep table_data a simple GArray, not AML, and reuse build_header to fill in headers. This way this file just deals with AML and ACPI spec. > + break; > + } > + default: This should assert. > + break; > + } > + build_append_array(parent_ctx->buf, child->buf); > +} > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 6d84f38..237080f 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -258,7 +258,6 @@ static void acpi_get_pci_info(PcPciInfo *info) > #define ACPI_BUILD_APPNAME6 "BOCHS " > #define ACPI_BUILD_APPNAME4 "BXPC" > > -#define ACPI_BUILD_TABLE_FILE "etc/acpi/tables" > #define ACPI_BUILD_RSDP_FILE "etc/acpi/rsdp" > #define ACPI_BUILD_TPMLOG_FILE "etc/tpm/log" > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > index 199f003..4033796 100644 > --- a/include/hw/acpi/aml-build.h > +++ b/include/hw/acpi/aml-build.h > @@ -5,6 +5,67 @@ > #include > #include "qemu/compiler.h" > > +#define ACPI_BUILD_TABLE_FILE "etc/acpi/tables" > + > +typedef enum { > + AML_HELPER = 0, > + AML_NON_BLOCK, > + AML_PACKAGE, > + AML_EXT_PACKAGE, > + AML_BUFFER, > + AML_RES_TEMPLATE, > + AML_DEF_BLOCK, > +} AmlBlockFlags; > + > +struct Aml { > + GArray *buf; > + > + /*< private >*/ > + uint8_t op; > + AmlBlockFlags block_flags; > + GArray *linker; > +}; > +typedef struct Aml Aml; > + > +/** > + * init_aml_allocator: > + * @linker: linker that used by API for registering ACPI tables > + * with linker firmware interfac > + * > + * Called for initializing API allocator which allow to use > + * AML API. > + * Returns: toplevel container which accumulates all other > + * ACPI tables. > + */ > +Aml *init_aml_allocator(GArray *linker); > + > +/** > + * free_aml_allocator: > + * > + * Releases all elements used by AML API, frees associated memory > + * and invalidates AML allocator. After this call @init_aml_allocator > + * should be called again if AML API is to be used again. > + */ > +void free_aml_allocator(void); > + > +/** > + * aml_append: > + * @parent_ctx: context to which @child element is added > + * @child: element that is copied into @parent_ctx context > + * > + * Joins Aml elements together and helps to construct AML tables > + * Examle of usage: > + * Aml *table = aml_def_block("SSDT", ...); > + * Aml *sb = aml_scope("\_SB"); > + * Aml *dev = aml_device("PCI0"); > + * > + * aml_append(dev, aml_name_decl("HID", aml_eisaid("PNP0A03"))); > + * aml_append(sb, dev); > + * aml_append(table, sb); > + */ > +void aml_append(Aml *parent_ctx, Aml *child); > + > +/* other helpers */ > GArray *build_alloc_array(void); > void build_free_array(GArray *array); > void build_prepend_byte(GArray *array, uint8_t val); > -- > 1.8.3.1