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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----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
> >




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux