| Message ID | 20200330153828.8265-1-frankja@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x: kvm: Fix number of cpu reports for stsi 3.2.2 | expand |
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>
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> >
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 --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));
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(-)