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

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

2021-06-21 Thread Pavel Hrdina

On Mon, Jun 21, 2021 at 06:04:15AM +, Duan, Zhenzhong wrote:
> 
> 
> > -Original Message-
> > From: Pavel Hrdina 
> > Sent: Friday, June 18, 2021 8:41 PM
> > To: Duan, Zhenzhong 
> > Cc: libvir-list@redhat.com; Yamahata, Isaku ;
> > Tian, Jun J ; Qiang, Chenyi 
> > 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 
> > > Signed-off-by: Zhenzhong Duan 
> > > ---
> > >  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?

No problem, should have been more explicit about that. I meant the whole
structure and code related to the structure.

> virDomainCapsFeatureTDXFormat(virTDXCapability *tdx) shows
> '' based on the value of tdx. If virTDXCapability is 
> empty,
> tdx is always NULL and '' showed.

Right, I missed that part during review. I see that it copies what we do
with AMD SEV where we also have a struct with capabilities.

Currently for Intel TDX there are no extra capabilities and I was not
able to find any patches posted to QEMU devel list which would implement
the QMP command 'query-tdx-capabilities' so right now all of this looks
useless. Unless there is a plan to add some capabilities and introduce
that command we should use boolean value for now.

Pavel

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

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

2021-06-21 Thread Duan, Zhenzhong




> -Original Message-
> From: Pavel Hrdina 
> Sent: Friday, June 18, 2021 8:41 PM
> To: Duan, Zhenzhong 
> Cc: libvir-list@redhat.com; Yamahata, Isaku ;
> Tian, Jun J ; Qiang, Chenyi 
> 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 
> > Signed-off-by: Zhenzhong Duan 
> > ---
> >  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
'' based on the value of tdx. If virTDXCapability is 
empty,
tdx is always NULL and '' 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.

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

2021-06-18 Thread Pavel Hrdina

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 
> Signed-off-by: Zhenzhong Duan 
> ---
>  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(>kvm);
>  virQEMUCapsAccelClear(>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, )) < 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

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

2021-06-18 Thread Zhenzhong Duan

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 
Signed-off-by: Zhenzhong Duan 
---
 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;
+};
+
 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(>kvm);
 virQEMUCapsAccelClear(>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, )) < 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 */

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

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

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

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

4 matches

Site Navigation

Mail list logo

Footer information