On 05/23/2018 11:55 AM, Ján Tomko wrote:
On Tue, May 22, 2018 at 04:44:51PM -0400, Stefan Berger wrote:
> This patch extends the TPM's device XML with TPM 2 support. This only
> works
> for the emulator type backend and looks as follows:
>
> <tpm model='tpm-tis'>
> <backend type='emulator' version='2'/>
> </tpm>
>
> The swtpm process now has --tpm2 as an additional parameter:
>
> system_u:system_r:svirt_t:s0:c597,c632 tss 18477 11.8 0.0 28364
> 3868 ? Rs 11:13 13:50 /usr/bin/swtpm socket --daemon --ctrl
> type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660
> --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm2,mode=0640 --log
> file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log --tpm2 --pid
> file=/var/run/libvirt/qemu/swtpm/testvm-swtpm.pid
>
> The version of the TPM can be changed and the state of the TPM is
> preserved.
>
> Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
> Reviewed-by: John Ferlan <jferlan(a)redhat.com>
> ---
> docs/formatdomain.html.in | 15 ++++-
> docs/schemas/domaincommon.rng | 12 ++++
> src/conf/domain_conf.c | 27 ++++++++-
> src/conf/domain_conf.h | 6 ++
> src/qemu/qemu_tpm.c | 64
> +++++++++++++++++++++-
> .../tpm-emulator-tpm2.x86_64-latest.args | 33 +++++++++++
> tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 30 ++++++++++
> tests/qemuxml2argvtest.c | 1 +
> tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml | 34 ++++++++++++
> 9 files changed, 217 insertions(+), 5 deletions(-)
> create mode 100644
> tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args
> create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
> create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml
>
> @@ -24941,6 +24963,9 @@ virDomainTPMDefFormat(virBufferPtr buf,
> virBufferAsprintf(buf, "<backend type='%s'",
> virDomainTPMBackendTypeToString(def->type));
>
> + if (def->version == VIR_DOMAIN_TPM_VERSION_2)
> + virBufferAddLit(buf, " version='2'");
> +
Any reason for not formatting version 1.2?
We should format implicit defaults in the XML if possible.
Basically I did it because the previous default 1.2 didn't have it. So I
though I'd keep it as is for 1.2 and only write out 2.
> switch (def->type) {
> case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> virBufferAddLit(buf, ">\n");
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 92466278ab..e2409899bc 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1291,12 +1291,18 @@ typedef enum {
> VIR_DOMAIN_TPM_TYPE_LAST
> } virDomainTPMBackendType;
>
> +typedef enum {
> + VIR_DOMAIN_TPM_VERSION_1_2,
> + VIR_DOMAIN_TPM_VERSION_2,
> +} virDomainTPMVersion;
With a corresponding VIR_ENUM_IMPL and VIR_ENUM_DECL,
you can use the *{To,From}String functions for parsing/formatting
the version.
> +
> # define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0"
>
> struct _virDomainTPMDef {
> virDomainTPMBackendType type;
> virDomainDeviceInfo info;
> virDomainTPMModel model;
> + virDomainTPMVersion version;
> union {
> struct {
> virDomainChrSourceDef source;
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 11b91aa915..508685c455 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -54,6 +54,41 @@ static char *swtpm_path;
> static char *swtpm_setup;
> static char *swtpm_ioctl;
>
> +bool swtpm_supports_tpm2;
> +
> +/*
> + * qemuTPMCheckForTPM2Support
> + *
> + * Check whether swtpm_setup supports TPM 2
> + */
> +static void
> +qemuTPMCheckForTPM2Support(void)
> +{
> + virCommandPtr cmd;
> + char *help = NULL;
> +
> + if (!swtpm_setup)
> + return;
> +
> + cmd = virCommandNew(swtpm_setup);
> + if (!cmd)
> + return;
> +
> + virCommandAddArg(cmd, "--help");
> + virCommandSetOutputBuffer(cmd, &help);
> +
> + if (virCommandRun(cmd, NULL) < 0)
> + goto cleanup;
> +
> + if (strstr(help, "--tpm2"))
> + swtpm_supports_tpm2 = true;
This bool is never read.
Maybe while doing some of the recent changes the reading of the variable
got lost.
Given that version 2 has to be requested in the XML and we don't try to
use it automatically, I'd suggest just dropping this function. We don't
need to parse another tool's --help output to make up for the removal
of parsing --help of qemu and qemu-img.
swtpm doesn't have all the bells and whistles of QEMU that we would have
a JSON interface to query the features from. So if a bad command line
parameter is passed to swtpm, it will dump the help screen. Here's the
output I get from trying to run a VM with an attached TPM 2 but there's
no TPM 2 support compiled into swtpm (basically because it was created
from the master branch not from the preview branch):
# virsh start testvm-tpm2
Error: Failed to start domain testvm-tpm2
error: internal error: Could not start 'swtpm'. exitstatus: 1, error:
socket: unrecognized option '--tpm2'
Usage: /usr/bin/swtpm socket [options]
The following options are supported:
-p|--port <port> : use the given port
-f|--fd <fd> : use the given socket file descriptor
[...]
I think a more controlled error message would be better basically
stating 'Local swtpm installation does not support TPM 2'.
Stefan
Jano