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

[Nina Schoetterl-Glausch]

On Thu, 2023-09-14 at 14:06 +0200, Nina Schoetterl-Glausch wrote:
> From: Pierre Morel <pmorel@linux.ibm.com>
> 
> 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>
> Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> Co-developed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> ---
>  MAINTAINERS                      |   1 +
>  qapi/machine-target.json         |  14 ++
>  include/hw/s390x/cpu-topology.h  |  23 +++
>  include/hw/s390x/sclp.h          |   1 +
>  target/s390x/cpu.h               |  75 +++++++
>  hw/s390x/cpu-topology.c          |   2 +
>  target/s390x/kvm/kvm.c           |   5 +-
>  target/s390x/kvm/stsi-topology.c | 338 +++++++++++++++++++++++++++++++
>  target/s390x/kvm/meson.build     |   3 +-
>  9 files changed, 460 insertions(+), 2 deletions(-)
>  create mode 100644 target/s390x/kvm/stsi-topology.c

[...]

> diff --git a/target/s390x/kvm/stsi-topology.c 
> b/target/s390x/kvm/stsi-topology.c
> new file mode 100644
> index 0000000000..22bac2b834
> --- /dev/null
> +++ b/target/s390x/kvm/stsi-topology.c

[...]

> +/**
> + * s390_topology_id_cmp:
> + * @l: first s390_topology_id
> + * @r: second s390_topology_id
> + *
> + * Compare two topology ids according to the sorting order specified by the 
> PoP.
> + *
> + * Returns a negative number if the first id is less than, 0 if it is equal 
> to
> + *     and positive if it is larger than the second id.
> + */
> +static int s390_topology_id_cmp(const s390_topology_id *l,
> +                                const s390_topology_id *r)
> +{
> +    /*
> +     * lexical order, compare less significant values only if more 
> significant
> +     * ones are equal
> +     */
> +    return l->sentinel - r->sentinel ?:
> +           l->drawer - r->drawer ?:
> +           l->book - r->book ?:
> +           l->socket - r->socket ?:
> +           l->type - r->type ?:
> +           -1 * (l->vertical - r->vertical) ?:
> +           -1 * (l->entitlement - r->entitlement) ?:
> +           -1 * (l->dedicated - r->dedicated) ?:

I'll switch around those expressions manually, since gcc complains:

error: ‘*’ in boolean context, suggest ‘&&’ instead 
[-Werror=int-in-bool-context]
  222 |            -1 * (l->vertical - r->vertical) ?:
      |            ~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I'll add a comment also, since the inverted logic is not as obvious as before.

> +           l->origin - r->origin;
> +}

[...]