linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86: selective remapping in parse_setup_data()
@ 2013-08-12 21:00 Linn Crosetto
  2013-08-12 21:08 ` H. Peter Anvin
  0 siblings, 1 reply; 13+ messages in thread
From: Linn Crosetto @ 2013-08-12 21:00 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86, yinghai, penberg, jacob.shin
  Cc: linux-kernel, Linn Crosetto

PCI devices may advertise a ROM size larger than early_memremap() is
able to handle. If this occurs it leads to a NULL dereference in
parse_setup_data(). Avoid this by remapping the data only when
necessary.

Signed-off-by: Linn Crosetto <linn@hp.com>
---
 arch/x86/kernel/setup.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f8ec578..2063a49 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -423,6 +423,17 @@ static void __init reserve_initrd(void)
 }
 #endif /* CONFIG_BLK_DEV_INITRD */
 
+static void __init remap_setup_data(u64 pa_data, u32 data_len,
+				    struct setup_data **data, u32 *map_len)
+{
+	if (data_len <= *map_len)
+		return;
+
+	early_iounmap(*data, *map_len);
+	*data = early_memremap(pa_data, data_len);
+	*map_len = data_len;
+}
+
 static void __init parse_setup_data(void)
 {
 	struct setup_data *data;
@@ -432,18 +443,16 @@ static void __init parse_setup_data(void)
 	while (pa_data) {
 		u32 data_len, map_len;
 
+		/* The data length may be too large for early remapping,
+		   remap the data only when needed. */
 		map_len = max(PAGE_SIZE - (pa_data & ~PAGE_MASK),
 			      (u64)sizeof(struct setup_data));
 		data = early_memremap(pa_data, map_len);
 		data_len = data->len + sizeof(struct setup_data);
-		if (data_len > map_len) {
-			early_iounmap(data, map_len);
-			data = early_memremap(pa_data, data_len);
-			map_len = data_len;
-		}
 
 		switch (data->type) {
 		case SETUP_E820_EXT:
+			remap_setup_data(pa_data, data_len, &data, &map_len);
 			parse_e820_ext(data);
 			break;
 		case SETUP_DTB:
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] x86: selective remapping in parse_setup_data()
  2013-08-12 21:00 [PATCH] x86: selective remapping in parse_setup_data() Linn Crosetto
@ 2013-08-12 21:08 ` H. Peter Anvin
  2013-08-12 23:01   ` [PATCH v2] " Linn Crosetto
  0 siblings, 1 reply; 13+ messages in thread
From: H. Peter Anvin @ 2013-08-12 21:08 UTC (permalink / raw)
  To: Linn Crosetto
  Cc: tglx, mingo, x86, yinghai, penberg, jacob.shin, linux-kernel

On 08/12/2013 02:00 PM, Linn Crosetto wrote:
> PCI devices may advertise a ROM size larger than early_memremap() is
> able to handle. If this occurs it leads to a NULL dereference in
> parse_setup_data(). Avoid this by remapping the data only when
> necessary.
> 
> Signed-off-by: Linn Crosetto <linn@hp.com>

I think you need to clarify the various subcases (the code looks right,
but the explanation needs to be improved.)

	-hpa


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2] x86: selective remapping in parse_setup_data()
  2013-08-12 21:08 ` H. Peter Anvin
@ 2013-08-12 23:01   ` Linn Crosetto
  2013-08-12 23:30     ` Yinghai Lu
  2013-08-13 21:46     ` [PATCH v3] x86: avoid remapping data " Linn Crosetto
  0 siblings, 2 replies; 13+ messages in thread
From: Linn Crosetto @ 2013-08-12 23:01 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86, yinghai, penberg, jacob.shin
  Cc: linux-kernel, Linn Crosetto

Type SETUP_PCI, added by setup_efi_pci(), may advertise a ROM size
larger than early_memremap() is able to handle, which is currently
limited to 256kB. If this occurs it leads to a NULL dereference in
parse_setup_data().

To avoid this, first remap the setup_data header, and remap the data
only for types actually parsed in parse_setup_data(). Type SETUP_PCI is
handled later by pcibios_add_device(), when ioremap() is available.

Signed-off-by: Linn Crosetto <linn@hp.com>
---
v2: add more detail to the explanation as requested by hpa
---
 arch/x86/kernel/setup.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f8ec578..2063a49 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -423,6 +423,17 @@ static void __init reserve_initrd(void)
 }
 #endif /* CONFIG_BLK_DEV_INITRD */
 
+static void __init remap_setup_data(u64 pa_data, u32 data_len,
+				    struct setup_data **data, u32 *map_len)
+{
+	if (data_len <= *map_len)
+		return;
+
+	early_iounmap(*data, *map_len);
+	*data = early_memremap(pa_data, data_len);
+	*map_len = data_len;
+}
+
 static void __init parse_setup_data(void)
 {
 	struct setup_data *data;
@@ -432,18 +443,16 @@ static void __init parse_setup_data(void)
 	while (pa_data) {
 		u32 data_len, map_len;
 
+		/* The data length may be too large for early remapping,
+		   remap the data only when needed. */
 		map_len = max(PAGE_SIZE - (pa_data & ~PAGE_MASK),
 			      (u64)sizeof(struct setup_data));
 		data = early_memremap(pa_data, map_len);
 		data_len = data->len + sizeof(struct setup_data);
-		if (data_len > map_len) {
-			early_iounmap(data, map_len);
-			data = early_memremap(pa_data, data_len);
-			map_len = data_len;
-		}
 
 		switch (data->type) {
 		case SETUP_E820_EXT:
+			remap_setup_data(pa_data, data_len, &data, &map_len);
 			parse_e820_ext(data);
 			break;
 		case SETUP_DTB:
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] x86: selective remapping in parse_setup_data()
  2013-08-12 23:01   ` [PATCH v2] " Linn Crosetto
