Re: [PATCH 1/2] Revert "target/s390x: Use probe_access_flags in s390_probe_access"

[David Hildenbrand]

On 23.08.22 23:38, Richard Henderson wrote:
> This reverts commit db9aab5783a2fb62250e12f0c4cfed5e1778c189.
> 
> This patch breaks the contract of s390_probe_access, in that
> it no longer returns an exception code, nor set __excp_addr.
> 
> Reported-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/tcg/mem_helper.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
> index fc52aa128b..4c0f8baa39 100644
> --- a/target/s390x/tcg/mem_helper.c
> +++ b/target/s390x/tcg/mem_helper.c
> @@ -142,12 +142,20 @@ static int s390_probe_access(CPUArchState *env, 
> target_ulong addr, int size,
>                               MMUAccessType access_type, int mmu_idx,
>                               bool nonfault, void **phost, uintptr_t ra)
>  {
> -#if defined(CONFIG_USER_ONLY)
> -    return probe_access_flags(env, addr, access_type, mmu_idx,
> -                              nonfault, phost, ra);
> -#else
>      int flags;
>  
> +#if defined(CONFIG_USER_ONLY)
> +    flags = page_get_flags(addr);
> +    if (!(flags & (access_type == MMU_DATA_LOAD ?  PAGE_READ : 
> PAGE_WRITE_ORG))) {
> +        env->__excp_addr = addr;
> +        flags = (flags & PAGE_VALID) ? PGM_PROTECTION : PGM_ADDRESSING;
> +        if (nonfault) {
> +            return flags;
> +        }
> +        tcg_s390_program_interrupt(env, flags, ra);
> +    }
> +    *phost = g2h(env_cpu(env), addr);
> +#else
>      /*
>       * For !CONFIG_USER_ONLY, we cannot rely on TLB_INVALID_MASK or 
> haddr==NULL
>       * to detect if there was an exception during tlb_fill().
> @@ -166,8 +174,8 @@ static int s390_probe_access(CPUArchState *env, 
> target_ulong addr, int size,
>                               (access_type == MMU_DATA_STORE
>                                ? BP_MEM_WRITE : BP_MEM_READ), ra);
>      }
> -    return 0;
>  #endif
> +    return 0;
>  }
>  
>  static int access_prepare_nf(S390Access *access, CPUS390XState *env,

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb