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

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

[Mathieu Rondonneau]

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

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

[Yinghai]

On 05/18/2010 10:35 PM, Mathieu Rondonneau wrote:
> 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,

do you notice any changes in /proc/iomem?

YH

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

[H. Peter Anvin]

On 05/19/2010 10:40 AM, Yinghai wrote:
> On 05/18/2010 10:35 PM, Mathieu Rondonneau wrote:
>> 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,
> 
> do you notice any changes in /proc/iomem?
> 

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

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

[Mathieu Rondonneau]

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


On Wed, May 19, 2010 at 10:43 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 05/19/2010 10:40 AM, Yinghai wrote:
>> On 05/18/2010 10:35 PM, Mathieu Rondonneau wrote:
>>> 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,
>>
>> do you notice any changes in /proc/iomem?
>>
>
> 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
>
>

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

[H. Peter Anvin]

On 05/19/2010 03:58 PM, Mathieu Rondonneau wrote:
> 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.
> 

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

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

[Mathieu Rondonneau]

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

On Wed, May 19, 2010 at 4:00 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 05/19/2010 03:58 PM, Mathieu Rondonneau wrote:
>> 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.
>>
>
> 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
>

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

[Yinghai]

On 05/19/2010 05:01 PM, Mathieu Rondonneau wrote:
> 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.

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)

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

[H. Peter Anvin]

On 05/19/2010 05:01 PM, Mathieu Rondonneau wrote:
> 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

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

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

[Alan Cox]

On Wed, 19 May 2010 17:01:41 -0700
Mathieu Rondonneau <mathieu.rondonneau@gmail.com> wrote:

> 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).

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

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

[Mathieu Rondonneau]

On Thu, May 20, 2010 at 5:12 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On Wed, 19 May 2010 17:01:41 -0700
> Mathieu Rondonneau <mathieu.rondonneau@gmail.com> wrote:
>
>> 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).
>
> 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
>

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

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

[Mathieu Rondonneau]

On Fri, May 21, 2010 at 8:15 AM, Mathieu Rondonneau
<mathieu.rondonneau@gmail.com> wrote:
> On Thu, May 20, 2010 at 5:12 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> On Wed, 19 May 2010 17:01:41 -0700
>> Mathieu Rondonneau <mathieu.rondonneau@gmail.com> wrote:
>>
>>> 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).
>>
>> 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
>>
>
> 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
>

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

-Mathieu