On 05.12.19 11:38, Cornelia Huck wrote: > We neglected to clean up pending interrupts and emergency signals; > fix that. > > Signed-off-by: Cornelia Huck <coh...@redhat.com> > --- > > Noted while looking at the fixes for the kvm reset handling. > > We now clear some fields twice in the paths for clear or initial reset; > but (a) we already do that for other fields and (b) it does not really > hurt. Maybe we should give the cpu structure some love in the future, > as it's not always clear whether some fields are tcg only. > > --- > target/s390x/cpu.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c > index 829ce6ad5491..f2572961dc3a 100644 > --- a/target/s390x/cpu.c > +++ b/target/s390x/cpu.c > @@ -133,6 +133,9 @@ static void s390_cpu_reset(CPUState *s, cpu_reset_type > type) > case S390_CPU_RESET_NORMAL: > env->pfault_token = -1UL; > env->bpbc = false; > + env->pending_int = 0; > + env->external_call_addr = 0; > + bitmap_zero(env->emergency_signals, S390_MAX_CPUS); > break; > default: > g_assert_not_reached(); >
I'd suggest we rework the memsetting instead, now that we always "fall through" in this call chain, e.g, something like diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c index bd39cb54b7..492f0c766d 100644 --- a/target/s390x/cpu.c +++ b/target/s390x/cpu.c @@ -95,12 +95,14 @@ static void s390_cpu_reset(CPUState *s, cpu_reset_type type) switch (type) { case S390_CPU_RESET_CLEAR: - memset(env, 0, offsetof(CPUS390XState, start_initial_reset_fields)); + memset(&env->start_clear_reset_fields, 0, + offsetof(CPUS390XState, end_clear_reset_fields) - + offsetof(CPUS390XState, start_clear_reset_fields)); /* fall through */ case S390_CPU_RESET_INITIAL: /* initial reset does not clear everything! */ memset(&env->start_initial_reset_fields, 0, - offsetof(CPUS390XState, end_reset_fields) - + offsetof(CPUS390XState, end_initial_reset_fields) - offsetof(CPUS390XState, start_initial_reset_fields)); /* architectured initial value for Breaking-Event-Address register */ @@ -123,8 +125,10 @@ static void s390_cpu_reset(CPUState *s, cpu_reset_type type) &env->fpu_status); /* fall through */ case S390_CPU_RESET_NORMAL: + memset(&env->start_normal_reset_fields, 0, + offsetof(CPUS390XState, end_normal_reset_fields) - + offsetof(CPUS390XState, start_normal_reset_fields)); env->pfault_token = -1UL; - env->bpbc = false; break; default: g_assert_not_reached(); diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index d2af13b345..fe2ab6b89e 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -51,6 +51,9 @@ typedef struct PSW { } PSW; struct CPUS390XState { + + struct {} start_clear_reset_fields; + uint64_t regs[16]; /* GP registers */ /* * The floating point registers are part of the vector registers. @@ -63,12 +66,11 @@ struct CPUS390XState { uint64_t etoken; /* etoken */ uint64_t etoken_extension; /* etoken extension */ - /* Fields up to this point are not cleared by initial CPU reset */ + struct {} end_clear_reset_fields; struct {} start_initial_reset_fields; uint32_t fpc; /* floating-point control register */ uint32_t cc_op; - bool bpbc; /* branch prediction blocking */ float_status fpu_status; /* passed to softfloat lib */ @@ -99,10 +101,6 @@ struct CPUS390XState { uint64_t cregs[16]; /* control registers */ - int pending_int; - uint16_t external_call_addr; - DECLARE_BITMAP(emergency_signals, S390_MAX_CPUS); - uint64_t ckc; uint64_t cputm; uint32_t todpr; @@ -114,8 +112,17 @@ struct CPUS390XState { uint64_t gbea; uint64_t pp; - /* Fields up to this point are cleared by a CPU reset */ - struct {} end_reset_fields; + struct {} end_initial_reset_fields; + struct {} start_normal_reset_fields; + + /* local interrupt state */ + int pending_int; + uint16_t external_call_addr; + DECLARE_BITMAP(emergency_signals, S390_MAX_CPUS); + + bool bpbc; /* branch prediction blocking */ + + struct {} end_normal_reset_fields; #if !defined(CONFIG_USER_ONLY) uint32_t core_id; /* PoP "CPU address", same as cpu_index */ -- 2.21.0 Wrapping in CONFIG_TCG requires more work. -- Thanks, David / dhildenb