diff mbox series

[2/9] tests: acpi: use AcpiSdtTable::aml in consistent way

Message ID 1544465415-207855-3-git-send-email-imammedo@redhat.com
State New
Headers show
Series tests: apci: consolidate and cleanup ACPI test code | expand

Commit Message

Igor Mammedov Dec. 10, 2018, 6:10 p.m. UTC
Currently in the 1st case we store table body fetched from QEMU in
AcpiSdtTable::aml minus it's header but in the 2nd case when we
load reference aml from disk, it holds whole blob including header.
More over in the 1st case, we read header in separate AcpiSdtTable::header
structure and then jump over hoops to fixup tables and combine both.

Treat AcpiSdtTable::aml as whole table blob approach in both cases
and when fetching tables from QEMU, first get table length and then
fetch whole table into AcpiSdtTable::aml instead if doing it field
by field.

As result
 * AcpiSdtTable::aml is used in consistent manner
 * FADT fixups use offsets from spec instead of being shifted by
   header length
 * calculating checksums and dumping blobs becomes simpler

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/acpi-utils.h       |  6 +++--
 tests/bios-tables-test.c | 59 +++++++++++++++++-------------------------------
 2 files changed, 25 insertions(+), 40 deletions(-)

Comments

Wainer dos Santos Moschetta Dec. 19, 2018, 7:02 p.m. UTC | #1
On 12/10/2018 04:10 PM, Igor Mammedov wrote:
> Currently in the 1st case we store table body fetched from QEMU in
> AcpiSdtTable::aml minus it's header but in the 2nd case when we
> load reference aml from disk, it holds whole blob including header.
> More over in the 1st case, we read header in separate AcpiSdtTable::header
> structure and then jump over hoops to fixup tables and combine both.
>
> Treat AcpiSdtTable::aml as whole table blob approach in both cases
> and when fetching tables from QEMU, first get table length and then
> fetch whole table into AcpiSdtTable::aml instead if doing it field
> by field.
>
> As result
>   * AcpiSdtTable::aml is used in consistent manner
>   * FADT fixups use offsets from spec instead of being shifted by
>     header length
>   * calculating checksums and dumping blobs becomes simpler
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>   tests/acpi-utils.h       |  6 +++--
>   tests/bios-tables-test.c | 59 +++++++++++++++++-------------------------------
>   2 files changed, 25 insertions(+), 40 deletions(-)
>
> diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
> index c8844a2..2244e8e 100644
> --- a/tests/acpi-utils.h
> +++ b/tests/acpi-utils.h
> @@ -18,8 +18,10 @@
>   
>   /* DSDT and SSDTs format */
>   typedef struct {
> -    AcpiTableHeader header;
> -    gchar *aml;            /* aml bytecode from guest */
> +    union {
> +        AcpiTableHeader *header;
> +        uint8_t *aml;            /* aml bytecode from guest */
> +    };
>       gsize aml_len;
>       gchar *aml_file;
>       gchar *asl;            /* asl code generated from aml */
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index 8749b77..1666cf7 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -161,29 +161,24 @@ static void sanitize_fadt_ptrs(test_data *data)
>       for (i = 0; i < data->tables->len; i++) {
>           AcpiSdtTable *sdt = &g_array_index(data->tables, AcpiSdtTable, i);
>   
> -        if (memcmp(&sdt->header.signature, "FACP", 4)) {
> +        if (memcmp(&sdt->header->signature, "FACP", 4)) {
>               continue;
>           }
>   
>           /* check original FADT checksum before sanitizing table */
> -        g_assert(!(uint8_t)(
> -            acpi_calc_checksum((uint8_t *)sdt, sizeof(AcpiTableHeader)) +
> -            acpi_calc_checksum((uint8_t *)sdt->aml, sdt->aml_len)
> -        ));
> +        g_assert(!acpi_calc_checksum((uint8_t *)sdt->aml, sdt->aml_len));

Being uint8_t * aml, does it still need that cast to uint8_t?

>   
>           /* sdt->aml field offset := spec offset - header size */

Need to change ^ comment too since offset is not relative to header 
length anymore.


- Wainer

> -        memset(sdt->aml + 0, 0, 4); /* sanitize FIRMWARE_CTRL(36) ptr */
> -        memset(sdt->aml + 4, 0, 4); /* sanitize DSDT(40) ptr */
> -        if (sdt->header.revision >= 3) {
> -            memset(sdt->aml + 96, 0, 8); /* sanitize X_FIRMWARE_CTRL(132) ptr */
> -            memset(sdt->aml + 104, 0, 8); /* sanitize X_DSDT(140) ptr */
> +        memset(sdt->aml + 36, 0, 4); /* sanitize FIRMWARE_CTRL ptr */
> +        memset(sdt->aml + 40, 0, 4); /* sanitize DSDT ptr */
> +        if (sdt->header->revision >= 3) {
> +            memset(sdt->aml + 132, 0, 8); /* sanitize X_FIRMWARE_CTRL ptr */
> +            memset(sdt->aml + 140, 0, 8); /* sanitize X_DSDT ptr */
>           }
>   
>           /* update checksum */
> -        sdt->header.checksum = 0;
> -        sdt->header.checksum -=
> -            acpi_calc_checksum((uint8_t *)sdt, sizeof(AcpiTableHeader)) +
> -            acpi_calc_checksum((uint8_t *)sdt->aml, sdt->aml_len);
> +        sdt->header->checksum = 0;
> +        sdt->header->checksum -= acpi_calc_checksum(sdt->aml, sdt->aml_len);
>           break;
>       }
>   }
> @@ -210,30 +205,21 @@ static void test_acpi_facs_table(test_data *data)
>    */
>   static void fetch_table(AcpiSdtTable *sdt_table, uint32_t addr)
>   {
> -    uint8_t checksum;
> -
> -    memset(sdt_table, 0, sizeof(*sdt_table));
> -    ACPI_READ_TABLE_HEADER(&sdt_table->header, addr);
> -
> -    sdt_table->aml_len = le32_to_cpu(sdt_table->header.length)
> -                         - sizeof(AcpiTableHeader);
> +    memread(addr + 4, &sdt_table->aml_len, 4); /* Length of ACPI table */
> +    sdt_table->aml_len = le32_to_cpu(sdt_table->aml_len);
>       sdt_table->aml = g_malloc0(sdt_table->aml_len);
> -    ACPI_READ_ARRAY_PTR(sdt_table->aml, sdt_table->aml_len, addr);
> +    memread(addr, sdt_table->aml, sdt_table->aml_len); /* get whole table */
>   
> -    checksum = acpi_calc_checksum((uint8_t *)sdt_table,
> -                                  sizeof(AcpiTableHeader)) +
> -               acpi_calc_checksum((uint8_t *)sdt_table->aml,
> -                                  sdt_table->aml_len);
> -    g_assert(!checksum);
> +    g_assert(!acpi_calc_checksum(sdt_table->aml, sdt_table->aml_len));
>   }
>   
>   static void test_acpi_dsdt_table(test_data *data)
>   {
> -    AcpiSdtTable dsdt_table;
> +    AcpiSdtTable dsdt_table = {};
>       uint32_t addr = le32_to_cpu(data->dsdt_addr);
>   
>       fetch_table(&dsdt_table, addr);
> -    ACPI_ASSERT_CMP(dsdt_table.header.signature, "DSDT");
> +    ACPI_ASSERT_CMP(dsdt_table.header->signature, "DSDT");
>   
>       /* Since DSDT isn't in RSDT, add DSDT to ASL test tables list manually */
>       g_array_append_val(data->tables, dsdt_table);
> @@ -246,7 +232,7 @@ static void fetch_rsdt_referenced_tables(test_data *data)
>       int i;
>   
>       for (i = 0; i < tables_nr; i++) {
> -        AcpiSdtTable ssdt_table;
> +        AcpiSdtTable ssdt_table = {};
>           uint32_t addr;
>   
>           addr = le32_to_cpu(data->rsdt_tables_addr[i]);
> @@ -273,7 +259,7 @@ static void dump_aml_files(test_data *data, bool rebuild)
>   
>           if (rebuild) {
>               aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
> -                                       (gchar *)&sdt->header.signature, ext);
> +                                       (gchar *)&sdt->header->signature, ext);
>               fd = g_open(aml_file, O_WRONLY|O_TRUNC|O_CREAT,
>                           S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH);
>           } else {
> @@ -282,8 +268,6 @@ static void dump_aml_files(test_data *data, bool rebuild)
>           }
>           g_assert(fd >= 0);
>   
> -        ret = qemu_write_full(fd, sdt, sizeof(AcpiTableHeader));
> -        g_assert(ret == sizeof(AcpiTableHeader));
>           ret = qemu_write_full(fd, sdt->aml, sdt->aml_len);
>           g_assert(ret == sdt->aml_len);
>   
> @@ -295,7 +279,7 @@ static void dump_aml_files(test_data *data, bool rebuild)
>   
>   static bool compare_signature(AcpiSdtTable *sdt, const char *signature)
>   {
> -   return !memcmp(&sdt->header.signature, signature, 4);
> +   return !memcmp(&sdt->header->signature, signature, 4);
>   }
>   
>   static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
> @@ -390,11 +374,10 @@ static GArray *load_expected_aml(test_data *data)
>           sdt = &g_array_index(data->tables, AcpiSdtTable, i);
>   
>           memset(&exp_sdt, 0, sizeof(exp_sdt));
> -        exp_sdt.header.signature = sdt->header.signature;
>   
>   try_again:
>           aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
> -                                   (gchar *)&sdt->header.signature, ext);
> +                                   (gchar *)&sdt->header->signature, ext);
>           if (getenv("V")) {
>               fprintf(stderr, "\nLooking for expected file '%s'\n", aml_file);
>           }
> @@ -410,7 +393,7 @@ try_again:
>           if (getenv("V")) {
>               fprintf(stderr, "\nUsing expected file '%s'\n", aml_file);
>           }
> -        ret = g_file_get_contents(aml_file, &exp_sdt.aml,
> +        ret = g_file_get_contents(aml_file, (gchar **)&exp_sdt.aml,
>                                     &exp_sdt.aml_len, &error);
>           g_assert(ret);
>           g_assert_no_error(error);
> @@ -454,7 +437,7 @@ static void test_acpi_asl(test_data *data)
>                   fprintf(stderr,
>                           "Warning! iasl couldn't parse the expected aml\n");
>               } else {
> -                uint32_t signature = cpu_to_le32(exp_sdt->header.signature);
> +                uint32_t signature = cpu_to_le32(exp_sdt->header->signature);
>                   sdt->tmp_files_retain = true;
>                   exp_sdt->tmp_files_retain = true;
>                   fprintf(stderr,
Igor Mammedov Dec. 20, 2018, 12:09 p.m. UTC | #2
On Wed, 19 Dec 2018 17:02:36 -0200
Wainer dos Santos Moschetta <wainersm@redhat.com> wrote:

> On 12/10/2018 04:10 PM, Igor Mammedov wrote:
> > Currently in the 1st case we store table body fetched from QEMU in
> > AcpiSdtTable::aml minus it's header but in the 2nd case when we
> > load reference aml from disk, it holds whole blob including header.
> > More over in the 1st case, we read header in separate AcpiSdtTable::header
> > structure and then jump over hoops to fixup tables and combine both.
> >
> > Treat AcpiSdtTable::aml as whole table blob approach in both cases
> > and when fetching tables from QEMU, first get table length and then
> > fetch whole table into AcpiSdtTable::aml instead if doing it field
> > by field.
> >
> > As result
> >   * AcpiSdtTable::aml is used in consistent manner
> >   * FADT fixups use offsets from spec instead of being shifted by
> >     header length
> >   * calculating checksums and dumping blobs becomes simpler
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >   tests/acpi-utils.h       |  6 +++--
> >   tests/bios-tables-test.c | 59 +++++++++++++++++-------------------------------
> >   2 files changed, 25 insertions(+), 40 deletions(-)
> >
> > diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
> > index c8844a2..2244e8e 100644
> > --- a/tests/acpi-utils.h
> > +++ b/tests/acpi-utils.h
> > @@ -18,8 +18,10 @@
> >   
> >   /* DSDT and SSDTs format */
> >   typedef struct {
> > -    AcpiTableHeader header;
> > -    gchar *aml;            /* aml bytecode from guest */
> > +    union {
> > +        AcpiTableHeader *header;
> > +        uint8_t *aml;            /* aml bytecode from guest */
> > +    };
> >       gsize aml_len;
> >       gchar *aml_file;
> >       gchar *asl;            /* asl code generated from aml */
> > diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> > index 8749b77..1666cf7 100644
> > --- a/tests/bios-tables-test.c
> > +++ b/tests/bios-tables-test.c
> > @@ -161,29 +161,24 @@ static void sanitize_fadt_ptrs(test_data *data)
> >       for (i = 0; i < data->tables->len; i++) {
> >           AcpiSdtTable *sdt = &g_array_index(data->tables, AcpiSdtTable, i);
> >   
> > -        if (memcmp(&sdt->header.signature, "FACP", 4)) {
> > +        if (memcmp(&sdt->header->signature, "FACP", 4)) {
> >               continue;
> >           }
> >   
> >           /* check original FADT checksum before sanitizing table */
> > -        g_assert(!(uint8_t)(
> > -            acpi_calc_checksum((uint8_t *)sdt, sizeof(AcpiTableHeader)) +
> > -            acpi_calc_checksum((uint8_t *)sdt->aml, sdt->aml_len)
> > -        ));
> > +        g_assert(!acpi_calc_checksum((uint8_t *)sdt->aml, sdt->aml_len));  
> 
> Being uint8_t * aml, does it still need that cast to uint8_t?
> 
> >   
> >           /* sdt->aml field offset := spec offset - header size */  
> 
> Need to change ^ comment too since offset is not relative to header 
> length anymore.
Thanks,
 I'll fix 2 above notes and check for other similar places

> 
> - Wainer
> 
> > -        memset(sdt->aml + 0, 0, 4); /* sanitize FIRMWARE_CTRL(36) ptr */
> > -        memset(sdt->aml + 4, 0, 4); /* sanitize DSDT(40) ptr */
> > -        if (sdt->header.revision >= 3) {
> > -            memset(sdt->aml + 96, 0, 8); /* sanitize X_FIRMWARE_CTRL(132) ptr */
> > -            memset(sdt->aml + 104, 0, 8); /* sanitize X_DSDT(140) ptr */
> > +        memset(sdt->aml + 36, 0, 4); /* sanitize FIRMWARE_CTRL ptr */
> > +        memset(sdt->aml + 40, 0, 4); /* sanitize DSDT ptr */
> > +        if (sdt->header->revision >= 3) {
> > +            memset(sdt->aml + 132, 0, 8); /* sanitize X_FIRMWARE_CTRL ptr */
> > +            memset(sdt->aml + 140, 0, 8); /* sanitize X_DSDT ptr */
> >           }
> >   
> >           /* update checksum */
> > -        sdt->header.checksum = 0;
> > -        sdt->header.checksum -=
> > -            acpi_calc_checksum((uint8_t *)sdt, sizeof(AcpiTableHeader)) +
> > -            acpi_calc_checksum((uint8_t *)sdt->aml, sdt->aml_len);
> > +        sdt->header->checksum = 0;
> > +        sdt->header->checksum -= acpi_calc_checksum(sdt->aml, sdt->aml_len);
> >           break;
> >       }
> >   }
> > @@ -210,30 +205,21 @@ static void test_acpi_facs_table(test_data *data)
> >    */
> >   static void fetch_table(AcpiSdtTable *sdt_table, uint32_t addr)
> >   {
> > -    uint8_t checksum;
> > -
> > -    memset(sdt_table, 0, sizeof(*sdt_table));
> > -    ACPI_READ_TABLE_HEADER(&sdt_table->header, addr);
> > -
> > -    sdt_table->aml_len = le32_to_cpu(sdt_table->header.length)
> > -                         - sizeof(AcpiTableHeader);
> > +    memread(addr + 4, &sdt_table->aml_len, 4); /* Length of ACPI table */
> > +    sdt_table->aml_len = le32_to_cpu(sdt_table->aml_len);
> >       sdt_table->aml = g_malloc0(sdt_table->aml_len);
> > -    ACPI_READ_ARRAY_PTR(sdt_table->aml, sdt_table->aml_len, addr);
> > +    memread(addr, sdt_table->aml, sdt_table->aml_len); /* get whole table */
> >   
> > -    checksum = acpi_calc_checksum((uint8_t *)sdt_table,
> > -                                  sizeof(AcpiTableHeader)) +
> > -               acpi_calc_checksum((uint8_t *)sdt_table->aml,
> > -                                  sdt_table->aml_len);
> > -    g_assert(!checksum);
> > +    g_assert(!acpi_calc_checksum(sdt_table->aml, sdt_table->aml_len));
> >   }
> >   
> >   static void test_acpi_dsdt_table(test_data *data)
> >   {
> > -    AcpiSdtTable dsdt_table;
> > +    AcpiSdtTable dsdt_table = {};
> >       uint32_t addr = le32_to_cpu(data->dsdt_addr);
> >   
> >       fetch_table(&dsdt_table, addr);
> > -    ACPI_ASSERT_CMP(dsdt_table.header.signature, "DSDT");
> > +    ACPI_ASSERT_CMP(dsdt_table.header->signature, "DSDT");
> >   
> >       /* Since DSDT isn't in RSDT, add DSDT to ASL test tables list manually */
> >       g_array_append_val(data->tables, dsdt_table);
> > @@ -246,7 +232,7 @@ static void fetch_rsdt_referenced_tables(test_data *data)
> >       int i;
> >   
> >       for (i = 0; i < tables_nr; i++) {
> > -        AcpiSdtTable ssdt_table;
> > +        AcpiSdtTable ssdt_table = {};
> >           uint32_t addr;
> >   
> >           addr = le32_to_cpu(data->rsdt_tables_addr[i]);
> > @@ -273,7 +259,7 @@ static void dump_aml_files(test_data *data, bool rebuild)
> >   
> >           if (rebuild) {
> >               aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
> > -                                       (gchar *)&sdt->header.signature, ext);
> > +                                       (gchar *)&sdt->header->signature, ext);
> >               fd = g_open(aml_file, O_WRONLY|O_TRUNC|O_CREAT,
> >                           S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH);
> >           } else {
> > @@ -282,8 +268,6 @@ static void dump_aml_files(test_data *data, bool rebuild)
> >           }
> >           g_assert(fd >= 0);
> >   
> > -        ret = qemu_write_full(fd, sdt, sizeof(AcpiTableHeader));
> > -        g_assert(ret == sizeof(AcpiTableHeader));
> >           ret = qemu_write_full(fd, sdt->aml, sdt->aml_len);
> >           g_assert(ret == sdt->aml_len);
> >   
> > @@ -295,7 +279,7 @@ static void dump_aml_files(test_data *data, bool rebuild)
> >   
> >   static bool compare_signature(AcpiSdtTable *sdt, const char *signature)
> >   {
> > -   return !memcmp(&sdt->header.signature, signature, 4);
> > +   return !memcmp(&sdt->header->signature, signature, 4);
> >   }
> >   
> >   static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
> > @@ -390,11 +374,10 @@ static GArray *load_expected_aml(test_data *data)
> >           sdt = &g_array_index(data->tables, AcpiSdtTable, i);
> >   
> >           memset(&exp_sdt, 0, sizeof(exp_sdt));
> > -        exp_sdt.header.signature = sdt->header.signature;
> >   
> >   try_again:
> >           aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
> > -                                   (gchar *)&sdt->header.signature, ext);
> > +                                   (gchar *)&sdt->header->signature, ext);
> >           if (getenv("V")) {
> >               fprintf(stderr, "\nLooking for expected file '%s'\n", aml_file);
> >           }
> > @@ -410,7 +393,7 @@ try_again:
> >           if (getenv("V")) {
> >               fprintf(stderr, "\nUsing expected file '%s'\n", aml_file);
> >           }
> > -        ret = g_file_get_contents(aml_file, &exp_sdt.aml,
> > +        ret = g_file_get_contents(aml_file, (gchar **)&exp_sdt.aml,
> >                                     &exp_sdt.aml_len, &error);
> >           g_assert(ret);
> >           g_assert_no_error(error);
> > @@ -454,7 +437,7 @@ static void test_acpi_asl(test_data *data)
> >                   fprintf(stderr,
> >                           "Warning! iasl couldn't parse the expected aml\n");
> >               } else {
> > -                uint32_t signature = cpu_to_le32(exp_sdt->header.signature);
> > +                uint32_t signature = cpu_to_le32(exp_sdt->header->signature);
> >                   sdt->tmp_files_retain = true;
> >                   exp_sdt->tmp_files_retain = true;
> >                   fprintf(stderr,  
>
diff mbox series

Patch

diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
index c8844a2..2244e8e 100644
--- a/tests/acpi-utils.h
+++ b/tests/acpi-utils.h
@@ -18,8 +18,10 @@ 
 
 /* DSDT and SSDTs format */
 typedef struct {
-    AcpiTableHeader header;
-    gchar *aml;            /* aml bytecode from guest */
+    union {
+        AcpiTableHeader *header;
+        uint8_t *aml;            /* aml bytecode from guest */
+    };
     gsize aml_len;
     gchar *aml_file;
     gchar *asl;            /* asl code generated from aml */
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 8749b77..1666cf7 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -161,29 +161,24 @@  static void sanitize_fadt_ptrs(test_data *data)
     for (i = 0; i < data->tables->len; i++) {
         AcpiSdtTable *sdt = &g_array_index(data->tables, AcpiSdtTable, i);
 
-        if (memcmp(&sdt->header.signature, "FACP", 4)) {
+        if (memcmp(&sdt->header->signature, "FACP", 4)) {
             continue;
         }
 
         /* check original FADT checksum before sanitizing table */
-        g_assert(!(uint8_t)(
-            acpi_calc_checksum((uint8_t *)sdt, sizeof(AcpiTableHeader)) +
-            acpi_calc_checksum((uint8_t *)sdt->aml, sdt->aml_len)
-        ));
+        g_assert(!acpi_calc_checksum((uint8_t *)sdt->aml, sdt->aml_len));
 
         /* sdt->aml field offset := spec offset - header size */
-        memset(sdt->aml + 0, 0, 4); /* sanitize FIRMWARE_CTRL(36) ptr */
-        memset(sdt->aml + 4, 0, 4); /* sanitize DSDT(40) ptr */
-        if (sdt->header.revision >= 3) {
-            memset(sdt->aml + 96, 0, 8); /* sanitize X_FIRMWARE_CTRL(132) ptr */
-            memset(sdt->aml + 104, 0, 8); /* sanitize X_DSDT(140) ptr */
+        memset(sdt->aml + 36, 0, 4); /* sanitize FIRMWARE_CTRL ptr */
+        memset(sdt->aml + 40, 0, 4); /* sanitize DSDT ptr */
+        if (sdt->header->revision >= 3) {
+            memset(sdt->aml + 132, 0, 8); /* sanitize X_FIRMWARE_CTRL ptr */
+            memset(sdt->aml + 140, 0, 8); /* sanitize X_DSDT ptr */
         }
 
         /* update checksum */
-        sdt->header.checksum = 0;
-        sdt->header.checksum -=
-            acpi_calc_checksum((uint8_t *)sdt, sizeof(AcpiTableHeader)) +
-            acpi_calc_checksum((uint8_t *)sdt->aml, sdt->aml_len);
+        sdt->header->checksum = 0;
+        sdt->header->checksum -= acpi_calc_checksum(sdt->aml, sdt->aml_len);
         break;
     }
 }
@@ -210,30 +205,21 @@  static void test_acpi_facs_table(test_data *data)
  */
 static void fetch_table(AcpiSdtTable *sdt_table, uint32_t addr)
 {
-    uint8_t checksum;
-
-    memset(sdt_table, 0, sizeof(*sdt_table));
-    ACPI_READ_TABLE_HEADER(&sdt_table->header, addr);
-
-    sdt_table->aml_len = le32_to_cpu(sdt_table->header.length)
-                         - sizeof(AcpiTableHeader);
+    memread(addr + 4, &sdt_table->aml_len, 4); /* Length of ACPI table */
+    sdt_table->aml_len = le32_to_cpu(sdt_table->aml_len);
     sdt_table->aml = g_malloc0(sdt_table->aml_len);
-    ACPI_READ_ARRAY_PTR(sdt_table->aml, sdt_table->aml_len, addr);
+    memread(addr, sdt_table->aml, sdt_table->aml_len); /* get whole table */
 
-    checksum = acpi_calc_checksum((uint8_t *)sdt_table,
-                                  sizeof(AcpiTableHeader)) +
-               acpi_calc_checksum((uint8_t *)sdt_table->aml,
-                                  sdt_table->aml_len);
-    g_assert(!checksum);
+    g_assert(!acpi_calc_checksum(sdt_table->aml, sdt_table->aml_len));
 }
 
 static void test_acpi_dsdt_table(test_data *data)
 {
-    AcpiSdtTable dsdt_table;
+    AcpiSdtTable dsdt_table = {};
     uint32_t addr = le32_to_cpu(data->dsdt_addr);
 
     fetch_table(&dsdt_table, addr);
-    ACPI_ASSERT_CMP(dsdt_table.header.signature, "DSDT");
+    ACPI_ASSERT_CMP(dsdt_table.header->signature, "DSDT");
 
     /* Since DSDT isn't in RSDT, add DSDT to ASL test tables list manually */
     g_array_append_val(data->tables, dsdt_table);
@@ -246,7 +232,7 @@  static void fetch_rsdt_referenced_tables(test_data *data)
     int i;
 
     for (i = 0; i < tables_nr; i++) {
-        AcpiSdtTable ssdt_table;
+        AcpiSdtTable ssdt_table = {};
         uint32_t addr;
 
         addr = le32_to_cpu(data->rsdt_tables_addr[i]);
@@ -273,7 +259,7 @@  static void dump_aml_files(test_data *data, bool rebuild)
 
         if (rebuild) {
             aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
-                                       (gchar *)&sdt->header.signature, ext);
+                                       (gchar *)&sdt->header->signature, ext);
             fd = g_open(aml_file, O_WRONLY|O_TRUNC|O_CREAT,
                         S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH);
         } else {
@@ -282,8 +268,6 @@  static void dump_aml_files(test_data *data, bool rebuild)
         }
         g_assert(fd >= 0);
 
-        ret = qemu_write_full(fd, sdt, sizeof(AcpiTableHeader));
-        g_assert(ret == sizeof(AcpiTableHeader));
         ret = qemu_write_full(fd, sdt->aml, sdt->aml_len);
         g_assert(ret == sdt->aml_len);
 
@@ -295,7 +279,7 @@  static void dump_aml_files(test_data *data, bool rebuild)
 
 static bool compare_signature(AcpiSdtTable *sdt, const char *signature)
 {
-   return !memcmp(&sdt->header.signature, signature, 4);
+   return !memcmp(&sdt->header->signature, signature, 4);
 }
 
 static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
@@ -390,11 +374,10 @@  static GArray *load_expected_aml(test_data *data)
         sdt = &g_array_index(data->tables, AcpiSdtTable, i);
 
         memset(&exp_sdt, 0, sizeof(exp_sdt));
-        exp_sdt.header.signature = sdt->header.signature;
 
 try_again:
         aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
-                                   (gchar *)&sdt->header.signature, ext);
+                                   (gchar *)&sdt->header->signature, ext);
         if (getenv("V")) {
             fprintf(stderr, "\nLooking for expected file '%s'\n", aml_file);
         }
@@ -410,7 +393,7 @@  try_again:
         if (getenv("V")) {
             fprintf(stderr, "\nUsing expected file '%s'\n", aml_file);
         }
-        ret = g_file_get_contents(aml_file, &exp_sdt.aml,
+        ret = g_file_get_contents(aml_file, (gchar **)&exp_sdt.aml,
                                   &exp_sdt.aml_len, &error);
         g_assert(ret);
         g_assert_no_error(error);
@@ -454,7 +437,7 @@  static void test_acpi_asl(test_data *data)
                 fprintf(stderr,
                         "Warning! iasl couldn't parse the expected aml\n");
             } else {
-                uint32_t signature = cpu_to_le32(exp_sdt->header.signature);
+                uint32_t signature = cpu_to_le32(exp_sdt->header->signature);
                 sdt->tmp_files_retain = true;
                 exp_sdt->tmp_files_retain = true;
                 fprintf(stderr,