[Qemu-devel] [PATCH 0/3] acpi unit-test: fixes and improvements

[Qemu-devel] [PATCH 0/3] acpi unit-test: fixes and improvements

[Marcel Apfelbaum]

patch 1: refactoring 
   Addressed Michael S. Tsirkin's prev comments:
   Renamed ssdt_tables to tables to avoid confusion.
patch 2: resolved iasl crash
    Addressed Michael S. Tsirkin's prev comments:
    Do not assume the order of ssdt tables and
    single ssdt table.
patch 3: do not fail on asl mismatch
    Rather than failing the test, print a 
    warning message including the asl file 
    names that can be compared offline.

Marcel Apfelbaum (3):
  acpi unit-test: renamed ssdt_tables to tables
  acpi unit-test: resolved iasl crash
  acpi unit-test: do not fail on asl mismatch

 tests/acpi-test-data/pc/SSDT  | Bin 2104 -> 2104 bytes
 tests/acpi-test-data/q35/DSDT | Bin 7344 -> 7344 bytes
 tests/acpi-test.c             |  79 ++++++++++++++++++++++++++----------------
 3 files changed, 50 insertions(+), 29 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/3] acpi unit-test: renamed ssdt_tables to tables

[Marcel Apfelbaum]

Just a refactoring, ssdt_tables name was confusing as
it included other tables as well.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 tests/acpi-test.c | 48 ++++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/tests/acpi-test.c b/tests/acpi-test.c
index 89c4b81..ef998e8 100644
--- a/tests/acpi-test.c
+++ b/tests/acpi-test.c
@@ -44,7 +44,7 @@ typedef struct {
     AcpiFacsDescriptorRev1 facs_table;
     uint32_t *rsdt_tables_addr;
     int rsdt_tables_nr;
-    GArray *ssdt_tables; /* first is DSDT */
+    GArray *tables;
 } test_data;
 
 #define LOW(x) ((x) & 0xff)
@@ -145,8 +145,8 @@ static void free_test_data(test_data *data)
         g_free(data->rsdt_tables_addr);
     }
 
-    for (i = 0; i < data->ssdt_tables->len; ++i) {
-        temp = &g_array_index(data->ssdt_tables, AcpiSdtTable, i);
+    for (i = 0; i < data->tables->len; ++i) {
+        temp = &g_array_index(data->tables, AcpiSdtTable, i);
         if (temp->aml) {
             g_free(temp->aml);
         }
@@ -167,7 +167,7 @@ static void free_test_data(test_data *data)
         }
     }
 
-    g_array_free(data->ssdt_tables, false);
+    g_array_free(data->tables, false);
 }
 
 static uint8_t acpi_checksum(const uint8_t *data, int len)
@@ -342,27 +342,27 @@ static void test_acpi_dsdt_table(test_data *data)
     uint32_t addr = data->fadt_table.dsdt;
 
     memset(&dsdt_table, 0, sizeof(dsdt_table));
-    data->ssdt_tables = g_array_new(false, true, sizeof(AcpiSdtTable));
+    data->tables = g_array_new(false, true, sizeof(AcpiSdtTable));
 
     test_dst_table(&dsdt_table, addr);
     g_assert_cmphex(dsdt_table.header.signature, ==, ACPI_DSDT_SIGNATURE);
 
     /* Place DSDT first */
-    g_array_append_val(data->ssdt_tables, dsdt_table);
+    g_array_append_val(data->tables, dsdt_table);
 }
 
