[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 31/31] target-arm: Add v8 mmu translation sup
From: |
Rob Herring |
Subject: |
Re: [Qemu-devel] [PATCH v3 31/31] target-arm: Add v8 mmu translation support |
Date: |
Wed, 26 Feb 2014 13:47:11 -0600 |
On Wed, Feb 26, 2014 at 4:31 AM, Peter Maydell <address@hidden> wrote:
> On 26 February 2014 03:32, Hu Tao <address@hidden> 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 <address@hidden>
>
>>> > /* 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
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;
} 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 (;;) {
- [Qemu-devel] [PATCH v3 22/31] target-arm: A64: Implement MSR (immediate) instructions, (continued)
- [Qemu-devel] [PATCH v3 22/31] target-arm: A64: Implement MSR (immediate) instructions, Peter Maydell, 2014/02/15
- [Qemu-devel] [PATCH v3 16/31] target-arm: Implement AArch64 dummy breakpoint and watchpoint registers, Peter Maydell, 2014/02/15
- [Qemu-devel] [PATCH v3 30/31] target-arm: Fix VFP enables for AArch32 EL0 under AArch64 EL1, Peter Maydell, 2014/02/15
- [Qemu-devel] [PATCH v3 28/31] target-arm: A64: Correctly fault FP/Neon if CPACR.FPEN set, Peter Maydell, 2014/02/15
- [Qemu-devel] [PATCH v3 05/31] target-arm: Implement AArch64 cache invalidate/clean ops, Peter Maydell, 2014/02/15
- [Qemu-devel] [PATCH v3 31/31] target-arm: Add v8 mmu translation support, Peter Maydell, 2014/02/15
[Qemu-devel] [PATCH v3 15/31] target-arm: Implement AArch64 ID and feature registers, Peter Maydell, 2014/02/15
[Qemu-devel] [PATCH v3 06/31] target-arm: Implement AArch64 TLB invalidate ops, Peter Maydell, 2014/02/15
[Qemu-devel] [PATCH v3 25/31] target-arm: Define exception record for AArch64 exceptions, Peter Maydell, 2014/02/15
[Qemu-devel] [PATCH v3 08/31] target-arm: Implement AArch64 memory attribute registers, Peter Maydell, 2014/02/15
[Qemu-devel] [PATCH v3 27/31] target-arm: Add support for generating exceptions with syndrome information, Peter Maydell, 2014/02/15
[Qemu-devel] [PATCH v3 26/31] target-arm: Provide correct syndrome information for cpreg access traps, Peter Maydell, 2014/02/15
[Qemu-devel] [PATCH v3 14/31] target-arm: Implement AArch64 generic timers, Peter Maydell, 2014/02/15
[Qemu-devel] [PATCH v3 02/31] target-arm: A64: Make cache ID registers visible to AArch64, Peter Maydell, 2014/02/15
[Qemu-devel] [PATCH v3 18/31] target-arm: Get MMU index information correct for A64 code, Peter Maydell, 2014/02/15