| Message ID | 1448029971-9875-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hello, On Fri, Nov 20, 2015 at 3:32 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > In an LPAE format descriptor in ARMv8 the address field extends > up to bit 47, not just bit 39. Correct the masking so we don't > give incorrect results if the output address size is greater > than 40 bits, as it can be for AArch64. > > (Note that we don't yet support the new-in-v8 Address Size fault which > should be generated if any translation table entry or TTBR contains > an address with non-zero bits above the most significant bit of the > maximum output address size.) > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > This is worth fixing for 2.5 I think. As the commit message notes, > we don't support the Addres Size faults we ought to take in some > cases, but that seems more 2.6-ish. > --- > target-arm/helper.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 4ecae61..afc4163 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -6642,6 +6642,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, > int ap, ns, xn, pxn; > uint32_t el = regime_el(env, mmu_idx); > bool ttbr1_valid = true; > + uint64_t descaddrmask; > > /* TODO: > * This code does not handle the different format TCR for VTCR_EL2. > @@ -6831,6 +6832,15 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, > descaddr = extract64(ttbr, 0, 48); > descaddr &= ~((1ULL << (inputsize - (stride * (4 - level)))) - 1); > > + /* The address field in the descriptor goes up to bit 39 for ARMv7 > + * but up to bit 47 for ARMv8. > + */ > + if (arm_feature(env, ARM_FEATURE_V8)) { > + descaddrmask = 0xfffffffff000ULL; > + } else { > + descaddrmask = 0xfffffff000ULL; > + } My understanding is that 48 bits are used if you are running AArch64 code, and 40 bits are used for 32-bit code even on an ARMv8 CPU, so checking for ARM_FEATURE_V8 is perhaps not enough. Thanks, Laurent > + > /* Secure accesses start with the page table in secure memory and > * can be downgraded to non-secure at any step. Non-secure accesses > * remain non-secure. We implement this by just ORing in the NSTable/NS > @@ -6854,7 +6864,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, > /* Invalid, or the Reserved level 3 encoding */ > goto do_fault; > } > - descaddr = descriptor & 0xfffffff000ULL; > + descaddr = descriptor & descaddrmask; > > if ((descriptor & 2) && (level < 3)) { > /* Table entry. The top five bits are attributes which may > -- > 1.9.1 >
On 20 November 2015 at 15:18, Laurent Desnogues <laurent.desnogues@gmail.com> wrote: > Hello, > > On Fri, Nov 20, 2015 at 3:32 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> In an LPAE format descriptor in ARMv8 the address field extends >> up to bit 47, not just bit 39. Correct the masking so we don't >> give incorrect results if the output address size is greater >> than 40 bits, as it can be for AArch64. >> >> (Note that we don't yet support the new-in-v8 Address Size fault which >> should be generated if any translation table entry or TTBR contains >> an address with non-zero bits above the most significant bit of the >> maximum output address size.) >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> + /* The address field in the descriptor goes up to bit 39 for ARMv7 >> + * but up to bit 47 for ARMv8. >> + */ >> + if (arm_feature(env, ARM_FEATURE_V8)) { >> + descaddrmask = 0xfffffffff000ULL; >> + } else { >> + descaddrmask = 0xfffffff000ULL; >> + } > > My understanding is that 48 bits are used if you are running AArch64 > code, and 40 bits are used for 32-bit code even on an ARMv8 CPU, so > checking for ARM_FEATURE_V8 is perhaps not enough. For v8 32-bit code the usable address width is only 40 bits, but setting a bit in [47:40] causes an AddressSize fault on v8 (but not v7). So the mask should be 48 bits for v8 regardless of 32-vs-64, and when we support AddressSize faults we'll then check the upper bits of the masked-out address and raise a fault if needed. thanks -- PMM
On Fri, Nov 20, 2015 at 4:20 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 20 November 2015 at 15:18, Laurent Desnogues > <laurent.desnogues@gmail.com> wrote: >> Hello, >> >> On Fri, Nov 20, 2015 at 3:32 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> In an LPAE format descriptor in ARMv8 the address field extends >>> up to bit 47, not just bit 39. Correct the masking so we don't >>> give incorrect results if the output address size is greater >>> than 40 bits, as it can be for AArch64. >>> >>> (Note that we don't yet support the new-in-v8 Address Size fault which >>> should be generated if any translation table entry or TTBR contains >>> an address with non-zero bits above the most significant bit of the >>> maximum output address size.) >>> >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > >>> + /* The address field in the descriptor goes up to bit 39 for ARMv7 >>> + * but up to bit 47 for ARMv8. >>> + */ >>> + if (arm_feature(env, ARM_FEATURE_V8)) { >>> + descaddrmask = 0xfffffffff000ULL; >>> + } else { >>> + descaddrmask = 0xfffffff000ULL; >>> + } >> >> My understanding is that 48 bits are used if you are running AArch64 >> code, and 40 bits are used for 32-bit code even on an ARMv8 CPU, so >> checking for ARM_FEATURE_V8 is perhaps not enough. > > For v8 32-bit code the usable address width is only 40 bits, but > setting a bit in [47:40] causes an AddressSize fault on v8 (but not > v7). So the mask should be 48 bits for v8 regardless of 32-vs-64, > and when we support AddressSize faults we'll then check the upper > bits of the masked-out address and raise a fault if needed. That makes sense. So here we go: Reviewed-by: <laurent.desnogues@gmail.com> Thanks, Laurent
On Fri, Nov 20, 2015 at 02:32:51PM +0000, Peter Maydell wrote: > In an LPAE format descriptor in ARMv8 the address field extends > up to bit 47, not just bit 39. Correct the masking so we don't > give incorrect results if the output address size is greater > than 40 bits, as it can be for AArch64. > > (Note that we don't yet support the new-in-v8 Address Size fault which > should be generated if any translation table entry or TTBR contains > an address with non-zero bits above the most significant bit of the > maximum output address size.) > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > --- > This is worth fixing for 2.5 I think. As the commit message notes, > we don't support the Addres Size faults we ought to take in some > cases, but that seems more 2.6-ish. > --- > target-arm/helper.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 4ecae61..afc4163 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -6642,6 +6642,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, > int ap, ns, xn, pxn; > uint32_t el = regime_el(env, mmu_idx); > bool ttbr1_valid = true; > + uint64_t descaddrmask; > > /* TODO: > * This code does not handle the different format TCR for VTCR_EL2. > @@ -6831,6 +6832,15 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, > descaddr = extract64(ttbr, 0, 48); > descaddr &= ~((1ULL << (inputsize - (stride * (4 - level)))) - 1); > > + /* The address field in the descriptor goes up to bit 39 for ARMv7 > + * but up to bit 47 for ARMv8. > + */ > + if (arm_feature(env, ARM_FEATURE_V8)) { > + descaddrmask = 0xfffffffff000ULL; > + } else { > + descaddrmask = 0xfffffff000ULL; > + } > + > /* Secure accesses start with the page table in secure memory and > * can be downgraded to non-secure at any step. Non-secure accesses > * remain non-secure. We implement this by just ORing in the NSTable/NS > @@ -6854,7 +6864,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, > /* Invalid, or the Reserved level 3 encoding */ > goto do_fault; > } > - descaddr = descriptor & 0xfffffff000ULL; > + descaddr = descriptor & descaddrmask; > > if ((descriptor & 2) && (level < 3)) { > /* Table entry. The top five bits are attributes which may > -- > 1.9.1 >
diff --git a/target-arm/helper.c b/target-arm/helper.c index 4ecae61..afc4163 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -6642,6 +6642,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, int ap, ns, xn, pxn; uint32_t el = regime_el(env, mmu_idx); bool ttbr1_valid = true; + uint64_t descaddrmask; /* TODO: * This code does not handle the different format TCR for VTCR_EL2. @@ -6831,6 +6832,15 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, descaddr = extract64(ttbr, 0, 48); descaddr &= ~((1ULL << (inputsize - (stride * (4 - level)))) - 1); + /* The address field in the descriptor goes up to bit 39 for ARMv7 + * but up to bit 47 for ARMv8. + */ + if (arm_feature(env, ARM_FEATURE_V8)) { + descaddrmask = 0xfffffffff000ULL; + } else { + descaddrmask = 0xfffffff000ULL; + } + /* Secure accesses start with the page table in secure memory and * can be downgraded to non-secure at any step. Non-secure accesses * remain non-secure. We implement this by just ORing in the NSTable/NS @@ -6854,7 +6864,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, /* Invalid, or the Reserved level 3 encoding */ goto do_fault; } - descaddr = descriptor & 0xfffffff000ULL; + descaddr = descriptor & descaddrmask; if ((descriptor & 2) && (level < 3)) { /* Table entry. The top five bits are attributes which may
In an LPAE format descriptor in ARMv8 the address field extends up to bit 47, not just bit 39. Correct the masking so we don't give incorrect results if the output address size is greater than 40 bits, as it can be for AArch64. (Note that we don't yet support the new-in-v8 Address Size fault which should be generated if any translation table entry or TTBR contains an address with non-zero bits above the most significant bit of the maximum output address size.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- This is worth fixing for 2.5 I think. As the commit message notes, we don't support the Addres Size faults we ought to take in some cases, but that seems more 2.6-ish. --- target-arm/helper.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)