From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35408) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UVRD0-0005L7-29 for qemu-devel@nongnu.org; Thu, 25 Apr 2013 14:47:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UVRCy-0003Vl-LT for qemu-devel@nongnu.org; Thu, 25 Apr 2013 14:47:53 -0400 Received: from mail-ob0-x234.google.com ([2607:f8b0:4003:c01::234]:43790) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UVRCy-0003Vf-FS for qemu-devel@nongnu.org; Thu, 25 Apr 2013 14:47:52 -0400 Received: by mail-ob0-f180.google.com with SMTP id uk5so2853529obc.11 for ; Thu, 25 Apr 2013 11:47:51 -0700 (PDT) From: Anthony Liguori In-Reply-To: <1366316544-1428-4-git-send-email-lersek@redhat.com> References: <1366316544-1428-1-git-send-email-lersek@redhat.com> <1366316544-1428-4-git-send-email-lersek@redhat.com> Date: Thu, 25 Apr 2013 13:47:22 -0500 Message-ID: <87zjwmjvlx.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH v4 3/7] hw/acpi: extract standard table headers as a standalone structure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek , mst@redhat.com, qemu-devel@nongnu.org Laszlo Ersek writes: > This enables reuse when preparing per-table fw_cfg blobs later. > > Signed-off-by: Laszlo Ersek > Acked-by: Michael S. Tsirkin > --- > include/hw/acpi/acpi.h | 11 +++++++++++ > hw/acpi/core.c | 48 +++++++++++++++++++++--------------------------- > 2 files changed, 32 insertions(+), 27 deletions(-) > > diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h > index 635be7b..0e26f63 100644 > --- a/include/hw/acpi/acpi.h > +++ b/include/hw/acpi/acpi.h > @@ -167,4 +167,15 @@ extern size_t acpi_tables_len; > > void acpi_table_add(const QemuOpts *opts, Error **errp); > > +typedef struct acpi_table_std_header { > + char sig[4]; /* ACPI signature (4 ASCII characters) */ > + uint32_t length; /* Length of table, in bytes, including header */ > + uint8_t revision; /* ACPI Specification minor version # */ > + uint8_t checksum; /* To make sum of entire table == 0 */ > + char oem_id[6]; /* OEM identification */ > + char oem_table_id[8]; /* OEM table identification */ > + uint32_t oem_revision; /* OEM revision number */ > + char asl_compiler_id[4]; /* ASL compiler vendor ID */ > + uint32_t asl_compiler_revision; /* ASL compiler revision number */ > +} QEMU_PACKED AcpiTableStdHdr; Since you're giving it a CamelCaseName why don't you do the same for the struct. After that: Reviewed-by: Anthony Liguori Regards, Anthony Liguori > #endif /* !QEMU_HW_ACPI_H */ > diff --git a/hw/acpi/core.c b/hw/acpi/core.c > index 69cadb0..d348f81 100644 > --- a/hw/acpi/core.c > +++ b/hw/acpi/core.c > @@ -31,21 +31,13 @@ > struct acpi_table_header { > uint16_t _length; /* our length, not actual part of the hdr */ > /* allows easier parsing for fw_cfg clients */ > - char sig[4]; /* ACPI signature (4 ASCII characters) */ > - uint32_t length; /* Length of table, in bytes, including header */ > - uint8_t revision; /* ACPI Specification minor version # */ > - uint8_t checksum; /* To make sum of entire table == 0 */ > - char oem_id[6]; /* OEM identification */ > - char oem_table_id[8]; /* OEM table identification */ > - uint32_t oem_revision; /* OEM revision number */ > - char asl_compiler_id[4]; /* ASL compiler vendor ID */ > - uint32_t asl_compiler_revision; /* ASL compiler revision number */ > + AcpiTableStdHdr std; > } QEMU_PACKED; > > -#define ACPI_TABLE_HDR_SIZE sizeof(struct acpi_table_header) > -#define ACPI_TABLE_PFX_SIZE sizeof(uint16_t) /* size of the extra prefix */ > +/* size of the extra prefix */ > +#define ACPI_TABLE_PFX_SIZE offsetof(struct acpi_table_header, std) > > -static const char unsigned dfl_hdr[ACPI_TABLE_HDR_SIZE - ACPI_TABLE_PFX_SIZE] = > +static const char unsigned dfl_hdr[sizeof(AcpiTableStdHdr)] = > "QEMU\0\0\0\0\1\0" /* sig (4), len(4), revno (1), csum (1) */ > "QEMUQEQEMUQEMU\1\0\0\0" /* OEM id (6), table (8), revno (4) */ > "QEMU\1\0\0\0" /* ASL compiler ID (4), version (4) */ > @@ -105,6 +97,7 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen, > size_t body_size, acpi_payload_size; > struct acpi_table_header *ext_hdr; > unsigned changed_fields; > + AcpiTableStdHdr *std; > > /* Calculate where the ACPI table body starts within the blob, plus where > * to copy the ACPI table header from. > @@ -177,46 +170,47 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen, > changed_fields = 0; > ext_hdr->_length = cpu_to_le16(acpi_payload_size); > > + std = &ext_hdr->std; > if (hdrs->has_sig) { > - strncpy(ext_hdr->sig, hdrs->sig, sizeof ext_hdr->sig); > + strncpy(std->sig, hdrs->sig, sizeof std->sig); > ++changed_fields; > } > > - if (has_header && le32_to_cpu(ext_hdr->length) != acpi_payload_size) { > + if (has_header && le32_to_cpu(std->length) != acpi_payload_size) { > fprintf(stderr, > "warning: ACPI table has wrong length, header says " > "%" PRIu32 ", actual size %zu bytes\n", > - le32_to_cpu(ext_hdr->length), acpi_payload_size); > + le32_to_cpu(std->length), acpi_payload_size); > } > - ext_hdr->length = cpu_to_le32(acpi_payload_size); > + std->length = cpu_to_le32(acpi_payload_size); > > if (hdrs->has_rev) { > - ext_hdr->revision = hdrs->rev; > + std->revision = hdrs->rev; > ++changed_fields; > } > > - ext_hdr->checksum = 0; > + std->checksum = 0; > > if (hdrs->has_oem_id) { > - strncpy(ext_hdr->oem_id, hdrs->oem_id, sizeof ext_hdr->oem_id); > + strncpy(std->oem_id, hdrs->oem_id, sizeof std->oem_id); > ++changed_fields; > } > if (hdrs->has_oem_table_id) { > - strncpy(ext_hdr->oem_table_id, hdrs->oem_table_id, > - sizeof ext_hdr->oem_table_id); > + strncpy(std->oem_table_id, hdrs->oem_table_id, > + sizeof std->oem_table_id); > ++changed_fields; > } > if (hdrs->has_oem_rev) { > - ext_hdr->oem_revision = cpu_to_le32(hdrs->oem_rev); > + std->oem_revision = cpu_to_le32(hdrs->oem_rev); > ++changed_fields; > } > if (hdrs->has_asl_compiler_id) { > - strncpy(ext_hdr->asl_compiler_id, hdrs->asl_compiler_id, > - sizeof ext_hdr->asl_compiler_id); > + strncpy(std->asl_compiler_id, hdrs->asl_compiler_id, > + sizeof std->asl_compiler_id); > ++changed_fields; > } > if (hdrs->has_asl_compiler_rev) { > - ext_hdr->asl_compiler_revision = cpu_to_le32(hdrs->asl_compiler_rev); > + std->asl_compiler_revision = cpu_to_le32(hdrs->asl_compiler_rev); > ++changed_fields; > } > > @@ -225,8 +219,8 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen, > } > > /* recalculate checksum */ > - ext_hdr->checksum = acpi_checksum((const char unsigned *)ext_hdr + > - ACPI_TABLE_PFX_SIZE, acpi_payload_size); > + std->checksum = acpi_checksum((const char unsigned *)std, > + acpi_payload_size); > } > > void acpi_table_add(const QemuOpts *opts, Error **errp) > -- > 1.7.1