| Message ID | 1427483446-31900-6-git-send-email-greg.bellows@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 27 March 2015 at 19:10, Greg Bellows <greg.bellows@linaro.org> wrote: > Add a utility function for choosing the correct TTBR system register based on > the specified MMU index. Add use of function on physical address lookup. > @@ -5376,20 +5390,26 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address, > int32_t tbi = 0; > TCR *tcr = regime_tcr(env, mmu_idx); > int ap, ns, xn, pxn; > + uint32_t el = regime_el(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 > + * it doesn't handle the different format TCR for 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))) { > + if (arm_el_is_aa64(env, el)) { > va_size = 64; > - if (extract64(address, 55, 1)) > - tbi = extract64(tcr->raw_tcr, 38, 1); > - else > - tbi = extract64(tcr->raw_tcr, 37, 1); > + if (el == 3 || el == 2) { > + tbi = extract64(tcr->raw_tcr, 20, 1); > + } else { > + if (extract64(address, 55, 1)) { > + tbi = extract64(tcr->raw_tcr, 38, 1); > + } else { > + tbi = extract64(tcr->raw_tcr, 37, 1); > + } > + } > tbi *= 8; > } The TTBR related parts of this patch are fine, but this section feels a bit incomplete. The location of the TBI bit is not the only thing about the TCR format that's different for TCR_EL2 and TCR_EL3. I think it would be better to move this hunk into a separate patch that also fixes the other points the TODO comment mentions (including the other parts of the function that access raw_tcr bitfields). -- PMM
On Thu, Apr 16, 2015 at 1:03 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 27 March 2015 at 19:10, Greg Bellows <greg.bellows@linaro.org> wrote: > > Add a utility function for choosing the correct TTBR system register > based on > > the specified MMU index. Add use of function on physical address lookup. > > > @@ -5376,20 +5390,26 @@ static int get_phys_addr_lpae(CPUARMState *env, > target_ulong address, > > int32_t tbi = 0; > > TCR *tcr = regime_tcr(env, mmu_idx); > > int ap, ns, xn, pxn; > > + uint32_t el = regime_el(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 > > + * it doesn't handle the different format TCR for 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))) { > > + if (arm_el_is_aa64(env, el)) { > > va_size = 64; > > - if (extract64(address, 55, 1)) > > - tbi = extract64(tcr->raw_tcr, 38, 1); > > - else > > - tbi = extract64(tcr->raw_tcr, 37, 1); > > + if (el == 3 || el == 2) { > > + tbi = extract64(tcr->raw_tcr, 20, 1); > > + } else { > > + if (extract64(address, 55, 1)) { > > + tbi = extract64(tcr->raw_tcr, 38, 1); > > + } else { > > + tbi = extract64(tcr->raw_tcr, 37, 1); > > + } > > + } > > tbi *= 8; > > } > > The TTBR related parts of this patch are fine, but this section > feels a bit incomplete. The location of the TBI bit is not the > only thing about the TCR format that's different for TCR_EL2 > and TCR_EL3. I think it would be better to move this hunk into > a separate patch that also fixes the other points the TODO comment > mentions (including the other parts of the function that access > raw_tcr bitfields). > > I broke up the TTBR and TCR changes into separate patches. I'll add support for the lack of TTBR1 in EL2/3 but the rest (VTCR2 and certain attributes (shareability & physical addr size) should be left as separate patches as they are unrelated to EL3 and already unsupported. > -- PMM >
diff --git a/target-arm/helper.c b/target-arm/helper.c index 00b457a..13fdf02 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -4892,6 +4892,21 @@ static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx) return &env->cp15.tcr_el[regime_el(env, mmu_idx)]; } +/* Return the TTBR associated with this translation regime */ +static inline uint32_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx, + int ttbrn) +{ + if (mmu_idx == ARMMMUIdx_S2NS) { + /* TODO: return VTTBR_EL2 */ + g_assert_not_reached(); + } + if (ttbrn == 0) { + return env->cp15.ttbr0_el[regime_el(env, mmu_idx)]; + } else { + return env->cp15.ttbr1_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) @@ -5090,7 +5105,6 @@ static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx, uint32_t *table, uint32_t address) { /* 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); if (address & tcr->mask) { @@ -5098,13 +5112,13 @@ static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx, /* Translation table walk disabled for TTBR1 */ return false; } - *table = env->cp15.ttbr1_el[el] & 0xffffc000; + *table = regime_ttbr(env, mmu_idx, 1) & 0xffffc000; } else { if (tcr->raw_tcr & TTBCR_PD0) { /* Translation table walk disabled for TTBR0 */ return false; } - *table = env->cp15.ttbr0_el[el] & tcr->base_mask; + *table = regime_ttbr(env, mmu_idx, 0) & tcr->base_mask; } *table |= (address >> 18) & 0x3ffc; return true; @@ -5376,20 +5390,26 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address, int32_t tbi = 0; TCR *tcr = regime_tcr(env, mmu_idx); int ap, ns, xn, pxn; + uint32_t el = regime_el(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 + * it doesn't handle the different format TCR for 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))) { + if (arm_el_is_aa64(env, el)) { va_size = 64; - if (extract64(address, 55, 1)) - tbi = extract64(tcr->raw_tcr, 38, 1); - else - tbi = extract64(tcr->raw_tcr, 37, 1); + if (el == 3 || el == 2) { + tbi = extract64(tcr->raw_tcr, 20, 1); + } else { + if (extract64(address, 55, 1)) { + tbi = extract64(tcr->raw_tcr, 38, 1); + } else { + tbi = extract64(tcr->raw_tcr, 37, 1); + } + } tbi *= 8; } @@ -5434,7 +5454,7 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address, * we will always flush the TLB any time the ASID is changed). */ if (ttbr_select == 0) { - ttbr = A32_BANKED_CURRENT_REG_GET(env, ttbr0); + ttbr = regime_ttbr(env, mmu_idx, 0); epd = extract32(tcr->raw_tcr, 7, 1); tsz = t0sz; @@ -5446,7 +5466,7 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address, granule_sz = 11; } } else { - ttbr = A32_BANKED_CURRENT_REG_GET(env, ttbr1); + ttbr = regime_ttbr(env, mmu_idx, 1); epd = extract32(tcr->raw_tcr, 23, 1); tsz = t1sz;
Add a utility function for choosing the correct TTBR system register based on the specified MMU index. Add use of function on physical address lookup. Signed-off-by: Greg Bellows <greg.bellows@linaro.org> --- target-arm/helper.c | 44 ++++++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 12 deletions(-)