2008-06-25 12:02:31

by Bernhard Walle

[permalink] [raw]
Subject: Limit E820 map when specifying mem parameter

This patch modifies the E820 map when specifying the mem kernel command line
parameter. That's the behaviour i386 had before the merging work in the
current "tip" tree.

As Yinghai Lu pointed out in email discussion, e820_update_range() should be
used for the updating instead of an own function. Two modifications in
e820_update_range() are necessary:

1. Fix a small bug that prevented the partically covered entry from
being stripped (size is not updated).

2. Small API extension to be able to specify size == ULLONG_MAX to
update the whole map from size to the end.

The modification is necessary that kexec can build the ELF core headers only
for the used memory. Once the exporting of the real, unmodified memory map is
in the kernel, kexec can use the raw map and still reboot with full memory
size.

The patch is against 2.6.26-rc7-tip and has been successfully tested on i386
and x86-64, with and without "mem" parameter.



2008-06-25 12:01:57

by Bernhard Walle

[permalink] [raw]
Subject: [PATCH 2/3] e820_update_range(): Allow specifying ULLONG_MAX

Allow the specifying of ULLONG_MAX to limit the whole E820 map from the
specified start to the end. Without the patch, there would be integer
overflows.


Signed-off-by: Bernhard Walle <[email protected]>
---
arch/x86/kernel/e820.c | 11 +++++++++--
1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index e466073..7d1109b 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -397,6 +397,9 @@ int __init copy_e820_map(struct e820entry *biosmap, int nr_map)
return __copy_e820_map(biosmap, nr_map);
}

+/*
+ * Pass size == ULLONG_MAX to update until the end.
+ */
u64 __init e820_update_range(u64 start, u64 size, unsigned old_type,
unsigned new_type)
{
@@ -412,14 +415,18 @@ u64 __init e820_update_range(u64 start, u64 size, unsigned old_type,
continue;
/* totally covered? */
if (ei->addr >= start &&
- (ei->addr + ei->size) <= (start + size)) {
+ (((ei->addr + ei->size) <= (start + size)) ||
+ (size == ULLONG_MAX))) {
ei->type = new_type;
real_updated_size += ei->size;
continue;
}
/* partially covered */
final_start = max(start, ei->addr);
- final_end = min(start + size, ei->addr + ei->size);
+ if (size == ULLONG_MAX)
+ final_end = ei->addr + ei->size;
+ else
+ final_end = min(start + size, ei->addr + ei->size);
if (final_start >= final_end)
continue;
ei->size -= final_end - final_start;
--
1.5.4.5

2008-06-25 12:02:43

by Bernhard Walle

[permalink] [raw]
Subject: [PATCH 1/3] e820_update_range(): Strip size of original region

This patch fixes a bug in e820_update_range(): Previously, if a region was
partially covered, then e820_update_range() only added a new E820 range but
didn't update the end/size of the previous range. That lead to duplicate
covering of a region.

Patch tested on i386 and x86-64 with patch that uses e820_update_range()
to limit the E820 map when "mem" parameter is specified on the command line.


Signed-off-by: Bernhard Walle <[email protected]>
---
arch/x86/kernel/e820.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index e285ea3..e466073 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -422,6 +422,7 @@ u64 __init e820_update_range(u64 start, u64 size, unsigned old_type,
final_end = min(start + size, ei->addr + ei->size);
if (final_start >= final_end)
continue;
+ ei->size -= final_end - final_start;
e820_add_region(final_start, final_end - final_start,
new_type);
real_updated_size += final_end - final_start;
--
1.5.4.5

2008-06-25 12:02:56

by Bernhard Walle

[permalink] [raw]
Subject: [PATCH 3/3] Limit E820 map when a user-defined memory map is specified

This patch brings back limiting of the E820 map when a user-defined
E820 map is specified. While the behaviour of i386 (32 bit) was to limit
the E820 map (and /proc/iomem), the behaviour of x86-64 (64 bit) was not to
limit.

That patch limits the E820 map again for both x86 architectures.

Code was tested for compilation and booting on a 32 bit and 64 bit system.


Signed-off-by: Bernhard Walle <[email protected]>
---
arch/x86/kernel/e820.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 7d1109b..19b7f05 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -979,6 +979,8 @@ static int __init parse_memopt(char *p)

mem_size = memparse(p, &p);
end_user_pfn = mem_size>>PAGE_SHIFT;
+ e820_update_range(mem_size, ULLONG_MAX, E820_RAM, E820_RESERVED);
+
return 0;
}
early_param("mem", parse_memopt);
@@ -1023,6 +1025,7 @@ static int __init parse_memmap_opt(char *p)
e820_add_region(start_at, mem_size, E820_RESERVED);
} else {
end_user_pfn = (mem_size >> PAGE_SHIFT);
+ e820_update_range(mem_size, ULLONG_MAX, E820_RAM, E820_RESERVED);
}
return *p == '\0' ? 0 : -EINVAL;
}
--
1.5.4.5

2008-06-25 15:56:43

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 1/3] e820_update_range(): Strip size of original region

