2008-06-24 18:48:42

by Alok Kataria

[permalink] [raw]
Subject: [PATCH 1/2] cleanup e820_setup_gap v2

This is a preparatory patch for the next patch in series.
Moves some code from e820_setup_gap to a new function e820_search_gap.
This patch is a part of a bug fix where we walk the ACPI table to calculate
a gap for PCI optional devices.

v1->v2: Patch on top of tip/master.
Fixes a bug introduced in the last patch about the typeof "last".
Also the new function e820_search_gap now returns if we found a gap in
e820_map.

Signed-off-by: Alok N Kataria <[email protected]>

Index: linux-x86-tree.git/arch/x86/kernel/e820.c
===================================================================
--- linux-x86-tree.git.orig/arch/x86/kernel/e820.c 2008-06-23 17:24:07.000000000 -0700
+++ linux-x86-tree.git/arch/x86/kernel/e820.c 2008-06-24 10:27:38.000000000 -0700
@@ -477,26 +477,22 @@
}

/*
- * Search for the biggest gap in the low 32 bits of the e820
- * memory space. We pass this space to PCI to assign MMIO resources
- * for hotplug or unconfigured devices in.
- * Hopefully the BIOS let enough space left.
+ * Search for a gap in the e820 memory space from start_addr to 2^32.
*/
-__init void e820_setup_gap(void)
+__init int e820_search_gap(unsigned long *gapstart, unsigned long *gapsize,
+ unsigned long start_addr)
{
- unsigned long gapstart, gapsize, round;
- unsigned long long last;
- int i;
+ unsigned long long last = 0x100000000ull;
+ int i = e820.nr_map;
int found = 0;

- last = 0x100000000ull;
- gapstart = 0x10000000;
- gapsize = 0x400000;
- i = e820.nr_map;
while (--i >= 0) {
unsigned long long start = e820.map[i].addr;
unsigned long long end = start + e820.map[i].size;

+ if (end < start_addr)
+ continue;
+
/*
* Since "last" is at most 4GB, we know we'll
* fit in 32 bits if this condition is true
@@ -504,15 +500,32 @@
if (last > end) {
unsigned long gap = last - end;

- if (gap > gapsize) {
- gapsize = gap;
- gapstart = end;
+ if (gap >= *gapsize) {
+ *gapsize = gap;
+ *gapstart = end;
found = 1;
}
}
if (start < last)
last = start;
}
+ return found;
+}
+
+/*
+ * Search for the biggest gap in the low 32 bits of the e820
+ * memory space. We pass this space to PCI to assign MMIO resources
+ * for hotplug or unconfigured devices in.
+ * Hopefully the BIOS let enough space left.
+ */
+__init void e820_setup_gap(void)
+{
+ unsigned long gapstart, gapsize, round;
+ int found;
+
+ gapstart = 0x10000000;
+ gapsize = 0x400000;
+ found = e820_search_gap(&gapstart, &gapsize, 0);

#ifdef CONFIG_X86_64
if (!found) {
Index: linux-x86-tree.git/include/asm-x86/e820.h
===================================================================
--- linux-x86-tree.git.orig/include/asm-x86/e820.h 2008-06-23 17:24:26.000000000 -0700
+++ linux-x86-tree.git/include/asm-x86/e820.h 2008-06-24 10:27:38.000000000 -0700
@@ -71,6 +71,8 @@
int checktype);
extern void update_e820(void);
extern void e820_setup_gap(void);
+extern int e820_search_gap(unsigned long *gapstart, unsigned long *gapsize,
+ unsigned long start_addr);
struct setup_data;
extern void parse_e820_ext(struct setup_data *data, unsigned long pa_data);




2008-06-25 16:08:40

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 1/2] cleanup e820_setup_gap v2


* Alok Kataria <[email protected]> wrote:

> This is a preparatory patch for the next patch in series.
> Moves some code from e820_setup_gap to a new function e820_search_gap.
> This patch is a part of a bug fix where we walk the ACPI table to calculate
> a gap for PCI optional devices.
>
> v1->v2: Patch on top of tip/master.
> Fixes a bug introduced in the last patch about the typeof "last".
> Also the new function e820_search_gap now returns if we found a gap in
> e820_map.

applied to tip/x86/setup-memory - thanks Alok.

should i put the other patch, "acpi based pci gap caluculation v2" into
tip/out-of-tree for more testing of this new e820 facility?

Ingo

2008-06-25 18:04:44

by Alok Kataria

[permalink] [raw]
Subject: Re: [PATCH 1/2] cleanup e820_setup_gap v2

On Wed, 2008-06-25 at 09:08 -0700, Ingo Molnar wrote:
> * Alok Kataria <[email protected]> wrote:
>
> > This is a preparatory patch for the next patch in series.
> > Moves some code from e820_setup_gap to a new function e820_search_gap.
> > This patch is a part of a bug fix where we walk the ACPI table to calculate
> > a gap for PCI optional devices.
> >
> > v1->v2: Patch on top of tip/master.
> > Fixes a bug introduced in the last patch about the typeof "last".
> > Also the new function e820_search_gap now returns if we found a gap in
> > e820_map.
>
> applied to tip/x86/setup-memory - thanks Alok.

Hi Ingo,

As mentioned in this mail,
http://marc.info/?l=linux-kernel&m=121441312409978&w=2

I have also added a end_addr parameter to e820_search_gap to search only
till the CRS objects end_addr instead of 2^32. In the normal case via
e820_setup_gap we search till 2^32.

Please apply this too.
Patch on top of tip/master
--

e820_search_gap also take a end_addr parameter to limit search from
start_addr to end_addr.

Signed-off-by: AloK N Kataria <[email protected]>

Index: linux-x86-tree.git/arch/x86/kernel/e820.c
===================================================================
--- linux-x86-tree.git.orig/arch/x86/kernel/e820.c 2008-06-25 10:05:21.000000000 -0700
+++ linux-x86-tree.git/arch/x86/kernel/e820.c 2008-06-25 10:07:30.000000000 -0700
@@ -486,17 +486,19 @@
printk(KERN_INFO "modified physical RAM map:\n");
e820_print_map("modified");
}
-
+#define MAX_GAP_END 0x100000000ull
/*
- * Search for a gap in the e820 memory space from start_addr to 2^32.
+ * Search for a gap in the e820 memory space from start_addr to end_addr.
*/
__init int e820_search_gap(unsigned long *gapstart, unsigned long *gapsize,
- unsigned long start_addr)
+ unsigned long start_addr, unsigned long long end_addr)
{
- unsigned long long last = 0x100000000ull;
+ unsigned long long last;
int i = e820.nr_map;
int found = 0;

+ last = (end_addr && end_addr < MAX_GAP_END) ? end_addr : MAX_GAP_END;
+
while (--i >= 0) {
unsigned long long start = e820.map[i].addr;
unsigned long long end = start + e820.map[i].size;
@@ -536,7 +538,7 @@

gapstart = 0x10000000;
gapsize = 0x400000;
- found = e820_search_gap(&gapstart, &gapsize, 0);
+ found = e820_search_gap(&gapstart, &gapsize, 0, MAX_GAP_END);

#ifdef CONFIG_X86_64
if (!found) {
Index: linux-x86-tree.git/include/asm-x86/e820.h
===================================================================
--- linux-x86-tree.git.orig/include/asm-x86/e820.h 2008-06-25 10:05:21.000000000 -0700
+++ linux-x86-tree.git/include/asm-x86/e820.h 2008-06-25 10:06:17.000000000 -0700
@@ -72,7 +72,7 @@
extern void update_e820(void);
extern void e820_setup_gap(void);
extern int e820_search_gap(unsigned long *gapstart, unsigned long *gapsize,
- unsigned long start_addr);
+ unsigned long start_addr, unsigned long long end_addr);
struct setup_data;
extern void parse_e820_ext(struct setup_data *data, unsigned long pa_data);



2008-06-26 18:02:17

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH 1/2] cleanup e820_setup_gap v2

On Wed, Jun 25, 2008 at 11:02 AM, Alok Kataria <[email protected]> wrote:
> On Wed, 2008-06-25 at 09:08 -0700, Ingo Molnar wrote:
>> * Alok Kataria <[email protected]> wrote:
>>
>> > This is a preparatory patch for the next patch in series.
>> > Moves some code from e820_setup_gap to a new function e820_search_gap.
>> > This patch is a part of a bug fix where we walk the ACPI table to calculate
>> > a gap for PCI optional devices.
>> >
>> > v1->v2: Patch on top of tip/master.
>> > Fixes a bug introduced in the last patch about the typeof "last".
>> > Also the new function e820_search_gap now returns if we found a gap in
>> > e820_map.
>>
>> applied to tip/x86/setup-memory - thanks Alok.
>
> Hi Ingo,
>
> As mentioned in this mail,
> http://marc.info/?l=linux-kernel&m=121441312409978&w=2
>
> I have also added a end_addr parameter to e820_search_gap to search only
> till the CRS objects end_addr instead of 2^32. In the normal case via
> e820_setup_gap we search till 2^32.
>
> Please apply this too.
> Patch on top of tip/master
> --
>
> e820_search_gap also take a end_addr parameter to limit search from
> start_addr to end_addr.
>
> Signed-off-by: AloK N Kataria <[email protected]>

Acked-by: Yinghai Lu <[email protected]>

>
> Index: linux-x86-tree.git/arch/x86/kernel/e820.c
> ===================================================================
> --- linux-x86-tree.git.orig/arch/x86/kernel/e820.c 2008-06-25 10:05:21.000000000 -0700
> +++ linux-x86-tree.git/arch/x86/kernel/e820.c 2008-06-25 10:07:30.000000000 -0700
> @@ -486,17 +486,19 @@
> printk(KERN_INFO "modified physical RAM map:\n");
> e820_print_map("modified");
> }
> -
> +#define MAX_GAP_END 0x100000000ull
> /*
> - * Search for a gap in the e820 memory space from start_addr to 2^32.
> + * Search for a gap in the e820 memory space from start_addr to end_addr.
> */
> __init int e820_search_gap(unsigned long *gapstart, unsigned long *gapsize,
> - unsigned long start_addr)
> + unsigned long start_addr, unsigned long long end_addr)
> {
> - unsigned long long last = 0x100000000ull;
> + unsigned long long last;
> int i = e820.nr_map;
> int found = 0;
>
> + last = (end_addr && end_addr < MAX_GAP_END) ? end_addr : MAX_GAP_END;
> +
> while (--i >= 0) {
> unsigned long long start = e820.map[i].addr;
> unsigned long long end = start + e820.map[i].size;
> @@ -536,7 +538,7 @@
>
> gapstart = 0x10000000;
> gapsize = 0x400000;
> - found = e820_search_gap(&gapstart, &gapsize, 0);
> + found = e820_search_gap(&gapstart, &gapsize, 0, MAX_GAP_END);
>
> #ifdef CONFIG_X86_64
> if (!found) {
> Index: linux-x86-tree.git/include/asm-x86/e820.h
> ===================================================================
> --- linux-x86-tree.git.orig/include/asm-x86/e820.h 2008-06-25 10:05:21.000000000 -0700
> +++ linux-x86-tree.git/include/asm-x86/e820.h 2008-06-25 10:06:17.000000000 -0700
> @@ -72,7 +72,7 @@
> extern void update_e820(void);
> extern void e820_setup_gap(void);
> extern int e820_search_gap(unsigned long *gapstart, unsigned long *gapsize,
> - unsigned long start_addr);
> + unsigned long start_addr, unsigned long long end_addr);
> struct setup_data;
> extern void parse_e820_ext(struct setup_data *data, unsigned long pa_data);
>
>
>
>
>

2008-07-03 12:53:07

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 1/2] cleanup e820_setup_gap v2


* Yinghai Lu <[email protected]> wrote:

> > e820_search_gap also take a end_addr parameter to limit search from
> > start_addr to end_addr.
> >
> > Signed-off-by: AloK N Kataria <[email protected]>
>
> Acked-by: Yinghai Lu <[email protected]>

applied to tip/x86/unify-setup - thanks!

Ingo

2008-07-03 13:01:19

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 1/2] cleanup e820_setup_gap v2


