[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] RFC: ACPI table loading
From: |
Michael Tokarev |
Subject: |
[Qemu-devel] RFC: ACPI table loading |
Date: |
Wed, 16 Mar 2011 16:47:01 +0300 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.1.16) Gecko/20110307 Icedove/3.0.11 |
16.03.2011 15:10, Isaku Yamahata wrote:
> On Wed, Mar 16, 2011 at 01:12:27PM +0300, Michael Tokarev wrote:
>> 16.03.2011 12:29, Isaku Yamahata wrote:
>>> Example:
>>> qemu-system-x86_64 ... -M pc_q35 -acpitable
>>> 'load_header,data=roms/seabios/src/q35-acpi-dsdt.aml
>>
>> My question is unrelated to your q35 work, but I have a suggestion
>> here: can we avoid this "load_header" thing please? I hacked this
>> area locally a while back while trying to run OEM-licensed windows
>> with a SLIC table in BIOS, and wanted to come with alternative
>> approach. Now when you reminded me that again I'd rather finish
>> that old thing and post a real patch...
>>
>> Here's my "idea". First, if there's no other options provided
>> except of data=..., just treat it as "headerful", ie, complete with
>> the header. Or alternatively (or at the same time), recognize
>> "file=" the same way as "data=". We can go even further and load
>> file/data first and patch in the other header field if specified,
>> so it'll be possible to overwrite only certain parts of the header
>> but load the rest of the table (complete with all other headers)
>> from a file.
>>
>> Does it make sense?
>
> It sounds reasonable. As long as the patch is acceptable,
> I'm willing to update the patch.
> Let me summarize it. Your suggestion for -acpitable is
>
> existing behavior your suggested way
> data= only
> no sig=,... header is filled with zero headerful
> (headerless) (new behaviour)
> useless behavior
>
> with sig=... header is created header is created
> (headerless) (headerless)
I just implemented the whole thing, and refined it at the same
time. With data= it works the same way as before, so no
new behavour is introduced. Only with file= it is possible
to specify whole table (with header) in a file, but other
header fields specified on the command line (sig= etc) will
work still, replacing the corresponding fields in the header
read from the file.
Something like the below, it's just an RFC, but it appears
to work.
/mjt
Subject: 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 <address@hidden>
diff --git a/hw/acpi.c b/hw/acpi.c
index 237526d..d12527e 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -22,18 +22,8 @@
#include "qemu-kvm.h"
#include "string.h"
-struct acpi_table_header
-{
- char signature [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 */
-} __attribute__((packed));
+
+#define ACPI_TABLE_HDR_SIZE (4+4+1+1+6+8+4+4+4)
char *acpi_tables;
size_t acpi_tables_len;
@@ -50,153 +40,220 @@ static int acpi_checksum(const uint8_t *data, int len)
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;
-
- memset(&acpi_hdr, 0, sizeof(acpi_hdr));
-
- if (get_param_value(buf, sizeof(buf), "sig", t)) {
- strncpy(acpi_hdr.signature, buf, 4);
- } else {
- strncpy(acpi_hdr.signature, dfl_id, 4);
- }
- if (get_param_value(buf, sizeof(buf), "rev", t)) {
+ size_t len, start;
+ bool has_header;
+ int changed;
+
+ /*XXX fixme: this function uses obsolete argument parsing interface */
+ /*XXX note: all 32bit accesses in there are misaligned */
+
+ if (get_param_value(buf, sizeof(buf), "data", t))
+ {
+ has_header = 0;
+ }
+ else if (get_param_value(buf, sizeof(buf), "file", t))
+ {
+ has_header = 1;
+ }
+ else {
+ 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)
+ {
+ /*XXX fixme: report error */
+ goto out;
+ }
+
+ for(;;)
+ {
+ char data[8192];
+ int 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)
+ {
+ /*XXX fixme: report error */
+ 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;
+ *(uint16_t*)f = cpu_to_le32(len);
+ 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 + 0, buf, 4);
+ ++changed;
+ }
+ else if (!has_header)
+ {
+ strncpy(f + 0, dfl_id, 4);
+ }
+
+ /* 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')
- goto out;
- } else {
- val = 1;
+ goto out; /*XXX fixme: report error */
+ f[8] = (uint8_t)val;
+ ++changed;
+ }
+ else if (!has_header)
+ {
+ f[8] = 1;
}
- acpi_hdr.revision = (int8_t)val;
- 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);
+ /* 9, checksum of entire table (1 byte) */
+
+ /* 10..15 OEM identification (6 bytes) */
+ if (get_param_value(buf, sizeof(buf), "oem_id", t))
+ {
+ strncpy(f + 10, buf, 6);
+ ++changed;
+ }
+ else if (!has_header)
+ {
+ strncpy(f + 10, dfl_id, 6);
}
- 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);
+ /* 16..23 OEM table identifiaction, 8 bytes */
+ if (get_param_value(buf, sizeof(buf), "oem_table_id", t))
+ {
+ strncpy(f + 16, buf, 8);
+ ++changed;
+ }
+ else if (!has_header)
+ {
+ strncpy(f + 16, dfl_id, 8);
}
- if (get_param_value(buf, sizeof(buf), "oem_rev", t)) {
+ /* 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')
- goto out;
- } else {
- val = 1;
+ goto out; /*XXX fixme: report error */
+ *(uint32_t*)(f + 24) = cpu_to_le32(val);
+ ++changed;
+ } else if (!has_header)
+ {
+ *(uint32_t*)(f + 24) = cpu_to_le32(1);
}
- acpi_hdr.oem_revision = cpu_to_le32(val);
- 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);
+ /* 28..31 ASL compiler vendor ID (4 bytes) */
+ if (get_param_value(buf, sizeof(buf), "asl_compiler_id", t))
+ {
+ strncpy(f + 28, buf, 4);
+ ++changed;
+ }
+ else if (!has_header)
+ {
+ strncpy(f + 28, dfl_id, 4);
}
- if (get_param_value(buf, sizeof(buf), "asl_compiler_rev", t)) {
+ /* 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;
+ goto out; /*XXX fixme: report error */
+ *(uint32_t*)(f + 32) = cpu_to_le32(val);
+ ++changed;
}
- acpi_hdr.asl_compiler_revision = cpu_to_le32(val);
-
- if (!get_param_value(buf, sizeof(buf), "data", t)) {
- buf[0] = '\0';
+ else if (!has_header) {
+ *(uint32_t*)(f + 32) = cpu_to_le32(1);
}
- length = sizeof(acpi_hdr);
+ /* 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 */
+ val = le32_to_cpu(*(uint32_t*)(f + 4));
+ if (val != len)
+ {
+ fprintf(stderr,
+ "warning: acpi table specified with file= has wrong length,"
+ " header says %lu, actual size %u\n",
+ val, len);
+ ++changed;
+ }
+ }
- 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));
- goto out;
- }
- length += s.st_size;
- if (!n)
- break;
- *n = ':';
- f = n + 1;
- }
-
- 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);
+ /* fix table length */
+ /* we may avoid putting length here if has_header is true */
+ *(uint32_t*)(f + 4) = cpu_to_le32(len);
+
+ /* 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[9] = 0;
+ f[9] = acpi_checksum((uint8_t*)f, len);
+ }
- 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;
- }
- }
-
- 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);
- }
-
- 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);
return 0;
+
out:
- if (acpi_tables) {
- qemu_free(acpi_tables);
- acpi_tables = NULL;
- }
+ acpi_tables_len = start;
return -1;
}
diff --git a/qemu-options.hx b/qemu-options.hx
index 18f54d2..e1d26b4 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -995,12 +995,17 @@ Enable virtio balloon device (default), optionally with
PCI address
ETEXI
DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
- "-acpitable
[sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compiler_id=str][,asl_compiler_rev=n][,data=file1[:file2]...]\n"
+ "-acpitable
[sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compiler_id=str][,asl_compiler_rev=n][,{data|file}=file1[:file2]...]\n"
" ACPI table description\n", QEMU_ARCH_I386)
STEXI
@item -acpitable
address@hidden,address@hidden,address@hidden,address@hidden,address@hidden
[,address@hidden,address@hidden,address@hidden:@var{file2}]...]
@findex -acpitable
Add ACPI table with specified header fields and context from specified files.
+For file=, take whole ACPI table from the specified files, including all
+ACPI headers (possible overridden by other options).
+For data=, only data
+portion of the table is used, all header information is specified in the
+command line.
ETEXI
DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
- Re: [Qemu-devel] Re: [PATCH 07/26] pci/p2pbr: generic pci p2p bridge, (continued)
[Qemu-devel] [PATCH 19/26] pc/piix_pci: factor out smram/pam logic, Isaku Yamahata, 2011/03/16
[Qemu-devel] [PATCH 12/26] usb/uhci: generalize initialization, Isaku Yamahata, 2011/03/16
[Qemu-devel] [PATCH 26/26] pc q35 based chipset emulator, Isaku Yamahata, 2011/03/16
[Qemu-devel] [PATCH 05/26] piix_pci: eliminate PIIX3State::pci_irq_levels, Isaku Yamahata, 2011/03/16
[Qemu-devel] ACPI table loading [was: q35 chipset support for native pci express support], Michael Tokarev, 2011/03/16