All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.