Re: [PATCH v14 01/11] s390x/cpu topology: adding s390 specificities to CPU topology

[Pierre Morel]

On 1/13/23 17:58, Nina Schoetterl-Glausch wrote:
> On Thu, 2023-01-05 at 15:53 +0100, Pierre Morel wrote:
> > S390 adds two new SMP levels, drawers and books to the CPU
> > topology.
> > The S390 CPU have specific toplogy features like dedication
> > and polarity to give to the guest indications on the host
> > vCPUs scheduling and help the guest take the best decisions
> > on the scheduling of threads on the vCPUs.
> >
> > Let us provide the SMP properties with books and drawers levels
> > and S390 CPU with dedication and polarity,
> >
> > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> > ---
> >   qapi/machine.json               | 14 ++++++++--
> >   include/hw/boards.h             | 10 ++++++-
> >   include/hw/s390x/cpu-topology.h | 23 ++++++++++++++++
> >   target/s390x/cpu.h              |  6 +++++
> >   hw/core/machine-smp.c           | 48 ++++++++++++++++++++++++++++-----
> >   hw/core/machine.c               |  4 +++
> >   hw/s390x/s390-virtio-ccw.c      |  2 ++
> >   softmmu/vl.c                    |  6 +++++
> >   target/s390x/cpu.c              | 10 +++++++
> >   qemu-options.hx                 |  6 +++--
> >   10 files changed, 117 insertions(+), 12 deletions(-)
> >   create mode 100644 include/hw/s390x/cpu-topology.h
> [...]
>
> > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> > index 7d6d01325b..39ea63a416 100644
> > --- a/target/s390x/cpu.h
> > +++ b/target/s390x/cpu.h
> > @@ -131,6 +131,12 @@ struct CPUArchState {
> >   
> >   #if !defined(CONFIG_USER_ONLY)
> >       uint32_t core_id; /* PoP "CPU address", same as cpu_index */
> > +    int32_t socket_id;
> > +    int32_t book_id;
> > +    int32_t drawer_id;
> > +    int32_t dedicated;
> > +    int32_t polarity;
>
> If I understood the architecture correctly, the polarity is a property of the 
> configuration,
> not the cpus. So this should be vertical_entitlement, and there should be a 
> machine (?) property
> specifying if the polarity is horizontal or vertical.

You are right, considering PTF only, the documentation says PTF([01]) 
does the following:

"... a process is initiated to place all CPUs in the configuration into 
the polarization specified by the function code, ..."

So on one side the polarization property is explicitly set on the CPU, 
and on the other side all CPU are supposed to be in the same 
polarization state.

So yes we can make the horizontal/vertical a machine property.
However, we do not need to set this tunable as the documentation says 
that the machine always start with horizontal polarization.

On the other hand the documentation mixes a lot vertical with different 
entitlement and horizontal polarization, for TLE order and slacks so I 
prefer to keep the complete description of the polarization as CPU 
properties in case we miss something.

PTF([01]) are no performance bottle neck and the number of CPU is likely 
to be small, even a maximum of 248 is possible KVM warns above 16 CPU so 
the loop for setting all CPU inside PTF interception is not very 
problematic I think.

Doing like you say should simplify PTF interception (no loop) but 
complicates (some more if/else) TLE handling and QMP information display 
on CPU.
So I will have a look at the implications and answer again on this.

Thanks,

Regards,
Pierre

--
Pierre Morel
IBM Lab Boeblingen