[PATCH v2] qapi: Cleanup SGX related comments and restore @section-size

Yang Zhong posted 1 patch 2 years, 3 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220119235720.371961-1-yang.zhong@intel.com
There is a newer version of this series
qapi/machine.json     |  4 ++--
qapi/misc-target.json | 17 ++++++++++++-----
hw/i386/sgx.c         | 11 +++++++++--
3 files changed, 23 insertions(+), 9 deletions(-)
[PATCH v2] qapi: Cleanup SGX related comments and restore @section-size
Posted by Yang Zhong 2 years, 3 months ago
The SGX NUMA patches were merged into Qemu 7.0 release, we need
clarify detailed version history information and also change
some related comments, which make SGX related comments clearer.

The QMP command schema promises backwards compatibility as standard.
We temporarily restore "@section-size", which can avoid incompatible
API breakage. The "@section-size" will be deprecated in 7.2 version.

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Yang Zhong <yang.zhong@intel.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qapi/machine.json     |  4 ++--
 qapi/misc-target.json | 17 ++++++++++++-----
 hw/i386/sgx.c         | 11 +++++++++--
 3 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/qapi/machine.json b/qapi/machine.json
index b6a37e17c4..cf47cb63a9 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1207,7 +1207,7 @@
 #
 # @memdev: memory backend linked with device
 #
-# @node: the numa node
+# @node: the numa node (Since: 7.0)
 #
 # Since: 6.2
 ##
@@ -1288,7 +1288,7 @@
 #
 # @memdev: memory backend linked with device
 #
-# @node: the numa node
+# @node: the numa node (Since: 7.0)
 #
 # Since: 6.2
 ##
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 1022aa0184..a87358ea44 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -344,9 +344,9 @@
 #
 # @node: the numa node
 #
-# @size: the size of epc section
+# @size: the size of EPC section
 #
-# Since: 6.2
+# Since: 7.0
 ##
 { 'struct': 'SGXEPCSection',
   'data': { 'node': 'int',
@@ -365,7 +365,9 @@
 #
 # @flc: true if FLC is supported
 #
-# @sections: The EPC sections info for guest
+# @section-size: The EPC section size for guest (Will be deprecated in 7.2)
+#
+# @sections: The EPC sections info for guest (Since: 7.0)
 #
 # Since: 6.2
 ##
@@ -374,6 +376,7 @@
             'sgx1': 'bool',
             'sgx2': 'bool',
             'flc': 'bool',
+            'section-size': 'uint64',
             'sections': ['SGXEPCSection']},
    'if': 'TARGET_I386' }
 
@@ -390,7 +393,9 @@
 #
 # -> { "execute": "query-sgx" }
 # <- { "return": { "sgx": true, "sgx1" : true, "sgx2" : true,
-#                  "flc": true, "section-size" : 0 } }
+#                  "flc": true,  "section-size" : 96468992,
+#                  "sections": [{"node": 0, "size": 67108864},
+#                  {"node": 1, "size": 29360128}]} }
 #
 ##
 { 'command': 'query-sgx', 'returns': 'SGXInfo', 'if': 'TARGET_I386' }
@@ -408,7 +413,9 @@
 #
 # -> { "execute": "query-sgx-capabilities" }
 # <- { "return": { "sgx": true, "sgx1" : true, "sgx2" : true,
-#                  "flc": true, "section-size" : 0 } }
+#                  "flc": true, "section-size" : 96468992,
+#                  "section" : [{"node": 0, "size": 67108864},
+#                  {"node": 1, "size": 29360128}]} }
 #
 ##
 { 'command': 'query-sgx-capabilities', 'returns': 'SGXInfo', 'if': 'TARGET_I386' }
diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
index 5de5dd0893..a2b318dd93 100644
--- a/hw/i386/sgx.c
+++ b/hw/i386/sgx.c
@@ -83,7 +83,7 @@ static uint64_t sgx_calc_section_metric(uint64_t low, uint64_t high)
            ((high & MAKE_64BIT_MASK(0, 20)) << 32);
 }
 
-static SGXEPCSectionList *sgx_calc_host_epc_sections(void)
+static SGXEPCSectionList *sgx_calc_host_epc_sections(uint64_t *size)
 {
     SGXEPCSectionList *head = NULL, **tail = &head;
     SGXEPCSection *section;
@@ -106,6 +106,7 @@ static SGXEPCSectionList *sgx_calc_host_epc_sections(void)
         section = g_new0(SGXEPCSection, 1);
         section->node = j++;
         section->size = sgx_calc_section_metric(ecx, edx);
+        *size += section->size;
         QAPI_LIST_APPEND(tail, section);
     }
 
