[PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Konstantin Khlebnikov]

This patch fixes booting when idmap pgd lays above 4gb. Commit
4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.

Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
carry flag must be added to the upper part.

Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
Cc: Cyril Chemparathy <cyril@ti.com>
Cc: Vitaly Andrianov <vitalya@ti.com>
---
 arch/arm/mm/proc-v7-3level.S |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
index 22e3ad6..f0481dd 100644
--- a/arch/arm/mm/proc-v7-3level.S
+++ b/arch/arm/mm/proc-v7-3level.S
@@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
 	mov	\tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
 	mov	\ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT		@ lower bits
 	addls	\ttbr1, \ttbr1, #TTBR1_OFFSET
-	mcrr	p15, 1, \ttbr1, \zero, c2			@ load TTBR1
+	adcls	\tmp, \tmp, #0
+	mcrr	p15, 1, \ttbr1, \tmp, c2			@ load TTBR1
 	mov	\tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
 	mov	\ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT		@ lower bits
-	mcrr	p15, 0, \ttbr0, \zero, c2			@ load TTBR0
-	mcrr	p15, 1, \ttbr1, \zero, c2			@ load TTBR1
-	mcrr	p15, 0, \ttbr0, \zero, c2			@ load TTBR0
+	mcrr	p15, 0, \ttbr0, \tmp, c2			@ load TTBR0
 	.endm
 
 	/*

[PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Konstantin Khlebnikov]

This patch fixes booting when idmap pgd lays above 4gb. Commit
4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.

Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
carry flag must be added to the upper part.

Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
Cc: Cyril Chemparathy <cyril@ti.com>
Cc: Vitaly Andrianov <vitalya@ti.com>
---
 arch/arm/mm/proc-v7-3level.S |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
index 22e3ad6..f0481dd 100644
--- a/arch/arm/mm/proc-v7-3level.S
+++ b/arch/arm/mm/proc-v7-3level.S
@@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
 	mov	\tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
 	mov	\ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT		@ lower bits
 	addls	\ttbr1, \ttbr1, #TTBR1_OFFSET
-	mcrr	p15, 1, \ttbr1, \zero, c2			@ load TTBR1
+	adcls	\tmp, \tmp, #0
+	mcrr	p15, 1, \ttbr1, \tmp, c2			@ load TTBR1
 	mov	\tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
 	mov	\ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT		@ lower bits
-	mcrr	p15, 0, \ttbr0, \zero, c2			@ load TTBR0
-	mcrr	p15, 1, \ttbr1, \zero, c2			@ load TTBR1
-	mcrr	p15, 0, \ttbr0, \zero, c2			@ load TTBR0
+	mcrr	p15, 0, \ttbr0, \tmp, c2			@ load TTBR0
 	.endm
 
 	/*

[PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Konstantin Khlebnikov]

idmap layout combines both phisical and virtual addresses.
Everything works fine if ram physically lays below PAGE_OFFSET.
Otherwise idmap starts punching huge holes in virtual memory layout.
It maps ram by 2MiB sections, but when it allocates new pmd page it
cuts 1GiB at once.

This patch makes a copy of all affected pmds from init_mm.
Only few (usually one) 2MiB sections will be lost.
This is not eliminates problem but makes it 512 times less likely.

Usually idmap is used only for a short period. For examle for booting
secondary cpu __turn_mmu_on (executed in physical addresses) jumps to
virtual address of __secondary_switched which calls secondary_start_kernel
which in turn calls cpu_switch_mm and switches to normal pgd from init_mm.
So everything will be fine if these functions aren't intersect with idmap.

Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
---
 arch/arm/mm/idmap.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mm/idmap.c b/arch/arm/mm/idmap.c
index 8e0e52e..dcd023c 100644
--- a/arch/arm/mm/idmap.c
+++ b/arch/arm/mm/idmap.c
@@ -25,6 +25,9 @@ static void idmap_add_pmd(pud_t *pud, unsigned long addr, unsigned long end,
 			pr_warning("Failed to allocate identity pmd.\n");
 			return;
 		}
+		if (!pud_none(*pud))
+			memcpy(pmd, pmd_offset(pud, 0),
+			       PTRS_PER_PMD * sizeof(pmd_t));
 		pud_populate(&init_mm, pud, pmd);
 		pmd += pmd_index(addr);
 	} else

[PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Konstantin Khlebnikov]

idmap layout combines both phisical and virtual addresses.
Everything works fine if ram physically lays below PAGE_OFFSET.
Otherwise idmap starts punching huge holes in virtual memory layout.
It maps ram by 2MiB sections, but when it allocates new pmd page it
cuts 1GiB at once.

This patch makes a copy of all affected pmds from init_mm.
Only few (usually one) 2MiB sections will be lost.
This is not eliminates problem but makes it 512 times less likely.

Usually idmap is used only for a short period. For examle for booting
secondary cpu __turn_mmu_on (executed in physical addresses) jumps to
virtual address of __secondary_switched which calls secondary_start_kernel
which in turn calls cpu_switch_mm and switches to normal pgd from init_mm.
So everything will be fine if these functions aren't intersect with idmap.

Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
---
 arch/arm/mm/idmap.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mm/idmap.c b/arch/arm/mm/idmap.c
index 8e0e52e..dcd023c 100644
--- a/arch/arm/mm/idmap.c
+++ b/arch/arm/mm/idmap.c
@@ -25,6 +25,9 @@ static void idmap_add_pmd(pud_t *pud, unsigned long addr, unsigned long end,
 			pr_warning("Failed to allocate identity pmd.\n");
 			return;
 		}
+		if (!pud_none(*pud))
+			memcpy(pmd, pmd_offset(pud, 0),
+			       PTRS_PER_PMD * sizeof(pmd_t));
 		pud_populate(&init_mm, pud, pmd);
 		pmd += pmd_index(addr);
 	} else

Re: [PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Will Deacon]

On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
> This patch fixes booting when idmap pgd lays above 4gb. Commit
> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
> 
> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
> carry flag must be added to the upper part.
> 
> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
> Cc: Cyril Chemparathy <cyril@ti.com>
> Cc: Vitaly Andrianov <vitalya@ti.com>
> ---
>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
> index 22e3ad6..f0481dd 100644
> --- a/arch/arm/mm/proc-v7-3level.S
> +++ b/arch/arm/mm/proc-v7-3level.S
> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>  	mov	\tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
>  	mov	\ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT		@ lower bits
>  	addls	\ttbr1, \ttbr1, #TTBR1_OFFSET
> -	mcrr	p15, 1, \ttbr1, \zero, c2			@ load TTBR1
> +	adcls	\tmp, \tmp, #0
> +	mcrr	p15, 1, \ttbr1, \tmp, c2			@ load TTBR1
>  	mov	\tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
>  	mov	\ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT		@ lower bits
> -	mcrr	p15, 0, \ttbr0, \zero, c2			@ load TTBR0
> -	mcrr	p15, 1, \ttbr1, \zero, c2			@ load TTBR1
> -	mcrr	p15, 0, \ttbr0, \zero, c2			@ load TTBR0
> +	mcrr	p15, 0, \ttbr0, \tmp, c2			@ load TTBR0

I must admit, the code you are removing here looks really strange. Was there
a badly resolved conflict somewhere along the way? It would be nice to see
if your fix (which seems ok to me) was actually present in the mailing list
posting of the patch that ended in the above mess.

Will

[PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Will Deacon]

On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
> This patch fixes booting when idmap pgd lays above 4gb. Commit
> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
> 
> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
> carry flag must be added to the upper part.
> 
> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
> Cc: Cyril Chemparathy <cyril@ti.com>
> Cc: Vitaly Andrianov <vitalya@ti.com>
> ---
>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
> index 22e3ad6..f0481dd 100644
> --- a/arch/arm/mm/proc-v7-3level.S
> +++ b/arch/arm/mm/proc-v7-3level.S
> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>  	mov	\tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
>  	mov	\ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT		@ lower bits
>  	addls	\ttbr1, \ttbr1, #TTBR1_OFFSET
> -	mcrr	p15, 1, \ttbr1, \zero, c2			@ load TTBR1
> +	adcls	\tmp, \tmp, #0
> +	mcrr	p15, 1, \ttbr1, \tmp, c2			@ load TTBR1
>  	mov	\tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
>  	mov	\ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT		@ lower bits
> -	mcrr	p15, 0, \ttbr0, \zero, c2			@ load TTBR0
> -	mcrr	p15, 1, \ttbr1, \zero, c2			@ load TTBR1
> -	mcrr	p15, 0, \ttbr0, \zero, c2			@ load TTBR0
> +	mcrr	p15, 0, \ttbr0, \tmp, c2			@ load TTBR0

I must admit, the code you are removing here looks really strange. Was there
a badly resolved conflict somewhere along the way? It would be nice to see
if your fix (which seems ok to me) was actually present in the mailing list
posting of the patch that ended in the above mess.

Will

Re: [PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Will Deacon]

On Tue, Jul 22, 2014 at 04:36:35PM +0100, Konstantin Khlebnikov wrote:
> idmap layout combines both phisical and virtual addresses.
> Everything works fine if ram physically lays below PAGE_OFFSET.
> Otherwise idmap starts punching huge holes in virtual memory layout.
> It maps ram by 2MiB sections, but when it allocates new pmd page it
> cuts 1GiB at once.
> 
> This patch makes a copy of all affected pmds from init_mm.
> Only few (usually one) 2MiB sections will be lost.
> This is not eliminates problem but makes it 512 times less likely.

I'm struggling to understand your commit message, but making a problem `512
times less likely' does sound like a bit of a hack to me. Can't we fix this
properly instead?

Will

[PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Will Deacon]

On Tue, Jul 22, 2014 at 04:36:35PM +0100, Konstantin Khlebnikov wrote:
> idmap layout combines both phisical and virtual addresses.
> Everything works fine if ram physically lays below PAGE_OFFSET.
> Otherwise idmap starts punching huge holes in virtual memory layout.
> It maps ram by 2MiB sections, but when it allocates new pmd page it
> cuts 1GiB at once.
> 
> This patch makes a copy of all affected pmds from init_mm.
> Only few (usually one) 2MiB sections will be lost.
> This is not eliminates problem but makes it 512 times less likely.

I'm struggling to understand your commit message, but making a problem `512
times less likely' does sound like a bit of a hack to me. Can't we fix this
properly instead?

Will

Re: [PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Konstantin Khlebnikov]

On Mon, Jul 28, 2014 at 10:14 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jul 22, 2014 at 04:36:35PM +0100, Konstantin Khlebnikov wrote:
>> idmap layout combines both phisical and virtual addresses.
>> Everything works fine if ram physically lays below PAGE_OFFSET.
>> Otherwise idmap starts punching huge holes in virtual memory layout.
>> It maps ram by 2MiB sections, but when it allocates new pmd page it
>> cuts 1GiB at once.
>>
>> This patch makes a copy of all affected pmds from init_mm.
>> Only few (usually one) 2MiB sections will be lost.
>> This is not eliminates problem but makes it 512 times less likely.
>
> I'm struggling to understand your commit message, but making a problem `512
> times less likely' does sound like a bit of a hack to me. Can't we fix this
> properly instead?

Yep, my comment sucks.

Usually idmap looks like this:

|0x00000000 -- <chunk of physical memory in identical mapping > --- |
TASK_SIZE -- <kernel space vm layoyt> --- 0xFFFFFFFF |

But when that physical memory chunk starts from 0xE8000000 or even
0xF2000000 evenything becomes very complicated.

>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Konstantin Khlebnikov]

On Mon, Jul 28, 2014 at 10:14 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jul 22, 2014 at 04:36:35PM +0100, Konstantin Khlebnikov wrote:
>> idmap layout combines both phisical and virtual addresses.
>> Everything works fine if ram physically lays below PAGE_OFFSET.
>> Otherwise idmap starts punching huge holes in virtual memory layout.
>> It maps ram by 2MiB sections, but when it allocates new pmd page it
>> cuts 1GiB at once.
>>
>> This patch makes a copy of all affected pmds from init_mm.
>> Only few (usually one) 2MiB sections will be lost.
>> This is not eliminates problem but makes it 512 times less likely.
>
> I'm struggling to understand your commit message, but making a problem `512
> times less likely' does sound like a bit of a hack to me. Can't we fix this
> properly instead?

Yep, my comment sucks.

Usually idmap looks like this:

|0x00000000 -- <chunk of physical memory in identical mapping > --- |
TASK_SIZE -- <kernel space vm layoyt> --- 0xFFFFFFFF |

But when that physical memory chunk starts from 0xE8000000 or even
0xF2000000 evenything becomes very complicated.

>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Konstantin Khlebnikov]

On Mon, Jul 28, 2014 at 10:12 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
>> This patch fixes booting when idmap pgd lays above 4gb. Commit
>> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
>>
>> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
>> carry flag must be added to the upper part.
>>
>> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
>> Cc: Cyril Chemparathy <cyril@ti.com>
>> Cc: Vitaly Andrianov <vitalya@ti.com>
>> ---
>>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>> index 22e3ad6..f0481dd 100644
>> --- a/arch/arm/mm/proc-v7-3level.S
>> +++ b/arch/arm/mm/proc-v7-3level.S
>> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
>> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>> +     adcls   \tmp, \tmp, #0
>> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
>
> I must admit, the code you are removing here looks really strange. Was there
> a badly resolved conflict somewhere along the way? It would be nice to see
> if your fix (which seems ok to me) was actually present in the mailing list
> posting of the patch that ended in the above mess.

Nope, no merge conflicts, source in original patch
https://lkml.org/lkml/2012/9/11/346

That mess completely harmless, this code is used only once on boot.
I don't have that email, so replying isn't trivial for me.

>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Konstantin Khlebnikov]

On Mon, Jul 28, 2014 at 10:12 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
>> This patch fixes booting when idmap pgd lays above 4gb. Commit
>> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
>>
>> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
>> carry flag must be added to the upper part.
>>
>> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
>> Cc: Cyril Chemparathy <cyril@ti.com>
>> Cc: Vitaly Andrianov <vitalya@ti.com>
>> ---
>>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>> index 22e3ad6..f0481dd 100644
>> --- a/arch/arm/mm/proc-v7-3level.S
>> +++ b/arch/arm/mm/proc-v7-3level.S
>> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
>> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>> +     adcls   \tmp, \tmp, #0
>> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
>
> I must admit, the code you are removing here looks really strange. Was there
> a badly resolved conflict somewhere along the way? It would be nice to see
> if your fix (which seems ok to me) was actually present in the mailing list
> posting of the patch that ended in the above mess.

Nope, no merge conflicts, source in original patch
https://lkml.org/lkml/2012/9/11/346

That mess completely harmless, this code is used only once on boot.
I don't have that email, so replying isn't trivial for me.

>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Will Deacon]

On Mon, Jul 28, 2014 at 07:25:14PM +0100, Konstantin Khlebnikov wrote:
> On Mon, Jul 28, 2014 at 10:14 PM, Will Deacon <will.deacon@arm.com> wrote:
> > On Tue, Jul 22, 2014 at 04:36:35PM +0100, Konstantin Khlebnikov wrote:
> >> idmap layout combines both phisical and virtual addresses.
> >> Everything works fine if ram physically lays below PAGE_OFFSET.
> >> Otherwise idmap starts punching huge holes in virtual memory layout.
> >> It maps ram by 2MiB sections, but when it allocates new pmd page it
> >> cuts 1GiB at once.
> >>
> >> This patch makes a copy of all affected pmds from init_mm.
> >> Only few (usually one) 2MiB sections will be lost.
> >> This is not eliminates problem but makes it 512 times less likely.
> >
> > I'm struggling to understand your commit message, but making a problem `512
> > times less likely' does sound like a bit of a hack to me. Can't we fix this
> > properly instead?
> 
> Yep, my comment sucks.
> 
> Usually idmap looks like this:
> 
> |0x00000000 -- <chunk of physical memory in identical mapping > --- |
> TASK_SIZE -- <kernel space vm layoyt> --- 0xFFFFFFFF |
> 
> But when that physical memory chunk starts from 0xE8000000 or even
> 0xF2000000 evenything becomes very complicated.

Why? As long as we don't clobber the kernel text (which would require
PHYS_OFFSET to be at a really weird alignment and very close to
PAGE_OFFSET), then you should be alright. Sure, you'll lose things like your
stack and the vmalloc area etc, but you're running in the idmap, so don't
use those things.

soft_restart is an example of code that deals with these issues. Which code
is causing you problems?

Will

[PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Will Deacon]

On Mon, Jul 28, 2014 at 07:25:14PM +0100, Konstantin Khlebnikov wrote:
> On Mon, Jul 28, 2014 at 10:14 PM, Will Deacon <will.deacon@arm.com> wrote:
> > On Tue, Jul 22, 2014 at 04:36:35PM +0100, Konstantin Khlebnikov wrote:
> >> idmap layout combines both phisical and virtual addresses.
> >> Everything works fine if ram physically lays below PAGE_OFFSET.
> >> Otherwise idmap starts punching huge holes in virtual memory layout.
> >> It maps ram by 2MiB sections, but when it allocates new pmd page it
> >> cuts 1GiB at once.
> >>
> >> This patch makes a copy of all affected pmds from init_mm.
> >> Only few (usually one) 2MiB sections will be lost.
> >> This is not eliminates problem but makes it 512 times less likely.
> >
> > I'm struggling to understand your commit message, but making a problem `512
> > times less likely' does sound like a bit of a hack to me. Can't we fix this
> > properly instead?
> 
> Yep, my comment sucks.
> 
> Usually idmap looks like this:
> 
> |0x00000000 -- <chunk of physical memory in identical mapping > --- |
> TASK_SIZE -- <kernel space vm layoyt> --- 0xFFFFFFFF |
> 
> But when that physical memory chunk starts from 0xE8000000 or even
> 0xF2000000 evenything becomes very complicated.

Why? As long as we don't clobber the kernel text (which would require
PHYS_OFFSET to be at a really weird alignment and very close to
PAGE_OFFSET), then you should be alright. Sure, you'll lose things like your
stack and the vmalloc area etc, but you're running in the idmap, so don't
use those things.

soft_restart is an example of code that deals with these issues. Which code
is causing you problems?

Will

Re: [PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Will Deacon]

On Mon, Jul 28, 2014 at 07:40:58PM +0100, Konstantin Khlebnikov wrote:
> On Mon, Jul 28, 2014 at 10:12 PM, Will Deacon <will.deacon@arm.com> wrote:
> > On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
> >> This patch fixes booting when idmap pgd lays above 4gb. Commit
> >> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
> >>
> >> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
> >> carry flag must be added to the upper part.
> >>
> >> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
> >> Cc: Cyril Chemparathy <cyril@ti.com>
> >> Cc: Vitaly Andrianov <vitalya@ti.com>
> >> ---
> >>  arch/arm/mm/proc-v7-3level.S |    7 +++----
> >>  1 file changed, 3 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
> >> index 22e3ad6..f0481dd 100644
> >> --- a/arch/arm/mm/proc-v7-3level.S
> >> +++ b/arch/arm/mm/proc-v7-3level.S
> >> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
> >>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
> >>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
> >>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
> >> +     adcls   \tmp, \tmp, #0
> >> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
> >>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
> >>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
> >> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
> >
> > I must admit, the code you are removing here looks really strange. Was there
> > a badly resolved conflict somewhere along the way? It would be nice to see
> > if your fix (which seems ok to me) was actually present in the mailing list
> > posting of the patch that ended in the above mess.
> 
> Nope, no merge conflicts, source in original patch
> https://lkml.org/lkml/2012/9/11/346
> 
> That mess completely harmless, this code is used only once on boot.
> I don't have that email, so replying isn't trivial for me.

How bizarre. Also, Cyril doesn't work for TI anymore (his email is
bouncing), so it's tricky to know what he meant here.

Your patch looks better than what we currently have though. Have you managed
to test it on a keystone platform (I don't have one)?

Will

[PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Will Deacon]

On Mon, Jul 28, 2014 at 07:40:58PM +0100, Konstantin Khlebnikov wrote:
> On Mon, Jul 28, 2014 at 10:12 PM, Will Deacon <will.deacon@arm.com> wrote:
> > On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
> >> This patch fixes booting when idmap pgd lays above 4gb. Commit
> >> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
> >>
> >> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
> >> carry flag must be added to the upper part.
> >>
> >> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
> >> Cc: Cyril Chemparathy <cyril@ti.com>
> >> Cc: Vitaly Andrianov <vitalya@ti.com>
> >> ---
> >>  arch/arm/mm/proc-v7-3level.S |    7 +++----
> >>  1 file changed, 3 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
> >> index 22e3ad6..f0481dd 100644
> >> --- a/arch/arm/mm/proc-v7-3level.S
> >> +++ b/arch/arm/mm/proc-v7-3level.S
> >> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
> >>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
> >>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
> >>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
> >> +     adcls   \tmp, \tmp, #0
> >> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
> >>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
> >>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
> >> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
> >
> > I must admit, the code you are removing here looks really strange. Was there
> > a badly resolved conflict somewhere along the way? It would be nice to see
> > if your fix (which seems ok to me) was actually present in the mailing list
> > posting of the patch that ended in the above mess.
> 
> Nope, no merge conflicts, source in original patch
> https://lkml.org/lkml/2012/9/11/346
> 
> That mess completely harmless, this code is used only once on boot.
> I don't have that email, so replying isn't trivial for me.

How bizarre. Also, Cyril doesn't work for TI anymore (his email is
bouncing), so it's tricky to know what he meant here.

Your patch looks better than what we currently have though. Have you managed
to test it on a keystone platform (I don't have one)?

Will

Re: [PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Konstantin Khlebnikov]

On Mon, Jul 28, 2014 at 10:41 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Jul 28, 2014 at 07:25:14PM +0100, Konstantin Khlebnikov wrote:
>> On Mon, Jul 28, 2014 at 10:14 PM, Will Deacon <will.deacon@arm.com> wrote:
>> > On Tue, Jul 22, 2014 at 04:36:35PM +0100, Konstantin Khlebnikov wrote:
>> >> idmap layout combines both phisical and virtual addresses.
>> >> Everything works fine if ram physically lays below PAGE_OFFSET.
>> >> Otherwise idmap starts punching huge holes in virtual memory layout.
>> >> It maps ram by 2MiB sections, but when it allocates new pmd page it
>> >> cuts 1GiB at once.
>> >>
>> >> This patch makes a copy of all affected pmds from init_mm.
>> >> Only few (usually one) 2MiB sections will be lost.
>> >> This is not eliminates problem but makes it 512 times less likely.
>> >
>> > I'm struggling to understand your commit message, but making a problem `512
>> > times less likely' does sound like a bit of a hack to me. Can't we fix this
>> > properly instead?
>>
>> Yep, my comment sucks.
>>
>> Usually idmap looks like this:
>>
>> |0x00000000 -- <chunk of physical memory in identical mapping > --- |
>> TASK_SIZE -- <kernel space vm layoyt> --- 0xFFFFFFFF |
>>
>> But when that physical memory chunk starts from 0xE8000000 or even
>> 0xF2000000 evenything becomes very complicated.
>
> Why? As long as we don't clobber the kernel text (which would require
> PHYS_OFFSET to be at a really weird alignment and very close to
> PAGE_OFFSET), then you should be alright. Sure, you'll lose things like your
> stack and the vmalloc area etc, but you're running in the idmap, so don't
> use those things.

Yep, we have piece of hardware with really weird aligned PHYS_OFFSET,
mostly all ram is above 4gb and we was lucky enough to get into the trouble.

It seems keystone has all memory above 4gb but small piece is mapped below
especially for booting. I suppose it's below PAGE_OFFSET so Cyril
hadn't seen that problem.

Also I seen comment somewhere in the code which tells that idrmap pgd is
always below 4gb which isn't quite true. Moreover, I had some experiments with
mapping ram to random places in qemu and seen that kernel cannot boot if
PHYS_OFFSET isn't alligned to 128mb which is strange.
So, it seems there is plenty bugs anound.

>
> soft_restart is an example of code that deals with these issues. Which code
> is causing you problems?

That was booting of secondary cpus, all of them.

>
> Will

[PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Konstantin Khlebnikov]

On Mon, Jul 28, 2014 at 10:41 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Jul 28, 2014 at 07:25:14PM +0100, Konstantin Khlebnikov wrote:
>> On Mon, Jul 28, 2014 at 10:14 PM, Will Deacon <will.deacon@arm.com> wrote:
>> > On Tue, Jul 22, 2014 at 04:36:35PM +0100, Konstantin Khlebnikov wrote:
>> >> idmap layout combines both phisical and virtual addresses.
>> >> Everything works fine if ram physically lays below PAGE_OFFSET.
>> >> Otherwise idmap starts punching huge holes in virtual memory layout.
>> >> It maps ram by 2MiB sections, but when it allocates new pmd page it
>> >> cuts 1GiB at once.
>> >>
>> >> This patch makes a copy of all affected pmds from init_mm.
>> >> Only few (usually one) 2MiB sections will be lost.
>> >> This is not eliminates problem but makes it 512 times less likely.
>> >
>> > I'm struggling to understand your commit message, but making a problem `512
>> > times less likely' does sound like a bit of a hack to me. Can't we fix this
>> > properly instead?
>>
>> Yep, my comment sucks.
>>
>> Usually idmap looks like this:
>>
>> |0x00000000 -- <chunk of physical memory in identical mapping > --- |
>> TASK_SIZE -- <kernel space vm layoyt> --- 0xFFFFFFFF |
>>
>> But when that physical memory chunk starts from 0xE8000000 or even
>> 0xF2000000 evenything becomes very complicated.
>
> Why? As long as we don't clobber the kernel text (which would require
> PHYS_OFFSET to be at a really weird alignment and very close to
> PAGE_OFFSET), then you should be alright. Sure, you'll lose things like your
> stack and the vmalloc area etc, but you're running in the idmap, so don't
> use those things.

Yep, we have piece of hardware with really weird aligned PHYS_OFFSET,
mostly all ram is above 4gb and we was lucky enough to get into the trouble.

It seems keystone has all memory above 4gb but small piece is mapped below
especially for booting. I suppose it's below PAGE_OFFSET so Cyril
hadn't seen that problem.

Also I seen comment somewhere in the code which tells that idrmap pgd is
always below 4gb which isn't quite true. Moreover, I had some experiments with
mapping ram to random places in qemu and seen that kernel cannot boot if
PHYS_OFFSET isn't alligned to 128mb which is strange.
So, it seems there is plenty bugs anound.

>
> soft_restart is an example of code that deals with these issues. Which code
> is causing you problems?

That was booting of secondary cpus, all of them.

>
> Will

Re: [PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Will Deacon]

On Mon, Jul 28, 2014 at 07:57:00PM +0100, Konstantin Khlebnikov wrote:
> On Mon, Jul 28, 2014 at 10:41 PM, Will Deacon <will.deacon@arm.com> wrote:
> > On Mon, Jul 28, 2014 at 07:25:14PM +0100, Konstantin Khlebnikov wrote:
> >> On Mon, Jul 28, 2014 at 10:14 PM, Will Deacon <will.deacon@arm.com> wrote:
> >> > On Tue, Jul 22, 2014 at 04:36:35PM +0100, Konstantin Khlebnikov wrote:
> >> >> idmap layout combines both phisical and virtual addresses.
> >> >> Everything works fine if ram physically lays below PAGE_OFFSET.
> >> >> Otherwise idmap starts punching huge holes in virtual memory layout.
> >> >> It maps ram by 2MiB sections, but when it allocates new pmd page it
> >> >> cuts 1GiB at once.
> >> >>
> >> >> This patch makes a copy of all affected pmds from init_mm.
> >> >> Only few (usually one) 2MiB sections will be lost.
> >> >> This is not eliminates problem but makes it 512 times less likely.
> >> >
> >> > I'm struggling to understand your commit message, but making a problem `512
> >> > times less likely' does sound like a bit of a hack to me. Can't we fix this
> >> > properly instead?
> >>
> >> Yep, my comment sucks.
> >>
> >> Usually idmap looks like this:
> >>
> >> |0x00000000 -- <chunk of physical memory in identical mapping > --- |
> >> TASK_SIZE -- <kernel space vm layoyt> --- 0xFFFFFFFF |
> >>
> >> But when that physical memory chunk starts from 0xE8000000 or even
> >> 0xF2000000 evenything becomes very complicated.
> >
> > Why? As long as we don't clobber the kernel text (which would require
> > PHYS_OFFSET to be at a really weird alignment and very close to
> > PAGE_OFFSET), then you should be alright. Sure, you'll lose things like your
> > stack and the vmalloc area etc, but you're running in the idmap, so don't
> > use those things.
> 
> Yep, we have piece of hardware with really weird aligned PHYS_OFFSET,
> mostly all ram is above 4gb and we was lucky enough to get into the trouble.
> 
> It seems keystone has all memory above 4gb but small piece is mapped below
> especially for booting. I suppose it's below PAGE_OFFSET so Cyril
> hadn't seen that problem.

Sure, I remember when the keystone support was merged. There's a low alias
of memory which isn't I/O coherent, so when we enable the MMU we rewrite the
page tables to use the high alias (which is also broken, as I pointed out on
the list today).

> Also I seen comment somewhere in the code which tells that idrmap pgd is
> always below 4gb which isn't quite true. Moreover, I had some experiments with
> mapping ram to random places in qemu and seen that kernel cannot boot if
> PHYS_OFFSET isn't alligned to 128mb which is strange.
> So, it seems there is plenty bugs anound.
> 
> >
> > soft_restart is an example of code that deals with these issues. Which code
> > is causing you problems?
> 
> That was booting of secondary cpus, all of them.

Ok. I think we need more specifics to progress with this patch. It's not
clear to me what's going wrong or why your platform is causing the issues
you're seeing.

Will

[PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Will Deacon]

On Mon, Jul 28, 2014 at 07:57:00PM +0100, Konstantin Khlebnikov wrote:
> On Mon, Jul 28, 2014 at 10:41 PM, Will Deacon <will.deacon@arm.com> wrote:
> > On Mon, Jul 28, 2014 at 07:25:14PM +0100, Konstantin Khlebnikov wrote:
> >> On Mon, Jul 28, 2014 at 10:14 PM, Will Deacon <will.deacon@arm.com> wrote:
> >> > On Tue, Jul 22, 2014 at 04:36:35PM +0100, Konstantin Khlebnikov wrote:
> >> >> idmap layout combines both phisical and virtual addresses.
> >> >> Everything works fine if ram physically lays below PAGE_OFFSET.
> >> >> Otherwise idmap starts punching huge holes in virtual memory layout.
> >> >> It maps ram by 2MiB sections, but when it allocates new pmd page it
> >> >> cuts 1GiB at once.
> >> >>
> >> >> This patch makes a copy of all affected pmds from init_mm.
> >> >> Only few (usually one) 2MiB sections will be lost.
> >> >> This is not eliminates problem but makes it 512 times less likely.
> >> >
> >> > I'm struggling to understand your commit message, but making a problem `512
> >> > times less likely' does sound like a bit of a hack to me. Can't we fix this
> >> > properly instead?
> >>
> >> Yep, my comment sucks.
> >>
> >> Usually idmap looks like this:
> >>
> >> |0x00000000 -- <chunk of physical memory in identical mapping > --- |
> >> TASK_SIZE -- <kernel space vm layoyt> --- 0xFFFFFFFF |
> >>
> >> But when that physical memory chunk starts from 0xE8000000 or even
> >> 0xF2000000 evenything becomes very complicated.
> >
> > Why? As long as we don't clobber the kernel text (which would require
> > PHYS_OFFSET to be at a really weird alignment and very close to
> > PAGE_OFFSET), then you should be alright. Sure, you'll lose things like your
> > stack and the vmalloc area etc, but you're running in the idmap, so don't
> > use those things.
> 
> Yep, we have piece of hardware with really weird aligned PHYS_OFFSET,
> mostly all ram is above 4gb and we was lucky enough to get into the trouble.
> 
> It seems keystone has all memory above 4gb but small piece is mapped below
> especially for booting. I suppose it's below PAGE_OFFSET so Cyril
> hadn't seen that problem.

Sure, I remember when the keystone support was merged. There's a low alias
of memory which isn't I/O coherent, so when we enable the MMU we rewrite the
page tables to use the high alias (which is also broken, as I pointed out on
the list today).

> Also I seen comment somewhere in the code which tells that idrmap pgd is
> always below 4gb which isn't quite true. Moreover, I had some experiments with
> mapping ram to random places in qemu and seen that kernel cannot boot if
> PHYS_OFFSET isn't alligned to 128mb which is strange.
> So, it seems there is plenty bugs anound.
> 
> >
> > soft_restart is an example of code that deals with these issues. Which code
> > is causing you problems?
> 
> That was booting of secondary cpus, all of them.

Ok. I think we need more specifics to progress with this patch. It's not
clear to me what's going wrong or why your platform is causing the issues
you're seeing.

Will

Re: [PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Russell King - ARM Linux]

On Mon, Jul 28, 2014 at 10:57:00PM +0400, Konstantin Khlebnikov wrote:
> Also I seen comment somewhere in the code which tells that idrmap pgd is
> always below 4gb which isn't quite true.

"idmap" means "identity map".  It's a region which maps the same value of
physical address to the same value of virtual address.

Since the virtual address space is limited to 4GB, there is /no way/ that
the physical address can be above 4GB, and it still be called an
"identity map".

The reason for this is that code in the identity map will be fetched with
the MMU off.  While this code is running, it will enable the MMU using the
identity map page table pointer, and the CPU must see _no_ change in the
instructions/data fetched from that region.  It will then branch to the
kernel proper, and the kernel will then switch away from the identity page
table.

Once the kernel has switched away from the identity page table, interrupts
and other exceptions can then be taken on the CPU - but not before.
Neither is it expected that the CPU will access any devices until it has
switched away from the identity page table.

What this also means is that the code in the identity map must remain
visible in the low 4GB of physical address space.

> Moreover, I had some experiments with
> mapping ram to random places in qemu and seen that kernel cannot boot if
> PHYS_OFFSET isn't alligned to 128mb which is strange.

That is intentional.  PHYS_OFFSET has a number of restrictions, one of
them is indeed that the physical offset /will/ be aligned to 128MB.
That was decided after looking at the platforms we have and is now
fixed at that value with platform-breaking consequences if it needs
changing.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

[PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Russell King - ARM Linux]

On Mon, Jul 28, 2014 at 10:57:00PM +0400, Konstantin Khlebnikov wrote:
> Also I seen comment somewhere in the code which tells that idrmap pgd is
> always below 4gb which isn't quite true.

"idmap" means "identity map".  It's a region which maps the same value of
physical address to the same value of virtual address.

Since the virtual address space is limited to 4GB, there is /no way/ that
the physical address can be above 4GB, and it still be called an
"identity map".

The reason for this is that code in the identity map will be fetched with
the MMU off.  While this code is running, it will enable the MMU using the
identity map page table pointer, and the CPU must see _no_ change in the
instructions/data fetched from that region.  It will then branch to the
kernel proper, and the kernel will then switch away from the identity page
table.

Once the kernel has switched away from the identity page table, interrupts
and other exceptions can then be taken on the CPU - but not before.
Neither is it expected that the CPU will access any devices until it has
switched away from the identity page table.

What this also means is that the code in the identity map must remain
visible in the low 4GB of physical address space.

> Moreover, I had some experiments with
> mapping ram to random places in qemu and seen that kernel cannot boot if
> PHYS_OFFSET isn't alligned to 128mb which is strange.

That is intentional.  PHYS_OFFSET has a number of restrictions, one of
them is indeed that the physical offset /will/ be aligned to 128MB.
That was decided after looking at the platforms we have and is now
fixed at that value with platform-breaking consequences if it needs
changing.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Konstantin Khlebnikov]

On Mon, Jul 28, 2014 at 11:13 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jul 28, 2014 at 10:57:00PM +0400, Konstantin Khlebnikov wrote:
>> Also I seen comment somewhere in the code which tells that idrmap pgd is
>> always below 4gb which isn't quite true.
>
> "idmap" means "identity map".  It's a region which maps the same value of
> physical address to the same value of virtual address.
>
> Since the virtual address space is limited to 4GB, there is /no way/ that
> the physical address can be above 4GB, and it still be called an
> "identity map".
>
> The reason for this is that code in the identity map will be fetched with
> the MMU off.  While this code is running, it will enable the MMU using the
> identity map page table pointer, and the CPU must see _no_ change in the
> instructions/data fetched from that region.  It will then branch to the
> kernel proper, and the kernel will then switch away from the identity page
> table.
>
> Once the kernel has switched away from the identity page table, interrupts
> and other exceptions can then be taken on the CPU - but not before.
> Neither is it expected that the CPU will access any devices until it has
> switched away from the identity page table.
>
> What this also means is that the code in the identity map must remain
> visible in the low 4GB of physical address space.

Ok, before switching from identity mapping to normal mapping kernel must
switch instruction pointer from physical address to virtual. It's a long jump
out idmap code section to some normal kernel function.
__secondary_switched in my case. And both physical source and virtual
destination addresses must present in one same mapping (idmap).

idmap pgd starts as copy of pgd from init_mm, it points to the same pmd pages.
When it populates identical mapping for that special code section it allocates
new pages from pmd entries (which covers 1Gb of vm layout) but only few of
them are filled. In case 3/1GB split If idmap touches somehing above 3Gb it
kills whole kernel vm layout and as result kernel cannot jump to any
virtual address.

>
>> Moreover, I had some experiments with
>> mapping ram to random places in qemu and seen that kernel cannot boot if
>> PHYS_OFFSET isn't alligned to 128mb which is strange.
>
> That is intentional.  PHYS_OFFSET has a number of restrictions, one of
> them is indeed that the physical offset /will/ be aligned to 128MB.
> That was decided after looking at the platforms we have and is now
> fixed at that value with platform-breaking consequences if it needs
> changing.
>
> --
> FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
> according to speedtest.net.

[PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Konstantin Khlebnikov]

On Mon, Jul 28, 2014 at 11:13 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jul 28, 2014 at 10:57:00PM +0400, Konstantin Khlebnikov wrote:
>> Also I seen comment somewhere in the code which tells that idrmap pgd is
>> always below 4gb which isn't quite true.
>
> "idmap" means "identity map".  It's a region which maps the same value of
> physical address to the same value of virtual address.
>
> Since the virtual address space is limited to 4GB, there is /no way/ that
> the physical address can be above 4GB, and it still be called an
> "identity map".
>
> The reason for this is that code in the identity map will be fetched with
> the MMU off.  While this code is running, it will enable the MMU using the
> identity map page table pointer, and the CPU must see _no_ change in the
> instructions/data fetched from that region.  It will then branch to the
> kernel proper, and the kernel will then switch away from the identity page
> table.
>
> Once the kernel has switched away from the identity page table, interrupts
> and other exceptions can then be taken on the CPU - but not before.
> Neither is it expected that the CPU will access any devices until it has
> switched away from the identity page table.
>
> What this also means is that the code in the identity map must remain
> visible in the low 4GB of physical address space.

Ok, before switching from identity mapping to normal mapping kernel must
switch instruction pointer from physical address to virtual. It's a long jump
out idmap code section to some normal kernel function.
__secondary_switched in my case. And both physical source and virtual
destination addresses must present in one same mapping (idmap).

idmap pgd starts as copy of pgd from init_mm, it points to the same pmd pages.
When it populates identical mapping for that special code section it allocates
new pages from pmd entries (which covers 1Gb of vm layout) but only few of
them are filled. In case 3/1GB split If idmap touches somehing above 3Gb it
kills whole kernel vm layout and as result kernel cannot jump to any
virtual address.

>
>> Moreover, I had some experiments with
>> mapping ram to random places in qemu and seen that kernel cannot boot if
>> PHYS_OFFSET isn't alligned to 128mb which is strange.
>
> That is intentional.  PHYS_OFFSET has a number of restrictions, one of
> them is indeed that the physical offset /will/ be aligned to 128MB.
> That was decided after looking at the platforms we have and is now
> fixed at that value with platform-breaking consequences if it needs
> changing.
>
> --
> FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
> according to speedtest.net.

Re: [PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Konstantin Khlebnikov]

On Mon, Jul 28, 2014 at 11:29 PM, Konstantin Khlebnikov
<koct9i@gmail.com> wrote:
> On Mon, Jul 28, 2014 at 11:13 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Mon, Jul 28, 2014 at 10:57:00PM +0400, Konstantin Khlebnikov wrote:
>>> Also I seen comment somewhere in the code which tells that idrmap pgd is
>>> always below 4gb which isn't quite true.
>>
>> "idmap" means "identity map".  It's a region which maps the same value of
>> physical address to the same value of virtual address.
>>
>> Since the virtual address space is limited to 4GB, there is /no way/ that
>> the physical address can be above 4GB, and it still be called an
>> "identity map".
>>
>> The reason for this is that code in the identity map will be fetched with
>> the MMU off.  While this code is running, it will enable the MMU using the
>> identity map page table pointer, and the CPU must see _no_ change in the
>> instructions/data fetched from that region.  It will then branch to the
>> kernel proper, and the kernel will then switch away from the identity page
>> table.
>>
>> Once the kernel has switched away from the identity page table, interrupts
>> and other exceptions can then be taken on the CPU - but not before.
>> Neither is it expected that the CPU will access any devices until it has
>> switched away from the identity page table.
>>
>> What this also means is that the code in the identity map must remain
>> visible in the low 4GB of physical address space.
>
> Ok, before switching from identity mapping to normal mapping kernel must
> switch instruction pointer from physical address to virtual. It's a long jump
> out idmap code section to some normal kernel function.
> __secondary_switched in my case. And both physical source and virtual
> destination addresses must present in one same mapping (idmap).
>
> idmap pgd starts as copy of pgd from init_mm, it points to the same pmd pages.
> When it populates identical mapping for that special code section it allocates
> new pages from pmd entries (which covers 1Gb of vm layout) but only few of
> them are filled. In case 3/1GB split If idmap touches somehing above 3Gb it
> kills whole kernel vm layout and as result kernel cannot jump to any
> virtual address.

Oh, my bad. That was response to Will Deacon's

> Ok. I think we need more specifics to progress with this patch. It's not
> clear to me what's going wrong or why your platform is causing the issues
> you're seeing.

>
>>
>>> Moreover, I had some experiments with
>>> mapping ram to random places in qemu and seen that kernel cannot boot if
>>> PHYS_OFFSET isn't alligned to 128mb which is strange.
>>
>> That is intentional.  PHYS_OFFSET has a number of restrictions, one of
>> them is indeed that the physical offset /will/ be aligned to 128MB.
>> That was decided after looking at the platforms we have and is now
>> fixed at that value with platform-breaking consequences if it needs
>> changing.
>>
>> --
>> FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
>> according to speedtest.net.

[PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Konstantin Khlebnikov]

On Mon, Jul 28, 2014 at 11:29 PM, Konstantin Khlebnikov
<koct9i@gmail.com> wrote:
> On Mon, Jul 28, 2014 at 11:13 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Mon, Jul 28, 2014 at 10:57:00PM +0400, Konstantin Khlebnikov wrote:
>>> Also I seen comment somewhere in the code which tells that idrmap pgd is
>>> always below 4gb which isn't quite true.
>>
>> "idmap" means "identity map".  It's a region which maps the same value of
>> physical address to the same value of virtual address.
>>
>> Since the virtual address space is limited to 4GB, there is /no way/ that
>> the physical address can be above 4GB, and it still be called an
>> "identity map".
>>
>> The reason for this is that code in the identity map will be fetched with
>> the MMU off.  While this code is running, it will enable the MMU using the
>> identity map page table pointer, and the CPU must see _no_ change in the
>> instructions/data fetched from that region.  It will then branch to the
>> kernel proper, and the kernel will then switch away from the identity page
>> table.
>>
>> Once the kernel has switched away from the identity page table, interrupts
>> and other exceptions can then be taken on the CPU - but not before.
>> Neither is it expected that the CPU will access any devices until it has
>> switched away from the identity page table.
>>
>> What this also means is that the code in the identity map must remain
>> visible in the low 4GB of physical address space.
>
> Ok, before switching from identity mapping to normal mapping kernel must
> switch instruction pointer from physical address to virtual. It's a long jump
> out idmap code section to some normal kernel function.
> __secondary_switched in my case. And both physical source and virtual
> destination addresses must present in one same mapping (idmap).
>
> idmap pgd starts as copy of pgd from init_mm, it points to the same pmd pages.
> When it populates identical mapping for that special code section it allocates
> new pages from pmd entries (which covers 1Gb of vm layout) but only few of
> them are filled. In case 3/1GB split If idmap touches somehing above 3Gb it
> kills whole kernel vm layout and as result kernel cannot jump to any
> virtual address.

Oh, my bad. That was response to Will Deacon's

> Ok. I think we need more specifics to progress with this patch. It's not
> clear to me what's going wrong or why your platform is causing the issues
> you're seeing.

>
>>
>>> Moreover, I had some experiments with
>>> mapping ram to random places in qemu and seen that kernel cannot boot if
>>> PHYS_OFFSET isn't alligned to 128mb which is strange.
>>
>> That is intentional.  PHYS_OFFSET has a number of restrictions, one of
>> them is indeed that the physical offset /will/ be aligned to 128MB.
>> That was decided after looking at the platforms we have and is now
>> fixed at that value with platform-breaking consequences if it needs
>> changing.
>>
>> --
>> FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
>> according to speedtest.net.

Re: [PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Russell King - ARM Linux]

On Mon, Jul 28, 2014 at 11:29:39PM +0400, Konstantin Khlebnikov wrote:
> Ok, before switching from identity mapping to normal mapping kernel must
> switch instruction pointer from physical address to virtual.

"switch instruction pointer from physical address to virtual."

There's no such distinction for the instruction pointer.

There is the mode where the MMU is disabled.  The CPU fetches from the
address in the program counter.  The instructions it fetches will
instruct it to perform operations to enable the MMU.

The CPU itself continues fetching instructions (with an unpredictable
number of instructions in the CPU's pipeline) across this boundary.

Hence, it is _impossible_ to know which exact instructions are fetched
with the MMU off, and which are fetched with the MMU on.

This is why we need the identity mapping: so that the CPU's instruction
fetches can continue unaffected by turning the MMU on.

Before the MMU is on, the CPU is fetching from an untranslated (== physical)
view of the address space, which is limited to 4GB in size.

After the MMU is on, the CPU is fetching from a translated (== virtual)
view of the address space, which and the translated version is also
limited to 4GB in size.

> It's a long jump
> out idmap code section to some normal kernel function.
> __secondary_switched in my case. And both physical source and virtual
> destination addresses must present in one same mapping (idmap).

... one which the code already caters for.

> idmap pgd starts as copy of pgd from init_mm, it points to the same pmd pages.
> When it populates identical mapping for that special code section it allocates
> new pages from pmd entries (which covers 1Gb of vm layout) but only few of
> them are filled. In case 3/1GB split If idmap touches somehing above 3Gb it
> kills whole kernel vm layout and as result kernel cannot jump to any
> virtual address.

It doesn't matter, provided the kernel text and data in the virtual
address space are not overwritten by the identity mapping.  If it
ends up in the vmalloc or IO space, that should not be a problem.

It also should not be a problem if the physical memory starts at
PAGE_OFFSET (iow, PHYS_OFFSET == PAGE_OFFSET).  Indeed, we have
some platforms where that is true.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

[PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Russell King - ARM Linux]

On Mon, Jul 28, 2014 at 11:29:39PM +0400, Konstantin Khlebnikov wrote:
> Ok, before switching from identity mapping to normal mapping kernel must
> switch instruction pointer from physical address to virtual.

"switch instruction pointer from physical address to virtual."

There's no such distinction for the instruction pointer.

There is the mode where the MMU is disabled.  The CPU fetches from the
address in the program counter.  The instructions it fetches will
instruct it to perform operations to enable the MMU.

The CPU itself continues fetching instructions (with an unpredictable
number of instructions in the CPU's pipeline) across this boundary.

Hence, it is _impossible_ to know which exact instructions are fetched
with the MMU off, and which are fetched with the MMU on.

This is why we need the identity mapping: so that the CPU's instruction
fetches can continue unaffected by turning the MMU on.

Before the MMU is on, the CPU is fetching from an untranslated (== physical)
view of the address space, which is limited to 4GB in size.

After the MMU is on, the CPU is fetching from a translated (== virtual)
view of the address space, which and the translated version is also
limited to 4GB in size.

> It's a long jump
> out idmap code section to some normal kernel function.
> __secondary_switched in my case. And both physical source and virtual
> destination addresses must present in one same mapping (idmap).

... one which the code already caters for.

> idmap pgd starts as copy of pgd from init_mm, it points to the same pmd pages.
> When it populates identical mapping for that special code section it allocates
> new pages from pmd entries (which covers 1Gb of vm layout) but only few of
> them are filled. In case 3/1GB split If idmap touches somehing above 3Gb it
> kills whole kernel vm layout and as result kernel cannot jump to any
> virtual address.

It doesn't matter, provided the kernel text and data in the virtual
address space are not overwritten by the identity mapping.  If it
ends up in the vmalloc or IO space, that should not be a problem.

It also should not be a problem if the physical memory starts at
PAGE_OFFSET (iow, PHYS_OFFSET == PAGE_OFFSET).  Indeed, we have
some platforms where that is true.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Konstantin Khlebnikov]

On Mon, Jul 28, 2014 at 11:42 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jul 28, 2014 at 11:29:39PM +0400, Konstantin Khlebnikov wrote:
>> Ok, before switching from identity mapping to normal mapping kernel must
>> switch instruction pointer from physical address to virtual.
>
> "switch instruction pointer from physical address to virtual."
>
> There's no such distinction for the instruction pointer.

I know. I mean "logically".

>
> There is the mode where the MMU is disabled.  The CPU fetches from the
> address in the program counter.  The instructions it fetches will
> instruct it to perform operations to enable the MMU.
>
> The CPU itself continues fetching instructions (with an unpredictable
> number of instructions in the CPU's pipeline) across this boundary.
>
> Hence, it is _impossible_ to know which exact instructions are fetched
> with the MMU off, and which are fetched with the MMU on.
>
> This is why we need the identity mapping: so that the CPU's instruction
> fetches can continue unaffected by turning the MMU on.
>
> Before the MMU is on, the CPU is fetching from an untranslated (== physical)
> view of the address space, which is limited to 4GB in size.
>
> After the MMU is on, the CPU is fetching from a translated (== virtual)
> view of the address space, which and the translated version is also
> limited to 4GB in size.

Sorry but I'm really look so dumb? Maybe it's true, it's almost
midnight at my side.

>
>> It's a long jump
>> out idmap code section to some normal kernel function.
>> __secondary_switched in my case. And both physical source and virtual
>> destination addresses must present in one same mapping (idmap).
>
> ... one which the code already caters for.
>
>> idmap pgd starts as copy of pgd from init_mm, it points to the same pmd pages.
>> When it populates identical mapping for that special code section it allocates
>> new pages from pmd entries (which covers 1Gb of vm layout) but only few of
>> them are filled. In case 3/1GB split If idmap touches somehing above 3Gb it
>> kills whole kernel vm layout and as result kernel cannot jump to any
>> virtual address.
>
> It doesn't matter, provided the kernel text and data in the virtual
> address space are not overwritten by the identity mapping.  If it
> ends up in the vmalloc or IO space, that should not be a problem.

In my case it's been overwritten.
And it always happens when PHYS_OFFSET >= PAGE_OFFSET
because in case of LPAE idmap always overwrites 1Gb at once.

>
> It also should not be a problem if the physical memory starts at
> PAGE_OFFSET (iow, PHYS_OFFSET == PAGE_OFFSET).  Indeed, we have
> some platforms where that is true.
>
> --
> FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
> according to speedtest.net.

[PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Konstantin Khlebnikov]

On Mon, Jul 28, 2014 at 11:42 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jul 28, 2014 at 11:29:39PM +0400, Konstantin Khlebnikov wrote:
>> Ok, before switching from identity mapping to normal mapping kernel must
>> switch instruction pointer from physical address to virtual.
>
> "switch instruction pointer from physical address to virtual."
>
> There's no such distinction for the instruction pointer.

I know. I mean "logically".

>
> There is the mode where the MMU is disabled.  The CPU fetches from the
> address in the program counter.  The instructions it fetches will
> instruct it to perform operations to enable the MMU.
>
> The CPU itself continues fetching instructions (with an unpredictable
> number of instructions in the CPU's pipeline) across this boundary.
>
> Hence, it is _impossible_ to know which exact instructions are fetched
> with the MMU off, and which are fetched with the MMU on.
>
> This is why we need the identity mapping: so that the CPU's instruction
> fetches can continue unaffected by turning the MMU on.
>
> Before the MMU is on, the CPU is fetching from an untranslated (== physical)
> view of the address space, which is limited to 4GB in size.
>
> After the MMU is on, the CPU is fetching from a translated (== virtual)
> view of the address space, which and the translated version is also
> limited to 4GB in size.

Sorry but I'm really look so dumb? Maybe it's true, it's almost
midnight at my side.

>
>> It's a long jump
>> out idmap code section to some normal kernel function.
>> __secondary_switched in my case. And both physical source and virtual
>> destination addresses must present in one same mapping (idmap).
>
> ... one which the code already caters for.
>
>> idmap pgd starts as copy of pgd from init_mm, it points to the same pmd pages.
>> When it populates identical mapping for that special code section it allocates
>> new pages from pmd entries (which covers 1Gb of vm layout) but only few of
>> them are filled. In case 3/1GB split If idmap touches somehing above 3Gb it
>> kills whole kernel vm layout and as result kernel cannot jump to any
>> virtual address.
>
> It doesn't matter, provided the kernel text and data in the virtual
> address space are not overwritten by the identity mapping.  If it
> ends up in the vmalloc or IO space, that should not be a problem.

In my case it's been overwritten.
And it always happens when PHYS_OFFSET >= PAGE_OFFSET
because in case of LPAE idmap always overwrites 1Gb at once.

>
> It also should not be a problem if the physical memory starts at
> PAGE_OFFSET (iow, PHYS_OFFSET == PAGE_OFFSET).  Indeed, we have
> some platforms where that is true.
>
> --
> FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
> according to speedtest.net.

Re: [PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Russell King - ARM Linux]

On Mon, Jul 28, 2014 at 11:57:16PM +0400, Konstantin Khlebnikov wrote:
> On Mon, Jul 28, 2014 at 11:42 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Jul 28, 2014 at 11:29:39PM +0400, Konstantin Khlebnikov wrote:
> >> Ok, before switching from identity mapping to normal mapping kernel must
> >> switch instruction pointer from physical address to virtual.
> >
> > "switch instruction pointer from physical address to virtual."
> >
> > There's no such distinction for the instruction pointer.
> 
> I know. I mean "logically".
> 
...
> 
> Sorry but I'm really look so dumb? Maybe it's true, it's almost
> midnight at my side.

When you use language which suggests a lack of understanding, then
I will explain things.

> > It doesn't matter, provided the kernel text and data in the virtual
> > address space are not overwritten by the identity mapping.  If it
> > ends up in the vmalloc or IO space, that should not be a problem.
> 
> In my case it's been overwritten.
> And it always happens when PHYS_OFFSET >= PAGE_OFFSET
> because in case of LPAE idmap always overwrites 1Gb at once.

Right, I now see what you're getting at.  Here's a better description
which I've used when committing your patch.  I've only taken patch 2
at the present time.



On LPAE, each level 1 (pgd) page table entry maps 1GiB, and the level 2
(pmd) entries map 2MiB.

When the identity mapping is created on LPAE, the pgd pointers are copied
from the swapper_pg_dir.  If we find that we need to modify the contents
of a pmd, we allocate a new empty pmd table and insert it into the
appropriate 1GB slot, before then filling it with the identity mapping.

However, if the 1GB slot covers the kernel lowmem mappings, we obliterate
those mappings.

When replacing a PMD, first copy the old PMD contents to the new PMD, so
that we preserve the existing mappings in the 1GiB region, particularly
the mappings of the kernel itself.


-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

[PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Russell King - ARM Linux]

On Mon, Jul 28, 2014 at 11:57:16PM +0400, Konstantin Khlebnikov wrote:
> On Mon, Jul 28, 2014 at 11:42 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Jul 28, 2014 at 11:29:39PM +0400, Konstantin Khlebnikov wrote:
> >> Ok, before switching from identity mapping to normal mapping kernel must
> >> switch instruction pointer from physical address to virtual.
> >
> > "switch instruction pointer from physical address to virtual."
> >
> > There's no such distinction for the instruction pointer.
> 
> I know. I mean "logically".
> 
...
> 
> Sorry but I'm really look so dumb? Maybe it's true, it's almost
> midnight at my side.

When you use language which suggests a lack of understanding, then
I will explain things.

> > It doesn't matter, provided the kernel text and data in the virtual
> > address space are not overwritten by the identity mapping.  If it
> > ends up in the vmalloc or IO space, that should not be a problem.
> 
> In my case it's been overwritten.
> And it always happens when PHYS_OFFSET >= PAGE_OFFSET
> because in case of LPAE idmap always overwrites 1Gb at once.

Right, I now see what you're getting at.  Here's a better description
which I've used when committing your patch.  I've only taken patch 2
at the present time.



On LPAE, each level 1 (pgd) page table entry maps 1GiB, and the level 2
(pmd) entries map 2MiB.

When the identity mapping is created on LPAE, the pgd pointers are copied
from the swapper_pg_dir.  If we find that we need to modify the contents
of a pmd, we allocate a new empty pmd table and insert it into the
appropriate 1GB slot, before then filling it with the identity mapping.

However, if the 1GB slot covers the kernel lowmem mappings, we obliterate
those mappings.

When replacing a PMD, first copy the old PMD contents to the new PMD, so
that we preserve the existing mappings in the 1GiB region, particularly
the mappings of the kernel itself.


-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Will Deacon]

On Mon, Jul 28, 2014 at 07:47:41PM +0100, Will Deacon wrote:
> On Mon, Jul 28, 2014 at 07:40:58PM +0100, Konstantin Khlebnikov wrote:
> > On Mon, Jul 28, 2014 at 10:12 PM, Will Deacon <will.deacon@arm.com> wrote:
> > > On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
> > >> This patch fixes booting when idmap pgd lays above 4gb. Commit
> > >> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
> > >>
> > >> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
> > >> carry flag must be added to the upper part.
> > >>
> > >> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
> > >> Cc: Cyril Chemparathy <cyril@ti.com>
> > >> Cc: Vitaly Andrianov <vitalya@ti.com>
> > >> ---
> > >>  arch/arm/mm/proc-v7-3level.S |    7 +++----
> > >>  1 file changed, 3 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
> > >> index 22e3ad6..f0481dd 100644
> > >> --- a/arch/arm/mm/proc-v7-3level.S
> > >> +++ b/arch/arm/mm/proc-v7-3level.S
> > >> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
> > >>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
> > >>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
> > >>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
> > >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
> > >> +     adcls   \tmp, \tmp, #0
> > >> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
> > >>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
> > >>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
> > >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
> > >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
> > >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
> > >> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
> > >
> > > I must admit, the code you are removing here looks really strange. Was there
> > > a badly resolved conflict somewhere along the way? It would be nice to see
> > > if your fix (which seems ok to me) was actually present in the mailing list
> > > posting of the patch that ended in the above mess.
> > 
> > Nope, no merge conflicts, source in original patch
> > https://lkml.org/lkml/2012/9/11/346
> > 
> > That mess completely harmless, this code is used only once on boot.
> > I don't have that email, so replying isn't trivial for me.
> 
> How bizarre. Also, Cyril doesn't work for TI anymore (his email is
> bouncing), so it's tricky to know what he meant here.
> 
> Your patch looks better than what we currently have though. Have you managed
> to test it on a keystone platform (I don't have one)?

Given that:

  Acked-by: Will Deacon <will.deacon@arm.com>

Will

[PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Will Deacon]

On Mon, Jul 28, 2014 at 07:47:41PM +0100, Will Deacon wrote:
> On Mon, Jul 28, 2014 at 07:40:58PM +0100, Konstantin Khlebnikov wrote:
> > On Mon, Jul 28, 2014 at 10:12 PM, Will Deacon <will.deacon@arm.com> wrote:
> > > On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
> > >> This patch fixes booting when idmap pgd lays above 4gb. Commit
> > >> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
> > >>
> > >> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
> > >> carry flag must be added to the upper part.
> > >>
> > >> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
> > >> Cc: Cyril Chemparathy <cyril@ti.com>
> > >> Cc: Vitaly Andrianov <vitalya@ti.com>
> > >> ---
> > >>  arch/arm/mm/proc-v7-3level.S |    7 +++----
> > >>  1 file changed, 3 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
> > >> index 22e3ad6..f0481dd 100644
> > >> --- a/arch/arm/mm/proc-v7-3level.S
> > >> +++ b/arch/arm/mm/proc-v7-3level.S
> > >> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
> > >>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
> > >>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
> > >>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
> > >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
> > >> +     adcls   \tmp, \tmp, #0
> > >> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
> > >>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
> > >>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
> > >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
> > >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
> > >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
> > >> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
> > >
> > > I must admit, the code you are removing here looks really strange. Was there
> > > a badly resolved conflict somewhere along the way? It would be nice to see
> > > if your fix (which seems ok to me) was actually present in the mailing list
> > > posting of the patch that ended in the above mess.
> > 
> > Nope, no merge conflicts, source in original patch
> > https://lkml.org/lkml/2012/9/11/346
> > 
> > That mess completely harmless, this code is used only once on boot.
> > I don't have that email, so replying isn't trivial for me.
> 
> How bizarre. Also, Cyril doesn't work for TI anymore (his email is
> bouncing), so it's tricky to know what he meant here.
> 
> Your patch looks better than what we currently have though. Have you managed
> to test it on a keystone platform (I don't have one)?

Given that:

  Acked-by: Will Deacon <will.deacon@arm.com>

Will

Re: [PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Konstantin Khlebnikov]

On Mon, Jul 28, 2014 at 10:47 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Jul 28, 2014 at 07:40:58PM +0100, Konstantin Khlebnikov wrote:
>> On Mon, Jul 28, 2014 at 10:12 PM, Will Deacon <will.deacon@arm.com> wrote:
>> > On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
>> >> This patch fixes booting when idmap pgd lays above 4gb. Commit
>> >> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
>> >>
>> >> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
>> >> carry flag must be added to the upper part.
>> >>
>> >> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
>> >> Cc: Cyril Chemparathy <cyril@ti.com>
>> >> Cc: Vitaly Andrianov <vitalya@ti.com>
>> >> ---
>> >>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>> >>  1 file changed, 3 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>> >> index 22e3ad6..f0481dd 100644
>> >> --- a/arch/arm/mm/proc-v7-3level.S
>> >> +++ b/arch/arm/mm/proc-v7-3level.S
>> >> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>> >>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>> >>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>> >>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>> >> +     adcls   \tmp, \tmp, #0
>> >> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>> >>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>> >>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>> >> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
>> >
>> > I must admit, the code you are removing here looks really strange. Was there
>> > a badly resolved conflict somewhere along the way? It would be nice to see
>> > if your fix (which seems ok to me) was actually present in the mailing list
>> > posting of the patch that ended in the above mess.
>>
>> Nope, no merge conflicts, source in original patch
>> https://lkml.org/lkml/2012/9/11/346
>>
>> That mess completely harmless, this code is used only once on boot.
>> I don't have that email, so replying isn't trivial for me.
>
> How bizarre. Also, Cyril doesn't work for TI anymore (his email is
> bouncing), so it's tricky to know what he meant here.
>
> Your patch looks better than what we currently have though. Have you managed
> to test it on a keystone platform (I don't have one)?

No, I don't have it too. As well as I don't have direct access to the
platform where
problem was found. I've debugged this in patched qemu.


Also this commit has wrong assumptions about idmap pgd address.
(at least in the commit message)

Probably suspend/resume path is affected too. I'll try to check it.

commit f3db3f4389dbd9a8c2b4477f37a6ebddfd670ad8
Author: Mahesh Sivasubramanian <msivasub@codeaurora.org>
Date:   Fri Nov 8 23:25:20 2013 +0100

    ARM: 7885/1: Save/Restore 64-bit TTBR registers on LPAE suspend/resume

    LPAE enabled kernels use the 64-bit version of TTBR0 and TTBR1
    registers. If we're running an LPAE kernel, fill the upper half
    of TTBR0 with 0 because we're setting it to the idmap here (the
    idmap is guaranteed to be < 4Gb) and fully restore TTBR1 instead
    of just restoring the lower 32 bits. Failure to do so can cause
    failures on resume from suspend when these registers are only
    half restored.

    Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org>
    Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
    Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
    Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>


>
> Will

[PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Konstantin Khlebnikov]

On Mon, Jul 28, 2014 at 10:47 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Jul 28, 2014 at 07:40:58PM +0100, Konstantin Khlebnikov wrote:
>> On Mon, Jul 28, 2014 at 10:12 PM, Will Deacon <will.deacon@arm.com> wrote:
>> > On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
>> >> This patch fixes booting when idmap pgd lays above 4gb. Commit
>> >> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
>> >>
>> >> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
>> >> carry flag must be added to the upper part.
>> >>
>> >> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
>> >> Cc: Cyril Chemparathy <cyril@ti.com>
>> >> Cc: Vitaly Andrianov <vitalya@ti.com>
>> >> ---
>> >>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>> >>  1 file changed, 3 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>> >> index 22e3ad6..f0481dd 100644
>> >> --- a/arch/arm/mm/proc-v7-3level.S
>> >> +++ b/arch/arm/mm/proc-v7-3level.S
>> >> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>> >>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>> >>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>> >>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>> >> +     adcls   \tmp, \tmp, #0
>> >> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>> >>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>> >>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>> >> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
>> >
>> > I must admit, the code you are removing here looks really strange. Was there
>> > a badly resolved conflict somewhere along the way? It would be nice to see
>> > if your fix (which seems ok to me) was actually present in the mailing list
>> > posting of the patch that ended in the above mess.
>>
>> Nope, no merge conflicts, source in original patch
>> https://lkml.org/lkml/2012/9/11/346
>>
>> That mess completely harmless, this code is used only once on boot.
>> I don't have that email, so replying isn't trivial for me.
>
> How bizarre. Also, Cyril doesn't work for TI anymore (his email is
> bouncing), so it's tricky to know what he meant here.
>
> Your patch looks better than what we currently have though. Have you managed
> to test it on a keystone platform (I don't have one)?

No, I don't have it too. As well as I don't have direct access to the
platform where
problem was found. I've debugged this in patched qemu.


Also this commit has wrong assumptions about idmap pgd address.
(at least in the commit message)

Probably suspend/resume path is affected too. I'll try to check it.

commit f3db3f4389dbd9a8c2b4477f37a6ebddfd670ad8
Author: Mahesh Sivasubramanian <msivasub@codeaurora.org>
Date:   Fri Nov 8 23:25:20 2013 +0100

    ARM: 7885/1: Save/Restore 64-bit TTBR registers on LPAE suspend/resume

    LPAE enabled kernels use the 64-bit version of TTBR0 and TTBR1
    registers. If we're running an LPAE kernel, fill the upper half
    of TTBR0 with 0 because we're setting it to the idmap here (the
    idmap is guaranteed to be < 4Gb) and fully restore TTBR1 instead
    of just restoring the lower 32 bits. Failure to do so can cause
    failures on resume from suspend when these registers are only
    half restored.

    Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org>
    Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
    Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
    Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>


>
> Will

Re: [PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Konstantin Khlebnikov]

On Tue, Jul 29, 2014 at 2:57 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jul 28, 2014 at 11:57:16PM +0400, Konstantin Khlebnikov wrote:
>> On Mon, Jul 28, 2014 at 11:42 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Mon, Jul 28, 2014 at 11:29:39PM +0400, Konstantin Khlebnikov wrote:
>> >> Ok, before switching from identity mapping to normal mapping kernel must
>> >> switch instruction pointer from physical address to virtual.
>> >
>> > "switch instruction pointer from physical address to virtual."
>> >
>> > There's no such distinction for the instruction pointer.
>>
>> I know. I mean "logically".
>>
> ...
>>
>> Sorry but I'm really look so dumb? Maybe it's true, it's almost
>> midnight at my side.
>
> When you use language which suggests a lack of understanding, then
> I will explain things.
>
>> > It doesn't matter, provided the kernel text and data in the virtual
>> > address space are not overwritten by the identity mapping.  If it
>> > ends up in the vmalloc or IO space, that should not be a problem.
>>
>> In my case it's been overwritten.
>> And it always happens when PHYS_OFFSET >= PAGE_OFFSET
>> because in case of LPAE idmap always overwrites 1Gb at once.
>
> Right, I now see what you're getting at.  Here's a better description
> which I've used when committing your patch.  I've only taken patch 2
> at the present time.
>
>

Ok. Thank you for your time.

>
> On LPAE, each level 1 (pgd) page table entry maps 1GiB, and the level 2
> (pmd) entries map 2MiB.
>
> When the identity mapping is created on LPAE, the pgd pointers are copied
> from the swapper_pg_dir.  If we find that we need to modify the contents
> of a pmd, we allocate a new empty pmd table and insert it into the
> appropriate 1GB slot, before then filling it with the identity mapping.
>
> However, if the 1GB slot covers the kernel lowmem mappings, we obliterate
> those mappings.
>
> When replacing a PMD, first copy the old PMD contents to the new PMD, so
> that we preserve the existing mappings in the 1GiB region, particularly
> the mappings of the kernel itself.
>
>
> --
> FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
> according to speedtest.net.

[PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout

[Konstantin Khlebnikov]

On Tue, Jul 29, 2014 at 2:57 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jul 28, 2014 at 11:57:16PM +0400, Konstantin Khlebnikov wrote:
>> On Mon, Jul 28, 2014 at 11:42 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Mon, Jul 28, 2014 at 11:29:39PM +0400, Konstantin Khlebnikov wrote:
>> >> Ok, before switching from identity mapping to normal mapping kernel must
>> >> switch instruction pointer from physical address to virtual.
>> >
>> > "switch instruction pointer from physical address to virtual."
>> >
>> > There's no such distinction for the instruction pointer.
>>
>> I know. I mean "logically".
>>
> ...
>>
>> Sorry but I'm really look so dumb? Maybe it's true, it's almost
>> midnight at my side.
>
> When you use language which suggests a lack of understanding, then
> I will explain things.
>
>> > It doesn't matter, provided the kernel text and data in the virtual
>> > address space are not overwritten by the identity mapping.  If it
>> > ends up in the vmalloc or IO space, that should not be a problem.
>>
>> In my case it's been overwritten.
>> And it always happens when PHYS_OFFSET >= PAGE_OFFSET
>> because in case of LPAE idmap always overwrites 1Gb at once.
>
> Right, I now see what you're getting at.  Here's a better description
> which I've used when committing your patch.  I've only taken patch 2
> at the present time.
>
>

Ok. Thank you for your time.

>
> On LPAE, each level 1 (pgd) page table entry maps 1GiB, and the level 2
> (pmd) entries map 2MiB.
>
> When the identity mapping is created on LPAE, the pgd pointers are copied
> from the swapper_pg_dir.  If we find that we need to modify the contents
> of a pmd, we allocate a new empty pmd table and insert it into the
> appropriate 1GB slot, before then filling it with the identity mapping.
>
> However, if the 1GB slot covers the kernel lowmem mappings, we obliterate
> those mappings.
>
> When replacing a PMD, first copy the old PMD contents to the new PMD, so
> that we preserve the existing mappings in the 1GiB region, particularly
> the mappings of the kernel itself.
>
>
> --
> FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
> according to speedtest.net.

Re: [PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Konstantin Khlebnikov]

On Tue, Jul 22, 2014 at 7:36 PM, Konstantin Khlebnikov
<k.khlebnikov@samsung.com> wrote:
> This patch fixes booting when idmap pgd lays above 4gb. Commit
> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
>
> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
> carry flag must be added to the upper part.
>
> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
> Cc: Cyril Chemparathy <cyril@ti.com>
> Cc: Vitaly Andrianov <vitalya@ti.com>
> ---
>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
> index 22e3ad6..f0481dd 100644
> --- a/arch/arm/mm/proc-v7-3level.S
> +++ b/arch/arm/mm/proc-v7-3level.S
> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>         mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>         mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>         addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
> -       mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
> +       adcls   \tmp, \tmp, #0


Oops, this carry fixup is wrong. addls above doesn't set carry flag.


> +       mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>         mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>         mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
> -       mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
> -       mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
> -       mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
> +       mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
>         .endm
>
>         /*
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Konstantin Khlebnikov]

On Tue, Jul 22, 2014 at 7:36 PM, Konstantin Khlebnikov
<k.khlebnikov@samsung.com> wrote:
> This patch fixes booting when idmap pgd lays above 4gb. Commit
> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
>
> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
> carry flag must be added to the upper part.
>
> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
> Cc: Cyril Chemparathy <cyril@ti.com>
> Cc: Vitaly Andrianov <vitalya@ti.com>
> ---
>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
> index 22e3ad6..f0481dd 100644
> --- a/arch/arm/mm/proc-v7-3level.S
> +++ b/arch/arm/mm/proc-v7-3level.S
> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>         mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>         mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>         addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
> -       mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
> +       adcls   \tmp, \tmp, #0


Oops, this carry fixup is wrong. addls above doesn't set carry flag.


> +       mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>         mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>         mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
> -       mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
> -       mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
> -       mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
> +       mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
>         .endm
>
>         /*
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Jassi Brar]

On Tue, Jul 29, 2014 at 5:59 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
> On Mon, Jul 28, 2014 at 10:47 PM, Will Deacon <will.deacon@arm.com> wrote:
>> On Mon, Jul 28, 2014 at 07:40:58PM +0100, Konstantin Khlebnikov wrote:
>>> On Mon, Jul 28, 2014 at 10:12 PM, Will Deacon <will.deacon@arm.com> wrote:
>>> > On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
>>> >> This patch fixes booting when idmap pgd lays above 4gb. Commit
>>> >> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
>>> >>
>>> >> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
>>> >> carry flag must be added to the upper part.
>>> >>
>>> >> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
>>> >> Cc: Cyril Chemparathy <cyril@ti.com>
>>> >> Cc: Vitaly Andrianov <vitalya@ti.com>
>>> >> ---
>>> >>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>>> >>  1 file changed, 3 insertions(+), 4 deletions(-)
>>> >>
>>> >> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>>> >> index 22e3ad6..f0481dd 100644
>>> >> --- a/arch/arm/mm/proc-v7-3level.S
>>> >> +++ b/arch/arm/mm/proc-v7-3level.S
>>> >> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>>> >>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>> >>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>>> >>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>> >> +     adcls   \tmp, \tmp, #0
>>> >> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>>> >>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>> >>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>> >> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
>>> >
>>> > I must admit, the code you are removing here looks really strange. Was there
>>> > a badly resolved conflict somewhere along the way? It would be nice to see
>>> > if your fix (which seems ok to me) was actually present in the mailing list
>>> > posting of the patch that ended in the above mess.
>>>
>>> Nope, no merge conflicts, source in original patch
>>> https://lkml.org/lkml/2012/9/11/346
>>>
>>> That mess completely harmless, this code is used only once on boot.
>>> I don't have that email, so replying isn't trivial for me.
>>
>> How bizarre. Also, Cyril doesn't work for TI anymore (his email is
>> bouncing), so it's tricky to know what he meant here.
>>
>> Your patch looks better than what we currently have though. Have you managed
>> to test it on a keystone platform (I don't have one)?
>
> No, I don't have it too. As well as I don't have direct access to the
> platform where
> problem was found. I've debugged this in patched qemu.
>
It seems the patch wasn't tested on any real platform, I did on my
CA15 based platform - it breaks boot. Simply reverting the patch gets
it going again. I am happy to try any fix.

Thanks
Jassi

[PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Jassi Brar]

On Tue, Jul 29, 2014 at 5:59 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
> On Mon, Jul 28, 2014 at 10:47 PM, Will Deacon <will.deacon@arm.com> wrote:
>> On Mon, Jul 28, 2014 at 07:40:58PM +0100, Konstantin Khlebnikov wrote:
>>> On Mon, Jul 28, 2014 at 10:12 PM, Will Deacon <will.deacon@arm.com> wrote:
>>> > On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
>>> >> This patch fixes booting when idmap pgd lays above 4gb. Commit
>>> >> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
>>> >>
>>> >> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
>>> >> carry flag must be added to the upper part.
>>> >>
>>> >> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
>>> >> Cc: Cyril Chemparathy <cyril@ti.com>
>>> >> Cc: Vitaly Andrianov <vitalya@ti.com>
>>> >> ---
>>> >>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>>> >>  1 file changed, 3 insertions(+), 4 deletions(-)
>>> >>
>>> >> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>>> >> index 22e3ad6..f0481dd 100644
>>> >> --- a/arch/arm/mm/proc-v7-3level.S
>>> >> +++ b/arch/arm/mm/proc-v7-3level.S
>>> >> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>>> >>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>> >>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>>> >>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>> >> +     adcls   \tmp, \tmp, #0
>>> >> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>>> >>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>> >>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>> >> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
>>> >
>>> > I must admit, the code you are removing here looks really strange. Was there
>>> > a badly resolved conflict somewhere along the way? It would be nice to see
>>> > if your fix (which seems ok to me) was actually present in the mailing list
>>> > posting of the patch that ended in the above mess.
>>>
>>> Nope, no merge conflicts, source in original patch
>>> https://lkml.org/lkml/2012/9/11/346
>>>
>>> That mess completely harmless, this code is used only once on boot.
>>> I don't have that email, so replying isn't trivial for me.
>>
>> How bizarre. Also, Cyril doesn't work for TI anymore (his email is
>> bouncing), so it's tricky to know what he meant here.
>>
>> Your patch looks better than what we currently have though. Have you managed
>> to test it on a keystone platform (I don't have one)?
>
> No, I don't have it too. As well as I don't have direct access to the
> platform where
> problem was found. I've debugged this in patched qemu.
>
It seems the patch wasn't tested on any real platform, I did on my
CA15 based platform - it breaks boot. Simply reverting the patch gets
it going again. I am happy to try any fix.

Thanks
Jassi

Re: [PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Konstantin Khlebnikov]

On Wed, Aug 27, 2014 at 7:26 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> On Tue, Jul 29, 2014 at 5:59 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>> On Mon, Jul 28, 2014 at 10:47 PM, Will Deacon <will.deacon@arm.com> wrote:
>>> On Mon, Jul 28, 2014 at 07:40:58PM +0100, Konstantin Khlebnikov wrote:
>>>> On Mon, Jul 28, 2014 at 10:12 PM, Will Deacon <will.deacon@arm.com> wrote:
>>>> > On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
>>>> >> This patch fixes booting when idmap pgd lays above 4gb. Commit
>>>> >> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
>>>> >>
>>>> >> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
>>>> >> carry flag must be added to the upper part.
>>>> >>
>>>> >> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
>>>> >> Cc: Cyril Chemparathy <cyril@ti.com>
>>>> >> Cc: Vitaly Andrianov <vitalya@ti.com>
>>>> >> ---
>>>> >>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>>>> >>  1 file changed, 3 insertions(+), 4 deletions(-)
>>>> >>
>>>> >> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>>>> >> index 22e3ad6..f0481dd 100644
>>>> >> --- a/arch/arm/mm/proc-v7-3level.S
>>>> >> +++ b/arch/arm/mm/proc-v7-3level.S
>>>> >> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>>>> >>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>> >>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>> >>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
>>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>>> >> +     adcls   \tmp, \tmp, #0
>>>> >> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>>>> >>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>> >>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>>> >> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
>>>> >
>>>> > I must admit, the code you are removing here looks really strange. Was there
>>>> > a badly resolved conflict somewhere along the way? It would be nice to see
>>>> > if your fix (which seems ok to me) was actually present in the mailing list
>>>> > posting of the patch that ended in the above mess.
>>>>
>>>> Nope, no merge conflicts, source in original patch
>>>> https://lkml.org/lkml/2012/9/11/346
>>>>
>>>> That mess completely harmless, this code is used only once on boot.
>>>> I don't have that email, so replying isn't trivial for me.
>>>
>>> How bizarre. Also, Cyril doesn't work for TI anymore (his email is
>>> bouncing), so it's tricky to know what he meant here.
>>>
>>> Your patch looks better than what we currently have though. Have you managed
>>> to test it on a keystone platform (I don't have one)?
>>
>> No, I don't have it too. As well as I don't have direct access to the
>> platform where
>> problem was found. I've debugged this in patched qemu.
>>
> It seems the patch wasn't tested on any real platform, I did on my
> CA15 based platform - it breaks boot. Simply reverting the patch gets
> it going again. I am happy to try any fix.
>

Please try to remove adcls line.
Seems like this affects only systems where PHYS_OFFSET == PAGE_OFFSET.

--- a/arch/arm/mm/proc-v7-3level.S
+++ b/arch/arm/mm/proc-v7-3level.S
@@ -146,7 +146,6 @@ ENDPROC(cpu_v7_set_pte_ext)
        mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
        mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
        addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
-       adcls   \tmp, \tmp, #0
        mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
        mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
        mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits


> Thanks
> Jassi

[PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Konstantin Khlebnikov]

On Wed, Aug 27, 2014 at 7:26 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> On Tue, Jul 29, 2014 at 5:59 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>> On Mon, Jul 28, 2014 at 10:47 PM, Will Deacon <will.deacon@arm.com> wrote:
>>> On Mon, Jul 28, 2014 at 07:40:58PM +0100, Konstantin Khlebnikov wrote:
>>>> On Mon, Jul 28, 2014 at 10:12 PM, Will Deacon <will.deacon@arm.com> wrote:
>>>> > On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
>>>> >> This patch fixes booting when idmap pgd lays above 4gb. Commit
>>>> >> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
>>>> >>
>>>> >> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
>>>> >> carry flag must be added to the upper part.
>>>> >>
>>>> >> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
>>>> >> Cc: Cyril Chemparathy <cyril@ti.com>
>>>> >> Cc: Vitaly Andrianov <vitalya@ti.com>
>>>> >> ---
>>>> >>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>>>> >>  1 file changed, 3 insertions(+), 4 deletions(-)
>>>> >>
>>>> >> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>>>> >> index 22e3ad6..f0481dd 100644
>>>> >> --- a/arch/arm/mm/proc-v7-3level.S
>>>> >> +++ b/arch/arm/mm/proc-v7-3level.S
>>>> >> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>>>> >>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>> >>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>> >>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
>>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>>> >> +     adcls   \tmp, \tmp, #0
>>>> >> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>>>> >>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>> >>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>>> >> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
>>>> >
>>>> > I must admit, the code you are removing here looks really strange. Was there
>>>> > a badly resolved conflict somewhere along the way? It would be nice to see
>>>> > if your fix (which seems ok to me) was actually present in the mailing list
>>>> > posting of the patch that ended in the above mess.
>>>>
>>>> Nope, no merge conflicts, source in original patch
>>>> https://lkml.org/lkml/2012/9/11/346
>>>>
>>>> That mess completely harmless, this code is used only once on boot.
>>>> I don't have that email, so replying isn't trivial for me.
>>>
>>> How bizarre. Also, Cyril doesn't work for TI anymore (his email is
>>> bouncing), so it's tricky to know what he meant here.
>>>
>>> Your patch looks better than what we currently have though. Have you managed
>>> to test it on a keystone platform (I don't have one)?
>>
>> No, I don't have it too. As well as I don't have direct access to the
>> platform where
>> problem was found. I've debugged this in patched qemu.
>>
> It seems the patch wasn't tested on any real platform, I did on my
> CA15 based platform - it breaks boot. Simply reverting the patch gets
> it going again. I am happy to try any fix.
>

Please try to remove adcls line.
Seems like this affects only systems where PHYS_OFFSET == PAGE_OFFSET.

--- a/arch/arm/mm/proc-v7-3level.S
+++ b/arch/arm/mm/proc-v7-3level.S
@@ -146,7 +146,6 @@ ENDPROC(cpu_v7_set_pte_ext)
        mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
        mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
        addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
-       adcls   \tmp, \tmp, #0
        mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
        mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
        mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits


> Thanks
> Jassi

Re: [PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Konstantin Khlebnikov]

On Wed, Aug 27, 2014 at 7:31 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
> On Wed, Aug 27, 2014 at 7:26 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>> On Tue, Jul 29, 2014 at 5:59 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>>> On Mon, Jul 28, 2014 at 10:47 PM, Will Deacon <will.deacon@arm.com> wrote:
>>>> On Mon, Jul 28, 2014 at 07:40:58PM +0100, Konstantin Khlebnikov wrote:
>>>>> On Mon, Jul 28, 2014 at 10:12 PM, Will Deacon <will.deacon@arm.com> wrote:
>>>>> > On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
>>>>> >> This patch fixes booting when idmap pgd lays above 4gb. Commit
>>>>> >> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
>>>>> >>
>>>>> >> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
>>>>> >> carry flag must be added to the upper part.
>>>>> >>
>>>>> >> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
>>>>> >> Cc: Cyril Chemparathy <cyril@ti.com>
>>>>> >> Cc: Vitaly Andrianov <vitalya@ti.com>
>>>>> >> ---
>>>>> >>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>>>>> >>  1 file changed, 3 insertions(+), 4 deletions(-)
>>>>> >>
>>>>> >> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>>>>> >> index 22e3ad6..f0481dd 100644
>>>>> >> --- a/arch/arm/mm/proc-v7-3level.S
>>>>> >> +++ b/arch/arm/mm/proc-v7-3level.S
>>>>> >> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>>>>> >>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>>> >>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>>> >>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
>>>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>>>> >> +     adcls   \tmp, \tmp, #0
>>>>> >> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>>>>> >>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>>> >>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>>>> >> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
>>>>> >
>>>>> > I must admit, the code you are removing here looks really strange. Was there
>>>>> > a badly resolved conflict somewhere along the way? It would be nice to see
>>>>> > if your fix (which seems ok to me) was actually present in the mailing list
>>>>> > posting of the patch that ended in the above mess.
>>>>>
>>>>> Nope, no merge conflicts, source in original patch
>>>>> https://lkml.org/lkml/2012/9/11/346
>>>>>
>>>>> That mess completely harmless, this code is used only once on boot.
>>>>> I don't have that email, so replying isn't trivial for me.
>>>>
>>>> How bizarre. Also, Cyril doesn't work for TI anymore (his email is
>>>> bouncing), so it's tricky to know what he meant here.
>>>>
>>>> Your patch looks better than what we currently have though. Have you managed
>>>> to test it on a keystone platform (I don't have one)?
>>>
>>> No, I don't have it too. As well as I don't have direct access to the
>>> platform where
>>> problem was found. I've debugged this in patched qemu.
>>>
>> It seems the patch wasn't tested on any real platform, I did on my
>> CA15 based platform - it breaks boot. Simply reverting the patch gets
>> it going again. I am happy to try any fix.
>>
>
> Please try to remove adcls line.
> Seems like this affects only systems where PHYS_OFFSET == PAGE_OFFSET.
>
> --- a/arch/arm/mm/proc-v7-3level.S
> +++ b/arch/arm/mm/proc-v7-3level.S
> @@ -146,7 +146,6 @@ ENDPROC(cpu_v7_set_pte_ext)
>         mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>         mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>         addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
> -       adcls   \tmp, \tmp, #0
>         mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>         mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>         mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>
>

...
I've dropped cyril@ti.com from CC, his email address is dead.


>> Thanks
>> Jassi

[PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Konstantin Khlebnikov]

On Wed, Aug 27, 2014 at 7:31 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
> On Wed, Aug 27, 2014 at 7:26 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>> On Tue, Jul 29, 2014 at 5:59 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>>> On Mon, Jul 28, 2014 at 10:47 PM, Will Deacon <will.deacon@arm.com> wrote:
>>>> On Mon, Jul 28, 2014 at 07:40:58PM +0100, Konstantin Khlebnikov wrote:
>>>>> On Mon, Jul 28, 2014 at 10:12 PM, Will Deacon <will.deacon@arm.com> wrote:
>>>>> > On Tue, Jul 22, 2014 at 04:36:23PM +0100, Konstantin Khlebnikov wrote:
>>>>> >> This patch fixes booting when idmap pgd lays above 4gb. Commit
>>>>> >> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
>>>>> >>
>>>>> >> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
>>>>> >> carry flag must be added to the upper part.
>>>>> >>
>>>>> >> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
>>>>> >> Cc: Cyril Chemparathy <cyril@ti.com>
>>>>> >> Cc: Vitaly Andrianov <vitalya@ti.com>
>>>>> >> ---
>>>>> >>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>>>>> >>  1 file changed, 3 insertions(+), 4 deletions(-)
>>>>> >>
>>>>> >> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>>>>> >> index 22e3ad6..f0481dd 100644
>>>>> >> --- a/arch/arm/mm/proc-v7-3level.S
>>>>> >> +++ b/arch/arm/mm/proc-v7-3level.S
>>>>> >> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>>>>> >>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>>> >>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>>> >>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
>>>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>>>> >> +     adcls   \tmp, \tmp, #0
>>>>> >> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>>>>> >>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>>> >>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>>>> >> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
>>>>> >
>>>>> > I must admit, the code you are removing here looks really strange. Was there
>>>>> > a badly resolved conflict somewhere along the way? It would be nice to see
>>>>> > if your fix (which seems ok to me) was actually present in the mailing list
>>>>> > posting of the patch that ended in the above mess.
>>>>>
>>>>> Nope, no merge conflicts, source in original patch
>>>>> https://lkml.org/lkml/2012/9/11/346
>>>>>
>>>>> That mess completely harmless, this code is used only once on boot.
>>>>> I don't have that email, so replying isn't trivial for me.
>>>>
>>>> How bizarre. Also, Cyril doesn't work for TI anymore (his email is
>>>> bouncing), so it's tricky to know what he meant here.
>>>>
>>>> Your patch looks better than what we currently have though. Have you managed
>>>> to test it on a keystone platform (I don't have one)?
>>>
>>> No, I don't have it too. As well as I don't have direct access to the
>>> platform where
>>> problem was found. I've debugged this in patched qemu.
>>>
>> It seems the patch wasn't tested on any real platform, I did on my
>> CA15 based platform - it breaks boot. Simply reverting the patch gets
>> it going again. I am happy to try any fix.
>>
>
> Please try to remove adcls line.
> Seems like this affects only systems where PHYS_OFFSET == PAGE_OFFSET.
>
> --- a/arch/arm/mm/proc-v7-3level.S
> +++ b/arch/arm/mm/proc-v7-3level.S
> @@ -146,7 +146,6 @@ ENDPROC(cpu_v7_set_pte_ext)
>         mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>         mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>         addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
> -       adcls   \tmp, \tmp, #0
>         mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>         mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>         mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>
>

...
I've dropped cyril at ti.com from CC, his email address is dead.


>> Thanks
>> Jassi

Re: [PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Jassi Brar]

On Wed, Aug 27, 2014 at 9:01 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
> On Wed, Aug 27, 2014 at 7:26 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>> On Tue, Jul 29, 2014 at 5:59 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:

>>>>> >> This patch fixes booting when idmap pgd lays above 4gb. Commit
>>>>> >> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
>>>>> >>
>>>>> >> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
>>>>> >> carry flag must be added to the upper part.
>>>>> >>
>>>>> >> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
>>>>> >> Cc: Cyril Chemparathy <cyril@ti.com>
>>>>> >> Cc: Vitaly Andrianov <vitalya@ti.com>
>>>>> >> ---
>>>>> >>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>>>>> >>  1 file changed, 3 insertions(+), 4 deletions(-)
>>>>> >>
>>>>> >> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>>>>> >> index 22e3ad6..f0481dd 100644
>>>>> >> --- a/arch/arm/mm/proc-v7-3level.S
>>>>> >> +++ b/arch/arm/mm/proc-v7-3level.S
>>>>> >> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>>>>> >>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>>> >>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>>> >>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
>>>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>>>> >> +     adcls   \tmp, \tmp, #0
>>>>> >> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>>>>> >>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>>> >>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>>>> >> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
>>>>> >
>>>>> > I must admit, the code you are removing here looks really strange. Was there
>>>>> > a badly resolved conflict somewhere along the way? It would be nice to see
>>>>> > if your fix (which seems ok to me) was actually present in the mailing list
>>>>> > posting of the patch that ended in the above mess.
>>>>>
>>>>> Nope, no merge conflicts, source in original patch
>>>>> https://lkml.org/lkml/2012/9/11/346
>>>>>
>>>>> That mess completely harmless, this code is used only once on boot.
>>>>> I don't have that email, so replying isn't trivial for me.
>>>>
>>>> How bizarre. Also, Cyril doesn't work for TI anymore (his email is
>>>> bouncing), so it's tricky to know what he meant here.
>>>>
>>>> Your patch looks better than what we currently have though. Have you managed
>>>> to test it on a keystone platform (I don't have one)?
>>>
>>> No, I don't have it too. As well as I don't have direct access to the
>>> platform where
>>> problem was found. I've debugged this in patched qemu.
>>>
>> It seems the patch wasn't tested on any real platform, I did on my
>> CA15 based platform - it breaks boot. Simply reverting the patch gets
>> it going again. I am happy to try any fix.
>>
>
> Please try to remove adcls line.
> Seems like this affects only systems where PHYS_OFFSET == PAGE_OFFSET.
>
> --- a/arch/arm/mm/proc-v7-3level.S
> +++ b/arch/arm/mm/proc-v7-3level.S
> @@ -146,7 +146,6 @@ ENDPROC(cpu_v7_set_pte_ext)
>         mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>         mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>         addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
> -       adcls   \tmp, \tmp, #0
>         mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>         mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>         mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>
Yup, restores boot.

Thanks
Jassi

[PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Jassi Brar]

On Wed, Aug 27, 2014 at 9:01 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
> On Wed, Aug 27, 2014 at 7:26 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>> On Tue, Jul 29, 2014 at 5:59 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:

>>>>> >> This patch fixes booting when idmap pgd lays above 4gb. Commit
>>>>> >> 4756dcbfd37 mostly had fixed this, but it'd failed to load upper bits.
>>>>> >>
>>>>> >> Also this fixes adding TTBR1_OFFSET to TTRR1: if lower part overflows
>>>>> >> carry flag must be added to the upper part.
>>>>> >>
>>>>> >> Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
>>>>> >> Cc: Cyril Chemparathy <cyril@ti.com>
>>>>> >> Cc: Vitaly Andrianov <vitalya@ti.com>
>>>>> >> ---
>>>>> >>  arch/arm/mm/proc-v7-3level.S |    7 +++----
>>>>> >>  1 file changed, 3 insertions(+), 4 deletions(-)
>>>>> >>
>>>>> >> diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
>>>>> >> index 22e3ad6..f0481dd 100644
>>>>> >> --- a/arch/arm/mm/proc-v7-3level.S
>>>>> >> +++ b/arch/arm/mm/proc-v7-3level.S
>>>>> >> @@ -140,12 +140,11 @@ ENDPROC(cpu_v7_set_pte_ext)
>>>>> >>       mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>>> >>       mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>>> >>       addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
>>>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>>>> >> +     adcls   \tmp, \tmp, #0
>>>>> >> +     mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>>>>> >>       mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>>> >>       mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>>>> >> -     mcrr    p15, 1, \ttbr1, \zero, c2                       @ load TTBR1
>>>>> >> -     mcrr    p15, 0, \ttbr0, \zero, c2                       @ load TTBR0
>>>>> >> +     mcrr    p15, 0, \ttbr0, \tmp, c2                        @ load TTBR0
>>>>> >
>>>>> > I must admit, the code you are removing here looks really strange. Was there
>>>>> > a badly resolved conflict somewhere along the way? It would be nice to see
>>>>> > if your fix (which seems ok to me) was actually present in the mailing list
>>>>> > posting of the patch that ended in the above mess.
>>>>>
>>>>> Nope, no merge conflicts, source in original patch
>>>>> https://lkml.org/lkml/2012/9/11/346
>>>>>
>>>>> That mess completely harmless, this code is used only once on boot.
>>>>> I don't have that email, so replying isn't trivial for me.
>>>>
>>>> How bizarre. Also, Cyril doesn't work for TI anymore (his email is
>>>> bouncing), so it's tricky to know what he meant here.
>>>>
>>>> Your patch looks better than what we currently have though. Have you managed
>>>> to test it on a keystone platform (I don't have one)?
>>>
>>> No, I don't have it too. As well as I don't have direct access to the
>>> platform where
>>> problem was found. I've debugged this in patched qemu.
>>>
>> It seems the patch wasn't tested on any real platform, I did on my
>> CA15 based platform - it breaks boot. Simply reverting the patch gets
>> it going again. I am happy to try any fix.
>>
>
> Please try to remove adcls line.
> Seems like this affects only systems where PHYS_OFFSET == PAGE_OFFSET.
>
> --- a/arch/arm/mm/proc-v7-3level.S
> +++ b/arch/arm/mm/proc-v7-3level.S
> @@ -146,7 +146,6 @@ ENDPROC(cpu_v7_set_pte_ext)
>         mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>         mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>         addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
> -       adcls   \tmp, \tmp, #0
>         mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>         mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>         mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>
Yup, restores boot.

Thanks
Jassi

Re: [PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Will Deacon]

On Wed, Aug 27, 2014 at 04:45:24PM +0100, Jassi Brar wrote:
> On Wed, Aug 27, 2014 at 9:01 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
> > On Wed, Aug 27, 2014 at 7:26 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> >> It seems the patch wasn't tested on any real platform, I did on my
> >> CA15 based platform - it breaks boot. Simply reverting the patch gets
> >> it going again. I am happy to try any fix.
> >>
> >
> > Please try to remove adcls line.
> > Seems like this affects only systems where PHYS_OFFSET == PAGE_OFFSET.
> >
> > --- a/arch/arm/mm/proc-v7-3level.S
> > +++ b/arch/arm/mm/proc-v7-3level.S
> > @@ -146,7 +146,6 @@ ENDPROC(cpu_v7_set_pte_ext)
> >         mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
> >         mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
> >         addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
> > -       adcls   \tmp, \tmp, #0
> >         mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
> >         mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
> >         mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
> >
> Yup, restores boot.

Can somebody write this up as a proper patch and put it in the patch system
please?

Will

[PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Will Deacon]

On Wed, Aug 27, 2014 at 04:45:24PM +0100, Jassi Brar wrote:
> On Wed, Aug 27, 2014 at 9:01 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
> > On Wed, Aug 27, 2014 at 7:26 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> >> It seems the patch wasn't tested on any real platform, I did on my
> >> CA15 based platform - it breaks boot. Simply reverting the patch gets
> >> it going again. I am happy to try any fix.
> >>
> >
> > Please try to remove adcls line.
> > Seems like this affects only systems where PHYS_OFFSET == PAGE_OFFSET.
> >
> > --- a/arch/arm/mm/proc-v7-3level.S
> > +++ b/arch/arm/mm/proc-v7-3level.S
> > @@ -146,7 +146,6 @@ ENDPROC(cpu_v7_set_pte_ext)
> >         mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
> >         mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
> >         addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
> > -       adcls   \tmp, \tmp, #0
> >         mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
> >         mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
> >         mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
> >
> Yup, restores boot.

Can somebody write this up as a proper patch and put it in the patch system
please?

Will

Re: [PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Konstantin Khlebnikov]

[-- Attachment #1: Type: text/plain, Size: 1421 bytes --]


On 2014-08-28 15:03, Will Deacon wrote:
> On Wed, Aug 27, 2014 at 04:45:24PM +0100, Jassi Brar wrote:
>> On Wed, Aug 27, 2014 at 9:01 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>>> On Wed, Aug 27, 2014 at 7:26 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>>>> It seems the patch wasn't tested on any real platform, I did on my
>>>> CA15 based platform - it breaks boot. Simply reverting the patch gets
>>>> it going again. I am happy to try any fix.
>>>>
>>> Please try to remove adcls line.
>>> Seems like this affects only systems where PHYS_OFFSET == PAGE_OFFSET.
>>>
>>> --- a/arch/arm/mm/proc-v7-3level.S
>>> +++ b/arch/arm/mm/proc-v7-3level.S
>>> @@ -146,7 +146,6 @@ ENDPROC(cpu_v7_set_pte_ext)
>>>          mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>          mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>          addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
>>> -       adcls   \tmp, \tmp, #0
>>>          mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>>>          mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>          mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>
>> Yup, restores boot.
> Can somebody write this up as a proper patch and put it in the patch system
> please?

Ok. Done.

http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8132/1

>
> Will
>


[-- Attachment #2: arm-lpae-remove-carry-flag-correction-after-adding-ttbr1_offset --]
[-- Type: text/plain, Size: 1481 bytes --]

ARM: LPAE: drop wrong carry flag correction after adding TTBR1_OFFSET

From: Konstantin Khlebnikov <k.khlebnikov@samsung.com>

In commit 7fb00c2fca4b6c58be521eb3676cf4b4ba8dde3b ("ARM: 8114/1: LPAE:
load upper bits of early TTBR0/TTBR1") part which fixes carrying in adding
TTBR1_OFFSET to TTRR1 was wrong:

	addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
	adcls   \tmp, \tmp, #0

addls doesn't update flags, adcls adds carry from cmp above:

	cmp	\ttbr1, \tmp			@ PHYS_OFFSET > PAGE_OFFSET?

Condition 'ls' means carry flag is clear or zero flag is set, thus only one
case is affected: when PHYS_OFFSET == PAGE_OFFSET.

It seems safer to remove this fixup. Bug is here for ages and nobody
complained. Let's fix it separately.

Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
Reported-and-tested-by: Jassi Brar <jassisinghbrar@gmail.com>
---
 arch/arm/mm/proc-v7-3level.S |    1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
index 1a24e92..b64e67c 100644
--- a/arch/arm/mm/proc-v7-3level.S
+++ b/arch/arm/mm/proc-v7-3level.S
@@ -146,7 +146,6 @@ ENDPROC(cpu_v7_set_pte_ext)
 	mov	\tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
 	mov	\ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT		@ lower bits
 	addls	\ttbr1, \ttbr1, #TTBR1_OFFSET
-	adcls	\tmp, \tmp, #0
 	mcrr	p15, 1, \ttbr1, \tmp, c2			@ load TTBR1
 	mov	\tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
 	mov	\ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT		@ lower bits

[PATCH 1/2] ARM: LPAE: load upper bits of early TTBR0/TTBR1

[Konstantin Khlebnikov]

On 2014-08-28 15:03, Will Deacon wrote:
> On Wed, Aug 27, 2014 at 04:45:24PM +0100, Jassi Brar wrote:
>> On Wed, Aug 27, 2014 at 9:01 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>>> On Wed, Aug 27, 2014 at 7:26 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>>>> It seems the patch wasn't tested on any real platform, I did on my
>>>> CA15 based platform - it breaks boot. Simply reverting the patch gets
>>>> it going again. I am happy to try any fix.
>>>>
>>> Please try to remove adcls line.
>>> Seems like this affects only systems where PHYS_OFFSET == PAGE_OFFSET.
>>>
>>> --- a/arch/arm/mm/proc-v7-3level.S
>>> +++ b/arch/arm/mm/proc-v7-3level.S
>>> @@ -146,7 +146,6 @@ ENDPROC(cpu_v7_set_pte_ext)
>>>          mov     \tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>          mov     \ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>          addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
>>> -       adcls   \tmp, \tmp, #0
>>>          mcrr    p15, 1, \ttbr1, \tmp, c2                        @ load TTBR1
>>>          mov     \tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)        @ upper bits
>>>          mov     \ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT             @ lower bits
>>>
>> Yup, restores boot.
> Can somebody write this up as a proper patch and put it in the patch system
> please?

Ok. Done.

http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8132/1

>
> Will
>

-------------- next part --------------
ARM: LPAE: drop wrong carry flag correction after adding TTBR1_OFFSET

From: Konstantin Khlebnikov <k.khlebnikov@samsung.com>

In commit 7fb00c2fca4b6c58be521eb3676cf4b4ba8dde3b ("ARM: 8114/1: LPAE:
load upper bits of early TTBR0/TTBR1") part which fixes carrying in adding
TTBR1_OFFSET to TTRR1 was wrong:

	addls   \ttbr1, \ttbr1, #TTBR1_OFFSET
	adcls   \tmp, \tmp, #0

addls doesn't update flags, adcls adds carry from cmp above:

	cmp	\ttbr1, \tmp			@ PHYS_OFFSET > PAGE_OFFSET?

Condition 'ls' means carry flag is clear or zero flag is set, thus only one
case is affected: when PHYS_OFFSET == PAGE_OFFSET.

It seems safer to remove this fixup. Bug is here for ages and nobody
complained. Let's fix it separately.

Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
Reported-and-tested-by: Jassi Brar <jassisinghbrar@gmail.com>
---
 arch/arm/mm/proc-v7-3level.S |    1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S
index 1a24e92..b64e67c 100644
--- a/arch/arm/mm/proc-v7-3level.S
+++ b/arch/arm/mm/proc-v7-3level.S
@@ -146,7 +146,6 @@ ENDPROC(cpu_v7_set_pte_ext)
 	mov	\tmp, \ttbr1, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
 	mov	\ttbr1, \ttbr1, lsl #ARCH_PGD_SHIFT		@ lower bits
 	addls	\ttbr1, \ttbr1, #TTBR1_OFFSET
-	adcls	\tmp, \tmp, #0
 	mcrr	p15, 1, \ttbr1, \tmp, c2			@ load TTBR1
 	mov	\tmp, \ttbr0, lsr #(32 - ARCH_PGD_SHIFT)	@ upper bits
 	mov	\ttbr0, \ttbr0, lsl #ARCH_PGD_SHIFT		@ lower bits