Re: [Qemu-devel] [PATCH v2 3/3] acpi unit-test: compare DSDT and SSDT tables against expected values

[Michael S. Tsirkin]

On Mon, Dec 23, 2013 at 12:13:26PM +0200, Marcel Apfelbaum wrote:
> This test will run only if iasl is installed on the host machine.
> The test plan:
>  1. Dumps the ACPI tables as AML on the disk.
>  2. Runs iasl to disassembly the tables into ASL files.
>  3. Runs iasl to disassembly the offline AML files into ASL files.
>  4. Compares the ASL files.
> 
> The test runs for both default machine and q35.
> In case the test fails, it can be easily tweaked to
> show the differences between the ASL files and
> understand the issue.
> 
> Signed-off-by: Marcel Apfelbaum <address@hidden>
> ---
>  tests/acpi-test.c | 257 
> +++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 236 insertions(+), 21 deletions(-)
> 
> diff --git a/tests/acpi-test.c b/tests/acpi-test.c
> index ca83b1d..4f0cca6 100644
> --- a/tests/acpi-test.c
> +++ b/tests/acpi-test.c
> @@ -18,14 +18,22 @@
>  #include "qemu/compiler.h"
>  #include "hw/i386/acpi-defs.h"
>  
> +#define MACHINE_PC "pc"
> +#define MACHINE_Q35 "q35"
> +
>  /* DSDT and SSDTs format */
>  typedef struct {
>      AcpiTableHeader header;
> -    uint8_t *aml;
> -    int aml_len;
> -} AcpiSdtTable;
> +    gchar *aml;            /* aml bytecode from guest */
> +    gsize aml_len;
> +    gchar *aml_file;
> +    gchar *asl;            /* asl code generated from aml */
> +    gsize asl_len;
> +    gchar *asl_file;
> +} QEMU_PACKED AcpiSdtTable;
>  
>  typedef struct {
> +    const char *machine;
>      uint32_t rsdp_addr;
>      AcpiRsdpDescriptor rsdp_table;
>      AcpiRsdtDescriptorRev1 rsdt_table;
> @@ -33,8 +41,7 @@ typedef struct {
>      AcpiFacsDescriptorRev1 facs_table;
>      uint32_t *rsdt_tables_addr;
>      int rsdt_tables_nr;
> -    AcpiSdtTable dsdt_table;
> -    GArray *ssdt_tables;
> +    GArray *ssdt_tables; /* first is DSDT */
>  } test_data;
>  
>  #define LOW(x) ((x) & 0xff)
> @@ -91,8 +98,10 @@ typedef struct {
>  
>  /* Boot sector code: write SIGNATURE into memory,
>   * then halt.
> + * Q35 machine requires a minimum 0x7e000 bytes disk.
> + * (bug or feature?)
>   */
> -static uint8_t boot_sector[0x200] = {
> +static uint8_t boot_sector[0x7e000] = {
>      /* 7c00: mov $0xdead,%ax */
>      [0x00] = 0xb8,
>      [0x01] = LOW(SIGNATURE),
> @@ -117,17 +126,40 @@ static uint8_t boot_sector[0x200] = {
>  };
>  
>  static const char *disk = "tests/acpi-test-disk.raw";
> +static const char *data_dir = "tests/acpi-test-data";
>  
>  static void free_test_data(test_data *data)
>  {
> +    AcpiSdtTable *temp;
>      int i;
>  
> -    g_free(data->rsdt_tables_addr);
> +    if (data->rsdt_tables_addr) {
> +        g_free(data->rsdt_tables_addr);
> +    }
> +
>      for (i = 0; i < data->ssdt_tables->len; ++i) {
> -        g_free(g_array_index(data->ssdt_tables, AcpiSdtTable, i).aml);
> +        temp = &g_array_index(data->ssdt_tables, AcpiSdtTable, i);
> +        if (temp->aml) {
> +            g_free(temp->aml);
> +        }
> +        if (temp->aml_file) {
> +            if (g_strstr_len(temp->aml_file, -1, "aml-")) {
> +                unlink(temp->aml_file);
> +            }
> +            g_free(temp->aml_file);
> +        }
> +        if (temp->asl) {
> +            g_free(temp->asl);
> +        }
> +        if (temp->asl_file) {
> +            if (g_strstr_len(temp->asl_file, -1, "asl-")) {
> +                unlink(temp->asl_file);
> +            }
> +            g_free(temp->asl_file);
> +        }
>      }
> +
>      g_array_free(data->ssdt_tables, false);
> -    g_free(data->dsdt_table.aml);
>  }
>  
>  static uint8_t acpi_checksum(const uint8_t *data, int len)
> @@ -292,34 +324,203 @@ static void test_dst_table(AcpiSdtTable *sdt_table, 
> uint32_t addr)
>      ACPI_READ_ARRAY_PTR(sdt_table->aml, sdt_table->aml_len, addr);
>  
>      checksum = acpi_checksum((uint8_t *)sdt_table, sizeof(AcpiTableHeader)) +
> -               acpi_checksum(sdt_table->aml, sdt_table->aml_len);
> +               acpi_checksum((uint8_t *)sdt_table->aml, sdt_table->aml_len);
>      g_assert(!checksum);
>  }
>  
>  static void test_acpi_dsdt_table(test_data *data)
>  {
> -    AcpiSdtTable *dsdt_table = &data->dsdt_table;
> +    AcpiSdtTable dsdt_table;
>      uint32_t addr = data->fadt_table.dsdt;
>  
> -    test_dst_table(dsdt_table, addr);
> -    g_assert_cmphex(dsdt_table->header.signature, ==, ACPI_DSDT_SIGNATURE);
> +    memset(&dsdt_table, 0, sizeof(dsdt_table));
> +    data->ssdt_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);
>  }
>  
>  static void test_acpi_ssdt_tables(test_data *data)
>  {
> -    GArray *ssdt_tables;
>      int ssdt_tables_nr = data->rsdt_tables_nr - 1; /* fadt is first */
>      int i;
>  
> -    ssdt_tables = g_array_sized_new(false, true, sizeof(AcpiSdtTable),
> -                                    ssdt_tables_nr);
>      for (i = 0; i < ssdt_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(ssdt_tables, ssdt_table);
> +        g_array_append_val(data->ssdt_tables, ssdt_table);
> +    }
> +}
> +
> +static bool iasl_installed(void)
> +{
> +    gchar *out = NULL, *out_err = NULL;
> +    bool ret;
> +
> +    /* pass 'out' and 'out_err' in order to be redirected */
> +    ret = g_spawn_command_line_sync("iasl", &out, &out_err, NULL, NULL);

One interesting option here is to include config-hosts.mak
from Makefile, then use IASL configured there.

Can be a patch on top though.

> +
> +    if (out_err) {
> +        ret = ret && (out_err[0] == '\0');
> +        g_free(out_err);
>      }
> -    data->ssdt_tables = ssdt_tables;
> +
> +    if (out) {
> +        g_free(out);
> +    }
> +
> +    return ret;
> +}
> +
> +static void dump_aml_files(test_data *data)
> +{
> +    AcpiSdtTable *sdt;
> +    GError *error = NULL;
> +    gint fd;
> +    int i;
> +
> +    for (i = 0; i < data->ssdt_tables->len; ++i) {
> +        sdt = &g_array_index(data->ssdt_tables, AcpiSdtTable, i);
> +        g_assert(sdt->aml);
> +
> +        fd = g_file_open_tmp("aml-XXXXXX", &sdt->aml_file, &error);
> +        g_assert_no_error(error);
> +
> +        write(fd, sdt, sizeof(AcpiTableHeader));
> +        write(fd, sdt->aml, sdt->aml_len);
> +        close(fd);
> +    }
> +}
> +
> +static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
> +{
> +    AcpiSdtTable *temp;
> +    GError *error = NULL;
> +    GString *command_line = g_string_new("'iasl' ");
> +    gint fd;
> +    gchar *out, *out_err;
> +    gboolean ret;
> +    int i;
> +
> +    fd = g_file_open_tmp("asl-XXXXXX.dsl", &sdt->asl_file, &error);
> +    g_assert_no_error(error);
> +    close(fd);
> +
> +    /* 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);
> +    }
> +    g_string_append_printf(command_line, "-d %s", sdt->aml_file);
> +
> +    /* pass 'out' and 'out_err' in order to be redirected */
> +    g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, 
> &error);
> +    g_assert_no_error(error);
> +
> +    ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> +                              &sdt->asl_len, &error);
> +    g_assert(ret);
> +    g_assert_no_error(error);
> +    g_assert(sdt->asl_len);
> +
> +    g_free(out);
> +    g_free(out_err);
> +    g_string_free(command_line, true);
> +}
> +
> +#define COMMENT_END "*/"
> +#define DEF_BLOCK "DefinitionBlock ("
> +#define BLOCK_NAME_END ".aml"
> +
> +static GString *normalize_asl(gchar *asl_code)
> +{
> +    GString *asl = g_string_new(asl_code);
> +    gchar *comment, *block_name;
> +
> +    /* strip comments (different generation days) */
> +    comment = g_strstr_len(asl->str, asl->len, COMMENT_END);
> +    if (comment) {
> +        asl = g_string_erase(asl, 0, comment + sizeof(COMMENT_END) - 
> asl->str);
> +    }
> +
> +    /* strip def block name (it has file path in it) */
> +    if (g_str_has_prefix(asl->str, DEF_BLOCK)) {
> +        block_name = g_strstr_len(asl->str, asl->len, BLOCK_NAME_END);
> +        g_assert(block_name);
> +        asl = g_string_erase(asl, 0,
> +                             block_name + sizeof(BLOCK_NAME_END) - asl->str);
> +    }
> +
> +    return asl;
> +}
> +
> +static GArray *load_expected_aml(test_data *data)
> +{
> +    int i;
> +    AcpiSdtTable *sdt;
> +    gchar *aml_file;
> +    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) {
> +        AcpiSdtTable exp_sdt;
> +        sdt = &g_array_index(data->ssdt_tables, AcpiSdtTable, i);
> +
> +        memset(&exp_sdt, 0, sizeof(exp_sdt));
> +        exp_sdt.header.signature = sdt->header.signature;
> +
> +        aml_file = g_strdup_printf("%s/%s/%.4s", data_dir, data->machine,
> +                                   (gchar *)&exp_sdt.header.signature);
> +        exp_sdt.aml_file = aml_file;
> +        g_assert(g_file_test(aml_file, G_FILE_TEST_EXISTS));
> +        ret = g_file_get_contents(aml_file, &exp_sdt.aml,
> +                                  &exp_sdt.aml_len, &error);
> +        g_assert(ret);
> +        g_assert_no_error(error);
> +        g_assert(exp_sdt.aml);
> +        g_assert(exp_sdt.aml_len);
> +
> +        g_array_append_val(exp_ssdt_tables, exp_sdt);
> +    }
> +
> +    return exp_ssdt_tables;
> +}
> +
> +static void test_acpi_asl(test_data *data)
> +{
> +    int i;
> +    AcpiSdtTable *sdt, *exp_sdt;
> +    test_data exp_data;
> +
> +    memset(&exp_data, 0, sizeof(exp_data));
> +    exp_data.ssdt_tables = load_expected_aml(data);
> +    dump_aml_files(data);
> +    for (i = 0; i < data->ssdt_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);
> +
> +        load_asl(data->ssdt_tables, sdt);
> +        asl = normalize_asl(sdt->asl);
> +
> +        load_asl(exp_data.ssdt_tables, exp_sdt);
> +        exp_asl = normalize_asl(exp_sdt->asl);
> +
> +        g_assert(!g_strcmp0(asl->str, exp_asl->str));
> +        g_string_free(asl, true);
> +        g_string_free(exp_asl, true);
> +    }
> +
> +    free_test_data(&exp_data);
>  }
>  
>  static void test_acpi_one(const char *params, test_data *data)
> @@ -329,10 +530,14 @@ static void test_acpi_one(const char *params, test_data 
> *data)
>      uint8_t signature_high;
>      uint16_t signature;
>      int i;
> +    const char *device = "";
>  
> -    memset(data, 0, sizeof(*data));
> -    args = g_strdup_printf("-net none -display none %s %s",
> -                           params ? params : "", disk);
> +    if (!g_strcmp0(data->machine, MACHINE_Q35)) {
> +        device = ",id=hd -device ide-hd,drive=hd";
> +    }
> +
> +    args = g_strdup_printf("-net none -display none %s -drive file=%s%s,",
> +                           params ? params : "", disk, device);
>      qtest_start(args);
>  
>     /* Wait at most 1 minute */
> @@ -362,6 +567,10 @@ static void test_acpi_one(const char *params, test_data 
> *data)
>      test_acpi_dsdt_table(data);
>      test_acpi_ssdt_tables(data);
>  
> +    if (iasl_installed()) {
> +        test_acpi_asl(data);
> +    }
> +
>      qtest_quit(global_qtest);
>      g_free(args);
>  }
> @@ -373,8 +582,14 @@ static void test_acpi_tcg(void)
>      /* Supplying -machine accel argument overrides the default (qtest).
>       * This is to make guest actually run.
>       */
> +    memset(&data, 0, sizeof(data));
> +    data.machine = MACHINE_PC;
>      test_acpi_one("-machine accel=tcg", &data);
> +    free_test_data(&data);
>  
> +    memset(&data, 0, sizeof(data));
> +    data.machine = MACHINE_Q35;
> +    test_acpi_one("-machine q35,accel=tcg", &data);
>      free_test_data(&data);
>  }
>  
> -- 
> 1.8.3.1