diff mbox series

[06/15] s390x: protvirt: Support unpack facility

Message ID 20191120114334.2287-7-frankja@linux.ibm.com
State New
Headers show
Series s390x: Protected Virtualization support | expand

Commit Message

Janosch Frank Nov. 20, 2019, 11:43 a.m. UTC
When a guest has saved a ipib of type 5 and call diagnose308 with
subcode 10, we have to setup the protected processing environment via
Ultravisor calls. The calls are done by KVM and are exposed via an API.

The following steps are necessary:
1. Create a VM (register it with the Ultravisor)
2. Create secure CPUs for all of our current cpus
3. Forward the secure header to the Ultravisor (has all information on
how to decrypt the image and VM information)
4. Protect image pages from the host and decrypt them
5. Verify the image integrity

Only after step 5 a protected VM is allowed to run.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 hw/s390x/Makefile.objs              |   1 +
 hw/s390x/ipl.c                      |  33 ++++++++
 hw/s390x/ipl.h                      |   2 +
 hw/s390x/pv.c                       | 118 ++++++++++++++++++++++++++++
 hw/s390x/pv.h                       |  26 ++++++
 hw/s390x/s390-virtio-ccw.c          |  45 ++++++++---
 target/s390x/cpu_features_def.inc.h |   1 +
 7 files changed, 216 insertions(+), 10 deletions(-)
 create mode 100644 hw/s390x/pv.c
 create mode 100644 hw/s390x/pv.h

Comments

Cornelia Huck Nov. 20, 2019, 1:43 p.m. UTC | #1
On Wed, 20 Nov 2019 06:43:25 -0500
Janosch Frank <frankja@linux.ibm.com> wrote:

> When a guest has saved a ipib of type 5 and call diagnose308 with
> subcode 10, we have to setup the protected processing environment via
> Ultravisor calls. The calls are done by KVM and are exposed via an API.
> 
> The following steps are necessary:
> 1. Create a VM (register it with the Ultravisor)
> 2. Create secure CPUs for all of our current cpus
> 3. Forward the secure header to the Ultravisor (has all information on
> how to decrypt the image and VM information)
> 4. Protect image pages from the host and decrypt them
> 5. Verify the image integrity
> 
> Only after step 5 a protected VM is allowed to run.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  hw/s390x/Makefile.objs              |   1 +
>  hw/s390x/ipl.c                      |  33 ++++++++
>  hw/s390x/ipl.h                      |   2 +
>  hw/s390x/pv.c                       | 118 ++++++++++++++++++++++++++++
>  hw/s390x/pv.h                       |  26 ++++++
>  hw/s390x/s390-virtio-ccw.c          |  45 ++++++++---
>  target/s390x/cpu_features_def.inc.h |   1 +
>  7 files changed, 216 insertions(+), 10 deletions(-)
>  create mode 100644 hw/s390x/pv.c
>  create mode 100644 hw/s390x/pv.h
> 
> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> index 94e57113d8..568bab9711 100644
> --- a/hw/s390x/Makefile.objs
> +++ b/hw/s390x/Makefile.objs
> @@ -31,6 +31,7 @@ obj-y += tod-qemu.o
>  obj-$(CONFIG_KVM) += tod-kvm.o
>  obj-$(CONFIG_KVM) += s390-skeys-kvm.o
>  obj-$(CONFIG_KVM) += s390-stattrib-kvm.o s390-mchk.o
> +obj-$(CONFIG_KVM) += pv.o

As this is kvm only...

