1. Patchew
  2. Libvirt
  3. [PATCH rfcv3 00/11] LIBVIRT: X86: TDX support
  4. View patch

[PATCH rfcv3 03/11] conf: expose TDX feature in domain capabilities

Zhenzhong Duan posted 11 patches 2 years, 2 months ago
There is a newer version of this series
[PATCH rfcv3 03/11] conf: expose TDX feature in domain capabilities
Posted by Zhenzhong Duan 2 years, 2 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    | 15 +++++++++++++++
 5 files changed, 27 insertions(+)

diff --git a/docs/formatdomaincaps.rst b/docs/formatdomaincaps.rst
index ef752a0f3a..3acc9a12b4 100644
--- a/docs/formatdomaincaps.rst
+++ b/docs/formatdomaincaps.rst
@@ -669,6 +669,7 @@ capabilities. All features occur as children of the main ``features`` element.
            <value>vapic</value>
          </enum>
        </hyperv>
+       <tdx supported='yes'/>
      </features>
    </domainCapabilities>
 
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index f6e09dc584..0f9ddb1e48 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 01bcfa2e39..cc44cf2363 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 8764df5e9d..0b4988256f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -6657,6 +6657,20 @@ 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) &&
+        virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) &&
+        (STREQ(domCaps->machine, "q35") || STRPREFIX(domCaps->machine, "pc-q35-")))
+            domCaps->features[VIR_DOMAIN_CAPS_FEATURE_TDX] = VIR_TRISTATE_BOOL_YES;
+}
+
+
 int
 virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
                           virArch hostarch,
@@ -6716,6 +6730,7 @@ virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
     virQEMUCapsFillDomainFeatureSGXCaps(qemuCaps, domCaps);
     virQEMUCapsFillDomainFeatureHypervCaps(qemuCaps, domCaps);
     virQEMUCapsFillDomainDeviceCryptoCaps(qemuCaps, crypto);
+    virQEMUCapsFillDomainFeatureTDXCaps(qemuCaps, domCaps);
 
     return 0;
 }
-- 
2.34.1
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH rfcv3 03/11] conf: expose TDX feature in domain capabilities
Posted by Daniel P. Berrangé 2 years, 1 month ago 
On Mon, Nov 27, 2023 at 04:55:13PM +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    | 15 +++++++++++++++
>  5 files changed, 27 insertions(+)
> 
> diff --git a/docs/formatdomaincaps.rst b/docs/formatdomaincaps.rst
> index ef752a0f3a..3acc9a12b4 100644
> --- a/docs/formatdomaincaps.rst
> +++ b/docs/formatdomaincaps.rst
> @@ -669,6 +669,7 @@ capabilities. All features occur as children of the main ``features`` element.
>             <value>vapic</value>
>           </enum>
>         </hyperv>
> +       <tdx supported='yes'/>
>       </features>
>     </domainCapabilities>
>  
> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> index f6e09dc584..0f9ddb1e48 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 01bcfa2e39..cc44cf2363 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 8764df5e9d..0b4988256f 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -6657,6 +6657,20 @@ 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) &&
> +        virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) &&

Checking QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT is overkill, as we
can assume that is implied to exist by virtue of QEMU_CAPS_TDX_GUEST
existing.

> +        (STREQ(domCaps->machine, "q35") || STRPREFIX(domCaps->machine, "pc-q35-")))

If QEMU has limited its support for TDX to just q35, then it would be
much better if QEMU patches for TDX provided a way to detect this via
QMP, so we don't need to do these string comparisons.

> +            domCaps->features[VIR_DOMAIN_CAPS_FEATURE_TDX] = VIR_TRISTATE_BOOL_YES;
> +}
> +
> +
>  int
>  virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
>                            virArch hostarch,
> @@ -6716,6 +6730,7 @@ virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
>      virQEMUCapsFillDomainFeatureSGXCaps(qemuCaps, domCaps);
>      virQEMUCapsFillDomainFeatureHypervCaps(qemuCaps, domCaps);
>      virQEMUCapsFillDomainDeviceCryptoCaps(qemuCaps, crypto);
> +    virQEMUCapsFillDomainFeatureTDXCaps(qemuCaps, domCaps);
>  
>      return 0;
>  }
> -- 
> 2.34.1
> 

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 :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
RE: [PATCH rfcv3 03/11] conf: expose TDX feature in domain capabilities
Posted by Duan, Zhenzhong 2 years, 1 month ago 

