All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] s390x: kvm: Fix number of cpu reports for stsi 3.2.2
@ 2020-03-30 15:38 Janosch Frank
  2020-03-30 16:04 ` David Hildenbrand
  0 siblings, 1 reply; 6+ messages in thread
From: Janosch Frank @ 2020-03-30 15:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: borntraeger, qemu-s390x, cohuck, david

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

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



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] s390x: kvm: Fix number of cpu reports for stsi 3.2.2
  2020-03-30 15:38 [PATCH] s390x: kvm: Fix number of cpu reports for stsi 3.2.2 Janosch Frank
@ 2020-03-30 16:04 ` David Hildenbrand
  2020-03-31 10:11   ` Cornelia Huck
  2020-03-31 11:01   ` [PATCH v2] " Janosch Frank
  0 siblings, 2 replies; 6+ messages in thread
From: David Hildenbrand @ 2020-03-30 16:04 UTC (permalink / raw)
  To: Janosch Frank, qemu-devel; +Cc: borntraeger, qemu-s390x, cohuck

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] s390x: kvm: Fix number of cpu reports for stsi 3.2.2
  2020-03-30 16:04 ` David Hildenbrand
@ 2020-03-31 10:11   ` Cornelia Huck
  2020-03-31 10:38     ` David Hildenbrand
  2020-03-31 11:01   ` [PATCH v2] " Janosch Frank
  1 sibling, 1 reply; 6+ messages in thread
From: Cornelia Huck @ 2020-03-31 10:11 UTC (permalink / raw)
  To: David Hildenbrand; +Cc: borntraeger, qemu-s390x, Janosch Frank, qemu-devel

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] s390x: kvm: Fix number of cpu reports for stsi 3.2.2
  2020-03-31 10:11   ` Cornelia Huck
@ 2020-03-31 10:38     ` David Hildenbrand
  0 siblings, 0 replies; 6+ messages in thread
From: David Hildenbrand @ 2020-03-31 10:38 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: borntraeger, qemu-s390x, Janosch Frank, qemu-devel

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.


-- 
Thanks,

David / dhildenb



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] s390x: kvm: Fix number of cpu reports for stsi 3.2.2
  2020-03-30 16:04 ` David Hildenbrand
  2020-03-31 10:11   ` Cornelia Huck
@ 2020-03-31 11:01   ` Janosch Frank
  2020-03-31 12:01     ` Cornelia Huck
  1 sibling, 1 reply; 6+ messages in thread
From: Janosch Frank @ 2020-03-31 11:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: borntraeger, qemu-s390x, cohuck, david

The cpu number reporting is handled by KVM and QEMU only fills in the
VM name, uuid and other values.

Unfortunately 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. To be consistent We use the same code to retrieve the cpu
numbers as the STSI TCG code in target/s390x/misc_helper.c:HELPER(stsi).

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---

* Fixed commit message and add rev-by
* Calculating total_cpus from configured + reserved

---
 target/s390x/kvm.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 3630c15f45a48864..69881a0da0b31f72 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 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,19 @@ 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++) {
+        if (ms->possible_cpus->cpus[i].cpu) {
+            conf_cpus++;
+        } else {
+            reserved_cpus++;
+        }
+    }
+    sysib.vm[0].total_cpus = conf_cpus + reserved_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));
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] s390x: kvm: Fix number of cpu reports for stsi 3.2.2
  2020-03-31 11:01   ` [PATCH v2] " Janosch Frank
@ 2020-03-31 12:01     ` Cornelia Huck
  0 siblings, 0 replies; 6+ messages in thread
From: Cornelia Huck @ 2020-03-31 12:01 UTC (permalink / raw)
  To: Janosch Frank; +Cc: borntraeger, qemu-s390x, qemu-devel, david

On Tue, 31 Mar 2020 07:01:23 -0400
Janosch Frank <frankja@linux.ibm.com> wrote:

> The cpu number reporting is handled by KVM and QEMU only fills in the
> VM name, uuid and other values.
> 
> Unfortunately 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. To be consistent We use the same code to retrieve the cpu

"...consistent, we..." (fixed up while applying)

> numbers as the STSI TCG code in target/s390x/misc_helper.c:HELPER(stsi).
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> ---
> 
> * Fixed commit message and add rev-by
> * Calculating total_cpus from configured + reserved
> 
> ---
>  target/s390x/kvm.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 3630c15f45a48864..69881a0da0b31f72 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 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));

This is against s390-next; rebased and applied to s390-fixes (please
double check).

[I'm holding off rebasing s390-next on top of s390-fixes resp. master;
I'll rather do that once after all pieces including the headers update
are in place.]



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-03-31 12:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-30 15:38 [PATCH] s390x: kvm: Fix number of cpu reports for stsi 3.2.2 Janosch Frank
2020-03-30 16:04 ` David Hildenbrand
2020-03-31 10:11   ` Cornelia Huck
2020-03-31 10:38     ` David Hildenbrand
2020-03-31 11:01   ` [PATCH v2] " Janosch Frank
2020-03-31 12:01     ` Cornelia Huck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.