| Message ID | 20200302175829.2183-9-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Misc cleanups surrounding TBI | expand |
On Mon, 2 Mar 2020 at 17:58, Richard Henderson <richard.henderson@linaro.org> wrote: > > We fail to validate the upper bits of a virtual address on a > translation disabled regime, as per AArch64.TranslateAddressS1Off. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/helper.c | 33 ++++++++++++++++++++++++++++++++- > 1 file changed, 32 insertions(+), 1 deletion(-) > > diff --git a/target/arm/helper.c b/target/arm/helper.c > index addbec91d8..0ef32d3c24 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -11634,7 +11634,38 @@ bool get_phys_addr(CPUARMState *env, target_ulong address, > /* Definitely a real MMU, not an MPU */ > > if (regime_translation_disabled(env, mmu_idx)) { > - /* MMU disabled. */ > + /* > + * MMU disabled. S1 addresses are still checked for bounds. > + * C.f. AArch64.TranslateAddressS1Off. > + */ > + if (is_a64(env) && mmu_idx != ARMMMUIdx_Stage2) { This looks weird -- why do we care about whether the current EL is aarch64, rather than looking at the controlling EL for the translation regime ? thanks -- PMM
On 3/5/20 6:21 AM, Peter Maydell wrote: >> if (regime_translation_disabled(env, mmu_idx)) { >> - /* MMU disabled. */ >> + /* >> + * MMU disabled. S1 addresses are still checked for bounds. >> + * C.f. AArch64.TranslateAddressS1Off. >> + */ >> + if (is_a64(env) && mmu_idx != ARMMMUIdx_Stage2) { > > This looks weird -- why do we care about whether the current > EL is aarch64, rather than looking at the controlling EL > for the translation regime ? You're right, it should be the aa64-ness of the regime_el. Thanks, r~
diff --git a/target/arm/helper.c b/target/arm/helper.c index addbec91d8..0ef32d3c24 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -11634,7 +11634,38 @@ bool get_phys_addr(CPUARMState *env, target_ulong address, /* Definitely a real MMU, not an MPU */ if (regime_translation_disabled(env, mmu_idx)) { - /* MMU disabled. */ + /* + * MMU disabled. S1 addresses are still checked for bounds. + * C.f. AArch64.TranslateAddressS1Off. + */ + if (is_a64(env) && mmu_idx != ARMMMUIdx_Stage2) { + int pamax = arm_pamax(env_archcpu(env)); + uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr; + int addrtop, tbi; + + tbi = aa64_va_parameter_tbi(tcr, mmu_idx); + if (access_type == MMU_INST_FETCH) { + tbi &= ~aa64_va_parameter_tbid(tcr, mmu_idx); + } + tbi = (tbi >> extract64(address, 55, 1)) & 1; + addrtop = (tbi ? 55 : 63); + + if (extract64(address, pamax, addrtop - pamax + 1) != 0) { + fi->type = ARMFault_AddressSize; + fi->level = 0; + fi->stage2 = false; + return 1; + } + + /* + * The ARM pseudocode copies bits [51:0] to addrdesc.paddress. + * Except for TBI, we've just validated everything above PAMax + * is zero. So we only need to drop TBI. + */ + if (tbi) { + address = extract64(address, 0, 56); + } + } *phys_ptr = address; *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; *page_size = TARGET_PAGE_SIZE;
We fail to validate the upper bits of a virtual address on a translation disabled regime, as per AArch64.TranslateAddressS1Off. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/helper.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) -- 2.20.1