Re: [PATCH v0] Export Runtime Configuration Interface table to sysfs

[<Narendra.K@dell.com>]

On Tue, Jul 02, 2019 at 09:57:24PM +0200, Ard Biesheuvel wrote:
> On Sat, 29 Jun 2019 at 13:23, <Narendra.K@dell.com> 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>
> > ---
> > RFC -> v0:
> >
> > - Removed rci2 table from struct efi and defined it in rci2_table.c similar to
> > the way uv_systab_phys is define in dmesg.
> > - Removed the oem_tables array and added rci2 to common_tables array
> > - Removed the string 'rci2' from the common_tables array so that it is
> > not printed in dmesg.
> > - Merged function 'efi_rci2_table_init' into 'efi_rci2_sysfs_init' function to
> > avoid calling early_memremap/unmap functions.
> >
> > Also, a note unrelated to this patch - compilation error is observed when
> > testing with make defconfig related to uv_systab_phys in
> > arch/x86/platform/efi/efi.c. It seems like it needs to be protected with
> > CONFIG_X86_UV in efi_tables array.
> >
> 
> Yes, I noticed this as well. This has been fixed now
> 
> >
> >  Documentation/ABI/testing/sysfs-firmware-efi |   8 +
> >  arch/x86/platform/efi/efi.c                  |   1 +
> >  drivers/firmware/efi/Makefile                |   2 +-
> >  drivers/firmware/efi/efi.c                   |   4 +
> >  drivers/firmware/efi/rci2_table.c            | 147 +++++++++++++++++++
> >  include/linux/efi.h                          |   6 +
> >  6 files changed, 167 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/firmware/efi/rci2_table.c
> >
> > diff --git a/Documentation/ABI/testing/sysfs-firmware-efi b/Documentation/ABI/testing/sysfs-firmware-efi
> > index e794eac32a90..f7822c522a46 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:          June 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 002078645969..6e1c1b0ce015 100644
> > --- a/arch/x86/platform/efi/efi.c
> > +++ b/arch/x86/platform/efi/efi.c
> > @@ -80,6 +80,7 @@ static const unsigned long * const efi_tables[] = {
> >         &efi.esrt,
> >         &efi.properties_table,
> >         &efi.mem_attr_table,
> > +       &rci2_table_phys,
> 
> Put #ifdef CONFIG_EFI_RCI2 around this line ^^^
> 
> >  };
> >
> >  u64 efi_setup;         /* efi setup_data physical address */
> > diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
> > index d2d0d2030620..db07828ca1ed 100644
> > --- a/drivers/firmware/efi/Makefile
> > +++ b/drivers/firmware/efi/Makefile
> > @@ -11,7 +11,7 @@
> >  KASAN_SANITIZE_runtime-wrappers.o      := n
> >
> >  obj-$(CONFIG_ACPI_BGRT)                += efi-bgrt.o
> > -obj-$(CONFIG_EFI)                      += efi.o vars.o reboot.o memattr.o tpm.o
> > +obj-$(CONFIG_EFI)                      += efi.o vars.o reboot.o memattr.o tpm.o rci2_table.o
> 
> Please introduce a kconfig symbol CONFIG_EFI_RCI2 for this, and only
> include this file when the symbol is set. You can make it default to y
> on X86, but for other architectures, it does not make a lot of sense
> to include this by default.

Apologies for the delay in responding. I will add the kconfig option and
related changes in the next version of the patch. I think the config
option could be N by default with a note added in the help section to
set it to Y for Dell EMC PowerEdge systems. Please share your thoughts
on setting it to N by default.

[...]

-- 
With regards,
Narendra K