All of lore.kernel.org
 help / color / mirror / Atom feed
* Improper TTBCR for arm 32bit kernel decompression
@ 2016-09-09 16:34 ` Srinivas Ramana
  0 siblings, 0 replies; 24+ messages in thread
From: Srinivas Ramana @ 2016-09-09 16:34 UTC (permalink / raw)
  To: linux, will.deacon, nicolas.pitre
  Cc: linux-arm-msm, linux-arm-kernel, linux-kernel

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<----------------------------------
diff --git a/arch/arm/boot/compressed/head.S 
b/arch/arm/boot/compressed/head.S
index af11c2f..5769f1f 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -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

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative 
Project.

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Improper TTBCR for arm 32bit kernel decompression
@ 2016-09-09 16:34 ` Srinivas Ramana
  0 siblings, 0 replies; 24+ messages in thread
From: Srinivas Ramana @ 2016-09-09 16:34 UTC (permalink / raw)
  To: linux-arm-kernel

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<----------------------------------
diff --git a/arch/arm/boot/compressed/head.S 
b/arch/arm/boot/compressed/head.S
index af11c2f..5769f1f 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -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

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative 
Project.

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: Improper TTBCR for arm 32bit kernel decompression
  2016-09-09 16:34 ` Srinivas Ramana
@ 2016-09-09 17:36   ` Nicolas Pitre
  -1 siblings, 0 replies; 24+ messages in thread
From: Nicolas Pitre @ 2016-09-09 17:36 UTC (permalink / raw)
  To: Srinivas Ramana
  Cc: linux, will.deacon, linux-arm-msm, linux-arm-kernel, linux-kernel

On Fri, 9 Sep 2016, Srinivas Ramana wrote:

> 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<----------------------------------
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index af11c2f..5769f1f 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Improper TTBCR for arm 32bit kernel decompression
@ 2016-09-09 17:36   ` Nicolas Pitre
  0 siblings, 0 replies; 24+ messages in thread
From: Nicolas Pitre @ 2016-09-09 17:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 9 Sep 2016, Srinivas Ramana wrote:

> 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<----------------------------------
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index af11c2f..5769f1f 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Improper TTBCR for arm 32bit kernel decompression
  2016-09-09 17:36   ` Nicolas Pitre
@ 2016-09-10  5:50     ` Srinivas Ramana
  -1 siblings, 0 replies; 24+ messages in thread
From: Srinivas Ramana @ 2016-09-10  5:50 UTC (permalink / raw)
  To: Ramana, Srinivas; +Cc: linux-arm-kernel, linux-kernel

On 09/09/2016 11:06 PM, Nicolas Pitre wrote:
> On Fri, 9 Sep 2016, Srinivas Ramana wrote:
>
>> 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<----------------------------------
>> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
>> index af11c2f..5769f1f 100644
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -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
Sure, Thanks for the suggestion. I can incorporate this and submit a 
patch. Can i use your Acked-by?

Thanks,
-- Srinivas R

>
> Nicolas
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative 
Project.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Improper TTBCR for arm 32bit kernel decompression
@ 2016-09-10  5:50     ` Srinivas Ramana
  0 siblings, 0 replies; 24+ messages in thread
From: Srinivas Ramana @ 2016-09-10  5:50 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/09/2016 11:06 PM, Nicolas Pitre wrote:
> On Fri, 9 Sep 2016, Srinivas Ramana wrote:
>
>> 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<----------------------------------
>> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
>> index af11c2f..5769f1f 100644
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -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
Sure, Thanks for the suggestion. I can incorporate this and submit a 
patch. Can i use your Acked-by?

Thanks,
-- Srinivas R

>
> 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
>


-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative 
Project.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Improper TTBCR for arm 32bit kernel decompression
  2016-09-09 17:36   ` Nicolas Pitre
@ 2016-09-10  8:12     ` Srinivas Ramana
  -1 siblings, 0 replies; 24+ messages in thread
From: Srinivas Ramana @ 2016-09-10  8:12 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: linux, will.deacon, linux-arm-msm, linux-arm-kernel, linux-kernel

On 09/09/2016 11:06 PM, Nicolas Pitre wrote:
> On Fri, 9 Sep 2016, Srinivas Ramana wrote:
>
>> 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<----------------------------------
>> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
>> index af11c2f..5769f1f 100644
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -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

Sure, I can do that. Thanks for the suggestion. Will send out a patch 
for review.

Thanks,
-- Srinivas R

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative 
Project.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Improper TTBCR for arm 32bit kernel decompression
@ 2016-09-10  8:12     ` Srinivas Ramana
  0 siblings, 0 replies; 24+ messages in thread
From: Srinivas Ramana @ 2016-09-10  8:12 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/09/2016 11:06 PM, Nicolas Pitre wrote:
> On Fri, 9 Sep 2016, Srinivas Ramana wrote:
>
>> 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<----------------------------------
>> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
>> index af11c2f..5769f1f 100644
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -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

Sure, I can do that. Thanks for the suggestion. Will send out a patch 
for review.

Thanks,
-- Srinivas R

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative 
Project.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
  2016-09-10  8:12     ` Srinivas Ramana
@ 2016-09-12  6:57       ` Srinivas Ramana
  -1 siblings, 0 replies; 24+ messages in thread
From: Srinivas Ramana @ 2016-09-12  6:57 UTC (permalink / raw)
  To: linux, nicolas.pitre, will.deacon
  Cc: linux-arm-kernel, linux-kernel, linux-arm-msm, 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 <sramana@codeaurora.org>
---
 arch/arm/boot/compressed/head.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index af11c2f8f3b7..fc6d541549a2 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -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
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., 
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
@ 2016-09-12  6:57       ` Srinivas Ramana
  0 siblings, 0 replies; 24+ messages in thread
From: Srinivas Ramana @ 2016-09-12  6:57 UTC (permalink / raw)
  To: linux-arm-kernel

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 <sramana@codeaurora.org>
---
 arch/arm/boot/compressed/head.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index af11c2f8f3b7..fc6d541549a2 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -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
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., 
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
  2016-09-12  6:57       ` Srinivas Ramana
@ 2016-09-12 10:23         ` Russell King - ARM Linux
  -1 siblings, 0 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2016-09-12 10:23 UTC (permalink / raw)
  To: Srinivas Ramana
  Cc: nicolas.pitre, will.deacon, linux-arm-kernel, linux-kernel,
	linux-arm-msm

On Mon, Sep 12, 2016 at 12:27:00PM +0530, Srinivas Ramana wrote:
> 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.

> 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.

> Signed-off-by: Srinivas Ramana <sramana@codeaurora.org>
> ---
>  arch/arm/boot/compressed/head.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index af11c2f8f3b7..fc6d541549a2 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -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.

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
@ 2016-09-12 10:23         ` Russell King - ARM Linux
  0 siblings, 0 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2016-09-12 10:23 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 12, 2016 at 12:27:00PM +0530, Srinivas Ramana wrote:
> 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.

> 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.

> Signed-off-by: Srinivas Ramana <sramana@codeaurora.org>
> ---
>  arch/arm/boot/compressed/head.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index af11c2f8f3b7..fc6d541549a2 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -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.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently@9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
  2016-09-12  6:57       ` Srinivas Ramana
@ 2016-09-12 17:51         ` Robin Murphy
  -1 siblings, 0 replies; 24+ messages in thread
From: Robin Murphy @ 2016-09-12 17:51 UTC (permalink / raw)
  To: Srinivas Ramana, linux, nicolas.pitre, will.deacon
  Cc: linux-arm-msm, linux-kernel, linux-arm-kernel

On 12/09/16 07:57, Srinivas Ramana wrote:
> 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 <sramana@codeaurora.org>
> ---
>  arch/arm/boot/compressed/head.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index af11c2f8f3b7..fc6d541549a2 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -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...

> -		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<---
diff --git a/arch/arm/boot/compressed/head.S
b/arch/arm/boot/compressed/head.S
index af11c2f8f3b7..536b7781024a 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -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.

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
@ 2016-09-12 17:51         ` Robin Murphy
  0 siblings, 0 replies; 24+ messages in thread
From: Robin Murphy @ 2016-09-12 17:51 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/09/16 07:57, Srinivas Ramana wrote:
> 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 <sramana@codeaurora.org>
> ---
>  arch/arm/boot/compressed/head.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index af11c2f8f3b7..fc6d541549a2 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -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...

