[PATCH v1 0/8] s390: Extended-Length SCCB & DIAGNOSE 0x318

Collin Walling posted 8 patches 3 years, 10 months ago
Test docker-mingw@fedora passed
Test checkpatch failed
Test asan passed
Test docker-quick@centos7 passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200508230823.22956-1-walling@linux.ibm.com
Maintainers: Richard Henderson <rth@twiddle.net>, Halil Pasic <pasic@linux.ibm.com>, Cornelia Huck <cohuck@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Christian Borntraeger <borntraeger@de.ibm.com>, "Michael S. Tsirkin" <mst@redhat.com>, David Hildenbrand <david@redhat.com>
There is a newer version of this series
hw/s390x/s390-virtio-ccw.c          |  45 +++++++++++++
hw/s390x/sclp.c                     | 101 +++++++++++++++++++++-------
include/hw/s390x/s390-virtio-ccw.h  |   1 +
include/hw/s390x/sclp.h             |   4 ++
linux-headers/asm-s390/kvm.h        |   5 ++
smp.max_cpus                        |   0
target/s390x/cpu.c                  |  19 ++++++
target/s390x/cpu.h                  |   4 ++
target/s390x/cpu_features.h         |   1 +
target/s390x/cpu_features_def.inc.h |   4 ++
target/s390x/cpu_models.c           |   1 +
target/s390x/gen-features.c         |   2 +
target/s390x/kvm-stub.c             |  10 +++
target/s390x/kvm.c                  |  52 ++++++++++++++
target/s390x/kvm_s390x.h            |   3 +
15 files changed, 229 insertions(+), 23 deletions(-)
create mode 100644 smp.max_cpus
[PATCH v1 0/8] s390: Extended-Length SCCB & DIAGNOSE 0x318
Posted by Collin Walling 3 years, 10 months ago
This patch series introduces two features for an s390 KVM quest:
    - Extended-Length SCCB (els) for the Read SCP/CPU Info SCLP 
        commands
    - DIAGNOSE 0x318 (diag318) enabling / migration handling

The diag318 feature depends on els and KVM support.

The els feature is handled entirely with QEMU, and does not require 
KVM support.

These patches are introduced together for two main reasons:
    - els allows diag318 to exist while retaining the original 248 
        VCPU max
    - diag318 is presented to show how els is useful

Full els support is dependant on the Linux kernel, which must react
to the SCLP response code and set an appropriate-length SCCB. 

A user should take care when tuning the CPU model for a VM.
If a user defines a VM with els support and specifies 248 CPUs, but
the guest Linux kernel cannot react to the SCLP response code, then
the guest will crash immediately upon kernel startup.

Since it has been some time since the last review and a few things
have changed, I've removed all ack's and set this submission to v1.

Collin L. Walling (8):
  s390/sclp: remove SCLPDevice param from prepare_cpu_entries
  s390/sclp: check sccb len before filling in data
  s390/sclp: rework sclp boundary and length checks
  s390/sclp: read sccb from mem based on sccb length
  s390/sclp: use cpu offset to locate cpu entries
  s390/sclp: add extended-length sccb support for kvm guest
  s390/kvm: header sync for diag318
  s390: diagnose 318 info reset and migration support

 hw/s390x/s390-virtio-ccw.c          |  45 +++++++++++++
 hw/s390x/sclp.c                     | 101 +++++++++++++++++++++-------
 include/hw/s390x/s390-virtio-ccw.h  |   1 +
 include/hw/s390x/sclp.h             |   4 ++
 linux-headers/asm-s390/kvm.h        |   5 ++
 smp.max_cpus                        |   0
 target/s390x/cpu.c                  |  19 ++++++
 target/s390x/cpu.h                  |   4 ++
 target/s390x/cpu_features.h         |   1 +
 target/s390x/cpu_features_def.inc.h |   4 ++
 target/s390x/cpu_models.c           |   1 +
 target/s390x/gen-features.c         |   2 +
 target/s390x/kvm-stub.c             |  10 +++
 target/s390x/kvm.c                  |  52 ++++++++++++++
 target/s390x/kvm_s390x.h            |   3 +
 15 files changed, 229 insertions(+), 23 deletions(-)
 create mode 100644 smp.max_cpus

