x86: ioremap() problem in X86_32 PAE

arch/x86/mm/ioremap.c line 66

Quoted 12 lines · Kenji Kaneshige

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

arch/x86/mm/ioremap.c +2 -2

@@ -62,8 +62,8 @@
  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;

I have a hard time understanding this change. pfn is always a physical address shifted by PAGE_SHIFT. So a 32-bit pfn supports up to 44-bit physical addresses. Are your addresses above 44-bits?

Quoted 11 lines · Matthew Wilcox

Quoted 10 lines · Kenji Kaneshige

Index: linux-2.6.34/arch/x86/mm/ioremap.c
===================================================================
--- linux-2.6.34.orig/arch/x86/mm/ioremap.c 2010-06-15 04:43:00.978332015 +0900
+++ linux-2.6.34/arch/x86/mm/ioremap.c 2010-06-15 05:32:59.291693007 +0900

arch/x86/mm/ioremap.c +0 -0

@@ -62,8 +62,8 @@
 static void __iomem *__ioremap_caller(resource_size_t phys_addr,
 unsigned long size, unsigned long prot_val, void *caller)
 {

Quoted 9 lines

arch/x86/mm/ioremap.c line 66

Quoted 4 lines

arch/x86/mm/ioremap.c +2 -2

- unsigned long pfn, offset, vaddr;
- resource_size_t last_addr;
+ unsigned long offset, vaddr;
+ resource_size_t pfn, last_pfn, last_addr;

I have a hard time understanding this change. pfn is always a physical address shifted by PAGE_SHIFT. So a 32-bit pfn supports up to 44-bit physical addresses. Are your addresses above 44-bits?

I think they might be. Kenji?

Quoted 1 line

PAGE_SHIFT) << PAGE_SHIFT)". The PHYSICAL_PAGE_MASK would become

Quoted 2 lines

I think they might be. Kenji?

No. My addresses are in the 44-bits range (around fc000000000). So it is not required for my problem. This change assumes that phys_addr can be above 44-bits (up to 52-bits (and higher in the future?)).

By the way, is there linux kernel limit regarding above 44-bits physical address in x86_32 PAE? For example, pfn above 32-bits is not supported?

#ifdef CONFIG_X86_PAE
/* 44=32+12, the limit we can fit into an unsigned long pfn */
#define __PHYSICAL_MASK_SHIFT 44
#define __VIRTUAL_MASK_SHIFT 32

If there is 44-bits physical address limit, I think it's better to use PHYSICAL_PAGE_MASK for masking physical address, instead of "(phys_addr

Quoted 1 line

Quoted 1 line

PAGE_SHIFT) << PAGE_SHIFT)". The PHYSICAL_PAGE_MASK would become

greater value when 44-bits physical address limit is eliminated. And maybe we need to change phys_addr_valid() returns error if physical address is above (1 << __PHYSICAL_MASK_SHIFT)?

Quoted 10 lines · Kenji Kaneshige

Quoted 2 lines

I think they might be. Kenji?

No. My addresses are in the 44-bits range (around fc000000000). So it is not required for my problem. This change assumes that phys_addr can be above 44-bits (up to 52-bits (and higher in the future?)).

By the way, is there linux kernel limit regarding above 44-bits physical address in x86_32 PAE? For example, pfn above 32-bits is not supported?

There are probably places at which PFNs are held in 32-bit numbers, although it would be good to track them down if it isn't too expensive to fix them (i.e. doesn't affect generic code.)

This also affects paravirt systems, i.e. right now Xen has to locate all 32-bit guests below 64 GB, which limits its usefulness.

Quoted 5 lines

#ifdef CONFIG_X86_PAE
/* 44=32+12, the limit we can fit into an unsigned long pfn */
#define __PHYSICAL_MASK_SHIFT 44
#define __VIRTUAL_MASK_SHIFT 32

Quoted 6 lines

If there is 44-bits physical address limit, I think it's better to use PHYSICAL_PAGE_MASK for masking physical address, instead of "(phys_addr

Quoted 1 line

Quoted 1 line

PAGE_SHIFT)<< PAGE_SHIFT)". The PHYSICAL_PAGE_MASK would become

greater value when 44-bits physical address limit is eliminated. And maybe we need to change phys_addr_valid() returns error if physical address is above (1<< __PHYSICAL_MASK_SHIFT)?

The real question is how much we can fix without an unreasonable cost.

Quoted 2 lines · Kenji Kaneshige

Quoted 2 lines

I think they might be. Kenji?

Quoted 8 lines

No. My addresses are in the 44-bits range (around fc000000000). So it is not required for my problem. This change assumes that phys_addr can be above 44-bits (up to 52-bits (and higher in the future?)).