@@ -156,6 +157,7 @@ SGXInfo *qmp_query_sgx_capabilities(Error **errp)
 {
     SGXInfo *info = NULL;
     uint32_t eax, ebx, ecx, edx;
+    uint64_t size = 0;
 
     int fd = qemu_open_old("/dev/sgx_vepc", O_RDWR);
     if (fd < 0) {
@@ -173,7 +175,8 @@ SGXInfo *qmp_query_sgx_capabilities(Error **errp)
     info->sgx1 = eax & (1U << 0) ? true : false;
     info->sgx2 = eax & (1U << 1) ? true : false;
 
-    info->sections = sgx_calc_host_epc_sections();
+    info->sections = sgx_calc_host_epc_sections(&size);
+    info->section_size = size;
 
     close(fd);
 
@@ -220,12 +223,14 @@ SGXInfo *qmp_query_sgx(Error **errp)
         return NULL;
     }
 
+    SGXEPCState *sgx_epc = &pcms->sgx_epc;
     info = g_new0(SGXInfo, 1);
 
     info->sgx = true;
     info->sgx1 = true;
     info->sgx2 = true;
     info->flc = true;
+    info->section_size = sgx_epc->size;
     info->sections = sgx_get_epc_sections_list();
 
     return info;
@@ -249,6 +254,8 @@ void hmp_info_sgx(Monitor *mon, const QDict *qdict)
                    info->sgx2 ? "enabled" : "disabled");
     monitor_printf(mon, "FLC support: %s\n",
                    info->flc ? "enabled" : "disabled");
