[patch 0/6] x86: Unification patches etc

[patch 0/6] x86: Unification patches etc

[Christoph Lameter]

Some patches that have so far not been merged. Various fixes
after running this on a variety of configurations. Builds on
8 different 32 bit / 64 bit configs.

-- 

[patch 1/6] x86: Unify current.h

[Christoph Lameter]

[-- Attachment #1: x86_unify_current_h --]
[-- Type: text/plain, Size: 2995 bytes --]

Simply stitch these together. There are just two definitions that are shared
but the file is resonably small and putting these things together shows that
further unifications requires a unification of the per cpu / pda handling
between both arches.

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

---
 include/asm-x86/current.h    |   42 ++++++++++++++++++++++++++++++++++++++----
 include/asm-x86/current_32.h |   17 -----------------
 include/asm-x86/current_64.h |   27 ---------------------------
 3 files changed, 38 insertions(+), 48 deletions(-)

Index: linux-2.6/include/asm-x86/current.h
===================================================================
--- linux-2.6.orig/include/asm-x86/current.h	2008-05-07 16:33:51.126164691 -0700
+++ linux-2.6/include/asm-x86/current.h	2008-05-09 18:28:05.666268549 -0700
@@ -1,5 +1,39 @@
+#ifndef _X86_CURRENT_H
+#define _X86_CURRENT_H
+
 #ifdef CONFIG_X86_32
-# include "current_32.h"
-#else
-# include "current_64.h"
-#endif
+#include <linux/compiler.h>
+#include <asm/percpu.h>
+
+struct task_struct;
+
+DECLARE_PER_CPU(struct task_struct *, current_task);
+static __always_inline struct task_struct *get_current(void)
+{
+	return x86_read_percpu(current_task);
+}
+
+#else /* X86_32 */
+
+#ifndef __ASSEMBLY__
+#include <asm/pda.h>
+
+struct task_struct;
+
+static __always_inline struct task_struct *get_current(void)
+{
+	return read_pda(pcurrent);
+}
+
+#else /* __ASSEMBLY__ */
+
+#include <asm/asm-offsets.h>
+#define GET_CURRENT(reg) movq %gs:(pda_pcurrent),reg
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* X86_32 */
+
+#define current get_current()
+
+#endif /* X86_CURRENT_H */
Index: linux-2.6/include/asm-x86/current_32.h
===================================================================
--- linux-2.6.orig/include/asm-x86/current_32.h	2008-05-07 16:33:51.130164801 -0700
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,17 +0,0 @@
-#ifndef _I386_CURRENT_H
-#define _I386_CURRENT_H
-
-#include <linux/compiler.h>
-#include <asm/percpu.h>
-
-struct task_struct;
-
-DECLARE_PER_CPU(struct task_struct *, current_task);
-static __always_inline struct task_struct *get_current(void)
-{
-	return x86_read_percpu(current_task);
-}
-
-#define current get_current()
-
-#endif /* !(_I386_CURRENT_H) */
Index: linux-2.6/include/asm-x86/current_64.h
===================================================================
--- linux-2.6.orig/include/asm-x86/current_64.h	2008-05-07 16:33:51.138164773 -0700
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,27 +0,0 @@
-#ifndef _X86_64_CURRENT_H
-#define _X86_64_CURRENT_H
-
-#if !defined(__ASSEMBLY__)
-struct task_struct;
-
-#include <asm/pda.h>
-
-static inline struct task_struct *get_current(void)
-{
-	struct task_struct *t = read_pda(pcurrent);
-	return t;
-}
-
-#define current get_current()
-
-#else
-
-#ifndef ASM_OFFSET_H
-#include <asm/asm-offsets.h>
-#endif
-
-#define GET_CURRENT(reg) movq %gs:(pda_pcurrent),reg
-
-#endif
-
-#endif /* !(_X86_64_CURRENT_H) */

-- 

Re: [patch 1/6] x86: Unify current.h

[Ingo Molnar]

* Christoph Lameter <clameter@sgi.com> wrote:

> Simply stitch these together. There are just two definitions that are 
> shared but the file is resonably small and putting these things 
> together shows that further unifications requires a unification of the 
> per cpu / pda handling between both arches.

applied, thanks.

	Ingo

[patch 2/6] x86: e820.h unification

[Christoph Lameter]

[-- Attachment #1: x86_unify_e820_h_merge --]
[-- Type: text/plain, Size: 8926 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    |  103 +++++++++++++++++++++++++++++++++++++++++++++-
 include/asm-x86/e820_32.h |   50 ----------------------
 include/asm-x86/e820_64.h |   56 -------------------------
 3 files changed, 101 insertions(+), 108 deletions(-)

Index: linux-2.6/include/asm-x86/e820.h
===================================================================
--- linux-2.6.orig/include/asm-x86/e820.h	2008-05-06 11:44:15.000000000 -0700
+++ linux-2.6/include/asm-x86/e820.h	2008-05-06 11:46:45.000000000 -0700
@@ -30,9 +30,108 @@ 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 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);
 #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__ */
 
Index: linux-2.6/include/asm-x86/e820_32.h
===================================================================
--- linux-2.6.orig/include/asm-x86/e820_32.h	2008-05-06 11:44:15.000000000 -0700
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,50 +0,0 @@
-/*
- * 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.
- *
- */
-#ifndef __E820_HEADER
-#define __E820_HEADER
-
-#include <linux/ioport.h>
-
-#define HIGH_MEMORY	(1024*1024)
-
-#ifndef __ASSEMBLY__
-
-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 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);
-#else
-static inline void e820_mark_nosave_regions(void)
-{
-}
-#endif
-
-
-#endif/*!__ASSEMBLY__*/
-#endif/*__E820_HEADER*/
Index: linux-2.6/include/asm-x86/e820_64.h
===================================================================
--- linux-2.6.orig/include/asm-x86/e820_64.h	2008-05-06 11:44:15.000000000 -0700
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,56 +0,0 @@
-/*
- * 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.
- */
-#ifndef __E820_HEADER
-#define __E820_HEADER
-
-#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 void 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/*__E820_HEADER*/

-- 

[patch 3/6] x86: e820 unification: Extract shared comments

[Christoph Lameter]

[-- Attachment #1: x86_unify_e820_h_comments --]
[-- Type: text/plain, Size: 2101 bytes --]

The introductory comments are basically the same. Take the 64 bit version.

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

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

Index: linux-2.6/include/asm-x86/e820.h
===================================================================
--- linux-2.6.orig/include/asm-x86/e820.h	2008-05-06 11:46:45.000000000 -0700
+++ linux-2.6/include/asm-x86/e820.h	2008-05-06 11:48:09.000000000 -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__
@@ -80,18 +80,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 4/6] x86: e820 unification: Common #ifdef __ASSEMBLY

[Christoph Lameter]

[-- Attachment #1: x86_unify_e820_h_ifdef --]
[-- 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-06 11:48:09.000000000 -0700
+++ linux-2.6/include/asm-x86/e820.h	2008-05-06 11:50:53.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);
 
@@ -77,10 +77,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,
@@ -118,9 +115,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 5/6] x86: e820 unification: Extract common functions

[Christoph Lameter]

[-- Attachment #1: x86_unify_e820_h_common_functions --]
[-- Type: text/plain, Size: 5581 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 for 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    |   50 ++++++++++++++++------------------------------
 3 files changed, 21 insertions(+), 36 deletions(-)

Index: linux-2.6/include/asm-x86/e820.h
===================================================================
--- linux-2.6.orig/include/asm-x86/e820.h	2008-05-09 16:05:13.000000000 -0700
+++ linux-2.6/include/asm-x86/e820.h	2008-05-09 16:05:23.000000000 -0700
@@ -44,32 +44,18 @@ 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)
+#if defined(CONFIG_X86_64) ||						\
+		(defined(CONFIG_PM) && defined(CONFIG_HIBERNATION))
 extern void e820_mark_nosave_regions(void);
 #else
 static inline void e820_mark_nosave_regions(void)
@@ -77,25 +63,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 +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_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/e820_64.c	2008-05-09 16:05:13.000000000 -0700
+++ linux-2.6/arch/x86/kernel/e820_64.c	2008-05-09 16:05:19.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-09 16:05:13.000000000 -0700
+++ linux-2.6/arch/x86/kernel/e820_32.c	2008-05-09 16:05:19.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;
 

-- 

[patch 6/6] x86: Add checks for virtual addresses in __phys_addr()

[Christoph Lameter]

[-- Attachment #1: vcp_check_virtual_addresses --]
[-- Type: text/plain, Size: 1831 bytes --]

Add checks to insure that virtual addresses are not used in invalid contexts.

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

---
 arch/x86/mm/ioremap.c     |   15 +++++++--------
 include/asm-x86/page_32.h |    7 ++++++-
 2 files changed, 13 insertions(+), 9 deletions(-)

Index: linux-2.6/arch/x86/mm/ioremap.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/ioremap.c	2008-05-07 16:27:09.792659191 -0700
+++ linux-2.6/arch/x86/mm/ioremap.c	2008-05-07 19:46:34.733509101 -0700
@@ -21,29 +21,28 @@
 #include <asm/pgalloc.h>
 #include <asm/pat.h>
 
-#ifdef CONFIG_X86_64
 
+#if defined(CONFIG_DEBUG_VM) || defined(CONFIG_X86_64)
 unsigned long __phys_addr(unsigned long x)
 {
+	VM_BUG_ON(is_vmalloc_addr((void *)x));
+#ifdef CONFIG_X86_64
 	if (x >= __START_KERNEL_map)
 		return x - __START_KERNEL_map + phys_base;
+#endif
 	return x - PAGE_OFFSET;
 }
 EXPORT_SYMBOL(__phys_addr);
+#endif
 
 static inline int phys_addr_valid(unsigned long addr)
 {
+#ifdef CONFIG_X86_64
 	return addr < (1UL << boot_cpu_data.x86_phys_bits);
-}
-
 #else
-
-static inline int phys_addr_valid(unsigned long addr)
-{
 	return 1;
-}
-
 #endif
+}
 
 int page_is_ram(unsigned long pagenr)
 {
Index: linux-2.6/include/asm-x86/page_32.h
===================================================================
--- linux-2.6.orig/include/asm-x86/page_32.h	2008-05-07 16:27:12.191419001 -0700
+++ linux-2.6/include/asm-x86/page_32.h	2008-05-07 19:30:46.444980299 -0700
@@ -64,7 +64,12 @@ typedef struct page *pgtable_t;
 #endif
 
 #ifndef __ASSEMBLY__
-#define __phys_addr(x)		((x) - PAGE_OFFSET)
+#ifdef CONFIG_DEBUG_VM
+unsigned long __phys_addr(unsigned long x)
+#else
+#define __phys_addr(x)	((x) - PAGE_OFFSET)
+#endif
+
 #define __phys_reloc_hide(x)	RELOC_HIDE((x), 0)
 
 #ifdef CONFIG_FLATMEM

-- 

Re: [patch 6/6] x86: Add checks for virtual addresses in __phys_addr()

[Sam Ravnborg]

On Fri, May 09, 2008 at 07:09:53PM -0700, Christoph Lameter wrote:
> Add checks to insure that virtual addresses are not used in invalid contexts.
> 
> Signed-off-by: Christoph Lameter <clameter@sgi.com>
> 
> ---
>  arch/x86/mm/ioremap.c     |   15 +++++++--------
>  include/asm-x86/page_32.h |    7 ++++++-
>  2 files changed, 13 insertions(+), 9 deletions(-)
> 
> Index: linux-2.6/arch/x86/mm/ioremap.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/mm/ioremap.c	2008-05-07 16:27:09.792659191 -0700
> +++ linux-2.6/arch/x86/mm/ioremap.c	2008-05-07 19:46:34.733509101 -0700
> @@ -21,29 +21,28 @@
>  #include <asm/pgalloc.h>
>  #include <asm/pat.h>
>  
> -#ifdef CONFIG_X86_64
>  
> +#if defined(CONFIG_DEBUG_VM) || defined(CONFIG_X86_64)
>  unsigned long __phys_addr(unsigned long x)
>  {
> +	VM_BUG_ON(is_vmalloc_addr((void *)x));
> +#ifdef CONFIG_X86_64
>  	if (x >= __START_KERNEL_map)
>  		return x - __START_KERNEL_map + phys_base;
> +#endif
>  	return x - PAGE_OFFSET;
>  }
>  EXPORT_SYMBOL(__phys_addr);
> +#endif
>  
>  static inline int phys_addr_valid(unsigned long addr)
>  {
> +#ifdef CONFIG_X86_64
>  	return addr < (1UL << boot_cpu_data.x86_phys_bits);
> -}
> -
>  #else
> -
> -static inline int phys_addr_valid(unsigned long addr)
> -{
>  	return 1;
> -}
> -
>  #endif
> +}
>  
>  int page_is_ram(unsigned long pagenr)
>  {
> Index: linux-2.6/include/asm-x86/page_32.h
> ===================================================================
> --- linux-2.6.orig/include/asm-x86/page_32.h	2008-05-07 16:27:12.191419001 -0700
> +++ linux-2.6/include/asm-x86/page_32.h	2008-05-07 19:30:46.444980299 -0700
> @@ -64,7 +64,12 @@ typedef struct page *pgtable_t;
>  #endif
>  
>  #ifndef __ASSEMBLY__
> -#define __phys_addr(x)		((x) - PAGE_OFFSET)
> +#ifdef CONFIG_DEBUG_VM
> +unsigned long __phys_addr(unsigned long x)

Should this have read:
> +extern unsigned long __phys_addr(unsigned long x);
?

Looks like a prototype with a missing ';'.
And x86 uses extern for prototypes in .h files (most of the time).

	Sam

Re: [patch 6/6] x86: Add checks for virtual addresses in __phys_addr()

[Christoph Lameter]

On Sat, 10 May 2008, Sam Ravnborg wrote:

 > Should this have read:
> > +extern unsigned long __phys_addr(unsigned long x);
> ?
> 
> Looks like a prototype with a missing ';'.
> And x86 uses extern for prototypes in .h files (most of the time).
> 

Right. And this means that I did not test a 32 bit config with 
CONFIG_VM_DEBUG on this one. The run of the mill stuff did not do it. 
Sigh.

This patch fixes the compile:

---
 include/asm-x86/page_32.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/include/asm-x86/page_32.h
===================================================================
--- linux-2.6.orig/include/asm-x86/page_32.h	2008-05-09 19:32:47.000000000 -0700
+++ linux-2.6/include/asm-x86/page_32.h	2008-05-09 19:32:55.000000000 -0700
@@ -65,7 +65,7 @@ typedef struct page *pgtable_t;
 
 #ifndef __ASSEMBLY__
 #ifdef CONFIG_DEBUG_VM
-unsigned long __phys_addr(unsigned long x)
+extern unsigned long __phys_addr(unsigned long x);
 #else
 #define __phys_addr(x)	((x) - PAGE_OFFSET)
 #endif

Re: [patch 6/6] x86: Add checks for virtual addresses in __phys_addr()

[Christoph Lameter]

We also need to address this issue with __pa expected to be a constant in 
32 bit code. Sigh.


---
 arch/x86/kernel/doublefault_32.c |    2 +-
 include/asm-x86/page_32.h        |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6/include/asm-x86/page_32.h
===================================================================
--- linux-2.6.orig/include/asm-x86/page_32.h	2008-05-09 19:32:47.000000000 -0700
+++ linux-2.6/include/asm-x86/page_32.h	2008-05-09 19:32:55.000000000 -0700
@@ -65,7 +65,7 @@ typedef struct page *pgtable_t;
 
 #ifndef __ASSEMBLY__
 #ifdef CONFIG_DEBUG_VM
-unsigned long __phys_addr(unsigned long x)
+extern unsigned long __phys_addr(unsigned long x);
 #else
 #define __phys_addr(x)	((x) - PAGE_OFFSET)
 #endif
Index: linux-2.6/arch/x86/kernel/doublefault_32.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/doublefault_32.c	2008-05-09 19:40:08.000000000 -0700
+++ linux-2.6/arch/x86/kernel/doublefault_32.c	2008-05-09 19:40:45.000000000 -0700
@@ -66,6 +66,6 @@ struct tss_struct doublefault_tss __cach
 		.ds		= __USER_DS,
 		.fs		= __KERNEL_PERCPU,
 
-		.__cr3		= __pa(swapper_pg_dir)
+		.__cr3		= (unsigned long)swapper_pg_dir - PAGE_OFFSET
 	}
 };

Re: [patch 6/6] x86: Add checks for virtual addresses in __phys_addr()

[Ingo Molnar]

* Christoph Lameter <clameter@sgi.com> wrote:

> We also need to address this issue with __pa expected to be a constant 
> in 32 bit code. Sigh.

hm, even with all the fixlets it breaks with:

  http://redhat.com/~mingo/misc/config-Tue_May_13_14_17_44_CEST_2008.bad

arch/x86/kernel/e820_32.c:586: error: static declaration of 'print_memory_map' follows non-static declaration
include/asm/e820.h:75: error: previous declaration of 'print_memory_map' was here

	Ingo