On Thu, Jan 11, 2024 at 03:43:34AM +0000, Duan, Zhenzhong wrote:
>
>
> >-----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.
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@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
