diff mbox

[v8,7/8] tests: Add unit tests for the VM Generation ID feature

Message ID 5353279f68be14c63f049bc34902675290e45b1d.1487286467.git.ben@skyportsystems.com (mailing list archive)
State New, archived
Headers show

Commit Message

ben@skyportsystems.com Feb. 16, 2017, 11:15 p.m. UTC
From: Ben Warren <ben@skyportsystems.com>

The following tests are implemented:
* test that a GUID passed in by command line is propagated to the guest.
  Read the GUID from guest memory
* test that the "auto" argument to the GUID generates a valid GUID, as
  seen by the guest.
* test that a GUID passed in can be queried from the monitor

  This patch is loosely based on a previous patch from:
  Gal Hammer <ghammer@redhat.com>  and Igor Mammedov <imammedo@redhat.com>

Signed-off-by: Ben Warren <ben@skyportsystems.com>
---
 tests/Makefile.include |   2 +
 tests/vmgenid-test.c   | 200 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 202 insertions(+)
 create mode 100644 tests/vmgenid-test.c

Comments

Igor Mammedov Feb. 20, 2017, 2:49 p.m. UTC | #1
On Thu, 16 Feb 2017 15:15:39 -0800
ben@skyportsystems.com wrote:

> From: Ben Warren <ben@skyportsystems.com>
> 
> The following tests are implemented:
> * test that a GUID passed in by command line is propagated to the guest.
>   Read the GUID from guest memory
> * test that the "auto" argument to the GUID generates a valid GUID, as
>   seen by the guest.
> * test that a GUID passed in can be queried from the monitor
> 
>   This patch is loosely based on a previous patch from:
>   Gal Hammer <ghammer@redhat.com>  and Igor Mammedov <imammedo@redhat.com>
> 
> Signed-off-by: Ben Warren <ben@skyportsystems.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  tests/Makefile.include |   2 +
>  tests/vmgenid-test.c   | 200 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 202 insertions(+)
>  create mode 100644 tests/vmgenid-test.c
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 143507e..8d36341 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -241,6 +241,7 @@ check-qtest-i386-y += tests/usb-hcd-xhci-test$(EXESUF)
>  gcov-files-i386-y += hw/usb/hcd-xhci.c
>  check-qtest-i386-y += tests/pc-cpu-test$(EXESUF)
>  check-qtest-i386-y += tests/q35-test$(EXESUF)
> +check-qtest-i386-y += tests/vmgenid-test$(EXESUF)
>  gcov-files-i386-y += hw/pci-host/q35.c
>  check-qtest-i386-$(CONFIG_VHOST_NET_TEST_i386) += tests/vhost-user-test$(EXESUF)
>  ifeq ($(CONFIG_VHOST_NET_TEST_i386),)
> @@ -726,6 +727,7 @@ tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o contrib/ivshmem-server/ivshmem
>  tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o contrib/libvhost-user/libvhost-user.o $(test-util-obj-y)
>  tests/test-uuid$(EXESUF): tests/test-uuid.o $(test-util-obj-y)
>  tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
> +tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/acpi-utils.o
>  
>  tests/migration/stress$(EXESUF): tests/migration/stress.o
>  	$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"LINK","$(TARGET_DIR)$@")
> diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c
> new file mode 100644
> index 0000000..123beae
> --- /dev/null
> +++ b/tests/vmgenid-test.c
> @@ -0,0 +1,200 @@
> +/*
> + * QTest testcase for VM Generation ID
> + *
> + * Copyright (c) 2016 Red Hat, Inc.
> + * Copyright (c) 2017 Skyport Systems
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include <glib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include "qemu/osdep.h"
> +#include "qemu/bitmap.h"
> +#include "qemu/uuid.h"
> +#include "hw/acpi/acpi-defs.h"
> +#include "acpi-utils.h"
> +#include "libqtest.h"
> +
> +#define VGID_GUID "324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
> +#define VMGENID_GUID_OFFSET 40   /* allow space for
> +                                  * OVMF SDT Header Probe Supressor
> +                                  */
> +#define RSDP_ADDR_INVALID 0x100000 /* RSDP must be below this address */
> +#define RSDP_SLEEP_US     100000   /* Sleep for 100ms between tries */
> +#define RSDP_TRIES_MAX    100      /* Max total time is 10 seconds */
> +
> +typedef struct {
> +    AcpiTableHeader header;
> +    gchar name_op;
> +    gchar vgia[4];
> +    gchar val_op;
> +    uint32_t vgia_val;
> +} QEMU_PACKED VgidTable;
> +
> +static uint32_t acpi_find_vgia(void)
> +{
> +    uint32_t off;
> +    AcpiRsdpDescriptor rsdp_table;
> +    uint32_t rsdt;
> +    AcpiRsdtDescriptorRev1 rsdt_table;
> +    int tables_nr;
> +    uint32_t *tables;
> +    AcpiTableHeader ssdt_table;
> +    VgidTable vgid_table;
> +    int i;
> +
> +    /* Tables may take a short time to be set up by the guest */
> +    for (i = 0; i < RSDP_TRIES_MAX; i++) {
> +        off = acpi_find_rsdp_address();
> +        if (off < RSDP_ADDR_INVALID) {
> +            break;
> +        }
> +        g_usleep(RSDP_SLEEP_US);
> +    }
> +    g_assert_cmphex(off, <, RSDP_ADDR_INVALID);
> +
> +    acpi_parse_rsdp_table(off, &rsdp_table);
> +
> +    rsdt = rsdp_table.rsdt_physical_address;
> +    /* read the header */
> +    ACPI_READ_TABLE_HEADER(&rsdt_table, rsdt);
> +    ACPI_ASSERT_CMP(rsdt_table.signature, "RSDT");
> +
> +    /* compute the table entries in rsdt */
> +    tables_nr = (rsdt_table.length - sizeof(AcpiRsdtDescriptorRev1)) /
> +                sizeof(uint32_t);
> +    g_assert_cmpint(tables_nr, >, 0);
> +
> +    /* get the addresses of the tables pointed by rsdt */
> +    tables = g_new0(uint32_t, tables_nr);
> +    ACPI_READ_ARRAY_PTR(tables, tables_nr, rsdt);
> +
> +    for (i = 0; i < tables_nr; i++) {
> +        ACPI_READ_TABLE_HEADER(&ssdt_table, tables[i]);
> +        if (!strncmp((char *)ssdt_table.oem_table_id, "VMGENID", 7)) {
> +            /* the first entry in the table should be VGIA
> +             * That's all we need
> +             */
> +            ACPI_READ_FIELD(vgid_table.name_op, tables[i]);
> +            g_assert(vgid_table.name_op == 0x08);  /* name */
> +            ACPI_READ_ARRAY(vgid_table.vgia, tables[i]);
> +            g_assert(memcmp(vgid_table.vgia, "VGIA", 4) == 0);
> +            ACPI_READ_FIELD(vgid_table.val_op, tables[i]);
> +            g_assert(vgid_table.val_op == 0x0C);  /* dword */
> +            ACPI_READ_FIELD(vgid_table.vgia_val, tables[i]);
> +            /* The GUID is written at a fixed offset into the fw_cfg file
> +             * in order to implement the "OVMF SDT Header probe suppressor"
> +             * see docs/specs/vmgenid.txt for more details
> +             */
> +            return vgid_table.vgia_val + VMGENID_GUID_OFFSET;
> +        }
> +    }
> +    return 0;
> +}
> +
> +static void read_guid_from_memory(QemuUUID *guid)
> +{
> +    uint32_t vmgenid_addr;
> +    int i;
> +
> +    vmgenid_addr = acpi_find_vgia();
> +    g_assert(vmgenid_addr);
> +
> +    /* Read the GUID directly from guest memory */
> +    for (i = 0; i < 16; i++) {
> +        guid->data[i] = readb(vmgenid_addr + i);
> +    }
> +    /* The GUID is in little-endian format in the guest, while QEMU
> +     * uses big-endian.  Swap after reading.
> +     */
> +    qemu_uuid_bswap(guid);
> +}
> +
> +static void read_guid_from_monitor(QemuUUID *guid)
> +{
> +    QDict *rsp, *rsp_ret;
> +    const char *guid_str;
> +
> +    rsp = qmp("{ 'execute': 'query-vm-generation-id' }");
> +    if (qdict_haskey(rsp, "return")) {
> +        rsp_ret = qdict_get_qdict(rsp, "return");
> +        g_assert(qdict_haskey(rsp_ret, "guid"));
> +        guid_str = qdict_get_str(rsp_ret, "guid");
> +        g_assert(qemu_uuid_parse(guid_str, guid) == 0);
> +    }
> +    QDECREF(rsp);
> +}
> +
> +static void vmgenid_set_guid_test(void)
> +{
> +    QemuUUID expected, measured;
> +    gchar *cmd;
> +
> +    g_assert(qemu_uuid_parse(VGID_GUID, &expected) == 0);
> +
> +    cmd = g_strdup_printf("-machine accel=tcg -device vmgenid,id=testvgid,"
> +                          "guid=%s", VGID_GUID);
> +    qtest_start(cmd);
> +
> +    /* Read the GUID from accessing guest memory */
> +    read_guid_from_memory(&measured);
> +    g_assert(memcmp(measured.data, expected.data, sizeof(measured.data)) == 0);
> +
> +    qtest_quit(global_qtest);
> +    g_free(cmd);
> +}
> +
> +static void vmgenid_set_guid_auto_test(void)
> +{
> +    const char *cmd;
> +    QemuUUID measured;
> +
> +    cmd = "-machine accel=tcg -device vmgenid,id=testvgid," "guid=auto";
> +    qtest_start(cmd);
> +
> +    read_guid_from_memory(&measured);
> +
> +    /* Just check that the GUID is non-null */
> +    g_assert(!qemu_uuid_is_null(&measured));
> +
> +    qtest_quit(global_qtest);
> +}
> +
> +static void vmgenid_query_monitor_test(void)
> +{
> +    QemuUUID expected, measured;
> +    gchar *cmd;
> +
> +    g_assert(qemu_uuid_parse(VGID_GUID, &expected) == 0);
> +
> +    cmd = g_strdup_printf("-machine accel=tcg -device vmgenid,id=testvgid,"
> +                          "guid=%s", VGID_GUID);
> +    qtest_start(cmd);
> +
> +    /* Read the GUID via the monitor */
> +    read_guid_from_monitor(&measured);
> +    g_assert(memcmp(measured.data, expected.data, sizeof(measured.data)) == 0);
> +
> +    qtest_quit(global_qtest);
> +    g_free(cmd);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    int ret;
> +
> +    g_test_init(&argc, &argv, NULL);
> +
> +    qtest_add_func("/vmgenid/vmgenid/set-guid",
> +                   vmgenid_set_guid_test);
> +    qtest_add_func("/vmgenid/vmgenid/set-guid-auto",
> +                   vmgenid_set_guid_auto_test);
> +    qtest_add_func("/vmgenid/vmgenid/query-monitor",
> +                   vmgenid_query_monitor_test);
> +    ret = g_test_run();
> +
> +    return ret;
> +}
Marc-André Lureau April 21, 2017, 10:14 a.m. UTC | #2
Hi,