+    monitor_printf(mon, "size: %" PRIu64 "\n",
+                   info->section_size);
 
     section_list = info->sections;
     for (section = section_list; section; section = section->next) {

Re: [PATCH v2] qapi: Cleanup SGX related comments and restore @section-size
Posted by Daniel P. Berrangé 2 years, 3 months ago
On Wed, Jan 19, 2022 at 06:57:20PM -0500, Yang Zhong wrote:
> The SGX NUMA patches were merged into Qemu 7.0 release, we need
> clarify detailed version history information and also change
> some related comments, which make SGX related comments clearer.
> 
> The QMP command schema promises backwards compatibility as standard.
> We temporarily restore "@section-size", which can avoid incompatible
> API breakage. The "@section-size" will be deprecated in 7.2 version.
> 
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  qapi/machine.json     |  4 ++--
>  qapi/misc-target.json | 17 ++++++++++++-----
>  hw/i386/sgx.c         | 11 +++++++++--
>  3 files changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/qapi/machine.json b/qapi/machine.json
> index b6a37e17c4..cf47cb63a9 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1207,7 +1207,7 @@
>  #
>  # @memdev: memory backend linked with device
>  #
> -# @node: the numa node
> +# @node: the numa node (Since: 7.0)
>  #
>  # Since: 6.2
>  ##
> @@ -1288,7 +1288,7 @@
>  #
>  # @memdev: memory backend linked with device
>  #
> -# @node: the numa node
> +# @node: the numa node (Since: 7.0)
>  #
>  # Since: 6.2
>  ##
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index 1022aa0184..a87358ea44 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -344,9 +344,9 @@
>  #
>  # @node: the numa node
>  #
> -# @size: the size of epc section
> +# @size: the size of EPC section
>  #
> -# Since: 6.2
> +# Since: 7.0
>  ##
>  { 'struct': 'SGXEPCSection',
>    'data': { 'node': 'int',
> @@ -365,7 +365,9 @@
>  #
>  # @flc: true if FLC is supported
>  #
> -# @sections: The EPC sections info for guest
> +# @section-size: The EPC section size for guest (Will be deprecated in 7.2)

I expected deprecation would start now (7.0, and it would be removed
in 7.2.

Also needs to be documented in docs/about/deprecated.rst



> +#
> +# @sections: The EPC sections info for guest (Since: 7.0)
>  #
>  # Since: 6.2
>  ##
> @@ -374,6 +376,7 @@
>              'sgx1': 'bool',
>              'sgx2': 'bool',
>              'flc': 'bool',
> +            'section-size': 'uint64',
>              'sections': ['SGXEPCSection']},
>     'if': 'TARGET_I386' }
>  
> @@ -390,7 +393,9 @@
>  #
>  # -> { "execute": "query-sgx" }
>  # <- { "return": { "sgx": true, "sgx1" : true, "sgx2" : true,
> -#                  "flc": true, "section-size" : 0 } }
> +#                  "flc": true,  "section-size" : 96468992,
> +#                  "sections": [{"node": 0, "size": 67108864},
> +#                  {"node": 1, "size": 29360128}]} }
>  #
>  ##
>  { 'command': 'query-sgx', 'returns': 'SGXInfo', 'if': 'TARGET_I386' }
> @@ -408,7 +413,9 @@
>  #
>  # -> { "execute": "query-sgx-capabilities" }
>  # <- { "return": { "sgx": true, "sgx1" : true, "sgx2" : true,
> -#                  "flc": true, "section-size" : 0 } }
> +#                  "flc": true, "section-size" : 96468992,
> +#                  "section" : [{"node": 0, "size": 67108864},
> +#                  {"node": 1, "size": 29360128}]} }
>  #
>  ##
>  { 'command': 'query-sgx-capabilities', 'returns': 'SGXInfo', 'if': 'TARGET_I386' }
> diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
> index 5de5dd0893..a2b318dd93 100644
> --- a/hw/i386/sgx.c
> +++ b/hw/i386/sgx.c
> @@ -83,7 +83,7 @@ static uint64_t sgx_calc_section_metric(uint64_t low, uint64_t high)
>             ((high & MAKE_64BIT_MASK(0, 20)) << 32);
>  }
>  
> -static SGXEPCSectionList *sgx_calc_host_epc_sections(void)
> +static SGXEPCSectionList *sgx_calc_host_epc_sections(uint64_t *size)
>  {
>      SGXEPCSectionList *head = NULL, **tail = &head;
>      SGXEPCSection *section;
> @@ -106,6 +106,7 @@ static SGXEPCSectionList *sgx_calc_host_epc_sections(void)
>          section = g_new0(SGXEPCSection, 1);
>          section->node = j++;
>          section->size = sgx_calc_section_metric(ecx, edx);
> +        *size += section->size;
>          QAPI_LIST_APPEND(tail, section);
>      }
>  
> @@ -156,6 +157,7 @@ SGXInfo *qmp_query_sgx_capabilities(Error **errp)
>  {
>      SGXInfo *info = NULL;
>      uint32_t eax, ebx, ecx, edx;
> +    uint64_t size = 0;
>  
>      int fd = qemu_open_old("/dev/sgx_vepc", O_RDWR);
>      if (fd < 0) {
> @@ -173,7 +175,8 @@ SGXInfo *qmp_query_sgx_capabilities(Error **errp)
>      info->sgx1 = eax & (1U << 0) ? true : false;
>      info->sgx2 = eax & (1U << 1) ? true : false;
>  
> -    info->sections = sgx_calc_host_epc_sections();
> +    info->sections = sgx_calc_host_epc_sections(&size);
> +    info->section_size = size;
>  
>      close(fd);
>  
> @@ -220,12 +223,14 @@ SGXInfo *qmp_query_sgx(Error **errp)
>          return NULL;
>      }
>  
> +    SGXEPCState *sgx_epc = &pcms->sgx_epc;
>      info = g_new0(SGXInfo, 1);
>  
>      info->sgx = true;
>      info->sgx1 = true;
>      info->sgx2 = true;
>      info->flc = true;
> +    info->section_size = sgx_epc->size;
>      info->sections = sgx_get_epc_sections_list();
>  
>      return info;
> @@ -249,6 +254,8 @@ void hmp_info_sgx(Monitor *mon, const QDict *qdict)
>                     info->sgx2 ? "enabled" : "disabled");
>      monitor_printf(mon, "FLC support: %s\n",
>                     info->flc ? "enabled" : "disabled");
> +    monitor_printf(mon, "size: %" PRIu64 "\n",
> +                   info->section_size);
>  
>      section_list = info->sections;
>      for (section = section_list; section; section = section->next) {
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH v2] qapi: Cleanup SGX related comments and restore @section-size
Posted by Yang Zhong 2 years, 3 months ago
On Thu, Jan 20, 2022 at 09:10:34AM +0000, Daniel P. Berrangé wrote:
> On Wed, Jan 19, 2022 at 06:57:20PM -0500, Yang Zhong wrote:
> > The SGX NUMA patches were merged into Qemu 7.0 release, we need
> > clarify detailed version history information and also change
> > some related comments, which make SGX related comments clearer.
> > 
> > The QMP command schema promises backwards compatibility as standard.
> > We temporarily restore "@section-size", which can avoid incompatible
> > API breakage. The "@section-size" will be deprecated in 7.2 version.
> > 
> > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  qapi/machine.json     |  4 ++--
> >  qapi/misc-target.json | 17 ++++++++++++-----
> >  hw/i386/sgx.c         | 11 +++++++++--
> >  3 files changed, 23 insertions(+), 9 deletions(-)
> > 
> > diff --git a/qapi/machine.json b/qapi/machine.json
> > index b6a37e17c4..cf47cb63a9 100644
> > --- a/qapi/machine.json
> > +++ b/qapi/machine.json
> > @@ -1207,7 +1207,7 @@
> >  #
> >  # @memdev: memory backend linked with device
> >  #
> > -# @node: the numa node
> > +# @node: the numa node (Since: 7.0)
> >  #
> >  # Since: 6.2
> >  ##
> > @@ -1288,7 +1288,7 @@
> >  #
> >  # @memdev: memory backend linked with device
> >  #
> > -# @node: the numa node
> > +# @node: the numa node (Since: 7.0)
> >  #
> >  # Since: 6.2
> >  ##
> > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > index 1022aa0184..a87358ea44 100644
> > --- a/qapi/misc-target.json
> > +++ b/qapi/misc-target.json
> > @@ -344,9 +344,9 @@
> >  #
> >  # @node: the numa node
> >  #
> > -# @size: the size of epc section
> > +# @size: the size of EPC section
> >  #
> > -# Since: 6.2
> > +# Since: 7.0
> >  ##
> >  { 'struct': 'SGXEPCSection',
> >    'data': { 'node': 'int',
> > @@ -365,7 +365,9 @@
> >  #
> >  # @flc: true if FLC is supported
> >  #
> > -# @sections: The EPC sections info for guest
> > +# @section-size: The EPC section size for guest (Will be deprecated in 7.2)
> 
> I expected deprecation would start now (7.0, and it would be removed
> in 7.2.
> 
> Also needs to be documented in docs/about/deprecated.rst
> 
> 
  
   Thanks Daniel, Please check if below comments are okay or not? If no
   problem, I will send v3 today, thanks! 
 
   diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
   index e21e07478f..810542427f 100644
   --- a/docs/about/deprecated.rst
   +++ b/docs/about/deprecated.rst
   @@ -441,3 +441,13 @@ nanoMIPS ISA

   The ``nanoMIPS`` ISA has never been upstreamed to any compiler toolchain.
   As it is hard to generate binaries for it, declare it deprecated.
   +
   +
   +SGX backwards compatibility
   +---------------------------
   +
   +@section-size (Since 7.0)
   +''''''''''''''''''''''''
   +
   +The ``@section-size`` will be replaced with ``@sections`` struct and declare
   +it deprecated.
   diff --git a/qapi/misc-target.json b/qapi/misc-target.json
   index a87358ea44..c88fd0c2a2 100644
   --- a/qapi/misc-target.json
   +++ b/qapi/misc-target.json
   @@ -365,7 +365,7 @@
   #
   # @flc: true if FLC is supported
   #
  -# @section-size: The EPC section size for guest (Will be deprecated in 7.2)
  +# @section-size: The EPC section size for guest (7.0, and it would be removed in 7.2)
   #
   # @sections: The EPC sections info for guest (Since: 7.0) 
   
   Regards,

   Yang


> 
> > +#
> > +# @sections: The EPC sections info for guest (Since: 7.0)
> >  #
> >  # Since: 6.2
> >  ##
> > @@ -374,6 +376,7 @@
> >              'sgx1': 'bool',
> >              'sgx2': 'bool',
> >              'flc': 'bool',
> > +            'section-size': 'uint64',
> >              'sections': ['SGXEPCSection']},
> >     'if': 'TARGET_I386' }
> >  
> > @@ -390,7 +393,9 @@
> >  #
> >  # -> { "execute": "query-sgx" }
> >  # <- { "return": { "sgx": true, "sgx1" : true, "sgx2" : true,
> > -#                  "flc": true, "section-size" : 0 } }
> > +#                  "flc": true,  "section-size" : 96468992,
> > +#                  "sections": [{"node": 0, "size": 67108864},
> > +#                  {"node": 1, "size": 29360128}]} }
> >  #
> >  ##
> >  { 'command': 'query-sgx', 'returns': 'SGXInfo', 'if': 'TARGET_I386' }
> > @@ -408,7 +413,9 @@
> >  #
> >  # -> { "execute": "query-sgx-capabilities" }
> >  # <- { "return": { "sgx": true, "sgx1" : true, "sgx2" : true,
> > -#                  "flc": true, "section-size" : 0 } }
> > +#                  "flc": true, "section-size" : 96468992,
> > +#                  "section" : [{"node": 0, "size": 67108864},
> > +#                  {"node": 1, "size": 29360128}]} }
> >  #
> >  ##
> >  { 'command': 'query-sgx-capabilities', 'returns': 'SGXInfo', 'if': 'TARGET_I386' }
> > diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
> > index 5de5dd0893..a2b318dd93 100644
> > --- a/hw/i386/sgx.c
> > +++ b/hw/i386/sgx.c
> > @@ -83,7 +83,7 @@ static uint64_t sgx_calc_section_metric(uint64_t low, uint64_t high)
> >             ((high & MAKE_64BIT_MASK(0, 20)) << 32);
> >  }
> >  
> > -static SGXEPCSectionList *sgx_calc_host_epc_sections(void)
> > +static SGXEPCSectionList *sgx_calc_host_epc_sections(uint64_t *size)
> >  {
> >      SGXEPCSectionList *head = NULL, **tail = &head;
> >      SGXEPCSection *section;
> > @@ -106,6 +106,7 @@ static SGXEPCSectionList *sgx_calc_host_epc_sections(void)
> >          section = g_new0(SGXEPCSection, 1);
> >          section->node = j++;
> >          section->size = sgx_calc_section_metric(ecx, edx);
> > +        *size += section->size;
> >          QAPI_LIST_APPEND(tail, section);
> >      }
> >  
> > @@ -156,6 +157,7 @@ SGXInfo *qmp_query_sgx_capabilities(Error **errp)
> >  {
> >      SGXInfo *info = NULL;
> >      uint32_t eax, ebx, ecx, edx;
> > +    uint64_t size = 0;
> >  
> >      int fd = qemu_open_old("/dev/sgx_vepc", O_RDWR);
> >      if (fd < 0) {
> > @@ -173,7 +175,8 @@ SGXInfo *qmp_query_sgx_capabilities(Error **errp)
> >      info->sgx1 = eax & (1U << 0) ? true : false;
> >      info->sgx2 = eax & (1U << 1) ? true : false;
> >  
> > -    info->sections = sgx_calc_host_epc_sections();
> > +    info->sections = sgx_calc_host_epc_sections(&size);
> > +    info->section_size = size;
> >  
> >      close(fd);
> >  
> > @@ -220,12 +223,14 @@ SGXInfo *qmp_query_sgx(Error **errp)
> >          return NULL;
> >      }
> >  
> > +    SGXEPCState *sgx_epc = &pcms->sgx_epc;
> >      info = g_new0(SGXInfo, 1);
> >  
> >      info->sgx = true;
> >      info->sgx1 = true;
> >      info->sgx2 = true;
> >      info->flc = true;
> > +    info->section_size = sgx_epc->size;
> >      info->sections = sgx_get_epc_sections_list();
> >  
> >      return info;
> > @@ -249,6 +254,8 @@ void hmp_info_sgx(Monitor *mon, const QDict *qdict)
> >                     info->sgx2 ? "enabled" : "disabled");
> >      monitor_printf(mon, "FLC support: %s\n",
> >                     info->flc ? "enabled" : "disabled");
> > +    monitor_printf(mon, "size: %" PRIu64 "\n",
> > +                   info->section_size);
> >  
> >      section_list = info->sections;
> >      for (section = section_list; section; section = section->next) {
> > 
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2] qapi: Cleanup SGX related comments and restore @section-size
Posted by Daniel P. Berrangé 2 years, 3 months ago
On Thu, Jan 20, 2022 at 05:16:01PM +0800, Yang Zhong wrote:
> On Thu, Jan 20, 2022 at 09:10:34AM +0000, Daniel P. Berrangé wrote:
> > On Wed, Jan 19, 2022 at 06:57:20PM -0500, Yang Zhong wrote:
> > > The SGX NUMA patches were merged into Qemu 7.0 release, we need
> > > clarify detailed version history information and also change
> > > some related comments, which make SGX related comments clearer.
> > > 
> > > The QMP command schema promises backwards compatibility as standard.
> > > We temporarily restore "@section-size", which can avoid incompatible
> > > API breakage. The "@section-size" will be deprecated in 7.2 version.
> > > 
> > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > > Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > >  qapi/machine.json     |  4 ++--
> > >  qapi/misc-target.json | 17 ++++++++++++-----
> > >  hw/i386/sgx.c         | 11 +++++++++--
> > >  3 files changed, 23 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/qapi/machine.json b/qapi/machine.json
> > > index b6a37e17c4..cf47cb63a9 100644
> > > --- a/qapi/machine.json
> > > +++ b/qapi/machine.json
> > > @@ -1207,7 +1207,7 @@
> > >  #
> > >  # @memdev: memory backend linked with device
> > >  #
> > > -# @node: the numa node
> > > +# @node: the numa node (Since: 7.0)
> > >  #
> > >  # Since: 6.2
> > >  ##
> > > @@ -1288,7 +1288,7 @@
> > >  #
> > >  # @memdev: memory backend linked with device
> > >  #
> > > -# @node: the numa node
> > > +# @node: the numa node (Since: 7.0)
> > >  #
> > >  # Since: 6.2
> > >  ##
> > > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > > index 1022aa0184..a87358ea44 100644
> > > --- a/qapi/misc-target.json
> > > +++ b/qapi/misc-target.json
> > > @@ -344,9 +344,9 @@
> > >  #
> > >  # @node: the numa node
> > >  #
> > > -# @size: the size of epc section
> > > +# @size: the size of EPC section
> > >  #
> > > -# Since: 6.2
> > > +# Since: 7.0
> > >  ##
> > >  { 'struct': 'SGXEPCSection',
> > >    'data': { 'node': 'int',
> > > @@ -365,7 +365,9 @@
> > >  #
> > >  # @flc: true if FLC is supported
> > >  #
> > > -# @sections: The EPC sections info for guest
> > > +# @section-size: The EPC section size for guest (Will be deprecated in 7.2)
> > 
> > I expected deprecation would start now (7.0, and it would be removed
> > in 7.2.
> > 
> > Also needs to be documented in docs/about/deprecated.rst
> > 
> > 
>   
>    Thanks Daniel, Please check if below comments are okay or not? If no
>    problem, I will send v3 today, thanks! 
>  
>    diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>    index e21e07478f..810542427f 100644
>    --- a/docs/about/deprecated.rst
>    +++ b/docs/about/deprecated.rst
>    @@ -441,3 +441,13 @@ nanoMIPS ISA
> 
>    The ``nanoMIPS`` ISA has never been upstreamed to any compiler toolchain.
>    As it is hard to generate binaries for it, declare it deprecated.
>    +
>    +
>    +SGX backwards compatibility
>    +---------------------------
>    +
>    +@section-size (Since 7.0)
>    +''''''''''''''''''''''''
>    +
>    +The ``@section-size`` will be replaced with ``@sections`` struct and declare
>    +it deprecated.

This needs to be higher up in the file - look at the section
with the heading 'QEMU Machine Protocol (QMP) commands' for
how we list QMP features we're removing.


>    diff --git a/qapi/misc-target.json b/qapi/misc-target.json
>    index a87358ea44..c88fd0c2a2 100644
>    --- a/qapi/misc-target.json
>    +++ b/qapi/misc-target.json
>    @@ -365,7 +365,7 @@
>    #
>    # @flc: true if FLC is supported
>    #
>   -# @section-size: The EPC section size for guest (Will be deprecated in 7.2)
>   +# @section-size: The EPC section size for guest (7.0, and it would be removed in 7.2)

We don't need this comment - see the other reply in this thread
about using an '@deprecated' tag instead, which gets turned into
a comment in the auto-generated documentation. 


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH v2] qapi: Cleanup SGX related comments and restore @section-size
Posted by Yang Zhong 2 years, 3 months ago
On Thu, Jan 20, 2022 at 09:44:34AM +0000, Daniel P. Berrangé wrote:
> On Thu, Jan 20, 2022 at 05:16:01PM +0800, Yang Zhong wrote:
> > On Thu, Jan 20, 2022 at 09:10:34AM +0000, Daniel P. Berrangé wrote:
> > > On Wed, Jan 19, 2022 at 06:57:20PM -0500, Yang Zhong wrote:
> > > > The SGX NUMA patches were merged into Qemu 7.0 release, we need
> > > > clarify detailed version history information and also change
> > > > some related comments, which make SGX related comments clearer.
> > > > 
> > > > The QMP command schema promises backwards compatibility as standard.
> > > > We temporarily restore "@section-size", which can avoid incompatible
> > > > API breakage. The "@section-size" will be deprecated in 7.2 version.
> > > > 
> > > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > > > Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> > > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > > > ---
> > > >  qapi/machine.json     |  4 ++--
> > > >  qapi/misc-target.json | 17 ++++++++++++-----
> > > >  hw/i386/sgx.c         | 11 +++++++++--
> > > >  3 files changed, 23 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/qapi/machine.json b/qapi/machine.json
> > > > index b6a37e17c4..cf47cb63a9 100644
> > > > --- a/qapi/machine.json
> > > > +++ b/qapi/machine.json
> > > > @@ -1207,7 +1207,7 @@
> > > >  #
> > > >  # @memdev: memory backend linked with device
> > > >  #
> > > > -# @node: the numa node
> > > > +# @node: the numa node (Since: 7.0)
> > > >  #
> > > >  # Since: 6.2
> > > >  ##
> > > > @@ -1288,7 +1288,7 @@
> > > >  #
> > > >  # @memdev: memory backend linked with device
> > > >  #
> > > > -# @node: the numa node
> > > > +# @node: the numa node (Since: 7.0)
> > > >  #
> > > >  # Since: 6.2
> > > >  ##
> > > > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > > > index 1022aa0184..a87358ea44 100644
> > > > --- a/qapi/misc-target.json
> > > > +++ b/qapi/misc-target.json
> > > > @@ -344,9 +344,9 @@
> > > >  #
> > > >  # @node: the numa node
> > > >  #
> > > > -# @size: the size of epc section
> > > > +# @size: the size of EPC section
> > > >  #
> > > > -# Since: 6.2
> > > > +# Since: 7.0
> > > >  ##
> > > >  { 'struct': 'SGXEPCSection',
> > > >    'data': { 'node': 'int',
> > > > @@ -365,7 +365,9 @@
> > > >  #
> > > >  # @flc: true if FLC is supported
> > > >  #
> > > > -# @sections: The EPC sections info for guest
> > > > +# @section-size: The EPC section size for guest (Will be deprecated in 7.2)
> > > 
> > > I expected deprecation would start now (7.0, and it would be removed
> > > in 7.2.
> > > 
> > > Also needs to be documented in docs/about/deprecated.rst
> > > 
> > > 
> >   
> >    Thanks Daniel, Please check if below comments are okay or not? If no
> >    problem, I will send v3 today, thanks! 
> >  
> >    diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> >    index e21e07478f..810542427f 100644
> >    --- a/docs/about/deprecated.rst
> >    +++ b/docs/about/deprecated.rst
> >    @@ -441,3 +441,13 @@ nanoMIPS ISA
> > 
> >    The ``nanoMIPS`` ISA has never been upstreamed to any compiler toolchain.
> >    As it is hard to generate binaries for it, declare it deprecated.
> >    +
> >    +
> >    +SGX backwards compatibility
> >    +---------------------------
> >    +
> >    +@section-size (Since 7.0)
> >    +''''''''''''''''''''''''
> >    +
> >    +The ``@section-size`` will be replaced with ``@sections`` struct and declare
> >    +it deprecated.
> 
> This needs to be higher up in the file - look at the section
> with the heading 'QEMU Machine Protocol (QMP) commands' for
> how we list QMP features we're removing.
>   

   Thanks, I will add this in v3.
 
> 
> >    diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> >    index a87358ea44..c88fd0c2a2 100644
> >    --- a/qapi/misc-target.json
> >    +++ b/qapi/misc-target.json
> >    @@ -365,7 +365,7 @@
> >    #
> >    # @flc: true if FLC is supported
> >    #
> >   -# @section-size: The EPC section size for guest (Will be deprecated in 7.2)
> >   +# @section-size: The EPC section size for guest (7.0, and it would be removed in 7.2)
> 
> We don't need this comment - see the other reply in this thread
> about using an '@deprecated' tag instead, which gets turned into
> a comment in the auto-generated documentation. 
> 

  Thanks Daniel, it is strange that my mail couldn't receive this mail. But I checked this
  from link(https://lists.nongnu.org/archive/html/qemu-devel/2022-01/msg04247.html).

  So here, I also thanks Philippe, who shared one helpful example for me. thanks again!

  Yang 


> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2] qapi: Cleanup SGX related comments and restore @section-size
Posted by Philippe Mathieu-Daudé via 2 years, 3 months ago
On 20/1/22 10:10, Daniel P. Berrangé wrote:
> On Wed, Jan 19, 2022 at 06:57:20PM -0500, Yang Zhong wrote:
>> The SGX NUMA patches were merged into Qemu 7.0 release, we need
>> clarify detailed version history information and also change
>> some related comments, which make SGX related comments clearer.
>>
>> The QMP command schema promises backwards compatibility as standard.
>> We temporarily restore "@section-size", which can avoid incompatible
>> API breakage. The "@section-size" will be deprecated in 7.2 version.
>>
>> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
>> Signed-off-by: Yang Zhong <yang.zhong@intel.com>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>   qapi/machine.json     |  4 ++--
>>   qapi/misc-target.json | 17 ++++++++++++-----
>>   hw/i386/sgx.c         | 11 +++++++++--
>>   3 files changed, 23 insertions(+), 9 deletions(-)

>> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
>> index 1022aa0184..a87358ea44 100644
>> --- a/qapi/misc-target.json
>> +++ b/qapi/misc-target.json
>> @@ -344,9 +344,9 @@
>>   #
>>   # @node: the numa node
>>   #
>> -# @size: the size of epc section
>> +# @size: the size of EPC section
>>   #
>> -# Since: 6.2
>> +# Since: 7.0
>>   ##
>>   { 'struct': 'SGXEPCSection',
>>     'data': { 'node': 'int',
>> @@ -365,7 +365,9 @@
>>   #
>>   # @flc: true if FLC is supported
>>   #
>> -# @sections: The EPC sections info for guest
>> +# @section-size: The EPC section size for guest (Will be deprecated in 7.2)
> 
> I expected deprecation would start now (7.0, and it would be removed
> in 7.2.
> 
> Also needs to be documented in docs/about/deprecated.rst

Isn't docs/about/deprecated.rst for user-facing changes *only*?

Machine-facing changes are already described in the QAPI schema.

Please correct me.

Thanks,

Phil.

Re: [PATCH v2] qapi: Cleanup SGX related comments and restore @section-size
Posted by Daniel P. Berrangé 2 years, 3 months ago
On Thu, Jan 20, 2022 at 04:31:14PM +0100, Philippe Mathieu-Daudé wrote:
> On 20/1/22 10:10, Daniel P. Berrangé wrote:
> > On Wed, Jan 19, 2022 at 06:57:20PM -0500, Yang Zhong wrote:
> > > The SGX NUMA patches were merged into Qemu 7.0 release, we need
> > > clarify detailed version history information and also change
> > > some related comments, which make SGX related comments clearer.
> > > 
> > > The QMP command schema promises backwards compatibility as standard.
> > > We temporarily restore "@section-size", which can avoid incompatible
> > > API breakage. The "@section-size" will be deprecated in 7.2 version.
> > > 
> > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > > Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > >   qapi/machine.json     |  4 ++--
> > >   qapi/misc-target.json | 17 ++++++++++++-----
> > >   hw/i386/sgx.c         | 11 +++++++++--
> > >   3 files changed, 23 insertions(+), 9 deletions(-)
> 
> > > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > > index 1022aa0184..a87358ea44 100644
> > > --- a/qapi/misc-target.json
> > > +++ b/qapi/misc-target.json
> > > @@ -344,9 +344,9 @@
> > >   #
> > >   # @node: the numa node
> > >   #
> > > -# @size: the size of epc section
> > > +# @size: the size of EPC section
> > >   #
> > > -# Since: 6.2
> > > +# Since: 7.0
> > >   ##
> > >   { 'struct': 'SGXEPCSection',
> > >     'data': { 'node': 'int',
> > > @@ -365,7 +365,9 @@
> > >   #
> > >   # @flc: true if FLC is supported
> > >   #
> > > -# @sections: The EPC sections info for guest
> > > +# @section-size: The EPC section size for guest (Will be deprecated in 7.2)
> > 
> > I expected deprecation would start now (7.0, and it would be removed
> > in 7.2.
> > 
> > Also needs to be documented in docs/about/deprecated.rst
> 
> Isn't docs/about/deprecated.rst for user-facing changes *only*?
> 
> Machine-facing changes are already described in the QAPI schema.
> 
> Please correct me.

Just because something is machine-facing, doesn't mean it isn't
also user-facing, as users' write the machines that talk to QEMU.

deprecated.rst documents *everything* that changes in one of our
publically consumable interfaces, whether CLI args, QAPI schema,
HMP commands, or device impls or more. There's a whole section
there just for QMP command changes already.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH v2] qapi: Cleanup SGX related comments and restore @section-size
Posted by Philippe Mathieu-Daudé via 2 years, 3 months ago
On 20/1/22 16:40, Daniel P. Berrangé wrote:
> On Thu, Jan 20, 2022 at 04:31:14PM +0100, Philippe Mathieu-Daudé wrote:
>> On 20/1/22 10:10, Daniel P. Berrangé wrote:
>>> On Wed, Jan 19, 2022 at 06:57:20PM -0500, Yang Zhong wrote:
>>>> The SGX NUMA patches were merged into Qemu 7.0 release, we need
>>>> clarify detailed version history information and also change
>>>> some related comments, which make SGX related comments clearer.
>>>>
>>>> The QMP command schema promises backwards compatibility as standard.
>>>> We temporarily restore "@section-size", which can avoid incompatible
>>>> API breakage. The "@section-size" will be deprecated in 7.2 version.
>>>>
>>>> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
>>>> Signed-off-by: Yang Zhong <yang.zhong@intel.com>
>>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>>> ---
>>>>    qapi/machine.json     |  4 ++--
>>>>    qapi/misc-target.json | 17 ++++++++++++-----
>>>>    hw/i386/sgx.c         | 11 +++++++++--
>>>>    3 files changed, 23 insertions(+), 9 deletions(-)
>>
>>>> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
>>>> index 1022aa0184..a87358ea44 100644
>>>> --- a/qapi/misc-target.json
>>>> +++ b/qapi/misc-target.json
>>>> @@ -344,9 +344,9 @@
>>>>    #
>>>>    # @node: the numa node
>>>>    #
>>>> -# @size: the size of epc section
>>>> +# @size: the size of EPC section
>>>>    #
>>>> -# Since: 6.2
>>>> +# Since: 7.0
>>>>    ##
>>>>    { 'struct': 'SGXEPCSection',
>>>>      'data': { 'node': 'int',
>>>> @@ -365,7 +365,9 @@
>>>>    #
>>>>    # @flc: true if FLC is supported
>>>>    #
>>>> -# @sections: The EPC sections info for guest
>>>> +# @section-size: The EPC section size for guest (Will be deprecated in 7.2)
>>>
>>> I expected deprecation would start now (7.0, and it would be removed
>>> in 7.2.
>>>
>>> Also needs to be documented in docs/about/deprecated.rst
>>
>> Isn't docs/about/deprecated.rst for user-facing changes *only*?
>>
>> Machine-facing changes are already described in the QAPI schema.
>>
>> Please correct me.
> 
> Just because something is machine-facing, doesn't mean it isn't
> also user-facing, as users' write the machines that talk to QEMU.
> 
> deprecated.rst documents *everything* that changes in one of our
> publically consumable interfaces, whether CLI args, QAPI schema,
> HMP commands, or device impls or more. There's a whole section
> there just for QMP command changes already.

OK got it, thanks!


Re: [PATCH v2] qapi: Cleanup SGX related comments and restore @section-size
Posted by Philippe Mathieu-Daudé via 2 years, 3 months ago
+Markus for QAPI deprecation

On 1/20/22 00:57, Yang Zhong wrote:
> The SGX NUMA patches were merged into Qemu 7.0 release, we need
> clarify detailed version history information and also change
> some related comments, which make SGX related comments clearer.
> 
> The QMP command schema promises backwards compatibility as standard.
> We temporarily restore "@section-size", which can avoid incompatible
> API breakage. The "@section-size" will be deprecated in 7.2 version.
> 
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  qapi/machine.json     |  4 ++--
>  qapi/misc-target.json | 17 ++++++++++++-----
>  hw/i386/sgx.c         | 11 +++++++++--
>  3 files changed, 23 insertions(+), 9 deletions(-)

> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index 1022aa0184..a87358ea44 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -344,9 +344,9 @@
>  #
>  # @node: the numa node
>  #
> -# @size: the size of epc section
> +# @size: the size of EPC section
>  #
> -# Since: 6.2
> +# Since: 7.0
>  ##
>  { 'struct': 'SGXEPCSection',
>    'data': { 'node': 'int',
> @@ -365,7 +365,9 @@
>  #
>  # @flc: true if FLC is supported
>  #
> -# @sections: The EPC sections info for guest
> +# @section-size: The EPC section size for guest (Will be deprecated in 7.2)

See commit 75ecee72625 ("qapi: Enable enum member introspection to show
more than name"). I'd change as:

  # @section-size: The EPC section size for guest
  #                 Redundant with @sections.  Just for backward
compatibility.

> +#
> +# @sections: The EPC sections info for guest (Since: 7.0)

and then add:


  # Features:
  # @deprecated: Member @section-size is deprecated.  Use @sections instead.

>  #
>  # Since: 6.2
>  ##
> @@ -374,6 +376,7 @@
>              'sgx1': 'bool',
>              'sgx2': 'bool',
>              'flc': 'bool',
> +            'section-size': 'uint64',
>              'sections': ['SGXEPCSection']},
>     'if': 'TARGET_I386' }