Improper TTBCR for arm 32bit kernel decompression
Subsystems (2)
Hello,
While trying to boot arm-32 bit kernel, I came across a problem where TTBCR is in improper state. If the bootloader uses the long descriptor format and jumps to kernel decompressor code, TTBCR may not be in the right state. So, as soon as the MMU is enabled, execution can not proceed further.
Before enabling the MMU, it is required to clear the TTBCR.PD0 field to use TTBR0 for translation table walks. Also, TTBCR.N should be reset to '0' to indicate the correct base address width. The 'commit dbece45894d3a ("ARM: 7501/1: decompressor: reset ttbcr for VMSA ARMv7 cores")' does the reset of TTBCR.N, but doesn't consider all the bits for the size of TTBCR.N.
when i tried the below change where i explicitly clear TTBCR.PD0 and use correct mask for TTBCR.N, I see proper memory after MMU is enabled and decompression succeeds.
Request your comments on the change below. If it looks good, I can submit a patch for inclusion.
---------------------8<----------------------------------
(quoted diff) +0 -0
b/arch/arm/boot/compressed/head.S
index af11c2f..5769f1f 100644
arch/arm/boot/compressed/head.S +1 -1
@@ -779,7 +779,8 @@ __armv7_mmu_cache_on: orrne r0, r0, #1 @ MMU enabled movne r1, #0xfffffffd @ domain 0 = client bic r6, r6, #1 << 31 @ 32-bit translation system - bic r6, r6, #3 << 0 @ use only ttbr0 + bic r6, r6, #7 << 0 @ width of base address
field
+ bic r6, r6, #1 << 4 @ use only ttbr0
mcrne p15, 0, r3, c2, c0, 0 @ load page table pointer
mcrne p15, 0, r1, c3, c0, 0 @ load domain access
control
mcrne p15, 0, r6, c2, c0, 2 @ load ttb control---------------------8<----------------------------------
Thanks,
-- Srinivas R
Signature · 3 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Quoted 26 lines · Srinivas Ramana
Hello,
While trying to boot arm-32 bit kernel, I came across a problem where TTBCR is in improper state. If the bootloader uses the long descriptor format and jumps to kernel decompressor code, TTBCR may not be in the right state. So, as soon as the MMU is enabled, execution can not proceed further.
Before enabling the MMU, it is required to clear the TTBCR.PD0 field to use TTBR0 for translation table walks. Also, TTBCR.N should be reset to '0' to indicate the correct base address width. The 'commit
dbece45894d3a("ARM:
7501/1: decompressor: reset ttbcr for VMSA ARMv7 cores")' does the reset of TTBCR.N, but doesn't consider all the bits for the size of TTBCR.N.when i tried the below change where i explicitly clear TTBCR.PD0 and use correct mask for TTBCR.N, I see proper memory after MMU is enabled and decompression succeeds.
Request your comments on the change below. If it looks good, I can submit a patch for inclusion.
---------------------8<----------------------------------
arch/arm/boot/compressed/head.S +0
-0
arch/arm/boot/compressed/head.S __armv7_mmu_cache_on: · line 783
Quoted 6 lines
arch/arm/boot/compressed/head.S +2
-1@@ -779,7 +779,8 @@ __armv7_mmu_cache_on: orrne r0, r0, #1 @ MMU enabled movne r1, #0xfffffffd @ domain 0 = client bic r6, r6, #1 << 31 @ 32-bit translation system - bic r6, r6, #3 << 0 @ use only ttbr0 + bic r6, r6, #7 << 0 @ width of base address field + bic r6, r6, #1 << 4 @ use only ttbr0
You could combine those instructions like this:
bic r6, r6, #(7 << 0) | (1 << 4) @ use only ttbr0
Nicolas
Quoted 37 lines · Nicolas Pitre
Quoted 26 lines · Srinivas Ramana
Hello,
While trying to boot arm-32 bit kernel, I came across a problem where TTBCR is in improper state. If the bootloader uses the long descriptor format and jumps to kernel decompressor code, TTBCR may not be in the right state. So, as soon as the MMU is enabled, execution can not proceed further.
Before enabling the MMU, it is required to clear the TTBCR.PD0 field to use TTBR0 for translation table walks. Also, TTBCR.N should be reset to '0' to indicate the correct base address width. The 'commit
dbece45894d3a("ARM:
7501/1: decompressor: reset ttbcr for VMSA ARMv7 cores")' does the reset of TTBCR.N, but doesn't consider all the bits for the size of TTBCR.N.when i tried the below change where i explicitly clear TTBCR.PD0 and use correct mask for TTBCR.N, I see proper memory after MMU is enabled and decompression succeeds.
Request your comments on the change below. If it looks good, I can submit a patch for inclusion.
---------------------8<----------------------------------
arch/arm/boot/compressed/head.S +0
-0arch/arm/boot/compressed/head.S __armv7_mmu_cache_on: · line 783
Quoted 6 lines
arch/arm/boot/compressed/head.S +2
-1@@ -779,7 +779,8 @@ __armv7_mmu_cache_on: orrne r0, r0, #1 @ MMU enabled movne r1, #0xfffffffd @ domain 0 = client bic r6, r6, #1 << 31 @ 32-bit translation system - bic r6, r6, #3 << 0 @ use only ttbr0 + bic r6, r6, #7 << 0 @ width of base address field + bic r6, r6, #1 << 4 @ use only ttbr0You could combine those instructions like this:
Quoted 1 line
bic r6, r6, #(7 << 0) | (1 << 4) @ use only ttbr0
Sure, Thanks for the suggestion. I can incorporate this and submit a patch. Can i use your Acked-by?
Thanks,
-- Srinivas R
Quoted 7 lines
Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 3 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Quoted 37 lines · Nicolas Pitre
Quoted 26 lines · Srinivas Ramana
Hello,
While trying to boot arm-32 bit kernel, I came across a problem where TTBCR is in improper state. If the bootloader uses the long descriptor format and jumps to kernel decompressor code, TTBCR may not be in the right state. So, as soon as the MMU is enabled, execution can not proceed further.
Before enabling the MMU, it is required to clear the TTBCR.PD0 field to use TTBR0 for translation table walks. Also, TTBCR.N should be reset to '0' to indicate the correct base address width. The 'commit
dbece45894d3a("ARM:
7501/1: decompressor: reset ttbcr for VMSA ARMv7 cores")' does the reset of TTBCR.N, but doesn't consider all the bits for the size of TTBCR.N.when i tried the below change where i explicitly clear TTBCR.PD0 and use correct mask for TTBCR.N, I see proper memory after MMU is enabled and decompression succeeds.
Request your comments on the change below. If it looks good, I can submit a patch for inclusion.
---------------------8<----------------------------------
arch/arm/boot/compressed/head.S +0
-0arch/arm/boot/compressed/head.S __armv7_mmu_cache_on: · line 783
Quoted 6 lines
arch/arm/boot/compressed/head.S +2
-1@@ -779,7 +779,8 @@ __armv7_mmu_cache_on: orrne r0, r0, #1 @ MMU enabled movne r1, #0xfffffffd @ domain 0 = client bic r6, r6, #1 << 31 @ 32-bit translation system - bic r6, r6, #3 << 0 @ use only ttbr0 + bic r6, r6, #7 << 0 @ width of base address field + bic r6, r6, #1 << 4 @ use only ttbr0You could combine those instructions like this:
Quoted 1 line
bic r6, r6, #(7 << 0) | (1 << 4) @ use only ttbr0
Sure, I can do that. Thanks for the suggestion. Will send out a patch for review.
Thanks,
-- Srinivas R
Signature · 3 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH 1/1 ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
If the bootloader uses the long descriptor format and jumps to kernel decompressor code, TTBCR may not be in a right state. Before enabling the MMU, it is required to clear the TTBCR.PD0 field to use TTBR0 for translation table walks.
The 'commit dbece45894d3a ("ARM: 7501/1: decompressor:
reset ttbcr for VMSA ARMv7 cores")' does the reset of TTBCR.N, but doesn't consider all the bits for the size of TTBCR.N.
Clear TTBCR.PD0 field and reset all the three bits of TTBCR.N to indicate the use of TTBR0 and the correct base address width.
--- arch/arm/boot/compressed/head.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/compressed/head.S +1 -1
@@ -779,7 +779,7 @@ __armv7_mmu_cache_on: orrne r0, r0, #1 @ MMU enabled movne r1, #0xfffffffd @ domain 0 = client bic r6, r6, #1 << 31 @ 32-bit translation system - bic r6, r6, #3 << 0 @ use only ttbr0 + bic r6, r6, #(7 << 0) | (1 << 4) @ use only ttbr0 mcrne p15, 0, r3, c2, c0, 0 @ load page table pointer mcrne p15, 0, r1, c3, c0, 0 @ load domain access control mcrne p15, 0, r6, c2, c0, 2 @ load ttb control
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Russell King - ARM Linux
maintainerRe: [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
Quoted 7 lines · Srinivas Ramana
If the bootloader uses the long descriptor format and jumps to kernel decompressor code, TTBCR may not be in a right state. Before enabling the MMU, it is required to clear the TTBCR.PD0 field to use TTBR0 for translation table walks.
The 'commit
dbece45894d3a("ARM: 7501/1: decompressor:
reset ttbcr for VMSA ARMv7 cores")' does the reset of TTBCR.N, but
There's no need to single-quote the commit part of this sentence.
Quoted 5 lines
doesn't consider all the bits for the size of TTBCR.N.
Clear TTBCR.PD0 field and reset all the three bits of TTBCR.N to indicate the use of TTBR0 and the correct base address width.
Adding a properly formatted Fixes: line here would be a bonus if we need to backport it to stable trees.
Quoted 10 lines
Signed-off-by Srinivas Ramana <[email protected]>--- arch/arm/boot/compressed/head.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm/boot/compressed/head.S +0
-0
arch/arm/boot/compressed/head.S __armv7_mmu_cache_on: · line 782
Quoted 5 lines
arch/arm/boot/compressed/head.S +1
-1@@ -779,7 +779,7 @@ __armv7_mmu_cache_on: orrne r0, r0, #1 @ MMU enabled movne r1, #0xfffffffd @ domain 0 = client bic r6, r6, #1 << 31 @ 32-bit translation system - bic r6, r6, #3 << 0 @ use only ttbr0 + bic r6, r6, #(7 << 0) | (1 << 4) @ use only ttbr0
Provided this is correct (I've not checked, I'd like an ack from one of the ARM people, and I'd prefer to see a tested-by as well), and the above points are addressed, it can be dropped into the patch system. Thanks.
Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: [email protected] down 400kbps up according to speedtest.net.
Re: [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
In reply to Srinivas RamanaQuoted 22 lines · Srinivas Ramana
If the bootloader uses the long descriptor format and jumps to kernel decompressor code, TTBCR may not be in a right state. Before enabling the MMU, it is required to clear the TTBCR.PD0 field to use TTBR0 for translation table walks.
The 'commit
dbece45894d3a("ARM: 7501/1: decompressor:
reset ttbcr for VMSA ARMv7 cores")' does the reset of TTBCR.N, but doesn't consider all the bits for the size of TTBCR.N.Clear TTBCR.PD0 field and reset all the three bits of TTBCR.N to indicate the use of TTBR0 and the correct base address width.
Signed-off-by Srinivas Ramana <[email protected]>--- arch/arm/boot/compressed/head.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm/boot/compressed/head.S +0
-0
arch/arm/boot/compressed/head.S __armv7_mmu_cache_on: · line 781
Quoted 3 lines
arch/arm/boot/compressed/head.S +0
-0@@ -779,7 +779,7 @@ __armv7_mmu_cache_on: orrne r0, r0, #1 @ MMU enabled movne r1, #0xfffffffd @ domain 0 = client bic r6, r6, #1 << 31 @ 32-bit translation system
Hmm, if TTBCR.EAE _was_ actually set...
arch/arm/boot/compressed/head.S __armv7_mmu_cache_on: · line 785
Quoted 5 lines
arch/arm/boot/compressed/head.S +1
-1- bic r6, r6, #3 << 0 @ use only ttbr0 + bic r6, r6, #(7 << 0) | (1 << 4) @ use only ttbr0 mcrne p15, 0, r3, c2, c0, 0 @ load page table pointer mcrne p15, 0, r1, c3, c0, 0 @ load domain access control mcrne p15, 0, r6, c2, c0, 2 @ load ttb control
...then strictly the TLBIALL needs to happen after the ISB following this update. Otherwise per B3.10.2 of DDI406C.c I think we might be into unpredictable territory - i.e. if the TLB happens to treat long- and short-descriptor entries differently then the TLBI beforehand (with EAE set) may be at liberty to only discard long-descriptor entries and leave bogus short-descriptor entries sitting around.
In other words, something like (completely untested):
---8<---
(quoted diff) +0 -0
b/arch/arm/boot/compressed/head.S
index af11c2f8f3b7..536b7781024a 100644
arch/arm/boot/compressed/head.S +4 -3
@@ -764,7 +764,6 @@ __armv7_mmu_cache_on: mov r0, #0 mcr p15, 0, r0, c7, c10, 4 @ drain write buffer tst r11, #0xf @ VMSA - mcrne p15, 0, r0, c8, c7, 0 @ flush I,D TLBs #endif mrc p15, 0, r0, c1, c0, 0 @ read control reg bic r0, r0, #1 << 28 @ clear SCTLR.TRE
@@ -783,8 +782,11 @@ __armv7_mmu_cache_on: mcrne p15, 0, r3, c2, c0, 0 @ load page table pointer mcrne p15, 0, r1, c3, c0, 0 @ load domain access control mcrne p15, 0, r6, c2, c0, 2 @ load ttb control -#endif mcr p15, 0, r0, c7, c5, 4 @ ISB + mcrne p15, 0, r0, c8, c7, 0 @ flush I,D TLBs +#else + mcr p15, 0, r0, c7, c5, 4 @ ISB +#endif mcr p15, 0, r0, c1, c0, 0 @ load control register mrc p15, 0, r0, c1, c0, 0 @ and read it back ---8<---
Robin.
Re: [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
Quoted 29 lines · Robin Murphy
Quoted 22 lines · Srinivas Ramana
If the bootloader uses the long descriptor format and jumps to kernel decompressor code, TTBCR may not be in a right state. Before enabling the MMU, it is required to clear the TTBCR.PD0 field to use TTBR0 for translation table walks.
The 'commit
dbece45894d3a("ARM: 7501/1: decompressor:
reset ttbcr for VMSA ARMv7 cores")' does the reset of TTBCR.N, but doesn't consider all the bits for the size of TTBCR.N.Clear TTBCR.PD0 field and reset all the three bits of TTBCR.N to indicate the use of TTBR0 and the correct base address width.
Signed-off-by Srinivas Ramana <[email protected]>--- arch/arm/boot/compressed/head.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm/boot/compressed/head.S +0
-0arch/arm/boot/compressed/head.S __armv7_mmu_cache_on: · line 781
Quoted 3 lines
arch/arm/boot/compressed/head.S +0
-0@@ -779,7 +779,7 @@ __armv7_mmu_cache_on: orrne r0, r0, #1 @ MMU enabled movne r1, #0xfffffffd @ domain 0 = client bic r6, r6, #1 << 31 @ 32-bit translation systemHmm, if TTBCR.EAE _was_ actually set...
Quoted 12 lines
arch/arm/boot/compressed/head.S __armv7_mmu_cache_on: · line 784
Quoted 5 lines
arch/arm/boot/compressed/head.S +1
-1- bic r6, r6, #3 << 0 @ use only ttbr0 + bic r6, r6, #(7 << 0) | (1 << 4) @ use only ttbr0 mcrne p15, 0, r3, c2, c0, 0 @ load page table pointer mcrne p15, 0, r1, c3, c0, 0 @ load domain access control mcrne p15, 0, r6, c2, c0, 2 @ load ttb control...then strictly the TLBIALL needs to happen after the ISB following this update. Otherwise per B3.10.2 of
DDI406C.cI think we might be into unpredictable territory - i.e. if the TLB happens to treat long- and short-descriptor entries differently then the TLBI beforehand (with EAE set) may be at liberty to only discard long-descriptor entries and leave bogus short-descriptor entries sitting around.
Yes, it seems this has to be taken care of, along with resetting TTBCR.PD0 and TTBCR.N. Do you say that this needs to be done in the same patch or a different one?
Quoted 10 lines
In other words, something like (completely untested):
---8<---
arch/arm/boot/compressed/head.S +0
-0
arch/arm/boot/compressed/head.S __armv7_mmu_cache_on: · line 767
Quoted 4 lines
arch/arm/boot/compressed/head.S +0
-1@@ -764,7 +764,6 @@ __armv7_mmu_cache_on: mov r0, #0 mcr p15, 0, r0, c7, c10, 4 @ drain write buffer tst r11, #0xf @ VMSA - mcrne p15, 0, r0, c8, c7, 0 @ flush I,D TLBs
Shouldn't this be still there for the same reason you explained above? I mean to discard the long descriptor entries when EAE was 1 (before we reset it).
Quoted 17 lines
arch/arm/boot/compressed/head.S +4
-2#endif mrc p15, 0, r0, c1, c0, 0 @ read control reg bic r0, r0, #1 << 28 @ clear SCTLR.TRE@@ -783,8 +782,11 @@ __armv7_mmu_cache_on: mcrne p15, 0, r3, c2, c0, 0 @ load page table pointer mcrne p15, 0, r1, c3, c0, 0 @ load domain access control mcrne p15, 0, r6, c2, c0, 2 @ load ttb control -#endif mcr p15, 0, r0, c7, c5, 4 @ ISB + mcrne p15, 0, r0, c8, c7, 0 @ flush I,D TLBs +#else + mcr p15, 0, r0, c7, c5, 4 @ ISB +#endif mcr p15, 0, r0, c1, c0, 0 @ load control register mrc p15, 0, r0, c1, c0, 0 @ and read it back ---8<---
Quoted 2 lines
Robin.
i have tested this change (flush I, D, TLBs after TTB control is written) and don't see any issue. But on my setup decompression is successful even without this (probably not hitting the case in discussion).
Thanks,
-- Srinivas R
Signature · 3 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
Hi Robin,
Quoted 90 lines · Srinivas Ramana
Quoted 36 lines · Robin Murphy
Quoted 23 lines · Srinivas Ramana
If the bootloader uses the long descriptor format and jumps to kernel decompressor code, TTBCR may not be in a right state. Before enabling the MMU, it is required to clear the TTBCR.PD0 field to use TTBR0 for translation table walks.
The 'commit
dbece45894d3a("ARM: 7501/1: decompressor:
reset ttbcr for VMSA ARMv7 cores")' does the reset of TTBCR.N, but doesn't consider all the bits for the size of TTBCR.N.Clear TTBCR.PD0 field and reset all the three bits of TTBCR.N to indicate the use of TTBR0 and the correct base address width.
Signed-off-by Srinivas Ramana <[email protected]>--- arch/arm/boot/compressed/head.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm/boot/compressed/head.S +0
-0arch/arm/boot/compressed/head.S __armv7_mmu_cache_on: · line 781
Quoted 3 lines
arch/arm/boot/compressed/head.S +0
-0@@ -779,7 +779,7 @@ __armv7_mmu_cache_on: orrne r0, r0, #1 @ MMU enabled movne r1, #0xfffffffd @ domain 0 = client bic r6, r6, #1 << 31 @ 32-bit translation systemHmm, if TTBCR.EAE _was_ actually set...
arch/arm/boot/compressed/head.S __armv7_mmu_cache_on: · line 784
Quoted 5 lines
arch/arm/boot/compressed/head.S +1
-1- bic r6, r6, #3 << 0 @ use only ttbr0 + bic r6, r6, #(7 << 0) | (1 << 4) @ use only ttbr0 mcrne p15, 0, r3, c2, c0, 0 @ load page table pointer mcrne p15, 0, r1, c3, c0, 0 @ load domain access control mcrne p15, 0, r6, c2, c0, 2 @ load ttb controlQuoted 6 lines
...then strictly the TLBIALL needs to happen after the ISB following this update. Otherwise per B3.10.2 of
DDI406C.cI think we might be into unpredictable territory - i.e. if the TLB happens to treat long- and short-descriptor entries differently then the TLBI beforehand (with EAE set) may be at liberty to only discard long-descriptor entries and leave bogus short-descriptor entries sitting around.Yes, it seems this has to be taken care of, along with resetting TTBCR.PD0 and TTBCR.N. Do you say that this needs to be done in the same patch or a different one?
Quoted 10 lines
In other words, something like (completely untested):
---8<---
arch/arm/boot/compressed/head.S +0
-0arch/arm/boot/compressed/head.S __armv7_mmu_cache_on: · line 767
Quoted 4 lines
arch/arm/boot/compressed/head.S +0
-1@@ -764,7 +764,6 @@ __armv7_mmu_cache_on: mov r0, #0 mcr p15, 0, r0, c7, c10, 4 @ drain write buffer tst r11, #0xf @ VMSA - mcrne p15, 0, r0, c8, c7, 0 @ flush I,D TLBsShouldn't this be still there for the same reason you explained above? I mean to discard the long descriptor entries when EAE was 1 (before we reset it).
Quoted 19 lines
arch/arm/boot/compressed/head.S +0
-0#endif mrc p15, 0, r0, c1, c0, 0 @ read control reg bic r0, r0, #1 << 28 @ clear SCTLR.TRE@@ -783,8 +782,11 @@ __armv7_mmu_cache_on: mcrne p15, 0, r3, c2, c0, 0 @ load page tablepointer
mcrne p15, 0, r1, c3, c0, 0 @ load domain access
controlmcrne p15, 0, r6, c2, c0, 2 @ load ttb controlarch/arm/boot/compressed/head.S +4
-2-#endif mcr p15, 0, r0, c7, c5, 4 @ ISB + mcrne p15, 0, r0, c8, c7, 0 @ flush I,D TLBs +#else + mcr p15, 0, r0, c7, c5, 4 @ ISB +#endif mcr p15, 0, r0, c1, c0, 0 @ load control register mrc p15, 0, r0, c1, c0, 0 @ and read it back ---8<---Quoted 2 lines
Robin.
i have tested this change (flush I, D, TLBs after TTB control is written) and don't see any issue. But on my setup decompression is successful even without this (probably not hitting the case in discussion).
Quoted 3 lines
Thanks,
-- Srinivas R
Would like your feedback on the above. Can we get the TTBCR fix merged first?(will send final patch with Russell Kings comments fixed)
For testing the TLB flush change we may have to check if we can create a failure case.
Thanks,
-- Srinivas R
Signature · 3 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Robin Murphy
Acked-byRe: [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
Quoted 1 line · Srinivas Ramana
Hi Robin,
Sorry! This one had slipped my mind already...
Quoted 97 lines · Srinivas Ramana
Quoted 92 lines · Robin Murphy
Quoted 37 lines · Srinivas Ramana
Quoted 23 lines
If the bootloader uses the long descriptor format and jumps to kernel decompressor code, TTBCR may not be in a right state. Before enabling the MMU, it is required to clear the TTBCR.PD0 field to use TTBR0 for translation table walks.
The 'commit
dbece45894d3a("ARM: 7501/1: decompressor:
reset ttbcr for VMSA ARMv7 cores")' does the reset of TTBCR.N, but doesn't consider all the bits for the size of TTBCR.N.Clear TTBCR.PD0 field and reset all the three bits of TTBCR.N to indicate the use of TTBR0 and the correct base address width.
Signed-off-by Srinivas Ramana <[email protected]>--- arch/arm/boot/compressed/head.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm/boot/compressed/head.S +0
-0arch/arm/boot/compressed/head.S __armv7_mmu_cache_on: · line 781
Quoted 3 lines
arch/arm/boot/compressed/head.S +0
-0@@ -779,7 +779,7 @@ __armv7_mmu_cache_on: orrne r0, r0, #1 @ MMU enabled movne r1, #0xfffffffd @ domain 0 = client bic r6, r6, #1 << 31 @ 32-bit translation systemHmm, if TTBCR.EAE _was_ actually set...
Quoted 6 lines
arch/arm/boot/compressed/head.S +1
-1- bic r6, r6, #3 << 0 @ use only ttbr0 + bic r6, r6, #(7 << 0) | (1 << 4) @ use only ttbr0 mcrne p15, 0, r3, c2, c0, 0 @ load page table pointer mcrne p15, 0, r1, c3, c0, 0 @ load domain accesscontrol
mcrne p15, 0, r6, c2, c0, 2 @ load ttb controlQuoted 6 lines
...then strictly the TLBIALL needs to happen after the ISB following this update. Otherwise per B3.10.2 of
DDI406C.cI think we might be into unpredictable territory - i.e. if the TLB happens to treat long- and short-descriptor entries differently then the TLBI beforehand (with EAE set) may be at liberty to only discard long-descriptor entries and leave bogus short-descriptor entries sitting around.Yes, it seems this has to be taken care of, along with resetting TTBCR.PD0 and TTBCR.N. Do you say that this needs to be done in the same patch or a different one?
Quoted 10 lines
In other words, something like (completely untested):
---8<---
arch/arm/boot/compressed/head.S +0
-0arch/arm/boot/compressed/head.S __armv7_mmu_cache_on: · line 767
Quoted 4 lines
arch/arm/boot/compressed/head.S +0
-1@@ -764,7 +764,6 @@ __armv7_mmu_cache_on: mov r0, #0 mcr p15, 0, r0, c7, c10, 4 @ drain write buffer tst r11, #0xf @ VMSA - mcrne p15, 0, r0, c8, c7, 0 @ flush I,D TLBsShouldn't this be still there for the same reason you explained above? I mean to discard the long descriptor entries when EAE was 1 (before we reset it).
Quoted 19 lines
arch/arm/boot/compressed/head.S +0
-0#endif mrc p15, 0, r0, c1, c0, 0 @ read control reg bic r0, r0, #1 << 28 @ clear SCTLR.TRE@@ -783,8 +782,11 @@ __armv7_mmu_cache_on: mcrne p15, 0, r3, c2, c0, 0 @ load page tablepointer
mcrne p15, 0, r1, c3, c0, 0 @ load domain access
controlmcrne p15, 0, r6, c2, c0, 2 @ load ttb controlarch/arm/boot/compressed/head.S +4
-2-#endif mcr p15, 0, r0, c7, c5, 4 @ ISB + mcrne p15, 0, r0, c8, c7, 0 @ flush I,D TLBs +#else + mcr p15, 0, r0, c7, c5, 4 @ ISB +#endif mcr p15, 0, r0, c1, c0, 0 @ load control register mrc p15, 0, r0, c1, c0, 0 @ and read it back ---8<---Quoted 2 lines
Robin.
i have tested this change (flush I, D, TLBs after TTB control is written) and don't see any issue. But on my setup decompression is successful even without this (probably not hitting the case in discussion).
Quoted 3 lines
Thanks,
-- Srinivas R
Quoted 5 lines
Would like your feedback on the above. Can we get the TTBCR fix merged first?(will send final patch with Russell Kings comments fixed)
For testing the TLB flush change we may have to check if we can create a failure case.
Yeah, the TLBI being in the wrong place is a separate, pre-existing problem; as far as this patch goes, it does what it claims to do, and matches what the ARMv7 (and ARMv6) docs say, so:
Quoted 4 lines
Thanks,
-- Srinivas R
PATCH 1/1 ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
If the bootloader uses the long descriptor format and jumps to kernel decompressor code, TTBCR may not be in a right state. Before enabling the MMU, it is required to clear the TTBCR.PD0 field to use TTBR0 for translation table walks.
The commit dbece45894d3a ("ARM: 7501/1: decompressor:
reset ttbcr for VMSA ARMv7 cores") does the reset of TTBCR.N, but doesn't consider all the bits for the size of TTBCR.N.
Clear TTBCR.PD0 field and reset all the three bits of TTBCR.N to indicate the use of TTBR0 and the correct base address width.
--- arch/arm/boot/compressed/head.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/compressed/head.S +1 -1
@@ -779,7 +779,7 @@ __armv7_mmu_cache_on: orrne r0, r0, #1 @ MMU enabled movne r1, #0xfffffffd @ domain 0 = client bic r6, r6, #1 << 31 @ 32-bit translation system - bic r6, r6, #3 << 0 @ use only ttbr0 + bic r6, r6, #(7 << 0) | (1 << 4) @ use only ttbr0 mcrne p15, 0, r3, c2, c0, 0 @ load page table pointer mcrne p15, 0, r1, c3, c0, 0 @ load domain access control mcrne p15, 0, r6, c2, c0, 2 @ load ttb control
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Russell King - ARM Linux
maintainerRe: [PATCH v1] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
Quoted 9 lines · Srinivas Ramana
If the bootloader uses the long descriptor format and jumps to kernel decompressor code, TTBCR may not be in a right state. Before enabling the MMU, it is required to clear the TTBCR.PD0 field to use TTBR0 for translation table walks.
The commit
dbece45894d3a("ARM: 7501/1: decompressor:
reset ttbcr for VMSA ARMv7 cores") does the reset of TTBCR.N, but doesn't consider all the bits for the size of TTBCR.N.
Quoted 6 lines
Clear TTBCR.PD0 field and reset all the three bits of TTBCR.N to indicate the use of TTBR0 and the correct base address width.
Fixes dbece45894d3 ("ARM: 7501/1: decompressor: reset ttbcr for VMSA ARMv7 cores")Acked-by Robin Murphy <[email protected]>Signed-off-by Srinivas Ramana <[email protected]>
Please submit to the patch system, thanks.
Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.