efi: Fix truncation of constant value
authorEugeniu Rosca <roscaeugeniu@gmail.com>
Sat, 14 Jul 2018 20:53:30 +0000 (22:53 +0200)
committerAlexander Graf <agraf@suse.de>
Mon, 20 Aug 2018 22:03:01 +0000 (00:03 +0200)
Starting with commit 867a6ac86dd8 ("efi: Add start-up library code"),
sparse constantly complains about truncated constant value in efi.h:

include/efi.h:176:35: warning: cast truncates bits from constant value (8000000000000000 becomes 0)

This can get quite noisy, preventing real issues to be noticed:

$ make defconfig
*** Default configuration is based on 'sandbox_defconfig'
$ make C=2 -j12 2>&1 | grep truncates | wc -l
441

After the patch is applied:
$ make C=2 -j12 2>&1 | grep truncates | wc -l
0
$ sparse --version
v0.5.2

Following the suggestion of Heinrich Schuchardt, instead of only
fixing the root-cause, I replaced the whole enum of _SHIFT values
by ULL defines. This matches both the UEFI 2.7 spec and the Linux
kernel implementation.

Some ELF size comparison before and after the patch (gcc 7.3.0):

efi-x86_payload64_defconfig:
text    data    bss     dec       hex   filename
407174  29432   278676  715282    aea12 u-boot.old
407152  29464   278676  715292    aea1c u-boot.new
-22     +32     0       +10

efi-x86_payload32_defconfig:
text    data    bss     dec       hex   filename
447075  30308   280076  757459    b8ed3 u-boot.old
447053  30340   280076  757469    b8edd u-boot.new
-22     +32     0       +10

Fixes: 867a6ac86dd8 ("efi: Add start-up library code")
Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Alexander Graf <agraf@suse.de>
cmd/efi.c
include/efi.h
lib/efi_loader/efi_memory.c

index 6c1eb88..92a565f 100644 (file)
--- a/cmd/efi.c
+++ b/cmd/efi.c
@@ -28,18 +28,18 @@ static const char *const type_name[] = {
 };
 
 static struct attr_info {
-       int shift;
+       u64 val;
        const char *name;
 } mem_attr[] = {
-       { EFI_MEMORY_UC_SHIFT, "uncached" },
-       { EFI_MEMORY_WC_SHIFT, "write-coalescing" },
-       { EFI_MEMORY_WT_SHIFT, "write-through" },
-       { EFI_MEMORY_WB_SHIFT, "write-back" },
-       { EFI_MEMORY_UCE_SHIFT, "uncached & exported" },
-       { EFI_MEMORY_WP_SHIFT, "write-protect" },
-       { EFI_MEMORY_RP_SHIFT, "read-protect" },
-       { EFI_MEMORY_XP_SHIFT, "execute-protect" },
-       { EFI_MEMORY_RUNTIME_SHIFT, "needs runtime mapping" }
+       { EFI_MEMORY_UC, "uncached" },
+       { EFI_MEMORY_WC, "write-coalescing" },
+       { EFI_MEMORY_WT, "write-through" },
+       { EFI_MEMORY_WB, "write-back" },
+       { EFI_MEMORY_UCE, "uncached & exported" },
+       { EFI_MEMORY_WP, "write-protect" },
+       { EFI_MEMORY_RP, "read-protect" },
+       { EFI_MEMORY_XP, "execute-protect" },
+       { EFI_MEMORY_RUNTIME, "needs runtime mapping" }
 };
 
 /* Maximum different attribute values we can track */
@@ -173,7 +173,7 @@ static void efi_print_mem_table(struct efi_entry_memmap *map,
                printf("%c%llx: ", attr & EFI_MEMORY_RUNTIME ? 'r' : ' ',
                       attr & ~EFI_MEMORY_RUNTIME);
                for (j = 0, first = true; j < ARRAY_SIZE(mem_attr); j++) {
-                       if (attr & (1ULL << mem_attr[j].shift)) {
+                       if (attr & mem_attr[j].val) {
                                if (first)
                                        first = false;
                                else
index 41530a7..cfdccd7 100644 (file)
@@ -170,20 +170,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 */
+#define EFI_MEMORY_WC          ((u64)0x0000000000000002ULL)    /* write-coalescing */
+#define EFI_MEMORY_WT          ((u64)0x0000000000000004ULL)    /* write-through */
+#define EFI_MEMORY_WB          ((u64)0x0000000000000008ULL)    /* write-back */
+#define EFI_MEMORY_UCE         ((u64)0x0000000000000010ULL)    /* uncached, exported */
+#define EFI_MEMORY_WP          ((u64)0x0000000000001000ULL)    /* write-protect */
+#define EFI_MEMORY_RP          ((u64)0x0000000000002000ULL)    /* read-protect */
+#define EFI_MEMORY_XP          ((u64)0x0000000000004000ULL)    /* execute-protect */
+#define EFI_MEMORY_RUNTIME     ((u64)0x8000000000000000ULL)    /* range requires runtime mapping */
+#define EFI_MEM_DESC_VERSION   1
 
 #define EFI_PAGE_SHIFT         12
 #define EFI_PAGE_SIZE          (1UL << EFI_PAGE_SHIFT)
index 3ee1079..e2b40aa 100644 (file)
@@ -178,14 +178,13 @@ uint64_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type,
        switch (memory_type) {
        case EFI_RUNTIME_SERVICES_CODE:
        case EFI_RUNTIME_SERVICES_DATA:
-               newlist->desc.attribute = (1 << EFI_MEMORY_WB_SHIFT) |
-                                         (1ULL << EFI_MEMORY_RUNTIME_SHIFT);
+               newlist->desc.attribute = EFI_MEMORY_WB | EFI_MEMORY_RUNTIME;
                break;
        case EFI_MMAP_IO:
-               newlist->desc.attribute = 1ULL << EFI_MEMORY_RUNTIME_SHIFT;
+               newlist->desc.attribute = EFI_MEMORY_RUNTIME;
                break;
        default:
-               newlist->desc.attribute = 1 << EFI_MEMORY_WB_SHIFT;
+               newlist->desc.attribute = EFI_MEMORY_WB;
                break;
        }