Re: [PATCH v2 3/3] s390x: Fix cpu normal reset ri clearing

2019-12-03 Thread David Hildenbrand
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)
>>fpu_status);
> /* fall through */
>  case S390_CPU_RESET_NORMAL:
> +env->psw.mask &= ~PSW_MASK_RI;
> +memset(>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




Re: [PATCH v2 3/3] s390x: Fix cpu normal reset ri clearing

2019-12-03 Thread Christian Borntraeger



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)
>>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(>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(>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
> 




[PATCH v2 3/3] s390x: Fix cpu normal reset ri clearing

2019-12-02 Thread Janosch Frank
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)
   >fpu_status);
/* fall through */
 case S390_CPU_RESET_NORMAL:
+env->psw.mask &= ~PSW_MASK_RI;
+memset(>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