[PATCH v3 0/9] Introducing TPM Proxy device support for PPC64
changes in v3 after reviews from Stefan Berger: - new patch 4: trivial change to allow g_autoptr() with virDomainTPMDefPtr - patch 5 (former 4): added g_autoptr(), removed VIR_FREE() calls - changed wording in patches 1 and 9 (former 8) to make it clear for users that this new device brings no benefits unless the guest is running on specialized PPC64 hardware - fixed identation in patch 6 (former 5). Added Stefan's r-b v2 link: https://www.redhat.com/archives/libvir-list/2020-May/msg00604.html v1 link: https://www.redhat.com/archives/libvir-list/2020-May/msg00604.html Daniel Henrique Barboza (9): docs: documentation and schema for the new TPM Proxy model qemu: Extend QEMU capabilities with 'spapr-tpm-proxy' conf, qemu: adding 'tpmproxy' in domain definition conf, domain: register AUTOPTR_CLEANUP_FUNC for virDomainTPMDef domain_conf.c: XML parsing for VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY tests: add XML schema tests for the TPM Proxy device qemu: build command line for the TPM Proxy device tests/qemuxml2argvtest.c: add TPM Proxy command line tests docs/news.xml: update for the new TPM Proxy device docs/formatdomain.html.in | 18 +++++- docs/news.xml | 17 +++++ docs/schemas/domaincommon.rng | 1 + src/conf/domain_audit.c | 3 + src/conf/domain_conf.c | 63 +++++++++++++++++-- src/conf/domain_conf.h | 3 + src/qemu/qemu_alias.c | 16 +++++ src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_cgroup.c | 12 +++- src/qemu/qemu_command.c | 21 +++++++ src/qemu/qemu_domain.c | 9 +-- src/qemu/qemu_validate.c | 12 ++++ src/security/security_dac.c | 14 +++++ src/security/security_selinux.c | 11 ++++ .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + tests/qemuxml2argvdata/ppc64-tpm-double.xml | 34 ++++++++++ .../ppc64-tpmproxy-double.xml | 38 +++++++++++ .../ppc64-tpmproxy-single.ppc64-latest.args | 34 ++++++++++ .../ppc64-tpmproxy-single.xml | 33 ++++++++++ .../ppc64-tpmproxy-with-tpm.ppc64-latest.args | 37 +++++++++++ .../ppc64-tpmproxy-with-tpm.xml | 36 +++++++++++ tests/qemuxml2argvtest.c | 15 +++++ .../ppc64-tpmproxy-single.ppc64-latest.xml | 42 +++++++++++++ .../ppc64-tpmproxy-with-tpm.ppc64-latest.xml | 46 ++++++++++++++ tests/qemuxml2xmltest.c | 2 + 27 files changed, 514 insertions(+), 12 deletions(-) create mode 100644 tests/qemuxml2argvdata/ppc64-tpm-double.xml create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-double.xml create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-single.ppc64-latest.args create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-single.xml create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.ppc64-latest.args create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.xml create mode 100644 tests/qemuxml2xmloutdata/ppc64-tpmproxy-single.ppc64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/ppc64-tpmproxy-with-tpm.ppc64-latest.xml -- 2.26.2
QEMU 4.1.0 introduced a new device type called TPM Proxy, currently implemented by PPC64 guests via a new virtual device called 'spapr-tpm-proxy' (see QEMU 0fb6bd073230 for more info). The TPM Proxy device interacts with a TPM Resource Manager, a host device capable of multiplexing the host TPM with multiple processes. This allows multiple guests to access some TPM features at the same time. Note that this mode of operation does not provide full TPM features to be available for the guest - for that case the guest still needs to assign a vTPM device (tpm-spapr for PPC64 guests). Although redundant, there is currently no technical limitation for a guest to assign both a vTPM and a TPM Proxy at the same time. This patch adds documentation and schema for a new TPM model type called 'spapr-tpm-proxy' that creates this new TPM Proxy device. This model is valid only for the 'passthrough' backend. An example of a TPM Proxy device connected to a TPM Resource Manager '/dev/tpmrm0' will look like this: <tpm model='spapr-tpm-proxy'> <backend type='passthrough'> <device path='/dev/tpmrm0'/> </backend> </tpm> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- docs/formatdomain.html.in | 18 +++++++++++++++++- docs/schemas/domaincommon.rng | 1 + 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 23eb029234..15a92aa4f4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -8792,6 +8792,17 @@ qemu-kvm -net nic,model=? /dev/null backend device is a TPM 2.0. <span class="since">Since 6.1.0</span>, pSeries guests on PPC64 are supported and the default is <code>tpm-spapr</code>. + + <span class="since">Since 6.4.0</span>, a new model called + <code>spapr-tpm-proxy</code> was added for pSeries guests. This model + only works with the 'passthrough' backend. It creates a TPM Proxy + device that communicates with an existing TPM Resource Manager in the host, + for example /dev/tpmrm0, enabling the guest to run in secure virtual machine + mode with the help of an Ultravisor. Adding a TPM Proxy to a pSeries guest + brings no security benefits unless the guest is running on a PPC64 host that + has an Ultravisor and a TPM Resource Manager. Only one TPM Proxy device is + allowed per guest, but a TPM Proxy device can be added together with + other TPM devices. </p> </dd> <dt><code>backend</code></dt> @@ -8804,7 +8815,7 @@ qemu-kvm -net nic,model=? /dev/null <dt><code>passthrough</code></dt> <dd> <p> - Use the host's TPM device. + Use the host's TPM or TPM Resource Manager device. </p> <p> This backend type requires exclusive access to a TPM device on @@ -8812,6 +8823,11 @@ qemu-kvm -net nic,model=? /dev/null qualified file name is specified by path attribute of the <code>source</code> element. If no file name is specified then /dev/tpm0 is automatically used. + + <span class="since">Since 6.4.0</span>, when choosing the + <code>spapr-tpm-proxy</code> model, the file name specified is + expected to be a TPM Resource Manager device, e.g. + /dev/tpmrm0. </p> </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9d60b090f3..50860419c3 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4610,6 +4610,7 @@ <value>tpm-tis</value> <value>tpm-crb</value> <value>tpm-spapr</value> + <value>spapr-tpm-proxy</value> </choice> </attribute> </optional> -- 2.26.2
On 5/13/20 3:30 PM, Daniel Henrique Barboza wrote:
QEMU 4.1.0 introduced a new device type called TPM Proxy, currently implemented by PPC64 guests via a new virtual device called 'spapr-tpm-proxy' (see QEMU 0fb6bd073230 for more info).
The TPM Proxy device interacts with a TPM Resource Manager, a host device capable of multiplexing the host TPM with multiple processes. This allows multiple guests to access some TPM features at the same time. Note that this mode of operation does not provide full TPM features to be available for the guest - for that case the guest still needs to assign a vTPM device (tpm-spapr for PPC64 guests). Although redundant, there is currently no technical limitation for a guest to assign both a vTPM and a TPM Proxy at the same time.
This patch adds documentation and schema for a new TPM model type called 'spapr-tpm-proxy' that creates this new TPM Proxy device. This model is valid only for the 'passthrough' backend. An example of a TPM Proxy device connected to a TPM Resource Manager '/dev/tpmrm0' will look like this:
<tpm model='spapr-tpm-proxy'> <backend type='passthrough'> <device path='/dev/tpmrm0'/> </backend> </tpm>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
--- docs/formatdomain.html.in | 18 +++++++++++++++++- docs/schemas/domaincommon.rng | 1 + 2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 23eb029234..15a92aa4f4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -8792,6 +8792,17 @@ qemu-kvm -net nic,model=? /dev/null backend device is a TPM 2.0. <span class="since">Since 6.1.0</span>, pSeries guests on PPC64 are supported and the default is <code>tpm-spapr</code>. + + <span class="since">Since 6.4.0</span>, a new model called + <code>spapr-tpm-proxy</code> was added for pSeries guests. This model + only works with the 'passthrough' backend. It creates a TPM Proxy + device that communicates with an existing TPM Resource Manager in the host, + for example /dev/tpmrm0, enabling the guest to run in secure virtual machine + mode with the help of an Ultravisor. Adding a TPM Proxy to a pSeries guest + brings no security benefits unless the guest is running on a PPC64 host that + has an Ultravisor and a TPM Resource Manager. Only one TPM Proxy device is + allowed per guest, but a TPM Proxy device can be added together with + other TPM devices. </p> </dd> <dt><code>backend</code></dt> @@ -8804,7 +8815,7 @@ qemu-kvm -net nic,model=? /dev/null <dt><code>passthrough</code></dt> <dd> <p> - Use the host's TPM device. + Use the host's TPM or TPM Resource Manager device. </p> <p> This backend type requires exclusive access to a TPM device on @@ -8812,6 +8823,11 @@ qemu-kvm -net nic,model=? /dev/null qualified file name is specified by path attribute of the <code>source</code> element. If no file name is specified then /dev/tpm0 is automatically used. + + <span class="since">Since 6.4.0</span>, when choosing the + <code>spapr-tpm-proxy</code> model, the file name specified is + expected to be a TPM Resource Manager device, e.g. + /dev/tpmrm0. </p> </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9d60b090f3..50860419c3 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4610,6 +4610,7 @@ <value>tpm-tis</value> <value>tpm-crb</value> <value>tpm-spapr</value> + <value>spapr-tpm-proxy</value> </choice> </attribute> </optional>
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
QEMU 4.1.0 introduced a new device type called TPM Proxy, currently implemented by PPC64 guests via a new virtual device called 'spapr-tpm-proxy' (see QEMU 0fb6bd073230 for more info).
The TPM Proxy device interacts with a TPM Resource Manager, a host device capable of multiplexing the host TPM with multiple processes. This allows multiple guests to access some TPM features at the same time. Note that this mode of operation does not provide full TPM features to be available for the guest - for that case the guest still needs to assign a vTPM device (tpm-spapr for PPC64 guests). Although redundant, there is currently no technical limitation for a guest to assign both a vTPM and a TPM Proxy at the same time.
This patch adds documentation and schema for a new TPM model type called 'spapr-tpm-proxy' that creates this new TPM Proxy device. This model is valid only for the 'passthrough' backend. An example of a TPM Proxy device connected to a TPM Resource Manager '/dev/tpmrm0' will look like this:
<tpm model='spapr-tpm-proxy'> <backend type='passthrough'> <device path='/dev/tpmrm0'/> </backend> </tpm>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- docs/formatdomain.html.in | 18 +++++++++++++++++- docs/schemas/domaincommon.rng | 1 + 2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 23eb029234..15a92aa4f4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -8792,6 +8792,17 @@ qemu-kvm -net nic,model=? /dev/null backend device is a TPM 2.0. <span class="since">Since 6.1.0</span>, pSeries guests on PPC64 are supported and the default is <code>tpm-spapr</code>. + + <span class="since">Since 6.4.0</span>, a new model called + <code>spapr-tpm-proxy</code> was added for pSeries guests. This model + only works with the 'passthrough' backend. It creates a TPM Proxy
<code>passthrough</code>
+ device that communicates with an existing TPM Resource Manager in the host, + for example /dev/tpmrm0, enabling the guest to run in secure virtual machine
<code>/dev/tpmrm0</code>
+ mode with the help of an Ultravisor. Adding a TPM Proxy to a pSeries guest + brings no security benefits unless the guest is running on a PPC64 host that + has an Ultravisor and a TPM Resource Manager. Only one TPM Proxy device is + allowed per guest, but a TPM Proxy device can be added together with + other TPM devices. </p> </dd> <dt><code>backend</code></dt> @@ -8804,7 +8815,7 @@ qemu-kvm -net nic,model=? /dev/null <dt><code>passthrough</code></dt> <dd> <p> - Use the host's TPM device. + Use the host's TPM or TPM Resource Manager device. </p> <p> This backend type requires exclusive access to a TPM device on @@ -8812,6 +8823,11 @@ qemu-kvm -net nic,model=? /dev/null qualified file name is specified by path attribute of the <code>source</code> element. If no file name is specified then /dev/tpm0 is automatically used. + + <span class="since">Since 6.4.0</span>, when choosing the + <code>spapr-tpm-proxy</code> model, the file name specified is + expected to be a TPM Resource Manager device, e.g. + /dev/tpmrm0.
<code>/dev/tpmrm0</code>
</p> </dd> </dl>
Expose the TPM Proxy support for PPC64 guests by creating a new cap called QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY. This device is part of the machinery the guest need to orchestrate with the PPC64 Ultravisor the transition to the Secure VM (SVM) mode. Inside QEMU, this device will be used with the H_TPM_COMM hypercall to connect with the TPM Resource Manager, enabling the guest to open and close TPM sessions with the host TPM. Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 3 +++ tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + 4 files changed, 9 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 7e711f22f8..d0d8b1ebf5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -582,6 +582,9 @@ VIR_ENUM_IMPL(virQEMUCaps, "tcg", "virtio-blk-pci.scsi.default.disabled", "pvscsi", + + /* 370 */ + "spapr-tpm-proxy", ); @@ -1304,6 +1307,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "vhost-user-fs-device", QEMU_CAPS_DEVICE_VHOST_USER_FS }, { "tcg-accel", QEMU_CAPS_TCG }, { "pvscsi", QEMU_CAPS_SCSI_PVSCSI }, + { "spapr-tpm-proxy", QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY }, }; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 6bfc7386e3..fa22856e12 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -564,6 +564,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_VIRTIO_BLK_SCSI_DEFAULT_DISABLED, /* virtio-blk-pci.scsi disabled by default */ QEMU_CAPS_SCSI_PVSCSI, /* -device pvscsi */ + /* 370 */ + QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY, /* -device spapr-tpm-proxy */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml index a68786ddc8..9df68ebfc1 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml @@ -185,6 +185,7 @@ <flag name='machine.pseries.cap-ibs'/> <flag name='tcg'/> <flag name='pvscsi'/> + <flag name='spapr-tpm-proxy'/> <version>4001050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml index c8cc07d954..77f51fe4d8 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml @@ -201,6 +201,7 @@ <flag name='tcg'/> <flag name='virtio-blk-pci.scsi.default.disabled'/> <flag name='pvscsi'/> + <flag name='spapr-tpm-proxy'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900241</microcodeVersion> -- 2.26.2
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
Expose the TPM Proxy support for PPC64 guests by creating a new cap called QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY.
This device is part of the machinery the guest need to orchestrate with the PPC64 Ultravisor the transition to the Secure VM (SVM) mode. Inside QEMU, this device will be used with the H_TPM_COMM hypercall to connect with the TPM Resource Manager, enabling the guest to open and close TPM sessions with the host TPM.
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 3 +++ tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + 4 files changed, 9 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
A TPM Proxy device can coexist with a regular TPM. The TPM Proxy is also always a 'passthrough' device of the 'spapr-tpm-proxy' model. This patch adds a pointer to this device in the domain definition called 'tpmproxy'. This pointer is handled like the existing 'tpm' pointer of the VIR_DOMAIN_TPM_TYPE_PASSTHROUGH type. Cgroup, DAC/SELinux and qemu validation code was adapted to handle this new domain device. XML functions to parse and format this new device from/to XML will be added in the next patch, together with the logic that will guarantee the assumptions made in the first paragraph. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_audit.c | 3 +++ src/conf/domain_conf.c | 18 ++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/qemu/qemu_cgroup.c | 12 +++++++++--- src/qemu/qemu_domain.c | 9 +++++---- src/qemu/qemu_validate.c | 12 ++++++++++++ src/security/security_dac.c | 14 ++++++++++++++ src/security/security_selinux.c | 11 +++++++++++ 8 files changed, 74 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 1b0abb21a0..4575f66e45 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -824,6 +824,9 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) if (vm->def->tpm) virDomainAuditTPM(vm, vm->def->tpm, "start", true); + if (vm->def->tpmproxy) + virDomainAuditTPM(vm, vm->def->tpmproxy, "start", true); + for (i = 0; i < vm->def->nshmems; i++) virDomainAuditShmem(vm, vm->def->shmems[i], "start", true); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c201fc901d..01a32f62d1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1165,6 +1165,7 @@ VIR_ENUM_IMPL(virDomainTPMModel, "tpm-tis", "tpm-crb", "tpm-spapr", + "spapr-tpm-proxy", ); VIR_ENUM_IMPL(virDomainTPMBackend, @@ -3480,6 +3481,7 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def->mems); virDomainTPMDefFree(def->tpm); + virDomainTPMDefFree(def->tpmproxy); for (i = 0; i < def->npanics; i++) virDomainPanicDefFree(def->panics[i]); @@ -4318,6 +4320,12 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, if ((rc = cb(def, &device, &def->tpm->info, opaque)) != 0) return rc; } + if (def->tpmproxy) { + device.type = VIR_DOMAIN_DEVICE_TPM; + device.data.tpm = def->tpmproxy; + if ((rc = cb(def, &device, &def->tpmproxy->info, opaque)) != 0) + return rc; + } device.type = VIR_DOMAIN_DEVICE_PANIC; for (i = 0; i < def->npanics; i++) { device.data.panic = def->panics[i]; @@ -24344,6 +24352,16 @@ virDomainDefCheckABIStabilityFlags(virDomainDefPtr src, goto error; } + if (src->tpmproxy && dst->tpmproxy) { + if (!virDomainTPMDefCheckABIStability(src->tpmproxy, dst->tpmproxy)) + goto error; + } else if (src->tpmproxy || dst->tpmproxy) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Either both target and source domains or none of " + "them must have TPM Proxy device present")); + goto error; + } + if (src->nmems != dst->nmems) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain memory device count %zu " diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ddc75d8de2..8f178ade34 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1293,6 +1293,7 @@ typedef enum { VIR_DOMAIN_TPM_MODEL_TIS, VIR_DOMAIN_TPM_MODEL_CRB, VIR_DOMAIN_TPM_MODEL_SPAPR, + VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY, VIR_DOMAIN_TPM_MODEL_LAST } virDomainTPMModel; @@ -2628,6 +2629,7 @@ struct _virDomainDef { virDomainMemballoonDefPtr memballoon; virDomainNVRAMDefPtr nvram; virDomainTPMDefPtr tpm; + virDomainTPMDefPtr tpmproxy; virCPUDefPtr cpu; virSysinfoDefPtr sysinfo; virDomainRedirFilterDefPtr redirfilter; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2e019b64af..2ed4341655 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -333,10 +333,13 @@ qemuSetupChardevCgroupCB(virDomainDefPtr def G_GNUC_UNUSED, static int -qemuSetupTPMCgroup(virDomainObjPtr vm) +qemuSetupTPMCgroup(virDomainObjPtr vm, + virDomainTPMDefPtr dev) { int ret = 0; - virDomainTPMDefPtr dev = vm->def->tpm; + + if (!dev) + return 0; switch (dev->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: @@ -806,7 +809,10 @@ qemuSetupDevicesCgroup(virDomainObjPtr vm) vm) < 0) return -1; - if (vm->def->tpm && qemuSetupTPMCgroup(vm) < 0) + if (qemuSetupTPMCgroup(vm, vm->def->tpm) < 0) + return -1; + + if (qemuSetupTPMCgroup(vm, vm->def->tpmproxy) < 0) return -1; for (i = 0; i < vm->def->nhostdevs; i++) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a1b250fd0b..a344f8a0e6 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11574,11 +11574,9 @@ qemuDomainSetupAllChardevs(virQEMUDriverConfigPtr cfg G_GNUC_UNUSED, static int qemuDomainSetupTPM(virQEMUDriverConfigPtr cfg G_GNUC_UNUSED, - virDomainObjPtr vm, + virDomainTPMDefPtr dev, const struct qemuDomainCreateDeviceData *data) { - virDomainTPMDefPtr dev = vm->def->tpm; - if (!dev) return 0; @@ -11823,7 +11821,10 @@ qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg, if (qemuDomainSetupAllChardevs(cfg, vm, &data) < 0) goto cleanup; - if (qemuDomainSetupTPM(cfg, vm, &data) < 0) + if (qemuDomainSetupTPM(cfg, vm->def->tpm, &data) < 0) + goto cleanup; + + if (qemuDomainSetupTPM(cfg, vm->def->tpmproxy, &data) < 0) goto cleanup; if (qemuDomainSetupAllGraphics(cfg, vm, &data) < 0) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 584d1375b8..7210be3532 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3602,6 +3602,7 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm, case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_PASSTHROUGH)) goto no_support; + break; case VIR_DOMAIN_TPM_TYPE_EMULATOR: @@ -3623,6 +3624,17 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm, case VIR_DOMAIN_TPM_MODEL_SPAPR: flag = QEMU_CAPS_DEVICE_TPM_SPAPR; break; + case VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY: + if (!ARCH_IS_PPC64(def->os.arch)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("TPM Proxy model %s is only available for " + "PPC64 guests"), + virDomainTPMModelTypeToString(tpm->model)); + return -1; + } + + flag = QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY; + break; case VIR_DOMAIN_TPM_MODEL_LAST: default: virReportEnumRangeError(virDomainTPMModel, tpm->model); diff --git a/src/security/security_dac.c b/src/security/security_dac.c index bdc2d7edf3..e0542d2839 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1980,6 +1980,13 @@ virSecurityDACRestoreAllLabel(virSecurityManagerPtr mgr, rc = -1; } + if (def->tpmproxy) { + if (virSecurityDACRestoreTPMFileLabel(mgr, + def, + def->tpmproxy) < 0) + rc = -1; + } + if (def->sev) { if (virSecurityDACRestoreSEVLabel(mgr, def) < 0) rc = -1; @@ -2159,6 +2166,13 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr, return -1; } + if (def->tpmproxy) { + if (virSecurityDACSetTPMFileLabel(mgr, + def, + def->tpmproxy) < 0) + return -1; + } + if (def->sev) { if (virSecurityDACSetSEVLabel(mgr, def) < 0) return -1; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 9a929debe1..e80d43c0a7 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -2763,6 +2763,12 @@ virSecuritySELinuxRestoreAllLabel(virSecurityManagerPtr mgr, rc = -1; } + if (def->tpmproxy) { + if (virSecuritySELinuxRestoreTPMFileLabelInt(mgr, def, + def->tpmproxy) < 0) + rc = -1; + } + struct _virSecuritySELinuxChardevCallbackData chardevData = { .mgr = mgr, .chardevStdioLogd = chardevStdioLogd @@ -3171,6 +3177,11 @@ virSecuritySELinuxSetAllLabel(virSecurityManagerPtr mgr, return -1; } + if (def->tpmproxy) { + if (virSecuritySELinuxSetTPMFileLabel(mgr, def, def->tpmproxy) < 0) + return -1; + } + struct _virSecuritySELinuxChardevCallbackData chardevData = { .mgr = mgr, .chardevStdioLogd = chardevStdioLogd -- 2.26.2
On 5/13/20 3:30 PM, Daniel Henrique Barboza wrote:
A TPM Proxy device can coexist with a regular TPM. The TPM Proxy is also always a 'passthrough' device of the 'spapr-tpm-proxy' model.
This patch adds a pointer to this device in the domain definition called 'tpmproxy'. This pointer is handled like the existing 'tpm' pointer of the VIR_DOMAIN_TPM_TYPE_PASSTHROUGH type. Cgroup, DAC/SELinux and qemu validation code was adapted to handle this new domain device.
XML functions to parse and format this new device from/to XML will be added in the next patch, together with the logic that will guarantee the assumptions made in the first paragraph.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
AppArmor shouldn't need anything. Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
--- src/conf/domain_audit.c | 3 +++ src/conf/domain_conf.c | 18 ++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/qemu/qemu_cgroup.c | 12 +++++++++--- src/qemu/qemu_domain.c | 9 +++++---- src/qemu/qemu_validate.c | 12 ++++++++++++ src/security/security_dac.c | 14 ++++++++++++++ src/security/security_selinux.c | 11 +++++++++++ 8 files changed, 74 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 1b0abb21a0..4575f66e45 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -824,6 +824,9 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) if (vm->def->tpm) virDomainAuditTPM(vm, vm->def->tpm, "start", true);
+ if (vm->def->tpmproxy) + virDomainAuditTPM(vm, vm->def->tpmproxy, "start", true); + for (i = 0; i < vm->def->nshmems; i++) virDomainAuditShmem(vm, vm->def->shmems[i], "start", true);
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c201fc901d..01a32f62d1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1165,6 +1165,7 @@ VIR_ENUM_IMPL(virDomainTPMModel, "tpm-tis", "tpm-crb", "tpm-spapr", + "spapr-tpm-proxy", );
VIR_ENUM_IMPL(virDomainTPMBackend, @@ -3480,6 +3481,7 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def->mems);
virDomainTPMDefFree(def->tpm); + virDomainTPMDefFree(def->tpmproxy);
for (i = 0; i < def->npanics; i++) virDomainPanicDefFree(def->panics[i]); @@ -4318,6 +4320,12 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, if ((rc = cb(def, &device, &def->tpm->info, opaque)) != 0) return rc; } + if (def->tpmproxy) { + device.type = VIR_DOMAIN_DEVICE_TPM; + device.data.tpm = def->tpmproxy; + if ((rc = cb(def, &device, &def->tpmproxy->info, opaque)) != 0) + return rc; + } device.type = VIR_DOMAIN_DEVICE_PANIC; for (i = 0; i < def->npanics; i++) { device.data.panic = def->panics[i]; @@ -24344,6 +24352,16 @@ virDomainDefCheckABIStabilityFlags(virDomainDefPtr src, goto error; }
+ if (src->tpmproxy && dst->tpmproxy) { + if (!virDomainTPMDefCheckABIStability(src->tpmproxy, dst->tpmproxy)) + goto error; + } else if (src->tpmproxy || dst->tpmproxy) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Either both target and source domains or none of " + "them must have TPM Proxy device present")); + goto error; + } + if (src->nmems != dst->nmems) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain memory device count %zu " diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ddc75d8de2..8f178ade34 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1293,6 +1293,7 @@ typedef enum { VIR_DOMAIN_TPM_MODEL_TIS, VIR_DOMAIN_TPM_MODEL_CRB, VIR_DOMAIN_TPM_MODEL_SPAPR, + VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY,
VIR_DOMAIN_TPM_MODEL_LAST } virDomainTPMModel; @@ -2628,6 +2629,7 @@ struct _virDomainDef { virDomainMemballoonDefPtr memballoon; virDomainNVRAMDefPtr nvram; virDomainTPMDefPtr tpm; + virDomainTPMDefPtr tpmproxy; virCPUDefPtr cpu; virSysinfoDefPtr sysinfo; virDomainRedirFilterDefPtr redirfilter; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2e019b64af..2ed4341655 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -333,10 +333,13 @@ qemuSetupChardevCgroupCB(virDomainDefPtr def G_GNUC_UNUSED,
static int -qemuSetupTPMCgroup(virDomainObjPtr vm) +qemuSetupTPMCgroup(virDomainObjPtr vm, + virDomainTPMDefPtr dev) { int ret = 0; - virDomainTPMDefPtr dev = vm->def->tpm; + + if (!dev) + return 0;
switch (dev->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: @@ -806,7 +809,10 @@ qemuSetupDevicesCgroup(virDomainObjPtr vm) vm) < 0) return -1;
- if (vm->def->tpm && qemuSetupTPMCgroup(vm) < 0) + if (qemuSetupTPMCgroup(vm, vm->def->tpm) < 0) + return -1; + + if (qemuSetupTPMCgroup(vm, vm->def->tpmproxy) < 0) return -1;
for (i = 0; i < vm->def->nhostdevs; i++) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a1b250fd0b..a344f8a0e6 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11574,11 +11574,9 @@ qemuDomainSetupAllChardevs(virQEMUDriverConfigPtr cfg G_GNUC_UNUSED,
static int qemuDomainSetupTPM(virQEMUDriverConfigPtr cfg G_GNUC_UNUSED, - virDomainObjPtr vm, + virDomainTPMDefPtr dev, const struct qemuDomainCreateDeviceData *data) { - virDomainTPMDefPtr dev = vm->def->tpm; - if (!dev) return 0;
@@ -11823,7 +11821,10 @@ qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg, if (qemuDomainSetupAllChardevs(cfg, vm, &data) < 0) goto cleanup;
- if (qemuDomainSetupTPM(cfg, vm, &data) < 0) + if (qemuDomainSetupTPM(cfg, vm->def->tpm, &data) < 0) + goto cleanup; + + if (qemuDomainSetupTPM(cfg, vm->def->tpmproxy, &data) < 0) goto cleanup;
if (qemuDomainSetupAllGraphics(cfg, vm, &data) < 0) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 584d1375b8..7210be3532 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3602,6 +3602,7 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm, case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_PASSTHROUGH)) goto no_support; + break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR: @@ -3623,6 +3624,17 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm, case VIR_DOMAIN_TPM_MODEL_SPAPR: flag = QEMU_CAPS_DEVICE_TPM_SPAPR; break; + case VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY: + if (!ARCH_IS_PPC64(def->os.arch)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("TPM Proxy model %s is only available for " + "PPC64 guests"), + virDomainTPMModelTypeToString(tpm->model)); + return -1; + } + + flag = QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY; + break; case VIR_DOMAIN_TPM_MODEL_LAST: default: virReportEnumRangeError(virDomainTPMModel, tpm->model); diff --git a/src/security/security_dac.c b/src/security/security_dac.c index bdc2d7edf3..e0542d2839 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1980,6 +1980,13 @@ virSecurityDACRestoreAllLabel(virSecurityManagerPtr mgr, rc = -1; }
+ if (def->tpmproxy) { + if (virSecurityDACRestoreTPMFileLabel(mgr, + def, + def->tpmproxy) < 0) + rc = -1; + } + if (def->sev) { if (virSecurityDACRestoreSEVLabel(mgr, def) < 0) rc = -1; @@ -2159,6 +2166,13 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr, return -1; }
+ if (def->tpmproxy) { + if (virSecurityDACSetTPMFileLabel(mgr, + def, + def->tpmproxy) < 0) + return -1; + } + if (def->sev) { if (virSecurityDACSetSEVLabel(mgr, def) < 0) return -1; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 9a929debe1..e80d43c0a7 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -2763,6 +2763,12 @@ virSecuritySELinuxRestoreAllLabel(virSecurityManagerPtr mgr, rc = -1; }
+ if (def->tpmproxy) { + if (virSecuritySELinuxRestoreTPMFileLabelInt(mgr, def, + def->tpmproxy) < 0) + rc = -1; + } + struct _virSecuritySELinuxChardevCallbackData chardevData = { .mgr = mgr, .chardevStdioLogd = chardevStdioLogd @@ -3171,6 +3177,11 @@ virSecuritySELinuxSetAllLabel(virSecurityManagerPtr mgr, return -1; }
+ if (def->tpmproxy) { + if (virSecuritySELinuxSetTPMFileLabel(mgr, def, def->tpmproxy) < 0) + return -1; + } + struct _virSecuritySELinuxChardevCallbackData chardevData = { .mgr = mgr, .chardevStdioLogd = chardevStdioLogd
Next patch will make use of g_autoptr() with virDomainTPMDefPtr. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8f178ade34..60dbba3b19 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3028,6 +3028,7 @@ virDomainDeviceInfoPtr virDomainDeviceGetInfo(virDomainDeviceDefPtr device); void virDomainDeviceSetData(virDomainDeviceDefPtr device, void *devicedata); void virDomainTPMDefFree(virDomainTPMDefPtr def); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainTPMDef, virDomainTPMDefFree); typedef int (*virDomainDeviceInfoCallback)(virDomainDefPtr def, virDomainDeviceDefPtr dev, -- 2.26.2
On 5/13/20 3:30 PM, Daniel Henrique Barboza wrote:
Next patch will make use of g_autoptr() with virDomainTPMDefPtr.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8f178ade34..60dbba3b19 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3028,6 +3028,7 @@ virDomainDeviceInfoPtr virDomainDeviceGetInfo(virDomainDeviceDefPtr device); void virDomainDeviceSetData(virDomainDeviceDefPtr device, void *devicedata); void virDomainTPMDefFree(virDomainTPMDefPtr def); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainTPMDef, virDomainTPMDefFree);
typedef int (*virDomainDeviceInfoCallback)(virDomainDefPtr def, virDomainDeviceDefPtr dev,
It seems there are several other ones of those now already in this file. So it should be good. Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Aside from trivial XML parsing/format changes, this patch adds additional rules for TPM device support to better accomodate all the available scenarios with the new TPM Proxy. The changes make no impact to existing domains. This means that the scenario of a domain with a single TPM device is still supported in the same way. The restriction of multiple TPM devices got alleviated to allow a TPM Proxy device to be added together with a TPM device in the same domain. All other combinations are still forbidden. To summarize, after this patch, the following combinations in the same domain are valid: - a single TPM device - a single TPM Proxy device - a single TPM + single TPM Proxy devices These combinations in the same domain are NOT allowed: - 2 or more TPM devices - 2 or more TPM Proxy devices Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 45 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 01a32f62d1..8164cd58c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13730,6 +13730,14 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } + /* TPM Proxy devices have 'passthrough' backend */ + if (def->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY && + def->type != VIR_DOMAIN_TPM_TYPE_PASSTHROUGH) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'Passthrough' backend is required for TPM Proxy devices")); + goto error; + } + if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0) goto error; @@ -21972,15 +21980,39 @@ virDomainDefParseXML(xmlDocPtr xml, if ((n = virXPathNodeSet("./devices/tpm", ctxt, &nodes)) < 0) goto error; - if (n > 1) { + if (n > 2) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("only a single TPM device is supported")); + _("a maximum of two TPM devices is supported, one of " + "them being a TPM Proxy device")); goto error; } if (n > 0) { - if (!(def->tpm = virDomainTPMDefParseXML(xmlopt, nodes[0], ctxt, flags))) - goto error; + for (i = 0; i < n; i++) { + g_autoptr(virDomainTPMDef) dev = NULL; + + if (!(dev = virDomainTPMDefParseXML(xmlopt, nodes[i], ctxt, flags))) + goto error; + + /* TPM Proxy devices must be held in def->tpmproxy. Error + * out if there's a TPM Proxy declared already */ + if (dev->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) { + if (def->tpmproxy) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only a single TPM Proxy device is supported")); + goto error; + } + def->tpmproxy = g_steal_pointer(&dev); + } else { + /* all other TPM devices goes to def->tpm */ + if (def->tpm) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only a single TPM non-proxy device is supported")); + goto error; + } + def->tpm = g_steal_pointer(&dev); + } + } } VIR_FREE(nodes); @@ -29807,6 +29839,11 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def, goto error; } + if (def->tpmproxy) { + if (virDomainTPMDefFormat(buf, def->tpmproxy, flags) < 0) + goto error; + } + for (n = 0; n < def->ngraphics; n++) { if (virDomainGraphicsDefFormat(buf, def->graphics[n], flags) < 0) goto error; -- 2.26.2
On 5/13/20 3:30 PM, Daniel Henrique Barboza wrote:
Aside from trivial XML parsing/format changes, this patch adds additional rules for TPM device support to better accomodate all the available scenarios with the new TPM Proxy.
The changes make no impact to existing domains. This means that the scenario of a domain with a single TPM device is still supported in the same way. The restriction of multiple TPM devices got alleviated to allow a TPM Proxy device to be added together with a TPM device in the same domain. All other combinations are still forbidden.
To summarize, after this patch, the following combinations in the same domain are valid:
- a single TPM device - a single TPM Proxy device - a single TPM + single TPM Proxy devices
These combinations in the same domain are NOT allowed:
- 2 or more TPM devices - 2 or more TPM Proxy devices
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
--- src/conf/domain_conf.c | 45 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 01a32f62d1..8164cd58c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13730,6 +13730,14 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; }
+ /* TPM Proxy devices have 'passthrough' backend */ + if (def->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY && + def->type != VIR_DOMAIN_TPM_TYPE_PASSTHROUGH) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'Passthrough' backend is required for TPM Proxy devices")); + goto error; + } + if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0) goto error;
@@ -21972,15 +21980,39 @@ virDomainDefParseXML(xmlDocPtr xml, if ((n = virXPathNodeSet("./devices/tpm", ctxt, &nodes)) < 0) goto error;
- if (n > 1) { + if (n > 2) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("only a single TPM device is supported")); + _("a maximum of two TPM devices is supported, one of " + "them being a TPM Proxy device")); goto error; }
if (n > 0) { - if (!(def->tpm = virDomainTPMDefParseXML(xmlopt, nodes[0], ctxt, flags))) - goto error; + for (i = 0; i < n; i++) { + g_autoptr(virDomainTPMDef) dev = NULL; + + if (!(dev = virDomainTPMDefParseXML(xmlopt, nodes[i], ctxt, flags))) + goto error; + + /* TPM Proxy devices must be held in def->tpmproxy. Error + * out if there's a TPM Proxy declared already */ + if (dev->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) { + if (def->tpmproxy) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only a single TPM Proxy device is supported")); + goto error; + } + def->tpmproxy = g_steal_pointer(&dev); + } else { + /* all other TPM devices goes to def->tpm */ + if (def->tpm) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only a single TPM non-proxy device is supported")); + goto error; + } + def->tpm = g_steal_pointer(&dev); + } + } } VIR_FREE(nodes);
@@ -29807,6 +29839,11 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def, goto error; }
+ if (def->tpmproxy) { + if (virDomainTPMDefFormat(buf, def->tpmproxy, flags) < 0) + goto error; + } + for (n = 0; n < def->ngraphics; n++) { if (virDomainGraphicsDefFormat(buf, def->graphics[n], flags) < 0) goto error;
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
Aside from trivial XML parsing/format changes, this patch adds additional rules for TPM device support to better accomodate all the available scenarios with the new TPM Proxy.
The changes make no impact to existing domains. This means that the scenario of a domain with a single TPM device is still supported in the same way. The restriction of multiple TPM devices got alleviated to allow a TPM Proxy device to be added together with a TPM device in the same domain. All other combinations are still forbidden.
To summarize, after this patch, the following combinations in the same domain are valid:
- a single TPM device - a single TPM Proxy device - a single TPM + single TPM Proxy devices
These combinations in the same domain are NOT allowed:
- 2 or more TPM devices - 2 or more TPM Proxy devices
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 45 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 01a32f62d1..8164cd58c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13730,6 +13730,14 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; }
+ /* TPM Proxy devices have 'passthrough' backend */ + if (def->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY && + def->type != VIR_DOMAIN_TPM_TYPE_PASSTHROUGH) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'Passthrough' backend is required for TPM Proxy devices")); + goto error; + } +
This check should be in a Validate function, not the parser.
if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0) goto error;
@@ -21972,15 +21980,39 @@ virDomainDefParseXML(xmlDocPtr xml, if ((n = virXPathNodeSet("./devices/tpm", ctxt, &nodes)) < 0) goto error;
- if (n > 1) { + if (n > 2) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("only a single TPM device is supported")); + _("a maximum of two TPM devices is supported, one of " + "them being a TPM Proxy device")); goto error; }
if (n > 0) { - if (!(def->tpm = virDomainTPMDefParseXML(xmlopt, nodes[0], ctxt, flags))) - goto error; + for (i = 0; i < n; i++) { + g_autoptr(virDomainTPMDef) dev = NULL; + + if (!(dev = virDomainTPMDefParseXML(xmlopt, nodes[i], ctxt, flags))) + goto error; + + /* TPM Proxy devices must be held in def->tpmproxy. Error + * out if there's a TPM Proxy declared already */ + if (dev->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) { + if (def->tpmproxy) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only a single TPM Proxy device is supported")); + goto error; + } + def->tpmproxy = g_steal_pointer(&dev); + } else { + /* all other TPM devices goes to def->tpm */
Any reason why you store them separately? It seems they are treated the same in every place except when building QEMU command line. Switching to a def->tpms array would better reflect the XML. The Validate function would then check wheteher there's just one copy of each device type. Jano
+ if (def->tpm) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only a single TPM non-proxy device is supported")); + goto error; + } + def->tpm = g_steal_pointer(&dev); + } + } } VIR_FREE(nodes);
On 5/14/20 11:09 AM, Ján Tomko wrote:
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
Aside from trivial XML parsing/format changes, this patch adds additional rules for TPM device support to better accomodate all the available scenarios with the new TPM Proxy.
The changes make no impact to existing domains. This means that the scenario of a domain with a single TPM device is still supported in the same way. The restriction of multiple TPM devices got alleviated to allow a TPM Proxy device to be added together with a TPM device in the same domain. All other combinations are still forbidden.
To summarize, after this patch, the following combinations in the same domain are valid:
- a single TPM device - a single TPM Proxy device - a single TPM + single TPM Proxy devices
These combinations in the same domain are NOT allowed:
- 2 or more TPM devices - 2 or more TPM Proxy devices
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 45 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 01a32f62d1..8164cd58c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13730,6 +13730,14 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, Â Â Â Â Â Â Â goto error; Â Â Â }
+Â Â Â /* TPM Proxy devices have 'passthrough' backend */ +Â Â Â if (def->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY && +Â Â Â Â Â Â Â def->type != VIR_DOMAIN_TPM_TYPE_PASSTHROUGH) { +Â Â Â Â Â Â Â virReportError(VIR_ERR_XML_ERROR, "%s", +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â _("'Passthrough' backend is required for TPM Proxy devices")); +Â Â Â Â Â Â Â goto error; +Â Â Â } +
This check should be in a Validate function, not the parser.
Good catch.
   if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)        goto error;
@@ -21972,15 +21980,39 @@ virDomainDefParseXML(xmlDocPtr xml, Â Â Â if ((n = virXPathNodeSet("./devices/tpm", ctxt, &nodes)) < 0) Â Â Â Â Â Â Â goto error;
-Â Â Â if (n > 1) { +Â Â Â if (n > 2) { Â Â Â Â Â Â Â virReportError(VIR_ERR_XML_ERROR, "%s", -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â _("only a single TPM device is supported")); +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â _("a maximum of two TPM devices is supported, one of " +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "them being a TPM Proxy device")); Â Â Â Â Â Â Â goto error; Â Â Â }
   if (n > 0) { -       if (!(def->tpm = virDomainTPMDefParseXML(xmlopt, nodes[0], ctxt, flags))) -           goto error; +       for (i = 0; i < n; i++) { +           g_autoptr(virDomainTPMDef) dev = NULL; + +           if (!(dev = virDomainTPMDefParseXML(xmlopt, nodes[i], ctxt, flags))) +               goto error; + +           /* TPM Proxy devices must be held in def->tpmproxy. Error +            * out if there's a TPM Proxy declared already */ +           if (dev->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) { +               if (def->tpmproxy) { +                   virReportError(VIR_ERR_XML_ERROR, "%s", +                                  _("only a single TPM Proxy device is supported")); +                   goto error; +               } +               def->tpmproxy = g_steal_pointer(&dev); +           } else { +               /* all other TPM devices goes to def->tpm */
Any reason why you store them separately?
It seems they are treated the same in every place except when building QEMU command line. Switching to a def->tpms array would better reflect the XML. The Validate function would then check wheteher there's just one copy of each device type.
It is an attempt to minimize the amount of changes needed. For example, making a def->tpms array would impact all the code related to the 'emulator' TPM type, in particular the files qemu_tpm.c and qemu_extdevice.c, that would need to handle an array instead of the def->tpm pointer. It is also an attempt of making it easier for any future "TPM Proxy" device style to be added in the future. Instead of revisiting the logic inside each def->tpms loop one would simply deal with the def->tpmproxy pointer directly. And, in the case this is wrong and no one else cares about it, at the very least we didn't change the design of all TPM devices because of one single PPC64 specific model. This is all up to debate, of course. If we we decide that changing to a def->tpms array is worth the extra lines of code I'll make it happen in the v4. Thanks, DHB
Jano
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (def->tpm) { +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â virReportError(VIR_ERR_XML_ERROR, "%s", +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â _("only a single TPM non-proxy device is supported")); +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto error; +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â } +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â def->tpm = g_steal_pointer(&dev); +Â Â Â Â Â Â Â Â Â Â Â } +Â Â Â Â Â Â Â } Â Â Â } Â Â Â VIR_FREE(nodes);
On 5/14/20 11:32 AM, Daniel Henrique Barboza wrote:
On 5/14/20 11:09 AM, Ján Tomko wrote:
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
Aside from trivial XML parsing/format changes, this patch adds additional rules for TPM device support to better accomodate
[...]
Any reason why you store them separately?
It seems they are treated the same in every place except when building QEMU command line. Switching to a def->tpms array would better reflect the XML. The Validate function would then check wheteher there's just one copy of each device type.
It is an attempt to minimize the amount of changes needed. For example, making a def->tpms array would impact all the code related to the 'emulator' TPM type, in particular the files qemu_tpm.c and qemu_extdevice.c, that would need to handle an array instead of the def->tpm pointer.
It is also an attempt of making it easier for any future "TPM Proxy" device style to be added in the future. Instead of revisiting the logic inside each def->tpms loop one would simply deal with the def->tpmproxy pointer directly. And, in the case this is wrong and no one else cares about it, at the very least we didn't change the design of all TPM devices because of one single PPC64 specific model.
This is all up to debate, of course. If we we decide that changing to a def->tpms array is worth the extra lines of code I'll make it happen in the v4.
I'm experimenting with the idea of def->tpms array and, aside from having to change additional files, it's not as bad as I thought. What I'm doing here to mitigate the changes in the 'emulator' backend code is to always assure that the 'emulator' device, if present, will be always at def->tpms[0]. This makes most changes in qemu_tpm.c and qemu_extdevice.c a replace of "def->tpm" to "def->tpms[0]", instead of having to insert a for loop to interact with the array. This also has the benefit of not having to duplicate the handling of def->tpm for the def->tpmproxy pointer, as Jano mentioned above. Jano, what do you think about this idea of making def->tpms[0] always a non-proxy device? DHB
Thanks,
DHB
Jano
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (def->tpm) { +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â virReportError(VIR_ERR_XML_ERROR, "%s", +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â _("only a single TPM non-proxy device is supported")); +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto error; +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â } +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â def->tpm = g_steal_pointer(&dev); +Â Â Â Â Â Â Â Â Â Â Â } +Â Â Â Â Â Â Â } Â Â Â } Â Â Â VIR_FREE(nodes);
On 5/14/20 10:07 PM, Daniel Henrique Barboza wrote:
On 5/14/20 11:32 AM, Daniel Henrique Barboza wrote:
On 5/14/20 11:09 AM, Ján Tomko wrote:
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
Aside from trivial XML parsing/format changes, this patch adds additional rules for TPM device support to better accomodate
[...]
Any reason why you store them separately?
It seems they are treated the same in every place except when building QEMU command line. Switching to a def->tpms array would better reflect the XML. The Validate function would then check wheteher there's just one copy of each device type.
Just sent a v4 with this approach. I attempted to do the trick I mentioned previously of having def->tpms[0] to always hold a non-proxy device, but in the end I decided it wasn't worth it - I would make everyone reading qemu_extdevices.c to wonder "why is this code passing def->tpms[0] instead of interacting the whole array". It was too messy for someone who doesn't know about the TPM Proxy history. Thanks, DHB
This tests aims to exercise how a TPM Proxy device can be added in the domain, either alone or with a regular TPM device. It also ensures that we do not allow bogus scenarios to slip by. Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- tests/qemuxml2argvdata/ppc64-tpm-double.xml | 34 ++++++++++++++ .../ppc64-tpmproxy-double.xml | 38 +++++++++++++++ .../ppc64-tpmproxy-single.xml | 33 +++++++++++++ .../ppc64-tpmproxy-with-tpm.xml | 36 +++++++++++++++ tests/qemuxml2argvtest.c | 12 +++++ .../ppc64-tpmproxy-single.ppc64-latest.xml | 42 +++++++++++++++++ .../ppc64-tpmproxy-with-tpm.ppc64-latest.xml | 46 +++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 8 files changed, 243 insertions(+) create mode 100644 tests/qemuxml2argvdata/ppc64-tpm-double.xml create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-double.xml create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-single.xml create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.xml create mode 100644 tests/qemuxml2xmloutdata/ppc64-tpmproxy-single.ppc64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/ppc64-tpmproxy-with-tpm.ppc64-latest.xml diff --git a/tests/qemuxml2argvdata/ppc64-tpm-double.xml b/tests/qemuxml2argvdata/ppc64-tpm-double.xml new file mode 100644 index 0000000000..8730547a4d --- /dev/null +++ b/tests/qemuxml2argvdata/ppc64-tpm-double.xml @@ -0,0 +1,34 @@ +<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='ppc64' machine='pseries'>hvm</type> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <tpm model='tpm-spapr'> + <backend type='emulator' version='2.0'/> + </tpm> + <tpm model='tpm-spapr'> + <backend type='emulator' version='2.0'/> + </tpm> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/ppc64-tpmproxy-double.xml b/tests/qemuxml2argvdata/ppc64-tpmproxy-double.xml new file mode 100644 index 0000000000..12abda509e --- /dev/null +++ b/tests/qemuxml2argvdata/ppc64-tpmproxy-double.xml @@ -0,0 +1,38 @@ +<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='ppc64' machine='pseries'>hvm</type> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <tpm model='spapr-tpm-proxy'> + <backend type='passthrough'> + <device path='/dev/tpmrm0'/> + </backend> + </tpm> + <tpm model='spapr-tpm-proxy'> + <backend type='passthrough'> + <device path='/dev/tpmrm0'/> + </backend> + </tpm> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/ppc64-tpmproxy-single.xml b/tests/qemuxml2argvdata/ppc64-tpmproxy-single.xml new file mode 100644 index 0000000000..729a2cdf28 --- /dev/null +++ b/tests/qemuxml2argvdata/ppc64-tpmproxy-single.xml @@ -0,0 +1,33 @@ +<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='ppc64' machine='pseries'>hvm</type> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <tpm model='spapr-tpm-proxy'> + <backend type='passthrough'> + <device path='/dev/tpmrm0'/> + </backend> + </tpm> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.xml b/tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.xml new file mode 100644 index 0000000000..a61ec9845c --- /dev/null +++ b/tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.xml @@ -0,0 +1,36 @@ +<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='ppc64' machine='pseries'>hvm</type> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <tpm model='spapr-tpm-proxy'> + <backend type='passthrough'> + <device path='/dev/tpmrm0'/> + </backend> + </tpm> + <tpm model='tpm-spapr'> + <backend type='emulator' version='2.0'/> + </tpm> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 43e76956cc..ba82da5f4b 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2966,6 +2966,18 @@ mymain(void) QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_DEVICE_QEMU_XHCI); + DO_TEST_PARSE_ERROR("ppc64-tpmproxy-double", + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_PCI_OHCI, + QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, + QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY); + + DO_TEST_PARSE_ERROR("ppc64-tpm-double", + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_PCI_OHCI, + QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, + QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY); + DO_TEST("aarch64-usb-controller-qemu-xhci", QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_NEC_USB_XHCI, diff --git a/tests/qemuxml2xmloutdata/ppc64-tpmproxy-single.ppc64-latest.xml b/tests/qemuxml2xmloutdata/ppc64-tpmproxy-single.ppc64-latest.xml new file mode 100644 index 0000000000..4e0e5f24b8 --- /dev/null +++ b/tests/qemuxml2xmloutdata/ppc64-tpmproxy-single.ppc64-latest.xml @@ -0,0 +1,42 @@ +<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='ppc64' machine='pseries'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>POWER9</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='usb' index='0' model='pci-ohci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> + <tpm model='spapr-tpm-proxy'> + <backend type='passthrough'> + <device path='/dev/tpmrm0'/> + </backend> + </tpm> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </memballoon> + <panic model='pseries'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/ppc64-tpmproxy-with-tpm.ppc64-latest.xml b/tests/qemuxml2xmloutdata/ppc64-tpmproxy-with-tpm.ppc64-latest.xml new file mode 100644 index 0000000000..2e0dab4b33 --- /dev/null +++ b/tests/qemuxml2xmloutdata/ppc64-tpmproxy-with-tpm.ppc64-latest.xml @@ -0,0 +1,46 @@ +<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='ppc64' machine='pseries'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>POWER9</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='usb' index='0' model='pci-ohci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> + <tpm model='tpm-spapr'> + <backend type='emulator' version='2.0'/> + <address type='spapr-vio' reg='0x00004000'/> + </tpm> + <tpm model='spapr-tpm-proxy'> + <backend type='passthrough'> + <device path='/dev/tpmrm0'/> + </backend> + </tpm> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </memballoon> + <panic model='pseries'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 033f81013e..8360d5eeca 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -609,6 +609,8 @@ mymain(void) DO_TEST("controller-usb-order", QEMU_CAPS_PIIX_DISABLE_S3, QEMU_CAPS_PIIX_DISABLE_S4); + DO_TEST_CAPS_ARCH_LATEST("ppc64-tpmproxy-single", "ppc64"); + DO_TEST_CAPS_ARCH_LATEST("ppc64-tpmproxy-with-tpm", "ppc64"); DO_TEST_FULL("seclabel-dynamic-baselabel", WHEN_INACTIVE, ARG_QEMU_CAPS, NONE); -- 2.26.2
This patch wraps it up all the wiring done in previous patches, enabling a PPC64 guest to launch a guest using a TPM Proxy device. Note that device validation is already being done in qemu_validate.c, qemuValidateDomainDeviceDefTPM(), on domain define time. We don't need to verify QEMU capabilities for this device again inside qemu_command.c. Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_alias.c | 16 ++++++++++++++++ src/qemu/qemu_command.c | 21 +++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index b0ea62af39..08fe5aa501 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -413,6 +413,18 @@ qemuAssignDeviceTPMAlias(virDomainTPMDefPtr tpm, } +static int +qemuAssignDeviceTPMProxyAlias(virDomainTPMDefPtr tpmproxy, + int idx) +{ + if (tpmproxy->info.alias) + return 0; + + tpmproxy->info.alias = g_strdup_printf("tpmproxy%d", idx); + return 0; +} + + int qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, virDomainRedirdevDefPtr redirdev, @@ -673,6 +685,10 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) if (qemuAssignDeviceTPMAlias(def->tpm, 0) < 0) return -1; } + if (def->tpmproxy) { + if (qemuAssignDeviceTPMProxyAlias(def->tpmproxy, 0) < 0) + return -1; + } for (i = 0; i < def->nmems; i++) { if (qemuAssignDeviceMemoryAlias(NULL, def->mems[i], false) < 0) return -1; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bfe70ed228..0b97db7388 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8981,6 +8981,24 @@ qemuBuildTPMCommandLine(virCommandPtr cmd, return 0; } +static int +qemuBuildTPMProxyCommandLine(virCommandPtr cmd, + const virDomainDef *def) +{ + const virDomainTPMDef *tpmproxy = def->tpmproxy; + + if (!tpmproxy) + return 0; + + virCommandAddArg(cmd, "-device"); + virCommandAddArgFormat(cmd, "%s,id=%s,host-path=%s", + virDomainTPMModelTypeToString(tpmproxy->model), + tpmproxy->info.alias, + tpmproxy->data.passthrough.source.data.file.path); + + return 0; +} + static int qemuBuildSEVCommandLine(virDomainObjPtr vm, virCommandPtr cmd, virDomainSEVDefPtr sev) @@ -9662,6 +9680,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildTPMCommandLine(cmd, def, qemuCaps) < 0) return NULL; + if (qemuBuildTPMProxyCommandLine(cmd, def) < 0) + return NULL; + if (qemuBuildInputCommandLine(cmd, def, qemuCaps) < 0) return NULL; -- 2.26.2
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
This patch wraps it up all the wiring done in previous patches, enabling a PPC64 guest to launch a guest using a TPM Proxy device.
Note that device validation is already being done in qemu_validate.c, qemuValidateDomainDeviceDefTPM(), on domain define time. We don't need to verify QEMU capabilities for this device again inside qemu_command.c.
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_alias.c | 16 ++++++++++++++++ src/qemu/qemu_command.c | 21 +++++++++++++++++++++ 2 files changed, 37 insertions(+)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index b0ea62af39..08fe5aa501 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -413,6 +413,18 @@ qemuAssignDeviceTPMAlias(virDomainTPMDefPtr tpm, }
+static int +qemuAssignDeviceTPMProxyAlias(virDomainTPMDefPtr tpmproxy, + int idx) +{ + if (tpmproxy->info.alias) + return 0; + + tpmproxy->info.alias = g_strdup_printf("tpmproxy%d", idx); + return 0; +} + + int qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, virDomainRedirdevDefPtr redirdev, @@ -673,6 +685,10 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) if (qemuAssignDeviceTPMAlias(def->tpm, 0) < 0) return -1; } + if (def->tpmproxy) { + if (qemuAssignDeviceTPMProxyAlias(def->tpmproxy, 0) < 0) + return -1; + } for (i = 0; i < def->nmems; i++) { if (qemuAssignDeviceMemoryAlias(NULL, def->mems[i], false) < 0) return -1; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bfe70ed228..0b97db7388 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8981,6 +8981,24 @@ qemuBuildTPMCommandLine(virCommandPtr cmd, return 0; }
+static int +qemuBuildTPMProxyCommandLine(virCommandPtr cmd, + const virDomainDef *def) +{ + const virDomainTPMDef *tpmproxy = def->tpmproxy; + + if (!tpmproxy) + return 0; + + virCommandAddArg(cmd, "-device"); + virCommandAddArgFormat(cmd, "%s,id=%s,host-path=%s", + virDomainTPMModelTypeToString(tpmproxy->model), + tpmproxy->info.alias, + tpmproxy->data.passthrough.source.data.file.path);
The path is user-supplied and needs to be comma-escaped. Jano
+ + return 0; +} + static int qemuBuildSEVCommandLine(virDomainObjPtr vm, virCommandPtr cmd, virDomainSEVDefPtr sev) @@ -9662,6 +9680,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildTPMCommandLine(cmd, def, qemuCaps) < 0) return NULL;
+ if (qemuBuildTPMProxyCommandLine(cmd, def) < 0) + return NULL; + if (qemuBuildInputCommandLine(cmd, def, qemuCaps) < 0) return NULL;
-- 2.26.2
Add tests for both supported scenarios: a single TPM Proxy and a TPM Proxy with a regular TPM device in the same domain. Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- .../ppc64-tpmproxy-single.ppc64-latest.args | 34 +++++++++++++++++ .../ppc64-tpmproxy-with-tpm.ppc64-latest.args | 37 +++++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 3 files changed, 74 insertions(+) create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-single.ppc64-latest.args create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.ppc64-latest.args diff --git a/tests/qemuxml2argvdata/ppc64-tpmproxy-single.ppc64-latest.args b/tests/qemuxml2argvdata/ppc64-tpmproxy-single.ppc64-latest.args new file mode 100644 index 0000000000..f606cee16b --- /dev/null +++ b/tests/qemuxml2argvdata/ppc64-tpmproxy-single.ppc64-latest.args @@ -0,0 +1,34 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-ppc64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pseries,accel=tcg,usb=off,dump-guest-core=off \ +-cpu POWER9 \ +-m 256 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device pci-ohci,id=usb,bus=pci.0,addr=0x1 \ +-device spapr-tpm-proxy,id=tpmproxy0,host-path=/dev/tpmrm0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.ppc64-latest.args b/tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.ppc64-latest.args new file mode 100644 index 0000000000..9908cd78e0 --- /dev/null +++ b/tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.ppc64-latest.args @@ -0,0 +1,37 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-ppc64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pseries,accel=tcg,usb=off,dump-guest-core=off \ +-cpu POWER9 \ +-m 256 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device pci-ohci,id=usb,bus=pci.0,addr=0x1 \ +-tpmdev emulator,id=tpm-tpm0,chardev=chrtpm \ +-chardev socket,id=chrtpm,path=/dev/test \ +-device tpm-spapr,tpmdev=tpm-tpm0,id=tpm0,reg=0x00004000 \ +-device spapr-tpm-proxy,id=tpmproxy0,host-path=/dev/tpmrm0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ba82da5f4b..6a57a4910d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2978,6 +2978,9 @@ mymain(void) QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY); + DO_TEST_CAPS_LATEST_PPC64("ppc64-tpmproxy-single"); + DO_TEST_CAPS_LATEST_PPC64("ppc64-tpmproxy-with-tpm"); + DO_TEST("aarch64-usb-controller-qemu-xhci", QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_NEC_USB_XHCI, -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- docs/news.xml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 4cef804aac..c22a0f0a18 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -44,6 +44,23 @@ <libvirt> <release version="v6.4.0" date="unreleased"> <section title="New features"> + <change> + <summary> + qemu: add TPM Proxy device support + </summary> + <description> + libvirt can now create guests using a new device type called + "TPM Proxy". The TPM Proxy connects to a TPM Resource Manager + present in the host, enabling the guest to run in secure virtual + machine mode with the help of an Ultravisor. Adding a TPM Proxy to + a pSeries guest brings no security benefits unless the guest is + running on a PPC64 host that has Ultravisor and TPM Resource Manager + support. Only one TPM Proxy is allowed per guest. A guest using + a TPM Proxy device can instantiate another TPM device at the same + time. This device is supported only for pSeries guests via the new + 'spapr-tpm-proxy' model of the TPM 'passthrough' backend. + </description> + </change> </section> <section title="Improvements"> </section> -- 2.26.2
On 5/13/20 3:30 PM, Daniel Henrique Barboza wrote:
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
--- docs/news.xml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 4cef804aac..c22a0f0a18 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -44,6 +44,23 @@ <libvirt> <release version="v6.4.0" date="unreleased"> <section title="New features"> + <change> + <summary> + qemu: add TPM Proxy device support + </summary> + <description> + libvirt can now create guests using a new device type called + "TPM Proxy". The TPM Proxy connects to a TPM Resource Manager + present in the host, enabling the guest to run in secure virtual + machine mode with the help of an Ultravisor. Adding a TPM Proxy to + a pSeries guest brings no security benefits unless the guest is + running on a PPC64 host that has Ultravisor and TPM Resource Manager + support. Only one TPM Proxy is allowed per guest. A guest using + a TPM Proxy device can instantiate another TPM device at the same + time. This device is supported only for pSeries guests via the new + 'spapr-tpm-proxy' model of the TPM 'passthrough' backend. + </description> + </change> </section> <section title="Improvements"> </section>
participants (3)
-
Daniel Henrique Barboza -
Ján Tomko -
Stefan Berger