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

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Wed, 7 Aug 2019 at 16:09, <Narendra.K@dell.com> wrote:
>
> On Thu, Jul 11, 2019 at 11:00:37PM +0000, Limonciello, Mario wrote:
> > > -----Original Message-----
> > > From: K, Narendra
> > > Sent: Wednesday, July 10, 2019 11:59 AM
> > > To: linux-efi@vger.kernel.org; ard.biesheuvel@linaro.org; pjones@redhat.com
> > > Cc: K, Narendra; Hayes, Stuart; Limonciello, Mario
> > > Subject: [PATCH v1] Export Runtime Configuration Interface table to sysfs
> > >
> > > 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>
>
> Hi Ard,
>
> Does the version 1 of the patch look good ? Please share your thoughts.
>

Thanks Narendra,

The patch looks mostly fine. I have pushed it to my efi/next branch,
and I will let you know if the autobuilders find any problems.

One possible enhancement would be to defer the second memremap() call
until the first call to raw_table_read(), so the mapping only exists
if you are actually interested in the contents of the table. If you do
decide to make any followup changes, please send them as delta patches
against https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/log/?h=next

Thanks,
Ard.




> >
> > > ---
> > > Hi Ard, the review comment in the v0 version of the patch suggested that the
> > > kconfig symbol be set to Y for X86. I made a change to the suggestion.
> > > In the v1 version, I have set the symbol to N by default and added a note to the
> > > help section to make it Y for Dell EMC PowerEdge systems. If it needs to be
> > > changed, I will resubmit the patch after changing it to implement the suggestion.
> > >
> > > The patch is created on 'next' branch of efi tree.
> > >
> > > v0 -> v1:
> > > - Introduced a new Kconfig symbol CONFIG_EFI_RCI2_TABLE and compile
> > > RCI2 table support if it is set. Set the symbol to N by default.
> > > - Removed calling 'efi_rci2_sysfs_init' from drivers/firmware/efi/efi.c and made
> > > it a 'late_initcall' in drivers/firmware/efi/rci2_table.c.
> > > Removed the function declaration from include/linux/efi.h.
> > >
> > > RFC -> v0:
> > > - Removed rci2 table from struct efi and defined it in rci2_table.c similar to the
> > > way uv_systab_phys is defined in arch/x86/platform/uv/bios_uv.c
> > > - 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.
>
> --
> With regards,
> Narendra K