[PATCH v2 3/3] s390x: Fix cpu normal reset ri clearing
As it turns out we need to clear the ri controls and PSW enablement bit to be architecture compliant. Signed-off-by: Janosch Frank --- target/s390x/cpu.c | 5 + target/s390x/cpu.h | 7 ++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c index 906285888e..c192e6b3b9 100644 --- a/target/s390x/cpu.c +++ b/target/s390x/cpu.c @@ -131,6 +131,11 @@ static void s390_cpu_reset(CPUState *s, cpu_reset_type type) &env->fpu_status); /* fall through */ case S390_CPU_RESET_NORMAL: +env->psw.mask &= ~PSW_MASK_RI; +memset(&env->start_normal_reset_fields, 0, + offsetof(CPUS390XState, end_reset_fields) - + offsetof(CPUS390XState, start_normal_reset_fields)); + env->pfault_token = -1UL; env->bpbc = false; break; diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index d5e18b096e..7f5fa1d35b 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -58,7 +58,6 @@ struct CPUS390XState { */ uint64_t vregs[32][2] QEMU_ALIGNED(16); /* vector registers */ uint32_t aregs[16];/* access registers */ -uint8_t riccb[64]; /* runtime instrumentation control */ uint64_t gscb[4]; /* guarded storage control */ uint64_t etoken; /* etoken */ uint64_t etoken_extension; /* etoken extension */ @@ -114,6 +113,10 @@ struct CPUS390XState { uint64_t gbea; uint64_t pp; +/* Fields up to this point are not cleared by normal CPU reset */ +struct {} start_normal_reset_fields; +uint8_t riccb[64]; /* runtime instrumentation control */ + /* Fields up to this point are cleared by a CPU reset */ struct {} end_reset_fields; @@ -252,6 +255,7 @@ extern const VMStateDescription vmstate_s390_cpu; #undef PSW_SHIFT_ASC #undef PSW_MASK_CC #undef PSW_MASK_PM +#undef PSW_MASK_RI #undef PSW_SHIFT_MASK_PM #undef PSW_MASK_64 #undef PSW_MASK_32 @@ -274,6 +278,7 @@ extern const VMStateDescription vmstate_s390_cpu; #define PSW_MASK_CC 0x3000ULL #define PSW_MASK_PM 0x0F00ULL #define PSW_SHIFT_MASK_PM 40 +#define PSW_MASK_RI 0x0080ULL #define PSW_MASK_64 0x0001ULL #define PSW_MASK_32 0x8000ULL #define PSW_MASK_ESA_ADDR 0x7fffULL -- 2.20.1
Re: [PATCH v2 3/3] s390x: Fix cpu normal reset ri clearing
On 02.12.19 15:01, Janosch Frank wrote: > As it turns out we need to clear the ri controls and PSW enablement > bit to be architecture compliant. > > Signed-off-by: Janosch Frank > --- > target/s390x/cpu.c | 5 + > target/s390x/cpu.h | 7 ++- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c > index 906285888e..c192e6b3b9 100644 > --- a/target/s390x/cpu.c > +++ b/target/s390x/cpu.c > @@ -131,6 +131,11 @@ static void s390_cpu_reset(CPUState *s, cpu_reset_type > type) >&env->fpu_status); > /* fall through */ As this is a fall through, do we want to change the INITIAL RESET to only clear up to start_normal_reset_fields, e.g. something like this on top diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c index 829ce6ad5491..58ac721687a9 100644 --- a/target/s390x/cpu.c +++ b/target/s390x/cpu.c @@ -108,7 +108,7 @@ static void s390_cpu_reset(CPUState *s, cpu_reset_type type) 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, start_normal_reset_fields) - offsetof(CPUS390XState, start_initial_reset_fields)); /* architectured initial value for Breaking-Event-Address register */ to avoid double memsetting. Other than this question Reviewed-by: Christian Borntraeger > case S390_CPU_RESET_NORMAL: > +env->psw.mask &= ~PSW_MASK_RI; > +memset(&env->start_normal_reset_fields, 0, > + offsetof(CPUS390XState, end_reset_fields) - > + offsetof(CPUS390XState, start_normal_reset_fields)); > + > env->pfault_token = -1UL; > env->bpbc = false; > break; > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h > index d5e18b096e..7f5fa1d35b 100644 > --- a/target/s390x/cpu.h > +++ b/target/s390x/cpu.h > @@ -58,7 +58,6 @@ struct CPUS390XState { > */ > uint64_t vregs[32][2] QEMU_ALIGNED(16); /* vector registers */ > uint32_t aregs[16];/* access registers */ > -uint8_t riccb[64]; /* runtime instrumentation control */ > uint64_t gscb[4]; /* guarded storage control */ > uint64_t etoken; /* etoken */ > uint64_t etoken_extension; /* etoken extension */ > @@ -114,6 +113,10 @@ struct CPUS390XState { > uint64_t gbea; > uint64_t pp; > > +/* Fields up to this point are not cleared by normal CPU reset */ > +struct {} start_normal_reset_fields; > +uint8_t riccb[64]; /* runtime instrumentation control */ > + > /* Fields up to this point are cleared by a CPU reset */ > struct {} end_reset_fields; > > @@ -252,6 +255,7 @@ extern const VMStateDescription vmstate_s390_cpu; > #undef PSW_SHIFT_ASC > #undef PSW_MASK_CC > #undef PSW_MASK_PM > +#undef PSW_MASK_RI > #undef PSW_SHIFT_MASK_PM > #undef PSW_MASK_64 > #undef PSW_MASK_32 > @@ -274,6 +278,7 @@ extern const VMStateDescription vmstate_s390_cpu; > #define PSW_MASK_CC 0x3000ULL > #define PSW_MASK_PM 0x0F00ULL > #define PSW_SHIFT_MASK_PM 40 > +#define PSW_MASK_RI 0x0080ULL > #define PSW_MASK_64 0x0001ULL > #define PSW_MASK_32 0x8000ULL > #define PSW_MASK_ESA_ADDR 0x7fffULL >
Re: [PATCH v2 3/3] s390x: Fix cpu normal reset ri clearing
On 02.12.19 15:01, Janosch Frank wrote: > As it turns out we need to clear the ri controls and PSW enablement > bit to be architecture compliant. > > Signed-off-by: Janosch Frank > --- > target/s390x/cpu.c | 5 + > target/s390x/cpu.h | 7 ++- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c > index 906285888e..c192e6b3b9 100644 > --- a/target/s390x/cpu.c > +++ b/target/s390x/cpu.c > @@ -131,6 +131,11 @@ static void s390_cpu_reset(CPUState *s, cpu_reset_type > type) >&env->fpu_status); > /* fall through */ > case S390_CPU_RESET_NORMAL: > +env->psw.mask &= ~PSW_MASK_RI; > +memset(&env->start_normal_reset_fields, 0, > + offsetof(CPUS390XState, end_reset_fields) - > + offsetof(CPUS390XState, start_normal_reset_fields)); > + > env->pfault_token = -1UL; > env->bpbc = false; > break; > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h > index d5e18b096e..7f5fa1d35b 100644 > --- a/target/s390x/cpu.h > +++ b/target/s390x/cpu.h > @@ -58,7 +58,6 @@ struct CPUS390XState { > */ > uint64_t vregs[32][2] QEMU_ALIGNED(16); /* vector registers */ > uint32_t aregs[16];/* access registers */ > -uint8_t riccb[64]; /* runtime instrumentation control */ > uint64_t gscb[4]; /* guarded storage control */ > uint64_t etoken; /* etoken */ > uint64_t etoken_extension; /* etoken extension */ > @@ -114,6 +113,10 @@ struct CPUS390XState { > uint64_t gbea; > uint64_t pp; > > +/* Fields up to this point are not cleared by normal CPU reset */ > +struct {} start_normal_reset_fields; > +uint8_t riccb[64]; /* runtime instrumentation control */ > + > /* Fields up to this point are cleared by a CPU reset */ > struct {} end_reset_fields; > > @@ -252,6 +255,7 @@ extern const VMStateDescription vmstate_s390_cpu; > #undef PSW_SHIFT_ASC > #undef PSW_MASK_CC > #undef PSW_MASK_PM > +#undef PSW_MASK_RI > #undef PSW_SHIFT_MASK_PM > #undef PSW_MASK_64 > #undef PSW_MASK_32 > @@ -274,6 +278,7 @@ extern const VMStateDescription vmstate_s390_cpu; > #define PSW_MASK_CC 0x3000ULL > #define PSW_MASK_PM 0x0F00ULL > #define PSW_SHIFT_MASK_PM 40 > +#define PSW_MASK_RI 0x0080ULL > #define PSW_MASK_64 0x0001ULL > #define PSW_MASK_32 0x8000ULL > #define PSW_MASK_ESA_ADDR 0x7fffULL > I'm afraid I can't help because the documentation is confidential. It does look sane to me, at least. -- Thanks, David / dhildenb