@ 2013-08-12 23:30     ` Yinghai Lu
  2013-08-13 21:46     ` [PATCH v3] x86: avoid remapping data " Linn Crosetto
  1 sibling, 0 replies; 13+ messages in thread
From: Yinghai Lu @ 2013-08-12 23:30 UTC (permalink / raw)
  To: Linn Crosetto
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	the arch/x86 maintainers, Pekka Enberg, Jacob Shin,
	Linux Kernel Mailing List

On Mon, Aug 12, 2013 at 4:01 PM, Linn Crosetto <linn@hp.com> wrote:
> Type SETUP_PCI, added by setup_efi_pci(), may advertise a ROM size
> larger than early_memremap() is able to handle, which is currently
> limited to 256kB. If this occurs it leads to a NULL dereference in
> parse_setup_data().
>
> To avoid this, first remap the setup_data header, and remap the data
> only for types actually parsed in parse_setup_data(). Type SETUP_PCI is
> handled later by pcibios_add_device(), when ioremap() is available.
>
> Signed-off-by: Linn Crosetto <linn@hp.com>
> ---
> v2: add more detail to the explanation as requested by hpa
> ---
>  arch/x86/kernel/setup.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index f8ec578..2063a49 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -423,6 +423,17 @@ static void __init reserve_initrd(void)
>  }
>  #endif /* CONFIG_BLK_DEV_INITRD */
>
> +static void __init remap_setup_data(u64 pa_data, u32 data_len,
> +                                   struct setup_data **data, u32 *map_len)
> +{
> +       if (data_len <= *map_len)
> +               return;
> +
> +       early_iounmap(*data, *map_len);
> +       *data = early_memremap(pa_data, data_len);
> +       *map_len = data_len;
> +}
> +
>  static void __init parse_setup_data(void)
>  {
>         struct setup_data *data;
> @@ -432,18 +443,16 @@ static void __init parse_setup_data(void)
>         while (pa_data) {
>                 u32 data_len, map_len;
>
> +               /* The data length may be too large for early remapping,
> +                  remap the data only when needed. */
>                 map_len = max(PAGE_SIZE - (pa_data & ~PAGE_MASK),
>                               (u64)sizeof(struct setup_data));
>                 data = early_memremap(pa_data, map_len);
>                 data_len = data->len + sizeof(struct setup_data);
> -               if (data_len > map_len) {
> -                       early_iounmap(data, map_len);
> -                       data = early_memremap(pa_data, data_len);
> -                       map_len = data_len;
> -               }
>
>                 switch (data->type) {
>                 case SETUP_E820_EXT:
> +                       remap_setup_data(pa_data, data_len, &data, &map_len);
>                         parse_e820_ext(data);
>                         break;
>                 case SETUP_DTB:

how about just passing phys_addr and length with parse_e820_ext and parse_dtb?
let them to worry about length and handle oversize case.

Thanks

Yinghai

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3] x86: avoid remapping data in parse_setup_data()
  2013-08-12 23:01   ` [PATCH v2] " Linn Crosetto
  2013-08-12 23:30     ` Yinghai Lu
@ 2013-08-13 21:46     ` Linn Crosetto
  2013-08-13 22:22       ` Yinghai Lu
                         ` (3 more replies)
  1 sibling, 4 replies; 13+ messages in thread
From: Linn Crosetto @ 2013-08-13 21:46 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86, paulmck, dhowells, mtk.manpages, yinghai,
	penberg, jacob.shin
  Cc: linux-kernel, Linn Crosetto

Type SETUP_PCI, added by setup_efi_pci(), may advertise a ROM size
larger than early_memremap() is able to handle, which is currently
limited to 256kB. If this occurs it leads to a NULL dereference in
parse_setup_data().

To avoid this, remap the setup_data header and allow parsing functions
for individual types to handle their own data remapping.

Signed-off-by: Linn Crosetto <linn@hp.com>
---
v3: Remove data remapping code from parse_setup_data() and add it to
parse_e820_ext().

 arch/x86/include/asm/e820.h |  2 +-
 arch/x86/kernel/e820.c      |  5 ++++-
 arch/x86/kernel/setup.c     | 19 ++++++++-----------
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/e820.h b/arch/x86/include/asm/e820.h
index cccd07f..779c2ef 100644
--- a/arch/x86/include/asm/e820.h
+++ b/arch/x86/include/asm/e820.h
@@ -29,7 +29,7 @@ extern void e820_setup_gap(void);
 extern int e820_search_gap(unsigned long *gapstart, unsigned long *gapsize,
 			unsigned long start_addr, unsigned long long end_addr);
 struct setup_data;
-extern void parse_e820_ext(struct setup_data *data);
+extern void parse_e820_ext(u64 phys_addr, u32 data_len);
 
 #if defined(CONFIG_X86_64) || \
 	(defined(CONFIG_X86_32) && defined(CONFIG_HIBERNATION))
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index d32abea..174da5f 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -658,15 +658,18 @@ __init void e820_setup_gap(void)
  * boot_params.e820_map, others are passed via SETUP_E820_EXT node of
  * linked list of struct setup_data, which is parsed here.
  */
-void __init parse_e820_ext(struct setup_data *sdata)
+void __init parse_e820_ext(u64 phys_addr, u32 data_len)
 {
 	int entries;
 	struct e820entry *extmap;
+	struct setup_data *sdata;
 
+	sdata = early_memremap(phys_addr, data_len);
 	entries = sdata->len / sizeof(struct e820entry);
 	extmap = (struct e820entry *)(sdata->data);
 	__append_e820_map(extmap, entries);
 	sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
+	early_iounmap(sdata, data_len);
 	printk(KERN_INFO "e820: extended physical RAM map:\n");
 	e820_print_map("extended");
 }
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f8ec578..234e1e3 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -426,25 +426,23 @@ static void __init reserve_initrd(void)
 static void __init parse_setup_data(void)
 {
 	struct setup_data *data;
-	u64 pa_data;
+	u64 pa_data, pa_next;
 
 	pa_data = boot_params.hdr.setup_data;
 	while (pa_data) {
-		u32 data_len, map_len;
+		u32 data_len, map_len, data_type;
 
 		map_len = max(PAGE_SIZE - (pa_data & ~PAGE_MASK),
 			      (u64)sizeof(struct setup_data));
 		data = early_memremap(pa_data, map_len);
 		data_len = data->len + sizeof(struct setup_data);
-		if (data_len > map_len) {
-			early_iounmap(data, map_len);
-			data = early_memremap(pa_data, data_len);
-			map_len = data_len;
-		}
+		data_type = data->type;
+		pa_next = data->next;
+		early_iounmap(data, map_len);
 
-		switch (data->type) {
+		switch (data_type) {
 		case SETUP_E820_EXT:
-			parse_e820_ext(data);
+			parse_e820_ext(pa_data, data_len);
 			break;
 		case SETUP_DTB:
 			add_dtb(pa_data);
@@ -452,8 +450,7 @@ static void __init parse_setup_data(void)
 		default:
 			break;
 		}
-		pa_data = data->next;
-		early_iounmap(data, map_len);
+		pa_data = pa_next;
 	}
 }
 
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] x86: avoid remapping data in parse_setup_data()
  2013-08-13 21:46     ` [PATCH v3] x86: avoid remapping data " Linn Crosetto
@ 2013-08-13 22:22       ` Yinghai Lu
  2013-08-14  6:26       ` Pekka Enberg
                         ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Yinghai Lu @ 2013-08-13 22:22 UTC (permalink / raw)
  To: Linn Crosetto
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	the arch/x86 maintainers, Paul McKenney, David Howells,
	Michael Kerrisk-manpages, Pekka Enberg, Jacob Shin,
	Linux Kernel Mailing List

On Tue, Aug 13, 2013 at 2:46 PM, Linn Crosetto <linn@hp.com> wrote:
> Type SETUP_PCI, added by setup_efi_pci(), may advertise a ROM size
> larger than early_memremap() is able to handle, which is currently
> limited to 256kB. If this occurs it leads to a NULL dereference in
> parse_setup_data().
>
> To avoid this, remap the setup_data header and allow parsing functions
> for individual types to handle their own data remapping.
>
> Signed-off-by: Linn Crosetto <linn@hp.com>
> ---
> v3: Remove data remapping code from parse_setup_data() and add it to
> parse_e820_ext().

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

>
>  arch/x86/include/asm/e820.h |  2 +-
>  arch/x86/kernel/e820.c      |  5 ++++-
>  arch/x86/kernel/setup.c     | 19 ++++++++-----------
>  3 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/include/asm/e820.h b/arch/x86/include/asm/e820.h
> index cccd07f..779c2ef 100644
> --- a/arch/x86/include/asm/e820.h
> +++ b/arch/x86/include/asm/e820.h
> @@ -29,7 +29,7 @@ extern void e820_setup_gap(void);
>  extern int e820_search_gap(unsigned long *gapstart, unsigned long *gapsize,
>                         unsigned long start_addr, unsigned long long end_addr);
>  struct setup_data;
> -extern void parse_e820_ext(struct setup_data *data);
> +extern void parse_e820_ext(u64 phys_addr, u32 data_len);
>
>  #if defined(CONFIG_X86_64) || \
>         (defined(CONFIG_X86_32) && defined(CONFIG_HIBERNATION))
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index d32abea..174da5f 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -658,15 +658,18 @@ __init void e820_setup_gap(void)
>   * boot_params.e820_map, others are passed via SETUP_E820_EXT node of
>   * linked list of struct setup_data, which is parsed here.
>   */
> -void __init parse_e820_ext(struct setup_data *sdata)
> +void __init parse_e820_ext(u64 phys_addr, u32 data_len)
>  {
>         int entries;
>         struct e820entry *extmap;
> +       struct setup_data *sdata;
>
> +       sdata = early_memremap(phys_addr, data_len);
>         entries = sdata->len / sizeof(struct e820entry);
>         extmap = (struct e820entry *)(sdata->data);
>         __append_e820_map(extmap, entries);
>         sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
> +       early_iounmap(sdata, data_len);
>         printk(KERN_INFO "e820: extended physical RAM map:\n");
>         e820_print_map("extended");
>  }
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index f8ec578..234e1e3 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -426,25 +426,23 @@ static void __init reserve_initrd(void)
>  static void __init parse_setup_data(void)
>  {
>         struct setup_data *data;
> -       u64 pa_data;
> +       u64 pa_data, pa_next;
>
>         pa_data = boot_params.hdr.setup_data;
>         while (pa_data) {
> -               u32 data_len, map_len;
> +               u32 data_len, map_len, data_type;
>
>                 map_len = max(PAGE_SIZE - (pa_data & ~PAGE_MASK),
>                               (u64)sizeof(struct setup_data));
>                 data = early_memremap(pa_data, map_len);
>                 data_len = data->len + sizeof(struct setup_data);
> -               if (data_len > map_len) {
> -                       early_iounmap(data, map_len);
> -                       data = early_memremap(pa_data, data_len);
> -                       map_len = data_len;
> -               }
> +               data_type = data->type;
> +               pa_next = data->next;
> +               early_iounmap(data, map_len);
>
> -               switch (data->type) {
> +               switch (data_type) {
>                 case SETUP_E820_EXT:
> -                       parse_e820_ext(data);
> +                       parse_e820_ext(pa_data, data_len);
>                         break;
>                 case SETUP_DTB:
>                         add_dtb(pa_data);
> @@ -452,8 +450,7 @@ static void __init parse_setup_data(void)
>                 default:
>                         break;
>                 }
> -               pa_data = data->next;
> -               early_iounmap(data, map_len);
> +               pa_data = pa_next;
>         }
>  }

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] x86: avoid remapping data in parse_setup_data()
  2013-08-13 21:46     ` [PATCH v3] x86: avoid remapping data " Linn Crosetto
  2013-08-13 22:22       ` Yinghai Lu
@ 2013-08-14  6:26       ` Pekka Enberg
  2013-09-01 11:40       ` [tip:x86/mm] " tip-bot for Linn Crosetto
  2013-09-01 23:15       ` [PATCH v3] " H. Peter Anvin
  3 siblings, 0 replies; 13+ messages in thread
From: Pekka Enberg @ 2013-08-14  6:26 UTC (permalink / raw)
  To: Linn Crosetto
  Cc: tglx, mingo, hpa, x86, paulmck, dhowells, mtk.manpages, yinghai,
	penberg, jacob.shin, linux-kernel

On 8/14/13 12:46 AM, Linn Crosetto wrote:
> Type SETUP_PCI, added by setup_efi_pci(), may advertise a ROM size
> larger than early_memremap() is able to handle, which is currently
> limited to 256kB. If this occurs it leads to a NULL dereference in
> parse_setup_data().
>
> To avoid this, remap the setup_data header and allow parsing functions
> for individual types to handle their own data remapping.
>
> Signed-off-by: Linn Crosetto <linn@hp.com>

Reviewed-by: Pekka Enberg <penberg@kernel.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [tip:x86/mm] x86: avoid remapping data in parse_setup_data()
  2013-08-13 21:46     ` [PATCH v3] x86: avoid remapping data " Linn Crosetto
  2013-08-13 22:22       ` Yinghai Lu
  2013-08-14  6:26       ` Pekka Enberg
@ 2013-09-01 11:40       ` tip-bot for Linn Crosetto
  2013-09-01 23:15       ` [PATCH v3] " H. Peter Anvin
  3 siblings, 0 replies; 13+ messages in thread
From: tip-bot for Linn Crosetto @ 2013-09-01 11:40 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, yinghai, penberg, linn, tglx, hpa

Commit-ID:  30e46b574a1db7d14404e52dca8e1aa5f5155fd2
Gitweb:     http://git.kernel.org/tip/30e46b574a1db7d14404e52dca8e1aa5f5155fd2
Author:     Linn Crosetto <linn@hp.com>
AuthorDate: Tue, 13 Aug 2013 15:46:41 -0600
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Tue, 13 Aug 2013 23:29:19 -0700

x86: avoid remapping data in parse_setup_data()

Type SETUP_PCI, added by setup_efi_pci(), may advertise a ROM size
larger than early_memremap() is able to handle, which is currently
limited to 256kB. If this occurs it leads to a NULL dereference in
parse_setup_data().

To avoid this, remap the setup_data header and allow parsing functions
for individual types to handle their own data remapping.

Signed-off-by: Linn Crosetto <linn@hp.com>
Link: http://lkml.kernel.org/r/1376430401-67445-1-git-send-email-linn@hp.com
Acked-by: Yinghai Lu <yinghai@kernel.org>
Reviewed-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/include/asm/e820.h |  2 +-
 arch/x86/kernel/e820.c      |  5 ++++-
 arch/x86/kernel/setup.c     | 19 ++++++++-----------
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/e820.h b/arch/x86/include/asm/e820.h
index cccd07f..779c2ef 100644
--- a/arch/x86/include/asm/e820.h
+++ b/arch/x86/include/asm/e820.h
@@ -29,7 +29,7 @@ extern void e820_setup_gap(void);
 extern int e820_search_gap(unsigned long *gapstart, unsigned long *gapsize,
 			unsigned long start_addr, unsigned long long end_addr);
 struct setup_data;
-extern void parse_e820_ext(struct setup_data *data);
+extern void parse_e820_ext(u64 phys_addr, u32 data_len);
 
 #if defined(CONFIG_X86_64) || \
 	(defined(CONFIG_X86_32) && defined(CONFIG_HIBERNATION))
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index d32abea..174da5f 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -658,15 +658,18 @@ __init void e820_setup_gap(void)
  * boot_params.e820_map, others are passed via SETUP_E820_EXT node of
  * linked list of struct setup_data, which is parsed here.
  */
-void __init parse_e820_ext(struct setup_data *sdata)
+void __init parse_e820_ext(u64 phys_addr, u32 data_len)
 {
 	int entries;
 	struct e820entry *extmap;
+	struct setup_data *sdata;
 
+	sdata = early_memremap(phys_addr, data_len);
 	entries = sdata->len / sizeof(struct e820entry);
 	extmap = (struct e820entry *)(sdata->data);
 	__append_e820_map(extmap, entries);
 	sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
+	early_iounmap(sdata, data_len);
 	printk(KERN_INFO "e820: extended physical RAM map:\n");
 	e820_print_map("extended");
 }
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index de33798..b6b45e4 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -426,25 +426,23 @@ static void __init reserve_initrd(void)
 static void __init parse_setup_data(void)
 {
 	struct setup_data *data;
-	u64 pa_data;
+	u64 pa_data, pa_next;
 
 	pa_data = boot_params.hdr.setup_data;
 	while (pa_data) {
-		u32 data_len, map_len;
+		u32 data_len, map_len, data_type;
 
 		map_len = max(PAGE_SIZE - (pa_data & ~PAGE_MASK),
 			      (u64)sizeof(struct setup_data));
 		data = early_memremap(pa_data, map_len);
 		data_len = data->len + sizeof(struct setup_data);
-		if (data_len > map_len) {
-			early_iounmap(data, map_len);
-			data = early_memremap(pa_data, data_len);
-			map_len = data_len;
-		}
+		data_type = data->type;
+		pa_next = data->next;
+		early_iounmap(data, map_len);
 
-		switch (data->type) {
+		switch (data_type) {
 		case SETUP_E820_EXT:
-			parse_e820_ext(data);
+			parse_e820_ext(pa_data, data_len);
 			break;
 		case SETUP_DTB:
 			add_dtb(pa_data);
@@ -452,8 +450,7 @@ static void __init parse_setup_data(void)
 		default:
 			break;
 		}
-		pa_data = data->next;
-		early_iounmap(data, map_len);
+		pa_data = pa_next;
 	}
 }
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] x86: avoid remapping data in parse_setup_data()
  2013-08-13 21:46     ` [PATCH v3] x86: avoid remapping data " Linn Crosetto
                         ` (2 preceding siblings ...)
  2013-09-01 11:40       ` [tip:x86/mm] " tip-bot for Linn Crosetto
@ 2013-09-01 23:15       ` H. Peter Anvin
  2013-09-06 18:06         ` [PATCH 0/3] x86/mm: fix early_memremap() sparse warnings Linn Crosetto
  3 siblings, 1 reply; 13+ messages in thread
From: H. Peter Anvin @ 2013-09-01 23:15 UTC (permalink / raw)
  To: Linn Crosetto
  Cc: tglx, mingo, x86, paulmck, dhowells, mtk.manpages, yinghai,
	penberg, jacob.shin, linux-kernel

On 08/13/2013 02:46 PM, Linn Crosetto wrote:
> Type SETUP_PCI, added by setup_efi_pci(), may advertise a ROM size
> larger than early_memremap() is able to handle, which is currently
> limited to 256kB. If this occurs it leads to a NULL dereference in
> parse_setup_data().
> 
> To avoid this, remap the setup_data header and allow parsing functions
> for individual types to handle their own data remapping.
> 
> Signed-off-by: Linn Crosetto <linn@hp.com>

So it seems this patch generates some new sparse warnings.  Can you
please create an incremental fixup patch to address those sparse
warnings?  (See email from Fengguang Wu's robot.)

	-hpa



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 0/3] x86/mm: fix early_memremap() sparse warnings
  2013-09-01 23:15       ` [PATCH v3] " H. Peter Anvin
@ 2013-09-06 18:06         ` Linn Crosetto
  2013-09-06 18:06           ` [PATCH 1/3] x86/mm: fix sparse warnings from early_memremap() Linn Crosetto
                             ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Linn Crosetto @ 2013-09-06 18:06 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86, luto, daniel.vetter, airlied, yinghai,
	jacob.shin, penberg, js1304, akpm, matt.fleming
  Cc: linux-kernel, Linn Crosetto

This set of patches cleans up some sparse warnings generated by callers of
early_memremap(). 

early_memremap() was created as an interface to to map normal memory (commit
1494177), in contrast to early_ioremap() for IO mappings. Later on,
early_memremap() was annotated with __iomem (commit 1d6cf1f) which generates
sparse warnings for callers using pointers not declared with __iomem. Callers
of early_memremap() were expected to use early_iounmap() to remove the mapping,
which generates more sparse warnings as the argument to early_iounmap() is also
annotated with __iomem.

To clean this up, remove __iomem from early_memremap() and create
early_memunmap() to be used for removing normal memory mappings.

Removes the following warnings:

arch/x86/kernel/setup.c:353:19: warning: incorrect type in assignment (different address spaces)
arch/x86/kernel/setup.c:355:31: warning: incorrect type in argument 1 (different address spaces)
arch/x86/kernel/setup.c:437:22: warning: incorrect type in assignment (different address spaces)
arch/x86/kernel/setup.c:441:31: warning: incorrect type in argument 1 (different address spaces)
arch/x86/kernel/setup.c:465:22: warning: incorrect type in assignment (different address spaces)
arch/x86/kernel/setup.c:470:31: warning: incorrect type in argument 1 (different address spaces)
arch/x86/kernel/setup.c:488:22: warning: incorrect type in assignment (different address spaces)
arch/x86/kernel/setup.c:491:31: warning: incorrect type in argument 1 (different address spaces)
arch/x86/kernel/e820.c:667:15: warning: incorrect type in assignment (different address spaces)
arch/x86/kernel/e820.c:672:23: warning: incorrect type in argument 1 (different address spaces)

Linn Crosetto (3):
  x86/mm: fix sparse warnings from early_memremap()
  x86: fix sparse warning in parse_e820_ext()
  x86: fix early_iounmap() sparse warnings in setup.c

 arch/x86/include/asm/io.h | 4 ++--
 arch/x86/kernel/e820.c    | 2 +-
 arch/x86/kernel/setup.c   | 8 ++++----
 arch/x86/mm/ioremap.c     | 9 +++++++--
 4 files changed, 14 insertions(+), 9 deletions(-)

-- 
1.7.11.3


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/3] x86/mm: fix sparse warnings from early_memremap()
  2013-09-06 18:06         ` [PATCH 0/3] x86/mm: fix early_memremap() sparse warnings Linn Crosetto
@ 2013-09-06 18:06           ` Linn Crosetto
  2013-09-06 18:06           ` [PATCH 2/3] x86: fix sparse warning in parse_e820_ext() Linn Crosetto
  2013-09-06 18:06           ` [PATCH 3/3] x86: fix early_iounmap() sparse warnings in setup.c Linn Crosetto
  2 siblings, 0 replies; 13+ messages in thread
From: Linn Crosetto @ 2013-09-06 18:06 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86, luto, daniel.vetter, airlied, yinghai,
	jacob.shin, penberg, js1304, akpm, matt.fleming
  Cc: linux-kernel, Linn Crosetto

This patch creates consistent early interfaces for mapping normal memory
and eliminates some sparse warnings.

early_memremap() was created to map normal memory, as opposed to the
ioremap interfaces for actual IO mappings, so remove the __iomem
annotation from early_memremap(). In addition, early_memunmap() is added
to provide an interface analogous to early_iounmap() for normal memory.

Fixes the following warnings:

arch/x86/kernel/setup.c:353:19: warning: incorrect type in assignment (different address spaces)
arch/x86/kernel/setup.c:437:22: warning: incorrect type in assignment (different address spaces)
arch/x86/kernel/setup.c:465:22: warning: incorrect type in assignment (different address spaces)
arch/x86/kernel/setup.c:488:22: warning: incorrect type in assignment (different address spaces)
arch/x86/kernel/e820.c:667:15: warning: incorrect type in assignment (different address spaces)

Signed-off-by: Linn Crosetto <linn@hp.com>
---
 arch/x86/include/asm/io.h | 4 ++--
 arch/x86/mm/ioremap.c     | 9 +++++++--
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 34f69cb..ae1ef3e 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -325,8 +325,8 @@ extern void early_ioremap_init(void);
 extern void early_ioremap_reset(void);
 extern void __iomem *early_ioremap(resource_size_t phys_addr,
 				   unsigned long size);
-extern void __iomem *early_memremap(resource_size_t phys_addr,
-				    unsigned long size);
+extern void *early_memremap(resource_size_t phys_addr, unsigned long size);
+extern void early_memunmap(void *addr, unsigned long size);
 extern void early_iounmap(void __iomem *addr, unsigned long size);
 extern void fixup_early_ioremap(void);
 extern bool is_early_ioremap_ptep(pte_t *ptep);
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 799580c..a144929 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -562,10 +562,15 @@ early_ioremap(resource_size_t phys_addr, unsigned long size)
 }
 
 /* Remap memory */
