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

[Cornelia Huck <cohuck@redhat.com>]

On Wed, 26 Feb 2020 07:20:20 -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.
> 
> 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...)

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

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

/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;
       ^