Was this patch intentionally dropped from the series?

On Mon, Feb 20, 2017 at 7:12 PM Igor Mammedov <imammedo@redhat.com> wrote:

> On Thu, 16 Feb 2017 15:15:39 -0800
> ben@skyportsystems.com wrote:
>
> > From: Ben Warren <ben@skyportsystems.com>
> >
> > The following tests are implemented:
> > * test that a GUID passed in by command line is propagated to the guest.
> >   Read the GUID from guest memory
> > * test that the "auto" argument to the GUID generates a valid GUID, as
> >   seen by the guest.
> > * test that a GUID passed in can be queried from the monitor
> >
> >   This patch is loosely based on a previous patch from:
> >   Gal Hammer <ghammer@redhat.com>  and Igor Mammedov <
> imammedo@redhat.com>
> >
> > Signed-off-by: Ben Warren <ben@skyportsystems.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>
> > ---
> >  tests/Makefile.include |   2 +
> >  tests/vmgenid-test.c   | 200
> +++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 202 insertions(+)
> >  create mode 100644 tests/vmgenid-test.c
> >
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 143507e..8d36341 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -241,6 +241,7 @@ check-qtest-i386-y +=
> tests/usb-hcd-xhci-test$(EXESUF)
> >  gcov-files-i386-y += hw/usb/hcd-xhci.c
> >  check-qtest-i386-y += tests/pc-cpu-test$(EXESUF)
> >  check-qtest-i386-y += tests/q35-test$(EXESUF)
> > +check-qtest-i386-y += tests/vmgenid-test$(EXESUF)
> >  gcov-files-i386-y += hw/pci-host/q35.c
> >  check-qtest-i386-$(CONFIG_VHOST_NET_TEST_i386) +=
> tests/vhost-user-test$(EXESUF)
> >  ifeq ($(CONFIG_VHOST_NET_TEST_i386),)
> > @@ -726,6 +727,7 @@ tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o
> contrib/ivshmem-server/ivshmem
> >  tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o
> contrib/libvhost-user/libvhost-user.o $(test-util-obj-y)
> >  tests/test-uuid$(EXESUF): tests/test-uuid.o $(test-util-obj-y)
> >  tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
> > +tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/acpi-utils.o
> >
> >  tests/migration/stress$(EXESUF): tests/migration/stress.o
> >       $(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@
> $< ,"LINK","$(TARGET_DIR)$@")
> > diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c
> > new file mode 100644
> > index 0000000..123beae
> > --- /dev/null
> > +++ b/tests/vmgenid-test.c
> > @@ -0,0 +1,200 @@
> > +/*
> > + * QTest testcase for VM Generation ID
> > + *
> > + * Copyright (c) 2016 Red Hat, Inc.
> > + * Copyright (c) 2017 Skyport Systems
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include <glib.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +#include "qemu/osdep.h"
> > +#include "qemu/bitmap.h"
> > +#include "qemu/uuid.h"
> > +#include "hw/acpi/acpi-defs.h"
> > +#include "acpi-utils.h"
> > +#include "libqtest.h"
> > +
> > +#define VGID_GUID "324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
> > +#define VMGENID_GUID_OFFSET 40   /* allow space for
> > +                                  * OVMF SDT Header Probe Supressor
> > +                                  */
> > +#define RSDP_ADDR_INVALID 0x100000 /* RSDP must be below this address */
> > +#define RSDP_SLEEP_US     100000   /* Sleep for 100ms between tries */
> > +#define RSDP_TRIES_MAX    100      /* Max total time is 10 seconds */
> > +
> > +typedef struct {
> > +    AcpiTableHeader header;
> > +    gchar name_op;
> > +    gchar vgia[4];
> > +    gchar val_op;
> > +    uint32_t vgia_val;
> > +} QEMU_PACKED VgidTable;
> > +
> > +static uint32_t acpi_find_vgia(void)
> > +{
> > +    uint32_t off;
> > +    AcpiRsdpDescriptor rsdp_table;
> > +    uint32_t rsdt;
> > +    AcpiRsdtDescriptorRev1 rsdt_table;
> > +    int tables_nr;
> > +    uint32_t *tables;
> > +    AcpiTableHeader ssdt_table;
> > +    VgidTable vgid_table;
> > +    int i;
> > +
> > +    /* Tables may take a short time to be set up by the guest */
> > +    for (i = 0; i < RSDP_TRIES_MAX; i++) {
> > +        off = acpi_find_rsdp_address();
> > +        if (off < RSDP_ADDR_INVALID) {
> > +            break;
> > +        }
> > +        g_usleep(RSDP_SLEEP_US);
> > +    }
> > +    g_assert_cmphex(off, <, RSDP_ADDR_INVALID);
> > +
> > +    acpi_parse_rsdp_table(off, &rsdp_table);
> > +
> > +    rsdt = rsdp_table.rsdt_physical_address;
> > +    /* read the header */
> > +    ACPI_READ_TABLE_HEADER(&rsdt_table, rsdt);
> > +    ACPI_ASSERT_CMP(rsdt_table.signature, "RSDT");
> > +
> > +    /* compute the table entries in rsdt */
> > +    tables_nr = (rsdt_table.length - sizeof(AcpiRsdtDescriptorRev1)) /
> > +                sizeof(uint32_t);
> > +    g_assert_cmpint(tables_nr, >, 0);
> > +
> > +    /* get the addresses of the tables pointed by rsdt */
> > +    tables = g_new0(uint32_t, tables_nr);
> > +    ACPI_READ_ARRAY_PTR(tables, tables_nr, rsdt);
> > +
> > +    for (i = 0; i < tables_nr; i++) {
> > +        ACPI_READ_TABLE_HEADER(&ssdt_table, tables[i]);
> > +        if (!strncmp((char *)ssdt_table.oem_table_id, "VMGENID", 7)) {
> > +            /* the first entry in the table should be VGIA
> > +             * That's all we need
> > +             */
> > +            ACPI_READ_FIELD(vgid_table.name_op, tables[i]);
> > +            g_assert(vgid_table.name_op == 0x08);  /* name */
> > +            ACPI_READ_ARRAY(vgid_table.vgia, tables[i]);
> > +            g_assert(memcmp(vgid_table.vgia, "VGIA", 4) == 0);
> > +            ACPI_READ_FIELD(vgid_table.val_op, tables[i]);
> > +            g_assert(vgid_table.val_op == 0x0C);  /* dword */
> > +            ACPI_READ_FIELD(vgid_table.vgia_val, tables[i]);
> > +            /* The GUID is written at a fixed offset into the fw_cfg
> file
> > +             * in order to implement the "OVMF SDT Header probe
> suppressor"
> > +             * see docs/specs/vmgenid.txt for more details
> > +             */
> > +            return vgid_table.vgia_val + VMGENID_GUID_OFFSET;
> > +        }
> > +    }
> > +    return 0;
> > +}
> > +
> > +static void read_guid_from_memory(QemuUUID *guid)
> > +{
> > +    uint32_t vmgenid_addr;
> > +    int i;
> > +
> > +    vmgenid_addr = acpi_find_vgia();
> > +    g_assert(vmgenid_addr);
> > +
> > +    /* Read the GUID directly from guest memory */
> > +    for (i = 0; i < 16; i++) {
> > +        guid->data[i] = readb(vmgenid_addr + i);
> > +    }
> > +    /* The GUID is in little-endian format in the guest, while QEMU
> > +     * uses big-endian.  Swap after reading.
> > +     */
> > +    qemu_uuid_bswap(guid);
> > +}
> > +
> > +static void read_guid_from_monitor(QemuUUID *guid)
> > +{
> > +    QDict *rsp, *rsp_ret;
> > +    const char *guid_str;
> > +
> > +    rsp = qmp("{ 'execute': 'query-vm-generation-id' }");
> > +    if (qdict_haskey(rsp, "return")) {
> > +        rsp_ret = qdict_get_qdict(rsp, "return");
> > +        g_assert(qdict_haskey(rsp_ret, "guid"));
> > +        guid_str = qdict_get_str(rsp_ret, "guid");
> > +        g_assert(qemu_uuid_parse(guid_str, guid) == 0);
> > +    }
> > +    QDECREF(rsp);
> > +}
> > +
> > +static void vmgenid_set_guid_test(void)
> > +{
> > +    QemuUUID expected, measured;
> > +    gchar *cmd;
> > +
> > +    g_assert(qemu_uuid_parse(VGID_GUID, &expected) == 0);
> > +
> > +    cmd = g_strdup_printf("-machine accel=tcg -device
> vmgenid,id=testvgid,"
> > +                          "guid=%s", VGID_GUID);
> > +    qtest_start(cmd);
> > +
> > +    /* Read the GUID from accessing guest memory */
> > +    read_guid_from_memory(&measured);
> > +    g_assert(memcmp(measured.data, expected.data,
> sizeof(measured.data)) == 0);
> > +
> > +    qtest_quit(global_qtest);
> > +    g_free(cmd);
> > +}
> > +
> > +static void vmgenid_set_guid_auto_test(void)
> > +{
> > +    const char *cmd;
> > +    QemuUUID measured;
> > +
> > +    cmd = "-machine accel=tcg -device vmgenid,id=testvgid," "guid=auto";
> > +    qtest_start(cmd);
> > +
> > +    read_guid_from_memory(&measured);
> > +
> > +    /* Just check that the GUID is non-null */
> > +    g_assert(!qemu_uuid_is_null(&measured));
> > +
> > +    qtest_quit(global_qtest);
> > +}
> > +
> > +static void vmgenid_query_monitor_test(void)
> > +{
> > +    QemuUUID expected, measured;
> > +    gchar *cmd;
> > +
> > +    g_assert(qemu_uuid_parse(VGID_GUID, &expected) == 0);
> > +
> > +    cmd = g_strdup_printf("-machine accel=tcg -device
> vmgenid,id=testvgid,"
> > +                          "guid=%s", VGID_GUID);
> > +    qtest_start(cmd);
> > +
> > +    /* Read the GUID via the monitor */
> > +    read_guid_from_monitor(&measured);
> > +    g_assert(memcmp(measured.data, expected.data,
> sizeof(measured.data)) == 0);
> > +
> > +    qtest_quit(global_qtest);
> > +    g_free(cmd);
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > +    int ret;
> > +
> > +    g_test_init(&argc, &argv, NULL);
> > +
> > +    qtest_add_func("/vmgenid/vmgenid/set-guid",
> > +                   vmgenid_set_guid_test);
> > +    qtest_add_func("/vmgenid/vmgenid/set-guid-auto",
> > +                   vmgenid_set_guid_auto_test);
> > +    qtest_add_func("/vmgenid/vmgenid/query-monitor",
> > +                   vmgenid_query_monitor_test);
> > +    ret = g_test_run();
> > +
> > +    return ret;
> > +}
>
>
> --
Marc-André Lureau
Zhijian Li (Fujitsu)" via April 21, 2017, 5:59 p.m. UTC | #3
Hi,

