Re: [PATCH] Prevent reserving RAM in the region already reserved by BIOS

Previous thread: linux-next: manual merge of the devicetree tree with the net tree by Stephen Rothwell on Tuesday, May 18, 2010 - 10:23 pm. (2 messages)

Next thread: [PATCH] Staging: winbond: Fix C99 Comment issues in mac_structures.h by Adam Latham on Tuesday, May 18, 2010 - 10:55 pm. (2 messages)
From: Mathieu Rondonneau
Date: Tuesday, May 18, 2010 - 10:35 pm

Does it make sense to prevent looking for stolen RAM below the ISA section.


Signed-off-by: Mathieu Rondonneau <mathieu.rondonneau@gmail.com>
---
 arch/x86/kernel/e820.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 7bca3c6..322c9c3 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1156,6 +1156,8 @@ void __init e820_reserve_resources_late(void)
                        end = MAX_RESOURCE_SIZE;
                if (start >= end)
                        continue;
+               if (end < ISA_START_ADDRESS)
+                       continue;
                printk(KERN_DEBUG "reserve RAM buffer: %016llx - %016llx ",
                               start, end);
                reserve_region_with_split(&iomem_resource, start, end,
-- 
1.6.3.3
--

From: Yinghai
Date: Wednesday, May 19, 2010 - 10:40 am

do you notice any changes in /proc/iomem?

YH
--

From: H. Peter Anvin
Date: Wednesday, May 19, 2010 - 10:43 am

It should be harmless to reserve memory which is already reserved, so
this patch is at best a no-op.  Furthermore, it introduces another
instance of special address space (ISA_START_ADDRESS in this case) which
is never a good thing.

	-hpa

--

From: H. Peter Anvin
Date: Wednesday, May 19, 2010 - 4:00 pm

Why are you mapping a fixed-address in ISA space to begin with?

Requests to a fixed address (as opposed to dynamic allocation) have to
be granted even in reserved space -- after all, that's what the address
might be reserved for!

	-hpa
--

From: Mathieu Rondonneau
Date: Wednesday, May 19, 2010 - 5:01 pm

it' s not in ISA space.
once loading the NVIDIA driver, the warnign oops shows up.
4K starting at 0x9f800 (i.e. to 0xa007ff) overlap with the ISA space
(starting at 0xa0000).

When I don' t load the driver, no oops.
So I am assuming nvidia driver request 4K of memory, that happens to
be available in the bios area.

I think that there is a check missing somewhere to report that
available RAM buffer is already reserved (by BIOS) so we need to get
it from somewhere else.

Does it make sense?
-Mathieu

--

From: Yinghai
Date: Wednesday, May 19, 2010 - 5:07 pm

please check if this patch fix the problem.

Subject: [PATCH] x86: Align e820 ram range to page

To workaround wrong BIOS memory map.

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

---
 arch/x86/kernel/e820.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

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
@@ -910,6 +910,47 @@ static int __init parse_memmap_opt(char
 }
 early_param("memmap", parse_memmap_opt);
 
+static void __init e820_align_ram_page(void)
+{
+	int i;
+	bool changed = false;;
+
+	for (i = 0; i < e820.nr_map; i++) {
+		struct e820entry *entry = &e820.map[i];
+		u64 start, end;
+		u64 start_aligned, end_aligned;
+
+		if (entry->type != E820_RAM)
+			continue;
+
+		start = entry->addr;
+		end = start + entry->size;
+
+		start_aligned = round_up(start, PAGE_SIZE);
+		end_aligned = round_down(end, PAGE_SIZE);
+
+		if (end_aligned <= start_aligned) {
+			e820_update_range(start, end - start, E820_RAM, E820_RESERVED);
+			changed = true;
+			continue;
+		}
+		if (start < start_aligned) {
+			e820_update_range(start, start_aligned - start, E820_RAM, E820_RESERVED);
+			changed = true;
+		}
+		if (end_aligned < end) {
+			e820_update_range(end_aligned, end - end_aligned, E820_RAM, E820_RESERVED);
+			changed = true;
+		}
+	}
+
+	if (changed) {
+		sanitize_e820_map();
+		printk(KERN_INFO "aligned physical RAM map:\n");
+		e820_print_map("aligned");
+	}
+}
+
 void __init finish_e820_parsing(void)
 {
 	if (userdef) {
@@ -922,6 +963,9 @@ void __init finish_e820_parsing(void)
 		printk(KERN_INFO "user-defined physical RAM map:\n");
 		e820_print_map("user");
 	}
+
+	/* In case, We have RAM entres that are not PAGE aligned */
+	e820_align_ram_page();
 }
 
 static inline const char *e820_type_to_string(int e820_type)
--

From: H. Peter Anvin
Date: Wednesday, May 19, 2010 - 5:09 pm

No.

The top page (0x9f000) is generally also reserved... there is pretty
much always BIOS data structures there.

It sounds like the Nvidia driver is trying to map those, and doing it
incorrectly.  -ENVIDIA.

	-hpa
--

From: Alan Cox
Date: Thursday, May 20, 2010 - 5:12 am

On Wed, 19 May 2010 17:01:41 -0700

If this is the proprietary Nvidia driver you need to take the matter up
with Nvidia. I assume they are trying to remap chunks of the EBA for some
reason.

And please can you not top post to this list, it makes it very hard to
follow when you are replying in a different order to everyone else.

Alan
--

From: Mathieu Rondonneau
Date: Friday, May 21, 2010 - 8:15 am

Thanks Alan, I will post on NVIDIA forum. Thanks for the feedback.

Yinghai, your patch had some missing parameter for the sanitize function.
It does fixes the bios side (/proc/iomem) that now shows only aligned sections.
But it does not prevent the Nvidia driver to try to allocate on non-4K
align (i.e. if this is what' s happening).

Thanks for the help,
-Mathieu
--

From: Mathieu Rondonneau
Date: Friday, May 21, 2010 - 9:58 pm

On Fri, May 21, 2010 at 8:15 AM, Mathieu Rondonneau

Found on Nvidia forum that they are aware of the problem. They said
that this will be fixed in the next driver version.

-Mathieu
--

From: Mathieu Rondonneau
Date: Wednesday, May 19, 2010 - 3:58 pm

Thanks for the feedback.

No, I don' t see any changes in the /proc/iomem.

I am trying to prevent a ioremap of a 4K size on a non aligned 4K
address that is below the ISA_START_ADDRESS.

The problem generates a oops about overlapping.
I have a fix which instruct to not to do any re-map if the section
name is "reserved".
Which is not really clean.
I am looking for a clean way to tell the ioremap function to not remap
bios reserved memory.
That' s why I thought the e820 would be a good start.

I will continue looking into this. It does not crash the systems. A
warning generates the oops.

-Mathieu


--

Previous thread: linux-next: manual merge of the devicetree tree with the net tree by Stephen Rothwell on Tuesday, May 18, 2010 - 10:23 pm. (2 messages)

Next thread: [PATCH] Staging: winbond: Fix C99 Comment issues in mac_structures.h by Adam Latham on Tuesday, May 18, 2010 - 10:55 pm. (2 messages)