-- 
2.21.1


Re: [PATCH v1 0/8] s390: Extended-Length SCCB & DIAGNOSE 0x318
Posted by no-reply@patchew.org 3 years, 10 months ago
Patchew URL: https://patchew.org/QEMU/20200508230823.22956-1-walling@linux.ibm.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20200508230823.22956-1-walling@linux.ibm.com
Subject: [PATCH v1 0/8] s390: Extended-Length SCCB & DIAGNOSE 0x318
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
101f5c5 s390: diagnose 318 info reset and migration support
07dbec2 s390/kvm: header sync for diag318
9985457 s390/sclp: add extended-length sccb support for kvm guest
8d13a8a s390/sclp: use cpu offset to locate cpu entries
4d17516 s390/sclp: read sccb from mem based on sccb length
4a1b469 s390/sclp: rework sclp boundary and length checks
699f978 s390/sclp: check sccb len before filling in data
e4476cc s390/sclp: remove SCLPDevice param from prepare_cpu_entries

=== OUTPUT BEGIN ===
1/8 Checking commit e4476ccd2e2b (s390/sclp: remove SCLPDevice param from prepare_cpu_entries)
2/8 Checking commit 699f978f26e8 (s390/sclp: check sccb len before filling in data)
WARNING: line over 80 characters
#22: FILE: hw/s390x/sclp.c:79:
+    if (be16_to_cpu(sccb->h.length) < (sizeof(ReadInfo) + cpu_count * sizeof(CPUEntry))) {

ERROR: line over 90 characters
#47: FILE: hw/s390x/sclp.c:137:
+    if (be16_to_cpu(sccb->h.length) < (sizeof(ReadCpuInfo) + cpu_count * sizeof(CPUEntry))) {

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#67: 
new file mode 100644

total: 1 errors, 2 warnings, 45 lines checked

Patch 2/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/8 Checking commit 4a1b469bbbcb (s390/sclp: rework sclp boundary and length checks)
4/8 Checking commit 4d17516859c5 (s390/sclp: read sccb from mem based on sccb length)
5/8 Checking commit 8d13a8a15966 (s390/sclp: use cpu offset to locate cpu entries)
6/8 Checking commit 9985457a4f23 (s390/sclp: add extended-length sccb support for kvm guest)
WARNING: line over 80 characters
#87: FILE: hw/s390x/sclp.c:128:
+        warn_report("insufficient sccb size to store full read scp info response");

WARNING: line over 80 characters
#111: FILE: target/s390x/cpu_features_def.inc.h:100:
+DEF_FEAT(EXTENDED_LENGTH_SCCB, "els", STFL, 140, "Extended-length SCCB facility")

total: 0 errors, 2 warnings, 74 lines checked

Patch 6/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/8 Checking commit 07dbec216007 (s390/kvm: header sync for diag318)
8/8 Checking commit 101f5c56e483 (s390: diagnose 318 info reset and migration support)
WARNING: line over 80 characters
#219: FILE: target/s390x/cpu_features_def.inc.h:126:
+DEF_FEAT(DIAG318, "diag318", SCLP_BYTE_134, 0, "Control program name and version codes")

total: 0 errors, 1 warnings, 255 lines checked

Patch 8/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200508230823.22956-1-walling@linux.ibm.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH v1 0/8] s390: Extended-Length SCCB & DIAGNOSE 0x318
Posted by Cornelia Huck 3 years, 10 months ago
On Fri,  8 May 2020 19:08:15 -0400
Collin Walling <walling@linux.ibm.com> wrote:

> Collin L. Walling (8):
>   s390/sclp: remove SCLPDevice param from prepare_cpu_entries

This looks like a simple cleanup...

>   s390/sclp: check sccb len before filling in data

...and that like a simple fix (for a problem that currently does not
trigger.) Would it help or hinder you if I went ahead and queued these
two patches already?

>   s390/sclp: rework sclp boundary and length checks
>   s390/sclp: read sccb from mem based on sccb length
>   s390/sclp: use cpu offset to locate cpu entries
>   s390/sclp: add extended-length sccb support for kvm guest
>   s390/kvm: header sync for diag318
>   s390: diagnose 318 info reset and migration support
> 
>  hw/s390x/s390-virtio-ccw.c          |  45 +++++++++++++
>  hw/s390x/sclp.c                     | 101 +++++++++++++++++++++-------
>  include/hw/s390x/s390-virtio-ccw.h  |   1 +
>  include/hw/s390x/sclp.h             |   4 ++
>  linux-headers/asm-s390/kvm.h        |   5 ++
>  smp.max_cpus                        |   0

Probably a stray file? (Should be stripped from patch 2.)

>  target/s390x/cpu.c                  |  19 ++++++
>  target/s390x/cpu.h                  |   4 ++
>  target/s390x/cpu_features.h         |   1 +
>  target/s390x/cpu_features_def.inc.h |   4 ++
>  target/s390x/cpu_models.c           |   1 +
>  target/s390x/gen-features.c         |   2 +
>  target/s390x/kvm-stub.c             |  10 +++
>  target/s390x/kvm.c                  |  52 ++++++++++++++
>  target/s390x/kvm_s390x.h            |   3 +
>  15 files changed, 229 insertions(+), 23 deletions(-)
>  create mode 100644 smp.max_cpus
> 


Re: [PATCH v1 0/8] s390: Extended-Length SCCB & DIAGNOSE 0x318
Posted by Collin Walling 3 years, 10 months ago
On 5/12/20 12:08 PM, Cornelia Huck wrote:
> On Fri,  8 May 2020 19:08:15 -0400
> Collin Walling <walling@linux.ibm.com> wrote:
> 
>> Collin L. Walling (8):
>>    s390/sclp: remove SCLPDevice param from prepare_cpu_entries
> 
> This looks like a simple cleanup...
> 
>>    s390/sclp: check sccb len before filling in data
> 
> ...and that like a simple fix (for a problem that currently does not
> trigger.) Would it help or hinder you if I went ahead and queued these
> two patches already?
> 

Let's wait for one more round if that's okay. I left a response to
David's feedback that may-or-may not add one more cleanup that can
be squeezed into patch 1 if it makes sense.

Thanks, though! :)