> -		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<---
diff --git a/arch/arm/boot/compressed/head.S
b/arch/arm/boot/compressed/head.S
index af11c2f8f3b7..536b7781024a 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -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.

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
  2016-09-12 17:51         ` Robin Murphy
@ 2016-09-13 14:52           ` Srinivas Ramana
  -1 siblings, 0 replies; 24+ messages in thread
From: Srinivas Ramana @ 2016-09-13 14:52 UTC (permalink / raw)
  To: Robin Murphy
  Cc: linux, nicolas.pitre, will.deacon, linux-arm-msm, linux-kernel,
	linux-arm-kernel

On 09/12/2016 11:21 PM, Robin Murphy wrote:
> On 12/09/16 07:57, Srinivas Ramana wrote:
>> 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 <sramana@codeaurora.org>
>> ---
>>   arch/arm/boot/compressed/head.S | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
>> index af11c2f8f3b7..fc6d541549a2 100644
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -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...
>
>> -		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.
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?
>
> In other words, something like (completely untested):
>
> ---8<---
> diff --git a/arch/arm/boot/compressed/head.S
> b/arch/arm/boot/compressed/head.S
> index af11c2f8f3b7..536b7781024a 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -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).
>   #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.
>
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

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative 
Project.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
@ 2016-09-13 14:52           ` Srinivas Ramana
  0 siblings, 0 replies; 24+ messages in thread
From: Srinivas Ramana @ 2016-09-13 14:52 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/12/2016 11:21 PM, Robin Murphy wrote:
> On 12/09/16 07:57, Srinivas Ramana wrote:
>> 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 <sramana@codeaurora.org>
>> ---
>>   arch/arm/boot/compressed/head.S | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
>> index af11c2f8f3b7..fc6d541549a2 100644
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -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...
>
>> -		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.
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?
>
> In other words, something like (completely untested):
>
> ---8<---
> diff --git a/arch/arm/boot/compressed/head.S
> b/arch/arm/boot/compressed/head.S
> index af11c2f8f3b7..536b7781024a 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -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).
>   #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.
>
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

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative 
Project.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
  2016-09-13 14:52           ` Srinivas Ramana
@ 2016-09-27 12:16             ` Srinivas Ramana
  -1 siblings, 0 replies; 24+ messages in thread
From: Srinivas Ramana @ 2016-09-27 12:16 UTC (permalink / raw)
  To: Robin Murphy
  Cc: linux, nicolas.pitre, will.deacon, linux-arm-msm, linux-kernel,
	linux-arm-kernel

Hi Robin,

On 09/13/2016 08:22 PM, Srinivas Ramana wrote:
> On 09/12/2016 11:21 PM, Robin Murphy wrote:
>> On 12/09/16 07:57, Srinivas Ramana wrote:
>>> 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 <sramana@codeaurora.org>
>>> ---
>>>   arch/arm/boot/compressed/head.S | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/compressed/head.S
>>> b/arch/arm/boot/compressed/head.S
>>> index af11c2f8f3b7..fc6d541549a2 100644
>>> --- a/arch/arm/boot/compressed/head.S
>>> +++ b/arch/arm/boot/compressed/head.S
>>> @@ -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...
>>
>>> -        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.
> 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?
>>
>> In other words, something like (completely untested):
>>
>> ---8<---
>> diff --git a/arch/arm/boot/compressed/head.S
>> b/arch/arm/boot/compressed/head.S
>> index af11c2f8f3b7..536b7781024a 100644
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -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).
>>   #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.
>>
> 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
>

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

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative 
Project.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
@ 2016-09-27 12:16             ` Srinivas Ramana
  0 siblings, 0 replies; 24+ messages in thread
From: Srinivas Ramana @ 2016-09-27 12:16 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Robin,

