From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60608) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNlJB-0003mi-OC for qemu-devel@nongnu.org; Tue, 17 Feb 2015 11:47:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YNlJ8-000151-IS for qemu-devel@nongnu.org; Tue, 17 Feb 2015 11:47:37 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52116) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNlJ8-00014k-Bj for qemu-devel@nongnu.org; Tue, 17 Feb 2015 11:47:34 -0500 Date: Tue, 17 Feb 2015 17:47:26 +0100 From: Igor Mammedov Message-ID: <20150217174726.29d69713@nial.brq.redhat.com> In-Reply-To: <20150217163555.GA25875@redhat.com> References: <1423479254-15342-1-git-send-email-imammedo@redhat.com> <1423479254-15342-6-git-send-email-imammedo@redhat.com> <20150217163555.GA25875@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 05/52] acpi: add aml_def_block() term List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: drjones@redhat.com, marcel.a@redhat.com, claudio.fontana@huawei.com, qemu-devel@nongnu.org, zhaoshenglong@huawei.com On Tue, 17 Feb 2015 17:35:55 +0100 "Michael S. Tsirkin" wrote: > On Mon, Feb 09, 2015 at 10:53:27AM +0000, Igor Mammedov wrote: > > Signed-off-by: Igor Mammedov > > --- > > hw/acpi/aml-build.c | 38 ++++++++++++++++++++++++++++++++++++++ > > hw/i386/acpi-build.c | 4 ---- > > include/hw/acpi/aml-build.h | 10 ++++++++++ > > 3 files changed, 48 insertions(+), 4 deletions(-) > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > index 67d1371..cb1a1bd 100644 > > --- a/hw/acpi/aml-build.c > > +++ b/hw/acpi/aml-build.c > > @@ -335,3 +335,41 @@ void aml_append(Aml *parent_ctx, Aml *child) > > } > > build_append_array(parent_ctx->buf, child->buf); > > } > > + > > +/* > > + * ACPI 1.0b: 16.2.1 Top Level AML > > + * 5.2.3 System Description Table Header > > + * > > + * ACPI 5.0: 20.2.1 Table and Table Header Encoding > > + */ > > +Aml *aml_def_block(const char *signature, uint8_t revision, > > + const char *oem_id, const char *oem_table_id, > > + uint32_t oem_revision, uint32_t creator_id, > > + uint32_t creator_revision) > > +{ > > + int len; > > + Aml *var = aml_alloc(); > > + var->block_flags = AML_DEF_BLOCK; > > + > > + assert(strlen(signature) == 4); > > + g_array_append_vals(var->buf, signature, 4); > > + build_append_value(var->buf, 0, 4); /* Length place holder */ > > + build_append_byte(var->buf, revision); > > + build_append_byte(var->buf, 0); /* place holder for Checksum */ > > + > > + len = strlen(oem_id); > > + assert(len <= 6); > > + g_array_append_vals(var->buf, oem_id, len); > > + g_array_append_vals(var->buf, "\0\0\0\0\0\0\0\0", 6 - len); > > + > > + len = strlen(oem_table_id); > > + assert(len <= 8); > > + g_array_append_vals(var->buf, oem_table_id, len); > > + g_array_append_vals(var->buf, "\0\0\0\0\0\0\0\0", 8 - len); > > + > > + build_append_value(var->buf, oem_revision, 4); > > + build_append_value(var->buf, creator_id, 4); > > + build_append_value(var->buf, creator_revision, 4); > > + > > + return var; > > > Please don't do this. > First, this is not a definition block encoding, so name > is wrong. > > Second, we already have functions to fill in headers. > So just call them. > > Filling structures byte by byte is unreadable - > I know ACPI spec often makes us do this but > when it doesn't, we should not do it. it does, in ACPI 5.1, 20.2.1 Table and Table Header Encoding AMLCode := DefBlockHeader TermList DefBlockHeader := TableSignature TableLength SpecCompliance CheckSum OemID OemTableID OemRevision CreatorID CreatorRevision > > Pls keep using build_header for now. > > > > +} > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index a1bf450..553c86b 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -254,10 +254,6 @@ static void acpi_get_pci_info(PcPciInfo *info) > > NULL); > > } > > > > -#define ACPI_BUILD_APPNAME "Bochs" > > -#define ACPI_BUILD_APPNAME6 "BOCHS " > > -#define ACPI_BUILD_APPNAME4 "BXPC" > > - > > #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 4033796..2610336 100644 > > --- a/include/hw/acpi/aml-build.h > > +++ b/include/hw/acpi/aml-build.h > > @@ -6,6 +6,10 @@ > > #include "qemu/compiler.h" > > > > #define ACPI_BUILD_TABLE_FILE "etc/acpi/tables" > > +#define ACPI_BUILD_APPNAME "Bochs" > > +#define ACPI_BUILD_APPNAME6 "BOCHS " > > +#define ACPI_BUILD_APPNAME4 "BXPC" > > +#define ACPI_BUILD_APPNAME4_HEX 0x43505842 > > > > typedef enum { > > AML_HELPER = 0, > > @@ -65,6 +69,12 @@ void free_aml_allocator(void); > > */ > > void aml_append(Aml *parent_ctx, Aml *child); > > > > +/* Block AML object primitives */ > > +Aml *aml_def_block(const char *signature, uint8_t revision, > > + const char *oem_id, const char *oem_table_id, > > + uint32_t oem_revision, uint32_t creator_id, > > + uint32_t creator_revision); > > + > > /* other helpers */ > > GArray *build_alloc_array(void); > > void build_free_array(GArray *array); > > -- > > 1.8.3.1