| Message ID | 1422037228-5363-10-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Jan 23, 2015 at 12:20 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > Now we have the mmu_idx in get_phys_addr(), use it correctly to > determine the behaviour of virtual to physical address translations, > rather than using just an is_user flag and the current CPU state. > > Some TODO comments have been added to indicate where changes will > need to be made to add EL2 and 64-bit EL3 support. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target-arm/helper.c | 200 > +++++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 151 insertions(+), 49 deletions(-) > > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 0ae04eb..0a06bbe 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -4556,13 +4556,88 @@ void arm_cpu_do_interrupt(CPUState *cs) > cs->interrupt_request |= CPU_INTERRUPT_EXITTB; > } > > + > +/* Return the exception level which controls this address translation > regime */ > +static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx) > +{ > + switch (mmu_idx) { > + case ARMMMUIdx_S2NS: > + case ARMMMUIdx_S1E2: > + return 2; > + case ARMMMUIdx_S1E3: > + return 3; > + case ARMMMUIdx_S1SE0: > + return arm_el_is_aa64(env, 3) ? 1 : 3; > + case ARMMMUIdx_S1SE1: > I think this should be handled the same way as S1SE0 as Secure EL1 maps to EL3 when EL3 is AArch32. > + case ARMMMUIdx_S1NSE0: > + case ARMMMUIdx_S1NSE1: > + return 1; > + default: > + g_assert_not_reached(); > + } > +} > + > +/* Return the SCTLR value which controls this address translation regime > */ > +static inline uint32_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx) > +{ > + return env->cp15.sctlr_el[regime_el(env, mmu_idx)]; > Given the above regime_el(), for S1SE1, this would return the non-secure SCTLR bank on a secure translation. Same below for TCR and all uses thereafter. > +} > + > +/* Return true if the specified stage of address translation is disabled > */ > +static inline bool regime_translation_disabled(CPUARMState *env, > + ARMMMUIdx mmu_idx) > +{ > + if (mmu_idx == ARMMMUIdx_S2NS) { > + return (env->cp15.hcr_el2 & HCR_VM) == 0; > + } > + return (regime_sctlr(env, mmu_idx) & SCTLR_M) == 0; > +} > + > +/* Return the TCR controlling this translation regime */ > +static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx) > +{ > + if (mmu_idx == ARMMMUIdx_S2NS) { > + /* TODO: return VTCR_EL2 */ > + g_assert_not_reached(); > + } > + return &env->cp15.tcr_el[regime_el(env, mmu_idx)]; > +} > + > +/* Return true if the translation regime is using LPAE format page tables > */ > +static inline bool regime_using_lpae_format(CPUARMState *env, > + ARMMMUIdx mmu_idx) > +{ > + int el = regime_el(env, mmu_idx); > + if (el == 2 || arm_el_is_aa64(env, el)) { > + return true; > + } > + if (arm_feature(env, ARM_FEATURE_LPAE) > + && (regime_tcr(env, mmu_idx)->raw_tcr & TTBCR_EAE)) { > + return true; > + } > + return false; > +} > + > +static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx) > +{ > + switch (mmu_idx) { > + case ARMMMUIdx_S1SE0: > + case ARMMMUIdx_S1NSE0: > The get_phys_addr path filters out ARMMMUIdx_S12NSE0 so calls to this function in this context don't matter, but what if it is called outside this context? How should it handle this index? > + return true; > + default: > + return false; > + } > +} > + > /* Check section/page access permissions. > Returns the page protection flags, or zero if the access is not > permitted. */ > -static inline int check_ap(CPUARMState *env, int ap, int domain_prot, > - int access_type, int is_user) > +static inline int check_ap(CPUARMState *env, ARMMMUIdx mmu_idx, > + int ap, int domain_prot, > + int access_type) > { > int prot_ro; > + bool is_user = regime_is_user(env, mmu_idx); > > if (domain_prot == 3) { > return PAGE_READ | PAGE_WRITE; > @@ -4580,7 +4655,7 @@ static inline int check_ap(CPUARMState *env, int ap, > int domain_prot, > } > if (access_type == 1) > return 0; > - switch (A32_BANKED_CURRENT_REG_GET(env, sctlr) & (SCTLR_S | > SCTLR_R)) { > + switch (regime_sctlr(env, mmu_idx) & (SCTLR_S | SCTLR_R)) { > case SCTLR_S: > return is_user ? 0 : PAGE_READ; > case SCTLR_R: > @@ -4612,35 +4687,32 @@ static inline int check_ap(CPUARMState *env, int > ap, int domain_prot, > } > } > > -static bool get_level1_table_address(CPUARMState *env, uint32_t *table, > - uint32_t address) > +static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx, > + uint32_t *table, uint32_t address) > { > - /* Get the TCR bank based on our security state */ > - TCR *tcr = &env->cp15.tcr_el[arm_is_secure(env) ? 3 : 1]; > + /* Note that we can only get here for an AArch32 PL0/PL1 lookup */ > + int el = regime_el(env, mmu_idx); > + TCR *tcr = regime_tcr(env, mmu_idx); > > - /* We only get here if EL1 is running in AArch32. If EL3 is running in > - * AArch32 there is a secure and non-secure instance of the > translation > - * table registers. > - */ > if (address & tcr->mask) { > if (tcr->raw_tcr & TTBCR_PD1) { > /* Translation table walk disabled for TTBR1 */ > return false; > } > - *table = A32_BANKED_CURRENT_REG_GET(env, ttbr1) & 0xffffc000; > + *table = env->cp15.ttbr1_el[el] & 0xffffc000; > } else { > if (tcr->raw_tcr & TTBCR_PD0) { > /* Translation table walk disabled for TTBR0 */ > return false; > } > - *table = A32_BANKED_CURRENT_REG_GET(env, ttbr0) & tcr->base_mask; > + *table = env->cp15.ttbr0_el[el] & tcr->base_mask; > } > *table |= (address >> 18) & 0x3ffc; > return true; > } > > static int get_phys_addr_v5(CPUARMState *env, uint32_t address, int > access_type, > - int is_user, hwaddr *phys_ptr, > + ARMMMUIdx mmu_idx, hwaddr *phys_ptr, > int *prot, target_ulong *page_size) > { > CPUState *cs = CPU(arm_env_get_cpu(env)); > @@ -4652,10 +4724,11 @@ static int get_phys_addr_v5(CPUARMState *env, > uint32_t address, int access_type, > int domain = 0; > int domain_prot; > hwaddr phys_addr; > + uint32_t dacr; > > /* Pagetable walk. */ > /* Lookup l1 descriptor. */ > - if (!get_level1_table_address(env, &table, address)) { > + if (!get_level1_table_address(env, mmu_idx, &table, address)) { > /* Section translation fault if page walk is disabled by PD0 or > PD1 */ > code = 5; > goto do_fault; > @@ -4663,7 +4736,12 @@ static int get_phys_addr_v5(CPUARMState *env, > uint32_t address, int access_type, > desc = ldl_phys(cs->as, table); > type = (desc & 3); > domain = (desc >> 5) & 0x0f; > - domain_prot = (A32_BANKED_CURRENT_REG_GET(env, dacr) >> (domain * 2)) > & 3; > + if (regime_el(env, mmu_idx) == 1) { > + dacr = env->cp15.dacr_ns; > + } else { > + dacr = env->cp15.dacr_s; > + } > + domain_prot = (dacr >> (domain * 2)) & 3; > if (type == 0) { > /* Section translation fault. > */ > code = 5; > @@ -4727,7 +4805,7 @@ static int get_phys_addr_v5(CPUARMState *env, > uint32_t address, int access_type, > } > code = 15; > } > - *prot = check_ap(env, ap, domain_prot, access_type, is_user); > + *prot = check_ap(env, mmu_idx, ap, domain_prot, access_type); > if (!*prot) { > /* Access permission fault. */ > goto do_fault; > @@ -4740,7 +4818,7 @@ do_fault: > } > > static int get_phys_addr_v6(CPUARMState *env, uint32_t address, int > access_type, > - int is_user, hwaddr *phys_ptr, > + ARMMMUIdx mmu_idx, hwaddr *phys_ptr, > int *prot, target_ulong *page_size) > { > CPUState *cs = CPU(arm_env_get_cpu(env)); > @@ -4754,10 +4832,11 @@ static int get_phys_addr_v6(CPUARMState *env, > uint32_t address, int access_type, > int domain = 0; > int domain_prot; > hwaddr phys_addr; > + uint32_t dacr; > > /* Pagetable walk. */ > /* Lookup l1 descriptor. */ > - if (!get_level1_table_address(env, &table, address)) { > + if (!get_level1_table_address(env, mmu_idx, &table, address)) { > /* Section translation fault if page walk is disabled by PD0 or > PD1 */ > code = 5; > goto do_fault; > @@ -4775,7 +4854,12 @@ static int get_phys_addr_v6(CPUARMState *env, > uint32_t address, int access_type, > /* Page or Section. */ > domain = (desc >> 5) & 0x0f; > } > - domain_prot = (A32_BANKED_CURRENT_REG_GET(env, dacr) >> (domain * 2)) > & 3; > + if (regime_el(env, mmu_idx) == 1) { > + dacr = env->cp15.dacr_ns; > + } else { > + dacr = env->cp15.dacr_s; > + } > + domain_prot = (dacr >> (domain * 2)) & 3; > if (domain_prot == 0 || domain_prot == 2) { > if (type != 1) { > code = 9; /* Section domain fault. */ > @@ -4829,20 +4913,20 @@ static int get_phys_addr_v6(CPUARMState *env, > uint32_t address, int access_type, > if (domain_prot == 3) { > *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; > } else { > - if (pxn && !is_user) { > + if (pxn && !regime_is_user(env, mmu_idx)) { > xn = 1; > } > if (xn && access_type == 2) > goto do_fault; > > /* The simplified model uses AP[0] as an access control bit. */ > - if ((A32_BANKED_CURRENT_REG_GET(env, sctlr) & SCTLR_AFE) > + if ((regime_sctlr(env, mmu_idx) & SCTLR_AFE) > && (ap & 1) == 0) { > /* Access flag fault. */ > code = (code == 15) ? 6 : 3; > goto do_fault; > } > - *prot = check_ap(env, ap, domain_prot, access_type, is_user); > + *prot = check_ap(env, mmu_idx, ap, domain_prot, access_type); > if (!*prot) { > /* Access permission fault. */ > goto do_fault; > @@ -4867,7 +4951,7 @@ typedef enum { > } MMUFaultType; > > static int get_phys_addr_lpae(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_ptr) > { > @@ -4887,9 +4971,17 @@ static int get_phys_addr_lpae(CPUARMState *env, > target_ulong address, > int32_t granule_sz = 9; > int32_t va_size = 32; > int32_t tbi = 0; > - TCR *tcr = &env->cp15.tcr_el[arm_is_secure(env) ? 3 : 1]; > - > - if (arm_el_is_aa64(env, 1)) { > + bool is_user; > + TCR *tcr = regime_tcr(env, mmu_idx); > + > + /* TODO: > + * This code assumes we're either a 64-bit EL1 or a 32-bit PL1; > + * it doesn't handle the different format TCR for TCR_EL2, TCR_EL3, > + * and VTCR_EL2, or the fact that those regimes don't have a split > + * TTBR0/TTBR1. Attribute and permission bit handling should also > + * be checked when adding support for those page table walks. > + */ > + if (arm_el_is_aa64(env, regime_el(env, mmu_idx))) { > va_size = 64; > if (extract64(address, 55, 1)) > tbi = extract64(tcr->raw_tcr, 38, 1); > @@ -4904,12 +4996,12 @@ static int get_phys_addr_lpae(CPUARMState *env, > target_ulong address, > * TTBCR/TTBR0/TTBR1 in accordance with ARM ARM DDI0406C table B-32: > */ > uint32_t t0sz = extract32(tcr->raw_tcr, 0, 6); > - if (arm_el_is_aa64(env, 1)) { > + if (va_size == 64) { > t0sz = MIN(t0sz, 39); > t0sz = MAX(t0sz, 16); > } > uint32_t t1sz = extract32(tcr->raw_tcr, 16, 6); > - if (arm_el_is_aa64(env, 1)) { > + if (va_size == 64) { > t1sz = MIN(t1sz, 39); > t1sz = MAX(t1sz, 16); > } > @@ -4964,6 +5056,10 @@ static int get_phys_addr_lpae(CPUARMState *env, > target_ulong address, > } > } > > + /* Here we should have set up all the parameters for the translation: > + * va_size, ttbr, epd, tsz, granule_sz, tbi > + */ > + > if (epd) { > /* Translation table walk disabled => Translation fault on TLB > miss */ > goto do_fault; > @@ -5049,6 +5145,7 @@ static int get_phys_addr_lpae(CPUARMState *env, > target_ulong address, > goto do_fault; > } > fault_type = permission_fault; > + is_user = regime_is_user(env, mmu_idx); > if (is_user && !(attrs & (1 << 4))) { > /* Unprivileged access not enabled */ > goto do_fault; > @@ -5083,12 +5180,13 @@ do_fault: > } > > static int get_phys_addr_mpu(CPUARMState *env, uint32_t address, > - int access_type, int is_user, > + int access_type, ARMMMUIdx mmu_idx, > hwaddr *phys_ptr, int *prot) > { > int n; > uint32_t mask; > uint32_t base; > + bool is_user = regime_is_user(env, mmu_idx); > > *phys_ptr = address; > for (n = 7; n >= 0; n--) { > @@ -5171,39 +5269,43 @@ static inline int get_phys_addr(CPUARMState *env, > target_ulong address, > hwaddr *phys_ptr, int *prot, > target_ulong *page_size) > { > - /* This is not entirely correct as get_phys_addr() can also be called > - * from ats_write() for an address translation of a specific regime. > - */ > - 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); > + if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) { > + /* TODO: when we support EL2 we should here call ourselves > recursively > + * to do the stage 1 and then stage 2 translations. The ldl_phys > + * calls for stage 1 will also need changing. > + * For non-EL2 CPUs a stage1+stage2 translation is just stage 1. > + */ > + assert(!arm_feature(env, ARM_FEATURE_EL2)); > + mmu_idx += ARMMMUIdx_S1NSE0; > + } > > /* Fast Context Switch Extension. */ > - if (address < 0x02000000) { > + if (address < 0x02000000 && mmu_idx != ARMMMUIdx_S2NS) { > address += A32_BANKED_CURRENT_REG_GET(env, fcseidr); > } > > - if ((sctlr & SCTLR_M) == 0) { > + if (regime_translation_disabled(env, mmu_idx)) { > /* MMU/MPU disabled. */ > *phys_ptr = address; > *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; > *page_size = TARGET_PAGE_SIZE; > return 0; > - } else if (arm_feature(env, ARM_FEATURE_MPU)) { > + } > + > + if (arm_feature(env, ARM_FEATURE_MPU)) { > *page_size = TARGET_PAGE_SIZE; > - return get_phys_addr_mpu(env, address, access_type, is_user, > phys_ptr, > - prot); > - } else if (extended_addresses_enabled(env)) { > - return get_phys_addr_lpae(env, address, access_type, is_user, > phys_ptr, > + return get_phys_addr_mpu(env, address, access_type, mmu_idx, > phys_ptr, > + prot); > + } > + > + if (regime_using_lpae_format(env, mmu_idx)) { > + return get_phys_addr_lpae(env, address, access_type, mmu_idx, > phys_ptr, > prot, page_size); > - } else if (sctlr & SCTLR_XP) { > - return get_phys_addr_v6(env, address, access_type, is_user, > phys_ptr, > + } else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) { > + return get_phys_addr_v6(env, address, access_type, mmu_idx, > phys_ptr, > prot, page_size); > } else { > - return get_phys_addr_v5(env, address, access_type, is_user, > phys_ptr, > + return get_phys_addr_v5(env, address, access_type, mmu_idx, > phys_ptr, > prot, page_size); > } > } > -- > 1.9.1 > >
On 27 January 2015 at 17:57, Greg Bellows <greg.bellows@linaro.org> wrote: > > > On Fri, Jan 23, 2015 at 12:20 PM, Peter Maydell <peter.maydell@linaro.org> > wrote: >> +/* Return the exception level which controls this address translation >> regime */ >> +static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx) >> +{ >> + switch (mmu_idx) { >> + case ARMMMUIdx_S2NS: >> + case ARMMMUIdx_S1E2: >> + return 2; >> + case ARMMMUIdx_S1E3: >> + return 3; >> + case ARMMMUIdx_S1SE0: >> + return arm_el_is_aa64(env, 3) ? 1 : 3; >> + case ARMMMUIdx_S1SE1: > > > I think this should be handled the same way as S1SE0 as Secure EL1 maps to > EL3 when EL3 is AArch32. If EL3 is AArch32 then you'll never be using this MMU index. By definition the S1SE1 index is for code executing at Secure EL1, and there isn't any of that unless EL3 is 64 bit. (Secure EL1 doesn't "map to" anything, it just doesn't exist/have any code running in it.) >> + case ARMMMUIdx_S1NSE0: >> + case ARMMMUIdx_S1NSE1: >> + return 1; >> + default: >> + g_assert_not_reached(); >> + } >> +} >> + >> +/* Return the SCTLR value which controls this address translation regime >> */ >> +static inline uint32_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx) >> +{ >> + return env->cp15.sctlr_el[regime_el(env, mmu_idx)]; > > > Given the above regime_el(), for S1SE1, this would return the non-secure > SCTLR bank on a secure translation. Same below for TCR and all uses > thereafter. That's correct, because S1SE1 implies "secure EL1 under a 64 bit EL3", in which case there is no system register banking and both Secure and NonSecure use the same underlying register. Compare the way that A32_BANKED_CURRENT_REG_GET/SET always use the NS version if arm_el_is_aa64(env, 3). >> +static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx) >> +{ >> + switch (mmu_idx) { >> + case ARMMMUIdx_S1SE0: >> + case ARMMMUIdx_S1NSE0: > > > The get_phys_addr path filters out ARMMMUIdx_S12NSE0 so calls to this > function in this context don't matter, but what if it is called outside this > context? How should it handle this index? g_assert_not_reached(), but it didn't seem worth cluttering the switch with a bunch of extra labels just to assert that they weren't reachable. thanks -- PMM
On Tue, Jan 27, 2015 at 12:12 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 27 January 2015 at 17:57, Greg Bellows <greg.bellows@linaro.org> wrote: > > > > > > On Fri, Jan 23, 2015 at 12:20 PM, Peter Maydell < > peter.maydell@linaro.org> > > wrote: > >> +/* Return the exception level which controls this address translation > >> regime */ > >> +static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx) > >> +{ > >> + switch (mmu_idx) { > >> + case ARMMMUIdx_S2NS: > >> + case ARMMMUIdx_S1E2: > >> + return 2; > >> + case ARMMMUIdx_S1E3: > >> + return 3; > >> + case ARMMMUIdx_S1SE0: > >> + return arm_el_is_aa64(env, 3) ? 1 : 3; > >> + case ARMMMUIdx_S1SE1: > > > > > > I think this should be handled the same way as S1SE0 as Secure EL1 maps > to > > EL3 when EL3 is AArch32. > > If EL3 is AArch32 then you'll never be using this MMU index. > By definition the S1SE1 index is for code executing at > Secure EL1, and there isn't any of that unless EL3 is 64 bit. > (Secure EL1 doesn't "map to" anything, it just doesn't > exist/have any code running in it.) > Ah yes and thanks for the pointer to the origin of mmu index I have now connected where we prevent that index with this code. > > >> + case ARMMMUIdx_S1NSE0: > >> + case ARMMMUIdx_S1NSE1: > >> + return 1; > >> + default: > >> + g_assert_not_reached(); > >> + } > >> +} > >> + > >> +/* Return the SCTLR value which controls this address translation > regime > >> */ > >> +static inline uint32_t regime_sctlr(CPUARMState *env, ARMMMUIdx > mmu_idx) > >> +{ > >> + return env->cp15.sctlr_el[regime_el(env, mmu_idx)]; > > > > > > Given the above regime_el(), for S1SE1, this would return the non-secure > > SCTLR bank on a secure translation. Same below for TCR and all uses > > thereafter. > > That's correct, because S1SE1 implies "secure EL1 under a 64 bit > EL3", in which case there is no system register banking and > both Secure and NonSecure use the same underlying register. > Compare the way that A32_BANKED_CURRENT_REG_GET/SET always > use the NS version if arm_el_is_aa64(env, 3). > > >> +static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx) > >> +{ > >> + switch (mmu_idx) { > >> + case ARMMMUIdx_S1SE0: > >> + case ARMMMUIdx_S1NSE0: > > > > > > The get_phys_addr path filters out ARMMMUIdx_S12NSE0 so calls to this > > function in this context don't matter, but what if it is called outside > this > > context? How should it handle this index? > > g_assert_not_reached(), but it didn't seem worth cluttering > the switch with a bunch of extra labels just to assert that > they weren't reachable. > > I see how it could clutter things, but given that the routine is generic we probably should just like we do in regime_el(). > thanks > -- PMM >
On 27 January 2015 at 19:49, Greg Bellows <greg.bellows@linaro.org> wrote: > On Tue, Jan 27, 2015 at 12:12 PM, Peter Maydell <peter.maydell@linaro.org> > wrote: >> g_assert_not_reached(), but it didn't seem worth cluttering >> the switch with a bunch of extra labels just to assert that >> they weren't reachable. >> > > I see how it could clutter things, but given that the routine is generic we > probably should just like we do in regime_el(). Not a big deal, so I'll add them, but the routine isn't generic -- it's purely a local utility routine for the benefit of the get_phys_addr family of functions and not intended to be called from elsewhere. -- PMM
On Fri, Jan 23, 2015 at 12:20 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > Now we have the mmu_idx in get_phys_addr(), use it correctly to > determine the behaviour of virtual to physical address translations, > rather than using just an is_user flag and the current CPU state. > > Some TODO comments have been added to indicate where changes will > need to be made to add EL2 and 64-bit EL3 support. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target-arm/helper.c | 200 > +++++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 151 insertions(+), 49 deletions(-) > > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 0ae04eb..0a06bbe 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -4556,13 +4556,88 @@ void arm_cpu_do_interrupt(CPUState *cs) > cs->interrupt_request |= CPU_INTERRUPT_EXITTB; > } > > + > +/* Return the exception level which controls this address translation > regime */ > +static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx) > +{ > + switch (mmu_idx) { > + case ARMMMUIdx_S2NS: > + case ARMMMUIdx_S1E2: > + return 2; > + case ARMMMUIdx_S1E3: > + return 3; > + case ARMMMUIdx_S1SE0: > + return arm_el_is_aa64(env, 3) ? 1 : 3; > + case ARMMMUIdx_S1SE1: > + case ARMMMUIdx_S1NSE0: > + case ARMMMUIdx_S1NSE1: > + return 1; > + default: > + g_assert_not_reached(); > + } > +} > + > +/* Return the SCTLR value which controls this address translation regime > */ > +static inline uint32_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx) > +{ > + return env->cp15.sctlr_el[regime_el(env, mmu_idx)]; > +} > + > +/* Return true if the specified stage of address translation is disabled > */ > +static inline bool regime_translation_disabled(CPUARMState *env, > + ARMMMUIdx mmu_idx) > +{ > + if (mmu_idx == ARMMMUIdx_S2NS) { > + return (env->cp15.hcr_el2 & HCR_VM) == 0; > + } > + return (regime_sctlr(env, mmu_idx) & SCTLR_M) == 0; > +} > + > +/* Return the TCR controlling this translation regime */ > +static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx) > +{ > + if (mmu_idx == ARMMMUIdx_S2NS) { > + /* TODO: return VTCR_EL2 */ > + g_assert_not_reached(); > + } > + return &env->cp15.tcr_el[regime_el(env, mmu_idx)]; > +} > + > +/* Return true if the translation regime is using LPAE format page tables > */ > +static inline bool regime_using_lpae_format(CPUARMState *env, > + ARMMMUIdx mmu_idx) > +{ > + int el = regime_el(env, mmu_idx); > + if (el == 2 || arm_el_is_aa64(env, el)) { > For the life of me, I can not figure out why EL2 is wired to always use LPAE. Is this stated in the spec somewhere? I found places where EL2 registers can vary depending on TTBCR.EAE bit settings which implies it is not always true. I realize EL2 is not supported yet, but wouldn't this break AArch32 if it were and the LPAE feature was not enabled? > + return true; > + } > + if (arm_feature(env, ARM_FEATURE_LPAE) > + && (regime_tcr(env, mmu_idx)->raw_tcr & TTBCR_EAE)) { > + return true; > + } > + return false; > +} > + > +static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx) > +{ > + switch (mmu_idx) { > + case ARMMMUIdx_S1SE0: > + case ARMMMUIdx_S1NSE0: > + return true; > + default: > + return false; > + } > +} > + > /* Check section/page access permissions. > Returns the page protection flags, or zero if the access is not > permitted. */ > -static inline int check_ap(CPUARMState *env, int ap, int domain_prot, > - int access_type, int is_user) > +static inline int check_ap(CPUARMState *env, ARMMMUIdx mmu_idx, > + int ap, int domain_prot, > + int access_type) > { > int prot_ro; > + bool is_user = regime_is_user(env, mmu_idx); > > if (domain_prot == 3) { > return PAGE_READ | PAGE_WRITE; > @@ -4580,7 +4655,7 @@ static inline int check_ap(CPUARMState *env, int ap, > int domain_prot, > } > if (access_type == 1) > return 0; > - switch (A32_BANKED_CURRENT_REG_GET(env, sctlr) & (SCTLR_S | > SCTLR_R)) { > + switch (regime_sctlr(env, mmu_idx) & (SCTLR_S | SCTLR_R)) { > case SCTLR_S: > return is_user ? 0 : PAGE_READ; > case SCTLR_R: > @@ -4612,35 +4687,32 @@ static inline int check_ap(CPUARMState *env, int > ap, int domain_prot, > } > } > > -static bool get_level1_table_address(CPUARMState *env, uint32_t *table, > - uint32_t address) > +static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx, > + uint32_t *table, uint32_t address) > { > - /* Get the TCR bank based on our security state */ > - TCR *tcr = &env->cp15.tcr_el[arm_is_secure(env) ? 3 : 1]; > + /* Note that we can only get here for an AArch32 PL0/PL1 lookup */ > + int el = regime_el(env, mmu_idx); > + TCR *tcr = regime_tcr(env, mmu_idx); > > - /* We only get here if EL1 is running in AArch32. If EL3 is running in > - * AArch32 there is a secure and non-secure instance of the > translation > - * table registers. > - */ > if (address & tcr->mask) { > if (tcr->raw_tcr & TTBCR_PD1) { > /* Translation table walk disabled for TTBR1 */ > return false; > } > - *table = A32_BANKED_CURRENT_REG_GET(env, ttbr1) & 0xffffc000; > + *table = env->cp15.ttbr1_el[el] & 0xffffc000; > Perhaps you plan to address this in a separate patch, but I believe TTBR1 is only applicable to EL1 and EL0 in AArch64. > } else { > if (tcr->raw_tcr & TTBCR_PD0) { > /* Translation table walk disabled for TTBR0 */ > return false; > } > - *table = A32_BANKED_CURRENT_REG_GET(env, ttbr0) & tcr->base_mask; > + *table = env->cp15.ttbr0_el[el] & tcr->base_mask; > } > *table |= (address >> 18) & 0x3ffc; > return true; > } > > static int get_phys_addr_v5(CPUARMState *env, uint32_t address, int > access_type, > - int is_user, hwaddr *phys_ptr, > + ARMMMUIdx mmu_idx, hwaddr *phys_ptr, > int *prot, target_ulong *page_size) > { > CPUState *cs = CPU(arm_env_get_cpu(env)); > @@ -4652,10 +4724,11 @@ static int get_phys_addr_v5(CPUARMState *env, > uint32_t address, int access_type, > int domain = 0; > int domain_prot; > hwaddr phys_addr; > + uint32_t dacr; > > /* Pagetable walk. */ > /* Lookup l1 descriptor. */ > - if (!get_level1_table_address(env, &table, address)) { > + if (!get_level1_table_address(env, mmu_idx, &table, address)) { > /* Section translation fault if page walk is disabled by PD0 or > PD1 */ > code = 5; > goto do_fault; > @@ -4663,7 +4736,12 @@ static int get_phys_addr_v5(CPUARMState *env, > uint32_t address, int access_type, > desc = ldl_phys(cs->as, table); > type = (desc & 3); > domain = (desc >> 5) & 0x0f; > - domain_prot = (A32_BANKED_CURRENT_REG_GET(env, dacr) >> (domain * 2)) > & 3; > + if (regime_el(env, mmu_idx) == 1) { > + dacr = env->cp15.dacr_ns; > + } else { > + dacr = env->cp15.dacr_s; > + } > + domain_prot = (dacr >> (domain * 2)) & 3; > Is there a reason that you did not add a regime_dacr() here like you did for SCTLR and TCR? Also, the ARMv8 ARM makes it sound like DACR has no value in AArch64. However, if it did have meaning in AArch64, then for S1SE1 would we be accessing the wrong bank as regime_el returns 1? This working off the understanding that an address reference from an instruction executed in S/EL1 and AArch64 would generate such an index. Maybe we need a regime_is_secure()? > if (type == 0) { > /* Section translation fault. */ > code = 5; > @@ -4727,7 +4805,7 @@ static int get_phys_addr_v5(CPUARMState *env, > uint32_t address, int access_type, > } > code = 15; > } > - *prot = check_ap(env, ap, domain_prot, access_type, is_user); > + *prot = check_ap(env, mmu_idx, ap, domain_prot, access_type); > if (!*prot) { > /* Access permission fault. */ > goto do_fault; > @@ -4740,7 +4818,7 @@ do_fault: > } > > static int get_phys_addr_v6(CPUARMState *env, uint32_t address, int > access_type, > - int is_user, hwaddr *phys_ptr, > + ARMMMUIdx mmu_idx, hwaddr *phys_ptr, > int *prot, target_ulong *page_size) > { > CPUState *cs = CPU(arm_env_get_cpu(env)); > @@ -4754,10 +4832,11 @@ static int get_phys_addr_v6(CPUARMState *env, > uint32_t address, int access_type, > int domain = 0; > int domain_prot; > hwaddr phys_addr; > + uint32_t dacr; > > /* Pagetable walk. */ > /* Lookup l1 descriptor. */ > - if (!get_level1_table_address(env, &table, address)) { > + if (!get_level1_table_address(env, mmu_idx, &table, address)) { > /* Section translation fault if page walk is disabled by PD0 or > PD1 */ > code = 5; > goto do_fault; > @@ -4775,7 +4854,12 @@ static int get_phys_addr_v6(CPUARMState *env, > uint32_t address, int access_type, > /* Page or Section. */ > domain = (desc >> 5) & 0x0f; > } > - domain_prot = (A32_BANKED_CURRENT_REG_GET(env, dacr) >> (domain * 2)) > & 3; > + if (regime_el(env, mmu_idx) == 1) { > + dacr = env->cp15.dacr_ns; > + } else { > + dacr = env->cp15.dacr_s; > + } > + domain_prot = (dacr >> (domain * 2)) & 3; > if (domain_prot == 0 || domain_prot == 2) { > if (type != 1) { > code = 9; /* Section domain fault. */ > @@ -4829,20 +4913,20 @@ static int get_phys_addr_v6(CPUARMState *env, > uint32_t address, int access_type, > if (domain_prot == 3) { > *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; > } else { > - if (pxn && !is_user) { > + if (pxn && !regime_is_user(env, mmu_idx)) { > xn = 1; > } > if (xn && access_type == 2) > goto do_fault; > > /* The simplified model uses AP[0] as an access control bit. */ > - if ((A32_BANKED_CURRENT_REG_GET(env, sctlr) & SCTLR_AFE) > + if ((regime_sctlr(env, mmu_idx) & SCTLR_AFE) > && (ap & 1) == 0) { > /* Access flag fault. */ > code = (code == 15) ? 6 : 3; > goto do_fault; > } > - *prot = check_ap(env, ap, domain_prot, access_type, is_user); > + *prot = check_ap(env, mmu_idx, ap, domain_prot, access_type); > if (!*prot) { > /* Access permission fault. */ > goto do_fault; > @@ -4867,7 +4951,7 @@ typedef enum { > } MMUFaultType; > > static int get_phys_addr_lpae(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_ptr) > { > @@ -4887,9 +4971,17 @@ static int get_phys_addr_lpae(CPUARMState *env, > target_ulong address, > int32_t granule_sz = 9; > int32_t va_size = 32; > int32_t tbi = 0; > - TCR *tcr = &env->cp15.tcr_el[arm_is_secure(env) ? 3 : 1]; > - > - if (arm_el_is_aa64(env, 1)) { > + bool is_user; > + TCR *tcr = regime_tcr(env, mmu_idx); > + > + /* TODO: > + * This code assumes we're either a 64-bit EL1 or a 32-bit PL1; > + * it doesn't handle the different format TCR for TCR_EL2, TCR_EL3, > + * and VTCR_EL2, or the fact that those regimes don't have a split > + * TTBR0/TTBR1. Attribute and permission bit handling should also > + * be checked when adding support for those page table walks. > + */ > Maybe copy this comment up above in get_level1_table_address(). > + if (arm_el_is_aa64(env, regime_el(env, mmu_idx))) { > va_size = 64; > if (extract64(address, 55, 1)) > tbi = extract64(tcr->raw_tcr, 38, 1); > @@ -4904,12 +4996,12 @@ static int get_phys_addr_lpae(CPUARMState *env, > target_ulong address, > * TTBCR/TTBR0/TTBR1 in accordance with ARM ARM DDI0406C table B-32: > */ > uint32_t t0sz = extract32(tcr->raw_tcr, 0, 6); > - if (arm_el_is_aa64(env, 1)) { > + if (va_size == 64) { > t0sz = MIN(t0sz, 39); > t0sz = MAX(t0sz, 16); > } > uint32_t t1sz = extract32(tcr->raw_tcr, 16, 6); > - if (arm_el_is_aa64(env, 1)) { > + if (va_size == 64) { > t1sz = MIN(t1sz, 39); > t1sz = MAX(t1sz, 16); > } > @@ -4964,6 +5056,10 @@ static int get_phys_addr_lpae(CPUARMState *env, > target_ulong address, > } > } > > + /* Here we should have set up all the parameters for the translation: > + * va_size, ttbr, epd, tsz, granule_sz, tbi > + */ > + > if (epd) { > /* Translation table walk disabled => Translation fault on TLB > miss */ > goto do_fault; > @@ -5049,6 +5145,7 @@ static int get_phys_addr_lpae(CPUARMState *env, > target_ulong address, > goto do_fault; > } > fault_type = permission_fault; > + is_user = regime_is_user(env, mmu_idx); > if (is_user && !(attrs & (1 << 4))) { > /* Unprivileged access not enabled */ > goto do_fault; > @@ -5083,12 +5180,13 @@ do_fault: > } > > static int get_phys_addr_mpu(CPUARMState *env, uint32_t address, > - int access_type, int is_user, > + int access_type, ARMMMUIdx mmu_idx, > hwaddr *phys_ptr, int *prot) > { > int n; > uint32_t mask; > uint32_t base; > + bool is_user = regime_is_user(env, mmu_idx); > > *phys_ptr = address; > for (n = 7; n >= 0; n--) { > @@ -5171,39 +5269,43 @@ static inline int get_phys_addr(CPUARMState *env, > target_ulong address, > hwaddr *phys_ptr, int *prot, > target_ulong *page_size) > { > - /* This is not entirely correct as get_phys_addr() can also be called > - * from ats_write() for an address translation of a specific regime. > - */ > - 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); > + if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) { > + /* TODO: when we support EL2 we should here call ourselves > recursively > + * to do the stage 1 and then stage 2 translations. The ldl_phys > + * calls for stage 1 will also need changing. > + * For non-EL2 CPUs a stage1+stage2 translation is just stage 1. > + */ > + assert(!arm_feature(env, ARM_FEATURE_EL2)); > + mmu_idx += ARMMMUIdx_S1NSE0; > + } > > /* Fast Context Switch Extension. */ > - if (address < 0x02000000) { > + if (address < 0x02000000 && mmu_idx != ARMMMUIdx_S2NS) { > address += A32_BANKED_CURRENT_REG_GET(env, fcseidr); > Now that the MMU index includes security state info we use in in certain circumstances to determine the security state. However, we don't seem to consistently use it. For example, earlier changes used the mmu_index to choose certain register banks but here we still rely on the BANKED macro. I see this inconsistency being prone to errors. Maybe we have just not gotten to change all of the cases over, but I thought I'd highlight it. > } > > - if ((sctlr & SCTLR_M) == 0) { > + if (regime_translation_disabled(env, mmu_idx)) { > /* MMU/MPU disabled. */ > *phys_ptr = address; > *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; > *page_size = TARGET_PAGE_SIZE; > return 0; > - } else if (arm_feature(env, ARM_FEATURE_MPU)) { > + } > + > + if (arm_feature(env, ARM_FEATURE_MPU)) { > *page_size = TARGET_PAGE_SIZE; > - return get_phys_addr_mpu(env, address, access_type, is_user, > phys_ptr, > - prot); > - } else if (extended_addresses_enabled(env)) { > - return get_phys_addr_lpae(env, address, access_type, is_user, > phys_ptr, > + return get_phys_addr_mpu(env, address, access_type, mmu_idx, > phys_ptr, > + prot); > + } > + > + if (regime_using_lpae_format(env, mmu_idx)) { > + return get_phys_addr_lpae(env, address, access_type, mmu_idx, > phys_ptr, > prot, page_size); > - } else if (sctlr & SCTLR_XP) { > - return get_phys_addr_v6(env, address, access_type, is_user, > phys_ptr, > + } else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) { > + return get_phys_addr_v6(env, address, access_type, mmu_idx, > phys_ptr, > prot, page_size); > } else { > - return get_phys_addr_v5(env, address, access_type, is_user, > phys_ptr, > + return get_phys_addr_v5(env, address, access_type, mmu_idx, > phys_ptr, > prot, page_size); > } > } > -- > 1.9.1 > >
On 28 January 2015 at 21:37, Greg Bellows <greg.bellows@linaro.org> wrote: > >> +/* Return true if the translation regime is using LPAE format page tables >> */ >> +static inline bool regime_using_lpae_format(CPUARMState *env, >> + ARMMMUIdx mmu_idx) >> +{ >> + int el = regime_el(env, mmu_idx); >> + if (el == 2 || arm_el_is_aa64(env, el)) { > > > For the life of me, I can not figure out why EL2 is wired to always use > LPAE. Is this stated in the spec somewhere? I found places where EL2 > registers can vary depending on TTBCR.EAE bit settings which implies it is > not always true. The only translation regimes controlled by EL2 are: (1) EL2's own stage 1 translations (2) the stage 2 translations These must both be LPAE format: see the v8 ARM ARM section G4.4: "the translation tables for the Non-secure PL2 stage 1 translations, and for the Non-secure PL1&0 stage 2 translations, must use the Long-descriptor translation table format." v7 ARM ARM B3.3 has similar text. (Basically, short-descriptors are obsolete and are only supported in the pre-Virtualization translation regimes, ie AArch32 EL1/3.) > I realize EL2 is not supported yet, but wouldn't this break AArch32 if it > were and the LPAE feature was not enabled? Implementations with Virtualization must include LPAE. >> -static bool get_level1_table_address(CPUARMState *env, uint32_t *table, >> - uint32_t address) >> +static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx, >> + uint32_t *table, uint32_t address) >> { >> - /* Get the TCR bank based on our security state */ >> - TCR *tcr = &env->cp15.tcr_el[arm_is_secure(env) ? 3 : 1]; >> + /* Note that we can only get here for an AArch32 PL0/PL1 lookup */ >> + int el = regime_el(env, mmu_idx); >> + TCR *tcr = regime_tcr(env, mmu_idx); >> >> - /* We only get here if EL1 is running in AArch32. If EL3 is running >> in >> - * AArch32 there is a secure and non-secure instance of the >> translation >> - * table registers. >> - */ >> if (address & tcr->mask) { >> if (tcr->raw_tcr & TTBCR_PD1) { >> /* Translation table walk disabled for TTBR1 */ >> return false; >> } >> - *table = A32_BANKED_CURRENT_REG_GET(env, ttbr1) & 0xffffc000; >> + *table = env->cp15.ttbr1_el[el] & 0xffffc000; > > > Perhaps you plan to address this in a separate patch, but I believe TTBR1 is > only applicable to EL1 and EL0 in AArch64. It's true that TTBR1 is only for EL0/EL1, but see the comment at the start of the function -- we can't get here except for EL0 and EL1, because this function is only used for some kinds of short-descriptor tables. >> @@ -4663,7 +4736,12 @@ static int get_phys_addr_v5(CPUARMState *env, >> uint32_t address, int access_type, >> desc = ldl_phys(cs->as, table); >> type = (desc & 3); >> domain = (desc >> 5) & 0x0f; >> - domain_prot = (A32_BANKED_CURRENT_REG_GET(env, dacr) >> (domain * 2)) >> & 3; >> + if (regime_el(env, mmu_idx) == 1) { >> + dacr = env->cp15.dacr_ns; >> + } else { >> + dacr = env->cp15.dacr_s; >> + } >> + domain_prot = (dacr >> (domain * 2)) & 3; > > > Is there a reason that you did not add a regime_dacr() here like you did for > SCTLR and TCR? Didn't seem necessary, since we know we only need to deal with S vs NS, and the concept isn't generally applicable to most regimes. If the dacr in env->cp15 was stored as dacr[4] I'd have used dacr[el] as I do above for the ttbr1_el[], but it isn't, hence the conditional. The TCR and SCTLR are used in LPAE format page tables so they apply for the whole set of translation regimes. > Also, the ARMv8 ARM makes it sound like DACR has no value in AArch64. Well, the DACR is only relevant to short-descriptor format page tables, so it's only consulted for AArch32 translations, and there's no equivalent register in AArch64. (There is a DACR32_EL2 64 bit register, but that is only there so a hypervisor can save and restore the state of a 32 bit VM (at EL1) that is using short-descriptor page tables.) > However, if it did have meaning in AArch64, then for S1SE1 would we be > accessing the wrong bank as regime_el returns 1? This working off the > understanding that an address reference from an instruction executed in > S/EL1 and AArch64 would generate such an index. We can only get here for regime S1SE1 if: * EL3 is AArch64 * EL1 is AArch32 Since EL3 is 64 bit, there is no banking of registers and regardless of whether EL1 is Secure or NonSecure we want the one and only register (which is in dacr_ns). (Compare the way we use ttbr1_el[regime_el(env, mmu_idx)] and so get TTBR1_EL1 whether this is Secure EL1 or NonSecure EL1.) If EL3 is 32 bit then there is banking of registers, but it's not possible to get here for S1SE1 in that case (only for S EL3 and NS EL1). >> + /* TODO: >> + * This code assumes we're either a 64-bit EL1 or a 32-bit PL1; >> + * it doesn't handle the different format TCR for TCR_EL2, TCR_EL3, >> + * and VTCR_EL2, or the fact that those regimes don't have a split >> + * TTBR0/TTBR1. Attribute and permission bit handling should also >> + * be checked when adding support for those page table walks. >> + */ > > > Maybe copy this comment up above in get_level1_table_address(). This is the correct location; see remarks above. >> @@ -5171,39 +5269,43 @@ static inline int get_phys_addr(CPUARMState *env, >> target_ulong address, >> hwaddr *phys_ptr, int *prot, >> target_ulong *page_size) >> { >> - /* This is not entirely correct as get_phys_addr() can also be called >> - * from ats_write() for an address translation of a specific regime. >> - */ >> - 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); >> + if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) { >> + /* TODO: when we support EL2 we should here call ourselves >> recursively >> + * to do the stage 1 and then stage 2 translations. The ldl_phys >> + * calls for stage 1 will also need changing. >> + * For non-EL2 CPUs a stage1+stage2 translation is just stage 1. >> + */ >> + assert(!arm_feature(env, ARM_FEATURE_EL2)); >> + mmu_idx += ARMMMUIdx_S1NSE0; >> + } >> >> /* Fast Context Switch Extension. */ >> - if (address < 0x02000000) { >> + if (address < 0x02000000 && mmu_idx != ARMMMUIdx_S2NS) { >> address += A32_BANKED_CURRENT_REG_GET(env, fcseidr); > > > Now that the MMU index includes security state info we use in in certain > circumstances to determine the security state. However, we don't seem to > consistently use it. For example, earlier changes used the mmu_index to > choose certain register banks but here we still rely on the BANKED macro. I > see this inconsistency being prone to errors. Maybe we have just not gotten > to change all of the cases over, but I thought I'd highlight it. Yes, you're right: if the Secure world does an NS ATS operation then we should be using the NS copy of the register. I think I mentally skipped over this requirement because the whole bit of code is irrelevant for v8 CPUs (where FSCEIDR is mandatorily RAZ/WI and so it doesn't matter which one we use). It would also I think be safer to explicitly guard this with a not-if-v8 check, because we don't actually implement that RAZ/WI behaviour. So something like: if (address < 0x02000000 && mmu_idx != ARMMMUIdx_S2NS && !arm_feature(env, ARM_FEATURE_V8)) { uint32_t fcseidr; if (regime_el(env, mmu_idx) == 3) { fcseidr = env->cp15.fcseidr_s; } else { fcseidr = env->cp15.fcseidr_ns; } address += fcseidr; } (note that stage 1 PL2 lookups need to use the NS FCSEIDR.) thanks -- PMM
On Wed, Jan 28, 2015 at 4:30 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 28 January 2015 at 21:37, Greg Bellows <greg.bellows@linaro.org> wrote: > > > >> +/* Return true if the translation regime is using LPAE format page > tables > >> */ > >> +static inline bool regime_using_lpae_format(CPUARMState *env, > >> + ARMMMUIdx mmu_idx) > >> +{ > >> + int el = regime_el(env, mmu_idx); > >> + if (el == 2 || arm_el_is_aa64(env, el)) { > > > > > > For the life of me, I can not figure out why EL2 is wired to always use > > LPAE. Is this stated in the spec somewhere? I found places where EL2 > > registers can vary depending on TTBCR.EAE bit settings which implies it > is > > not always true. > > The only translation regimes controlled by EL2 are: > (1) EL2's own stage 1 translations > (2) the stage 2 translations > > These must both be LPAE format: see the v8 ARM ARM section G4.4: > "the translation tables for the Non-secure PL2 stage 1 translations, > and for the Non-secure PL1&0 stage 2 translations, must use the > Long-descriptor translation table format." v7 ARM ARM B3.3 has > similar text. > I see that now, thanks for pointing this out > > (Basically, short-descriptors are obsolete and are only supported in > the pre-Virtualization translation regimes, ie AArch32 EL1/3.) > > > I realize EL2 is not supported yet, but wouldn't this break AArch32 if it > > were and the LPAE feature was not enabled? > > Implementations with Virtualization must include LPAE. > > >> -static bool get_level1_table_address(CPUARMState *env, uint32_t *table, > >> - uint32_t address) > >> +static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx > mmu_idx, > >> + uint32_t *table, uint32_t address) > >> { > >> - /* Get the TCR bank based on our security state */ > >> - TCR *tcr = &env->cp15.tcr_el[arm_is_secure(env) ? 3 : 1]; > >> + /* Note that we can only get here for an AArch32 PL0/PL1 lookup */ > >> + int el = regime_el(env, mmu_idx); > >> + TCR *tcr = regime_tcr(env, mmu_idx); > >> > >> - /* We only get here if EL1 is running in AArch32. If EL3 is running > >> in > >> - * AArch32 there is a secure and non-secure instance of the > >> translation > >> - * table registers. > >> - */ > >> if (address & tcr->mask) { > >> if (tcr->raw_tcr & TTBCR_PD1) { > >> /* Translation table walk disabled for TTBR1 */ > >> return false; > >> } > >> - *table = A32_BANKED_CURRENT_REG_GET(env, ttbr1) & 0xffffc000; > >> + *table = env->cp15.ttbr1_el[el] & 0xffffc000; > > > > > > Perhaps you plan to address this in a separate patch, but I believe > TTBR1 is > > only applicable to EL1 and EL0 in AArch64. > > It's true that TTBR1 is only for EL0/EL1, but see the comment at the > start of the function -- we can't get here except for EL0 and EL1, > because this function is only used for some kinds of short-descriptor > tables. > I saw that comment, but was not sure whether it was stale given we were adding EL3. I now see how AArch64 is routed away from this function. > > >> @@ -4663,7 +4736,12 @@ static int get_phys_addr_v5(CPUARMState *env, > >> uint32_t address, int access_type, > >> desc = ldl_phys(cs->as, table); > >> type = (desc & 3); > >> domain = (desc >> 5) & 0x0f; > >> - domain_prot = (A32_BANKED_CURRENT_REG_GET(env, dacr) >> (domain * > 2)) > >> & 3; > >> + if (regime_el(env, mmu_idx) == 1) { > >> + dacr = env->cp15.dacr_ns; > >> + } else { > >> + dacr = env->cp15.dacr_s; > >> + } > >> + domain_prot = (dacr >> (domain * 2)) & 3; > > > > > > Is there a reason that you did not add a regime_dacr() here like you did > for > > SCTLR and TCR? > > Didn't seem necessary, since we know we only need to deal with S vs NS, > and the concept isn't generally applicable to most regimes. If the > dacr in env->cp15 was stored as dacr[4] I'd have used dacr[el] as > I do above for the ttbr1_el[], but it isn't, hence the conditional. > > Fair enough, was more a question of consistency since you do the same thing in both the v5 and v6 code. > The TCR and SCTLR are used in LPAE format page tables so they apply > for the whole set of translation regimes. > > > Also, the ARMv8 ARM makes it sound like DACR has no value in AArch64. > > Well, the DACR is only relevant to short-descriptor format page tables, > so it's only consulted for AArch32 translations, and there's no > equivalent register in AArch64. (There is a DACR32_EL2 64 bit register, > but that is only there so a hypervisor can save and restore the state > of a 32 bit VM (at EL1) that is using short-descriptor page tables.) > > > However, if it did have meaning in AArch64, then for S1SE1 would we be > > accessing the wrong bank as regime_el returns 1? This working off the > > understanding that an address reference from an instruction executed in > > S/EL1 and AArch64 would generate such an index. > So this comment is moot given my misunderstanding that we would not be in this code if AArch64. > > We can only get here for regime S1SE1 if: > * EL3 is AArch64 > * EL1 is AArch32 > > Since EL3 is 64 bit, there is no banking of registers and regardless > of whether EL1 is Secure or NonSecure we want the one and only > register (which is in dacr_ns). (Compare the way we use > ttbr1_el[regime_el(env, mmu_idx)] and so get TTBR1_EL1 whether > this is Secure EL1 or NonSecure EL1.) > > If EL3 is 32 bit then there is banking of registers, but it's > not possible to get here for S1SE1 in that case (only for S EL3 > and NS EL1). > Right, that all makes sense. now. > > >> + /* TODO: > >> + * This code assumes we're either a 64-bit EL1 or a 32-bit PL1; > >> + * it doesn't handle the different format TCR for TCR_EL2, TCR_EL3, > >> + * and VTCR_EL2, or the fact that those regimes don't have a split > >> + * TTBR0/TTBR1. Attribute and permission bit handling should also > >> + * be checked when adding support for those page table walks. > >> + */ > > > > > > Maybe copy this comment up above in get_level1_table_address(). > > This is the correct location; see remarks above. > > >> @@ -5171,39 +5269,43 @@ static inline int get_phys_addr(CPUARMState > *env, > >> target_ulong address, > >> hwaddr *phys_ptr, int *prot, > >> target_ulong *page_size) > >> { > >> - /* This is not entirely correct as get_phys_addr() can also be > called > >> - * from ats_write() for an address translation of a specific > regime. > >> - */ > >> - 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); > >> + if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) { > >> + /* TODO: when we support EL2 we should here call ourselves > >> recursively > >> + * to do the stage 1 and then stage 2 translations. The > ldl_phys > >> + * calls for stage 1 will also need changing. > >> + * For non-EL2 CPUs a stage1+stage2 translation is just stage > 1. > >> + */ > >> + assert(!arm_feature(env, ARM_FEATURE_EL2)); > >> + mmu_idx += ARMMMUIdx_S1NSE0; > >> + } > >> > >> /* Fast Context Switch Extension. */ > >> - if (address < 0x02000000) { > >> + if (address < 0x02000000 && mmu_idx != ARMMMUIdx_S2NS) { > >> address += A32_BANKED_CURRENT_REG_GET(env, fcseidr); > > > > > > Now that the MMU index includes security state info we use in in certain > > circumstances to determine the security state. However, we don't seem to > > consistently use it. For example, earlier changes used the mmu_index to > > choose certain register banks but here we still rely on the BANKED > macro. I > > see this inconsistency being prone to errors. Maybe we have just not > gotten > > to change all of the cases over, but I thought I'd highlight it. > > Yes, you're right: if the Secure world does an NS ATS operation > then we should be using the NS copy of the register. I think > I mentally skipped over this requirement because the whole bit > of code is irrelevant for v8 CPUs (where FSCEIDR is mandatorily > RAZ/WI and so it doesn't matter which one we use). > > It would also I think be safer to explicitly guard this with > a not-if-v8 check, because we don't actually implement that > RAZ/WI behaviour. So something like: > > if (address < 0x02000000 && mmu_idx != ARMMMUIdx_S2NS > && !arm_feature(env, ARM_FEATURE_V8)) { > uint32_t fcseidr; > if (regime_el(env, mmu_idx) == 3) { > fcseidr = env->cp15.fcseidr_s; > } else { > fcseidr = env->cp15.fcseidr_ns; > } > address += fcseidr; > } > > (note that stage 1 PL2 lookups need to use the NS FCSEIDR.) > > Yeah, this approach makes sense. > thanks > -- PMM >
diff --git a/target-arm/helper.c b/target-arm/helper.c index 0ae04eb..0a06bbe 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -4556,13 +4556,88 @@ void arm_cpu_do_interrupt(CPUState *cs) cs->interrupt_request |= CPU_INTERRUPT_EXITTB; } + +/* Return the exception level which controls this address translation regime */ +static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx) +{ + switch (mmu_idx) { + case ARMMMUIdx_S2NS: + case ARMMMUIdx_S1E2: + return 2; + case ARMMMUIdx_S1E3: + return 3; + case ARMMMUIdx_S1SE0: + return arm_el_is_aa64(env, 3) ? 1 : 3; + case ARMMMUIdx_S1SE1: + case ARMMMUIdx_S1NSE0: + case ARMMMUIdx_S1NSE1: + return 1; + default: + g_assert_not_reached(); + } +} + +/* Return the SCTLR value which controls this address translation regime */ +static inline uint32_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx) +{ + return env->cp15.sctlr_el[regime_el(env, mmu_idx)]; +} + +/* Return true if the specified stage of address translation is disabled */ +static inline bool regime_translation_disabled(CPUARMState *env, + ARMMMUIdx mmu_idx) +{ + if (mmu_idx == ARMMMUIdx_S2NS) { + return (env->cp15.hcr_el2 & HCR_VM) == 0; + } + return (regime_sctlr(env, mmu_idx) & SCTLR_M) == 0; +} + +/* Return the TCR controlling this translation regime */ +static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx) +{ + if (mmu_idx == ARMMMUIdx_S2NS) { + /* TODO: return VTCR_EL2 */ + g_assert_not_reached(); + } + return &env->cp15.tcr_el[regime_el(env, mmu_idx)]; +} + +/* Return true if the translation regime is using LPAE format page tables */ +static inline bool regime_using_lpae_format(CPUARMState *env, + ARMMMUIdx mmu_idx) +{ + int el = regime_el(env, mmu_idx); + if (el == 2 || arm_el_is_aa64(env, el)) { + return true; + } + if (arm_feature(env, ARM_FEATURE_LPAE) + && (regime_tcr(env, mmu_idx)->raw_tcr & TTBCR_EAE)) { + return true; + } + return false; +} + +static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx) +{ + switch (mmu_idx) { + case ARMMMUIdx_S1SE0: + case ARMMMUIdx_S1NSE0: + return true; + default: + return false; + } +} + /* Check section/page access permissions. Returns the page protection flags, or zero if the access is not permitted. */ -static inline int check_ap(CPUARMState *env, int ap, int domain_prot, - int access_type, int is_user) +static inline int check_ap(CPUARMState *env, ARMMMUIdx mmu_idx, + int ap, int domain_prot, + int access_type) { int prot_ro; + bool is_user = regime_is_user(env, mmu_idx); if (domain_prot == 3) { return PAGE_READ | PAGE_WRITE; @@ -4580,7 +4655,7 @@ static inline int check_ap(CPUARMState *env, int ap, int domain_prot, } if (access_type == 1) return 0; - switch (A32_BANKED_CURRENT_REG_GET(env, sctlr) & (SCTLR_S | SCTLR_R)) { + switch (regime_sctlr(env, mmu_idx) & (SCTLR_S | SCTLR_R)) { case SCTLR_S: return is_user ? 0 : PAGE_READ; case SCTLR_R: @@ -4612,35 +4687,32 @@ static inline int check_ap(CPUARMState *env, int ap, int domain_prot, } } -static bool get_level1_table_address(CPUARMState *env, uint32_t *table, - uint32_t address) +static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx, + uint32_t *table, uint32_t address) { - /* Get the TCR bank based on our security state */ - TCR *tcr = &env->cp15.tcr_el[arm_is_secure(env) ? 3 : 1]; + /* Note that we can only get here for an AArch32 PL0/PL1 lookup */ + int el = regime_el(env, mmu_idx); + TCR *tcr = regime_tcr(env, mmu_idx); - /* We only get here if EL1 is running in AArch32. If EL3 is running in - * AArch32 there is a secure and non-secure instance of the translation - * table registers. - */ if (address & tcr->mask) { if (tcr->raw_tcr & TTBCR_PD1) { /* Translation table walk disabled for TTBR1 */ return false; } - *table = A32_BANKED_CURRENT_REG_GET(env, ttbr1) & 0xffffc000; + *table = env->cp15.ttbr1_el[el] & 0xffffc000; } else { if (tcr->raw_tcr & TTBCR_PD0) { /* Translation table walk disabled for TTBR0 */ return false; } - *table = A32_BANKED_CURRENT_REG_GET(env, ttbr0) & tcr->base_mask; + *table = env->cp15.ttbr0_el[el] & tcr->base_mask; } *table |= (address >> 18) & 0x3ffc; return true; } static int get_phys_addr_v5(CPUARMState *env, uint32_t address, int access_type, - int is_user, hwaddr *phys_ptr, + ARMMMUIdx mmu_idx, hwaddr *phys_ptr, int *prot, target_ulong *page_size) { CPUState *cs = CPU(arm_env_get_cpu(env)); @@ -4652,10 +4724,11 @@ static int get_phys_addr_v5(CPUARMState *env, uint32_t address, int access_type, int domain = 0; int domain_prot; hwaddr phys_addr; + uint32_t dacr; /* Pagetable walk. */ /* Lookup l1 descriptor. */ - if (!get_level1_table_address(env, &table, address)) { + if (!get_level1_table_address(env, mmu_idx, &table, address)) { /* Section translation fault if page walk is disabled by PD0 or PD1 */ code = 5; goto do_fault; @@ -4663,7 +4736,12 @@ static int get_phys_addr_v5(CPUARMState *env, uint32_t address, int access_type, desc = ldl_phys(cs->as, table); type = (desc & 3); domain = (desc >> 5) & 0x0f; - domain_prot = (A32_BANKED_CURRENT_REG_GET(env, dacr) >> (domain * 2)) & 3; + if (regime_el(env, mmu_idx) == 1) { + dacr = env->cp15.dacr_ns; + } else { + dacr = env->cp15.dacr_s; + } + domain_prot = (dacr >> (domain * 2)) & 3; if (type == 0) { /* Section translation fault. */ code = 5; @@ -4727,7 +4805,7 @@ static int get_phys_addr_v5(CPUARMState *env, uint32_t address, int access_type, } code = 15; } - *prot = check_ap(env, ap, domain_prot, access_type, is_user); + *prot = check_ap(env, mmu_idx, ap, domain_prot, access_type); if (!*prot) { /* Access permission fault. */ goto do_fault; @@ -4740,7 +4818,7 @@ do_fault: } static int get_phys_addr_v6(CPUARMState *env, uint32_t address, int access_type, - int is_user, hwaddr *phys_ptr, + ARMMMUIdx mmu_idx, hwaddr *phys_ptr, int *prot, target_ulong *page_size) { CPUState *cs = CPU(arm_env_get_cpu(env)); @@ -4754,10 +4832,11 @@ static int get_phys_addr_v6(CPUARMState *env, uint32_t address, int access_type, int domain = 0; int domain_prot; hwaddr phys_addr; + uint32_t dacr; /* Pagetable walk. */ /* Lookup l1 descriptor. */ - if (!get_level1_table_address(env, &table, address)) { + if (!get_level1_table_address(env, mmu_idx, &table, address)) { /* Section translation fault if page walk is disabled by PD0 or PD1 */ code = 5; goto do_fault; @@ -4775,7 +4854,12 @@ static int get_phys_addr_v6(CPUARMState *env, uint32_t address, int access_type, /* Page or Section. */ domain = (desc >> 5) & 0x0f; } - domain_prot = (A32_BANKED_CURRENT_REG_GET(env, dacr) >> (domain * 2)) & 3; + if (regime_el(env, mmu_idx) == 1) { + dacr = env->cp15.dacr_ns; + } else { + dacr = env->cp15.dacr_s; + } + domain_prot = (dacr >> (domain * 2)) & 3; if (domain_prot == 0 || domain_prot == 2) { if (type != 1) { code = 9; /* Section domain fault. */ @@ -4829,20 +4913,20 @@ static int get_phys_addr_v6(CPUARMState *env, uint32_t address, int access_type, if (domain_prot == 3) { *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; } else { - if (pxn && !is_user) { + if (pxn && !regime_is_user(env, mmu_idx)) { xn = 1; } if (xn && access_type == 2) goto do_fault; /* The simplified model uses AP[0] as an access control bit. */ - if ((A32_BANKED_CURRENT_REG_GET(env, sctlr) & SCTLR_AFE) + if ((regime_sctlr(env, mmu_idx) & SCTLR_AFE) && (ap & 1) == 0) { /* Access flag fault. */ code = (code == 15) ? 6 : 3; goto do_fault; } - *prot = check_ap(env, ap, domain_prot, access_type, is_user); + *prot = check_ap(env, mmu_idx, ap, domain_prot, access_type); if (!*prot) { /* Access permission fault. */ goto do_fault; @@ -4867,7 +4951,7 @@ typedef enum { } MMUFaultType; static int get_phys_addr_lpae(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_ptr) { @@ -4887,9 +4971,17 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address, int32_t granule_sz = 9; int32_t va_size = 32; int32_t tbi = 0; - TCR *tcr = &env->cp15.tcr_el[arm_is_secure(env) ? 3 : 1]; - - if (arm_el_is_aa64(env, 1)) { + bool is_user; + TCR *tcr = regime_tcr(env, mmu_idx); + + /* TODO: + * This code assumes we're either a 64-bit EL1 or a 32-bit PL1; + * it doesn't handle the different format TCR for TCR_EL2, TCR_EL3, + * and VTCR_EL2, or the fact that those regimes don't have a split + * TTBR0/TTBR1. Attribute and permission bit handling should also + * be checked when adding support for those page table walks. + */ + if (arm_el_is_aa64(env, regime_el(env, mmu_idx))) { va_size = 64; if (extract64(address, 55, 1)) tbi = extract64(tcr->raw_tcr, 38, 1); @@ -4904,12 +4996,12 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address, * TTBCR/TTBR0/TTBR1 in accordance with ARM ARM DDI0406C table B-32: */ uint32_t t0sz = extract32(tcr->raw_tcr, 0, 6); - if (arm_el_is_aa64(env, 1)) { + if (va_size == 64) { t0sz = MIN(t0sz, 39); t0sz = MAX(t0sz, 16); } uint32_t t1sz = extract32(tcr->raw_tcr, 16, 6); - if (arm_el_is_aa64(env, 1)) { + if (va_size == 64) { t1sz = MIN(t1sz, 39); t1sz = MAX(t1sz, 16); } @@ -4964,6 +5056,10 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address, } } + /* Here we should have set up all the parameters for the translation: + * va_size, ttbr, epd, tsz, granule_sz, tbi + */ + if (epd) { /* Translation table walk disabled => Translation fault on TLB miss */ goto do_fault; @@ -5049,6 +5145,7 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address, goto do_fault; } fault_type = permission_fault; + is_user = regime_is_user(env, mmu_idx); if (is_user && !(attrs & (1 << 4))) { /* Unprivileged access not enabled */ goto do_fault; @@ -5083,12 +5180,13 @@ do_fault: } static int get_phys_addr_mpu(CPUARMState *env, uint32_t address, - int access_type, int is_user, + int access_type, ARMMMUIdx mmu_idx, hwaddr *phys_ptr, int *prot) { int n; uint32_t mask; uint32_t base; + bool is_user = regime_is_user(env, mmu_idx); *phys_ptr = address; for (n = 7; n >= 0; n--) { @@ -5171,39 +5269,43 @@ static inline int get_phys_addr(CPUARMState *env, target_ulong address, hwaddr *phys_ptr, int *prot, target_ulong *page_size) { - /* This is not entirely correct as get_phys_addr() can also be called - * from ats_write() for an address translation of a specific regime. - */ - 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); + if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) { + /* TODO: when we support EL2 we should here call ourselves recursively + * to do the stage 1 and then stage 2 translations. The ldl_phys + * calls for stage 1 will also need changing. + * For non-EL2 CPUs a stage1+stage2 translation is just stage 1. + */ + assert(!arm_feature(env, ARM_FEATURE_EL2)); + mmu_idx += ARMMMUIdx_S1NSE0; + } /* Fast Context Switch Extension. */ - if (address < 0x02000000) { + if (address < 0x02000000 && mmu_idx != ARMMMUIdx_S2NS) { address += A32_BANKED_CURRENT_REG_GET(env, fcseidr); } - if ((sctlr & SCTLR_M) == 0) { + if (regime_translation_disabled(env, mmu_idx)) { /* MMU/MPU disabled. */ *phys_ptr = address; *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; *page_size = TARGET_PAGE_SIZE; return 0; - } else if (arm_feature(env, ARM_FEATURE_MPU)) { + } + + if (arm_feature(env, ARM_FEATURE_MPU)) { *page_size = TARGET_PAGE_SIZE; - return get_phys_addr_mpu(env, address, access_type, is_user, phys_ptr, - prot); - } else if (extended_addresses_enabled(env)) { - return get_phys_addr_lpae(env, address, access_type, is_user, phys_ptr, + return get_phys_addr_mpu(env, address, access_type, mmu_idx, phys_ptr, + prot); + } + + if (regime_using_lpae_format(env, mmu_idx)) { + return get_phys_addr_lpae(env, address, access_type, mmu_idx, phys_ptr, prot, page_size); - } else if (sctlr & SCTLR_XP) { - return get_phys_addr_v6(env, address, access_type, is_user, phys_ptr, + } else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) { + return get_phys_addr_v6(env, address, access_type, mmu_idx, phys_ptr, prot, page_size); } else { - return get_phys_addr_v5(env, address, access_type, is_user, phys_ptr, + return get_phys_addr_v5(env, address, access_type, mmu_idx, phys_ptr, prot, page_size); } }
Now we have the mmu_idx in get_phys_addr(), use it correctly to determine the behaviour of virtual to physical address translations, rather than using just an is_user flag and the current CPU state. Some TODO comments have been added to indicate where changes will need to be made to add EL2 and 64-bit EL3 support. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target-arm/helper.c | 200 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 151 insertions(+), 49 deletions(-)