On 09/13/2016 08:22 PM, Srinivas Ramana wrote:
> On 09/12/2016 11:21 PM, Robin Murphy wrote:
>> On 12/09/16 07:57, Srinivas Ramana wrote:
>>> 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 <sramana@codeaurora.org>
>>> ---
>>>   arch/arm/boot/compressed/head.S | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/compressed/head.S
>>> b/arch/arm/boot/compressed/head.S
>>> index af11c2f8f3b7..fc6d541549a2 100644
>>> --- a/arch/arm/boot/compressed/head.S
>>> +++ b/arch/arm/boot/compressed/head.S
>>> @@ -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...
>>
>>> -        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.
> 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?
>>
>> In other words, something like (completely untested):
>>
>> ---8<---
>> diff --git a/arch/arm/boot/compressed/head.S
>> b/arch/arm/boot/compressed/head.S
>> index af11c2f8f3b7..536b7781024a 100644
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -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).
>>   #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.
>>
> 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
>

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

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative 
Project.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
  2016-09-27 12:16             ` Srinivas Ramana
@ 2016-09-27 12:54               ` Robin Murphy
  -1 siblings, 0 replies; 24+ messages in thread
From: Robin Murphy @ 2016-09-27 12:54 UTC (permalink / raw)
  To: Srinivas Ramana
  Cc: nicolas.pitre, linux-arm-msm, will.deacon, linux, linux-kernel,
	linux-arm-kernel

On 27/09/16 13:16, Srinivas Ramana wrote:
> Hi Robin,

Sorry! This one had slipped my mind already...

> On 09/13/2016 08:22 PM, Srinivas Ramana wrote:
>> On 09/12/2016 11:21 PM, Robin Murphy wrote:
>>> On 12/09/16 07:57, Srinivas Ramana wrote:
>>>> 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 <sramana@codeaurora.org>
>>>> ---
>>>>   arch/arm/boot/compressed/head.S | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/boot/compressed/head.S
>>>> b/arch/arm/boot/compressed/head.S
>>>> index af11c2f8f3b7..fc6d541549a2 100644
>>>> --- a/arch/arm/boot/compressed/head.S
>>>> +++ b/arch/arm/boot/compressed/head.S
>>>> @@ -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...
>>>
>>>> -        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.
>> 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?
>>>
>>> In other words, something like (completely untested):
>>>
>>> ---8<---
>>> diff --git a/arch/arm/boot/compressed/head.S
>>> b/arch/arm/boot/compressed/head.S
>>> index af11c2f8f3b7..536b7781024a 100644
>>> --- a/arch/arm/boot/compressed/head.S
>>> +++ b/arch/arm/boot/compressed/head.S
>>> @@ -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).
>>>   #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.
>>>
>> 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
>>
> 
> 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:

Acked-by: Robin Murphy <robin.murphy@arm.com>

> 
> Thanks,
> -- Srinivas R
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
@ 2016-09-27 12:54               ` Robin Murphy
  0 siblings, 0 replies; 24+ messages in thread
From: Robin Murphy @ 2016-09-27 12:54 UTC (permalink / raw)
  To: linux-arm-kernel

On 27/09/16 13:16, Srinivas Ramana wrote:
> Hi Robin,

Sorry! This one had slipped my mind already...

> On 09/13/2016 08:22 PM, Srinivas Ramana wrote:
>> On 09/12/2016 11:21 PM, Robin Murphy wrote:
>>> On 12/09/16 07:57, Srinivas Ramana wrote:
>>>> 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 <sramana@codeaurora.org>
>>>> ---
>>>>   arch/arm/boot/compressed/head.S | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/boot/compressed/head.S
>>>> b/arch/arm/boot/compressed/head.S
>>>> index af11c2f8f3b7..fc6d541549a2 100644
>>>> --- a/arch/arm/boot/compressed/head.S
>>>> +++ b/arch/arm/boot/compressed/head.S
>>>> @@ -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...
>>>
>>>> -        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.
>> 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?
>>>
>>> In other words, something like (completely untested):
>>>
>>> ---8<---
>>> diff --git a/arch/arm/boot/compressed/head.S
>>> b/arch/arm/boot/compressed/head.S
>>> index af11c2f8f3b7..536b7781024a 100644
>>> --- a/arch/arm/boot/compressed/head.S
>>> +++ b/arch/arm/boot/compressed/head.S
>>> @@ -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).
>>>   #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.
>>>
>> 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
>>
> 
> 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:

Acked-by: Robin Murphy <robin.murphy@arm.com>

> 
> Thanks,
> -- Srinivas R
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
  2016-09-27 12:54               ` Robin Murphy
