efi: Constify all the things
Subsystems (10)
cover efi: Constify all the things
When disassembling the EFI stub I noticed that GUIDs and efi_char16_t strings are not placed in rodata but generated on the stack at runtime. GUIDs occupy up to 80 bytes of text instead of just 16 bytes rodata which annoyed me enough to go down the rabbit hole of fixing it. This series constifies all GUIDs and EFI-related strings I could find.
The most important patch is [10/10] to constify the EFI_GUID() macro. This has to be at the end of the series to avoid compiler warnings. Reviewers may want to read its commit message first as it contains background information. In particular it explains why this series only fixes the kernel side of things. Additional work is necessary on the compiler side to actually have all the GUIDs and strings in rodata. At the moment both gcc and clang achieve that only partially.
Ard Biesheuvel expressed an interest in these patches in November but stressed the importance that arguments to EFI boot/runtime/protocol services may only be declared const if the spec marks them "IN":
https://lkml.org/lkml/2016/11/21/459
Hence I've split the series by boot/runtime/protocol services to allow for easy verification of this fact. Constification of GetVariable() / SetVariable() is further split in one patch for the function signatures and another for the actual arguments because changing the function signatures is quite involved in this case.
efi.git/next needs to be rebased on v4.10-rc1 before this series can be applied because it requires commits f6697df36bdf ("x86/efi: Prevent mixed mode boot corruption with CONFIG_VMAP_STACK=y") and 2fbadc3002c5 ("arm/arm64: xen: Move shared architecture headers to include/xen/arm").
Unfortunately I was not able to compile-test on ia64, there's no cross- compiler package available on Debian. :-( I did not receive complaints from 0-day, so I'm cautiously optimistic that I didn't cause breakage there.
I've pushed the patches to GitHub to ease reviewing/fetching:
https://github.com/l1k/linux/commits/efi_const_v1
Thanks,
Lukas
Ard Biesheuvel (1):
efi: use typed function pointers for runtime services table
Lukas Wunner (9):
efi: Constify GetVariable() / SetVariable() signatures
efi: Constify GetVariable() / SetVariable() arguments
efi: Constify HandleProtocol() / LocateHandle() / LocateProtocol()
efi: Constify InstallConfigurationTable()
efi: Constify EFI_FILE_PROTOCOL.GetInfo()
efi: Constify EFI_RNG_PROTOCOL.GetRNG()
efi: Constify EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.OutputString()
efi: Constify efi_guidcmp() arguments
uuid: Constify UUID compound literals
arch/ia64/kernel/efi.c | 24 ++++------ arch/x86/boot/compressed/eboot.c | 50 ++++++++++----------- arch/x86/include/asm/xen/interface.h | 1 + arch/x86/platform/efi/efi_64.c | 18 ++++---- arch/x86/platform/efi/quirks.c | 2 +- drivers/firmware/efi/efi-pstore.c | 10 ++--- drivers/firmware/efi/efibc.c | 3 +- drivers/firmware/efi/libstub/arm-stub.c | 17 +++---- drivers/firmware/efi/libstub/arm32-stub.c | 2 +- drivers/firmware/efi/libstub/efi-stub-helper.c | 7 ++- drivers/firmware/efi/libstub/efistub.h | 2 +- drivers/firmware/efi/libstub/fdt.c | 3 +- drivers/firmware/efi/libstub/gop.c | 12 +++-- drivers/firmware/efi/libstub/random.c | 16 +++---- drivers/firmware/efi/runtime-wrappers.c | 15 ++++--- drivers/firmware/google/gsmi.c | 10 ++--- drivers/infiniband/hw/hfi1/efivar.c | 6 +-- drivers/scsi/isci/probe_roms.c | 2 +- drivers/xen/efi.c | 12 ++--- include/linux/efi.h | 61 ++++++++++++++------------ include/uapi/linux/uuid.h | 4 +- include/xen/arm/interface.h | 1 + include/xen/interface/platform.h | 11 ++++- include/xen/xen-ops.h | 12 ++--- 24 files changed, 143 insertions(+), 158 deletions(-)
Signature · 1 line
2.11.0
A message that is not in the archive (sent off-list or not yet received): cover.1483318593.git.lukas-JFq808J9C/[email protected]
PATCH 1/10 efi: use typed function pointers for runtime services table
From: Ard Biesheuvel <[email protected]>
Instead of using void pointers, and casting them to correctly typed function pointers upon use, declare the runtime services pointers as function pointers using their respective prototypes, for which typedefs are already available.
--- include/linux/efi.h | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-)
include/linux/efi.h +18 -18
@@ -509,24 +509,6 @@ struct efi_boot_memmap {u64 query_variable_info; } efi_runtime_services_64_t; -typedef struct { -efi_table_hdr_t hdr; -void *get_time; -void *set_time; -void *get_wakeup_time; -void *set_wakeup_time; -void *set_virtual_address_map; -void *convert_pointer; -void *get_variable; -void *get_next_variable; -void *set_variable; -void *get_next_high_mono_count; -void *reset_system; -void *update_capsule; -void *query_capsule_caps; -void *query_variable_info; -}efi_runtime_services_t ; -typedef efi_status_t efi_get_time_t (efi_time_t *tm,efi_time_cap_t *tc);typedef efi_status_t efi_set_time_t (efi_time_t *tm); typedef efi_status_t efi_get_wakeup_time_t (efi_bool_t *enabled, efi_bool_t *pending,
@@ -561,6 +543,24 @@ typedef efi_status_t efi_query_variable_store_t(u32 attributes,unsigned long size, bool nonblocking); +typedef struct { +efi_table_hdr_t hdr; +efi_get_time_t *get_time; +efi_set_time_t *set_time; +efi_get_wakeup_time_t *get_wakeup_time; +efi_set_wakeup_time_t *set_wakeup_time; +efi_set_virtual_address_map_t *set_virtual_address_map; +void *convert_pointer; +efi_get_variable_t *get_variable; +efi_get_next_variable_t *get_next_variable; +efi_set_variable_t *set_variable; +efi_get_next_high_mono_count_t *get_next_high_mono_count; +efi_reset_system_t *reset_system; +efi_update_capsule_t *update_capsule; +efi_query_capsule_caps_t *query_capsule_caps; +efi_query_variable_info_t *query_variable_info; +}efi_runtime_services_t ; +void efi_native_runtime_setup (void );/*
Signature · 1 line
2.11.0
PATCH 2/10 efi: Constify GetVariable() / SetVariable() signatures
In reply to a message not in the archiveGetVariable() / SetVariable() is typically called with immutable VendorGuid and VariableName arguments. SetVariable() is also often called with an immutable Data argument. However declaring these const in the caller results in a compiler warning since they're not marked const in the function signatures. Rectify that, in keeping with the EFI spec which marks these arguments "IN".
--- arch/ia64/kernel/efi.c | 9 ++++----- arch/x86/include/asm/xen/interface.h | 1 + arch/x86/platform/efi/efi_64.c | 18 +++++++++--------- drivers/firmware/efi/runtime-wrappers.c | 15 ++++++++------- drivers/firmware/google/gsmi.c | 10 +++++----- drivers/xen/efi.c | 12 ++++++------ include/linux/efi.h | 10 ++++++---- include/xen/arm/interface.h | 1 + include/xen/interface/platform.h | 11 +++++++++-- include/xen/xen-ops.h | 12 ++++++------ 10 files changed, 55 insertions(+), 44 deletions(-)
arch/ia64/kernel/efi.c +4 -5
@@ -125,8 +125,8 @@#define STUB_GET_VARIABLE (prefix, adjust_arg) \ static efi_status_t \ -prefix##_get_variable (efi_char16_t *name, efi_guid_t *vendor, u32 *attr, \ - unsigned long *data_size, void *data) \ +prefix##_get_variable (const efi_char16_t *name, const efi_guid_t *vendor, \ + u32 *attr, unsigned long *data_size, void *data) \ { \ struct ia64_fpreg fr[6]; \ u32 *aattr = NULL; \
@@ -161,9 +161,8 @@#define STUB_SET_VARIABLE (prefix, adjust_arg) \ static efi_status_t \ -prefix##_set_variable (efi_char16_t *name, efi_guid_t *vendor, \ - u32 attr, unsigned long data_size, \ - void *data) \ +prefix##_set_variable (const efi_char16_t *name, const efi_guid_t *vendor, \ + u32 attr, unsigned long data_size, const void *data) \ { \ struct ia64_fpreg fr[6]; \ efi_status_t ret; \
arch/x86/include/asm/xen/interface.h +1 -0
@@ -86,6 +86,7 @@/* Guest handles for primitive C types. */ __DEFINE_GUEST_HANDLE(uchar, unsigned char); __DEFINE_GUEST_HANDLE(uint, unsigned int); +__DEFINE_GUEST_HANDLE(cvoid, const void);DEFINE_GUEST_HANDLE (char);DEFINE_GUEST_HANDLE (int);DEFINE_GUEST_HANDLE (void);
arch/x86/platform/efi/efi_64.c +9 -9
@@ -632,13 +632,13 @@ static efi_status_t efi_thunk_set_time(efi_time_t *tm)return status; } -static unsigned long efi_name_size (efi_char16_t *name) +static unsigned long efi_name_size (const efi_char16_t *name) {return ucs2_strsize (name,EFI_VAR_NAME_LEN ) +1 ; }static efi_status_t -efi_thunk_get_variable (efi_char16_t *name,efi_guid_t *vendor, +efi_thunk_get_variable (const efi_char16_t *name,const efi_guid_t *vendor,u32 *attr,unsigned long *data_size,void *data) {efi_status_t status;
@@ -646,8 +646,8 @@ static unsigned long efi_name_size(efi_char16_t *name)u32 phys_data_size, phys_data; phys_data_size =virt_to_phys_or_null (data_size); - phys_vendor =virt_to_phys_or_null (vendor); - phys_name =virt_to_phys_or_null_size (name,efi_name_size (name)); + phys_vendor =virt_to_phys_or_null ((void *)vendor); + phys_name =virt_to_phys_or_null_size ((void *)name,efi_name_size (name)); phys_attr =virt_to_phys_or_null (attr); phys_data =virt_to_phys_or_null_size (data, *data_size);
@@ -658,15 +658,15 @@ static unsigned long efi_name_size(efi_char16_t *name) }static efi_status_t -efi_thunk_set_variable (efi_char16_t *name,efi_guid_t *vendor, -u32 attr,unsigned long data_size,void *data) +efi_thunk_set_variable (const efi_char16_t *name,const efi_guid_t *vendor, +u32 attr,unsigned long data_size,const void *data) {u32 phys_name, phys_vendor, phys_data;efi_status_t status; - phys_name =virt_to_phys_or_null_size (name,efi_name_size (name)); - phys_vendor =virt_to_phys_or_null (vendor); - phys_data =virt_to_phys_or_null_size (data, data_size); + phys_name =virt_to_phys_or_null_size ((void *)name,efi_name_size (name)); + phys_vendor =virt_to_phys_or_null ((void *)vendor); + phys_data =virt_to_phys_or_null_size ((void *)data, data_size);/* If data_size is > sizeof(u32) we've got problems */ status = efi_thunk(set_variable, phys_name, phys_vendor,
drivers/firmware/efi/runtime-wrappers.c +8 -7
@@ -136,8 +136,8 @@ static efi_status_t virt_efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)return status; } -static efi_status_t virt_efi_get_variable (efi_char16_t *name, -efi_guid_t *vendor, +static efi_status_t virt_efi_get_variable (const efi_char16_t *name, +const efi_guid_t *vendor,u32 *attr,unsigned long *data_size,void *data)
@@ -165,11 +165,11 @@ static efi_status_t virt_efi_get_next_variable(unsigned long *name_size,return status; } -static efi_status_t virt_efi_set_variable (efi_char16_t *name, -efi_guid_t *vendor, +static efi_status_t virt_efi_set_variable (const efi_char16_t *name, +const efi_guid_t *vendor,u32 attr,unsigned long data_size, -void *data) +const void *data) {efi_status_t status;
@@ -182,9 +182,10 @@ static efi_status_t virt_efi_set_variable(efi_char16_t *name, }static efi_status_t -virt_efi_set_variable_nonblocking (efi_char16_t *name,efi_guid_t *vendor, +virt_efi_set_variable_nonblocking (const efi_char16_t *name, +const efi_guid_t *vendor,u32 attr,unsigned long data_size, -void *data) +const void *data) {efi_status_t status;
drivers/firmware/google/gsmi.c +5 -5
@@ -289,8 +289,8 @@ static int gsmi_exec(u8 func, u8 sub)return rc; } -static efi_status_t gsmi_get_variable (efi_char16_t *name, -efi_guid_t *vendor,u32 *attr, +static efi_status_t gsmi_get_variable (const efi_char16_t *name, +const efi_guid_t *vendor,u32 *attr,unsigned long *data_size,void *data) {
@@ -410,11 +410,11 @@ static efi_status_t gsmi_get_next_variable(unsigned long *name_size,return ret; } -static efi_status_t gsmi_set_variable (efi_char16_t *name, -efi_guid_t *vendor, +static efi_status_t gsmi_set_variable (const efi_char16_t *name, +const efi_guid_t *vendor,u32 attr,unsigned long data_size, -void *data) +const void *data) {struct gsmi_nvram_var_param param = { .name_ptr = gsmi_dev.name_buf->address,
drivers/xen/efi.c +6 -6
@@ -117,9 +117,9 @@ efi_status_t xen_efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm) }EXPORT_SYMBOL_GPL (xen_efi_set_wakeup_time); -efi_status_t xen_efi_get_variable (efi_char16_t *name,efi_guid_t *vendor, -u32 *attr,unsigned long *data_size, -void *data) +efi_status_t xen_efi_get_variable (const efi_char16_t *name, +const efi_guid_t *vendor,u32 *attr, +unsigned long *data_size,void *data) {struct xen_platform_op op =INIT_EFI_OP (get_variable);
@@ -165,9 +165,9 @@ efi_status_t xen_efi_get_next_variable(unsigned long *name_size, }EXPORT_SYMBOL_GPL (xen_efi_get_next_variable); -efi_status_t xen_efi_set_variable (efi_char16_t *name,efi_guid_t *vendor, -u32 attr,unsigned long data_size, -void *data) +efi_status_t xen_efi_set_variable (const efi_char16_t *name, +const efi_guid_t *vendor,u32 attr, +unsigned long data_size,const void *data) {struct xen_platform_op op =INIT_EFI_OP (set_variable);
include/linux/efi.h +6 -4
@@ -514,13 +514,15 @@ struct efi_boot_memmap {typedef efi_status_t efi_get_wakeup_time_t (efi_bool_t *enabled,efi_bool_t *pending,efi_time_t *tm);typedef efi_status_t efi_set_wakeup_time_t (efi_bool_t enabled,efi_time_t *tm); -typedef efi_status_t efi_get_variable_t (efi_char16_t *name,efi_guid_t *vendor,u32 *attr, +typedef efi_status_t efi_get_variable_t (const efi_char16_t *name, +const efi_guid_t *vendor,u32 *attr,unsigned long *data_size,void *data);typedef efi_status_t efi_get_next_variable_t (unsigned long *name_size,efi_char16_t *name,efi_guid_t *vendor); -typedef efi_status_t efi_set_variable_t (efi_char16_t *name,efi_guid_t *vendor, -u32 attr,unsigned long data_size, -void *data); +typedef efi_status_t efi_set_variable_t (const efi_char16_t *name, +const efi_guid_t *vendor,u32 attr, +unsigned long data_size, +const void *data);typedef efi_status_t efi_get_next_high_mono_count_t (u32 *count);typedef void efi_reset_system_t (int reset_type,efi_status_t status,unsigned long data_size,efi_char16_t *data);
include/xen/arm/interface.h +1 -0
@@ -47,6 +47,7 @@/* Guest handles for primitive C types. */ __DEFINE_GUEST_HANDLE(uchar, unsigned char); __DEFINE_GUEST_HANDLE(uint, unsigned int); +__DEFINE_GUEST_HANDLE(cvoid, const void);DEFINE_GUEST_HANDLE (char);DEFINE_GUEST_HANDLE (int);DEFINE_GUEST_HANDLE (void);
include/xen/interface/platform.h +9 -2
@@ -171,7 +171,7 @@ struct xenpf_efi_runtime_call {#define XEN_EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002#define XEN_EFI_VARIABLE_RUNTIME_ACCESS 0x00000004 struct { - GUEST_HANDLE(void) name; /* UCS-2/UTF-16 string */ + GUEST_HANDLE(cvoid) name; /* UCS-2/UTF-16 string */ xen_ulong_t size; GUEST_HANDLE(void) data; struct xenpf_efi_guid {
@@ -180,7 +180,14 @@ struct xenpf_efi_runtime_call {uint16_t data3;uint8_t data4[8 ]; } vendor_guid; - } get_variable, set_variable; + } get_variable; + +struct { +GUEST_HANDLE (cvoid) name;/* UCS-2/UTF-16 string */ +xen_ulong_t size; +GUEST_HANDLE (cvoid) data; +struct xenpf_efi_guid vendor_guid; + } set_variable;struct {xen_ulong_t size;
include/xen/xen-ops.h +6 -6
@@ -103,14 +103,14 @@ int xen_xlate_map_ballooned_pages(xen_pfn_t **pfns, void **vaddr,efi_status_t xen_efi_get_wakeup_time (efi_bool_t *enabled,efi_bool_t *pending,efi_time_t *tm);efi_status_t xen_efi_set_wakeup_time (efi_bool_t enabled,efi_time_t *tm); -efi_status_t xen_efi_get_variable (efi_char16_t *name,efi_guid_t *vendor, -u32 *attr,unsigned long *data_size, -void *data); +efi_status_t xen_efi_get_variable (const efi_char16_t *name, +const efi_guid_t *vendor,u32 *attr, +unsigned long *data_size,void *data);efi_status_t xen_efi_get_next_variable (unsigned long *name_size,efi_char16_t *name,efi_guid_t *vendor); -efi_status_t xen_efi_set_variable (efi_char16_t *name,efi_guid_t *vendor, -u32 attr,unsigned long data_size, -void *data); +efi_status_t xen_efi_set_variable (const efi_char16_t *name, +const efi_guid_t *vendor,u32 attr, +unsigned long data_size,const void *data);efi_status_t xen_efi_query_variable_info (u32 attr,u64 *storage_space,u64 *remaining_space,u64 *max_variable_size);
Signature · 1 line
2.11.0
A message that is not in the archive (sent off-list or not yet received): bd030e11cd114fb1d568b21eddfc1fa75cd42e5e.1483318593.git.lukas-JFq808J9C/[email protected]
Juergen Gross
Acked-byRe: [PATCH 02/10] efi: Constify GetVariable() / SetVariable() signatures
Quoted 7 lines · Lukas Wunner
GetVariable()/SetVariable()is typically called with immutable VendorGuid and VariableName arguments.SetVariable()is also often called with an immutable Data argument. However declaring these const in the caller results in a compiler warning since they're not marked const in the function signatures. Rectify that, in keeping with the EFI spec which marks these arguments "IN".
Quoted 8 lines
Cc Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/[email protected]>Cc Ard Biesheuvel <[email protected]>Cc Tony Luck <[email protected]>Cc Fenghua Yu <[email protected]>Cc Boris Ostrovsky <[email protected]>Cc David Vrabel <[email protected]>Cc Juergen Gross <jgross-IBi9RG/[email protected]>Signed-off-by Lukas Wunner <lukas-JFq808J9C/[email protected]>
Xen parts:
Juergen
PATCH 3/10 efi: Constify GetVariable() / SetVariable() arguments
In reply to a message not in the archiveGUIDs and variable names passed to GetVariable() / SetVariable() are not modified, so declare them const. This allows the compiler to place them in the rodata section instead of generating them on the stack at runtime. Pass GUIDs as literals rather than assigning them to temporary variables to save a bit on code.
--- arch/ia64/kernel/efi.c | 15 ++++----------- arch/x86/platform/efi/quirks.c | 2 +- drivers/firmware/efi/efi-pstore.c | 4 ++-- drivers/firmware/efi/efibc.c | 3 +-- drivers/firmware/efi/libstub/arm-stub.c | 5 ++--- drivers/infiniband/hw/hfi1/efivar.c | 6 +----- drivers/scsi/isci/probe_roms.c | 2 +- 7 files changed, 12 insertions(+), 25 deletions(-)
arch/ia64/kernel/efi.c +4 -11
@@ -919,22 +919,15 @@ static void __init handle_palo(unsigned long phys_addr)efi_uart_console_only (void) {efi_status_t status; -char *s, name[] ="ConOut" ; -efi_guid_t guid =EFI_GLOBAL_VARIABLE_GUID ; -efi_char16_t *utf16, name_utf16[32 ]; +const char name[] ="ConOut" ; +const efi_char16_t name_utf16[] = {'C' ,'o' ,'n' ,'O' ,'u' ,'t' ,0 };unsigned char data[1024 ];unsigned long size =sizeof (data);struct efi_generic_dev_path *hdr, *end_addr;int uart =0 ; -/* Convert to UTF-16 */ - utf16 = name_utf16; - s = name; -while (*s) - *utf16++ = *s++ &0x7f ; - *utf16 =0 ; - - status = efi.get_variable (name_utf16, &guid,NULL , &size, data); + status = efi.get_variable (name_utf16, &EFI_GLOBAL_VARIABLE_GUID , +NULL , &size, data);if (status !=EFI_SUCCESS ) {printk (KERN_ERR "No EFI %s variable?\n" , name);return 0 ;
arch/x86/platform/efi/quirks.c +1 -1
@@ -19,7 +19,7 @@#define EFI_DUMMY_GUID \ EFI_GUID(0x4424ac57, 0xbe4b, 0x47dd, 0x9e, 0x97, 0xed, 0x50, 0xf0, 0x9f, 0x92, 0xa9) -static efi_char16_t efi_dummy_name[6 ] = {'D' ,'U' ,'M' ,'M' ,'Y' ,0 }; +static const efi_char16_t efi_dummy_name[6 ] = {'D' ,'U' ,'M' ,'M' ,'Y' ,0 };static bool efi_no_storage_paranoia;
drivers/firmware/efi/efi-pstore.c +2 -2
@@ -265,7 +265,6 @@ static int efi_pstore_write(enum pstore_type_id type, {char name[DUMP_NAME_LEN ];efi_char16_t efi_name[DUMP_NAME_LEN ]; -efi_guid_t vendor =LINUX_EFI_CRASH_GUID ;int i, ret =0 ; sprintf(name, "dump-type%u-%u-%d-%lu-%c", type, part, count,
@@ -274,7 +273,8 @@ static int efi_pstore_write(enum pstore_type_id type,for (i =0 ; i <DUMP_NAME_LEN ; i++) efi_name[i] = name[i]; -efivar_entry_set_safe (efi_name, vendor,PSTORE_EFI_ATTRIBUTES , +efivar_entry_set_safe (efi_name,LINUX_EFI_CRASH_GUID , +PSTORE_EFI_ATTRIBUTES , !pstore_cannot_block_path (reason), size, psi->buf);
drivers/firmware/efi/efibc.c +1 -2
@@ -32,7 +32,6 @@ static void efibc_str_to_str16(const char *str, efi_char16_t *str16)static int efibc_set_variable (const char *name,const char *value) {int ret; -efi_guid_t guid =LINUX_EFI_LOADER_ENTRY_GUID ;struct efivar_entry *entry;size_t size = (strlen (value) +1 ) *sizeof (efi_char16_t);
@@ -49,7 +48,7 @@ static int efibc_set_variable(const char *name, const char *value)efibc_str_to_str16 (name, entry->var.VariableName);efibc_str_to_str16 (value, (efi_char16_t *)entry->var.Data); -memcpy (&entry->var.VendorGuid, &guid,sizeof (guid)); + entry->var.VendorGuid =LINUX_EFI_LOADER_ENTRY_GUID ; ret =efivar_entry_set (entry, EFI_VARIABLE_NON_VOLATILE
drivers/firmware/efi/libstub/arm-stub.c +2 -3
@@ -27,13 +27,12 @@ static int efi_get_secureboot(efi_system_table_t *sys_table_arg)static efi_char16_t const sm_var_name[] = {'S' ,'e' ,'t' ,'u' ,'p' ,'M' ,'o' ,'d' ,'e' ,0 }; -efi_guid_t var_guid =EFI_GLOBAL_VARIABLE_GUID ;efi_get_variable_t *f_getvar = sys_table_arg->runtime->get_variable;u8 val;unsigned long size =sizeof (val);efi_status_t status; - status =f_getvar ((efi_char16_t *)sb_var_name, (efi_guid_t *)&var_guid, + status =f_getvar (sb_var_name, &EFI_GLOBAL_VARIABLE_GUID ,NULL , &size, &val);if (status !=EFI_SUCCESS )
@@ -42,7 +41,7 @@ static int efi_get_secureboot(efi_system_table_t *sys_table_arg)if (val ==0 )return 0 ; - status =f_getvar ((efi_char16_t *)sm_var_name, (efi_guid_t *)&var_guid, + status =f_getvar (sm_var_name, &EFI_GLOBAL_VARIABLE_GUID ,NULL , &size, &val);if (status !=EFI_SUCCESS )
drivers/infiniband/hw/hfi1/efivar.c +1 -5
@@ -66,7 +66,6 @@ static int read_efi_var(const char *name, unsigned long *size, {efi_status_t status;efi_char16_t *uni_name; -efi_guid_t guid;unsigned long temp_size;void *temp_buffer;void *data;
@@ -95,13 +94,10 @@ static int read_efi_var(const char *name, unsigned long *size,for (i =0 ; name[i]; i++) uni_name[i] = name[i]; -/* need a variable for our GUID */ - guid =HFI1_EFIVAR_GUID ; -/* call into EFI runtime services */ status = efi.get_variable ( uni_name, - &guid, + &HFI1_EFIVAR_GUID ,NULL , &temp_size, temp_buffer);
drivers/scsi/isci/probe_roms.c +1 -1
@@ -34,7 +34,7 @@#include "task.h" #include "probe_roms.h" -static efi_char16_t isci_efivar_name[] = { +static const efi_char16_t isci_efivar_name[] = {'R' ,'s' ,'t' ,'S' ,'c' ,'u' ,'O' };
Signature · 1 line
2.11.0
A message that is not in the archive (sent off-list or not yet received): 2be92e87ff7855bcd88c86a7436f46d3488483f0.1483318593.git.lukas-JFq808J9C/[email protected]
Dennis Dalessandro
Acked-byRe: [PATCH 03/10] efi: Constify GetVariable() / SetVariable() arguments
For the hfi1 bits below:
Quoted 27 lines · Lukas Wunner
iff --git
a/drivers/infiniband/hw/hfi1/efivar.cb/drivers/infiniband/hw/hfi1/efivar.cindex 106349f..f769cc2 100644drivers/infiniband/hw/hfi1/efivar.c +1
-5@@ -66,7 +66,6 @@ static int read_efi_var(const char *name, unsigned long *size, {efi_status_t status;efi_char16_t *uni_name; -efi_guid_t guid;unsigned long temp_size;void *temp_buffer;void *data;@@ -95,13 +94,10 @@ static int read_efi_var(const char *name, unsigned long *size,for (i =0 ; name[i]; i++) uni_name[i] = name[i]; -/* need a variable for our GUID */ - guid =HFI1_EFIVAR_GUID ; -/* call into EFI runtime services */ status = efi.get_variable ( uni_name, - &guid, + &HFI1_EFIVAR_GUID ,NULL , &temp_size, temp_buffer);
Re: [PATCH 03/10] efi: Constify GetVariable() / SetVariable() arguments
In reply to a message not in the archiveQuoted 32 lines · Lukas Wunner
GUIDs and variable names passed to
GetVariable()/SetVariable()are not modified, so declare them const. This allows the compiler to place them in the rodata section instead of generating them on the stack at runtime. Pass GUIDs as literals rather than assigning them to temporary variables to save a bit on code.Cc Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/[email protected]>Cc Ard Biesheuvel <[email protected]>Cc Tony Luck <[email protected]>Cc Fenghua Yu <[email protected]>Cc Anton Vorontsov <[email protected]>Cc Colin Cross <[email protected]>Cc Kees Cook <[email protected]>Cc Artur Paszkiewicz <[email protected]>Cc Mike Marciniszyn <[email protected]>Cc Dennis Dalessandro <[email protected]>Signed-off-by Lukas Wunner <lukas-JFq808J9C/[email protected]>--- arch/ia64/kernel/efi.c | 15 ++++----------- arch/x86/platform/efi/quirks.c | 2 +- drivers/firmware/efi/efi-pstore.c | 4 ++-- drivers/firmware/efi/efibc.c | 3 +-- drivers/firmware/efi/libstub/arm-stub.c | 5 ++--- drivers/infiniband/hw/hfi1/efivar.c | 6 +----- drivers/scsi/isci/probe_roms.c | 2 +- 7 files changed, 12 insertions(+), 25 deletions(-)arch/ia64/kernel/efi.c +0
-0
arch/ia64/kernel/efi.c static void __init handle_palo(unsigned long phys_addr) · line 923
Quoted 8 lines
arch/ia64/kernel/efi.c +2
-3@@ -919,22 +919,15 @@ static void __init handle_palo(unsigned long phys_addr)efi_uart_console_only (void) {efi_status_t status; -char *s, name[] ="ConOut" ; -efi_guid_t guid =EFI_GLOBAL_VARIABLE_GUID ; -efi_char16_t *utf16, name_utf16[32 ]; +const char name[] ="ConOut" ; +const efi_char16_t name_utf16[] = {'C' ,'o' ,'n' ,'O' ,'u' ,'t' ,0 };
I know you mention 'static' in 10/10, but I don't understand why you don't make these static to begin with. That also allows you to annotate them as __initconst, which is not possible for compound initializers (including the GUID)
Quoted 145 lines
arch/ia64/kernel/efi.c +2
-8unsigned char data[1024 ];unsigned long size =sizeof (data);struct efi_generic_dev_path *hdr, *end_addr;int uart =0 ; -/* Convert to UTF-16 */ - utf16 = name_utf16; - s = name; -while (*s) - *utf16++ = *s++ &0x7f ; - *utf16 =0 ; - - status = efi.get_variable (name_utf16, &guid,NULL , &size, data); + status = efi.get_variable (name_utf16, &EFI_GLOBAL_VARIABLE_GUID , +NULL , &size, data);if (status !=EFI_SUCCESS ) {printk (KERN_ERR "No EFI %s variable?\n" , name);return 0 ;arch/x86/platform/efi/quirks.c +1
-1@@ -19,7 +19,7 @@#define EFI_DUMMY_GUID \ EFI_GUID(0x4424ac57, 0xbe4b, 0x47dd, 0x9e, 0x97, 0xed, 0x50, 0xf0, 0x9f, 0x92, 0xa9) -static efi_char16_t efi_dummy_name[6 ] = {'D' ,'U' ,'M' ,'M' ,'Y' ,0 }; +static const efi_char16_t efi_dummy_name[6 ] = {'D' ,'U' ,'M' ,'M' ,'Y' ,0 };static bool efi_no_storage_paranoia;drivers/firmware/efi/efi-pstore.c +2
-2@@ -265,7 +265,6 @@ static int efi_pstore_write(enum pstore_type_id type, {char name[DUMP_NAME_LEN ];efi_char16_t efi_name[DUMP_NAME_LEN ]; -efi_guid_t vendor =LINUX_EFI_CRASH_GUID ;int i, ret =0 ; sprintf(name, "dump-type%u-%u-%d-%lu-%c", type, part, count,@@ -274,7 +273,8 @@ static int efi_pstore_write(enum pstore_type_id type,for (i =0 ; i <DUMP_NAME_LEN ; i++) efi_name[i] = name[i]; -efivar_entry_set_safe (efi_name, vendor,PSTORE_EFI_ATTRIBUTES , +efivar_entry_set_safe (efi_name,LINUX_EFI_CRASH_GUID , +PSTORE_EFI_ATTRIBUTES , !pstore_cannot_block_path (reason), size, psi->buf);drivers/firmware/efi/efibc.c +1
-2@@ -32,7 +32,6 @@ static void efibc_str_to_str16(const char *str, efi_char16_t *str16)static int efibc_set_variable (const char *name,const char *value) {int ret; -efi_guid_t guid =LINUX_EFI_LOADER_ENTRY_GUID ;struct efivar_entry *entry;size_t size = (strlen (value) +1 ) *sizeof (efi_char16_t);@@ -49,7 +48,7 @@ static int efibc_set_variable(const char *name, const char *value)efibc_str_to_str16 (name, entry->var.VariableName);efibc_str_to_str16 (value, (efi_char16_t *)entry->var.Data); -memcpy (&entry->var.VendorGuid, &guid,sizeof (guid)); + entry->var.VendorGuid =LINUX_EFI_LOADER_ENTRY_GUID ; ret =efivar_entry_set (entry, EFI_VARIABLE_NON_VOLATILEdrivers/firmware/efi/libstub/arm-stub.c +2
-3@@ -27,13 +27,12 @@ static int efi_get_secureboot(efi_system_table_t *sys_table_arg)static efi_char16_t const sm_var_name[] = {'S' ,'e' ,'t' ,'u' ,'p' ,'M' ,'o' ,'d' ,'e' ,0 }; -efi_guid_t var_guid =EFI_GLOBAL_VARIABLE_GUID ;efi_get_variable_t *f_getvar = sys_table_arg->runtime->get_variable;u8 val;unsigned long size =sizeof (val);efi_status_t status; - status =f_getvar ((efi_char16_t *)sb_var_name, (efi_guid_t *)&var_guid, + status =f_getvar (sb_var_name, &EFI_GLOBAL_VARIABLE_GUID ,NULL , &size, &val);if (status !=EFI_SUCCESS )@@ -42,7 +41,7 @@ static int efi_get_secureboot(efi_system_table_t *sys_table_arg)if (val ==0 )return 0 ; - status =f_getvar ((efi_char16_t *)sm_var_name, (efi_guid_t *)&var_guid, + status =f_getvar (sm_var_name, &EFI_GLOBAL_VARIABLE_GUID ,NULL , &size, &val);if (status !=EFI_SUCCESS )drivers/infiniband/hw/hfi1/efivar.c +1
-5@@ -66,7 +66,6 @@ static int read_efi_var(const char *name, unsigned long *size, {efi_status_t status;efi_char16_t *uni_name; -efi_guid_t guid;unsigned long temp_size;void *temp_buffer;void *data;@@ -95,13 +94,10 @@ static int read_efi_var(const char *name, unsigned long *size,for (i =0 ; name[i]; i++) uni_name[i] = name[i]; -/* need a variable for our GUID */ - guid =HFI1_EFIVAR_GUID ; -/* call into EFI runtime services */ status = efi.get_variable ( uni_name, - &guid, + &HFI1_EFIVAR_GUID ,NULL , &temp_size, temp_buffer);drivers/scsi/isci/probe_roms.c +1
-2@@ -34,7 +34,7 @@#include "task.h" #include "probe_roms.h" -static efi_char16_t isci_efivar_name[] = { +static const efi_char16_t isci_efivar_name[] = {'R' ,'s' ,'t' ,'S' ,'c' ,'u' ,'O' }; --2.11.0
A message that is not in the archive (sent off-list or not yet received): CAKv+Gu8UnAzQGsmyWYsWsOY3B+J9HXechwBX_Vk-rntBtO+Dug-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Re: [PATCH 03/10] efi: Constify GetVariable() / SetVariable() arguments
Quoted 38 lines · Ard Biesheuvel
Quoted 37 lines · Lukas Wunner
GUIDs and variable names passed to
GetVariable()/SetVariable()are not modified, so declare them const. This allows the compiler to place them in the rodata section instead of generating them on the stack at runtime. Pass GUIDs as literals rather than assigning them to temporary variables to save a bit on code.Cc Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/[email protected]>Cc Ard Biesheuvel <[email protected]>Cc Tony Luck <[email protected]>Cc Fenghua Yu <[email protected]>Cc Anton Vorontsov <[email protected]>Cc Colin Cross <[email protected]>Cc Kees Cook <[email protected]>Cc Artur Paszkiewicz <[email protected]>Cc Mike Marciniszyn <[email protected]>Cc Dennis Dalessandro <[email protected]>Signed-off-by Lukas Wunner <lukas-JFq808J9C/[email protected]>--- arch/ia64/kernel/efi.c | 15 ++++----------- arch/x86/platform/efi/quirks.c | 2 +- drivers/firmware/efi/efi-pstore.c | 4 ++-- drivers/firmware/efi/efibc.c | 3 +-- drivers/firmware/efi/libstub/arm-stub.c | 5 ++--- drivers/infiniband/hw/hfi1/efivar.c | 6 +----- drivers/scsi/isci/probe_roms.c | 2 +- 7 files changed, 12 insertions(+), 25 deletions(-)arch/ia64/kernel/efi.c +0
-2@@ -919,22 +919,15 @@ static void __init handle_palo(unsigned long phys_addr)efi_uart_console_only (void) {efi_status_t status; -char *s, name[] ="ConOut" ; -efi_guid_t guid =EFI_GLOBAL_VARIABLE_GUID ;
Quoted 8 lines
arch/ia64/kernel/efi.c static void __init handle_palo(unsigned long phys_addr) · line 923
Quoted 3 lines
arch/ia64/kernel/efi.c +2
-1-efi_char16_t *utf16, name_utf16[32 ]; +const char name[] ="ConOut" ; +const efi_char16_t name_utf16[] = {'C' ,'o' ,'n' ,'O' ,'u' ,'t' ,0 };I know you mention 'static' in 10/10, but I don't understand why you don't make these static to begin with. That also allows you to annotate them as
__initconst, which is not possible for compound initializers (including the GUID)
Right, I hadn't thought of __initconst. In this particular case indeed it might make sense to use "static const efi_guid_t guid __initconst = ...", and arguably for the strings as well. But where should I draw the line?
There are a handful of other GUIDs used in __init functions, e.g. in arch/ia64/ there's ESI_TABLE_GUID in kernel/esi.c:esi_init() and SAL_SYSTEM_TABLE_GUID in sn/kernel/setup.c:early_sn_setup(). This patch set results in them being placed in rodata, but not init.rodata. Should I declare a temporary "static const __initconst" variable for each of them? That would give us 16 bytes in each of the two cases being freed after kernel initialization.
However string literals aren't put in init.rodata either even though they're declared in an __init function. E.g. early_sn_setup() contains a string literal "failed to find SAL entry point\n". That's 31 bytes. esi_init() contains even 103 bytes of string literals.
So I'm not sure if it's worth to move the GUIDs to init.rodata (at the expense of an additional LoC to declare a temporary variable) even though we waste more space for string literals.
Thanks,
Lukas
Quoted 146 lines
Quoted 146 lines
arch/ia64/kernel/efi.c +2
-8unsigned char data[1024 ];unsigned long size =sizeof (data);struct efi_generic_dev_path *hdr, *end_addr;int uart =0 ; -/* Convert to UTF-16 */ - utf16 = name_utf16; - s = name; -while (*s) - *utf16++ = *s++ &0x7f ; - *utf16 =0 ; - - status = efi.get_variable (name_utf16, &guid,NULL , &size, data); + status = efi.get_variable (name_utf16, &EFI_GLOBAL_VARIABLE_GUID , +NULL , &size, data);if (status !=EFI_SUCCESS ) {printk (KERN_ERR "No EFI %s variable?\n" , name);return 0 ;arch/x86/platform/efi/quirks.c +1
-1@@ -19,7 +19,7 @@#define EFI_DUMMY_GUID \ EFI_GUID(0x4424ac57, 0xbe4b, 0x47dd, 0x9e, 0x97, 0xed, 0x50, 0xf0, 0x9f, 0x92, 0xa9) -static efi_char16_t efi_dummy_name[6 ] = {'D' ,'U' ,'M' ,'M' ,'Y' ,0 }; +static const efi_char16_t efi_dummy_name[6 ] = {'D' ,'U' ,'M' ,'M' ,'Y' ,0 };static bool efi_no_storage_paranoia;drivers/firmware/efi/efi-pstore.c +2
-2@@ -265,7 +265,6 @@ static int efi_pstore_write(enum pstore_type_id type, {char name[DUMP_NAME_LEN ];efi_char16_t efi_name[DUMP_NAME_LEN ]; -efi_guid_t vendor =LINUX_EFI_CRASH_GUID ;int i, ret =0 ; sprintf(name, "dump-type%u-%u-%d-%lu-%c", type, part, count,@@ -274,7 +273,8 @@ static int efi_pstore_write(enum pstore_type_id type,for (i =0 ; i <DUMP_NAME_LEN ; i++) efi_name[i] = name[i]; -efivar_entry_set_safe (efi_name, vendor,PSTORE_EFI_ATTRIBUTES , +efivar_entry_set_safe (efi_name,LINUX_EFI_CRASH_GUID , +PSTORE_EFI_ATTRIBUTES , !pstore_cannot_block_path (reason), size, psi->buf);drivers/firmware/efi/efibc.c +1
-2@@ -32,7 +32,6 @@ static void efibc_str_to_str16(const char *str, efi_char16_t *str16)static int efibc_set_variable (const char *name,const char *value) {int ret; -efi_guid_t guid =LINUX_EFI_LOADER_ENTRY_GUID ;struct efivar_entry *entry;size_t size = (strlen (value) +1 ) *sizeof (efi_char16_t);@@ -49,7 +48,7 @@ static int efibc_set_variable(const char *name, const char *value)efibc_str_to_str16 (name, entry->var.VariableName);efibc_str_to_str16 (value, (efi_char16_t *)entry->var.Data); -memcpy (&entry->var.VendorGuid, &guid,sizeof (guid)); + entry->var.VendorGuid =LINUX_EFI_LOADER_ENTRY_GUID ; ret =efivar_entry_set (entry, EFI_VARIABLE_NON_VOLATILEdrivers/firmware/efi/libstub/arm-stub.c +2
-3@@ -27,13 +27,12 @@ static int efi_get_secureboot(efi_system_table_t *sys_table_arg)static efi_char16_t const sm_var_name[] = {'S' ,'e' ,'t' ,'u' ,'p' ,'M' ,'o' ,'d' ,'e' ,0 }; -efi_guid_t var_guid =EFI_GLOBAL_VARIABLE_GUID ;efi_get_variable_t *f_getvar = sys_table_arg->runtime->get_variable;u8 val;unsigned long size =sizeof (val);efi_status_t status; - status =f_getvar ((efi_char16_t *)sb_var_name, (efi_guid_t *)&var_guid, + status =f_getvar (sb_var_name, &EFI_GLOBAL_VARIABLE_GUID ,NULL , &size, &val);if (status !=EFI_SUCCESS )@@ -42,7 +41,7 @@ static int efi_get_secureboot(efi_system_table_t *sys_table_arg)if (val ==0 )return 0 ; - status =f_getvar ((efi_char16_t *)sm_var_name, (efi_guid_t *)&var_guid, + status =f_getvar (sm_var_name, &EFI_GLOBAL_VARIABLE_GUID ,NULL , &size, &val);if (status !=EFI_SUCCESS )drivers/infiniband/hw/hfi1/efivar.c +1
-5@@ -66,7 +66,6 @@ static int read_efi_var(const char *name, unsigned long *size, {efi_status_t status;efi_char16_t *uni_name; -efi_guid_t guid;unsigned long temp_size;void *temp_buffer;void *data;@@ -95,13 +94,10 @@ static int read_efi_var(const char *name, unsigned long *size,for (i =0 ; name[i]; i++) uni_name[i] = name[i]; -/* need a variable for our GUID */ - guid =HFI1_EFIVAR_GUID ; -/* call into EFI runtime services */ status = efi.get_variable ( uni_name, - &guid, + &HFI1_EFIVAR_GUID ,NULL , &temp_size, temp_buffer);drivers/scsi/isci/probe_roms.c +1
-2@@ -34,7 +34,7 @@#include "task.h" #include "probe_roms.h" -static efi_char16_t isci_efivar_name[] = { +static const efi_char16_t isci_efivar_name[] = {'R' ,'s' ,'t' ,'S' ,'c' ,'u' ,'O' }; --2.11.0
A message that is not in the archive (sent off-list or not yet received): 20170106101652.GB21850-JFq808J9C/[email protected]
Re: [PATCH 03/10] efi: Constify GetVariable() / SetVariable() arguments
Quoted 60 lines · Lukas Wunner
Quoted 38 lines · Ard Biesheuvel
Quoted 37 lines · Lukas Wunner
GUIDs and variable names passed to
GetVariable()/SetVariable()are not modified, so declare them const. This allows the compiler to place them in the rodata section instead of generating them on the stack at runtime. Pass GUIDs as literals rather than assigning them to temporary variables to save a bit on code.Cc Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/[email protected]>Cc Ard Biesheuvel <[email protected]>Cc Tony Luck <[email protected]>Cc Fenghua Yu <[email protected]>Cc Anton Vorontsov <[email protected]>Cc Colin Cross <[email protected]>Cc Kees Cook <[email protected]>Cc Artur Paszkiewicz <[email protected]>Cc Mike Marciniszyn <[email protected]>Cc Dennis Dalessandro <[email protected]>Signed-off-by Lukas Wunner <lukas-JFq808J9C/[email protected]>--- arch/ia64/kernel/efi.c | 15 ++++----------- arch/x86/platform/efi/quirks.c | 2 +- drivers/firmware/efi/efi-pstore.c | 4 ++-- drivers/firmware/efi/efibc.c | 3 +-- drivers/firmware/efi/libstub/arm-stub.c | 5 ++--- drivers/infiniband/hw/hfi1/efivar.c | 6 +----- drivers/scsi/isci/probe_roms.c | 2 +- 7 files changed, 12 insertions(+), 25 deletions(-)arch/ia64/kernel/efi.c +0
-2@@ -919,22 +919,15 @@ static void __init handle_palo(unsigned long phys_addr)efi_uart_console_only (void) {efi_status_t status; -char *s, name[] ="ConOut" ; -efi_guid_t guid =EFI_GLOBAL_VARIABLE_GUID ;Quoted 8 lines
arch/ia64/kernel/efi.c static void __init handle_palo(unsigned long phys_addr) · line 923
Quoted 3 lines
arch/ia64/kernel/efi.c +2
-1-efi_char16_t *utf16, name_utf16[32 ]; +const char name[] ="ConOut" ; +const efi_char16_t name_utf16[] = {'C' ,'o' ,'n' ,'O' ,'u' ,'t' ,0 };I know you mention 'static' in 10/10, but I don't understand why you don't make these static to begin with. That also allows you to annotate them as
__initconst, which is not possible for compound initializers (including the GUID)Right, I hadn't thought of
__initconst. In this particular case indeed it might make sense to use "static constefi_guid_tguid__initconst= ...", and arguably for the strings as well. But where should I draw the line?There are a handful of other GUIDs used in
__initfunctions, e.g. inarch/ia64/there'sESI_TABLE_GUIDin kernel/esi.c:esi_init() andSAL_SYSTEM_TABLE_GUIDin sn/kernel/setup.c:early_sn_setup(). This patch set results in them being placed in rodata, but not init.rodata. Should I declare a temporary "static const__initconst" variable for each of them? That would give us 16 bytes in each of the two cases being freed after kernel initialization.
Quoted 9 lines
However string literals aren't put in init.rodata either even though they're declared in an
__initfunction. E.g.early_sn_setup()contains a string literal "failed to find SAL entry point\n". That's 31 bytes.esi_init()contains even 103 bytes of string literals.So I'm not sure if it's worth to move the GUIDs to init.rodata (at the expense of an additional LoC to declare a temporary variable) even though we waste more space for string literals.
For the string literals I think there are some solutions possible involving compiler plugins. Kees?
Quoted 146 lines
Quoted 146 lines
Quoted 146 lines
arch/ia64/kernel/efi.c +2
-8unsigned char data[1024 ];unsigned long size =sizeof (data);struct efi_generic_dev_path *hdr, *end_addr;int uart =0 ; -/* Convert to UTF-16 */ - utf16 = name_utf16; - s = name; -while (*s) - *utf16++ = *s++ &0x7f ; - *utf16 =0 ; - - status = efi.get_variable (name_utf16, &guid,NULL , &size, data); + status = efi.get_variable (name_utf16, &EFI_GLOBAL_VARIABLE_GUID , +NULL , &size, data);if (status !=EFI_SUCCESS ) {printk (KERN_ERR "No EFI %s variable?\n" , name);return 0 ;arch/x86/platform/efi/quirks.c +1
-1@@ -19,7 +19,7 @@#define EFI_DUMMY_GUID \ EFI_GUID(0x4424ac57, 0xbe4b, 0x47dd, 0x9e, 0x97, 0xed, 0x50, 0xf0, 0x9f, 0x92, 0xa9) -static efi_char16_t efi_dummy_name[6 ] = {'D' ,'U' ,'M' ,'M' ,'Y' ,0 }; +static const efi_char16_t efi_dummy_name[6 ] = {'D' ,'U' ,'M' ,'M' ,'Y' ,0 };static bool efi_no_storage_paranoia;drivers/firmware/efi/efi-pstore.c +2
-2@@ -265,7 +265,6 @@ static int efi_pstore_write(enum pstore_type_id type, {char name[DUMP_NAME_LEN ];efi_char16_t efi_name[DUMP_NAME_LEN ]; -efi_guid_t vendor =LINUX_EFI_CRASH_GUID ;int i, ret =0 ; sprintf(name, "dump-type%u-%u-%d-%lu-%c", type, part, count,@@ -274,7 +273,8 @@ static int efi_pstore_write(enum pstore_type_id type,for (i =0 ; i <DUMP_NAME_LEN ; i++) efi_name[i] = name[i]; -efivar_entry_set_safe (efi_name, vendor,PSTORE_EFI_ATTRIBUTES , +efivar_entry_set_safe (efi_name,LINUX_EFI_CRASH_GUID , +PSTORE_EFI_ATTRIBUTES , !pstore_cannot_block_path (reason), size, psi->buf);drivers/firmware/efi/efibc.c +1
-2@@ -32,7 +32,6 @@ static void efibc_str_to_str16(const char *str, efi_char16_t *str16)static int efibc_set_variable (const char *name,const char *value) {int ret; -efi_guid_t guid =LINUX_EFI_LOADER_ENTRY_GUID ;struct efivar_entry *entry;size_t size = (strlen (value) +1 ) *sizeof (efi_char16_t);@@ -49,7 +48,7 @@ static int efibc_set_variable(const char *name, const char *value)efibc_str_to_str16 (name, entry->var.VariableName);efibc_str_to_str16 (value, (efi_char16_t *)entry->var.Data); -memcpy (&entry->var.VendorGuid, &guid,sizeof (guid)); + entry->var.VendorGuid =LINUX_EFI_LOADER_ENTRY_GUID ; ret =efivar_entry_set (entry, EFI_VARIABLE_NON_VOLATILEdrivers/firmware/efi/libstub/arm-stub.c +2
-3@@ -27,13 +27,12 @@ static int efi_get_secureboot(efi_system_table_t *sys_table_arg)static efi_char16_t const sm_var_name[] = {'S' ,'e' ,'t' ,'u' ,'p' ,'M' ,'o' ,'d' ,'e' ,0 }; -efi_guid_t var_guid =EFI_GLOBAL_VARIABLE_GUID ;efi_get_variable_t *f_getvar = sys_table_arg->runtime->get_variable;u8 val;unsigned long size =sizeof (val);efi_status_t status; - status =f_getvar ((efi_char16_t *)sb_var_name, (efi_guid_t *)&var_guid, + status =f_getvar (sb_var_name, &EFI_GLOBAL_VARIABLE_GUID ,NULL , &size, &val);if (status !=EFI_SUCCESS )@@ -42,7 +41,7 @@ static int efi_get_secureboot(efi_system_table_t *sys_table_arg)if (val ==0 )return 0 ; - status =f_getvar ((efi_char16_t *)sm_var_name, (efi_guid_t *)&var_guid, + status =f_getvar (sm_var_name, &EFI_GLOBAL_VARIABLE_GUID ,NULL , &size, &val);if (status !=EFI_SUCCESS )drivers/infiniband/hw/hfi1/efivar.c +1
-5@@ -66,7 +66,6 @@ static int read_efi_var(const char *name, unsigned long *size, {efi_status_t status;efi_char16_t *uni_name; -efi_guid_t guid;unsigned long temp_size;void *temp_buffer;void *data;@@ -95,13 +94,10 @@ static int read_efi_var(const char *name, unsigned long *size,for (i =0 ; name[i]; i++) uni_name[i] = name[i]; -/* need a variable for our GUID */ - guid =HFI1_EFIVAR_GUID ; -/* call into EFI runtime services */ status = efi.get_variable ( uni_name, - &guid, + &HFI1_EFIVAR_GUID ,NULL , &temp_size, temp_buffer);drivers/scsi/isci/probe_roms.c +1
-2@@ -34,7 +34,7 @@#include "task.h" #include "probe_roms.h" -static efi_char16_t isci_efivar_name[] = { +static const efi_char16_t isci_efivar_name[] = {'R' ,'s' ,'t' ,'S' ,'c' ,'u' ,'O' }; --2.11.0
A message that is not in the archive (sent off-list or not yet received): CAKv+Gu-izRwdrUu7zZLSnf_mjCwFFZ0AsyF5iMR49+UXkmpsgQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Re: [PATCH 03/10] efi: Constify GetVariable() / SetVariable() arguments
Quoted 66 lines · Ard Biesheuvel
Quoted 65 lines · Lukas Wunner
Quoted 38 lines · Ard Biesheuvel
Quoted 37 lines · Lukas Wunner
GUIDs and variable names passed to
GetVariable()/SetVariable()are not modified, so declare them const. This allows the compiler to place them in the rodata section instead of generating them on the stack at runtime. Pass GUIDs as literals rather than assigning them to temporary variables to save a bit on code.Cc Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/[email protected]>Cc Ard Biesheuvel <[email protected]>Cc Tony Luck <[email protected]>Cc Fenghua Yu <[email protected]>Cc Anton Vorontsov <[email protected]>Cc Colin Cross <[email protected]>Cc Kees Cook <[email protected]>Cc Artur Paszkiewicz <[email protected]>Cc Mike Marciniszyn <[email protected]>Cc Dennis Dalessandro <[email protected]>Signed-off-by Lukas Wunner <lukas-JFq808J9C/[email protected]>--- arch/ia64/kernel/efi.c | 15 ++++----------- arch/x86/platform/efi/quirks.c | 2 +- drivers/firmware/efi/efi-pstore.c | 4 ++-- drivers/firmware/efi/efibc.c | 3 +-- drivers/firmware/efi/libstub/arm-stub.c | 5 ++--- drivers/infiniband/hw/hfi1/efivar.c | 6 +----- drivers/scsi/isci/probe_roms.c | 2 +- 7 files changed, 12 insertions(+), 25 deletions(-)arch/ia64/kernel/efi.c +0
-2@@ -919,22 +919,15 @@ static void __init handle_palo(unsigned long phys_addr)efi_uart_console_only (void) {efi_status_t status; -char *s, name[] ="ConOut" ; -efi_guid_t guid =EFI_GLOBAL_VARIABLE_GUID ;Quoted 8 lines
arch/ia64/kernel/efi.c static void __init handle_palo(unsigned long phys_addr) · line 923
Quoted 3 lines
arch/ia64/kernel/efi.c +2
-1-efi_char16_t *utf16, name_utf16[32 ]; +const char name[] ="ConOut" ; +const efi_char16_t name_utf16[] = {'C' ,'o' ,'n' ,'O' ,'u' ,'t' ,0 };I know you mention 'static' in 10/10, but I don't understand why you don't make these static to begin with. That also allows you to annotate them as
__initconst, which is not possible for compound initializers (including the GUID)Right, I hadn't thought of
__initconst. In this particular case indeed it might make sense to use "static constefi_guid_tguid__initconst= ...", and arguably for the strings as well. But where should I draw the line?There are a handful of other GUIDs used in
__initfunctions, e.g. inarch/ia64/there'sESI_TABLE_GUIDin kernel/esi.c:esi_init() andSAL_SYSTEM_TABLE_GUIDin sn/kernel/setup.c:early_sn_setup(). This patch set results in them being placed in rodata, but not init.rodata. Should I declare a temporary "static const__initconst" variable for each of them? That would give us 16 bytes in each of the two cases being freed after kernel initialization.However string literals aren't put in init.rodata either even though they're declared in an
__initfunction. E.g.early_sn_setup()contains a string literal "failed to find SAL entry point\n". That's 31 bytes.esi_init()contains even 103 bytes of string literals.
Quoted 7 lines
Quoted 4 lines
So I'm not sure if it's worth to move the GUIDs to init.rodata (at the expense of an additional LoC to declare a temporary variable) even though we waste more space for string literals.
For the string literals I think there are some solutions possible involving compiler plugins. Kees?
Not yet, but coming. The "initify" plugin will move string literals to init, IIUC. There are still some false-positive warnings generated by modpost when initify is enabled, so I haven't push it into my -next tree yet.
-Kees
Signature · 2 lines
Kees Cook Nexus Security
PATCH 4/10 efi: Constify HandleProtocol() / LocateHandle() / LocateProtocol()
In reply to a message not in the archiveGUIDs passed to HandleProtocol() / LocateHandle() / LocateProtocol() are not modified, so declare them const. This allows the compiler to place them in the rodata section instead of generating them on the stack at runtime. Pass GUIDs as literals rather than assigning them to temporary variables to save a bit on code. (Except for UGA and GOP GUIDs as they are passed around between functions.) Constification of the GUIDs is in compliance with the EFI spec which marks them "IN" for said boot services.
--- arch/x86/boot/compressed/eboot.c | 48 ++++++++++++++++----------------- drivers/firmware/efi/libstub/arm-stub.c | 10 +++---- drivers/firmware/efi/libstub/gop.c | 12 ++++----- drivers/firmware/efi/libstub/random.c | 6 ++--- include/linux/efi.h | 9 ++++--- 5 files changed, 39 insertions(+), 46 deletions(-)
arch/x86/boot/compressed/eboot.c +23 -25
@@ -43,13 +43,12 @@ static inline efi_status_t __open_volume32(void *__image, void **__fh)efi_file_io_interface_t *io;efi_loaded_image_32_t *image = __image;efi_file_handle_32_t *fh; -efi_guid_t fs_proto =EFI_FILE_SYSTEM_GUID ;efi_status_t status;void *handle = (void *)(unsigned long )image->device_handle;unsigned long func; status =efi_call_early (handle_protocol, handle, - &fs_proto, (void **)&io); + &EFI_FILE_SYSTEM_GUID , (void **)&io);if (status !=EFI_SUCCESS ) {efi_printk (sys_table,"Failed to handle fs_proto\n" );return status;
@@ -69,13 +68,12 @@ static inline efi_status_t __open_volume64(void *__image, void **__fh)efi_file_io_interface_t *io;efi_loaded_image_64_t *image = __image;efi_file_handle_64_t *fh; -efi_guid_t fs_proto =EFI_FILE_SYSTEM_GUID ;efi_status_t status;void *handle = (void *)(unsigned long )image->device_handle;unsigned long func; status =efi_call_early (handle_protocol, handle, - &fs_proto, (void **)&io); + &EFI_FILE_SYSTEM_GUID , (void **)&io);if (status !=EFI_SUCCESS ) {efi_printk (sys_table,"Failed to handle fs_proto\n" );return status;
@@ -173,7 +171,6 @@ void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str) unsigned long size) {efi_pci_io_protocol_32 *pci =NULL ; -efi_guid_t pci_proto =EFI_PCI_IO_PROTOCOL_GUID ;u32 *handles = (u32 *)(unsigned long )pci_handle;efi_status_t status;unsigned long nr_pci;
@@ -191,7 +188,8 @@ void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str)u32 h = handles[i]; status =efi_call_early (handle_protocol, h, - &pci_proto, (void **)&pci); + &EFI_PCI_IO_PROTOCOL_GUID , + (void **)&pci);if (status !=EFI_SUCCESS )continue ;
@@ -280,7 +278,6 @@ void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str) unsigned long size) {efi_pci_io_protocol_64 *pci =NULL ; -efi_guid_t pci_proto =EFI_PCI_IO_PROTOCOL_GUID ;u64 *handles = (u64 *)(unsigned long )pci_handle;efi_status_t status;unsigned long nr_pci;
@@ -298,7 +295,8 @@ void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str)u64 h = handles[i]; status =efi_call_early (handle_protocol, h, - &pci_proto, (void **)&pci); + &EFI_PCI_IO_PROTOCOL_GUID , + (void **)&pci);if (status !=EFI_SUCCESS )continue ;
@@ -333,12 +331,12 @@ static void setup_efi_pci(struct boot_params *params) {efi_status_t status;void **pci_handle =NULL ; -efi_guid_t pci_proto =EFI_PCI_IO_PROTOCOL_GUID ;unsigned long size =0 ; status =efi_call_early (locate_handle,EFI_LOCATE_BY_PROTOCOL , - &pci_proto,NULL , &size, pci_handle); + &EFI_PCI_IO_PROTOCOL_GUID , +NULL , &size, pci_handle);if (status ==EFI_BUFFER_TOO_SMALL ) { status = efi_call_early(allocate_pool,
@@ -351,7 +349,8 @@ static void setup_efi_pci(struct boot_params *params) } status =efi_call_early (locate_handle, -EFI_LOCATE_BY_PROTOCOL , &pci_proto, +EFI_LOCATE_BY_PROTOCOL , + &EFI_PCI_IO_PROTOCOL_GUID ,NULL , &size, pci_handle); }
@@ -369,13 +368,13 @@ static void setup_efi_pci(struct boot_params *params)static void retrieve_apple_device_properties (struct boot_params *boot_params) { -efi_guid_t guid =APPLE_PROPERTIES_PROTOCOL_GUID ;struct setup_data *data, *new;efi_status_t status;u32 size =0 ;void *p; - status =efi_call_early (locate_protocol, &guid,NULL , &p); + status =efi_call_early (locate_protocol, + &APPLE_PROPERTIES_PROTOCOL_GUID ,NULL , &p);if (status !=EFI_SUCCESS )return ;
@@ -434,7 +433,7 @@ static void setup_quirks(struct boot_params *boot_params) setup_uga32(void **uga_handle, unsigned long size, u32 *width, u32 *height) {struct efi_uga_draw_protocol *uga =NULL , *first_uga; -efi_guid_t uga_proto =EFI_UGA_PROTOCOL_GUID ; +const efi_guid_t uga_proto =EFI_UGA_PROTOCOL_GUID ;unsigned long nr_ugas;u32 *handles = (u32 *)uga_handle;;efi_status_t status =EFI_INVALID_PARAMETER ;
@@ -443,7 +442,6 @@ static void setup_quirks(struct boot_params *boot_params) first_uga =NULL ; nr_ugas = size /sizeof (u32);for (i =0 ; i < nr_ugas; i++) { -efi_guid_t pciio_proto =EFI_PCI_IO_PROTOCOL_GUID ;u32 w, h, depth, refresh;void *pciio;u32 handle = handles[i];
@@ -453,7 +451,8 @@ static void setup_quirks(struct boot_params *boot_params)if (status !=EFI_SUCCESS )continue ; -efi_call_early (handle_protocol, handle, &pciio_proto, &pciio); +efi_call_early (handle_protocol, handle, + &EFI_PCI_IO_PROTOCOL_GUID , &pciio); status = efi_early->call ((unsigned long )uga->get_mode, uga, &w, &h, &depth, &refresh);
@@ -479,7 +478,7 @@ static void setup_quirks(struct boot_params *boot_params) setup_uga64(void **uga_handle, unsigned long size, u32 *width, u32 *height) {struct efi_uga_draw_protocol *uga =NULL , *first_uga; -efi_guid_t uga_proto =EFI_UGA_PROTOCOL_GUID ; +const efi_guid_t uga_proto =EFI_UGA_PROTOCOL_GUID ;unsigned long nr_ugas;u64 *handles = (u64 *)uga_handle;;efi_status_t status =EFI_INVALID_PARAMETER ;
@@ -488,7 +487,6 @@ static void setup_quirks(struct boot_params *boot_params) first_uga =NULL ; nr_ugas = size /sizeof (u64);for (i =0 ; i < nr_ugas; i++) { -efi_guid_t pciio_proto =EFI_PCI_IO_PROTOCOL_GUID ;u32 w, h, depth, refresh;void *pciio;u64 handle = handles[i];
@@ -498,7 +496,8 @@ static void setup_quirks(struct boot_params *boot_params)if (status !=EFI_SUCCESS )continue ; -efi_call_early (handle_protocol, handle, &pciio_proto, &pciio); +efi_call_early (handle_protocol, handle, + &EFI_PCI_IO_PROTOCOL_GUID , &pciio); status = efi_early->call ((unsigned long )uga->get_mode, uga, &w, &h, &depth, &refresh);
@@ -523,8 +522,8 @@ static void setup_quirks(struct boot_params *boot_params)/* * See if we have Universal Graphics Adapter (UGA) protocol */ -static efi_status_t setup_uga (struct screen_info *si,efi_guid_t *uga_proto, -unsigned long size) +static efi_status_t setup_uga (struct screen_info *si, +const efi_guid_t *uga_proto,unsigned long size) {efi_status_t status;u32 width, height;
@@ -575,9 +574,9 @@ static efi_status_t setup_uga(struct screen_info *si, efi_guid_t *uga_proto,void setup_graphics (struct boot_params *boot_params) { -efi_guid_t graphics_proto =EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID ; +const efi_guid_t graphics_proto =EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID ;struct screen_info *si; -efi_guid_t uga_proto =EFI_UGA_PROTOCOL_GUID ; +const efi_guid_t uga_proto =EFI_UGA_PROTOCOL_GUID ;efi_status_t status;unsigned long size;void **gop_handle =NULL ;
@@ -618,7 +617,6 @@ struct boot_params *make_boot_params(struct efi_config *c)struct setup_header *hdr;efi_loaded_image_t *image;void *options, *handle; -efi_guid_t proto =LOADED_IMAGE_PROTOCOL_GUID ;int options_size =0 ;efi_status_t status;char *cmdline_ptr;
@@ -642,7 +640,7 @@ struct boot_params *make_boot_params(struct efi_config *c)setup_boot_services32 (efi_early); status =efi_call_early (handle_protocol, handle, - &proto, (void *)&image); + &LOADED_IMAGE_PROTOCOL_GUID , (void *)&image);if (status !=EFI_SUCCESS ) {efi_printk (sys_table,"Failed to get handle for LOADED_IMAGE_PROTOCOL\n" );return NULL ;
drivers/firmware/efi/libstub/arm-stub.c +4 -6
@@ -71,12 +71,11 @@ efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg,efi_file_io_interface_t *io;efi_loaded_image_t *image = __image;efi_file_handle_t *fh; -efi_guid_t fs_proto =EFI_FILE_SYSTEM_GUID ;efi_status_t status;void *handle = (void *)(unsigned long )image->device_handle; - status = sys_table_arg->boottime->handle_protocol (handle, - &fs_proto, (void **)&io); + status =efi_call_early (handle_protocol, handle, + &EFI_FILE_SYSTEM_GUID , (void **)&io);if (status !=EFI_SUCCESS ) {efi_printk (sys_table_arg,"Failed to handle fs_proto\n" );return status;
@@ -101,7 +100,7 @@ void efi_char16_printk(efi_system_table_t *sys_table_arg,static struct screen_info *setup_graphics (efi_system_table_t *sys_table_arg) { -efi_guid_t gop_proto =EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID ; +const efi_guid_t gop_proto =EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID ;efi_status_t status;unsigned long size;void **gop_handle =NULL ;
@@ -153,7 +152,6 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table,char *cmdline_ptr =NULL ;int cmdline_size =0 ;unsigned long new_fdt_addr; -efi_guid_t loaded_image_proto =LOADED_IMAGE_PROTOCOL_GUID ;unsigned long reserve_addr =0 ;unsigned long reserve_size =0 ;int secure_boot =0 ;
@@ -175,7 +173,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table,* line. */ status = sys_table->boottime->handle_protocol (handle, - &loaded_image_proto, (void *)&image); + &LOADED_IMAGE_PROTOCOL_GUID , (void *)&image);if (status !=EFI_SUCCESS ) {pr_efi_err (sys_table,"Failed to get loaded image protocol\n" ); goto fail;
drivers/firmware/efi/libstub/gop.c +5 -7
@@ -111,7 +111,7 @@ static void find_bits(unsigned long mask, u8 *pos, u8 *size)static efi_status_t setup_gop32 (efi_system_table_t *sys_table_arg,struct screen_info *si, -efi_guid_t *proto,unsigned long size,void **gop_handle) +const efi_guid_t *proto,unsigned long size,void **gop_handle) {struct efi_graphics_output_protocol_32 *gop32, *first_gop;unsigned long nr_gops;
@@ -131,7 +131,6 @@ static void find_bits(unsigned long mask, u8 *pos, u8 *size) nr_gops = size /sizeof (u32);for (i =0 ; i < nr_gops; i++) {struct efi_graphics_output_mode_info *info =NULL ; -efi_guid_t conout_proto =EFI_CONSOLE_OUT_DEVICE_GUID ;bool conout_found = false;void *dummy =NULL ;efi_handle_t h = (efi_handle_t )(unsigned long )handles[i];
@@ -143,7 +142,7 @@ static void find_bits(unsigned long mask, u8 *pos, u8 *size)continue ; status =efi_call_early (handle_protocol, h, - &conout_proto, &dummy); + &EFI_CONSOLE_OUT_DEVICE_GUID , &dummy);if (status ==EFI_SUCCESS ) conout_found = true;
@@ -228,7 +227,7 @@ static void find_bits(unsigned long mask, u8 *pos, u8 *size)static efi_status_t setup_gop64 (efi_system_table_t *sys_table_arg,struct screen_info *si, -efi_guid_t *proto,unsigned long size,void **gop_handle) +const efi_guid_t *proto,unsigned long size,void **gop_handle) {struct efi_graphics_output_protocol_64 *gop64, *first_gop;unsigned long nr_gops;
@@ -248,7 +247,6 @@ static void find_bits(unsigned long mask, u8 *pos, u8 *size) nr_gops = size /sizeof (u64);for (i =0 ; i < nr_gops; i++) {struct efi_graphics_output_mode_info *info =NULL ; -efi_guid_t conout_proto =EFI_CONSOLE_OUT_DEVICE_GUID ;bool conout_found = false;void *dummy =NULL ;efi_handle_t h = (efi_handle_t )(unsigned long )handles[i];
@@ -260,7 +258,7 @@ static void find_bits(unsigned long mask, u8 *pos, u8 *size)continue ; status =efi_call_early (handle_protocol, h, - &conout_proto, &dummy); + &EFI_CONSOLE_OUT_DEVICE_GUID , &dummy);if (status ==EFI_SUCCESS ) conout_found = true;
@@ -323,7 +321,7 @@ static void find_bits(unsigned long mask, u8 *pos, u8 *size)* See if we have Graphics Output Protocol */ efi_status_t efi_setup_gop (efi_system_table_t *sys_table_arg, -struct screen_info *si,efi_guid_t *proto, +struct screen_info *si,const efi_guid_t *proto,unsigned long size) {efi_status_t status;
drivers/firmware/efi/libstub/random.c +2 -4
@@ -23,11 +23,10 @@ struct efi_rng_protocol {efi_status_t efi_get_random_bytes (efi_system_table_t *sys_table_arg,unsigned long size,u8 *out) { -efi_guid_t rng_proto =EFI_RNG_PROTOCOL_GUID ;efi_status_t status;struct efi_rng_protocol *rng; - status =efi_call_early (locate_protocol, &rng_proto,NULL , + status =efi_call_early (locate_protocol, &EFI_RNG_PROTOCOL_GUID ,NULL , (void **)&rng);if (status !=EFI_SUCCESS )return status;
@@ -149,14 +148,13 @@ efi_status_t efi_random_alloc(efi_system_table_t *sys_table_arg,efi_status_t efi_random_get_seed (efi_system_table_t *sys_table_arg) { -efi_guid_t rng_proto =EFI_RNG_PROTOCOL_GUID ;efi_guid_t rng_algo_raw =EFI_RNG_ALGORITHM_RAW ;efi_guid_t rng_table_guid =LINUX_EFI_RANDOM_SEED_TABLE_GUID ;struct efi_rng_protocol *rng;struct linux_efi_random_seed *seed;efi_status_t status; - status =efi_call_early (locate_protocol, &rng_proto,NULL , + status =efi_call_early (locate_protocol, &EFI_RNG_PROTOCOL_GUID ,NULL , (void **)&rng);if (status !=EFI_SUCCESS )return status;
include/linux/efi.h +5 -4
@@ -293,10 +293,11 @@ struct efi_boot_memmap {void *install_protocol_interface;void *reinstall_protocol_interface;void *uninstall_protocol_interface; -efi_status_t (*handle_protocol)(efi_handle_t ,efi_guid_t *,void **); +efi_status_t (*handle_protocol)(efi_handle_t ,const efi_guid_t *, +void **);void *__reserved;void *register_protocol_notify; -efi_status_t (*locate_handle)(int ,efi_guid_t *,void *, +efi_status_t (*locate_handle)(int ,const efi_guid_t *,void *,unsigned long *,efi_handle_t *);void *locate_device_path;efi_status_t (*install_configuration_table)(efi_guid_t *,void *);
@@ -315,7 +316,7 @@ struct efi_boot_memmap {void *open_protocol_information;void *protocols_per_handle;void *locate_handle_buffer; -efi_status_t (*locate_protocol)(efi_guid_t *,void *,void **); +efi_status_t (*locate_protocol)(const efi_guid_t *,void *,void **);void *install_multiple_protocol_interfaces;void *uninstall_multiple_protocol_interfaces;void *calculate_crc32;
@@ -1472,7 +1473,7 @@ efi_status_t handle_cmdline_files(efi_system_table_t *sys_table_arg,efi_status_t efi_parse_options (char *cmdline);efi_status_t efi_setup_gop (efi_system_table_t *sys_table_arg, -struct screen_info *si,efi_guid_t *proto, +struct screen_info *si,const efi_guid_t *proto,unsigned long size);bool efi_runtime_disabled (void );
Signature · 1 line
2.11.0
PATCH 5/10 efi: Constify InstallConfigurationTable()
In reply to a message not in the archiveGUIDs passed to InstallConfigurationTable() are not modified, so declare them const. This allows the compiler to place them in the rodata section instead of generating them on the stack at runtime. Pass GUIDs as literals rather than assigning them to temporary variables to save a bit on code. Constification of the GUIDs is in compliance with the EFI spec which marks them "IN" for this boot service.
--- drivers/firmware/efi/libstub/arm32-stub.c | 2 +- drivers/firmware/efi/libstub/random.c | 5 ++--- include/linux/efi.h | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-)
drivers/firmware/efi/libstub/arm32-stub.c +1 -1
@@ -26,7 +26,7 @@ efi_status_t check_platform_features(efi_system_table_t *sys_table_arg)return EFI_SUCCESS ; } -static efi_guid_t screen_info_guid =LINUX_EFI_ARM_SCREEN_INFO_TABLE_GUID ; +static const efi_guid_t screen_info_guid =LINUX_EFI_ARM_SCREEN_INFO_TABLE_GUID ;struct screen_info *alloc_screen_info (efi_system_table_t *sys_table_arg) {
drivers/firmware/efi/libstub/random.c +2 -3
@@ -149,7 +149,6 @@ efi_status_t efi_random_alloc(efi_system_table_t *sys_table_arg,efi_status_t efi_random_get_seed (efi_system_table_t *sys_table_arg) {efi_guid_t rng_algo_raw =EFI_RNG_ALGORITHM_RAW ; -efi_guid_t rng_table_guid =LINUX_EFI_RANDOM_SEED_TABLE_GUID ;struct efi_rng_protocol *rng;struct linux_efi_random_seed *seed;efi_status_t status;
@@ -179,8 +178,8 @@ efi_status_t efi_random_get_seed(efi_system_table_t *sys_table_arg) goto err_freepool; seed->size =RANDOM_SEED_SIZE ; - status =efi_call_early (install_configuration_table, &rng_table_guid, - seed); + status =efi_call_early (install_configuration_table, + &LINUX_EFI_RANDOM_SEED_TABLE_GUID , seed);if (status !=EFI_SUCCESS ) goto err_freepool;
include/linux/efi.h +1 -1
@@ -300,7 +300,7 @@ struct efi_boot_memmap {efi_status_t (*locate_handle)(int ,const efi_guid_t *,void *,unsigned long *,efi_handle_t *);void *locate_device_path; -efi_status_t (*install_configuration_table)(efi_guid_t *,void *); +efi_status_t (*install_configuration_table)(const efi_guid_t *,void *);void *load_image;void *start_image;void *exit;
Signature · 1 line
2.11.0
PATCH 6/10 efi: Constify EFI_FILE_PROTOCOL.GetInfo()
In reply to a message not in the archiveGUIDs passed to EFI_FILE_PROTOCOL.GetInfo() are not modified, so declare them const. This allows the compiler to place them in the rodata section instead of generating them on the stack at runtime. Pass GUIDs as literals rather than assigning them to temporary variables to save a bit on code. Constification of the GUIDs is in compliance with the EFI spec which marks them "IN" for this protocol function.
--- drivers/firmware/efi/libstub/efi-stub-helper.c | 5 ++--- include/linux/efi.h | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-)
drivers/firmware/efi/libstub/efi-stub-helper.c +2 -3
@@ -345,7 +345,6 @@ static efi_status_t efi_file_size(efi_system_table_t *sys_table_arg, void *__fh,efi_file_handle_t *h, *fh = __fh;efi_file_info_t *info;efi_status_t status; -efi_guid_t info_guid =EFI_FILE_INFO_ID ;unsigned long info_sz; status = efi_call_proto(efi_file_handle, open, fh, &h, filename_16,
@@ -360,7 +359,7 @@ static efi_status_t efi_file_size(efi_system_table_t *sys_table_arg, void *__fh, *handle = h; info_sz =0 ; - status =efi_call_proto (efi_file_handle, get_info, h, &info_guid, + status =efi_call_proto (efi_file_handle, get_info, h, &EFI_FILE_INFO_ID , &info_sz,NULL );if (status !=EFI_BUFFER_TOO_SMALL ) {efi_printk (sys_table_arg,"Failed to get file info size\n" );
@@ -375,7 +374,7 @@ static efi_status_t efi_file_size(efi_system_table_t *sys_table_arg, void *__fh,return status; } - status =efi_call_proto (efi_file_handle, get_info, h, &info_guid, + status =efi_call_proto (efi_file_handle, get_info, h, &EFI_FILE_INFO_ID , &info_sz, info);if (status ==EFI_BUFFER_TOO_SMALL ) {efi_call_early (free_pool, info);
include/linux/efi.h +1 -1
@@ -838,7 +838,7 @@ struct efi_fdt_params {void *write;void *get_position;void *set_position; -efi_status_t (*get_info)(struct _efi_file_handle *,efi_guid_t *, +efi_status_t (*get_info)(struct _efi_file_handle *,const efi_guid_t *,unsigned long *,void *);void *set_info;void *flush;
Signature · 1 line
2.11.0
PATCH 7/10 efi: Constify EFI_RNG_PROTOCOL.GetRNG()
In reply to a message not in the archiveGUIDs passed to EFI_RNG_PROTOCOL.GetRNG() are not modified, so declare them const. This allows the compiler to place them in the rodata section instead of generating them on the stack at runtime. Pass GUIDs as literals rather than assigning them to temporary variables to save a bit on code. Constification of the GUIDs is in compliance with the EFI spec which marks them "IN" for this protocol function.
--- drivers/firmware/efi/libstub/random.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/firmware/efi/libstub/random.c +2 -3
@@ -17,7 +17,7 @@ struct efi_rng_protocol {efi_status_t (*get_info)(struct efi_rng_protocol *,unsigned long *,efi_guid_t *);efi_status_t (*get_rng)(struct efi_rng_protocol *, -efi_guid_t *,unsigned long ,u8 *out); +const efi_guid_t *,unsigned long ,u8 *out); }; efi_status_t efi_get_random_bytes(efi_system_table_t *sys_table_arg,
@@ -148,7 +148,6 @@ efi_status_t efi_random_alloc(efi_system_table_t *sys_table_arg,efi_status_t efi_random_get_seed (efi_system_table_t *sys_table_arg) { -efi_guid_t rng_algo_raw =EFI_RNG_ALGORITHM_RAW ;struct efi_rng_protocol *rng;struct linux_efi_random_seed *seed;efi_status_t status;
@@ -164,7 +163,7 @@ efi_status_t efi_random_get_seed(efi_system_table_t *sys_table_arg)if (status !=EFI_SUCCESS )return status; - status = rng->get_rng (rng, &rng_algo_raw,RANDOM_SEED_SIZE , + status = rng->get_rng (rng, &EFI_RNG_ALGORITHM_RAW ,RANDOM_SEED_SIZE , seed->bits);if (status ==EFI_UNSUPPORTED )/*
Signature · 1 line
2.11.0
PATCH 8/10 efi: Constify EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.OutputString()
In reply to a message not in the archiveUCS-2 strings passed to EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.OutputString() are not modified, so can be declared const. This allows the compiler to place them in the rodata section instead of generating them on the stack at runtime, such as with the '\r' string passed from efi_printk() to efi_char16_printk(). Constification of such strings is in compliance with the EFI spec which marks them "IN" for this protocol function.
--- arch/x86/boot/compressed/eboot.c | 2 +- drivers/firmware/efi/libstub/arm-stub.c | 2 +- drivers/firmware/efi/libstub/efi-stub-helper.c | 2 +- drivers/firmware/efi/libstub/efistub.h | 2 +- include/linux/efi.h | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-)
arch/x86/boot/compressed/eboot.c +1 -1
@@ -97,7 +97,7 @@ static inline efi_status_t __open_volume64(void *__image, void **__fh)return __open_volume32 (__image, __fh); } -void efi_char16_printk (efi_system_table_t *table,efi_char16_t *str) +void efi_char16_printk (efi_system_table_t *table,const efi_char16_t *str) {efi_call_proto (efi_simple_text_output_protocol, output_string, efi_early->text_output, str);
drivers/firmware/efi/libstub/arm-stub.c +1 -1
@@ -90,7 +90,7 @@ efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg, }void efi_char16_printk (efi_system_table_t *sys_table_arg, -efi_char16_t *str) +const efi_char16_t *str) {struct efi_simple_text_output_protocol *out;
drivers/firmware/efi/libstub/efi-stub-helper.c +1 -1
@@ -48,7 +48,7 @@ void efi_printk(efi_system_table_t *sys_table_arg, char *str) ch[0 ] = *s8;if (*s8 =='\n' ) { -efi_char16_t nl[2 ] = {'\r' ,0 }; +const efi_char16_t nl[2 ] = {'\r' ,0 };efi_char16_printk (sys_table_arg, nl); }
drivers/firmware/efi/libstub/efistub.h +1 -1
@@ -24,7 +24,7 @@#define EFI_ALLOC_ALIGN EFI_PAGE_SIZE#endif -void efi_char16_printk (efi_system_table_t *,efi_char16_t *); +void efi_char16_printk (efi_system_table_t *,const efi_char16_t *);efi_status_t efi_open_volume (efi_system_table_t *sys_table_arg,void *__image,void **__fh);
include/linux/efi.h +1 -1
@@ -1257,7 +1257,7 @@ struct efivar_entry {struct efi_simple_text_output_protocol {void *reset; -efi_status_t (*output_string)(void *,void *); +efi_status_t (*output_string)(void *,const void *);void *test_string; };
Signature · 1 line
2.11.0
PATCH 9/10 efi: Constify efi_guidcmp() arguments
In reply to a message not in the archiveGUIDs passed to efi_guidcmp() are not modified, so declare them const. This allows the compiler to place them in the rodata section instead of generating them on the stack at runtime. Pass GUIDs as literals rather than assigning them to temporary variables to save a bit on code.
--- drivers/firmware/efi/efi-pstore.c | 6 ++---- drivers/firmware/efi/libstub/fdt.c | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-)
drivers/firmware/efi/efi-pstore.c +2 -4
@@ -46,7 +46,6 @@ static inline u64 generic_id(unsigned long timestamp,static int efi_pstore_read_func (struct efivar_entry *entry,void *data) { -efi_guid_t vendor =LINUX_EFI_CRASH_GUID ;struct pstore_read_data *cb_data = data;char name[DUMP_NAME_LEN ], data_type;int i;
@@ -54,7 +53,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry, void *data)unsigned int part;unsigned long time, size; -if (efi_guidcmp (entry->var.VendorGuid, vendor)) +if (efi_guidcmp (entry->var.VendorGuid,LINUX_EFI_CRASH_GUID ))return 0 ;for (i =0 ; i <DUMP_NAME_LEN ; i++)
@@ -299,14 +298,13 @@ struct pstore_erase_data {static int efi_pstore_erase_func (struct efivar_entry *entry,void *data) {struct pstore_erase_data *ed = data; -efi_guid_t vendor =LINUX_EFI_CRASH_GUID ;efi_char16_t efi_name_old[DUMP_NAME_LEN ];efi_char16_t *efi_name = ed->name;unsigned long ucs2_len =ucs2_strlen (ed->name);char name_old[DUMP_NAME_LEN ];int i; -if (efi_guidcmp (entry->var.VendorGuid, vendor)) +if (efi_guidcmp (entry->var.VendorGuid,LINUX_EFI_CRASH_GUID ))return 0 ; if (ucs2_strncmp(entry->var.VariableName,
drivers/firmware/efi/libstub/fdt.c +1 -2
@@ -332,7 +332,6 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,void *get_fdt (efi_system_table_t *sys_table,unsigned long *fdt_size) { -efi_guid_t fdt_guid =DEVICE_TREE_GUID ;efi_config_table_t *tables;void *fdt;int i;
@@ -341,7 +340,7 @@ void *get_fdt(efi_system_table_t *sys_table, unsigned long *fdt_size) fdt =NULL ;for (i =0 ; i < sys_table->nr_tables; i++) -if (efi_guidcmp (tables[i].guid, fdt_guid) ==0 ) { +if (efi_guidcmp (tables[i].guid,DEVICE_TREE_GUID ) ==0 ) { fdt = (void *) tables[i].table;if (fdt_check_header (fdt) !=0 ) {pr_efi_err (sys_table,"Invalid header detected on UEFI supplied FDT, ignoring ...\n" );
Signature · 1 line
2.11.0
PATCH 10/10 uuid: Constify UUID compound literals
In reply to a message not in the archiveThe UUID_BE() and UUID_LE() macros (as well as the derived EFI_GUID()) generate a compound literal for a 128-bit ID. By default, these are not stored in the rodata section but generated on the stack at runtime in a very time- and space-inefficient manner.
E.g. EFI_FILE_SYSTEM_GUID (964E5B22-6459-11D2-8E39-00A0C969723B) results in x86_64 code which requires 80 bytes text instead of 16 bytes rodata:
c6 44 24 20 22 movb $0x22,0x20(%rsp)
c6 44 24 21 5b movb $0x5b,0x21(%rsp)
c6 44 24 22 4e movb $0x4e,0x22(%rsp)
c6 44 24 23 96 movb $0x96,0x23(%rsp)
c6 44 24 24 59 movb $0x59,0x24(%rsp)
c6 44 24 25 64 movb $0x64,0x25(%rsp)
c6 44 24 26 d2 movb $0xd2,0x26(%rsp)
c6 44 24 27 11 movb $0x11,0x27(%rsp)
c6 44 24 28 8e movb $0x8e,0x28(%rsp)
c6 44 24 29 39 movb $0x39,0x29(%rsp)
c6 44 24 2a 00 movb $0x0,0x2a(%rsp)
c6 44 24 2b a0 movb $0xa0,0x2b(%rsp)
c6 44 24 2c c9 movb $0xc9,0x2c(%rsp)
c6 44 24 2d 69 movb $0x69,0x2d(%rsp)
c6 44 24 2e 72 movb $0x72,0x2e(%rsp)
c6 44 24 2f 3b movb $0x3b,0x2f(%rsp)gcc 7 improves on this by using 2x movabs + 2x mov instructions, amounting to 30 bytes text instead of 16 bytes rodata. That's more space-efficient than 80 bytes but still undesirable from a performance and security perspective:
48 b8 22 5b 4e 96 59 64 d2 11 movabs $0x11d26459964e5b,%rax
48 89 44 24 20 mov %rax,0x20(%rsp)
48 b8 8e 39 00 a0 c9 69 72 3b movabs $0x3b7269c9a00039,%rax
48 89 44 24 28 mov %rax,0x28(%rsp)The reason for generating compound literals on the stack is that they're allowed to be modified. E.g. a pointer to a UUID might be used to change some of its 16 bytes. In the context of UUIDs this seems silly
though: The uuid_be, uuid_le and efi_guid_t data types hide the distinct bytes making up the ID, the bytes are not meant to be modified. Moreover, the UUIDs are usually pre-defined in a spec such as UEFI and there's no reason at all to modify them.
Thus, change the UUID_BE and UUID_LE macros to declare the generated compound literal const, allowing the compiler to put it in rodata.
When using the compound literal as an initializer for a variable, that variable has to be declared const as well for the UUID to become rodata.
Is that all? Unfortunately not. Under certain conditions gcc refuses to store compound literals in rodata despite them being declared const. Rasmus Villemoes opened a bugzilla entry for this in December 2015:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68725
The upshot is that if a UUID is passed by value to a function (either directly or as a variable), it is stored in rodata. This is the case for efi_guidcmp(). But if the UUID is passed by reference, it is generated on the stack, in spite of it being declared const and the function parameter being declared const. This is the case for the various EFI boot and runtime services calls such as GetVariable. A workaround is to declare a *static* const variable, assign the UUID to it and pass the variable by reference to the function.
This issue also affects efi_char16_t strings which are only declared const (and not static const). They're likewise generated on the stack at runtime.
clang 3.9 is not better than gcc: It does the exact opposite by putting UUIDs in rodata if they're passed by reference, and generating them on the stack if they're passed by value.
--- include/uapi/linux/uuid.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
include/uapi/linux/uuid.h +2 -2
@@ -29,14 +29,14 @@ } uuid_be;#define UUID_LE (a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \ -((uuid_le) \ +((const uuid_le) \ {{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \ (b) & 0xff, ((b) >> 8) & 0xff, \ (c) & 0xff, ((c) >> 8) & 0xff, \ (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})#define UUID_BE (a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \ -((uuid_be) \ +((const uuid_be) \ {{ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff, \ ((b) >> 8) & 0xff, (b) & 0xff, \ ((c) >> 8) & 0xff, (c) & 0xff, \
Signature · 1 line
2.11.0
A message that is not in the archive (sent off-list or not yet received): c61bbe4733f78c830f6073e86f32159751bbda8c.1483318593.git.lukas-JFq808J9C/[email protected]
Re: [PATCH 10/10] uuid: Constify UUID compound literals
Hi Lukas,
Quoted 5 lines · Lukas Wunner
The
UUID_BE()andUUID_LE()macros (as well as the derivedEFI_GUID()) generate a compound literal for a 128-bit ID. By default, these are not stored in the rodata section but generated on the stack at runtime in a very time- and space-inefficient manner.
This is not true if you declare those variables static const (in function scope).
In general, I think we should avoid touching uapi headers for this, so I would like to explore if we can reach the same result without this patch. Another objection I have to this patch is that it is context dependent whether the UUID_[LE_BE]() macros in fact generate compound
literals: this is true for an assignment to a pointer variable, but when they are used in initializers this is not the case, and so adding 'const' to their definitions is not always appropriate imo.
Are there any instances where simply turning those automatic variables into static const doesn't work?
Quoted 99 lines
E.g.
EFI_FILE_SYSTEM_GUID(964E5B22-6459-11D2-8E39-00A0C969723B) results inx86_64code which requires 80 bytes text instead of 16 bytes rodata:c6 44 24 20 22 movb $0x22,0x20(%rsp) c6 44 24 21 5b movb $0x5b,0x21(%rsp) c6 44 24 22 4e movb $0x4e,0x22(%rsp) c6 44 24 23 96 movb $0x96,0x23(%rsp) c6 44 24 24 59 movb $0x59,0x24(%rsp) c6 44 24 25 64 movb $0x64,0x25(%rsp) c6 44 24 26 d2 movb $0xd2,0x26(%rsp) c6 44 24 27 11 movb $0x11,0x27(%rsp) c6 44 24 28 8e movb $0x8e,0x28(%rsp) c6 44 24 29 39 movb $0x39,0x29(%rsp) c6 44 24 2a 00 movb $0x0,0x2a(%rsp) c6 44 24 2b a0 movb $0xa0,0x2b(%rsp) c6 44 24 2c c9 movb $0xc9,0x2c(%rsp) c6 44 24 2d 69 movb $0x69,0x2d(%rsp) c6 44 24 2e 72 movb $0x72,0x2e(%rsp) c6 44 24 2f 3b movb $0x3b,0x2f(%rsp)gcc 7 improves on this by using 2x movabs + 2x mov instructions, amounting to 30 bytes text instead of 16 bytes rodata. That's more space-efficient than 80 bytes but still undesirable from a performance and security perspective:
48 b8 22 5b 4e 96 59 64 d2 11 movabs $0x11d26459964e5b,%rax 48 89 44 24 20 mov %rax,0x20(%rsp) 48 b8 8e 39 00 a0 c9 69 72 3b movabs $0x3b7269c9a00039,%rax 48 89 44 24 28 mov %rax,0x28(%rsp)The reason for generating compound literals on the stack is that they're allowed to be modified. E.g. a pointer to a UUID might be used to change some of its 16 bytes. In the context of UUIDs this seems silly
though: Theuuid_be,uuid_leandefi_guid_tdata types hide the distinct bytes making up the ID, the bytes are not meant to be modified. Moreover, the UUIDs are usually pre-defined in a spec such as UEFI and there's no reason at all to modify them.Thus, change the
UUID_BEandUUID_LEmacros to declare the generated compound literal const, allowing the compiler to put it in rodata.When using the compound literal as an initializer for a variable, that variable has to be declared const as well for the UUID to become rodata.
Is that all? Unfortunately not. Under certain conditions gcc refuses to store compound literals in rodata despite them being declared const. Rasmus Villemoes opened a bugzilla entry for this in December 2015:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68725The upshot is that if a UUID is passed by value to a function (either directly or as a variable), it is stored in rodata. This is the case for
efi_guidcmp(). But if the UUID is passed by reference, it is generated on the stack, in spite of it being declared const and the function parameter being declared const. This is the case for the various EFI boot and runtime services calls such as GetVariable. A workaround is to declare a *static* const variable, assign the UUID to it and pass the variable by reference to the function.This issue also affects
efi_char16_tstrings which are only declared const (and not static const). They're likewise generated on the stack at runtime.clang 3.9 is not better than gcc: It does the exact opposite by putting UUIDs in rodata if they're passed by reference, and generating them on the stack if they're passed by value.
Cc Huang Ying <[email protected]>Cc Rasmus Villemoes <linux-qQsb+v5E8BnlAoU/[email protected]>Cc Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/[email protected]>Cc Ard Biesheuvel <[email protected]>Signed-off-by Lukas Wunner <lukas-JFq808J9C/[email protected]>--- include/uapi/linux/uuid.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)include/uapi/linux/uuid.h +2
-3@@ -29,14 +29,14 @@ } uuid_be;#define UUID_LE (a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \ -((uuid_le) \ +((const uuid_le) \ {{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \ (b) & 0xff, ((b) >> 8) & 0xff, \ (c) & 0xff, ((c) >> 8) & 0xff, \ (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})#define UUID_BE (a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \ -((uuid_be) \ +((const uuid_be) \ {{ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff, \ ((b) >> 8) & 0xff, (b) & 0xff, \ ((c) >> 8) & 0xff, (c) & 0xff, \ --2.11.0
A message that is not in the archive (sent off-list or not yet received): CAKv+Gu8GqCrjOM6nTTHQckyUzrv+BO-B=st35yOptPfjA7Hitg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Re: [PATCH 10/10] uuid: Constify UUID compound literals
Quoted 17 lines · Ard Biesheuvel
Quoted 4 lines · Lukas Wunner
The
UUID_BE()andUUID_LE()macros (as well as the derivedEFI_GUID()) generate a compound literal for a 128-bit ID. By default, these are not stored in the rodata section but generated on the stack at runtime in a very time- and space-inefficient manner.This is not true if you declare those variables static const (in function scope).
In general, I think we should avoid touching uapi headers for this, so I would like to explore if we can reach the same result without this patch. Another objection I have to this patch is that it is context dependent whether the UUID_[LE_BE]() macros in fact generate compound
literals: this is true for an assignment to a pointer variable, but when they are used in initializers this is not the case, and so adding 'const' to their definitions is not always appropriate imo.
Quoted 2 lines
Are there any instances where simply turning those automatic variables into static const doesn't work?
The number of GUIDs used in the kernel is vast, it's many more than this patch set makes it appear. In a lot of cases, they're used as literals. I would have to declare a temporary "static const" variable for each of them and this patch set would become much larger and more intrusive. Additionally the necessity to use a static const variable instead of a literal seems clumsy and ugly.
As an example, consider arch/ia64/kernel/mca_drv.c:mca_make_slidx() which contains an if-else-if ladder with 9 entries, each using a GUID literal. I would have expected objections on grounds of severe ugliness if I had changed all of these to temporary "static const" variables.
By contrast, just changing the UUID_LE() and UUID_BE() macros causes these literals to be placed in rodata automatically.
It seems that the only reason for uuid.h to be in uapi is its usage by two other uapi headers, hyperv.h and mei.h. The two don't even use the UUID_LE() and UUID_BE() macros.
The impact of adding const to these macros is limited: If the macros are used as an initializer in a user space application and the variable they're assigned to isn't declared const, gcc will silently make them non-const.
Worst thing that can happen is that compiling the user space application results in a compiler warning. This only occurs if the UUID is passed by reference into a function. E.g. if this patch wasn't at the end of the series, these 3 warnings would occur:
arch/x86/platform/efi/quirks.c: In function 'efi_delete_dummy_variable':arch/x86/platform/efi/quirks.c:52:35: warning: passing argument 2 of 'efi.set_variable' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
efi.set_variable(efi_dummy_name, &EFI_DUMMY_GUID,
^
arch/x86/platform/efi/quirks.c:52:35: note: expected 'efi_guid_t * {aka struct <anonymous> *}' but argument is of type 'const uuid_le * {aka const struct <anonymous> *}'
arch/x86/platform/efi/quirks.c: In function 'efi_query_variable_store':arch/x86/platform/efi/quirks.c:129:45: warning: passing argument 2 of 'efi.set_variable' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
status = efi.set_variable(efi_dummy_name, &EFI_DUMMY_GUID,
^
arch/x86/platform/efi/quirks.c:129:45: note: expected 'efi_guid_t * {aka struct <anonymous> *}' but argument is of type 'const uuid_le * {aka const struct <anonymous> *}'
drivers/scsi/isci/probe_roms.c: In function 'isci_get_efi_var':drivers/scsi/isci/probe_roms.c:190:8: warning: passing argument 2 of 'get_efi()->get_variable' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
&ISCI_EFI_VENDOR_GUID,
^drivers/scsi/isci/probe_roms.c:190:8: note: expected 'efi_guid_t * {aka struct <anonymous> *}' but argument is of type 'const uuid_le * {aka const struct <anonymous> *}'
This can easily be fixed in the user space application by adding "const" to function declarations.
To reiterate what I wrote in the commit message, I don't see why anyone would want to modify the distinct bytes making up the UUID after declaring it, so marking UUIDs const seems natural.
Thanks,
Lukas
Quoted 100 lines
Quoted 100 lines
E.g.
EFI_FILE_SYSTEM_GUID(964E5B22-6459-11D2-8E39-00A0C969723B) results inx86_64code which requires 80 bytes text instead of 16 bytes rodata:c6 44 24 20 22 movb $0x22,0x20(%rsp) c6 44 24 21 5b movb $0x5b,0x21(%rsp) c6 44 24 22 4e movb $0x4e,0x22(%rsp) c6 44 24 23 96 movb $0x96,0x23(%rsp) c6 44 24 24 59 movb $0x59,0x24(%rsp) c6 44 24 25 64 movb $0x64,0x25(%rsp) c6 44 24 26 d2 movb $0xd2,0x26(%rsp) c6 44 24 27 11 movb $0x11,0x27(%rsp) c6 44 24 28 8e movb $0x8e,0x28(%rsp) c6 44 24 29 39 movb $0x39,0x29(%rsp) c6 44 24 2a 00 movb $0x0,0x2a(%rsp) c6 44 24 2b a0 movb $0xa0,0x2b(%rsp) c6 44 24 2c c9 movb $0xc9,0x2c(%rsp) c6 44 24 2d 69 movb $0x69,0x2d(%rsp) c6 44 24 2e 72 movb $0x72,0x2e(%rsp) c6 44 24 2f 3b movb $0x3b,0x2f(%rsp)gcc 7 improves on this by using 2x movabs + 2x mov instructions, amounting to 30 bytes text instead of 16 bytes rodata. That's more space-efficient than 80 bytes but still undesirable from a performance and security perspective:
48 b8 22 5b 4e 96 59 64 d2 11 movabs $0x11d26459964e5b,%rax 48 89 44 24 20 mov %rax,0x20(%rsp) 48 b8 8e 39 00 a0 c9 69 72 3b movabs $0x3b7269c9a00039,%rax 48 89 44 24 28 mov %rax,0x28(%rsp)The reason for generating compound literals on the stack is that they're allowed to be modified. E.g. a pointer to a UUID might be used to change some of its 16 bytes. In the context of UUIDs this seems silly
though: Theuuid_be,uuid_leandefi_guid_tdata types hide the distinct bytes making up the ID, the bytes are not meant to be modified. Moreover, the UUIDs are usually pre-defined in a spec such as UEFI and there's no reason at all to modify them.Thus, change the
UUID_BEandUUID_LEmacros to declare the generated compound literal const, allowing the compiler to put it in rodata.When using the compound literal as an initializer for a variable, that variable has to be declared const as well for the UUID to become rodata.
Is that all? Unfortunately not. Under certain conditions gcc refuses to store compound literals in rodata despite them being declared const. Rasmus Villemoes opened a bugzilla entry for this in December 2015:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68725The upshot is that if a UUID is passed by value to a function (either directly or as a variable), it is stored in rodata. This is the case for
efi_guidcmp(). But if the UUID is passed by reference, it is generated on the stack, in spite of it being declared const and the function parameter being declared const. This is the case for the various EFI boot and runtime services calls such as GetVariable. A workaround is to declare a *static* const variable, assign the UUID to it and pass the variable by reference to the function.This issue also affects
efi_char16_tstrings which are only declared const (and not static const). They're likewise generated on the stack at runtime.clang 3.9 is not better than gcc: It does the exact opposite by putting UUIDs in rodata if they're passed by reference, and generating them on the stack if they're passed by value.
Cc Huang Ying <[email protected]>Cc Rasmus Villemoes <linux-qQsb+v5E8BnlAoU/[email protected]>Cc Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/[email protected]>Cc Ard Biesheuvel <[email protected]>Signed-off-by Lukas Wunner <lukas-JFq808J9C/[email protected]>--- include/uapi/linux/uuid.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)include/uapi/linux/uuid.h +2
-3@@ -29,14 +29,14 @@ } uuid_be;#define UUID_LE (a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \ -((uuid_le) \ +((const uuid_le) \ {{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \ (b) & 0xff, ((b) >> 8) & 0xff, \ (c) & 0xff, ((c) >> 8) & 0xff, \ (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})#define UUID_BE (a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \ -((uuid_be) \ +((const uuid_be) \ {{ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff, \ ((b) >> 8) & 0xff, (b) & 0xff, \ ((c) >> 8) & 0xff, (c) & 0xff, \ --2.11.0
A message that is not in the archive (sent off-list or not yet received): 20170106095711.GA21850-JFq808J9C/[email protected]
Re: [PATCH 10/10] uuid: Constify UUID compound literals
Quoted 28 lines · Lukas Wunner
Quoted 17 lines · Ard Biesheuvel
Quoted 4 lines · Lukas Wunner
The
UUID_BE()andUUID_LE()macros (as well as the derivedEFI_GUID()) generate a compound literal for a 128-bit ID. By default, these are not stored in the rodata section but generated on the stack at runtime in a very time- and space-inefficient manner.This is not true if you declare those variables static const (in function scope).
In general, I think we should avoid touching uapi headers for this, so I would like to explore if we can reach the same result without this patch. Another objection I have to this patch is that it is context dependent whether the UUID_[LE_BE]() macros in fact generate compound
literals: this is true for an assignment to a pointer variable, but when they are used in initializers this is not the case, and so adding 'const' to their definitions is not always appropriate imo.Quoted 2 lines
Are there any instances where simply turning those automatic variables into static const doesn't work?
The number of GUIDs used in the kernel is vast, it's many more than this patch set makes it appear. In a lot of cases, they're used as literals. I would have to declare a temporary "static const" variable for each of them and this patch set would become much larger and more intrusive. Additionally the necessity to use a static const variable instead of a literal seems clumsy and ugly.
Quoted 5 lines
As an example, consider arch/ia64/kernel/mca_drv.c:mca_make_slidx() which contains an if-else-if ladder with 9 entries, each using a GUID literal. I would have expected objections on grounds of severe ugliness if I had changed all of these to temporary "static const" variables.
Yes, but we can macroize that: something like
#define __efiguid_ref(x) ({static const efi_guid_t __x = (x); __x; })
and
#define __efiguid_initref(x) ({static const __initconst efi_guid_t __x = (x); __x; })
should do the trick I think?
Quoted 145 lines
By contrast, just changing the
UUID_LE()andUUID_BE()macros causes these literals to be placed in rodata automatically.It seems that the only reason for
uuid.hto be in uapi is its usage by two other uapi headers,hyperv.handmei.h. The two don't even use theUUID_LE()andUUID_BE()macros.The impact of adding const to these macros is limited: If the macros are used as an initializer in a user space application and the variable they're assigned to isn't declared const, gcc will silently make them non-const.
Worst thing that can happen is that compiling the user space application results in a compiler warning. This only occurs if the UUID is passed by reference into a function. E.g. if this patch wasn't at the end of the series, these 3 warnings would occur:
arch/x86/platform/efi/quirks.c: In function 'efi_delete_dummy_variable':arch/x86/platform/efi/quirks.c:52:35: warning: passing argument 2 of 'efi.set_variable' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
efi.set_variable(efi_dummy_name,&EFI_DUMMY_GUID,^
arch/x86/platform/efi/quirks.c:52:35: note: expected 'efi_guid_t* {aka struct <anonymous> *}' but argument is of type 'constuuid_le* {aka const struct <anonymous> *}'
arch/x86/platform/efi/quirks.c: In function 'efi_query_variable_store':arch/x86/platform/efi/quirks.c:129:45: warning: passing argument 2 of 'efi.set_variable' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
status = efi.set_variable(efi_dummy_name,&EFI_DUMMY_GUID,^
arch/x86/platform/efi/quirks.c:129:45: note: expected 'efi_guid_t* {aka struct <anonymous> *}' but argument is of type 'constuuid_le* {aka const struct <anonymous> *}'
drivers/scsi/isci/probe_roms.c: In function 'isci_get_efi_var':drivers/scsi/isci/probe_roms.c:190:8: warning: passing argument 2 of 'get_efi()->get_variable' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]&ISCI_EFI_VENDOR_GUID, ^
drivers/scsi/isci/probe_roms.c:190:8: note: expected 'efi_guid_t* {aka struct <anonymous> *}' but argument is of type 'constuuid_le* {aka const struct <anonymous> *}'This can easily be fixed in the user space application by adding "const" to function declarations.
To reiterate what I wrote in the commit message, I don't see why anyone would want to modify the distinct bytes making up the UUID after declaring it, so marking UUIDs const seems natural.
Thanks,
Lukas
Quoted 100 lines
Quoted 100 lines
E.g.
EFI_FILE_SYSTEM_GUID(964E5B22-6459-11D2-8E39-00A0C969723B) results inx86_64code which requires 80 bytes text instead of 16 bytes rodata:c6 44 24 20 22 movb $0x22,0x20(%rsp) c6 44 24 21 5b movb $0x5b,0x21(%rsp) c6 44 24 22 4e movb $0x4e,0x22(%rsp) c6 44 24 23 96 movb $0x96,0x23(%rsp) c6 44 24 24 59 movb $0x59,0x24(%rsp) c6 44 24 25 64 movb $0x64,0x25(%rsp) c6 44 24 26 d2 movb $0xd2,0x26(%rsp) c6 44 24 27 11 movb $0x11,0x27(%rsp) c6 44 24 28 8e movb $0x8e,0x28(%rsp) c6 44 24 29 39 movb $0x39,0x29(%rsp) c6 44 24 2a 00 movb $0x0,0x2a(%rsp) c6 44 24 2b a0 movb $0xa0,0x2b(%rsp) c6 44 24 2c c9 movb $0xc9,0x2c(%rsp) c6 44 24 2d 69 movb $0x69,0x2d(%rsp) c6 44 24 2e 72 movb $0x72,0x2e(%rsp) c6 44 24 2f 3b movb $0x3b,0x2f(%rsp)gcc 7 improves on this by using 2x movabs + 2x mov instructions, amounting to 30 bytes text instead of 16 bytes rodata. That's more space-efficient than 80 bytes but still undesirable from a performance and security perspective:
48 b8 22 5b 4e 96 59 64 d2 11 movabs $0x11d26459964e5b,%rax 48 89 44 24 20 mov %rax,0x20(%rsp) 48 b8 8e 39 00 a0 c9 69 72 3b movabs $0x3b7269c9a00039,%rax 48 89 44 24 28 mov %rax,0x28(%rsp)The reason for generating compound literals on the stack is that they're allowed to be modified. E.g. a pointer to a UUID might be used to change some of its 16 bytes. In the context of UUIDs this seems silly
though: Theuuid_be,uuid_leandefi_guid_tdata types hide the distinct bytes making up the ID, the bytes are not meant to be modified. Moreover, the UUIDs are usually pre-defined in a spec such as UEFI and there's no reason at all to modify them.Thus, change the
UUID_BEandUUID_LEmacros to declare the generated compound literal const, allowing the compiler to put it in rodata.When using the compound literal as an initializer for a variable, that variable has to be declared const as well for the UUID to become rodata.
Is that all? Unfortunately not. Under certain conditions gcc refuses to store compound literals in rodata despite them being declared const. Rasmus Villemoes opened a bugzilla entry for this in December 2015:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68725The upshot is that if a UUID is passed by value to a function (either directly or as a variable), it is stored in rodata. This is the case for
efi_guidcmp(). But if the UUID is passed by reference, it is generated on the stack, in spite of it being declared const and the function parameter being declared const. This is the case for the various EFI boot and runtime services calls such as GetVariable. A workaround is to declare a *static* const variable, assign the UUID to it and pass the variable by reference to the function.This issue also affects
efi_char16_tstrings which are only declared const (and not static const). They're likewise generated on the stack at runtime.clang 3.9 is not better than gcc: It does the exact opposite by putting UUIDs in rodata if they're passed by reference, and generating them on the stack if they're passed by value.
Cc Huang Ying <[email protected]>Cc Rasmus Villemoes <linux-qQsb+v5E8BnlAoU/[email protected]>Cc Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/[email protected]>Cc Ard Biesheuvel <[email protected]>Signed-off-by Lukas Wunner <lukas-JFq808J9C/[email protected]>--- include/uapi/linux/uuid.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)include/uapi/linux/uuid.h +2
-3@@ -29,14 +29,14 @@ } uuid_be;#define UUID_LE (a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \ -((uuid_le) \ +((const uuid_le) \ {{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \ (b) & 0xff, ((b) >> 8) & 0xff, \ (c) & 0xff, ((c) >> 8) & 0xff, \ (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})#define UUID_BE (a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \ -((uuid_be) \ +((const uuid_be) \ {{ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff, \ ((b) >> 8) & 0xff, (b) & 0xff, \ ((c) >> 8) & 0xff, (c) & 0xff, \ --2.11.0
Kees Cook
Reviewed-byQuoted 46 lines · Lukas Wunner
When disassembling the EFI stub I noticed that GUIDs and
efi_char16_tstrings are not placed in rodata but generated on the stack at runtime. GUIDs occupy up to 80 bytes of text instead of just 16 bytes rodata which annoyed me enough to go down the rabbit hole of fixing it. This series constifies all GUIDs and EFI-related strings I could find.The most important patch is [10/10] to constify the
EFI_GUID()macro. This has to be at the end of the series to avoid compiler warnings. Reviewers may want to read its commit message first as it contains background information. In particular it explains why this series only fixes the kernel side of things. Additional work is necessary on the compiler side to actually have all the GUIDs and strings in rodata. At the moment both gcc and clang achieve that only partially.Ard Biesheuvel expressed an interest in these patches in November but stressed the importance that arguments to EFI boot/runtime/protocol services may only be declared const if the spec marks them "IN":
https://lkml.org/lkml/2016/11/21/459Hence I've split the series by boot/runtime/protocol services to allow for easy verification of this fact. Constification of
GetVariable()/SetVariable()is further split in one patch for the function signatures and another for the actual arguments because changing the function signatures is quite involved in this case.efi.git/next needs to be rebased on v4.10-rc1 before this series can be applied because it requires commits
f6697df36bdf("x86/efi: Prevent mixed mode boot corruption withCONFIG_VMAP_STACK=y") and2fbadc3002c5("arm/arm64: xen: Move shared architecture headers toinclude/xen/arm").Unfortunately I was not able to compile-test on ia64, there's no cross- compiler package available on Debian. :-( I did not receive complaints from 0-day, so I'm cautiously optimistic that I didn't cause breakage there.
I've pushed the patches to GitHub to ease reviewing/fetching:
https://github.com/l1k/linux/commits/efi_const_v1Thanks,
Lukas
Ard Biesheuvel (1):
efi: use typed function pointers for runtime services table
Quoted 10 lines
Lukas Wunner (9):
efi: ConstifyGetVariable()/SetVariable()signatures
efi: ConstifyGetVariable()/SetVariable()arguments
efi: ConstifyHandleProtocol()/LocateHandle()/LocateProtocol()
efi: ConstifyInstallConfigurationTable()
efi: ConstifyEFI_FILE_PROTOCOL.GetInfo()
efi: ConstifyEFI_RNG_PROTOCOL.GetRNG()
efi: ConstifyEFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.OutputString()
efi: Constifyefi_guidcmp()arguments
uuid: Constify UUID compound literals
Cool! Thanks for doing this!
-Kees
Signature · 2 lines
Kees Cook Nexus Security
Hi Lukas,
Thanks for putting this together.
Quoted 14 lines · Lukas Wunner
When disassembling the EFI stub I noticed that GUIDs and
efi_char16_tstrings are not placed in rodata but generated on the stack at runtime. GUIDs occupy up to 80 bytes of text instead of just 16 bytes rodata which annoyed me enough to go down the rabbit hole of fixing it. This series constifies all GUIDs and EFI-related strings I could find.The most important patch is [10/10] to constify the
EFI_GUID()macro. This has to be at the end of the series to avoid compiler warnings. Reviewers may want to read its commit message first as it contains background information. In particular it explains why this series only fixes the kernel side of things. Additional work is necessary on the compiler side to actually have all the GUIDs and strings in rodata. At the moment both gcc and clang achieve that only partially.
Quoted 5 lines
Ard Biesheuvel expressed an interest in these patches in November but stressed the importance that arguments to EFI boot/runtime/protocol services may only be declared const if the spec marks them "IN":
https://lkml.org/lkml/2016/11/21/459
Indeed. I think it is reasonable to infer constness from the IN (and not OUT) annotation of by-reference arguments. In general, I think the complete lack of distinction between read-write and read-execute is a huge oversight, and anything that improves the situation on either side of the firmware-OS boundary is a welcome change IMO.
Quoted 67 lines
Hence I've split the series by boot/runtime/protocol services to allow for easy verification of this fact. Constification of
GetVariable()/SetVariable()is further split in one patch for the function signatures and another for the actual arguments because changing the function signatures is quite involved in this case.efi.git/next needs to be rebased on v4.10-rc1 before this series can be applied because it requires commits
f6697df36bdf("x86/efi: Prevent mixed mode boot corruption withCONFIG_VMAP_STACK=y") and2fbadc3002c5("arm/arm64: xen: Move shared architecture headers toinclude/xen/arm").Unfortunately I was not able to compile-test on ia64, there's no cross- compiler package available on Debian. :-( I did not receive complaints from 0-day, so I'm cautiously optimistic that I didn't cause breakage there.
I've pushed the patches to GitHub to ease reviewing/fetching:
https://github.com/l1k/linux/commits/efi_const_v1Thanks,
Lukas
Ard Biesheuvel (1):
efi: use typed function pointers for runtime services tableLukas Wunner (9):
efi: ConstifyGetVariable()/SetVariable()signatures
efi: ConstifyGetVariable()/SetVariable()arguments
efi: ConstifyHandleProtocol()/LocateHandle()/LocateProtocol()
efi: ConstifyInstallConfigurationTable()
efi: ConstifyEFI_FILE_PROTOCOL.GetInfo()
efi: ConstifyEFI_RNG_PROTOCOL.GetRNG()
efi: ConstifyEFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.OutputString()
efi: Constifyefi_guidcmp()arguments
uuid: Constify UUID compound literalsarch/ia64/kernel/efi.c | 24 ++++------ arch/x86/boot/compressed/eboot.c | 50 ++++++++++----------- arch/x86/include/asm/xen/interface.h | 1 + arch/x86/platform/efi/efi_64.c | 18 ++++---- arch/x86/platform/efi/quirks.c | 2 +- drivers/firmware/efi/efi-pstore.c | 10 ++--- drivers/firmware/efi/efibc.c | 3 +- drivers/firmware/efi/libstub/arm-stub.c | 17 +++---- drivers/firmware/efi/libstub/arm32-stub.c | 2 +- drivers/firmware/efi/libstub/efi-stub-helper.c | 7 ++- drivers/firmware/efi/libstub/efistub.h | 2 +- drivers/firmware/efi/libstub/fdt.c | 3 +- drivers/firmware/efi/libstub/gop.c | 12 +++-- drivers/firmware/efi/libstub/random.c | 16 +++---- drivers/firmware/efi/runtime-wrappers.c | 15 ++++--- drivers/firmware/google/gsmi.c | 10 ++--- drivers/infiniband/hw/hfi1/efivar.c | 6 +-- drivers/scsi/isci/probe_roms.c | 2 +- drivers/xen/efi.c | 12 ++--- include/linux/efi.h | 61 ++++++++++++++------------ include/uapi/linux/uuid.h | 4 +- include/xen/arm/interface.h | 1 + include/xen/interface/platform.h | 11 ++++- include/xen/xen-ops.h | 12 ++--- 24 files changed, 143 insertions(+), 158 deletions(-)--
2.11.0