Groups > linux.kernel > #1254264 > unrolled thread

[PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

Started by	Taku Izumi <izumi.taku@jp.fujitsu.com>
First post	2015-10-23 02:50 +0200
Last post	2015-10-23 10:50 +0200
Articles	18 — 6 participants

Back to article view | Back to linux.kernel

  [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Taku Izumi <izumi.taku@jp.fujitsu.com> - 2015-10-23 02:50 +0200
    Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2015-10-23 10:40 +0200
      Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit  builds Ingo Molnar <mingo@kernel.org> - 2015-10-23 11:00 +0200
        [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2015-10-23 11:50 +0200
          [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2015-10-23 11:50 +0200
            Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86  32-bit builds Ingo Molnar <mingo@kernel.org> - 2015-10-23 12:30 +0200
              Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86  32-bit builds Ingo Molnar <mingo@kernel.org> - 2015-10-23 12:40 +0200
                Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86  32-bit builds Ingo Molnar <mingo@kernel.org> - 2015-10-23 12:40 +0200
                  Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86  32-bit builds Matt Fleming <matt@codeblueprint.co.uk> - 2015-10-27 22:20 +0100
                    Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86  32-bit builds Ingo Molnar <mingo@kernel.org> - 2015-10-28 12:40 +0100
            Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86  32-bit builds Matt Fleming <matt@codeblueprint.co.uk> - 2015-10-27 22:20 +0100
            [tip:core/efi] efi:   Fix warning of int-to-pointer-cast on x86 32-bit builds tip-bot for Taku Izumi <tipbot@zytor.com> - 2015-10-28 22:00 +0100
          Re: [PATCH 1/2] efi: use correct type for struct  efi_memory_map::phys_map Matt Fleming <matt@codeblueprint.co.uk> - 2015-10-27 22:10 +0100
          [tip:core/efi] efi: Use correct type for struct efi_memory_map::  phys_map tip-bot for Ard Biesheuvel <tipbot@zytor.com> - 2015-10-28 22:00 +0100
      Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit  builds Matt Fleming <matt@codeblueprint.co.uk> - 2015-10-26 22:10 +0100
        Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2015-10-27 03:40 +0100
          Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit  builds Matt Fleming <matt@codeblueprint.co.uk> - 2015-10-27 22:10 +0100
    Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit  builds Ingo Molnar <mingo@kernel.org> - 2015-10-23 10:50 +0200

#1254264 — [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

From	Taku Izumi <izumi.taku@jp.fujitsu.com>
Date	2015-10-23 02:50 +0200
Subject	[PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds
Message-ID	<qmyIq-2en-9@gated-at.bofh.it>

commit-0f96a99 introduces the following warning message:

  drivers/firmware/efi/fake_mem.c:186:20: warning: cast to pointer
  from integer of different size [-Wint-to-pointer-cast]

new_memmap_phy was defined as a u64 value and casted to void*.
This causes a warning of int-to-pointer-cast on x86 32-bit
environment.

This patch changes the type of "new_memmap_phy" variable
from "u64" into "ulong" to avoid it.

v1 -> v2:
 - change the type of "new_memmap_phy" from phys_addr_t
   into ulong according to Ard's comment

Reported-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 drivers/firmware/efi/fake_mem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
index 32bcb14..1f483b4 100644
--- a/drivers/firmware/efi/fake_mem.c
+++ b/drivers/firmware/efi/fake_mem.c
@@ -59,7 +59,7 @@ void __init efi_fake_memmap(void)
 	u64 start, end, m_start, m_end, m_attr;
 	int new_nr_map = memmap.nr_map;
 	efi_memory_desc_t *md;
-	u64 new_memmap_phy;
+	ulong new_memmap_phy;
 	void *new_memmap;
 	void *old, *new;
 	int i;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1254387

From	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date	2015-10-23 10:40 +0200
Message-ID	<qmG3h-4qs-15@gated-at.bofh.it>
In reply to	#1254264

On 23 October 2015 at 11:50, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> commit-0f96a99 introduces the following warning message:
>
>   drivers/firmware/efi/fake_mem.c:186:20: warning: cast to pointer
>   from integer of different size [-Wint-to-pointer-cast]
>
> new_memmap_phy was defined as a u64 value and casted to void*.
> This causes a warning of int-to-pointer-cast on x86 32-bit
> environment.
>
> This patch changes the type of "new_memmap_phy" variable
> from "u64" into "ulong" to avoid it.
>
> v1 -> v2:
>  - change the type of "new_memmap_phy" from phys_addr_t
>    into ulong according to Ard's comment
>
> Reported-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
>  drivers/firmware/efi/fake_mem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
> index 32bcb14..1f483b4 100644
> --- a/drivers/firmware/efi/fake_mem.c
> +++ b/drivers/firmware/efi/fake_mem.c
> @@ -59,7 +59,7 @@ void __init efi_fake_memmap(void)
>         u64 start, end, m_start, m_end, m_attr;
>         int new_nr_map = memmap.nr_map;
>         efi_memory_desc_t *md;
> -       u64 new_memmap_phy;
> +       ulong new_memmap_phy;
>         void *new_memmap;
>         void *old, *new;
>         int i;

After looking at the original (already merged) patch 11/11 again, I
realize this is still not right: the problem is that efi_memory_map's
phys_map member uses a void* type to hold a physical address, which
happens to be correct in the normal case even when phys_addr_t is
larger than void* (like on ARM with LPAE enabled) since the address it
holds is the address of an allocation performed by the firmware, which
only uses 1:1 addressable memory.

However, overwriting memmap.phys_map with a value produced my
memblock_alloc() is problematic, since the allocation may be above 4
GB on 32-bit (L)PAE platforms. So the correct way to do this would be
to set the memblock limit to 4GB before memblock_alloc() on 32-bit
platforms, and restore it afterwards. This is a bit of a kludge,
though, and it would be more correct to change the type of
efi_memory_map::phys_map to phys_addr_t, although I don't know what
the potential fallout of that change is. Matt?

So that means your v1 of this patch is correct after all, and the
warning spotted by Ingo uncovered a problem with the original series
that requires an additional fix.

Regards,
Ard.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1254404 — Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

From	Ingo Molnar <mingo@kernel.org>
Date	2015-10-23 11:00 +0200
Subject	Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds
Message-ID	<qmGmD-4MY-15@gated-at.bofh.it>
In reply to	#1254387

* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> On 23 October 2015 at 11:50, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
> > commit-0f96a99 introduces the following warning message:
> >
> >   drivers/firmware/efi/fake_mem.c:186:20: warning: cast to pointer
> >   from integer of different size [-Wint-to-pointer-cast]
> >
> > new_memmap_phy was defined as a u64 value and casted to void*.
> > This causes a warning of int-to-pointer-cast on x86 32-bit
> > environment.
> >
> > This patch changes the type of "new_memmap_phy" variable
> > from "u64" into "ulong" to avoid it.
> >
> > v1 -> v2:
> >  - change the type of "new_memmap_phy" from phys_addr_t
> >    into ulong according to Ard's comment
> >
> > Reported-by: Ingo Molnar <mingo@kernel.org>
> > Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> > ---
> >  drivers/firmware/efi/fake_mem.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
> > index 32bcb14..1f483b4 100644
> > --- a/drivers/firmware/efi/fake_mem.c
> > +++ b/drivers/firmware/efi/fake_mem.c
> > @@ -59,7 +59,7 @@ void __init efi_fake_memmap(void)
> >         u64 start, end, m_start, m_end, m_attr;
> >         int new_nr_map = memmap.nr_map;
> >         efi_memory_desc_t *md;
> > -       u64 new_memmap_phy;
> > +       ulong new_memmap_phy;
> >         void *new_memmap;
> >         void *old, *new;
> >         int i;
> 
> 
> After looking at the original (already merged) patch 11/11 again, I
> realize this is still not right: the problem is that efi_memory_map's
> phys_map member uses a void* type to hold a physical address, which
> happens to be correct in the normal case even when phys_addr_t is
> larger than void* (like on ARM with LPAE enabled) since the address it
> holds is the address of an allocation performed by the firmware, which
> only uses 1:1 addressable memory.
> 
> However, overwriting memmap.phys_map with a value produced my
> memblock_alloc() is problematic, since the allocation may be above 4
> GB on 32-bit (L)PAE platforms. So the correct way to do this would be
> to set the memblock limit to 4GB before memblock_alloc() on 32-bit
> platforms, and restore it afterwards. This is a bit of a kludge,
> though, and it would be more correct to change the type of
> efi_memory_map::phys_map to phys_addr_t, although I don't know what
> the potential fallout of that change is. Matt?
> 
> So that means your v1 of this patch is correct after all, and the
> warning spotted by Ingo uncovered a problem with the original series
> that requires an additional fix.

No, v1 is not right either. This type cast loses information:

        memmap.phys_map = (void *)new_memmap_phy;

Because there are countless platforms where 'void *' is 32-bit while physical 
addresses are wider.

No way of fudging around the type of 'new_memmap_phy' will solve that bug, it 
might only make the symptoms and the warning go away ...

The real problem is with the inappropriate (too narrow) type of memmap.phys_map, 
not with the type of new_memmap_phy. The cast just hides it.

One more page in the '1000 real-life examples of why type casts are evil' book.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1254437 — [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map

From	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date	2015-10-23 11:50 +0200
Subject	[PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map
Message-ID	<qmH8Z-5X0-7@gated-at.bofh.it>
In reply to	#1254404

We have been getting away with using a void* for the physical
address of the UEFI memory map, since, even on 32-bit platforms
with 64-bit physical addresses, no truncation takes place if the
memory map has been allocated by the firmware (which only uses
1:1 virtually addressable memory), which is usually the case.

However, commit 0f96a99dab36 ("efi: Add "efi_fake_mem" boot option")
adds code that clones and modifies the UEFI memory map, and the
clone may live above 4 GB on 32-bit platforms. This means our use
of void* for struct efi_memory_map::phys_map has graduated from
'incorrect but working' to 'incorrect and broken', and we need to
fix it.

So redefine struct efi_memory_map::phys_map as phys_addr_t, and
get rid of a bunch of casts that are now unneeded.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/kernel/efi.c     | 4 ++--
 arch/x86/platform/efi/efi.c | 4 ++--
 drivers/firmware/efi/efi.c  | 8 ++++----
 include/linux/efi.h         | 2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 4b7df346e388..61eb1d17586a 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -208,7 +208,7 @@ void __init efi_init(void)
 
 	memblock_reserve(params.mmap & PAGE_MASK,
 			 PAGE_ALIGN(params.mmap_size + (params.mmap & ~PAGE_MASK)));
-	memmap.phys_map = (void *)params.mmap;
+	memmap.phys_map = params.mmap;
 	memmap.map = early_memremap(params.mmap, params.mmap_size);
 	memmap.map_end = memmap.map + params.mmap_size;
 	memmap.desc_size = params.desc_size;
@@ -282,7 +282,7 @@ static int __init arm64_enable_runtime_services(void)
 	pr_info("Remapping and enabling EFI services.\n");
 
 	mapsize = memmap.map_end - memmap.map;
-	memmap.map = (__force void *)ioremap_cache((phys_addr_t)memmap.phys_map,
+	memmap.map = (__force void *)ioremap_cache(memmap.phys_map,
 						   mapsize);
 	if (!memmap.map) {
 		pr_err("Failed to remap EFI memory map\n");
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 3e1d09e885c0..ad285404ea7f 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -194,7 +194,7 @@ static void __init do_add_efi_memmap(void)
 int __init efi_memblock_x86_reserve_range(void)
 {
 	struct efi_info *e = &boot_params.efi_info;
-	unsigned long pmap;
+	phys_addr_t pmap;
 
 	if (efi_enabled(EFI_PARAVIRT))
 		return 0;
@@ -209,7 +209,7 @@ int __init efi_memblock_x86_reserve_range(void)
 #else
 	pmap = (e->efi_memmap |	((__u64)e->efi_memmap_hi << 32));
 #endif
-	memmap.phys_map		= (void *)pmap;
+	memmap.phys_map		= pmap;
 	memmap.nr_map		= e->efi_memmap_size /
 				  e->efi_memdesc_size;
 	memmap.desc_size	= e->efi_memdesc_size;
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 31fc864eb037..027ca212179f 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -254,7 +254,7 @@ subsys_initcall(efisubsys_init);
 int __init efi_mem_desc_lookup(u64 phys_addr, efi_memory_desc_t *out_md)
 {
 	struct efi_memory_map *map = efi.memmap;
-	void *p, *e;
+	phys_addr_t p, e;
 
 	if (!efi_enabled(EFI_MEMMAP)) {
 		pr_err_once("EFI_MEMMAP is not enabled.\n");
@@ -286,10 +286,10 @@ int __init efi_mem_desc_lookup(u64 phys_addr, efi_memory_desc_t *out_md)
 		 * So just always get our own virtual map on the CPU.
 		 *
 		 */
-		md = early_memremap((phys_addr_t)p, sizeof (*md));
+		md = early_memremap(p, sizeof (*md));
 		if (!md) {
-			pr_err_once("early_memremap(%p, %zu) failed.\n",
-				    p, sizeof (*md));
+			pr_err_once("early_memremap(%pa, %zu) failed.\n",
+				    &p, sizeof (*md));
 			return -ENOMEM;
 		}
 
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 4d01c1033fce..569b5a866bb1 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -680,7 +680,7 @@ typedef struct {
 } efi_system_table_t;
 
 struct efi_memory_map {
-	void *phys_map;
+	phys_addr_t phys_map;
 	void *map;
 	void *map_end;
 	int nr_map;
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1254441 — [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

From	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date	2015-10-23 11:50 +0200
Subject	[PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds
Message-ID	<qmH90-5X0-19@gated-at.bofh.it>
In reply to	#1254437

From: Taku Izumi <izumi.taku@jp.fujitsu.com>

Commit 0f96a99dab36 ("efi: Add "efi_fake_mem" boot option")
introduces the following warning message:

  drivers/firmware/efi/fake_mem.c:186:20: warning: cast to pointer
  from integer of different size [-Wint-to-pointer-cast]

new_memmap_phy was defined as a u64 value and cast to void*, causing
a int-to-pointer-cast warning on x86 32-bit builds.
However, since the void* type is inappropriate for a physical
address, the definition of struct efi_memory_map::phys_map has been
changed to phys_addr_t in the previous patch, and so the cast can be
dropped entirely.

This patch also changes the type of the "new_memmap_phy" variable
from "u64" to "phys_addr_t" to align with the types of
memblock_alloc() and struct efi_memory_map::phys_map.

Reported-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
[ard.biesheuvel: removed void* cast, updated commit log]
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/fake_mem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
index 32bcb14df2c8..ed3a854950cc 100644
--- a/drivers/firmware/efi/fake_mem.c
+++ b/drivers/firmware/efi/fake_mem.c
@@ -59,7 +59,7 @@ void __init efi_fake_memmap(void)
 	u64 start, end, m_start, m_end, m_attr;
 	int new_nr_map = memmap.nr_map;
 	efi_memory_desc_t *md;
-	u64 new_memmap_phy;
+	phys_addr_t new_memmap_phy;
 	void *new_memmap;
 	void *old, *new;
 	int i;
@@ -183,7 +183,7 @@ void __init efi_fake_memmap(void)
 	/* swap into new EFI memmap */
 	efi_unmap_memmap();
 	memmap.map = new_memmap;
-	memmap.phys_map = (void *)new_memmap_phy;
+	memmap.phys_map = new_memmap_phy;
 	memmap.nr_map = new_nr_map;
 	memmap.map_end = memmap.map + memmap.nr_map * memmap.desc_size;
 	set_bit(EFI_MEMMAP, &efi.flags);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1254458 — Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

From	Ingo Molnar <mingo@kernel.org>
Date	2015-10-23 12:30 +0200
Subject	Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds
Message-ID	<qmHLI-72j-25@gated-at.bofh.it>
In reply to	#1254441

* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> From: Taku Izumi <izumi.taku@jp.fujitsu.com>
> 
> Commit 0f96a99dab36 ("efi: Add "efi_fake_mem" boot option")
> introduces the following warning message:
> 
>   drivers/firmware/efi/fake_mem.c:186:20: warning: cast to pointer
>   from integer of different size [-Wint-to-pointer-cast]
> 
> new_memmap_phy was defined as a u64 value and cast to void*, causing
> a int-to-pointer-cast warning on x86 32-bit builds.
> However, since the void* type is inappropriate for a physical
> address, the definition of struct efi_memory_map::phys_map has been
> changed to phys_addr_t in the previous patch, and so the cast can be
> dropped entirely.
> 
> This patch also changes the type of the "new_memmap_phy" variable
> from "u64" to "phys_addr_t" to align with the types of
> memblock_alloc() and struct efi_memory_map::phys_map.
> 
> Reported-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> [ard.biesheuvel: removed void* cast, updated commit log]
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/firmware/efi/fake_mem.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
> index 32bcb14df2c8..ed3a854950cc 100644
> --- a/drivers/firmware/efi/fake_mem.c
> +++ b/drivers/firmware/efi/fake_mem.c
> @@ -59,7 +59,7 @@ void __init efi_fake_memmap(void)
>  	u64 start, end, m_start, m_end, m_attr;
>  	int new_nr_map = memmap.nr_map;
>  	efi_memory_desc_t *md;
> -	u64 new_memmap_phy;
> +	phys_addr_t new_memmap_phy;
>  	void *new_memmap;
>  	void *old, *new;
>  	int i;
> @@ -183,7 +183,7 @@ void __init efi_fake_memmap(void)
>  	/* swap into new EFI memmap */
>  	efi_unmap_memmap();
>  	memmap.map = new_memmap;
> -	memmap.phys_map = (void *)new_memmap_phy;
> +	memmap.phys_map = new_memmap_phy;
>  	memmap.nr_map = new_nr_map;
>  	memmap.map_end = memmap.map + memmap.nr_map * memmap.desc_size;

Please guys, think for a change!

How is this supposed to work with:

   include/linux/efi.h:    void *phys_map;

?

Thanks,

	Ingo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1254462 — Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

From	Ingo Molnar <mingo@kernel.org>
Date	2015-10-23 12:40 +0200
Subject	Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds
Message-ID	<qmHVn-7dR-3@gated-at.bofh.it>
In reply to	#1254458

* Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 
> > From: Taku Izumi <izumi.taku@jp.fujitsu.com>
> > 
> > Commit 0f96a99dab36 ("efi: Add "efi_fake_mem" boot option")
> > introduces the following warning message:
> > 
> >   drivers/firmware/efi/fake_mem.c:186:20: warning: cast to pointer
> >   from integer of different size [-Wint-to-pointer-cast]
> > 
> > new_memmap_phy was defined as a u64 value and cast to void*, causing
> > a int-to-pointer-cast warning on x86 32-bit builds.
> > However, since the void* type is inappropriate for a physical
> > address, the definition of struct efi_memory_map::phys_map has been
> > changed to phys_addr_t in the previous patch, and so the cast can be
> > dropped entirely.
> > 
> > This patch also changes the type of the "new_memmap_phy" variable
> > from "u64" to "phys_addr_t" to align with the types of
> > memblock_alloc() and struct efi_memory_map::phys_map.
> > 
> > Reported-by: Ingo Molnar <mingo@kernel.org>
> > Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> > [ard.biesheuvel: removed void* cast, updated commit log]
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  drivers/firmware/efi/fake_mem.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
> > index 32bcb14df2c8..ed3a854950cc 100644
> > --- a/drivers/firmware/efi/fake_mem.c
> > +++ b/drivers/firmware/efi/fake_mem.c
> > @@ -59,7 +59,7 @@ void __init efi_fake_memmap(void)
> >  	u64 start, end, m_start, m_end, m_attr;
> >  	int new_nr_map = memmap.nr_map;
> >  	efi_memory_desc_t *md;
> > -	u64 new_memmap_phy;
> > +	phys_addr_t new_memmap_phy;
> >  	void *new_memmap;
> >  	void *old, *new;
> >  	int i;
> > @@ -183,7 +183,7 @@ void __init efi_fake_memmap(void)
> >  	/* swap into new EFI memmap */
> >  	efi_unmap_memmap();
> >  	memmap.map = new_memmap;
> > -	memmap.phys_map = (void *)new_memmap_phy;
> > +	memmap.phys_map = new_memmap_phy;
> >  	memmap.nr_map = new_nr_map;
> >  	memmap.map_end = memmap.map + memmap.nr_map * memmap.desc_size;
> 
> Please guys, think for a change!
> 
> How is this supposed to work with:
> 
>    include/linux/efi.h:    void *phys_map;
> 
> ?

Ah, I take that back, now I see your patch 1/2 that changes it to phys_addr_t.

That's the right solution, and it also cleans up the code:

  Reviewed-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1254464 — Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

From	Ingo Molnar <mingo@kernel.org>
Date	2015-10-23 12:40 +0200
Subject	Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds
Message-ID	<qmHVn-7dR-11@gated-at.bofh.it>
In reply to	#1254462

* Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Ingo Molnar <mingo@kernel.org> wrote:
> 
> > 
> > * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > 
> > > From: Taku Izumi <izumi.taku@jp.fujitsu.com>
> > > 
> > > Commit 0f96a99dab36 ("efi: Add "efi_fake_mem" boot option")
> > > introduces the following warning message:
> > > 
> > >   drivers/firmware/efi/fake_mem.c:186:20: warning: cast to pointer
> > >   from integer of different size [-Wint-to-pointer-cast]
> > > 
> > > new_memmap_phy was defined as a u64 value and cast to void*, causing
> > > a int-to-pointer-cast warning on x86 32-bit builds.
> > > However, since the void* type is inappropriate for a physical
> > > address, the definition of struct efi_memory_map::phys_map has been
> > > changed to phys_addr_t in the previous patch, and so the cast can be
> > > dropped entirely.
> > > 
> > > This patch also changes the type of the "new_memmap_phy" variable
> > > from "u64" to "phys_addr_t" to align with the types of
> > > memblock_alloc() and struct efi_memory_map::phys_map.
> > > 
> > > Reported-by: Ingo Molnar <mingo@kernel.org>
> > > Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> > > [ard.biesheuvel: removed void* cast, updated commit log]
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > ---
> > >  drivers/firmware/efi/fake_mem.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
> > > index 32bcb14df2c8..ed3a854950cc 100644
> > > --- a/drivers/firmware/efi/fake_mem.c
> > > +++ b/drivers/firmware/efi/fake_mem.c
> > > @@ -59,7 +59,7 @@ void __init efi_fake_memmap(void)
> > >  	u64 start, end, m_start, m_end, m_attr;
> > >  	int new_nr_map = memmap.nr_map;
> > >  	efi_memory_desc_t *md;
> > > -	u64 new_memmap_phy;
> > > +	phys_addr_t new_memmap_phy;
> > >  	void *new_memmap;
> > >  	void *old, *new;
> > >  	int i;
> > > @@ -183,7 +183,7 @@ void __init efi_fake_memmap(void)
> > >  	/* swap into new EFI memmap */
> > >  	efi_unmap_memmap();
> > >  	memmap.map = new_memmap;
> > > -	memmap.phys_map = (void *)new_memmap_phy;
> > > +	memmap.phys_map = new_memmap_phy;
> > >  	memmap.nr_map = new_nr_map;
> > >  	memmap.map_end = memmap.map + memmap.nr_map * memmap.desc_size;
> > 
> > Please guys, think for a change!
> > 
> > How is this supposed to work with:
> > 
> >    include/linux/efi.h:    void *phys_map;
> > 
> > ?
> 
> Ah, I take that back, now I see your patch 1/2 that changes it to phys_addr_t.
> 
> That's the right solution, and it also cleans up the code:
> 
>   Reviewed-by: Ingo Molnar <mingo@kernel.org>

Matt, do you want to take these fixes, or should I apply them directly?

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1257249 — Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

From	Matt Fleming <matt@codeblueprint.co.uk>
Date	2015-10-27 22:20 +0100
Subject	Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds
Message-ID	<qojOW-1Qn-27@gated-at.bofh.it>
In reply to	#1254464

On Fri, 23 Oct, at 12:33:29PM, Ingo Molnar wrote:
> 
> Matt, do you want to take these fixes, or should I apply them directly?

Could you please apply them directly with my Reviewed-by tag?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1257932 — Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

From	Ingo Molnar <mingo@kernel.org>
Date	2015-10-28 12:40 +0100
Subject	Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds
Message-ID	<qoxfc-21J-23@gated-at.bofh.it>
In reply to	#1257249

* Matt Fleming <matt@codeblueprint.co.uk> wrote:

> On Fri, 23 Oct, at 12:33:29PM, Ingo Molnar wrote:
> > 
> > Matt, do you want to take these fixes, or should I apply them directly?
> 
> Could you please apply them directly with my Reviewed-by tag?

Sure, done!

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1257266 — Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

From	Matt Fleming <matt@codeblueprint.co.uk>
Date	2015-10-27 22:20 +0100
Subject	Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds
Message-ID	<qojOY-1Qn-63@gated-at.bofh.it>
In reply to	#1254441

On Fri, 23 Oct, at 11:48:17AM, Ard Biesheuvel wrote:
> From: Taku Izumi <izumi.taku@jp.fujitsu.com>
> 
> Commit 0f96a99dab36 ("efi: Add "efi_fake_mem" boot option")
> introduces the following warning message:
> 
>   drivers/firmware/efi/fake_mem.c:186:20: warning: cast to pointer
>   from integer of different size [-Wint-to-pointer-cast]
> 
> new_memmap_phy was defined as a u64 value and cast to void*, causing
> a int-to-pointer-cast warning on x86 32-bit builds.
> However, since the void* type is inappropriate for a physical
> address, the definition of struct efi_memory_map::phys_map has been
> changed to phys_addr_t in the previous patch, and so the cast can be
> dropped entirely.
> 
> This patch also changes the type of the "new_memmap_phy" variable
> from "u64" to "phys_addr_t" to align with the types of
> memblock_alloc() and struct efi_memory_map::phys_map.
> 
> Reported-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> [ard.biesheuvel: removed void* cast, updated commit log]
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/firmware/efi/fake_mem.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1258415 — [tip:core/efi] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

From	tip-bot for Taku Izumi <tipbot@zytor.com>
Date	2015-10-28 22:00 +0100
Subject	[tip:core/efi] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds
Message-ID	<qoFZ7-7Bh-3@gated-at.bofh.it>
In reply to	#1254441

Commit-ID:  78b9bc947b18ed16b6c2c573d774e6d54ad9452d
Gitweb:     http://git.kernel.org/tip/78b9bc947b18ed16b6c2c573d774e6d54ad9452d
Author:     Taku Izumi <izumi.taku@jp.fujitsu.com>
AuthorDate: Fri, 23 Oct 2015 11:48:17 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 28 Oct 2015 12:28:06 +0100

efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

Commit:

  0f96a99dab36 ("efi: Add "efi_fake_mem" boot option")

introduced the following warning message:

  drivers/firmware/efi/fake_mem.c:186:20: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]

new_memmap_phy was defined as a u64 value and cast to void*,
causing a int-to-pointer-cast warning on x86 32-bit builds.
However, since the void* type is inappropriate for a physical
address, the definition of struct efi_memory_map::phys_map has
been changed to phys_addr_t in the previous patch, and so the
cast can be dropped entirely.

This patch also changes the type of the "new_memmap_phy"
variable from "u64" to "phys_addr_t" to align with the types of
memblock_alloc() and struct efi_memory_map::phys_map.

Reported-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
[ Removed void* cast, updated commit log]
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: kamezawa.hiroyu@jp.fujitsu.com
Cc: linux-efi@vger.kernel.org
Cc: matt.fleming@intel.com
Link: http://lkml.kernel.org/r/1445593697-1342-2-git-send-email-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/fake_mem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
index 32bcb14..ed3a854 100644
--- a/drivers/firmware/efi/fake_mem.c
+++ b/drivers/firmware/efi/fake_mem.c
@@ -59,7 +59,7 @@ void __init efi_fake_memmap(void)
 	u64 start, end, m_start, m_end, m_attr;
 	int new_nr_map = memmap.nr_map;
 	efi_memory_desc_t *md;
-	u64 new_memmap_phy;
+	phys_addr_t new_memmap_phy;
 	void *new_memmap;
 	void *old, *new;
 	int i;
@@ -183,7 +183,7 @@ void __init efi_fake_memmap(void)
 	/* swap into new EFI memmap */
 	efi_unmap_memmap();
 	memmap.map = new_memmap;
-	memmap.phys_map = (void *)new_memmap_phy;
+	memmap.phys_map = new_memmap_phy;
 	memmap.nr_map = new_nr_map;
 	memmap.map_end = memmap.map + memmap.nr_map * memmap.desc_size;
 	set_bit(EFI_MEMMAP, &efi.flags);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1257236 — Re: [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map

From	Matt Fleming <matt@codeblueprint.co.uk>
Date	2015-10-27 22:10 +0100
Subject	Re: [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map
Message-ID	<qojFi-1MD-61@gated-at.bofh.it>
In reply to	#1254437

On Fri, 23 Oct, at 11:48:16AM, Ard Biesheuvel wrote:
> We have been getting away with using a void* for the physical
> address of the UEFI memory map, since, even on 32-bit platforms
> with 64-bit physical addresses, no truncation takes place if the
> memory map has been allocated by the firmware (which only uses
> 1:1 virtually addressable memory), which is usually the case.
> 
> However, commit 0f96a99dab36 ("efi: Add "efi_fake_mem" boot option")
> adds code that clones and modifies the UEFI memory map, and the
> clone may live above 4 GB on 32-bit platforms. This means our use
> of void* for struct efi_memory_map::phys_map has graduated from
> 'incorrect but working' to 'incorrect and broken', and we need to
> fix it.
> 
> So redefine struct efi_memory_map::phys_map as phys_addr_t, and
> get rid of a bunch of casts that are now unneeded.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm64/kernel/efi.c     | 4 ++--
>  arch/x86/platform/efi/efi.c | 4 ++--
>  drivers/firmware/efi/efi.c  | 8 ++++----
>  include/linux/efi.h         | 2 +-
>  4 files changed, 9 insertions(+), 9 deletions(-)

Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1258418 — [tip:core/efi] efi: Use correct type for struct efi_memory_map:: phys_map

From	tip-bot for Ard Biesheuvel <tipbot@zytor.com>
Date	2015-10-28 22:00 +0100
Subject	[tip:core/efi] efi: Use correct type for struct efi_memory_map:: phys_map
Message-ID	<qoFZ8-7Bh-13@gated-at.bofh.it>
In reply to	#1254437

Commit-ID:  44511fb9e55ada760822b0b0d7be9d150576f17f
Gitweb:     http://git.kernel.org/tip/44511fb9e55ada760822b0b0d7be9d150576f17f
Author:     Ard Biesheuvel <ard.biesheuvel@linaro.org>
AuthorDate: Fri, 23 Oct 2015 11:48:16 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 28 Oct 2015 12:28:06 +0100

efi: Use correct type for struct efi_memory_map::phys_map

We have been getting away with using a void* for the physical
address of the UEFI memory map, since, even on 32-bit platforms
with 64-bit physical addresses, no truncation takes place if the
memory map has been allocated by the firmware (which only uses
1:1 virtually addressable memory), which is usually the case.

However, commit:

  0f96a99dab36 ("efi: Add "efi_fake_mem" boot option")

adds code that clones and modifies the UEFI memory map, and the
clone may live above 4 GB on 32-bit platforms.

This means our use of void* for struct efi_memory_map::phys_map has
graduated from 'incorrect but working' to 'incorrect and
broken', and we need to fix it.

So redefine struct efi_memory_map::phys_map as phys_addr_t, and
get rid of a bunch of casts that are now unneeded.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: izumi.taku@jp.fujitsu.com
Cc: kamezawa.hiroyu@jp.fujitsu.com
Cc: linux-efi@vger.kernel.org
Cc: matt.fleming@intel.com
Link: http://lkml.kernel.org/r/1445593697-1342-1-git-send-email-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/arm64/kernel/efi.c     | 4 ++--
 arch/x86/platform/efi/efi.c | 4 ++--
 drivers/firmware/efi/efi.c  | 8 ++++----
 include/linux/efi.h         | 2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 4b7df34..61eb1d1 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -208,7 +208,7 @@ void __init efi_init(void)
 
 	memblock_reserve(params.mmap & PAGE_MASK,
 			 PAGE_ALIGN(params.mmap_size + (params.mmap & ~PAGE_MASK)));
-	memmap.phys_map = (void *)params.mmap;
+	memmap.phys_map = params.mmap;
 	memmap.map = early_memremap(params.mmap, params.mmap_size);
 	memmap.map_end = memmap.map + params.mmap_size;
 	memmap.desc_size = params.desc_size;
@@ -282,7 +282,7 @@ static int __init arm64_enable_runtime_services(void)
 	pr_info("Remapping and enabling EFI services.\n");
 
 	mapsize = memmap.map_end - memmap.map;
-	memmap.map = (__force void *)ioremap_cache((phys_addr_t)memmap.phys_map,
+	memmap.map = (__force void *)ioremap_cache(memmap.phys_map,
 						   mapsize);
 	if (!memmap.map) {
 		pr_err("Failed to remap EFI memory map\n");
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 3e1d09e..ad28540 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -194,7 +194,7 @@ static void __init do_add_efi_memmap(void)
 int __init efi_memblock_x86_reserve_range(void)
 {
 	struct efi_info *e = &boot_params.efi_info;
-	unsigned long pmap;
+	phys_addr_t pmap;
 
 	if (efi_enabled(EFI_PARAVIRT))
 		return 0;
@@ -209,7 +209,7 @@ int __init efi_memblock_x86_reserve_range(void)
 #else
 	pmap = (e->efi_memmap |	((__u64)e->efi_memmap_hi << 32));
 #endif
-	memmap.phys_map		= (void *)pmap;
+	memmap.phys_map		= pmap;
 	memmap.nr_map		= e->efi_memmap_size /
 				  e->efi_memdesc_size;
 	memmap.desc_size	= e->efi_memdesc_size;
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 31fc864..027ca21 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -254,7 +254,7 @@ subsys_initcall(efisubsys_init);
 int __init efi_mem_desc_lookup(u64 phys_addr, efi_memory_desc_t *out_md)
 {
 	struct efi_memory_map *map = efi.memmap;
-	void *p, *e;
+	phys_addr_t p, e;
 
 	if (!efi_enabled(EFI_MEMMAP)) {
 		pr_err_once("EFI_MEMMAP is not enabled.\n");
@@ -286,10 +286,10 @@ int __init efi_mem_desc_lookup(u64 phys_addr, efi_memory_desc_t *out_md)
 		 * So just always get our own virtual map on the CPU.
 		 *
 		 */
-		md = early_memremap((phys_addr_t)p, sizeof (*md));
+		md = early_memremap(p, sizeof (*md));
 		if (!md) {
-			pr_err_once("early_memremap(%p, %zu) failed.\n",
-				    p, sizeof (*md));
+			pr_err_once("early_memremap(%pa, %zu) failed.\n",
+				    &p, sizeof (*md));
 			return -ENOMEM;
 		}
 
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 4d01c10..569b5a8 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -680,7 +680,7 @@ typedef struct {
 } efi_system_table_t;
 
 struct efi_memory_map {
-	void *phys_map;
+	phys_addr_t phys_map;
 	void *map;
 	void *map_end;
 	int nr_map;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1256267 — Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

From	Matt Fleming <matt@codeblueprint.co.uk>
Date	2015-10-26 22:10 +0100
Subject	Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds
Message-ID	<qnXbI-4On-5@gated-at.bofh.it>
In reply to	#1254387

On Fri, 23 Oct, at 10:37:46AM, Ard Biesheuvel wrote:
> 
> After looking at the original (already merged) patch 11/11 again, I
> realize this is still not right: the problem is that efi_memory_map's
> phys_map member uses a void* type to hold a physical address, which
> happens to be correct in the normal case even when phys_addr_t is
> larger than void* (like on ARM with LPAE enabled) since the address it
> holds is the address of an allocation performed by the firmware, which
> only uses 1:1 addressable memory.
> 
> However, overwriting memmap.phys_map with a value produced my
> memblock_alloc() is problematic, since the allocation may be above 4
> GB on 32-bit (L)PAE platforms. So the correct way to do this would be
> to set the memblock limit to 4GB before memblock_alloc() on 32-bit
> platforms, and restore it afterwards. This is a bit of a kludge,
> though, and it would be more correct to change the type of
> efi_memory_map::phys_map to phys_addr_t, although I don't know what
> the potential fallout of that change is. Matt?

I think that should be fine. The only potentially tricky situation we
could encounter is where 32-bit x86 firmware uses PAE but the kernel
is built without support.

But that's not something I've ever seen enabled in the firmware and
there's a bunch of assumptions in the kernel already that would break
in that case.

Given that addresses are always 64-bit in the UEFI spec, even on
32-bit platforms, what's the downside of picking u64 instead of
phys_addr_t for efi_memory_map::phys_map?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1256419

From	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date	2015-10-27 03:40 +0100
Message-ID	<qo2l3-7OT-5@gated-at.bofh.it>
In reply to	#1256267

On 27 October 2015 at 06:02, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Fri, 23 Oct, at 10:37:46AM, Ard Biesheuvel wrote:
>>
>> After looking at the original (already merged) patch 11/11 again, I
>> realize this is still not right: the problem is that efi_memory_map's
>> phys_map member uses a void* type to hold a physical address, which
>> happens to be correct in the normal case even when phys_addr_t is
>> larger than void* (like on ARM with LPAE enabled) since the address it
>> holds is the address of an allocation performed by the firmware, which
>> only uses 1:1 addressable memory.
>>
>> However, overwriting memmap.phys_map with a value produced my
>> memblock_alloc() is problematic, since the allocation may be above 4
>> GB on 32-bit (L)PAE platforms. So the correct way to do this would be
>> to set the memblock limit to 4GB before memblock_alloc() on 32-bit
>> platforms, and restore it afterwards. This is a bit of a kludge,
>> though, and it would be more correct to change the type of
>> efi_memory_map::phys_map to phys_addr_t, although I don't know what
>> the potential fallout of that change is. Matt?
>
> I think that should be fine. The only potentially tricky situation we
> could encounter is where 32-bit x86 firmware uses PAE but the kernel
> is built without support.
>
> But that's not something I've ever seen enabled in the firmware and
> there's a bunch of assumptions in the kernel already that would break
> in that case.
>

Does UEFI even allow that? Even if it can describe memory over 4 GB,
it uses a flat mapping so allocations done by the stub (which
retrieves the memory map) should never reside at addresses over 4 GB.

-- 
Ard.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1257231 — Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

From	Matt Fleming <matt@codeblueprint.co.uk>
Date	2015-10-27 22:10 +0100
Subject	Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds
Message-ID	<qojFi-1MD-55@gated-at.bofh.it>
In reply to	#1256419

On Tue, 27 Oct, at 11:33:55AM, Ard Biesheuvel wrote:
> 
> Does UEFI even allow that? Even if it can describe memory over 4 GB,
> it uses a flat mapping so allocations done by the stub (which
> retrieves the memory map) should never reside at addresses over 4 GB.

Right, exactly. We're good with phys_addr_t.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1254396 — Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

From	Ingo Molnar <mingo@kernel.org>
Date	2015-10-23 10:50 +0200
Subject	Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds
Message-ID	<qmGcV-4Bz-1@gated-at.bofh.it>
In reply to	#1254264

* Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:

> commit-0f96a99 introduces the following warning message:
> 
>   drivers/firmware/efi/fake_mem.c:186:20: warning: cast to pointer
>   from integer of different size [-Wint-to-pointer-cast]
> 
> new_memmap_phy was defined as a u64 value and casted to void*.
> This causes a warning of int-to-pointer-cast on x86 32-bit
> environment.
> 
> This patch changes the type of "new_memmap_phy" variable
> from "u64" into "ulong" to avoid it.
> 
> v1 -> v2:
>  - change the type of "new_memmap_phy" from phys_addr_t
>    into ulong according to Ard's comment
> 
> Reported-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
>  drivers/firmware/efi/fake_mem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
> index 32bcb14..1f483b4 100644
> --- a/drivers/firmware/efi/fake_mem.c
> +++ b/drivers/firmware/efi/fake_mem.c
> @@ -59,7 +59,7 @@ void __init efi_fake_memmap(void)
>  	u64 start, end, m_start, m_end, m_attr;
>  	int new_nr_map = memmap.nr_map;
>  	efi_memory_desc_t *md;
> -	u64 new_memmap_phy;
> +	ulong new_memmap_phy;
>  	void *new_memmap;
>  	void *old, *new;
>  	int i;

Sight, this just makes the code outright buggy in another place.

Please don't just try to 'avoid a warning', that's the wrong mindset!

You need to look at how the various values are used, what their natural types are, 
how they are used in other places, and accordingly fix the code.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

csiph-web

[PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

Contents

#1254264 — [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

#1254387

#1254404 — Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

#1254437 — [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map

#1254441 — [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

#1254458 — Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

#1254462 — Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

#1254464 — Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

#1257249 — Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

#1257932 — Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

#1257266 — Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

#1258415 — [tip:core/efi] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

#1257236 — Re: [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map

#1258418 — [tip:core/efi] efi: Use correct type for struct efi_memory_map:: phys_map

#1256267 — Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

#1256419

#1257231 — Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds

#1254396 — Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds