Re: [PATCH v5 1/2] target/s390x: Implement the MVPG condition-code-option bit

[Richard Henderson <richard.henderson@linaro.org>]

On 3/11/21 10:17 AM, David Hildenbrand wrote:
> +#if !defined(CONFIG_USER_ONLY)
> +    /*
> +     * For !CONFIG_USER_ONLY, we cannot rely on TLB_INVALID_MASK or haddr==NULL
> +     * to detect if there was an exception during tlb_fill().
> +     */
> +    env->tlb_fill_exc = 0;
> +#endif
> +    flags = probe_access_flags(env, vaddr1, access_type, mmu_idx,
> +                               nofault, &haddr1, ra);
> +#if !defined(CONFIG_USER_ONLY)
> +    if (env->tlb_fill_exc) {
> +        return env->tlb_fill_exc;
> +    }
> +#else
> +    if (!haddr1) {
> +        env->__excp_addr = vaddr1;
> +        return PGM_ADDRESSING;
> +    }
> +#endif

The assumption of PGM_ADDRESSING is incorrect here -- it could still be 
PGM_PROTECTION, depending on how the page is mapped.

I guess this should be done like

#ifdef CONFIG_USER_ONLY
     flags = page_get_flags(vaddr1);
     if (!flags & (access_type == MMU_DATA_LOAD
                   ? PAGE_READ : PAGE_WRITE)) {
         env->__excp_addr = vaddr1;
         if (nofault) {
             return (flags & PAGE_VALID
                 ? PGM_PROTECTION : PGM_ADDRESSING);
         }
         raise exception.
     }
     haddr1 = g2h(vaddr1);
#else
     env->tlb_fill_exc = 0;
     flags = probe_access_flags(...);
     if (env->tlb_fill_exc) {
         return env->tlb_fill_exc;
     }
#endif

which is pretty ugly, but no worse than what you have above.


r~