hw/s390x/ipl.c | 44 ++++++++++++++++++++++++--- hw/s390x/ipl.h | 16 ++++++++-- hw/s390x/s390-virtio-ccw.c | 49 +++++++++++++++++++++++++----- include/hw/s390x/s390-virtio-ccw.h | 2 -- target/s390x/cpu.h | 26 ++++++++++++++++ target/s390x/diag.c | 61 +++----------------------------------- target/s390x/internal.h | 6 ---- target/s390x/kvm.c | 2 +- 8 files changed, 127 insertions(+), 79 deletions(-)
Calling pause_all_vcpus()/resume_all_vcpus() from a VCPU thread might
not be the best idea. As pause_all_vcpus() temporarily drops the qemu
mutex, two parallel calls to pause_all_vcpus() can be active at a time,
resulting in a deadlock. (either by two VCPUs or by the main thread and a
VCPU)
Let's handle it via the main loop instead, as suggested by Paolo. If we
would have two parallel reset requests by two different VCPUs at the
same time, the last one would win.
We use the existing ipl device to handle it. The nice side effect is
that we can get rid of reipl_requested.
This change implies that all reset handling now goes via the common
path, so "no-reboot" handling is now active for all kinds of reboots.
Let's execute any CPU initialization code on the target CPU using
run_on_cpu.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
Did only a quick check with TCG. I think this way it is way easier to
control what is getting reset.
hw/s390x/ipl.c | 44 ++++++++++++++++++++++++---
hw/s390x/ipl.h | 16 ++++++++--
hw/s390x/s390-virtio-ccw.c | 49 +++++++++++++++++++++++++-----
include/hw/s390x/s390-virtio-ccw.h | 2 --
target/s390x/cpu.h | 26 ++++++++++++++++
target/s390x/diag.c | 61 +++-----------------------------------
target/s390x/internal.h | 6 ----
target/s390x/kvm.c | 2 +-
8 files changed, 127 insertions(+), 79 deletions(-)
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index fb554ab156..f7589d20f1 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -26,6 +26,7 @@
#include "qemu/config-file.h"
#include "qemu/cutils.h"
#include "qemu/option.h"
+#include "exec/exec-all.h"
#define KERN_IMAGE_START 0x010000UL
#define KERN_PARM_AREA 0x010480UL
@@ -484,11 +485,22 @@ IplParameterBlock *s390_ipl_get_iplb(void)
return &ipl->iplb;
}
-void s390_reipl_request(void)
+void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
{
S390IPLState *ipl = get_ipl_device();
- ipl->reipl_requested = true;
+ if (reset_type == S390_RESET_EXTERNAL || reset_type == S390_RESET_REIPL) {
+ /* let's always use CPU 0 */
+ ipl->reset_cpu = NULL;
+ } else {
+ ipl->reset_cpu = cs;
+ }
+ ipl->reset_type = reset_type;
+
+ if (reset_type != S390_RESET_REIPL) {
+ goto no_reipl;
+ }
+
if (ipl->iplb_valid &&
!ipl->netboot &&
ipl->iplb.pbt == S390_IPL_TYPE_CCW &&
@@ -505,7 +517,32 @@ void s390_reipl_request(void)
ipl->iplb_valid = s390_gen_initial_iplb(ipl);
}
}
+no_reipl:
qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+ /* as this is triggered by a CPU, make sure to exit the loop */
+ if (tcg_enabled()) {
+ cpu_loop_exit(cs);
+ }
+}
+
+void s390_ipl_get_reset_request(CPUState **cs, enum s390_reset *reset_type)
+{
+ S390IPLState *ipl = get_ipl_device();
+
+ *cs = ipl->reset_cpu;
+ if (!ipl->reset_cpu) {
+ /* use CPU 0 as default */
+ *cs = qemu_get_cpu(0);
+ }
+ *reset_type = ipl->reset_type;
+}
+
+void s390_ipl_clear_reset_request(void)
+{
+ S390IPLState *ipl = get_ipl_device();
+
+ ipl->reset_type = S390_RESET_EXTERNAL;
+ ipl->reset_cpu = NULL;
}
static void s390_ipl_prepare_qipl(S390CPU *cpu)
@@ -552,11 +589,10 @@ static void s390_ipl_reset(DeviceState *dev)
{
S390IPLState *ipl = S390_IPL(dev);
- if (!ipl->reipl_requested) {
+ if (ipl->reset_type != S390_RESET_REIPL) {
ipl->iplb_valid = false;
memset(&ipl->iplb, 0, sizeof(IplParameterBlock));
}
- ipl->reipl_requested = false;
}
static void s390_ipl_class_init(ObjectClass *klass, void *data)
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 0570d0ad75..102f1ea7af 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -87,7 +87,17 @@ int s390_ipl_set_loadparm(uint8_t *loadparm);
void s390_ipl_update_diag308(IplParameterBlock *iplb);
void s390_ipl_prepare_cpu(S390CPU *cpu);
IplParameterBlock *s390_ipl_get_iplb(void);
-void s390_reipl_request(void);
+
+enum s390_reset {
+ /* default is a reset not triggered by a CPU e.g. issued by QMP */
+ S390_RESET_EXTERNAL = 0,
+ S390_RESET_REIPL,
+ S390_RESET_MODIFIED_CLEAR,
+ S390_RESET_LOAD_NORMAL,
+};
+void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type);
+void s390_ipl_get_reset_request(CPUState **cs, enum s390_reset *reset_type);
+void s390_ipl_clear_reset_request(void);
#define QIPL_ADDRESS 0xcc
@@ -129,9 +139,11 @@ struct S390IPLState {
bool enforce_bios;
IplParameterBlock iplb;
bool iplb_valid;
- bool reipl_requested;
bool netboot;
QemuIplParameters qipl;
+ /* reset related properties don't have to be migrated or reset */
+ enum s390_reset reset_type;
+ CPUState *reset_cpu;
/*< public >*/
char *kernel;
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 435f7c99e7..f625900435 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -93,7 +93,7 @@ static const char *const reset_dev_types[] = {
"diag288",
};
-void subsystem_reset(void)
+static void subsystem_reset(void)
{
DeviceState *dev;
int i;
@@ -364,17 +364,52 @@ static void s390_cpu_plug(HotplugHandler *hotplug_dev,
}
}
+static inline void s390_do_cpu_ipl(CPUState *cs, run_on_cpu_data arg)
+{
+ S390CPU *cpu = S390_CPU(cs);
+
+ s390_ipl_prepare_cpu(cpu);
+ s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
+}
+
static void s390_machine_reset(void)
{
- S390CPU *ipl_cpu = S390_CPU(qemu_get_cpu(0));
+ enum s390_reset reset_type;
+ CPUState *cs, *t;
+ /* get the reset parameters, reset them once done */
+ s390_ipl_get_reset_request(&cs, &reset_type);
+
+ /* all CPUs are paused and synchronized at this point */
s390_cmma_reset();
- qemu_devices_reset();
- s390_crypto_reset();
- /* all cpus are stopped - configure and start the ipl cpu only */
- s390_ipl_prepare_cpu(ipl_cpu);
- s390_cpu_set_state(S390_CPU_STATE_OPERATING, ipl_cpu);
+ switch (reset_type) {
+ case S390_RESET_EXTERNAL:
+ case S390_RESET_REIPL:
+ 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_MODIFIED_CLEAR:
+ 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:
+ CPU_FOREACH(t) {
+ run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
+ }
+ subsystem_reset();
+ run_on_cpu(cs, s390_do_cpu_initital_reset, RUN_ON_CPU_NULL);
+ run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
+ break;
+ }
+ s390_ipl_clear_reset_request();
}
static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
index ac896e31ea..ab88d49d10 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -53,6 +53,4 @@ bool cpu_model_allowed(void);
*/
bool css_migration_enabled(void);
-void subsystem_reset(void);
-
#endif
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 3ee40f08b7..0c2583fd3c 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -686,6 +686,32 @@ static inline uint64_t s390_build_validity_mcic(void)
return mcic;
}
+static inline void s390_do_cpu_full_reset(CPUState *cs, run_on_cpu_data arg)
+{
+ cpu_reset(cs);
+}
+
+static inline void s390_do_cpu_reset(CPUState *cs, run_on_cpu_data arg)
+{
+ S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
+
+ scc->cpu_reset(cs);
+}
+
+static inline void s390_do_cpu_initital_reset(CPUState *cs, run_on_cpu_data arg)
+{
+ S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
+
+ scc->initial_cpu_reset(cs);
+}
+
+static inline void s390_do_cpu_load_normal(CPUState *cs, run_on_cpu_data arg)
+{
+ S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
+
+ scc->load_normal(cs);
+}
+
/* cpu.c */
int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low);
diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index a755837ad5..ac2c40f363 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -22,51 +22,6 @@
#include "hw/s390x/ipl.h"
#include "hw/s390x/s390-virtio-ccw.h"
-static int modified_clear_reset(S390CPU *cpu)
-{
- S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
- CPUState *t;
-
- pause_all_vcpus();
- cpu_synchronize_all_states();
- CPU_FOREACH(t) {
- run_on_cpu(t, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
- }
- s390_cmma_reset();
- subsystem_reset();
- s390_crypto_reset();
- scc->load_normal(CPU(cpu));
- cpu_synchronize_all_post_reset();
- resume_all_vcpus();
- return 0;
-}
-
-static inline void s390_do_cpu_reset(CPUState *cs, run_on_cpu_data arg)
-{
- S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
-
- scc->cpu_reset(cs);
-}
-
-static int load_normal_reset(S390CPU *cpu)
-{
- S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
- CPUState *t;
-
- pause_all_vcpus();
- cpu_synchronize_all_states();
- CPU_FOREACH(t) {
- run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
- }
- s390_cmma_reset();
- subsystem_reset();
- scc->initial_cpu_reset(CPU(cpu));
- scc->load_normal(CPU(cpu));
- cpu_synchronize_all_post_reset();
- resume_all_vcpus();
- return 0;
-}
-
int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
{
uint64_t func = env->regs[r1];
@@ -101,6 +56,7 @@ int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
{
+ CPUState *cs = CPU(s390_env_get_cpu(env));
uint64_t addr = env->regs[r1];
uint64_t subcode = env->regs[r3];
IplParameterBlock *iplb;
@@ -117,22 +73,13 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
switch (subcode) {
case 0:
- modified_clear_reset(s390_env_get_cpu(env));
- if (tcg_enabled()) {
- cpu_loop_exit(CPU(s390_env_get_cpu(env)));
- }
+ s390_ipl_reset_request(cs, S390_RESET_MODIFIED_CLEAR);
break;
case 1:
- load_normal_reset(s390_env_get_cpu(env));
- if (tcg_enabled()) {
- cpu_loop_exit(CPU(s390_env_get_cpu(env)));
- }
+ s390_ipl_reset_request(cs, S390_RESET_LOAD_NORMAL);
break;
case 3:
- s390_reipl_request();
- if (tcg_enabled()) {
- cpu_loop_exit(CPU(s390_env_get_cpu(env)));
- }
+ s390_ipl_reset_request(cs, S390_RESET_REIPL);
break;
case 5:
if ((r1 & 1) || (addr & 0x0fffULL)) {
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index d911e84958..e392a02d12 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -273,12 +273,6 @@ static inline hwaddr decode_basedisp_s(CPUS390XState *env, uint32_t ipb,
/* Base/displacement are at the same locations. */
#define decode_basedisp_rs decode_basedisp_s
-static inline void s390_do_cpu_full_reset(CPUState *cs, run_on_cpu_data arg)
-{
- cpu_reset(cs);
-}
-
-
/* arch_dump.c */
int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
int cpuid, void *opaque);
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index fb59d92def..b033d53f69 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1785,7 +1785,7 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
ret = handle_intercept(cpu);
break;
case KVM_EXIT_S390_RESET:
- s390_reipl_request();
+ s390_ipl_reset_request(cs, S390_RESET_REIPL);
break;
case KVM_EXIT_S390_TSCH:
ret = handle_tsch(cpu);
--
2.14.3
On 12/04/2018 21:26, David Hildenbrand wrote:
> Calling pause_all_vcpus()/resume_all_vcpus() from a VCPU thread might
> not be the best idea. As pause_all_vcpus() temporarily drops the qemu
> mutex, two parallel calls to pause_all_vcpus() can be active at a time,
> resulting in a deadlock. (either by two VCPUs or by the main thread and a
> VCPU)
>
> Let's handle it via the main loop instead, as suggested by Paolo. If we
> would have two parallel reset requests by two different VCPUs at the
> same time, the last one would win.
>
> We use the existing ipl device to handle it. The nice side effect is
> that we can get rid of reipl_requested.
>
> This change implies that all reset handling now goes via the common
> path, so "no-reboot" handling is now active for all kinds of reboots.
>
> Let's execute any CPU initialization code on the target CPU using
> run_on_cpu.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>
> Did only a quick check with TCG. I think this way it is way easier to
> control what is getting reset.
Happy that it went well (there's always some trepidation when suggesting
a refactoring :)). For what I can understand, it looks sane.
Paolo
> hw/s390x/ipl.c | 44 ++++++++++++++++++++++++---
> hw/s390x/ipl.h | 16 ++++++++--
> hw/s390x/s390-virtio-ccw.c | 49 +++++++++++++++++++++++++-----
> include/hw/s390x/s390-virtio-ccw.h | 2 --
> target/s390x/cpu.h | 26 ++++++++++++++++
> target/s390x/diag.c | 61 +++-----------------------------------
> target/s390x/internal.h | 6 ----
> target/s390x/kvm.c | 2 +-
> 8 files changed, 127 insertions(+), 79 deletions(-)
>
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index fb554ab156..f7589d20f1 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -26,6 +26,7 @@
> #include "qemu/config-file.h"
> #include "qemu/cutils.h"
> #include "qemu/option.h"
> +#include "exec/exec-all.h"
>
> #define KERN_IMAGE_START 0x010000UL
> #define KERN_PARM_AREA 0x010480UL
> @@ -484,11 +485,22 @@ IplParameterBlock *s390_ipl_get_iplb(void)
> return &ipl->iplb;
> }
>
> -void s390_reipl_request(void)
> +void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
> {
> S390IPLState *ipl = get_ipl_device();
>
> - ipl->reipl_requested = true;
> + if (reset_type == S390_RESET_EXTERNAL || reset_type == S390_RESET_REIPL) {
> + /* let's always use CPU 0 */
> + ipl->reset_cpu = NULL;
> + } else {
> + ipl->reset_cpu = cs;
> + }
> + ipl->reset_type = reset_type;
> +
> + if (reset_type != S390_RESET_REIPL) {
> + goto no_reipl;
> + }
> +
> if (ipl->iplb_valid &&
> !ipl->netboot &&
> ipl->iplb.pbt == S390_IPL_TYPE_CCW &&
> @@ -505,7 +517,32 @@ void s390_reipl_request(void)
> ipl->iplb_valid = s390_gen_initial_iplb(ipl);
> }
> }
> +no_reipl:
> qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> + /* as this is triggered by a CPU, make sure to exit the loop */
> + if (tcg_enabled()) {
> + cpu_loop_exit(cs);
> + }
> +}
> +
> +void s390_ipl_get_reset_request(CPUState **cs, enum s390_reset *reset_type)
> +{
> + S390IPLState *ipl = get_ipl_device();
> +
> + *cs = ipl->reset_cpu;
> + if (!ipl->reset_cpu) {
> + /* use CPU 0 as default */
> + *cs = qemu_get_cpu(0);
> + }
> + *reset_type = ipl->reset_type;
> +}
> +
> +void s390_ipl_clear_reset_request(void)
> +{
> + S390IPLState *ipl = get_ipl_device();
> +
> + ipl->reset_type = S390_RESET_EXTERNAL;
> + ipl->reset_cpu = NULL;
> }
>
> static void s390_ipl_prepare_qipl(S390CPU *cpu)
> @@ -552,11 +589,10 @@ static void s390_ipl_reset(DeviceState *dev)
> {
> S390IPLState *ipl = S390_IPL(dev);
>
> - if (!ipl->reipl_requested) {
> + if (ipl->reset_type != S390_RESET_REIPL) {
> ipl->iplb_valid = false;
> memset(&ipl->iplb, 0, sizeof(IplParameterBlock));
> }
> - ipl->reipl_requested = false;
> }
>
> static void s390_ipl_class_init(ObjectClass *klass, void *data)
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index 0570d0ad75..102f1ea7af 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -87,7 +87,17 @@ int s390_ipl_set_loadparm(uint8_t *loadparm);
> void s390_ipl_update_diag308(IplParameterBlock *iplb);
> void s390_ipl_prepare_cpu(S390CPU *cpu);
> IplParameterBlock *s390_ipl_get_iplb(void);
> -void s390_reipl_request(void);
> +
> +enum s390_reset {
> + /* default is a reset not triggered by a CPU e.g. issued by QMP */
> + S390_RESET_EXTERNAL = 0,
> + S390_RESET_REIPL,
> + S390_RESET_MODIFIED_CLEAR,
> + S390_RESET_LOAD_NORMAL,
> +};
> +void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type);
> +void s390_ipl_get_reset_request(CPUState **cs, enum s390_reset *reset_type);
> +void s390_ipl_clear_reset_request(void);
>
> #define QIPL_ADDRESS 0xcc
>
> @@ -129,9 +139,11 @@ struct S390IPLState {
> bool enforce_bios;
> IplParameterBlock iplb;
> bool iplb_valid;
> - bool reipl_requested;
> bool netboot;
> QemuIplParameters qipl;
> + /* reset related properties don't have to be migrated or reset */
> + enum s390_reset reset_type;
> + CPUState *reset_cpu;
>
> /*< public >*/
> char *kernel;
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 435f7c99e7..f625900435 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -93,7 +93,7 @@ static const char *const reset_dev_types[] = {
> "diag288",
> };
>
> -void subsystem_reset(void)
> +static void subsystem_reset(void)
> {
> DeviceState *dev;
> int i;
> @@ -364,17 +364,52 @@ static void s390_cpu_plug(HotplugHandler *hotplug_dev,
> }
> }
>
> +static inline void s390_do_cpu_ipl(CPUState *cs, run_on_cpu_data arg)
> +{
> + S390CPU *cpu = S390_CPU(cs);
> +
> + s390_ipl_prepare_cpu(cpu);
> + s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
> +}
> +
> static void s390_machine_reset(void)
> {
> - S390CPU *ipl_cpu = S390_CPU(qemu_get_cpu(0));
> + enum s390_reset reset_type;
> + CPUState *cs, *t;
>
> + /* get the reset parameters, reset them once done */
> + s390_ipl_get_reset_request(&cs, &reset_type);
> +
> + /* all CPUs are paused and synchronized at this point */
> s390_cmma_reset();
> - qemu_devices_reset();
> - s390_crypto_reset();
>
> - /* all cpus are stopped - configure and start the ipl cpu only */
> - s390_ipl_prepare_cpu(ipl_cpu);
> - s390_cpu_set_state(S390_CPU_STATE_OPERATING, ipl_cpu);
> + switch (reset_type) {
> + case S390_RESET_EXTERNAL:
> + case S390_RESET_REIPL:
> + 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_MODIFIED_CLEAR:
> + 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:
> + CPU_FOREACH(t) {
> + run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
> + }
> + subsystem_reset();
> + run_on_cpu(cs, s390_do_cpu_initital_reset, RUN_ON_CPU_NULL);
> + run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
> + break;
> + }
> + s390_ipl_clear_reset_request();
> }
>
> static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
> index ac896e31ea..ab88d49d10 100644
> --- a/include/hw/s390x/s390-virtio-ccw.h
> +++ b/include/hw/s390x/s390-virtio-ccw.h
> @@ -53,6 +53,4 @@ bool cpu_model_allowed(void);
> */
> bool css_migration_enabled(void);
>
> -void subsystem_reset(void);
> -
> #endif
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 3ee40f08b7..0c2583fd3c 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -686,6 +686,32 @@ static inline uint64_t s390_build_validity_mcic(void)
> return mcic;
> }
>
> +static inline void s390_do_cpu_full_reset(CPUState *cs, run_on_cpu_data arg)
> +{
> + cpu_reset(cs);
> +}
> +
> +static inline void s390_do_cpu_reset(CPUState *cs, run_on_cpu_data arg)
> +{
> + S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
> +
> + scc->cpu_reset(cs);
> +}
> +
> +static inline void s390_do_cpu_initital_reset(CPUState *cs, run_on_cpu_data arg)
> +{
> + S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
> +
> + scc->initial_cpu_reset(cs);
> +}
> +
> +static inline void s390_do_cpu_load_normal(CPUState *cs, run_on_cpu_data arg)
> +{
> + S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
> +
> + scc->load_normal(cs);
> +}
> +
>
> /* cpu.c */
> int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low);
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index a755837ad5..ac2c40f363 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -22,51 +22,6 @@
> #include "hw/s390x/ipl.h"
> #include "hw/s390x/s390-virtio-ccw.h"
>
> -static int modified_clear_reset(S390CPU *cpu)
> -{
> - S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
> - CPUState *t;
> -
> - pause_all_vcpus();
> - cpu_synchronize_all_states();
> - CPU_FOREACH(t) {
> - run_on_cpu(t, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
> - }
> - s390_cmma_reset();
> - subsystem_reset();
> - s390_crypto_reset();
> - scc->load_normal(CPU(cpu));
> - cpu_synchronize_all_post_reset();
> - resume_all_vcpus();
> - return 0;
> -}
> -
> -static inline void s390_do_cpu_reset(CPUState *cs, run_on_cpu_data arg)
> -{
> - S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
> -
> - scc->cpu_reset(cs);
> -}
> -
> -static int load_normal_reset(S390CPU *cpu)
> -{
> - S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
> - CPUState *t;
> -
> - pause_all_vcpus();
> - cpu_synchronize_all_states();
> - CPU_FOREACH(t) {
> - run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
> - }
> - s390_cmma_reset();
> - subsystem_reset();
> - scc->initial_cpu_reset(CPU(cpu));
> - scc->load_normal(CPU(cpu));
> - cpu_synchronize_all_post_reset();
> - resume_all_vcpus();
> - return 0;
> -}
> -
> int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
> {
> uint64_t func = env->regs[r1];
> @@ -101,6 +56,7 @@ int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
>
> void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
> {
> + CPUState *cs = CPU(s390_env_get_cpu(env));
> uint64_t addr = env->regs[r1];
> uint64_t subcode = env->regs[r3];
> IplParameterBlock *iplb;
> @@ -117,22 +73,13 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
>
> switch (subcode) {
> case 0:
> - modified_clear_reset(s390_env_get_cpu(env));
> - if (tcg_enabled()) {
> - cpu_loop_exit(CPU(s390_env_get_cpu(env)));
> - }
> + s390_ipl_reset_request(cs, S390_RESET_MODIFIED_CLEAR);
> break;
> case 1:
> - load_normal_reset(s390_env_get_cpu(env));
> - if (tcg_enabled()) {
> - cpu_loop_exit(CPU(s390_env_get_cpu(env)));
> - }
> + s390_ipl_reset_request(cs, S390_RESET_LOAD_NORMAL);
> break;
> case 3:
> - s390_reipl_request();
> - if (tcg_enabled()) {
> - cpu_loop_exit(CPU(s390_env_get_cpu(env)));
> - }
> + s390_ipl_reset_request(cs, S390_RESET_REIPL);
> break;
> case 5:
> if ((r1 & 1) || (addr & 0x0fffULL)) {
> diff --git a/target/s390x/internal.h b/target/s390x/internal.h
> index d911e84958..e392a02d12 100644
> --- a/target/s390x/internal.h
> +++ b/target/s390x/internal.h
> @@ -273,12 +273,6 @@ static inline hwaddr decode_basedisp_s(CPUS390XState *env, uint32_t ipb,
> /* Base/displacement are at the same locations. */
> #define decode_basedisp_rs decode_basedisp_s
>
> -static inline void s390_do_cpu_full_reset(CPUState *cs, run_on_cpu_data arg)
> -{
> - cpu_reset(cs);
> -}
> -
> -
> /* arch_dump.c */
> int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
> int cpuid, void *opaque);
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index fb59d92def..b033d53f69 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -1785,7 +1785,7 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
> ret = handle_intercept(cpu);
> break;
> case KVM_EXIT_S390_RESET:
> - s390_reipl_request();
> + s390_ipl_reset_request(cs, S390_RESET_REIPL);
> break;
> case KVM_EXIT_S390_TSCH:
> ret = handle_tsch(cpu);
>
On 12.04.2018 21:26, David Hildenbrand wrote:
> Calling pause_all_vcpus()/resume_all_vcpus() from a VCPU thread might
> not be the best idea. As pause_all_vcpus() temporarily drops the qemu
> mutex, two parallel calls to pause_all_vcpus() can be active at a time,
> resulting in a deadlock. (either by two VCPUs or by the main thread and a
> VCPU)
>
> Let's handle it via the main loop instead, as suggested by Paolo. If we
> would have two parallel reset requests by two different VCPUs at the
> same time, the last one would win.
>
> We use the existing ipl device to handle it. The nice side effect is
> that we can get rid of reipl_requested.
>
> This change implies that all reset handling now goes via the common
> path, so "no-reboot" handling is now active for all kinds of reboots.
>
> Let's execute any CPU initialization code on the target CPU using
> run_on_cpu.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>
> Did only a quick check with TCG. I think this way it is way easier to
> control what is getting reset.
>
> hw/s390x/ipl.c | 44 ++++++++++++++++++++++++---
> hw/s390x/ipl.h | 16 ++++++++--
> hw/s390x/s390-virtio-ccw.c | 49 +++++++++++++++++++++++++-----
> include/hw/s390x/s390-virtio-ccw.h | 2 --
> target/s390x/cpu.h | 26 ++++++++++++++++
> target/s390x/diag.c | 61 +++-----------------------------------
> target/s390x/internal.h | 6 ----
> target/s390x/kvm.c | 2 +-
> 8 files changed, 127 insertions(+), 79 deletions(-)
>
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index fb554ab156..f7589d20f1 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -26,6 +26,7 @@
> #include "qemu/config-file.h"
> #include "qemu/cutils.h"
> #include "qemu/option.h"
> +#include "exec/exec-all.h"
>
> #define KERN_IMAGE_START 0x010000UL
> #define KERN_PARM_AREA 0x010480UL
> @@ -484,11 +485,22 @@ IplParameterBlock *s390_ipl_get_iplb(void)
> return &ipl->iplb;
> }
>
> -void s390_reipl_request(void)
> +void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
> {
> S390IPLState *ipl = get_ipl_device();
>
> - ipl->reipl_requested = true;
> + if (reset_type == S390_RESET_EXTERNAL || reset_type == S390_RESET_REIPL) {
> + /* let's always use CPU 0 */
> + ipl->reset_cpu = NULL;
> + } else {
> + ipl->reset_cpu = cs;
> + }
> + ipl->reset_type = reset_type;
> +
> + if (reset_type != S390_RESET_REIPL) {
> + goto no_reipl;
Could we please avoid goto in this case and simply put the code below
into a "if (reset_type == S390_RESET_REIPL)" block?
> + }
> +
> if (ipl->iplb_valid &&
> !ipl->netboot &&
> ipl->iplb.pbt == S390_IPL_TYPE_CCW &&
> @@ -505,7 +517,32 @@ void s390_reipl_request(void)
> ipl->iplb_valid = s390_gen_initial_iplb(ipl);
> }
> }
> +no_reipl:
> qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> + /* as this is triggered by a CPU, make sure to exit the loop */
> + if (tcg_enabled()) {
> + cpu_loop_exit(cs);
> + }
> +}
Thomas
On 18.04.2018 16:08, Thomas Huth wrote:
> On 12.04.2018 21:26, David Hildenbrand wrote:
>> Calling pause_all_vcpus()/resume_all_vcpus() from a VCPU thread might
>> not be the best idea. As pause_all_vcpus() temporarily drops the qemu
>> mutex, two parallel calls to pause_all_vcpus() can be active at a time,
>> resulting in a deadlock. (either by two VCPUs or by the main thread and a
>> VCPU)
>>
>> Let's handle it via the main loop instead, as suggested by Paolo. If we
>> would have two parallel reset requests by two different VCPUs at the
>> same time, the last one would win.
>>
>> We use the existing ipl device to handle it. The nice side effect is
>> that we can get rid of reipl_requested.
>>
>> This change implies that all reset handling now goes via the common
>> path, so "no-reboot" handling is now active for all kinds of reboots.
>>
>> Let's execute any CPU initialization code on the target CPU using
>> run_on_cpu.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>
>> Did only a quick check with TCG. I think this way it is way easier to
>> control what is getting reset.
>>
>> hw/s390x/ipl.c | 44 ++++++++++++++++++++++++---
>> hw/s390x/ipl.h | 16 ++++++++--
>> hw/s390x/s390-virtio-ccw.c | 49 +++++++++++++++++++++++++-----
>> include/hw/s390x/s390-virtio-ccw.h | 2 --
>> target/s390x/cpu.h | 26 ++++++++++++++++
>> target/s390x/diag.c | 61 +++-----------------------------------
>> target/s390x/internal.h | 6 ----
>> target/s390x/kvm.c | 2 +-
>> 8 files changed, 127 insertions(+), 79 deletions(-)
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index fb554ab156..f7589d20f1 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -26,6 +26,7 @@
>> #include "qemu/config-file.h"
>> #include "qemu/cutils.h"
>> #include "qemu/option.h"
>> +#include "exec/exec-all.h"
>>
>> #define KERN_IMAGE_START 0x010000UL
>> #define KERN_PARM_AREA 0x010480UL
>> @@ -484,11 +485,22 @@ IplParameterBlock *s390_ipl_get_iplb(void)
>> return &ipl->iplb;
>> }
>>
>> -void s390_reipl_request(void)
>> +void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
>> {
>> S390IPLState *ipl = get_ipl_device();
>>
>> - ipl->reipl_requested = true;
>> + if (reset_type == S390_RESET_EXTERNAL || reset_type == S390_RESET_REIPL) {
>> + /* let's always use CPU 0 */
>> + ipl->reset_cpu = NULL;
>> + } else {
>> + ipl->reset_cpu = cs;
>> + }
>> + ipl->reset_type = reset_type;
>> +
>> + if (reset_type != S390_RESET_REIPL) {
>> + goto no_reipl;
>
> Could we please avoid goto in this case and simply put the code below
> into a "if (reset_type == S390_RESET_REIPL)" block?
>
Sure, I wanted to keep changes this RFC minimal for better overview.
>> + }
>> +
>> if (ipl->iplb_valid &&
>> !ipl->netboot &&
>> ipl->iplb.pbt == S390_IPL_TYPE_CCW &&
>> @@ -505,7 +517,32 @@ void s390_reipl_request(void)
>> ipl->iplb_valid = s390_gen_initial_iplb(ipl);
>> }
>> }
>> +no_reipl:
>> qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
>> + /* as this is triggered by a CPU, make sure to exit the loop */
>> + if (tcg_enabled()) {
>> + cpu_loop_exit(cs);
>> + }
>> +}
>
> Thomas
>
--
Thanks,
David / dhildenb
> static void s390_ipl_class_init(ObjectClass *klass, void *data)
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index 0570d0ad75..102f1ea7af 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -87,7 +87,17 @@ int s390_ipl_set_loadparm(uint8_t *loadparm);
> void s390_ipl_update_diag308(IplParameterBlock *iplb);
> void s390_ipl_prepare_cpu(S390CPU *cpu);
> IplParameterBlock *s390_ipl_get_iplb(void);
> -void s390_reipl_request(void);
> +
> +enum s390_reset {
> + /* default is a reset not triggered by a CPU e.g. issued by QMP */
> + S390_RESET_EXTERNAL = 0,
> + S390_RESET_REIPL,
> + S390_RESET_MODIFIED_CLEAR,
> + S390_RESET_LOAD_NORMAL,
> +};
> +void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type);
> +void s390_ipl_get_reset_request(CPUState **cs, enum s390_reset *reset_type);
> +void s390_ipl_clear_reset_request(void);
>
> #define QIPL_ADDRESS 0xcc
>
> @@ -129,9 +139,11 @@ struct S390IPLState {
> bool enforce_bios;
> IplParameterBlock iplb;
> bool iplb_valid;
> - bool reipl_requested;
> bool netboot;
> QemuIplParameters qipl;
> + /* reset related properties don't have to be migrated or reset */
> + enum s390_reset reset_type;
> + CPUState *reset_cpu;
Wondering if storing the cpu number would be nicer. Opinions?
--
Thanks,
David / dhildenb
On Wed, 18 Apr 2018 16:33:13 +0200
David Hildenbrand <david@redhat.com> wrote:
> > static void s390_ipl_class_init(ObjectClass *klass, void *data)
> > diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> > index 0570d0ad75..102f1ea7af 100644
> > --- a/hw/s390x/ipl.h
> > +++ b/hw/s390x/ipl.h
> > @@ -87,7 +87,17 @@ int s390_ipl_set_loadparm(uint8_t *loadparm);
> > void s390_ipl_update_diag308(IplParameterBlock *iplb);
> > void s390_ipl_prepare_cpu(S390CPU *cpu);
> > IplParameterBlock *s390_ipl_get_iplb(void);
> > -void s390_reipl_request(void);
> > +
> > +enum s390_reset {
> > + /* default is a reset not triggered by a CPU e.g. issued by QMP */
> > + S390_RESET_EXTERNAL = 0,
> > + S390_RESET_REIPL,
> > + S390_RESET_MODIFIED_CLEAR,
> > + S390_RESET_LOAD_NORMAL,
> > +};
> > +void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type);
> > +void s390_ipl_get_reset_request(CPUState **cs, enum s390_reset *reset_type);
> > +void s390_ipl_clear_reset_request(void);
> >
> > #define QIPL_ADDRESS 0xcc
> >
> > @@ -129,9 +139,11 @@ struct S390IPLState {
> > bool enforce_bios;
> > IplParameterBlock iplb;
> > bool iplb_valid;
> > - bool reipl_requested;
> > bool netboot;
> > QemuIplParameters qipl;
> > + /* reset related properties don't have to be migrated or reset */
> > + enum s390_reset reset_type;
> > + CPUState *reset_cpu;
>
> Wondering if storing the cpu number would be nicer. Opinions?
>
Is there any difference in the number of QOM operations?
Also, do you need to distinguish between 'specified cpu' and
'default/any cpu'? Should be deductible from the reset type, though.
On 23.04.2018 12:45, Cornelia Huck wrote:
> On Wed, 18 Apr 2018 16:33:13 +0200
> David Hildenbrand <david@redhat.com> wrote:
>
>>> static void s390_ipl_class_init(ObjectClass *klass, void *data)
>>> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
>>> index 0570d0ad75..102f1ea7af 100644
>>> --- a/hw/s390x/ipl.h
>>> +++ b/hw/s390x/ipl.h
>>> @@ -87,7 +87,17 @@ int s390_ipl_set_loadparm(uint8_t *loadparm);
>>> void s390_ipl_update_diag308(IplParameterBlock *iplb);
>>> void s390_ipl_prepare_cpu(S390CPU *cpu);
>>> IplParameterBlock *s390_ipl_get_iplb(void);
>>> -void s390_reipl_request(void);
>>> +
>>> +enum s390_reset {
>>> + /* default is a reset not triggered by a CPU e.g. issued by QMP */
>>> + S390_RESET_EXTERNAL = 0,
>>> + S390_RESET_REIPL,
>>> + S390_RESET_MODIFIED_CLEAR,
>>> + S390_RESET_LOAD_NORMAL,
>>> +};
>>> +void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type);
>>> +void s390_ipl_get_reset_request(CPUState **cs, enum s390_reset *reset_type);
>>> +void s390_ipl_clear_reset_request(void);
>>>
>>> #define QIPL_ADDRESS 0xcc
>>>
>>> @@ -129,9 +139,11 @@ struct S390IPLState {
>>> bool enforce_bios;
>>> IplParameterBlock iplb;
>>> bool iplb_valid;
>>> - bool reipl_requested;
>>> bool netboot;
>>> QemuIplParameters qipl;
>>> + /* reset related properties don't have to be migrated or reset */
>>> + enum s390_reset reset_type;
>>> + CPUState *reset_cpu;
>>
>> Wondering if storing the cpu number would be nicer. Opinions?
>>
>
> Is there any difference in the number of QOM operations?
Not that it would matter much. Just another VCPU lookup by ID.
>
> Also, do you need to distinguish between 'specified cpu' and
> 'default/any cpu'? Should be deductible from the reset type, though.
>
Yes, I deduct that from the reset type.
--
Thanks,
David / dhildenb
On Thu, 12 Apr 2018 21:26:02 +0200
David Hildenbrand <david@redhat.com> wrote:
> Calling pause_all_vcpus()/resume_all_vcpus() from a VCPU thread might
> not be the best idea. As pause_all_vcpus() temporarily drops the qemu
> mutex, two parallel calls to pause_all_vcpus() can be active at a time,
> resulting in a deadlock. (either by two VCPUs or by the main thread and a
> VCPU)
>
> Let's handle it via the main loop instead, as suggested by Paolo. If we
> would have two parallel reset requests by two different VCPUs at the
> same time, the last one would win.
>
> We use the existing ipl device to handle it. The nice side effect is
> that we can get rid of reipl_requested.
>
> This change implies that all reset handling now goes via the common
> path, so "no-reboot" handling is now active for all kinds of reboots.
>
> Let's execute any CPU initialization code on the target CPU using
> run_on_cpu.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>
> Did only a quick check with TCG. I think this way it is way easier to
> control what is getting reset.
Yes, I like this patch.
>
> hw/s390x/ipl.c | 44 ++++++++++++++++++++++++---
> hw/s390x/ipl.h | 16 ++++++++--
> hw/s390x/s390-virtio-ccw.c | 49 +++++++++++++++++++++++++-----
> include/hw/s390x/s390-virtio-ccw.h | 2 --
> target/s390x/cpu.h | 26 ++++++++++++++++
> target/s390x/diag.c | 61 +++-----------------------------------
> target/s390x/internal.h | 6 ----
> target/s390x/kvm.c | 2 +-
> 8 files changed, 127 insertions(+), 79 deletions(-)
>
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index fb554ab156..f7589d20f1 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -26,6 +26,7 @@
> #include "qemu/config-file.h"
> #include "qemu/cutils.h"
> #include "qemu/option.h"
> +#include "exec/exec-all.h"
>
> #define KERN_IMAGE_START 0x010000UL
> #define KERN_PARM_AREA 0x010480UL
> @@ -484,11 +485,22 @@ IplParameterBlock *s390_ipl_get_iplb(void)
> return &ipl->iplb;
> }
>
> -void s390_reipl_request(void)
> +void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
> {
> S390IPLState *ipl = get_ipl_device();
>
> - ipl->reipl_requested = true;
> + if (reset_type == S390_RESET_EXTERNAL || reset_type == S390_RESET_REIPL) {
> + /* let's always use CPU 0 */
> + ipl->reset_cpu = NULL;
> + } else {
> + ipl->reset_cpu = cs;
> + }
> + ipl->reset_type = reset_type;
> +
> + if (reset_type != S390_RESET_REIPL) {
Pull the check for S390_RESET_REIPL into the if condition below? Gets
rid of the goto :)
> + goto no_reipl;
> + }
> +
> if (ipl->iplb_valid &&
> !ipl->netboot &&
> ipl->iplb.pbt == S390_IPL_TYPE_CCW &&
> @@ -505,7 +517,32 @@ void s390_reipl_request(void)
> ipl->iplb_valid = s390_gen_initial_iplb(ipl);
> }
> }
> +no_reipl:
> qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> + /* as this is triggered by a CPU, make sure to exit the loop */
> + if (tcg_enabled()) {
> + cpu_loop_exit(cs);
> + }
> +}
> +
> +void s390_ipl_get_reset_request(CPUState **cs, enum s390_reset *reset_type)
> +{
> + S390IPLState *ipl = get_ipl_device();
> +
> + *cs = ipl->reset_cpu;
> + if (!ipl->reset_cpu) {
> + /* use CPU 0 as default */
> + *cs = qemu_get_cpu(0);
That looks somewhat ugly. Maybe just stuff cpu 0 in there at init time?
As an aside: Are we guaranteed to always have cpu 0? IIRC there was
some code relying on that; but just grabbing an 'any' cpu looks more
like what we want.
> + }
> + *reset_type = ipl->reset_type;
> +}
> static void s390_machine_reset(void)
> {
> - S390CPU *ipl_cpu = S390_CPU(qemu_get_cpu(0));
> + enum s390_reset reset_type;
> + CPUState *cs, *t;
>
> + /* get the reset parameters, reset them once done */
> + s390_ipl_get_reset_request(&cs, &reset_type);
> +
> + /* all CPUs are paused and synchronized at this point */
> s390_cmma_reset();
> - qemu_devices_reset();
> - s390_crypto_reset();
>
> - /* all cpus are stopped - configure and start the ipl cpu only */
> - s390_ipl_prepare_cpu(ipl_cpu);
> - s390_cpu_set_state(S390_CPU_STATE_OPERATING, ipl_cpu);
> + switch (reset_type) {
> + case S390_RESET_EXTERNAL:
> + case S390_RESET_REIPL:
> + 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_MODIFIED_CLEAR:
> + 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:
> + CPU_FOREACH(t) {
> + run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
> + }
> + subsystem_reset();
> + run_on_cpu(cs, s390_do_cpu_initital_reset, RUN_ON_CPU_NULL);
'initital' looks like a typo; 'initial'?
(But you made the same typo twice, so it compiles :)
> + run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
> + break;
Moan on default case hit?
> + }
> + s390_ipl_clear_reset_request();
> }
>
> static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
On 23.04.2018 12:42, Cornelia Huck wrote:
> On Thu, 12 Apr 2018 21:26:02 +0200
> David Hildenbrand <david@redhat.com> wrote:
>
>> Calling pause_all_vcpus()/resume_all_vcpus() from a VCPU thread might
>> not be the best idea. As pause_all_vcpus() temporarily drops the qemu
>> mutex, two parallel calls to pause_all_vcpus() can be active at a time,
>> resulting in a deadlock. (either by two VCPUs or by the main thread and a
>> VCPU)
>>
>> Let's handle it via the main loop instead, as suggested by Paolo. If we
>> would have two parallel reset requests by two different VCPUs at the
>> same time, the last one would win.
>>
>> We use the existing ipl device to handle it. The nice side effect is
>> that we can get rid of reipl_requested.
>>
>> This change implies that all reset handling now goes via the common
>> path, so "no-reboot" handling is now active for all kinds of reboots.
>>
>> Let's execute any CPU initialization code on the target CPU using
>> run_on_cpu.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>
>> Did only a quick check with TCG. I think this way it is way easier to
>> control what is getting reset.
>
> Yes, I like this patch.
>
>>
>> hw/s390x/ipl.c | 44 ++++++++++++++++++++++++---
>> hw/s390x/ipl.h | 16 ++++++++--
>> hw/s390x/s390-virtio-ccw.c | 49 +++++++++++++++++++++++++-----
>> include/hw/s390x/s390-virtio-ccw.h | 2 --
>> target/s390x/cpu.h | 26 ++++++++++++++++
>> target/s390x/diag.c | 61 +++-----------------------------------
>> target/s390x/internal.h | 6 ----
>> target/s390x/kvm.c | 2 +-
>> 8 files changed, 127 insertions(+), 79 deletions(-)
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index fb554ab156..f7589d20f1 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -26,6 +26,7 @@
>> #include "qemu/config-file.h"
>> #include "qemu/cutils.h"
>> #include "qemu/option.h"
>> +#include "exec/exec-all.h"
>>
>> #define KERN_IMAGE_START 0x010000UL
>> #define KERN_PARM_AREA 0x010480UL
>> @@ -484,11 +485,22 @@ IplParameterBlock *s390_ipl_get_iplb(void)
>> return &ipl->iplb;
>> }
>>
>> -void s390_reipl_request(void)
>> +void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
>> {
>> S390IPLState *ipl = get_ipl_device();
>>
>> - ipl->reipl_requested = true;
>> + if (reset_type == S390_RESET_EXTERNAL || reset_type == S390_RESET_REIPL) {
>> + /* let's always use CPU 0 */
>> + ipl->reset_cpu = NULL;
>> + } else {
>> + ipl->reset_cpu = cs;
>> + }
>> + ipl->reset_type = reset_type;
>> +
>> + if (reset_type != S390_RESET_REIPL) {
>
> Pull the check for S390_RESET_REIPL into the if condition below? Gets
> rid of the goto :)
Yes, that's the thing I was no realizing, while hacking on this. I
thought I would have to shift the whole block.
>
>> + goto no_reipl;
>> + }
>> +
>> if (ipl->iplb_valid &&
>> !ipl->netboot &&
>> ipl->iplb.pbt == S390_IPL_TYPE_CCW &&
>> @@ -505,7 +517,32 @@ void s390_reipl_request(void)
>> ipl->iplb_valid = s390_gen_initial_iplb(ipl);
>> }
>> }
>> +no_reipl:
>> qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
>> + /* as this is triggered by a CPU, make sure to exit the loop */
>> + if (tcg_enabled()) {
>> + cpu_loop_exit(cs);
>> + }
>> +}
>> +
>> +void s390_ipl_get_reset_request(CPUState **cs, enum s390_reset *reset_type)
>> +{
>> + S390IPLState *ipl = get_ipl_device();
>> +
>> + *cs = ipl->reset_cpu;
>> + if (!ipl->reset_cpu) {
>> + /* use CPU 0 as default */
>> + *cs = qemu_get_cpu(0);
>
> That looks somewhat ugly. Maybe just stuff cpu 0 in there at init time?
Well, it's the same we already had just at a different place :)
>
> As an aside: Are we guaranteed to always have cpu 0? IIRC there was
> some code relying on that; but just grabbing an 'any' cpu looks more
> like what we want.
We will always create at least one CPU. The fist CPU will always be CPU
with ID 0. As cpu_index corresponds to ID for us, we will always get CPU
0 via qemu_get_cpu(). (and we don't have CPU unplug)
However I might just store the cpu id in there instead of an reference.
So we could "fallback" here to some random CPU that exists (in case
qemu_get_cpu() returns NULL).
>
>> + }
>> + *reset_type = ipl->reset_type;
>> +}
>
>> static void s390_machine_reset(void)
>> {
>> - S390CPU *ipl_cpu = S390_CPU(qemu_get_cpu(0));
>> + enum s390_reset reset_type;
>> + CPUState *cs, *t;
>>
>> + /* get the reset parameters, reset them once done */
>> + s390_ipl_get_reset_request(&cs, &reset_type);
>> +
>> + /* all CPUs are paused and synchronized at this point */
>> s390_cmma_reset();
>> - qemu_devices_reset();
>> - s390_crypto_reset();
>>
>> - /* all cpus are stopped - configure and start the ipl cpu only */
>> - s390_ipl_prepare_cpu(ipl_cpu);
>> - s390_cpu_set_state(S390_CPU_STATE_OPERATING, ipl_cpu);
>> + switch (reset_type) {
>> + case S390_RESET_EXTERNAL:
>> + case S390_RESET_REIPL:
>> + 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_MODIFIED_CLEAR:
>> + 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:
>> + CPU_FOREACH(t) {
>> + run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
>> + }
>> + subsystem_reset();
>> + run_on_cpu(cs, s390_do_cpu_initital_reset, RUN_ON_CPU_NULL);
>
> 'initital' looks like a typo; 'initial'?
I keep making this spelling error over and over again :D (I think my
brain memorized how to type "initial" fast, the wrong way)
Thanks!
>
> (But you made the same typo twice, so it compiles :)
>
>> + run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
>> + break;
>
> Moan on default case hit?
Yes, makes sense!
>
>> + }
>> + s390_ipl_clear_reset_request();
>> }
>>
>> static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
Thanks!
--
Thanks,
David / dhildenb
Not a full test, but reboot and kdump seem to work ok with KVM.
On 04/12/2018 09:26 PM, David Hildenbrand wrote:
> Calling pause_all_vcpus()/resume_all_vcpus() from a VCPU thread might
> not be the best idea. As pause_all_vcpus() temporarily drops the qemu
> mutex, two parallel calls to pause_all_vcpus() can be active at a time,
> resulting in a deadlock. (either by two VCPUs or by the main thread and a
> VCPU)
>
> Let's handle it via the main loop instead, as suggested by Paolo. If we
> would have two parallel reset requests by two different VCPUs at the
> same time, the last one would win.
>
> We use the existing ipl device to handle it. The nice side effect is
> that we can get rid of reipl_requested.
>
> This change implies that all reset handling now goes via the common
> path, so "no-reboot" handling is now active for all kinds of reboots.
>
> Let's execute any CPU initialization code on the target CPU using
> run_on_cpu.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>
> Did only a quick check with TCG. I think this way it is way easier to
> control what is getting reset.
>
> hw/s390x/ipl.c | 44 ++++++++++++++++++++++++---
> hw/s390x/ipl.h | 16 ++++++++--
> hw/s390x/s390-virtio-ccw.c | 49 +++++++++++++++++++++++++-----
> include/hw/s390x/s390-virtio-ccw.h | 2 --
> target/s390x/cpu.h | 26 ++++++++++++++++
> target/s390x/diag.c | 61 +++-----------------------------------
> target/s390x/internal.h | 6 ----
> target/s390x/kvm.c | 2 +-
> 8 files changed, 127 insertions(+), 79 deletions(-)
>
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index fb554ab156..f7589d20f1 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -26,6 +26,7 @@
> #include "qemu/config-file.h"
> #include "qemu/cutils.h"
> #include "qemu/option.h"
> +#include "exec/exec-all.h"
>
> #define KERN_IMAGE_START 0x010000UL
> #define KERN_PARM_AREA 0x010480UL
> @@ -484,11 +485,22 @@ IplParameterBlock *s390_ipl_get_iplb(void)
> return &ipl->iplb;
> }
>
> -void s390_reipl_request(void)
> +void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
> {
> S390IPLState *ipl = get_ipl_device();
>
> - ipl->reipl_requested = true;
> + if (reset_type == S390_RESET_EXTERNAL || reset_type == S390_RESET_REIPL) {
> + /* let's always use CPU 0 */
> + ipl->reset_cpu = NULL;
> + } else {
> + ipl->reset_cpu = cs;
> + }
> + ipl->reset_type = reset_type;
> +
> + if (reset_type != S390_RESET_REIPL) {
> + goto no_reipl;
> + }
> +
> if (ipl->iplb_valid &&
> !ipl->netboot &&
> ipl->iplb.pbt == S390_IPL_TYPE_CCW &&
> @@ -505,7 +517,32 @@ void s390_reipl_request(void)
> ipl->iplb_valid = s390_gen_initial_iplb(ipl);
> }
> }
> +no_reipl:
> qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> + /* as this is triggered by a CPU, make sure to exit the loop */
> + if (tcg_enabled()) {
> + cpu_loop_exit(cs);
> + }
> +}
> +
> +void s390_ipl_get_reset_request(CPUState **cs, enum s390_reset *reset_type)
> +{
> + S390IPLState *ipl = get_ipl_device();
> +
> + *cs = ipl->reset_cpu;
> + if (!ipl->reset_cpu) {
> + /* use CPU 0 as default */
> + *cs = qemu_get_cpu(0);
> + }
> + *reset_type = ipl->reset_type;
> +}
> +
> +void s390_ipl_clear_reset_request(void)
> +{
> + S390IPLState *ipl = get_ipl_device();
> +
> + ipl->reset_type = S390_RESET_EXTERNAL;
> + ipl->reset_cpu = NULL;
> }
>
> static void s390_ipl_prepare_qipl(S390CPU *cpu)
> @@ -552,11 +589,10 @@ static void s390_ipl_reset(DeviceState *dev)
> {
> S390IPLState *ipl = S390_IPL(dev);
>
> - if (!ipl->reipl_requested) {
> + if (ipl->reset_type != S390_RESET_REIPL) {
> ipl->iplb_valid = false;
> memset(&ipl->iplb, 0, sizeof(IplParameterBlock));
> }
> - ipl->reipl_requested = false;
> }
>
> static void s390_ipl_class_init(ObjectClass *klass, void *data)
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index 0570d0ad75..102f1ea7af 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -87,7 +87,17 @@ int s390_ipl_set_loadparm(uint8_t *loadparm);
> void s390_ipl_update_diag308(IplParameterBlock *iplb);
> void s390_ipl_prepare_cpu(S390CPU *cpu);
> IplParameterBlock *s390_ipl_get_iplb(void);
> -void s390_reipl_request(void);
> +
> +enum s390_reset {
> + /* default is a reset not triggered by a CPU e.g. issued by QMP */
> + S390_RESET_EXTERNAL = 0,
> + S390_RESET_REIPL,
> + S390_RESET_MODIFIED_CLEAR,
> + S390_RESET_LOAD_NORMAL,
> +};
> +void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type);
> +void s390_ipl_get_reset_request(CPUState **cs, enum s390_reset *reset_type);
> +void s390_ipl_clear_reset_request(void);
>
> #define QIPL_ADDRESS 0xcc
>
> @@ -129,9 +139,11 @@ struct S390IPLState {
> bool enforce_bios;
> IplParameterBlock iplb;
> bool iplb_valid;
> - bool reipl_requested;
> bool netboot;
> QemuIplParameters qipl;
> + /* reset related properties don't have to be migrated or reset */
> + enum s390_reset reset_type;
> + CPUState *reset_cpu;
>
> /*< public >*/
> char *kernel;
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 435f7c99e7..f625900435 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -93,7 +93,7 @@ static const char *const reset_dev_types[] = {
> "diag288",
> };
>
> -void subsystem_reset(void)
> +static void subsystem_reset(void)
> {
> DeviceState *dev;
> int i;
> @@ -364,17 +364,52 @@ static void s390_cpu_plug(HotplugHandler *hotplug_dev,
> }
> }
>
> +static inline void s390_do_cpu_ipl(CPUState *cs, run_on_cpu_data arg)
> +{
> + S390CPU *cpu = S390_CPU(cs);
> +
> + s390_ipl_prepare_cpu(cpu);
> + s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
> +}
> +
> static void s390_machine_reset(void)
> {
> - S390CPU *ipl_cpu = S390_CPU(qemu_get_cpu(0));
> + enum s390_reset reset_type;
> + CPUState *cs, *t;
>
> + /* get the reset parameters, reset them once done */
> + s390_ipl_get_reset_request(&cs, &reset_type);
> +
> + /* all CPUs are paused and synchronized at this point */
> s390_cmma_reset();
> - qemu_devices_reset();
> - s390_crypto_reset();
>
> - /* all cpus are stopped - configure and start the ipl cpu only */
> - s390_ipl_prepare_cpu(ipl_cpu);
> - s390_cpu_set_state(S390_CPU_STATE_OPERATING, ipl_cpu);
> + switch (reset_type) {
> + case S390_RESET_EXTERNAL:
> + case S390_RESET_REIPL:
> + 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_MODIFIED_CLEAR:
> + 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:
> + CPU_FOREACH(t) {
> + run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
> + }
> + subsystem_reset();
> + run_on_cpu(cs, s390_do_cpu_initital_reset, RUN_ON_CPU_NULL);
> + run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
> + break;
> + }
> + s390_ipl_clear_reset_request();
> }
>
> static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
> index ac896e31ea..ab88d49d10 100644
> --- a/include/hw/s390x/s390-virtio-ccw.h
> +++ b/include/hw/s390x/s390-virtio-ccw.h
> @@ -53,6 +53,4 @@ bool cpu_model_allowed(void);
> */
> bool css_migration_enabled(void);
>
> -void subsystem_reset(void);
> -
> #endif
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 3ee40f08b7..0c2583fd3c 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -686,6 +686,32 @@ static inline uint64_t s390_build_validity_mcic(void)
> return mcic;
> }
>
> +static inline void s390_do_cpu_full_reset(CPUState *cs, run_on_cpu_data arg)
> +{
> + cpu_reset(cs);
> +}
> +
> +static inline void s390_do_cpu_reset(CPUState *cs, run_on_cpu_data arg)
> +{
> + S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
> +
> + scc->cpu_reset(cs);
> +}
> +
> +static inline void s390_do_cpu_initital_reset(CPUState *cs, run_on_cpu_data arg)
> +{
> + S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
> +
> + scc->initial_cpu_reset(cs);
> +}
> +
> +static inline void s390_do_cpu_load_normal(CPUState *cs, run_on_cpu_data arg)
> +{
> + S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
> +
> + scc->load_normal(cs);
> +}
> +
>
> /* cpu.c */
> int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low);
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index a755837ad5..ac2c40f363 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -22,51 +22,6 @@
> #include "hw/s390x/ipl.h"
> #include "hw/s390x/s390-virtio-ccw.h"
>
> -static int modified_clear_reset(S390CPU *cpu)
> -{
> - S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
> - CPUState *t;
> -
> - pause_all_vcpus();
> - cpu_synchronize_all_states();
> - CPU_FOREACH(t) {
> - run_on_cpu(t, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
> - }
> - s390_cmma_reset();
> - subsystem_reset();
> - s390_crypto_reset();
> - scc->load_normal(CPU(cpu));
> - cpu_synchronize_all_post_reset();
> - resume_all_vcpus();
> - return 0;
> -}
> -
> -static inline void s390_do_cpu_reset(CPUState *cs, run_on_cpu_data arg)
> -{
> - S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
> -
> - scc->cpu_reset(cs);
> -}
> -
> -static int load_normal_reset(S390CPU *cpu)
> -{
> - S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
> - CPUState *t;
> -
> - pause_all_vcpus();
> - cpu_synchronize_all_states();
> - CPU_FOREACH(t) {
> - run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
> - }
> - s390_cmma_reset();
> - subsystem_reset();
> - scc->initial_cpu_reset(CPU(cpu));
> - scc->load_normal(CPU(cpu));
> - cpu_synchronize_all_post_reset();
> - resume_all_vcpus();
> - return 0;
> -}
> -
> int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
> {
> uint64_t func = env->regs[r1];
> @@ -101,6 +56,7 @@ int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
>
> void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
> {
> + CPUState *cs = CPU(s390_env_get_cpu(env));
> uint64_t addr = env->regs[r1];
> uint64_t subcode = env->regs[r3];
> IplParameterBlock *iplb;
> @@ -117,22 +73,13 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
>
> switch (subcode) {
> case 0:
> - modified_clear_reset(s390_env_get_cpu(env));
> - if (tcg_enabled()) {
> - cpu_loop_exit(CPU(s390_env_get_cpu(env)));
> - }
> + s390_ipl_reset_request(cs, S390_RESET_MODIFIED_CLEAR);
> break;
> case 1:
> - load_normal_reset(s390_env_get_cpu(env));
> - if (tcg_enabled()) {
> - cpu_loop_exit(CPU(s390_env_get_cpu(env)));
> - }
> + s390_ipl_reset_request(cs, S390_RESET_LOAD_NORMAL);
> break;
> case 3:
> - s390_reipl_request();
> - if (tcg_enabled()) {
> - cpu_loop_exit(CPU(s390_env_get_cpu(env)));
> - }
> + s390_ipl_reset_request(cs, S390_RESET_REIPL);
> break;
> case 5:
> if ((r1 & 1) || (addr & 0x0fffULL)) {
> diff --git a/target/s390x/internal.h b/target/s390x/internal.h
> index d911e84958..e392a02d12 100644
> --- a/target/s390x/internal.h
> +++ b/target/s390x/internal.h
> @@ -273,12 +273,6 @@ static inline hwaddr decode_basedisp_s(CPUS390XState *env, uint32_t ipb,
> /* Base/displacement are at the same locations. */
> #define decode_basedisp_rs decode_basedisp_s
>
> -static inline void s390_do_cpu_full_reset(CPUState *cs, run_on_cpu_data arg)
> -{
> - cpu_reset(cs);
> -}
> -
> -
> /* arch_dump.c */
> int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
> int cpuid, void *opaque);
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index fb59d92def..b033d53f69 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -1785,7 +1785,7 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
> ret = handle_intercept(cpu);
> break;
> case KVM_EXIT_S390_RESET:
> - s390_reipl_request();
> + s390_ipl_reset_request(cs, S390_RESET_REIPL);
> break;
> case KVM_EXIT_S390_TSCH:
> ret = handle_tsch(cpu);
>
© 2016 - 2024 Red Hat, Inc.