mbox series

[v3,00/17] s390x: Protected Virtualization support

Message ID 20200214151636.8764-1-frankja@linux.ibm.com (mailing list archive)
Headers show
Series s390x: Protected Virtualization support | expand

Message

Janosch Frank Feb. 14, 2020, 3:16 p.m. UTC
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.

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

Comments

no-reply@patchew.org Feb. 14, 2020, 4:33 p.m. UTC | #1
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
Cornelia Huck Feb. 18, 2020, 1:13 p.m. UTC | #2
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?
Janosch Frank Feb. 18, 2020, 1:15 p.m. UTC | #3
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?
Cornelia Huck Feb. 18, 2020, 1:24 p.m. UTC | #4
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?)
Janosch Frank Feb. 18, 2020, 1:56 p.m. UTC | #5
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