linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/efi: Fix EFI memmap pointer size warning
@ 2016-11-06 13:02 Borislav Petkov
  2016-11-07  9:59 ` Matt Fleming
  0 siblings, 1 reply; 2+ messages in thread
From: Borislav Petkov @ 2016-11-06 13:02 UTC (permalink / raw)
  To: Matt Fleming; +Cc: linux-efi, lkml

Hi Matt,

please doublecheck me on this but I think we're fine using an unsigned
long.

Thanks.

---
From: Borislav Petkov <bp@suse.de>
Date: Sun, 6 Nov 2016 13:49:10 +0100
Subject: [PATCH] x86/efi: Fix EFI memmap pointer size warning
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fix this when building on 32-bit:

  arch/x86/platform/efi/efi.c: In function ‘__efi_enter_virtual_mode’:
  arch/x86/platform/efi/efi.c:911:5: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
       (efi_memory_desc_t *)pa);
       ^
  arch/x86/platform/efi/efi.c:918:5: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
       (efi_memory_desc_t *)pa);
       ^

The @pa local variable is declared as phys_addr_t and that is a u64 when
CONFIG_PHYS_ADDR_T_64BIT=y. (The last is enabled on 32-bit on a PAE
build.)

However, its value comes from __pa() which is basically doing pointer
arithmetic and checking, and returns unsigned long as it is the native
pointer width.

So let's use an unsigned long too. It should be fine to do so because
the later users cast it to a pointer too.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
---
 arch/x86/platform/efi/efi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index bf99aa7005eb..936a488d6cf6 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -861,7 +861,7 @@ static void __init __efi_enter_virtual_mode(void)
 	int count = 0, pg_shift = 0;
 	void *new_memmap = NULL;
 	efi_status_t status;
-	phys_addr_t pa;
+	unsigned long pa;
 
 	efi.systab = NULL;
 
-- 
2.10.0


-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

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

* Re: [PATCH] x86/efi: Fix EFI memmap pointer size warning
  2016-11-06 13:02 [PATCH] x86/efi: Fix EFI memmap pointer size warning Borislav Petkov
@ 2016-11-07  9:59 ` Matt Fleming
  0 siblings, 0 replies; 2+ messages in thread
From: Matt Fleming @ 2016-11-07  9:59 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: linux-efi, lkml, Ard Biesheuvel

On Sun, 06 Nov, at 02:02:54PM, Borislav Petkov wrote:
> Hi Matt,
> 
> please doublecheck me on this but I think we're fine using an unsigned
> long.
> 
> Thanks.
> 
> ---
> From: Borislav Petkov <bp@suse.de>
> Date: Sun, 6 Nov 2016 13:49:10 +0100
> Subject: [PATCH] x86/efi: Fix EFI memmap pointer size warning
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Fix this when building on 32-bit:
> 
>   arch/x86/platform/efi/efi.c: In function ‘__efi_enter_virtual_mode’:
>   arch/x86/platform/efi/efi.c:911:5: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>        (efi_memory_desc_t *)pa);
>        ^
>   arch/x86/platform/efi/efi.c:918:5: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>        (efi_memory_desc_t *)pa);
>        ^
> 
> The @pa local variable is declared as phys_addr_t and that is a u64 when
> CONFIG_PHYS_ADDR_T_64BIT=y. (The last is enabled on 32-bit on a PAE
> build.)
> 
> However, its value comes from __pa() which is basically doing pointer
> arithmetic and checking, and returns unsigned long as it is the native
> pointer width.
> 
> So let's use an unsigned long too. It should be fine to do so because
> the later users cast it to a pointer too.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> ---
>  arch/x86/platform/efi/efi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index bf99aa7005eb..936a488d6cf6 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -861,7 +861,7 @@ static void __init __efi_enter_virtual_mode(void)
>  	int count = 0, pg_shift = 0;
>  	void *new_memmap = NULL;
>  	efi_status_t status;
> -	phys_addr_t pa;
> +	unsigned long pa;
>  
>  	efi.systab = NULL;
>  

Right, this does look correct given that __pa() uses 'unsigned long'.

I think the reason this is safe on PAE is that __get_free_pages() is
guaranteed to return a 32-bit address, and will not return HIGHMEM
pages. Which makes __pa() legal and 'unsigned long' the correct type.

Want me to take this through the EFI tree?

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

end of thread, other threads:[~2016-11-07  9:59 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-06 13:02 [PATCH] x86/efi: Fix EFI memmap pointer size warning Borislav Petkov
2016-11-07  9:59 ` Matt Fleming

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