diff mbox series

s390x: kvm: Fix number of cpu reports for stsi 3.2.2

Message ID 20200330153828.8265-1-frankja@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: kvm: Fix number of cpu reports for stsi 3.2.2 | expand

Commit Message

Janosch Frank March 30, 2020, 3:38 p.m. UTC
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
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.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 target/s390x/kvm.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

Comments

David Hildenbrand March 30, 2020, 4:04 p.m. UTC | #1
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>
Cornelia Huck March 31, 2020, 10:11 a.m. UTC | #2
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>
>
David Hildenbrand March 31, 2020, 10:38 a.m. UTC | #3
On 31.03.20 12:11, Cornelia Huck wrote:
> 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?

Fine with me.
diff mbox series

Patch

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++;
+        }
+    }
+    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));