Re: [PATCH v5 00/18] s390x: Protected Virtualization support

[Janosch Frank]

On 2/26/20 9:09 PM, Cornelia Huck wrote:
> On Wed, 26 Feb 2020 07:20:20 -0500
> Janosch Frank <address@hidden> 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.
>>
>> v5:
>>      * Added balloon inhibition
>>      * Moved docs into docs/system
>>      * Some more enable/disable changes
>>      * Moved enablement/disablement of pv in separate functions
>>      * Some review fixes
>>
>> v4:
>>      * Sync with KVM changes
>>      * Review changes
>>
>> 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
>>
>>
>> Christian Borntraeger (1):
>>   s390x: Add unpack facility feature to GA1
>>
>> Janosch Frank (17):
>>   s390x: Use constant for ESA PSW address
>>   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: Inhibit balloon when switching to protected mode
>>   s390x: protvirt: KVM intercept changes
>>   s390x: Add SIDA memory ops
>>   s390x: protvirt: Move STSI data over SIDAD
>>   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
>>   docs: Add protvirt docs
>>
>>  docs/system/index.rst               |   1 +
>>  docs/system/protvirt.rst            |  57 +++++++++++
>>  hw/s390x/Makefile.objs              |   1 +
>>  hw/s390x/ipl.c                      |  82 +++++++++++++++-
>>  hw/s390x/ipl.h                      |  33 +++++++
>>  hw/s390x/pv.c                       | 106 +++++++++++++++++++++
>>  hw/s390x/pv.h                       |  34 +++++++
>>  hw/s390x/s390-virtio-ccw.c          | 143 +++++++++++++++++++++++++++-
>>  hw/s390x/sclp.c                     |  17 ++++
>>  include/hw/s390x/s390-virtio-ccw.h  |   1 +
>>  include/hw/s390x/sclp.h             |   2 +
>>  linux-headers/linux/kvm.h           |  45 ++++++++-
>>  target/s390x/cpu.c                  |  27 ++++--
>>  target/s390x/cpu.h                  |   9 +-
>>  target/s390x/cpu_features_def.inc.h |   1 +
>>  target/s390x/diag.c                 |  61 ++++++++++--
>>  target/s390x/gen-features.c         |   1 +
>>  target/s390x/helper.c               |   4 +
>>  target/s390x/ioinst.c               | 113 +++++++++++++++-------
>>  target/s390x/kvm.c                  |  54 ++++++++++-
>>  target/s390x/kvm_s390x.h            |   2 +
>>  target/s390x/mmu_helper.c           |  14 +++
>>  target/s390x/sigp.c                 |   1 +
>>  23 files changed, 744 insertions(+), 65 deletions(-)
>>  create mode 100644 docs/system/protvirt.rst
>>  create mode 100644 hw/s390x/pv.c
>>  create mode 100644 hw/s390x/pv.h
>>
> 
> I guess this is on top of my published s390-next branch? (At least I
> can apply it there...)

Yes, you can also pick from here:
https://github.com/frankjaa/qemu/commits/protvirt

> 
> When I try to build the result on x86 with clang, I get

I forgot to add static inline to the non kvm functions...
The latest github branch state has that fixed

> 
> In file included from /home/cohuck/git/qemu/hw/s390x/ipl.c:36:
> /home/cohuck/git/qemu/hw/s390x/pv.h:25:5: error: no previous prototype for
>       function 's390_pv_vm_enable' [-Werror,-Wmissing-prototypes]
> int s390_pv_vm_enable(void) { return 0; }
>     ^
> /home/cohuck/git/qemu/hw/s390x/pv.h:26:6: error: no previous prototype for
>       function 's390_pv_vm_disable' [-Werror,-Wmissing-prototypes]
> void s390_pv_vm_disable(void) {}
>      ^
> /home/cohuck/git/qemu/hw/s390x/pv.h:27:5: error: no previous prototype for
>       function 's390_pv_set_sec_parms' [-Werror,-Wmissing-prototypes]
> int s390_pv_set_sec_parms(uint64_t origin, uint64_t length) { return 0; }
>     ^
> /home/cohuck/git/qemu/hw/s390x/pv.h:28:5: error: no previous prototype for
>       function 's390_pv_unpack' [-Werror,-Wmissing-prototypes]
> int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak) { return 0; }
>     ^
> /home/cohuck/git/qemu/hw/s390x/pv.h:29:6: error: no previous prototype for
>       function 's390_pv_perf_clear_reset' [-Werror,-Wmissing-prototypes]
> void s390_pv_perf_clear_reset(void) {}
>      ^
> /home/cohuck/git/qemu/hw/s390x/pv.h:30:5: error: no previous prototype for
>       function 's390_pv_verify' [-Werror,-Wmissing-prototypes]
> int s390_pv_verify(void) { return 0; }
>     ^
> /home/cohuck/git/qemu/hw/s390x/pv.h:31:6: error: no previous prototype for
>       function 's390_pv_unshare' [-Werror,-Wmissing-prototypes]
> void s390_pv_unshare(void) {}
>      ^
> 
> and

I'll look into it

> 
> /home/cohuck/git/qemu/hw/s390x/s390-virtio-ccw.c:355:9: error: variable 'rc' 
> is
>       used uninitialized whenever 'if' condition is true
>       [-Werror,-Wsometimes-uninitialized]
>     if (local_err) {
>         ^~~~~~~~~
> /home/cohuck/git/qemu/hw/s390x/s390-virtio-ccw.c:391:12: note: uninitialized 
> use
>       occurs here
>     return rc;
>            ^~
> /home/cohuck/git/qemu/hw/s390x/s390-virtio-ccw.c:355:5: note: remove the 'if' 
> if
>       its condition is always false
>     if (local_err) {
>     ^~~~~~~~~~~~~~~~
> /home/cohuck/git/qemu/hw/s390x/s390-virtio-ccw.c:347:11: note: initialize the
>       variable 'rc' to silence this warning
>     int rc;
>           ^
>            = 0
> /home/cohuck/git/qemu/hw/s390x/s390-virtio-ccw.c:396:26: error: incomplete
>       definition of type 'struct kvm_run'
>     int r1 = (cs->kvm_run->s390_sieic.ipa & 0x00f0) >> 4;
>               ~~~~~~~~~~~^
> /home/cohuck/git/qemu/include/hw/core/cpu.h:265:8: note: forward declaration 
> of
>       'struct kvm_run'
> struct kvm_run;
>        ^
>

signature.asc
Description: OpenPGP digital signature