[PATCH 1/2] cleanup e820_setup_gap v2

[PATCH 1/2] cleanup e820_setup_gap v2

[Alok Kataria]

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 <akataria@vmware.com>

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

Re: [PATCH 1/2] cleanup e820_setup_gap v2

[Ingo Molnar]

* Alok Kataria <akataria@vmware.com> 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

Re: [PATCH 1/2] cleanup e820_setup_gap v2

[Alok Kataria]

On Wed, 2008-06-25 at 09:08 -0700, Ingo Molnar wrote:
> * Alok Kataria <akataria@vmware.com> 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 <akataria@vmware.com>

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

Re: [PATCH 1/2] cleanup e820_setup_gap v2

[Yinghai Lu]

On Wed, Jun 25, 2008 at 11:02 AM, Alok Kataria <akataria@vmware.com> wrote:
> On Wed, 2008-06-25 at 09:08 -0700, Ingo Molnar wrote:
>> * Alok Kataria <akataria@vmware.com> 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 <akataria@vmware.com>

Acked-by: Yinghai Lu <yhlu.kernel@gmail.com>

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

Re: [PATCH 1/2] cleanup e820_setup_gap v2

[Ingo Molnar]

* Yinghai Lu <yhlu.kernel@gmail.com> 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 <akataria@vmware.com>
> 
> Acked-by: Yinghai Lu <yhlu.kernel@gmail.com>

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

	Ingo

Re: [PATCH 1/2] cleanup e820_setup_gap v2

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> 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 <akataria@vmware.com>
> > 
> > Acked-by: Yinghai Lu <yhlu.kernel@gmail.com>
> 
> 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 <akataria@vmware.com>
| Date:   Tue Jun 24 11:48:46 2008 -0700
|
|    acpi based pci gap caluculation v2

?

	Ingo

Re: [PATCH 1/2] cleanup e820_setup_gap v2

[Alok Kataria]

On Thu, 2008-07-03 at 06:00 -0700, Ingo Molnar wrote:
> * Ingo Molnar <mingo@elte.hu> 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 <akataria@vmware.com>
> > >
> > > Acked-by: Yinghai Lu <yhlu.kernel@gmail.com>
> >
> > 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 <akataria@vmware.com>
> | 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

Re: [PATCH 1/2] cleanup e820_setup_gap v2

[Alok Kataria]

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 <mingo@elte.hu> 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 <akataria@vmware.com>
> > > >
> > > > Acked-by: Yinghai Lu <yhlu.kernel@gmail.com>
> > >
> > > 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 <akataria@vmware.com>
> > | 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