Date:	Fri, 11 Jan 2013 13:33:48 +0100
From:	Thomas Renninger <trenn@...e.de>
To:	Yinghai Lu <yinghai@...nel.org>
Cc:	MUNEDA Takahiro <muneda.takahiro@...fujitsu.com>,
	Takao Indoh <indou.takao@...fujitsu.com>,
	linux-pci@...r.kernel.org, x86@...nel.org,
	linux-kernel@...r.kernel.org, andi@...stfloor.org,
	tokunaga.keiich@...fujitsu.com, kexec@...ts.infradead.org,
	hbabu@...ibm.com, mingo@...hat.com, ddutile@...hat.com,
	vgoyal@...hat.com, ishii.hironobu@...fujitsu.com, hpa@...or.com,
	bhelgaas@...gle.com, tglx@...utronix.de, khalid@...ehiking.org,
	horms@...ge.net.au
Subject: [PATCH] x86 e820: only void usable memory areas in memmap=exactmap case

On Friday, January 11, 2013 12:34:37 AM Yinghai Lu wrote:
> On Wed, Jan 9, 2013 at 7:21 PM, Thomas Renninger <trenn@...e.de> wrote:
...
> > Can kexec simply pass the memory to use via memmap=X@Y
> > Then take the original e820 table, but not the usable entries (those
> > are coming from above memmap=X@Y).
> > That would mean that the kexec kernel takes all the
> > original ACPI, ACPI NVS, reserved, unusable (everthing but usable)
> > entries from the original e820 table and identifies the usable memory
> > from memmap boot param?
> 
> kdump scripts already do that for acpi regions, need to update it
> to append that for mmconf.

No, this must get fixed properly:
Use "unusable (ACPI, reserved, whatever..)" regions from the e820 table
passed through bootloader structure.

Only replace "usable" memory areas with the ones passed via memmap=
if memmap=exactmap is passed.

Only taking mmconf area is wrong, kdump kernel has to honour *all*
original reserved memory areas and the info is already there.
I also do not see any kernel vs kexec version incompatibilities with
my approach. Future kexec version can clean up and do not need to
pass ACPI memory area/range via memmap=X#Y anymore.

You find a suitable patch at the end.
I just zeroed out the e820 usable entries (same as e820_remove_range()
above), sanitize_e820_map() should fix that up and it's ensured that
it is called in memmap=exactmap case.

Find serial output of a try (same machine as with my previous posts).
There you find the correctly modified e820 user defined table (all
unusable entries, but usable entries are adjusted) until mmconf
is used gracefully.

> > This would be much smarter than trying to pass the mmconf reserved
> > area and I could imagine other issues will show up if the reserved
> > areas do not match the original ones in the kexec kernel.
> >
> > If this really can be done and memmap=exactmap was only used by kexec,
> > it's logic could be redefined from "drop all e820 entries" to
> > "drop all usable e820 entries" and no further adjustings in
> > kexec/kernel are needed to get mmconf working (and other issues may be
> > avoided before they happen). Beside that ACPI reserved aread is not
> > needed anymore to get passed via memmap=X#Y by kexec.

> yes, we have other user for debug  like simulating user memmap for some
> bugs.
> current problem for exactmap is that we don't scan that at first.
> attached patch could help that.

Yep, this is what I would have come up as well or similar. I looked
at it, but I had no time for doing it and trying out.

You may want to add:
Reviewed-by: Thomas Renninger <trenn@...e.de>
if someone reposts.

   Thomas

-------------------
x86 e820: only void usable memory areas in memmap=exactmap case

All unusable (reserved, ACPI, ACPI NVS,...) areas have to be
honored in kdump case.
Othwerise ACPI parts will quickly run into trouble when trying
to for example early_ioremap reserved areas which are not
declared reserved in kdump kernel.
mmconf area must also be a reserved mem region.
...

Passing unusable memory via memmap= is a design flaw as
this information is already (exactly for this purpose) passed
via bootloader structure.
In kdump case (when memmap=exactmap is passed), only void
(do not use) usable memory regions from the passed e820 table
and use memory areas defined via memmap=X@Y boot parameter instead.
But do still use the "unusable" memory regions from the original e820
table.

Signed-off-by: Thomas Renninger <trenn@...e.de>

---
 arch/x86/kernel/e820.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index dc0b9f0..ae2d657 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -559,6 +559,19 @@ u64 __init e820_remove_range(u64 start, u64 size, unsigned old_type,
 	return real_removed_size;
 }
 
+static void __init e820_remove_range_type(u32 type)
+{
+	int i;
+
+	for (i = 0; i < e820.nr_map; i++) {
+		struct e820entry *ei = &e820.map[i];
+		if (ei->type == type) {
+			memset(ei, 0, sizeof(struct e820entry));
+			continue;
+		}
+	}
+}
+
 void __init update_e820(void)
 {
 	u32 nr_map;
@@ -858,7 +871,11 @@ static int __init parse_memmap_one(char *p)
 		 */
 		saved_max_pfn = e820_end_of_ram_pfn();
 #endif
-		e820.nr_map = 0;
+		/*
+		 * Remove all usable memory (this is for kdump), usable
+		 * memory will be passed via memmap=X@Y parameter
+		 */
+		e820_remove_range_type(E820_RAM);
 		userdef = 1;
 		return 0;
 	}

View attachment "screen.xy" of type "text/plain" (16496 bytes)

Powered by blists - more mailing lists