> -----Original Message-----
> From: Pavel Hrdina <phrdina@xxxxxxxxxx>
> Sent: Friday, June 18, 2021 8:41 PM
> To: Duan, Zhenzhong <zhenzhong.duan@xxxxxxxxx>
> Cc: libvir-list@xxxxxxxxxx; Yamahata, Isaku <isaku.yamahata@xxxxxxxxx>;
> Tian, Jun J <jun.j.tian@xxxxxxxxx>; Qiang, Chenyi <chenyi.qiang@xxxxxxxxx>
> Subject: Re: [RFC PATCH 1/7] qemu: provide support to query the TDX
> capabilities
>
> On Fri, Jun 18, 2021 at 04:50:46PM +0800, Zhenzhong Duan wrote:
> > QEMU provides support for launching an encrypted VMs on Intel x86
> > platform using Trust Domain Extension (TDX) feature. This patch adds
> > support to query the TDX capabilities from the QEMU.
> >
> > Currently there is no elements in TDX capabilities except a placeholder.
> >
> > Signed-off-by: Chenyi Qiang <chenyi.qiang@xxxxxxxxx>
> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@xxxxxxxxx>
> > ---
> > src/conf/domain_capabilities.c | 8 +++++
> > src/conf/domain_capabilities.h | 10 +++++++
> > src/libvirt_private.syms | 1 +
> > src/qemu/qemu_capabilities.c | 30 +++++++++++++++++++
> > src/qemu/qemu_capabilities.h | 1 +
> > src/qemu/qemu_monitor.c | 8 +++++
> > src/qemu/qemu_monitor.h | 3 ++
> > src/qemu/qemu_monitor_json.c | 53
> ++++++++++++++++++++++++++++++++++
> > src/qemu/qemu_monitor_json.h | 3 ++
> > 9 files changed, 117 insertions(+)
> >
> > diff --git a/src/conf/domain_capabilities.c
> > b/src/conf/domain_capabilities.c index cb90ae0176..31577095e9 100644
> > --- a/src/conf/domain_capabilities.c
> > +++ b/src/conf/domain_capabilities.c
> > @@ -76,6 +76,14 @@ virSEVCapabilitiesFree(virSEVCapability *cap)
> > g_free(cap);
> > }
> >
> > +void
> > +virTDXCapabilitiesFree(virTDXCapability *cap) {
> > + if (!cap)
> > + return;
> > +
> > + VIR_FREE(cap);
> > +}
> >
> > static void
> > virDomainCapsDispose(void *obj)
> > diff --git a/src/conf/domain_capabilities.h
> > b/src/conf/domain_capabilities.h index b6433b20c9..e099788da9 100644
> > --- a/src/conf/domain_capabilities.h
> > +++ b/src/conf/domain_capabilities.h
> > @@ -173,6 +173,12 @@ struct _virSEVCapability {
> > unsigned int reduced_phys_bits;
> > };
> >
> > +typedef struct _virTDXCapability virTDXCapability; struct
> > +_virTDXCapability {
> > + /* no elements for Intel TDX for now, just put a placeholder */
> > + uint64_t placeholder;
> > +};
> > +
>
> There is no need to add unused code into libvirt especially if the impact is
> only internal and it is not exposed to users. It can be added in the future if
> needed. Because this patch series doesn't add any TDX specific capability into
> this structure please drop all related code to this placeholder.
Sorry, I didn't fully understand which part to preserve. Do you mean removing
placeholder from struct virTDXCapability or removing virTDXCapability as
it's empty?
virDomainCapsFeatureTDXFormat(virTDXCapability *tdx) shows
'<tdx supported='yes'/>' based on the value of tdx. If virTDXCapability is empty,
tdx is always NULL and '<tdx supported='no'/>' showed.
Thanks
Zhenzhong
>
> Pavel
>
> > typedef enum {
> > VIR_DOMAIN_CAPS_FEATURE_IOTHREADS = 0,
> > VIR_DOMAIN_CAPS_FEATURE_VMCOREINFO,
> > @@ -254,3 +260,7 @@ void
> > virSEVCapabilitiesFree(virSEVCapability *capabilities);
> >
> > G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSEVCapability,
> > virSEVCapabilitiesFree);
> > +
> > +void virTDXCapabilitiesFree(virTDXCapability *capabilities);
> > +
> > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virTDXCapability,
> > +virTDXCapabilitiesFree);
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index
> > 2efa787664..8cbb60b577 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -218,6 +218,7 @@ virDomainCapsEnumSet; virDomainCapsFormat;
> > virDomainCapsNew; virSEVCapabilitiesFree;
> > +virTDXCapabilitiesFree;
> >
> >
> > # conf/domain_conf.h
> > diff --git a/src/qemu/qemu_capabilities.c
> > b/src/qemu/qemu_capabilities.c index 059d6badf2..a143e453f4 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -636,6 +636,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
> > /* 405 */
> > "confidential-guest-support",
> > "query-display-options",
> > + "tdx-guest",
> > );
> >
> >
> > @@ -716,6 +717,8 @@ struct _virQEMUCaps {
> >
> > virSEVCapability *sevCapabilities;
> >
> > + virTDXCapability *tdxCapabilities;
> > +
> > /* Capabilities which may differ depending on the accelerator. */
> > virQEMUCapsAccel kvm;
> > virQEMUCapsAccel tcg;
> > @@ -1354,6 +1357,7 @@ struct virQEMUCapsStringFlags
> virQEMUCapsObjectTypes[] = {
> > { "input-linux", QEMU_CAPS_INPUT_LINUX },
> > { "virtio-gpu-gl-pci", QEMU_CAPS_VIRTIO_GPU_GL_PCI },
> > { "virtio-vga-gl", QEMU_CAPS_VIRTIO_VGA_GL },
> > + { "tdx-guest", QEMU_CAPS_TDX_GUEST},
> > };
> >
> >
> > @@ -2027,6 +2031,7 @@ void virQEMUCapsDispose(void *obj)
> > g_free(qemuCaps->gicCapabilities);
> >
> > virSEVCapabilitiesFree(qemuCaps->sevCapabilities);
> > + virTDXCapabilitiesFree(qemuCaps->tdxCapabilities);
> >
> > virQEMUCapsAccelClear(&qemuCaps->kvm);
> > virQEMUCapsAccelClear(&qemuCaps->tcg);
> > @@ -3354,6 +3359,29 @@
> virQEMUCapsProbeQMPSEVCapabilities(virQEMUCaps *qemuCaps,
> > return 0;
> > }
> >
> > +static int
> > +virQEMUCapsProbeQMPTDXCapabilities(virQEMUCaps *qemuCaps,
> > + qemuMonitor *mon) {
> > + int rc = -1;
> > + virTDXCapability *caps = NULL;
> > +
> > + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST))
> > + return 0;
> > +
> > + if ((rc = qemuMonitorGetTDXCapabilities(mon, &caps)) < 0)
> > + return -1;
> > +
> > + /* TDX isn't actually supported */
> > + if (rc == 0) {
> > + virQEMUCapsClear(qemuCaps, QEMU_CAPS_TDX_GUEST);
> > + return 0;
> > + }
> > +
> > + virTDXCapabilitiesFree(qemuCaps->tdxCapabilities);
> > + qemuCaps->tdxCapabilities = caps;
> > + return 0;
> > +}
> >
> > /*
> > * Filter for features which should never be passed to QEMU. Either
> > because @@ -5316,6 +5344,8 @@
> virQEMUCapsInitQMPMonitor(virQEMUCaps *qemuCaps,
> > return -1;
> > if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0)
> > return -1;
> > + if (virQEMUCapsProbeQMPTDXCapabilities(qemuCaps, mon) < 0)
> > + return -1;
> >
> > virQEMUCapsInitProcessCaps(qemuCaps);
> >
> > diff --git a/src/qemu/qemu_capabilities.h
> > b/src/qemu/qemu_capabilities.h index b2878312ac..a51bd9a256 100644
> > --- a/src/qemu/qemu_capabilities.h
> > +++ b/src/qemu/qemu_capabilities.h
> > @@ -616,6 +616,7 @@ typedef enum { /* virQEMUCapsFlags grouping
> marker for syntax-check */
> > /* 405 */
> > QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT, /* -machine
> confidential-guest-support */
> > QEMU_CAPS_QUERY_DISPLAY_OPTIONS, /* 'query-display-options' qmp
> > command present */
> > + QEMU_CAPS_TDX_GUEST, /* -object tdx-guest,... */
> >
> > QEMU_CAPS_LAST /* this must always be the last item */ }
> > virQEMUCapsFlags; diff --git a/src/qemu/qemu_monitor.c
> > b/src/qemu/qemu_monitor.c index 8f35b4240f..f2a3badeec 100644
> > --- a/src/qemu/qemu_monitor.c
> > +++ b/src/qemu/qemu_monitor.c
> > @@ -3946,6 +3946,14 @@ qemuMonitorNBDServerStart(qemuMonitor
> *mon,
> > return qemuMonitorJSONNBDServerStart(mon, server, tls_alias); }
> >
> > +int
> > +qemuMonitorGetTDXCapabilities(qemuMonitor *mon,
> > + virTDXCapability **capabilities) {
> > + QEMU_CHECK_MONITOR(mon);
> > +
> > + return qemuMonitorJSONGetTDXCapabilities(mon, capabilities); }
> >
> > int
> > qemuMonitorNBDServerAdd(qemuMonitor *mon, diff --git
> > a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index
> > 6a25def78b..48c18c5220 100644
> > --- a/src/qemu/qemu_monitor.h
> > +++ b/src/qemu/qemu_monitor.h
> > @@ -859,6 +859,9 @@ int qemuMonitorGetGICCapabilities(qemuMonitor
> > *mon, int qemuMonitorGetSEVCapabilities(qemuMonitor *mon,
> > virSEVCapability **capabilities);
> >
> > +int qemuMonitorGetTDXCapabilities(qemuMonitor *mon,
> > + virTDXCapability **capabilities);
> > +
> > typedef enum {
> > QEMU_MONITOR_MIGRATE_BACKGROUND = 1 << 0,
> > QEMU_MONITOR_MIGRATE_NON_SHARED_DISK = 1 << 1, /* migration
> with
> > non-shared storage with full disk copy */ diff --git
> > a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index
> > 223777739d..c58152e86f 100644
> > --- a/src/qemu/qemu_monitor_json.c
> > +++ b/src/qemu/qemu_monitor_json.c
> > @@ -7028,6 +7028,59 @@
> qemuMonitorJSONGetSEVCapabilities(qemuMonitor *mon,
> > return ret;
> > }
> >
> > +/**
> > + * qemuMonitorJSONGetTDXCapabilities:
> > + * @mon: qemu monitor object
> > + * @capabilities: pointer to pointer to a TDX capability structure to
> > +be filled
> > + *
> > + * Returns -1 on error, 0 if TDX is not supported, and 1 if TDX is
> > +supported on
> > + * the platform.
> > + */
> > +int
> > +qemuMonitorJSONGetTDXCapabilities(qemuMonitor *mon,
> > + virTDXCapability **capabilities) {
> > + int ret = -1;
> > + virJSONValue *cmd;
> > + virJSONValue *reply = NULL;
> > + g_autoptr(virTDXCapability) capability = NULL;
> > +
> > + *capabilities = NULL;
> > +
> > + if (!(cmd = qemuMonitorJSONMakeCommand("query-tdx-capabilities",
> > + NULL)))
> > + return -1;
> > +
> > + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> > + goto cleanup;
> > +
> > + /* QEMU has only compiled-in support of TDX */
> > + if (qemuMonitorJSONHasError(reply, "GenericError")) {
> > + ret = 0;
> > + goto cleanup;
> > + }
> > +
> > + if (qemuMonitorJSONCheckError(cmd, reply) < 0)
> > + goto cleanup;
> > +
> > + if (!virJSONValueObjectGetObject(reply, "return")) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > + _("query-tdx-capabilities reply was missing return"));
> > + return -1;
> > + }
> > +
> > + capability = g_new0(virTDXCapability, 1);
> > +
> > + *capabilities = g_steal_pointer(&capability);
> > +
> > + ret = 1;
> > + cleanup:
> > + virJSONValueFree(cmd);
> > + virJSONValueFree(reply);
> > +
> > + return ret;
> > +}
> > +
> > static virJSONValue *
> > qemuMonitorJSONBuildInetSocketAddress(const char *host,
> > const char *port) diff --git
> > a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index
> > 01a3ba25f1..3fe6a34c4c 100644
> > --- a/src/qemu/qemu_monitor_json.h
> > +++ b/src/qemu/qemu_monitor_json.h
> > @@ -157,6 +157,9 @@ int
> qemuMonitorJSONGetGICCapabilities(qemuMonitor
> > *mon, int qemuMonitorJSONGetSEVCapabilities(qemuMonitor *mon,
> > virSEVCapability
> > **capabilities);
> >
> > +int qemuMonitorJSONGetTDXCapabilities(qemuMonitor *mon,
> > + virTDXCapability
> > +**capabilities);
> > +
> > int qemuMonitorJSONMigrate(qemuMonitor *mon,
> > unsigned int flags,
> > const char *uri);
> > --
> > 2.25.1
> >
