[RFC] FLATMEM: allow memory to start at pfn != 0

[RFC] FLATMEM: allow memory to start at pfn != 0

[Franck Bui-Huu]

This patchset relaxes this constraint. Basically you just need to
define in your platform code (include/asm-mips/mach-foo/spaces.h
probably) PHYS_OFFSET that corresponds to the start of your
physical memory and you're done.

The first patch is just a fix for HIGHMEM, I just found it while
writing this patchset. I haven't done any tests though, I have
no hardwares to play with.

The second and third patchs are the real meat. There are 2 points
that I'm not really sure:

	- PHYS_OFFSET is defined in page.h, I'm not sure if it's
	the right place though.

	- ARCH_PFN_OFFSET is always defined whatever the memory
	model. I don't think it will hurt since physical memory
	always has a starting point in all cases.



Please consider.

		Franck
---

 arch/mips/kernel/setup.c |   30 ++++++++++++++++++++++--------
 arch/mips/mm/init.c      |   30 +++++++++++++-----------------
 include/asm-mips/dma.h   |    1 +
 include/asm-mips/io.h    |    4 ++--
 include/asm-mips/page.h  |   25 +++++++++++++++++++++----
 5 files changed, 59 insertions(+), 31 deletions(-)

[PATCH 1/3] paging_init(): use highend_pfn/highstart_pfn

[Franck Bui-Huu]

From: Franck Bui-Huu <fbuihuu@gmail.com>

This patch makes paging_init() use highend_pfn/highstart_pfn globals.

It removes the need of 'high' local which was needed only by
HIGHMEM config.

