| Message ID | 20191129094809.26684-5-frankja@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x: Protected Virtualization support | expand |
On 29.11.19 10:48, Janosch Frank wrote: > Now that we know the protection state off the cpus, we can start > handling all diag 308 subcodes in the protected state. > > For subcodes 0 and 1 we need to unshare all pages before continuing, > so the guest doesn't accidentally expose data when dumping. > > For subcode 3/4 we tear down the protected VM and reboot into > unprotected mode. We do not provide a secure reboot. > > Before we can do the unshare calls, we need to mark all cpus as > stopped. This patch should be squashed into the previous one IMHO ... > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > hw/s390x/s390-virtio-ccw.c | 31 ++++++++++++++++++++++++++++--- > target/s390x/diag.c | 4 ++++ > 2 files changed, 32 insertions(+), 3 deletions(-) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index f9481ccace..e2a302398d 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -319,11 +319,26 @@ static inline void s390_do_cpu_ipl(CPUState *cs, run_on_cpu_data arg) > s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu); > } > > +static void s390_pv_prepare_reset(CPUS390XState *env) > +{ > + CPUState *cs; > + > + if (!env->pv) { > + return; > + } I'd suggest doing that check in the callers. (just like e.g., in "case S390_RESET_REIPL" already) > + CPU_FOREACH(cs) { > + s390_cpu_set_state(S390_CPU_STATE_STOPPED, S390_CPU(cs)); > + } > + s390_pv_unshare(); > + s390_pv_perf_clear_reset(); > +} > + > static void s390_machine_reset(MachineState *machine) > { > enum s390_reset reset_type; > CPUState *cs, *t; > S390CPU *cpu; > + CPUS390XState *env; > > /* get the reset parameters, reset them once done */ > s390_ipl_get_reset_request(&cs, &reset_type); > @@ -332,10 +347,18 @@ static void s390_machine_reset(MachineState *machine) > s390_cmma_reset(); > > cpu = S390_CPU(cs); > + env = &cpu->env; > > switch (reset_type) { > case S390_RESET_EXTERNAL: > case S390_RESET_REIPL: > + if (env->pv) { > + CPU_FOREACH(t) { > + s390_pv_vcpu_destroy(t); > + } > + s390_pv_vm_destroy(); > + } > + > qemu_devices_reset(); > s390_crypto_reset(); > > @@ -343,21 +366,23 @@ static void s390_machine_reset(MachineState *machine) > run_on_cpu(cs, s390_do_cpu_ipl, RUN_ON_CPU_NULL); > break; > case S390_RESET_MODIFIED_CLEAR: > + subsystem_reset(); > + s390_crypto_reset(); > + s390_pv_prepare_reset(env); > CPU_FOREACH(t) { > run_on_cpu(t, s390_do_cpu_full_reset, RUN_ON_CPU_NULL); > } > - subsystem_reset(); > - s390_crypto_reset(); > run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL); > break; > case S390_RESET_LOAD_NORMAL: > + subsystem_reset(); > + s390_pv_prepare_reset(env); > CPU_FOREACH(t) { > if (t == cs) { > continue; > } > run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL); > } > - subsystem_reset(); I do wonder if changing the orders here is okay ... why do you have to move subsystem_reset()?
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index f9481ccace..e2a302398d 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -319,11 +319,26 @@ static inline void s390_do_cpu_ipl(CPUState *cs, run_on_cpu_data arg) s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu); } +static void s390_pv_prepare_reset(CPUS390XState *env) +{ + CPUState *cs; + + if (!env->pv) { + return; + } + CPU_FOREACH(cs) { + s390_cpu_set_state(S390_CPU_STATE_STOPPED, S390_CPU(cs)); + } + s390_pv_unshare(); + s390_pv_perf_clear_reset(); +} + static void s390_machine_reset(MachineState *machine) { enum s390_reset reset_type; CPUState *cs, *t; S390CPU *cpu; + CPUS390XState *env; /* get the reset parameters, reset them once done */ s390_ipl_get_reset_request(&cs, &reset_type); @@ -332,10 +347,18 @@ static void s390_machine_reset(MachineState *machine) s390_cmma_reset(); cpu = S390_CPU(cs); + env = &cpu->env; switch (reset_type) { case S390_RESET_EXTERNAL: case S390_RESET_REIPL: + if (env->pv) { + CPU_FOREACH(t) { + s390_pv_vcpu_destroy(t); + } + s390_pv_vm_destroy(); + } + qemu_devices_reset(); s390_crypto_reset(); @@ -343,21 +366,23 @@ static void s390_machine_reset(MachineState *machine) run_on_cpu(cs, s390_do_cpu_ipl, RUN_ON_CPU_NULL); break; case S390_RESET_MODIFIED_CLEAR: + subsystem_reset(); + s390_crypto_reset(); + s390_pv_prepare_reset(env); CPU_FOREACH(t) { run_on_cpu(t, s390_do_cpu_full_reset, RUN_ON_CPU_NULL); } - subsystem_reset(); - s390_crypto_reset(); run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL); break; case S390_RESET_LOAD_NORMAL: + subsystem_reset(); + s390_pv_prepare_reset(env); CPU_FOREACH(t) { if (t == cs) { continue; } run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL); } - subsystem_reset(); 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; diff --git a/target/s390x/diag.c b/target/s390x/diag.c index 112a6c92e0..5489fc721a 100644 --- a/target/s390x/diag.c +++ b/target/s390x/diag.c @@ -68,6 +68,10 @@ int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3) static int diag308_parm_check(CPUS390XState *env, uint64_t r1, uint64_t addr, uintptr_t ra, bool write) { + /* Handled by the Ultravisor */ + if (env->pv) { + return 0; + } if ((r1 & 1) || (addr & ~TARGET_PAGE_MASK)) { s390_program_interrupt(env, PGM_SPECIFICATION, ra); return -1;
Now that we know the protection state off the cpus, we can start handling all diag 308 subcodes in the protected state. For subcodes 0 and 1 we need to unshare all pages before continuing, so the guest doesn't accidentally expose data when dumping. For subcode 3/4 we tear down the protected VM and reboot into unprotected mode. We do not provide a secure reboot. Before we can do the unshare calls, we need to mark all cpus as stopped. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- hw/s390x/s390-virtio-ccw.c | 31 ++++++++++++++++++++++++++++--- target/s390x/diag.c | 4 ++++ 2 files changed, 32 insertions(+), 3 deletions(-)