| Message ID | 20190812150452.27983-5-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 1c5fecb61255aa12a16c4c06335ab68979865914 |
| Headers | show |
| Series | EFI updates for v5.4 | expand |
On Tue, 1 Oct 2019 at 10:51, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Ard, Narendra, > > On Mon, Aug 12, 2019 at 5:07 PM Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: > > From: Narendra K <Narendra.K@dell.com> > > > > System firmware advertises the address of the 'Runtime > > Configuration Interface table version 2 (RCI2)' via > > an EFI Configuration Table entry. This code retrieves the RCI2 > > table from the address and exports it to sysfs as a binary > > attribute 'rci2' under /sys/firmware/efi/tables directory. > > The approach adopted is similar to the attribute 'DMI' under > > /sys/firmware/dmi/tables. > > > > RCI2 table contains BIOS HII in XML format and is used to populate > > BIOS setup page in Dell EMC OpenManage Server Administrator tool. > > The BIOS setup page contains BIOS tokens which can be configured. > > > > Signed-off-by: Narendra K <Narendra.K@dell.com> > > Reviewed-by: Mario Limonciello <mario.limonciello@dell.com> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Thanks, this is now commit 1c5fecb61255aa12 ("efi: Export Runtime > Configuration Interface table to sysfs"). > > > --- a/drivers/firmware/efi/Kconfig > > +++ b/drivers/firmware/efi/Kconfig > > @@ -180,6 +180,19 @@ config RESET_ATTACK_MITIGATION > > have been evicted, since otherwise it will trigger even on clean > > reboots. > > > > +config EFI_RCI2_TABLE > > + bool "EFI Runtime Configuration Interface Table Version 2 Support" > > + help > > + Displays the content of the Runtime Configuration Interface > > + Table version 2 on Dell EMC PowerEdge systems as a binary > > + attribute 'rci2' under /sys/firmware/efi/tables directory. > > + > > + RCI2 table contains BIOS HII in XML format and is used to populate > > + BIOS setup page in Dell EMC OpenManage Server Administrator tool. > > + The BIOS setup page contains BIOS tokens which can be configured. > > + > > + Say Y here for Dell EMC PowerEdge systems. > > A quick Google search tells me these are Intel Xeon. > Are arm/arm64/ia64 variants available, too? > If not, this should be protected by "depends on x86" ("|| COMPILE_TEST"?). > Hello Geert, The code in question is entirely architecture agnostic, and defaults to 'n', so I am not convinced this is needed. (It came up in the review as well)
On Tue, Oct 01, 2019 at 01:20:46PM +0000, Limonciello, Mario wrote: [...] > > > > > > +config EFI_RCI2_TABLE > > > > > > + bool "EFI Runtime Configuration Interface Table Version 2 Support" > > > > > > + help > > > > > > + Displays the content of the Runtime Configuration Interface > > > > > > + Table version 2 on Dell EMC PowerEdge systems as a binary > > > > > > + attribute 'rci2' under /sys/firmware/efi/tables directory. > > > > > > + > > > > > > + RCI2 table contains BIOS HII in XML format and is used to populate > > > > > > + BIOS setup page in Dell EMC OpenManage Server Administrator tool. > > > > > > + The BIOS setup page contains BIOS tokens which can be configured. > > > > > > + > > > > > > + Say Y here for Dell EMC PowerEdge systems. > > > > > > > > > > A quick Google search tells me these are Intel Xeon. > > > > > Are arm/arm64/ia64 variants available, too? > > > > > If not, this should be protected by "depends on x86" ("|| COMPILE_TEST"?). > > > > > > > > The code in question is entirely architecture agnostic, and defaults > > > > to 'n', so I am not convinced this is needed. (It came up in the > > > > review as well) > > > > > > "make oldconfig" still asks me the question on e.g. arm64, where it is > > > irrelevant, until arm64 variants of the hardware show up. > > > > > > So IMHO it should have "depends on X86 || COMPILE_TEST". > > > > > > > Fair enough. I am going to send out a bunch of EFI fixes this week, so > > I'll accept a patch that makes the change above. > > Is it really a problem to just say n? > > I think this seems like a needless change that would slow down adoption of > !x86 if Dell EMC PowerEdge systems did start going that route, especially > when it comes to distributions that move glacially slow with picking up new > kernel code. Hi Ard/Geert, Any additional thoughts here ? -- With regards, Narendra K
On Tue, Oct 01, 2019 at 08:23:51PM +0200, Geert Uytterhoeven wrote: [...] > > > > > > > > +config EFI_RCI2_TABLE > > > > > > > > + bool "EFI Runtime Configuration Interface Table Version 2 Support" [...] > > > Is it really a problem to just say n? > > > > > > I think this seems like a needless change that would slow down adoption of > > > !x86 if Dell EMC PowerEdge systems did start going that route, especially > > > when it comes to distributions that move glacially slow with picking up new > > > kernel code. > > > > Hi Ard/Geert, > > > > Any additional thoughts here ? > > Sure ;-) > > A typical platform-specific sarm/arm64 .config file has almost 3000 > config options > disabled. Hence that means I have to say "n" almost 3000 times. > Fortunately I started doing this several years ago, so I can do this > incrementally ;-) > > Perhaps someone should try to remove all lines like "depends on ... || > COMPILE_TEST", run "make oldconfig", read all help texts before saying "n", > and time the whole operation... > > I hope I managed to convince you of the benefits. Thank you Geert. The description is helpful. I am working on it. As I understand, the issue is 'make oldconfig' provides a prompt to the user and user is expecting that a prompt is not needed as the option is not relevant. I cloned upstream kernel 5.3.2 as it does not have EFI_RCI2_TABLE option and generated a .config by calling 'make defconfig'. The .config has COMPILE_TEST set to n. I copied it to 5.4-rc1 and added 'depends on COMPILE_TEST' to drivers/firmware/efi/Kconfig (did not add CONFIG_X86 because it is set to y by the defconfig from 5.3.2). 'make oldconfig' still provides a prompt for CONFIG_EFI_RCI2_TABLE. I removed 'depends on COMPILE_TEST' from Kconfig and modified it to include the below change - config EFI_RCI2_TABLE bool prompt "EFI Runtime Configuration Interface Table Version 2 Support" if COMPILE_TEST default n help Adding the condition to the 'prompt' section seems to have desired result. With this change, 'make oldconfig' did not provide a prompt. It seems like 'make oldconfig' will provide a prompt to the user if the CONFIG option is new and providing the prompt does not depend on the 'depends on' section. It seems to be dependent on the 'prompt' section. Any thoughts ? If the above understanding is correct, I will work to submit a patch with 'prompt' section modified to contain prompt "EFI Runtime Configuration Interface Table Version 2 Support" if X86 || COMPILE_TEST -- With regards, Narendra K
diff --git a/Documentation/ABI/testing/sysfs-firmware-efi b/Documentation/ABI/testing/sysfs-firmware-efi index e794eac32a90..5e4d0b27cdfe 100644 --- a/Documentation/ABI/testing/sysfs-firmware-efi +++ b/Documentation/ABI/testing/sysfs-firmware-efi @@ -28,3 +28,11 @@ Description: Displays the physical addresses of all EFI Configuration versions are always printed first, i.e. ACPI20 comes before ACPI. Users: dmidecode + +What: /sys/firmware/efi/tables/rci2 +Date: July 2019 +Contact: Narendra K <Narendra.K@dell.com>, linux-bugs@dell.com +Description: Displays the content of the Runtime Configuration Interface + Table version 2 on Dell EMC PowerEdge systems in binary format +Users: It is used by Dell EMC OpenManage Server Administrator tool to + populate BIOS setup page. diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index 6697c109c449..c202e1b07e29 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -82,6 +82,9 @@ static const unsigned long * const efi_tables[] = { &efi.esrt, &efi.properties_table, &efi.mem_attr_table, +#ifdef CONFIG_EFI_RCI2_TABLE + &rci2_table_phys, +#endif }; u64 efi_setup; /* efi setup_data physical address */ diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig index d4ea929e8b34..178ee8106828 100644 --- a/drivers/firmware/efi/Kconfig +++ b/drivers/firmware/efi/Kconfig @@ -180,6 +180,19 @@ config RESET_ATTACK_MITIGATION have been evicted, since otherwise it will trigger even on clean reboots. +config EFI_RCI2_TABLE + bool "EFI Runtime Configuration Interface Table Version 2 Support" + help + Displays the content of the Runtime Configuration Interface + Table version 2 on Dell EMC PowerEdge systems as a binary + attribute 'rci2' under /sys/firmware/efi/tables directory. + + RCI2 table contains BIOS HII in XML format and is used to populate + BIOS setup page in Dell EMC OpenManage Server Administrator tool. + The BIOS setup page contains BIOS tokens which can be configured. + + Say Y here for Dell EMC PowerEdge systems. + endmenu config UEFI_CPER diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile index d2d0d2030620..4ac2de4dfa72 100644 --- a/drivers/firmware/efi/Makefile +++ b/drivers/firmware/efi/Makefile @@ -25,6 +25,7 @@ obj-$(CONFIG_EFI_BOOTLOADER_CONTROL) += efibc.o obj-$(CONFIG_EFI_TEST) += test/ obj-$(CONFIG_EFI_DEV_PATH_PARSER) += dev-path-parser.o obj-$(CONFIG_APPLE_PROPERTIES) += apple-properties.o +obj-$(CONFIG_EFI_RCI2_TABLE) += rci2-table.o arm-obj-$(CONFIG_EFI) := arm-init.o arm-runtime.o obj-$(CONFIG_ARM) += $(arm-obj-y) diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index 801925c5bcfb..8f1ab04f6743 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -465,6 +465,9 @@ static __initdata efi_config_table_type_t common_tables[] = { {LINUX_EFI_TPM_EVENT_LOG_GUID, "TPMEventLog", &efi.tpm_log}, {LINUX_EFI_TPM_FINAL_LOG_GUID, "TPMFinalLog", &efi.tpm_final_log}, {LINUX_EFI_MEMRESERVE_TABLE_GUID, "MEMRESERVE", &efi.mem_reserve}, +#ifdef CONFIG_EFI_RCI2_TABLE + {DELLEMC_EFI_RCI2_TABLE_GUID, NULL, &rci2_table_phys}, +#endif {NULL_GUID, NULL, NULL}, }; diff --git a/drivers/firmware/efi/rci2-table.c b/drivers/firmware/efi/rci2-table.c new file mode 100644 index 000000000000..3e290f96620a --- /dev/null +++ b/drivers/firmware/efi/rci2-table.c @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Export Runtime Configuration Interface Table Version 2 (RCI2) + * to sysfs + * + * Copyright (C) 2019 Dell Inc + * by Narendra K <Narendra.K@dell.com> + * + * System firmware advertises the address of the RCI2 Table via + * an EFI Configuration Table entry. This code retrieves the RCI2 + * table from the address and exports it to sysfs as a binary + * attribute 'rci2' under /sys/firmware/efi/tables directory. + */ + +#include <linux/kobject.h> +#include <linux/device.h> +#include <linux/sysfs.h> +#include <linux/efi.h> +#include <linux/types.h> +#include <linux/io.h> + +#define RCI_SIGNATURE "_RC_" + +struct rci2_table_global_hdr { + u16 type; + u16 resvd0; + u16 hdr_len; + u8 rci2_sig[4]; + u16 resvd1; + u32 resvd2; + u32 resvd3; + u8 major_rev; + u8 minor_rev; + u16 num_of_structs; + u32 rci2_len; + u16 rci2_chksum; +} __packed; + +static u8 *rci2_base; +static u32 rci2_table_len; +unsigned long rci2_table_phys __ro_after_init = EFI_INVALID_TABLE_ADDR; + +static ssize_t raw_table_read(struct file *file, struct kobject *kobj, + struct bin_attribute *attr, char *buf, + loff_t pos, size_t count) +{ + memcpy(buf, attr->private + pos, count); + return count; +} + +static BIN_ATTR(rci2, S_IRUSR, raw_table_read, NULL, 0); + +static u16 checksum(void) +{ + u8 len_is_odd = rci2_table_len % 2; + u32 chksum_len = rci2_table_len; + u16 *base = (u16 *)rci2_base; + u8 buf[2] = {0}; + u32 offset = 0; + u16 chksum = 0; + + if (len_is_odd) + chksum_len -= 1; + + while (offset < chksum_len) { + chksum += *base; + offset += 2; + base++; + } + + if (len_is_odd) { + buf[0] = *(u8 *)base; + chksum += *(u16 *)(buf); + } + + return chksum; +} + +int __init efi_rci2_sysfs_init(void) +{ + struct kobject *tables_kobj; + int ret = -ENOMEM; + + rci2_base = memremap(rci2_table_phys, + sizeof(struct rci2_table_global_hdr), + MEMREMAP_WB); + if (!rci2_base) { + pr_debug("RCI2 table init failed - could not map RCI2 table\n"); + goto err; + } + + if (strncmp(rci2_base + + offsetof(struct rci2_table_global_hdr, rci2_sig), + RCI_SIGNATURE, 4)) { + pr_debug("RCI2 table init failed - incorrect signature\n"); + ret = -ENODEV; + goto err_unmap; + } + + rci2_table_len = *(u32 *)(rci2_base + + offsetof(struct rci2_table_global_hdr, + rci2_len)); + + memunmap(rci2_base); + + if (!rci2_table_len) { + pr_debug("RCI2 table init failed - incorrect table length\n"); + goto err; + } + + rci2_base = memremap(rci2_table_phys, rci2_table_len, MEMREMAP_WB); + if (!rci2_base) { + pr_debug("RCI2 table - could not map RCI2 table\n"); + goto err; + } + + if (checksum() != 0) { + pr_debug("RCI2 table - incorrect checksum\n"); + ret = -ENODEV; + goto err_unmap; + } + + tables_kobj = kobject_create_and_add("tables", efi_kobj); + if (!tables_kobj) { + pr_debug("RCI2 table - tables_kobj creation failed\n"); + goto err_unmap; + } + + bin_attr_rci2.size = rci2_table_len; + bin_attr_rci2.private = rci2_base; + ret = sysfs_create_bin_file(tables_kobj, &bin_attr_rci2); + if (ret != 0) { + pr_debug("RCI2 table - rci2 sysfs bin file creation failed\n"); + kobject_del(tables_kobj); + kobject_put(tables_kobj); + goto err_unmap; + } + + return 0; + + err_unmap: + memunmap(rci2_base); + err: + pr_debug("RCI2 table - sysfs initialization failed\n"); + return ret; +} +late_initcall(efi_rci2_sysfs_init); diff --git a/include/linux/efi.h b/include/linux/efi.h index f88318b85fb0..bd3837022307 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -692,6 +692,9 @@ void efi_native_runtime_setup(void); #define LINUX_EFI_TPM_FINAL_LOG_GUID EFI_GUID(0x1e2ed096, 0x30e2, 0x4254, 0xbd, 0x89, 0x86, 0x3b, 0xbe, 0xf8, 0x23, 0x25) #define LINUX_EFI_MEMRESERVE_TABLE_GUID EFI_GUID(0x888eb0c6, 0x8ede, 0x4ff5, 0xa8, 0xf0, 0x9a, 0xee, 0x5c, 0xb9, 0x77, 0xc2) +/* OEM GUIDs */ +#define DELLEMC_EFI_RCI2_TABLE_GUID EFI_GUID(0x2d9f28a2, 0xa886, 0x456a, 0x97, 0xa8, 0xf1, 0x1e, 0xf2, 0x4f, 0xf4, 0x55) + typedef struct { efi_guid_t guid; u64 table; @@ -1713,6 +1716,8 @@ struct efi_tcg2_final_events_table { }; extern int efi_tpm_final_log_size; +extern unsigned long rci2_table_phys; + /* * efi_runtime_service() function identifiers. * "NONE" is used by efi_recover_from_page_fault() to check if the page