Re: [Qemu-devel] [PATCH-for-4.2 v2 2/6] s390x/tcg: Rework MMU selection for instruction fetches

[Cornelia Huck <cohuck@redhat.com>]

On Wed, 14 Aug 2019 09:23:51 +0200
David Hildenbrand <david@redhat.com> wrote:

> Instructions are always fetched from primary address space, except when
> in home address mode. Perform the selection directly in cpu_mmu_index().
> 
> get_mem_index() is only used to perform data access, instructions are
> fetched via cpu_lduw_code(), which translates to cpu_mmu_index(env, true).
> 
> We don't care about restricting the access permissions of the TLB
> entries anymore, as we no longer enter PRIMARY entries into the
> SECONDARY MMU. Cleanup related code a bit.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/cpu.h        |  7 +++++++
>  target/s390x/mmu_helper.c | 38 +++++++++++++++-----------------------
>  2 files changed, 22 insertions(+), 23 deletions(-)
> 

(...)

> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> index 6e9c4d6151..c34e8d2021 100644
> --- a/target/s390x/mmu_helper.c
> +++ b/target/s390x/mmu_helper.c
> @@ -349,8 +349,9 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
>  {
>      static S390SKeysState *ss;
>      static S390SKeysClass *skeyclass;
> -    int r = -1;
> +    uint64_t asce;
>      uint8_t key;
> +    int r;
>  
>      if (unlikely(!ss)) {
>          ss = s390_get_skeys_device();
> @@ -380,36 +381,21 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
>  
>      if (!(env->psw.mask & PSW_MASK_DAT)) {
>          *raddr = vaddr;
> -        r = 0;
> -        goto out;
> +        goto nodat;
>      }
>  
>      switch (asc) {
>      case PSW_ASC_PRIMARY:
>          PTE_DPRINTF("%s: asc=primary\n", __func__);
> -        r = mmu_translate_asce(env, vaddr, asc, env->cregs[1], raddr, flags,
> -                               rw, exc);
> +        asce = env->cregs[1];
>          break;
>      case PSW_ASC_HOME:
>          PTE_DPRINTF("%s: asc=home\n", __func__);
> -        r = mmu_translate_asce(env, vaddr, asc, env->cregs[13], raddr, flags,
> -                               rw, exc);
> +        asce = env->cregs[13];
>          break;
>      case PSW_ASC_SECONDARY:
>          PTE_DPRINTF("%s: asc=secondary\n", __func__);
> -        /*
> -         * Instruction: Primary
> -         * Data: Secondary
> -         */
> -        if (rw == MMU_INST_FETCH) {
> -            r = mmu_translate_asce(env, vaddr, PSW_ASC_PRIMARY, env->cregs[1],
> -                                   raddr, flags, rw, exc);
> -            *flags &= ~(PAGE_READ | PAGE_WRITE);
> -        } else {
> -            r = mmu_translate_asce(env, vaddr, PSW_ASC_SECONDARY, env->cregs[7],
> -                                   raddr, flags, rw, exc);
> -            *flags &= ~(PAGE_EXEC);
> -        }
> +        asce = env->cregs[7];
>          break;
>      case PSW_ASC_ACCREG:
>      default:
> @@ -417,11 +403,17 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
>          break;
>      }
>  
> - out:
> +    /* perform the DAT translation */
> +    r = mmu_translate_asce(env, vaddr, asc, asce, raddr, flags, rw, exc);
> +    if (r) {
> +        return r;
> +    }
> +
> +nodat:
>      /* Convert real address -> absolute address */
>      *raddr = mmu_real2abs(env, *raddr);
>  
> -    if (r == 0 && *raddr < ram_size) {
> +    if (*raddr < ram_size) {
>          if (skeyclass->get_skeys(ss, *raddr / TARGET_PAGE_SIZE, 1, &key)) {
>              trace_get_skeys_nonzero(r);

I think you might up here with an uninitialized r before patch 4?

>              return 0;
> @@ -441,7 +433,7 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
>          }
>      }
>  
> -    return r;
> +    return 0;
>  }
>  
>  /**