On Mon, 2023-01-16 at 18:28 +0100, Pierre Morel wrote:
>
> 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@xxxxxxxxxxxxx>
> > > ---
> > > 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.
I'm worried about STSI showing both horizontal and vertical CPUs at the same time.
I don't know if this is allowed.
If it is not, you need a way to switch between those atomically, which is harder
if every CPU has this property.
>
> 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.
Yeah, I'm not worried about that.
>
> 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
>
