| Message ID | 20191202140146.3910-4-frankja@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x: Increase architectural compliance | expand |
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 <frankja@linux.ibm.com> > --- > 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 <borntraeger@de.ibm.com> > 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 0x0000300000000000ULL > #define PSW_MASK_PM 0x00000F0000000000ULL > #define PSW_SHIFT_MASK_PM 40 > +#define PSW_MASK_RI 0x0000008000000000ULL > #define PSW_MASK_64 0x0000000100000000ULL > #define PSW_MASK_32 0x0000000080000000ULL > #define PSW_MASK_ESA_ADDR 0x000000007fffffffULL >
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 <frankja@linux.ibm.com> > --- > 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 0x0000300000000000ULL > #define PSW_MASK_PM 0x00000F0000000000ULL > #define PSW_SHIFT_MASK_PM 40 > +#define PSW_MASK_RI 0x0000008000000000ULL > #define PSW_MASK_64 0x0000000100000000ULL > #define PSW_MASK_32 0x0000000080000000ULL > #define PSW_MASK_ESA_ADDR 0x000000007fffffffULL > I'm afraid I can't help because the documentation is confidential. It does look sane to me, at least.
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 0x0000300000000000ULL #define PSW_MASK_PM 0x00000F0000000000ULL #define PSW_SHIFT_MASK_PM 40 +#define PSW_MASK_RI 0x0000008000000000ULL #define PSW_MASK_64 0x0000000100000000ULL #define PSW_MASK_32 0x0000000080000000ULL #define PSW_MASK_ESA_ADDR 0x000000007fffffffULL
As it turns out we need to clear the ri controls and PSW enablement bit to be architecture compliant. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- target/s390x/cpu.c | 5 +++++ target/s390x/cpu.h | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-)