@ 2016-09-28 12:45                 ` Srinivas Ramana
  -1 siblings, 0 replies; 24+ messages in thread
From: Srinivas Ramana @ 2016-09-28 12:45 UTC (permalink / raw)
  To: linux, nicolas.pitre, will.deacon, robin.murphy
  Cc: linux-arm-kernel, linux-kernel, linux-arm-msm, 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.

Fixes: dbece45894d3 ("ARM: 7501/1: decompressor: reset ttbcr for VMSA ARMv7 cores")
Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Srinivas Ramana <sramana@codeaurora.org>
---
 arch/arm/boot/compressed/head.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index af11c2f8f3b7..fc6d541549a2 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -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
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., 
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v1] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
@ 2016-09-28 12:45                 ` Srinivas Ramana
  0 siblings, 0 replies; 24+ messages in thread
From: Srinivas Ramana @ 2016-09-28 12:45 UTC (permalink / raw)
  To: linux-arm-kernel

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.

Fixes: dbece45894d3 ("ARM: 7501/1: decompressor: reset ttbcr for VMSA ARMv7 cores")
Acked-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Srinivas Ramana <sramana@codeaurora.org>
---
 arch/arm/boot/compressed/head.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index af11c2f8f3b7..fc6d541549a2 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -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
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., 
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v1] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
  2016-09-28 12:45                 ` Srinivas Ramana
@ 2016-09-28 22:00                   ` Russell King - ARM Linux
  -1 siblings, 0 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2016-09-28 22:00 UTC (permalink / raw)
  To: Srinivas Ramana
  Cc: nicolas.pitre, will.deacon, robin.murphy, linux-arm-kernel,
	linux-kernel, linux-arm-msm

On Wed, Sep 28, 2016 at 06:15:28PM +0530, Srinivas Ramana wrote:
> 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.
> 
> Fixes: dbece45894d3 ("ARM: 7501/1: decompressor: reset ttbcr for VMSA ARMv7 cores")
> Acked-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Srinivas Ramana <sramana@codeaurora.org>

Please submit to the patch system, thanks.

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7
@ 2016-09-28 22:00                   ` Russell King - ARM Linux
  0 siblings, 0 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2016-09-28 22:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 28, 2016 at 06:15:28PM +0530, Srinivas Ramana wrote:
> 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.
> 
> Fixes: dbece45894d3 ("ARM: 7501/1: decompressor: reset ttbcr for VMSA ARMv7 cores")
> Acked-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Srinivas Ramana <sramana@codeaurora.org>

Please submit to the patch system, thanks.

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2016-09-28 22:00 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-09 16:34 Improper TTBCR for arm 32bit kernel decompression Srinivas Ramana
2016-09-09 16:34 ` Srinivas Ramana
2016-09-09 17:36 ` Nicolas Pitre
2016-09-09 17:36   ` Nicolas Pitre
2016-09-10  5:50   ` Srinivas Ramana
2016-09-10  5:50     ` Srinivas Ramana
2016-09-10  8:12   ` Srinivas Ramana
2016-09-10  8:12     ` Srinivas Ramana
2016-09-12  6:57     ` [PATCH] ARM: decompressor: reset ttbcr fields to use TTBR0 on ARMv7 Srinivas Ramana
2016-09-12  6:57       ` Srinivas Ramana
2016-09-12 10:23       ` Russell King - ARM Linux
2016-09-12 10:23         ` Russell King - ARM Linux
2016-09-12 17:51       ` Robin Murphy
2016-09-12 17:51         ` Robin Murphy
2016-09-13 14:52         ` Srinivas Ramana
2016-09-13 14:52           ` Srinivas Ramana
2016-09-27 12:16           ` Srinivas Ramana
2016-09-27 12:16             ` Srinivas Ramana
2016-09-27 12:54             ` Robin Murphy
2016-09-27 12:54               ` Robin Murphy
2016-09-28 12:45               ` [PATCH v1] " Srinivas Ramana
2016-09-28 12:45                 ` Srinivas Ramana
2016-09-28 22:00                 ` Russell King - ARM Linux
2016-09-28 22:00                   ` Russell King - ARM Linux

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.