Re: [PATCH v9 02/10] s390x/cpu topology: core_id sets s390x CPU topology

[Nico Boehr]

Quoting Pierre Morel (2022-09-02 09:55:23)
[...]
> diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c
> new file mode 100644
> index 0000000000..a6ca006ec5
> --- /dev/null
> +++ b/hw/s390x/cpu-topology.c
[...]
> +void s390_topology_new_cpu(int core_id)
> +{
[...]
> +    socket_id = core_id / topo->cores;

The comment below is essential for understanding all of this. Move it before 
this line.

> +
> +    bit = core_id;
> +    origin = bit / 64;
> +    bit %= 64;
> +    bit = 63 - bit;
> +
> +    /*
> +     * At the core level, each CPU is represented by a bit in a 64bit
> +     * unsigned long. Set on plug and clear on unplug of a CPU.

cleared                                  ^

[...]
> +     * In that case the origin field, representing the offset of the first 
> CPU
> +     * in the CPU container allows to represent up to the maximal number of
> +     * CPU inside several CPU containers inside the socket container.

How about:
"In that case the origin variable represents the offset of the first CPU in the
CPU container. More than 64 CPUs per socket are represented in several CPU
containers inside the socket container."

> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index b5ca154e2f..15cefd104b 100644
[...]
> @@ -247,6 +248,12 @@ static void ccw_init(MachineState *machine)
>      /* init memory + setup max page size. Required for the CPU model */
>      s390_memory_init(machine->ram);
>  
> +    /* Adding the topology must be done before CPU intialization*/

space                                                              ^