-void __init __iomem *
+void __init *
 early_memremap(resource_size_t phys_addr, unsigned long size)
 {
-	return __early_ioremap(phys_addr, size, PAGE_KERNEL);
+	return (__force void *)__early_ioremap(phys_addr, size, PAGE_KERNEL);
+}
+
+void __init early_memunmap(void *addr, unsigned long size)
+{
+	early_iounmap((void __iomem *)addr, size);
 }
 
 void __init early_iounmap(void __iomem *addr, unsigned long size)
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/3] x86: fix sparse warning in parse_e820_ext()
  2013-09-06 18:06         ` [PATCH 0/3] x86/mm: fix early_memremap() sparse warnings Linn Crosetto
  2013-09-06 18:06           ` [PATCH 1/3] x86/mm: fix sparse warnings from early_memremap() Linn Crosetto
@ 2013-09-06 18:06           ` Linn Crosetto
  2013-09-06 18:06           ` [PATCH 3/3] x86: fix early_iounmap() sparse warnings in setup.c Linn Crosetto
  2 siblings, 0 replies; 13+ messages in thread
From: Linn Crosetto @ 2013-09-06 18:06 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86, luto, daniel.vetter, airlied, yinghai,
	jacob.shin, penberg, js1304, akpm, matt.fleming
  Cc: linux-kernel, Linn Crosetto

