Re: [PATCH v3 05/17] s390x: protvirt: Support unpack facility
On 2/20/20 11:39 AM, Cornelia Huck wrote:
> On Fri, 14 Feb 2020 10:16:24 -0500
> Janosch Frank wrote:
>
>> When a guest has saved a ipib of type 5 and call diagnose308 with
>
> s/call/calls/
>
>> 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
>> Signed-off-by: Christian Borntraeger [Changes
>> to machine]
>> ---
>> hw/s390x/Makefile.objs | 1 +
>> hw/s390x/ipl.c | 32 ++
>> hw/s390x/ipl.h | 2 +
>> hw/s390x/pv.c | 154
>> hw/s390x/pv.h | 38 +++
>> hw/s390x/s390-virtio-ccw.c | 79 ++
>> include/hw/s390x/s390-virtio-ccw.h | 1 +
>> target/s390x/cpu.c | 4 +
>> target/s390x/cpu.h | 1 +
>> target/s390x/cpu_features_def.inc.h | 1 +
>> 10 files changed, 313 insertions(+)
>> create mode 100644 hw/s390x/pv.c
>> create mode 100644 hw/s390x/pv.h
>
> (...)
>
>> diff --git a/hw/s390x/pv.c b/hw/s390x/pv.c
>> new file mode 100644
>> index 00..5b6a26cba9
>> --- /dev/null
>> +++ b/hw/s390x/pv.c
>> @@ -0,0 +1,154 @@
>> +/*
>> + * Secure execution functions
>> + *
>> + * Copyright IBM Corp. 2019
>
> Update the year?
ack.
>
>> + * Author(s):
>> + * Janosch Frank
>> + *
>> + * 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.
>> + */
>
> (...)
>
>> +void s390_pv_vm_destroy(void)
>> +{
>> + s390_pv_cmd_exit(KVM_PV_VM_DESTROY, NULL);
>
> Why does this exit()? Should Never Happen?
Yes, and we can't recover from this.
>
>> +}
>> +
>> +int s390_pv_vcpu_create(CPUState *cs)
>> +{
>> +int rc;
>> +
>> +rc = s390_pv_cmd_vcpu(cs, KVM_PV_VCPU_CREATE, NULL);
>> +if (!rc) {
>> +S390_CPU(cs)->env.pv = true;
>> +}
>> +
>> +return rc;
>> +}
>> +
>> +void s390_pv_vcpu_destroy(CPUState *cs)
>> +{
>> +s390_pv_cmd_vcpu_exit(cs, KVM_PV_VCPU_DESTROY, NULL);
>
> dito
>
>> +S390_CPU(cs)->env.pv = false;
>> +}
>
> (...)
>
>> +void s390_pv_perf_clear_reset(void)
>> +{
>> +s390_pv_cmd_exit(KVM_PV_VM_PREP_RESET, NULL);
>
> And here. Or is that because the machine should not be left around in
> an undefined state?
If it failed, we could only try again, there's no fixing the problem.
So I chose to rather exit instead of looping around something which most
likely will never recover after the first error.
>
>> +}
>> +
>> +int s390_pv_verify(void)
>> +{
>> +return s390_pv_cmd(KVM_PV_VM_VERIFY, NULL);
>> +}
>> +
>> +void s390_pv_unshare(void)
>> +{
>> +s390_pv_cmd_exit(KVM_PV_VM_UNSHARE_ALL, NULL);
>> +}
>> diff --git a/hw/s390x/pv.h b/hw/s390x/pv.h
>> new file mode 100644
>> index 00..7d20bdd12e
>> --- /dev/null
>> +++ b/hw/s390x/pv.h
>> @@ -0,0 +1,38 @@
>> +/*
>> + * Protected Virtualization header
>> + *
>> + * Copyright IBM Corp. 2019
>
> Year++
>
>> + * Author(s):
>> + * Janosch Frank
>> + *
>> + * 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
>> +
>> +#ifdef CONFIG_KVM
>> +int s390_pv_vm_create(void);
>> +void s390_pv_vm_destroy(void);
>> +void 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);
>> +void s390_pv_perf_clear_reset(void);
>> +int s390_pv_verify(void);
>> +void s390_pv_unshare(void);
>> +#else
>> +int s390_pv_vm_create(void) { return 0; }
>
> I'm wondering why you return 0 here (and below). These function should
> not be called for !KVM, but just to help catch logic error, use -EINVAL
> or so?
>
>> +void s390_pv_vm_destroy(void) {}
>> +void s390_pv_vcpu_destroy(CPUState *cs) {}
>> +int s390_pv_vcpu_create(CPUState *cs) { return 0; }
>> +int s390_pv_set_sec_parms(uint64_t origin, uint64_t length) { return 0; }
>> +int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak) { return
>> 0: }
>> +void s390_pv_perf_clear_reset(void) {}
>> +int s390_pv_verify(void) { return 0; }
>> +void s390_pv_unshare(void) {}
>> +#endif
>> +
>> +#endif /* HW_S390_PV_H */
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index
Re: [PATCH v3 05/17] s390x: protvirt: Support unpack facility
On Fri, 14 Feb 2020 10:16:24 -0500
Janosch Frank wrote:
> When a guest has saved a ipib of type 5 and call diagnose308 with
s/call/calls/
> 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
> Signed-off-by: Christian Borntraeger [Changes
> to machine]
> ---
> hw/s390x/Makefile.objs | 1 +
> hw/s390x/ipl.c | 32 ++
> hw/s390x/ipl.h | 2 +
> hw/s390x/pv.c | 154
> hw/s390x/pv.h | 38 +++
> hw/s390x/s390-virtio-ccw.c | 79 ++
> include/hw/s390x/s390-virtio-ccw.h | 1 +
> target/s390x/cpu.c | 4 +
> target/s390x/cpu.h | 1 +
> target/s390x/cpu_features_def.inc.h | 1 +
> 10 files changed, 313 insertions(+)
> create mode 100644 hw/s390x/pv.c
> create mode 100644 hw/s390x/pv.h
(...)
> diff --git a/hw/s390x/pv.c b/hw/s390x/pv.c
> new file mode 100644
> index 00..5b6a26cba9
> --- /dev/null
> +++ b/hw/s390x/pv.c
> @@ -0,0 +1,154 @@
> +/*
> + * Secure execution functions
> + *
> + * Copyright IBM Corp. 2019
Update the year?
> + * Author(s):
> + * Janosch Frank
> + *
> + * 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.
> + */
(...)
> +void s390_pv_vm_destroy(void)
> +{
> + s390_pv_cmd_exit(KVM_PV_VM_DESTROY, NULL);
Why does this exit()? Should Never Happen?
> +}
> +
> +int s390_pv_vcpu_create(CPUState *cs)
> +{
> +int rc;
> +
> +rc = s390_pv_cmd_vcpu(cs, KVM_PV_VCPU_CREATE, NULL);
> +if (!rc) {
> +S390_CPU(cs)->env.pv = true;
> +}
> +
> +return rc;
> +}
> +
> +void s390_pv_vcpu_destroy(CPUState *cs)
> +{
> +s390_pv_cmd_vcpu_exit(cs, KVM_PV_VCPU_DESTROY, NULL);
dito
> +S390_CPU(cs)->env.pv = false;
> +}
(...)
> +void s390_pv_perf_clear_reset(void)
> +{
> +s390_pv_cmd_exit(KVM_PV_VM_PREP_RESET, NULL);
And here. Or is that because the machine should not be left around in
an undefined state?
> +}
> +
> +int s390_pv_verify(void)
> +{
> +return s390_pv_cmd(KVM_PV_VM_VERIFY, NULL);
> +}
> +
> +void s390_pv_unshare(void)
> +{
> +s390_pv_cmd_exit(KVM_PV_VM_UNSHARE_ALL, NULL);
> +}
> diff --git a/hw/s390x/pv.h b/hw/s390x/pv.h
> new file mode 100644
> index 00..7d20bdd12e
> --- /dev/null
> +++ b/hw/s390x/pv.h
> @@ -0,0 +1,38 @@
> +/*
> + * Protected Virtualization header
> + *
> + * Copyright IBM Corp. 2019
Year++
> + * Author(s):
> + * Janosch Frank
> + *
> + * 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
> +
> +#ifdef CONFIG_KVM
> +int s390_pv_vm_create(void);
> +void s390_pv_vm_destroy(void);
> +void 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);
> +void s390_pv_perf_clear_reset(void);
> +int s390_pv_verify(void);
> +void s390_pv_unshare(void);
> +#else
> +int s390_pv_vm_create(void) { return 0; }
I'm wondering why you return 0 here (and below). These function should
not be called for !KVM, but just to help catch logic error, use -EINVAL
or so?
> +void s390_pv_vm_destroy(void) {}
> +void s390_pv_vcpu_destroy(CPUState *cs) {}
> +int s390_pv_vcpu_create(CPUState *cs) { return 0; }
> +int s390_pv_set_sec_parms(uint64_t origin, uint64_t length) { return 0; }
> +int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak) { return 0:
> }
> +void s390_pv_perf_clear_reset(void) {}
> +int s390_pv_verify(void) { return 0; }
> +void s390_pv_unshare(void) {}
> +#endif
> +
> +#endif /* HW_S390_PV_H */
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index e759eb5f83..5fa4372083 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)
> {
> @@ -240,9 +241,11 @@ static void s390_create_sclpconsole(const char *type,
> Chardev *chardev)
> static void ccw_init(MachineState *machine)
> {
> int ret;
> +
[PATCH v3 05/17] s390x: protvirt: Support unpack facility
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
Signed-off-by: Christian Borntraeger [Changes
to machine]
---
hw/s390x/Makefile.objs | 1 +
hw/s390x/ipl.c | 32 ++
hw/s390x/ipl.h | 2 +
hw/s390x/pv.c | 154
hw/s390x/pv.h | 38 +++
hw/s390x/s390-virtio-ccw.c | 79 ++
include/hw/s390x/s390-virtio-ccw.h | 1 +
target/s390x/cpu.c | 4 +
target/s390x/cpu.h | 1 +
target/s390x/cpu_features_def.inc.h | 1 +
10 files changed, 313 insertions(+)
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 e02ed80b68..a46a1c7894 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
+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 db97a888ff..3290832f1e 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_START0x01UL
#define LINUX_MAGIC_ADDR0x010008UL
@@ -677,6 +678,37 @@ static void s390_ipl_prepare_qipl(S390CPU *cpu)
cpu_physical_memory_unmap(addr, len, 1, len);
}
+int s390_ipl_prepare_pv_header(void)
+{
+S390IPLState *ipl = get_ipl_device();
+IPLBlockPV *ipib_pv = >iplb_pbt5.pv;
+void *hdr = g_malloc(ipib_pv->pv_header_len);
+int rc;
+
+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;
+S390IPLState *ipl = get_ipl_device();
+IPLBlockPV *ipib_pv = >iplb_pbt5.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 0;
+}
+
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 d0b8d90fa4..b5a13012ff 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -104,6 +104,8 @@ typedef union IplParameterBlock IplParameterBlock;
int s390_ipl_set_loadparm(uint8_t *loadparm);
int s390_ipl_pv_check_components(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 00..5b6a26cba9
--- /dev/null
+++ b/hw/s390x/pv.c
@@ -0,0 +1,154 @@
+/*
+ * Secure execution functions
+ *
+ * Copyright IBM Corp. 2019
+ * Author(s):
+ * Janosch Frank
+ *
+ * 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
+
+#include
+
+#include "qemu/error-report.h"
+#include "sysemu/kvm.h"
+#include "pv.h"
+
+const char* cmd_names[] = {
+"VM_CREATE",
+"VM_DESTROY",
+"VM_SET_SEC_PARAMS",
+"VM_UNPACK",
+"VM_VERIFY",
+"VM_PREP_RESET",
+"VM_UNSHARE_ALL",
+"VCPU_CREATE",
+"VCPU_DESTROY",
+NULL
+};
+
+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, _cmd);
+if (rc) {
+error_report("KVM PV command %d (%s) failed: header rc %x rrc %x "
+ "IOCTL rc: %d", cmd, cmd_names[cmd], pv_cmd.rc,
pv_cmd.rrc,
+ rc);
+}
+return rc;
+}
+
+static void s390_pv_cmd_exit(uint32_t cmd, void