> On Apr 21, 2017, at 3:14 AM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> 
> Hi,
> 
> Was this patch intentionally dropped from the series?
> 

Good question.  I thought the whole patch series was pulled in, but it looks like this one was not.  I guess we’ll see what Michael has to say.

—Ben
<snip>
> 
> 
> -- 
> Marc-André Lureau
Laszlo Ersek April 24, 2017, 12:28 p.m. UTC | #4
On 04/21/17 19:59, Ben Warren wrote:
> Hi,
> 
>> On Apr 21, 2017, at 3:14 AM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
>>
>> Hi,
>>
>> Was this patch intentionally dropped from the series?
>>
> 
> Good question.  I thought the whole patch series was pulled in, but it looks like this one was not.  I guess we’ll see what Michael has to say.

Unfortunately, the patch was lost due to a race condition in maintenance.

(1) The VMGENID unit test, added in this patch, can only succeed if the
SeaBIOS binary bundled with QEMU contains VMGENID patches as well. When
this v8 patch series was originally posted (Feb 16th), the SeaBIOS
binary bundled with QEMU wasn't ready for this patch. So the patch was
delayed / postponed.

(2) For that reason, Michael sent the PULL req with the *rest* of the
patches, on March 1st:

http://mid.mail-archive.com/1488435591-17882-1-git-send-email-mst@redhat.com

