Re: [qemu-s390x] [PATCH v1 for-2.12 2/5] s390x/tcg: fix and cleanup mcck injection

[Thomas Huth]

On 04.12.2017 13:55, David Hildenbrand wrote:
> The architecture mode indication wasn't stored. The split of certain
> 64bit fields was unnecessary. Also, the complete clock comparator, not
> just bit 0-55 (starting at byte 1) was stored.
> 
> We now generate a proper MCIC via the same helper we use for KVM.
> 
> While at it, also get rid of two local variables. There is be more to
> clean up, but we will change the other parts later on either way.
> 
> Signed-off-by: David Hildenbrand <address@hidden>
> ---
>  target/s390x/excp_helper.c | 18 ++++++++----------
>  target/s390x/internal.h    |  6 +++---
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
> index d831537544..840cf7641a 100644
> --- a/target/s390x/excp_helper.c
> +++ b/target/s390x/excp_helper.c
> @@ -369,7 +369,6 @@ static void do_io_interrupt(CPUS390XState *env)
>  static void do_mchk_interrupt(CPUS390XState *env)
>  {
>      S390CPU *cpu = s390_env_get_cpu(env);
> -    uint64_t mask, addr;
>      LowCore *lowcore;
>      MchkQueue *q;
>      int i;
> @@ -395,6 +394,9 @@ static void do_mchk_interrupt(CPUS390XState *env)
>  
>      lowcore = cpu_map_lowcore(env);
>  
> +    /* we are always in z/Architecture mode */
> +    lowcore->ar_access_id = 1;
> +
>      for (i = 0; i < 16; i++) {
>          lowcore->floating_pt_save_area[i] = cpu_to_be64(get_freg(env, 
> i)->ll);
>          lowcore->gpregs_save_area[i] = cpu_to_be64(env->regs[i]);
> @@ -404,17 +406,12 @@ static void do_mchk_interrupt(CPUS390XState *env)
>      lowcore->prefixreg_save_area = cpu_to_be32(env->psa);
>      lowcore->fpt_creg_save_area = cpu_to_be32(env->fpc);
>      lowcore->tod_progreg_save_area = cpu_to_be32(env->todpr);
> -    lowcore->cpu_timer_save_area[0] = cpu_to_be32(env->cputm >> 32);
> -    lowcore->cpu_timer_save_area[1] = cpu_to_be32((uint32_t)env->cputm);
> -    lowcore->clock_comp_save_area[0] = cpu_to_be32(env->ckc >> 32);
> -    lowcore->clock_comp_save_area[1] = cpu_to_be32((uint32_t)env->ckc);
> +    lowcore->cpu_timer_save_area = cpu_to_be64(env->cputm);
> +    lowcore->clock_comp_save_area = cpu_to_be64(env->ckc >> 8);
>  
> -    lowcore->mcck_interruption_code[0] = cpu_to_be32(0x00400f1d);
> -    lowcore->mcck_interruption_code[1] = cpu_to_be32(0x40330000);
> +    lowcore->mcic = cpu_to_be64(s390_build_validity_mcic() | MCIC_SC_CP);
>      lowcore->mcck_old_psw.mask = cpu_to_be64(get_psw_mask(env));
>      lowcore->mcck_old_psw.addr = cpu_to_be64(env->psw.addr);
> -    mask = be64_to_cpu(lowcore->mcck_new_psw.mask);
> -    addr = be64_to_cpu(lowcore->mcck_new_psw.addr);
>  
>      cpu_unmap_lowcore(lowcore);

Lowcore gets unmapped here...

> @@ -426,7 +423,8 @@ static void do_mchk_interrupt(CPUS390XState *env)
>      DPRINTF("%s: %" PRIx64 " %" PRIx64 "\n", __func__,
>              env->psw.mask, env->psw.addr);
>  
> -    load_psw(env, mask, addr);
> +    load_psw(env, be64_to_cpu(lowcore->mcck_new_psw.mask),
> +             be64_to_cpu(lowcore->mcck_new_psw.addr));

... but here you still touch it? Looks like a bug to me...?

 Thomas