From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43459) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YEbaY-00037a-I2 for qemu-devel@nongnu.org; Fri, 23 Jan 2015 05:35:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YEbaT-000466-3E for qemu-devel@nongnu.org; Fri, 23 Jan 2015 05:35:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45777) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YEbaS-00045z-Kd for qemu-devel@nongnu.org; Fri, 23 Jan 2015 05:35:36 -0500 Date: Fri, 23 Jan 2015 11:35:29 +0100 From: Igor Mammedov Message-ID: <20150123113529.7954abc0@nial.brq.redhat.com> In-Reply-To: <20150123081119.GE26711@redhat.com> References: <1421938231-25698-1-git-send-email-imammedo@redhat.com> <1421938231-25698-2-git-send-email-imammedo@redhat.com> <20150123081119.GE26711@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 01/47] acpi: introduce AML composer aml_append() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: pbonzini@redhat.com, drjones@redhat.com, claudio.fontana@huawei.com, qemu-devel@nongnu.org, marcel.a@redhat.com On Fri, 23 Jan 2015 10:11:19 +0200 "Michael S. Tsirkin" wrote: > On Thu, Jan 22, 2015 at 02:49:45PM +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: > > > > AcpiAml scope = acpi_scope("PCI0") > > AcpiAml dev = acpi_device("PM") > > aml_append(&dev, acpi_name_decl("_ADR", acpi_int(addr))) > > aml_append(&scope, dev); > > > > Signed-off-by: Igor Mammedov > > --- > > hw/acpi/acpi-build-utils.c | 39 ++++++++++++++++++++++++++++++++++++++ > > include/hw/acpi/acpi-build-utils.h | 16 ++++++++++++++++ > > 2 files changed, 55 insertions(+) > > > > diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c > > index 602e68c..547ecaa 100644 > > --- a/hw/acpi/acpi-build-utils.c > > +++ b/hw/acpi/acpi-build-utils.c > > @@ -267,3 +267,42 @@ void build_append_int(GArray *table, uint32_t value) > > build_append_value(table, value, 4); > > } > > } > > + > > +static void build_prepend_int(GArray *array, uint32_t value) > > +{ > > + GArray *data = build_alloc_array(); > > + > > + build_append_int(data, value); > > + g_array_prepend_vals(array, data->data, data->len); > > + build_free_array(data); > > +} > > I don't think prepend is generally justified: > it makes code hard to follow and debug. > > Adding length is different: of course you need > to first have the package before you can add length. > > We currently have build_prepend_package_length - just move it > to utils, and use everywhere. [...] > > + case BUFFER: > > + build_prepend_int(child.buf, child.buf->len); > > + build_package(child.buf, child.op); Buffer uses the same concept as package, but adds its own additional length. Therefore I've added build_prepend_int(), I can create build_buffer() and mimic build_package() but it won't change picture. As for moving to to another file, during all this series lowlevel build_(some_aml_related_costruct_helper)s are moved into this file and should be make static to hide from user lowlevel helpers (including build_package). That will leave only high level API available. TODO for me: make sure that moved lowlevel helpers are static > > + break; > > + default: > > + break; > > + } > > + build_append_array(parent_ctx->buf, child.buf); > > + build_free_array(child.buf); > > +} > > diff --git a/include/hw/acpi/acpi-build-utils.h b/include/hw/acpi/acpi-build-utils.h > > index 199f003..64e7ec3 100644 > > --- a/include/hw/acpi/acpi-build-utils.h > > +++ b/include/hw/acpi/acpi-build-utils.h > > @@ -5,6 +5,22 @@ > > #include > > #include "qemu/compiler.h" > > > > +typedef enum { > > + NON_BLOCK, > > + PACKAGE, > > + EXT_PACKAGE, > > + BUFFER, > > + RES_TEMPLATE, > > +} AcpiBlockFlags; > > + > > +typedef struct AcpiAml { > > + GArray *buf; > > + uint8_t op; > > + AcpiBlockFlags block_flags; > > +} AcpiAml; > > + > > +void aml_append(AcpiAml *parent_ctx, AcpiAml child); > > + > > GArray *build_alloc_array(void); > > void build_free_array(GArray *array); > > void build_prepend_byte(GArray *array, uint8_t val); > > -- > > 1.8.3.1