[patch 0/4] [PATCH] x86: Merge e820_32/64

[patch 0/4] [PATCH] x86: Merge e820_32/64

[Christoph Lameter]

The following patchset merges the 32 and 64 bit of e820 header files into one.

First just move the stuff into a single file and then gradually extract common
code.

Patch against Linus git of today.

-- 

[patch 1/4] x86: e820.h unification

[Christoph Lameter]

[-- Attachment #1: e820_unify --]
[-- Type: text/plain, Size: 4448 bytes --]

The first patch simply puts both 32 and 64 bit headers into one header file.
The #ifdef sequence at the top is shared.

Signed-off-by: Christoph Lameter <clameter@sgi.com>

---
 include/asm-x86/e820.h |  102 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 100 insertions(+), 2 deletions(-)

Index: linux-next/include/asm-x86/e820.h
===================================================================
--- linux-next.orig/include/asm-x86/e820.h	2008-05-04 23:01:53.699852096 -0700
+++ linux-next/include/asm-x86/e820.h	2008-05-04 23:03:16.084868751 -0700
@@ -30,9 +30,107 @@ struct e820map {
 
 #ifdef __KERNEL__
 #ifdef CONFIG_X86_32
-# include "e820_32.h"
+/*
+ * structures and definitions for the int 15, ax=e820 memory map
+ * scheme.
+ *
+ * In a nutshell, arch/i386/boot/setup.S populates a scratch table
+ * in the empty_zero_block that contains a list of usable address/size
+ * duples.   In arch/i386/kernel/setup.c, this information is
+ * transferred into the e820map, and in arch/i386/mm/init.c, that
+ * new information is used to mark pages reserved or not.
+ *
+ */
+#include <linux/ioport.h>
+
+#define HIGH_MEMORY	(1024*1024)
+
+#ifndef __ASSEMBLY__
+
+extern void setup_memory_map(void);
+extern void finish_e820_parsing(void);
+
+extern struct e820map e820;
+extern void update_e820(void);
+
+extern int e820_all_mapped(unsigned long start, unsigned long end,
+			   unsigned type);
+extern int e820_any_mapped(u64 start, u64 end, unsigned type);
+extern void propagate_e820_map(void);
+extern void register_bootmem_low_pages(unsigned long max_low_pfn);
+extern void add_memory_region(unsigned long long start,
+			      unsigned long long size, int type);
+extern u64 update_memory_range(u64 start, u64 size, unsigned old_type,
+				unsigned new_type);
+extern void e820_register_memory(void);
+extern void limit_regions(unsigned long long size);
+extern void init_iomem_resources(struct resource *code_resource,
+				 struct resource *data_resource,
+				 struct resource *bss_resource);
+
+#if defined(CONFIG_PM) && defined(CONFIG_HIBERNATION)
+extern void e820_mark_nosave_regions(void);
 #else
-# include "e820_64.h"
+static inline void e820_mark_nosave_regions(void)
+{
+}
+#endif
+
+
+#endif/*!__ASSEMBLY__*/
+#else /* X86_32 */
+/*
+ * structures and definitions for the int 15, ax=e820 memory map
+ * scheme.
+ *
+ * In a nutshell, setup.S populates a scratch table in the
+ * empty_zero_block that contains a list of usable address/size
+ * duples.  setup.c, this information is transferred into the e820map,
+ * and in init.c/numa.c, that new information is used to mark pages
+ * reserved or not.
+ */
+#include <linux/ioport.h>
+
+#ifndef __ASSEMBLY__
+extern unsigned long find_e820_area(unsigned long start, unsigned long end,
+				    unsigned long size, unsigned long align);
+extern unsigned long find_e820_area_size(unsigned long start,
+					 unsigned long *sizep,
+					 unsigned long align);
+extern void add_memory_region(unsigned long start, unsigned long size,
+			      int type);
+extern u64 update_memory_range(u64 start, u64 size, unsigned old_type,
+				unsigned new_type);
+extern void setup_memory_region(void);
+extern void contig_e820_setup(void);
+extern unsigned long e820_end_of_ram(void);
+extern void e820_reserve_resources(void);
+extern void e820_mark_nosave_regions(void);
+extern int e820_any_mapped(unsigned long start, unsigned long end,
+			   unsigned type);
+extern int e820_all_mapped(unsigned long start, unsigned long end,
+			   unsigned type);
+extern int e820_any_non_reserved(unsigned long start, unsigned long end);
+extern int is_memory_any_valid(unsigned long start, unsigned long end);
+extern int e820_all_non_reserved(unsigned long start, unsigned long end);
+extern int is_memory_all_valid(unsigned long start, unsigned long end);
+extern unsigned long e820_hole_size(unsigned long start, unsigned long end);
+
+extern void e820_setup_gap(void);
+extern void e820_register_active_regions(int nid, unsigned long start_pfn,
+					 unsigned long end_pfn);
+
+extern void finish_e820_parsing(void);
+
+extern struct e820map e820;
+extern void update_e820(void);
+
+extern void reserve_early(unsigned long start, unsigned long end, char *name);
+extern void free_early(unsigned long start, unsigned long end);
+extern void early_res_to_bootmem(unsigned long start, unsigned long end);
+
+#endif/*!__ASSEMBLY__*/
+
 #endif
 #endif /* __KERNEL__ */
 

-- 

[patch 2/4] x86: e820 unification: Extract shared comments

[Christoph Lameter]

[-- Attachment #1: e820_comments --]
[-- Type: text/plain, Size: 2114 bytes --]

The comments are basically the same. Take the 64 bit version as the common
comments.

Signed-off-by: Christoph Lameter <clameter@sgi.com>

---
 include/asm-x86/e820.h |   26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

Index: linux-next/include/asm-x86/e820.h
===================================================================
--- linux-next.orig/include/asm-x86/e820.h	2008-05-04 23:03:30.799851792 -0700
+++ linux-next/include/asm-x86/e820.h	2008-05-04 23:04:37.939859869 -0700
@@ -29,20 +29,20 @@ struct e820map {
 #define BIOS_END		0x00100000
 
 #ifdef __KERNEL__
-#ifdef CONFIG_X86_32
+
 /*
  * structures and definitions for the int 15, ax=e820 memory map
  * scheme.
  *
- * In a nutshell, arch/i386/boot/setup.S populates a scratch table
- * in the empty_zero_block that contains a list of usable address/size
- * duples.   In arch/i386/kernel/setup.c, this information is
- * transferred into the e820map, and in arch/i386/mm/init.c, that
- * new information is used to mark pages reserved or not.
- *
+ * In a nutshell, setup.S populates a scratch table in the
+ * empty_zero_block that contains a list of usable address/size
+ * duples.  setup.c, this information is transferred into the e820map,
+ * and in init.c/numa.c, that new information is used to mark pages
+ * reserved or not.
  */
 #include <linux/ioport.h>
 
+#ifdef CONFIG_X86_32
 #define HIGH_MEMORY	(1024*1024)
 
 #ifndef __ASSEMBLY__
@@ -79,18 +79,6 @@ static inline void e820_mark_nosave_regi
 
 #endif/*!__ASSEMBLY__*/
 #else /* X86_32 */
-/*
- * structures and definitions for the int 15, ax=e820 memory map
- * scheme.
- *
- * In a nutshell, setup.S populates a scratch table in the
- * empty_zero_block that contains a list of usable address/size
- * duples.  setup.c, this information is transferred into the e820map,
- * and in init.c/numa.c, that new information is used to mark pages
- * reserved or not.
- */
-#include <linux/ioport.h>
-
 #ifndef __ASSEMBLY__
 extern unsigned long find_e820_area(unsigned long start, unsigned long end,
 				    unsigned long size, unsigned long align);

-- 

[patch 3/4] x86: e820 unification: Common #ifdef __ASSEMBLY

[Christoph Lameter]

[-- Attachment #1: e820_1 --]
[-- Type: text/plain, Size: 1415 bytes --]

Both include files use #ifdef __ASSEMBLY__ which can be moved outside
of the #ifdef CONFIG_X86_32.

Signed-off-by: Christoph Lameter <clameter@sgi.com>

---
 include/asm-x86/e820.h |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Index: linux-2.6/include/asm-x86/e820.h
===================================================================
--- linux-2.6.orig/include/asm-x86/e820.h	2008-05-05 11:59:18.000000000 -0700
+++ linux-2.6/include/asm-x86/e820.h	2008-05-05 11:59:56.000000000 -0700
@@ -42,11 +42,11 @@ struct e820map {
  */
 #include <linux/ioport.h>
 
+#ifndef __ASSEMBLY__
+
 #ifdef CONFIG_X86_32
 #define HIGH_MEMORY	(1024*1024)
 
-#ifndef __ASSEMBLY__
-
 extern void setup_memory_map(void);
 extern void finish_e820_parsing(void);
 
@@ -76,10 +76,7 @@ static inline void e820_mark_nosave_regi
 }
 #endif
 
-
-#endif/*!__ASSEMBLY__*/
 #else /* X86_32 */
-#ifndef __ASSEMBLY__
 extern unsigned long find_e820_area(unsigned long start, unsigned long end,
 				    unsigned long size, unsigned long align);
 extern unsigned long find_e820_area_size(unsigned long start,
@@ -117,9 +114,8 @@ extern void reserve_early(unsigned long 
 extern void free_early(unsigned long start, unsigned long end);
 extern void early_res_to_bootmem(unsigned long start, unsigned long end);
 
+#endif /* X86_32 */
 #endif/*!__ASSEMBLY__*/
-
-#endif
 #endif /* __KERNEL__ */
 
 #endif  /* __ASM_E820_H */

-- 

[patch 4/4] x86: e820 unification: Extract common functions

[Christoph Lameter]

[-- Attachment #1: e820_2 --]
[-- Type: text/plain, Size: 4417 bytes --]

Extract the common functions to the common area. The parameters of
e820_any_mapped vary between 32 and 64 bit. Convert them to unsigned long
for 32 bit so that they match.

Signed-off-by: Christoph Lameter <clameter@sgi.com>

---
 arch/x86/kernel/e820_32.c |    2 +-
 include/asm-x86/e820.h    |   43 +++++++++++++++----------------------------
 2 files changed, 16 insertions(+), 29 deletions(-)

Index: linux-2.6/include/asm-x86/e820.h
===================================================================
--- linux-2.6.orig/include/asm-x86/e820.h	2008-05-05 11:59:56.000000000 -0700
+++ linux-2.6/include/asm-x86/e820.h	2008-05-05 12:00:58.000000000 -0700
@@ -44,29 +44,17 @@ struct e820map {
 
 #ifndef __ASSEMBLY__
 
-#ifdef CONFIG_X86_32
-#define HIGH_MEMORY	(1024*1024)
-
-extern void setup_memory_map(void);
 extern void finish_e820_parsing(void);
-
 extern struct e820map e820;
 extern void update_e820(void);
-
 extern int e820_all_mapped(unsigned long start, unsigned long end,
 			   unsigned type);
-extern int e820_any_mapped(u64 start, u64 end, unsigned type);
-extern void propagate_e820_map(void);
-extern void register_bootmem_low_pages(unsigned long max_low_pfn);
+extern int e820_any_mapped(unsigned long start, unsigned long end,
+			unsigned type);
 extern void add_memory_region(unsigned long long start,
 			      unsigned long long size, int type);
 extern u64 update_memory_range(u64 start, u64 size, unsigned old_type,
 				unsigned new_type);
-extern void e820_register_memory(void);
-extern void limit_regions(unsigned long long size);
-extern void init_iomem_resources(struct resource *code_resource,
-				 struct resource *data_resource,
-				 struct resource *bss_resource);
 
 #if defined(CONFIG_PM) && defined(CONFIG_HIBERNATION)
 extern void e820_mark_nosave_regions(void);
@@ -76,25 +64,29 @@ static inline void e820_mark_nosave_regi
 }
 #endif
 
+#ifdef CONFIG_X86_32
+#define HIGH_MEMORY	(1024*1024)
+
+extern void setup_memory_map(void);
+
+extern void propagate_e820_map(void);
+extern void register_bootmem_low_pages(unsigned long max_low_pfn);
+extern void e820_register_memory(void);
+extern void limit_regions(unsigned long long size);
+extern void init_iomem_resources(struct resource *code_resource,
+				 struct resource *data_resource,
+				 struct resource *bss_resource);
+
 #else /* X86_32 */
 extern unsigned long find_e820_area(unsigned long start, unsigned long end,
 				    unsigned long size, unsigned long align);
 extern unsigned long find_e820_area_size(unsigned long start,
 					 unsigned long *sizep,
 					 unsigned long align);
-extern void add_memory_region(unsigned long start, unsigned long size,
-			      int type);
-extern u64 update_memory_range(u64 start, u64 size, unsigned old_type,
-				unsigned new_type);
 extern void setup_memory_region(void);
 extern void contig_e820_setup(void);
 extern unsigned long e820_end_of_ram(void);
 extern void e820_reserve_resources(void);
-extern void e820_mark_nosave_regions(void);
-extern int e820_any_mapped(unsigned long start, unsigned long end,
-			   unsigned type);
-extern int e820_all_mapped(unsigned long start, unsigned long end,
-			   unsigned type);
 extern int e820_any_non_reserved(unsigned long start, unsigned long end);
 extern int is_memory_any_valid(unsigned long start, unsigned long end);
 extern int e820_all_non_reserved(unsigned long start, unsigned long end);
@@ -105,11 +97,6 @@ extern void e820_setup_gap(void);
 extern void e820_register_active_regions(int nid, unsigned long start_pfn,
 					 unsigned long end_pfn);
 
-extern void finish_e820_parsing(void);
-
-extern struct e820map e820;
-extern void update_e820(void);
-
 extern void reserve_early(unsigned long start, unsigned long end, char *name);
 extern void free_early(unsigned long start, unsigned long end);
 extern void early_res_to_bootmem(unsigned long start, unsigned long end);
Index: linux-2.6/arch/x86/kernel/e820_32.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/e820_32.c	2008-04-23 12:07:32.000000000 -0700
+++ linux-2.6/arch/x86/kernel/e820_32.c	2008-05-05 12:00:01.000000000 -0700
@@ -645,7 +645,7 @@ void __init limit_regions(unsigned long 
  * with type.
  */
 int
-e820_any_mapped(u64 start, u64 end, unsigned type)
+e820_any_mapped(unsigned long start,unsigned long end, unsigned type)
 {
 	int i;
 	for (i = 0; i < e820.nr_map; i++) {

-- 

Re: [patch 4/4] x86: e820 unification: Extract common functions

[Adrian Bunk]

On Mon, May 05, 2008 at 12:02:17PM -0700, Christoph Lameter wrote:
> Extract the common functions to the common area. The parameters of
> e820_any_mapped vary between 32 and 64 bit. Convert them to unsigned long
> for 32 bit so that they match.
>...
> --- linux-2.6.orig/arch/x86/kernel/e820_32.c	2008-04-23 12:07:32.000000000 -0700
> +++ linux-2.6/arch/x86/kernel/e820_32.c	2008-05-05 12:00:01.000000000 -0700
> @@ -645,7 +645,7 @@ void __init limit_regions(unsigned long 
>   * with type.
>   */
>  int
> -e820_any_mapped(u64 start, u64 end, unsigned type)
> +e820_any_mapped(unsigned long start,unsigned long end, unsigned type)
>  {
>  	int i;
>  	for (i = 0; i < e820.nr_map; i++) {

This changes the parameters from 64bit to 32bit which looks fishy.

Are you sure the solution that would both match and be correct isn't to 
change the 64bit one to u64?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [patch 4/4] x86: e820 unification: Extract common functions

[Ingo Molnar]

* Adrian Bunk <bunk@kernel.org> wrote:

> >  int
> > -e820_any_mapped(u64 start, u64 end, unsigned type)
> > +e820_any_mapped(unsigned long start,unsigned long end, unsigned type)
> >  {
> >  	int i;
> >  	for (i = 0; i < e820.nr_map; i++) {
> 
> This changes the parameters from 64bit to 32bit which looks fishy.

good catch! This could lead to bugs on 32-bit that has physical RAM 
addresses over 4GB.

> Are you sure the solution that would both match and be correct isn't 
> to change the 64bit one to u64?

agreed.

	Ingo

[patch 4/4] x86: e820 unification: Extract common functions

[Christoph Lameter]

[-- Attachment #1: e820_2 --]
[-- Type: text/plain, Size: 5429 bytes --]

Extract the common functions to the common area.

The parameters of e820_any_mapped vary between 32 and 64 bit.
Convert them to u64 so that they match.

Same issue is there fore add_memory_region. 32 bit uses unsigned long long
there. Convert both platforms to use u64.

Signed-off-by: Christoph Lameter <clameter@sgi.com>

---
 arch/x86/kernel/e820_32.c |    3 --
 arch/x86/kernel/e820_64.c |    4 +--
 include/asm-x86/e820.h    |   47 +++++++++++++++-------------------------------
 3 files changed, 19 insertions(+), 35 deletions(-)

Index: linux-2.6/include/asm-x86/e820.h
===================================================================
--- linux-2.6.orig/include/asm-x86/e820.h	2008-05-06 12:29:06.000000000 -0700
+++ linux-2.6/include/asm-x86/e820.h	2008-05-06 12:29:06.000000000 -0700
@@ -44,30 +44,15 @@ struct e820map {
 
 #ifndef __ASSEMBLY__
 
-#ifdef CONFIG_X86_32
-#define HIGH_MEMORY	(1024*1024)
-
-extern void setup_memory_map(void);
 extern void finish_e820_parsing(void);
-
 extern struct e820map e820;
 extern void update_e820(void);
-
 extern int e820_all_mapped(unsigned long start, unsigned long end,
 			   unsigned type);
 extern int e820_any_mapped(u64 start, u64 end, unsigned type);
-extern void propagate_e820_map(void);
-extern void register_bootmem_low_pages(unsigned long max_low_pfn);
-extern void add_memory_region(unsigned long long start,
-			      unsigned long long size, int type);
-extern u64 update_memory_range(u64 start, u64 size, unsigned old_type,
+extern void add_memory_region(u64 start, u64 size, int type);
+extern void update_memory_range(u64 start, u64 size, unsigned old_type,
 				unsigned new_type);
-extern void e820_register_memory(void);
-extern void limit_regions(unsigned long long size);
-extern void print_memory_map(char *who);
-extern void init_iomem_resources(struct resource *code_resource,
-				 struct resource *data_resource,
-				 struct resource *bss_resource);
 
 #if defined(CONFIG_PM) && defined(CONFIG_HIBERNATION)
 extern void e820_mark_nosave_regions(void);
@@ -77,25 +62,30 @@ static inline void e820_mark_nosave_regi
 }
 #endif
 
+#ifdef CONFIG_X86_32
+#define HIGH_MEMORY	(1024*1024)
+
+extern void setup_memory_map(void);
+
+extern void propagate_e820_map(void);
+extern void register_bootmem_low_pages(unsigned long max_low_pfn);
+extern void e820_register_memory(void);
+extern void limit_regions(unsigned long long size);
+extern void print_memory_map(char *who);
+extern void init_iomem_resources(struct resource *code_resource,
+				 struct resource *data_resource,
+				 struct resource *bss_resource);
+
 #else /* X86_32 */
 extern unsigned long find_e820_area(unsigned long start, unsigned long end,
 				    unsigned long size, unsigned long align);
 extern unsigned long find_e820_area_size(unsigned long start,
 					 unsigned long *sizep,
 					 unsigned long align);
-extern void add_memory_region(unsigned long start, unsigned long size,
-			      int type);
-extern u64 update_memory_range(u64 start, u64 size, unsigned old_type,
-				unsigned new_type);
 extern void setup_memory_region(void);
 extern void contig_e820_setup(void);
 extern unsigned long e820_end_of_ram(void);
 extern void e820_reserve_resources(void);
-extern void e820_mark_nosave_regions(void);
-extern int e820_any_mapped(unsigned long start, unsigned long end,
-			   unsigned type);
-extern int e820_all_mapped(unsigned long start, unsigned long end,
-			   unsigned type);
 extern int e820_any_non_reserved(unsigned long start, unsigned long end);
 extern int is_memory_any_valid(unsigned long start, unsigned long end);
 extern int e820_all_non_reserved(unsigned long start, unsigned long end);
@@ -106,11 +96,6 @@ extern void e820_setup_gap(void);
 extern void e820_register_active_regions(int nid, unsigned long start_pfn,
 					 unsigned long end_pfn);
 
-extern void finish_e820_parsing(void);
-
-extern struct e820map e820;
-extern void update_e820(void);
-
 extern void reserve_early(unsigned long start, unsigned long end, char *name);
 extern void free_early(unsigned long start, unsigned long end);
 extern void early_res_to_bootmem(unsigned long start, unsigned long end);
Index: linux-2.6/arch/x86/kernel/e820_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/e820_64.c	2008-05-06 12:29:02.000000000 -0700
+++ linux-2.6/arch/x86/kernel/e820_64.c	2008-05-06 12:29:06.000000000 -0700
@@ -181,7 +181,7 @@ again:
  * with type.
  */
 int
-e820_any_mapped(unsigned long start, unsigned long end, unsigned type)
+e820_any_mapped(u64 start, u64 end, unsigned type)
 {
 	int i;
 
@@ -437,7 +437,7 @@ e820_register_active_regions(int nid, un
 /*
  * Add a memory region to the kernel e820 map.
  */
-void __init add_memory_region(unsigned long start, unsigned long size, int type)
+void __init add_memory_region(u64 start, u64 size, int type)
 {
 	int x = e820.nr_map;
 
Index: linux-2.6/arch/x86/kernel/e820_32.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/e820_32.c	2008-05-06 12:29:04.000000000 -0700
+++ linux-2.6/arch/x86/kernel/e820_32.c	2008-05-06 12:29:08.000000000 -0700
@@ -255,8 +255,7 @@ void __init e820_mark_nosave_regions(voi
 }
 #endif
 
-void __init add_memory_region(unsigned long long start,
-			      unsigned long long size, int type)
+void __init add_memory_region(u64 start, u64 size, int type)
 {
 	int x;
 

--