Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2
On Thu, May 24, 2018 at 07:22:30AM -0400, Stefan Berger wrote: On 05/24/2018 03:08 AM, Ján Tomko wrote: On Wed, May 23, 2018 at 02:33:08PM -0400, Stefan Berger wrote: swtpm doesn't have all the bells and whistles of QEMU that we would have a JSON interface to query the features from. With QEMU, we actually need to know some capabilities upfront, because different versions have had different ways of requesting the same functionality. Giving nicer errors for unsupported features is just a bonus. With qemu-img, we only care about one way of representing the functionality and let it print an error if something's compiled out. Ok, then let me remove it. v7 will have that change. Not sure what to do about the existing Reviewed-by's. Intend to keep them. The usual thing to do is keep them unless you make a significant change to the commit. Its presence in the commit message should tell the reviewer that he does not need to look at it again. Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2
On Thu, May 24, 2018 at 02:17:13PM +0200, Marc Hartmayer wrote: On Tue, May 22, 2018 at 10:44 PM +0200, 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: 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 Reviewed-by: John Ferlan --- 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 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 08a57bd751..043c8da56f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7719,7 +7719,7 @@ qemu-kvm -net nic,model=? /dev/null ...@@ -7769,6 +7769,19 @@ qemu-kvm -net nic,model=? /dev/null + version + + + The version attribute indicates the version + of the TPM. By default a TPM 1.2 is created. This attribute + only works with the emulator backend. The following + versions are supported: + + + '1.2' : creates a TPM 1.2 + '2' : creates a TPM 2 + + NVRAM device diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3582cb5019..f11833075a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4130,6 +4130,18 @@ + + + + + +1.2 +2 + + + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 15dd490d17..79904789ee 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12657,7 +12657,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt, * or like this: * * - * + * * */ static virDomainTPMDefPtr @@ -12670,6 +12670,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, char *path = NULL; char *model = NULL; char *backend = NULL; +char *version = NULL; virDomainTPMDefPtr def; xmlNodePtr save = ctxt->node; xmlNodePtr *backends = NULL; @@ -12716,6 +12717,20 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } +version = virXMLPropString(backends[0], "version"); +if (!version || STREQ(version, "1.2")) { +def->version = VIR_DOMAIN_TPM_VERSION_1_2; +/* only TIS available for emulator */ +if (def->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) +def->model = VIR_DOMAIN_TPM_MODEL_TIS; This will silently overwrite an already defined model - is this intended? Also this seems like some kind of validation logic - not sure if virDomainTPMDefParseXML is the right place for this. Yes, DefParse would ideally just convert what was provided in the XML to our internal data types. Setting defaults belongs in PostParse (either in src/conf or in src/qemu) and for validation we have qemu.*DefValidate. Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list - +
Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2
On 05/24/2018 08:17 AM, Marc Hartmayer wrote: On Tue, May 22, 2018 at 10:44 PM +0200, 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: 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 Reviewed-by: John Ferlan --- 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 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 08a57bd751..043c8da56f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7719,7 +7719,7 @@ qemu-kvm -net nic,model=? /dev/null ...@@ -7769,6 +7769,19 @@ qemu-kvm -net nic,model=? /dev/null + version + + + The version attribute indicates the version + of the TPM. By default a TPM 1.2 is created. This attribute + only works with the emulator backend. The following + versions are supported: + + + '1.2' : creates a TPM 1.2 + '2' : creates a TPM 2 + + NVRAM device diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3582cb5019..f11833075a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4130,6 +4130,18 @@ + + + + + +1.2 +2 + + + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 15dd490d17..79904789ee 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12657,7 +12657,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt, * or like this: * * - * + * * */ static virDomainTPMDefPtr @@ -12670,6 +12670,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, char *path = NULL; char *model = NULL; char *backend = NULL; +char *version = NULL; virDomainTPMDefPtr def; xmlNodePtr save = ctxt->node; xmlNodePtr *backends = NULL; @@ -12716,6 +12717,20 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } +version = virXMLPropString(backends[0], "version"); +if (!version || STREQ(version, "1.2")) { +def->version = VIR_DOMAIN_TPM_VERSION_1_2; +/* only TIS available for emulator */ +if (def->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) +def->model = VIR_DOMAIN_TPM_MODEL_TIS; This will silently overwrite an already defined model - is this intended? Also this seems like some kind of validation logic - not sure if virDomainTPMDefParseXML is the right place for this. TPM 1.2 can typically only be used with the TIS. The CRB interface works only with TPM 2. So, yes, it's intentional. Stefan +} else if (STREQ(version, "2")) { +def->version = VIR_DOMAIN_TPM_VERSION_2; […snip] Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list - +
Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2
On Tue, May 22, 2018 at 10:44 PM +0200, 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: > > > > > > 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 > Reviewed-by: John Ferlan > --- > 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 > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index 08a57bd751..043c8da56f 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -7719,7 +7719,7 @@ qemu-kvm -net nic,model=? /dev/null >... >> > @@ -7769,6 +7769,19 @@ qemu-kvm -net nic,model=? /dev/null > > > > + version > + > + > + The version attribute indicates the version > + of the TPM. By default a TPM 1.2 is created. This attribute > + only works with the emulator backend. The following > + versions are supported: > + > + > + '1.2' : creates a TPM 1.2 > + '2' : creates a TPM 2 > + > + > > > NVRAM device > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index 3582cb5019..f11833075a 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -4130,6 +4130,18 @@ > > > > + > + > + > + > + > +1.2 > +2 > + > + > + > + > + > > > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 15dd490d17..79904789ee 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -12657,7 +12657,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr > xmlopt, > * or like this: > * > * > - * > + * > * > */ > static virDomainTPMDefPtr > @@ -12670,6 +12670,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, > char *path = NULL; > char *model = NULL; > char *backend = NULL; > +char *version = NULL; > virDomainTPMDefPtr def; > xmlNodePtr save = ctxt->node; > xmlNodePtr *backends = NULL; > @@ -12716,6 +12717,20 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, > goto error; > } > > +version = virXMLPropString(backends[0], "version"); > +if (!version || STREQ(version, "1.2")) { > +def->version = VIR_DOMAIN_TPM_VERSION_1_2; > +/* only TIS available for emulator */ > +if (def->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) > +def->model = VIR_DOMAIN_TPM_MODEL_TIS; This will silently overwrite an already defined model - is this intended? Also this seems like some kind of validation logic - not sure if virDomainTPMDefParseXML is the right place for this. > +} else if (STREQ(version, "2")) { > +def->version = VIR_DOMAIN_TPM_VERSION_2; […snip] Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list> - >> + > >
Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2
On 05/24/2018 03:08 AM, Ján Tomko wrote: On Wed, May 23, 2018 at 02:33:08PM -0400, Stefan Berger wrote: On 05/23/2018 11:55 AM, Ján Tomko wrote: On Tue, May 22, 2018 at 04:44:51PM -0400, Stefan Berger wrote: @@ -24941,6 +24963,9 @@ virDomainTPMDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "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. What previous default? is introduced by this series. We should format the configurable attributes even when they are at their default values. I'll post a v7 with this and other changes. switch (def->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: virBufferAddLit(buf, ">\n"); 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. With QEMU, we actually need to know some capabilities upfront, because different versions have had different ways of requesting the same functionality. Giving nicer errors for unsupported features is just a bonus. With qemu-img, we only care about one way of representing the functionality and let it print an error if something's compiled out. Ok, then let me remove it. v7 will have that change. Not sure what to do about the existing Reviewed-by's. Intend to keep them. Stefan 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 : use the given port -f|--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'. Yes, but not worth introducing all the code and extra exec() for all the successful starts. Jano Stefan Jano -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2
On Wed, May 23, 2018 at 02:33:08PM -0400, Stefan Berger wrote: On 05/23/2018 11:55 AM, Ján Tomko wrote: On Tue, May 22, 2018 at 04:44:51PM -0400, Stefan Berger wrote: @@ -24941,6 +24963,9 @@ virDomainTPMDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "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. What previous default? is introduced by this series. We should format the configurable attributes even when they are at their default values. switch (def->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: virBufferAddLit(buf, ">\n"); 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. With QEMU, we actually need to know some capabilities upfront, because different versions have had different ways of requesting the same functionality. Giving nicer errors for unsupported features is just a bonus. With qemu-img, we only care about one way of representing the functionality and let it print an error if something's compiled out. 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 : use the given port -f|--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'. Yes, but not worth introducing all the code and extra exec() for all the successful starts. Jano Stefan Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2
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: 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 Reviewed-by: John Ferlan --- 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, "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 : use the given port -f|--fd : use the given socket file descriptor [...] I think a more controlled error message would be b
Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2
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: 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 Reviewed-by: John Ferlan --- 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, "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. 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. 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. Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2
This patch extends the TPM's device XML with TPM 2 support. This only works for the emulator type backend and looks as follows: 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 Reviewed-by: John Ferlan --- 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 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 08a57bd751..043c8da56f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7719,7 +7719,7 @@ qemu-kvm -net nic,model=? /dev/null ...@@ -7769,6 +7769,19 @@ qemu-kvm -net nic,model=? /dev/null + version + + + The version attribute indicates the version + of the TPM. By default a TPM 1.2 is created. This attribute + only works with the emulator backend. The following + versions are supported: + + + '1.2' : creates a TPM 1.2 + '2' : creates a TPM 2 + + NVRAM device diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3582cb5019..f11833075a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4130,6 +4130,18 @@ + + + + + +1.2 +2 + + + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 15dd490d17..79904789ee 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12657,7 +12657,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt, * or like this: * * - * + * * */ static virDomainTPMDefPtr @@ -12670,6 +12670,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, char *path = NULL; char *model = NULL; char *backend = NULL; +char *version = NULL; virDomainTPMDefPtr def; xmlNodePtr save = ctxt->node; xmlNodePtr *backends = NULL; @@ -12716,6 +12717,20 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } +version = virXMLPropString(backends[0], "version"); +if (!version || STREQ(version, "1.2")) { +def->version = VIR_DOMAIN_TPM_VERSION_1_2; +/* only TIS available for emulator */ +if (def->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) +def->model = VIR_DOMAIN_TPM_MODEL_TIS; +} else if (STREQ(version, "2")) { +def->version = VIR_DOMAIN_TPM_VERSION_2; +} else { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported TPM version '%s'"), + version); +} + switch (def->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: path = virXPathString("string(./backend/device/@path)", ctxt); @@ -12740,6 +12755,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(model); VIR_FREE(backend); VIR_FREE(backends); +VIR_FREE(version); ctxt->node = save; return def; @@ -21836,6 +21852,12 @@ virDomainTPMDefCheckABIStability(virDomainTPMDefPtr src, return false; } +if (src->version != dst->version) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Target TPM version doesn't match source")); +return false; +} + return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info); } @@ -24941,6 +24963,9 @@ virDomainTPMDefFormat(virBufferPtr buf, virBufferAsprintf( - +