(3) Gerd updated the SeaBIOS binary meanwhile, to a revision that
contained Ben's matching bios patches:

$ git log v2.8.0..v2.9.0 -- pc-bios/bios.bin

commit 8779fccbef0c2e97fd6564ddf9f1df9fc724f2f0
Author: Gerd Hoffmann <kraxel@redhat.com>
Date:   Tue Feb 28 09:52:32 2017 +0100

    seabios: update to 1.10.2 release

    git shortlog rel-1.10.1..rel-1.10.2
    ===================================

    Ben Warren (5):
          QEMU DMA: Add DMA write capability
          romfile-loader: Switch to using named structs
          QEMU fw_cfg: Add command to write back address of file
          QEMU fw_cfg: Add functions for accessing files by key
          QEMU fw_cfg: Write fw_cfg back on S3 resume

    Kevin O'Connor (1):
          ps2port: Disable keyboard/mouse prior to resetting ps2 controller

    Ladi Prosek (1):
          ahci: Set upper 32-bit registers to zero

    Paul Menzel (1):
          vgasrc: Increase debug level

    Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

(4) In the end, the BIOS update commit, and the VMGENID v8 commits, are
*not* in an ancestor/descandant relationship either way -- they are
siblings:

  $ git-contains 42697d88de6b 8779fccbef0c
  42697d88de6b [42697d88de6b MAINTAINERS: Add VM Generation ID entries]
  NEITHER CONTAINS NOR IS CONTAINED BY
  8779fccbef0c [8779fccbef0c seabios: update to 1.10.2 release]

