Re: [PATCH v23 01/20] CPU topology: extend with s390 specifics

[Nina Schoetterl-Glausch]

On Tue, 2023-09-19 at 14:47 +0200, Markus Armbruster wrote:
> Nina Schoetterl-Glausch <nsg@linux.ibm.com> writes:
> 
> > From: Pierre Morel <pmorel@linux.ibm.com>
> > 
> > S390 adds two new SMP levels, drawers and books to the CPU
> > topology.
> > S390 CPUs have specific topology features like dedication and
> > entitlement. These indicate to the guest information on host
> > vCPU scheduling and help the guest make better scheduling decisions.
> > 
> > Let us provide the SMP properties with books and drawers levels
> > and S390 CPU with dedication and entitlement,
> > 
> > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> > Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > Co-developed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > ---
> >  qapi/machine-common.json            | 21 +++++++++++++
> >  qapi/machine.json                   | 19 ++++++++++--
> >  include/hw/boards.h                 | 10 +++++-
> >  include/hw/qdev-properties-system.h |  4 +++
> >  target/s390x/cpu.h                  |  6 ++++
> >  hw/core/machine-smp.c               | 48 ++++++++++++++++++++++++-----
> >  hw/core/machine.c                   |  4 +++
> >  hw/core/qdev-properties-system.c    | 13 ++++++++
> >  hw/s390x/s390-virtio-ccw.c          |  4 +++
> >  softmmu/vl.c                        |  6 ++++
> >  target/s390x/cpu.c                  |  7 +++++
> >  qapi/meson.build                    |  1 +
> >  qemu-options.hx                     |  7 +++--
> >  13 files changed, 137 insertions(+), 13 deletions(-)
> >  create mode 100644 qapi/machine-common.json
> > 
> > diff --git a/qapi/machine-common.json b/qapi/machine-common.json
> > new file mode 100644
> > index 0000000000..e40421bb37
> > --- /dev/null
> > +++ b/qapi/machine-common.json
> 
> Why do you need a separate QAPI sub-module?
> 
> > @@ -0,0 +1,21 @@
> > +# -*- Mode: Python -*-
> > +# vim: filetype=python
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2 or later.
> > +# See the COPYING file in the top-level directory.
> > +
> > +##
> > +# = Machines S390 data types
> > +##
> > +
> > +##
> > +# @CpuS390Entitlement:
> > +#
> > +# An enumeration of cpu entitlements that can be assumed by a virtual
> > +# S390 CPU
> > +#
> > +# Since: 8.2
> > +##
> > +{ 'enum': 'CpuS390Entitlement',
> > +  'prefix': 'S390_CPU_ENTITLEMENT',
> > +  'data': [ 'auto', 'low', 'medium', 'high' ] }
> > diff --git a/qapi/machine.json b/qapi/machine.json
> > index a08b6576ca..a63cb951d2 100644
> > --- a/qapi/machine.json
> > +++ b/qapi/machine.json
> > @@ -9,6 +9,7 @@
>    ##
>    # = Machines
> >  ##
> >  
> >  { 'include': 'common.json' }
> > +{ 'include': 'machine-common.json' }
> 
> Section structure is borked :)
> 
> Existing section "Machine" now ends at the new "Machines S390 data
> types" you pull in here.  The contents of below moves from "Machines" to
> "Machines S390 data types".
> 
> Before I explain how to avoid this, I'd like to understand why we need a
> new sub-module.

Should I just move the include statements above the section header?
I assume I could also include it in qapi-schema.json before the machine.json
include and that that is the reason we don't have
the same problem with e.g. migration.json.
But just moving the includes seems cleaner.


[...]