On Fri, Jan 23, 2015 at 12:20 PM, Peter Maydell wrote: > Make all the callers of get_phys_addr() pass it the correct > mmu_idx rather than just a simple "is_user" flag. This includes > properly decoding the AT/ATS system instructions; we include the > logic for handling all the opc1/opc2 cases because we'll need > them later for supporting EL2/EL3, even if we don't have the > regdef stanzas yet. > > Signed-off-by: Peter Maydell > --- > target-arm/helper.c | 110 > +++++++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 96 insertions(+), 14 deletions(-) > > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 04bc0a1..0ae04eb 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -13,7 +13,7 @@ > > #ifndef CONFIG_USER_ONLY > static inline int get_phys_addr(CPUARMState *env, target_ulong address, > - int access_type, int is_user, > + int access_type, ARMMMUIdx mmu_idx, > hwaddr *phys_ptr, int *prot, > target_ulong *page_size); > > @@ -1436,7 +1436,7 @@ static CPAccessResult ats_access(CPUARMState *env, > const ARMCPRegInfo *ri) > } > > static uint64_t do_ats_write(CPUARMState *env, uint64_t value, > - int access_type, int is_user) > + int access_type, ARMMMUIdx mmu_idx) > { > hwaddr phys_addr; > target_ulong page_size; > @@ -1444,7 +1444,7 @@ static uint64_t do_ats_write(CPUARMState *env, > uint64_t value, > int ret; > uint64_t par64; > > - ret = get_phys_addr(env, value, access_type, is_user, > + ret = get_phys_addr(env, value, access_type, mmu_idx, > &phys_addr, &prot, &page_size); > if (extended_addresses_enabled(env)) { > /* ret is a DFSR/IFSR value for the long descriptor > @@ -1486,11 +1486,58 @@ static uint64_t do_ats_write(CPUARMState *env, > uint64_t value, > > static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t > value) > { > - int is_user = ri->opc2 & 2; > int access_type = ri->opc2 & 1; > uint64_t par64; > + ARMMMUIdx mmu_idx; > + int el = arm_current_el(env); > + bool secure = arm_is_secure_below_el3(env); > > - par64 = do_ats_write(env, value, access_type, is_user); > + switch (ri->opc2 & 6) { > + case 0: > + /* stage 1 current state PL1 */ > + switch (el) { > + case 3: > + mmu_idx = ARMMMUIdx_S1E3; > + break; > + case 2: > + mmu_idx = ARMMMUIdx_S1NSE1; > + break; > + case 1: > + mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_S1NSE1; > + break; > + default: > + g_assert_not_reached(); > + } > + break; > + case 2: > + /* stage 1 current state PL0 */ > + switch (el) { > + case 3: > + mmu_idx = ARMMMUIdx_S1SE0; > + break; > + case 2: > + mmu_idx = ARMMMUIdx_S1NSE0; > + break; > + case 1: > + mmu_idx = secure ? ARMMMUIdx_S1SE0 : ARMMMUIdx_S1NSE0; > + break; > + default: > + g_assert_not_reached(); > + } > + break; > + case 4: > + /* stage 1+2 NonSecure PL1 */ > + mmu_idx = ARMMMUIdx_S12NSE1; > + break; > + case 6: > + /* stage 1+2 NonSecure PL0 */ > + mmu_idx = ARMMMUIdx_S12NSE0; > + break; > + default: > + g_assert_not_reached(); > + } > + > + par64 = do_ats_write(env, value, access_type, mmu_idx); > > A32_BANKED_CURRENT_REG_SET(env, par, par64); > } > @@ -1498,10 +1545,40 @@ static void ats_write(CPUARMState *env, const > ARMCPRegInfo *ri, uint64_t value) > static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri, > uint64_t value) > { > - int is_user = ri->opc2 & 2; > int access_type = ri->opc2 & 1; > + ARMMMUIdx mmu_idx; > + int secure = arm_is_secure_below_el3(env); > + > + switch (ri->opc2 & 6) { > + case 0: > + switch (ri->opc1) { > + case 0: > + mmu_idx = secure ? ARMMMUIdx_S1SE1 : ARMMMUIdx_S1NSE1; > + break; > + case 4: > + mmu_idx = ARMMMUIdx_S1E2; > + break; > + case 6: > + mmu_idx = ARMMMUIdx_S1E3; > + break; > + default: > + g_assert_not_reached(); > + } > + break; > + case 2: > + mmu_idx = secure ? ARMMMUIdx_S1SE0 : ARMMMUIdx_S1NSE0; > + break; > + case 4: > + mmu_idx = ARMMMUIdx_S12NSE1; > + break; > + case 6: > + mmu_idx = ARMMMUIdx_S12NSE0; > + break; > + default: > + g_assert_not_reached(); > + } > ​The above cases would be more readable if each case had a comment identifying the corresponding AT instruction. Just faster for reference purposes. > > - env->cp15.par_el[1] = do_ats_write(env, value, access_type, is_user); > + env->cp15.par_el[1] = do_ats_write(env, value, access_type, mmu_idx); > } > #endif > > @@ -5084,13 +5161,13 @@ static int get_phys_addr_mpu(CPUARMState *env, > uint32_t address, > * @env: CPUARMState > * @address: virtual address to get physical address for > * @access_type: 0 for read, 1 for write, 2 for execute > - * @is_user: 0 for privileged access, 1 for user > + * @mmu_idx: MMU index indicating required translation regime > * @phys_ptr: set to the physical address corresponding to the virtual > address > * @prot: set to the permissions for the page containing phys_ptr > * @page_size: set to the size of the page containing phys_ptr > */ > static inline int get_phys_addr(CPUARMState *env, target_ulong address, > - int access_type, int is_user, > + int access_type, ARMMMUIdx mmu_idx, > hwaddr *phys_ptr, int *prot, > target_ulong *page_size) > { > @@ -5099,6 +5176,11 @@ static inline int get_phys_addr(CPUARMState *env, > target_ulong address, > */ > uint32_t sctlr = A32_BANKED_CURRENT_REG_GET(env, sctlr); > > + /* This will go away when we handle mmu_idx properly here */ > + int is_user = (mmu_idx == ARMMMUIdx_S12NSE0 || > + mmu_idx == ARMMMUIdx_S1SE0 || > + mmu_idx == ARMMMUIdx_S1NSE0); > + > /* Fast Context Switch Extension. */ > if (address < 0x02000000) { > address += A32_BANKED_CURRENT_REG_GET(env, fcseidr); > @@ -5134,13 +5216,11 @@ int arm_cpu_handle_mmu_fault(CPUState *cs, vaddr > address, > hwaddr phys_addr; > target_ulong page_size; > int prot; > - int ret, is_user; > + int ret; > uint32_t syn; > bool same_el = (arm_current_el(env) != 0); > > - /* TODO: pass the translation regime to get_phys_addr */ > - is_user = (arm_mmu_idx_to_el(mmu_idx) == 0); > - ret = get_phys_addr(env, address, access_type, is_user, &phys_addr, > &prot, > + ret = get_phys_addr(env, address, access_type, mmu_idx, &phys_addr, > &prot, > &page_size); > if (ret == 0) { > /* Map a single [sub]page. */ > @@ -5176,12 +5256,14 @@ int arm_cpu_handle_mmu_fault(CPUState *cs, vaddr > address, > hwaddr arm_cpu_get_phys_page_debug(CPUState *cs, vaddr addr) > { > ARMCPU *cpu = ARM_CPU(cs); > + CPUARMState *env = &cpu->env; > hwaddr phys_addr; > target_ulong page_size; > int prot; > int ret; > > - ret = get_phys_addr(&cpu->env, addr, 0, 0, &phys_addr, &prot, > &page_size); > + ret = get_phys_addr(env, addr, 0, cpu_mmu_index(env), &phys_addr, > + &prot, &page_size); > > if (ret != 0) { > return -1; > -- > 1.9.1 > > > ​Otherwise, Reviewed-by: Greg Bellows ​