[BUG][PATCH 0/2 (v.3)] x86: ioremap() problem in X86_32 PAE

[BUG][PATCH 0/2 (v.3)] x86: ioremap() problem in X86_32 PAE

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

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

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

[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 <kaneshige.kenji@jp.fujitsu.com>

---
 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
===================================================================
--- linux-2.6.34.orig/arch/x86/mm/ioremap.c
+++ 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,
Index: linux-2.6.34/include/linux/vmalloc.h
===================================================================
--- linux-2.6.34.orig/include/linux/vmalloc.h
+++ 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;
 };
 
Index: linux-2.6.34/lib/ioremap.c
===================================================================
--- linux-2.6.34.orig/lib/ioremap.c
+++ 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;
Index: linux-2.6.34/include/linux/io.h
===================================================================
--- linux-2.6.34.orig/include/linux/io.h
+++ 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;
 }
Index: linux-2.6.34/mm/vmalloc.c
===================================================================
--- linux-2.6.34.orig/mm/vmalloc.c
+++ 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");

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

[Jeremy Fitzhardinge]

On 06/18/2010 04:22 AM, Kenji Kaneshige wrote:
> 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 <kaneshige.kenji@jp.fujitsu.com>
>
> ---
>  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
> ===================================================================
> --- linux-2.6.34.orig/arch/x86/mm/ioremap.c
> +++ 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;
>   

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.

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

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

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

[Kenji Kaneshige]

(2010/06/18 20:07), Jeremy Fitzhardinge wrote:
> On 06/18/2010 04:22 AM, Kenji Kaneshige wrote:
>> 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<kaneshige.kenji@jp.fujitsu.com>
>>
>> ---
>>   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
>> ===================================================================
>> --- linux-2.6.34.orig/arch/x86/mm/ioremap.c
>> +++ 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;
>>
>
> 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?


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

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

[tip-bot for Kenji Kaneshige]

Commit-ID:  ffa71f33a820d1ab3f2fc5723819ac60fb76080b
Gitweb:     http://git.kernel.org/tip/ffa71f33a820d1ab3f2fc5723819ac60fb76080b
Author:     Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
AuthorDate: Fri, 18 Jun 2010 12:22:40 +0900
Committer:  H. Peter Anvin <hpa@linux.intel.com>
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 <kaneshige.kenji@jp.fujitsu.com>
LKML-Reference: <4C1AE680.7090408@jp.fujitsu.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 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(-)

diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 12e4d2d..754cb4c 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -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,
diff --git a/include/linux/io.h b/include/linux/io.h
index 6c7f0ba..7fd2d21 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -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;
 }
diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 227c2a5..de05e96 100644
--- a/include/linux/vmalloc.h
+++ b/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;
 };
 
diff --git a/lib/ioremap.c b/lib/ioremap.c
index 14c6078..5730ecd 100644
--- a/lib/ioremap.c
+++ b/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, 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;
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index ae00746..b7e314b 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -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");

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

[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 <kaneshige.kenji@jp.fujitsu.com>

---
 arch/x86/mm/ioremap.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.34/arch/x86/mm/ioremap.c
===================================================================
--- linux-2.6.34.orig/arch/x86/mm/ioremap.c
+++ 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)))

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

[tip-bot for Kenji Kaneshige]

Commit-ID:  35be1b716a475717611b2dc04185e9d80b9cb693
Gitweb:     http://git.kernel.org/tip/35be1b716a475717611b2dc04185e9d80b9cb693
Author:     Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
AuthorDate: Fri, 18 Jun 2010 12:23:57 +0900
Committer:  H. Peter Anvin <hpa@linux.intel.com>
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 <kaneshige.kenji@jp.fujitsu.com>
LKML-Reference: <4C1AE6CD.1080704@jp.fujitsu.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/mm/ioremap.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 754cb4c..d41d3a9 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -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)))

Re: [BUG][PATCH 0/2 (v.3)] x86: ioremap() problem in X86_32 PAE

[H. Peter Anvin]

On 06/17/2010 08:21 PM, Kenji Kaneshige wrote:
> 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

Re: [BUG][PATCH 0/2 (v.3)] x86: ioremap() problem in X86_32 PAE

[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 <kaneshige.kenji@jp.fujitsu.com>:
> 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 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/
>

Re: [BUG][PATCH 0/2 (v.3)] x86: ioremap() problem in X86_32 PAE

[Kenji Kaneshige]

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

(2011/02/10 23:11), TAKADA Yoshihito wrote:
> 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<kaneshige.kenji@jp.fujitsu.com>:
>> 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 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/
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

[BUG][PATCH 0/2 (v.2)] x86: ioremap() problem in X86_32 PAE

[Kenji Kaneshige]

Hi,

Here is a updated version (v.2) of patchset to fix ioremap() problem
found in x86_32 PAE mode. The problem is that ioremap() maps wrong
address for the devices to which phisical addresses higer than 32-bit
are assigned (ioat device in my case).

Changelog
----
v.1 => v.2:
Thanks to the feedbacks, I found that some of my v.1 patches are totally
wrong because they were based on my mis-understanding about architectural
limit and linux memory management limit of physical address. In addition,
it turned out that there are bugs that handles physical address improperly
(higher 32-bits are cleared unexpectedly) also in the other places in the
ioremap() code path. Major changes are
- Removed wrong changes against phys_addr_valid() ([PATCH 2/4] in v.1).
- Removed wrong changes against wraning message in ioremap() ([PATCH 3/4]
  in v.1)
- Changed not to use PHYSICAL_PAGE_MASK because the PHYSICAL_PAGE_MASK
  would not work for physical address higher than 44-bit.
- Added fixes for remaining bugs of physical address handling in ioremap()
  code path according to the feedbacks.
- Added a fix for s_show() in vmalloc.c to show high physical address properly.
----

I also found the bug in PCI MSI-X code that handles physical address
improperly, which causes the problem MSI-X doesn't work on my devices. I'll
send a patch for this to PCI mail list separately.

The v.2 patches are:

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

Thanks,
Kenji Kaneshige

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

[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 <kaneshige.kenji@jp.fujitsu.com>

---
 arch/x86/mm/ioremap.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.34/arch/x86/mm/ioremap.c
===================================================================
--- linux-2.6.34.orig/arch/x86/mm/ioremap.c	2010-06-15 05:33:06.272381451 +0900
+++ linux-2.6.34/arch/x86/mm/ioremap.c	2010-06-15 05:33:21.086585690 +0900
@@ -101,7 +101,7 @@
 	 * 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)))