diff mbox series

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

Message ID 20220119235720.371961-1-yang.zhong@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2] qapi: Cleanup SGX related comments and restore @section-size | expand

Commit Message

Yang Zhong Jan. 19, 2022, 11:57 p.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé Jan. 20, 2022, 12:24 a.m. UTC | #1
+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' }
Daniel P. Berrangé Jan. 20, 2022, 9:10 a.m. UTC | #2
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
Yang Zhong Jan. 20, 2022, 9:16 a.m. UTC | #3
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 :|
Daniel P. Berrangé Jan. 20, 2022, 9:44 a.m. UTC | #4
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
Yang Zhong Jan. 20, 2022, 1:17 p.m. UTC | #5
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 :|
Philippe Mathieu-Daudé Jan. 20, 2022, 3:31 p.m. UTC | #6
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.
Daniel P. Berrangé Jan. 20, 2022, 3:40 p.m. UTC | #7
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
Philippe Mathieu-Daudé Jan. 20, 2022, 3:58 p.m. UTC | #8
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!
diff mbox series

Patch

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