From patchwork Thu Mar 17 03:35:01 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Isaku Yamahata X-Patchwork-Id: 87333 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id BF090B6FDD for ; Thu, 17 Mar 2011 14:37:31 +1100 (EST) Received: from localhost ([127.0.0.1]:53663 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q040a-0006Yg-31 for incoming@patchwork.ozlabs.org; Wed, 16 Mar 2011 23:36:20 -0400 Received: from [140.186.70.92] (port=36901 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q03zb-0006N7-10 for qemu-devel@nongnu.org; Wed, 16 Mar 2011 23:35:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q03zN-0007j3-Ou for qemu-devel@nongnu.org; Wed, 16 Mar 2011 23:35:07 -0400 Received: from mail.valinux.co.jp ([210.128.90.3]:55659) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q03zM-0007i4-M5 for qemu-devel@nongnu.org; Wed, 16 Mar 2011 23:35:05 -0400 Received: from ps.local.valinux.co.jp (vagw.valinux.co.jp [210.128.90.14]) by mail.valinux.co.jp (Postfix) with SMTP id 76410188FF; Thu, 17 Mar 2011 12:35:01 +0900 (JST) Received: (nullmailer pid 1701 invoked by uid 1000); Thu, 17 Mar 2011 03:35:01 -0000 Date: Thu, 17 Mar 2011 12:35:01 +0900 From: Isaku Yamahata To: Michael Tokarev Subject: Re: [Qemu-devel] RFC: ACPI table loading Message-ID: <20110317033501.GH16841@valinux.co.jp> References: <4D808D0B.302@msgid.tls.msk.ru> <20110316121039.GF16841@valinux.co.jp> <4D80BF55.5010208@msgid.tls.msk.ru> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4D80BF55.5010208@msgid.tls.msk.ru> User-Agent: Mutt/1.5.19 (2009-01-05) X-Virus-Scanned: clamav-milter 0.95.2 at va-mail.local.valinux.co.jp X-Virus-Status: Clean X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 210.128.90.3 Cc: qemu-devel@nongnu.org X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org The behavior seems reasonable. So I cleaned it up for upstream merge. thanks, From cd336e50ebda63ecd84f8172dcf4d4456059d615 Mon Sep 17 00:00:00 2001 Message-Id: From: Isaku Yamahata Date: Thu, 17 Mar 2011 12:28:52 +0900 Subject: [PATCH] acpi: rewamp acpitable parsing, and allow specifying complete file with headers This patch almost rewrites acpi_table_add() function (but still leaves it using old get_param_value() interface). The result is that it's now possible to specify whole table (together with a header) in an external file, instead of just data portion, with a new file= parameter, but at the same time it's still possible to specify header fields as before. Signed-off-by: Michael Tokarev Signed-off-by: Isaku Yamahata --- Changes v2: - rebased to 31d3c9b8c15d7b42f508d5fc2adc4abb7c732b70 - style fix - eliminated unaligned access - replace magic numbers with symbolic constants - error report --- hw/acpi.c | 294 ++++++++++++++++++++++++++++++++++++++----------------------- 1 files changed, 184 insertions(+), 110 deletions(-) diff --git a/hw/acpi.c b/hw/acpi.c index 8071e7b..b058e20 100644 --- a/hw/acpi.c +++ b/hw/acpi.c @@ -32,6 +32,30 @@ struct acpi_table_header uint32_t asl_compiler_revision; /* ASL compiler revision number */ } __attribute__((packed)); +#define ACPI_TABLE_OFF(x) (offsetof(struct acpi_table_header, x)) +#define ACPI_TABLE_SIZE(x) (sizeof(((struct acpi_table_header*)0)->x)) + +#define ACPI_TABLE_SIG_OFF ACPI_TABLE_OFF(signature) +#define ACPI_TABLE_SIG_SIZE ACPI_TABLE_SIZE(signature) +#define ACPI_TABLE_LEN_OFF ACPI_TABLE_OFF(length) +#define ACPI_TABLE_LEN_SIZE ACPI_TABLE_SIZE(length) +#define ACPI_TABLE_REV_OFF ACPI_TABLE_OFF(revision) +#define ACPI_TABLE_REV_SIZE ACPI_TABLE_SIZE(revision) +#define ACPI_TABLE_CSUM_OFF ACPI_TABLE_OFF(checksum) +#define ACPI_TABLE_CSUM_SIZE ACPI_TABLE_SIZE(checksum) +#define ACPI_TABLE_OEM_ID_OFF ACPI_TABLE_OFF(oem_id) +#define ACPI_TABLE_OEM_ID_SIZE ACPI_TABLE_SIZE(oem_id) +#define ACPI_TABLE_OEM_TABLE_ID_OFF ACPI_TABLE_OFF(oem_table_id) +#define ACPI_TABLE_OEM_TABLE_ID_SIZE ACPI_TABLE_SIZE(oem_table_id) +#define ACPI_TABLE_OEM_REV_OFF ACPI_TABLE_OFF(oem_revision) +#define ACPI_TABLE_OEM_REV_SIZE ACPI_TABLE_SIZE(oem_revision) +#define ACPI_TABLE_ASL_COMPILER_ID_OFF ACPI_TABLE_OFF(asl_compiler_id) +#define ACPI_TABLE_ASL_COMPILER_ID_SIZE ACPI_TABLE_SIZE(asl_compiler_id) +#define ACPI_TABLE_ASL_COMPILER_REV_OFF ACPI_TABLE_OFF(asl_compiler_revision) +#define ACPI_TABLE_ASL_COMPILER_REV_SIZE ACPI_TABLE_SIZE(asl_compiler_revision) + +#define ACPI_TABLE_HDR_SIZE sizeof(struct acpi_table_header) + char *acpi_tables; size_t acpi_tables_len; @@ -44,156 +68,206 @@ static int acpi_checksum(const uint8_t *data, int len) return (-sum) & 0xff; } +/* XXX fixme: this function uses obsolete argument parsing interface */ int acpi_table_add(const char *t) { static const char *dfl_id = "QEMUQEMU"; - char buf[1024], *p, *f; - struct acpi_table_header acpi_hdr; + char buf[1024], *f, *p; unsigned long val; - uint32_t length; - struct acpi_table_header *acpi_hdr_p; - size_t off; + uint16_t val16; + uint32_t val32; + size_t len, start; + bool has_header; + int changed; - memset(&acpi_hdr, 0, sizeof(acpi_hdr)); - - if (get_param_value(buf, sizeof(buf), "sig", t)) { - strncpy(acpi_hdr.signature, buf, 4); + if (get_param_value(buf, sizeof(buf), "data", t)) { + has_header = false; + } else if (get_param_value(buf, sizeof(buf), "file", t)) { + has_header = true; } else { - strncpy(acpi_hdr.signature, dfl_id, 4); + has_header = 0; + buf[0] = '\0'; + } + + if (!acpi_tables) { + acpi_tables_len = sizeof(uint16_t); + acpi_tables = qemu_mallocz(acpi_tables_len); + } + start = acpi_tables_len; + + len = sizeof(uint16_t) + ACPI_TABLE_HDR_SIZE; + acpi_tables = qemu_realloc(acpi_tables, acpi_tables_len + len); + acpi_tables_len += sizeof(uint16_t); + + if (!has_header) { + memset(acpi_tables + acpi_tables_len, 0, ACPI_TABLE_HDR_SIZE); + acpi_tables_len += ACPI_TABLE_HDR_SIZE; } + + /* now read in the data files, reallocating buffer as needed */ + for(f = strtok(buf, ":"); f; f = strtok(NULL, ":")) { + int fd = open(f, O_RDONLY); + if(fd < 0) { + fprintf(stderr, "can't open file %s: %s\n", f, strerror(errno)); + goto out; + } + + for(;;) { + char data[8192]; + ssize_t r = read(fd, data, sizeof(data)); + if (r == 0) { + break; + } else if (r > 0) { + acpi_tables = qemu_realloc(acpi_tables, acpi_tables_len + r); + memcpy(acpi_tables + acpi_tables_len, data, r); + acpi_tables_len += r; + } else if (errno != EINTR) { + fprintf(stderr, "can't read file %s: %s\n", + f, strerror(errno)); + close(fd); + goto out; + } + } + + close(fd); + } + + /* fill in the complete length of the table */ + len = acpi_tables_len - start - sizeof(uint16_t); + f = acpi_tables + start; + val16 = cpu_to_le16(len); + memcpy(f, &val16, sizeof(uint16_t)); + f += sizeof(uint16_t); + + /* now fill in the header fields */ + changed = 0; + + /* 0..3, signature, string (4 bytes) */ + if (get_param_value(buf, sizeof(buf), "sig", t)) { + strncpy(f + ACPI_TABLE_SIG_OFF, buf, ACPI_TABLE_SIG_SIZE); + ++changed; + } else if (!has_header) { + strncpy(f + ACPI_TABLE_SIG_OFF, dfl_id, ACPI_TABLE_SIG_SIZE); + } + + /* 4..7, length of the table, in bytes, including header (4 bytes) */ + + /* 8, ACPI specification minor version #, 1 byte */ if (get_param_value(buf, sizeof(buf), "rev", t)) { val = strtoul(buf, &p, 10); - if (val > 255 || *p != '\0') + if (val > 255 || *p != '\0') { + fprintf(stderr, "invalid acpi rev.\n"); goto out; - } else { - val = 1; + } + f[ACPI_TABLE_REV_OFF] = (uint8_t)val; + ++changed; + } else if (!has_header) { + f[ACPI_TABLE_REV_OFF] = 1; } - acpi_hdr.revision = (int8_t)val; + /* 9, checksum of entire table (1 byte) + this will be processed after all the headers are modified */ + + /* 10..15 OEM identification (6 bytes) */ if (get_param_value(buf, sizeof(buf), "oem_id", t)) { - strncpy(acpi_hdr.oem_id, buf, 6); - } else { - strncpy(acpi_hdr.oem_id, dfl_id, 6); + strncpy(f + ACPI_TABLE_OEM_ID_OFF, buf, ACPI_TABLE_OEM_ID_SIZE); + ++changed; + } else if (!has_header) { + strncpy(f + ACPI_TABLE_OEM_ID_OFF, dfl_id, ACPI_TABLE_OEM_ID_SIZE); } + /* 16..23 OEM table identifiaction, 8 bytes */ if (get_param_value(buf, sizeof(buf), "oem_table_id", t)) { - strncpy(acpi_hdr.oem_table_id, buf, 8); - } else { - strncpy(acpi_hdr.oem_table_id, dfl_id, 8); + strncpy(f + ACPI_TABLE_OEM_TABLE_ID_OFF, buf, + ACPI_TABLE_OEM_TABLE_ID_SIZE); + ++changed; + } else if (!has_header) { + strncpy(f + ACPI_TABLE_OEM_TABLE_ID_OFF, dfl_id, + ACPI_TABLE_OEM_TABLE_ID_SIZE); } + /* 24..27 OEM revision number, 4 bytes */ if (get_param_value(buf, sizeof(buf), "oem_rev", t)) { val = strtol(buf, &p, 10); - if(*p != '\0') + if(*p != '\0') { + fprintf(stderr, "invalid acpi oem_rev.\n"); goto out; - } else { - val = 1; + } + val32 = cpu_to_le32(val); + memcpy(f + ACPI_TABLE_OEM_REV_OFF, &val32, ACPI_TABLE_OEM_REV_SIZE); + ++changed; + } else if (!has_header) { + val32 = cpu_to_le32(1); + memcpy(f + ACPI_TABLE_OEM_REV_OFF, &val32, ACPI_TABLE_OEM_REV_SIZE); } - acpi_hdr.oem_revision = cpu_to_le32(val); + /* 28..31 ASL compiler vendor ID (4 bytes) */ if (get_param_value(buf, sizeof(buf), "asl_compiler_id", t)) { - strncpy(acpi_hdr.asl_compiler_id, buf, 4); - } else { - strncpy(acpi_hdr.asl_compiler_id, dfl_id, 4); + strncpy(f + ACPI_TABLE_ASL_COMPILER_ID_OFF, buf, + ACPI_TABLE_ASL_COMPILER_ID_SIZE); + ++changed; + } else if (!has_header) { + strncpy(f + ACPI_TABLE_ASL_COMPILER_ID_OFF, dfl_id, + ACPI_TABLE_ASL_COMPILER_ID_SIZE); } + /* 32..35 ASL compiler revision number (4 bytes) */ if (get_param_value(buf, sizeof(buf), "asl_compiler_rev", t)) { val = strtol(buf, &p, 10); - if(*p != '\0') - goto out; - } else { - val = 1; - } - acpi_hdr.asl_compiler_revision = cpu_to_le32(val); - - if (!get_param_value(buf, sizeof(buf), "data", t)) { - buf[0] = '\0'; - } - - length = sizeof(acpi_hdr); - - f = buf; - while (buf[0]) { - struct stat s; - char *n = strchr(f, ':'); - if (n) - *n = '\0'; - if(stat(f, &s) < 0) { - fprintf(stderr, "Can't stat file '%s': %s\n", f, strerror(errno)); + if(*p != '\0') { + fprintf(stderr, "invalid acpi asl_compiler_rev.\n"); goto out; } - length += s.st_size; - if (!n) - break; - *n = ':'; - f = n + 1; - } + val32 = cpu_to_le32(val); + memcpy(f + ACPI_TABLE_ASL_COMPILER_REV_OFF, &val32, + ACPI_TABLE_ASL_COMPILER_REV_SIZE); + ++changed; + } else if (!has_header) { + val32 = cpu_to_le32(1); + memcpy(f + ACPI_TABLE_ASL_COMPILER_REV_OFF, &val32, + ACPI_TABLE_ASL_COMPILER_REV_SIZE); - if (!acpi_tables) { - acpi_tables_len = sizeof(uint16_t); - acpi_tables = qemu_mallocz(acpi_tables_len); } - acpi_tables = qemu_realloc(acpi_tables, - acpi_tables_len + sizeof(uint16_t) + length); - p = acpi_tables + acpi_tables_len; - acpi_tables_len += sizeof(uint16_t) + length; - - *(uint16_t*)p = cpu_to_le32(length); - p += sizeof(uint16_t); - memcpy(p, &acpi_hdr, sizeof(acpi_hdr)); - off = sizeof(acpi_hdr); - - f = buf; - while (buf[0]) { - struct stat s; - int fd; - char *n = strchr(f, ':'); - if (n) - *n = '\0'; - fd = open(f, O_RDONLY); - - if(fd < 0) - goto out; - if(fstat(fd, &s) < 0) { - close(fd); - goto out; - } - /* off < length is necessary because file size can be changed - under our foot */ - while(s.st_size && off < length) { - int r; - r = read(fd, p + off, s.st_size); - if (r > 0) { - off += r; - s.st_size -= r; - } else if ((r < 0 && errno != EINTR) || r == 0) { - close(fd); - goto out; - } + /* 4..7 length of the table including header, in bytes (4 bytes) */ + if (!has_header) { + if (!changed) { + fprintf(stderr, + "warning: acpi table specified with data=" + " but no table headers are provided, defaults are used\n"); + } + } else { + /* check if actual length is correct */ + memcpy(&val32, f + ACPI_TABLE_LEN_OFF, ACPI_TABLE_LEN_SIZE); + val = le32_to_cpu(val32); + if (val != len) { + fprintf(stderr, + "warning: acpi table specified with file= has wrong length," + " header says %lu, actual size %zu\n", + val, len); + ++changed; } - - close(fd); - if (!n) - break; - f = n + 1; } - if (off < length) { - /* don't pass random value in process to guest */ - memset(p + off, 0, length - off); + + /* fix table length */ + /* we may avoid putting length here if has_header is true */ + val32 = cpu_to_le32(len); + memcpy(f + ACPI_TABLE_LEN_OFF, &val32, ACPI_TABLE_LEN_SIZE); + + /* 9 checksum (1 byte) */ + /* we may as well leave checksum intact if has_header is true */ + /* alternatively there may be a way to set cksum to a given value */ + if (changed || !has_header || 1) { + f[ACPI_TABLE_CSUM_OFF] = 0; + f[ACPI_TABLE_CSUM_OFF] = acpi_checksum((uint8_t*)f, len); } - acpi_hdr_p = (struct acpi_table_header*)p; - acpi_hdr_p->length = cpu_to_le32(length); - acpi_hdr_p->checksum = acpi_checksum((uint8_t*)p, length); /* increase number of tables */ (*(uint16_t*)acpi_tables) = - cpu_to_le32(le32_to_cpu(*(uint16_t*)acpi_tables) + 1); + cpu_to_le16(le16_to_cpu(*(uint16_t*)acpi_tables) + 1); return 0; + out: - if (acpi_tables) { - qemu_free(acpi_tables); - acpi_tables = NULL; - } + acpi_tables_len = start; return -1; }