On 2/28/19 12:40 PM, Laszlo Ersek wrote:
On 02/27/19 11:04, Michal Privoznik wrote:The firmware selection code will enable the feature if needed. There's no need to require SMM to be enabled in that case. Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> --- src/qemu/qemu_domain.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cc3a01397c..b25f26f8b0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4157,7 +4157,9 @@ qemuDomainDefValidate(const virDomainDef *def, goto cleanup; }- if (def->features[VIR_DOMAIN_FEATURE_SMM] != VIR_TRISTATE_SWITCH_ON) {+ /* SMM will be enabled by qemuFirmwareFillDomain() if needed. */ + if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE && + def->features[VIR_DOMAIN_FEATURE_SMM] != VIR_TRISTATE_SWITCH_ON) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Secure boot requires SMM feature enabled")); goto cleanup;I'm not familiar with the ordering between the code in the previous patch and the code in this patch. What is the order? Do we first validate and then prepare/fill, or vice versa?
Yeah, I can probably put this one before 11/15 which enables SMM if needed.
Also, what happens if the domain config explicitly disables SMM emulation in <features>, but the firmware requires it? See my point (2) in message <https://www.redhat.com/archives/libvir-list/2019-February/msg01669.html>. Will qemuFirmwareEnableFeatures() simply overwrite the setting? I think we should reject the firmware descriptor instead, in qemuFirmwareMatchDomain().
Good point. I'll make qemuFirmwareEnableFeatures() fail in that case and thus starting the domain will fail. After all, in point (1) you suggest that for matching FW @secure == REQUIRES_SMM, therefore transitively if @secure='yes' then def->features[SMM] = VIR_TRISTATE_SWITCH_ON. Hence, @secure='yes' and SMM='off' is nonsensical combination.
Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
