Re: [PATCH] target/sparc: resolve ASI_USERTXT correctly

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

On 4/14/24 15:48, M Bazz wrote:
> I noticed that cpu_mmu_index() would have returned MMU_USER_IDX
> if the supervisor bit hadn't happened to be set (not sure if this
> execution path can occur for lda).

No, it cannot.

> Note that this check is gone in your patch.

Correct.  Since 'lda' has already checked that supervisor mode has been enabled, the 
translator may jump directly to the desired result of MMU_KERNEL_IDX.

> If I understand everything you've taught me, then the following patch would
> have also satisfied the permissions issue. Could you confirm this please?
> The essential change is the MMU_USER_IDX in the call to make_memop_idx()
> 
> diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
> index e581bb42ac..be3c03a3b6 100644
> --- a/target/sparc/ldst_helper.c
> +++ b/target/sparc/ldst_helper.c
> @@ -702,6 +702,24 @@ uint64_t helper_ld_asi(CPUSPARCState *env,
> target_ulong addr,
>               break;
>           }
>           break;
> +    case ASI_USERTXT: /* User code access */
> +        oi = make_memop_idx(memop, MMU_USER_IDX);
> +        switch (size) {
> +        case 1:
> +            ret = cpu_ldb_code_mmu(env, addr, oi, GETPC());
> +            break;
> +        case 2:
> +            ret = cpu_ldw_code_mmu(env, addr, oi, GETPC());
> +            break;
> +        default:
> +        case 4:
> +            ret = cpu_ldl_code_mmu(env, addr, oi, GETPC());
> +            break;
> +        case 8:
> +            ret = cpu_ldq_code_mmu(env, addr, oi, GETPC());
> +            break;
> +        }
> +        break;

Correct, that would also work.


r~