>-----Original Message-----
>From: Daniel P. Berrangé <berrange@redhat.com>
>Subject: Re: [PATCH rfcv3 03/11] conf: expose TDX feature in domain
>capabilities
>
>On Mon, Nov 27, 2023 at 04:55:13PM +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    | 15 +++++++++++++++
>>  5 files changed, 27 insertions(+)
>>
>> diff --git a/docs/formatdomaincaps.rst b/docs/formatdomaincaps.rst
>> index ef752a0f3a..3acc9a12b4 100644
>> --- a/docs/formatdomaincaps.rst
>> +++ b/docs/formatdomaincaps.rst
>> @@ -669,6 +669,7 @@ capabilities. All features occur as children of the
>main ``features`` element.
>>             <value>vapic</value>
>>           </enum>
>>         </hyperv>
>> +       <tdx supported='yes'/>
>>       </features>
>>     </domainCapabilities>
>>
>> diff --git a/src/conf/domain_capabilities.c
>b/src/conf/domain_capabilities.c
>> index f6e09dc584..0f9ddb1e48 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 01bcfa2e39..cc44cf2363 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 8764df5e9d..0b4988256f 100644
>> --- a/src/qemu/qemu_capabilities.c
>> +++ b/src/qemu/qemu_capabilities.c
>> @@ -6657,6 +6657,20 @@
>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) &&
>> +        virQEMUCapsGet(qemuCaps,
>QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) &&
>
>Checking QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT is
>overkill, as we
>can assume that is implied to exist by virtue of QEMU_CAPS_TDX_GUEST
>existing.

Good catch, will remove it.

>
>> +        (STREQ(domCaps->machine, "q35") || STRPREFIX(domCaps-
>>machine, "pc-q35-")))
>
>If QEMU has limited its support for TDX to just q35, then it would be
>much better if QEMU patches for TDX provided a way to detect this via
>QMP, so we don't need to do these string comparisons.

IIUC, QEMU has no limitation on using i440FX with TDX. We had this
check in libvirt as we thought i440FX is deprecated.

@Yamahata, Isaku, @Li, Xiaoyao, so now we have two choices:
1. remove this limitation in libvirt
2. add QMP on QEMU side to report this limitation to libvirt

Which choice do you like?

Thanks
Zhenzhong

>
>> +            domCaps->features[VIR_DOMAIN_CAPS_FEATURE_TDX] =
>VIR_TRISTATE_BOOL_YES;
>> +}
>> +
>> +
>>  int
>>  virQEMUCapsFillDomainCaps(virQEMUCaps *qemuCaps,
>>                            virArch hostarch,
>> @@ -6716,6 +6730,7 @@ virQEMUCapsFillDomainCaps(virQEMUCaps
>*qemuCaps,
>>      virQEMUCapsFillDomainFeatureSGXCaps(qemuCaps, domCaps);
>>      virQEMUCapsFillDomainFeatureHypervCaps(qemuCaps, domCaps);
>>      virQEMUCapsFillDomainDeviceCryptoCaps(qemuCaps, crypto);
>> +    virQEMUCapsFillDomainFeatureTDXCaps(qemuCaps, domCaps);
>>
>>      return 0;
>>  }
>> --
>> 2.34.1
>>
>
>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 :|

_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH rfcv3 03/11] conf: expose TDX feature in domain capabilities
Posted by Daniel P. Berrangé 2 years, 1 month ago 
On Thu, Jan 11, 2024 at 03:43:34AM +0000, Duan, Zhenzhong wrote:
> 
> 
> >-----Original Message-----
> >From: Daniel P. Berrangé <berrange@redhat.com>
> >Subject: Re: [PATCH rfcv3 03/11] conf: expose TDX feature in domain
> >capabilities
> >
> >On Mon, Nov 27, 2023 at 04:55:13PM +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    | 15 +++++++++++++++
> >>  5 files changed, 27 insertions(+)
> >>
> >> diff --git a/docs/formatdomaincaps.rst b/docs/formatdomaincaps.rst
> >> index ef752a0f3a..3acc9a12b4 100644
> >> --- a/docs/formatdomaincaps.rst
> >> +++ b/docs/formatdomaincaps.rst
> >> @@ -669,6 +669,7 @@ capabilities. All features occur as children of the
> >main ``features`` element.
> >>             <value>vapic</value>
> >>           </enum>
> >>         </hyperv>
> >> +       <tdx supported='yes'/>
> >>       </features>
> >>     </domainCapabilities>
> >>
> >> diff --git a/src/conf/domain_capabilities.c
> >b/src/conf/domain_capabilities.c
> >> index f6e09dc584..0f9ddb1e48 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 01bcfa2e39..cc44cf2363 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 8764df5e9d..0b4988256f 100644
> >> --- a/src/qemu/qemu_capabilities.c
> >> +++ b/src/qemu/qemu_capabilities.c
> >> @@ -6657,6 +6657,20 @@
> >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) &&
> >> +        virQEMUCapsGet(qemuCaps,
> >QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) &&
> >
> >Checking QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT is
> >overkill, as we
> >can assume that is implied to exist by virtue of QEMU_CAPS_TDX_GUEST
> >existing.
> 
> Good catch, will remove it.
> 
> >
> >> +        (STREQ(domCaps->machine, "q35") || STRPREFIX(domCaps-
> >>machine, "pc-q35-")))
> >
> >If QEMU has limited its support for TDX to just q35, then it would be
> >much better if QEMU patches for TDX provided a way to detect this via
> >QMP, so we don't need to do these string comparisons.
> 
> IIUC, QEMU has no limitation on using i440FX with TDX. We had this
> check in libvirt as we thought i440FX is deprecated.

