On Thu, Jan 11, 2024 at 03:43:34AM +0000, Duan, Zhenzhong wrote:
>-----Original Message-----
>From: Daniel P. Berrangé <berrange(a)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(a)intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan(a)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 :|