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@gmx.de