Re: [PATCH for-6.2 v2 00/11] machine: smp parsing fixes and improvement

[wangyanan (Y)]

On 2021/7/21 21:52, Pankaj Gupta wrote:
> > > On Mon, Jul 19 2021, Yanan Wang <wangyanan55@huawei.com> wrote:
> > >
> > > > Hi,
> > > >
> > > > This is v2 of the series [1] that I have posted to introduce some smp parsing
> > > > fixes and improvement, much more work has been processed compared to RFC v1.
> > > >
> > > > [1] https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg00259.html
> > > >
> > > > The purpose of this series is to improve/fix the parsing logic. Explicitly
> > > > specifying a CPU topology parameter as zero is not allowed any more, and
> > > > maxcpus is now uniformly used to calculate the omitted parameters. It's also
> > > > suggested that we should start to prefer cores over sockets over threads on
> > > > the newer machine types, which will make the computed virtual topology more
> > > > reflective of the real hardware.
> > > >
> > > > In order to reduce code duplication and ease the code maintenance, smp_parse
> > > > in now converted into a parser generic enough for all arches, so that the PC
> > > > specific one can be removed. It's also convenient to introduce more topology
> > > > members (e.g. cluster) to the generic parser in the future.
> > > Cc:ing Pierre, as he also had been looking at the smp parsing code (for
> > > s390x) recently.
> > >
> > > Also, please keep me on cc: for patches that touch s390x.
> > Sure, I will. Sorry about the missing. :)
> >
> > Thanks,
> > Yanan
> > .
> > > > Finally, a QEMU unit test for the parsing of given SMP configuration is added.
> > > > Since all the parsing logic is in generic function smp_parse(), this test
> > > > passes diffenent SMP configurations to the function and compare the parsing
> > > > result with what is expected. In the test, all possible collections of the
> > > > topology parameters and the corressponding expected results are listed,
> > > > including the valid and invalid ones. The preference of sockets over cores
> > > > and the preference of cores over sockets, and the support of multi-dies are
> > > > also taken into consideration.
> > > >
> > > > ---
> > > >
> > > > Changelogs:
> > > >
> > > > v1->v2:
> > > > - disallow "anything=0" in the smp configuration (Andrew)
> > > > - make function smp_parse() a generic helper for all arches
> > > > - improve the error reporting in the parser
> > > > - start to prefer cores over sockets since 6.2 (Daniel)
> > > > - add a unit test for the smp parsing (Daniel)
> > > >
> > > > ---
> > > >
> > > > Yanan Wang (11):
> > > >     machine: Disallow specifying topology parameters as zero
> > > >     machine: Make smp_parse generic enough for all arches
> > > >     machine: Uniformly use maxcpus to calculate the omitted parameters
> > > >     machine: Use the computed parameters to calculate omitted cpus
> > > >     machine: Improve the error reporting of smp parsing
> > > >     hw: Add compat machines for 6.2
> > > >     machine: Prefer cores over sockets in smp parsing since 6.2
> > > >     machine: Use ms instead of global current_machine in sanity-check
> > > >     machine: Tweak the order of topology members in struct CpuTopology
> > > >     machine: Split out the smp parsing code
> > > >     tests/unit: Add a unit test for smp parsing
> > > >
> > > >    MAINTAINERS                 |    2 +
> > > >    hw/arm/virt.c               |   10 +-
> > > >    hw/core/machine-smp.c       |  124 ++++
> > > >    hw/core/machine.c           |   68 +--
> > > >    hw/core/meson.build         |    1 +
> > > >    hw/i386/pc.c                |   66 +--
> > > >    hw/i386/pc_piix.c           |   15 +-
> > > >    hw/i386/pc_q35.c            |   14 +-
> > > >    hw/ppc/spapr.c              |   16 +-
> > > >    hw/s390x/s390-virtio-ccw.c  |   15 +-
> > > >    include/hw/boards.h         |   13 +-
> > > >    include/hw/i386/pc.h        |    3 +
> > > >    qapi/machine.json           |    6 +-
> > > >    qemu-options.hx             |    4 +-
> > > >    tests/unit/meson.build      |    1 +
> > > >    tests/unit/test-smp-parse.c | 1117 +++++++++++++++++++++++++++++++++++
> > > >    16 files changed, 1338 insertions(+), 137 deletions(-)
> > > >    create mode 100644 hw/core/machine-smp.c
> > > >    create mode 100644 tests/unit/test-smp-parse.c
> Please add me in CC as I reviewed one of the patch and was in middle
> of reviewing few other patches
> but missed the latest version.
Ok, I will, and thanks for the reviewing. v2 i.e. this version is now 
the latest. :)

Thanks,
Yanan