By the way, is there linux kernel limit regarding above 44-bits physical address in x86_32 PAE? For example, pfn above 32-bits is not supported?

There are probably places at which PFNs are held in 32-bit numbers, although it would be good to track them down if it isn't too expensive to fix them (i.e. doesn't affect generic code.)

This also affects paravirt systems, i.e. right now Xen has to locate all 32-bit guests below 64 GB, which limits its usefulness.

Quoted 8 lines

#ifdef CONFIG_X86_PAE
/* 44=32+12, the limit we can fit into an unsigned long pfn */
#define __PHYSICAL_MASK_SHIFT 44
#define __VIRTUAL_MASK_SHIFT 32

If there is 44-bits physical address limit, I think it's better to use PHYSICAL_PAGE_MASK for masking physical address, instead of "(phys_addr

Quoted 6 lines

Quoted 2 lines

Quoted 2 lines

PAGE_SHIFT) << PAGE_SHIFT)". The PHYSICAL_PAGE_MASK would become

greater value when 44-bits physical address limit is eliminated. And maybe we need to change phys_addr_valid() returns error if physical address is above (1 << __PHYSICAL_MASK_SHIFT)?

The real question is how much we can fix without an unreasonable cost.

I guess it would be possible to special-case ioremap to allow the creation of such mappings, but I don't know what kind of system-wide fallout would happen as a result. The consequences of something trying to extract a pfn from one of those ptes would be

Quoted 3 lines

Quoted 3 lines

By the way, is there linux kernel limit regarding above 44-bits physical address in x86_32 PAE? For example, pfn above 32-bits is not supported?

That's an awkward situation. I would tend to suggest that you not support this type of machine with a 32-bit kernel. Is it a sparse memory system, or is there a device mapped in that range?

I guess it would be possible to special-case ioremap to allow the creation of such mappings, but I don't know what kind of system-wide fallout would happen as a result. The consequences of something trying to extract a pfn from one of those ptes would be

Quoted 4 lines

There are probably places at which PFNs are held in 32-bit numbers, although it would be good to track them down if it isn't too expensive to fix them (i.e. doesn't affect generic code.)

There are many places which hold pfns in 32 bit variables on 32 bit systems; the standard type for pfns is "unsigned long", pretty much everywhere in the kernel. It might be worth defining a pfn_t and converting usage over to that, but it would be a pervasive change.

Quoted 18 lines · Jeremy Fitzhardinge

Quoted 3 lines

Quoted 3 lines

By the way, is there linux kernel limit regarding above 44-bits physical address in x86_32 PAE? For example, pfn above 32-bits is not supported?

That's an awkward situation. I would tend to suggest that you not support this type of machine with a 32-bit kernel. Is it a sparse memory system, or is there a device mapped in that range?

I guess it would be possible to special-case ioremap to allow the creation of such mappings, but I don't know what kind of system-wide fallout would happen as a result. The consequences of something trying to extract a pfn from one of those ptes would be

Quoted 4 lines

There are probably places at which PFNs are held in 32-bit numbers, although it would be good to track them down if it isn't too expensive to fix them (i.e. doesn't affect generic code.)

Quoted 5 lines

There are many places which hold pfns in 32 bit variables on 32 bit systems; the standard type for pfns is "unsigned long", pretty much everywhere in the kernel. It might be worth defining a pfn_t and converting usage over to that, but it would be a pervasive change.

I think you're right, and just making 2^44 work correctly would be good enough. Doing special forwarding of all 52 bits of the real physical address in the paravirt case (where it is self-contained and doesn't spill into the rest of the kernel) would probably be a good thing, though.

-hpa

Quoted 8 lines · H. Peter Anvin

Quoted 6 lines · Kenji Kaneshige

Quoted 2 lines

I think they might be. Kenji?

No. My addresses are in the 44-bits range (around fc000000000). So it is not required for my problem. This change assumes that phys_addr can be above 44-bits (up to 52-bits (and higher in the future?)).

Quoted 4 lines

Quoted 4 lines

By the way, is there linux kernel limit regarding above 44-bits physical address in x86_32 PAE? For example, pfn above 32-bits is not supported?

That's an awkward situation. I would tend to suggest that you not support this type of machine with a 32-bit kernel. Is it a sparse memory system, or is there a device mapped in that range?

I guess it would be possible to special-case ioremap to allow the creation of such mappings, but I don't know what kind of system-wide fallout would happen as a result. The consequences of something trying to extract a pfn from one of those ptes would be

Quoted 4 lines

There are probably places at which PFNs are held in 32-bit numbers, although it would be good to track them down if it isn't too expensive to fix them (i.e. doesn't affect generic code.)

