[PATCH rfcv4 04/13] conf: expose TDX feature in domain capabilities

Zhenzhong Duan posted 13 patches 1 year, 8 months ago
There is a newer version of this series
[PATCH rfcv4 04/13] conf: expose TDX feature in domain capabilities
Posted by Zhenzhong Duan 1 year, 8 months ago
Extend qemu TDX capability to domain capabilities.

Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 docs/formatdomaincaps.rst       |  1 +
 src/conf/domain_capabilities.c  |  1 +
 src/conf/domain_capabilities.h  |  1 +
 src/conf/schemas/domaincaps.rng |  9 +++++++++
 src/qemu/qemu_capabilities.c    | 13 +++++++++++++
 5 files changed, 25 insertions(+)

diff --git a/docs/formatdomaincaps.rst b/docs/formatdomaincaps.rst
index 609a767189..36d56a433c 100644
--- a/docs/formatdomaincaps.rst
+++ b/docs/formatdomaincaps.rst
@@ -651,6 +651,7 @@ capabilities. All features occur as children of the main ``features`` element.
        <backingStoreInput supported='yes'/>
        <backup supported='yes'/>
        <async-teardown supported='yes'/>
+       <tdx supported='yes'/>
        <sev>
          <cbitpos>47</cbitpos>
          <reduced-phys-bits>1</reduced-phys-bits>
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 68eb3c9797..8bfe19d774 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -42,6 +42,7 @@ VIR_ENUM_IMPL(virDomainCapsFeature,
               "backup",
               "async-teardown",
               "s390-pv",
+              "tdx",
 );
 
 static virClass *virDomainCapsClass;
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index fadc30cdd7..690e2a1f03 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -250,6 +250,7 @@ typedef enum {
     VIR_DOMAIN_CAPS_FEATURE_BACKUP,
     VIR_DOMAIN_CAPS_FEATURE_ASYNC_TEARDOWN,
     VIR_DOMAIN_CAPS_FEATURE_S390_PV,
+    VIR_DOMAIN_CAPS_FEATURE_TDX,
 
     VIR_DOMAIN_CAPS_FEATURE_LAST
 } virDomainCapsFeature;
diff --git a/src/conf/schemas/domaincaps.rng b/src/conf/schemas/domaincaps.rng
index e7aa4a1066..a5522b1e67 100644
--- a/src/conf/schemas/domaincaps.rng
+++ b/src/conf/schemas/domaincaps.rng
@@ -308,6 +308,9 @@
       <optional>
         <ref name="s390-pv"/>
       </optional>
+      <optional>
+        <ref name="tdx"/>
+      </optional>
       <optional>
         <ref name="sev"/>
       </optional>
@@ -363,6 +366,12 @@
     </element>
   </define>
 
+  <define name="tdx">
+    <element name="tdx">
+      <ref name="supported"/>
+    </element>
+  </define>
+
   <define name="sev">
     <element name="sev">
       <ref name="supported"/>
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 33d2b5f392..53a773b853 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -6675,6 +6675,18 @@ virQEMUCapsFillDomainFeatureHypervCaps(virQEMUCaps *qemuCaps,
 }
 
 
+static void
+virQEMUCapsFillDomainFeatureTDXCaps(virQEMUCaps *qemuCaps,
+                                    virDomainCaps *domCaps)
+{
+    if (domCaps->arch == VIR_ARCH_X86_64 &&
+        domCaps->virttype == VIR_DOMAIN_VIRT_KVM &&
+        virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST) &&
+        virQEMUCapsGetKVMSupportsSecureGuest(qemuCaps))
+            domCaps->features[VIR_DOMAIN_CAPS_FEATURE_TDX] = VIR_TRISTATE_BOOL_YES;
+}
+
+
 int
 virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
                           virArch hostarch,
@@ -6734,6 +6746,7 @@ virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
     virQEMUCapsFillDomainFeatureSGXCaps(qemuCaps, domCaps);
     virQEMUCapsFillDomainFeatureHypervCaps(qemuCaps, domCaps);
     virQEMUCapsFillDomainDeviceCryptoCaps(qemuCaps, crypto);
+    virQEMUCapsFillDomainFeatureTDXCaps(qemuCaps, domCaps);
 
     return 0;
 }
