x86: ioremap ram check fix
PATCH 1/1 x86: ioremap ram check fix
Hi Ingo,
bdd3cee2e4b7279457139058615ced6c2b41e7de (x86: ioremap(), extend check to all RAM pages) breaks OLPC's ioremap call. The ioremap that OLPC uses is:
romsig = ioremap(0xffffffc0, 16);
The commit that breaks it is basically:
- for (pfn = phys_addr >> PAGE_SHIFT; pfn < max_pfn_mapped &&
- (pfn << PAGE_SHIFT) < last_addr; pfn++) {
+ for (pfn = phys_addr >> PAGE_SHIFT;
+ (pfn << PAGE_SHIFT) < last_addr; pfn++) {
+
Previously, the 'pfn < max_pfn_mapped' check would've caused us to not enter the loop. Removing that check means we loop infinitely. The reason for that is because pfn is 0xfffff, and last_addr is 0xffffffcf. The remaining check that is used to exit the loop is not sufficient; when pfn<<PAGE_SHIFT is 0xfffff000, that is less than 0xffffffcf; when we increment pfn and it overflows (pfn == 0x100000), pfn<<PAGE_SHIFT ends up being 0. That, of course, is less than last_addr. In effect, pfn<<PAGE_SHIFT is never lower than last_addr.
The simple fix for this is to limit the last_addr check to the PAGE_MASK; a patch is below.
--- arch/x86/mm/ioremap.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
arch/x86/mm/ioremap.c +2 -1
@@ -149,7 +149,8 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,* Don't allow anybody to remap normal RAM that we're using.. */ for (pfn = phys_addr >>PAGE_SHIFT ; - (pfn <<PAGE_SHIFT ) < last_addr; pfn++) { + (pfn <<PAGE_SHIFT ) < (last_addr &PAGE_MASK ); + pfn++) {int is_ram =page_is_ram (pfn);
Signature · 1 line
1.5.5
Quoted 40 lines · Andres Salomon
Hi Ingo,
bdd3cee2e4b7279457139058615ced6c2b41e7de(x86:ioremap(), extend check to all RAM pages) breaks OLPC's ioremap call. The ioremap that OLPC uses is:romsig = ioremap(0xffffffc0, 16);The commit that breaks it is basically:
- for (pfn =
phys_addr>>PAGE_SHIFT; pfn <max_pfn_mapped&&
- (pfn <<PAGE_SHIFT) <last_addr; pfn++) {
+ for (pfn =phys_addr>>PAGE_SHIFT;
+ (pfn <<PAGE_SHIFT) <last_addr; pfn++) {
+Previously, the 'pfn <
max_pfn_mapped' check would've caused us to not enter the loop. Removing that check means we loop infinitely. The reason for that is because pfn is0xfffff, andlast_addris0xffffffcf. The remaining check that is used to exit the loop is not sufficient; when pfn<<PAGE_SHIFT is0xfffff000, that is less than0xffffffcf; when we increment pfn and it overflows (pfn ==0x100000), pfn<<PAGE_SHIFT ends up being 0. That, of course, is less thanlast_addr. In effect, pfn<<PAGE_SHIFT is never lower thanlast_addr.The simple fix for this is to limit the
last_addrcheck to thePAGE_MASK; a patch is below.Signed-off-by Andres Salomon <[email protected]>---
arch/x86/mm/ioremap.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)diff --git
a/arch/x86/mm/ioremap.cb/arch/x86/mm/ioremap.c
index 804de18..fb960f5 100644
---a/arch/x86/mm/ioremap.c
+++b/arch/x86/mm/ioremap.c
@@ -149,7 +149,8 @@ static void__iomem*__ioremap_caller(resource_size_tphys_addr,
Quoted 9 lines
* Don't allow anybody to remap normal RAM that we're using.. */ for (pfn = phys_addr >> PAGE_SHIFT; - (pfn << PAGE_SHIFT) < last_addr; pfn++) { + (pfn << PAGE_SHIFT) < (last_addr & PAGE_MASK); + pfn++) {int is_ram = page_is_ram(pfn);
This fixes two 150-second solid freezes during bootup on a P4 I have as well. Acked!
Vegard
Signature · 4 lines
"The animistic metaphor of the bug that maliciously sneaked in while the programmer was not looking is intellectually dishonest as it disguises that the error is the programmer's own creation." -- E. W. Dijkstra, EWD1036
Quoted 5 lines · Andres Salomon
Hi Ingo,
bdd3cee2e4b7279457139058615ced6c2b41e7de(x86:ioremap(), extend check to all RAM pages) breaks OLPC's ioremap call. The ioremap that OLPC uses is:
that breakage has been reported independently as well and the revert has been pushed to Linus yesterday - it's not upstream yet. See this commit in x86.git:
commit 1b8104a0ec138de829bb351f6597d534c7c134dc
Author: Ingo Molnar <[email protected]>
Date: Tue Apr 29 12:04:51 2008 +0200
revert: "x86: ioremap(), extend check to all RAM pages"
Vegard Nossum reported a large (150 seconds) boot delay during bootup, and bisected it to "x86: ioremap(), extend check to all RAM pages" (commit bdd3cee2e4b). Revert this commit for now.
Ingo
Quoted 5 lines · On Wed, 30 Apr 2008 20:10:15 +0200 Ingo Molnar <[email protected]> wrote:
Quoted 2 lines · Andres Salomon
Hi Ingo,
Quoted 8 lines
Quoted 3 lines
bdd3cee2e4b7279457139058615ced6c2b41e7de(x86:ioremap(), extend check to all RAM pages) breaks OLPC's ioremap call. The ioremap that OLPC uses is:that breakage has been reported independently as well and the revert has been pushed to Linus yesterday - it's not upstream yet. See this commit in x86.git:
Sure, either way. FWIW, Vegard reported that the PAGE_MASK change made the 150s delay disappear.
Quoted 14 lines
commit
1b8104a0ec138de829bb351f6597d534c7c134dc
Author: Ingo Molnar <[email protected]>
Date: Tue Apr 29 12:04:51 2008 +0200revert: "x86:
ioremap(), extend check to all RAM pages"Vegard Nossum reported a large (150 seconds) boot delay during bootup, and bisected it to "x86:
ioremap(), extend check to all RAM pages" (commitbdd3cee2e4b). Revert this commit for now.Bisected-by Vegard Nossum <[email protected]>Signed-off-by Ingo Molnar <[email protected]>Ingo
Quoted 6 lines · Andres Salomon
Quoted 3 lines
that breakage has been reported independently as well and the revert has been pushed to Linus yesterday - it's not upstream yet. See this commit in x86.git:
Sure, either way. FWIW, Vegard reported that the
PAGE_MASKchange made the 150s delay disappear.
sorry, i forgot to mention that your fix is the right one [it actually fixes the problem instead of reverting the change] so i've queued that up too :-) Thanks for fixing this!
Ingo