[libvirt] [PATCH v3 0/3] Support tpm-crb TPM interface for QEMU

This patch series adds support for the recently added tpm-crb TPM interface for QEMU (2.12). Stefan Stefan Berger (3): tpm: Enable TPM CRB interface in the domain XML and test it qemu: Extend the capabilities with tpm-crb device tests: add test case for tpm-crb QEMU device command line docs/formatdomain.html.in | 6 +++- docs/schemas/domaincommon.rng | 5 +++- src/conf/domain_conf.c | 5 ++-- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 5 ++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 16 +++++++++- tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + tests/qemuxml2argvdata/tpm-passthrough-crb.args | 26 ++++++++++++++++ tests/qemuxml2argvdata/tpm-passthrough-crb.xml | 32 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ tests/qemuxml2xmloutdata/tpm-passthrough-crb.xml | 36 +++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 13 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/tpm-passthrough-crb.args create mode 100644 tests/qemuxml2argvdata/tpm-passthrough-crb.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-passthrough-crb.xml -- 2.5.5

Enable the TPM CRB to be specified in the domain XML. This now allows to describe the TPM device like this: <tpm model='tpm-crb'> <backend type='passthrough'> <device path='/dev/tpm0'/> </backend> </tpm> Extend the XML schema to also allow tpm-crb. Extend the documentation. Add a test case for testing the XML parser and formatter. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 6 +++- docs/schemas/domaincommon.rng | 5 +++- src/conf/domain_conf.c | 5 ++-- src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/tpm-passthrough-crb.xml | 32 +++++++++++++++++++++ tests/qemuxml2xmloutdata/tpm-passthrough-crb.xml | 36 ++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/tpm-passthrough-crb.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-passthrough-crb.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 8e864f7..d48e335 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7625,7 +7625,8 @@ qemu-kvm -net nic,model=? /dev/null <p> The TPM device enables a QEMU guest to have access to TPM - functionality. + functionality. The TPM device may either be a TPM 1.2 or + a TPM 2. </p> <p> The TPM passthrough device type provides access to the host's TPM @@ -7655,6 +7656,9 @@ qemu-kvm -net nic,model=? /dev/null The <code>model</code> attribute specifies what device model QEMU provides to the guest. If no model name is provided, <code>tpm-tis</code> will automatically be chosen. + <span class="since">Since 4.3.0</span>, another available choice + is the <code>tpm-crb</code>, which should only be used when the + backend device is a TPM 2. </p> </dd> <dt><code>backend</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3569b92..1a73c4c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4115,7 +4115,10 @@ <element name="tpm"> <optional> <attribute name="model"> - <value>tpm-tis</value> + <choice> + <value>tpm-tis</value> + <value>tpm-crb</value> + </choice> </attribute> </optional> <ref name="tpm-backend"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b025706..6a2e28a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -860,7 +860,8 @@ VIR_ENUM_IMPL(virDomainRNGBackend, "egd"); VIR_ENUM_IMPL(virDomainTPMModel, VIR_DOMAIN_TPM_MODEL_LAST, - "tpm-tis") + "tpm-tis", + "tpm-crb") VIR_ENUM_IMPL(virDomainTPMBackend, VIR_DOMAIN_TPM_TYPE_LAST, "passthrough") @@ -12606,8 +12607,6 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unknown TPM frontend model '%s'"), model); goto error; - } else { - def->model = VIR_DOMAIN_TPM_MODEL_TIS; } ctxt->node = node; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3c7eccb..8b82cf9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1282,6 +1282,7 @@ struct _virDomainHubDef { typedef enum { VIR_DOMAIN_TPM_MODEL_TIS, + VIR_DOMAIN_TPM_MODEL_CRB, VIR_DOMAIN_TPM_MODEL_LAST } virDomainTPMModel; diff --git a/tests/qemuxml2argvdata/tpm-passthrough-crb.xml b/tests/qemuxml2argvdata/tpm-passthrough-crb.xml new file mode 100644 index 0000000..2fce5ca --- /dev/null +++ b/tests/qemuxml2argvdata/tpm-passthrough-crb.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>TPM-VM</name> + <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>512288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <tpm model='tpm-crb'> + <backend type='passthrough'> + <device path='/dev/tpm0'/> + </backend> + </tpm> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/tpm-passthrough-crb.xml b/tests/qemuxml2xmloutdata/tpm-passthrough-crb.xml new file mode 100644 index 0000000..67ada46 --- /dev/null +++ b/tests/qemuxml2xmloutdata/tpm-passthrough-crb.xml @@ -0,0 +1,36 @@ +<domain type='qemu'> + <name>TPM-VM</name> + <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>512288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <tpm model='tpm-crb'> + <backend type='passthrough'> + <device path='/dev/tpm0'/> + </backend> + </tpm> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 4b5aa23..21fb411 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -672,6 +672,7 @@ mymain(void) DO_TEST("usb-ich9-ehci-addr", NONE); DO_TEST("disk-copy_on_read", NONE); DO_TEST("tpm-passthrough", NONE); + DO_TEST("tpm-passthrough-crb", NONE); DO_TEST("metadata", NONE); DO_TEST("metadata-duplicate", NONE); -- 2.5.5

