On Thu, May 22, 2014 at 09:27:11PM -0400, Gabriel L. Somlo wrote:
> 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 :)
Sure, fine.
> 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
Looks good to me.
>
> 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);
> }
