Re: [PATCH v3 4/4] s390x: topology: implementating Store Topology System Information

[Pierre Morel]

On 10/13/21 10:20, Thomas Huth wrote:
> On 16/09/2021 15.50, Pierre Morel wrote:
> > The handling of STSI is enhanced with the interception of the
> > function code 15 for storing CPU topology.
> >
> > Using the objects built during the pluging of CPU, we build the
> > SYSIB 15_1_x structures.
> >
> > With this patch the maximum MNEST level is 2, this is also
> > the only level allowed and only SYSIB 15_1_2 will be built.
> >
> > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> > ---
> >   target/s390x/kvm/kvm.c | 101 +++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 101 insertions(+)
> >
> > diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> > index dd036961fe..0a5f2aced2 100644
> > --- a/target/s390x/kvm/kvm.c
> > +++ b/target/s390x/kvm/kvm.c
> > @@ -52,6 +52,7 @@
> >   #include "hw/s390x/s390-virtio-ccw.h"
> >   #include "hw/s390x/s390-virtio-hcall.h"
> >   #include "hw/s390x/pv.h"
> > +#include "hw/s390x/cpu-topology.h"
> >   #ifndef DEBUG_KVM
> >   #define DEBUG_KVM  0
> > @@ -1908,6 +1909,102 @@ static void insert_stsi_3_2_2(S390CPU *cpu, 
> > __u64 addr, uint8_t ar)
> >       }
> >   }
>
> Could you maybe put the new code in a separate file under target/s390x/ 
> (maybe target/s390x/topology.c ?), in case we ever want to implement 
> this for TCG, too?

Yes right, did not thing about TCG as I saw it as pure Z.
But in fact it may be adapted for TCG too.


> > +static int stsi_15_cpus(void *p, S390TopologyCores *cd)
> > +{
> > +    SysIBTl_cpu *tle = (SysIBTl_cpu *)p;
> > +
> > +    tle->nl = 0;
> > +    tle->dedicated = cd->dedicated;
> > +    tle->polarity = cd->polarity;
> > +    tle->type = cd->cputype;
> > +    tle->origin = cd->origin;
>
> If we want to use this for TCG, too, one day:
>
>     tle->origin = be16_to_cpu(cd->origin);

yes.

> > +    tle->mask = cd->mask;
> > +
> > +    return sizeof(*tle);
> > +}
> > +
> > +static int set_socket(const MachineState *ms, void *p,
> > +                      S390TopologySocket *socket)
> > +{
> > +    BusChild *kid;
> > +    int l, len = 0;
> > +
> > +    len += stsi_15_container(p, 1, socket->socket_id);
> > +    p += len;
> > +
> > +    QTAILQ_FOREACH_REVERSE(kid, &socket->bus->children, sibling) {
> > +        l = stsi_15_cpus(p, S390_TOPOLOGY_CORES(kid->child));
> > +        p += l;
> > +        len += l;
> > +    }
> > +    return len;
> > +}
> > +
> > +static void insert_stsi_15_1_2(const MachineState *ms, void *p)
> > +{
> > +    S390TopologyBook *book;
> > +    SysIB_151x *sysib;
> > +    BusChild *kid;
> > +    int level = 2;
> > +    int len, l;
> > +
> > +    sysib = (SysIB_151x *)p;
> > +    sysib->mnest = level;
> > +    sysib->mag[TOPOLOGY_NR_MAG2] = ms->smp.sockets;
> > +    sysib->mag[TOPOLOGY_NR_MAG1] = ms->smp.cores;
> > +
> > +    book = s390_get_topology();
> > +    len = sizeof(SysIB_151x);
> > +    p += len;
> > +
> > +    QTAILQ_FOREACH_REVERSE(kid, &book->bus->children, sibling) {
> > +        l = set_socket(ms, p, S390_TOPOLOGY_SOCKET(kid->child));
> > +        p += l;
> > +        len += l;
> > +    }
> > +
> > +    sysib->length = len;
>
> For TCG awareness:
>
>      sysib->length = be16_to_cpu(len);

OK

Yes, thanks I will take care of TCG in the next spin.
Regards,
Pierre

--
Pierre Morel
IBM Lab Boeblingen