Message ID | 20110729014943.GK14976@valinux.co.jp |
---|---|
State | New |
Headers | show |
29.07.2011 05:49, Isaku Yamahata wrote: > On Fri, Jul 15, 2011 at 07:51:43PM +0300, Blue Swirl wrote: >> On Fri, Jul 15, 2011 at 6:18 PM, John Baboval >> <john.baboval@virtualcomputer.com> wrote: >>> Is there something I can do to help take this patch the rest of the way? >>> >>> I'd hate to see it die because of a style issue and a compiler warning. >> >> There's also suspicious missing 'break' statement. How about fixing >> the issues and submitting the patch? > > I fixed the compile error. Oh, another one? :) > I think the missing break is intentional, so added an comment there. Michael? Yes it's intentional, you're right. > Blue, can you please take a look of it? I think it's hopeless. /mjt
Thanks, applied. On Fri, Jul 29, 2011 at 4:49 AM, Isaku Yamahata <yamahata@valinux.co.jp> wrote: > On Fri, Jul 15, 2011 at 07:51:43PM +0300, Blue Swirl wrote: >> On Fri, Jul 15, 2011 at 6:18 PM, John Baboval >> <john.baboval@virtualcomputer.com> wrote: >> > Is there something I can do to help take this patch the rest of the way? >> > >> > I'd hate to see it die because of a style issue and a compiler warning. >> >> There's also suspicious missing 'break' statement. How about fixing >> the issues and submitting the patch? > > I fixed the compile error. > I think the missing break is intentional, so added an comment there. Michael? > Blue, can you please take a look of it? > > > From 9a5e4158074ea251ab064a946927bcaf861f5c1e Mon Sep 17 00:00:00 2001 > Message-Id: <9a5e4158074ea251ab064a946927bcaf861f5c1e.1311903724.git.yamahata@valinux.co.jp> > From: Michael Tokarev <mjt@tls.msk.ru> > Date: Thu, 12 May 2011 18:44:17 +0400 > Subject: [PATCH] revamp acpitable parsing and allow to specify complete (headerful) table > > From Michael Tokarev <mjt@tls.msk.ru> > > 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. > > Now with the checkpatch.pl formatting fixes, thanks to > Stefan Hajnoczi for suggestions, with changes from > Isaku Yamahata, and with my further refinements. > > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> > Cc:: Isaku Yamahata <yamahata@valinux.co.jp> > Cc: John Baboval <john.baboval@virtualcomputer.com> > Cc: Blue Swirl <blauwirbel@gmail.com> > > --- > v5: rediffed against current qemu/master. > v6: fix one "} else {" coding style defect (noted by Blue Swirl) > v7: style fix and added an comment for suspicious break > I think that the missing break of case 0 is intentional. > I added the fallthrough comment there. > --- > hw/acpi.c | 298 ++++++++++++++++++++++++++++++++----------------------- > qemu-options.hx | 7 +- > 2 files changed, 178 insertions(+), 127 deletions(-) > > diff --git a/hw/acpi.c b/hw/acpi.c > index ad40fb4..79ec66c 100644 > --- a/hw/acpi.c > +++ b/hw/acpi.c > @@ -20,19 +20,30 @@ > #include "pc.h" > #include "acpi.h" > > -struct acpi_table_header > -{ > - char signature [4]; /* ACPI signature (4 ASCII characters) */ > +struct acpi_table_header { > + uint16_t _length; /* our length, not actual part of the hdr */ > + /* XXX why we have 2 length fields here? */ > + 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 */ > + 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 */ > + 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 sizeof(struct acpi_table_header) > +#define ACPI_TABLE_PFX_SIZE sizeof(uint16_t) /* size of the extra prefix */ > + > +static const char dfl_hdr[ACPI_TABLE_HDR_SIZE] = > + "\0\0" /* fake _length (2) */ > + "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) */ > + ; > + > char *acpi_tables; > size_t acpi_tables_len; > > @@ -40,163 +51,198 @@ static int acpi_checksum(const uint8_t *data, int len) > { > int sum, i; > sum = 0; > - for(i = 0; i < len; i++) > + for (i = 0; i < len; i++) { > sum += data[i]; > + } > return (-sum) & 0xff; > } > > +/* like strncpy() but zero-fills the tail of destination */ > +static void strzcpy(char *dst, const char *src, size_t size) > +{ > + size_t len = strlen(src); > + if (len >= size) { > + len = size; > + } else { > + memset(dst + len, 0, size - len); > + } > + memcpy(dst, src, len); > +} > + > +/* 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; > unsigned long val; > - uint32_t length; > - struct acpi_table_header *acpi_hdr_p; > - size_t off; > + size_t len, start, allen; > + bool has_header; > + int changed; > + int r; > + struct acpi_table_header hdr; > + > + r = 0; > + r |= get_param_value(buf, sizeof(buf), "data", t) ? 1 : 0; > + r |= get_param_value(buf, sizeof(buf), "file", t) ? 2 : 0; > + switch (r) { > + case 0: > + buf[0] = '\0'; > + /* fallthrough for default behavior */ > + case 1: > + has_header = false; > + break; > + case 2: > + has_header = true; > + break; > + default: > + fprintf(stderr, "acpitable: both data and file are specified\n"); > + return -1; > + } > > - memset(&acpi_hdr, 0, sizeof(acpi_hdr)); > - > - if (get_param_value(buf, sizeof(buf), "sig", t)) { > - strncpy(acpi_hdr.signature, buf, 4); > + if (!acpi_tables) { > + allen = sizeof(uint16_t); > + acpi_tables = qemu_mallocz(allen); > } else { > - strncpy(acpi_hdr.signature, dfl_id, 4); > + allen = acpi_tables_len; > } > + > + start = allen; > + acpi_tables = qemu_realloc(acpi_tables, start + ACPI_TABLE_HDR_SIZE); > + allen += has_header ? ACPI_TABLE_PFX_SIZE : 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)); > + return -1; > + } > + > + for (;;) { > + char data[8192]; > + r = read(fd, data, sizeof(data)); > + if (r == 0) { > + break; > + } else if (r > 0) { > + acpi_tables = qemu_realloc(acpi_tables, allen + r); > + memcpy(acpi_tables + allen, data, r); > + allen += r; > + } else if (errno != EINTR) { > + fprintf(stderr, "can't read file %s: %s\n", > + f, strerror(errno)); > + close(fd); > + return -1; > + } > + } > + > + close(fd); > + } > + > + /* now fill in the header fields */ > + > + f = acpi_tables + start; /* start of the table */ > + changed = 0; > + > + /* copy the header to temp place to align the fields */ > + memcpy(&hdr, has_header ? f : dfl_hdr, ACPI_TABLE_HDR_SIZE); > + > + /* length of the table minus our prefix */ > + len = allen - start - ACPI_TABLE_PFX_SIZE; > + > + hdr._length = cpu_to_le16(len); > + > + if (get_param_value(buf, sizeof(buf), "sig", t)) { > + strzcpy(hdr.sig, buf, sizeof(hdr.sig)); > + ++changed; > + } > + > + /* length of the table including header, in bytes */ > + if (has_header) { > + /* check if actual length is correct */ > + val = le32_to_cpu(hdr.length); > + if (val != len) { > + fprintf(stderr, > + "warning: acpitable has wrong length," > + " header says %lu, actual size %zu bytes\n", > + val, len); > + ++changed; > + } > + } > + /* we may avoid putting length here if has_header is true */ > + hdr.length = cpu_to_le32(len); > + > if (get_param_value(buf, sizeof(buf), "rev", t)) { > - val = strtoul(buf, &p, 10); > - if (val > 255 || *p != '\0') > - goto out; > - } else { > - val = 1; > + val = strtoul(buf, &p, 0); > + if (val > 255 || *p) { > + fprintf(stderr, "acpitable: \"rev=%s\" is invalid\n", buf); > + return -1; > + } > + hdr.revision = (uint8_t)val; > + ++changed; > } > - 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); > + strzcpy(hdr.oem_id, buf, sizeof(hdr.oem_id)); > + ++changed; > } > > 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); > + strzcpy(hdr.oem_table_id, buf, sizeof(hdr.oem_table_id)); > + ++changed; > } > > if (get_param_value(buf, sizeof(buf), "oem_rev", t)) { > - val = strtol(buf, &p, 10); > - if(*p != '\0') > - goto out; > - } else { > - val = 1; > + val = strtol(buf, &p, 0); > + if (*p) { > + fprintf(stderr, "acpitable: \"oem_rev=%s\" is invalid\n", buf); > + return -1; > + } > + hdr.oem_revision = cpu_to_le32(val); > + ++changed; > } > - 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); > + strzcpy(hdr.asl_compiler_id, buf, sizeof(hdr.asl_compiler_id)); > + ++changed; > } > > 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)); > - goto out; > + val = strtol(buf, &p, 0); > + if (*p) { > + fprintf(stderr, "acpitable: \"%s=%s\" is invalid\n", > + "asl_compiler_rev", buf); > + return -1; > } > - length += s.st_size; > - if (!n) > - break; > - *n = ':'; > - f = n + 1; > + hdr.asl_compiler_revision = cpu_to_le32(val); > + ++changed; > } > > - if (!acpi_tables) { > - acpi_tables_len = sizeof(uint16_t); > - acpi_tables = qemu_mallocz(acpi_tables_len); > + if (!has_header && !changed) { > + fprintf(stderr, "warning: acpitable: no table headers are specified\n"); > } > - 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; > - } > - } > > - 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); > + /* now calculate checksum of the table, complete with the header */ > + /* 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 */ > + hdr.checksum = 0; /* for checksum calculation */ > + > + /* put header back */ > + memcpy(f, &hdr, sizeof(hdr)); > + > + if (changed || !has_header || 1) { > + ((struct acpi_table_header *)f)->checksum = > + acpi_checksum((uint8_t *)f + ACPI_TABLE_PFX_SIZE, 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); > + (*(uint16_t *)acpi_tables) = > + cpu_to_le32(le32_to_cpu(*(uint16_t *)acpi_tables) + 1); > + > + acpi_tables_len = allen; > return 0; > -out: > - if (acpi_tables) { > - qemu_free(acpi_tables); > - acpi_tables = NULL; > - } > - return -1; > + > } > > /* ACPI PM1a EVT */ > diff --git a/qemu-options.hx b/qemu-options.hx > index 1d57f64..74c0edb 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -1062,12 +1062,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 [sig=@var{str}][,rev=@var{n}][,oem_id=@var{str}][,oem_table_id=@var{str}][,oem_rev=@var{n}] [,asl_compiler_id=@var{str}][,asl_compiler_rev=@var{n}][,data=@var{file1}[:@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, > -- > 1.7.1.1 > > > > -- > yamahata >
Thanks. Somehow completely missed Blue's response back on the 15th. Glad to see this in. When using this for SLIC I had to patch the OEM ID fields in the other tables to match at runtime in order to make windows licensing happy. But thats a BIOS change, not something in QEMU.... On Jul 30, 2011, at 2:41 AM, "Blue Swirl" <blauwirbel@gmail.com> wrote: > Thanks, applied. > > On Fri, Jul 29, 2011 at 4:49 AM, Isaku Yamahata <yamahata@valinux.co.jp> wrote: >> On Fri, Jul 15, 2011 at 07:51:43PM +0300, Blue Swirl wrote: >>> On Fri, Jul 15, 2011 at 6:18 PM, John Baboval >>> <john.baboval@virtualcomputer.com> wrote: >>>> Is there something I can do to help take this patch the rest of the way? >>>> >>>> I'd hate to see it die because of a style issue and a compiler warning. >>> >>> There's also suspicious missing 'break' statement. How about fixing >>> the issues and submitting the patch? >> >> I fixed the compile error. >> I think the missing break is intentional, so added an comment there. Michael? >> Blue, can you please take a look of it? >> >> >> From 9a5e4158074ea251ab064a946927bcaf861f5c1e Mon Sep 17 00:00:00 2001 >> Message-Id: <9a5e4158074ea251ab064a946927bcaf861f5c1e.1311903724.git.yamahata@valinux.co.jp> >> From: Michael Tokarev <mjt@tls.msk.ru> >> Date: Thu, 12 May 2011 18:44:17 +0400 >> Subject: [PATCH] revamp acpitable parsing and allow to specify complete (headerful) table >> >> From Michael Tokarev <mjt@tls.msk.ru> >> >> 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. >> >> Now with the checkpatch.pl formatting fixes, thanks to >> Stefan Hajnoczi for suggestions, with changes from >> Isaku Yamahata, and with my further refinements. >> >> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> >> Cc:: Isaku Yamahata <yamahata@valinux.co.jp> >> Cc: John Baboval <john.baboval@virtualcomputer.com> >> Cc: Blue Swirl <blauwirbel@gmail.com> >> >> --- >> v5: rediffed against current qemu/master. >> v6: fix one "} else {" coding style defect (noted by Blue Swirl) >> v7: style fix and added an comment for suspicious break >> I think that the missing break of case 0 is intentional. >> I added the fallthrough comment there. >> --- >> hw/acpi.c | 298 ++++++++++++++++++++++++++++++++----------------------- >> qemu-options.hx | 7 +- >> 2 files changed, 178 insertions(+), 127 deletions(-) >> >> diff --git a/hw/acpi.c b/hw/acpi.c >> index ad40fb4..79ec66c 100644 >> --- a/hw/acpi.c >> +++ b/hw/acpi.c >> @@ -20,19 +20,30 @@ >> #include "pc.h" >> #include "acpi.h" >> >> -struct acpi_table_header >> -{ >> - char signature [4]; /* ACPI signature (4 ASCII characters) */ >> +struct acpi_table_header { >> + uint16_t _length; /* our length, not actual part of the hdr */ >> + /* XXX why we have 2 length fields here? */ >> + 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 */ >> + 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 */ >> + 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 sizeof(struct acpi_table_header) >> +#define ACPI_TABLE_PFX_SIZE sizeof(uint16_t) /* size of the extra prefix */ >> + >> +static const char dfl_hdr[ACPI_TABLE_HDR_SIZE] = >> + "\0\0" /* fake _length (2) */ >> + "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) */ >> + ; >> + >> char *acpi_tables; >> size_t acpi_tables_len; >> >> @@ -40,163 +51,198 @@ static int acpi_checksum(const uint8_t *data, int len) >> { >> int sum, i; >> sum = 0; >> - for(i = 0; i < len; i++) >> + for (i = 0; i < len; i++) { >> sum += data[i]; >> + } >> return (-sum) & 0xff; >> } >> >> +/* like strncpy() but zero-fills the tail of destination */ >> +static void strzcpy(char *dst, const char *src, size_t size) >> +{ >> + size_t len = strlen(src); >> + if (len >= size) { >> + len = size; >> + } else { >> + memset(dst + len, 0, size - len); >> + } >> + memcpy(dst, src, len); >> +} >> + >> +/* 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; >> unsigned long val; >> - uint32_t length; >> - struct acpi_table_header *acpi_hdr_p; >> - size_t off; >> + size_t len, start, allen; >> + bool has_header; >> + int changed; >> + int r; >> + struct acpi_table_header hdr; >> + >> + r = 0; >> + r |= get_param_value(buf, sizeof(buf), "data", t) ? 1 : 0; >> + r |= get_param_value(buf, sizeof(buf), "file", t) ? 2 : 0; >> + switch (r) { >> + case 0: >> + buf[0] = '\0'; >> + /* fallthrough for default behavior */ >> + case 1: >> + has_header = false; >> + break; >> + case 2: >> + has_header = true; >> + break; >> + default: >> + fprintf(stderr, "acpitable: both data and file are specified\n"); >> + return -1; >> + } >> >> - memset(&acpi_hdr, 0, sizeof(acpi_hdr)); >> - >> - if (get_param_value(buf, sizeof(buf), "sig", t)) { >> - strncpy(acpi_hdr.signature, buf, 4); >> + if (!acpi_tables) { >> + allen = sizeof(uint16_t); >> + acpi_tables = qemu_mallocz(allen); >> } else { >> - strncpy(acpi_hdr.signature, dfl_id, 4); >> + allen = acpi_tables_len; >> } >> + >> + start = allen; >> + acpi_tables = qemu_realloc(acpi_tables, start + ACPI_TABLE_HDR_SIZE); >> + allen += has_header ? ACPI_TABLE_PFX_SIZE : 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)); >> + return -1; >> + } >> + >> + for (;;) { >> + char data[8192]; >> + r = read(fd, data, sizeof(data)); >> + if (r == 0) { >> + break; >> + } else if (r > 0) { >> + acpi_tables = qemu_realloc(acpi_tables, allen + r); >> + memcpy(acpi_tables + allen, data, r); >> + allen += r; >> + } else if (errno != EINTR) { >> + fprintf(stderr, "can't read file %s: %s\n", >> + f, strerror(errno)); >> + close(fd); >> + return -1; >> + } >> + } >> + >> + close(fd); >> + } >> + >> + /* now fill in the header fields */ >> + >> + f = acpi_tables + start; /* start of the table */ >> + changed = 0; >> + >> + /* copy the header to temp place to align the fields */ >> + memcpy(&hdr, has_header ? f : dfl_hdr, ACPI_TABLE_HDR_SIZE); >> + >> + /* length of the table minus our prefix */ >> + len = allen - start - ACPI_TABLE_PFX_SIZE; >> + >> + hdr._length = cpu_to_le16(len); >> + >> + if (get_param_value(buf, sizeof(buf), "sig", t)) { >> + strzcpy(hdr.sig, buf, sizeof(hdr.sig)); >> + ++changed; >> + } >> + >> + /* length of the table including header, in bytes */ >> + if (has_header) { >> + /* check if actual length is correct */ >> + val = le32_to_cpu(hdr.length); >> + if (val != len) { >> + fprintf(stderr, >> + "warning: acpitable has wrong length," >> + " header says %lu, actual size %zu bytes\n", >> + val, len); >> + ++changed; >> + } >> + } >> + /* we may avoid putting length here if has_header is true */ >> + hdr.length = cpu_to_le32(len); >> + >> if (get_param_value(buf, sizeof(buf), "rev", t)) { >> - val = strtoul(buf, &p, 10); >> - if (val > 255 || *p != '\0') >> - goto out; >> - } else { >> - val = 1; >> + val = strtoul(buf, &p, 0); >> + if (val > 255 || *p) { >> + fprintf(stderr, "acpitable: \"rev=%s\" is invalid\n", buf); >> + return -1; >> + } >> + hdr.revision = (uint8_t)val; >> + ++changed; >> } >> - 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); >> + strzcpy(hdr.oem_id, buf, sizeof(hdr.oem_id)); >> + ++changed; >> } >> >> 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); >> + strzcpy(hdr.oem_table_id, buf, sizeof(hdr.oem_table_id)); >> + ++changed; >> } >> >> if (get_param_value(buf, sizeof(buf), "oem_rev", t)) { >> - val = strtol(buf, &p, 10); >> - if(*p != '\0') >> - goto out; >> - } else { >> - val = 1; >> + val = strtol(buf, &p, 0); >> + if (*p) { >> + fprintf(stderr, "acpitable: \"oem_rev=%s\" is invalid\n", buf); >> + return -1; >> + } >> + hdr.oem_revision = cpu_to_le32(val); >> + ++changed; >> } >> - 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); >> + strzcpy(hdr.asl_compiler_id, buf, sizeof(hdr.asl_compiler_id)); >> + ++changed; >> } >> >> 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)); >> - goto out; >> + val = strtol(buf, &p, 0); >> + if (*p) { >> + fprintf(stderr, "acpitable: \"%s=%s\" is invalid\n", >> + "asl_compiler_rev", buf); >> + return -1; >> } >> - length += s.st_size; >> - if (!n) >> - break; >> - *n = ':'; >> - f = n + 1; >> + hdr.asl_compiler_revision = cpu_to_le32(val); >> + ++changed; >> } >> >> - if (!acpi_tables) { >> - acpi_tables_len = sizeof(uint16_t); >> - acpi_tables = qemu_mallocz(acpi_tables_len); >> + if (!has_header && !changed) { >> + fprintf(stderr, "warning: acpitable: no table headers are specified\n"); >> } >> - 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; >> - } >> - } >> >> - 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); >> + /* now calculate checksum of the table, complete with the header */ >> + /* 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 */ >> + hdr.checksum = 0; /* for checksum calculation */ >> + >> + /* put header back */ >> + memcpy(f, &hdr, sizeof(hdr)); >> + >> + if (changed || !has_header || 1) { >> + ((struct acpi_table_header *)f)->checksum = >> + acpi_checksum((uint8_t *)f + ACPI_TABLE_PFX_SIZE, 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); >> + (*(uint16_t *)acpi_tables) = >> + cpu_to_le32(le32_to_cpu(*(uint16_t *)acpi_tables) + 1); >> + >> + acpi_tables_len = allen; >> return 0; >> -out: >> - if (acpi_tables) { >> - qemu_free(acpi_tables); >> - acpi_tables = NULL; >> - } >> - return -1; >> + >> } >> >> /* ACPI PM1a EVT */ >> diff --git a/qemu-options.hx b/qemu-options.hx >> index 1d57f64..74c0edb 100644 >> --- a/qemu-options.hx >> +++ b/qemu-options.hx >> @@ -1062,12 +1062,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 [sig=@var{str}][,rev=@var{n}][,oem_id=@var{str}][,oem_table_id=@var{str}][,oem_rev=@var{n}] [,asl_compiler_id=@var{str}][,asl_compiler_rev=@var{n}][,data=@var{file1}[:@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, >> -- >> 1.7.1.1 >> >> >> >> -- >> yamahata >>
diff --git a/hw/acpi.c b/hw/acpi.c index ad40fb4..79ec66c 100644 --- a/hw/acpi.c +++ b/hw/acpi.c @@ -20,19 +20,30 @@ #include "pc.h" #include "acpi.h" -struct acpi_table_header -{ - char signature [4]; /* ACPI signature (4 ASCII characters) */ +struct acpi_table_header { + uint16_t _length; /* our length, not actual part of the hdr */ + /* XXX why we have 2 length fields here? */ + 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 */ + 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 */ + 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 sizeof(struct acpi_table_header) +#define ACPI_TABLE_PFX_SIZE sizeof(uint16_t) /* size of the extra prefix */ + +static const char dfl_hdr[ACPI_TABLE_HDR_SIZE] = + "\0\0" /* fake _length (2) */ + "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) */ + ; + char *acpi_tables; size_t acpi_tables_len; @@ -40,163 +51,198 @@ static int acpi_checksum(const uint8_t *data, int len) { int sum, i; sum = 0; - for(i = 0; i < len; i++) + for (i = 0; i < len; i++) { sum += data[i]; + } return (-sum) & 0xff; } +/* like strncpy() but zero-fills the tail of destination */ +static void strzcpy(char *dst, const char *src, size_t size) +{ + size_t len = strlen(src); + if (len >= size) { + len = size; + } else { + memset(dst + len, 0, size - len); + } + memcpy(dst, src, len); +} + +/* 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; unsigned long val; - uint32_t length; - struct acpi_table_header *acpi_hdr_p; - size_t off; + size_t len, start, allen; + bool has_header; + int changed; + int r; + struct acpi_table_header hdr; + + r = 0; + r |= get_param_value(buf, sizeof(buf), "data", t) ? 1 : 0; + r |= get_param_value(buf, sizeof(buf), "file", t) ? 2 : 0; + switch (r) { + case 0: + buf[0] = '\0'; + /* fallthrough for default behavior */ + case 1: + has_header = false; + break; + case 2: + has_header = true; + break; + default: + fprintf(stderr, "acpitable: both data and file are specified\n"); + return -1; + } - memset(&acpi_hdr, 0, sizeof(acpi_hdr)); - - if (get_param_value(buf, sizeof(buf), "sig", t)) { - strncpy(acpi_hdr.signature, buf, 4); + if (!acpi_tables) { + allen = sizeof(uint16_t); + acpi_tables = qemu_mallocz(allen); } else { - strncpy(acpi_hdr.signature, dfl_id, 4); + allen = acpi_tables_len; } + + start = allen; + acpi_tables = qemu_realloc(acpi_tables, start + ACPI_TABLE_HDR_SIZE); + allen += has_header ? ACPI_TABLE_PFX_SIZE : 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)); + return -1; + } + + for (;;) { + char data[8192]; + r = read(fd, data, sizeof(data)); + if (r == 0) { + break; + } else if (r > 0) { + acpi_tables = qemu_realloc(acpi_tables, allen + r); + memcpy(acpi_tables + allen, data, r); + allen += r; + } else if (errno != EINTR) { + fprintf(stderr, "can't read file %s: %s\n", + f, strerror(errno)); + close(fd); + return -1; + } + } + + close(fd); + } + + /* now fill in the header fields */ + + f = acpi_tables + start; /* start of the table */ + changed = 0; + + /* copy the header to temp place to align the fields */ + memcpy(&hdr, has_header ? f : dfl_hdr, ACPI_TABLE_HDR_SIZE); + + /* length of the table minus our prefix */ + len = allen - start - ACPI_TABLE_PFX_SIZE; + + hdr._length = cpu_to_le16(len); + + if (get_param_value(buf, sizeof(buf), "sig", t)) { + strzcpy(hdr.sig, buf, sizeof(hdr.sig)); + ++changed; + } + + /* length of the table including header, in bytes */ + if (has_header) { + /* check if actual length is correct */ + val = le32_to_cpu(hdr.length); + if (val != len) { + fprintf(stderr, + "warning: acpitable has wrong length," + " header says %lu, actual size %zu bytes\n", + val, len); + ++changed; + } + } + /* we may avoid putting length here if has_header is true */ + hdr.length = cpu_to_le32(len); + if (get_param_value(buf, sizeof(buf), "rev", t)) { - val = strtoul(buf, &p, 10); - if (val > 255 || *p != '\0') - goto out; - } else { - val = 1; + val = strtoul(buf, &p, 0); + if (val > 255 || *p) { + fprintf(stderr, "acpitable: \"rev=%s\" is invalid\n", buf); + return -1; + } + hdr.revision = (uint8_t)val; + ++changed; } - 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); + strzcpy(hdr.oem_id, buf, sizeof(hdr.oem_id)); + ++changed; } 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); + strzcpy(hdr.oem_table_id, buf, sizeof(hdr.oem_table_id)); + ++changed; } if (get_param_value(buf, sizeof(buf), "oem_rev", t)) { - val = strtol(buf, &p, 10); - if(*p != '\0') - goto out; - } else { - val = 1; + val = strtol(buf, &p, 0); + if (*p) { + fprintf(stderr, "acpitable: \"oem_rev=%s\" is invalid\n", buf); + return -1; + } + hdr.oem_revision = cpu_to_le32(val); + ++changed; } - 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); + strzcpy(hdr.asl_compiler_id, buf, sizeof(hdr.asl_compiler_id)); + ++changed; } 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)); - goto out; + val = strtol(buf, &p, 0); + if (*p) { + fprintf(stderr, "acpitable: \"%s=%s\" is invalid\n", + "asl_compiler_rev", buf); + return -1; } - length += s.st_size; - if (!n) - break; - *n = ':'; - f = n + 1; + hdr.asl_compiler_revision = cpu_to_le32(val); + ++changed; } - if (!acpi_tables) { - acpi_tables_len = sizeof(uint16_t); - acpi_tables = qemu_mallocz(acpi_tables_len); + if (!has_header && !changed) { + fprintf(stderr, "warning: acpitable: no table headers are specified\n"); } - 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; - } - } - 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); + /* now calculate checksum of the table, complete with the header */ + /* 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 */ + hdr.checksum = 0; /* for checksum calculation */ + + /* put header back */ + memcpy(f, &hdr, sizeof(hdr)); + + if (changed || !has_header || 1) { + ((struct acpi_table_header *)f)->checksum = + acpi_checksum((uint8_t *)f + ACPI_TABLE_PFX_SIZE, 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); + (*(uint16_t *)acpi_tables) = + cpu_to_le32(le32_to_cpu(*(uint16_t *)acpi_tables) + 1); + + acpi_tables_len = allen; return 0; -out: - if (acpi_tables) { - qemu_free(acpi_tables); - acpi_tables = NULL; - } - return -1; + } /* ACPI PM1a EVT */ diff --git a/qemu-options.hx b/qemu-options.hx index 1d57f64..74c0edb 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1062,12 +1062,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 [sig=@var{str}][,rev=@var{n}][,oem_id=@var{str}][,oem_table_id=@var{str}][,oem_rev=@var{n}] [,asl_compiler_id=@var{str}][,asl_compiler_rev=@var{n}][,data=@var{file1}[:@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,