[PATCH 1/5] conf: Add support for tpm-spapr to domain XML

Stefan Berger posted 5 patches 6 years ago
There is a newer version of this series
[PATCH 1/5] conf: Add support for tpm-spapr to domain XML
Posted by Stefan Berger 6 years ago
From: Stefan Berger <stefanb@linux.vnet.ibm.com>

This patch adds support for the tpm-spapr device model for ppc64. The XML for
this type of TPM looks as follows:

 <tpm model='tpm-spapr'>
   <backend type='emulator'/>
 </tpm>

Extend the documentation.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 docs/formatdomain.html.in     |  4 +++-
 docs/schemas/domaincommon.rng |  4 ++++
 src/conf/domain_conf.c        | 24 +++++++++++++++++-------
 src/conf/domain_conf.h        |  1 +
 src/qemu/qemu_domain.c        |  6 ++++--
 5 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 60a103d7c6..5a9835fbfe 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -8508,7 +8508,9 @@ qemu-kvm -net nic,model=? /dev/null
         <p>
           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.
+          <code>tpm-tis</code> will automatically be chosen for non-ppc64
+          architectures. For ppc64/pseries guests, <code>tpm-spapr</code>
+          is the default.
           <span class="since">Since 4.4.0</span>, another available choice
           is the <code>tpm-crb</code>, which should only be used when the
           backend device is a TPM 2.0.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index ea237a05e5..9577d26c2a 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4383,6 +4383,7 @@
           <choice>
             <value>tpm-tis</value>
             <value>tpm-crb</value>
+            <value>tpm-spapr</value>
           </choice>
         </attribute>
       </optional>
@@ -4390,6 +4391,9 @@
       <optional>
         <ref name="alias"/>
       </optional>
+      <optional>
+        <ref name="address"/>
+      </optional>
     </element>
   </define>
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9b60db7ecd..a55be400fc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1127,6 +1127,7 @@ VIR_ENUM_IMPL(virDomainTPMModel,
               VIR_DOMAIN_TPM_MODEL_LAST,
               "tpm-tis",
               "tpm-crb",
+              "tpm-spapr",
 );
 
 VIR_ENUM_IMPL(virDomainTPMBackend,
@@ -13242,7 +13243,8 @@ static virDomainTPMDefPtr
 virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
                         xmlNodePtr node,
                         xmlXPathContextPtr ctxt,
-                        unsigned int flags)
+                        unsigned int flags,
+                        virArch arch)
 {
     virDomainTPMDefPtr def;
     VIR_XPATH_NODE_AUTORESTORE(ctxt);
@@ -13258,11 +13260,17 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
         return NULL;
 
     model = virXMLPropString(node, "model");
-    if (model != NULL &&
-        (def->model = virDomainTPMModelTypeFromString(model)) < 0) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+    if (model != NULL) {
+        if ((def->model = virDomainTPMModelTypeFromString(model)) < 0) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("Unknown TPM frontend model '%s'"), model);
-        goto error;
+            goto error;
+        }
+    } else {
+        if (ARCH_IS_PPC64(arch))
+            def->model = VIR_DOMAIN_TPM_MODEL_SPAPR;
+        else
+            def->model = VIR_DOMAIN_TPM_MODEL_TIS;
     }
 
     ctxt->node = node;
