RE: [RFC PATCH 1/7] qemu: provide support to query the TDX capabilities

-----Original Message----- From: Pavel Hrdina <phrdina(a)redhat.com&gt; Sent: Friday, June 18, 2021 8:41 PM To: Duan, Zhenzhong <zhenzhong.duan(a)intel.com&gt; Cc: libvir-list(a)redhat.com; Yamahata, Isaku <isaku.yamahata(a)intel.com>; Tian, Jun J <jun.j.tian(a)intel.com>; Qiang, Chenyi <chenyi.qiang(a)intel.com&gt; 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(a)intel.com&gt; > Signed-off-by: Zhenzhong Duan <zhenzhong.duan(a)intel.com&gt; > --- > 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.

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 >