-- 
2.34.1
Re: [PATCH rfcv4 04/13] conf: expose TDX feature in domain capabilities
Posted by Jim Fehlig via Devel 10 months, 2 weeks ago
On 5/24/24 00:21, Zhenzhong Duan wrote:
> Extend qemu TDX capability to domain capabilities.
> 
> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   docs/formatdomaincaps.rst       |  1 +
>   src/conf/domain_capabilities.c  |  1 +
>   src/conf/domain_capabilities.h  |  1 +
>   src/conf/schemas/domaincaps.rng |  9 +++++++++
>   src/qemu/qemu_capabilities.c    | 13 +++++++++++++
>   5 files changed, 25 insertions(+)
> 
> diff --git a/docs/formatdomaincaps.rst b/docs/formatdomaincaps.rst
> index 609a767189..36d56a433c 100644
> --- a/docs/formatdomaincaps.rst
> +++ b/docs/formatdomaincaps.rst
> @@ -651,6 +651,7 @@ capabilities. All features occur as children of the main ``features`` element.
>          <backingStoreInput supported='yes'/>
>          <backup supported='yes'/>
>          <async-teardown supported='yes'/>
> +       <tdx supported='yes'/>
>          <sev>
>            <cbitpos>47</cbitpos>
>            <reduced-phys-bits>1</reduced-phys-bits>
> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> index 68eb3c9797..8bfe19d774 100644
> --- a/src/conf/domain_capabilities.c
> +++ b/src/conf/domain_capabilities.c
> @@ -42,6 +42,7 @@ VIR_ENUM_IMPL(virDomainCapsFeature,
>                 "backup",
>                 "async-teardown",
>                 "s390-pv",
> +              "tdx",
>   );
>   
>   static virClass *virDomainCapsClass;
> diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
> index fadc30cdd7..690e2a1f03 100644
> --- a/src/conf/domain_capabilities.h
> +++ b/src/conf/domain_capabilities.h
> @@ -250,6 +250,7 @@ typedef enum {
>       VIR_DOMAIN_CAPS_FEATURE_BACKUP,
>       VIR_DOMAIN_CAPS_FEATURE_ASYNC_TEARDOWN,
>       VIR_DOMAIN_CAPS_FEATURE_S390_PV,
> +    VIR_DOMAIN_CAPS_FEATURE_TDX,
>   
>       VIR_DOMAIN_CAPS_FEATURE_LAST
>   } virDomainCapsFeature;
> diff --git a/src/conf/schemas/domaincaps.rng b/src/conf/schemas/domaincaps.rng
> index e7aa4a1066..a5522b1e67 100644
> --- a/src/conf/schemas/domaincaps.rng
> +++ b/src/conf/schemas/domaincaps.rng
> @@ -308,6 +308,9 @@
>         <optional>
>           <ref name="s390-pv"/>
>         </optional>
> +      <optional>
> +        <ref name="tdx"/>
> +      </optional>
>         <optional>
>           <ref name="sev"/>
>         </optional>
> @@ -363,6 +366,12 @@
>       </element>
>     </define>
>   
> +  <define name="tdx">
> +    <element name="tdx">
> +      <ref name="supported"/>
> +    </element>
> +  </define>
> +
>     <define name="sev">
>       <element name="sev">
>         <ref name="supported"/>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 33d2b5f392..53a773b853 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -6675,6 +6675,18 @@ virQEMUCapsFillDomainFeatureHypervCaps(virQEMUCaps *qemuCaps,
>   }
>   
>   
> +static void
> +virQEMUCapsFillDomainFeatureTDXCaps(virQEMUCaps *qemuCaps,
> +                                    virDomainCaps *domCaps)
> +{
> +    if (domCaps->arch == VIR_ARCH_X86_64 &&
> +        domCaps->virttype == VIR_DOMAIN_VIRT_KVM &&
> +        virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST) &&
> +        virQEMUCapsGetKVMSupportsSecureGuest(qemuCaps))
> +            domCaps->features[VIR_DOMAIN_CAPS_FEATURE_TDX] = VIR_TRISTATE_BOOL_YES;
> +}
> +
> +
>   int
>   virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
>                             virArch hostarch,
> @@ -6734,6 +6746,7 @@ virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
>       virQEMUCapsFillDomainFeatureSGXCaps(qemuCaps, domCaps);
>       virQEMUCapsFillDomainFeatureHypervCaps(qemuCaps, domCaps);
>       virQEMUCapsFillDomainDeviceCryptoCaps(qemuCaps, crypto);
> +    virQEMUCapsFillDomainFeatureTDXCaps(qemuCaps, domCaps);

