The two checks are semantically different, so it makes sense to perform them separately. We will soon extend the first one. While at it, start printing out the value of isConfidential. We could print the value of each firmware feature it's derived from, but that would make things unnecessarily verbose; at the same time, knowing that libvirt believes that the firmware build is targeting the confidential use case can be useful for debugging so it's worth including it. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_firmware.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 04e81355df..c78e8e7833 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1590,16 +1590,23 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw, * VMs also don't support EFI variable storage in NVRAM, instead * the secureboot state is hardcoded to enabled. */ - if ((!isConfidential && - (supportsSecureBoot != requiresSMM)) || - (hasEnrolledKeys && !supportsSecureBoot)) { + if (!isConfidential && + supportsSecureBoot != requiresSMM) { VIR_WARN("Firmware description '%s' has invalid set of features: " - "%s = %d, %s = %d, %s = %d", + "%s = %d, %s = %d (isConfidential = %d)", filename, qemuFirmwareFeatureTypeToString(QEMU_FIRMWARE_FEATURE_REQUIRES_SMM), requiresSMM, qemuFirmwareFeatureTypeToString(QEMU_FIRMWARE_FEATURE_SECURE_BOOT), supportsSecureBoot, + isConfidential); + } + if (hasEnrolledKeys && !supportsSecureBoot) { + VIR_WARN("Firmware description '%s' has invalid set of features: " + "%s = %d, %s = %d", + filename, + qemuFirmwareFeatureTypeToString(QEMU_FIRMWARE_FEATURE_SECURE_BOOT), + supportsSecureBoot, qemuFirmwareFeatureTypeToString(QEMU_FIRMWARE_FEATURE_ENROLLED_KEYS), hasEnrolledKeys); } -- 2.53.0