Re: [PATCH v15 03/11] target/s390x/cpu topology: handle STSI(15) and build the SYSIB

[Pierre Morel]

On 2/9/23 17:39, Nina Schoetterl-Glausch wrote:
> On Wed, 2023-02-01 at 14:20 +0100, Pierre Morel wrote:
> > On interception of STSI(15.1.x) the System Information Block
> > (SYSIB) is built from the list of pre-ordered topology entries.
> >
> > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> > ---
> >   include/hw/s390x/cpu-topology.h |  22 +++
> >   include/hw/s390x/sclp.h         |   1 +
> >   target/s390x/cpu.h              |  72 +++++++
> >   hw/s390x/cpu-topology.c         |  10 +
> >   target/s390x/kvm/cpu_topology.c | 335 ++++++++++++++++++++++++++++++++
> >   target/s390x/kvm/kvm.c          |   5 +-
> >   target/s390x/kvm/meson.build    |   3 +-
> >   7 files changed, 446 insertions(+), 2 deletions(-)
> >   create mode 100644 target/s390x/kvm/cpu_topology.c
> [...]
> > +
> > +/**
> > + * s390_topology_from_cpu:
> > + * @cpu: The S390CPU
> > + *
> > + * Initialize the topology id from the CPU environment.
> > + */
> > +static s390_topology_id s390_topology_from_cpu(S390CPU *cpu)
> > +{
> > +    s390_topology_id topology_id = {0};
> > +
> > +    topology_id.drawer = cpu->env.drawer_id;
> > +    topology_id.book = cpu->env.book_id;
> > +    topology_id.socket = cpu->env.socket_id;
> > +    topology_id.origin = cpu->env.core_id / 64;
> > +    topology_id.type = S390_TOPOLOGY_CPU_IFL;
> > +    topology_id.dedicated = cpu->env.dedicated;
> > +
> > +    if (s390_topology.polarity == POLARITY_VERTICAL) {
> > +        /*
> > +         * Vertical polarity with dedicated CPU implies
> > +         * vertical high entitlement.
> > +         */
> > +        if (topology_id.dedicated) {
> > +            topology_id.polarity |= POLARITY_VERTICAL_HIGH;
> > +        } else {
> > +            topology_id.polarity |= cpu->env.entitlement;
> > +        }
>
> Why |= instead of an assignment?
> Anyway, I think you can get rid of this in the next version.
> If you define the entitlement via qapi you can just put a little switch
> here and convert it to the hardware definition of polarization.
> (Or you just do +1, but I think the switch is easier to understand)

Oh! right thanks, it is a leftover from when dedication and polarity 
were in the same variable.

I change it with the QAPI enum.

Regards,
Pierre

--
Pierre Morel
IBM Lab Boeblingen