There are many places which hold pfns in 32 bit variables on 32 bit systems; the standard type for pfns is "unsigned long", pretty much everywhere in the kernel. It might be worth defining a pfn_t and converting usage over to that, but it would be a pervasive change.

Quoted 3 lines

This also affects paravirt systems, i.e. right now Xen has to locate all 32-bit guests below 64 GB, which limits its usefulness.

I don't think the limit is 64GB. A 32-bit PFN limits us to 2^44, which is 16TB. (32-bit PV Xen guests have another unrelated limit of around 160GB physical memory because that as much m2p table will fit into the Xen hole in the kernel mapping.)

Quoted 8 lines

Quoted 8 lines

#ifdef CONFIG_X86_PAE
/* 44=32+12, the limit we can fit into an unsigned long pfn */
#define __PHYSICAL_MASK_SHIFT 44
#define __VIRTUAL_MASK_SHIFT 32

If there is 44-bits physical address limit, I think it's better to use PHYSICAL_PAGE_MASK for masking physical address, instead of "(phys_addr

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

Quoted 2 lines

PAGE_SHIFT)<< PAGE_SHIFT)". The PHYSICAL_PAGE_MASK would become

greater value when 44-bits physical address limit is eliminated. And maybe we need to change phys_addr_valid() returns error if physical address is above (1<< __PHYSICAL_MASK_SHIFT)?

The real question is how much we can fix without an unreasonable cost.

I think it would be a pretty large change. From the Xen's perspective, any machine even approximately approaching the 2^44 limit will be capable of running Xen guests in hvm mode, so PV isn't really a concern.

J

Quoted 1 line · H. Peter Anvin

On 06/16/2010 09:55 PM, Kenji Kaneshige wrote:

Quoted 6 lines

Quoted 4 lines

greater value when 44-bits physical address limit is eliminated. And maybe we need to change phys_addr_valid() returns error if physical address is above (1 << __PHYSICAL_MASK_SHIFT)?

The real question is how much we can fix without an unreasonable cost.

I think it would be a pretty large change. From the Xen's perspective, any machine even approximately approaching the 2^44 limit will be capable of running Xen guests in hvm mode, so PV isn't really a concern.

Quoted 4 lines

I think it would be a pretty large change. From the Xen's perspective, any machine even approximately approaching the 2^44 limit will be capable of running Xen guests in hvm mode, so PV isn't really a concern.

Hi Jeremy,

Is the implication of that statement that HVM is preferred where supported by HW?

Quoted 9 lines · Simon Horman

Quoted 4 lines

I think it would be a pretty large change. From the Xen's perspective, any machine even approximately approaching the 2^44 limit will be capable of running Xen guests in hvm mode, so PV isn't really a concern.

Hi Jeremy,

Is the implication of that statement that HVM is preferred where supported by HW?

I wouldn't go that far; the PV vs HVM choice is pretty complex, and depends on what your workload is and what hardware you have available. All I meant was what I said: that if you're running on a machine with a large amount of memory, then you should run your 32-bit domains as HVM rather than PV. Though Xen could easily keep domains limited to memory that they can actually use (it already does this, in fact).

arch/x86/mm/ioremap.c line 66

Quoted 12 lines · Kenji Kaneshige

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

arch/x86/mm/ioremap.c +2 -2

@@ -62,8 +62,8 @@
  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;

I have a hard time understanding this change. pfn is always a physical address shifted by PAGE_SHIFT. So a 32-bit pfn supports up to 44-bit physical addresses. Are your addresses above 44-bits?

arch/x86/mm/ioremap.c line 116

Quoted 6 lines

arch/x86/mm/ioremap.c +1 -1

@@ -115,7 +113,7 @@
  	 * Mappings have to be page-aligned
  	 */
  	offset = phys_addr&  ~PAGE_MASK;
-	phys_addr&= PAGE_MASK;
+	phys_addr = (phys_addr>>  PAGE_SHIFT)<<  PAGE_SHIFT;

I'd rather see PAGE_MASK fixed. Would this work?

  #define PAGE_SIZE       (_AC(1,UL)<<  PAGE_SHIFT)

Commit-ID:  39d8c3ff39443825b6a21b28249fc4904809203f
Gitweb:     http://git.kernel.org/tip/39d8c3ff39443825b6a21b28249fc4904809203f
Author:     Kenji Kaneshige <[email protected]>
AuthorDate: Thu, 17 Jun 2010 10:30:06 +0900
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Fri, 9 Jul 2010 10:51:39 -0700

x86, pae: Fix handling of large physical addresses in ioremap

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