Replace the call to early_iounmap() with early_memunmap() to eliminate
the following sparse warning:

arch/x86/kernel/e820.c:672:23: sparse: incorrect type in argument 1 (different address spaces)

Signed-off-by: Linn Crosetto <linn@hp.com>
---
 arch/x86/kernel/e820.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 174da5f..06e0c4a 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -669,7 +669,7 @@ void __init parse_e820_ext(u64 phys_addr, u32 data_len)
 	extmap = (struct e820entry *)(sdata->data);
 	__append_e820_map(extmap, entries);
 	sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
-	early_iounmap(sdata, data_len);
+	early_memunmap(sdata, data_len);
 	printk(KERN_INFO "e820: extended physical RAM map:\n");
 	e820_print_map("extended");
 }
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/3] x86: fix early_iounmap() sparse warnings in setup.c
  2013-09-06 18:06         ` [PATCH 0/3] x86/mm: fix early_memremap() sparse warnings Linn Crosetto
  2013-09-06 18:06           ` [PATCH 1/3] x86/mm: fix sparse warnings from early_memremap() Linn Crosetto
  2013-09-06 18:06           ` [PATCH 2/3] x86: fix sparse warning in parse_e820_ext() Linn Crosetto
@ 2013-09-06 18:06           ` Linn Crosetto
  2 siblings, 0 replies; 13+ messages in thread
