[libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2

[Stefan Berger]

On 05/24/2018 08:17 AM, Marc Hartmayer wrote:
> On Tue, May 22, 2018 at 10:44 PM +0200, Stefan Berger <stefanb at linux.vnet.ibm.com> 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 at linux.vnet.ibm.com>
>> Reviewed-by: John Ferlan <jferlan at 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
>>
>> 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
>>     ...
>>     <devices>
>>       <tpm model='tpm-tis'>
>> -      <backend type='emulator'>
>> +      <backend type='emulator' version='2'>
>>         </backend>
>>       </tpm>
>>     </devices>
>> @@ -7769,6 +7769,19 @@ qemu-kvm -net nic,model=? /dev/null
>>             </dd>
>>           </dl>
>>         </dd>
>> +      <dt><code>version</code></dt>
>> +      <dd>
>> +        <p>
>> +          The <code>version</code> attribute indicates the version
>> +          of the TPM. By default a TPM 1.2 is created. This attribute
>> +          only works with the <code>emulator</code> backend. The following
>> +          versions are supported:
>> +        </p>
>> +        <ul>
>> +          <li>'1.2' : creates a TPM 1.2</li>
>> +          <li>'2' :  creates a TPM 2</li>
>> +        </ul>
>> +      </dd>
>>       </dl>
>>
>>       <h4><a id="elementsNVRAM">NVRAM device</a></h4>
>> 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 @@
>>             </attribute>
>>           </group>
>>         </choice>
>> +      <choice>
>> +        <group>
>> +          <optional>
>> +            <attribute name="version">
>> +              <choice>
>> +                <value>1.2</value>
>> +                <value>2</value>
>> +              </choice>
>> +           </attribute>
>> +          </optional>
>> +        </group>
>> +      </choice>
>>       </element>
>>     </define>
>>
>> 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:
>>    *
>>    * <tpm model='tpm-tis'>
>> - *   <backend type='emulator'/>
>> + *   <backend type='emulator' version='2'/>
>>    * </tpm>
>>    */
>>   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