On Wed, Jun 25, 2008 at 5:02 AM, Bernhard Walle <[email protected]> wrote:
> This patch fixes a bug in e820_update_range(): Previously, if a region was
> partially covered, then e820_update_range() only added a new E820 range but
> didn't update the end/size of the previous range. That lead to duplicate
> covering of a region.
>
> Patch tested on i386 and x86-64 with patch that uses e820_update_range()
> to limit the E820 map when "mem" parameter is specified on the command line.
>
>
> Signed-off-by: Bernhard Walle <[email protected]>
> ---
> arch/x86/kernel/e820.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index e285ea3..e466073 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -422,6 +422,7 @@ u64 __init e820_update_range(u64 start, u64 size, unsigned old_type,
> final_end = min(start + size, ei->addr + ei->size);
> if (final_start >= final_end)
> continue;
> + ei->size -= final_end - final_start;
> e820_add_region(final_start, final_end - final_start,
> new_type);
> real_updated_size += final_end - final_start;
> --

this one is not needed, I sent one updated to Ingo, and it is in
tip/setup-memory
[PATCH] x86: change size if e820_update/remove_range

YH

2008-06-25 16:01:17

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 3/3] Limit E820 map when a user-defined memory map is specified

On Wed, Jun 25, 2008 at 5:02 AM, Bernhard Walle <[email protected]> wrote:
> This patch brings back limiting of the E820 map when a user-defined
> E820 map is specified. While the behaviour of i386 (32 bit) was to limit
> the E820 map (and /proc/iomem), the behaviour of x86-64 (64 bit) was not to
> limit.
>
> That patch limits the E820 map again for both x86 architectures.
>
> Code was tested for compilation and booting on a 32 bit and 64 bit system.
>
>
> Signed-off-by: Bernhard Walle <[email protected]>
> ---
> arch/x86/kernel/e820.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index 7d1109b..19b7f05 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -979,6 +979,8 @@ static int __init parse_memopt(char *p)
>
> mem_size = memparse(p, &p);
> end_user_pfn = mem_size>>PAGE_SHIFT;
> + e820_update_range(mem_size, ULLONG_MAX, E820_RAM, E820_RESERVED);
> +
> return 0;
> }
> early_param("mem", parse_memopt);
> @@ -1023,6 +1025,7 @@ static int __init parse_memmap_opt(char *p)
> e820_add_region(start_at, mem_size, E820_RESERVED);
> } else {
> end_user_pfn = (mem_size >> PAGE_SHIFT);
> + e820_update_range(mem_size, ULLONG_MAX, E820_RAM, E820_RESERVED);
> }
> return *p == '\0' ? 0 : -EINVAL;
> }
> --

only this one is needed now. but please change ULLONG_MAX to
ULLONG_MAX - mem_size

YH

2008-06-25 16:02:51

by Bernhard Walle

[permalink] [raw]
Subject: Re: [PATCH 3/3] Limit E820 map when a user-defined memory map is specified

* Yinghai Lu [2008-06-25 09:01]:
> On Wed, Jun 25, 2008 at 5:02 AM, Bernhard Walle <[email protected]> wrote:
> > }
> > early_param("mem", parse_memopt);
> > @@ -1023,6 +1025,7 @@ static int __init parse_memmap_opt(char *p)
> > e820_add_region(start_at, mem_size, E820_RESERVED);
> > } else {
> > end_user_pfn = (mem_size >> PAGE_SHIFT);
> > + e820_update_range(mem_size, ULLONG_MAX, E820_RAM, E820_RESERVED);
> > }
> > return *p == '\0' ? 0 : -EINVAL;
> > }
> > --
>
> only this one is needed now. but please change ULLONG_MAX to
> ULLONG_MAX - mem_size

Why can't we add that check at the beginning of e820_update_range() as
you suggested?


Bernhard

2008-06-25 16:59:16

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 3/3] Limit E820 map when a user-defined memory map is specified

On Wed, Jun 25, 2008 at 9:03 AM, Bernhard Walle <[email protected]> wrote:
> * Yinghai Lu [2008-06-25 09:01]:
>> On Wed, Jun 25, 2008 at 5:02 AM, Bernhard Walle <[email protected]> wrote:
>> > }
>> > early_param("mem", parse_memopt);
>> > @@ -1023,6 +1025,7 @@ static int __init parse_memmap_opt(char *p)
>> > e820_add_region(start_at, mem_size, E820_RESERVED);
>> > } else {
>> > end_user_pfn = (mem_size >> PAGE_SHIFT);
>> > + e820_update_range(mem_size, ULLONG_MAX, E820_RAM, E820_RESERVED);
>> > }
>> > return *p == '\0' ? 0 : -EINVAL;
>> > }
>> > --
>>
>> only this one is needed now. but please change ULLONG_MAX to
>> ULLONG_MAX - mem_size
>
> Why can't we add that check at the beginning of e820_update_range() as
> you suggested?
that patch about fixing e820_update_rang is in tip/setup-memory.

so the one is supposed to be ok, but it is good to keep good input
parameter too.

YH

2008-06-25 19:40:14

by Bernhard Walle

[permalink] [raw]
Subject: Re: [PATCH 3/3] Limit E820 map when a user-defined memory map is specified

* "Yinghai Lu" <[email protected]> [2008-06-25 09:58]:
> On Wed, Jun 25, 2008 at 9:03 AM, Bernhard Walle <[email protected]> wrote:
> >> only this one is needed now. but please change ULLONG_MAX to
> >> ULLONG_MAX - mem_size
> >
> > Why can't we add that check at the beginning of e820_update_range() as
> > you suggested?
> that patch about fixing e820_update_rang is in tip/setup-memory.
>
> so the one is supposed to be ok, but it is good to keep good input
> parameter too.

Okay, thanks for review and comments. Patch sent.



Bernhard
--
Bernhard Walle, SUSE LINUX Products GmbH, Architecture Development