On Mon, 2013-12-23 at 14:06 +0200, Michael S. Tsirkin wrote: > 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 <marce...@redhat.com> > > --- > > 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. Would be better, I agree, expect I have no idea how to include this file. I'll look it up and send it as different patch as you suggested.
Thanks for the review! Marcel > > 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