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

[Cornelia Huck <cohuck@redhat.com>]

On Mon, 30 Mar 2020 18:04:09 +0200
David Hildenbrand <david@redhat.com> wrote:

> 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)

I'd vote for queuing this one on s390-fixes now (with the patch
description tweaks) and doing any cleanup on top for the next release.
Ok?

> 
> > +    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>
>