Re: [PATCH] s390x: kvm: Fix number of cpu reports for stsi 3.2.2

[David Hildenbrand <david@redhat.com>]

On 30.03.20 17:38, Janosch Frank wrote:
> The cpu number reporting is handled by KVM and QEMU only fills in the
> VM name, uuid and other values.
> 
> Unfortuantely KVM doesn't report reserved cpus and doesn't even know

s/Unfortuantely/Unfortunately/

> they exist until the are created via the ioctl.
> 
> So let's fix up the cpu values after KVM has written its values to the
> 3.2.2 sysib.

Maybe mention "similar to TCG in target/s390x/misc_helper.c:HELPER(stsi)".

> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  target/s390x/kvm.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 3630c15f45a48864..a1c4890bdf0c65e4 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -1819,8 +1819,10 @@ static int handle_tsch(S390CPU *cpu)
>  
>  static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
>  {
> +    const MachineState *ms = MACHINE(qdev_get_machine());
> +    uint16_t total_cpus = 0, conf_cpus = 0, reserved_cpus = 0;
>      SysIB_322 sysib;
> -    int del;
> +    int del, i;
>  
>      if (s390_is_pv()) {
>          s390_cpu_pv_mem_read(cpu, 0, &sysib, sizeof(sysib));
> @@ -1842,6 +1844,20 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
>          memset(sysib.ext_names[del], 0,
>                 sizeof(sysib.ext_names[0]) * (sysib.count - del));
>      }
> +
> +    /* count the cpus and split them into configured and reserved ones */
> +    for (i = 0; i < ms->possible_cpus->len; i++) {
> +        total_cpus++;
> +        if (ms->possible_cpus->cpus[i].cpu) {
> +            conf_cpus++;
> +        } else {
> +            reserved_cpus++;
> +        }
> +    }

We could of course factor this calculation out :)

(and one could shrink the variables from 3 to 2)

> +    sysib.vm[0].total_cpus = total_cpus;
> +    sysib.vm[0].conf_cpus = conf_cpus;
> +    sysib.vm[0].reserved_cpus = reserved_cpus;
> +
>      /* Insert short machine name in EBCDIC, padded with blanks */
>      if (qemu_name) {
>          memset(sysib.vm[0].name, 0x40, sizeof(sysib.vm[0].name));
> 

Looks sane to me

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb