Re: [Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
Wow you're replying to an old post... ;)
17.01.2013 13:50, TeLeMan wrote:
On Thu, May 12, 2011 at 10:44 PM, Michael Tokarev wrote:
This patch almost rewrites acpi_table_add() function
[]
+
+/* now read in the data files, reallocating buffer as needed */
+
+for (f = strtok(buf, ":"); f; f = strtok(NULL, ":")) {
+int fd = open(f, O_RDONLY);
The acpi table is the binary file, so it should be opened by O_RDONLY
| O_BINARY.
On Unix, O_BINARY is defined as 0, it has no effect whatsoever.
But yes it is a bug on other platforms. I'll fix that in a moment.
Thanks,
/mnt
Re: [Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
On Thu, May 12, 2011 at 10:44 PM, Michael Tokarev wrote:
> 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.
>
> v5: rediffed against current qemu/master.
> v6: fix one "} else {" coding style defect
>
> Signed-off-by: Michael Tokarev
> ---
> hw/acpi.c | 291
> ---
> qemu-options.hx |7 +-
> 2 files changed, 174 insertions(+), 124 deletions(-)
>
> diff --git a/hw/acpi.c b/hw/acpi.c
> index ad40fb4..b8cd866 100644
> --- a/hw/acpi.c
> +++ b/hw/acpi.c
> @@ -22,17 +22,29 @@
>
> struct acpi_table_header
> {
> -char signature [4];/* ACPI signature (4 ASCII characters) */
> +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;
>
> @@ -45,158 +57,191 @@ static int acpi_checksum(const uint8_t *data, int len)
> 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';
> +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);
The acpi table is the binary file, so it should be opened by O_RDONLY
| O_BINARY.
> +
> +if (fd < 0) {
> +fprintf(stderr, "can't open file %s: %s\n", f, strerror(errno));
> +return -1;
> +}
>
Re: [Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
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" wrote:
> Thanks, applied.
>
> On Fri, Jul 29, 2011 at 4:49 AM, 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
>>> 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.yamah...@valinux.co.jp>
>> From: Michael Tokarev
>> Date: Thu, 12 May 2011 18:44:17 +0400
>> Subject: [PATCH] revamp acpitable parsing and allow to specify complete
>> (headerful) table
>>
>> From Michael Tokarev
>>
>> 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
>> Cc:: Isaku Yamahata
>> Cc: John Baboval
>> Cc: Blue Swirl
>>
>> ---
>> 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 ar
Re: [Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
Thanks, applied.
On Fri, Jul 29, 2011 at 4:49 AM, 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
>> 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.yamah...@valinux.co.jp>
> From: Michael Tokarev
> Date: Thu, 12 May 2011 18:44:17 +0400
> Subject: [PATCH] revamp acpitable parsing and allow to specify complete
> (headerful) table
>
> From Michael Tokarev
>
> 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
> Cc:: Isaku Yamahata
> Cc: John Baboval
> Cc: Blue Swirl
>
> ---
> 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
Re: [Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
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 >> 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
Re: [Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
On Fri, Jul 15, 2011 at 07:51:43PM +0300, Blue Swirl wrote:
> On Fri, Jul 15, 2011 at 6:18 PM, John Baboval
> 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.yamah...@valinux.co.jp>
From: Michael Tokarev
Date: Thu, 12 May 2011 18:44:17 +0400
Subject: [PATCH] revamp acpitable parsing and allow to specify complete
(headerful) table
>From Michael Tokarev
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
Cc:: Isaku Yamahata
Cc: John Baboval
Cc: Blue Swirl
---
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;
+
Re: [Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
On Fri, Jul 15, 2011 at 6:18 PM, John Baboval
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?
> -John
>
> On 06/15/2011 02:19 PM, Blue Swirl wrote:
>>
>> On Sat, Jun 11, 2011 at 1:29 AM, Michael Tokarev wrote:
>>>
>>> I've given up on this one. Personally I don't need
>>> this stuff for my win7 guests since I can hack either
>>> bios or the O/S loader to include all the necessary
>>> verifications for the win7 activation to work. I
>>> tried to make this process to be legal (no hacks
>>> or "cracks" needed) and easy for others, but since
>>> noone is interested in this after 6 versions and 5
>>> resends, I wont continue - what I am trying to achieve
>>> by pushing this so hard, after all?
>>>
>>> Thanks to everyone who gave (mostly code style) comments -
>>> at least I know the changes has been read by someone.
>>>
>>> Frustrated,
>>
>> Sorry about that. I get this error:
>> /src/qemu/hw/acpi.c: In function 'acpi_table_add':
>> /src/qemu/hw/acpi.c:167: error: format '%u' expects type 'unsigned
>> int', but argument 4 has type 'size_t'
>>
>>> /mjt
>>>
>>> 12.05.2011 18:44, Michael Tokarev wrote:
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.
v5: rediffed against current qemu/master.
v6: fix one "} else {" coding style defect (noted by Blue Swirl)
Signed-off-by: Michael Tokarev
---
hw/acpi.c | 292
---
qemu-options.hx | 7 +-
2 files changed, 175 insertions(+), 124 deletions(-)
diff --git a/hw/acpi.c b/hw/acpi.c
index ad40fb4..4316189 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -22,17 +22,29 @@
struct acpi_table_header
{
- char signature [4]; /* ACPI signature (4 ASCII characters) */
+ 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;
@@ -45,158 +57,192 @@ static int acpi_checksum(const uint8_t *data, int
len)
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;
Re: [Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
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.
-John
On 06/15/2011 02:19 PM, Blue Swirl wrote:
On Sat, Jun 11, 2011 at 1:29 AM, Michael Tokarev wrote:
I've given up on this one. Personally I don't need
this stuff for my win7 guests since I can hack either
bios or the O/S loader to include all the necessary
verifications for the win7 activation to work. I
tried to make this process to be legal (no hacks
or "cracks" needed) and easy for others, but since
noone is interested in this after 6 versions and 5
resends, I wont continue - what I am trying to achieve
by pushing this so hard, after all?
Thanks to everyone who gave (mostly code style) comments -
at least I know the changes has been read by someone.
Frustrated,
Sorry about that. I get this error:
/src/qemu/hw/acpi.c: In function 'acpi_table_add':
/src/qemu/hw/acpi.c:167: error: format '%u' expects type 'unsigned
int', but argument 4 has type 'size_t'
/mjt
12.05.2011 18:44, Michael Tokarev wrote:
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.
v5: rediffed against current qemu/master.
v6: fix one "} else {" coding style defect (noted by Blue Swirl)
Signed-off-by: Michael Tokarev
---
hw/acpi.c | 292 ---
qemu-options.hx |7 +-
2 files changed, 175 insertions(+), 124 deletions(-)
diff --git a/hw/acpi.c b/hw/acpi.c
index ad40fb4..4316189 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -22,17 +22,29 @@
struct acpi_table_header
{
-char signature [4];/* ACPI signature (4 ASCII characters) */
+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;
@@ -45,158 +57,192 @@ static int acpi_checksum(const uint8_t *data, int len)
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';
Missing 'break' or comment about fall through.
+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;
+}
+
+if (!acpi_tables) {
+allen = sizeof(uint16_t);
+
Re: [Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
Sorry for coming late to this thread.
I've tested this patch, after fixing the format specifier, and it works.
(Though I did test with Xen, and not KVM.)
It's quite convenient. I'm planning on including it in the build we ship
with our product.
It would be nice to see the style issues cleaned up and this to make it
into the tree.
-John
On 06/15/2011 02:19 PM, Blue Swirl wrote:
On Sat, Jun 11, 2011 at 1:29 AM, Michael Tokarev wrote:
I've given up on this one. Personally I don't need
this stuff for my win7 guests since I can hack either
bios or the O/S loader to include all the necessary
verifications for the win7 activation to work. I
tried to make this process to be legal (no hacks
or "cracks" needed) and easy for others, but since
noone is interested in this after 6 versions and 5
resends, I wont continue - what I am trying to achieve
by pushing this so hard, after all?
Thanks to everyone who gave (mostly code style) comments -
at least I know the changes has been read by someone.
Frustrated,
Sorry about that. I get this error:
/src/qemu/hw/acpi.c: In function 'acpi_table_add':
/src/qemu/hw/acpi.c:167: error: format '%u' expects type 'unsigned
int', but argument 4 has type 'size_t'
/mjt
12.05.2011 18:44, Michael Tokarev wrote:
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.
v5: rediffed against current qemu/master.
v6: fix one "} else {" coding style defect (noted by Blue Swirl)
Signed-off-by: Michael Tokarev
---
hw/acpi.c | 292 ---
qemu-options.hx |7 +-
2 files changed, 175 insertions(+), 124 deletions(-)
diff --git a/hw/acpi.c b/hw/acpi.c
index ad40fb4..4316189 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -22,17 +22,29 @@
struct acpi_table_header
{
-char signature [4];/* ACPI signature (4 ASCII characters) */
+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;
@@ -45,158 +57,192 @@ static int acpi_checksum(const uint8_t *data, int len)
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';
Missing 'break' or comment about fall through.
+case 1:
+has_header = false;
+break;
+case 2:
+has_header = true;
+
Re: [Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
On Sat, Jun 11, 2011 at 1:29 AM, Michael Tokarev wrote:
> I've given up on this one. Personally I don't need
> this stuff for my win7 guests since I can hack either
> bios or the O/S loader to include all the necessary
> verifications for the win7 activation to work. I
> tried to make this process to be legal (no hacks
> or "cracks" needed) and easy for others, but since
> noone is interested in this after 6 versions and 5
> resends, I wont continue - what I am trying to achieve
> by pushing this so hard, after all?
>
> Thanks to everyone who gave (mostly code style) comments -
> at least I know the changes has been read by someone.
>
> Frustrated,
Sorry about that. I get this error:
/src/qemu/hw/acpi.c: In function 'acpi_table_add':
/src/qemu/hw/acpi.c:167: error: format '%u' expects type 'unsigned
int', but argument 4 has type 'size_t'
> /mjt
>
> 12.05.2011 18:44, Michael Tokarev wrote:
>> 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.
>>
>> v5: rediffed against current qemu/master.
>> v6: fix one "} else {" coding style defect (noted by Blue Swirl)
>>
>> Signed-off-by: Michael Tokarev
>> ---
>> hw/acpi.c | 292
>> ---
>> qemu-options.hx | 7 +-
>> 2 files changed, 175 insertions(+), 124 deletions(-)
>>
>> diff --git a/hw/acpi.c b/hw/acpi.c
>> index ad40fb4..4316189 100644
>> --- a/hw/acpi.c
>> +++ b/hw/acpi.c
>> @@ -22,17 +22,29 @@
>>
>> struct acpi_table_header
>> {
>> - char signature [4]; /* ACPI signature (4 ASCII characters) */
>> + 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;
>>
>> @@ -45,158 +57,192 @@ static int acpi_checksum(const uint8_t *data, int len)
>> 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';
Missing 'break' or comment about fall through.
>> + case 1:
>> + has_header = false;
>> + break;
>> + case 2:
>> + has_header = true;
>> + break;
>> + default:
>> + fprintf(stderr, "acpitable: both data an
Re: [Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
No more comments on the patch?
All comments were addressed and there is no comments so far.
And this patch only adds one more option to -acpi, thus it's quite safe
to merge this patch. So please pick this patch up.
Anyway who is the maintainer of acpi.c(or acpi related files)?
According to MAINTAINER file, there is no specific assigned maintainer.
By using git log, I found some people involved and added them to Cc.
If the lack of maintainer is the issue, I'm willing to stand up
(unless Michael Tokarev wants to be.)
thanks
On Sat, Jun 11, 2011 at 02:29:38AM +0400, Michael Tokarev wrote:
> I've given up on this one. Personally I don't need
> this stuff for my win7 guests since I can hack either
> bios or the O/S loader to include all the necessary
> verifications for the win7 activation to work. I
> tried to make this process to be legal (no hacks
> or "cracks" needed) and easy for others, but since
> noone is interested in this after 6 versions and 5
> resends, I wont continue - what I am trying to achieve
> by pushing this so hard, after all?
>
> Thanks to everyone who gave (mostly code style) comments -
> at least I know the changes has been read by someone.
>
> Frustrated,
> /mjt
>
> 12.05.2011 18:44, Michael Tokarev wrote:
> > 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.
> >
> > v5: rediffed against current qemu/master.
> > v6: fix one "} else {" coding style defect (noted by Blue Swirl)
> >
> > Signed-off-by: Michael Tokarev
> > ---
> > hw/acpi.c | 292
> > ---
> > qemu-options.hx |7 +-
> > 2 files changed, 175 insertions(+), 124 deletions(-)
> >
> > diff --git a/hw/acpi.c b/hw/acpi.c
> > index ad40fb4..4316189 100644
> > --- a/hw/acpi.c
> > +++ b/hw/acpi.c
> > @@ -22,17 +22,29 @@
> >
> > struct acpi_table_header
> > {
> > -char signature [4];/* ACPI signature (4 ASCII characters) */
> > +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;
> >
> > @@ -45,158 +57,192 @@ static int acpi_checksum(const uint8_t *data, int len)
> > 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;
> > +
> > +
Re: [Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
I've given up on this one. Personally I don't need
this stuff for my win7 guests since I can hack either
bios or the O/S loader to include all the necessary
verifications for the win7 activation to work. I
tried to make this process to be legal (no hacks
or "cracks" needed) and easy for others, but since
noone is interested in this after 6 versions and 5
resends, I wont continue - what I am trying to achieve
by pushing this so hard, after all?
Thanks to everyone who gave (mostly code style) comments -
at least I know the changes has been read by someone.
Frustrated,
/mjt
12.05.2011 18:44, Michael Tokarev wrote:
> 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.
>
> v5: rediffed against current qemu/master.
> v6: fix one "} else {" coding style defect (noted by Blue Swirl)
>
> Signed-off-by: Michael Tokarev
> ---
> hw/acpi.c | 292
> ---
> qemu-options.hx |7 +-
> 2 files changed, 175 insertions(+), 124 deletions(-)
>
> diff --git a/hw/acpi.c b/hw/acpi.c
> index ad40fb4..4316189 100644
> --- a/hw/acpi.c
> +++ b/hw/acpi.c
> @@ -22,17 +22,29 @@
>
> struct acpi_table_header
> {
> -char signature [4];/* ACPI signature (4 ASCII characters) */
> +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;
>
> @@ -45,158 +57,192 @@ static int acpi_checksum(const uint8_t *data, int len)
> 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';
> +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;
> +}
> +
> +if (!acpi_tables) {
> +allen = sizeof(uint16_t);
> +acpi_tables = qemu_mallocz(allen);
> +}
> +else {
> +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;
> +
> +/
[Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
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.
v5: rediffed against current qemu/master.
v6: fix one "} else {" coding style defect
Signed-off-by: Michael Tokarev
---
hw/acpi.c | 291 ---
qemu-options.hx |7 +-
2 files changed, 174 insertions(+), 124 deletions(-)
diff --git a/hw/acpi.c b/hw/acpi.c
index ad40fb4..b8cd866 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -22,17 +22,29 @@
struct acpi_table_header
{
-char signature [4];/* ACPI signature (4 ASCII characters) */
+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;
@@ -45,158 +57,191 @@ static int acpi_checksum(const uint8_t *data, int len)
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';
+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) {
+fpr
[Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
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.
v5: rediffed against current qemu/master.
v6: fix one "} else {" coding style defect (noted by Blue Swirl)
Signed-off-by: Michael Tokarev
---
hw/acpi.c | 292 ---
qemu-options.hx |7 +-
2 files changed, 175 insertions(+), 124 deletions(-)
diff --git a/hw/acpi.c b/hw/acpi.c
index ad40fb4..4316189 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -22,17 +22,29 @@
struct acpi_table_header
{
-char signature [4];/* ACPI signature (4 ASCII characters) */
+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;
@@ -45,158 +57,192 @@ static int acpi_checksum(const uint8_t *data, int len)
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';
+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;
+}
+
+if (!acpi_tables) {
+allen = sizeof(uint16_t);
+acpi_tables = qemu_mallocz(allen);
+}
+else {
+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;
+}
+}
+
+