I'm not sure how you'll want to handle this, but 
virQEMUCapsFillDomainLaunchSecurity (introduced by commit 66df7992d8e) must also 
account for TDX. E.g.

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index aeccf877ea..37011d122b 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -6752,6 +6752,8 @@ virQEMUCapsFillDomainLaunchSecurity(virQEMUCaps *qemuCaps,
      if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_S390_PV_GUEST) &&
          virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT))
          VIR_DOMAIN_CAPS_ENUM_SET(launchSecurity->sectype, 
VIR_DOMAIN_LAUNCH_SECURITY_PV);
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST))
+        VIR_DOMAIN_CAPS_ENUM_SET(launchSecurity->sectype, 
VIR_DOMAIN_LAUNCH_SECURITY_TDX);

      if (launchSecurity->sectype.values == 0) {
          launchSecurity->supported = VIR_TRISTATE_BOOL_NO;

But VIR_DOMAIN_LAUNCH_SECURITY_TDX is introduced by the next patch. Maybe swap 
the order of the patches?

Regards,
Jim
RE: [PATCH rfcv4 04/13] conf: expose TDX feature in domain capabilities
Posted by Duan, Zhenzhong 10 months, 1 week ago

>-----Original Message-----
>From: Jim Fehlig <jfehlig@suse.com>
>Subject: Re: [PATCH rfcv4 04/13] conf: expose TDX feature in domain capabilities
>
>On 5/24/24 00:21, Zhenzhong Duan wrote:
>> Extend qemu TDX capability to domain capabilities.
>>
>> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   docs/formatdomaincaps.rst       |  1 +
>>   src/conf/domain_capabilities.c  |  1 +
>>   src/conf/domain_capabilities.h  |  1 +
>>   src/conf/schemas/domaincaps.rng |  9 +++++++++
>>   src/qemu/qemu_capabilities.c    | 13 +++++++++++++
>>   5 files changed, 25 insertions(+)
>>
>> diff --git a/docs/formatdomaincaps.rst b/docs/formatdomaincaps.rst
>> index 609a767189..36d56a433c 100644
>> --- a/docs/formatdomaincaps.rst
>> +++ b/docs/formatdomaincaps.rst
>> @@ -651,6 +651,7 @@ capabilities. All features occur as children of the main
>``features`` element.
>>          <backingStoreInput supported='yes'/>
>>          <backup supported='yes'/>
>>          <async-teardown supported='yes'/>
>> +       <tdx supported='yes'/>
>>          <sev>
>>            <cbitpos>47</cbitpos>
>>            <reduced-phys-bits>1</reduced-phys-bits>
>> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
>> index 68eb3c9797..8bfe19d774 100644
>> --- a/src/conf/domain_capabilities.c
>> +++ b/src/conf/domain_capabilities.c
>> @@ -42,6 +42,7 @@ VIR_ENUM_IMPL(virDomainCapsFeature,
>>                 "backup",
>>                 "async-teardown",
>>                 "s390-pv",
>> +              "tdx",
>>   );
>>
>>   static virClass *virDomainCapsClass;
>> diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
>> index fadc30cdd7..690e2a1f03 100644
>> --- a/src/conf/domain_capabilities.h
>> +++ b/src/conf/domain_capabilities.h
>> @@ -250,6 +250,7 @@ typedef enum {
>>       VIR_DOMAIN_CAPS_FEATURE_BACKUP,
>>       VIR_DOMAIN_CAPS_FEATURE_ASYNC_TEARDOWN,
>>       VIR_DOMAIN_CAPS_FEATURE_S390_PV,
>> +    VIR_DOMAIN_CAPS_FEATURE_TDX,
>>
>>       VIR_DOMAIN_CAPS_FEATURE_LAST
>>   } virDomainCapsFeature;
>> diff --git a/src/conf/schemas/domaincaps.rng
>b/src/conf/schemas/domaincaps.rng
>> index e7aa4a1066..a5522b1e67 100644
>> --- a/src/conf/schemas/domaincaps.rng
>> +++ b/src/conf/schemas/domaincaps.rng
>> @@ -308,6 +308,9 @@
>>         <optional>
>>           <ref name="s390-pv"/>
>>         </optional>
>> +      <optional>
>> +        <ref name="tdx"/>
>> +      </optional>
>>         <optional>
>>           <ref name="sev"/>
>>         </optional>
>> @@ -363,6 +366,12 @@
>>       </element>
>>     </define>
>>
>> +  <define name="tdx">
>> +    <element name="tdx">
>> +      <ref name="supported"/>
>> +    </element>
>> +  </define>
>> +
>>     <define name="sev">
>>       <element name="sev">
>>         <ref name="supported"/>
>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>> index 33d2b5f392..53a773b853 100644
>> --- a/src/qemu/qemu_capabilities.c
>> +++ b/src/qemu/qemu_capabilities.c
>> @@ -6675,6 +6675,18 @@
>virQEMUCapsFillDomainFeatureHypervCaps(virQEMUCaps *qemuCaps,
>>   }
>>
>>
>> +static void
>> +virQEMUCapsFillDomainFeatureTDXCaps(virQEMUCaps *qemuCaps,
>> +                                    virDomainCaps *domCaps)
>> +{
>> +    if (domCaps->arch == VIR_ARCH_X86_64 &&
>> +        domCaps->virttype == VIR_DOMAIN_VIRT_KVM &&
>> +        virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST) &&
>> +        virQEMUCapsGetKVMSupportsSecureGuest(qemuCaps))
>> +            domCaps->features[VIR_DOMAIN_CAPS_FEATURE_TDX] =
>VIR_TRISTATE_BOOL_YES;
>> +}
>> +
>> +
>>   int
>>   virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
>>                             virArch hostarch,
>> @@ -6734,6 +6746,7 @@ virQEMUCapsFillDomainCaps(virQEMUCaps
>*qemuCaps,
>>       virQEMUCapsFillDomainFeatureSGXCaps(qemuCaps, domCaps);
>>       virQEMUCapsFillDomainFeatureHypervCaps(qemuCaps, domCaps);
>>       virQEMUCapsFillDomainDeviceCryptoCaps(qemuCaps, crypto);
>> +    virQEMUCapsFillDomainFeatureTDXCaps(qemuCaps, domCaps);
>
>I'm not sure how you'll want to handle this, but
>virQEMUCapsFillDomainLaunchSecurity (introduced by commit 66df7992d8e)
>must also
>account for TDX. E.g.

Yes, it will be in next version, see https://github.com/intel/libvirt-tdx/commit/14ec3e14e07f5cee6f86e9086f0bdee227532ad5

>
>diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>index aeccf877ea..37011d122b 100644
>--- a/src/qemu/qemu_capabilities.c
>+++ b/src/qemu/qemu_capabilities.c
>@@ -6752,6 +6752,8 @@ virQEMUCapsFillDomainLaunchSecurity(virQEMUCaps
>*qemuCaps,
>      if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_S390_PV_GUEST) &&
>          virQEMUCapsGet(qemuCaps,
>QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT))
>          VIR_DOMAIN_CAPS_ENUM_SET(launchSecurity->sectype,
>VIR_DOMAIN_LAUNCH_SECURITY_PV);
>+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST))
>+        VIR_DOMAIN_CAPS_ENUM_SET(launchSecurity->sectype,
>VIR_DOMAIN_LAUNCH_SECURITY_TDX);
>
>      if (launchSecurity->sectype.values == 0) {
>          launchSecurity->supported = VIR_TRISTATE_BOOL_NO;
>
>But VIR_DOMAIN_LAUNCH_SECURITY_TDX is introduced by the next patch.
>Maybe swap
>the order of the patches?

I put it after patch 'qemu: Add command line and validation for TDX type' as validation check take effect after it and it acts as start point supporting tdx.
See https://github.com/intel/libvirt-tdx/commits/tdx_for_upstream_v1.wip/

Thanks
Zhenzhong
Re: [PATCH rfcv4 04/13] conf: expose TDX feature in domain capabilities
Posted by Daniel P. Berrangé 1 year, 8 months ago
On Fri, May 24, 2024 at 02:21:19PM +0800, Zhenzhong Duan wrote:
> Extend qemu TDX capability to domain capabilities.
> 
> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  docs/formatdomaincaps.rst       |  1 +
>  src/conf/domain_capabilities.c  |  1 +
>  src/conf/domain_capabilities.h  |  1 +
>  src/conf/schemas/domaincaps.rng |  9 +++++++++
>  src/qemu/qemu_capabilities.c    | 13 +++++++++++++
>  5 files changed, 25 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|