Re: [PATCH v3 08/18] target/s390: Return exception from mmu_translate_real

[David Hildenbrand]

On 26.09.19 18:26, Richard Henderson wrote:
> Do not raise the exception directly within mmu_translate_real,
> but pass it back so that caller may do so.
> 
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/s390x/internal.h    | 2 +-
>  target/s390x/excp_helper.c | 4 ++--
>  target/s390x/mmu_helper.c  | 8 ++++----
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/target/s390x/internal.h b/target/s390x/internal.h
> index c243fa725b..c4388aaf23 100644
> --- a/target/s390x/internal.h
> +++ b/target/s390x/internal.h
> @@ -362,7 +362,7 @@ void probe_write_access(CPUS390XState *env, uint64_t 
> addr, uint64_t len,
>  int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t 
> asc,
>                    target_ulong *raddr, int *flags, bool exc);
>  int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
> -                       target_ulong *addr, int *flags);
> +                       target_ulong *addr, int *flags, uint64_t *tec);
>  
>  
>  /* misc_helper.c */
> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
> index ab2ed47fef..906b87c071 100644
> --- a/target/s390x/excp_helper.c
> +++ b/target/s390x/excp_helper.c
> @@ -147,8 +147,8 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int 
> size,
>          if (!(env->psw.mask & PSW_MASK_64)) {
>              vaddr &= 0x7fffffff;
>          }
> -        fail = mmu_translate_real(env, vaddr, access_type, &raddr, &prot);
> -        excp = 0; /* exception already raised */
> +        excp = mmu_translate_real(env, vaddr, access_type, &raddr, &prot, 
> &tec);
> +        fail = excp;
>      } else {
>          g_assert_not_reached();
>      }
> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> index 3ef40a37a7..b783c62bd7 100644
> --- a/target/s390x/mmu_helper.c
> +++ b/target/s390x/mmu_helper.c
> @@ -523,10 +523,10 @@ void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, 
> uintptr_t ra)
>   * @param rw     0 = read, 1 = write, 2 = code fetch
>   * @param addr   the translated address is stored to this pointer
>   * @param flags  the PAGE_READ/WRITE/EXEC flags are stored to this pointer
> - * @return       0 if the translation was successful, < 0 if a fault occurred
> + * @return       0 = success, != 0, the exception to raise
>   */
>  int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
> -                       target_ulong *addr, int *flags)
> +                       target_ulong *addr, int *flags, uint64_t *tec)
>  {
>      const bool lowprot_enabled = env->cregs[0] & CR0_LOWPROT;
>  
> @@ -535,8 +535,8 @@ int mmu_translate_real(CPUS390XState *env, target_ulong 
> raddr, int rw,
>          /* see comment in mmu_translate() how this works */
>          *flags |= PAGE_WRITE_INV;
>          if (is_low_address(raddr) && rw == MMU_DATA_STORE) {
> -            trigger_access_exception(env, PGM_PROTECTION, ILEN_AUTO, 0);
> -            return -EACCES;
> +            *tec = 0;
> +            return PGM_PROTECTION;
>          }
>      }
>  
> 

Note that

[PATCH v1 2/5] s390x/mmu: Implement ESOP-2 and
access-exception-fetch/store-indication facility

also messes with the tec (which is okay), but also with the ILEN on
instruction fetches.

Apart from that

Reviewed-by: David Hildenbrand <address@hidden>

-- 

Thanks,

David / dhildenb