Skip to contentSkip to conversation

x86: ioremap() problem in X86_32 PAE

PATCH In review Kenji Kaneshige 4 participants 10 messages Jun 18, 2010 – Feb 11, 2011

Subsystems (5)
Memory management x86 Library code x86 MM +1 more subsystems
lore
Share & tools
lore

Kenji Kaneshige

author
#0 lore raw

cover x86: ioremap() problem in X86_32 PAE

Hi,

Here is a updated version (v.3) of patchset to fix ioremap() related problem found in x86 32-bit mode. The problem is that ioremap() maps wrong address for the device to which phisical addres higer than 32-bit is assigned (ioat device in my case).

The v.3 patches are:

- [PATCH 1/2] x86: ioremap: fix wrong physical address handling
- [PATCH 2/2] x86: ioremap: fix normal ram range check

Changelog
---------
v.2=>v.3:
- Matthew Wilcox suggested changing PAGE_MASK definition. But I think it is a little risky. Instead of changing PAGE_MASK, v.3 patch uses PHYSICAL_PAGE_MASK(*) to mask physical address again, as v.1 patch did. The PHYSICAL_PAGE_MASK doesn't work for higher than 2^44 physical address because __PHYSICAL_MASK_SHIFT is 44 in x86_32 PAE(*). But this time, I focus on making 2^44 work correctly.

(*) Current X86_32 PAE kernel would not work with physical address
higher than 2^44, because PFNs are handled using 32-bit variable (unsigned long) probably in many places. This seems the reason why __PHYSICAL_MASK_SHIFT is limited to 44 (=32+12 is the limit we can fit into an unsigned long pfn). When x86 PAE linux becomes above 2^44 physical address capable in someday, __PHYSICAL_MASK_SHIFT should become greater value than 44.

v.1=>v.2:
The v.1 patchset was based on my misunderstanding about architectural limit and linux memory management limit of physical address. So some patches in v.1 were totally wrong fixes and they are removed. Thanks to the feedbacks for v.1 patchset, it turned out that phsical address is handled improperly (higher 32-bits are cleared unexpectedly) also in the other places than where v.1 patchset fixed. Here are summary of changes.

- Stop using PHYSICAL_PAGE_MASK to align physical address because PHYSICAL_PAGE_MASK would not work if physical address is higher than 44-bit.
- Fix remaining bugs of physical address handling in ioremap() code path according to the feedbacks.
- Fix s_show() in vmalloc.c to show high physical address properly.
- Removed changes against phys_addr_valid() ([PATCH 2/4] in v.1).
- Removed changes against warning message in ioremap() ([PATCH 3/4] in v.1).

Thanks,
Kenji Kaneshige

author
#1 lore raw

PATCH 1/2 x86: ioremap: fix wrong physical address handling

Current x86 ioremap() doesn't handle physical address higher than 32-bit properly in X86_32 PAE mode. When physical address higher than 32-bit is passed to ioremap(), higher 32-bits in physical address is cleared wrongly. Due to this bug, ioremap() can map wrong address to linear address space.

In my case, 64-bit MMIO region was assigned to a PCI device (ioat device) on my system. Because of the ioremap()'s bug, wrong physical address (instead of MMIO region) was mapped to linear address space. Because of this, loading ioatdma driver caused unexpected behavior (kernel panic, kernel hangup, ...).

Signed-off-by Kenji Kaneshige <[email protected]>
---
 arch/x86/mm/ioremap.c   |   12 +++++-------
 include/linux/io.h      |    4 ++--
 include/linux/vmalloc.h |    2 +-
 lib/ioremap.c           |   10 +++++-----
 mm/vmalloc.c            |    2 +-
 5 files changed, 14 insertions(+), 16 deletions(-)
