ARM: LPAE: load upper bits of early TTBR0/TTBR1
PATCH 1/2 ARM: LPAE: load upper bits of early TTBR0/TTBR1
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.
--- arch/arm/mm/proc-v7-3level.S | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
arch/arm/mm/proc-v7-3level.S +3 -4
@@ -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
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.
--- arch/arm/mm/idmap.c | 3 +++ 1 file changed, 3 insertions(+)
arch/arm/mm/idmap.c +3 -0
@@ -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 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout
Quoted 9 lines · 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.
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
Quoted 7 lines · Will Deacon
Quoted 6 lines · 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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 7 lines
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
Quoted 18 lines · Konstantin Khlebnikov
Quoted 7 lines · Will Deacon
Quoted 6 lines · 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.Quoted 7 lines
Quoted 3 lines
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.
Quoted 7 lines
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
0xE8000000or even0xF2000000evenything 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 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout
Quoted 24 lines · Will Deacon
Quoted 23 lines · Konstantin Khlebnikov
Quoted 7 lines · Will Deacon
Quoted 6 lines · 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.Quoted 7 lines
Quoted 3 lines
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 |
Quoted 8 lines
Quoted 2 lines
But when that physical memory chunk starts from
0xE8000000or even0xF2000000evenything becomes very complicated.Why? As long as we don't clobber the kernel text (which would require
PHYS_OFFSETto be at a really weird alignment and very close toPAGE_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.
Quoted 3 lines
soft_restartis an example of code that deals with these issues. Which code is causing you problems?
That was booting of secondary cpus, all of them.
Quoted 2 lines
Will
Re: [PATCH 2/2] ARM: LPAE: reduce damage caused by idmap to virtual memory layout
Quoted 34 lines · Konstantin Khlebnikov
Quoted 24 lines · Will Deacon
Quoted 23 lines · Konstantin Khlebnikov
Quoted 7 lines · Will Deacon
Quoted 6 lines · 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.Quoted 7 lines
Quoted 3 lines
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 |Quoted 8 lines
Quoted 2 lines
But when that physical memory chunk starts from
0xE8000000or even0xF2000000evenything becomes very complicated.Why? As long as we don't clobber the kernel text (which would require
PHYS_OFFSETto be at a really weird alignment and very close toPAGE_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.
Quoted 6 lines
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_OFFSETso 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).
Quoted 11 lines
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_OFFSETisn't alligned to 128mb which is strange.
So, it seems there is plenty bugs anound.Quoted 3 lines
soft_restartis 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
In reply to Konstantin KhlebnikovQuoted 2 lines · Konstantin Khlebnikov
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.
Quoted 3 lines
Moreover, I had some experiments with
mapping ram to random places in qemu and seen that kernel cannot boot ifPHYS_OFFSETisn'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.
Signature · 2 lines
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
Quoted 18 lines · Russell King - ARM Linux
Quoted 2 lines · Konstantin Khlebnikov
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.
Quoted 7 lines
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.
Quoted 14 lines
Quoted 4 lines
Moreover, I had some experiments with
mapping ram to random places in qemu and seen that kernel cannot boot ifPHYS_OFFSETisn't alligned to 128mb which is strange.That is intentional.
PHYS_OFFSEThas 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
Quoted 34 lines · Konstantin Khlebnikov
Quoted 18 lines · Russell King - ARM Linux
Quoted 2 lines · Konstantin Khlebnikov
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.
Quoted 7 lines
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_switchedin my case. And both physical source and virtual destination addresses must present in one same mapping (idmap).
Quoted 6 lines
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
Quoted 19 lines
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.
Quoted 14 lines
Quoted 4 lines
Moreover, I had some experiments with
mapping ram to random places in qemu and seen that kernel cannot boot ifPHYS_OFFSETisn't alligned to 128mb which is strange.That is intentional.
PHYS_OFFSEThas 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
In reply to Konstantin KhlebnikovQuoted 2 lines · Konstantin Khlebnikov
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.
Quoted 4 lines
It's a long jump
out idmap code section to some normal kernel function.__secondary_switchedin my case. And both physical source and virtual destination addresses must present in one same mapping (idmap).
... one which the code already caters for.
Quoted 6 lines
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.
Signature · 2 lines
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
Quoted 7 lines · Russell King - ARM Linux
Quoted 2 lines · Konstantin Khlebnikov
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".
Quoted 14 lines
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.
Quoted 6 lines
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.
Quoted 15 lines
Quoted 5 lines
It's a long jump
out idmap code section to some normal kernel function.__secondary_switchedin my case. And both physical source and virtual destination addresses must present in one same mapping (idmap).... one which the code already caters for.
Quoted 6 lines
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.
Quoted 3 lines
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.
Quoted 8 lines
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
Quoted 12 lines · Konstantin Khlebnikov
Quoted 7 lines · Russell King - ARM Linux
Quoted 2 lines · Konstantin Khlebnikov
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".
...
Quoted 3 lines
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.
Quoted 7 lines
Quoted 3 lines
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 whenPHYS_OFFSET>=PAGE_OFFSETbecause 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.
Signature · 2 lines
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
Quoted 25 lines · Russell King - ARM Linux
Quoted 12 lines · Konstantin Khlebnikov
Quoted 7 lines · Russell King - ARM Linux
Quoted 2 lines · Konstantin Khlebnikov
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".
...
Quoted 3 lines
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.
Quoted 4 lines
Quoted 3 lines
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.
Quoted 9 lines
Quoted 3 lines
In my case it's been overwritten.
And it always happens whenPHYS_OFFSET>=PAGE_OFFSETbecause 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.
Quoted 20 lines
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.
Quoted 20 lines · Konstantin Khlebnikov
This patch fixes booting when idmap pgd lays above 4gb. Commit
4756dcbfd37mostly had fixed this, but it'd failed to load upper bits.Also this fixes adding
TTBR1_OFFSETto TTRR1: if lower part overflows carry flag must be added to the upper part.Signed-off-by Konstantin Khlebnikov <[email protected]>Cc Cyril Chemparathy <[email protected]>Cc Vitaly Andrianov <[email protected]>--- arch/arm/mm/proc-v7-3level.S | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)arch/arm/mm/proc-v7-3level.S +0
-0@@ -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
arch/arm/mm/proc-v7-3level.S ENDPROC(cpu_v7_set_pte_ext) · line 147
Quoted 10 lines
arch/arm/mm/proc-v7-3level.S +3
-4addls \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
Quoted 28 lines · Will Deacon
Quoted 27 lines · Konstantin Khlebnikov
This patch fixes booting when idmap pgd lays above 4gb. Commit
4756dcbfd37mostly had fixed this, but it'd failed to load upper bits.Also this fixes adding
TTBR1_OFFSETto TTRR1: if lower part overflows carry flag must be added to the upper part.Signed-off-by Konstantin Khlebnikov <[email protected]>Cc Cyril Chemparathy <[email protected]>Cc Vitaly Andrianov <[email protected]>--- arch/arm/mm/proc-v7-3level.S | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)arch/arm/mm/proc-v7-3level.S +2
-2@@ -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
Quoted 8 lines
arch/arm/mm/proc-v7-3level.S ENDPROC(cpu_v7_set_pte_ext) · line 147
Quoted 3 lines
arch/arm/mm/proc-v7-3level.S +1
-2- mcrr p15, 1, \ttbr1, \zero, c2 @ load TTBR1 - mcrr p15, 0, \ttbr0, \zero, c2 @ load TTBR0 + mcrr p15, 0, \ttbr0, \tmp, c2 @ load TTBR0I 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.
Quoted 7 lines
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 38 lines · Konstantin Khlebnikov
Quoted 28 lines · Will Deacon
Quoted 27 lines · Konstantin Khlebnikov
This patch fixes booting when idmap pgd lays above 4gb. Commit
4756dcbfd37mostly had fixed this, but it'd failed to load upper bits.Also this fixes adding
TTBR1_OFFSETto TTRR1: if lower part overflows carry flag must be added to the upper part.Signed-off-by Konstantin Khlebnikov <[email protected]>Cc Cyril Chemparathy <[email protected]>Cc Vitaly Andrianov <[email protected]>--- arch/arm/mm/proc-v7-3level.S | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)arch/arm/mm/proc-v7-3level.S +2
-2@@ -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 TTBR0Quoted 8 lines
arch/arm/mm/proc-v7-3level.S ENDPROC(cpu_v7_set_pte_ext) · line 147
Quoted 3 lines
arch/arm/mm/proc-v7-3level.S +1
-2- mcrr p15, 1, \ttbr1, \zero, c2 @ load TTBR1 - mcrr p15, 0, \ttbr0, \zero, c2 @ load TTBR0 + mcrr p15, 0, \ttbr0, \tmp, c2 @ load TTBR0I 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.
Quoted 5 lines
Nope, no merge conflicts, source in original patch
https://lkml.org/lkml/2012/9/11/346That 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
Will Deacon
Acked-byQuoted 42 lines · Will Deacon
Quoted 41 lines · Konstantin Khlebnikov
Quoted 28 lines · Will Deacon
Quoted 27 lines · Konstantin Khlebnikov
This patch fixes booting when idmap pgd lays above 4gb. Commit
4756dcbfd37mostly had fixed this, but it'd failed to load upper bits.Also this fixes adding
TTBR1_OFFSETto TTRR1: if lower part overflows carry flag must be added to the upper part.Signed-off-by Konstantin Khlebnikov <[email protected]>Cc Cyril Chemparathy <[email protected]>Cc Vitaly Andrianov <[email protected]>--- arch/arm/mm/proc-v7-3level.S | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)arch/arm/mm/proc-v7-3level.S +2
-2@@ -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 TTBR0Quoted 8 lines
arch/arm/mm/proc-v7-3level.S ENDPROC(cpu_v7_set_pte_ext) · line 147
Quoted 3 lines
arch/arm/mm/proc-v7-3level.S +1
-2- mcrr p15, 1, \ttbr1, \zero, c2 @ load TTBR1 - mcrr p15, 0, \ttbr0, \zero, c2 @ load TTBR0 + mcrr p15, 0, \ttbr0, \tmp, c2 @ load TTBR0I 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
Quoted 8 lines
Quoted 2 lines
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:
Will
Konstantin Khlebnikov
Acked-by Santosh ShilimkarQuoted 42 lines · Will Deacon
Quoted 41 lines · Konstantin Khlebnikov
Quoted 28 lines · Will Deacon
Quoted 27 lines · Konstantin Khlebnikov
This patch fixes booting when idmap pgd lays above 4gb. Commit
4756dcbfd37mostly had fixed this, but it'd failed to load upper bits.Also this fixes adding
TTBR1_OFFSETto TTRR1: if lower part overflows carry flag must be added to the upper part.Signed-off-by Konstantin Khlebnikov <[email protected]>Cc Cyril Chemparathy <[email protected]>Cc Vitaly Andrianov <[email protected]>--- arch/arm/mm/proc-v7-3level.S | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)arch/arm/mm/proc-v7-3level.S +2
-2@@ -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 TTBR0Quoted 8 lines
arch/arm/mm/proc-v7-3level.S ENDPROC(cpu_v7_set_pte_ext) · line 147
Quoted 3 lines
arch/arm/mm/proc-v7-3level.S +1
-2- mcrr p15, 1, \ttbr1, \zero, c2 @ load TTBR1 - mcrr p15, 0, \ttbr0, \zero, c2 @ load TTBR0 + mcrr p15, 0, \ttbr0, \tmp, c2 @ load TTBR0I 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
Quoted 8 lines
Quoted 2 lines
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 <[email protected]> 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.
Quoted 2 lines
Will
Quoted 49 lines · Konstantin Khlebnikov
Quoted 48 lines · Will Deacon
Quoted 38 lines · Konstantin Khlebnikov
Quoted 28 lines · Will Deacon
Quoted 27 lines · Konstantin Khlebnikov
This patch fixes booting when idmap pgd lays above 4gb. Commit
4756dcbfd37mostly had fixed this, but it'd failed to load upper bits.Also this fixes adding
TTBR1_OFFSETto TTRR1: if lower part overflows carry flag must be added to the upper part.Signed-off-by Konstantin Khlebnikov <[email protected]>Cc Cyril Chemparathy <[email protected]>Cc Vitaly Andrianov <[email protected]>--- arch/arm/mm/proc-v7-3level.S | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)arch/arm/mm/proc-v7-3level.S +2
-2@@ -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 TTBR0Quoted 8 lines
arch/arm/mm/proc-v7-3level.S ENDPROC(cpu_v7_set_pte_ext) · line 147
Quoted 3 lines
arch/arm/mm/proc-v7-3level.S +1
-2- mcrr p15, 1, \ttbr1, \zero, c2 @ load TTBR1 - mcrr p15, 0, \ttbr0, \zero, c2 @ load TTBR0 + mcrr p15, 0, \ttbr0, \tmp, c2 @ load TTBR0I 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.
Quoted 5 lines
Nope, no merge conflicts, source in original patch
https://lkml.org/lkml/2012/9/11/346That 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.
Quoted 7 lines
Quoted 2 lines
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
Quoted 53 lines · Jassi Brar
Quoted 52 lines · Konstantin Khlebnikov
Quoted 50 lines · Will Deacon
Quoted 38 lines · Konstantin Khlebnikov
Quoted 28 lines · Will Deacon
Quoted 27 lines · Konstantin Khlebnikov
This patch fixes booting when idmap pgd lays above 4gb. Commit
4756dcbfd37mostly had fixed this, but it'd failed to load upper bits.Also this fixes adding
TTBR1_OFFSETto TTRR1: if lower part overflows carry flag must be added to the upper part.Signed-off-by Konstantin Khlebnikov <[email protected]>Cc Cyril Chemparathy <[email protected]>Cc Vitaly Andrianov <[email protected]>--- arch/arm/mm/proc-v7-3level.S | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)arch/arm/mm/proc-v7-3level.S +2
-2@@ -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 TTBR0Quoted 8 lines
arch/arm/mm/proc-v7-3level.S ENDPROC(cpu_v7_set_pte_ext) · line 147
Quoted 3 lines
arch/arm/mm/proc-v7-3level.S +1
-2- mcrr p15, 1, \ttbr1, \zero, c2 @ load TTBR1 - mcrr p15, 0, \ttbr0, \zero, c2 @ load TTBR0 + mcrr p15, 0, \ttbr0, \tmp, c2 @ load TTBR0I 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.
Quoted 5 lines
Nope, no merge conflicts, source in original patch
https://lkml.org/lkml/2012/9/11/346That 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)?
Quoted 8 lines
Quoted 4 lines
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.
arch/arm/mm/proc-v7-3level.S +0 -1
@@ -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
Quoted 2 lines
Thanks
Jassi
Quoted 71 lines · Konstantin Khlebnikov
Quoted 53 lines · Jassi Brar
Quoted 52 lines · Konstantin Khlebnikov
Quoted 50 lines · Will Deacon
Quoted 38 lines · Konstantin Khlebnikov
Quoted 28 lines · Will Deacon
Quoted 27 lines · Konstantin Khlebnikov
This patch fixes booting when idmap pgd lays above 4gb. Commit
4756dcbfd37mostly had fixed this, but it'd failed to load upper bits.Also this fixes adding
TTBR1_OFFSETto TTRR1: if lower part overflows carry flag must be added to the upper part.Signed-off-by Konstantin Khlebnikov <[email protected]>Cc Cyril Chemparathy <[email protected]>Cc Vitaly Andrianov <[email protected]>--- arch/arm/mm/proc-v7-3level.S | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)arch/arm/mm/proc-v7-3level.S +2
-2@@ -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 TTBR0Quoted 8 lines
arch/arm/mm/proc-v7-3level.S ENDPROC(cpu_v7_set_pte_ext) · line 147
Quoted 3 lines
arch/arm/mm/proc-v7-3level.S +1
-2- mcrr p15, 1, \ttbr1, \zero, c2 @ load TTBR1 - mcrr p15, 0, \ttbr0, \zero, c2 @ load TTBR0 + mcrr p15, 0, \ttbr0, \tmp, c2 @ load TTBR0I 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.
Quoted 5 lines
Nope, no merge conflicts, source in original patch
https://lkml.org/lkml/2012/9/11/346That 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)?
Quoted 8 lines
Quoted 4 lines
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 wherePHYS_OFFSET==PAGE_OFFSET.
arch/arm/mm/proc-v7-3level.S ENDPROC(cpu_v7_set_pte_ext) · line 151
Quoted 7 lines
arch/arm/mm/proc-v7-3level.S +0
-1@@ -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.
Quoted 2 lines
Quoted 2 lines
Thanks
Jassi
Quoted 67 lines · Konstantin Khlebnikov
Quoted 50 lines · Jassi Brar
Quoted 48 lines · Konstantin Khlebnikov
Quoted 47 lines
Quoted 36 lines
Quoted 27 lines
Quoted 27 lines
This patch fixes booting when idmap pgd lays above 4gb. Commit
4756dcbfd37mostly had fixed this, but it'd failed to load upper bits.Also this fixes adding
TTBR1_OFFSETto TTRR1: if lower part overflows carry flag must be added to the upper part.Signed-off-by Konstantin Khlebnikov <[email protected]>Cc Cyril Chemparathy <[email protected]>Cc Vitaly Andrianov <[email protected]>--- arch/arm/mm/proc-v7-3level.S | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)arch/arm/mm/proc-v7-3level.S +2
-2@@ -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 TTBR0Quoted 8 lines
arch/arm/mm/proc-v7-3level.S ENDPROC(cpu_v7_set_pte_ext) · line 147
Quoted 3 lines
arch/arm/mm/proc-v7-3level.S +1
-2- mcrr p15, 1, \ttbr1, \zero, c2 @ load TTBR1 - mcrr p15, 0, \ttbr0, \zero, c2 @ load TTBR0 + mcrr p15, 0, \ttbr0, \tmp, c2 @ load TTBR0I 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.
Quoted 5 lines
Nope, no merge conflicts, source in original patch
https://lkml.org/lkml/2012/9/11/346That 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)?
Quoted 8 lines
Quoted 4 lines
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 wherePHYS_OFFSET==PAGE_OFFSET.
arch/arm/mm/proc-v7-3level.S ENDPROC(cpu_v7_set_pte_ext) · line 151
Quoted 7 lines
arch/arm/mm/proc-v7-3level.S +0
-1@@ -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
Quoted 10 lines · Jassi Brar
Quoted 9 lines · Konstantin Khlebnikov
Quoted 4 lines · Jassi Brar
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 wherePHYS_OFFSET==PAGE_OFFSET.
Quoted 12 lines
arch/arm/mm/proc-v7-3level.S ENDPROC(cpu_v7_set_pte_ext) · line 151
Quoted 11 lines
arch/arm/mm/proc-v7-3level.S +0
-1@@ -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 bitsYup, restores boot.
Can somebody write this up as a proper patch and put it in the patch system please?
Will
Quoted 22 lines · Will Deacon
Quoted 9 lines · Jassi Brar
Quoted 8 lines · Konstantin Khlebnikov
Quoted 4 lines · Jassi Brar
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 wherePHYS_OFFSET==PAGE_OFFSET.Quoted 12 lines
arch/arm/mm/proc-v7-3level.S ENDPROC(cpu_v7_set_pte_ext) · line 151
Quoted 11 lines
arch/arm/mm/proc-v7-3level.S +0
-1@@ -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 bitsYup, restores boot.
Quoted 2 lines
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
Quoted 3 lines
Will
-------------- next part --------------
ARM: LPAE: drop wrong carry flag correction after adding TTBR1_OFFSET
From: Konstantin Khlebnikov <[email protected]>
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.
--- arch/arm/mm/proc-v7-3level.S | 1 - 1 file changed, 1 deletion(-)
arch/arm/mm/proc-v7-3level.S +0 -1
@@ -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
Quoted 18 lines · Konstantin Khlebnikov
This patch fixes booting when idmap pgd lays above 4gb. Commit
4756dcbfd37mostly had fixed this, but it'd failed to load upper bits.Also this fixes adding
TTBR1_OFFSETto TTRR1: if lower part overflows carry flag must be added to the upper part.Signed-off-by Konstantin Khlebnikov <[email protected]>Cc Cyril Chemparathy <[email protected]>Cc Vitaly Andrianov <[email protected]>--- arch/arm/mm/proc-v7-3level.S | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)arch/arm/mm/proc-v7-3level.S +0
-0
arch/arm/mm/proc-v7-3level.S ENDPROC(cpu_v7_set_pte_ext) · line 143
Quoted 5 lines
arch/arm/mm/proc-v7-3level.S +1
-1@@ -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.
Quoted 16 lines
arch/arm/mm/proc-v7-3level.S +2
-4+ 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/