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

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

On 4/13/24 18:54, M Bazz wrote:
> This thought just came to me. `lda` is a privileged instruction. It has to
> run in supervisor mode. So, I'm struggling to understand how the
> kernel permission was wrong. Isn't that the right permission for this instruction?

The "current" permission, as computed by

> -    case ASI_KERNELTXT: /* Supervisor code access */
> -        oi = make_memop_idx(memop, cpu_mmu_index(env_cpu(env), true));

was correct for ASI_KERNELTXT, because as you say "lda" is a supervisor-only instruction 
prior to sparcv9.

However, using the same value for ASI_USERTXT would be incorrect.  For ASI_USERTXT and 
ASI_USERDATA (and the new names for sparcv9, ASI_AIU*, "as-if user"), we need permissions 
for the user context.

That's what

> +        case ASI_USERTXT:     /* User text access */
> +            mem_idx = MMU_USER_IDX;

this is for in my patch.  This gets passed into the helper,

> +    case GET_ASI_CODE:
> +#if !defined(CONFIG_USER_ONLY) && !defined(TARGET_SPARC64)
> +        {
> +            MemOpIdx oi = make_memop_idx(da->memop, da->mem_idx);
> +            TCGv_i64 t64 = tcg_temp_new_i64();
> +
> +            gen_helper_ld_code(t64, tcg_env, addr, tcg_constant_i32(oi));

and then into the core memory access functions,

> +        ret = cpu_ldl_code_mmu(env, addr, oi, ra);

Unfortunately, we do not have any good documentation for tcg softmmu or the intended role 
of the mmu_idx.  Partly that's due to the final use of the mmu_idx is target-specific.

For sparc32, there are 3 mmu_idx:

> #define MMU_USER_IDX   0
> #define MMU_KERNEL_IDX 1
> #define MMU_PHYS_IDX   2

The interpretation of mmu_idx happens in target/sparc/mmu_helper.c, get_physical_address 
(note there are two versions, for sparc32 and sparc64).

Ignoring MMU_PHYS_IDX, which is handled early, the difference between kernel and user is

     is_user = mmu_idx == MMU_USER_IDX;
...
     full->prot = perm_table[is_user][access_perms];

This controls the read/write/execute permissions for the page.  Note that perm_table 
matches the Access Allowed table on page 248 of the Sparc V8 architecture manual.


r~