* Ingo Molnar <[email protected]> wrote:

> > > e820_search_gap also take a end_addr parameter to limit search
> > > from start_addr to end_addr.
> > >
> > > Signed-off-by: AloK N Kataria <[email protected]>
> >
> > Acked-by: Yinghai Lu <[email protected]>
>
> applied to tip/x86/unify-setup - thanks!

hm, it doesnt work too well:

arch/x86/pci/acpi.c: In function 'search_gap':
arch/x86/pci/acpi.c:130: error: too few arguments to function 'e820_search_gap'

was i supposed to revert:

| commit 2c0262493239814b06a8aaabd1cf09b2f8fa3519
| Author: Alok Kataria <[email protected]>
| Date: Tue Jun 24 11:48:46 2008 -0700
|
| acpi based pci gap caluculation v2

?

Ingo

2008-07-03 17:10:27

by Alok Kataria

[permalink] [raw]
Subject: Re: [PATCH 1/2] cleanup e820_setup_gap v2

On Thu, 2008-07-03 at 06:00 -0700, Ingo Molnar wrote:
> * Ingo Molnar <[email protected]> wrote:
>
> > > > e820_search_gap also take a end_addr parameter to limit search
> > > > from start_addr to end_addr.
> > > >
> > > > Signed-off-by: AloK N Kataria <[email protected]>
> > >
> > > Acked-by: Yinghai Lu <[email protected]>
> >
> > applied to tip/x86/unify-setup - thanks!
>
> hm, it doesnt work too well:
>
> arch/x86/pci/acpi.c: In function 'search_gap':
> arch/x86/pci/acpi.c:130: error: too few arguments to function 'e820_search_gap'
>
> was i supposed to revert:
>
> | commit 2c0262493239814b06a8aaabd1cf09b2f8fa3519
> | Author: Alok Kataria <[email protected]>
> | Date: Tue Jun 24 11:48:46 2008 -0700
> |
> | acpi based pci gap caluculation v2
>
> ?

