Hi Henry, I want to thank you for every chance I get to learn from you. Each email excites me.
On Sun, Apr 14, 2024 at 1:20 PM Richard Henderson <richard.hender...@linaro.org> wrote: > 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. > 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). Note that this check is gone in your patch. I consider you my sensei, so while I'm confident in your work I also want to show the things I catch. 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; case ASI_M_TXTC_TAG: /* SparcStation 5 I-cache tag */ case ASI_M_TXTC_DATA: /* SparcStation 5 I-cache data */ case ASI_M_DATAC_TAG: /* SparcStation 5 D-cache tag */ @@ -779,7 +797,6 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong addr, case 0x4c: /* SuperSPARC MMU Breakpoint Action */ ret = env->mmubpaction; break; - case ASI_USERTXT: /* User code access, XXX */ default: sparc_raise_mmu_fault(cs, addr, false, false, asi, size, GETPC()); ret = 0; > 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. I love learning from code, but the lack of documentation definitely increases the value of your discourse with me. Thank you, Sincerely, -bazz