I'll change the $SUBJ to be: conf: Enable TPM CRB interface in the domain XML [follows convention we typically use] On 04/26/2018 01:42 PM, Stefan Berger wrote:
Enable the TPM CRB to be specified in the domain XML. This now allows to describe the TPM device like this:
<tpm model='tpm-crb'> <backend type='passthrough'> <device path='/dev/tpm0'/> </backend> </tpm>
Extend the XML schema to also allow tpm-crb. Extend the documentation. Add a test case for testing the XML parser and formatter.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 6 +++- docs/schemas/domaincommon.rng | 5 +++- src/conf/domain_conf.c | 5 ++-- src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/tpm-passthrough-crb.xml | 32 +++++++++++++++++++++ tests/qemuxml2xmloutdata/tpm-passthrough-crb.xml | 36 ++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/tpm-passthrough-crb.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-passthrough-crb.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 8e864f7..d48e335 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7625,7 +7625,8 @@ qemu-kvm -net nic,model=? /dev/null
<p> The TPM device enables a QEMU guest to have access to TPM - functionality. + functionality. The TPM device may either be a TPM 1.2 or + a TPM 2. </p> <p> The TPM passthrough device type provides access to the host's TPM @@ -7655,6 +7656,9 @@ qemu-kvm -net nic,model=? /dev/null The <code>model</code> attribute specifies what device model QEMU provides to the guest. If no model name is provided, <code>tpm-tis</code> will automatically be chosen. + <span class="since">Since 4.3.0</span>, another available choice
I'll change this to 4.4.0 before pushing...
+ is the <code>tpm-crb</code>, which should only be used when the + backend device is a TPM 2. </p> </dd> <dt><code>backend</code></dt>
Reviewed-by: John Ferlan <jferlan@redhat.com> John [...]