From: Linn Crosetto @ 2013-09-06 18:06 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86, luto, daniel.vetter, airlied, yinghai,
	jacob.shin, penberg, js1304, akpm, matt.fleming
  Cc: linux-kernel, Linn Crosetto

Replace calls to early_iounmap() with early_memunmap() for pointers
mapped with early_memremap() to eliminate the following sparse warnings:

arch/x86/kernel/setup.c:355:31: warning: incorrect type in argument 1 (different address spaces)
arch/x86/kernel/setup.c:441:31: warning: incorrect type in argument 1 (different address spaces)
arch/x86/kernel/setup.c:470:31: warning: incorrect type in argument 1 (different address spaces)
arch/x86/kernel/setup.c:491:31: warning: incorrect type in argument 1 (different address spaces)

Signed-off-by: Linn Crosetto <linn@hp.com>
---
 arch/x86/kernel/setup.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f0de629..3081fc6 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -352,7 +352,7 @@ static void __init relocate_initrd(void)
 		mapaddr = ramdisk_image & PAGE_MASK;
 		p = early_memremap(mapaddr, clen+slop);
 		memcpy(q, p+slop, clen);
-		early_iounmap(p, clen+slop);
+		early_memunmap(p, clen+slop);
 		q += clen;
 		ramdisk_image += clen;
 		ramdisk_size  -= clen;