@@ -16639,7 +16647,8 @@ virDomainDeviceDefParse(const char *xmlStr,
             return NULL;
         break;
     case VIR_DOMAIN_DEVICE_TPM:
-        if (!(dev->data.tpm = virDomainTPMDefParseXML(xmlopt, node, ctxt, flags)))
+        if (!(dev->data.tpm = virDomainTPMDefParseXML(xmlopt, node, ctxt, flags,
+                                                      def->os.arch)))
             return NULL;
         break;
     case VIR_DOMAIN_DEVICE_PANIC:
@@ -21464,7 +21473,8 @@ virDomainDefParseXML(xmlDocPtr xml,
     }
 
     if (n > 0) {
-        if (!(def->tpm = virDomainTPMDefParseXML(xmlopt, nodes[0], ctxt, flags)))
+        if (!(def->tpm = virDomainTPMDefParseXML(xmlopt, nodes[0], ctxt, flags,
+                                                 def->os.arch)))
             goto error;
     }
     VIR_FREE(nodes);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e144f3aad3..19732fcfc9 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1254,6 +1254,7 @@ struct _virDomainHubDef {
 typedef enum {
     VIR_DOMAIN_TPM_MODEL_TIS,
     VIR_DOMAIN_TPM_MODEL_CRB,
+    VIR_DOMAIN_TPM_MODEL_SPAPR,
 
     VIR_DOMAIN_TPM_MODEL_LAST
 } virDomainTPMModel;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d3045b4bcd..ace611909d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7756,9 +7756,10 @@ qemuDomainDeviceDefValidateTPM(virDomainTPMDef *tpm,
 
     switch (tpm->version) {
     case VIR_DOMAIN_TPM_VERSION_1_2:
-        /* only TIS available for emulator */
+        /* only TIS available for emulator (non-ppc64 case) */
         if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
-            tpm->model != VIR_DOMAIN_TPM_MODEL_TIS) {
+            tpm->model != VIR_DOMAIN_TPM_MODEL_TIS &&
+            !ARCH_IS_PPC64(def->os.arch)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("Unsupported interface %s for TPM 1.2"),
                            virDomainTPMModelTypeToString(tpm->model));
@@ -7793,6 +7794,7 @@ qemuDomainDeviceDefValidateTPM(virDomainTPMDef *tpm,
     case VIR_DOMAIN_TPM_MODEL_CRB:
         flag = QEMU_CAPS_DEVICE_TPM_CRB;
         break;
+    case VIR_DOMAIN_TPM_MODEL_SPAPR:
     case VIR_DOMAIN_TPM_MODEL_LAST:
     default:
         virReportEnumRangeError(virDomainTPMModel, tpm->model);
-- 
2.17.1


Re: [PATCH 1/5] conf: Add support for tpm-spapr to domain XML
Posted by Ján Tomko 6 years ago
On Fri, Jan 31, 2020 at 04:17:39PM -0500, Stefan Berger wrote:
>From: Stefan Berger <stefanb@linux.vnet.ibm.com>
>
>This patch adds support for the tpm-spapr device model for ppc64. The XML for
>this type of TPM looks as follows:
>
> <tpm model='tpm-spapr'>
>   <backend type='emulator'/>
> </tpm>
>
>Extend the documentation.
>
>Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>---
> docs/formatdomain.html.in     |  4 +++-
> docs/schemas/domaincommon.rng |  4 ++++
> src/conf/domain_conf.c        | 24 +++++++++++++++++-------
> src/conf/domain_conf.h        |  1 +
> src/qemu/qemu_domain.c        |  6 ++++--
> 5 files changed, 29 insertions(+), 10 deletions(-)
>
>diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>index 60a103d7c6..5a9835fbfe 100644
>--- a/docs/formatdomain.html.in
>+++ b/docs/formatdomain.html.in
>@@ -8508,7 +8508,9 @@ qemu-kvm -net nic,model=? /dev/null
>         <p>
>           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.
>+          <code>tpm-tis</code> will automatically be chosen for non-ppc64
>+          architectures. For ppc64/pseries guests, <code>tpm-spapr</code>

<span class="since">Since 6.1.0</span>, ...

>+          is the default.
>           <span class="since">Since 4.4.0</span>, another available choice
>           is the <code>tpm-crb</code>, which should only be used when the
>           backend device is a TPM 2.0.

>@@ -13242,7 +13243,8 @@ static virDomainTPMDefPtr
> virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
>                         xmlNodePtr node,
>                         xmlXPathContextPtr ctxt,
>-                        unsigned int flags)
>+                        unsigned int flags,
>+                        virArch arch)
> {
>     virDomainTPMDefPtr def;
>     VIR_XPATH_NODE_AUTORESTORE(ctxt);
>@@ -13258,11 +13260,17 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
>         return NULL;
>
>     model = virXMLPropString(node, "model");
>-    if (model != NULL &&
>-        (def->model = virDomainTPMModelTypeFromString(model)) < 0) {
>-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+    if (model != NULL) {
>+        if ((def->model = virDomainTPMModelTypeFromString(model)) < 0) {
>+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                        _("Unknown TPM frontend model '%s'"), model);
>-        goto error;
>+            goto error;
>+        }
>+    } else {
>+        if (ARCH_IS_PPC64(arch))
>+            def->model = VIR_DOMAIN_TPM_MODEL_SPAPR;
>+        else
>+            def->model = VIR_DOMAIN_TPM_MODEL_TIS;
>     }

Default models should be set in qemuDomainDeviceDefPostParse.
(that might require a new VIR_DOMAIN_TPM_MODEL_DEFAULT, since so far we
  relied on VIR_DOMAIN_TPM_MODEL_TIS having the value of 0)
The parser should just translate XML into libvirt structs.

>     ctxt->node = node;
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index d3045b4bcd..ace611909d 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -7756,9 +7756,10 @@ qemuDomainDeviceDefValidateTPM(virDomainTPMDef *tpm,
>
>     switch (tpm->version) {
>     case VIR_DOMAIN_TPM_VERSION_1_2:
>-        /* only TIS available for emulator */
>+        /* only TIS available for emulator (non-ppc64 case) */
>         if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
>-            tpm->model != VIR_DOMAIN_TPM_MODEL_TIS) {
>+            tpm->model != VIR_DOMAIN_TPM_MODEL_TIS &&

>+            !ARCH_IS_PPC64(def->os.arch)) {

   tpm->model != VIR_DOMAIN_TPM_MODEL_SPAPR

no need to mention the architecture here.

Alternatively, are the comment and the condition just trying to say
1.2 + tpm-crb + emulator do not mix?

That might be shorter to write.

Jano

>             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                            _("Unsupported interface %s for TPM 1.2"),
>                            virDomainTPMModelTypeToString(tpm->model));
>@@ -7793,6 +7794,7 @@ qemuDomainDeviceDefValidateTPM(virDomainTPMDef *tpm,
>     case VIR_DOMAIN_TPM_MODEL_CRB:
>         flag = QEMU_CAPS_DEVICE_TPM_CRB;
>         break;
>+    case VIR_DOMAIN_TPM_MODEL_SPAPR:
>     case VIR_DOMAIN_TPM_MODEL_LAST:
>     default:
>         virReportEnumRangeError(virDomainTPMModel, tpm->model);
>-- 
>2.17.1
>
>
Re: [PATCH 1/5] conf: Add support for tpm-spapr to domain XML
Posted by Stefan Berger 6 years ago
On 2/2/20 9:12 AM, Ján Tomko wrote:
> On Fri, Jan 31, 2020 at 04:17:39PM -0500, Stefan Berger wrote:
>
>> @@ -13242,7 +13243,8 @@ static virDomainTPMDefPtr
>> virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
>>                         xmlNodePtr node,
>>                         xmlXPathContextPtr ctxt,
>> -                        unsigned int flags)
>> +                        unsigned int flags,
>> +                        virArch arch)
>> {
>>     virDomainTPMDefPtr def;
>>     VIR_XPATH_NODE_AUTORESTORE(ctxt);
>> @@ -13258,11 +13260,17 @@ 
>> virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
>>         return NULL;
>>
>>     model = virXMLPropString(node, "model");
>> -    if (model != NULL &&
>> -        (def->model = virDomainTPMModelTypeFromString(model)) < 0) {
>> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +    if (model != NULL) {
>> +        if ((def->model = virDomainTPMModelTypeFromString(model)) < 
>> 0) {
>> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>                        _("Unknown TPM frontend model '%s'"), model);
>> -        goto error;
>> +            goto error;
>> +        }
>> +    } else {
>> +        if (ARCH_IS_PPC64(arch))
>> +            def->model = VIR_DOMAIN_TPM_MODEL_SPAPR;
>> +        else
>> +            def->model = VIR_DOMAIN_TPM_MODEL_TIS;
>>     }
>
> Default models should be set in qemuDomainDeviceDefPostParse.
> (that might require a new VIR_DOMAIN_TPM_MODEL_DEFAULT, since so far we
>  relied on VIR_DOMAIN_TPM_MODEL_TIS having the value of 0)
> The parser should just translate XML into libvirt structs.


Fixed in v2.


>
>>     ctxt->node = node;
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index d3045b4bcd..ace611909d 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -7756,9 +7756,10 @@ qemuDomainDeviceDefValidateTPM(virDomainTPMDef 
>> *tpm,
>>
>>     switch (tpm->version) {
>>     case VIR_DOMAIN_TPM_VERSION_1_2:
>> -        /* only TIS available for emulator */
>> +        /* only TIS available for emulator (non-ppc64 case) */
>>         if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
>> -            tpm->model != VIR_DOMAIN_TPM_MODEL_TIS) {
>> +            tpm->model != VIR_DOMAIN_TPM_MODEL_TIS &&
>
>> + !ARCH_IS_PPC64(def->os.arch)) {
>
>   tpm->model != VIR_DOMAIN_TPM_MODEL_SPAPR
>
> no need to mention the architecture here.
>
> Alternatively, are the comment and the condition just trying to say
> 1.2 + tpm-crb + emulator do not mix?


Right, it's TPM 1.2 + tpm-crb that do not work. TPM 1.2 + tpm-tis or TPM 
1.2 + tpm-spapr work fine. Will repost soon with one additional patch 
added to the front that introduces the default model that immediately 
gets adjusted to TIS (as previous) on non-ppc64 and tpm-spapr for ppc64 
IF the user didn't provide a model.


    Stefam


>
> That might be shorter to write.
>
> Jano
>
>> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>                            _("Unsupported interface %s for TPM 1.2"),
>> virDomainTPMModelTypeToString(tpm->model));
>> @@ -7793,6 +7794,7 @@ qemuDomainDeviceDefValidateTPM(virDomainTPMDef 
>> *tpm,
>>     case VIR_DOMAIN_TPM_MODEL_CRB:
>>         flag = QEMU_CAPS_DEVICE_TPM_CRB;
>>         break;
>> +    case VIR_DOMAIN_TPM_MODEL_SPAPR:
>>     case VIR_DOMAIN_TPM_MODEL_LAST:
>>     default:
>>         virReportEnumRangeError(virDomainTPMModel, tpm->model);
>> -- 
>> 2.17.1
>>
>>