Re: [Qemu-devel] [PATCH v4 0/3] SMBIOS cleanup round

[Gabriel L. Somlo]

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);
 }