[v2,2/3] firmware: introduce FFI for SMBIOS entry.
Commit Message
1. Some bootloaders do not support EFI, and the transfer of
firmware information can only be done through DTS,
such as Coreboot.
2. Some arches do not have a reserved address segment that
can be used to pass firmware parameters like x86.
3. Based on this, we have added an interface to obtain firmware
information through FDT, named FFI.
4. We not only use FFI to pass SMBIOS entry,
but also pass other firmware information as an extension.
Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
MAINTAINERS | 6 ++
drivers/firmware/Kconfig | 11 ++++
drivers/firmware/Makefile | 1 +
drivers/firmware/dmi_scan.c | 128 +++++++++++++++++-------------------
drivers/firmware/ffi.c | 36 ++++++++++
include/linux/ffi.h | 15 +++++
6 files changed, 128 insertions(+), 69 deletions(-)
create mode 100644 drivers/firmware/ffi.c
create mode 100644 include/linux/ffi.h
Comments
Hey,
On Sun, Jul 02, 2023 at 05:57:33PM +0800, Yunhui Cui wrote:
> 1. Some bootloaders do not support EFI, and the transfer of
> firmware information can only be done through DTS,
> such as Coreboot.
>
> 2. Some arches do not have a reserved address segment that
> can be used to pass firmware parameters like x86.
>
> 3. Based on this, we have added an interface to obtain firmware
> information through FDT, named FFI.
>
> 4. We not only use FFI to pass SMBIOS entry,
> but also pass other firmware information as an extension.
nit: please don't write your commit messages as bullet lists
> +FDT FIRMWARE INTERFACE (FFI)
> +M: Yunhui Cui cuiyunhui@bytedance.com
> +S: Maintained
> +F: drivers/firmware/ffi.c
> +F: include/linux/ffi.h
Are you going to apply patches for this, or is someone else?
> EXTERNAL CONNECTOR SUBSYSTEM (EXTCON)
> M: MyungJoo Ham <myungjoo.ham@samsung.com>
> M: Chanwoo Choi <cw00.choi@samsung.com>
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index b59e3041fd62..ea0149fb4683 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -303,6 +303,17 @@ config TURRIS_MOX_RWTM
> other manufacturing data and also utilize the Entropy Bit Generator
> for hardware random number generation.
>
> +config FDT_FW_INTERFACE
> + bool "An interface for passing firmware info through FDT"
> + depends on OF && OF_FLATTREE
> + default n
> + help
> + When some bootloaders do not support EFI, and the arch does not
> + support SMBIOS_ENTRY_POINT_SCAN_START, then you can enable this option
> + to support the transfer of firmware information, such as smbios tables.
Could you express this dependency on !SMBIOS_ENTRY_POINT_SCAN_START in
Kconfig & then simply the text to:
"Enable this option to support the transfer of firmware information,
such as smbios tables, for bootloaders that do not support EFI."
since it would not even appear if the arch supports scanning for the
entry point?
If I was was a punter trying to configure my kernel in menuconfig or
whatever, I should be able to decide based on the help text if I need
this, not going grepping for #defines in headers.
> static void __init dmi_scan_machine(void)
> @@ -660,58 +686,22 @@ static void __init dmi_scan_machine(void)
> char __iomem *p, *q;
> char buf[32];
>
> +#ifdef CONFIG_FDT_FW_INTERFACE
> + if (dmi_sacn_smbios(ffi.smbios3, ffi.smbios))
"dmi_sacn_smbios"
> + goto error;
> +#endif
Does this not mean that if FDT_FW_INTERFACE is enabled, but the platform
wants to use EFI, it won't be able to? The `goto error;` makes this look
mutually exclusive to my efi-unaware eyes.
> if (efi_enabled(EFI_CONFIG_TABLES)) {
> - /*
> - * According to the DMTF SMBIOS reference spec v3.0.0, it is
> - * allowed to define both the 64-bit entry point (smbios3) and
> - * the 32-bit entry point (smbios), in which case they should
> - * either both point to the same SMBIOS structure table, or the
> - * table pointed to by the 64-bit entry point should contain a
> - * superset of the table contents pointed to by the 32-bit entry
> - * point (section 5.2)
> - * This implies that the 64-bit entry point should have
> - * precedence if it is defined and supported by the OS. If we
> - * have the 64-bit entry point, but fail to decode it, fall
> - * back to the legacy one (if available)
> - */
> - if (efi.smbios3 != EFI_INVALID_TABLE_ADDR) {
> - p = dmi_early_remap(efi.smbios3, 32);
> - if (p == NULL)
> - goto error;
> - memcpy_fromio(buf, p, 32);
> - dmi_early_unmap(p, 32);
> -
> - if (!dmi_smbios3_present(buf)) {
> - dmi_available = 1;
> - return;
> - }
> - }
> - if (efi.smbios == EFI_INVALID_TABLE_ADDR)
> + if (dmi_sacn_smbios(efi.smbios3, efi.smbios))
> goto error;
> -
> - /* This is called as a core_initcall() because it isn't
> - * needed during early boot. This also means we can
> - * iounmap the space when we're done with it.
> - */
> - p = dmi_early_remap(efi.smbios, 32);
> - if (p == NULL)
> - goto error;
> - memcpy_fromio(buf, p, 32);
> - dmi_early_unmap(p, 32);
> -
> - if (!dmi_present(buf)) {
> - dmi_available = 1;
> - return;
> - }
> diff --git a/drivers/firmware/ffi.c b/drivers/firmware/ffi.c
> new file mode 100644
> index 000000000000..169802b4a7a8
> --- /dev/null
> +++ b/drivers/firmware/ffi.c
> @@ -0,0 +1,36 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/of.h>
> +#include <linux/of_fdt.h>
> +#include <linux/libfdt.h>
> +#include <linux/ffi.h>
> +
> +#define FFI_INVALID_TABLE_ADDR (~0UL)
> +
> +struct ffi __read_mostly ffi = {
> + .smbios = FFI_INVALID_TABLE_ADDR,
> + .smbios3 = FFI_INVALID_TABLE_ADDR,
> +};
> +EXPORT_SYMBOL(ffi);
> +// SPDX-License-Identifier: GPL-2.0-only
Why not EXPORT_SYMBOL_GPL? But also, who is the user of this export?
> +
> +void __init ffi_smbios_root_pointer(void)
> +{
> + int cfgtbl, len;
> + fdt64_t *prop;
> +
> + cfgtbl = fdt_path_offset(initial_boot_params, "/cfgtables");
These DT properties need to be documented in a binding.
> + if (cfgtbl < 0) {
> + pr_info("firmware table not found.\n");
Isn't it perfectly valid for a DT not to contain this table? This print
should be, at the very least, a pr_debug().
> + return;
> + }
> + prop = fdt_getprop_w(initial_boot_params, cfgtbl, "smbios_phy_ptr", &len);
Again, undocumented DT property. Please document them in a binding.
> + if (!prop || len != sizeof(u64))
> + pr_info("smbios entry point not found.\n");
> + else
> + ffi.smbios = fdt64_to_cpu(*prop);
> +
> + pr_info("smbios root pointer: %lx\n", ffi.smbios);
ffi.smbios is not set if (!prop || len != sizeof(u64)), looks like your
"if" should return and the contents of the else become unconditional?
Otherwise, this print seems wrong.
> +}
> +
> diff --git a/include/linux/ffi.h b/include/linux/ffi.h
> new file mode 100644
> index 000000000000..95298a805222
> --- /dev/null
> +++ b/include/linux/ffi.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _LINUX_FFI_H
> +#define _LINUX_FFI_H
> +
> +extern struct ffi {
> + unsigned long smbios; /* SMBIOS table (32 bit entry point) */
> + unsigned long smbios3; /* SMBIOS table (64 bit entry point) */
> + unsigned long flags;
> +
> +} ffi;
> +
> +void ffi_smbios_root_pointer(void);
Please provide a stub for !FDT_FW_INTERFACE so that we don't need
ifdeffery at callsites.
Cheers,
Conor.
Hi Conor,
> nit: please don't write your commit messages as bullet lists
Okay, thanks for your suggestion.
> > +FDT FIRMWARE INTERFACE (FFI)
> > +M: Yunhui Cui cuiyunhui@bytedance.com
> > +S: Maintained
> > +F: drivers/firmware/ffi.c
> > +F: include/linux/ffi.h
>
> Are you going to apply patches for this, or is someone else?
Yes, it will be used by patch 3/3.
>
> > EXTERNAL CONNECTOR SUBSYSTEM (EXTCON)
> > M: MyungJoo Ham <myungjoo.ham@samsung.com>
> > M: Chanwoo Choi <cw00.choi@samsung.com>
> > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> > index b59e3041fd62..ea0149fb4683 100644
> > --- a/drivers/firmware/Kconfig
> > +++ b/drivers/firmware/Kconfig
> > @@ -303,6 +303,17 @@ config TURRIS_MOX_RWTM
> > other manufacturing data and also utilize the Entropy Bit Generator
> > for hardware random number generation.
> >
> > +config FDT_FW_INTERFACE
> > + bool "An interface for passing firmware info through FDT"
> > + depends on OF && OF_FLATTREE
> > + default n
> > + help
> > + When some bootloaders do not support EFI, and the arch does not
> > + support SMBIOS_ENTRY_POINT_SCAN_START, then you can enable this option
> > + to support the transfer of firmware information, such as smbios tables.
>
> Could you express this dependency on !SMBIOS_ENTRY_POINT_SCAN_START in
> Kconfig & then simply the text to:
> "Enable this option to support the transfer of firmware information,
> such as smbios tables, for bootloaders that do not support EFI."
> since it would not even appear if the arch supports scanning for the
> entry point?
> If I was was a punter trying to configure my kernel in menuconfig or
> whatever, I should be able to decide based on the help text if I need
> this, not going grepping for #defines in headers.
Okay， I'll update on v3.
>
> > static void __init dmi_scan_machine(void)
> > @@ -660,58 +686,22 @@ static void __init dmi_scan_machine(void)
> > char __iomem *p, *q;
> > char buf[32];
> >
> > +#ifdef CONFIG_FDT_FW_INTERFACE
> > + if (dmi_sacn_smbios(ffi.smbios3, ffi.smbios))
>
> "dmi_sacn_smbios"
>
> > + goto error;
> > +#endif
>
> Does this not mean that if FDT_FW_INTERFACE is enabled, but the platform
> wants to use EFI, it won't be able to? The `goto error;` makes this look
> mutually exclusive to my efi-unaware eyes.
If you have enabled FFI, then if something goes wrong, you should goto error.
Just like the origin code:
if (efi_enabled(EFI_CONFIG_TABLES)) {
if (dmi_sacn_smbios(efi.smbios3, efi.smbios))
goto error;
} else if (IS_ENABLED(CONFIG_DMI_SCAN_MACHINE_NON_EFI_FALLBACK)) {
p = dmi_early_remap(SMBIOS_ENTRY_POINT_SCAN_START, 0x10000);
if (p == NULL)
goto error;
....
}
>
> > if (efi_enabled(EFI_CONFIG_TABLES)) {
> > - /*
> > - * According to the DMTF SMBIOS reference spec v3.0.0, it is
> > - * allowed to define both the 64-bit entry point (smbios3) and
> > - * the 32-bit entry point (smbios), in which case they should
> > - * either both point to the same SMBIOS structure table, or the
> > - * table pointed to by the 64-bit entry point should contain a
> > - * superset of the table contents pointed to by the 32-bit entry
> > - * point (section 5.2)
> > - * This implies that the 64-bit entry point should have
> > - * precedence if it is defined and supported by the OS. If we
> > - * have the 64-bit entry point, but fail to decode it, fall
> > - * back to the legacy one (if available)
> > - */
> > - if (efi.smbios3 != EFI_INVALID_TABLE_ADDR) {
> > - p = dmi_early_remap(efi.smbios3, 32);
> > - if (p == NULL)
> > - goto error;
> > - memcpy_fromio(buf, p, 32);
> > - dmi_early_unmap(p, 32);
> > -
> > - if (!dmi_smbios3_present(buf)) {
> > - dmi_available = 1;
> > - return;
> > - }
> > - }
> > - if (efi.smbios == EFI_INVALID_TABLE_ADDR)
> > + if (dmi_sacn_smbios(efi.smbios3, efi.smbios))
> > goto error;
> > -
> > - /* This is called as a core_initcall() because it isn't
> > - * needed during early boot. This also means we can
> > - * iounmap the space when we're done with it.
> > - */
> > - p = dmi_early_remap(efi.smbios, 32);
> > - if (p == NULL)
> > - goto error;
> > - memcpy_fromio(buf, p, 32);
> > - dmi_early_unmap(p, 32);
> > -
> > - if (!dmi_present(buf)) {
> > - dmi_available = 1;
> > - return;
> > - }
> > diff --git a/drivers/firmware/ffi.c b/drivers/firmware/ffi.c
> > new file mode 100644
> > index 000000000000..169802b4a7a8
> > --- /dev/null
> > +++ b/drivers/firmware/ffi.c
> > @@ -0,0 +1,36 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/of.h>
> > +#include <linux/of_fdt.h>
> > +#include <linux/libfdt.h>
> > +#include <linux/ffi.h>
> > +
> > +#define FFI_INVALID_TABLE_ADDR (~0UL)
> > +
> > +struct ffi __read_mostly ffi = {
> > + .smbios = FFI_INVALID_TABLE_ADDR,
> > + .smbios3 = FFI_INVALID_TABLE_ADDR,
> > +};
>
> > +EXPORT_SYMBOL(ffi);
>
> > +// SPDX-License-Identifier: GPL-2.0-only
>
> Why not EXPORT_SYMBOL_GPL? But also, who is the user of this export?
Just like efi.
>
> > +
> > +void __init ffi_smbios_root_pointer(void)
> > +{
> > + int cfgtbl, len;
> > + fdt64_t *prop;
> > +
> > + cfgtbl = fdt_path_offset(initial_boot_params, "/cfgtables");
>
> These DT properties need to be documented in a binding.
>
> > + if (cfgtbl < 0) {
> > + pr_info("firmware table not found.\n");
>
> Isn't it perfectly valid for a DT not to contain this table? This print
> should be, at the very least, a pr_debug().
>
> > + return;
> > + }
> > + prop = fdt_getprop_w(initial_boot_params, cfgtbl, "smbios_phy_ptr", &len);
>
> Again, undocumented DT property. Please document them in a binding.
Okay, I'll add them into binding.
>
> > + if (!prop || len != sizeof(u64))
> > + pr_info("smbios entry point not found.\n");
> > + else
> > + ffi.smbios = fdt64_to_cpu(*prop);
> > +
> > + pr_info("smbios root pointer: %lx\n", ffi.smbios);
>
> ffi.smbios is not set if (!prop || len != sizeof(u64)), looks like your
> "if" should return and the contents of the else become unconditional?
> Otherwise, this print seems wrong.
OK, I will optimize this logic and print.
>
> > +}
> > +
> > diff --git a/include/linux/ffi.h b/include/linux/ffi.h
> > new file mode 100644
> > index 000000000000..95298a805222
> > --- /dev/null
> > +++ b/include/linux/ffi.h
> > @@ -0,0 +1,15 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef _LINUX_FFI_H
> > +#define _LINUX_FFI_H
> > +
> > +extern struct ffi {
> > + unsigned long smbios; /* SMBIOS table (32 bit entry point) */
> > + unsigned long smbios3; /* SMBIOS table (64 bit entry point) */
> > + unsigned long flags;
> > +
> > +} ffi;
> > +
> > +void ffi_smbios_root_pointer(void);
>
> Please provide a stub for !FDT_FW_INTERFACE so that we don't need
> ifdeffery at callsites.
OK, update it on v3.
Thanks,
Yunhui
Hey,
On Mon, Jul 03, 2023 at 04:23:53PM +0800, 运辉崔 wrote:
>
> > nit: please don't write your commit messages as bullet lists
> Okay, thanks for your suggestion.
>
> > > +FDT FIRMWARE INTERFACE (FFI)
> > > +M: Yunhui Cui cuiyunhui@bytedance.com
> > > +S: Maintained
> > > +F: drivers/firmware/ffi.c
> > > +F: include/linux/ffi.h
> >
> > Are you going to apply patches for this, or is someone else?
> Yes, it will be used by patch 3/3.
That's not what I asked :(
> > > EXTERNAL CONNECTOR SUBSYSTEM (EXTCON)
> > > M: MyungJoo Ham <myungjoo.ham@samsung.com>
> > > M: Chanwoo Choi <cw00.choi@samsung.com>
> > > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> > > index b59e3041fd62..ea0149fb4683 100644
> > > --- a/drivers/firmware/Kconfig
> > > +++ b/drivers/firmware/Kconfig
> > > @@ -303,6 +303,17 @@ config TURRIS_MOX_RWTM
> > > other manufacturing data and also utilize the Entropy Bit Generator
> > > for hardware random number generation.
> > >
> > > +config FDT_FW_INTERFACE
> > > + bool "An interface for passing firmware info through FDT"
> > > + depends on OF && OF_FLATTREE
> > > + default n
> > > + help
> > > + When some bootloaders do not support EFI, and the arch does not
> > > + support SMBIOS_ENTRY_POINT_SCAN_START, then you can enable this option
> > > + to support the transfer of firmware information, such as smbios tables.
> >
> > Could you express this dependency on !SMBIOS_ENTRY_POINT_SCAN_START in
> > Kconfig & then simply the text to:
> > "Enable this option to support the transfer of firmware information,
> > such as smbios tables, for bootloaders that do not support EFI."
> > since it would not even appear if the arch supports scanning for the
> > entry point?
> > If I was was a punter trying to configure my kernel in menuconfig or
> > whatever, I should be able to decide based on the help text if I need
> > this, not going grepping for #defines in headers.
> Okay， I'll update on v3.
>
>
> >
> > > static void __init dmi_scan_machine(void)
> > > @@ -660,58 +686,22 @@ static void __init dmi_scan_machine(void)
> > > char __iomem *p, *q;
> > > char buf[32];
> > >
> > > +#ifdef CONFIG_FDT_FW_INTERFACE
> > > + if (dmi_sacn_smbios(ffi.smbios3, ffi.smbios))
> >
> > "dmi_sacn_smbios"
> >
> > > + goto error;
> > > +#endif
> >
> > Does this not mean that if FDT_FW_INTERFACE is enabled, but the platform
> > wants to use EFI, it won't be able to? The `goto error;` makes this look
> > mutually exclusive to my efi-unaware eyes.
>
> If you have enabled FFI, then if something goes wrong, you should goto error.
> Just like the origin code:
> if (efi_enabled(EFI_CONFIG_TABLES)) {
> if (dmi_sacn_smbios(efi.smbios3, efi.smbios))
> goto error;
> } else if (IS_ENABLED(CONFIG_DMI_SCAN_MACHINE_NON_EFI_FALLBACK)) {
> p = dmi_early_remap(SMBIOS_ENTRY_POINT_SCAN_START, 0x10000);
> if (p == NULL)
> goto error;
Does this not make FFI and EFI mutually exclusive Kconfig options?
Suppose you are on a system that does not implement FFI, but does
implement EFI - what's going to happen then?
AFAICT, dmi_sacn_smbios(ffi.smbios3, ffi.smbios) will fail & you'll do a
`goto error` & skip the EFI code. What am I missing?
> > > if (efi_enabled(EFI_CONFIG_TABLES)) {
> > > - /*
> > > - * According to the DMTF SMBIOS reference spec v3.0.0, it is
> > > - * allowed to define both the 64-bit entry point (smbios3) and
> > > - * the 32-bit entry point (smbios), in which case they should
> > > - * either both point to the same SMBIOS structure table, or the
> > > - * table pointed to by the 64-bit entry point should contain a
> > > - * superset of the table contents pointed to by the 32-bit entry
> > > - * point (section 5.2)
> > > - * This implies that the 64-bit entry point should have
> > > - * precedence if it is defined and supported by the OS. If we
> > > - * have the 64-bit entry point, but fail to decode it, fall
> > > - * back to the legacy one (if available)
> > > - */
> > > - if (efi.smbios3 != EFI_INVALID_TABLE_ADDR) {
> > > - p = dmi_early_remap(efi.smbios3, 32);
> > > - if (p == NULL)
> > > - goto error;
> > > - memcpy_fromio(buf, p, 32);
> > > - dmi_early_unmap(p, 32);
> > > -
> > > - if (!dmi_smbios3_present(buf)) {
> > > - dmi_available = 1;
> > > - return;
> > > - }
> > > - }
> > > - if (efi.smbios == EFI_INVALID_TABLE_ADDR)
> > > + if (dmi_sacn_smbios(efi.smbios3, efi.smbios))
> > > goto error;
> > > -
> > > - /* This is called as a core_initcall() because it isn't
> > > - * needed during early boot. This also means we can
> > > - * iounmap the space when we're done with it.
> > > - */
> > > - p = dmi_early_remap(efi.smbios, 32);
> > > - if (p == NULL)
> > > - goto error;
> > > - memcpy_fromio(buf, p, 32);
> > > - dmi_early_unmap(p, 32);
> > > -
> > > - if (!dmi_present(buf)) {
> > > - dmi_available = 1;
> > > - return;
> > > - }
> > > diff --git a/drivers/firmware/ffi.c b/drivers/firmware/ffi.c
> > > new file mode 100644
> > > index 000000000000..169802b4a7a8
> > > --- /dev/null
> > > +++ b/drivers/firmware/ffi.c
> > > @@ -0,0 +1,36 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +
> > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > +
> > > +#include <linux/of.h>
> > > +#include <linux/of_fdt.h>
> > > +#include <linux/libfdt.h>
> > > +#include <linux/ffi.h>
> > > +
> > > +#define FFI_INVALID_TABLE_ADDR (~0UL)
> > > +
> > > +struct ffi __read_mostly ffi = {
> > > + .smbios = FFI_INVALID_TABLE_ADDR,
> > > + .smbios3 = FFI_INVALID_TABLE_ADDR,
> > > +};
> >
> > > +EXPORT_SYMBOL(ffi);
> >
> > > +// SPDX-License-Identifier: GPL-2.0-only
> >
> > Why not EXPORT_SYMBOL_GPL? But also, who is the user of this export?
> Just like efi.
I don't really understand how that is an answer to the questions.
> > > +
> > > +void __init ffi_smbios_root_pointer(void)
> > > +{
> > > + int cfgtbl, len;
> > > + fdt64_t *prop;
> > > +
> > > + cfgtbl = fdt_path_offset(initial_boot_params, "/cfgtables");
> >
> > These DT properties need to be documented in a binding.
> >
> > > + if (cfgtbl < 0) {
> > > + pr_info("firmware table not found.\n");
> >
> > Isn't it perfectly valid for a DT not to contain this table? This print
> > should be, at the very least, a pr_debug().
> >
> > > + return;
> > > + }
> > > + prop = fdt_getprop_w(initial_boot_params, cfgtbl, "smbios_phy_ptr", &len);
> >
> > Again, undocumented DT property. Please document them in a binding.
> Okay, I'll add them into binding.
>
>
> >
> > > + if (!prop || len != sizeof(u64))
> > > + pr_info("smbios entry point not found.\n");
> > > + else
> > > + ffi.smbios = fdt64_to_cpu(*prop);
> > > +
> > > + pr_info("smbios root pointer: %lx\n", ffi.smbios);
> >
> > ffi.smbios is not set if (!prop || len != sizeof(u64)), looks like your
> > "if" should return and the contents of the else become unconditional?
> > Otherwise, this print seems wrong.
> OK, I will optimize this logic and print.
It's not an optimisation. If the if branch of your code is taken, it
currently will do
pr_info("smbios entry point not found.\n");
pr_info("smbios root pointer: %lx\n", ffi.smbios);
which makes no sense...
Thanks,
Conor.
Hi Conor,
Thanks for your comments.
On Mon, Jul 3, 2023 at 4:36 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> Hey,
>
> On Mon, Jul 03, 2023 at 04:23:53PM +0800, 运辉崔 wrote:
> >
> > > nit: please don't write your commit messages as bullet lists
> > Okay, thanks for your suggestion.
> >
> > > > +FDT FIRMWARE INTERFACE (FFI)
> > > > +M: Yunhui Cui cuiyunhui@bytedance.com
> > > > +S: Maintained
> > > > +F: drivers/firmware/ffi.c
> > > > +F: include/linux/ffi.h
> > >
> > > Are you going to apply patches for this, or is someone else?
> > Yes, it will be used by patch 3/3.
>
> That's not what I asked :(
Sorry, ok, what do you want to ask?
> > > > EXTERNAL CONNECTOR SUBSYSTEM (EXTCON)
> > > > M: MyungJoo Ham <myungjoo.ham@samsung.com>
> > > > M: Chanwoo Choi <cw00.choi@samsung.com>
> > > > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> > > > index b59e3041fd62..ea0149fb4683 100644
> > > > --- a/drivers/firmware/Kconfig
> > > > +++ b/drivers/firmware/Kconfig
> > > > @@ -303,6 +303,17 @@ config TURRIS_MOX_RWTM
> > > > other manufacturing data and also utilize the Entropy Bit Generator
> > > > for hardware random number generation.
> > > >
> > > > +config FDT_FW_INTERFACE
> > > > + bool "An interface for passing firmware info through FDT"
> > > > + depends on OF && OF_FLATTREE
> > > > + default n
> > > > + help
> > > > + When some bootloaders do not support EFI, and the arch does not
> > > > + support SMBIOS_ENTRY_POINT_SCAN_START, then you can enable this option
> > > > + to support the transfer of firmware information, such as smbios tables.
> > >
> > > Could you express this dependency on !SMBIOS_ENTRY_POINT_SCAN_START in
> > > Kconfig & then simply the text to:
> > > "Enable this option to support the transfer of firmware information,
> > > such as smbios tables, for bootloaders that do not support EFI."
> > > since it would not even appear if the arch supports scanning for the
> > > entry point?
> > > If I was was a punter trying to configure my kernel in menuconfig or
> > > whatever, I should be able to decide based on the help text if I need
> > > this, not going grepping for #defines in headers.
> > Okay， I'll update on v3.
> >
> >
> > >
> > > > static void __init dmi_scan_machine(void)
> > > > @@ -660,58 +686,22 @@ static void __init dmi_scan_machine(void)
> > > > char __iomem *p, *q;
> > > > char buf[32];
> > > >
> > > > +#ifdef CONFIG_FDT_FW_INTERFACE
> > > > + if (dmi_sacn_smbios(ffi.smbios3, ffi.smbios))
> > >
> > > "dmi_sacn_smbios"
> > >
> > > > + goto error;
> > > > +#endif
> > >
> > > Does this not mean that if FDT_FW_INTERFACE is enabled, but the platform
> > > wants to use EFI, it won't be able to? The `goto error;` makes this look
> > > mutually exclusive to my efi-unaware eyes.
> >
> > If you have enabled FFI, then if something goes wrong, you should goto error.
> > Just like the origin code:
> > if (efi_enabled(EFI_CONFIG_TABLES)) {
> > if (dmi_sacn_smbios(efi.smbios3, efi.smbios))
> > goto error;
> > } else if (IS_ENABLED(CONFIG_DMI_SCAN_MACHINE_NON_EFI_FALLBACK)) {
> > p = dmi_early_remap(SMBIOS_ENTRY_POINT_SCAN_START, 0x10000);
> > if (p == NULL)
> > goto error;
>
> Does this not make FFI and EFI mutually exclusive Kconfig options?
> Suppose you are on a system that does not implement FFI, but does
> implement EFI - what's going to happen then?
> AFAICT, dmi_sacn_smbios(ffi.smbios3, ffi.smbios) will fail & you'll do a
> `goto error` & skip the EFI code. What am I missing?
Code is not intended to be mutually exclusive, get the correct value and return,
The code is going to be changed to this:
#ifdef CONFIG_FDT_FW_INTERFACE
if (ffi_enabled(FFI_CONFIG_TABLES)) {
if (!dmi_sacn_smbios(ffi.smbios3, ffi.smbios))
return;
}
#endif
>
> > > > if (efi_enabled(EFI_CONFIG_TABLES)) {
> > > > - /*
> > > > - * According to the DMTF SMBIOS reference spec v3.0.0, it is
> > > > - * allowed to define both the 64-bit entry point (smbios3) and
> > > > - * the 32-bit entry point (smbios), in which case they should
> > > > - * either both point to the same SMBIOS structure table, or the
> > > > - * table pointed to by the 64-bit entry point should contain a
> > > > - * superset of the table contents pointed to by the 32-bit entry
> > > > - * point (section 5.2)
> > > > - * This implies that the 64-bit entry point should have
> > > > - * precedence if it is defined and supported by the OS. If we
> > > > - * have the 64-bit entry point, but fail to decode it, fall
> > > > - * back to the legacy one (if available)
> > > > - */
> > > > - if (efi.smbios3 != EFI_INVALID_TABLE_ADDR) {
> > > > - p = dmi_early_remap(efi.smbios3, 32);
> > > > - if (p == NULL)
> > > > - goto error;
> > > > - memcpy_fromio(buf, p, 32);
> > > > - dmi_early_unmap(p, 32);
> > > > -
> > > > - if (!dmi_smbios3_present(buf)) {
> > > > - dmi_available = 1;
> > > > - return;
> > > > - }
> > > > - }
> > > > - if (efi.smbios == EFI_INVALID_TABLE_ADDR)
> > > > + if (dmi_sacn_smbios(efi.smbios3, efi.smbios))
> > > > goto error;
> > > > -
> > > > - /* This is called as a core_initcall() because it isn't
> > > > - * needed during early boot. This also means we can
> > > > - * iounmap the space when we're done with it.
> > > > - */
> > > > - p = dmi_early_remap(efi.smbios, 32);
> > > > - if (p == NULL)
> > > > - goto error;
> > > > - memcpy_fromio(buf, p, 32);
> > > > - dmi_early_unmap(p, 32);
> > > > -
> > > > - if (!dmi_present(buf)) {
> > > > - dmi_available = 1;
> > > > - return;
> > > > - }
> > > > diff --git a/drivers/firmware/ffi.c b/drivers/firmware/ffi.c
> > > > new file mode 100644
> > > > index 000000000000..169802b4a7a8
> > > > --- /dev/null
> > > > +++ b/drivers/firmware/ffi.c
> > > > @@ -0,0 +1,36 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > +
> > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > > +
> > > > +#include <linux/of.h>
> > > > +#include <linux/of_fdt.h>
> > > > +#include <linux/libfdt.h>
> > > > +#include <linux/ffi.h>
> > > > +
> > > > +#define FFI_INVALID_TABLE_ADDR (~0UL)
> > > > +
> > > > +struct ffi __read_mostly ffi = {
> > > > + .smbios = FFI_INVALID_TABLE_ADDR,
> > > > + .smbios3 = FFI_INVALID_TABLE_ADDR,
> > > > +};
> > >
> > > > +EXPORT_SYMBOL(ffi);
> > >
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > >
> > > Why not EXPORT_SYMBOL_GPL? But also, who is the user of this export?
> > Just like efi.
>
> I don't really understand how that is an answer to the questions.
I checked, the code is executed as the system starts, either Y or N, M
will not appear, and the same is true for ffi's user DMI.
So no need for EXPORT.
>
> > > > +
> > > > +void __init ffi_smbios_root_pointer(void)
> > > > +{
> > > > + int cfgtbl, len;
> > > > + fdt64_t *prop;
> > > > +
> > > > + cfgtbl = fdt_path_offset(initial_boot_params, "/cfgtables");
> > >
> > > These DT properties need to be documented in a binding.
> > >
> > > > + if (cfgtbl < 0) {
> > > > + pr_info("firmware table not found.\n");
> > >
> > > Isn't it perfectly valid for a DT not to contain this table? This print
> > > should be, at the very least, a pr_debug().
> > >
> > > > + return;
> > > > + }
> > > > + prop = fdt_getprop_w(initial_boot_params, cfgtbl, "smbios_phy_ptr", &len);
> > >
> > > Again, undocumented DT property. Please document them in a binding.
> > Okay, I'll add them into binding.
> >
> >
> > >
> > > > + if (!prop || len != sizeof(u64))
> > > > + pr_info("smbios entry point not found.\n");
> > > > + else
> > > > + ffi.smbios = fdt64_to_cpu(*prop);
> > > > +
> > > > + pr_info("smbios root pointer: %lx\n", ffi.smbios);
> > >
> > > ffi.smbios is not set if (!prop || len != sizeof(u64)), looks like your
> > > "if" should return and the contents of the else become unconditional?
> > > Otherwise, this print seems wrong.
>
> > OK, I will optimize this logic and print.
>
> It's not an optimisation. If the if branch of your code is taken, it
> currently will do
> pr_info("smbios entry point not found.\n");
> pr_info("smbios root pointer: %lx\n", ffi.smbios);
> which makes no sense...
Yeah, I got it, that's what I mean by "optimize".
>
> Thanks,
> Conor.
>
Thanks,
Yunhui
On Mon, Jul 03, 2023 at 08:41:30PM +0800, 运辉崔 wrote:
> On Mon, Jul 3, 2023 at 4:36 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> > On Mon, Jul 03, 2023 at 04:23:53PM +0800, 运辉崔 wrote:
> > > > > +FDT FIRMWARE INTERFACE (FFI)
> > > > > +M: Yunhui Cui cuiyunhui@bytedance.com
> > > > > +S: Maintained
> > > > > +F: drivers/firmware/ffi.c
> > > > > +F: include/linux/ffi.h
> > > >
> > > > Are you going to apply patches for this, or is someone else?
> > > Yes, it will be used by patch 3/3.
> >
> > That's not what I asked :(
>
> Sorry, ok, what do you want to ask?
Who is going to apply patches for drivers/firmware/ffi*?
> > > > > static void __init dmi_scan_machine(void)
> > > > > @@ -660,58 +686,22 @@ static void __init dmi_scan_machine(void)
> > > > > char __iomem *p, *q;
> > > > > char buf[32];
> > > > >
> > > > > +#ifdef CONFIG_FDT_FW_INTERFACE
> > > > > + if (dmi_sacn_smbios(ffi.smbios3, ffi.smbios))
> > > >
> > > > "dmi_sacn_smbios"
> > > >
> > > > > + goto error;
> > > > > +#endif
> > > >
> > > > Does this not mean that if FDT_FW_INTERFACE is enabled, but the platform
> > > > wants to use EFI, it won't be able to? The `goto error;` makes this look
> > > > mutually exclusive to my efi-unaware eyes.
> > >
> > > If you have enabled FFI, then if something goes wrong, you should goto error.
> > > Just like the origin code:
> > > if (efi_enabled(EFI_CONFIG_TABLES)) {
> > > if (dmi_sacn_smbios(efi.smbios3, efi.smbios))
> > > goto error;
> > > } else if (IS_ENABLED(CONFIG_DMI_SCAN_MACHINE_NON_EFI_FALLBACK)) {
> > > p = dmi_early_remap(SMBIOS_ENTRY_POINT_SCAN_START, 0x10000);
> > > if (p == NULL)
> > > goto error;
> >
> > Does this not make FFI and EFI mutually exclusive Kconfig options?
> > Suppose you are on a system that does not implement FFI, but does
> > implement EFI - what's going to happen then?
> > AFAICT, dmi_sacn_smbios(ffi.smbios3, ffi.smbios) will fail & you'll do a
> > `goto error` & skip the EFI code. What am I missing?
>
> Code is not intended to be mutually exclusive, get the correct value and return,
> The code is going to be changed to this:
>
> #ifdef CONFIG_FDT_FW_INTERFACE
Ideally, these would be IS_ENABLED() instead of #ifdef - but if you copy
what EFI does, then you don't need either, as there will always be an
ffi_enabled() defined.
> if (ffi_enabled(FFI_CONFIG_TABLES)) {
I don't know what this function is, but this code seems like a step in
the right direction.
> if (!dmi_sacn_smbios(ffi.smbios3, ffi.smbios))
> return;
> }
> #endif
Thanks,
Conor.
Hi Conor,
On Mon, Jul 3, 2023 at 9:03 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Mon, Jul 03, 2023 at 08:41:30PM +0800, 运辉崔 wrote:
> > On Mon, Jul 3, 2023 at 4:36 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> > > On Mon, Jul 03, 2023 at 04:23:53PM +0800, 运辉崔 wrote:
>
> > > > > > +FDT FIRMWARE INTERFACE (FFI)
> > > > > > +M: Yunhui Cui cuiyunhui@bytedance.com
> > > > > > +S: Maintained
> > > > > > +F: drivers/firmware/ffi.c
> > > > > > +F: include/linux/ffi.h
> > > > >
> > > > > Are you going to apply patches for this, or is someone else?
> > > > Yes, it will be used by patch 3/3.
> > >
> > > That's not what I asked :(
> >
> > Sorry, ok, what do you want to ask?
>
> Who is going to apply patches for drivers/firmware/ffi*?
I'll update to v3:
F: drivers/firmware/ffi*
F: include/linux/ffi*
And I'll plan to maintain them.
But now, I don't know how to apply the patches. Could you give some
suggestions?
>
> > > > > > static void __init dmi_scan_machine(void)
> > > > > > @@ -660,58 +686,22 @@ static void __init dmi_scan_machine(void)
> > > > > > char __iomem *p, *q;
> > > > > > char buf[32];
> > > > > >
> > > > > > +#ifdef CONFIG_FDT_FW_INTERFACE
> > > > > > + if (dmi_sacn_smbios(ffi.smbios3, ffi.smbios))
> > > > >
> > > > > "dmi_sacn_smbios"
> > > > >
> > > > > > + goto error;
> > > > > > +#endif
> > > > >
> > > > > Does this not mean that if FDT_FW_INTERFACE is enabled, but the platform
> > > > > wants to use EFI, it won't be able to? The `goto error;` makes this look
> > > > > mutually exclusive to my efi-unaware eyes.
> > > >
> > > > If you have enabled FFI, then if something goes wrong, you should goto error.
> > > > Just like the origin code:
> > > > if (efi_enabled(EFI_CONFIG_TABLES)) {
> > > > if (dmi_sacn_smbios(efi.smbios3, efi.smbios))
> > > > goto error;
> > > > } else if (IS_ENABLED(CONFIG_DMI_SCAN_MACHINE_NON_EFI_FALLBACK)) {
> > > > p = dmi_early_remap(SMBIOS_ENTRY_POINT_SCAN_START, 0x10000);
> > > > if (p == NULL)
> > > > goto error;
> > >
> > > Does this not make FFI and EFI mutually exclusive Kconfig options?
> > > Suppose you are on a system that does not implement FFI, but does
> > > implement EFI - what's going to happen then?
> > > AFAICT, dmi_sacn_smbios(ffi.smbios3, ffi.smbios) will fail & you'll do a
> > > `goto error` & skip the EFI code. What am I missing?
> >
> > Code is not intended to be mutually exclusive, get the correct value and return,
> > The code is going to be changed to this:
> >
> > #ifdef CONFIG_FDT_FW_INTERFACE
>
> Ideally, these would be IS_ENABLED() instead of #ifdef - but if you copy
> what EFI does, then you don't need either, as there will always be an
> ffi_enabled() defined.
Okay， this can refer to the code of EFI. :)
>
> > if (ffi_enabled(FFI_CONFIG_TABLES)) {
>
> I don't know what this function is, but this code seems like a step in
> the right direction.
Okay, I'll update it on v3.
>
> > if (!dmi_sacn_smbios(ffi.smbios3, ffi.smbios))
> > return;
> > }
> > #endif
>
> Thanks,
> Conor.
Thanks,
Yunhui
@@ -7871,6 +7871,12 @@ F: arch/x86/platform/efi/
F: drivers/firmware/efi/
F: include/linux/efi*.h
+FDT FIRMWARE INTERFACE (FFI)
+M: Yunhui Cui cuiyunhui@bytedance.com
+S: Maintained
+F: drivers/firmware/ffi.c
+F: include/linux/ffi.h
+
EXTERNAL CONNECTOR SUBSYSTEM (EXTCON)
M: MyungJoo Ham <myungjoo.ham@samsung.com>
M: Chanwoo Choi <cw00.choi@samsung.com>
@@ -303,6 +303,17 @@ config TURRIS_MOX_RWTM
other manufacturing data and also utilize the Entropy Bit Generator
for hardware random number generation.
+config FDT_FW_INTERFACE
+ bool "An interface for passing firmware info through FDT"
+ depends on OF && OF_FLATTREE
+ default n
+ help
+ When some bootloaders do not support EFI, and the arch does not
+ support SMBIOS_ENTRY_POINT_SCAN_START, then you can enable this option
+ to support the transfer of firmware information, such as smbios tables.
+
+ Say Y here if you want to pass firmware information by FDT.
+
source "drivers/firmware/arm_ffa/Kconfig"
source "drivers/firmware/broadcom/Kconfig"
source "drivers/firmware/cirrus/Kconfig"
@@ -33,6 +33,7 @@ obj-y += cirrus/
obj-y += meson/
obj-$(CONFIG_GOOGLE_FIRMWARE) += google/
obj-y += efi/
+obj-$(CONFIG_FDT_FW_INTERFACE) += ffi.o
obj-y += imx/
obj-y += psci/
obj-y += smccc/
@@ -6,6 +6,7 @@
#include <linux/ctype.h>
#include <linux/dmi.h>
#include <linux/efi.h>
+#include <linux/ffi.h>
#include <linux/memblock.h>
#include <linux/random.h>
#include <asm/dmi.h>
@@ -628,31 +629,56 @@ static int __init dmi_present(const u8 *buf)
}
/*
- * Check for the SMBIOS 3.0 64-bit entry point signature. Unlike the legacy
- * 32-bit entry point, there is no embedded DMI header (_DMI_) in here.
+ * According to the DMTF SMBIOS reference spec v3.0.0, it is
+ * allowed to define both the 64-bit entry point (smbios3) and
+ * the 32-bit entry point (smbios), in which case they should
+ * either both point to the same SMBIOS structure table, or the
+ * table pointed to by the 64-bit entry point should contain a
+ * superset of the table contents pointed to by the 32-bit entry
+ * point (section 5.2)
+ * This implies that the 64-bit entry point should have
+ * precedence if it is defined and supported by the OS. If we
+ * have the 64-bit entry point, but fail to decode it, fall
+ * back to the legacy one (if available)
*/
-static int __init dmi_smbios3_present(const u8 *buf)
+static int __init dmi_sacn_smbios(unsigned long smbios3, unsigned long smbios)
{
- if (memcmp(buf, "_SM3_", 5) == 0 &&
- buf[6] >= 24 && buf[6] <= 32 &&
- dmi_checksum(buf, buf[6])) {
- dmi_ver = get_unaligned_be24(buf + 7);
- dmi_num = 0; /* No longer specified */
- dmi_len = get_unaligned_le32(buf + 12);
- dmi_base = get_unaligned_le64(buf + 16);
- smbios_entry_point_size = buf[6];
- memcpy(smbios_entry_point, buf, smbios_entry_point_size);
+ char __iomem *p;
+ char buf[32];
+ #define INVALID_TABLE_ADDR (~0UL)
- if (dmi_walk_early(dmi_decode) == 0) {
- pr_info("SMBIOS %d.%d.%d present.\n",
- dmi_ver >> 16, (dmi_ver >> 8) & 0xFF,
- dmi_ver & 0xFF);
- dmi_format_ids(dmi_ids_string, sizeof(dmi_ids_string));
- pr_info("DMI: %s\n", dmi_ids_string);
+ if (smbios3 != INVALID_TABLE_ADDR) {
+ p = dmi_early_remap(smbios3, 32);
+ if (p == NULL)
+ return -1;
+ memcpy_fromio(buf, p, 32);
+ dmi_early_unmap(p, 32);
+
+ if (!dmi_smbios3_present(buf)) {
+ dmi_available = 1;
return 0;
}
}
- return 1;
+
+ if (smbios == INVALID_TABLE_ADDR)
+ return -1;
+
+ /*
+ * This is called as a core_initcall() because it isn't
+ * needed during early boot. This also means we can
+ * iounmap the space when we're done with it.
+ */
+ p = dmi_early_remap(smbios, 32);
+ if (p == NULL)
+ return -1;
+ memcpy_fromio(buf, p, 32);
+ dmi_early_unmap(p, 32);
+
+ if (!dmi_present(buf)) {
+ dmi_available = 1;
+ return 0;
+ }
+ return -1;
}
static void __init dmi_scan_machine(void)
@@ -660,58 +686,22 @@ static void __init dmi_scan_machine(void)
char __iomem *p, *q;
char buf[32];
+#ifdef CONFIG_FDT_FW_INTERFACE
+ if (dmi_sacn_smbios(ffi.smbios3, ffi.smbios))
+ goto error;
+#endif
if (efi_enabled(EFI_CONFIG_TABLES)) {
- /*
- * According to the DMTF SMBIOS reference spec v3.0.0, it is
- * allowed to define both the 64-bit entry point (smbios3) and
- * the 32-bit entry point (smbios), in which case they should
- * either both point to the same SMBIOS structure table, or the
- * table pointed to by the 64-bit entry point should contain a
- * superset of the table contents pointed to by the 32-bit entry
- * point (section 5.2)
- * This implies that the 64-bit entry point should have
- * precedence if it is defined and supported by the OS. If we
- * have the 64-bit entry point, but fail to decode it, fall
- * back to the legacy one (if available)
- */
- if (efi.smbios3 != EFI_INVALID_TABLE_ADDR) {
- p = dmi_early_remap(efi.smbios3, 32);
- if (p == NULL)
- goto error;
- memcpy_fromio(buf, p, 32);
- dmi_early_unmap(p, 32);
-
- if (!dmi_smbios3_present(buf)) {
- dmi_available = 1;
- return;
- }
- }
- if (efi.smbios == EFI_INVALID_TABLE_ADDR)
+ if (dmi_sacn_smbios(efi.smbios3, efi.smbios))
goto error;
-
- /* This is called as a core_initcall() because it isn't
- * needed during early boot. This also means we can
- * iounmap the space when we're done with it.
- */
- p = dmi_early_remap(efi.smbios, 32);
- if (p == NULL)
- goto error;
- memcpy_fromio(buf, p, 32);
- dmi_early_unmap(p, 32);
-
- if (!dmi_present(buf)) {
- dmi_available = 1;
- return;
- }
} else if (IS_ENABLED(CONFIG_DMI_SCAN_MACHINE_NON_EFI_FALLBACK)) {
p = dmi_early_remap(SMBIOS_ENTRY_POINT_SCAN_START, 0x10000);
if (p == NULL)
goto error;
/*
- * Same logic as above, look for a 64-bit entry point
- * first, and if not found, fall back to 32-bit entry point.
- */
+ * Same logic as above, look for a 64-bit entry point
+ * first, and if not found, fall back to 32-bit entry point.
+ */
memcpy_fromio(buf, p, 16);
for (q = p + 16; q < p + 0x10000; q += 16) {
memcpy_fromio(buf + 16, q, 16);
@@ -724,12 +714,12 @@ static void __init dmi_scan_machine(void)
}
/*
- * Iterate over all possible DMI header addresses q.
- * Maintain the 32 bytes around q in buf. On the
- * first iteration, substitute zero for the
- * out-of-range bytes so there is no chance of falsely
- * detecting an SMBIOS header.
- */
+ * Iterate over all possible DMI header addresses q.
+ * Maintain the 32 bytes around q in buf. On the
+ * first iteration, substitute zero for the
+ * out-of-range bytes so there is no chance of falsely
+ * detecting an SMBIOS header.
+ */
memset(buf, 0, 16);
for (q = p; q < p + 0x10000; q += 16) {
memcpy_fromio(buf + 16, q, 16);
new file mode 100644
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/libfdt.h>
+#include <linux/ffi.h>
+
+#define FFI_INVALID_TABLE_ADDR (~0UL)
+
+struct ffi __read_mostly ffi = {
+ .smbios = FFI_INVALID_TABLE_ADDR,
+ .smbios3 = FFI_INVALID_TABLE_ADDR,
+};
+EXPORT_SYMBOL(ffi);
+
+void __init ffi_smbios_root_pointer(void)
+{
+ int cfgtbl, len;
+ fdt64_t *prop;
+
+ cfgtbl = fdt_path_offset(initial_boot_params, "/cfgtables");
+ if (cfgtbl < 0) {
+ pr_info("firmware table not found.\n");
+ return;
+ }
+ prop = fdt_getprop_w(initial_boot_params, cfgtbl, "smbios_phy_ptr", &len);
+ if (!prop || len != sizeof(u64))
+ pr_info("smbios entry point not found.\n");
+ else
+ ffi.smbios = fdt64_to_cpu(*prop);
+
+ pr_info("smbios root pointer: %lx\n", ffi.smbios);
+}
+
new file mode 100644
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LINUX_FFI_H
+#define _LINUX_FFI_H
+
+extern struct ffi {
+ unsigned long smbios; /* SMBIOS table (32 bit entry point) */
+ unsigned long smbios3; /* SMBIOS table (64 bit entry point) */
+ unsigned long flags;
+
+} ffi;
+
+void ffi_smbios_root_pointer(void);
+
+#endif /* _LINUX_FFI_H */