* [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds @ 2015-10-23 9:50 Taku Izumi 2015-10-23 8:37 ` Ard Biesheuvel 2015-10-23 8:40 ` Ingo Molnar 0 siblings, 2 replies; 18+ messages in thread From: Taku Izumi @ 2015-10-23 9:50 UTC (permalink / raw) To: linux-tip-commits, linux-kernel, mingo.kernel.org, ard.biesheuvel, matt.fleming Cc: kamezawa.hiroyu, Taku Izumi 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds 2015-10-23 9:50 [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Taku Izumi @ 2015-10-23 8:37 ` Ard Biesheuvel 2015-10-23 8:50 ` Ingo Molnar 2015-10-26 21:02 ` [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Matt Fleming 2015-10-23 8:40 ` Ingo Molnar 1 sibling, 2 replies; 18+ messages in thread From: Ard Biesheuvel @ 2015-10-23 8:37 UTC (permalink / raw) To: Taku Izumi, Matt Fleming, Ingo Molnar Cc: linux-kernel, kamezawa.hiroyu, linux-efi 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds 2015-10-23 8:37 ` Ard Biesheuvel @ 2015-10-23 8:50 ` Ingo Molnar 2015-10-23 9:48 ` [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map Ard Biesheuvel 2015-10-26 21:02 ` [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Matt Fleming 1 sibling, 1 reply; 18+ messages in thread From: Ingo Molnar @ 2015-10-23 8:50 UTC (permalink / raw) To: Ard Biesheuvel Cc: Taku Izumi, Matt Fleming, Ingo Molnar, linux-kernel, kamezawa.hiroyu, linux-efi * 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map 2015-10-23 8:50 ` Ingo Molnar @ 2015-10-23 9:48 ` Ard Biesheuvel 2015-10-23 9:48 ` [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Ard Biesheuvel ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Ard Biesheuvel @ 2015-10-23 9:48 UTC (permalink / raw) To: linux-efi, izumi.taku, matt.fleming, linux-kernel, mingo Cc: kamezawa.hiroyu, Ard Biesheuvel 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds 2015-10-23 9:48 ` [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map Ard Biesheuvel @ 2015-10-23 9:48 ` Ard Biesheuvel 2015-10-23 10:27 ` Ingo Molnar ` (2 more replies) 2015-10-27 21:09 ` [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map Matt Fleming 2015-10-28 20:57 ` [tip:core/efi] efi: Use correct type for struct efi_memory_map:: phys_map tip-bot for Ard Biesheuvel 2 siblings, 3 replies; 18+ messages in thread From: Ard Biesheuvel @ 2015-10-23 9:48 UTC (permalink / raw) To: linux-efi, izumi.taku, matt.fleming, linux-kernel, mingo Cc: kamezawa.hiroyu, Ard Biesheuvel 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds 2015-10-23 9:48 ` [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Ard Biesheuvel @ 2015-10-23 10:27 ` Ingo Molnar 2015-10-23 10:30 ` Ingo Molnar 2015-10-27 21:11 ` Matt Fleming 2015-10-28 20:57 ` [tip:core/efi] " tip-bot for Taku Izumi 2 siblings, 1 reply; 18+ messages in thread From: Ingo Molnar @ 2015-10-23 10:27 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-efi, izumi.taku, matt.fleming, linux-kernel, kamezawa.hiroyu * 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds 2015-10-23 10:27 ` Ingo Molnar @ 2015-10-23 10:30 ` Ingo Molnar 2015-10-23 10:33 ` Ingo Molnar 0 siblings, 1 reply; 18+ messages in thread From: Ingo Molnar @ 2015-10-23 10:30 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-efi, izumi.taku, matt.fleming, linux-kernel, kamezawa.hiroyu * 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds 2015-10-23 10:30 ` Ingo Molnar @ 2015-10-23 10:33 ` Ingo Molnar 2015-10-27 21:12 ` Matt Fleming 0 siblings, 1 reply; 18+ messages in thread From: Ingo Molnar @ 2015-10-23 10:33 UTC (permalink / raw) To: Ard Biesheuvel, Matt Fleming Cc: linux-efi, izumi.taku, matt.fleming, linux-kernel, kamezawa.hiroyu * 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds 2015-10-23 10:33 ` Ingo Molnar @ 2015-10-27 21:12 ` Matt Fleming 2015-10-28 11:28 ` Ingo Molnar 0 siblings, 1 reply; 18+ messages in thread From: Matt Fleming @ 2015-10-27 21:12 UTC (permalink / raw) To: Ingo Molnar Cc: Ard Biesheuvel, linux-efi, izumi.taku, linux-kernel, kamezawa.hiroyu 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? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds 2015-10-27 21:12 ` Matt Fleming @ 2015-10-28 11:28 ` Ingo Molnar 0 siblings, 0 replies; 18+ messages in thread From: Ingo Molnar @ 2015-10-28 11:28 UTC (permalink / raw) To: Matt Fleming Cc: Ard Biesheuvel, linux-efi, izumi.taku, linux-kernel, kamezawa.hiroyu * 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds 2015-10-23 9:48 ` [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Ard Biesheuvel 2015-10-23 10:27 ` Ingo Molnar @ 2015-10-27 21:11 ` Matt Fleming 2015-10-28 20:57 ` [tip:core/efi] " tip-bot for Taku Izumi 2 siblings, 0 replies; 18+ messages in thread From: Matt Fleming @ 2015-10-27 21:11 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-efi, izumi.taku, linux-kernel, mingo, kamezawa.hiroyu 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> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [tip:core/efi] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds 2015-10-23 9:48 ` [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Ard Biesheuvel 2015-10-23 10:27 ` Ingo Molnar 2015-10-27 21:11 ` Matt Fleming @ 2015-10-28 20:57 ` tip-bot for Taku Izumi 2 siblings, 0 replies; 18+ messages in thread From: tip-bot for Taku Izumi @ 2015-10-28 20:57 UTC (permalink / raw) To: linux-tip-commits Cc: hpa, linux-kernel, tglx, torvalds, izumi.taku, matt, mingo, peterz, ard.biesheuvel 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); ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map 2015-10-23 9:48 ` [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map Ard Biesheuvel 2015-10-23 9:48 ` [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Ard Biesheuvel @ 2015-10-27 21:09 ` Matt Fleming 2015-10-28 20:57 ` [tip:core/efi] efi: Use correct type for struct efi_memory_map:: phys_map tip-bot for Ard Biesheuvel 2 siblings, 0 replies; 18+ messages in thread From: Matt Fleming @ 2015-10-27 21:09 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-efi, izumi.taku, linux-kernel, mingo, kamezawa.hiroyu 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> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [tip:core/efi] efi: Use correct type for struct efi_memory_map:: phys_map 2015-10-23 9:48 ` [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map Ard Biesheuvel 2015-10-23 9:48 ` [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Ard Biesheuvel 2015-10-27 21:09 ` [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map Matt Fleming @ 2015-10-28 20:57 ` tip-bot for Ard Biesheuvel 2 siblings, 0 replies; 18+ messages in thread From: tip-bot for Ard Biesheuvel @ 2015-10-28 20:57 UTC (permalink / raw) To: linux-tip-commits Cc: tglx, torvalds, peterz, mingo, matt, hpa, linux-kernel, ard.biesheuvel 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; ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds 2015-10-23 8:37 ` Ard Biesheuvel 2015-10-23 8:50 ` Ingo Molnar @ 2015-10-26 21:02 ` Matt Fleming 2015-10-27 2:33 ` Ard Biesheuvel 1 sibling, 1 reply; 18+ messages in thread From: Matt Fleming @ 2015-10-26 21:02 UTC (permalink / raw) To: Ard Biesheuvel Cc: Taku Izumi, Ingo Molnar, linux-kernel, kamezawa.hiroyu, linux-efi 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? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds 2015-10-26 21:02 ` [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Matt Fleming @ 2015-10-27 2:33 ` Ard Biesheuvel 2015-10-27 21:08 ` Matt Fleming 0 siblings, 1 reply; 18+ messages in thread From: Ard Biesheuvel @ 2015-10-27 2:33 UTC (permalink / raw) To: Matt Fleming Cc: Taku Izumi, Ingo Molnar, linux-kernel, kamezawa.hiroyu, linux-efi 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds 2015-10-27 2:33 ` Ard Biesheuvel @ 2015-10-27 21:08 ` Matt Fleming 0 siblings, 0 replies; 18+ messages in thread From: Matt Fleming @ 2015-10-27 21:08 UTC (permalink / raw) To: Ard Biesheuvel Cc: Taku Izumi, Ingo Molnar, linux-kernel, kamezawa.hiroyu, linux-efi 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds 2015-10-23 9:50 [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Taku Izumi 2015-10-23 8:37 ` Ard Biesheuvel @ 2015-10-23 8:40 ` Ingo Molnar 1 sibling, 0 replies; 18+ messages in thread From: Ingo Molnar @ 2015-10-23 8:40 UTC (permalink / raw) To: Taku Izumi Cc: linux-tip-commits, linux-kernel, mingo.kernel.org, ard.biesheuvel, matt.fleming, kamezawa.hiroyu * 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2015-10-28 20:58 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-10-23 9:50 [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Taku Izumi 2015-10-23 8:37 ` Ard Biesheuvel 2015-10-23 8:50 ` Ingo Molnar 2015-10-23 9:48 ` [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map Ard Biesheuvel 2015-10-23 9:48 ` [PATCH 2/2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Ard Biesheuvel 2015-10-23 10:27 ` Ingo Molnar 2015-10-23 10:30 ` Ingo Molnar 2015-10-23 10:33 ` Ingo Molnar 2015-10-27 21:12 ` Matt Fleming 2015-10-28 11:28 ` Ingo Molnar 2015-10-27 21:11 ` Matt Fleming 2015-10-28 20:57 ` [tip:core/efi] " tip-bot for Taku Izumi 2015-10-27 21:09 ` [PATCH 1/2] efi: use correct type for struct efi_memory_map::phys_map Matt Fleming 2015-10-28 20:57 ` [tip:core/efi] efi: Use correct type for struct efi_memory_map:: phys_map tip-bot for Ard Biesheuvel 2015-10-26 21:02 ` [PATCH v2] efi: Fix warning of int-to-pointer-cast on x86 32-bit builds Matt Fleming 2015-10-27 2:33 ` Ard Biesheuvel 2015-10-27 21:08 ` Matt Fleming 2015-10-23 8:40 ` Ingo Molnar
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).