[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-1.7] acpi unit-test: ensure both 1.6 machine
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH for-1.7] acpi unit-test: ensure both 1.6 machine and default machine received the same acpi tables |
Date: |
Wed, 13 Nov 2013 21:43:15 +0200 |
On Wed, Nov 13, 2013 at 06:11:25PM +0200, Marcel Apfelbaum wrote:
> Machines before 1.6 used ACPI supplied by SeaBios,
> for 1.7+ machines the tables are build by qemu.
> This patch checks that both machines have the same ACPI tables.
>
> Also checks the signature of FACS table.
>
> Signed-off-by: Marcel Apfelbaum <address@hidden>
> ---
> Note that for the moment we check only that the
> machines have the same tables and not the content of the tables.
>
> This patch assumes that the new bios.bin is used.
>
> tests/acpi-test.c | 106
> ++++++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 83 insertions(+), 23 deletions(-)
>
> diff --git a/tests/acpi-test.c b/tests/acpi-test.c
> index d6ff66f..ba66963 100644
> --- a/tests/acpi-test.c
> +++ b/tests/acpi-test.c
> @@ -30,10 +30,11 @@ typedef struct {
> AcpiRsdpDescriptor rsdp_table;
> AcpiRsdtDescriptorRev1 rsdt_table;
> AcpiFadtDescriptorRev1 fadt_table;
> + AcpiFacsDescriptorRev1 facs_table;
> uint32_t *rsdt_tables_addr;
> int rsdt_tables_nr;
> AcpiSdtTable dsdt_table;
> - AcpiSdtTable *ssdt_tables;
> + GArray *ssdt_tables;
> } test_data;
>
> #define LOW(x) ((x) & 0xff)
> @@ -117,6 +118,18 @@ static uint8_t boot_sector[0x200] = {
>
> static const char *disk = "tests/acpi-test-disk.raw";
>
> +static void free_test_data(test_data *data)
> +{
> + int i;
> +
> + 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);
> + }
> + g_array_free(data->ssdt_tables, false);
> + g_free(data->dsdt_table.aml);
> +}
> +
> static uint8_t acpi_checksum(const uint8_t *data, int len)
> {
> int i;
> @@ -252,6 +265,22 @@ static void test_acpi_fadt_table(test_data *data)
> g_assert(!acpi_checksum((uint8_t *)fadt_table, fadt_table->length));
> }
>
> +static void test_acpi_facs_table(test_data *data)
> +{
> + AcpiFacsDescriptorRev1 *facs_table = &data->facs_table;
> + uint32_t addr = data->fadt_table.firmware_ctrl;
> +
> + ACPI_READ_FIELD(facs_table->signature, addr);
> + ACPI_READ_FIELD(facs_table->length, addr);
> + ACPI_READ_FIELD(facs_table->hardware_signature, addr);
> + ACPI_READ_FIELD(facs_table->firmware_waking_vector, addr);
> + ACPI_READ_FIELD(facs_table->global_lock, addr);
> + ACPI_READ_FIELD(facs_table->flags, addr);
> + ACPI_READ_ARRAY(facs_table->resverved3, addr);
> +
> + g_assert_cmphex(facs_table->signature, ==, ACPI_FACS_SIGNATURE);
> +}
> +
> static void test_dst_table(AcpiSdtTable *sdt_table, uint32_t addr)
> {
> uint8_t checksum;
> @@ -278,30 +307,30 @@ static void test_acpi_dsdt_table(test_data *data)
>
> static void test_acpi_ssdt_tables(test_data *data)
> {
> - AcpiSdtTable *ssdt_tables;
> + GArray *ssdt_tables;
> int ssdt_tables_nr = data->rsdt_tables_nr - 1; /* fadt is first */
> int i;
>
> - ssdt_tables = g_new0(AcpiSdtTable, ssdt_tables_nr);
> + ssdt_tables = g_array_sized_new(false, true, sizeof(AcpiSdtTable),
> + ssdt_tables_nr);
> for (i = 0; i < ssdt_tables_nr; i++) {
> - AcpiSdtTable *ssdt_table = &ssdt_tables[i];
> + AcpiSdtTable ssdt_table;
> uint32_t addr = data->rsdt_tables_addr[i + 1]; /* fadt is first */
> -
> - test_dst_table(ssdt_table, addr);
> + test_dst_table(&ssdt_table, addr);
> + g_array_append_val(ssdt_tables, ssdt_table);
> }
> data->ssdt_tables = ssdt_tables;
> }
>
> -static void test_acpi_one(const char *params)
> +static void test_acpi_one(const char *params, test_data *data)
> {
> char *args;
> uint8_t signature_low;
> uint8_t signature_high;
> uint16_t signature;
> int i;
> - test_data data;
>
> - memset(&data, 0, sizeof(data));
> + memset(data, 0, sizeof(*data));
> args = g_strdup_printf("-net none -display none %s %s",
> params ? params : "", disk);
> qtest_start(args);
> @@ -325,30 +354,61 @@ static void test_acpi_one(const char *params)
> }
> g_assert_cmphex(signature, ==, SIGNATURE);
>
> - test_acpi_rsdp_address(&data);
> - test_acpi_rsdp_table(&data);
> - test_acpi_rsdt_table(&data);
> - test_acpi_fadt_table(&data);
> - test_acpi_dsdt_table(&data);
> - test_acpi_ssdt_tables(&data);
> -
> - g_free(data.rsdt_tables_addr);
> - for (i = 0; i < (data.rsdt_tables_nr - 1); ++i) {
> - g_free(data.ssdt_tables[i].aml);
> - }
> - g_free(data.ssdt_tables);
> - g_free(data.dsdt_table.aml);
> + test_acpi_rsdp_address(data);
> + test_acpi_rsdp_table(data);
> + test_acpi_rsdt_table(data);
> + test_acpi_fadt_table(data);
> + test_acpi_facs_table(data);
> + test_acpi_dsdt_table(data);
> + test_acpi_ssdt_tables(data);
>
> qtest_quit(global_qtest);
> g_free(args);
> }
>
> +static gint compare_table_signature(gconstpointer a, gconstpointer b)
> +{
> + AcpiSdtTable *table1 = (AcpiSdtTable *)a;
> + AcpiSdtTable *table2 = (AcpiSdtTable *)b;
> + return (gint)table1->header.signature - (gint)table2->header.signature;
> +}
> +
> +static void test_acpi_compare_machines(test_data *data1, test_data *data2)
> +{
> + int i;
> +
> + /*
> + * Check that both machine have the same acpi tables.
> + * We only need to check the ssdt tables, because all
> + * the other tables are always present */
> + g_assert_cmpuint(data1->ssdt_tables->len, ==, data2->ssdt_tables->len);
> +
> + g_array_sort(data1->ssdt_tables, compare_table_signature);
> + g_array_sort(data2->ssdt_tables, compare_table_signature);
> +
> + for (i = 0; i < data1->ssdt_tables->len; ++i) {
> + AcpiSdtTable *table1 = &g_array_index(data1->ssdt_tables,
> AcpiSdtTable, i);
> + AcpiSdtTable *table2 = &g_array_index(data2->ssdt_tables,
> AcpiSdtTable, i);
> +
> + g_assert_cmphex(table1->header.signature, ==,
> table2->header.signature);
> + }
> +}
> +
> static void test_acpi_tcg(void)
> {
> + test_data data, data_16;
> +
> /* Supplying -machine accel argument overrides the default (qtest).
> * This is to make guest actually run.
> */
> - test_acpi_one("-machine accel=tcg");
> + test_acpi_one("-machine accel=tcg", &data);
> + test_acpi_one("-machine pc-i440fx-1.6,accel=tcg", &data_16);
I'm not sure this is a good idea: we can change tables and
they won't match 1.6 anymore.
How about storing expected tables under tests/?
This way we'll just update the expected values when we
change something.
> +
> + /* for 1.6 the acpi tables come from bios */
> + test_acpi_compare_machines(&data, &data_16);
> +
> + free_test_data(&data);
> + free_test_data(&data_16);
> }
>
> int main(int argc, char *argv[])
> --
> 1.8.3.1