| Message ID | 20200214151636.8764-1-frankja@linux.ibm.com |
|---|---|
| Headers | show |
| Series | s390x: Protected Virtualization support | expand |
Patchew URL: https://patchew.org/QEMU/20200214151636.8764-1-frankja@linux.ibm.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH v3 00/17] s390x: Protected Virtualization support
Message-id: 20200214151636.8764-1-frankja@linux.ibm.com
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'
8ffbebb s390x: For now add unpack feature to GA1
a480bb0 s390x: protvirt: Handle SIGP store status correctly
c5157a7 s390x: protvirt: Move IO control structures over SIDA
cfa7ecd s390x: protvirt: Disable address checks for PV guest IO emulation
24e8dda s390x: protvirt: Move diag 308 data over SIDAD
1730605 s390x: protvirt: Set guest IPL PSW
476b4a4 s390x: protvirt: SCLP interpretation
4a0434d s390x: Add SIDA memory ops
f077e58 s390: protvirt: Move STSI data over SIDAD
b63f901 s390x: protvirt: KVM intercept changes
194b56c s390x: protvirt: Handle diag 308 subcodes 0,1,3,4
6424c01 s390x: protvirt: Add migration blocker
ef65f16 s390x: protvirt: Support unpack facility
439c439 s390x: protvirt: Add diag308 subcodes 8 - 10
4c50a4d Sync pv
c5e938d s390x: Add missing vcpu reset functions
820a1a9 Header sync
=== OUTPUT BEGIN ===
1/17 Checking commit 820a1a9f5c25 (Header sync)
ERROR: Missing Signed-off-by: line(s)
total: 1 errors, 0 warnings, 16 lines checked
Patch 1/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/17 Checking commit c5e938d73f53 (s390x: Add missing vcpu reset functions)
3/17 Checking commit 4c50a4d5dbb9 (Sync pv)
4/17 Checking commit 439c439526ae (s390x: protvirt: Add diag308 subcodes 8 - 10)
5/17 Checking commit ef65f167a065 (s390x: protvirt: Support unpack facility)
ERROR: braces {} are necessary for all arms of this statement
#79: FILE: hw/s390x/ipl.c:706:
+ if (rc)
[...]
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#102:
new file mode 100644
ERROR: "foo* bar" should be "foo *bar"
#127: FILE: hw/s390x/pv.c:21:
+const char* cmd_names[] = {
ERROR: spaces required around that ':' (ctx:VxW)
#298: FILE: hw/s390x/pv.h:32:
+int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak) { return 0: }
^
total: 3 errors, 1 warnings, 410 lines checked
Patch 5/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/17 Checking commit 6424c0195f3e (s390x: protvirt: Add migration blocker)
7/17 Checking commit 194b56c59d1a (s390x: protvirt: Handle diag 308 subcodes 0,1,3,4)
8/17 Checking commit b63f90167c46 (s390x: protvirt: KVM intercept changes)
ERROR: switch and case should be at the same indent
#42: FILE: target/s390x/kvm.c:1696:
switch (icpt_code) {
[...]
+ case ICPT_PV_INSTR:
+ case ICPT_PV_INSTR_NOTIFICATION:
total: 1 errors, 0 warnings, 16 lines checked
Patch 8/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/17 Checking commit f077e582c4b8 (s390: protvirt: Move STSI data over SIDAD)
10/17 Checking commit 4a0434d0a66b (s390x: Add SIDA memory ops)
11/17 Checking commit 476b4a40966d (s390x: protvirt: SCLP interpretation)
12/17 Checking commit 17306057f1e4 (s390x: protvirt: Set guest IPL PSW)
13/17 Checking commit 24e8dda2b422 (s390x: protvirt: Move diag 308 data over SIDAD)
14/17 Checking commit cfa7ecd534cd (s390x: protvirt: Disable address checks for PV guest IO emulation)
15/17 Checking commit c5157a79c674 (s390x: protvirt: Move IO control structures over SIDA)
16/17 Checking commit a480bb062c57 (s390x: protvirt: Handle SIGP store status correctly)
17/17 Checking commit 8ffbebb43d51 (s390x: For now add unpack feature to GA1)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20200214151636.8764-1-frankja@linux.ibm.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
On Fri, 14 Feb 2020 10:16:19 -0500 Janosch Frank <frankja@linux.ibm.com> wrote: > Most of the QEMU changes for PV are related to the new IPL type with > subcodes 8 - 10 and the execution of the necessary Ultravisor calls to > IPL secure guests. Note that we can only boot into secure mode from > normal mode, i.e. stfle 161 is not active in secure mode. > > The other changes related to data gathering for emulation and > disabling addressing checks in secure mode, as well as CPU resets. Does it make sense to start looking at this series now, or I should I wait until review on the kernel side has settled? (I've lost track of the state of the interfaces...) > > V3: > * Use dedicated functions to access SIDA > * Smaller cleanups and segfault fixes > * Error reporting for Ultravisor calls > * Inject of RC of diag308 subcode 10 fails > > V2: > * Split out cleanups > * Internal PV state tracking > * Review feedback > > Janosch Frank (17): > Header sync > s390x: Add missing vcpu reset functions > Sync pv > s390x: protvirt: Add diag308 subcodes 8 - 10 > s390x: protvirt: Support unpack facility > s390x: protvirt: Add migration blocker > s390x: protvirt: Handle diag 308 subcodes 0,1,3,4 > s390x: protvirt: KVM intercept changes > s390: protvirt: Move STSI data over SIDAD > s390x: Add SIDA memory ops > s390x: protvirt: SCLP interpretation > s390x: protvirt: Set guest IPL PSW > s390x: protvirt: Move diag 308 data over SIDAD > s390x: protvirt: Disable address checks for PV guest IO emulation > s390x: protvirt: Move IO control structures over SIDA > s390x: protvirt: Handle SIGP store status correctly > s390x: For now add unpack feature to GA1 > > hw/s390x/Makefile.objs | 1 + > hw/s390x/ipl.c | 80 +++++++++++++- > hw/s390x/ipl.h | 33 ++++++ > hw/s390x/pv.c | 160 ++++++++++++++++++++++++++++ > hw/s390x/pv.h | 40 +++++++ > hw/s390x/s390-virtio-ccw.c | 136 ++++++++++++++++++++++- > hw/s390x/sclp.c | 17 +++ > include/hw/s390x/s390-virtio-ccw.h | 1 + > include/hw/s390x/sclp.h | 2 + > linux-headers/linux/kvm.h | 48 ++++++++- > target/s390x/cpu.c | 41 +++++-- > target/s390x/cpu.h | 8 +- > target/s390x/cpu_features_def.inc.h | 1 + > target/s390x/diag.c | 63 +++++++++-- > target/s390x/gen-features.c | 1 + > target/s390x/helper.c | 4 + > target/s390x/ioinst.c | 113 ++++++++++++++------ > target/s390x/kvm-stub.c | 10 +- > target/s390x/kvm.c | 89 ++++++++++++++-- > target/s390x/kvm_s390x.h | 6 +- > target/s390x/mmu_helper.c | 9 ++ > target/s390x/sigp.c | 1 + > 22 files changed, 789 insertions(+), 75 deletions(-) > create mode 100644 hw/s390x/pv.c > create mode 100644 hw/s390x/pv.h > -ENODOC; can you add something under docs/ that describes how you configure a pv guest and what the initialization/teardown flow is?
On 2/18/20 2:13 PM, Cornelia Huck wrote: > On Fri, 14 Feb 2020 10:16:19 -0500 > Janosch Frank <frankja@linux.ibm.com> wrote: > >> Most of the QEMU changes for PV are related to the new IPL type with >> subcodes 8 - 10 and the execution of the necessary Ultravisor calls to >> IPL secure guests. Note that we can only boot into secure mode from >> normal mode, i.e. stfle 161 is not active in secure mode. >> >> The other changes related to data gathering for emulation and >> disabling addressing checks in secure mode, as well as CPU resets. > > Does it make sense to start looking at this series now, or I should I > wait until review on the kernel side has settled? (I've lost track of > the state of the interfaces...) There won't be too many change because of KVM review. Most noteworthy is the switch from vm/vcpu create to a global switch through the VM, but that's just merging IOCTLs. > >> >> V3: >> * Use dedicated functions to access SIDA >> * Smaller cleanups and segfault fixes >> * Error reporting for Ultravisor calls >> * Inject of RC of diag308 subcode 10 fails >> >> V2: >> * Split out cleanups >> * Internal PV state tracking >> * Review feedback >> >> Janosch Frank (17): >> Header sync >> s390x: Add missing vcpu reset functions >> Sync pv >> s390x: protvirt: Add diag308 subcodes 8 - 10 >> s390x: protvirt: Support unpack facility >> s390x: protvirt: Add migration blocker >> s390x: protvirt: Handle diag 308 subcodes 0,1,3,4 >> s390x: protvirt: KVM intercept changes >> s390: protvirt: Move STSI data over SIDAD >> s390x: Add SIDA memory ops >> s390x: protvirt: SCLP interpretation >> s390x: protvirt: Set guest IPL PSW >> s390x: protvirt: Move diag 308 data over SIDAD >> s390x: protvirt: Disable address checks for PV guest IO emulation >> s390x: protvirt: Move IO control structures over SIDA >> s390x: protvirt: Handle SIGP store status correctly >> s390x: For now add unpack feature to GA1 >> >> hw/s390x/Makefile.objs | 1 + >> hw/s390x/ipl.c | 80 +++++++++++++- >> hw/s390x/ipl.h | 33 ++++++ >> hw/s390x/pv.c | 160 ++++++++++++++++++++++++++++ >> hw/s390x/pv.h | 40 +++++++ >> hw/s390x/s390-virtio-ccw.c | 136 ++++++++++++++++++++++- >> hw/s390x/sclp.c | 17 +++ >> include/hw/s390x/s390-virtio-ccw.h | 1 + >> include/hw/s390x/sclp.h | 2 + >> linux-headers/linux/kvm.h | 48 ++++++++- >> target/s390x/cpu.c | 41 +++++-- >> target/s390x/cpu.h | 8 +- >> target/s390x/cpu_features_def.inc.h | 1 + >> target/s390x/diag.c | 63 +++++++++-- >> target/s390x/gen-features.c | 1 + >> target/s390x/helper.c | 4 + >> target/s390x/ioinst.c | 113 ++++++++++++++------ >> target/s390x/kvm-stub.c | 10 +- >> target/s390x/kvm.c | 89 ++++++++++++++-- >> target/s390x/kvm_s390x.h | 6 +- >> target/s390x/mmu_helper.c | 9 ++ >> target/s390x/sigp.c | 1 + >> 22 files changed, 789 insertions(+), 75 deletions(-) >> create mode 100644 hw/s390x/pv.c >> create mode 100644 hw/s390x/pv.h >> > > -ENODOC; can you add something under docs/ that describes how you > configure a pv guest and what the initialization/teardown flow is? Sure, but could you give me a bit more detail? What do you mean by configure? Command line options?
On Tue, 18 Feb 2020 14:15:32 +0100 Janosch Frank <frankja@linux.ibm.com> wrote: > On 2/18/20 2:13 PM, Cornelia Huck wrote: > > -ENODOC; can you add something under docs/ that describes how you > > configure a pv guest and what the initialization/teardown flow is? > > Sure, but could you give me a bit more detail? > What do you mean by configure? > Command line options? Basically: What does someone using QEMU need to know if they want to set it up to run pv guests? So a quick overview, command line options, prereqs, etc. (Maybe you can also lift some stuff from the kernel doc?)
On 2/18/20 2:24 PM, Cornelia Huck wrote: > On Tue, 18 Feb 2020 14:15:32 +0100 > Janosch Frank <frankja@linux.ibm.com> wrote: > >> On 2/18/20 2:13 PM, Cornelia Huck wrote: > >>> -ENODOC; can you add something under docs/ that describes how you >>> configure a pv guest and what the initialization/teardown flow is? >> >> Sure, but could you give me a bit more detail? >> What do you mean by configure? >> Command line options? > > Basically: What does someone using QEMU need to know if they want to > set it up to run pv guests? So a quick overview, command line options, > prereqs, etc. > > (Maybe you can also lift some stuff from the kernel doc?) > Ok, will do