Re: [PATCH v2 08/15] test: Introduce qemufirmwaretest

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/07/19 10:29, Michal Privoznik wrote:
> Test firmware description parsing so far.
> 
> The test files come from three locations:
> 1) ovmf-sb-keys.json and ovmf-sb.json come from OVMF
> package from RHEL-7 (with slight name change to reflect their
> features in filename too),
> 
> 2) bios.json and aavmf.json come form comments from

(a) s/form/from/

(b) s/comments/example JSON documents/ -- I'm emphasizing this because
QEMU itself is pretty strict about calling those comment sections
"Examples".

> firmware.json from qemu's git (3a0adfc9bf),
> 
> 3) ovmf.json is then copied from ovmf-sb.json and stripped
> of SECURE_BOOT and REQUIRES_SMM flags (plus OVMF path change).

The way you derived "ovmf.json" from "ovmf-sb.json" is not bad, but it
should be improved. I'll make comments on the file itself, below.

(c) Please don't forget to update this section of the commit message in
sync.

> 
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
>  tests/Makefile.am                        |  9 +++
>  tests/qemufirmwaredata/aavmf.json        | 35 +++++++++++
>  tests/qemufirmwaredata/bios.json         | 35 +++++++++++
>  tests/qemufirmwaredata/ovmf-sb-keys.json | 36 ++++++++++++
>  tests/qemufirmwaredata/ovmf-sb.json      | 35 +++++++++++
>  tests/qemufirmwaredata/ovmf.json         | 33 +++++++++++
>  tests/qemufirmwaretest.c                 | 75 ++++++++++++++++++++++++
>  7 files changed, 258 insertions(+)
>  create mode 100644 tests/qemufirmwaredata/aavmf.json
>  create mode 100644 tests/qemufirmwaredata/bios.json
>  create mode 100644 tests/qemufirmwaredata/ovmf-sb-keys.json
>  create mode 100644 tests/qemufirmwaredata/ovmf-sb.json
>  create mode 100644 tests/qemufirmwaredata/ovmf.json
>  create mode 100644 tests/qemufirmwaretest.c
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 72f0420bab..b3449fa96b 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -132,6 +132,7 @@ EXTRA_DIST = \
>  	qemuxml2xmloutdata \
>  	qemustatusxml2xmldata \
>  	qemumemlockdata \
> +	qemufirmwaredata \
>  	secretxml2xmlin \
>  	securityselinuxhelperdata \
>  	securityselinuxlabeldata \
> @@ -292,6 +293,7 @@ test_programs += qemuxml2argvtest qemuxml2xmltest \
>  	qemublocktest \
>  	qemumigparamstest \
>  	qemusecuritytest \
> +	qemufirmwaretest \
>  	$(NULL)
>  test_helpers += qemucapsprobe
>  test_libraries += libqemumonitortestutils.la \
> @@ -700,6 +702,12 @@ qemusecuritytest_SOURCES = \
>  	testutilsqemu.h testutilsqemu.c
>  qemusecuritytest_LDADD = $(qemu_LDADDS) $(LDADDS)
>  
> +qemufirmwaretest_SOURCES = \
> +	qemufirmwaretest.c \
> +	testutils.h testutils.c \
> +	$(NULL)
> +qemufirmwaretest_LDADD = $(qemu_LDADDS) $(LDADDS)
> +
>  else ! WITH_QEMU
>  EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \
>  	domainsnapshotxml2xmltest.c \
> @@ -713,6 +721,7 @@ EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \
>  	qemumigparamstest.c \
>  	qemusecuritytest.c qemusecuritytest.h \
>  	qemusecuritymock.c \
> +	qemufirmwaretest.c \
>  	$(QEMUMONITORTESTUTILS_SOURCES)
>  endif ! WITH_QEMU
>  
> diff --git a/tests/qemufirmwaredata/aavmf.json b/tests/qemufirmwaredata/aavmf.json
> new file mode 100644
> index 0000000000..114d1475a2
> --- /dev/null
> +++ b/tests/qemufirmwaredata/aavmf.json
> @@ -0,0 +1,35 @@
> +{
> +    "description": "UEFI firmware for ARM64 virtual machines",
> +    "interface-types": [
> +        "uefi"
> +    ],
> +    "mapping": {
> +        "device": "flash",
> +        "executable": {
> +            "filename": "/usr/share/AAVMF/AAVMF_CODE.fd",
> +            "format": "raw"
> +        },
> +        "nvram-template": {
> +            "filename": "/usr/share/AAVMF/AAVMF_VARS.fd",
> +            "format": "raw"
> +        }
> +    },
> +    "targets": [
> +        {
> +            "architecture": "aarch64",
> +            "machines": [
> +                "virt-*"
> +            ]
> +        }
> +    ],
> +    "features": [
> +
> +    ],
> +    "tags": [
> +        "-a AARCH64",
> +        "-p ArmVirtPkg/ArmVirtQemu.dsc",
> +        "-t GCC48",
> +        "-b DEBUG",
> +        "-D DEBUG_PRINT_ERROR_LEVEL=0x80000000"
> +    ]
> +}
> diff --git a/tests/qemufirmwaredata/bios.json b/tests/qemufirmwaredata/bios.json
> new file mode 100644
> index 0000000000..137ff70779
> --- /dev/null
> +++ b/tests/qemufirmwaredata/bios.json
> @@ -0,0 +1,35 @@
> +{
> +    "description": "SeaBIOS",
> +    "interface-types": [
> +        "bios"
> +    ],
> +    "mapping": {
> +        "device": "memory",
> +        "filename": "/usr/share/seabios/bios-256k.bin"
> +    },
> +    "targets": [
> +        {
> +            "architecture": "i386",
> +            "machines": [
> +                "pc-i440fx-*",
> +                "pc-q35-*"
> +            ]
> +        },
> +        {
> +            "architecture": "x86_64",
> +            "machines": [
> +                "pc-i440fx-*",
> +                "pc-q35-*"
> +            ]
> +        }
> +    ],
> +    "features": [
> +        "acpi-s3",
> +        "acpi-s4"
> +    ],
> +    "tags": [
> +        "CONFIG_BOOTSPLASH=n",
> +        "CONFIG_ROM_SIZE=256",
> +        "CONFIG_USE_SMM=n"
> +    ]
> +}
> diff --git a/tests/qemufirmwaredata/ovmf-sb-keys.json b/tests/qemufirmwaredata/ovmf-sb-keys.json
> new file mode 100644
> index 0000000000..c804ac1038
> --- /dev/null
> +++ b/tests/qemufirmwaredata/ovmf-sb-keys.json
> @@ -0,0 +1,36 @@
> +{
> +    "description": "OVMF with SB+SMM, SB enabled, MS certs enrolled",
> +    "interface-types": [
> +        "uefi"
> +    ],
> +    "mapping": {
> +        "device": "flash",
> +        "executable": {
> +            "filename": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
> +            "format": "raw"
> +        },
> +        "nvram-template": {
> +            "filename": "/usr/share/OVMF/OVMF_VARS.secboot.fd",
> +            "format": "raw"
> +        }
> +    },
> +    "targets": [
> +        {
> +            "architecture": "x86_64",
> +            "machines": [
> +                "pc-q35-*"
> +            ]
> +        }
> +    ],
> +    "features": [
> +        "acpi-s3",
> +        "amd-sev",
> +        "enrolled-keys",
> +        "requires-smm",
> +        "secure-boot",
> +        "verbose-dynamic"
> +    ],
> +    "tags": [
> +
> +    ]
> +}
> diff --git a/tests/qemufirmwaredata/ovmf-sb.json b/tests/qemufirmwaredata/ovmf-sb.json
> new file mode 100644
> index 0000000000..5e8a94ae78
> --- /dev/null
> +++ b/tests/qemufirmwaredata/ovmf-sb.json
> @@ -0,0 +1,35 @@
> +{
> +    "description": "OVMF with SB+SMM, empty varstore",
> +    "interface-types": [
> +        "uefi"
> +    ],
> +    "mapping": {
> +        "device": "flash",
> +        "executable": {
> +            "filename": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
> +            "format": "raw"
> +        },
> +        "nvram-template": {
> +            "filename": "/usr/share/OVMF/OVMF_VARS.fd",
> +            "format": "raw"
> +        }
> +    },
> +    "targets": [
> +        {
> +            "architecture": "x86_64",
> +            "machines": [
> +                "pc-q35-*"
> +            ]
> +        }
> +    ],
> +    "features": [
> +        "acpi-s3",
> +        "amd-sev",
> +        "requires-smm",
> +        "secure-boot",
> +        "verbose-dynamic"
> +    ],
> +    "tags": [
> +
> +    ]
> +}
> diff --git a/tests/qemufirmwaredata/ovmf.json b/tests/qemufirmwaredata/ovmf.json
> new file mode 100644
> index 0000000000..9d53094778
> --- /dev/null
> +++ b/tests/qemufirmwaredata/ovmf.json
> @@ -0,0 +1,33 @@
> +{
> +    "description": "OVMF with SB+SMM, empty varstore",

"SB", "SMM", and "empty varstore" are each either wrong or useless to
mention here.

(d) So I suggest retrofitting the AAVMF description instead -- just say
"UEFI firmware for x86_64 virtual machines".

> +    "interface-types": [
> +        "uefi"
> +    ],
> +    "mapping": {
> +        "device": "flash",
> +        "executable": {
> +            "filename": "/usr/share/OVMF/OVMF_CODE.fd",
> +            "format": "raw"
> +        },
> +        "nvram-template": {
> +            "filename": "/usr/share/OVMF/OVMF_VARS.fd",
> +            "format": "raw"
> +        }
> +    },
> +    "targets": [
> +        {
> +            "architecture": "x86_64",
> +            "machines": [
> +                "pc-q35-*"

(e) Because this build does not include the SMM driver stack, we should
permit "pc-i440fx-*", in addition to "pc-q35-*".


(Points (d) and (e) are not really relevant for the self-test in
libvirt, it's just that the examples should be as "meaningful" as possible.)

Other than that, thanks for addressing my comments. With the above updates:

Acked-by: Laszlo Ersek <lersek@xxxxxxxxxx>

Thanks
Laszlo

> +            ]
> +        }
> +    ],
> +    "features": [
> +        "acpi-s3",
> +        "amd-sev",
> +        "verbose-dynamic"
> +    ],
> +    "tags": [
> +
> +    ]
> +}
> diff --git a/tests/qemufirmwaretest.c b/tests/qemufirmwaretest.c
> new file mode 100644
> index 0000000000..176cf0920d
> --- /dev/null
> +++ b/tests/qemufirmwaretest.c
> @@ -0,0 +1,75 @@
> +#include <config.h>
> +
> +#include "testutils.h"
> +#include "qemu/qemu_firmware.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_QEMU
> +
> +/* A very basic test. Parse given JSON firmware description into
> + * an internal structure, format it back and compare with the
> + * contents of the file (minus some keys that are not parsed).
> + */
> +static int
> +testParseFormatFW(const void *opaque)
> +{
> +    const char *filename = opaque;
> +    VIR_AUTOFREE(char *) path = NULL;
> +    VIR_AUTOPTR(qemuFirmware) fw = NULL;
> +    VIR_AUTOFREE(char *) buf = NULL;
> +    VIR_AUTOPTR(virJSONValue) json = NULL;
> +    VIR_AUTOFREE(char *) expected = NULL;
> +    VIR_AUTOFREE(char *) actual = NULL;
> +
> +    if (virAsprintf(&path, "%s/qemufirmwaredata/%s",
> +                    abs_srcdir, filename) < 0)
> +        return -1;
> +
> +    if (!(fw = qemuFirmwareParse(path)))
> +        return -1;
> +
> +    if (virFileReadAll(path,
> +                       1024 * 1024, /* 1MiB */
> +                       &buf) < 0)
> +        return -1;
> +
> +    if (!(json = virJSONValueFromString(buf)))
> +        return -1;
> +
> +    /* Description and tags are not parsed. */
> +    if (virJSONValueObjectRemoveKey(json, "description", NULL) < 0 ||
> +        virJSONValueObjectRemoveKey(json, "tags", NULL) < 0)
> +        return -1;
> +
> +    if (!(expected = virJSONValueToString(json, true)))
> +        return -1;
> +
> +    if (!(actual = qemuFirmwareFormat(fw)))
> +        return -1;
> +
> +    return virTestCompareToString(expected, actual);
> +}
> +
> +
> +static int
> +mymain(void)
> +{
> +    int ret = 0;
> +
> +#define DO_PARSE_TEST(filename) \
> +    do { \
> +        if (virTestRun("QEMU FW " filename, \
> +                       testParseFormatFW, filename) < 0) \
> +            ret = -1; \
> +    } while (0)
> +
> +    DO_PARSE_TEST("bios.json");
> +    DO_PARSE_TEST("ovmf-sb-keys.json");
> +    DO_PARSE_TEST("ovmf-sb.json");
> +    DO_PARSE_TEST("ovmf.json");
> +    DO_PARSE_TEST("aavmf.json");
> +
> +    return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
> +}
> +
> +
> +VIR_TEST_MAIN(mymain);
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux