[PATCH 0/2] Only parse exactmap once, introduce memmap=resetusablemap

[PATCH 0/2] Only parse exactmap once, introduce memmap=resetusablemap

[Thomas Renninger]

this is a clean resend of Yinghai's memmap= extensions.
They were discussed on lkml in a thread with subject:
[PATCH] x86 e820: only void usable memory areas in memmap=exactmap case

The patches are based on origin/x86/mm2 where another memmap= 
enhancement from Yinghai is already queued.

Would be great if these can be queued there as well.

Thanks,

   Thomas

[PATCH 1/2] x86 e820: Check for exactmap appearance when parsing first memmap option

[Thomas Renninger]

From: Yinghai Lu <yinghai@kernel.org>

memmap=exactmap will throw away all original, but also until then
user defined (through other provided memmap= parameters) areas.
That means all memmap= boot parameters passed before a memmap=exactmap
parameter are not recognized.
Without this fix:
memmap=x@y memmap=exactmap memmap=i#k
only i#k would get recognized.

This is wrong, this fix will only throw away all original e820 areas once
when memmap=exactmap is found in the whole boot command line and before
any other memmap= option is parsed.

Reviewed-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <trenn@suse.de>
---
 arch/x86/kernel/e820.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index d32abea..dc0b9f0 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -835,6 +835,8 @@ static int __init parse_memopt(char *p)
 }
 early_param("mem", parse_memopt);
 
