[U-Boot] [PATCH v2 1/3] efi: Fix truncation of constant value

[Heinrich Schuchardt]

On 07/16/2018 08:12 AM, Eugeniu Rosca wrote:
> Hi Heinrich,
> 
> Thanks for your review comments. See my reply below.
> 
> On Mon, Jul 16, 2018 at 07:52:20AM +0200, Heinrich Schuchardt wrote:
> [--snip--]
>>> diff --git a/include/efi.h b/include/efi.h
>>> index 0fe15e65c06c..eb2a569fe010 100644
>>> --- a/include/efi.h
>>> +++ b/include/efi.h
>>> @@ -162,20 +162,16 @@ enum efi_mem_type {
>>>   };
>>>   
>>>   /* Attribute values */
>>> -enum {
>>> -	EFI_MEMORY_UC_SHIFT	= 0,	/* uncached */
>>> -	EFI_MEMORY_WC_SHIFT	= 1,	/* write-coalescing */
>>> -	EFI_MEMORY_WT_SHIFT	= 2,	/* write-through */
>>> -	EFI_MEMORY_WB_SHIFT	= 3,	/* write-back */
>>> -	EFI_MEMORY_UCE_SHIFT	= 4,	/* uncached, exported */
>>> -	EFI_MEMORY_WP_SHIFT	= 12,	/* write-protect */
>>> -	EFI_MEMORY_RP_SHIFT	= 13,	/* read-protect */
>>> -	EFI_MEMORY_XP_SHIFT	= 14,	/* execute-protect */
>>> -	EFI_MEMORY_RUNTIME_SHIFT = 63,	/* range requires runtime mapping */
>>> -
>>> -	EFI_MEMORY_RUNTIME = 1ULL << EFI_MEMORY_RUNTIME_SHIFT,
>>> -	EFI_MEM_DESC_VERSION	= 1,
>>> -};
>>> +#define EFI_MEMORY_UC		((u64)0x0000000000000001ULL)	/* uncached */
>>
>> Unsigned long long int (ULL) is at least 64bit wide and unsigned (cf.
>> https://gcc.gnu.org/onlinedocs/gcc/Long-Long.html). Why do you introduce
>> the (u64) conversion?
> 
> Because this is how it appears in Linux kernel:
> 
> $ git blame ./include/linux/efi.h | grep 'define EFI_MEMORY_.*u64'
> ^1da177e4c3f4 (Linus Torvalds      2005-04-16 15:20:36 -0700   90) #define EFI_MEMORY_UC		((u64)0x0000000000000001ULL)	/* uncached */
> ^1da177e4c3f4 (Linus Torvalds      2005-04-16 15:20:36 -0700   91) #define EFI_MEMORY_WC		((u64)0x0000000000000002ULL)	/* write-coalescing */
> ^1da177e4c3f4 (Linus Torvalds      2005-04-16 15:20:36 -0700   92) #define EFI_MEMORY_WT		((u64)0x0000000000000004ULL)	/* write-through */
> ^1da177e4c3f4 (Linus Torvalds      2005-04-16 15:20:36 -0700   93) #define EFI_MEMORY_WB		((u64)0x0000000000000008ULL)	/* write-back */
> 9c97e0bdd4b4a (Laszlo Ersek        2014-09-03 13:32:19 +0200   94) #define EFI_MEMORY_UCE		((u64)0x0000000000000010ULL)	/* uncached, exported */
> ^1da177e4c3f4 (Linus Torvalds      2005-04-16 15:20:36 -0700   95) #define EFI_MEMORY_WP		((u64)0x0000000000001000ULL)	/* write-protect */
> ^1da177e4c3f4 (Linus Torvalds      2005-04-16 15:20:36 -0700   96) #define EFI_MEMORY_RP		((u64)0x0000000000002000ULL)	/* read-protect */
> ^1da177e4c3f4 (Linus Torvalds      2005-04-16 15:20:36 -0700   97) #define EFI_MEMORY_XP		((u64)0x0000000000004000ULL)	/* execute-protect */
> c016ca08f89c6 (Robert Elliott      2016-02-01 22:07:06 +0000   98) #define EFI_MEMORY_NV		((u64)0x0000000000008000ULL)	/* non-volatile */
> 87db73aebf555 (Ard Biesheuvel      2015-08-07 09:36:54 +0100  101) #define EFI_MEMORY_RO		((u64)0x0000000000020000ULL)	/* read-only */
> ^1da177e4c3f4 (Linus Torvalds      2005-04-16 15:20:36 -0700  102) #define EFI_MEMORY_RUNTIME	((u64)0x8000000000000000ULL)	/* range requires runtime mapping */
> 
> Unless we see a potential issue with that (and I don't see), IMO we
> should stick to kernel sources as much as possible, since this makes
> integration/re-sync process easier.
> 

Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>