Libvirt has not deprecated i440FX, and neither has QEMU upstream.

RHEL, as a downstream, though has deprecated it.

> @Yamahata, Isaku, @Li, Xiaoyao, so now we have two choices:
> 1. remove this limitation in libvirt

Just remove this limitation in libvirt, since RHEL support
limitations are not relevant to the code.

> 2. add QMP on QEMU side to report this limitation to libvirt
> 
> Which choice do you like?

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 :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
RE: [PATCH rfcv3 03/11] conf: expose TDX feature in domain capabilities
Posted by Duan, Zhenzhong 2 years ago 

>-----Original Message-----
>From: Daniel P. Berrangé <berrange@redhat.com>
>Subject: Re: [PATCH rfcv3 03/11] conf: expose TDX feature in domain
>capabilities
>
>On Thu, Jan 11, 2024 at 03:43:34AM +0000, Duan, Zhenzhong wrote:
>>
>>
>> >-----Original Message-----
>> >From: Daniel P. Berrangé <berrange@redhat.com>
>> >Subject: Re: [PATCH rfcv3 03/11] conf: expose TDX feature in domain
>> >capabilities
>> >
>> >On Mon, Nov 27, 2023 at 04:55:13PM +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    | 15 +++++++++++++++
>> >>  5 files changed, 27 insertions(+)
>> >>
>> >> diff --git a/docs/formatdomaincaps.rst b/docs/formatdomaincaps.rst
>> >> index ef752a0f3a..3acc9a12b4 100644
>> >> --- a/docs/formatdomaincaps.rst
>> >> +++ b/docs/formatdomaincaps.rst
>> >> @@ -669,6 +669,7 @@ capabilities. All features occur as children of the
>> >main ``features`` element.
>> >>             <value>vapic</value>
>> >>           </enum>
>> >>         </hyperv>
>> >> +       <tdx supported='yes'/>
>> >>       </features>
>> >>     </domainCapabilities>
>> >>
>> >> diff --git a/src/conf/domain_capabilities.c
>> >b/src/conf/domain_capabilities.c
>> >> index f6e09dc584..0f9ddb1e48 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 01bcfa2e39..cc44cf2363 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 8764df5e9d..0b4988256f 100644
>> >> --- a/src/qemu/qemu_capabilities.c
>> >> +++ b/src/qemu/qemu_capabilities.c
>> >> @@ -6657,6 +6657,20 @@
>> >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) &&
>> >> +        virQEMUCapsGet(qemuCaps,
>> >QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) &&
>> >
>> >Checking QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT is
>> >overkill, as we
>> >can assume that is implied to exist by virtue of QEMU_CAPS_TDX_GUEST
>> >existing.
>>
>> Good catch, will remove it.
>>
>> >
>> >> +        (STREQ(domCaps->machine, "q35") || STRPREFIX(domCaps-
>> >>machine, "pc-q35-")))
>> >
>> >If QEMU has limited its support for TDX to just q35, then it would be
>> >much better if QEMU patches for TDX provided a way to detect this via
>> >QMP, so we don't need to do these string comparisons.
>>
>> IIUC, QEMU has no limitation on using i440FX with TDX. We had this
>> check in libvirt as we thought i440FX is deprecated.
>
>Libvirt has not deprecated i440FX, and neither has QEMU upstream.
>
>RHEL, as a downstream, though has deprecated it.
>
>> @Yamahata, Isaku, @Li, Xiaoyao, so now we have two choices:
>> 1. remove this limitation in libvirt
>
>Just remove this limitation in libvirt, since RHEL support
>limitations are not relevant to the code.

Got it, will do.

Thanks
Zhenzhong
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.