Hi Michal,
thanks a lot for posting this. One question:
On 07/27/16 10:43, Michal Privoznik wrote:
Since its release of 2.4.0 qemu is able to enable System
Management Module in the firmware, or disable it. We should
expose this capability in the XML. Unfortunately, there's no good
way to determine whether the binary we are talking to supports
it. I mean, if qemu's run with real machine type, the smm
attribute can be seen in 'qom-list /machine' output. But it's not
there when qemu's run with -M none. Therefore we're stuck with
version based check.
Where does your information come from that says QEMU 2.4 is good enough
for this? To my knowledge, the pc-q35-2.5 machine type is minimally
required, i.e., no i440fx at all, and for q35, 2.5 or later, which can
only be provided by QEMU v2.5.0 or later.
Hmmm... I found QEMU commit 355023f2010c4 ("pc: add SMM property"), and
it's indeed part of v2.4.0. I wonder where *I* got the information from
that only pc-q35-2.5+ machtypes are suitable for SMM (as OVMF needs it).
Perhaps there were other requirements too (SMRAM emulation etc). Ah,
wait, *large* SMRAM (T-SEG) is Q35-only. The SMM property is available
on i440fx as well.
Hm, even Paolo's presentation ("Securing secure boot with
System Management Mode") mentions "QEMU: released in 2.4".
... Ah, I think I might now why. In this RHBZ:
https://bugzilla.redhat.com/show_bug.cgi?id=1202822
Gerd set
Gerd Hoffmann 2016-03-21 08:23:06 CET
Fixed In Version: qemu-2.5
I checked
git log --no-merges --oneline --reverse v2.4.0..v2.5.0
but nothing says "smm", "smram", or "tseg"
(case-insensitively).
I recall commit bafc90bdc594 ("q35: implement TSEG") as one of Gerd's
QEMU patches, but that's also part of v2.4.0. Confirmed by:
<
http://wiki.qemu.org/ChangeLog/2.4#x86>.
... Gerd's above metadata change is dated 2016-03-21, at which point
both QEMU v2.4.0 and v2.5.0 had been tagged (on 2015-08-11 and on
2015-12-16, respectively). I guess Gerd may have mis-remembered the
exact QEMU release which included SMM support?
I don't remember ever testing SMM (using OVMF) with released 2.4 machine
types, so I feel a bit uncertain about mentioning and looking for 2.4 in
this patch.
... Anyway, looking over your patch quickly, I think it really only
concerns itself with "-machine smm=on", and for that, the 2.4 version
check should suffice. It's only OVMF that needs a lot more than that.
Acked-by: Laszlo Ersek <lersek(a)redhat.com>
Thanks!
Laszlo
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
docs/formatdomain.html.in | 6 +++++
docs/schemas/domaincommon.rng | 9 +++++++
src/conf/domain_conf.c | 5 +++-
src/conf/domain_conf.h | 1 +
src/qemu/qemu_capabilities.c | 16 +++++++++++++
src/qemu/qemu_capabilities.h | 4 ++++
src/qemu/qemu_command.c | 12 ++++++++++
tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 +
.../caps_2.6.0-gicv2.aarch64.xml | 1 +
.../caps_2.6.0-gicv3.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 +
.../qemuxml2argv-machine-smm-opt.args | 25 +++++++++++++++++++
.../qemuxml2argv-machine-smm-opt.xml | 28 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 7 ++++++
16 files changed, 118 insertions(+), 1 deletion(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 59a8bb9..3f67182 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1655,6 +1655,12 @@
values are <code>2</code>, <code>3</code> and
<code>host</code>.
<span class="since">Since 1.2.16</span>
</dd>
+ <dt><code>smm</code></dt>
+ <dd>Enable System Management Mode. Possible values are
+ <code>on</code> and <code>off</code>. The default is
left
+ for hypervisor to decide.
+ <span class="since">Since 2.1.0</span>
+ </dd>
</dl>
<h3><a name="elementsTime">Time keeping</a></h3>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 741f268..39fcb7e 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4291,6 +4291,15 @@
</optional>
</element>
</optional>
+ <optional>
+ <element name="smm">
+ <optional>
+ <attribute name="state">
+ <ref name="virOnOff"/>
+ </attribute>
+ </optional>
+ </element>
+ </optional>
</interleave>
</element>
</optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 13e5dc9..3b6493e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -137,7 +137,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
"capabilities",
"pmu",
"vmport",
- "gic")
+ "gic",
+ "smm")
VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST,
"default",
@@ -16318,6 +16319,7 @@ virDomainDefParseXML(xmlDocPtr xml,
case VIR_DOMAIN_FEATURE_PMU:
case VIR_DOMAIN_FEATURE_PVSPINLOCK:
case VIR_DOMAIN_FEATURE_VMPORT:
+ case VIR_DOMAIN_FEATURE_SMM:
node = ctxt->node;
ctxt->node = nodes[i];
if ((tmp = virXPathString("string(./@state)", ctxt))) {
@@ -23291,6 +23293,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
case VIR_DOMAIN_FEATURE_PMU:
case VIR_DOMAIN_FEATURE_PVSPINLOCK:
case VIR_DOMAIN_FEATURE_VMPORT:
+ case VIR_DOMAIN_FEATURE_SMM:
switch ((virTristateSwitch) def->features[i]) {
case VIR_TRISTATE_SWITCH_LAST:
case VIR_TRISTATE_SWITCH_ABSENT:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3c2f182..60b4be5 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1600,6 +1600,7 @@ typedef enum {
VIR_DOMAIN_FEATURE_PMU,
VIR_DOMAIN_FEATURE_VMPORT,
VIR_DOMAIN_FEATURE_GIC,
+ VIR_DOMAIN_FEATURE_SMM,
VIR_DOMAIN_FEATURE_LAST
} virDomainFeature;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d5b73e6..0b36819 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -339,6 +339,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"tls-creds-x509", /* 230 */
"display",
"intel-iommu",
+ "smm",
);
@@ -3533,6 +3534,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
if (qemuCaps->version >= 2004000)
virQEMUCapsSet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE);
+ /* smm option is supported from v2.4.0 */
+ if (qemuCaps->version >= 2004000)
+ virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT);
+
/* Since 2.4.50 ARM virt machine supports gic-version option */
if (qemuCaps->version >= 2004050)
virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACH_VIRT_GIC_VERSION);
@@ -4052,6 +4057,17 @@ virQEMUCapsSupportsVmport(virQEMUCapsPtr qemuCaps,
bool
+virQEMUCapsSupportsSMM(virQEMUCapsPtr qemuCaps,
+ const virDomainDef *def)
+{
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT))
+ return false;
+
+ return qemuDomainMachineIsQ35(def);
+}
+
+
+bool
virQEMUCapsIsMachineSupported(virQEMUCapsPtr qemuCaps,
const char *canonical_machine)
{
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index bd5c6d9..8c4bc95 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -372,6 +372,7 @@ typedef enum {
QEMU_CAPS_OBJECT_TLS_CREDS_X509, /* -object tls-creds-x509 */
QEMU_CAPS_DISPLAY, /* -display */
QEMU_CAPS_DEVICE_INTEL_IOMMU, /* -device intel-iommu */
+ QEMU_CAPS_MACHINE_SMM_OPT, /* -machine xxx,smm=on/off/auto */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
@@ -408,6 +409,9 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps,
bool virQEMUCapsSupportsVmport(virQEMUCapsPtr qemuCaps,
const virDomainDef *def);
+bool virQEMUCapsSupportsSMM(virQEMUCapsPtr qemuCaps,
+ const virDomainDef *def);
+
char *virQEMUCapsFlagsString(virQEMUCapsPtr qemuCaps);
const char *virQEMUCapsGetBinary(virQEMUCapsPtr qemuCaps);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6295eeb..831aba1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6791,6 +6791,7 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
}
} else {
virTristateSwitch vmport = def->features[VIR_DOMAIN_FEATURE_VMPORT];
+ virTristateSwitch smm = def->features[VIR_DOMAIN_FEATURE_SMM];
virCommandAddArg(cmd, "-machine");
virBufferAdd(&buf, def->os.machine, -1);
@@ -6820,6 +6821,17 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
virTristateSwitchTypeToString(vmport));
}
+ if (smm) {
+ if (!virQEMUCapsSupportsSMM(qemuCaps, def)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("smm is not available with this QEMU
binary"));
+ goto cleanup;
+ }
+
+ virBufferAsprintf(&buf, ",smm=%s",
+ virTristateSwitchTypeToString(smm));
+ }
+
if (def->mem.dump_core) {
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
index 80085d5..339ee1f 100644
--- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
@@ -183,6 +183,7 @@
<flag name='drive-detect-zeroes'/>
<flag name='display'/>
<flag name='intel-iommu'/>
+ <flag name='smm'/>
<version>2004000</version>
<kvmVersion>0</kvmVersion>
<package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
index fad3291..c1a68d0 100644
--- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
@@ -188,6 +188,7 @@
<flag name='tls-creds-x509'/>
<flag name='display'/>
<flag name='intel-iommu'/>
+ <flag name='smm'/>
<version>2005000</version>
<kvmVersion>0</kvmVersion>
<package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
index 4ed88bc..85d7d3f 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
@@ -157,6 +157,7 @@
<flag name='drive-detect-zeroes'/>
<flag name='tls-creds-x509'/>
<flag name='display'/>
+ <flag name='smm'/>
<version>2005094</version>
<kvmVersion>0</kvmVersion>
<package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
index 024596d..deb1257 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
@@ -157,6 +157,7 @@
<flag name='drive-detect-zeroes'/>
<flag name='tls-creds-x509'/>
<flag name='display'/>
+ <flag name='smm'/>
<version>2005094</version>
<kvmVersion>0</kvmVersion>
<package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
index e66433c..2b7ea0e 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
@@ -151,6 +151,7 @@
<flag name='drive-detect-zeroes'/>
<flag name='tls-creds-x509'/>
<flag name='display'/>
+ <flag name='smm'/>
<version>2005094</version>
<kvmVersion>0</kvmVersion>
<package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
index 653ec75..495c114 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
@@ -194,6 +194,7 @@
<flag name='tls-creds-x509'/>
<flag name='display'/>
<flag name='intel-iommu'/>
+ <flag name='smm'/>
<version>2006000</version>
<kvmVersion>0</kvmVersion>
<package></package>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.args
b/tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.args
new file mode 100644
index 0000000..e49d7e9
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.args
@@ -0,0 +1,25 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-machine q35,accel=tcg,smm=on \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
+-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \
+-device virtio-scsi-pci,id=scsi0,bus=pci.2,addr=0x1 \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-scsi0-0-0-0 \
+-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\
+drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x2
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.xml
b/tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.xml
new file mode 100644
index 0000000..b964b5e
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.xml
@@ -0,0 +1,28 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='q35'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <smm state='on'/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='sda' bus='scsi'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='scsi' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a5d51a8..f9588d5 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -618,6 +618,13 @@ mymain(void)
QEMU_CAPS_DUMP_GUEST_CORE);
DO_TEST_FAILURE("machine-core-on", NONE);
DO_TEST_FAILURE("machine-core-on", QEMU_CAPS_MACHINE_OPT);
+ DO_TEST("machine-smm-opt",
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_ICH9_AHCI,
+ QEMU_CAPS_MACHINE_OPT,
+ QEMU_CAPS_MACHINE_SMM_OPT,
+ QEMU_CAPS_VIRTIO_SCSI);
DO_TEST("machine-usb-opt", QEMU_CAPS_MACHINE_OPT,
QEMU_CAPS_MACHINE_USB_OPT);
DO_TEST("machine-vmport-opt", QEMU_CAPS_MACHINE_OPT,