On Fri, May 23, 2014 at 12:00:12PM +0300, Michael S. Tsirkin wrote:
> > 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.
So I was about to send a patch with acpi-test.c renamed to
bios-tables-test.c, but the patch is basically removing all of
acpi-test.c, and creating a new file bios-tables-test.c.
Do you have a better way to rename the file first, and then I can
send a patch against it ? Or should we give up on renaming it
altogether ? Or should I just bite the bullet and cut'n'paste your
test harness into a new file specific to smbios ?
It's not particularly important to me which way we go -- I want to do
the right thing, whatever you decide that is :)
Thanks,
--Gabriel
> > 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);
> > }
--
------------------------------------
Gabriel L. Somlo
Director of Computing Services
Information Networking Institute
Carnegie Mellon University
4616 Henry St., Pittsburgh, PA 15213
+1.412.268.9310 www.ini.cmu.edu