-static void test_acpi_ssdt_tables(test_data *data)
+static void test_acpi_tables(test_data *data)
 {
-    int ssdt_tables_nr = data->rsdt_tables_nr - 1; /* fadt is first */
+    int tables_nr = data->rsdt_tables_nr - 1; /* fadt is first */
     int i;
 
-    for (i = 0; i < ssdt_tables_nr; i++) {
+    for (i = 0; i < tables_nr; i++) {
         AcpiSdtTable ssdt_table;
 
         memset(&ssdt_table, 0 , sizeof(ssdt_table));
         uint32_t addr = data->rsdt_tables_addr[i + 1]; /* fadt is first */
         test_dst_table(&ssdt_table, addr);
-        g_array_append_val(data->ssdt_tables, ssdt_table);
+        g_array_append_val(data->tables, ssdt_table);
     }
 }
 
@@ -375,8 +375,8 @@ static void dump_aml_files(test_data *data, bool rebuild)
     ssize_t ret;
     int i;
 
-    for (i = 0; i < data->ssdt_tables->len; ++i) {
-        sdt = &g_array_index(data->ssdt_tables, AcpiSdtTable, i);
+    for (i = 0; i < data->tables->len; ++i) {
+        sdt = &g_array_index(data->tables, AcpiSdtTable, i);
         g_assert(sdt->aml);
 
         if (rebuild) {
@@ -474,10 +474,10 @@ static GArray *load_expected_aml(test_data *data)
     GError *error = NULL;
     gboolean ret;
 
-    GArray *exp_ssdt_tables = g_array_new(false, true, sizeof(AcpiSdtTable));
-    for (i = 0; i < data->ssdt_tables->len; ++i) {
+    GArray *exp_tables = g_array_new(false, true, sizeof(AcpiSdtTable));
+    for (i = 0; i < data->tables->len; ++i) {
         AcpiSdtTable exp_sdt;
-        sdt = &g_array_index(data->ssdt_tables, AcpiSdtTable, i);
+        sdt = &g_array_index(data->tables, AcpiSdtTable, i);
 
         memset(&exp_sdt, 0, sizeof(exp_sdt));
         exp_sdt.header.signature = sdt->header.signature;
@@ -493,10 +493,10 @@ static GArray *load_expected_aml(test_data *data)
         g_assert(exp_sdt.aml);
         g_assert(exp_sdt.aml_len);
 
-        g_array_append_val(exp_ssdt_tables, exp_sdt);
+        g_array_append_val(exp_tables, exp_sdt);
     }
 
-    return exp_ssdt_tables;
+    return exp_tables;
 }
 
 static void test_acpi_asl(test_data *data)
@@ -506,18 +506,18 @@ static void test_acpi_asl(test_data *data)
     test_data exp_data;
 
     memset(&exp_data, 0, sizeof(exp_data));
-    exp_data.ssdt_tables = load_expected_aml(data);
+    exp_data.tables = load_expected_aml(data);
     dump_aml_files(data, false);
-    for (i = 0; i < data->ssdt_tables->len; ++i) {
+    for (i = 0; i < data->tables->len; ++i) {
         GString *asl, *exp_asl;
 
-        sdt = &g_array_index(data->ssdt_tables, AcpiSdtTable, i);
-        exp_sdt = &g_array_index(exp_data.ssdt_tables, AcpiSdtTable, i);
+        sdt = &g_array_index(data->tables, AcpiSdtTable, i);
+        exp_sdt = &g_array_index(exp_data.tables, AcpiSdtTable, i);
 
-        load_asl(data->ssdt_tables, sdt);
+        load_asl(data->tables, sdt);
         asl = normalize_asl(sdt->asl);
 
-        load_asl(exp_data.ssdt_tables, exp_sdt);
+        load_asl(exp_data.tables, exp_sdt);
         exp_asl = normalize_asl(exp_sdt->asl);
 
         g_assert(!g_strcmp0(asl->str, exp_asl->str));
@@ -570,7 +570,7 @@ static void test_acpi_one(const char *params, test_data *data)
     test_acpi_fadt_table(data);
     test_acpi_facs_table(data);
     test_acpi_dsdt_table(data);
-    test_acpi_ssdt_tables(data);
+    test_acpi_tables(data);
 
     if (iasl) {
         if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/3] acpi unit-test: resolved iasl crash

[Marcel Apfelbaum]

It seems that iasl has an issue when disassembles
some ACPI tables using the command line:
iasl -e DSDT -e SSDT -d HPET

Modified the iasl command line to "iasl -d HPET"
until the problem is solved. The command line
remained the same for DSDT and SSDT tables.

Reported-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 tests/acpi-test.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/tests/acpi-test.c b/tests/acpi-test.c
index ef998e8..c73b3b1 100644
--- a/tests/acpi-test.c
+++ b/tests/acpi-test.c
@@ -23,6 +23,7 @@
 #define MACHINE_Q35 "q35"
 
 #define ACPI_REBUILD_EXPECTED_AML "TEST_ACPI_REBUILD_AML"
+#define ACPI_SSDT_SIGNATURE 0x54445353 /* SSDT */
 
 /* DSDT and SSDTs format */
 typedef struct {
@@ -403,6 +404,11 @@ static void dump_aml_files(test_data *data, bool rebuild)
     }
 }
 
+static bool compare_signature(AcpiSdtTable *sdt, uint32_t signature)
+{
+   return sdt->header.signature == signature;
+}
+
 static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
 {
     AcpiSdtTable *temp;
@@ -419,9 +425,15 @@ static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
 
     /* build command line */
     g_string_append_printf(command_line, " -p %s ", sdt->asl_file);
-    for (i = 0; i < 2; ++i) { /* reference DSDT and SSDT */
-        temp = &g_array_index(sdts, AcpiSdtTable, i);
-        g_string_append_printf(command_line, "-e %s ", temp->aml_file);
+    if (compare_signature(sdt, ACPI_DSDT_SIGNATURE) ||
+        compare_signature(sdt, ACPI_SSDT_SIGNATURE)) {
+        for (i = 0; i < sdts->len; ++i) {
+            temp = &g_array_index(sdts, AcpiSdtTable, i);
+            if (compare_signature(temp, ACPI_DSDT_SIGNATURE) ||
+                compare_signature(temp, ACPI_SSDT_SIGNATURE)) {
+                g_string_append_printf(command_line, "-e %s ", temp->aml_file);
+            }
+        }
     }
     g_string_append_printf(command_line, "-d %s", sdt->aml_file);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/3] acpi unit-test: do not fail on asl mismatch

[Marcel Apfelbaum]

The asl comparison will break every time the ACPI
tables are updated. This may break the git bisect.
Instead of failing print a warning on stderr
including the retained asl files, so they can be
compared offline.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 tests/acpi-test.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tests/acpi-test.c b/tests/acpi-test.c
index c73b3b1..41da079 100644
--- a/tests/acpi-test.c
+++ b/tests/acpi-test.c
@@ -34,6 +34,7 @@ typedef struct {
     gchar *asl;            /* asl code generated from aml */
     gsize asl_len;
     gchar *asl_file;
+    bool asl_file_retain;   /* do not delete the temp asl */
 } QEMU_PACKED AcpiSdtTable;
 
 typedef struct {
@@ -161,7 +162,7 @@ static void free_test_data(test_data *data)
             g_free(temp->asl);
         }
         if (temp->asl_file) {
-            if (g_strstr_len(temp->asl_file, -1, "asl-")) {
+            if (!temp->asl_file_retain) {
                 unlink(temp->asl_file);
             }
             g_free(temp->asl_file);
@@ -532,7 +533,15 @@ static void test_acpi_asl(test_data *data)
         load_asl(exp_data.tables, exp_sdt);
         exp_asl = normalize_asl(exp_sdt->asl);
 
-        g_assert(!g_strcmp0(asl->str, exp_asl->str));
+        if (g_strcmp0(asl->str, exp_asl->str)) {
+            sdt->asl_file_retain = true;
+            exp_sdt->asl_file_retain = true;
+            fprintf(stderr,
+                    "acpi-test: Warning! %.4s mismatch. "
+                    "Orig asl: %s, expected asl %s.\n",
+                    (gchar *)&exp_sdt->header.signature,
+                    sdt->asl_file, exp_sdt->asl_file);
+        }
         g_string_free(asl, true);
         g_string_free(exp_asl, true);
     }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/3] acpi unit-test: fixes and improvements

[Michael S. Tsirkin]

On Thu, Jan 16, 2014 at 05:50:45PM +0200, Marcel Apfelbaum wrote:
> patch 1: refactoring 
>    Addressed Michael S. Tsirkin's prev comments:
>    Renamed ssdt_tables to tables to avoid confusion.
> patch 2: resolved iasl crash
>     Addressed Michael S. Tsirkin's prev comments:
>     Do not assume the order of ssdt tables and
>     single ssdt table.
> patch 3: do not fail on asl mismatch
>     Rather than failing the test, print a 
>     warning message including the asl file 
>     names that can be compared offline.

Thanks, applied.

> Marcel Apfelbaum (3):
>   acpi unit-test: renamed ssdt_tables to tables
>   acpi unit-test: resolved iasl crash
>   acpi unit-test: do not fail on asl mismatch
> 
>  tests/acpi-test-data/pc/SSDT  | Bin 2104 -> 2104 bytes
>  tests/acpi-test-data/q35/DSDT | Bin 7344 -> 7344 bytes
>  tests/acpi-test.c             |  79 ++++++++++++++++++++++++++----------------
>  3 files changed, 50 insertions(+), 29 deletions(-)
> 
> -- 
> 1.8.3.1