From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57805) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNl87-0003CW-Fg for qemu-devel@nongnu.org; Tue, 17 Feb 2015 11:36:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YNl84-00045F-10 for qemu-devel@nongnu.org; Tue, 17 Feb 2015 11:36:11 -0500 Received: from mail.kernel.org ([198.145.29.136]:35950) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNl83-00044u-N9 for qemu-devel@nongnu.org; Tue, 17 Feb 2015 11:36:07 -0500 Date: Tue, 17 Feb 2015 17:35:55 +0100 From: "Michael S. Tsirkin" Message-ID: <20150217163555.GA25875@redhat.com> References: <1423479254-15342-1-git-send-email-imammedo@redhat.com> <1423479254-15342-6-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-6-git-send-email-imammedo@redhat.com> 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: 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: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. 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