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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




>-----Original Message-----
>From: Daniel P. Berrangé <berrange@xxxxxxxxxx>
>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@xxxxxxxxx>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@xxxxxxxxx>
>> ---
>>  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@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux