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@redhat.com> --- 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@redhat.com> 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);