1. Patchew
  2. Libvirt
  3. [libvirt][PATCH v13 0/6] Support query and use SGX
  4. View patch

[libvirt][PATCH v13 1/6] Define SGX capabilities structs

Lin Yang posted 6 patches 3 years, 7 months ago
There is a newer version of this series
[libvirt][PATCH v13 1/6] Define SGX capabilities structs
Posted by Lin Yang 3 years, 7 months ago 
From: Haibin Huang <haibin.huang@intel.com>

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Haibin Huang <haibin.huang@intel.com>
---
 src/conf/domain_capabilities.c | 10 ++++++++++
 src/conf/domain_capabilities.h | 24 ++++++++++++++++++++++++
 src/libvirt_private.syms       |  1 +
 3 files changed, 35 insertions(+)

diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 895e8d00e8..27f3fb8d36 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -76,6 +76,16 @@ virSEVCapabilitiesFree(virSEVCapability *cap)
 }
 
 
+void
+virSGXCapabilitiesFree(virSGXCapability *cap)
+{
+    if (!cap)
+        return;
+
+    g_free(cap);
+}
+
+
 static void
 virDomainCapsDispose(void *obj)
 {
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index f2eed80b15..dac1098e98 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -192,6 +192,24 @@ struct _virSEVCapability {
     unsigned int max_es_guests;
 };
 
+typedef struct _virSection virSection;
+typedef virSection *virSectionPtr;
+struct _virSection {
+    unsigned long long size;
+    unsigned int node;
+};
+
+typedef struct _virSGXCapability virSGXCapability;
+typedef virSGXCapability *virSGXCapabilityPtr;
+struct _virSGXCapability {
+    bool flc;
+    bool sgx1;
+    bool sgx2;
+    unsigned long long section_size;
+    size_t nSections;
+    virSectionPtr pSections;
+};
+
 typedef enum {
     VIR_DOMAIN_CAPS_FEATURE_IOTHREADS = 0,
     VIR_DOMAIN_CAPS_FEATURE_VMCOREINFO,
@@ -228,6 +246,7 @@ struct _virDomainCaps {
 
     virDomainCapsFeatureGIC gic;
     virSEVCapability *sev;
+    virSGXCapability *sgx;
     /* add new domain features here */
 
     virTristateBool features[VIR_DOMAIN_CAPS_FEATURE_LAST];
@@ -276,3 +295,8 @@ void
 virSEVCapabilitiesFree(virSEVCapability *capabilities);
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSEVCapability, virSEVCapabilitiesFree);
+
+void
+virSGXCapabilitiesFree(virSGXCapability *capabilities);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSGXCapability, virSGXCapabilitiesFree);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 76bcc64eb0..5d17890746 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -216,6 +216,7 @@ virDomainCapsEnumSet;
 virDomainCapsFormat;
 virDomainCapsNew;
 virSEVCapabilitiesFree;
+virSGXCapabilitiesFree;
 
 
 # conf/domain_conf.h
-- 
2.25.1
Re: [libvirt][PATCH v13 1/6] Define SGX capabilities structs
Posted by Michal Prívozník 3 years, 6 months ago 
On 7/1/22 21:14, Lin Yang wrote:
> From: Haibin Huang <haibin.huang@intel.com>
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> Signed-off-by: Haibin Huang <haibin.huang@intel.com>
> ---
>  src/conf/domain_capabilities.c | 10 ++++++++++
>  src/conf/domain_capabilities.h | 24 ++++++++++++++++++++++++
>  src/libvirt_private.syms       |  1 +
>  3 files changed, 35 insertions(+)
> 
> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> index 895e8d00e8..27f3fb8d36 100644
> --- a/src/conf/domain_capabilities.c
> +++ b/src/conf/domain_capabilities.c
> @@ -76,6 +76,16 @@ virSEVCapabilitiesFree(virSEVCapability *cap)
>  }
>  
>  
> +void
> +virSGXCapabilitiesFree(virSGXCapability *cap)
> +{
> +    if (!cap)
> +        return;
> +

This leaks cap->pSections.

> +    g_free(cap);
> +}
> +
> +
>  static void
>  virDomainCapsDispose(void *obj)
>  {
> diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
> index f2eed80b15..dac1098e98 100644
> --- a/src/conf/domain_capabilities.h
> +++ b/src/conf/domain_capabilities.h
> @@ -192,6 +192,24 @@ struct _virSEVCapability {
>      unsigned int max_es_guests;
>  };
>  
> +typedef struct _virSection virSection;
> +typedef virSection *virSectionPtr;

No, if we can help it I'd rather avoid introducing virXXXPtr typedef.
We've worked hard to get rid of them and I don't quite see a reason to
reintroduce them.

> +struct _virSection {
> +    unsigned long long size;
> +    unsigned int node;

While it's true that QEMU with its current code does not ever report a
negative number for 'node', at the QAPI level it's declared as signed
integer and thus we ought to reflect that.

> +};
> +
> +typedef struct _virSGXCapability virSGXCapability;
> +typedef virSGXCapability *virSGXCapabilityPtr;
> +struct _virSGXCapability {
> +    bool flc;
> +    bool sgx1;
> +    bool sgx2;
> +    unsigned long long section_size;
> +    size_t nSections;
> +    virSectionPtr pSections;

We tend to use: 'nitems' and 'items', or 'nsections' and 'sections' in
cases like this.

> +};
> +

Michal
RE: [libvirt][PATCH v13 1/6] Define SGX capabilities structs
Posted by Huang, Haibin 3 years, 6 months ago 

> -----Original Message-----
> From: Michal Prívozník <mprivozn@redhat.com>
> Sent: Wednesday, July 20, 2022 7:27 PM
> To: Yang, Lin A <lin.a.yang@intel.com>; libvir-list@redhat.com; Huang, Haibin
> <haibin.huang@intel.com>; Ding, Jian-feng <jian-feng.ding@intel.com>
> Subject: Re: [libvirt][PATCH v13 1/6] Define SGX capabilities structs
> 
> On 7/1/22 21:14, Lin Yang wrote:
> > From: Haibin Huang <haibin.huang@intel.com>
> >
> > Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> > Signed-off-by: Haibin Huang <haibin.huang@intel.com>
> > ---
> >  src/conf/domain_capabilities.c | 10 ++++++++++
> > src/conf/domain_capabilities.h | 24 ++++++++++++++++++++++++
> >  src/libvirt_private.syms       |  1 +
> >  3 files changed, 35 insertions(+)
> >
> > diff --git a/src/conf/domain_capabilities.c
> > b/src/conf/domain_capabilities.c index 895e8d00e8..27f3fb8d36 100644
> > --- a/src/conf/domain_capabilities.c
> > +++ b/src/conf/domain_capabilities.c
> > @@ -76,6 +76,16 @@ virSEVCapabilitiesFree(virSEVCapability *cap)  }
> >
> >
> > +void
> > +virSGXCapabilitiesFree(virSGXCapability *cap) {
> > +    if (!cap)
> > +        return;
> > +
> 
> This leaks cap->pSections.
[Haibin] OK
> 
> > +    g_free(cap);
> > +}
> > +
> > +
> >  static void
> >  virDomainCapsDispose(void *obj)
> >  {
> > diff --git a/src/conf/domain_capabilities.h
> > b/src/conf/domain_capabilities.h index f2eed80b15..dac1098e98 100644
> > --- a/src/conf/domain_capabilities.h
> > +++ b/src/conf/domain_capabilities.h
> > @@ -192,6 +192,24 @@ struct _virSEVCapability {
> >      unsigned int max_es_guests;
> >  };
> >
> > +typedef struct _virSection virSection; typedef virSection
> > +*virSectionPtr;
> 
> No, if we can help it I'd rather avoid introducing virXXXPtr typedef.
> We've worked hard to get rid of them and I don't quite see a reason to
> reintroduce them.
> 
> > +struct _virSection {
> > +    unsigned long long size;
> > +    unsigned int node;
> 
> While it's true that QEMU with its current code does not ever report a
> negative number for 'node', at the QAPI level it's declared as signed integer
> and thus we ought to reflect that.
> 
[Haibin] OK
> > +};
> > +
> > +typedef struct _virSGXCapability virSGXCapability; typedef
> > +virSGXCapability *virSGXCapabilityPtr; struct _virSGXCapability {
> > +    bool flc;
> > +    bool sgx1;
> > +    bool sgx2;
> > +    unsigned long long section_size;
> > +    size_t nSections;
> > +    virSectionPtr pSections;
> 
> We tend to use: 'nitems' and 'items', or 'nsections' and 'sections' in cases like
> this.
[Haibin] OK
> 
> > +};
> > +
> 
> Michal

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.