| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMj1kXEKh9O-ndk3QFibJMYfMbG7vm-cLN2vVQM5eDsYK84NzQ@mail.gmail.com>
Date: Wed, 26 Apr 2023 08:08:51 +0100
From: Ard Biesheuvel <ardb@...nel.org>
To: Yunhui Cui <cuiyunhui@...edance.com>,
Mark Rutland <mark.rutland@....com>,
Lorenzo Pieralisi <lpieralisi@...nel.org>
Cc: rafael@...nel.org, lenb@...nel.org, jdelvare@...e.com,
cujomalainey@...omium.org, yc.hung@...iatek.com,
angelogioacchino.delregno@...labora.com,
allen-kh.cheng@...iatek.com, pierre-louis.bossart@...ux.intel.com,
tinghan.shen@...iatek.com, linux-kernel@...r.kernel.org,
linux-acpi@...r.kernel.org
Subject: Re: [PATCH] firmware: added a firmware information passing method FFI
On Wed, 26 Apr 2023 at 04:40, Yunhui Cui <cuiyunhui@...edance.com> wrote:
>
> Some BootLoaders do not support UEFI and cannot pass ACPI/SBMIOS table
> addresses through UEFI, such as coreboot.
>
> On the x86 platform, we pass the ACPI/SMBIOS table through the reserved
> address segment 0xF0000, but other arches usually do not reserve this
> address segment.
>
> We have added a new firmware information transmission method named FFI
> (FDT FIRMWARE INTERFACE), through FDT to obtain firmware information,
> such as the base address of the ACPI and SMBIOS table.
>
> Signed-off-by: Yunhui Cui <cuiyunhui@...edance.com>
Hello Yunhui,
I am not sure this is a good idea: this is clearly intended for arm64,
which cannot use ACPI without the EFI memory map, which it uses to
cross reference memory opregion accesses, to determine the correct
memory type attributes.
What is the use case you are trying to accommodate here?
> ---
> MAINTAINERS | 6 +++
> drivers/acpi/osl.c | 8 ++++
> drivers/firmware/Kconfig | 12 +++++
> drivers/firmware/Makefile | 1 +
> drivers/firmware/dmi_scan.c | 96 ++++++++++++++++++++++---------------
> drivers/firmware/ffi.c | 56 ++++++++++++++++++++++
> include/linux/ffi.h | 15 ++++++
> 7 files changed, 155 insertions(+), 39 deletions(-)
> create mode 100644 drivers/firmware/ffi.c
> create mode 100644 include/linux/ffi.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8d5bc223f305..94664f3b4c96 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7750,6 +7750,12 @@ F: arch/x86/platform/efi/
> F: drivers/firmware/efi/
> F: include/linux/efi*.h
>
> +FDT FIRMWARE INTERFACE (FFI)
> +M: Yunhui Cui cuiyunhui@...edance.com
> +S: Maintained
> +F: drivers/firmware/ffi.c
> +F: include/linux/ffi.h
> +
> EXTERNAL CONNECTOR SUBSYSTEM (EXTCON)
> M: MyungJoo Ham <myungjoo.ham@...sung.com>
> M: Chanwoo Choi <cw00.choi@...sung.com>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 3269a888fb7a..d45000041d2b 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -25,6 +25,7 @@
> #include <linux/nmi.h>
> #include <linux/acpi.h>
> #include <linux/efi.h>
> +#include <linux/ffi.h>
> #include <linux/ioport.h>
> #include <linux/list.h>
> #include <linux/jiffies.h>
> @@ -206,6 +207,13 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
> if (pa)
> return pa;
>
> +#ifdef CONFIG_FDT_FW_INTERFACE
> + if (fdt_fwtbl.acpi20 != FDT_INVALID_FWTBL_ADDR)
> + return fdt_fwtbl.acpi20;
> + if (fdt_fwtbl.acpi != FDT_INVALID_FWTBL_ADDR)
> + return fdt_fwtbl.acpi;
> + pr_err("Fdt system description tables not found\n");
> +#endif
> if (efi_enabled(EFI_CONFIG_TABLES)) {
> if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
> return efi.acpi20;
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index b59e3041fd62..13c67b50c17a 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -303,6 +303,18 @@ 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 UEFI, 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 acpi, 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"
> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> index 28fcddcd688f..3b8b5d0868a6 100644
> --- a/drivers/firmware/Makefile
> +++ b/drivers/firmware/Makefile
> @@ -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/
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 015c95a825d3..1e1a74ed7d3b 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -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>
> @@ -655,54 +656,71 @@ static int __init dmi_smbios3_present(const u8 *buf)
> return 1;
> }
>
> -static void __init dmi_scan_machine(void)
> +/*
> + * 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_sacn_smbios(unsigned long smbios3, unsigned long smbios)
> {
> - char __iomem *p, *q;
> + char __iomem *p;
> char buf[32];
> + #define INVALID_TABLE_ADDR (~0UL)
>
> - 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)
> - 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 (smbios3 != INVALID_TABLE_ADDR) {
> + p = dmi_early_remap(smbios3, 32);
> if (p == NULL)
> - goto error;
> + return -1;
> memcpy_fromio(buf, p, 32);
> dmi_early_unmap(p, 32);
>
> - if (!dmi_present(buf)) {
> + if (!dmi_smbios3_present(buf)) {
> dmi_available = 1;
> - return;
> + return 0;
> }
> + }
> +
> + 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)
> +{
> + char __iomem *p, *q;
> + char buf[32];
> +
> +#ifdef CONFIG_FDT_FW_INTERFACE
> + if (dmi_sacn_smbios(fdt_fwtbl.smbios3, fdt_fwtbl.smbios))
> + goto error;
> +#endif
> + 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)
> diff --git a/drivers/firmware/ffi.c b/drivers/firmware/ffi.c
> new file mode 100644
> index 000000000000..83c7abf22220
> --- /dev/null
> +++ b/drivers/firmware/ffi.c
> @@ -0,0 +1,56 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/kobject.h>
> +#include <linux/ffi.h>
> +#include <linux/of_fdt.h>
> +#include <linux/libfdt.h>
> +
> +struct fdt_fwtable __read_mostly fdt_fwtbl = {
> + .acpi = FDT_INVALID_FWTBL_ADDR,
> + .acpi20 = FDT_INVALID_FWTBL_ADDR,
> + .smbios = FDT_INVALID_FWTBL_ADDR,
> + .smbios3 = FDT_INVALID_FWTBL_ADDR,
> +};
> +EXPORT_SYMBOL(fdt_fwtbl);
> +
> +void __init of_fdt_fwtbl(void)
> +{
> + int cfgtbl, len;
> + fdt64_t *prop;
> +
> + cfgtbl = fdt_path_offset(initial_boot_params, "/cfgtables");
> + if (cfgtbl < 0) {
> + pr_info("cfgtables not found.\n");
> + return;
> + }
> + prop = fdt_getprop_w(initial_boot_params, cfgtbl, "smbios_phy_ptr", &len);
> + if (!prop || len != sizeof(u64))
> + pr_info("smbios_phy_ptr not found.\n");
> + else
> + fdt_fwtbl.smbios = fdt64_to_cpu(*prop);
> +
> + prop = fdt_getprop_w(initial_boot_params, cfgtbl, "smbios3_phy_ptr", &len);
> + if (!prop || len != sizeof(u64))
> + pr_info("smbios3_phy_ptr not found.\n");
> + else
> + fdt_fwtbl.smbios3 = fdt64_to_cpu(*prop);
> +
> + prop = fdt_getprop_w(initial_boot_params, cfgtbl, "acpi_phy_ptr", &len);
> + if (!prop || len != sizeof(u64))
> + pr_info("acpi_phy_ptr not found.\n");
> + else
> + fdt_fwtbl.acpi = fdt64_to_cpu(*prop);
> +
> + prop = fdt_getprop_w(initial_boot_params, cfgtbl, "acpi20_phy_ptr", &len);
> + if (!prop || len != sizeof(u64))
> + pr_info("acpi20_phy_ptr not found.\n");
> + else
> + fdt_fwtbl.acpi20 = fdt64_to_cpu(*prop);
> +}
> +
> +void __init fdt_fwtbl_init(void)
> +{
> + of_fdt_fwtbl();
> +}
> diff --git a/include/linux/ffi.h b/include/linux/ffi.h
> new file mode 100644
> index 000000000000..ffb50810a01e
> --- /dev/null
> +++ b/include/linux/ffi.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _LINUX_FDT_FW_H
> +#define _LINUX_FDT_FW_H
> +
> +#define FDT_INVALID_FWTBL_ADDR (~0UL)
> +extern struct fdt_fwtable {
> + unsigned long acpi;
> + unsigned long acpi20;
> + unsigned long smbios;
> + unsigned long smbios3;
> + unsigned long flags;
> +} fdt_fwtbl;
> +
> +#endif
> --
> 2.20.1
>
Powered by blists - more mailing lists