EFI changes for v4.2

Hi folks,

Please pull the following new material for v4.2. The most interesting change is support for the EFI System Resource Table contributed by Peter Jones. ESRT is new in the UEFI 2.5 specification (released April 2015) and is used by Fedora's fwupdate tool when performing firmware updates.

The following changes since commit b787f68c36d49bb1d9236f403813641efa74a031:

Linux 4.1-rc1 (2015-04-26 17:59:10 -0700)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git tags/efi-next

for you to fetch changes up to c208358c2cc832eeb5b341a465eee30be19cd5a0:

efi: Add 'systab' information to Documentation/ABI (2015-05-27 15:40:20 +0100)

----------------------------------------------------------------
* Use idiomatic negative error values in efivar_create_sysfs_entry() instead of returning '1' to indicate error - Dan Carpenter

* New support to expose the EFI System Resource Tables in sysfs, which provides information for performing firmware updates - Peter Jones

* Documentation cleanup in the EFI handover protocol section which falsely claimed that 'cmdline_size' needed to be filled out by the boot loader - Alex Smith

* Align the order of SMBIOS tables in /sys/firmware/efi/systab to match the way that we do things for ACPI and add documentation to Documentation/ABI - Jean Delvare

----------------------------------------------------------------
Alex Smith (1):
x86, doc: Remove cmdline_size from list of fields to be filled in for EFI handover

Dan Carpenter (1):
efi: efivar_create_sysfs_entry() should return negative error codes

Jean Delvare (1):
efi: dmi: List SMBIOS3 table before SMBIOS table

Matt Fleming (1):
efi: Add 'systab' information to Documentation/ABI

Peter Jones (2):
efi: Add esrt support
efi/esrt: Fix some compiler warnings

 Documentation/ABI/testing/sysfs-firmware-efi      |  10 +
 Documentation/ABI/testing/sysfs-firmware-efi-esrt |  81 ++++
 Documentation/x86/boot.txt                        |   1 -
 arch/x86/platform/efi/efi.c                       |   2 +
 drivers/firmware/efi/Makefile                     |   2 +-
 drivers/firmware/efi/efi.c                        |  91 ++++-
 drivers/firmware/efi/efivars.c                    |  11 +-
 drivers/firmware/efi/esrt.c                       | 471 ++++++++++++++++++++++
 include/linux/efi.h                               |   8 +
 9 files changed, 667 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-firmware-efi-esrt
 create mode 100644 drivers/firmware/efi/esrt.c

Pulled, thanks a lot Matt!

I'm wondering about this commit:

f2f6b587c553 ("efi/esrt: Fix some compiler warnings")

It says, in part:

This patch also fixes one other warning about an uninitialized variable some compiler versions seem to see. You can't actually hit the code path where it would be uninitialized, because there's a prior test that would error out, but gcc hasn't figured that out. Anyway, it now has a test and returns the error at both places.

(quoted diff) +6 -5

@@ -167,7 +167,6 @@ static struct kset *esrt_kset;
 
 static int esre_create_sysfs_entry(void *esre, int entry_num)
 {
-       int rc = 0;
        struct esre_entry *entry;
        char name[20];
 

@@ -180,13 +179,15 @@ static int esre_create_sysfs_entry(void *esre, int entry_num)
        entry->kobj.kset = esrt_kset;
 
        if (esrt->fw_resource_version == 1) {
+               int rc = 0;
+
                entry->esre.esre1 = esre;
                rc = kobject_init_and_add(&entry->kobj, &esre1_ktype, NULL,
                                          "%s", name);
-       }
-       if (rc) {
-               kfree(entry);
-               return rc;
+               if (rc) {
+                       kfree(entry);
+                       return rc;
+               }
        }
 
        list_add_tail(&entry->list, &entry_list);

How can a compiler ever have warned about 'rc' being uninitialized? It's defined straight at function entry, with initialization to 0. It can never be uninitialized.

(quoted diff) +6 -5

@@ -167,7 +167,6 @@ static struct kset *esrt_kset;
 
 static int esre_create_sysfs_entry(void *esre, int entry_num)
 {
-       int rc = 0;
        struct esre_entry *entry;
        char name[20];
 

@@ -180,13 +179,15 @@ static int esre_create_sysfs_entry(void *esre, int entry_num)
        entry->kobj.kset = esrt_kset;
 
        if (esrt->fw_resource_version == 1) {
+               int rc = 0;
+
                entry->esre.esre1 = esre;
                rc = kobject_init_and_add(&entry->kobj, &esre1_ktype, NULL,
                                          "%s", name);
-       }
-       if (rc) {
-               kfree(entry);
-               return rc;
+               if (rc) {
+                       kfree(entry);
+                       return rc;
+               }
        }
 
        list_add_tail(&entry->list, &entry_list);

How can a compiler ever have warned about 'rc' being uninitialized? It's defined straight at function entry, with initialization to 0. It can never be uninitialized.

I pulled it, because I agree with the change itself, as it's always better to define and use variables in the narrowest scope possible, but I think it's a cleanup, not a compiler warning fix.

Quoted 29 lines · Ingo Molnar

(quoted diff) +6 -5

@@ -167,7 +167,6 @@ static struct kset *esrt_kset;
 
 static int esre_create_sysfs_entry(void *esre, int entry_num)
 {
-       int rc = 0;
        struct esre_entry *entry;
        char name[20];
 

@@ -180,13 +179,15 @@ static int esre_create_sysfs_entry(void *esre, int entry_num)
        entry->kobj.kset = esrt_kset;
 
        if (esrt->fw_resource_version == 1) {
+               int rc = 0;
+
                entry->esre.esre1 = esre;
                rc = kobject_init_and_add(&entry->kobj, &esre1_ktype, NULL,
                                          "%s", name);
-       }
-       if (rc) {
-               kfree(entry);
-               return rc;
+               if (rc) {
+                       kfree(entry);
+                       return rc;
+               }
        }
 
        list_add_tail(&entry->list, &entry_list);

Quoted 7 lines

How can a compiler ever have warned about 'rc' being uninitialized? It's defined straight at function entry, with initialization to 0. It can never be uninitialized.

I pulled it, because I agree with the change itself, as it's always better to define and use variables in the narrowest scope possible, but I think it's a cleanup, not a compiler warning fix.

Well, apparently I failed to explain it well - the warning was about "esre" rather than "rc". Basically before we were testing the version in register_entries() (i.e. this function's caller) and never calling the this function if it's not version 1. The compiler didn't figure out that when we set "entry->esre.esre1 = esre;", esre can not be null because the function wouldn't be called. Adding the explicit check on the version here silenced the warning about entry plausibly being NULL.

I'm guessing that this is because it's checking that the same conditional test is involved - that the initialization is in the same "...version == 1" test that the usage is. But that's just a guess.

Would you like another patch to add this email to the commit message, or do you want to add it in your tree, or what?