[PATCH v3 0/2] fixes for mem= option

[PATCH v3 0/2] fixes for mem= option

[Wen Congyang]

The documentation and implementation of 'mem=' option doesn't match, and the
option can't work for efi platform. This patchset updates the documentation
and make the option to work for efi platform.

Changes from v2 to v3:
    Patch1: X86-32 ===> X86

Changes from v1 to v2:
    Patch1: Just fix a typo error(ingoring -> ignoring).

Wen Congyang (2):
  update mem= option's spec according to its implementation
  x86: make 'mem=' option to work for efi platform

 Documentation/kernel-parameters.txt |  7 ++++---
 arch/x86/kernel/e820.c              | 29 +++++++++++++++++++++++++----
 2 files changed, 29 insertions(+), 7 deletions(-)

-- 
1.8.0

[PATCH v3 1/2] update mem= option's spec according to its implementation

[Wen Congyang]

Current mem= implementation seems buggy because specification and
implementation doesn't match. Current mem= has been working
for many years and it's not buggy, it works as expected. So
we should update the specification.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Sort-of-tentatively-acked-by: Rob Landley <rob@landley.net>
---
 Documentation/kernel-parameters.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 9776f06..91863a5 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1481,9 +1481,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	mem=nn[KMG]	[KNL,BOOT] Force usage of a specific amount of memory
 			Amount of memory to be used when the kernel is not able
 			to see the whole system memory or for test.
-			[X86-32] Use together with memmap= to avoid physical
-			address space collisions. Without memmap= PCI devices
-			could be placed at addresses belonging to unused RAM.
+			[X86] Work as limiting max address. Use together
+			with memmap= to avoid physical address space collisions.
+			Without memmap= PCI devices could be placed at addresses
+			belonging to unused RAM.
 
 	mem=nopentium	[BUGS=X86-32] Disable usage of 4MB pages for kernel
 			memory.
-- 
1.8.0

[PATCH v3 2/2] x86: make 'mem=' option to work for efi platform

[Wen Congyang]

Current mem boot option only can work for non efi environment. If the user
specifies add_efi_memmap, it cannot work for efi environment. In
the efi environment, we call e820_add_region() to add the memory map. So
we can modify __e820_add_region() and the mem boot option can work for
efi environment.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 arch/x86/kernel/e820.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index df06ade..b15b341 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -47,6 +47,7 @@ unsigned long pci_mem_start = 0xaeedbabe;
 #ifdef CONFIG_PCI
 EXPORT_SYMBOL(pci_mem_start);
 #endif
+static u64 mem_limit = ~0ULL;
 
 /*
  * This function checks if any part of the range <start,end> is mapped
@@ -119,6 +120,20 @@ static void __init __e820_add_region(struct e820map *e820x, u64 start, u64 size,
 		return;
 	}
 
+	if (start >= mem_limit) {
+		printk(KERN_ERR "e820: ignoring [mem %#010llx-%#010llx]\n",
+		       (unsigned long long)start,
+		       (unsigned long long)(start + size - 1));
+		return;
+	}
+
+	if (mem_limit - start < size) {
+		printk(KERN_ERR "e820: ignoring [mem %#010llx-%#010llx]\n",
+		       (unsigned long long)mem_limit,
+		       (unsigned long long)(start + size - 1));
+		size = mem_limit - start;
+	}
+
 	e820x->map[x].addr = start;
 	e820x->map[x].size = size;
 	e820x->map[x].type = type;
@@ -809,7 +824,7 @@ static int userdef __initdata;
 /* "mem=nopentium" disables the 4MB page tables. */
 static int __init parse_memopt(char *p)
 {
-	u64 mem_size;
+	char *oldp;
 
 	if (!p)
 		return -EINVAL;
@@ -825,11 +840,11 @@ static int __init parse_memopt(char *p)
 	}
 
 	userdef = 1;
-	mem_size = memparse(p, &p);
+	oldp = p;
+	mem_limit = memparse(p, &p);
 	/* don't remove all of memory when handling "mem={invalid}" param */
-	if (mem_size == 0)
+	if (mem_limit == 0 || p == oldp)
 		return -EINVAL;
-	e820_remove_range(mem_size, ULLONG_MAX - mem_size, E820_RAM, 1);
 
 	return 0;
 }
@@ -881,6 +896,12 @@ early_param("memmap", parse_memmap_opt);
 
 void __init finish_e820_parsing(void)
 {
+	if (mem_limit != ~0ULL) {
+		userdef = 1;
+		e820_remove_range(mem_limit, ULLONG_MAX - mem_limit,
+				  E820_RAM, 1);
+	}
+
 	if (userdef) {
 		u32 nr = e820.nr_map;
 
-- 
1.8.0

Re: [PATCH v3 1/2] update mem= option's spec according to its implementation

[richard -rw- weinberger]

On Mon, Oct 29, 2012 at 9:48 AM, Wen Congyang <wency@cn.fujitsu.com> wrote:
> Current mem= implementation seems buggy because specification and
> implementation doesn't match. Current mem= has been working
> for many years and it's not buggy, it works as expected. So
> we should update the specification.
>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Sort-of-tentatively-acked-by: Rob Landley <rob@landley.net>

So, is this an ACK or not?

-- 
Thanks,
//richard

Re: [PATCH v3 1/2] update mem= option's spec according to its implementation

[Wen Congyang]

At 10/29/2012 06:48 PM, richard -rw- weinberger Wrote:
> On Mon, Oct 29, 2012 at 9:48 AM, Wen Congyang <wency@cn.fujitsu.com> wrote:
>> Current mem= implementation seems buggy because specification and
>> implementation doesn't match. Current mem= has been working
>> for many years and it's not buggy, it works as expected. So
>> we should update the specification.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> Sort-of-tentatively-acked-by: Rob Landley <rob@landley.net>
> 
> So, is this an ACK or not?
> 

I don't know.

Here is the origin message:

At 06/15/2012 04:22 AM, Rob Landley Wrote:
> I have no objection to this but can't confirm it's true or not without
> an awful lot more digging through the code I don't have time for right
> now. (All the x86-32 machines I've used just had the 640k->1m hole and
> the rest was contiguous memory, so the behavior would be the same either
> way...)
> 
> Sort-of-tentatively-acked-by: Rob Landley <rob@landley.net>
> 
> Rob