| Message ID | 20191011134744.2477-3-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v5,01/22] target/arm: Add MTE_ACTIVE to tb_flags | expand |
On Fri, 11 Oct 2019 at 14:48, Richard Henderson <richard.henderson@linaro.org> wrote: > > A translation with 2 ranges has both positive and negative addresses. > This is true for the EL1&0 and the as-yet unimplemented EL2&0 regimes. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/internals.h | 14 ++++++++++++++ > target/arm/helper.c | 22 +++++----------------- > target/arm/translate-a64.c | 3 +-- > 3 files changed, 20 insertions(+), 19 deletions(-) > > diff --git a/target/arm/internals.h b/target/arm/internals.h > index dcc5d6cca3..9486680b87 100644 > --- a/target/arm/internals.h > +++ b/target/arm/internals.h > @@ -804,6 +804,20 @@ static inline void arm_call_el_change_hook(ARMCPU *cpu) > } > } > > +/* Return true if this address translation regime has two ranges. */ > +static inline bool regime_has_2_ranges(ARMMMUIdx mmu_idx) > +{ > + switch (mmu_idx) { > + case ARMMMUIdx_S12NSE0: > + case ARMMMUIdx_S12NSE1: > + case ARMMMUIdx_S1NSE0: > + case ARMMMUIdx_S1NSE1: > + return true; Don't S1SE0 and S1SE1 also need to be here? > + default: > + return false; > + } > +} > + > /* Return true if this address translation regime is secure */ > static inline bool regime_is_secure(CPUARMState *env, ARMMMUIdx mmu_idx) > { > diff --git a/target/arm/helper.c b/target/arm/helper.c > index b690eda136..f9dee51ede 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -8774,15 +8774,8 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64, > } > > if (is_aa64) { > - switch (regime_el(env, mmu_idx)) { > - case 1: > - if (!is_user) { > - xn = pxn || (user_rw & PAGE_WRITE); > - } > - break; > - case 2: > - case 3: > - break; > + if (regime_has_2_ranges(mmu_idx) && !is_user) { > + xn = pxn || (user_rw & PAGE_WRITE); > } (I was sceptical that 'regime_has_2_ranges()' was the right condition here, but the Arm ARM really does define it as "valid only when stage 1 of the translation regime can support two VA ranges".) > } else if (arm_feature(env, ARM_FEATURE_V7)) { > switch (regime_el(env, mmu_idx)) { > @@ -9316,7 +9309,6 @@ ARMVAParameters aa64_va_parameters_both(CPUARMState *env, uint64_t va, > ARMMMUIdx mmu_idx) > { > uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr; > - uint32_t el = regime_el(env, mmu_idx); > bool tbi, tbid, epd, hpd, tcma, using16k, using64k; > int select, tsz; > > @@ -9326,7 +9318,7 @@ ARMVAParameters aa64_va_parameters_both(CPUARMState *env, uint64_t va, > */ > select = extract64(va, 55, 1); > > - if (el > 1) { > + if (!regime_has_2_ranges(mmu_idx)) { > tsz = extract32(tcr, 0, 6); > using64k = extract32(tcr, 14, 1); > using16k = extract32(tcr, 15, 1); > @@ -9486,10 +9478,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, > param = aa64_va_parameters(env, address, mmu_idx, > access_type != MMU_INST_FETCH); > level = 0; > - /* If we are in 64-bit EL2 or EL3 then there is no TTBR1, so mark it > - * invalid. > - */ > - ttbr1_valid = (el < 2); > + ttbr1_valid = regime_has_2_ranges(mmu_idx); > addrsize = 64 - 8 * param.tbi; > inputsize = 64 - param.tsz; > } else { > @@ -11095,8 +11084,7 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc, > ARMVAParameters p0 = aa64_va_parameters_both(env, 0, stage1); > int tbii; > > - /* FIXME: ARMv8.1-VHE S2 translation regime. */ > - if (regime_el(env, stage1) < 2) { > + if (regime_has_2_ranges(mmu_idx)) { Now that the rebuild_hflags patchset has landed this is in rebuild_hflags_a64(). > ARMVAParameters p1 = aa64_va_parameters_both(env, -1, stage1); > tbid = (p1.tbi << 1) | p0.tbi; > tbii = tbid & ~((p1.tbid << 1) | p0.tbid); > diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c > index 51f3af9cd9..c85db69db4 100644 > --- a/target/arm/translate-a64.c > +++ b/target/arm/translate-a64.c > @@ -175,8 +175,7 @@ static void gen_top_byte_ignore(DisasContext *s, TCGv_i64 dst, > if (tbi == 0) { > /* Load unmodified address */ > tcg_gen_mov_i64(dst, src); > - } else if (s->current_el >= 2) { > - /* FIXME: ARMv8.1-VHE S2 translation regime. */ > + } else if (!regime_has_2_ranges(s->mmu_idx)) { > /* Force tag byte to all zero */ > tcg_gen_extract_i64(dst, src, 0, 56); > } else { The comment above this function also needs updating to no longer refer to "EL2 and EL3" vs "EL0 and EL1". (You might also remove the use of the imperial 'We' in the last sentence in it ;-)) Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 12/3/19 11:01 AM, Peter Maydell wrote: >> +/* Return true if this address translation regime has two ranges. */ >> +static inline bool regime_has_2_ranges(ARMMMUIdx mmu_idx) >> +{ >> + switch (mmu_idx) { >> + case ARMMMUIdx_S12NSE0: >> + case ARMMMUIdx_S12NSE1: >> + case ARMMMUIdx_S1NSE0: >> + case ARMMMUIdx_S1NSE1: >> + return true; > > Don't S1SE0 and S1SE1 also need to be here? Whoops, yes. I'll need to fix that in the VHE patch set too. r~
diff --git a/target/arm/internals.h b/target/arm/internals.h index dcc5d6cca3..9486680b87 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -804,6 +804,20 @@ static inline void arm_call_el_change_hook(ARMCPU *cpu) } } +/* Return true if this address translation regime has two ranges. */ +static inline bool regime_has_2_ranges(ARMMMUIdx mmu_idx) +{ + switch (mmu_idx) { + case ARMMMUIdx_S12NSE0: + case ARMMMUIdx_S12NSE1: + case ARMMMUIdx_S1NSE0: + case ARMMMUIdx_S1NSE1: + return true; + default: + return false; + } +} + /* Return true if this address translation regime is secure */ static inline bool regime_is_secure(CPUARMState *env, ARMMMUIdx mmu_idx) { diff --git a/target/arm/helper.c b/target/arm/helper.c index b690eda136..f9dee51ede 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -8774,15 +8774,8 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64, } if (is_aa64) { - switch (regime_el(env, mmu_idx)) { - case 1: - if (!is_user) { - xn = pxn || (user_rw & PAGE_WRITE); - } - break; - case 2: - case 3: - break; + if (regime_has_2_ranges(mmu_idx) && !is_user) { + xn = pxn || (user_rw & PAGE_WRITE); } } else if (arm_feature(env, ARM_FEATURE_V7)) { switch (regime_el(env, mmu_idx)) { @@ -9316,7 +9309,6 @@ ARMVAParameters aa64_va_parameters_both(CPUARMState *env, uint64_t va, ARMMMUIdx mmu_idx) { uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr; - uint32_t el = regime_el(env, mmu_idx); bool tbi, tbid, epd, hpd, tcma, using16k, using64k; int select, tsz; @@ -9326,7 +9318,7 @@ ARMVAParameters aa64_va_parameters_both(CPUARMState *env, uint64_t va, */ select = extract64(va, 55, 1); - if (el > 1) { + if (!regime_has_2_ranges(mmu_idx)) { tsz = extract32(tcr, 0, 6); using64k = extract32(tcr, 14, 1); using16k = extract32(tcr, 15, 1); @@ -9486,10 +9478,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, param = aa64_va_parameters(env, address, mmu_idx, access_type != MMU_INST_FETCH); level = 0; - /* If we are in 64-bit EL2 or EL3 then there is no TTBR1, so mark it - * invalid. - */ - ttbr1_valid = (el < 2); + ttbr1_valid = regime_has_2_ranges(mmu_idx); addrsize = 64 - 8 * param.tbi; inputsize = 64 - param.tsz; } else { @@ -11095,8 +11084,7 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc, ARMVAParameters p0 = aa64_va_parameters_both(env, 0, stage1); int tbii; - /* FIXME: ARMv8.1-VHE S2 translation regime. */ - if (regime_el(env, stage1) < 2) { + if (regime_has_2_ranges(mmu_idx)) { ARMVAParameters p1 = aa64_va_parameters_both(env, -1, stage1); tbid = (p1.tbi << 1) | p0.tbi; tbii = tbid & ~((p1.tbid << 1) | p0.tbid); diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index 51f3af9cd9..c85db69db4 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -175,8 +175,7 @@ static void gen_top_byte_ignore(DisasContext *s, TCGv_i64 dst, if (tbi == 0) { /* Load unmodified address */ tcg_gen_mov_i64(dst, src); - } else if (s->current_el >= 2) { - /* FIXME: ARMv8.1-VHE S2 translation regime. */ + } else if (!regime_has_2_ranges(s->mmu_idx)) { /* Force tag byte to all zero */ tcg_gen_extract_i64(dst, src, 0, 56); } else {
A translation with 2 ranges has both positive and negative addresses. This is true for the EL1&0 and the as-yet unimplemented EL2&0 regimes. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/internals.h | 14 ++++++++++++++ target/arm/helper.c | 22 +++++----------------- target/arm/translate-a64.c | 3 +-- 3 files changed, 20 insertions(+), 19 deletions(-) -- 2.17.1