From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50616) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WneGh-0008UO-31 for qemu-devel@nongnu.org; Thu, 22 May 2014 21:27:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WneGU-0001b0-Rd for qemu-devel@nongnu.org; Thu, 22 May 2014 21:27:31 -0400 Received: from mail-qg0-x235.google.com ([2607:f8b0:400d:c04::235]:34910) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WneGU-0001Zt-Jr for qemu-devel@nongnu.org; Thu, 22 May 2014 21:27:18 -0400 Received: by mail-qg0-f53.google.com with SMTP id f51so6977741qge.12 for ; Thu, 22 May 2014 18:27:17 -0700 (PDT) Date: Thu, 22 May 2014 21:27:11 -0400 From: "Gabriel L. Somlo" Message-ID: <20140523012711.GA6121@crash.ini.cmu.edu> References: <1400508595-23024-1-git-send-email-somlo@cmu.edu> <20140519194448.GA14502@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140519194448.GA14502@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 0/3] SMBIOS cleanup round List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: pbonzini@redhat.com, lersek@redhat.com, qemu-devel@nongnu.org, kraxel@redhat.com Michael, On Mon, May 19, 2014 at 10:44:48PM +0300, Michael S. Tsirkin wrote: > One question: we don't seem to have unit-test for this > interface in qemu, do we? > I would like to see at least a basic test along the lines of > tests/acpi-test.c: run bios to load tables from QEMU, > then do some basic checks on the tables that bios loaded. So I took a closer look at tests/acpi-test.c today, and things slowly started to shift into focus :) So now I have two questions: 1. There's a fairly complex setup (create a boot disk, start the guest, loop around waiting for the bios to finish booting, watch when your disk-based boot loader runs, etc.) before starting to examine the guest memory for the presence and correctness of the acpi tables. Would it make sense to rename this file to something like e.g. tests/biostables-test.c, and add checks for smbios to the already started and booted guest ? If not, I'd have to replicate most of your test-harness code, which is almost half of acpi-test.c. That shouldn't be hard (you already did the heavy lifting on that one), but I intuitively dislike multiple cut'n'paste clones of significant code fragments :) 2. Assuming we can run smbios tests alongside acpi, what do you think of the patch below ? I've currently stopped after finding and checking the integrity of the smbios entry point structure. Not sure if I really need to walk the tables themselves, or what I'd be testing for if I did :) Feedback/comments/flames welcome ! :) Thanks much, --Gabriel diff --git a/tests/acpi-test.c b/tests/acpi-test.c index 76fbccf..66cde26 100644 --- a/tests/acpi-test.c +++ b/tests/acpi-test.c @@ -18,6 +18,7 @@ #include "libqtest.h" #include "qemu/compiler.h" #include "hw/i386/acpi-defs.h" +#include "hw/i386/smbios.h" #define MACHINE_PC "pc" #define MACHINE_Q35 "q35" @@ -46,6 +47,8 @@ typedef struct { uint32_t *rsdt_tables_addr; int rsdt_tables_nr; GArray *tables; + uint32_t smbios_ep_addr; + struct smbios_entry_point smbios_ep_table; } test_data; #define LOW(x) ((x) & 0xff) @@ -220,6 +223,28 @@ static void test_acpi_rsdp_address(test_data *data) data->rsdp_addr = off; } +static void test_smbios_ep_address(test_data *data) +{ + uint32_t off; + + /* find smbios entry point structure */ + for (off = 0xf0000; off < 0x100000; off += 0x10) { + uint8_t sig[] = "_SM_"; + int i; + + for (i = 0; i < sizeof sig - 1; ++i) { + sig[i] = readb(off + i); + } + + if (!memcmp(sig, "_SM_", sizeof sig)) { + break; + } + } + + g_assert_cmphex(off, <, 0x100000); + data->smbios_ep_addr = off; +} + static void test_acpi_rsdp_table(test_data *data) { AcpiRsdpDescriptor *rsdp_table = &data->rsdp_table; @@ -238,6 +263,34 @@ static void test_acpi_rsdp_table(test_data *data) g_assert(!acpi_checksum((uint8_t *)rsdp_table, 20)); } +static void test_smbios_ep_table(test_data *data) +{ + struct smbios_entry_point *ep_table = &data->smbios_ep_table; + uint32_t addr = data->smbios_ep_addr; + + ACPI_READ_ARRAY(ep_table->anchor_string, addr); + g_assert(!memcmp(ep_table->anchor_string, "_SM_", 4)); + ACPI_READ_FIELD(ep_table->checksum, addr); + ACPI_READ_FIELD(ep_table->length, addr); + ACPI_READ_FIELD(ep_table->smbios_major_version, addr); + ACPI_READ_FIELD(ep_table->smbios_minor_version, addr); + ACPI_READ_FIELD(ep_table->max_structure_size, addr); + ACPI_READ_FIELD(ep_table->entry_point_revision, addr); + ACPI_READ_ARRAY(ep_table->formatted_area, addr); + ACPI_READ_ARRAY(ep_table->intermediate_anchor_string, addr); + g_assert(!memcmp(ep_table->intermediate_anchor_string, "_DMI_", 5)); + ACPI_READ_FIELD(ep_table->intermediate_checksum, addr); + ACPI_READ_FIELD(ep_table->structure_table_length, addr); + g_assert_cmpuint(ep_table->structure_table_length, >, 0); + ACPI_READ_FIELD(ep_table->structure_table_address, addr); + ACPI_READ_FIELD(ep_table->number_of_structures, addr); + g_assert_cmpuint(ep_table->number_of_structures, >, 0); + ACPI_READ_FIELD(ep_table->smbios_bcd_revision, addr); + g_assert(!acpi_checksum((uint8_t *)ep_table, sizeof *ep_table)); + g_assert(!acpi_checksum((uint8_t *)ep_table + 0x10, + sizeof *ep_table - 0x10)); +} + static void test_acpi_rsdt_table(test_data *data) { AcpiRsdtDescriptorRev1 *rsdt_table = &data->rsdt_table; @@ -633,6 +686,9 @@ static void test_acpi_one(const char *params, test_data *data) } } + test_smbios_ep_address(data); + test_smbios_ep_table(data); + qtest_quit(global_qtest); g_free(args); }