And then the patch missed v2.9.0.


I suggest to repost the patch now, and then maybe include it in 2.9.1
stable.

Thanks
Laszlo
diff mbox

Patch

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 143507e..8d36341 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -241,6 +241,7 @@  check-qtest-i386-y += tests/usb-hcd-xhci-test$(EXESUF)
 gcov-files-i386-y += hw/usb/hcd-xhci.c
 check-qtest-i386-y += tests/pc-cpu-test$(EXESUF)
 check-qtest-i386-y += tests/q35-test$(EXESUF)
+check-qtest-i386-y += tests/vmgenid-test$(EXESUF)
 gcov-files-i386-y += hw/pci-host/q35.c
 check-qtest-i386-$(CONFIG_VHOST_NET_TEST_i386) += tests/vhost-user-test$(EXESUF)
 ifeq ($(CONFIG_VHOST_NET_TEST_i386),)
@@ -726,6 +727,7 @@  tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o contrib/ivshmem-server/ivshmem
 tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o contrib/libvhost-user/libvhost-user.o $(test-util-obj-y)
 tests/test-uuid$(EXESUF): tests/test-uuid.o $(test-util-obj-y)
 tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
+tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/acpi-utils.o
 
 tests/migration/stress$(EXESUF): tests/migration/stress.o
 	$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"LINK","$(TARGET_DIR)$@")
diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c
new file mode 100644
index 0000000..123beae
--- /dev/null
+++ b/tests/vmgenid-test.c
@@ -0,0 +1,200 @@ 
+/*
+ * QTest testcase for VM Generation ID
+ *
+ * Copyright (c) 2016 Red Hat, Inc.
+ * Copyright (c) 2017 Skyport Systems
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include <glib.h>
+#include <string.h>
+#include <unistd.h>
+#include "qemu/osdep.h"
+#include "qemu/bitmap.h"
+#include "qemu/uuid.h"
+#include "hw/acpi/acpi-defs.h"
+#include "acpi-utils.h"
+#include "libqtest.h"
+
+#define VGID_GUID "324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
+#define VMGENID_GUID_OFFSET 40   /* allow space for
+                                  * OVMF SDT Header Probe Supressor
+                                  */
+#define RSDP_ADDR_INVALID 0x100000 /* RSDP must be below this address */
+#define RSDP_SLEEP_US     100000   /* Sleep for 100ms between tries */
+#define RSDP_TRIES_MAX    100      /* Max total time is 10 seconds */
+
+typedef struct {
+    AcpiTableHeader header;
+    gchar name_op;
+    gchar vgia[4];
+    gchar val_op;
+    uint32_t vgia_val;
+} QEMU_PACKED VgidTable;
+
+static uint32_t acpi_find_vgia(void)
+{
+    uint32_t off;
+    AcpiRsdpDescriptor rsdp_table;
+    uint32_t rsdt;
+    AcpiRsdtDescriptorRev1 rsdt_table;
+    int tables_nr;
+    uint32_t *tables;
+    AcpiTableHeader ssdt_table;
+    VgidTable vgid_table;
+    int i;
+
+    /* Tables may take a short time to be set up by the guest */
+    for (i = 0; i < RSDP_TRIES_MAX; i++) {
+        off = acpi_find_rsdp_address();
+        if (off < RSDP_ADDR_INVALID) {
+            break;
+        }
+        g_usleep(RSDP_SLEEP_US);
+    }
+    g_assert_cmphex(off, <, RSDP_ADDR_INVALID);
+
+    acpi_parse_rsdp_table(off, &rsdp_table);
+
+    rsdt = rsdp_table.rsdt_physical_address;
+    /* read the header */
+    ACPI_READ_TABLE_HEADER(&rsdt_table, rsdt);
+    ACPI_ASSERT_CMP(rsdt_table.signature, "RSDT");
+
+    /* compute the table entries in rsdt */
+    tables_nr = (rsdt_table.length - sizeof(AcpiRsdtDescriptorRev1)) /
+                sizeof(uint32_t);
+    g_assert_cmpint(tables_nr, >, 0);
+
+    /* get the addresses of the tables pointed by rsdt */
+    tables = g_new0(uint32_t, tables_nr);
+    ACPI_READ_ARRAY_PTR(tables, tables_nr, rsdt);
+
+    for (i = 0; i < tables_nr; i++) {
+        ACPI_READ_TABLE_HEADER(&ssdt_table, tables[i]);
+        if (!strncmp((char *)ssdt_table.oem_table_id, "VMGENID", 7)) {
+            /* the first entry in the table should be VGIA
+             * That's all we need
+             */
+            ACPI_READ_FIELD(vgid_table.name_op, tables[i]);
+            g_assert(vgid_table.name_op == 0x08);  /* name */
+            ACPI_READ_ARRAY(vgid_table.vgia, tables[i]);
+            g_assert(memcmp(vgid_table.vgia, "VGIA", 4) == 0);
+            ACPI_READ_FIELD(vgid_table.val_op, tables[i]);
+            g_assert(vgid_table.val_op == 0x0C);  /* dword */
+            ACPI_READ_FIELD(vgid_table.vgia_val, tables[i]);
+            /* The GUID is written at a fixed offset into the fw_cfg file
+             * in order to implement the "OVMF SDT Header probe suppressor"
+             * see docs/specs/vmgenid.txt for more details
+             */
+            return vgid_table.vgia_val + VMGENID_GUID_OFFSET;
+        }
+    }
+    return 0;
+}
+
+static void read_guid_from_memory(QemuUUID *guid)
+{
+    uint32_t vmgenid_addr;
+    int i;
+
+    vmgenid_addr = acpi_find_vgia();
+    g_assert(vmgenid_addr);
+
+    /* Read the GUID directly from guest memory */
+    for (i = 0; i < 16; i++) {
+        guid->data[i] = readb(vmgenid_addr + i);
+    }
+    /* The GUID is in little-endian format in the guest, while QEMU
+     * uses big-endian.  Swap after reading.
+     */
+    qemu_uuid_bswap(guid);
+}
+
+static void read_guid_from_monitor(QemuUUID *guid)
+{
+    QDict *rsp, *rsp_ret;
+    const char *guid_str;
+
+    rsp = qmp("{ 'execute': 'query-vm-generation-id' }");
+    if (qdict_haskey(rsp, "return")) {
+        rsp_ret = qdict_get_qdict(rsp, "return");
+        g_assert(qdict_haskey(rsp_ret, "guid"));
+        guid_str = qdict_get_str(rsp_ret, "guid");
+        g_assert(qemu_uuid_parse(guid_str, guid) == 0);
+    }
+    QDECREF(rsp);
+}
+
+static void vmgenid_set_guid_test(void)
+{
+    QemuUUID expected, measured;
+    gchar *cmd;
+
+    g_assert(qemu_uuid_parse(VGID_GUID, &expected) == 0);
+
+    cmd = g_strdup_printf("-machine accel=tcg -device vmgenid,id=testvgid,"
+                          "guid=%s", VGID_GUID);
+    qtest_start(cmd);
+
+    /* Read the GUID from accessing guest memory */
+    read_guid_from_memory(&measured);
+    g_assert(memcmp(measured.data, expected.data, sizeof(measured.data)) == 0);
+
+    qtest_quit(global_qtest);
+    g_free(cmd);
+}
+
+static void vmgenid_set_guid_auto_test(void)
+{
+    const char *cmd;
+    QemuUUID measured;
+
+    cmd = "-machine accel=tcg -device vmgenid,id=testvgid," "guid=auto";
+    qtest_start(cmd);
+
+    read_guid_from_memory(&measured);
+
+    /* Just check that the GUID is non-null */
+    g_assert(!qemu_uuid_is_null(&measured));
+
+    qtest_quit(global_qtest);
+}
+
+static void vmgenid_query_monitor_test(void)
+{
+    QemuUUID expected, measured;
+    gchar *cmd;
+
+    g_assert(qemu_uuid_parse(VGID_GUID, &expected) == 0);
+
+    cmd = g_strdup_printf("-machine accel=tcg -device vmgenid,id=testvgid,"
+                          "guid=%s", VGID_GUID);
+    qtest_start(cmd);
+
+    /* Read the GUID via the monitor */
+    read_guid_from_monitor(&measured);
+    g_assert(memcmp(measured.data, expected.data, sizeof(measured.data)) == 0);
+
+    qtest_quit(global_qtest);
+    g_free(cmd);
+}
+
+int main(int argc, char **argv)
+{
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+
+    qtest_add_func("/vmgenid/vmgenid/set-guid",
+                   vmgenid_set_guid_test);
+    qtest_add_func("/vmgenid/vmgenid/set-guid-auto",
+                   vmgenid_set_guid_auto_test);
+    qtest_add_func("/vmgenid/vmgenid/query-monitor",
+                   vmgenid_query_monitor_test);
+    ret = g_test_run();
+
+    return ret;
+}