>  obj-y += s390-ccw.o
>  obj-y += ap-device.o
>  obj-y += ap-bridge.o
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index a077926f36..50501fcd27 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -33,6 +33,7 @@
>  #include "qemu/cutils.h"
>  #include "qemu/option.h"
>  #include "exec/exec-all.h"
> +#include "pv.h"
>  
>  #define KERN_IMAGE_START                0x010000UL
>  #define LINUX_MAGIC_ADDR                0x010008UL
> @@ -668,6 +669,38 @@ static void s390_ipl_prepare_qipl(S390CPU *cpu)
>      cpu_physical_memory_unmap(addr, len, 1, len);
>  }
>  
> +int s390_ipl_prepare_pv_header(void)
> +{
> +    int rc;
> +    IplParameterBlock *iplb = s390_ipl_get_iplb_secure();
> +    IPLBlockPV *ipib_pv = &iplb->pv;
> +    void *hdr = g_malloc(ipib_pv->pv_header_len);
> +
> +    cpu_physical_memory_read(ipib_pv->pv_header_addr, hdr,
> +                             ipib_pv->pv_header_len);
> +    rc = s390_pv_set_sec_parms((uint64_t)hdr,
> +                               ipib_pv->pv_header_len);
> +    g_free(hdr);
> +    return rc;
> +}
> +
> +int s390_ipl_pv_unpack(void)
> +{
> +    int i, rc;
> +    IplParameterBlock *iplb = s390_ipl_get_iplb_secure();
> +    IPLBlockPV *ipib_pv = &iplb->pv;
> +
> +    for (i = 0; i < ipib_pv->num_comp; i++) {
> +        rc = s390_pv_unpack(ipib_pv->components[i].addr,
> +                            TARGET_PAGE_ALIGN(ipib_pv->components[i].size),
> +                            ipib_pv->components[i].tweak_pref);

...you probably need a stub version of the pv functions as well, right?

> +        if (rc) {
> +            return rc;
> +        }
> +    }
> +    return rc;
> +}
> +
>  void s390_ipl_prepare_cpu(S390CPU *cpu)
>  {
>      S390IPLState *ipl = get_ipl_device();
David Hildenbrand Nov. 21, 2019, 11:27 a.m. UTC | #2
On 20.11.19 12:43, Janosch Frank wrote:
> When a guest has saved a ipib of type 5 and call diagnose308 with
> subcode 10, we have to setup the protected processing environment via
> Ultravisor calls. The calls are done by KVM and are exposed via an API.
> 
> The following steps are necessary:
> 1. Create a VM (register it with the Ultravisor)
> 2. Create secure CPUs for all of our current cpus
> 3. Forward the secure header to the Ultravisor (has all information on
> how to decrypt the image and VM information)
> 4. Protect image pages from the host and decrypt them
> 5. Verify the image integrity
> 
> Only after step 5 a protected VM is allowed to run.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   hw/s390x/Makefile.objs              |   1 +
>   hw/s390x/ipl.c                      |  33 ++++++++
>   hw/s390x/ipl.h                      |   2 +
>   hw/s390x/pv.c                       | 118 ++++++++++++++++++++++++++++
>   hw/s390x/pv.h                       |  26 ++++++
>   hw/s390x/s390-virtio-ccw.c          |  45 ++++++++---
>   target/s390x/cpu_features_def.inc.h |   1 +
>   7 files changed, 216 insertions(+), 10 deletions(-)
>   create mode 100644 hw/s390x/pv.c
>   create mode 100644 hw/s390x/pv.h
> 
> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> index 94e57113d8..568bab9711 100644
> --- a/hw/s390x/Makefile.objs
> +++ b/hw/s390x/Makefile.objs
> @@ -31,6 +31,7 @@ obj-y += tod-qemu.o
>   obj-$(CONFIG_KVM) += tod-kvm.o
>   obj-$(CONFIG_KVM) += s390-skeys-kvm.o
>   obj-$(CONFIG_KVM) += s390-stattrib-kvm.o s390-mchk.o
> +obj-$(CONFIG_KVM) += pv.o
>   obj-y += s390-ccw.o
>   obj-y += ap-device.o
>   obj-y += ap-bridge.o
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index a077926f36..50501fcd27 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -33,6 +33,7 @@
>   #include "qemu/cutils.h"
>   #include "qemu/option.h"
>   #include "exec/exec-all.h"
> +#include "pv.h"
>   
>   #define KERN_IMAGE_START                0x010000UL
>   #define LINUX_MAGIC_ADDR                0x010008UL
> @@ -668,6 +669,38 @@ static void s390_ipl_prepare_qipl(S390CPU *cpu)
>       cpu_physical_memory_unmap(addr, len, 1, len);
>   }
>   
> +int s390_ipl_prepare_pv_header(void)
> +{
> +    int rc;
> +    IplParameterBlock *iplb = s390_ipl_get_iplb_secure();
> +    IPLBlockPV *ipib_pv = &iplb->pv;
> +    void *hdr = g_malloc(ipib_pv->pv_header_len);
> +
> +    cpu_physical_memory_read(ipib_pv->pv_header_addr, hdr,
> +                             ipib_pv->pv_header_len);
> +    rc = s390_pv_set_sec_parms((uint64_t)hdr,
> +                               ipib_pv->pv_header_len);
> +    g_free(hdr);
> +    return rc;
> +}
> +
> +int s390_ipl_pv_unpack(void)
> +{
> +    int i, rc;
> +    IplParameterBlock *iplb = s390_ipl_get_iplb_secure();
> +    IPLBlockPV *ipib_pv = &iplb->pv;
> +
> +    for (i = 0; i < ipib_pv->num_comp; i++) {
> +        rc = s390_pv_unpack(ipib_pv->components[i].addr,
> +                            TARGET_PAGE_ALIGN(ipib_pv->components[i].size),
> +                            ipib_pv->components[i].tweak_pref);
> +        if (rc) {
> +            return rc;
> +        }
> +    }
> +    return rc;
> +}
> +
>   void s390_ipl_prepare_cpu(S390CPU *cpu)
>   {
>       S390IPLState *ipl = get_ipl_device();
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index 7b8a493509..e848602c16 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -105,6 +105,8 @@ typedef union IplParameterBlock IplParameterBlock;
>   int s390_ipl_set_loadparm(uint8_t *loadparm);
>   int s390_ipl_pv_check_comp(IplParameterBlock *iplb);
>   void s390_ipl_update_diag308(IplParameterBlock *iplb);
> +int s390_ipl_prepare_pv_header(void);
> +int s390_ipl_pv_unpack(void);
>   void s390_ipl_prepare_cpu(S390CPU *cpu);
>   IplParameterBlock *s390_ipl_get_iplb(void);
>   IplParameterBlock *s390_ipl_get_iplb_secure(void);
> diff --git a/hw/s390x/pv.c b/hw/s390x/pv.c
> new file mode 100644
> index 0000000000..0218070322
> --- /dev/null
> +++ b/hw/s390x/pv.c
> @@ -0,0 +1,118 @@
> +/*
> + * Secure execution functions
> + *
> + * Copyright IBM Corp. 2019
> + * Author(s):
> + *  Janosch Frank <frankja@linux.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +#include "qemu/osdep.h"
> +#include <sys/ioctl.h>
> +
> +#include <linux/kvm.h>
> +
> +#include "qemu/error-report.h"
> +#include "sysemu/kvm.h"
> +#include "pv.h"
> +
> +static int s390_pv_cmd(uint32_t cmd, void *data)
> +{
> +    int rc;
> +    struct kvm_pv_cmd pv_cmd = {
> +        .cmd = cmd,
> +        .data = (uint64_t)data,
> +    };
> +
> +    rc = kvm_vm_ioctl(kvm_state, KVM_S390_PV_COMMAND, &pv_cmd);
> +    if (rc) {
> +        error_report("KVM PV command failed cmd: %d rc: %d", cmd, rc);
> +        exit(1);
> +    }
> +    return rc;
> +}
> +
> +static int s390_pv_cmd_vcpu(CPUState *cs, uint32_t cmd, void *data)
> +{
> +    int rc;
> +    struct kvm_pv_cmd pv_cmd = {
> +        .cmd = cmd,
> +        .data = (uint64_t)data,
> +    };
> +
> +    rc = kvm_vcpu_ioctl(cs, KVM_S390_PV_COMMAND_VCPU, &pv_cmd);
> +    if (rc) {
> +        error_report("KVM PV VCPU command failed cmd: %d rc: %d", cmd, rc);
> +        exit(1);
> +    }
> +    return rc;
> +}
> +
> +int s390_pv_vm_create(void)
> +{
> +    return s390_pv_cmd(KVM_PV_VM_CREATE, NULL);
> +}
> +
> +int s390_pv_vm_destroy(void)
> +{
> +    return s390_pv_cmd(KVM_PV_VM_DESTROY, NULL);
> +}
> +
> +int s390_pv_vcpu_create(CPUState *cs)
> +{
> +    return s390_pv_cmd_vcpu(cs, KVM_PV_VCPU_CREATE, NULL);
> +}
> +
> +int s390_pv_vcpu_destroy(CPUState *cs)
> +{
> +    S390CPU *cpu = S390_CPU(cs);
> +    CPUS390XState *env = &cpu->env;
> +    int rc;
> +
> +    rc = s390_pv_cmd_vcpu(cs, KVM_PV_VCPU_DESTROY, NULL);
> +    if (!rc) {
> +        env->pv = false;
> +    }
> +    return rc;
> +}
> +
> +int s390_pv_set_sec_parms(uint64_t origin, uint64_t length)
> +{
> +    struct kvm_s390_pv_sec_parm args = {
> +        .origin = origin,
> +        .length = length,
> +    };
> +
> +    return s390_pv_cmd(KVM_PV_VM_SET_SEC_PARMS, &args);
> +}
> +
> +/*
> + * Called for each component in the SE type IPL parameter block 0.
> + */
> +int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak)
> +{
> +    struct kvm_s390_pv_unp args = {
> +        .addr = addr,
> +        .size = size,
> +        .tweak = tweak,
> +    };
> +
> +    return s390_pv_cmd(KVM_PV_VM_UNPACK, &args);
> +}
> +
> +int s390_pv_perf_clear_reset(void)
> +{
> +    return s390_pv_cmd(KVM_PV_VM_PERF_CLEAR_RESET, NULL);
> +}
> +
> +int s390_pv_verify(void)
> +{
> +    return s390_pv_cmd(KVM_PV_VM_VERIFY, NULL);
> +}
> +
> +int s390_pv_unshare(void)
> +{
> +    return s390_pv_cmd(KVM_PV_VM_UNSHARE, NULL);
> +}
> diff --git a/hw/s390x/pv.h b/hw/s390x/pv.h
> new file mode 100644
> index 0000000000..eb074e4bc9
> --- /dev/null
> +++ b/hw/s390x/pv.h
> @@ -0,0 +1,26 @@
> +/*
> + * Protected Virtualization header
> + *
> + * Copyright IBM Corp. 2019
> + * Author(s):
> + *  Janosch Frank <frankja@linux.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +
> +#ifndef HW_S390_PV_H
> +#define HW_S390_PV_H
> +
> +int s390_pv_vm_create(void);
> +int s390_pv_vm_destroy(void);
> +int s390_pv_vcpu_destroy(CPUState *cs);
> +int s390_pv_vcpu_create(CPUState *cs);
> +int s390_pv_set_sec_parms(uint64_t origin, uint64_t length);
> +int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak);
> +int s390_pv_perf_clear_reset(void);
> +int s390_pv_verify(void);
> +int s390_pv_unshare(void);
> +
> +#endif /* HW_S390_PV_H */
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index c1d1440272..7262453616 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -41,6 +41,7 @@
>   #include "hw/qdev-properties.h"
>   #include "hw/s390x/tod.h"
>   #include "sysemu/sysemu.h"
> +#include "hw/s390x/pv.h"
>   
>   S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>   {
> @@ -322,6 +323,7 @@ static void s390_machine_reset(MachineState *machine)
>   {
>       enum s390_reset reset_type;
>       CPUState *cs, *t;
> +    S390CPU *cpu;
>   
>       /* get the reset parameters, reset them once done */
>       s390_ipl_get_reset_request(&cs, &reset_type);
> @@ -329,16 +331,10 @@ static void s390_machine_reset(MachineState *machine)
>       /* all CPUs are paused and synchronized at this point */
>       s390_cmma_reset();
>   
> -    switch (reset_type) {
> -    case S390_RESET_EXTERNAL:
> -    case S390_RESET_REIPL:
> -        qemu_devices_reset();
> -        s390_crypto_reset();
> +    cpu = S390_CPU(cs);
>   
> -        /* configure and start the ipl CPU only */
> -        run_on_cpu(cs, s390_do_cpu_ipl, RUN_ON_CPU_NULL);
> -        break;
> -    case S390_RESET_MODIFIED_CLEAR:
> +    switch (reset_type) {
> +    case S390_RESET_MODIFIED_CLEAR:  /* Subcode 0 */
>           CPU_FOREACH(t) {
>               run_on_cpu(t, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
>           }
> @@ -346,7 +342,7 @@ static void s390_machine_reset(MachineState *machine)
>           s390_crypto_reset();
>           run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
>           break;
> -    case S390_RESET_LOAD_NORMAL:
> +    case S390_RESET_LOAD_NORMAL: /* Subcode 1*/
>           CPU_FOREACH(t) {
>               if (t == cs) {
>                   continue;
> @@ -357,6 +353,35 @@ static void s390_machine_reset(MachineState *machine)
>           run_on_cpu(cs, s390_do_cpu_initial_reset, RUN_ON_CPU_NULL);
>           run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
>           break;
> +    case S390_RESET_EXTERNAL:
> +    case S390_RESET_REIPL: /* Subcode 4 */
> +        qemu_devices_reset();
> +        s390_crypto_reset();
> +        /* configure and start the ipl CPU only */
> +        run_on_cpu(cs, s390_do_cpu_ipl, RUN_ON_CPU_NULL);
> +        break;

Is there a way to modify this patch to not change unrelated code that 
heavily? Makes it harder to review.

> +    case S390_RESET_PV: /* Subcode 10 */
> +        subsystem_reset();
> +        s390_crypto_reset();
> +
> +        CPU_FOREACH(t) {
> +            run_on_cpu(t, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
> +        }
> +
> +        /* Create SE VM */
> +        s390_pv_vm_create();
> +        CPU_FOREACH(t) {
> +            s390_pv_vcpu_create(t);
> +        }
> +
> +        /* Set SE header and unpack */
> +        s390_ipl_prepare_pv_header();
> +        /* Decrypt image */
> +        s390_ipl_pv_unpack();
> +        /* Verify integrity */
> +        s390_pv_verify();
> +        s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
> +        break;
>       default:
>           g_assert_not_reached();
>       }
> diff --git a/target/s390x/cpu_features_def.inc.h b/target/s390x/cpu_features_def.inc.h
> index 31dff0d84e..60db28351d 100644
> --- a/target/s390x/cpu_features_def.inc.h
> +++ b/target/s390x/cpu_features_def.inc.h
> @@ -107,6 +107,7 @@ DEF_FEAT(DEFLATE_BASE, "deflate-base", STFL, 151, "Deflate-conversion facility (
>   DEF_FEAT(VECTOR_PACKED_DECIMAL_ENH, "vxpdeh", STFL, 152, "Vector-Packed-Decimal-Enhancement Facility")
>   DEF_FEAT(MSA_EXT_9, "msa9-base", STFL, 155, "Message-security-assist-extension-9 facility (excluding subfunctions)")
>   DEF_FEAT(ETOKEN, "etoken", STFL, 156, "Etoken facility")
> +DEF_FEAT(UNPACK, "unpack", STFL, 161, "Unpack facility")
>   
>   /* Features exposed via SCLP SCCB Byte 80 - 98  (bit numbers relative to byte-80) */
>   DEF_FEAT(SIE_GSLS, "gsls", SCLP_CONF_CHAR, 40, "SIE: Guest-storage-limit-suppression facility")
>
Janosch Frank Nov. 21, 2019, 11:33 a.m. UTC | #3
On 11/20/19 2:43 PM, Cornelia Huck wrote:
> On Wed, 20 Nov 2019 06:43:25 -0500
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> When a guest has saved a ipib of type 5 and call diagnose308 with
>> subcode 10, we have to setup the protected processing environment via
>> Ultravisor calls. The calls are done by KVM and are exposed via an API.
>>
>> The following steps are necessary:
>> 1. Create a VM (register it with the Ultravisor)
>> 2. Create secure CPUs for all of our current cpus
>> 3. Forward the secure header to the Ultravisor (has all information on
>> how to decrypt the image and VM information)
>> 4. Protect image pages from the host and decrypt them
>> 5. Verify the image integrity
>>
>> Only after step 5 a protected VM is allowed to run.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  hw/s390x/Makefile.objs              |   1 +
>>  hw/s390x/ipl.c                      |  33 ++++++++
>>  hw/s390x/ipl.h                      |   2 +
>>  hw/s390x/pv.c                       | 118 ++++++++++++++++++++++++++++
>>  hw/s390x/pv.h                       |  26 ++++++
>>  hw/s390x/s390-virtio-ccw.c          |  45 ++++++++---
>>  target/s390x/cpu_features_def.inc.h |   1 +
>>  7 files changed, 216 insertions(+), 10 deletions(-)
>>  create mode 100644 hw/s390x/pv.c
>>  create mode 100644 hw/s390x/pv.h
>>
>> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
>> index 94e57113d8..568bab9711 100644
>> --- a/hw/s390x/Makefile.objs
>> +++ b/hw/s390x/Makefile.objs
>> @@ -31,6 +31,7 @@ obj-y += tod-qemu.o
>>  obj-$(CONFIG_KVM) += tod-kvm.o
>>  obj-$(CONFIG_KVM) += s390-skeys-kvm.o
>>  obj-$(CONFIG_KVM) += s390-stattrib-kvm.o s390-mchk.o
>> +obj-$(CONFIG_KVM) += pv.o
> 
> As this is kvm only...
> 
>>  obj-y += s390-ccw.o
>>  obj-y += ap-device.o
>>  obj-y += ap-bridge.o
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index a077926f36..50501fcd27 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -33,6 +33,7 @@
>>  #include "qemu/cutils.h"
>>  #include "qemu/option.h"
>>  #include "exec/exec-all.h"
>> +#include "pv.h"
>>  
>>  #define KERN_IMAGE_START                0x010000UL
>>  #define LINUX_MAGIC_ADDR                0x010008UL
>> @@ -668,6 +669,38 @@ static void s390_ipl_prepare_qipl(S390CPU *cpu)
>>      cpu_physical_memory_unmap(addr, len, 1, len);
>>  }
>>  
>> +int s390_ipl_prepare_pv_header(void)
>> +{
>> +    int rc;
>> +    IplParameterBlock *iplb = s390_ipl_get_iplb_secure();
>> +    IPLBlockPV *ipib_pv = &iplb->pv;
>> +    void *hdr = g_malloc(ipib_pv->pv_header_len);
>> +
>> +    cpu_physical_memory_read(ipib_pv->pv_header_addr, hdr,
>> +                             ipib_pv->pv_header_len);
>> +    rc = s390_pv_set_sec_parms((uint64_t)hdr,
>> +                               ipib_pv->pv_header_len);
>> +    g_free(hdr);
>> +    return rc;
>> +}
>> +
>> +int s390_ipl_pv_unpack(void)
>> +{
>> +    int i, rc;
>> +    IplParameterBlock *iplb = s390_ipl_get_iplb_secure();
>> +    IPLBlockPV *ipib_pv = &iplb->pv;
>> +
>> +    for (i = 0; i < ipib_pv->num_comp; i++) {
>> +        rc = s390_pv_unpack(ipib_pv->components[i].addr,
>> +                            TARGET_PAGE_ALIGN(ipib_pv->components[i].size),
>> +                            ipib_pv->components[i].tweak_pref);
> 
> ...you probably need a stub version of the pv functions as well, right?

Yes, I'm working on it

> 
>> +        if (rc) {
>> +            return rc;
>> +        }
>> +    }
>> +    return rc;
>> +}
>> +
>>  void s390_ipl_prepare_cpu(S390CPU *cpu)
>>  {
>>      S390IPLState *ipl = get_ipl_device();
>
Janosch Frank Nov. 21, 2019, 2:25 p.m. UTC | #4
On 11/21/19 12:27 PM, David Hildenbrand wrote:
> On 20.11.19 12:43, Janosch Frank wrote:

>> @@ -357,6 +353,35 @@ static void s390_machine_reset(MachineState *machine)
>>           run_on_cpu(cs, s390_do_cpu_initial_reset, RUN_ON_CPU_NULL);
>>           run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
>>           break;
>> +    case S390_RESET_EXTERNAL:
>> +    case S390_RESET_REIPL: /* Subcode 4 */
>> +        qemu_devices_reset();
>> +        s390_crypto_reset();
>> +        /* configure and start the ipl CPU only */
>> +        run_on_cpu(cs, s390_do_cpu_ipl, RUN_ON_CPU_NULL);
>> +        break;
> 
> Is there a way to modify this patch to not change unrelated code that 
> heavily? Makes it harder to review.

https://github.com/frankjaa/qemu/commit/8c53d5c8a6bbcc53496c7a2877c7cbffc435b708

And please trim your emails.

> 
>> +    case S390_RESET_PV: /* Subcode 10 */
>> +        subsystem_reset();
>> +        s390_crypto_reset();
>> +
>> +        CPU_FOREACH(t) {
>> +            run_on_cpu(t, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
>> +        }
>> +
>> +        /* Create SE VM */
>> +        s390_pv_vm_create();
>> +        CPU_FOREACH(t) {
>> +            s390_pv_vcpu_create(t);
>> +        }
>> +
>> +        /* Set SE header and unpack */
>> +        s390_ipl_prepare_pv_header();
>> +        /* Decrypt image */
>> +        s390_ipl_pv_unpack();
>> +        /* Verify integrity */
>> +        s390_pv_verify();
>> +        s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
>> +        break;
>>       default:
>>           g_assert_not_reached();
>>       }
>> diff --git a/target/s390x/cpu_features_def.inc.h b/target/s390x/cpu_features_def.inc.h
>> index 31dff0d84e..60db28351d 100644
>> --- a/target/s390x/cpu_features_def.inc.h
>> +++ b/target/s390x/cpu_features_def.inc.h
>> @@ -107,6 +107,7 @@ DEF_FEAT(DEFLATE_BASE, "deflate-base", STFL, 151, "Deflate-conversion facility (
>>   DEF_FEAT(VECTOR_PACKED_DECIMAL_ENH, "vxpdeh", STFL, 152, "Vector-Packed-Decimal-Enhancement Facility")
>>   DEF_FEAT(MSA_EXT_9, "msa9-base", STFL, 155, "Message-security-assist-extension-9 facility (excluding subfunctions)")
>>   DEF_FEAT(ETOKEN, "etoken", STFL, 156, "Etoken facility")
>> +DEF_FEAT(UNPACK, "unpack", STFL, 161, "Unpack facility")
>>   
>>   /* Features exposed via SCLP SCCB Byte 80 - 98  (bit numbers relative to byte-80) */
>>   DEF_FEAT(SIE_GSLS, "gsls", SCLP_CONF_CHAR, 40, "SIE: Guest-storage-limit-suppression facility")
>>
> 
>
David Hildenbrand Nov. 21, 2019, 2:28 p.m. UTC | #5
On 21.11.19 15:25, Janosch Frank wrote:
> On 11/21/19 12:27 PM, David Hildenbrand wrote:
>> On 20.11.19 12:43, Janosch Frank wrote:
> 
>>> @@ -357,6 +353,35 @@ static void s390_machine_reset(MachineState *machine)
>>>            run_on_cpu(cs, s390_do_cpu_initial_reset, RUN_ON_CPU_NULL);
>>>            run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
>>>            break;
>>> +    case S390_RESET_EXTERNAL:
>>> +    case S390_RESET_REIPL: /* Subcode 4 */
>>> +        qemu_devices_reset();
>>> +        s390_crypto_reset();
>>> +        /* configure and start the ipl CPU only */
>>> +        run_on_cpu(cs, s390_do_cpu_ipl, RUN_ON_CPU_NULL);
>>> +        break;
>>
>> Is there a way to modify this patch to not change unrelated code that
>> heavily? Makes it harder to review.
> 
> https://github.com/frankjaa/qemu/commit/8c53d5c8a6bbcc53496c7a2877c7cbffc435b708
> 
> And please trim your emails.
> 

If you use Thunderbird I suggest QuoteCollapse ... because nobody got 
time for that ;)
Christian Borntraeger Nov. 21, 2019, 2:31 p.m. UTC | #6
On 21.11.19 15:28, David Hildenbrand wrote:
>> And please trim your emails.
>>
> 
> If you use Thunderbird I suggest QuoteCollapse ... because nobody got time for that ;)

neat.
Janosch Frank Nov. 21, 2019, 2:32 p.m. UTC | #7
On 11/21/19 3:31 PM, Christian Borntraeger wrote:
> 
> 
> On 21.11.19 15:28, David Hildenbrand wrote:
>>> And please trim your emails.
>>>
>>
>> If you use Thunderbird I suggest QuoteCollapse ... because nobody got time for that ;)
> 
> neat.
> 

Yeah, seems like I'm already too old-school for fancy addons at my young
age ;-)
Cornelia Huck Nov. 22, 2019, 1:39 p.m. UTC | #8
On Wed, 20 Nov 2019 06:43:25 -0500
Janosch Frank <frankja@linux.ibm.com> wrote:

> When a guest has saved a ipib of type 5 and call diagnose308 with
> subcode 10, we have to setup the protected processing environment via
> Ultravisor calls. The calls are done by KVM and are exposed via an API.
> 
> The following steps are necessary:
> 1. Create a VM (register it with the Ultravisor)
> 2. Create secure CPUs for all of our current cpus
> 3. Forward the secure header to the Ultravisor (has all information on
> how to decrypt the image and VM information)
> 4. Protect image pages from the host and decrypt them
> 5. Verify the image integrity
> 
> Only after step 5 a protected VM is allowed to run.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  hw/s390x/Makefile.objs              |   1 +
>  hw/s390x/ipl.c                      |  33 ++++++++
>  hw/s390x/ipl.h                      |   2 +
>  hw/s390x/pv.c                       | 118 ++++++++++++++++++++++++++++
>  hw/s390x/pv.h                       |  26 ++++++
>  hw/s390x/s390-virtio-ccw.c          |  45 ++++++++---
>  target/s390x/cpu_features_def.inc.h |   1 +
>  7 files changed, 216 insertions(+), 10 deletions(-)
>  create mode 100644 hw/s390x/pv.c
>  create mode 100644 hw/s390x/pv.h
> 

> +static int s390_pv_cmd(uint32_t cmd, void *data)
> +{
> +    int rc;
> +    struct kvm_pv_cmd pv_cmd = {
> +        .cmd = cmd,
> +        .data = (uint64_t)data,
> +    };
> +
> +    rc = kvm_vm_ioctl(kvm_state, KVM_S390_PV_COMMAND, &pv_cmd);
> +    if (rc) {
> +        error_report("KVM PV command failed cmd: %d rc: %d", cmd, rc);
> +        exit(1);
> +    }
> +    return rc;
> +}
> +
> +static int s390_pv_cmd_vcpu(CPUState *cs, uint32_t cmd, void *data)
> +{
> +    int rc;
> +    struct kvm_pv_cmd pv_cmd = {
> +        .cmd = cmd,
> +        .data = (uint64_t)data,
> +    };
> +
> +    rc = kvm_vcpu_ioctl(cs, KVM_S390_PV_COMMAND_VCPU, &pv_cmd);
> +    if (rc) {
> +        error_report("KVM PV VCPU command failed cmd: %d rc: %d", cmd, rc);
> +        exit(1);
> +    }
> +    return rc;
> +}

If you do a hard exit for any rc != 0 anyway, returning rc does not
sound very useful :)
Janosch Frank Nov. 22, 2019, 1:49 p.m. UTC | #9
On 11/22/19 2:39 PM, Cornelia Huck wrote:
> On Wed, 20 Nov 2019 06:43:25 -0500
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> When a guest has saved a ipib of type 5 and call diagnose308 with
>> subcode 10, we have to setup the protected processing environment via
>> Ultravisor calls. The calls are done by KVM and are exposed via an API.
>>
>> The following steps are necessary:
>> 1. Create a VM (register it with the Ultravisor)
>> 2. Create secure CPUs for all of our current cpus
>> 3. Forward the secure header to the Ultravisor (has all information on
>> how to decrypt the image and VM information)
>> 4. Protect image pages from the host and decrypt them
>> 5. Verify the image integrity
>>
>> Only after step 5 a protected VM is allowed to run.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  hw/s390x/Makefile.objs              |   1 +
>>  hw/s390x/ipl.c                      |  33 ++++++++
>>  hw/s390x/ipl.h                      |   2 +
>>  hw/s390x/pv.c                       | 118 ++++++++++++++++++++++++++++
>>  hw/s390x/pv.h                       |  26 ++++++
>>  hw/s390x/s390-virtio-ccw.c          |  45 ++++++++---
>>  target/s390x/cpu_features_def.inc.h |   1 +
>>  7 files changed, 216 insertions(+), 10 deletions(-)
>>  create mode 100644 hw/s390x/pv.c
>>  create mode 100644 hw/s390x/pv.h
>>
> 
>> +static int s390_pv_cmd(uint32_t cmd, void *data)
>> +{
>> +    int rc;
>> +    struct kvm_pv_cmd pv_cmd = {
>> +        .cmd = cmd,
>> +        .data = (uint64_t)data,
>> +    };
>> +
>> +    rc = kvm_vm_ioctl(kvm_state, KVM_S390_PV_COMMAND, &pv_cmd);
>> +    if (rc) {
>> +        error_report("KVM PV command failed cmd: %d rc: %d", cmd, rc);
>> +        exit(1);
>> +    }
>> +    return rc;
>> +}
>> +
>> +static int s390_pv_cmd_vcpu(CPUState *cs, uint32_t cmd, void *data)
>> +{
>> +    int rc;
>> +    struct kvm_pv_cmd pv_cmd = {
>> +        .cmd = cmd,
>> +        .data = (uint64_t)data,
>> +    };
>> +
>> +    rc = kvm_vcpu_ioctl(cs, KVM_S390_PV_COMMAND_VCPU, &pv_cmd);
>> +    if (rc) {
>> +        error_report("KVM PV VCPU command failed cmd: %d rc: %d", cmd, rc);
>> +        exit(1);
>> +    }
>> +    return rc;
>> +}
> 
> If you do a hard exit for any rc != 0 anyway, returning rc does not
> sound very useful :)
> 

Yeah, that's also what Pierre said.
I'll need to rethink if there's actually a call where we could recover
from an error and if not make everything void.
Thomas Huth Nov. 28, 2019, 2:07 p.m. UTC | #10
On 20/11/2019 12.43, Janosch Frank wrote:
> When a guest has saved a ipib of type 5 and call diagnose308 with
> subcode 10, we have to setup the protected processing environment via
> Ultravisor calls. The calls are done by KVM and are exposed via an API.
> 
> The following steps are necessary:
> 1. Create a VM (register it with the Ultravisor)
> 2. Create secure CPUs for all of our current cpus
> 3. Forward the secure header to the Ultravisor (has all information on
> how to decrypt the image and VM information)
> 4. Protect image pages from the host and decrypt them
> 5. Verify the image integrity
> 
> Only after step 5 a protected VM is allowed to run.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
[...]
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index a077926f36..50501fcd27 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -33,6 +33,7 @@
>  #include "qemu/cutils.h"
>  #include "qemu/option.h"
>  #include "exec/exec-all.h"
> +#include "pv.h"
>  
>  #define KERN_IMAGE_START                0x010000UL
>  #define LINUX_MAGIC_ADDR                0x010008UL
> @@ -668,6 +669,38 @@ static void s390_ipl_prepare_qipl(S390CPU *cpu)
>      cpu_physical_memory_unmap(addr, len, 1, len);
>  }
>  
> +int s390_ipl_prepare_pv_header(void)
> +{
> +    int rc;
> +    IplParameterBlock *iplb = s390_ipl_get_iplb_secure();
> +    IPLBlockPV *ipib_pv = &iplb->pv;
> +    void *hdr = g_malloc(ipib_pv->pv_header_len);
> +
> +    cpu_physical_memory_read(ipib_pv->pv_header_addr, hdr,
> +                             ipib_pv->pv_header_len);
> +    rc = s390_pv_set_sec_parms((uint64_t)hdr,
> +                               ipib_pv->pv_header_len);
> +    g_free(hdr);
> +    return rc;
> +}
> +
> +int s390_ipl_pv_unpack(void)
> +{
> +    int i, rc;
> +    IplParameterBlock *iplb = s390_ipl_get_iplb_secure();
> +    IPLBlockPV *ipib_pv = &iplb->pv;
> +
> +    for (i = 0; i < ipib_pv->num_comp; i++) {
> +        rc = s390_pv_unpack(ipib_pv->components[i].addr,
> +                            TARGET_PAGE_ALIGN(ipib_pv->components[i].size),
> +                            ipib_pv->components[i].tweak_pref);
> +        if (rc) {
> +            return rc;
> +        }
> +    }
> +    return rc;
> +}

For both functions, s390_ipl_prepare_pv_header() and
s390_ipl_pv_unpack(), you're ignoring the return code at the calling
site, so errors go completely unnoticed. I suggest to either do an
error_report() here or at the calling site...

> diff --git a/hw/s390x/pv.c b/hw/s390x/pv.c
> new file mode 100644
> index 0000000000..0218070322
> --- /dev/null
> +++ b/hw/s390x/pv.c
> @@ -0,0 +1,118 @@
> +/*
> + * Secure execution functions
> + *
> + * Copyright IBM Corp. 2019
> + * Author(s):
> + *  Janosch Frank <frankja@linux.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +#include "qemu/osdep.h"
> +#include <sys/ioctl.h>
> +
> +#include <linux/kvm.h>
> +
> +#include "qemu/error-report.h"
> +#include "sysemu/kvm.h"
> +#include "pv.h"
> +
> +static int s390_pv_cmd(uint32_t cmd, void *data)
> +{
> +    int rc;
> +    struct kvm_pv_cmd pv_cmd = {
> +        .cmd = cmd,
> +        .data = (uint64_t)data,
> +    };
> +
> +    rc = kvm_vm_ioctl(kvm_state, KVM_S390_PV_COMMAND, &pv_cmd);
> +    if (rc) {
> +        error_report("KVM PV command failed cmd: %d rc: %d", cmd, rc);
> +        exit(1);
> +    }
> +    return rc;
> +}
> +
> +static int s390_pv_cmd_vcpu(CPUState *cs, uint32_t cmd, void *data)
> +{
> +    int rc;
> +    struct kvm_pv_cmd pv_cmd = {
> +        .cmd = cmd,
> +        .data = (uint64_t)data,
> +    };
> +
> +    rc = kvm_vcpu_ioctl(cs, KVM_S390_PV_COMMAND_VCPU, &pv_cmd);
> +    if (rc) {
> +        error_report("KVM PV VCPU command failed cmd: %d rc: %d", cmd, rc);
> +        exit(1);
> +    }
> +    return rc;
> +}

... or even better, since these functions exit on error anyway, make
these functions and all the others "void" instead?

 Thomas
Janosch Frank Nov. 28, 2019, 2:20 p.m. UTC | #11
On 11/28/19 3:07 PM, Thomas Huth wrote:
> On 20/11/2019 12.43, Janosch Frank wrote:
>> When a guest has saved a ipib of type 5 and call diagnose308 with
>> subcode 10, we have to setup the protected processing environment via
>> Ultravisor calls. The calls are done by KVM and are exposed via an API.
>>
>> The following steps are necessary:
>> 1. Create a VM (register it with the Ultravisor)
>> 2. Create secure CPUs for all of our current cpus
>> 3. Forward the secure header to the Ultravisor (has all information on
>> how to decrypt the image and VM information)
>> 4. Protect image pages from the host and decrypt them
>> 5. Verify the image integrity
>>
>> Only after step 5 a protected VM is allowed to run.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
> [...]
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index a077926f36..50501fcd27 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -33,6 +33,7 @@
>>  #include "qemu/cutils.h"
>>  #include "qemu/option.h"
>>  #include "exec/exec-all.h"
>> +#include "pv.h"
>>  
>>  #define KERN_IMAGE_START                0x010000UL
>>  #define LINUX_MAGIC_ADDR                0x010008UL
>> @@ -668,6 +669,38 @@ static void s390_ipl_prepare_qipl(S390CPU *cpu)
>>      cpu_physical_memory_unmap(addr, len, 1, len);
>>  }
>>  
>> +int s390_ipl_prepare_pv_header(void)
>> +{
>> +    int rc;
>> +    IplParameterBlock *iplb = s390_ipl_get_iplb_secure();
>> +    IPLBlockPV *ipib_pv = &iplb->pv;
>> +    void *hdr = g_malloc(ipib_pv->pv_header_len);
>> +
>> +    cpu_physical_memory_read(ipib_pv->pv_header_addr, hdr,
>> +                             ipib_pv->pv_header_len);
>> +    rc = s390_pv_set_sec_parms((uint64_t)hdr,
>> +                               ipib_pv->pv_header_len);
>> +    g_free(hdr);
>> +    return rc;
>> +}
>> +
>> +int s390_ipl_pv_unpack(void)
>> +{
>> +    int i, rc;
>> +    IplParameterBlock *iplb = s390_ipl_get_iplb_secure();
>> +    IPLBlockPV *ipib_pv = &iplb->pv;
>> +
>> +    for (i = 0; i < ipib_pv->num_comp; i++) {
>> +        rc = s390_pv_unpack(ipib_pv->components[i].addr,
>> +                            TARGET_PAGE_ALIGN(ipib_pv->components[i].size),
>> +                            ipib_pv->components[i].tweak_pref);
>> +        if (rc) {
>> +            return rc;
>> +        }
>> +    }
>> +    return rc;
>> +}
> 
> For both functions, s390_ipl_prepare_pv_header() and
> s390_ipl_pv_unpack(), you're ignoring the return code at the calling
> site, so errors go completely unnoticed. I suggest to either do an
> error_report() here or at the calling site...
> 
>> diff --git a/hw/s390x/pv.c b/hw/s390x/pv.c
>> new file mode 100644
>> index 0000000000..0218070322
>> --- /dev/null
>> +++ b/hw/s390x/pv.c
>> @@ -0,0 +1,118 @@
>> +/*
>> + * Secure execution functions
>> + *
>> + * Copyright IBM Corp. 2019
>> + * Author(s):
>> + *  Janosch Frank <frankja@linux.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
>> + * your option) any later version. See the COPYING file in the top-level
>> + * directory.
>> + */
>> +#include "qemu/osdep.h"
>> +#include <sys/ioctl.h>
>> +
>> +#include <linux/kvm.h>
>> +
>> +#include "qemu/error-report.h"
>> +#include "sysemu/kvm.h"
>> +#include "pv.h"
>> +
>> +static int s390_pv_cmd(uint32_t cmd, void *data)
>> +{
>> +    int rc;
>> +    struct kvm_pv_cmd pv_cmd = {
>> +        .cmd = cmd,
>> +        .data = (uint64_t)data,
>> +    };
>> +
>> +    rc = kvm_vm_ioctl(kvm_state, KVM_S390_PV_COMMAND, &pv_cmd);
>> +    if (rc) {
>> +        error_report("KVM PV command failed cmd: %d rc: %d", cmd, rc);
>> +        exit(1);
>> +    }
>> +    return rc;
>> +}
>> +
>> +static int s390_pv_cmd_vcpu(CPUState *cs, uint32_t cmd, void *data)
>> +{
>> +    int rc;
>> +    struct kvm_pv_cmd pv_cmd = {
>> +        .cmd = cmd,
>> +        .data = (uint64_t)data,
>> +    };
>> +
>> +    rc = kvm_vcpu_ioctl(cs, KVM_S390_PV_COMMAND_VCPU, &pv_cmd);
>> +    if (rc) {
>> +        error_report("KVM PV VCPU command failed cmd: %d rc: %d", cmd, rc);
>> +        exit(1);
>> +    }
>> +    return rc;
>> +}
> 
> ... or even better, since these functions exit on error anyway, make
> these functions and all the others "void" instead?
> 
>  Thomas
> 

You're not the first stating this idea :-)
We might get a return code for diag308 that signals that we failed to
start the secure guest and then I might need them again.

If we don't get a rc, I'll definitely make all functions void, if we do
I'll make nearly all void:)
diff mbox series

Patch

diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index 94e57113d8..568bab9711 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -31,6 +31,7 @@  obj-y += tod-qemu.o
 obj-$(CONFIG_KVM) += tod-kvm.o
 obj-$(CONFIG_KVM) += s390-skeys-kvm.o
 obj-$(CONFIG_KVM) += s390-stattrib-kvm.o s390-mchk.o
+obj-$(CONFIG_KVM) += pv.o
 obj-y += s390-ccw.o
 obj-y += ap-device.o
 obj-y += ap-bridge.o
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index a077926f36..50501fcd27 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -33,6 +33,7 @@ 
 #include "qemu/cutils.h"
 #include "qemu/option.h"
 #include "exec/exec-all.h"
+#include "pv.h"
 
 #define KERN_IMAGE_START                0x010000UL
 #define LINUX_MAGIC_ADDR                0x010008UL
@@ -668,6 +669,38 @@  static void s390_ipl_prepare_qipl(S390CPU *cpu)
     cpu_physical_memory_unmap(addr, len, 1, len);
 }
 
+int s390_ipl_prepare_pv_header(void)
+{
+    int rc;
+    IplParameterBlock *iplb = s390_ipl_get_iplb_secure();
+    IPLBlockPV *ipib_pv = &iplb->pv;
+    void *hdr = g_malloc(ipib_pv->pv_header_len);
+
+    cpu_physical_memory_read(ipib_pv->pv_header_addr, hdr,
+                             ipib_pv->pv_header_len);
+    rc = s390_pv_set_sec_parms((uint64_t)hdr,
+                               ipib_pv->pv_header_len);
+    g_free(hdr);
+    return rc;
+}
+
+int s390_ipl_pv_unpack(void)
+{
+    int i, rc;
+    IplParameterBlock *iplb = s390_ipl_get_iplb_secure();
+    IPLBlockPV *ipib_pv = &iplb->pv;
+
+    for (i = 0; i < ipib_pv->num_comp; i++) {
+        rc = s390_pv_unpack(ipib_pv->components[i].addr,
+                            TARGET_PAGE_ALIGN(ipib_pv->components[i].size),
+                            ipib_pv->components[i].tweak_pref);
+        if (rc) {
+            return rc;
+        }
+    }
+    return rc;
+}
+
 void s390_ipl_prepare_cpu(S390CPU *cpu)
 {
     S390IPLState *ipl = get_ipl_device();
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 7b8a493509..e848602c16 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -105,6 +105,8 @@  typedef union IplParameterBlock IplParameterBlock;
 int s390_ipl_set_loadparm(uint8_t *loadparm);
 int s390_ipl_pv_check_comp(IplParameterBlock *iplb);
 void s390_ipl_update_diag308(IplParameterBlock *iplb);
+int s390_ipl_prepare_pv_header(void);
+int s390_ipl_pv_unpack(void);
 void s390_ipl_prepare_cpu(S390CPU *cpu);
 IplParameterBlock *s390_ipl_get_iplb(void);
 IplParameterBlock *s390_ipl_get_iplb_secure(void);
diff --git a/hw/s390x/pv.c b/hw/s390x/pv.c
new file mode 100644
index 0000000000..0218070322
--- /dev/null
+++ b/hw/s390x/pv.c
@@ -0,0 +1,118 @@ 
+/*
+ * Secure execution functions
+ *
+ * Copyright IBM Corp. 2019
+ * Author(s):
+ *  Janosch Frank <frankja@linux.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+#include "qemu/osdep.h"
+#include <sys/ioctl.h>
+
+#include <linux/kvm.h>
+
+#include "qemu/error-report.h"
+#include "sysemu/kvm.h"
+#include "pv.h"
+
+static int s390_pv_cmd(uint32_t cmd, void *data)
+{
+    int rc;
+    struct kvm_pv_cmd pv_cmd = {
+        .cmd = cmd,
+        .data = (uint64_t)data,
+    };
+
+    rc = kvm_vm_ioctl(kvm_state, KVM_S390_PV_COMMAND, &pv_cmd);
+    if (rc) {
+        error_report("KVM PV command failed cmd: %d rc: %d", cmd, rc);
+        exit(1);
+    }
+    return rc;
+}
+
+static int s390_pv_cmd_vcpu(CPUState *cs, uint32_t cmd, void *data)
+{
+    int rc;
+    struct kvm_pv_cmd pv_cmd = {
+        .cmd = cmd,
+        .data = (uint64_t)data,
+    };
+
+    rc = kvm_vcpu_ioctl(cs, KVM_S390_PV_COMMAND_VCPU, &pv_cmd);
+    if (rc) {
+        error_report("KVM PV VCPU command failed cmd: %d rc: %d", cmd, rc);
+        exit(1);
+    }
+    return rc;
+}
+
+int s390_pv_vm_create(void)
+{
+    return s390_pv_cmd(KVM_PV_VM_CREATE, NULL);
+}
+
+int s390_pv_vm_destroy(void)
+{
+    return s390_pv_cmd(KVM_PV_VM_DESTROY, NULL);
+}
+
+int s390_pv_vcpu_create(CPUState *cs)
+{
+    return s390_pv_cmd_vcpu(cs, KVM_PV_VCPU_CREATE, NULL);
+}
+
+int s390_pv_vcpu_destroy(CPUState *cs)
+{
+    S390CPU *cpu = S390_CPU(cs);
+    CPUS390XState *env = &cpu->env;
+    int rc;
+
+    rc = s390_pv_cmd_vcpu(cs, KVM_PV_VCPU_DESTROY, NULL);
+    if (!rc) {
+        env->pv = false;
+    }
+    return rc;
+}
+
+int s390_pv_set_sec_parms(uint64_t origin, uint64_t length)
+{
+    struct kvm_s390_pv_sec_parm args = {
+        .origin = origin,
+        .length = length,
+    };
+
+    return s390_pv_cmd(KVM_PV_VM_SET_SEC_PARMS, &args);
+}
+
+/*
+ * Called for each component in the SE type IPL parameter block 0.
+ */
+int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak)
+{
+    struct kvm_s390_pv_unp args = {
+        .addr = addr,
+        .size = size,
+        .tweak = tweak,
+    };
+
+    return s390_pv_cmd(KVM_PV_VM_UNPACK, &args);
+}
+
+int s390_pv_perf_clear_reset(void)
+{
+    return s390_pv_cmd(KVM_PV_VM_PERF_CLEAR_RESET, NULL);
+}
+
+int s390_pv_verify(void)
+{
+    return s390_pv_cmd(KVM_PV_VM_VERIFY, NULL);
+}
+
+int s390_pv_unshare(void)
+{
+    return s390_pv_cmd(KVM_PV_VM_UNSHARE, NULL);
+}
diff --git a/hw/s390x/pv.h b/hw/s390x/pv.h
new file mode 100644
index 0000000000..eb074e4bc9
--- /dev/null
+++ b/hw/s390x/pv.h
@@ -0,0 +1,26 @@ 
+/*
+ * Protected Virtualization header
+ *
+ * Copyright IBM Corp. 2019
+ * Author(s):
+ *  Janosch Frank <frankja@linux.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#ifndef HW_S390_PV_H
+#define HW_S390_PV_H
+
+int s390_pv_vm_create(void);
+int s390_pv_vm_destroy(void);
+int s390_pv_vcpu_destroy(CPUState *cs);
+int s390_pv_vcpu_create(CPUState *cs);
+int s390_pv_set_sec_parms(uint64_t origin, uint64_t length);
+int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak);
+int s390_pv_perf_clear_reset(void);
+int s390_pv_verify(void);
+int s390_pv_unshare(void);
+
+#endif /* HW_S390_PV_H */
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index c1d1440272..7262453616 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -41,6 +41,7 @@ 
 #include "hw/qdev-properties.h"
 #include "hw/s390x/tod.h"
 #include "sysemu/sysemu.h"
+#include "hw/s390x/pv.h"
 
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
 {
@@ -322,6 +323,7 @@  static void s390_machine_reset(MachineState *machine)
 {
     enum s390_reset reset_type;
     CPUState *cs, *t;
+    S390CPU *cpu;
 
     /* get the reset parameters, reset them once done */
     s390_ipl_get_reset_request(&cs, &reset_type);
@@ -329,16 +331,10 @@  static void s390_machine_reset(MachineState *machine)
     /* all CPUs are paused and synchronized at this point */
     s390_cmma_reset();
 
-    switch (reset_type) {
-    case S390_RESET_EXTERNAL:
-    case S390_RESET_REIPL:
-        qemu_devices_reset();
-        s390_crypto_reset();
+    cpu = S390_CPU(cs);
 
-        /* configure and start the ipl CPU only */
-        run_on_cpu(cs, s390_do_cpu_ipl, RUN_ON_CPU_NULL);
-        break;
-    case S390_RESET_MODIFIED_CLEAR:
+    switch (reset_type) {
+    case S390_RESET_MODIFIED_CLEAR:  /* Subcode 0 */
         CPU_FOREACH(t) {
             run_on_cpu(t, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
         }
@@ -346,7 +342,7 @@  static void s390_machine_reset(MachineState *machine)
         s390_crypto_reset();
         run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
         break;
-    case S390_RESET_LOAD_NORMAL:
+    case S390_RESET_LOAD_NORMAL: /* Subcode 1*/
         CPU_FOREACH(t) {
             if (t == cs) {
                 continue;
@@ -357,6 +353,35 @@  static void s390_machine_reset(MachineState *machine)
         run_on_cpu(cs, s390_do_cpu_initial_reset, RUN_ON_CPU_NULL);
         run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
         break;
+    case S390_RESET_EXTERNAL:
+    case S390_RESET_REIPL: /* Subcode 4 */
+        qemu_devices_reset();
+        s390_crypto_reset();
+        /* configure and start the ipl CPU only */
+        run_on_cpu(cs, s390_do_cpu_ipl, RUN_ON_CPU_NULL);
+        break;
+    case S390_RESET_PV: /* Subcode 10 */
+        subsystem_reset();
+        s390_crypto_reset();
+
+        CPU_FOREACH(t) {
+            run_on_cpu(t, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
+        }
+
+        /* Create SE VM */
+        s390_pv_vm_create();
+        CPU_FOREACH(t) {
+            s390_pv_vcpu_create(t);
+        }
+
+        /* Set SE header and unpack */
+        s390_ipl_prepare_pv_header();
+        /* Decrypt image */
+        s390_ipl_pv_unpack();
+        /* Verify integrity */
+        s390_pv_verify();
+        s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
+        break;
     default:
         g_assert_not_reached();
     }
diff --git a/target/s390x/cpu_features_def.inc.h b/target/s390x/cpu_features_def.inc.h
index 31dff0d84e..60db28351d 100644
--- a/target/s390x/cpu_features_def.inc.h
+++ b/target/s390x/cpu_features_def.inc.h
@@ -107,6 +107,7 @@  DEF_FEAT(DEFLATE_BASE, "deflate-base", STFL, 151, "Deflate-conversion facility (
 DEF_FEAT(VECTOR_PACKED_DECIMAL_ENH, "vxpdeh", STFL, 152, "Vector-Packed-Decimal-Enhancement Facility")
 DEF_FEAT(MSA_EXT_9, "msa9-base", STFL, 155, "Message-security-assist-extension-9 facility (excluding subfunctions)")
 DEF_FEAT(ETOKEN, "etoken", STFL, 156, "Etoken facility")
+DEF_FEAT(UNPACK, "unpack", STFL, 161, "Unpack facility")
 
 /* Features exposed via SCLP SCCB Byte 80 - 98  (bit numbers relative to byte-80) */
 DEF_FEAT(SIE_GSLS, "gsls", SCLP_CONF_CHAR, 40, "SIE: Guest-storage-limit-suppression facility")