No...I had sent a incremental patch to the above patch..
http://marc.info/?l=linux-kernel&m=121441818818598&w=2

Please apply this one too.

Thanks,
Alok
>
> Ingo

2008-07-07 18:45:50

by Alok Kataria

[permalink] [raw]
Subject: Re: [PATCH 1/2] cleanup e820_setup_gap v2

On Thu, 2008-07-03 at 10:10 -0700, Alok Kataria wrote:
> On Thu, 2008-07-03 at 06:00 -0700, Ingo Molnar wrote:
> > * Ingo Molnar <[email protected]> wrote:
> >
> > > > > e820_search_gap also take a end_addr parameter to limit search
> > > > > from start_addr to end_addr.
> > > > >
> > > > > Signed-off-by: AloK N Kataria <[email protected]>
> > > >
> > > > Acked-by: Yinghai Lu <[email protected]>
> > >
> > > applied to tip/x86/unify-setup - thanks!
> >
> > hm, it doesnt work too well:
> >
> > arch/x86/pci/acpi.c: In function 'search_gap':
> > arch/x86/pci/acpi.c:130: error: too few arguments to function 'e820_search_gap'
> >
> > was i supposed to revert:
> >
> > | commit 2c0262493239814b06a8aaabd1cf09b2f8fa3519
> > | Author: Alok Kataria <[email protected]>
> > | Date: Tue Jun 24 11:48:46 2008 -0700
> > |
> > | acpi based pci gap caluculation v2
> >
> > ?
>
> No...I had sent a incremental patch to the above patch..
> http://marc.info/?l=linux-kernel&m=121441818818598&w=2
>
> Please apply this one too.
>

Hi Ingo,

Please let me know if you need a single patch instead of this
incremental patch.

Thanks,
Alok