@@ -438,7 +438,7 @@ static void __init parse_setup_data(void)
 		data_len = data->len + sizeof(struct setup_data);
 		data_type = data->type;
 		pa_next = data->next;
-		early_iounmap(data, map_len);
+		early_memunmap(data, map_len);
 
 		switch (data_type) {
 		case SETUP_E820_EXT:
@@ -467,7 +467,7 @@ static void __init e820_reserve_setup_data(void)
 			 E820_RAM, E820_RESERVED_KERN);
 		found = 1;
 		pa_data = data->next;
-		early_iounmap(data, sizeof(*data));
+		early_memunmap(data, sizeof(*data));
 	}
 	if (!found)
 		return;
@@ -488,7 +488,7 @@ static void __init memblock_x86_reserve_range_setup_data(void)
 		data = early_memremap(pa_data, sizeof(*data));
 		memblock_reserve(pa_data, sizeof(*data) + data->len);
 		pa_data = data->next;
-		early_iounmap(data, sizeof(*data));
+		early_memunmap(data, sizeof(*data));
 	}
 }
 
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2013-09-06 18:10 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-12 21:00 [PATCH] x86: selective remapping in parse_setup_data() Linn Crosetto
2013-08-12 21:08 ` H. Peter Anvin
2013-08-12 23:01   ` [PATCH v2] " Linn Crosetto
2013-08-12 23:30     ` Yinghai Lu
2013-08-13 21:46     ` [PATCH v3] x86: avoid remapping data " Linn Crosetto
2013-08-13 22:22       ` Yinghai Lu
2013-08-14  6:26       ` Pekka Enberg
2013-09-01 11:40       ` [tip:x86/mm] " tip-bot for Linn Crosetto
2013-09-01 23:15       ` [PATCH v3] " H. Peter Anvin
2013-09-06 18:06         ` [PATCH 0/3] x86/mm: fix early_memremap() sparse warnings Linn Crosetto
2013-09-06 18:06           ` [PATCH 1/3] x86/mm: fix sparse warnings from early_memremap() Linn Crosetto
2013-09-06 18:06           ` [PATCH 2/3] x86: fix sparse warning in parse_e820_ext() Linn Crosetto
2013-09-06 18:06           ` [PATCH 3/3] x86: fix early_iounmap() sparse warnings in setup.c Linn Crosetto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).