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

[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 <address@hidden>
> ---
>  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.

-- 
Thanks,

David / dhildenb