diff mbox

[v3,31/31] target-arm: Add v8 mmu translation support

Message ID CABGGisyyXsJ1-76w4E1S6-Pa5mLAeoDtygO=ds0hWyLZ2a=diw@mail.gmail.com
State New
Headers show

Commit Message

Rob Herring Feb. 26, 2014, 7:47 p.m. UTC 
On Wed, Feb 26, 2014 at 4:31 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 26 February 2014 03:32, Hu Tao <hutao@cn.fujitsu.com> wrote:
>> On Wed, Feb 26, 2014 at 10:49:59AM +0800, Hu Tao wrote:
>>> On Sat, Feb 15, 2014 at 04:07:24PM +0000, Peter Maydell wrote:
>>> > From: Rob Herring <rob.herring@linaro.org>
>
>>> >      /* Determine whether this address is in the region controlled by
>>> >       * TTBR0 or TTBR1 (or if it is in neither region and should fault).
>>> >       * This is a Non-secure PL0/1 stage 1 translation, so controlled by
>>> >       * TTBCR/TTBR0/TTBR1 in accordance with ARM ARM DDI0406C table B-32:
>>> >       */
>>> > -    uint32_t t0sz = extract32(env->cp15.c2_control, 0, 3);
>>> > -    uint32_t t1sz = extract32(env->cp15.c2_control, 16, 3);
>>> > -    if (t0sz && !extract32(address, 32 - t0sz, t0sz)) {
>>> > +    uint32_t t0sz = extract32(env->cp15.c2_control, 0, 5);
>>> > +    uint32_t t1sz = extract32(env->cp15.c2_control, 16, 5);
>>>
>>> t0sz is bit [5:0], so shouldn't we extract 6 bits here? same for t1sz.
>
> Yes.
>
>>> > +    if (t0sz && !extract64(address, va_size - t0sz, t0sz)) {
>>> >          /* there is a ttbr0 region and we are in it (high bits all zero) */
>>> >          ttbr_select = 0;
>>> > -    } else if (t1sz && !extract32(~address, 32 - t1sz, t1sz)) {
>>> > +    } else if (t1sz && !extract64(~address, va_size - t1sz, t1sz)) {
>>> >          /* there is a ttbr1 region and we are in it (high bits all one) */
>>> >          ttbr_select = 1;
>>> >      } else if (!t0sz) {
>>>
>>> Can't be true for Aarch64. the VA address space has a maximum address width
>>> of 48 bits(page D5-1712 of ARM DDI 0487A.a), that means t0sz and t1sz should
>>> have a minimum value of 16.
>>
>> It doesn't matter here. Maybe we should check the value when writing to
>> TCR_EL1. What's the behaviour when writing an invalid tsz to TCR_EL1?
>
> I haven't checked through all the details, but it looks like the answer is
> you can write anything, and the pseudocode for AArch64.TranslationTableWalk
> specifies what happens if the tsz is outside the expected range (we
> clamp tablesize to be 25 <= tablesize <= 48).
>
> I'm not sure we've correctly implemented the handling specified under
> the AddrTop() pseudocode function either.

Tagged addresses would probably be broken in other places as well as I
don't think we handle all of the BranchTo() pseudocode. I'm not sure
anything is using tagged addresses ATM.

I've fixed the above issues and found another issue on v7 LPAE with
the ttbr masking. I believe it to be correct now, but my formula does
not match the pseudocode which is:

baselowerbound = 3 + tablesize - stride*(3-level) - grainsize;
baseaddress = base<47:baselowerbound>:Zeros(baselowerbound);

This formula appears to be wrong AFAICT. Take a couple of examples:

level=1,tablesize=32: 3 + 32 - 12*2 - 9 = 2 (correct value is 5)
level=0,tablesize=48: 3 + 48 - 12*3 - 9 = 6 (correct value is 12)

Here are the fixes I've made. I've pushed an updated patch here:

git://git.linaro.org/people/rob.herring/qemu.git v8-mmu

Rob

     } else if (!t0sz) {
@@ -3403,7 +3421,7 @@ static int get_phys_addr_lpae(CPUARMState *env,
target_ulong address,

     /* Now we can extract the actual base address from the TTBR */
     descaddr = extract64(ttbr, 0, 48);
-    descaddr &= ~descmask;
+    descaddr &= ~((1ULL << (va_size - tsz - (granule_sz * (4 - level)))) - 1);

     tableattrs = 0;
     for (;;) {

Comments

Peter Maydell March 1, 2014, 7:58 p.m. UTC | #1 
On 26 February 2014 19:47, Rob Herring <rob.herring@linaro.org> wrote:
> Tagged addresses would probably be broken in other places as well as I
> don't think we handle all of the BranchTo() pseudocode. I'm not sure
> anything is using tagged addresses ATM.

Yeah, I agree we would need to fix other places too at some point.

> I've fixed the above issues and found another issue on v7 LPAE with
> the ttbr masking. I believe it to be correct now, but my formula does
> not match the pseudocode which is:
>
> baselowerbound = 3 + tablesize - stride*(3-level) - grainsize;
> baseaddress = base<47:baselowerbound>:Zeros(baselowerbound);
>
> This formula appears to be wrong AFAICT. Take a couple of examples:
>
> level=1,tablesize=32: 3 + 32 - 12*2 - 9 = 2 (correct value is 5)
> level=0,tablesize=48: 3 + 48 - 12*3 - 9 = 6 (correct value is 12)

Your examples seem to have the stride and grainsize the wrong
way round. grainsize is 16, 14, or 12, and stride is grainsize - 3.
So for the first one: 3 + 32 - 9*2 - 12 == 5
and for the second: 3 + 48 - 9*3 - 12 == 12

thanks
-- PMM
diff mbox

Patch

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 8849df6..9f7b4f0 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3315,19 +3315,37 @@  static int get_phys_addr_lpae(CPUARMState
*env, target_ulong address,
     target_ulong page_size;
     uint32_t attrs;
     int32_t granule_sz = 9;
-    int32_t va_size = arm_el_is_aa64(env, 1) ? 64 : 32;
+    int32_t va_size = 32;
+    int32_t tbi = 0;
+
+    if (arm_el_is_aa64(env, 1)) {
+        va_size = 64;
+        if (extract64(address, 55, 1))
+            tbi = extract32(env->cp15.c2_control, 38, 1);
+        else
+            tbi = extract32(env->cp15.c2_control, 37, 1);
+        tbi *= 8;
+    }

     /* Determine whether this address is in the region controlled by
      * TTBR0 or TTBR1 (or if it is in neither region and should fault).
      * This is a Non-secure PL0/1 stage 1 translation, so controlled by
      * TTBCR/TTBR0/TTBR1 in accordance with ARM ARM DDI0406C table B-32:
      */
-    uint32_t t0sz = extract32(env->cp15.c2_control, 0, 5);
-    uint32_t t1sz = extract32(env->cp15.c2_control, 16, 5);
-    if (t0sz && !extract64(address, va_size - t0sz, t0sz)) {
+    uint32_t t0sz = extract32(env->cp15.c2_control, 0, 6);
+    if (arm_el_is_aa64(env, 1)) {
+        t0sz = MIN(t0sz, 39);
+        t0sz = MAX(t0sz, 16);
+    }
+    uint32_t t1sz = extract32(env->cp15.c2_control, 16, 6);
+    if (arm_el_is_aa64(env, 1)) {
+        t1sz = MIN(t1sz, 39);
+        t1sz = MAX(t1sz, 16);
+    }
+    if (t0sz && !extract64(address, va_size - t0sz, t0sz - tbi)) {
         /* there is a ttbr0 region and we are in it (high bits all zero) */
         ttbr_select = 0;
-    } else if (t1sz && !extract64(~address, va_size - t1sz, t1sz)) {
+    } else if (t1sz && !extract64(~address, va_size - t1sz, t1sz - tbi)) {
         /* there is a ttbr1 region and we are in it (high bits all one) */
         ttbr_select = 1;