+static bool __initdata exactmap_parsed;
+
 static int __init parse_memmap_one(char *p)
 {
 	char *oldp;
@@ -844,6 +846,10 @@ static int __init parse_memmap_one(char *p)
 		return -EINVAL;
 
 	if (!strncmp(p, "exactmap", 8)) {
+		if (exactmap_parsed)
+			return 0;
+
+		exactmap_parsed = true;
 #ifdef CONFIG_CRASH_DUMP
 		/*
 		 * If we are doing a crash dump, we still need to know
@@ -879,6 +885,12 @@ static int __init parse_memmap_one(char *p)
 }
 static int __init parse_memmap_opt(char *str)
 {
+	char *p = boot_command_line;
+
+	p = strstr(p, "exactmap");
+	if (p)
+		parse_memmap_one("exactmap");
+
 	while (str) {
 		char *k = strchr(str, ',');
 
-- 
1.7.6.1

Re: [PATCH 1/2] x86 e820: Check for exactmap appearance when parsing first memmap option

[Yinghai Lu]

On Tue, Jan 22, 2013 at 7:20 AM, Thomas Renninger <trenn@suse.de> wrote:
>
> memmap=exactmap will throw away all original, but also until then
> user defined (through other provided memmap= parameters) areas.
> That means all memmap= boot parameters passed before a memmap=exactmap
> parameter are not recognized.
> Without this fix:
> memmap=x@y memmap=exactmap memmap=i#k
> only i#k would get recognized.
>
> This is wrong, this fix will only throw away all original e820 areas once
> when memmap=exactmap is found in the whole boot command line and before
> any other memmap= option is parsed.

Actually I put this patch already in for-x86-boot branch.

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commit;h=bd40ee79abf2351109ebd469b0b9ba6a8f3b0872

And will update change log to following to merge two change log.

---
Subject: [PATCH] x86: Handle multiple exactmaps and out of order exactmap

Current code expect that we only have one exactmap and exactmap need to
first one in memmap=.

memmap=exactmap will throw away all original entries, but also until
then user defined (through other provided memmap= parameters) areas.
That means all memmap= boot parameters passed before a memmap=exactmap
parameter are not recognized.

Without this fix:
        memmap=x@y memmap=exactmap memmap=i#k
only i#k would get recognized.  This is wrong.

This fix will scan the boot_command_line to find if there is exactmap at
first and only throw away all original e820 entries once, and then parse
other memmap= option.

-v2: incorporate change log from Thomas Renninger.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Reviewed-by: Thomas Renninger <trenn@suse.de>

Re: [PATCH 1/2] x86 e820: Check for exactmap appearance when parsing first memmap option

[H. Peter Anvin]

On 01/22/2013 07:20 AM, Thomas Renninger wrote:
> From: Yinghai Lu <yinghai@kernel.org>
> 
> memmap=exactmap will throw away all original, but also until then
> user defined (through other provided memmap= parameters) areas.
> That means all memmap= boot parameters passed before a memmap=exactmap
> parameter are not recognized.
> Without this fix:
> memmap=x@y memmap=exactmap memmap=i#k
> only i#k would get recognized.
> 
> This is wrong, this fix will only throw away all original e820 areas once
> when memmap=exactmap is found in the whole boot command line and before
> any other memmap= option is parsed.
> 

I don't understand why this is wrong.  The kernel command line is always
parsed from left to right, and I don't see anything inherently
problematic with that with something like a big hammer like exactmap.

	-hpa

Re: [PATCH 1/2] x86 e820: Check for exactmap appearance when parsing first memmap option

[Yinghai Lu]

On Mon, Jan 28, 2013 at 5:09 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 01/22/2013 07:20 AM, Thomas Renninger wrote:
>> From: Yinghai Lu <yinghai@kernel.org>
>>
>> memmap=exactmap will throw away all original, but also until then
>> user defined (through other provided memmap= parameters) areas.
>> That means all memmap= boot parameters passed before a memmap=exactmap
>> parameter are not recognized.
>> Without this fix:
>> memmap=x@y memmap=exactmap memmap=i#k
>> only i#k would get recognized.
>>
>> This is wrong, this fix will only throw away all original e820 areas once
>> when memmap=exactmap is found in the whole boot command line and before
>> any other memmap= option is parsed.
>>
>
> I don't understand why this is wrong.  The kernel command line is always
> parsed from left to right, and I don't see anything inherently
> problematic with that with something like a big hammer like exactmap.

Ok, let's drop it.

[PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[Thomas Renninger]

From: Yinghai Lu <yinghai@kernel.org>

kdump voided the whole original e820 map and half way made
it up via memmap= options passed via kdump boot params again.

But this is conceptionally wrong. The whole original memory ranges
which are declared reserved, ACPI data/nvs or however are not usable
must stay the same and get honored by the kdump kernel.

Therefore memmap=resetusablemap gets introduced.
kdump passes this one and only the usable e820 ranges are removed.
kdump passes the usable ranges to use via memmap=x@y parameter(s).
The not usable e820 ranges are preserved.

This for example fixes mmconf (extended PCI config access) and
possibly other kernel parts which rely on remapped memory to be
in reserved or ACPI (data/nvs) declared e820 memory areas.

Tested-by: Thomas Renninger <trenn@suse.de>
Reviewed-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <trenn@suse.de>
---
 Documentation/kernel-parameters.txt |    9 +++++++++
 arch/x86/kernel/e820.c              |   13 +++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 9776f06..fe2f677 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1498,6 +1498,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			BIOS output or other requirements. See the memmap=nn@ss
 			option description.
 
+	memmap=resetusablemap
+			[KNL,X86] Similar to exactmap, but instead of discarding
+			the whole original e820 map, only the usable memory
+			areas are ignored.
+			Those must be passed via memmap=X@Y parameters,
+			reserved, ACPI and ACPI NVS areas are preserved.
+			This boot parameter is intended for kdump usage.
+			Also passing exactmap overrules resetusablemap.
+
 	memmap=nn[KMG]@ss[KMG]
 			[KNL] Force usage of a specific region of memory
 			Region of memory to be used, from ss to ss+nn.
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index dc0b9f0..4e0ef9d 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -845,7 +845,7 @@ static int __init parse_memmap_one(char *p)
 	if (!p)
 		return -EINVAL;
 
-	if (!strncmp(p, "exactmap", 8)) {
+	if (!strncmp(p, "exactmap", 8) || !strncmp(p, "resetusablemap", 14)) {
 		if (exactmap_parsed)
 			return 0;
 
@@ -858,7 +858,11 @@ static int __init parse_memmap_one(char *p)
 		 */
 		saved_max_pfn = e820_end_of_ram_pfn();
 #endif
-		e820.nr_map = 0;
+		if (!strncmp(p, "resetusablemap", 14)) {
+			/* remove all old E820_RAM ranges */
+			e820_remove_range(0, ULLONG_MAX, E820_RAM, 1);
+		} else
+			e820.nr_map = 0;
 		userdef = 1;
 		return 0;
 	}
@@ -890,6 +894,11 @@ static int __init parse_memmap_opt(char *str)
 	p = strstr(p, "exactmap");
 	if (p)
 		parse_memmap_one("exactmap");
+	else {
+		p = strstr(boot_command_line, "resetusablemap");
+		if (p)
+			parse_memmap_one("resetusablemap");
+	}
 
 	while (str) {
 		char *k = strchr(str, ',');
-- 
1.7.6.1

Re: [PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[H. Peter Anvin]

On 01/22/2013 09:20 AM, Thomas Renninger wrote:
> From: Yinghai Lu <yinghai@kernel.org>
> 
> kdump voided the whole original e820 map and half way made
> it up via memmap= options passed via kdump boot params again.
> 
> But this is conceptionally wrong. The whole original memory ranges
> which are declared reserved, ACPI data/nvs or however are not usable
> must stay the same and get honored by the kdump kernel.
> 
> Therefore memmap=resetusablemap gets introduced.
> kdump passes this one and only the usable e820 ranges are removed.
> kdump passes the usable ranges to use via memmap=x@y parameter(s).
> The not usable e820 ranges are preserved.
> 
> This for example fixes mmconf (extended PCI config access) and
> possibly other kernel parts which rely on remapped memory to be
> in reserved or ACPI (data/nvs) declared e820 memory areas.
> 
> Tested-by: Thomas Renninger <trenn@suse.de>
> Reviewed-by: Thomas Renninger <trenn@suse.de>
> Signed-off-by: Thomas Renninger <trenn@suse.de>

Tested-by: and Reviewed-by: are rather redundant with Signed-off-by:.
Also, you should have a Signed-off-by: from the author (Yinghai).

However, when thinking about it this really doesn't seem to be the right
interface, either.  Something like "memmap=reserveram" which turns all
RAM areas into reserved areas, which can then be overridden by memmap=
options would make more sense.

Even more sense would be to pass the modified memmap to kexec...

	-hpa

Re: [PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[Thomas Renninger]

On Tuesday, January 22, 2013 04:54:08 PM H. Peter Anvin wrote:
> On 01/22/2013 09:20 AM, Thomas Renninger wrote:
> > From: Yinghai Lu <yinghai@kernel.org>
> > 
> > kdump voided the whole original e820 map and half way made
> > it up via memmap= options passed via kdump boot params again.
> > 
> > But this is conceptionally wrong. The whole original memory ranges
> > which are declared reserved, ACPI data/nvs or however are not usable
> > must stay the same and get honored by the kdump kernel.
> > 
> > Therefore memmap=resetusablemap gets introduced.
> > kdump passes this one and only the usable e820 ranges are removed.
> > kdump passes the usable ranges to use via memmap=x@y parameter(s).
> > The not usable e820 ranges are preserved.
> > 
> > This for example fixes mmconf (extended PCI config access) and
> > possibly other kernel parts which rely on remapped memory to be
> > in reserved or ACPI (data/nvs) declared e820 memory areas.
> > 
> > Tested-by: Thomas Renninger <trenn@suse.de>
> > Reviewed-by: Thomas Renninger <trenn@suse.de>
> > Signed-off-by: Thomas Renninger <trenn@suse.de>
> 
> Tested-by: and Reviewed-by: are rather redundant with Signed-off-by:.
Yes, the Signed-off-by slipped in.

> Also, you should have a Signed-off-by: from the author (Yinghai).
From: and Signed-off-by: are also redundant?
 
> However, when thinking about it this really doesn't seem to be the right
> interface, either.
Why?

> Something like "memmap=reserveram" which turns all
> RAM areas into reserved areas, which can then be overridden by memmap=
> options would make more sense.
What for?
This would be wrong. If kexec tries to declare usable memory via memmap=
which is set to reserved by the BIOS, a WARN_ON() or even better BUG_ON()
can/should be issued. This is then not possible anymore.

Your suggestion would again make the e820 data useless. One would again 
not know whether an area which is reserved really is meant for BIOS/HW
communication. And for example broken mmconf platforms for which
the check "is mmconf area is in reserved space" returns false, would now
try to setup mmconf in kdump case. Do you want to introduce another
e820 type: KDUMP_RESERVED which formerly was usable? This would preserve
e820 info, but I cannot see for what this should be good for.

> Even more sense would be to pass the modified memmap to kexec...
Not sure what you mean with this. When kexec is called, you are in a 
kernel which does not have a modified memmap. How can a modified map
get passed to kexec?

Again: Please explain what is bad with this solution.
I cannot see a better and more robust way for kdump other than
reserving the original reserved memory areas as declared by the BIOS.

   Thomas

Re: [PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[H. Peter Anvin]

On 01/22/2013 10:23 AM, Thomas Renninger wrote:
> This would be wrong. If kexec tries to declare usable memory via memmap=
> which is set to reserved by the BIOS, a WARN_ON() or even better BUG_ON()
> can/should be issued. This is then not possible anymore.

You can't do that anyway, because memmap= is not kexec-specific.

> Your suggestion would again make the e820 data useless. One would again 
> not know whether an area which is reserved really is meant for BIOS/HW
> communication. And for example broken mmconf platforms for which
> the check "is mmconf area is in reserved space" returns false, would now
> try to setup mmconf in kdump case. Do you want to introduce another
> e820 type: KDUMP_RESERVED which formerly was usable? This would preserve
> e820 info, but I cannot see for what this should be good for.

You just answered your own question.  Yes, we already have such "kernel
reserved" types, and we could make a "memmap reserved"

>> Even more sense would be to pass the modified memmap to kexec...
> Not sure what you mean with this. When kexec is called, you are in a 
> kernel which does not have a modified memmap. How can a modified map
> get passed to kexec?

The memory map is simply a data structure which is passed by kexec.  It
can be modified at will by the kexec tools.  This is the Right Way to do
any of this; using the command line is just a horribly fscked up hack in
the first place.

> Again: Please explain what is bad with this solution.
> I cannot see a better and more robust way for kdump other than
> reserving the original reserved memory areas as declared by the BIOS.

It is bad because it creates more complexity than is needed.

The whole point is that what we want is simply to switch type 1 to type
X, with the sole exceptions being the areas explicitly reserved for the
kdump kernel.

	-hpa

Re: [PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[Yinghai Lu]

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

On Tue, Jan 22, 2013 at 8:32 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>> Again: Please explain what is bad with this solution.
>> I cannot see a better and more robust way for kdump other than
>> reserving the original reserved memory areas as declared by the BIOS.
>
> It is bad because it creates more complexity than is needed.
>
> The whole point is that what we want is simply to switch type 1 to type
> X, with the sole exceptions being the areas explicitly reserved for the
> kdump kernel.

Do you prefer to  "reserveram" way in attached patch?

Thanks

Yinghai

[-- Attachment #2: e820_reserveram.patch --]
[-- Type: application/octet-stream, Size: 1770 bytes --]

---
 arch/x86/kernel/e820.c |   18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Index: linux-2.6/arch/x86/kernel/e820.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/e820.c
+++ linux-2.6/arch/x86/kernel/e820.c
@@ -836,6 +836,7 @@ static int __init parse_memopt(char *p)
 early_param("mem", parse_memopt);
 
 static bool __initdata exactmap_parsed;
+static bool __initdata reserveram_parsed;
 
 static int __init parse_memmap_one(char *p)
 {
@@ -845,7 +846,7 @@ static int __init parse_memmap_one(char
 	if (!p)
 		return -EINVAL;
 
-	if (!strncmp(p, "exactmap", 8)) {
+	if (!strncmp(p, "exactmap", 8) || !strncmp(p, "reserveram", 10)) {
 		if (exactmap_parsed)
 			return 0;
 
@@ -858,7 +859,11 @@ static int __init parse_memmap_one(char
 		 */
 		saved_max_pfn = e820_end_of_ram_pfn();
 #endif
-		e820.nr_map = 0;
+		if (!strncmp(p, "reserveram", 10)) {
+			e820_update_range(0, ULLONG_MAX, E820_RAM, E820_RESERVED);
+			reserveram_parsed = true;
+		} else
+			e820.nr_map = 0;
 		userdef = 1;
 		return 0;
 	}
@@ -871,6 +876,10 @@ static int __init parse_memmap_one(char
 	userdef = 1;
 	if (*p == '@') {
 		start_at = memparse(p+1, &p);
+		if (reserveram_parsed) {
+			/* remove old reserved so new ram could take over */
+			e820_remove_range(start_at, mem_size, E820_RESERVED, 0);
+		}
 		e820_add_region(start_at, mem_size, E820_RAM);
 	} else if (*p == '#') {
 		start_at = memparse(p+1, &p);
@@ -890,6 +899,11 @@ static int __init parse_memmap_opt(char
 	p = strstr(p, "exactmap");
 	if (p)
 		parse_memmap_one("exactmap");
+	else {
+		p = strstr(boot_command_line, "reserveram");
+		if (p)
+			parse_memmap_one("reserveram");
+	}
 
 	while (str) {
 		char *k = strchr(str, ',');

Re: [PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[Yinghai Lu]

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

On Tue, Jan 22, 2013 at 12:06 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Tue, Jan 22, 2013 at 8:32 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>>> Again: Please explain what is bad with this solution.
>>> I cannot see a better and more robust way for kdump other than
>>> reserving the original reserved memory areas as declared by the BIOS.
>>
>> It is bad because it creates more complexity than is needed.
>>
>> The whole point is that what we want is simply to switch type 1 to type
>> X, with the sole exceptions being the areas explicitly reserved for the
>> kdump kernel.
>
> Do you prefer to  "reserveram" way in attached patch?

Hi, Thomas,

Can you please check attached reserveram version on your setup?

If it is ok, i will put it in for-x86-boot patchset and send it to
Peter for v3.9.

Thanks

Yinghai

[-- Attachment #2: e820_reserveram_v2.patch --]
[-- Type: application/octet-stream, Size: 2803 bytes --]

Subject: [PATCH] x86, mm: Introduce memmap=reserveram

kdump voided the whole original e820 map and half way made
it up via memmap= options passed via kdump boot params again.

But this is conceptionally wrong. The whole original memory ranges
which are declared reserved, ACPI data/nvs or however are not usable
must stay the same and get honored by the kdump kernel.

Therefore memmap=reserveram gets introduced.
kdump passes this one and only the usable e820 ranges are updated to
reserved at first by kernel, then when kdump passes the usable ranges
to use via memmap=x@y parameter(s), kernel will remove reserved range
and add usable range accordingly.

This for example fixes mmconf (extended PCI config access) and
possibly other kernel parts which rely on remapped memory to be
in reserved or ACPI (data/nvs) declared e820 memory areas.

Changelog is from Thomas Renninger, and updated to reserveram

-v2: According to HPA, use reserveram instead of resetusablemap.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/kernel/e820.c |   19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

Index: linux-2.6/arch/x86/kernel/e820.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/e820.c
+++ linux-2.6/arch/x86/kernel/e820.c
@@ -836,6 +836,7 @@ static int __init parse_memopt(char *p)
 early_param("mem", parse_memopt);
 
 static bool __initdata exactmap_parsed;
+static bool __initdata reserveram_parsed;
 
 static int __init parse_memmap_one(char *p)
 {
@@ -845,7 +846,7 @@ static int __init parse_memmap_one(char
 	if (!p)
 		return -EINVAL;
 
-	if (!strncmp(p, "exactmap", 8)) {
+	if (!strncmp(p, "exactmap", 8) || !strncmp(p, "reserveram", 10)) {
 		if (exactmap_parsed)
 			return 0;
 
@@ -858,7 +859,12 @@ static int __init parse_memmap_one(char
 		 */
 		saved_max_pfn = e820_end_of_ram_pfn();
 #endif
-		e820.nr_map = 0;
+		if (!strncmp(p, "reserveram", 10)) {
+			e820_update_range(0, ULLONG_MAX, E820_RAM,
+					  E820_RESERVED);
+			reserveram_parsed = true;
+		} else
+			e820.nr_map = 0;
 		userdef = 1;
 		return 0;
 	}
@@ -871,6 +877,10 @@ static int __init parse_memmap_one(char
 	userdef = 1;
 	if (*p == '@') {
 		start_at = memparse(p+1, &p);
+		if (reserveram_parsed) {
+			/* Remove old reserved so new ram could take over. */
+			e820_remove_range(start_at, mem_size, E820_RESERVED, 0);
+		}
 		e820_add_region(start_at, mem_size, E820_RAM);
 	} else if (*p == '#') {
 		start_at = memparse(p+1, &p);
@@ -890,6 +900,11 @@ static int __init parse_memmap_opt(char
 	p = strstr(p, "exactmap");
 	if (p)
 		parse_memmap_one("exactmap");
+	else {
+		p = strstr(boot_command_line, "reserveram");
+		if (p)
+			parse_memmap_one("reserveram");
+	}
 
 	while (str) {
 		char *k = strchr(str, ',');

Re: [PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[Thomas Renninger]

On Wednesday, January 23, 2013 08:07:19 PM Yinghai Lu wrote:
> On Tue, Jan 22, 2013 at 12:06 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> > On Tue, Jan 22, 2013 at 8:32 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> >>> Again: Please explain what is bad with this solution.
> >>> I cannot see a better and more robust way for kdump other than
> >>> reserving the original reserved memory areas as declared by the BIOS.
> >> 
> >> It is bad because it creates more complexity than is needed.
> >> 
> >> The whole point is that what we want is simply to switch type 1 to type
> >> X, with the sole exceptions being the areas explicitly reserved for the
> >> kdump kernel.
> > 
> > Do you prefer to  "reserveram" way in attached patch?
> 
> Hi, Thomas,
> 
> Can you please check attached reserveram version on your setup?
> 
> If it is ok, i will put it in for-x86-boot patchset and send it to
> Peter for v3.9.

But this (converting usable memory to reserved one before usable kdump memory
is added) will let machines run into problems again for which the check:
"mmconf area must be in reserved memory" got added?

If, then memory which was usable before has to be converted to a special
E820_KUMP (or whatever type) to make sure existing checks which look for
"is reserved memory" still work the same way as in a productive kernel.

Advantage of this would be that the info what originally was usable
memory is preserved and can be used in future kdump related patches.

So I guess the final patch should be:
   - Add a new e820 type:
        E820_KDUMP_RESERVED /* Originally usable memory where the crashed
                                                    kernel kernel resided in */
  - Use Yinghai's last posted patch, but instead of:
+			e820_update_range(0, ULLONG_MAX, E820_RAM,
+					  E820_RESERVED);
...
+			e820_remove_range(start_at, mem_size, E820_RESERVED, 0);
do:
+			e820_update_range(0, ULLONG_MAX, E820_RAM,
+					  E820_KDUMP_RESERVED);
...
+			e820_remove_range(start_at, mem_size, E820_KDUMP_RESERVED, 0);

  - Come up with another memmap=kdump_reserve_ram memmap option name
    or however it should get named...

If this proposal gets accepted, I can send a tested patch...

       Thomas

Re: [PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[H. Peter Anvin]

On 01/28/2013 05:05 PM, Thomas Renninger wrote:
> 
> But this (converting usable memory to reserved one before usable kdump memory
> is added) will let machines run into problems again for which the check:
> "mmconf area must be in reserved memory" got added?
> 
> If, then memory which was usable before has to be converted to a special
> E820_KUMP (or whatever type) to make sure existing checks which look for
> "is reserved memory" still work the same way as in a productive kernel.
> 
> Advantage of this would be that the info what originally was usable
> memory is preserved and can be used in future kdump related patches.
> 
> So I guess the final patch should be:
>    - Add a new e820 type:
>         E820_KDUMP_RESERVED /* Originally usable memory where the crashed
>                                                     kernel kernel resided in */
>   - Use Yinghai's last posted patch, but instead of:
> +			e820_update_range(0, ULLONG_MAX, E820_RAM,
> +					  E820_RESERVED);
> ...
> +			e820_remove_range(start_at, mem_size, E820_RESERVED, 0);
> do:
> +			e820_update_range(0, ULLONG_MAX, E820_RAM,
> +					  E820_KDUMP_RESERVED);
> ...
> +			e820_remove_range(start_at, mem_size, E820_KDUMP_RESERVED, 0);
> 
>   - Come up with another memmap=kdump_reserve_ram memmap option name
>     or however it should get named...
> 
> If this proposal gets accepted, I can send a tested patch...
> 

Yes, this is much saner.  There really shouldn't need to be an option,
even; since the tools need to be modified anyway, just modify the actual
memory map data structure itself.

	-hpa

Re: [PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[Yinghai Lu]

On Mon, Jan 28, 2013 at 5:11 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> So I guess the final patch should be:
>>    - Add a new e820 type:
>>         E820_KDUMP_RESERVED /* Originally usable memory where the crashed
>>                                                     kernel kernel resided in */
>>   - Use Yinghai's last posted patch, but instead of:
>> +                     e820_update_range(0, ULLONG_MAX, E820_RAM,
>> +                                       E820_RESERVED);
>> ...
>> +                     e820_remove_range(start_at, mem_size, E820_RESERVED, 0);
>> do:
>> +                     e820_update_range(0, ULLONG_MAX, E820_RAM,
>> +                                       E820_KDUMP_RESERVED);
>> ...
>> +                     e820_remove_range(start_at, mem_size, E820_KDUMP_RESERVED, 0);
>>
>>   - Come up with another memmap=kdump_reserve_ram memmap option name
>>     or however it should get named...
>>
>> If this proposal gets accepted, I can send a tested patch...
>>
>
> Yes, this is much saner.  There really shouldn't need to be an option,
> even; since the tools need to be modified anyway, just modify the actual
> memory map data structure itself.

yes,

kexec-tools will change that to E820_KDUMP_RESERVED (or other good name).

We only need to update kernel to get old max_pfn by
checking E820_KDUMP_RESERVED.

Yinghai

Re: [PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[H. Peter Anvin]

On 01/28/2013 06:10 PM, Yinghai Lu wrote:
>
> kexec-tools will change that to E820_KDUMP_RESERVED (or other good name).
>
> We only need to update kernel to get old max_pfn by
> checking E820_KDUMP_RESERVED.
>

OK, I have asked this before, but I still have not gotten any acceptable 
answer:

Why do we still have max_*_pfn at all?  Shouldn't it all be based on 
memblocks by now?

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[Yinghai Lu]

On Mon, Jan 28, 2013 at 6:11 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 01/28/2013 06:10 PM, Yinghai Lu wrote:
>>
>>
>> kexec-tools will change that to E820_KDUMP_RESERVED (or other good name).
>>
>> We only need to update kernel to get old max_pfn by
>> checking E820_KDUMP_RESERVED.
>>
>
> OK, I have asked this before, but I still have not gotten any acceptable
> answer:
>
> Why do we still have max_*_pfn at all?  Shouldn't it all be based on
> memblocks by now?

saved_max_pfn is used for kdump:
drivers/char/mem.c::read_oldmem will stop there.
...
        while (count) {
                pfn = *ppos / PAGE_SIZE;
                if (pfn > saved_max_pfn)
                        return read;
...

Re: [PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[H. Peter Anvin]

On 01/28/2013 06:19 PM, Yinghai Lu wrote:
> On Mon, Jan 28, 2013 at 6:11 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 01/28/2013 06:10 PM, Yinghai Lu wrote:
>>>
>>>
>>> kexec-tools will change that to E820_KDUMP_RESERVED (or other good name).
>>>
>>> We only need to update kernel to get old max_pfn by
>>> checking E820_KDUMP_RESERVED.
>>>
>>
>> OK, I have asked this before, but I still have not gotten any acceptable
>> answer:
>>
>> Why do we still have max_*_pfn at all?  Shouldn't it all be based on
>> memblocks by now?
>
> saved_max_pfn is used for kdump:
> drivers/char/mem.c::read_oldmem will stop there.
> ...
>          while (count) {
>                  pfn = *ppos / PAGE_SIZE;
>                  if (pfn > saved_max_pfn)
>                          return read;
> ...

That is a non-answer.

Why do we have *any* instances of max_pfn or max_low_pfn in the kernel 
anymore?

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[H. Peter Anvin]

To be more clear: the max_pfn stuff seems like a relic of the past, and I am wondering what it would take to get rid of it.

It clearly has the wrong semantics, except perhaps in the most trivial allocator models.

Yinghai Lu <yinghai@kernel.org> wrote:

>On Mon, Jan 28, 2013 at 6:11 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 01/28/2013 06:10 PM, Yinghai Lu wrote:
>>>
>>>
>>> kexec-tools will change that to E820_KDUMP_RESERVED (or other good
>name).
>>>
>>> We only need to update kernel to get old max_pfn by
>>> checking E820_KDUMP_RESERVED.
>>>
>>
>> OK, I have asked this before, but I still have not gotten any
>acceptable
>> answer:
>>
>> Why do we still have max_*_pfn at all?  Shouldn't it all be based on
>> memblocks by now?
>
>saved_max_pfn is used for kdump:
>drivers/char/mem.c::read_oldmem will stop there.
>...
>        while (count) {
>                pfn = *ppos / PAGE_SIZE;
>                if (pfn > saved_max_pfn)
>                        return read;
>...

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[Yinghai Lu]

On Mon, Jan 28, 2013 at 6:27 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> To be more clear: the max_pfn stuff seems like a relic of the past, and I am wondering what it would take to get rid of it.
>
> It clearly has the wrong semantics, except perhaps in the most trivial allocator models.
>
one thing i think could be : use that decide if we need iommu/swiotlb.
like in
arch/x86/kernel/amd_gart_64.c
arch/x86/kernel/pci-swiotlb.c
drivers/iommu/amd_iommu.c
...

Yinghai

Re: [PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[H. Peter Anvin]

I guess that very limited use might make sense.

Yinghai Lu <yinghai@kernel.org> wrote:

>On Mon, Jan 28, 2013 at 6:27 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> To be more clear: the max_pfn stuff seems like a relic of the past,
>and I am wondering what it would take to get rid of it.
>>
>> It clearly has the wrong semantics, except perhaps in the most
>trivial allocator models.
>>
>one thing i think could be : use that decide if we need iommu/swiotlb.
>like in
>arch/x86/kernel/amd_gart_64.c
>arch/x86/kernel/pci-swiotlb.c
>drivers/iommu/amd_iommu.c
>...
>
>Yinghai

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [PATCH 2/2] x86 e820: Introduce memmap=resetusablemap for kdump usage

[Thomas Renninger]

On Tuesday, January 29, 2013 03:10:38 AM Yinghai Lu wrote:
> On Mon, Jan 28, 2013 at 5:11 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> >> So I guess the final patch should be:
> >>    - Add a new e820 type:
> >>         E820_KDUMP_RESERVED /* Originally usable memory where the crashed
> >>                                                     kernel kernel resided in */
> >>   - Use Yinghai's last posted patch, but instead of:
> >> +                     e820_update_range(0, ULLONG_MAX, E820_RAM,
> >> +                                       E820_RESERVED);
> >> ...
> >> +                     e820_remove_range(start_at, mem_size, E820_RESERVED, 0);
> >> do:
> >> +                     e820_update_range(0, ULLONG_MAX, E820_RAM,
> >> +                                       E820_KDUMP_RESERVED);
> >> ...
> >> +                     e820_remove_range(start_at, mem_size, E820_KDUMP_RESERVED, 0);
> >>
> >>   - Come up with another memmap=kdump_reserve_ram memmap option name
> >>     or however it should get named...
> >>
> >> If this proposal gets accepted, I can send a tested patch...
> >>
> >
> > Yes, this is much saner.  There really shouldn't need to be an option,
> > even; since the tools need to be modified anyway, just modify the actual
> > memory map data structure itself.
> 
> yes,
> 
> kexec-tools will change that to E820_KDUMP_RESERVED (or other good name).
> 
> We only need to update kernel to get old max_pfn by
> checking E820_KDUMP_RESERVED.

Wait, above proposal does not include kexec-tools mangling of the
e820 table, for several reasons:

- Keep the boot interface clean and pass the original table
- Only one possible error source on e820 table modifications
- While hpa proposed kexec-tools to pass a modified e820 table to
  make things easier, exactly the opposite is the case:
  If kexec-tools and the kernel modify the table, things are more
  complex and hard to understand in case of debugging where things
  went wrong
- It's really easy to do that in the kernel. As shown above it should
  simply be this line to change usable areas into E820_KDUMP_RESERVED
  ones:
  e820_update_range(0, ULLONG_MAX, E820_RAM, E820_KDUMP_RESERVED);
  and possibly slight adjusting when the memmap=X#Y memory
  the kdump kernel uses is added (has to override E820_KDUMP_RESERVED
  areas with usable memory again)

My previously posted kexec-tools patches should simply work,
it's just that the memmap option name changes to:
memmap=kdump_reserve_ram

This is what I proposed and is IMO the best and less complex
way to go. I guess I still wait another day for comments and
will send something if you agree.

   Thomas