QEMU on x86_64 (since v2.12) can support tpm-crb devices. Introduce qemu capabilities for this device. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + 3 files changed, 7 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 833c755..22d86da 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -473,6 +473,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, /* 290 */ "query-cpus-fast", "disk-write-cache", + "tpm-crb", ); @@ -2342,6 +2343,10 @@ const struct tpmTypeToCaps virQEMUCapsTPMModelsToCaps[] = { .type = VIR_DOMAIN_TPM_MODEL_TIS, .caps = QEMU_CAPS_DEVICE_TPM_TIS, }, + { + .type = VIR_DOMAIN_TPM_MODEL_CRB, + .caps = QEMU_CAPS_DEVICE_TPM_CRB, + }, }; static int diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f08cfc2..d51d710 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -457,6 +457,7 @@ typedef enum { /* 290 */ QEMU_CAPS_QUERY_CPUS_FAST, /* query-cpus-fast command */ QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache param */ + QEMU_CAPS_DEVICE_TPM_CRB, /* -device tpm-crb */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml index d809a78..f573fb7 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml @@ -195,6 +195,7 @@ <flag name='seccomp-blacklist'/> <flag name='query-cpus-fast'/> <flag name='disk-write-cache'/> + <flag name='tpm-crb'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>390060</microcodeVersion> -- 2.5.5

On 04/26/2018 01:42 PM, Stefan Berger wrote:
QEMU on x86_64 (since v2.12) can support tpm-crb devices. Introduce qemu capabilities for this device.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + 3 files changed, 7 insertions(+)
I'll merge this with latest top... Reviewed-by: John Ferlan <jferlan@redhat.com> John

On 05/01/2018 09:13 AM, John Ferlan wrote:
On 04/26/2018 01:42 PM, Stefan Berger wrote:
QEMU on x86_64 (since v2.12) can support tpm-crb devices. Introduce qemu capabilities for this device.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + 3 files changed, 7 insertions(+)
I'll merge this with latest top...
Great! Thanks. Stefan
Reviewed-by: John Ferlan <jferlan@redhat.com>
John

Add a test case for the formation of the tpm-crb QEMU device command line. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 16 ++++++++++++++- tests/qemuxml2argvdata/tpm-passthrough-crb.args | 26 +++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/tpm-passthrough-crb.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 418729b..198b44e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9387,8 +9387,22 @@ qemuBuildTPMDevStr(const virDomainDef *def, virBuffer buf = VIR_BUFFER_INITIALIZER; const virDomainTPMDef *tpm = def->tpm; const char *model = virDomainTPMModelTypeToString(tpm->model); + virQEMUCapsFlags flag; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_TIS)) { + switch (tpm->model) { + case VIR_DOMAIN_TPM_MODEL_TIS: + flag = QEMU_CAPS_DEVICE_TPM_TIS; + break; + case VIR_DOMAIN_TPM_MODEL_CRB: + flag = QEMU_CAPS_DEVICE_TPM_CRB; + break; + case VIR_DOMAIN_TPM_MODEL_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown TPM device model %d"), tpm->model); + goto error; + } + + if (!virQEMUCapsGet(qemuCaps, flag)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("The QEMU executable %s does not support TPM " "model %s"), diff --git a/tests/qemuxml2argvdata/tpm-passthrough-crb.args b/tests/qemuxml2argvdata/tpm-passthrough-crb.args new file mode 100644 index 0000000..010495d --- /dev/null +++ b/tests/qemuxml2argvdata/tpm-passthrough-crb.args @@ -0,0 +1,26 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name TPM-VM \ +-S \ +-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off \ +-m 2048 -\ +smp 1,sockets=1,cores=1,threads=1 \ +-uuid 11d7cd22-da89-3094-6212-079a48a309a1 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-TPM-VM/monitor.sock,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot order=c,menu=on \ +-usb \ +-tpmdev passthrough,id=tpm-tpm0,path=/dev/tpm0,\ +cancel-path=/sys/class/misc/tpm0/device/cancel \ +-device tpm-crb,tpmdev=tpm-tpm0,id=tpm0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 5b3bd4a..fe2cca4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2001,6 +2001,8 @@ mymain(void) DO_TEST("tpm-passthrough", QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS); + DO_TEST("tpm-passthrough-crb", + QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_CRB); DO_TEST_PARSE_ERROR("tpm-no-backend-invalid", QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS); -- 2.5.5

I'll change the $subj to be: qemu: Add tpm-crb QEMU device to the command line On 04/26/2018 01:42 PM, Stefan Berger wrote:
Add a test case for the formation of the tpm-crb QEMU device command line.
And the commit msg changes to: Alter qemuBuildTPMDevStr to format the tpm-crb on the command line and use the enum range checking for valid model. Add a test case for the formation of the tpm-crb QEMU device command line. The qemuxml2argvtest changes cannot use the newer DO_TEST_CAPS_LATEST since building of the command line involves calling qemuBuildTPMBackendStr which attempts to open the path to the device (e.g. /dev/tmp0).
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 16 ++++++++++++++- tests/qemuxml2argvdata/tpm-passthrough-crb.args | 26 +++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/tpm-passthrough-crb.args
So this does bring up a couple of interesting notes for "additional" work or checks... 1. More recent adjustments for libvirt have begun to use the qemuProcessPrepare* type functions in order to perform similar things like the open on the /dev/tpm0 in qemuBuildTPMBackendStr. This leaves only building the command line in the qemu_command.c code. I'm not even sure this works with the TPM model given how the code passes the tpmfd to the command line. Still see qemuProcessPrepareChardevDevice (and it's corollary qemuProcessCleanupChardevDevice). There is another series upstream with a similar problem, see: https://www.redhat.com/archives/libvir-list/2018-April/msg01797.html and in particular patch 2 of the series. That may "solve" this open as well. 2. Does /dev/tpm0 need to play nicely in the QEMU name space code? See qemuDomainNamespace{Setup|Teardown}* functions. I have a feeling it may, but I'm not 100% sure. Michal Privoznik would be the contact point.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 418729b..198b44e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9387,8 +9387,22 @@ qemuBuildTPMDevStr(const virDomainDef *def, virBuffer buf = VIR_BUFFER_INITIALIZER; const virDomainTPMDef *tpm = def->tpm; const char *model = virDomainTPMModelTypeToString(tpm->model); + virQEMUCapsFlags flag;
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_TIS)) { + switch (tpm->model) { + case VIR_DOMAIN_TPM_MODEL_TIS: + flag = QEMU_CAPS_DEVICE_TPM_TIS; + break; + case VIR_DOMAIN_TPM_MODEL_CRB: + flag = QEMU_CAPS_DEVICE_TPM_CRB; + break; + case VIR_DOMAIN_TPM_MODEL_LAST:
Added the default: case; otherwise, ... [1]
+ virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown TPM device model %d"), tpm->model);
change this to virReportEnumRangeError(virDomainTPMModel, tpm->model);
+ goto error; + } + + if (!virQEMUCapsGet(qemuCaps, flag)) {
[1] ... this failed during my build because @flag wouldn't be defined supposedly ...
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("The QEMU executable %s does not support TPM " "model %s"), diff --git a/tests/qemuxml2argvdata/tpm-passthrough-crb.args b/tests/qemuxml2argvdata/tpm-passthrough-crb.args new file mode 100644 index 0000000..010495d --- /dev/null +++ b/tests/qemuxml2argvdata/tpm-passthrough-crb.args
syntax-check tells me there's some long lines in here that need to be wrapped using: tests/test-wrap-argv.pl -i tests/qemuxml2argvdata/tpm-passthrough-crb.args I've made the adjustments and will push the series once 4.4.0 opens (assuming no one else jumps in and comes up with something I've missed). Reviewed-by: John Ferlan <jferlan@redhat.com> John
@@ -0,0 +1,26 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name TPM-VM \ +-S \ +-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off \ +-m 2048 -\ +smp 1,sockets=1,cores=1,threads=1 \ +-uuid 11d7cd22-da89-3094-6212-079a48a309a1 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-TPM-VM/monitor.sock,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot order=c,menu=on \ +-usb \ +-tpmdev passthrough,id=tpm-tpm0,path=/dev/tpm0,\ +cancel-path=/sys/class/misc/tpm0/device/cancel \ +-device tpm-crb,tpmdev=tpm-tpm0,id=tpm0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 5b3bd4a..fe2cca4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2001,6 +2001,8 @@ mymain(void)
DO_TEST("tpm-passthrough", QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS); + DO_TEST("tpm-passthrough-crb", + QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_CRB); DO_TEST_PARSE_ERROR("tpm-no-backend-invalid", QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS);

On 05/01/2018 09:14 AM, John Ferlan wrote:
I'll change the $subj to be:
qemu: Add tpm-crb QEMU device to the command line
On 04/26/2018 01:42 PM, Stefan Berger wrote:
Add a test case for the formation of the tpm-crb QEMU device command line. And the commit msg changes to:
Alter qemuBuildTPMDevStr to format the tpm-crb on the command line and use the enum range checking for valid model.
Add a test case for the formation of the tpm-crb QEMU device command line. The qemuxml2argvtest changes cannot use the newer DO_TEST_CAPS_LATEST since building of the command line involves calling qemuBuildTPMBackendStr which attempts to open the path to the device (e.g. /dev/tmp0).
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 16 ++++++++++++++- tests/qemuxml2argvdata/tpm-passthrough-crb.args | 26 +++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/tpm-passthrough-crb.args
So this does bring up a couple of interesting notes for "additional" work or checks...
1. More recent adjustments for libvirt have begun to use the qemuProcessPrepare* type functions in order to perform similar things like the open on the /dev/tpm0 in qemuBuildTPMBackendStr. This leaves only building the command line in the qemu_command.c code. I'm not even sure this works with the TPM model given how the code passes the tpmfd to the command line. Still see qemuProcessPrepareChardevDevice (and it's corollary qemuProcessCleanupChardevDevice). There is another series upstream with a similar problem, see:
https://www.redhat.com/archives/libvir-list/2018-April/msg01797.html
and in particular patch 2 of the series. That may "solve" this open as well.
2. Does /dev/tpm0 need to play nicely in the QEMU name space code? See qemuDomainNamespace{Setup|Teardown}* functions. I have a feeling it may, but I'm not 100% sure. Michal Privoznik would be the contact point.
I just tried it using the passthrough device. I don't have a TPM 2 on my machine, so I had to use the TPM 1.2 /dev/tpm0. I had to stop SELinux due to the below missing rule but QEMU at least started up. Since TPM 1.2 +CRB combination isn't typically seen in the field, software doesn't support it, but at least QEMU talks to it. missing SELinux rule: allow svirt_t tpm_device_t:chr_file { read write }; The QEMU command line parameters looked like this: [...] -tpmdev passthrough,id=tpm-tpm0,path=/dev/fdset/1,cancel-path=/dev/fdset/2 -add-fd set=1,fd=29 -add-fd set=2,fd=30 -device tpm-crb,tpmdev=tpm-tpm0,id=tpm0 [...] This looks ok.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 418729b..198b44e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9387,8 +9387,22 @@ qemuBuildTPMDevStr(const virDomainDef *def, virBuffer buf = VIR_BUFFER_INITIALIZER; const virDomainTPMDef *tpm = def->tpm; const char *model = virDomainTPMModelTypeToString(tpm->model); + virQEMUCapsFlags flag;
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_TIS)) { + switch (tpm->model) { + case VIR_DOMAIN_TPM_MODEL_TIS: + flag = QEMU_CAPS_DEVICE_TPM_TIS; + break; + case VIR_DOMAIN_TPM_MODEL_CRB: + flag = QEMU_CAPS_DEVICE_TPM_CRB; + break; + case VIR_DOMAIN_TPM_MODEL_LAST: Added the default: case; otherwise, ... [1]
+ virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown TPM device model %d"), tpm->model); change this to
virReportEnumRangeError(virDomainTPMModel, tpm->model);
+ goto error; + } + + if (!virQEMUCapsGet(qemuCaps, flag)) { [1] ... this failed during my build because @flag wouldn't be defined supposedly ...
I saw that later on, too. FC23 compiler was fine without it, FC27 complains.
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("The QEMU executable %s does not support TPM " "model %s"), diff --git a/tests/qemuxml2argvdata/tpm-passthrough-crb.args b/tests/qemuxml2argvdata/tpm-passthrough-crb.args new file mode 100644 index 0000000..010495d --- /dev/null +++ b/tests/qemuxml2argvdata/tpm-passthrough-crb.args
syntax-check tells me there's some long lines in here that need to be wrapped using:
tests/test-wrap-argv.pl -i tests/qemuxml2argvdata/tpm-passthrough-crb.args
I've made the adjustments and will push the series once 4.4.0 opens (assuming no one else jumps in and comes up with something I've missed).
Thank you. Stefan
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
@@ -0,0 +1,26 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name TPM-VM \ +-S \ +-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off \ +-m 2048 -\ +smp 1,sockets=1,cores=1,threads=1 \ +-uuid 11d7cd22-da89-3094-6212-079a48a309a1 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-TPM-VM/monitor.sock,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot order=c,menu=on \ +-usb \ +-tpmdev passthrough,id=tpm-tpm0,path=/dev/tpm0,\ +cancel-path=/sys/class/misc/tpm0/device/cancel \ +-device tpm-crb,tpmdev=tpm-tpm0,id=tpm0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 5b3bd4a..fe2cca4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2001,6 +2001,8 @@ mymain(void)
DO_TEST("tpm-passthrough", QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS); + DO_TEST("tpm-passthrough-crb", + QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_CRB); DO_TEST_PARSE_ERROR("tpm-no-backend-invalid", QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS);

On 04/26/2018 01:42 PM, Stefan Berger wrote:
This patch series adds support for the recently added tpm-crb TPM interface for QEMU (2.12).
Stefan
Stefan Berger (3): tpm: Enable TPM CRB interface in the domain XML and test it qemu: Extend the capabilities with tpm-crb device tests: add test case for tpm-crb QEMU device command line
docs/formatdomain.html.in | 6 +++- docs/schemas/domaincommon.rng | 5 +++- src/conf/domain_conf.c | 5 ++-- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 5 ++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 16 +++++++++- tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + tests/qemuxml2argvdata/tpm-passthrough-crb.args | 26 ++++++++++++++++ tests/qemuxml2argvdata/tpm-passthrough-crb.xml | 32 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ tests/qemuxml2xmloutdata/tpm-passthrough-crb.xml | 36 +++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 13 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/tpm-passthrough-crb.args create mode 100644 tests/qemuxml2argvdata/tpm-passthrough-crb.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-passthrough-crb.xml
This is now pushed... Next up the other parts of your changes. John

On 05/03/2018 11:35 AM, John Ferlan wrote:
On 04/26/2018 01:42 PM, Stefan Berger wrote:
This patch series adds support for the recently added tpm-crb TPM interface for QEMU (2.12).
Stefan
Stefan Berger (3): tpm: Enable TPM CRB interface in the domain XML and test it qemu: Extend the capabilities with tpm-crb device tests: add test case for tpm-crb QEMU device command line
docs/formatdomain.html.in | 6 +++- docs/schemas/domaincommon.rng | 5 +++- src/conf/domain_conf.c | 5 ++-- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 5 ++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 16 +++++++++- tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + tests/qemuxml2argvdata/tpm-passthrough-crb.args | 26 ++++++++++++++++ tests/qemuxml2argvdata/tpm-passthrough-crb.xml | 32 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ tests/qemuxml2xmloutdata/tpm-passthrough-crb.xml | 36 +++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 13 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/tpm-passthrough-crb.args create mode 100644 tests/qemuxml2argvdata/tpm-passthrough-crb.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-passthrough-crb.xml
This is now pushed... Next up the other parts of your changes.
I'll repost the broken up patch... Stefan
John
participants (2)
-
John Ferlan
-
Stefan Berger