On 18.09.2017 17:59, David Hildenbrand wrote:
> This will now also detect crashes under TCG. We can directly use
> cpu->env.psw.addr instead of kvm_run, as we do a
> cpu_synchronize_state().
>
> Signed-off-by: David Hildenbrand <da...@redhat.com>
> ---
> target/s390x/helper.c | 28 ++++++++++++++++++++++------
> target/s390x/internal.h | 1 +
> target/s390x/kvm.c | 15 +--------------
> 3 files changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> index e22b93258b..75ceb0bf2b 100644
> --- a/target/s390x/helper.c
> +++ b/target/s390x/helper.c
> @@ -26,6 +26,7 @@
> #include "qemu/timer.h"
> #include "exec/exec-all.h"
> #include "hw/s390x/ioinst.h"
> +#include "sysemu/hw_accel.h"
> #ifndef CONFIG_USER_ONLY
> #include "sysemu/sysemu.h"
> #endif
> @@ -113,6 +114,26 @@ hwaddr s390_cpu_get_phys_addr_debug(CPUState *cs, vaddr
> vaddr)
> return phys_addr;
> }
>
> +static inline bool is_special_wait_psw(uint64_t psw_addr)
> +{
> + /* signal quiesce */
> + return psw_addr == 0xfffUL;
> +}
> +
> +void handle_wait(S390CPU *cpu)
"handle_wait" is a very generic name ... could you maybe at least name
this "s390x_handle_wait" or so?
> +{
> + cpu_synchronize_state(CPU(cpu));
> + if (s390_cpu_halt(cpu) == 0) {
> +#ifndef CONFIG_USER_ONLY
> + if (is_special_wait_psw(cpu->env.psw.addr)) {
> + qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> + } else {
> + qemu_system_guest_panicked(NULL);
> + }
> +#endif
> + }
> +}
> +
> void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr)
> {
> uint64_t old_mask = env->psw.mask;
> @@ -128,12 +149,7 @@ void load_psw(CPUS390XState *env, uint64_t mask,
> uint64_t addr)
> }
>
> if (mask & PSW_MASK_WAIT) {
> - S390CPU *cpu = s390_env_get_cpu(env);
> - if (s390_cpu_halt(cpu) == 0) {
> -#ifndef CONFIG_USER_ONLY
> - qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> -#endif
> - }
> + handle_wait(s390_env_get_cpu(env));
> }
> }
>
> diff --git a/target/s390x/internal.h b/target/s390x/internal.h
> index 15743ec40f..cb331f35ea 100644
> --- a/target/s390x/internal.h
> +++ b/target/s390x/internal.h
> @@ -280,6 +280,7 @@ const char *cc_name(enum cc_op cc_op);
> void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr);
> uint32_t calc_cc(CPUS390XState *env, uint32_t cc_op, uint64_t src, uint64_t
> dst,
> uint64_t vr);
> +void handle_wait(S390CPU *cpu);
>
>
> /* cpu.c */
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 3f9983154f..14f864697d 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -1936,12 +1936,6 @@ static int handle_instruction(S390CPU *cpu, struct
> kvm_run *run)
> return r;
> }
>
> -static bool is_special_wait_psw(CPUState *cs)
> -{
> - /* signal quiesce */
> - return cs->kvm_run->psw_addr == 0xfffUL;
> -}
> -
> static void unmanageable_intercept(S390CPU *cpu, const char *str, int
> pswoffset)
> {
> CPUState *cs = CPU(cpu);
> @@ -2012,14 +2006,7 @@ static int handle_intercept(S390CPU *cpu)
> break;
> case ICPT_WAITPSW:
> /* disabled wait, since enabled wait is handled in kernel */
> - cpu_synchronize_state(cs);
Maybe we should rather keep the cpu_synchronize_state() here instead of
putting it in helper.c? No other function in helper.c seems to call that
on its own yet...
> - if (s390_cpu_halt(cpu) == 0) {
> - if (is_special_wait_psw(cs)) {
> -
> qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> - } else {
> - qemu_system_guest_panicked(NULL);
> - }
> - }
> + handle_wait(cpu);
> r = EXCP_HALTED;
> break;
> case ICPT_CPU_STOP:
>
Thomas