arch/x86/mm/ioremap.c +5 -7
Index: linux-2.6.34/arch/x86/mm/ioremap.c
===================================================================
@@ -62,8 +62,8 @@ int ioremap_change_attr(unsigned long va
 static void __iomem *__ioremap_caller(resource_size_t phys_addr,
 		unsigned long size, unsigned long prot_val, void *caller)
 {
-	unsigned long pfn, offset, vaddr;
-	resource_size_t last_addr;
+	unsigned long offset, vaddr;
+	resource_size_t pfn, last_pfn, last_addr;
 	const resource_size_t unaligned_phys_addr = phys_addr;
 	const unsigned long unaligned_size = size;
 	struct vm_struct *area;

@@ -100,10 +100,8 @@ static void __iomem *__ioremap_caller(re
 	/*
 	 * Don't allow anybody to remap normal RAM that we're using..
 	 */
-	for (pfn = phys_addr >> PAGE_SHIFT;
-				(pfn << PAGE_SHIFT) < (last_addr & PAGE_MASK);
-				pfn++) {
-
+	last_pfn = last_addr >> PAGE_SHIFT;
+	for (pfn = phys_addr >> PAGE_SHIFT; pfn < last_pfn; pfn++) {
 		int is_ram = page_is_ram(pfn);
 
 		if (is_ram && pfn_valid(pfn) && !PageReserved(pfn_to_page(pfn)))

@@ -115,7 +113,7 @@ static void __iomem *__ioremap_caller(re
 	 * Mappings have to be page-aligned
 	 */
 	offset = phys_addr & ~PAGE_MASK;
-	phys_addr &= PAGE_MASK;
+	phys_addr &= PHYSICAL_PAGE_MASK;
 	size = PAGE_ALIGN(last_addr+1) - phys_addr;
 
 	retval = reserve_memtype(phys_addr, (u64)phys_addr + size,
include/linux/vmalloc.h +1 -1
Index: linux-2.6.34/include/linux/vmalloc.h
===================================================================
@@ -30,7 +30,7 @@ struct vm_struct {
 	unsigned long		flags;
 	struct page		**pages;
 	unsigned int		nr_pages;
-	unsigned long		phys_addr;
+	phys_addr_t		phys_addr;
 	void			*caller;
 };
lib/ioremap.c +5 -5
Index: linux-2.6.34/lib/ioremap.c
===================================================================
@@ -13,10 +13,10 @@
 #include <asm/pgtable.h>
 
 static int ioremap_pte_range(pmd_t *pmd, unsigned long addr,
-		unsigned long end, unsigned long phys_addr, pgprot_t prot)
+		unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
 {
 	pte_t *pte;
-	unsigned long pfn;
+	u64 pfn;
 
 	pfn = phys_addr >> PAGE_SHIFT;
 	pte = pte_alloc_kernel(pmd, addr);

@@ -31,7 +31,7 @@ static int ioremap_pte_range(pmd_t *pmd,
 }
 
 static inline int ioremap_pmd_range(pud_t *pud, unsigned long addr,
-		unsigned long end, unsigned long phys_addr, pgprot_t prot)
+		unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
 {
 	pmd_t *pmd;
 	unsigned long next;

@@ -49,7 +49,7 @@ static inline int ioremap_pmd_range(pud_
 }
 
 static inline int ioremap_pud_range(pgd_t *pgd, unsigned long addr,
-		unsigned long end, unsigned long phys_addr, pgprot_t prot)
+		unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
 {
 	pud_t *pud;
 	unsigned long next;

@@ -67,7 +67,7 @@ static inline int ioremap_pud_range(pgd_
 }
 
 int ioremap_page_range(unsigned long addr,
-		       unsigned long end, unsigned long phys_addr, pgprot_t prot)
+		       unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
 {
 	pgd_t *pgd;
 	unsigned long start;
include/linux/io.h +2 -2
Index: linux-2.6.34/include/linux/io.h
===================================================================
@@ -29,10 +29,10 @@ void __iowrite64_copy(void __iomem *to, 
 
 #ifdef CONFIG_MMU
 int ioremap_page_range(unsigned long addr, unsigned long end,
-		       unsigned long phys_addr, pgprot_t prot);
+		       phys_addr_t phys_addr, pgprot_t prot);
 #else
 static inline int ioremap_page_range(unsigned long addr, unsigned long end,
-				     unsigned long phys_addr, pgprot_t prot)
+				     phys_addr_t phys_addr, pgprot_t prot)
 {
 	return 0;
 }
mm/vmalloc.c +1 -1
Index: linux-2.6.34/mm/vmalloc.c
===================================================================
@@ -2403,7 +2403,7 @@ static int s_show(struct seq_file *m, vo
 		seq_printf(m, " pages=%d", v->nr_pages);
 
 	if (v->phys_addr)
-		seq_printf(m, " phys=%lx", v->phys_addr);
+		seq_printf(m, " phys=%llx", (unsigned long long)v->phys_addr);
 
 	if (v->flags & VM_IOREMAP)
 		seq_printf(m, " ioremap");
#2 lore raw

Re: [PATCH 1/2] x86: ioremap: fix wrong physical address handling

Quoted 27 lines · Kenji Kaneshige

Current x86 ioremap() doesn't handle physical address higher than 32-bit properly in X86_32 PAE mode. When physical address higher than 32-bit is passed to ioremap(), higher 32-bits in physical address is cleared wrongly. Due to this bug, ioremap() can map wrong address to linear address space.

In my case, 64-bit MMIO region was assigned to a PCI device (ioat device) on my system. Because of the ioremap()'s bug, wrong physical address (instead of MMIO region) was mapped to linear address space. Because of this, loading ioatdma driver caused unexpected behavior (kernel panic, kernel hangup, ...).

Signed-off-by Kenji Kaneshige <[email protected]>
---
 arch/x86/mm/ioremap.c   |   12 +++++-------
 include/linux/io.h      |    4 ++--
 include/linux/vmalloc.h |    2 +-
 lib/ioremap.c           |   10 +++++-----
 mm/vmalloc.c            |    2 +-
 5 files changed, 14 insertions(+), 16 deletions(-)

Index: linux-2.6.34/arch/x86/mm/ioremap.c
===================================================================

arch/x86/mm/ioremap.c int ioremap_change_attr(unsigned long va · line 67

Quoted 8 lines
arch/x86/mm/ioremap.c +2 -2
@@ -62,8 +62,8 @@ int ioremap_change_attr(unsigned long va
 static void __iomem *__ioremap_caller(resource_size_t phys_addr,
 		unsigned long size, unsigned long prot_val, void *caller)
 {
-	unsigned long pfn, offset, vaddr;
-	resource_size_t last_addr;
+	unsigned long offset, vaddr;
+	resource_size_t pfn, last_pfn, last_addr;

Why is pfn resource_size_t here? Is it to avoid casting, or does it actually need to hold more than 32 bits? I don't see any use of pfn aside from the page_is_ram loop, and I don't think that can go beyond 32 bits. If you're worried about boundary conditions at the 2^44 limit, then you can make last_pfn inclusive, or compute num_pages and use that for the loop condition.

arch/x86/mm/ioremap.c static void __iomem *__ioremap_caller(re · line 104

Quoted 13 lines
arch/x86/mm/ioremap.c +0 -0
 	const resource_size_t unaligned_phys_addr = phys_addr;
 	const unsigned long unaligned_size = size;
arch/x86/mm/ioremap.c +1 -4
 	struct vm_struct *area;

@@ -100,10 +100,8 @@ static void __iomem *__ioremap_caller(re
 	/*
 	 * Don't allow anybody to remap normal RAM that we're using..
 	 */
-	for (pfn = phys_addr >> PAGE_SHIFT;
-				(pfn << PAGE_SHIFT) < (last_addr & PAGE_MASK);
-				pfn++) {
-
+	last_pfn = last_addr >> PAGE_SHIFT;

If last_addr can be non-page aligned, should it be rounding up to the next pfn rather than rounding down? Ah, looks like you fix it in the second patch.

J

author
#3 lore raw

Re: [PATCH 1/2] x86: ioremap: fix wrong physical address handling

Quoted 35 lines · Jeremy Fitzhardinge
Quoted 34 lines · Kenji Kaneshige

Current x86 ioremap() doesn't handle physical address higher than 32-bit properly in X86_32 PAE mode. When physical address higher than 32-bit is passed to ioremap(), higher 32-bits in physical address is cleared wrongly. Due to this bug, ioremap() can map wrong address to linear address space.

In my case, 64-bit MMIO region was assigned to a PCI device (ioat device) on my system. Because of the ioremap()'s bug, wrong physical address (instead of MMIO region) was mapped to linear address space. Because of this, loading ioatdma driver caused unexpected behavior (kernel panic, kernel hangup, ...).

Signed-off-by Kenji Kaneshige<[email protected]>
---
  arch/x86/mm/ioremap.c   |   12 +++++-------
  include/linux/io.h      |    4 ++--
  include/linux/vmalloc.h |    2 +-
  lib/ioremap.c           |   10 +++++-----
  mm/vmalloc.c            |    2 +-
  5 files changed, 14 insertions(+), 16 deletions(-)

Index: linux-2.6.34/arch/x86/mm/ioremap.c
===================================================================

arch/x86/mm/ioremap.c +2 -2
@@ -62,8 +62,8 @@ int ioremap_change_attr(unsigned long va
  static void __iomem *__ioremap_caller(resource_size_t phys_addr,
  		unsigned long size, unsigned long prot_val, void *caller)
  {
-	unsigned long pfn, offset, vaddr;
-	resource_size_t last_addr;
+	unsigned long offset, vaddr;
+	resource_size_t pfn, last_pfn, last_addr;
Quoted 9 lines
Quoted 1 line

Why is pfn resource_size_t here? Is it to avoid casting, or does it actually need to hold more than 32 bits? I don't see any use of pfn aside from the page_is_ram loop, and I don't think that can go beyond 32 bits. If you're worried about boundary conditions at the 2^44 limit, then you can make last_pfn inclusive, or compute num_pages and use that for the loop condition.

The reason I changed here was phys_addr might be higher than 2^44. After the discussion, I realized there would probably be many other codes that cannot handle more than 32-bits pfn, and this would cause problems even if I changed ioremap() to be able to handle more than 32-bits pfn. So I decided to focus on making 44-bits physical address work properly this time. But, I didn't find any reason to make it go back to unsigned long. So I still make it resource_size_t even in v.3. Is there any problem on this change? And I don't understand why pfn can't go beyond 32-bits. Could you tell me why?

Quoted 9 lines
Quoted 9 lines
  	const resource_size_t unaligned_phys_addr = phys_addr;
  	const unsigned long unaligned_size = size;
  	struct vm_struct *area;
arch/x86/mm/ioremap.c +0 -2
@@ -100,10 +100,8 @@ static void __iomem *__ioremap_caller(re
  	/*
  	 * Don't allow anybody to remap normal RAM that we're using..
  	 */
-	for (pfn = phys_addr>>  PAGE_SHIFT;
-				(pfn<<  PAGE_SHIFT)<  (last_addr&  PAGE_MASK);
Quoted 9 lines

arch/x86/mm/ioremap.c static void __iomem *__ioremap_caller(re · line 106

Quoted 4 lines
arch/x86/mm/ioremap.c +1 -2
-				pfn++) {
-
+	last_pfn = last_addr>>  PAGE_SHIFT;

If last_addr can be non-page aligned, should it be rounding up to the next pfn rather than rounding down? Ah, looks like you fix it in the second patch.

Yes, I fixed it in the [PATCH 2/2].

Thanks,
Kenji Kaneshige

author
#4 lore raw

[tip:x86/mm] x86, ioremap: Fix incorrect physical address handling in PAE mode

In reply to Kenji Kaneshige
Commit-ID:  ffa71f33a820d1ab3f2fc5723819ac60fb76080b
Gitweb:     http://git.kernel.org/tip/ffa71f33a820d1ab3f2fc5723819ac60fb76080b
Author:     Kenji Kaneshige <[email protected]>
AuthorDate: Fri, 18 Jun 2010 12:22:40 +0900
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Fri, 9 Jul 2010 11:42:03 -0700

x86, ioremap: Fix incorrect physical address handling in PAE mode

Current x86 ioremap() doesn't handle physical address higher than 32-bit properly in X86_32 PAE mode. When physical address higher than 32-bit is passed to ioremap(), higher 32-bits in physical address is cleared wrongly. Due to this bug, ioremap() can map wrong address to linear address space.

In my case, 64-bit MMIO region was assigned to a PCI device (ioat device) on my system. Because of the ioremap()'s bug, wrong physical address (instead of MMIO region) was mapped to linear address space. Because of this, loading ioatdma driver caused unexpected behavior (kernel panic, kernel hangup, ...).

Signed-off-by Kenji Kaneshige <[email protected]>

LKML-Reference: <[email protected]>

Signed-off-by H. Peter Anvin <[email protected]>
---
 arch/x86/mm/ioremap.c   |   12 +++++-------
 include/linux/io.h      |    4 ++--
 include/linux/vmalloc.h |    2 +-
 lib/ioremap.c           |   10 +++++-----
 mm/vmalloc.c            |    2 +-
 5 files changed, 14 insertions(+), 16 deletions(-)
arch/x86/mm/ioremap.c +5 -7
index 12e4d2d..754cb4c 100644
@@ -62,8 +62,8 @@ int ioremap_change_attr(unsigned long vaddr, unsigned long size,
 static void __iomem *__ioremap_caller(resource_size_t phys_addr,
 		unsigned long size, unsigned long prot_val, void *caller)
 {
-	unsigned long pfn, offset, vaddr;
-	resource_size_t last_addr;
+	unsigned long offset, vaddr;
+	resource_size_t pfn, last_pfn, last_addr;
 	const resource_size_t unaligned_phys_addr = phys_addr;
 	const unsigned long unaligned_size = size;
 	struct vm_struct *area;

@@ -100,10 +100,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 & PAGE_MASK);
-				pfn++) {
-
+	last_pfn = last_addr >> PAGE_SHIFT;
+	for (pfn = phys_addr >> PAGE_SHIFT; pfn < last_pfn; pfn++) {
 		int is_ram = page_is_ram(pfn);
 
 		if (is_ram && pfn_valid(pfn) && !PageReserved(pfn_to_page(pfn)))

@@ -115,7 +113,7 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
 	 * Mappings have to be page-aligned
 	 */
 	offset = phys_addr & ~PAGE_MASK;
-	phys_addr &= PAGE_MASK;
+	phys_addr &= PHYSICAL_PAGE_MASK;
 	size = PAGE_ALIGN(last_addr+1) - phys_addr;
 
 	retval = reserve_memtype(phys_addr, (u64)phys_addr + size,
include/linux/io.h +2 -2
index 6c7f0ba..7fd2d21 100644
@@ -29,10 +29,10 @@ void __iowrite64_copy(void __iomem *to, const void *from, size_t count);
 
 #ifdef CONFIG_MMU
 int ioremap_page_range(unsigned long addr, unsigned long end,
-		       unsigned long phys_addr, pgprot_t prot);
+		       phys_addr_t phys_addr, pgprot_t prot);
 #else
 static inline int ioremap_page_range(unsigned long addr, unsigned long end,
-				     unsigned long phys_addr, pgprot_t prot)
+				     phys_addr_t phys_addr, pgprot_t prot)
 {
 	return 0;
 }
include/linux/vmalloc.h +1 -1
index 227c2a5..de05e96 100644
@@ -30,7 +30,7 @@ struct vm_struct {
 	unsigned long		flags;
 	struct page		**pages;
 	unsigned int		nr_pages;
-	unsigned long		phys_addr;
+	phys_addr_t		phys_addr;
 	void			*caller;
 };
lib/ioremap.c +5 -5
index 14c6078..5730ecd 100644
@@ -13,10 +13,10 @@
 #include <asm/pgtable.h>
 
 static int ioremap_pte_range(pmd_t *pmd, unsigned long addr,
-		unsigned long end, unsigned long phys_addr, pgprot_t prot)
+		unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
 {
 	pte_t *pte;
-	unsigned long pfn;
+	u64 pfn;
 
 	pfn = phys_addr >> PAGE_SHIFT;
 	pte = pte_alloc_kernel(pmd, addr);

@@ -31,7 +31,7 @@ static int ioremap_pte_range(pmd_t *pmd, unsigned long addr,
 }
 
 static inline int ioremap_pmd_range(pud_t *pud, unsigned long addr,
-		unsigned long end, unsigned long phys_addr, pgprot_t prot)
+		unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
 {
 	pmd_t *pmd;
 	unsigned long next;

@@ -49,7 +49,7 @@ static inline int ioremap_pmd_range(pud_t *pud, unsigned long addr,
 }
 
 static inline int ioremap_pud_range(pgd_t *pgd, unsigned long addr,
-		unsigned long end, unsigned long phys_addr, pgprot_t prot)
+		unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
 {
 	pud_t *pud;
 	unsigned long next;

@@ -67,7 +67,7 @@ static inline int ioremap_pud_range(pgd_t *pgd, unsigned long addr,
 }
 
 int ioremap_page_range(unsigned long addr,
-		       unsigned long end, unsigned long phys_addr, pgprot_t prot)
+		       unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
 {
 	pgd_t *pgd;
 	unsigned long start;
mm/vmalloc.c +1 -1
index ae00746..b7e314b 100644
@@ -2403,7 +2403,7 @@ static int s_show(struct seq_file *m, void *p)
 		seq_printf(m, " pages=%d", v->nr_pages);
 
 	if (v->phys_addr)
-		seq_printf(m, " phys=%lx", v->phys_addr);
+		seq_printf(m, " phys=%llx", (unsigned long long)v->phys_addr);
 
 	if (v->flags & VM_IOREMAP)
 		seq_printf(m, " ioremap");
author
#5 lore raw

PATCH 2/2 x86: ioremap: fix normal ram range check

In reply to Kenji Kaneshige

Check for norma RAM in x86 ioremap() code seems to not work for the last page frame in the specified physical address range.

Signed-off-by Kenji Kaneshige <[email protected]>
---
 arch/x86/mm/ioremap.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/mm/ioremap.c +1 -1
Index: linux-2.6.34/arch/x86/mm/ioremap.c
===================================================================
@@ -101,7 +101,7 @@ static void __iomem *__ioremap_caller(re
 	 * Don't allow anybody to remap normal RAM that we're using..
 	 */
 	last_pfn = last_addr >> PAGE_SHIFT;
-	for (pfn = phys_addr >> PAGE_SHIFT; pfn < last_pfn; pfn++) {
+	for (pfn = phys_addr >> PAGE_SHIFT; pfn <= last_pfn; pfn++) {
 		int is_ram = page_is_ram(pfn);
 
 		if (is_ram && pfn_valid(pfn) && !PageReserved(pfn_to_page(pfn)))
author
#6 lore raw

[tip:x86/mm] x86, ioremap: Fix normal ram range check

Commit-ID:  35be1b716a475717611b2dc04185e9d80b9cb693
Gitweb:     http://git.kernel.org/tip/35be1b716a475717611b2dc04185e9d80b9cb693
Author:     Kenji Kaneshige <[email protected]>
AuthorDate: Fri, 18 Jun 2010 12:23:57 +0900
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Fri, 9 Jul 2010 11:42:11 -0700

x86, ioremap: Fix normal ram range check

Check for normal RAM in x86 ioremap() code seems to not work for the last page frame in the specified physical address range.

Signed-off-by Kenji Kaneshige <[email protected]>

LKML-Reference: <[email protected]>

Signed-off-by H. Peter Anvin <[email protected]>
---
 arch/x86/mm/ioremap.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/x86/mm/ioremap.c +1 -1
index 754cb4c..d41d3a9 100644
@@ -101,7 +101,7 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
 	 * Don't allow anybody to remap normal RAM that we're using..
 	 */
 	last_pfn = last_addr >> PAGE_SHIFT;
-	for (pfn = phys_addr >> PAGE_SHIFT; pfn < last_pfn; pfn++) {
+	for (pfn = phys_addr >> PAGE_SHIFT; pfn <= last_pfn; pfn++) {
 		int is_ram = page_is_ram(pfn);
 
 		if (is_ram && pfn_valid(pfn) && !PageReserved(pfn_to_page(pfn)))
reviewer
#7 lore raw
In reply to Kenji Kaneshige
Quoted 7 lines · Kenji Kaneshige

Hi,

Here is a updated version (v.3) of patchset to fix ioremap() related problem found in x86 32-bit mode. The problem is that ioremap() maps wrong address for the device to which phisical addres higer than 32-bit is assigned (ioat device in my case).

Will look at it first thing in the morning.

-hpa

#8 lore raw
In reply to Kenji Kaneshige

Hi.
I faced same probrem, too. I use x86_64 kernel.
Your patch fix it. it's very effective.
Thanks!

2010年6月18日12:21 Kenji Kaneshige <[email protected]>:

Quoted 59 lines

Hi,

Here is a updated version (v.3) of patchset to fix ioremap() related problem found in x86 32-bit mode. The problem is that ioremap() maps wrong address for the device to which phisical addres higer than 32-bit is assigned (ioat device in my case).

The v.3 patches are:

- [PATCH 1/2] x86: ioremap: fix wrong physical address handling
- [PATCH 2/2] x86: ioremap: fix normal ram range check

Changelog
---------
v.2=>v.3:
- Matthew Wilcox suggested changing PAGE_MASK definition. But I think it is a little risky. Instead of changing PAGE_MASK, v.3 patch uses PHYSICAL_PAGE_MASK(*) to mask physical address again, as v.1 patch did. The PHYSICAL_PAGE_MASK doesn't work for higher than 2^44 physical address because __PHYSICAL_MASK_SHIFT is 44 in x86_32 PAE(*). But this time, I focus on making 2^44 work correctly.

(*) Current X86_32 PAE kernel would not work with physical address
higher than 2^44, because PFNs are handled using 32-bit variable (unsigned long) probably in many places. This seems the reason why __PHYSICAL_MASK_SHIFT is limited to 44 (=32+12 is the limit we can fit into an unsigned long pfn). When x86 PAE linux becomes above 2^44 physical address capable in someday, __PHYSICAL_MASK_SHIFT should become greater value than 44.

v.1=>v.2:
The v.1 patchset was based on my misunderstanding about architectural limit and linux memory management limit of physical address. So some patches in v.1 were totally wrong fixes and they are removed. Thanks to the feedbacks for v.1 patchset, it turned out that phsical address is handled improperly (higher 32-bits are cleared unexpectedly) also in the other places than where v.1 patchset fixed. Here are summary of changes.

- Stop using PHYSICAL_PAGE_MASK to align physical address because PHYSICAL_PAGE_MASK would not work if physical address is higher than 44-bit.
- Fix remaining bugs of physical address handling in ioremap() code path according to the feedbacks.
- Fix s_show() in vmalloc.c to show high physical address properly.
- Removed changes against phys_addr_valid() ([PATCH 2/4] in v.1).
- Removed changes against warning message in ioremap() ([PATCH 3/4] in v.1).

Thanks,
Kenji Kaneshige

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

author
#9 lore raw

Hi,

I don't know what version of kernel you're using. But I guess you might also need 4302e0fb7fa5b071e30f3cfb68e85155b3d69d9b (PCI: fix wrong memory address handling in MSI-X).

Regards,
Kenji Kaneshige

Quoted 71 lines · TAKADA Yoshihito

Hi.
I faced same probrem, too. I use x86_64 kernel.
Your patch fix it. it's very effective.
Thanks!

2010年6月18日12:21 Kenji Kaneshige<[email protected]>:

Quoted 50 lines

Hi,

Here is a updated version (v.3) of patchset to fix ioremap() related problem found in x86 32-bit mode. The problem is that ioremap() maps wrong address for the device to which phisical addres higer than 32-bit is assigned (ioat device in my case).

The v.3 patches are:

- [PATCH 1/2] x86: ioremap: fix wrong physical address handling
- [PATCH 2/2] x86: ioremap: fix normal ram range check

Changelog
---------
v.2=>v.3:
- Matthew Wilcox suggested changing PAGE_MASK definition. But I think it is a little risky. Instead of changing PAGE_MASK, v.3 patch uses PHYSICAL_PAGE_MASK(*) to mask physical address again, as v.1 patch did. The PHYSICAL_PAGE_MASK doesn't work for higher than 2^44 physical address because __PHYSICAL_MASK_SHIFT is 44 in x86_32 PAE(*). But this time, I focus on making 2^44 work correctly.

(*) Current X86_32 PAE kernel would not work with physical address
higher than 2^44, because PFNs are handled using 32-bit variable (unsigned long) probably in many places. This seems the reason why __PHYSICAL_MASK_SHIFT is limited to 44 (=32+12 is the limit we can fit into an unsigned long pfn). When x86 PAE linux becomes above 2^44 physical address capable in someday, __PHYSICAL_MASK_SHIFT should become greater value than 44.

v.1=>v.2:
The v.1 patchset was based on my misunderstanding about architectural limit and linux memory management limit of physical address. So some patches in v.1 were totally wrong fixes and they are removed. Thanks to the feedbacks for v.1 patchset, it turned out that phsical address is handled improperly (higher 32-bits are cleared unexpectedly) also in the other places than where v.1 patchset fixed. Here are summary of changes.

- Stop using PHYSICAL_PAGE_MASK to align physical address because PHYSICAL_PAGE_MASK would not work if physical address is higher than 44-bit.
- Fix remaining bugs of physical address handling in ioremap() code path according to the feedbacks.
- Fix s_show() in vmalloc.c to show high physical address properly.
- Removed changes against phys_addr_valid() ([PATCH 2/4] in v.1).
- Removed changes against warning message in ioremap() ([PATCH 3/4] in v.1).

Quoted 9 lines

Thanks,
Kenji Kaneshige

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html