>>    s390/sclp: rework sclp boundary and length checks
>>    s390/sclp: read sccb from mem based on sccb length
>>    s390/sclp: use cpu offset to locate cpu entries
>>    s390/sclp: add extended-length sccb support for kvm guest
>>    s390/kvm: header sync for diag318
>>    s390: diagnose 318 info reset and migration support
>>
>>   hw/s390x/s390-virtio-ccw.c          |  45 +++++++++++++
>>   hw/s390x/sclp.c                     | 101 +++++++++++++++++++++-------
>>   include/hw/s390x/s390-virtio-ccw.h  |   1 +
>>   include/hw/s390x/sclp.h             |   4 ++
>>   linux-headers/asm-s390/kvm.h        |   5 ++
>>   smp.max_cpus                        |   0
> 
> Probably a stray file? (Should be stripped from patch 2.)

Uhhh not sure how that got there :) Probably silly editor doing silly 
things. I'll make sure next round has that stripped.

> 
>>   target/s390x/cpu.c                  |  19 ++++++
>>   target/s390x/cpu.h                  |   4 ++
>>   target/s390x/cpu_features.h         |   1 +
>>   target/s390x/cpu_features_def.inc.h |   4 ++
>>   target/s390x/cpu_models.c           |   1 +
>>   target/s390x/gen-features.c         |   2 +
>>   target/s390x/kvm-stub.c             |  10 +++
>>   target/s390x/kvm.c                  |  52 ++++++++++++++
>>   target/s390x/kvm_s390x.h            |   3 +
>>   15 files changed, 229 insertions(+), 23 deletions(-)
>>   create mode 100644 smp.max_cpus
>>
> 


-- 
--
Regards,
Collin

Stay safe and stay healthy