>-----Original Message-----
>From: Jim Fehlig <jfeh...@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.qi...@intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@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