More important perhaps, it fixes a bug when HIGHMEM is set
but there's actually no physical highmem (highend_pfn = 0)

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 arch/mips/mm/init.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index 1db991d..30245c0 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -341,7 +341,7 @@ static int __init page_is_ram(unsigned l
 void __init paging_init(void)
 {
 	unsigned long zones_size[MAX_NR_ZONES] = { 0, };
-	unsigned long max_dma, high, low;
+	unsigned long max_dma, low;
 #ifndef CONFIG_FLATMEM
 	unsigned long zholes_size[MAX_NR_ZONES] = { 0, };
 	unsigned long i, j, pfn;
@@ -356,7 +356,6 @@ void __init paging_init(void)
 
 	max_dma = virt_to_phys((char *)MAX_DMA_ADDRESS) >> PAGE_SHIFT;
 	low = max_low_pfn;
-	high = highend_pfn;
 
 #ifdef CONFIG_ISA
 	if (low < max_dma)
@@ -369,13 +368,13 @@ void __init paging_init(void)
 	zones_size[ZONE_DMA] = low;
 #endif
 #ifdef CONFIG_HIGHMEM
-	if (cpu_has_dc_aliases) {
-		printk(KERN_WARNING "This processor doesn't support highmem.");
-		if (high - low)
-			printk(" %ldk highmem ignored", high - low);
-		printk("\n");
-	} else
-		zones_size[ZONE_HIGHMEM] = high - low;
+	zones_size[ZONE_HIGHMEM] = highend_pfn - highstart_pfn;
+
+	if (cpu_has_dc_aliases && zones_size[ZONE_HIGHMEM]) {
+		printk(KERN_WARNING "This processor doesn't support highmem."
+		       " %ldk highmem ignored\n", zones_size[ZONE_HIGHMEM]);
+		zones_size[ZONE_HIGHMEM] = 0;
+	}
 #endif
 
 #ifdef CONFIG_FLATMEM
-- 
1.4.4.1

[PATCH 2/3] Setup min_low_pfn/max_low_pfn correctly

[Franck Bui-Huu]

From: Franck Bui-Huu <fbuihuu@gmail.com>

The old code was assuming that min_low_pfn was always 0. This
means that we can't handle platforms with a big hole at start
of memory since mem_map[] size would blew up.

This patch does not relax this constraint but it's a first
step to achieve that.

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 arch/mips/kernel/setup.c |   27 +++++++++++++++++++--------
 arch/mips/mm/init.c      |   15 ++++++---------
 include/asm-mips/dma.h   |    1 +
 3 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 89440a0..8e58d7f 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -271,7 +271,6 @@ static void __init bootmem_init(void)
 static void __init bootmem_init(void)
 {
 	unsigned long reserved_end;
-	unsigned long highest = 0;
 	unsigned long mapstart = -1UL;
 	unsigned long bootmap_size;
 	int i;
@@ -284,6 +283,13 @@ static void __init bootmem_init(void)
 	reserved_end = max(init_initrd(), PFN_UP(__pa_symbol(&_end)));
 
 	/*
+	 * max_low_pfn is not a number of pages. The number of pages
+	 * of the system is given by 'max_low_pfn - min_low_pfn'.
+	 */
+	min_low_pfn = -1UL;
+	max_low_pfn = 0;
+
+	/*
 	 * Find the highest page frame number we have available.
 	 */
 	for (i = 0; i < boot_mem_map.nr_map; i++) {
@@ -296,8 +302,10 @@ static void __init bootmem_init(void)
 		end = PFN_DOWN(boot_mem_map.map[i].addr
 				+ boot_mem_map.map[i].size);
 
-		if (end > highest)
-			highest = end;
+		if (end > max_low_pfn)
+			max_low_pfn = end;
+		if (start < min_low_pfn)
+			min_low_pfn = start;
 		if (end <= reserved_end)
 			continue;
 		if (start >= mapstart)
@@ -305,22 +313,25 @@ static void __init bootmem_init(void)
 		mapstart = max(reserved_end, start);
 	}
 
+	if (min_low_pfn >= max_low_pfn)
+		panic("Boggus memory mapping !!!");
+
 	/*
 	 * Determine low and high memory ranges
 	 */
-	if (highest > PFN_DOWN(HIGHMEM_START)) {
+	if (max_low_pfn > PFN_DOWN(HIGHMEM_START)) {
 #ifdef CONFIG_HIGHMEM
 		highstart_pfn = PFN_DOWN(HIGHMEM_START);
-		highend_pfn = highest;
+		highend_pfn = max_low_pfn;
 #endif
-		highest = PFN_DOWN(HIGHMEM_START);
+		max_low_pfn = PFN_DOWN(HIGHMEM_START);
 	}
 
 	/*
 	 * Initialize the boot-time allocator with low memory only.
 	 */
-	bootmap_size = init_bootmem(mapstart, highest);
-
+	bootmap_size = init_bootmem_node(NODE_DATA(0), mapstart,
+					 min_low_pfn, max_low_pfn);
 	/*
 	 * Register fully available low RAM pages with the bootmem allocator.
 	 */
diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index 30245c0..e78a1b1 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -341,7 +341,6 @@ static int __init page_is_ram(unsigned l
 void __init paging_init(void)
 {
 	unsigned long zones_size[MAX_NR_ZONES] = { 0, };
-	unsigned long max_dma, low;
 #ifndef CONFIG_FLATMEM
 	unsigned long zholes_size[MAX_NR_ZONES] = { 0, };
 	unsigned long i, j, pfn;
@@ -354,18 +353,16 @@ void __init paging_init(void)
 #endif
 	kmap_coherent_init();
 
-	max_dma = virt_to_phys((char *)MAX_DMA_ADDRESS) >> PAGE_SHIFT;
-	low = max_low_pfn;
-
 #ifdef CONFIG_ISA
-	if (low < max_dma)
-		zones_size[ZONE_DMA] = low;
+	if (max_low_pfn < MAX_DMA_PFN)
+		zones_size[ZONE_DMA] = max_low_pfn;
 	else {
-		zones_size[ZONE_DMA] = max_dma;
-		zones_size[ZONE_NORMAL] = low - max_dma;
+		unsigned long start_normal_zone = max(min_low_pfn, MAX_DMA_PFN);
+		zones_size[ZONE_DMA] = MAX_DMA_PFN;
+		zones_size[ZONE_NORMAL] = max_low_pfn - start_normal_zone;
 	}
 #else
-	zones_size[ZONE_DMA] = low;
+	zones_size[ZONE_DMA] = max_low_pfn - min_low_pfn;
 #endif
 #ifdef CONFIG_HIGHMEM
 	zones_size[ZONE_HIGHMEM] = highend_pfn - highstart_pfn;
diff --git a/include/asm-mips/dma.h b/include/asm-mips/dma.h
index 23f789c..e06ef07 100644
--- a/include/asm-mips/dma.h
+++ b/include/asm-mips/dma.h
@@ -91,6 +91,7 @@
 #else
 #define MAX_DMA_ADDRESS		(PAGE_OFFSET + 0x01000000)
 #endif
+#define MAX_DMA_PFN		PFN_DOWN(virt_to_phys((void *)MAX_DMA_ADDRESS))
 
 /* 8237 DMA controllers */
 #define IO_DMA1_BASE	0x00	/* 8 bit slave DMA, channels 0..3 */
-- 
1.4.4.1

Re: [PATCH 2/3] Setup min_low_pfn/max_low_pfn correctly

[Ralf Baechle]

On Wed, Dec 06, 2006 at 04:48:29PM +0100, Franck Bui-Huu wrote:

> The old code was assuming that min_low_pfn was always 0. This
> means that we can't handle platforms with a big hole at start
> of memory since mem_map[] size would blew up.

It was - and IP22 was paying a high price for that.  It's currently 1MB
on 32-bit and 1.75MB on 64-bit kernels - but used to be much higher in
the past.

Btw, I think you're confusing the terms here; your patch description reads
wrong while the patch looks fine.  PFN is page frame number, not physical
frame number.  To avoid wasting dead mem_map[] entries idealy the pfn for
the first allocatable page should be zero.
So ignoring NUMA and discontig memory for a moment (those make everything
more complicated ...) PFN 0 is the first entry in mem_map[] - which usually
but not necessarily is physical address 0x0.

> This patch does not relax this constraint but it's a first
> step to achieve that.

> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index 89440a0..8e58d7f 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -271,7 +271,6 @@ static void __init bootmem_init(void)
>  static void __init bootmem_init(void)
>  {
>  	unsigned long reserved_end;
> -	unsigned long highest = 0;
>  	unsigned long mapstart = -1UL;

Assigning a negative number to an unsigned long variable ...

>  	unsigned long bootmap_size;
>  	int i;
> @@ -284,6 +283,13 @@ static void __init bootmem_init(void)
>  	reserved_end = max(init_initrd(), PFN_UP(__pa_symbol(&_end)));
>  
>  	/*
> +	 * max_low_pfn is not a number of pages. The number of pages
> +	 * of the system is given by 'max_low_pfn - min_low_pfn'.
> +	 */
> +	min_low_pfn = -1UL;

Assigning a negative number to an unsigned long variable ...

  Ralf

Re: [PATCH 2/3] Setup min_low_pfn/max_low_pfn correctly

[Franck Bui-Huu]

On 12/11/06, Ralf Baechle <ralf@linux-mips.org> wrote:
> On Wed, Dec 06, 2006 at 04:48:29PM +0100, Franck Bui-Huu wrote:
>
> > The old code was assuming that min_low_pfn was always 0. This
> > means that we can't handle platforms with a big hole at start
> > of memory since mem_map[] size would blew up.
>
> It was - and IP22 was paying a high price for that.  It's currently 1MB
> on 32-bit and 1.75MB on 64-bit kernels - but used to be much higher in
> the past.
>
> Btw, I think you're confusing the terms here; your patch description reads
> wrong while the patch looks fine.  PFN is page frame number, not physical
> frame number.  To avoid wasting dead mem_map[] entries idealy the pfn for
> the first allocatable page should be zero.
> So ignoring NUMA and discontig memory for a moment (those make everything
> more complicated ...) PFN 0 is the first entry in mem_map[] - which usually
> but not necessarily is physical address 0x0.
>

well if you look at the generic definition of pfn_to_page(), you find:

        #define __pfn_to_page(pfn)      (mem_map + ((pfn) - ARCH_PFN_OFFSET))

With the current implementation, PFN 0 is something that does not
exist. PFN ARCH_PFN_OFFSET is the first entry in mem_map[]...

I think I'm missing your point....

> > This patch does not relax this constraint but it's a first
> > step to achieve that.
>
> > diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> > index 89440a0..8e58d7f 100644
> > --- a/arch/mips/kernel/setup.c
> > +++ b/arch/mips/kernel/setup.c
> > @@ -271,7 +271,6 @@ static void __init bootmem_init(void)
> >  static void __init bootmem_init(void)
> >  {
> >       unsigned long reserved_end;
> > -     unsigned long highest = 0;
> >       unsigned long mapstart = -1UL;
>
> Assigning a negative number to an unsigned long variable ...
>

Well, it's a convenient way to set mapstart to 0xffffffff on both
32bits and 64bits kernels as you guessed. I think it's quite readable
thanks to the 'UL' suffix. But I'll change the whole line with:

        unsigned long mapstart = ~(0UL);

> >       unsigned long bootmap_size;
> >       int i;
> > @@ -284,6 +283,13 @@ static void __init bootmem_init(void)
> >       reserved_end = max(init_initrd(), PFN_UP(__pa_symbol(&_end)));
> >
> >       /*
> > +      * max_low_pfn is not a number of pages. The number of pages
> > +      * of the system is given by 'max_low_pfn - min_low_pfn'.
> > +      */
> > +     min_low_pfn = -1UL;
>
> Assigning a negative number to an unsigned long variable ...

ditto

thanks
-- 
               Franck

[PATCH 3/3] FLATMEM: allow memory to start at pfn != 0

[Franck Bui-Huu]

From: Franck Bui-Huu <fbuihuu@gmail.com>

This patch introduces PHYS_OFFSET define which is the start of the
physical memory and uses it wherever needed. Specially when converting
physical/virtual addresses into virtual/physical ones.

Currently all platforms defines PHYS_OFFSET to 0.

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 arch/mips/kernel/setup.c |    3 +++
 include/asm-mips/io.h    |    4 ++--
 include/asm-mips/page.h  |   25 +++++++++++++++++++++----
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 8e58d7f..46312c2 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -315,6 +315,9 @@ static void __init bootmem_init(void)
 
 	if (min_low_pfn >= max_low_pfn)
 		panic("Boggus memory mapping !!!");
+	if (min_low_pfn != ARCH_PFN_OFFSET)
+		panic("PHYS_OFFSET(%lx) and your mem start(%lx) don't match",
+		      PHYS_OFFSET, PFN_PHYS(min_low_pfn));
 
 	/*
 	 * Determine low and high memory ranges
diff --git a/include/asm-mips/io.h b/include/asm-mips/io.h
index 38d1399..e1592af 100644
--- a/include/asm-mips/io.h
+++ b/include/asm-mips/io.h
@@ -115,7 +115,7 @@ static inline void set_io_port_base(unsi
  */
 static inline unsigned long virt_to_phys(volatile const void *address)
 {
-	return (unsigned long)address - PAGE_OFFSET;
+	return (unsigned long)address - PAGE_OFFSET + PHYS_OFFSET;
 }
 
 /*
@@ -132,7 +132,7 @@ static inline unsigned long virt_to_phys
  */
 static inline void * phys_to_virt(unsigned long address)
 {
-	return (void *)(address + PAGE_OFFSET);
+	return (void *)(address + PAGE_OFFSET - PHYS_OFFSET);
 }
 
 /*
diff --git a/include/asm-mips/page.h b/include/asm-mips/page.h
index 5c4284b..6894dc2 100644
--- a/include/asm-mips/page.h
+++ b/include/asm-mips/page.h
@@ -34,6 +34,20 @@
 
 #ifndef __ASSEMBLY__
 
+/*
+ * This gives the physical RAM offset.
+ */
+#ifndef PHYS_OFFSET
+#define PHYS_OFFSET		0UL
+#endif
+
+/*
+ * It's normally defined only for FLATMEM config but it's
+ * used in our early mem init code for all memory models.
+ * So always define it.
+ */
+#define ARCH_PFN_OFFSET		PFN_UP(PHYS_OFFSET)
+
 #include <linux/pfn.h>
 #include <asm/cpu-features.h>
 #include <asm/io.h>
@@ -133,20 +147,23 @@ typedef struct { unsigned long pgprot; }
 /* to align the pointer to the (next) page boundary */
 #define PAGE_ALIGN(addr)	(((addr) + PAGE_SIZE - 1) & PAGE_MASK)
 
+/*
+ * __pa()/__va() should be used only during mem init.
+ */
 #if defined(CONFIG_64BIT) && !defined(CONFIG_BUILD_ELF64)
 #define __pa_page_offset(x)	((unsigned long)(x) < CKSEG0 ? PAGE_OFFSET : CKSEG0)
 #else
 #define __pa_page_offset(x)	PAGE_OFFSET
 #endif
-#define __pa(x)			((unsigned long)(x) - __pa_page_offset(x))
-#define __pa_symbol(x)		__pa(RELOC_HIDE((unsigned long)(x),0))
-#define __va(x)			((void *)((unsigned long)(x) + PAGE_OFFSET))
+#define __pa(x)		((unsigned long)(x) - __pa_page_offset(x) + PHYS_OFFSET)
+#define __va(x)		((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
+#define __pa_symbol(x)	__pa(RELOC_HIDE((unsigned long)(x),0))
 
 #define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
 
 #ifdef CONFIG_FLATMEM
 
-#define pfn_valid(pfn)		((pfn) < max_mapnr)
+#define pfn_valid(pfn)		((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
 
 #elif defined(CONFIG_SPARSEMEM)
 
-- 
1.4.4.1

Re: [RFC] FLATMEM: allow memory to start at pfn != 0

[Ralf Baechle]

On Wed, Dec 06, 2006 at 04:48:27PM +0100, Franck Bui-Huu wrote:

I just tested this on a Malta.  So patch 2/3 makes Malta die pretty
spectacularly, so I'm going to remve patches 2/3 and 3/3 again from my
tree.

Btw, there's spelling mistake in 2/3:

+               panic("Boggus memory mapping !!!");

  Ralf

Re: [RFC] FLATMEM: allow memory to start at pfn != 0

[Franck Bui-Huu]

Hi Ralf,

On 12/11/06, Ralf Baechle <ralf@linux-mips.org> wrote:
> On Wed, Dec 06, 2006 at 04:48:27PM +0100, Franck Bui-Huu wrote:
>
> I just tested this on a Malta.  So patch 2/3 makes Malta die pretty
> spectacularly, so I'm going to remve patches 2/3 and 3/3 again from my
> tree.
>

Thanks for your review ! Could you give me some info about your config
to help me to find out the issue ? For example what's your value of
PHYS_OFFSET ? your memory mapping ?

> Btw, there's spelling mistake in 2/3:
>
> +               panic("Boggus memory mapping !!!");

I'll fix it by "Incorrect memory mapping !!!"

thanks
-- 
               Franck

Re: [RFC] FLATMEM: allow memory to start at pfn != 0

[Franck Bui-Huu]

On 12/11/06, Ralf Baechle <ralf@linux-mips.org> wrote:
> On Wed, Dec 06, 2006 at 04:48:27PM +0100, Franck Bui-Huu wrote:
>
> I just tested this on a Malta.  So patch 2/3 makes Malta die pretty
> spectacularly, so I'm going to remve patches 2/3 and 3/3 again from my
> tree.
>

When you'll have time, could you test only patch 2/3. It's only a
clean up patch, which eases integration of patch 3/3 ? This clean up
should improve current code even if you don't merge patch 3/3. And
more importantly,  knowing that patch 2/3 woks should help me to find
out what's wrong with your config.

Thanks
                Franck