[PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[Yunhui Cui]

Here's version 3 of patch series.

V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
consensus with the Maintainers.
Please refer to:
https://patches.linaro.org/project/linux-acpi/patch/20230426034001.16-1-cuiyunhui@bytedance.com/

Changes v1->v2:
Adjusted the code structure, put the ACPI part under the RISC-V architecture,
and put the general part of obtaining SMBIOS entry through FFI
under driver/firmware/.
Please refer to:
https://lore.kernel.org/lkml/20230703-71f67eb66a037f5c0fb825c6@orel/T/

Changes v2->v3: 
According to the suggestions of maintainers, the code has been modified as follows:
1. Modified the commit log.
2. Added description of "ffitbl" subnod in dt-bindings.
3. Add stub function to the function
4. arch/riscv/ and driver/firmware/ use CONFIG_FDT_FW_INTERFACE to control
5. Modified the ffi_smbios_root_pointer() function logic and printing
etc.

Yunhui Cui (4):
  riscv: obtain ACPI RSDP from devicetree
  firmware: introduce FFI for SMBIOS entry
  riscv: obtain SMBIOS entry from FFI
  dt-bindings: firmware: Document ffitbl binding

 .../devicetree/bindings/firmware/ffitbl.txt   | 27 ++++++
 MAINTAINERS                                   | 13 +++
 arch/riscv/include/asm/acpi.h                 |  9 ++
 arch/riscv/include/asm/ffi.h                  | 14 +++
 arch/riscv/kernel/Makefile                    |  1 +
 arch/riscv/kernel/ffi.c                       | 40 ++++++++
 arch/riscv/kernel/setup.c                     |  2 +
 drivers/firmware/Kconfig                      | 11 +++
 drivers/firmware/Makefile                     |  1 +
 drivers/firmware/dmi_scan.c                   | 97 +++++++++++--------
 drivers/firmware/ffi.c                        | 42 ++++++++
 include/linux/ffi.h                           | 29 ++++++
 12 files changed, 246 insertions(+), 40 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
 create mode 100644 arch/riscv/include/asm/ffi.h
 create mode 100644 arch/riscv/kernel/ffi.c
 create mode 100644 drivers/firmware/ffi.c
 create mode 100644 include/linux/ffi.h

-- 
2.20.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v3 1/4] riscv: obtain ACPI RSDP from devicetree

[Yunhui Cui]

On RISC-V, Coreboot does not support booting using EFI, only devicetree
nor does RISC-V have a reserved address segment.
To allow using Coreboot on platforms that require ACPI, the ACPI RSDP
needs to be passed to supervisor mode software using devicetree.

Add support for parsing the "ffitbl" devicetree node to find the
ACPI entry point and use wire up acpi_arch_get_root_pointer().
This feature is known as FDT Firmware Interface (FFI).

Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
 MAINTAINERS                   |  6 ++++++
 arch/riscv/include/asm/acpi.h |  9 +++++++++
 arch/riscv/include/asm/ffi.h  | 14 +++++++++++++
 arch/riscv/kernel/Makefile    |  1 +
 arch/riscv/kernel/ffi.c       | 38 +++++++++++++++++++++++++++++++++++
 arch/riscv/kernel/setup.c     |  2 ++
 6 files changed, 70 insertions(+)
 create mode 100644 arch/riscv/include/asm/ffi.h
 create mode 100644 arch/riscv/kernel/ffi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index cd5388a33410..e592f489e757 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18363,6 +18363,12 @@ F:	arch/riscv/boot/dts/
 X:	arch/riscv/boot/dts/allwinner/
 X:	arch/riscv/boot/dts/renesas/
 
+RISC-V FDT FIRMWARE INTERFACE (FFI) SUPPORT
+M:	Yunhui Cui cuiyunhui@bytedance.com
+S:	Maintained
+F:	arch/riscv/include/asm/ffi.h
+F:	arch/riscv/kernel/ffi.c
+
 RISC-V PMU DRIVERS
 M:	Atish Patra <atishp@atishpatra.org>
 R:	Anup Patel <anup@brainfault.org>
diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h
index f71ce21ff684..5574f9a152f5 100644
--- a/arch/riscv/include/asm/acpi.h
+++ b/arch/riscv/include/asm/acpi.h
@@ -15,6 +15,8 @@
 /* Basic configuration for ACPI */
 #ifdef CONFIG_ACPI
 
+#include <asm/ffi.h>
+
 typedef u64 phys_cpuid_t;
 #define PHYS_CPUID_INVALID INVALID_HARTID
 
@@ -66,6 +68,13 @@ int acpi_get_riscv_isa(struct acpi_table_header *table,
 		       unsigned int cpu, const char **isa);
 
 static inline int acpi_numa_get_nid(unsigned int cpu) { return NUMA_NO_NODE; }
+
+#define ACPI_HAVE_ARCH_GET_ROOT_POINTER
+static inline u64 acpi_arch_get_root_pointer(void)
+{
+	return riscv_acpi_rsdp();
+}
+
 #else
 static inline void acpi_init_rintc_map(void) { }
 static inline struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu)
diff --git a/arch/riscv/include/asm/ffi.h b/arch/riscv/include/asm/ffi.h
new file mode 100644
index 000000000000..d5e8309cc06f
--- /dev/null
+++ b/arch/riscv/include/asm/ffi.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _ASM_FFI_H
+#define _ASM_FFI_H
+
+#ifdef CONFIG_FDT_FW_INTERFACE
+extern void ffi_init(void);
+extern u64 riscv_acpi_rsdp(void);
+#else
+#define ffi_init()
+static inline u64 riscv_acpi_rsdp(void) { return 0; }
+#endif
+
+#endif /* _ASM_FFI_H */
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index 506cc4a9a45a..71831cf7f934 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -92,6 +92,7 @@ obj-$(CONFIG_CRASH_CORE)	+= crash_core.o
 obj-$(CONFIG_JUMP_LABEL)	+= jump_label.o
 
 obj-$(CONFIG_EFI)		+= efi.o
+obj-$(CONFIG_FDT_FW_INTERFACE)	+= ffi.o
 obj-$(CONFIG_COMPAT)		+= compat_syscall_table.o
 obj-$(CONFIG_COMPAT)		+= compat_signal.o
 obj-$(CONFIG_COMPAT)		+= compat_vdso/
diff --git a/arch/riscv/kernel/ffi.c b/arch/riscv/kernel/ffi.c
new file mode 100644
index 000000000000..147d06a5acff
--- /dev/null
+++ b/arch/riscv/kernel/ffi.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ffi.c - FDT FIRMWARE INTERFACE
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/libfdt.h>
+
+static u64 acpi_rsdp;
+
+void __init ffi_acpi_root_pointer(void)
+{
+	u32 ffitbl, acpi, len;
+	fdt64_t *prop;
+
+	ffitbl = fdt_subnode_offset(initial_boot_params, 0, "ffitbl");
+	acpi = fdt_subnode_offset(initial_boot_params, ffitbl, "acpi");
+	prop = fdt_getprop_w(initial_boot_params, acpi, "entry", &len);
+	if (!prop || len != sizeof(u64)) {
+		pr_debug("acpi rsdp not found.\n");
+		return;
+	}
+	acpi_rsdp = fdt64_to_cpu(*prop);
+	pr_debug("acpi rsdp: %llx\n", acpi_rsdp);
+}
+
+u64 __init riscv_acpi_rsdp(void)
+{
+	return acpi_rsdp;
+}
+
+void __init ffi_init(void)
+{
+	ffi_acpi_root_pointer();
+}
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 971fe776e2f8..5a933d6b6acb 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -36,6 +36,7 @@
 #include <asm/thread_info.h>
 #include <asm/kasan.h>
 #include <asm/efi.h>
+#include <asm/ffi.h>
 
 #include "head.h"
 
@@ -279,6 +280,7 @@ void __init setup_arch(char **cmdline_p)
 	parse_early_param();
 
 	efi_init();
+	ffi_init();
 	paging_init();
 
 	/* Parse the ACPI tables for possible boot-time configuration */
-- 
2.20.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v3 2/4] firmware: introduce FFI for SMBIOS entry

[Yunhui Cui]

To enable DMI functionality on platforms where the bootloader
does not support EFI and the architecture does not support
reserved addresses.

Add a "ffitbl" device tree node to find the SMBIOS entry
point and use wire up dmi_scan_machine().This feature is
known as the FDT Firmware Interface (FFI).

Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
 MAINTAINERS                 |  6 +++
 drivers/firmware/Kconfig    | 11 +++++
 drivers/firmware/Makefile   |  1 +
 drivers/firmware/dmi_scan.c | 97 ++++++++++++++++++++++---------------
 drivers/firmware/ffi.c      | 42 ++++++++++++++++
 include/linux/ffi.h         | 29 +++++++++++
 6 files changed, 146 insertions(+), 40 deletions(-)
 create mode 100644 drivers/firmware/ffi.c
 create mode 100644 include/linux/ffi.h

diff --git a/MAINTAINERS b/MAINTAINERS
index e592f489e757..9b886ef36587 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -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>
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index b59e3041fd62..3579d9bc22ff 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
+	 Enable this option to support the transfer of firmware information,
+	 such as smbios tables, for bootloaders that do not support EFI, such
+	 as coreboot.
+
+	 Say Y here if you want to transfer 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..c29ea050b5b9 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,70 @@ 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];
-
-	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);
+	#define INVALID_TABLE_ADDR (~0UL)
+	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];
+
+	if (ffi_enabled(FFI_CONFIG_TABLES)) {
+		if (!dmi_sacn_smbios(ffi.smbios3, ffi.smbios))
+			return;
+	}
+
+	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..d4994a22438b
--- /dev/null
+++ b/drivers/firmware/ffi.c
@@ -0,0 +1,42 @@
+// 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,
+};
+
+void __init ffi_smbios_root_pointer(void)
+{
+	u32 ffitbl, smbios, len, ver;
+	u64 entry;
+	fdt64_t *prop;
+
+	ffitbl = fdt_subnode_offset(initial_boot_params, 0, "ffitbl");
+	smbios = fdt_subnode_offset(initial_boot_params, ffitbl, "smbios");
+	prop = fdt_getprop_w(initial_boot_params, smbios, "entry", &len);
+	if (!prop || len != sizeof(u64)) {
+		pr_debug("smbios entry not found.\n");
+		return;
+	}
+	entry = fdt64_to_cpu(*prop);
+
+	prop = fdt_getprop_w(initial_boot_params, smbios, "reg", &len);
+	if (!prop || len != sizeof(u32)) {
+		pr_debug("smbios reg not found.\n");
+		return;
+	}
+	ver = fdt32_to_cpu(*prop);
+	(ver == 3) ? (ffi.smbios3 = entry) : (ffi.smbios = entry);
+	set_bit(FFI_CONFIG_TABLES, &ffi.flags);
+	pr_debug("smbios%d entry: %llx\n", ver, entry);
+}
+
diff --git a/include/linux/ffi.h b/include/linux/ffi.h
new file mode 100644
index 000000000000..a99c2a290556
--- /dev/null
+++ b/include/linux/ffi.h
@@ -0,0 +1,29 @@
+/* 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;
+
+#define FFI_CONFIG_TABLES	2	/* Can we use FFI config tables? */
+
+#ifdef CONFIG_FDT_FW_INTERFACE
+extern void ffi_smbios_root_pointer(void);
+static inline bool ffi_enabled(int feature)
+{
+	return test_bit(feature, &ffi.flags) != 0;
+}
+#else
+static inline bool ffi_enabled(int feature)
+{
+	return false;
+}
+void ffi_smbios_root_pointer(void) { return; };
+#endif
+
+#endif /* _LINUX_FFI_H */
-- 
2.20.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v3 3/4] riscv: obtain SMBIOS entry from FFI

[Yunhui Cui]

On RISC-V, Coreboot does not support EFI booting, only supports
devicetree, and RISC-V does not have reserved address segments.
To support Coreboot on RISC-V platforms that require DMI
functionality, SMBIOS entry need to be passed through FFI.

Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
 arch/riscv/kernel/ffi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/kernel/ffi.c b/arch/riscv/kernel/ffi.c
index 147d06a5acff..b959d16fe0b3 100644
--- a/arch/riscv/kernel/ffi.c
+++ b/arch/riscv/kernel/ffi.c
@@ -8,6 +8,7 @@
 #include <linux/of.h>
 #include <linux/of_fdt.h>
 #include <linux/libfdt.h>
+#include <linux/ffi.h>
 
 static u64 acpi_rsdp;
 
@@ -35,4 +36,5 @@ u64 __init riscv_acpi_rsdp(void)
 void __init ffi_init(void)
 {
 	ffi_acpi_root_pointer();
+	ffi_smbios_root_pointer();
 }
-- 
2.20.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

[Yunhui Cui]

Add the description for ffitbl subnode.

Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
 .../devicetree/bindings/firmware/ffitbl.txt   | 27 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt

diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
new file mode 100644
index 000000000000..c42368626199
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
@@ -0,0 +1,27 @@
+FFI(FDT FIRMWARE INTERFACE) driver
+
+Required properties:
+ - entry		: acpi or smbios root pointer, u64
+ - reg			: acpi or smbios version, u32
+
+Some bootloaders, such as Coreboot do not support EFI,
+only devicetree and some arches do not have a reserved
+address segment. Add "ffitbl" subnode to obtain ACPI RSDP
+and SMBIOS entry.
+This feature is known as FDT Firmware Interface (FFI).
+
+Example:
+	ffitbl {
+
+		smbios {
+				entry = "";
+				reg = < 0x03 >;
+
+		}
+		acpi {
+				entry = "";
+				reg = < 0x06 >;
+
+		}
+	}
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 9b886ef36587..008257e55062 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7874,6 +7874,7 @@ F:	include/linux/efi*.h
 FDT FIRMWARE INTERFACE (FFI)
 M:	Yunhui Cui cuiyunhui@bytedance.com
 S:	Maintained
+F:	Documentation/devicetree/bindings/firmware/ffitbl.txt
 F:	drivers/firmware/ffi.c
 F:	include/linux/ffi.h
 
-- 
2.20.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[Palmer Dabbelt]

On Wed, 05 Jul 2023 04:42:47 PDT (-0700), cuiyunhui@bytedance.com wrote:
> Here's version 3 of patch series.
>
> V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
> consensus with the Maintainers.
> Please refer to:
> https://patches.linaro.org/project/linux-acpi/patch/20230426034001.16-1-cuiyunhui@bytedance.com/

From looking at that thread it seems that the consensus is this is a bad 
idea?  Sorry if I'm just missing something...

> Changes v1->v2:
> Adjusted the code structure, put the ACPI part under the RISC-V architecture,
> and put the general part of obtaining SMBIOS entry through FFI
> under driver/firmware/.
> Please refer to:
> https://lore.kernel.org/lkml/20230703-71f67eb66a037f5c0fb825c6@orel/T/
>
> Changes v2->v3:
> According to the suggestions of maintainers, the code has been modified as follows:
> 1. Modified the commit log.
> 2. Added description of "ffitbl" subnod in dt-bindings.
> 3. Add stub function to the function
> 4. arch/riscv/ and driver/firmware/ use CONFIG_FDT_FW_INTERFACE to control
> 5. Modified the ffi_smbios_root_pointer() function logic and printing
> etc.
>
> Yunhui Cui (4):
>   riscv: obtain ACPI RSDP from devicetree
>   firmware: introduce FFI for SMBIOS entry
>   riscv: obtain SMBIOS entry from FFI
>   dt-bindings: firmware: Document ffitbl binding
>
>  .../devicetree/bindings/firmware/ffitbl.txt   | 27 ++++++
>  MAINTAINERS                                   | 13 +++
>  arch/riscv/include/asm/acpi.h                 |  9 ++
>  arch/riscv/include/asm/ffi.h                  | 14 +++
>  arch/riscv/kernel/Makefile                    |  1 +
>  arch/riscv/kernel/ffi.c                       | 40 ++++++++
>  arch/riscv/kernel/setup.c                     |  2 +
>  drivers/firmware/Kconfig                      | 11 +++
>  drivers/firmware/Makefile                     |  1 +
>  drivers/firmware/dmi_scan.c                   | 97 +++++++++++--------
>  drivers/firmware/ffi.c                        | 42 ++++++++
>  include/linux/ffi.h                           | 29 ++++++
>  12 files changed, 246 insertions(+), 40 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
>  create mode 100644 arch/riscv/include/asm/ffi.h
>  create mode 100644 arch/riscv/kernel/ffi.c
>  create mode 100644 drivers/firmware/ffi.c
>  create mode 100644 include/linux/ffi.h

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2191 bytes --]

Hey,

On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
> Add the description for ffitbl subnode.
> 
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> ---
>  .../devicetree/bindings/firmware/ffitbl.txt   | 27 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
> 
> diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> new file mode 100644
> index 000000000000..c42368626199
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt

Firstly, new dt-bindings need to be done in yaml, not in text form.
Secondly, you didn't re-run get_maintainer.pl after adding this binding,
so you have not CCed any of the other dt-binding maintainers nor the
devicetree mailing list.

> @@ -0,0 +1,27 @@

> +FFI(FDT FIRMWARE INTERFACE) driver
> +
> +Required properties:
> + - entry		: acpi or smbios root pointer, u64
> + - reg			: acpi or smbios version, u32

Please go look at any other dt-binding (or the example schema) as to how
these properties should be used. A "reg" certainly should not be being
used to store the revision...

Cheers,
Conor.

> +
> +Some bootloaders, such as Coreboot do not support EFI,
> +only devicetree and some arches do not have a reserved
> +address segment. Add "ffitbl" subnode to obtain ACPI RSDP
> +and SMBIOS entry.
> +This feature is known as FDT Firmware Interface (FFI).
> +
> +Example:
> +	ffitbl {
> +
> +		smbios {
> +				entry = "";
> +				reg = < 0x03 >;
> +
> +		}
> +		acpi {
> +				entry = "";
> +				reg = < 0x06 >;
> +
> +		}
> +	}
> +
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9b886ef36587..008257e55062 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7874,6 +7874,7 @@ F:	include/linux/efi*.h
>  FDT FIRMWARE INTERFACE (FFI)
>  M:	Yunhui Cui cuiyunhui@bytedance.com
>  S:	Maintained
> +F:	Documentation/devicetree/bindings/firmware/ffitbl.txt
>  F:	drivers/firmware/ffi.c
>  F:	include/linux/ffi.h
>  
> -- 
> 2.20.1
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2617 bytes --]

Hey,

On Wed, Jul 05, 2023 at 07:17:29AM -0700, Palmer Dabbelt wrote:
> On Wed, 05 Jul 2023 04:42:47 PDT (-0700), cuiyunhui@bytedance.com wrote:
> > Here's version 3 of patch series.
> > 
> > V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
> > consensus with the Maintainers.
> > Please refer to:
> > https://patches.linaro.org/project/linux-acpi/patch/20230426034001.16-1-cuiyunhui@bytedance.com/
> 
> From looking at that thread it seems that the consensus is this is a bad
> idea?  Sorry if I'm just missing something...

"consensus" meaning that Ard told them what he was and was not prepared
to accept in common code, and left the decision on what he was not up to
the RISC-V maintainers.

While this version of the series seems to address some of my general
code comments on the v2 (although I have not yet looked more than skin
deep), there are some outstanding, higher level, questions that were
asked on v2 that do not seem to have been answered satisfactorily yet:

- "So, could you please bring this topic for discussion in on the riscv
  tech-brs mailing list (https://lists.riscv.org/g/tech-brs) and get
  agreement?" Sunil has asked this as RVI specs have an interest in
  cross-os booting contracts. See:
  https://lore.kernel.org/linux-riscv/CAEEQ3wnsedWJYEEg8z+3C_HuCca0nD50NGpCdU3scxavrrOucA@mail.gmail.com/

- "I am curious how do you handle EFI memory map dependencies." to
  which the answer was "a memory node in DTS can solve it."
  I don't see anything in this version of the patchset that actually
  reads a DTS node when ACPI is enabled. If I am missing some codepath
  that does this, please let point it out. See:
  https://lore.kernel.org/linux-riscv/CAEEQ3wnsedWJYEEg8z+3C_HuCca0nD50NGpCdU3scxavrrOucA@mail.gmail.com/

- "I'm not a big fan of adding yet another interface. Have you considered
  doing something like [1]?" where [1] was:
  https://github.com/tianocore/tianocore.github.io/wiki/UefiPayloadPkg
  The response to this question was "This has been discussed many times
  with Ard, Please refer to <v1>". I don't see how this answers the
  question to be honest & Andrew's follow-up question asking for
  clarification went unanswered:
  https://lore.kernel.org/linux-riscv/20230703-6ac90a2de15f1017bc0ced74@orel/
  Jess, Emil and Bjorn have all also commented about you could load a
  small EFI payload from Coreboot. I don't see any responses to those
  questions.

运辉崔, please try to address all outstanding comments (and give people
a chance to reply to you) before sending new versions.

Cheers,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[运辉崔]

Hi Palmer,

On Wed, Jul 5, 2023 at 10:17 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Wed, 05 Jul 2023 04:42:47 PDT (-0700), cuiyunhui@bytedance.com wrote:
> > Here's version 3 of patch series.
> >
> > V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
> > consensus with the Maintainers.
> > Please refer to:
> > https://patches.linaro.org/project/linux-acpi/patch/20230426034001.16-1-cuiyunhui@bytedance.com/
>
> From looking at that thread it seems that the consensus is this is a bad
> idea?  Sorry if I'm just missing something...
>

First of all, Coreboot does not support EFI, Ron has expressed, as follows:
"I am wondering if we can focus on risc-v here, and not drag in ARM,
b/c the ARM ACPI+UEFI ship has sailed. I had that discussion in 2013
;-) and it's clear we don't want to redo it.
In general, in my world, because of the many problems that come with
UEFI (security, code quality, performance), we'd like to avoid
requiring a dependency on UEFI just to get ACPI on RISC-V. It also
seems, from other discussions I'm having, that there is some belief
that ACPI will be wanted on RISC-V. It would be nice to separate those
pieces on RISC-V; certainly they were separate for a very long time in
the x86 world (we had ACPI+SMM on coreboot laptops without UEFI for
example)."

Then, a consensus was reached with Ard, that FFI can be applied to RISC-V.

Please look at this patch again: [PATCH v3 1/4] riscv: obtain ACPI
RSDP from devicetree
Why do you think it is a bad idea?


> > Changes v1->v2:
> > Adjusted the code structure, put the ACPI part under the RISC-V architecture,
> > and put the general part of obtaining SMBIOS entry through FFI
> > under driver/firmware/.
> > Please refer to:
> > https://lore.kernel.org/lkml/20230703-71f67eb66a037f5c0fb825c6@orel/T/
> >
> > Changes v2->v3:
> > According to the suggestions of maintainers, the code has been modified as follows:
> > 1. Modified the commit log.
> > 2. Added description of "ffitbl" subnod in dt-bindings.
> > 3. Add stub function to the function
> > 4. arch/riscv/ and driver/firmware/ use CONFIG_FDT_FW_INTERFACE to control
> > 5. Modified the ffi_smbios_root_pointer() function logic and printing
> > etc.
> >
> > Yunhui Cui (4):
> >   riscv: obtain ACPI RSDP from devicetree
> >   firmware: introduce FFI for SMBIOS entry
> >   riscv: obtain SMBIOS entry from FFI
> >   dt-bindings: firmware: Document ffitbl binding
> >
> >  .../devicetree/bindings/firmware/ffitbl.txt   | 27 ++++++
> >  MAINTAINERS                                   | 13 +++
> >  arch/riscv/include/asm/acpi.h                 |  9 ++
> >  arch/riscv/include/asm/ffi.h                  | 14 +++
> >  arch/riscv/kernel/Makefile                    |  1 +
> >  arch/riscv/kernel/ffi.c                       | 40 ++++++++
> >  arch/riscv/kernel/setup.c                     |  2 +
> >  drivers/firmware/Kconfig                      | 11 +++
> >  drivers/firmware/Makefile                     |  1 +
> >  drivers/firmware/dmi_scan.c                   | 97 +++++++++++--------
> >  drivers/firmware/ffi.c                        | 42 ++++++++
> >  include/linux/ffi.h                           | 29 ++++++
> >  12 files changed, 246 insertions(+), 40 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
> >  create mode 100644 arch/riscv/include/asm/ffi.h
> >  create mode 100644 arch/riscv/kernel/ffi.c
> >  create mode 100644 drivers/firmware/ffi.c
> >  create mode 100644 include/linux/ffi.h

Thanks,
Yunhui

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

[运辉崔]

Hi Conor,

Added dts Maintainers,

On Wed, Jul 5, 2023 at 11:07 PM Conor Dooley <conor@kernel.org> wrote:
>
> Hey,
>
> On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
> > Add the description for ffitbl subnode.
> >
> > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > ---
> >  .../devicetree/bindings/firmware/ffitbl.txt   | 27 +++++++++++++++++++
> >  MAINTAINERS                                   |  1 +
> >  2 files changed, 28 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
> >
> > diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> > new file mode 100644
> > index 000000000000..c42368626199
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
>
> Firstly, new dt-bindings need to be done in yaml, not in text form.
> Secondly, you didn't re-run get_maintainer.pl after adding this binding,
> so you have not CCed any of the other dt-binding maintainers nor the
> devicetree mailing list.

Re-run get_maintainer.pl and added maintainers into the maillist.
emm.. There is some *txt in
Documentation/devicetree/bindings/firmware/, isn't it?

>
> > @@ -0,0 +1,27 @@
>
> > +FFI(FDT FIRMWARE INTERFACE) driver
> > +
> > +Required properties:
> > + - entry             : acpi or smbios root pointer, u64
> > + - reg                       : acpi or smbios version, u32
>
> Please go look at any other dt-binding (or the example schema) as to how
> these properties should be used. A "reg" certainly should not be being
> used to store the revision...

Okay, If so，I'll add a property "version" into the dts instead of
"reg", just like, WDYT?
ffitbl {
    smbios {
        entry = "";
        version = < 0x02 >;
    }
   acpi {
         entry = "";
         version = < 0x06 >;
  }
}

>
> Cheers,
> Conor.
>
> > +
> > +Some bootloaders, such as Coreboot do not support EFI,
> > +only devicetree and some arches do not have a reserved
> > +address segment. Add "ffitbl" subnode to obtain ACPI RSDP
> > +and SMBIOS entry.
> > +This feature is known as FDT Firmware Interface (FFI).
> > +
> > +Example:
> > +     ffitbl {
> > +
> > +             smbios {
> > +                             entry = "";
> > +                             reg = < 0x03 >;
> > +
> > +             }
> > +             acpi {
> > +                             entry = "";
> > +                             reg = < 0x06 >;
> > +
> > +             }
> > +     }
> > +
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 9b886ef36587..008257e55062 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7874,6 +7874,7 @@ F:      include/linux/efi*.h
> >  FDT FIRMWARE INTERFACE (FFI)
> >  M:   Yunhui Cui cuiyunhui@bytedance.com
> >  S:   Maintained
> > +F:   Documentation/devicetree/bindings/firmware/ffitbl.txt
> >  F:   drivers/firmware/ffi.c
> >  F:   include/linux/ffi.h
> >
> > --
> > 2.20.1
> >

Thanks,
Yunhui

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

[Krzysztof Kozlowski]

On 06/07/2023 05:43, 运辉崔 wrote:
> Hi Conor,
> 
> Added dts Maintainers,
> 
> On Wed, Jul 5, 2023 at 11:07 PM Conor Dooley <conor@kernel.org> wrote:
>>
>> Hey,
>>
>> On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
>>> Add the description for ffitbl subnode.
>>>
>>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
>>> ---
>>>  .../devicetree/bindings/firmware/ffitbl.txt   | 27 +++++++++++++++++++
>>>  MAINTAINERS                                   |  1 +
>>>  2 files changed, 28 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
>>> new file mode 100644
>>> index 000000000000..c42368626199
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
>>
>> Firstly, new dt-bindings need to be done in yaml, not in text form.
>> Secondly, you didn't re-run get_maintainer.pl after adding this binding,
>> so you have not CCed any of the other dt-binding maintainers nor the
>> devicetree mailing list.
> 
> Re-run get_maintainer.pl and added maintainers into the maillist.


This does not work like this.

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC.  It might happen, that command when run on an older
kernel, gives you outdated entries.  Therefore please be sure you base
your patches on recent Linux kernel.

You missed at least DT list (maybe more), so this won't be tested by our
tools. Performing review on untested code might be a waste of time, thus
I will skip this patch entirely till you follow the process allowing the
patch to be tested.

Please kindly resend and include all necessary To/Cc entries.

> emm.. There is some *txt in
> Documentation/devicetree/bindings/firmware/, isn't it?

And what about it? Do you claim they were added recently?

Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

[运辉崔]

Hi Krzysztof,

On Thu, Jul 6, 2023 at 2:01 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 06/07/2023 05:43, 运辉崔 wrote:
> > Hi Conor,
> >
> > Added dts Maintainers,
> >
> > On Wed, Jul 5, 2023 at 11:07 PM Conor Dooley <conor@kernel.org> wrote:
> >>
> >> Hey,
> >>
> >> On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
> >>> Add the description for ffitbl subnode.
> >>>
> >>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> >>> ---
> >>>  .../devicetree/bindings/firmware/ffitbl.txt   | 27 +++++++++++++++++++
> >>>  MAINTAINERS                                   |  1 +
> >>>  2 files changed, 28 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> >>> new file mode 100644
> >>> index 000000000000..c42368626199
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> >>
> >> Firstly, new dt-bindings need to be done in yaml, not in text form.
> >> Secondly, you didn't re-run get_maintainer.pl after adding this binding,
> >> so you have not CCed any of the other dt-binding maintainers nor the
> >> devicetree mailing list.
> >
> > Re-run get_maintainer.pl and added maintainers into the maillist.
>
>
> This does not work like this.
>
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC.  It might happen, that command when run on an older
> kernel, gives you outdated entries.  Therefore please be sure you base
> your patches on recent Linux kernel.
>
> You missed at least DT list (maybe more), so this won't be tested by our
> tools. Performing review on untested code might be a waste of time, thus
> I will skip this patch entirely till you follow the process allowing the
> patch to be tested.
>
> Please kindly resend and include all necessary To/Cc entries.

This set of patches is applied on the tag next-20230706, and to
generate the mail list by scripts/get_maintainers.pl on the tag

./scripts/get_maintainer.pl
../riscv/linux/v3-0004-dt-bindings-firmware-Document-ffitbl-binding.patch
Yunhui Cui cuiyunhui@bytedance.com (maintainer:FDT FIRMWARE INTERFACE (FFI))
Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND
FLATTENED DEVICE TREE BINDINGS)
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
(maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
Conor Dooley <conor+dt@kernel.org> (maintainer:OPEN FIRMWARE AND
FLATTENED DEVICE TREE BINDINGS)
devicetree@vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED
DEVICE TREE BINDINGS)
linux-kernel@vger.kernel.org (open list)

What am I missing ?


> > emm.. There is some *txt in
> > Documentation/devicetree/bindings/firmware/, isn't it?
>
> And what about it? Do you claim they were added recently?
>
> Best regards,
> Krzysztof
>

Thanks,
Yunhui

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

[Krzysztof Kozlowski]

On 06/07/2023 08:24, 运辉崔 wrote:
> Hi Krzysztof,
> 
> On Thu, Jul 6, 2023 at 2:01 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 06/07/2023 05:43, 运辉崔 wrote:
>>> Hi Conor,
>>>
>>> Added dts Maintainers,
>>>
>>> On Wed, Jul 5, 2023 at 11:07 PM Conor Dooley <conor@kernel.org> wrote:
>>>>
>>>> Hey,
>>>>
>>>> On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
>>>>> Add the description for ffitbl subnode.
>>>>>
>>>>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
>>>>> ---
>>>>>  .../devicetree/bindings/firmware/ffitbl.txt   | 27 +++++++++++++++++++
>>>>>  MAINTAINERS                                   |  1 +
>>>>>  2 files changed, 28 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
>>>>> new file mode 100644
>>>>> index 000000000000..c42368626199
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
>>>>
>>>> Firstly, new dt-bindings need to be done in yaml, not in text form.
>>>> Secondly, you didn't re-run get_maintainer.pl after adding this binding,
>>>> so you have not CCed any of the other dt-binding maintainers nor the
>>>> devicetree mailing list.
>>>
>>> Re-run get_maintainer.pl and added maintainers into the maillist.
>>
>>
>> This does not work like this.
>>
>> Please use scripts/get_maintainers.pl to get a list of necessary people
>> and lists to CC.  It might happen, that command when run on an older
>> kernel, gives you outdated entries.  Therefore please be sure you base
>> your patches on recent Linux kernel.
>>
>> You missed at least DT list (maybe more), so this won't be tested by our
>> tools. Performing review on untested code might be a waste of time, thus
>> I will skip this patch entirely till you follow the process allowing the
>> patch to be tested.
>>
>> Please kindly resend and include all necessary To/Cc entries.
> 
> This set of patches is applied on the tag next-20230706, and to
> generate the mail list by scripts/get_maintainers.pl on the tag
> 
> ./scripts/get_maintainer.pl
> ../riscv/linux/v3-0004-dt-bindings-firmware-Document-ffitbl-binding.patch
> Yunhui Cui cuiyunhui@bytedance.com (maintainer:FDT FIRMWARE INTERFACE (FFI))
> Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS)
> Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
> Conor Dooley <conor+dt@kernel.org> (maintainer:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS)
> devicetree@vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS)
> linux-kernel@vger.kernel.org (open list)
> 
> What am I missing ?

I did not receive the original patch. Neither did Patchwork. You cannot
just reply to some comment and hope it will fix something. We don't have
this patch simply.



Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2443 bytes --]

On Thu, Jul 06, 2023 at 11:43:55AM +0800, 运辉崔 wrote:
> On Wed, Jul 5, 2023 at 11:07 PM Conor Dooley <conor@kernel.org> wrote:
> > On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
> > > Add the description for ffitbl subnode.
> > >
> > > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > > ---
> > >  .../devicetree/bindings/firmware/ffitbl.txt   | 27 +++++++++++++++++++
> > >  MAINTAINERS                                   |  1 +
> > >  2 files changed, 28 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> > > new file mode 100644
> > > index 000000000000..c42368626199
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> >
> > Firstly, new dt-bindings need to be done in yaml, not in text form.
> > Secondly, you didn't re-run get_maintainer.pl after adding this binding,
> > so you have not CCed any of the other dt-binding maintainers nor the
> > devicetree mailing list.
> 
> Re-run get_maintainer.pl and added maintainers into the maillist.
> emm.. There is some *txt in
> Documentation/devicetree/bindings/firmware/, isn't it?

There might be, but that's not an excuse for adding _new_ ones, sorry.

> > > +FFI(FDT FIRMWARE INTERFACE) driver
> > > +
> > > +Required properties:
> > > + - entry             : acpi or smbios root pointer, u64
> > > + - reg                       : acpi or smbios version, u32
> >
> > Please go look at any other dt-binding (or the example schema) as to how
> > these properties should be used. A "reg" certainly should not be being
> > used to store the revision...
> 
> Okay, If so，I'll add a property "version" into the dts instead of
> "reg", just like, WDYT?
> ffitbl {

Firstly, I'd much rather you spelt this out, like "ffi-table".

>     smbios {
>         entry = "";

I still don't understand why "entry", which is an address, is being
represented by an empty string.
I also don't really get why you have not used "reg" to describe its
start address and size.

>         version = < 0x02 >;

Probably missing a vendor prefix, and the spaces are unusual, but better
than it was, yes.

>     }
>    acpi {
>          entry = "";
>          version = < 0x06 >;
>   }
> }

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

[运辉崔]

Hi Krzysztof,


On Thu, Jul 6, 2023 at 2:41 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 06/07/2023 08:24, 运辉崔 wrote:
> > Hi Krzysztof,
> >
> > On Thu, Jul 6, 2023 at 2:01 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 06/07/2023 05:43, 运辉崔 wrote:
> >>> Hi Conor,
> >>>
> >>> Added dts Maintainers,
> >>>
> >>> On Wed, Jul 5, 2023 at 11:07 PM Conor Dooley <conor@kernel.org> wrote:
> >>>>
> >>>> Hey,
> >>>>
> >>>> On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
> >>>>> Add the description for ffitbl subnode.
> >>>>>
> >>>>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> >>>>> ---
> >>>>>  .../devicetree/bindings/firmware/ffitbl.txt   | 27 +++++++++++++++++++
> >>>>>  MAINTAINERS                                   |  1 +
> >>>>>  2 files changed, 28 insertions(+)
> >>>>>  create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> >>>>> new file mode 100644
> >>>>> index 000000000000..c42368626199
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> >>>>
> >>>> Firstly, new dt-bindings need to be done in yaml, not in text form.
> >>>> Secondly, you didn't re-run get_maintainer.pl after adding this binding,
> >>>> so you have not CCed any of the other dt-binding maintainers nor the
> >>>> devicetree mailing list.
> >>>
> >>> Re-run get_maintainer.pl and added maintainers into the maillist.
> >>
> >>
> >> This does not work like this.
> >>
> >> Please use scripts/get_maintainers.pl to get a list of necessary people
> >> and lists to CC.  It might happen, that command when run on an older
> >> kernel, gives you outdated entries.  Therefore please be sure you base
> >> your patches on recent Linux kernel.
> >>
> >> You missed at least DT list (maybe more), so this won't be tested by our
> >> tools. Performing review on untested code might be a waste of time, thus
> >> I will skip this patch entirely till you follow the process allowing the
> >> patch to be tested.
> >>
> >> Please kindly resend and include all necessary To/Cc entries.
> >
> > This set of patches is applied on the tag next-20230706, and to
> > generate the mail list by scripts/get_maintainers.pl on the tag
> >
> > ./scripts/get_maintainer.pl
> > ../riscv/linux/v3-0004-dt-bindings-firmware-Document-ffitbl-binding.patch
> > Yunhui Cui cuiyunhui@bytedance.com (maintainer:FDT FIRMWARE INTERFACE (FFI))
> > Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND
> > FLATTENED DEVICE TREE BINDINGS)
> > Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
> > Conor Dooley <conor+dt@kernel.org> (maintainer:OPEN FIRMWARE AND
> > FLATTENED DEVICE TREE BINDINGS)
> > devicetree@vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED
> > DEVICE TREE BINDINGS)
> > linux-kernel@vger.kernel.org (open list)
> >
> > What am I missing ?
>
> I did not receive the original patch. Neither did Patchwork. You cannot
> just reply to some comment and hope it will fix something. We don't have
> this patch simply.

Oh, I see, you only received the middle mail, and did not receive the patch.
Okay, I'll post it after the next version is updated.

>
> Best regards,
> Krzysztof
>

Thanks,
Yunhui

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[Ard Biesheuvel]

On Thu, 6 Jul 2023 at 04:04, 运辉崔 <cuiyunhui@bytedance.com> wrote:
>
> Hi Palmer,
>
> On Wed, Jul 5, 2023 at 10:17 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> > On Wed, 05 Jul 2023 04:42:47 PDT (-0700), cuiyunhui@bytedance.com wrote:
> > > Here's version 3 of patch series.
> > >
> > > V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
> > > consensus with the Maintainers.
> > > Please refer to:
> > > https://patches.linaro.org/project/linux-acpi/patch/20230426034001.16-1-cuiyunhui@bytedance.com/
> >
> > From looking at that thread it seems that the consensus is this is a bad
> > idea?  Sorry if I'm just missing something...
> >
>
> First of all, Coreboot does not support EFI, Ron has expressed, as follows:
> "I am wondering if we can focus on risc-v here, and not drag in ARM,
> b/c the ARM ACPI+UEFI ship has sailed. I had that discussion in 2013
> ;-) and it's clear we don't want to redo it.
> In general, in my world, because of the many problems that come with
> UEFI (security, code quality, performance), we'd like to avoid
> requiring a dependency on UEFI just to get ACPI on RISC-V. It also
> seems, from other discussions I'm having, that there is some belief
> that ACPI will be wanted on RISC-V. It would be nice to separate those
> pieces on RISC-V; certainly they were separate for a very long time in
> the x86 world (we had ACPI+SMM on coreboot laptops without UEFI for
> example)."
>

There appears to be a bit of cargo cult going on here.

I agree that the traditional BIOS vendors did a terrible job pivoting
to (U)EFI when it became a requirement for booting Windows on x86 PCs,
and coreboot did an excellent job providing a retrofit alternative
that was more secure and robust.

However, it makes sense to distinguish between
a) the UEFI specification
b) the UEFI reference implementation (edk2)
c) commercial implementations created by BIOS vendors for x86 PC OEMs
that do not perform any testing beyond booting Windows.

coreboot decided not to implement EFI at all, which on x86 means
booting in a mode that is similar to BIOS boot. Given how the ACPI and
DMTF (for SMBIOS) specifications were already under development when
UEFI was being rolled out on x86, those specs contain provisions
defining how to obtain the ACPI and SMBIOS tables by scanning regions
of memory and looking for magic strings. But this is only defined for
x86, and only works on x86 because all x86 machines are essentially
PCs with a highly uniform system topology.

The ARM case is very different, and while I am no expect on RISC-V,
the following probably applies to it as well:
- there is no need to work around buggy proprietary firmware that can
boot Windows but not Linux
- there is no 'prior art' when it comes to pre-EFI boot interfaces
except for embedded style bare metal boot where all initialization is
done by the kernel (e.g., PCI enumeration and resource assignment
etc), and this is fundamentally arch specific
- ACPI is a rich firmware interface, and the ACPI specification layers
it on top of UEFI so the OS can make certain assumptions about the
extent to which the platform has been initialized by the time it hands
over.

This is why the maintainers of the arm64 and RISC-V ports appear to
agree that ACPI will only be supported when booting from firmware that
implements the EFI specification. Note that this does not impose any
requirement at all regarding which EFI implementation is going to be
used: suggestions have been made on the thread to use a) a coreboot
specific minimal EFI shim that describes the firmware tables and the
EFI memory map, b) the UPL payload for coreboot, and c) U-Boot's EFI
implementation.

I will also note that booting according to the EFI spec is not
fundamentally  more secure or faster: I have done some experiments on
arm64 comparing bare metal boot with EFI boot using a minimal
implementation in Rust, for booting virtual machines under KVM. Due to
cache maintenance overhead and execution with the MMU disabled, bare
metal boot is actually slightly slower. And due to the fact that the
minimal EFI firmware enables the MMU and caches straight out of reset,
it is also arguably more secure, given that all memory permission
based protections and other page table based hardening measures (e.g.,
BTI) are always enabled.

In summary, I think it may be time to stop extrapolating from bad
experiences with buggy proprietary x86 PC firmware created by
traditional BIOS vendors for booting Windows (and nothing else) 15+
years ago. The situation is very different for non-x86 Linux
architectures, where we are trying hard to beat some sense into the
fragmented embedded ecosystem, where every SoC vendor used to have its
own fork of u-boot that booted in a slightly different manner,
requiring a lot of effort on the part of the distros to track all
those moving targets.


> Then, a consensus was reached with Ard, that FFI can be applied to RISC-V.
>

For the record, I would not characterize this as consensus. What I said was
- SMBIOS has very little significance to the kernel itself or impact
on its internal operation, and so it can be exposed via DT in a
generic manner;
- ACPI without UEFI on non-x86 is a) a bad idea, and b) fundamentally
broken on arm64. So b) is out of the question, but it is not up to me
to decide whether or not the RISC-V maintainers should entertain bad
ideas.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

[运辉崔]

Hi Conor,

On Thu, Jul 6, 2023 at 2:45 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Thu, Jul 06, 2023 at 11:43:55AM +0800, 运辉崔 wrote:
> > On Wed, Jul 5, 2023 at 11:07 PM Conor Dooley <conor@kernel.org> wrote:
> > > On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
> > > > Add the description for ffitbl subnode.
> > > >
> > > > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > > > ---
> > > >  .../devicetree/bindings/firmware/ffitbl.txt   | 27 +++++++++++++++++++
> > > >  MAINTAINERS                                   |  1 +
> > > >  2 files changed, 28 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> > > > new file mode 100644
> > > > index 000000000000..c42368626199
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> > >
> > > Firstly, new dt-bindings need to be done in yaml, not in text form.
> > > Secondly, you didn't re-run get_maintainer.pl after adding this binding,
> > > so you have not CCed any of the other dt-binding maintainers nor the
> > > devicetree mailing list.
> >
> > Re-run get_maintainer.pl and added maintainers into the maillist.
> > emm.. There is some *txt in
> > Documentation/devicetree/bindings/firmware/, isn't it?
>
> There might be, but that's not an excuse for adding _new_ ones, sorry.

Okay, I'll update it on v4.


> > > > +FFI(FDT FIRMWARE INTERFACE) driver
> > > > +
> > > > +Required properties:
> > > > + - entry             : acpi or smbios root pointer, u64
> > > > + - reg                       : acpi or smbios version, u32
> > >
> > > Please go look at any other dt-binding (or the example schema) as to how
> > > these properties should be used. A "reg" certainly should not be being
> > > used to store the revision...
> >
> > Okay, If so，I'll add a property "version" into the dts instead of
> > "reg", just like, WDYT?
> > ffitbl {
>
> Firstly, I'd much rather you spelt this out, like "ffi-table".
>
> >     smbios {
> >         entry = "";
>
> I still don't understand why "entry", which is an address, is being
> represented by an empty string.
> I also don't really get why you have not used "reg" to describe its
> start address and size.
>
> >         version = < 0x02 >;
>
> Probably missing a vendor prefix, and the spaces are unusual, but better
> than it was, yes.

Based on your review, I plan to modify it as follows:

ffi-table {
#address-cells = <2>;
#size-cells = <0>;
        smbios@entry1 {
                compatible = "smbios";
                reg = <entry1>;
                version = <2>;
        };
        smbios@entry2 {
                compatible = "smbios";
                reg = <entry2>;
                version = <3>;
        };
        acpi@entry {
                compatible = "acpi";
                reg = <entry>;
                version = <6>;
        };
}

The reason why #size-cells is 0 is because only one item is needed,
#address-cells = <2>; because two u32 are needed to express the 64-bit
address.
The memory for the SMBIOS table itself will be preserved in "reserved
memory" , so we don't have to worry about this piece of memory getting
corrupted. ACPI as well. WDYT?

> >     }
> >    acpi {
> >          entry = "";
> >          version = < 0x06 >;
> >   }
> > }
>
> Thanks,
> Conor.

Thanks,
Yunhui

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[Palmer Dabbelt]

On Thu, 06 Jul 2023 01:53:47 PDT (-0700), Ard Biesheuvel wrote:
> On Thu, 6 Jul 2023 at 04:04, 运辉崔 <cuiyunhui@bytedance.com> wrote:
>>
>> Hi Palmer,
>>
>> On Wed, Jul 5, 2023 at 10:17 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> >
>> > On Wed, 05 Jul 2023 04:42:47 PDT (-0700), cuiyunhui@bytedance.com wrote:
>> > > Here's version 3 of patch series.
>> > >
>> > > V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
>> > > consensus with the Maintainers.
>> > > Please refer to:
>> > > https://patches.linaro.org/project/linux-acpi/patch/20230426034001.16-1-cuiyunhui@bytedance.com/
>> >
>> > From looking at that thread it seems that the consensus is this is a bad
>> > idea?  Sorry if I'm just missing something...
>> >
>>
>> First of all, Coreboot does not support EFI, Ron has expressed, as follows:
>> "I am wondering if we can focus on risc-v here, and not drag in ARM,
>> b/c the ARM ACPI+UEFI ship has sailed. I had that discussion in 2013
>> ;-) and it's clear we don't want to redo it.
>> In general, in my world, because of the many problems that come with
>> UEFI (security, code quality, performance), we'd like to avoid
>> requiring a dependency on UEFI just to get ACPI on RISC-V. It also
>> seems, from other discussions I'm having, that there is some belief
>> that ACPI will be wanted on RISC-V. It would be nice to separate those
>> pieces on RISC-V; certainly they were separate for a very long time in
>> the x86 world (we had ACPI+SMM on coreboot laptops without UEFI for
>> example)."
>>
>
> There appears to be a bit of cargo cult going on here.
>
> I agree that the traditional BIOS vendors did a terrible job pivoting
> to (U)EFI when it became a requirement for booting Windows on x86 PCs,
> and coreboot did an excellent job providing a retrofit alternative
> that was more secure and robust.
>
> However, it makes sense to distinguish between
> a) the UEFI specification
> b) the UEFI reference implementation (edk2)
> c) commercial implementations created by BIOS vendors for x86 PC OEMs
> that do not perform any testing beyond booting Windows.
>
> coreboot decided not to implement EFI at all, which on x86 means
> booting in a mode that is similar to BIOS boot. Given how the ACPI and
> DMTF (for SMBIOS) specifications were already under development when
> UEFI was being rolled out on x86, those specs contain provisions
> defining how to obtain the ACPI and SMBIOS tables by scanning regions
> of memory and looking for magic strings. But this is only defined for

In theory we have that in RISC-V as well: on boot we don't actually have 
a DT pointer, but instead a "config string" pointer.  That's a bit of a 
retcon from when we were planning on adding our own firmware probing 
interface, but in order to appear to have never made a mistake we just 
said that config strings can be anything and have magic numbers to 
differentiate between the flavors.

IIUC we don't take advantage of that in Linux, though, so maybe let's 
just pretend it doesn't exist?

> x86, and only works on x86 because all x86 machines are essentially
> PCs with a highly uniform system topology.
>
> The ARM case is very different, and while I am no expect on RISC-V,
> the following probably applies to it as well:
> - there is no need to work around buggy proprietary firmware that can
> boot Windows but not Linux
> - there is no 'prior art' when it comes to pre-EFI boot interfaces
> except for embedded style bare metal boot where all initialization is
> done by the kernel (e.g., PCI enumeration and resource assignment
> etc), and this is fundamentally arch specific
> - ACPI is a rich firmware interface, and the ACPI specification layers
> it on top of UEFI so the OS can make certain assumptions about the
> extent to which the platform has been initialized by the time it hands
> over.
>
> This is why the maintainers of the arm64 and RISC-V ports appear to
> agree that ACPI will only be supported when booting from firmware that

Yes, we're basically in the same spot as arm64 is here -- or at least 
we're aiming to be, we've yet to even release a kernel that boots with 
ACPI so we have no legacy compatibility yet.

> implements the EFI specification. Note that this does not impose any
> requirement at all regarding which EFI implementation is going to be
> used: suggestions have been made on the thread to use a) a coreboot
> specific minimal EFI shim that describes the firmware tables and the
> EFI memory map, b) the UPL payload for coreboot, and c) U-Boot's EFI
> implementation.
>
> I will also note that booting according to the EFI spec is not
> fundamentally  more secure or faster: I have done some experiments on
> arm64 comparing bare metal boot with EFI boot using a minimal
> implementation in Rust, for booting virtual machines under KVM. Due to
> cache maintenance overhead and execution with the MMU disabled, bare
> metal boot is actually slightly slower. And due to the fact that the
> minimal EFI firmware enables the MMU and caches straight out of reset,
> it is also arguably more secure, given that all memory permission
> based protections and other page table based hardening measures (e.g.,
> BTI) are always enabled.
>
> In summary, I think it may be time to stop extrapolating from bad
> experiences with buggy proprietary x86 PC firmware created by
> traditional BIOS vendors for booting Windows (and nothing else) 15+
> years ago. The situation is very different for non-x86 Linux
> architectures, where we are trying hard to beat some sense into the
> fragmented embedded ecosystem, where every SoC vendor used to have its
> own fork of u-boot that booted in a slightly different manner,
> requiring a lot of effort on the part of the distros to track all
> those moving targets.

That's roughly where we're trying to go in RISC-V land, at least for 
most software people.  Everyone gets their own ISA, which obviously 
causes a ton of fragmentation, but not really anything we can do about 
that.  At least we can avoid adding additional sources of fragmentation 
from the software side of things, though.

>> Then, a consensus was reached with Ard, that FFI can be applied to RISC-V.
>>
>
> For the record, I would not characterize this as consensus. What I said was
> - SMBIOS has very little significance to the kernel itself or impact
> on its internal operation, and so it can be exposed via DT in a
> generic manner;
> - ACPI without UEFI on non-x86 is a) a bad idea, and b) fundamentally
> broken on arm64. So b) is out of the question, but it is not up to me
> to decide whether or not the RISC-V maintainers should entertain bad
> ideas.

IMO we have enough bad ideas in RISC-V already and thus should avoid 
adding more.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 179 bytes --]

Hey Ron,

On Thu, Jul 06, 2023 at 02:39:13PM -0700, ron minnich wrote:

Please do not top-post or send HTML mails to LKML, they are rejected by
the list services.

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[ron minnich]

oh, gmail, it popped back into html mode for some reason. I kinda wish
it would stop doing me so many favors.


On Thu, Jul 6, 2023 at 2:47 PM Conor Dooley <conor@kernel.org> wrote:
>
> Hey Ron,
>
> On Thu, Jul 06, 2023 at 02:39:13PM -0700, ron minnich wrote:
>
> Please do not top-post or send HTML mails to LKML, they are rejected by
> the list services.
>
> Thanks,
> Conor.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 9801 bytes --]

Hey,

I've tried to reformat this a bit, probably gone wrong in the process
somewhere.

On Thu, Jul 06, 2023 at 02:39:13PM -0700, ron minnich wrote:
> On Thu, Jul 6, 2023 at 8:32 AM Palmer Dabbelt <palmer@dabbelt.com<mailto:palmer@dabbelt.com>> wrote:
> > On Thu, 06 Jul 2023 01:53:47 PDT (-0700), Ard Biesheuvel wrote:
> > > On Thu, 6 Jul 2023 at 04:04, 运辉崔 <cuiyunhui@bytedance.com<mailto:cuiyunhui@bytedance.com>> wrote:
> > >> On Wed, Jul 5, 2023 at 10:17 PM Palmer Dabbelt <palmer@dabbelt.com<mailto:palmer@dabbelt.com>> wrote:
> > >> > On Wed, 05 Jul 2023 04:42:47 PDT (-0700), cuiyunhui@bytedance.com<mailto:cuiyunhui@bytedance.com> wrote:
> > >> > > Here's version 3 of patch series.
> > >> > >
> > >> > > V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
> > >> > > consensus with the Maintainers.
> > >> > > Please refer to:
> > >> > > https://patches.linaro.org/project/linux-acpi/patch/20230426034001.16-1-cuiyunhui@bytedance.com/
> > >> >
> > >> > From looking at that thread it seems that the consensus is this is a bad
> > >> > idea?  Sorry if I'm just missing something...
> > >> >
> > >>
> > >> First of all, Coreboot does not support EFI, Ron has expressed, as follows:
> > >> "I am wondering if we can focus on risc-v here, and not drag in ARM,
> > >> b/c the ARM ACPI+UEFI ship has sailed. I had that discussion in 2013
> > >> ;-) and it's clear we don't want to redo it.
> > >> In general, in my world, because of the many problems that come with
> > >> UEFI (security, code quality, performance), we'd like to avoid
> > >> requiring a dependency on UEFI just to get ACPI on RISC-V. It also
> > >> seems, from other discussions I'm having, that there is some belief
> > >> that ACPI will be wanted on RISC-V. It would be nice to separate those
> > >> pieces on RISC-V; certainly they were separate for a very long time in
> > >> the x86 world (we had ACPI+SMM on coreboot laptops without UEFI for
> > >> example)."
> > >>
> > >
> > > There appears to be a bit of cargo cult going on here.
> > >
> > > I agree that the traditional BIOS vendors did a terrible job pivoting
> > > to (U)EFI when it became a requirement for booting Windows on x86 PCs,
> > > and coreboot did an excellent job providing a retrofit alternative
> > > that was more secure and robust.
> > >
> > > However, it makes sense to distinguish between
> > > a) the UEFI specification
> > > b) the UEFI reference implementation (edk2)
> > > c) commercial implementations created by BIOS vendors for x86 PC OEMs
> > > that do not perform any testing beyond booting Windows.
> > >
> > > coreboot decided not to implement EFI at all, which on x86 means
> > > booting in a mode that is similar to BIOS boot. Given how the ACPI and
> > > DMTF (for SMBIOS) specifications were already under development when
> > > UEFI was being rolled out on x86, those specs contain provisions
> > > defining how to obtain the ACPI and SMBIOS tables by scanning regions
> > > of memory and looking for magic strings. But this is only defined for
> > 
> > In theory we have that in RISC-V as well: on boot we don't actually have
> > a DT pointer, but instead a "config string" pointer.  That's a bit of a
> > retcon from when we were planning on adding our own firmware probing
> > interface, but in order to appear to have never made a mistake we just
> > said that config strings can be anything and have magic numbers to
> > differentiate between the flavors.
> > 
> > IIUC we don't take advantage of that in Linux, though, so maybe let's
> > just pretend it doesn't exist?
> > 
> > > x86, and only works on x86 because all x86 machines are essentially
> > > PCs with a highly uniform system topology.
> > >
> > > The ARM case is very different, and while I am no expect on RISC-V,
> > > the following probably applies to it as well:
> > > - there is no need to work around buggy proprietary firmware that can
> > > boot Windows but not Linux
> > > - there is no 'prior art' when it comes to pre-EFI boot interfaces
> > > except for embedded style bare metal boot where all initialization is
> > > done by the kernel (e.g., PCI enumeration and resource assignment
> > > etc), and this is fundamentally arch specific
> > > - ACPI is a rich firmware interface, and the ACPI specification layers
> > > it on top of UEFI so the OS can make certain assumptions about the
> > > extent to which the platform has been initialized by the time it hands
> > > over.
> > >
> > > This is why the maintainers of the arm64 and RISC-V ports appear to
> > > agree that ACPI will only be supported when booting from firmware that
> > 
> > Yes, we're basically in the same spot as arm64 is here -- or at least
> > we're aiming to be, we've yet to even release a kernel that boots with
> > ACPI so we have no legacy compatibility yet.
> > 
> > > implements the EFI specification. Note that this does not impose any
> > > requirement at all regarding which EFI implementation is going to be
> > > used: suggestions have been made on the thread to use a) a coreboot
> > > specific minimal EFI shim that describes the firmware tables and the
> > > EFI memory map, b) the UPL payload for coreboot, and c) U-Boot's EFI
> > > implementation.
> > >
> > > I will also note that booting according to the EFI spec is not
> > > fundamentally  more secure or faster: I have done some experiments on
> > > arm64 comparing bare metal boot with EFI boot using a minimal
> > > implementation in Rust, for booting virtual machines under KVM. Due to
> > > cache maintenance overhead and execution with the MMU disabled, bare
> > > metal boot is actually slightly slower. And due to the fact that the
> > > minimal EFI firmware enables the MMU and caches straight out of reset,
> > > it is also arguably more secure, given that all memory permission
> > > based protections and other page table based hardening measures (e.g.,
> > > BTI) are always enabled.
> > >
> > > In summary, I think it may be time to stop extrapolating from bad
> > > experiences with buggy proprietary x86 PC firmware created by
> > > traditional BIOS vendors for booting Windows (and nothing else) 15+
> > > years ago. The situation is very different for non-x86 Linux
> > > architectures, where we are trying hard to beat some sense into the
> > > fragmented embedded ecosystem, where every SoC vendor used to have its
> > > own fork of u-boot that booted in a slightly different manner,
> > > requiring a lot of effort on the part of the distros to track all
> > > those moving targets.
> > 
> > That's roughly where we're trying to go in RISC-V land, at least for
> > most software people.  Everyone gets their own ISA, which obviously
> > causes a ton of fragmentation, but not really anything we can do about
> > that.  At least we can avoid adding additional sources of fragmentation
> > from the software side of things, though.
> > 
> > >> Then, a consensus was reached with Ard, that FFI can be applied to RISC-V.
> > >>
> > >
> > > For the record, I would not characterize this as consensus. What I said was
> > > - SMBIOS has very little significance to the kernel itself or impact
> > > on its internal operation, and so it can be exposed via DT in a
> > > generic manner;
> > > - ACPI without UEFI on non-x86 is a) a bad idea, and b) fundamentally
> > > broken on arm64. So b) is out of the question, but it is not up to me
> > > to decide whether or not the RISC-V maintainers should entertain bad
> > > ideas.
> > 
> > IMO we have enough bad ideas in RISC-V already and thus should avoid
> > adding more.

> ACPI was not originally part of UEFI. ACPI works just fine on
> Chromebooks, and has for 12 years, and on coreboot since 2006,
> without UEFI. I've integrated support for ACPI into several
> code bases, including Plan 9 on non-UEFI systems.
> 
> I am unable to understand the claim that ACPI on non-UEFI
> RISC-V is a bad idea. Clearly, I am not alone.
> 
> But, all that said, I get the impression that the gatekeepers
> are absolutely immovable on this question?

> Perhaps the right way
> to move forward is to find a way to extract what we need from ACPI
> and move forward on systems that can function without UEFI AND ACPI?
> Would that be preferable?

Isn't this exactly the type of thing that has been proposed by this
series, that everyone seems to be against? Or are you suggesting that we
would, on a DT system, read some ACPI information, and then revert to
being DT based?

> Just so we're all on the same page, I just now asked Mark Himelstein
> of RISC-V International if there is anything in RISC-V standards that
> requires UEFI, and the answer is a solid "no."

Huh? Firstly, running off to invoke RVI is not productive - they don't
maintain the various operating system kernels etc.
Secondly, that does not seem to be true. The platform spec mandates UEFI
for the OS-A server platform, alongside ACPI:
https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#32-boot-process
and the OS-A embedded platform needs to comply with EBBR & use DT:
https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#32-boot-process

EBBR does say that systems must not provide both ACPI and DT to the OS
loader, but I am far from an expert on these kind of things & am not
sure where something like this where the DT "contains" ACPI would stand.

The RISC-V ACPI spec also says "UEFI firmware is mandatory to support
ACPI":
https://github.com/riscv-non-isa/riscv-acpi/blob/master/riscv-acpi-guidance.adoc

Jess, Sunil or Ard on the EBBR front perhaps, please correct me here if I
have got anything wrong.

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[Sunil V L]

On Fri, Jul 07, 2023 at 09:38:30AM +0100, Conor Dooley wrote:
> Hey,
> 
> I've tried to reformat this a bit, probably gone wrong in the process
> somewhere.
> 
> On Thu, Jul 06, 2023 at 02:39:13PM -0700, ron minnich wrote:
> > On Thu, Jul 6, 2023 at 8:32 AM Palmer Dabbelt <palmer@dabbelt.com<mailto:palmer@dabbelt.com>> wrote:
> > > On Thu, 06 Jul 2023 01:53:47 PDT (-0700), Ard Biesheuvel wrote:
> > > > On Thu, 6 Jul 2023 at 04:04, 运辉崔 <cuiyunhui@bytedance.com<mailto:cuiyunhui@bytedance.com>> wrote:
> > > >> On Wed, Jul 5, 2023 at 10:17 PM Palmer Dabbelt <palmer@dabbelt.com<mailto:palmer@dabbelt.com>> wrote:
> > > >> > On Wed, 05 Jul 2023 04:42:47 PDT (-0700), cuiyunhui@bytedance.com<mailto:cuiyunhui@bytedance.com> wrote:
> > > >> > > Here's version 3 of patch series.
> > > >> > >
> > > >> > > V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
> > > >> > > consensus with the Maintainers.
> > > >> > > Please refer to:
> > > >> > > https://patches.linaro.org/project/linux-acpi/patch/20230426034001.16-1-cuiyunhui@bytedance.com/
> > > >> >
> > > >> > From looking at that thread it seems that the consensus is this is a bad
> > > >> > idea?  Sorry if I'm just missing something...
> > > >> >
> > > >>
> > > >> First of all, Coreboot does not support EFI, Ron has expressed, as follows:
> > > >> "I am wondering if we can focus on risc-v here, and not drag in ARM,
> > > >> b/c the ARM ACPI+UEFI ship has sailed. I had that discussion in 2013
> > > >> ;-) and it's clear we don't want to redo it.
> > > >> In general, in my world, because of the many problems that come with
> > > >> UEFI (security, code quality, performance), we'd like to avoid
> > > >> requiring a dependency on UEFI just to get ACPI on RISC-V. It also
> > > >> seems, from other discussions I'm having, that there is some belief
> > > >> that ACPI will be wanted on RISC-V. It would be nice to separate those
> > > >> pieces on RISC-V; certainly they were separate for a very long time in
> > > >> the x86 world (we had ACPI+SMM on coreboot laptops without UEFI for
> > > >> example)."
> > > >>
> > > >
> > > > There appears to be a bit of cargo cult going on here.
> > > >
> > > > I agree that the traditional BIOS vendors did a terrible job pivoting
> > > > to (U)EFI when it became a requirement for booting Windows on x86 PCs,
> > > > and coreboot did an excellent job providing a retrofit alternative
> > > > that was more secure and robust.
> > > >
> > > > However, it makes sense to distinguish between
> > > > a) the UEFI specification
> > > > b) the UEFI reference implementation (edk2)
> > > > c) commercial implementations created by BIOS vendors for x86 PC OEMs
> > > > that do not perform any testing beyond booting Windows.
> > > >
> > > > coreboot decided not to implement EFI at all, which on x86 means
> > > > booting in a mode that is similar to BIOS boot. Given how the ACPI and
> > > > DMTF (for SMBIOS) specifications were already under development when
> > > > UEFI was being rolled out on x86, those specs contain provisions
> > > > defining how to obtain the ACPI and SMBIOS tables by scanning regions
> > > > of memory and looking for magic strings. But this is only defined for
> > > 
> > > In theory we have that in RISC-V as well: on boot we don't actually have
> > > a DT pointer, but instead a "config string" pointer.  That's a bit of a
> > > retcon from when we were planning on adding our own firmware probing
> > > interface, but in order to appear to have never made a mistake we just
> > > said that config strings can be anything and have magic numbers to
> > > differentiate between the flavors.
> > > 
> > > IIUC we don't take advantage of that in Linux, though, so maybe let's
> > > just pretend it doesn't exist?
> > > 
> > > > x86, and only works on x86 because all x86 machines are essentially
> > > > PCs with a highly uniform system topology.
> > > >
> > > > The ARM case is very different, and while I am no expect on RISC-V,
> > > > the following probably applies to it as well:
> > > > - there is no need to work around buggy proprietary firmware that can
> > > > boot Windows but not Linux
> > > > - there is no 'prior art' when it comes to pre-EFI boot interfaces
> > > > except for embedded style bare metal boot where all initialization is
> > > > done by the kernel (e.g., PCI enumeration and resource assignment
> > > > etc), and this is fundamentally arch specific
> > > > - ACPI is a rich firmware interface, and the ACPI specification layers
> > > > it on top of UEFI so the OS can make certain assumptions about the
> > > > extent to which the platform has been initialized by the time it hands
> > > > over.
> > > >
> > > > This is why the maintainers of the arm64 and RISC-V ports appear to
> > > > agree that ACPI will only be supported when booting from firmware that
> > > 
> > > Yes, we're basically in the same spot as arm64 is here -- or at least
> > > we're aiming to be, we've yet to even release a kernel that boots with
> > > ACPI so we have no legacy compatibility yet.
> > > 
> > > > implements the EFI specification. Note that this does not impose any
> > > > requirement at all regarding which EFI implementation is going to be
> > > > used: suggestions have been made on the thread to use a) a coreboot
> > > > specific minimal EFI shim that describes the firmware tables and the
> > > > EFI memory map, b) the UPL payload for coreboot, and c) U-Boot's EFI
> > > > implementation.
> > > >
> > > > I will also note that booting according to the EFI spec is not
> > > > fundamentally  more secure or faster: I have done some experiments on
> > > > arm64 comparing bare metal boot with EFI boot using a minimal
> > > > implementation in Rust, for booting virtual machines under KVM. Due to
> > > > cache maintenance overhead and execution with the MMU disabled, bare
> > > > metal boot is actually slightly slower. And due to the fact that the
> > > > minimal EFI firmware enables the MMU and caches straight out of reset,
> > > > it is also arguably more secure, given that all memory permission
> > > > based protections and other page table based hardening measures (e.g.,
> > > > BTI) are always enabled.
> > > >
> > > > In summary, I think it may be time to stop extrapolating from bad
> > > > experiences with buggy proprietary x86 PC firmware created by
> > > > traditional BIOS vendors for booting Windows (and nothing else) 15+
> > > > years ago. The situation is very different for non-x86 Linux
> > > > architectures, where we are trying hard to beat some sense into the
> > > > fragmented embedded ecosystem, where every SoC vendor used to have its
> > > > own fork of u-boot that booted in a slightly different manner,
> > > > requiring a lot of effort on the part of the distros to track all
> > > > those moving targets.
> > > 
> > > That's roughly where we're trying to go in RISC-V land, at least for
> > > most software people.  Everyone gets their own ISA, which obviously
> > > causes a ton of fragmentation, but not really anything we can do about
> > > that.  At least we can avoid adding additional sources of fragmentation
> > > from the software side of things, though.
> > > 
> > > >> Then, a consensus was reached with Ard, that FFI can be applied to RISC-V.
> > > >>
> > > >
> > > > For the record, I would not characterize this as consensus. What I said was
> > > > - SMBIOS has very little significance to the kernel itself or impact
> > > > on its internal operation, and so it can be exposed via DT in a
> > > > generic manner;
> > > > - ACPI without UEFI on non-x86 is a) a bad idea, and b) fundamentally
> > > > broken on arm64. So b) is out of the question, but it is not up to me
> > > > to decide whether or not the RISC-V maintainers should entertain bad
> > > > ideas.
> > > 
> > > IMO we have enough bad ideas in RISC-V already and thus should avoid
> > > adding more.
> 
> > ACPI was not originally part of UEFI. ACPI works just fine on
> > Chromebooks, and has for 12 years, and on coreboot since 2006,
> > without UEFI. I've integrated support for ACPI into several
> > code bases, including Plan 9 on non-UEFI systems.
> > 

As per the section  5.2.5 of ACPI spec [1], there are only two ways
defined to locate the RSDP. IA-PC is not applicable to RISC-V and only
other method defined is via UEFI.

[1] - https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#root-system-description-pointer-rsdp

> > I am unable to understand the claim that ACPI on non-UEFI
> > RISC-V is a bad idea. Clearly, I am not alone.
> > 
> > But, all that said, I get the impression that the gatekeepers
> > are absolutely immovable on this question?
> 
> > Perhaps the right way
> > to move forward is to find a way to extract what we need from ACPI
> > and move forward on systems that can function without UEFI AND ACPI?
> > Would that be preferable?
> 
> Isn't this exactly the type of thing that has been proposed by this
> series, that everyone seems to be against? Or are you suggesting that we
> would, on a DT system, read some ACPI information, and then revert to
> being DT based?
> 
> > Just so we're all on the same page, I just now asked Mark Himelstein
> > of RISC-V International if there is anything in RISC-V standards that
> > requires UEFI, and the answer is a solid "no."
> 
> Huh? Firstly, running off to invoke RVI is not productive - they don't
> maintain the various operating system kernels etc.
> Secondly, that does not seem to be true. The platform spec mandates UEFI
> for the OS-A server platform, alongside ACPI:
> https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#32-boot-process
> and the OS-A embedded platform needs to comply with EBBR & use DT:
> https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#32-boot-process
> 
> EBBR does say that systems must not provide both ACPI and DT to the OS
> loader, but I am far from an expert on these kind of things & am not
> sure where something like this where the DT "contains" ACPI would stand.
> 
> The RISC-V ACPI spec also says "UEFI firmware is mandatory to support
> ACPI":
> https://github.com/riscv-non-isa/riscv-acpi/blob/master/riscv-acpi-guidance.adoc
> 
> Jess, Sunil or Ard on the EBBR front perhaps, please correct me here if I
> have got anything wrong.
> 
IMO, if the question is generic like "Is UEFI mandatory for RISC-V?",
the answer will be solid "no" because we can use DT without UEFI. But if
you ask whether UEFI is mandatory for ACPI support on RISC-V, then the
answer will be "yes".

Thanks,
Sunil

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[Sunil V L]

On Fri, Jul 07, 2023 at 08:05:48PM +0800, 葛士建 wrote:
> Hi Sunil,
> 
> From Sunil:
> IMO, if the question is generic like "Is UEFI mandatory for RISC-V?",
> the answer will be solid "no" because we can use DT without UEFI. But if
> you ask whether UEFI is mandatory for ACPI support on RISC-V, then the
> answer will be "yes".
> ---- Why UEFI is mandatory for ACPI support on RISC-V?  As we know, on X86,
> ACPI works well without UEFI. Is there any limitation on RISC-V
> architecture?
Yes, the limitation is RISC-V can not use IA-PC BIOS. Please see
section 5.2.5 and 15 in ACPI spec.

I don't have much to add to Ard's reasons.
https://lore.kernel.org/linux-riscv/CAMj1kXFZren0Q19DimwQaETCLz64D4bZQC5B2N=i3SAWHygkTQ@mail.gmail.com/

> BTW, I don't think ACPI was from UEFI, and ACPI works well with coreboot on
> Chromebook as Ron said.
> 
> + Dong Wei for ARM ISA..
> 
> Thanks,
> -Nill
> 
> 
> On Fri, Jul 7, 2023 at 6:43 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> 
> > On Fri, Jul 07, 2023 at 09:38:30AM +0100, Conor Dooley wrote:
> > > Hey,
> > >
> > > I've tried to reformat this a bit, probably gone wrong in the process
> > > somewhere.
> > >
> > > On Thu, Jul 06, 2023 at 02:39:13PM -0700, ron minnich wrote:
> > > > On Thu, Jul 6, 2023 at 8:32 AM Palmer Dabbelt <palmer@dabbelt.com
> > <mailto:palmer@dabbelt.com>> wrote:
> > > > > On Thu, 06 Jul 2023 01:53:47 PDT (-0700), Ard Biesheuvel wrote:
> > > > > > On Thu, 6 Jul 2023 at 04:04, 运辉崔 <cuiyunhui@bytedance.com<mailto:
> > cuiyunhui@bytedance.com>> wrote:
> > > > > >> On Wed, Jul 5, 2023 at 10:17 PM Palmer Dabbelt <
> > palmer@dabbelt.com<mailto:palmer@dabbelt.com>> wrote:
> > > > > >> > On Wed, 05 Jul 2023 04:42:47 PDT (-0700),
> > cuiyunhui@bytedance.com<mailto:cuiyunhui@bytedance.com> wrote:
> > > > > >> > > Here's version 3 of patch series.
> > > > > >> > >
> > > > > >> > > V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
> > > > > >> > > consensus with the Maintainers.
> > > > > >> > > Please refer to:
> > > > > >> > >
> > https://patches.linaro.org/project/linux-acpi/patch/20230426034001.16-1-cuiyunhui@bytedance.com/
> > > > > >> >
> > > > > >> > From looking at that thread it seems that the consensus is this
> > is a bad
> > > > > >> > idea?  Sorry if I'm just missing something...
> > > > > >> >
> > > > > >>
> > > > > >> First of all, Coreboot does not support EFI, Ron has expressed,
> > as follows:
> > > > > >> "I am wondering if we can focus on risc-v here, and not drag in
> > ARM,
> > > > > >> b/c the ARM ACPI+UEFI ship has sailed. I had that discussion in
> > 2013
> > > > > >> ;-) and it's clear we don't want to redo it.
> > > > > >> In general, in my world, because of the many problems that come
> > with
> > > > > >> UEFI (security, code quality, performance), we'd like to avoid
> > > > > >> requiring a dependency on UEFI just to get ACPI on RISC-V. It also
> > > > > >> seems, from other discussions I'm having, that there is some
> > belief
> > > > > >> that ACPI will be wanted on RISC-V. It would be nice to separate
> > those
> > > > > >> pieces on RISC-V; certainly they were separate for a very long
> > time in
> > > > > >> the x86 world (we had ACPI+SMM on coreboot laptops without UEFI
> > for
> > > > > >> example)."
> > > > > >>
> > > > > >
> > > > > > There appears to be a bit of cargo cult going on here.
> > > > > >
> > > > > > I agree that the traditional BIOS vendors did a terrible job
> > pivoting
> > > > > > to (U)EFI when it became a requirement for booting Windows on x86
> > PCs,
> > > > > > and coreboot did an excellent job providing a retrofit alternative
> > > > > > that was more secure and robust.
> > > > > >
> > > > > > However, it makes sense to distinguish between
> > > > > > a) the UEFI specification
> > > > > > b) the UEFI reference implementation (edk2)
> > > > > > c) commercial implementations created by BIOS vendors for x86 PC
> > OEMs
> > > > > > that do not perform any testing beyond booting Windows.
> > > > > >
> > > > > > coreboot decided not to implement EFI at all, which on x86 means
> > > > > > booting in a mode that is similar to BIOS boot. Given how the ACPI
> > and
> > > > > > DMTF (for SMBIOS) specifications were already under development
> > when
> > > > > > UEFI was being rolled out on x86, those specs contain provisions
> > > > > > defining how to obtain the ACPI and SMBIOS tables by scanning
> > regions
> > > > > > of memory and looking for magic strings. But this is only defined
> > for
> > > > >
> > > > > In theory we have that in RISC-V as well: on boot we don't actually
> > have
> > > > > a DT pointer, but instead a "config string" pointer.  That's a bit
> > of a
> > > > > retcon from when we were planning on adding our own firmware probing
> > > > > interface, but in order to appear to have never made a mistake we
> > just
> > > > > said that config strings can be anything and have magic numbers to
> > > > > differentiate between the flavors.
> > > > >
> > > > > IIUC we don't take advantage of that in Linux, though, so maybe let's
> > > > > just pretend it doesn't exist?
> > > > >
> > > > > > x86, and only works on x86 because all x86 machines are essentially
> > > > > > PCs with a highly uniform system topology.
> > > > > >
> > > > > > The ARM case is very different, and while I am no expect on RISC-V,
> > > > > > the following probably applies to it as well:
> > > > > > - there is no need to work around buggy proprietary firmware that
> > can
> > > > > > boot Windows but not Linux
> > > > > > - there is no 'prior art' when it comes to pre-EFI boot interfaces
> > > > > > except for embedded style bare metal boot where all initialization
> > is
> > > > > > done by the kernel (e.g., PCI enumeration and resource assignment
> > > > > > etc), and this is fundamentally arch specific
> > > > > > - ACPI is a rich firmware interface, and the ACPI specification
> > layers
> > > > > > it on top of UEFI so the OS can make certain assumptions about the
> > > > > > extent to which the platform has been initialized by the time it
> > hands
> > > > > > over.
> > > > > >
> > > > > > This is why the maintainers of the arm64 and RISC-V ports appear to
> > > > > > agree that ACPI will only be supported when booting from firmware
> > that
> > > > >
> > > > > Yes, we're basically in the same spot as arm64 is here -- or at least
> > > > > we're aiming to be, we've yet to even release a kernel that boots
> > with
> > > > > ACPI so we have no legacy compatibility yet.
> > > > >
> > > > > > implements the EFI specification. Note that this does not impose
> > any
> > > > > > requirement at all regarding which EFI implementation is going to
> > be
> > > > > > used: suggestions have been made on the thread to use a) a coreboot
> > > > > > specific minimal EFI shim that describes the firmware tables and
> > the
> > > > > > EFI memory map, b) the UPL payload for coreboot, and c) U-Boot's
> > EFI
> > > > > > implementation.
> > > > > >
> > > > > > I will also note that booting according to the EFI spec is not
> > > > > > fundamentally  more secure or faster: I have done some experiments
> > on
> > > > > > arm64 comparing bare metal boot with EFI boot using a minimal
> > > > > > implementation in Rust, for booting virtual machines under KVM.
> > Due to
> > > > > > cache maintenance overhead and execution with the MMU disabled,
> > bare
> > > > > > metal boot is actually slightly slower. And due to the fact that
> > the
> > > > > > minimal EFI firmware enables the MMU and caches straight out of
> > reset,
> > > > > > it is also arguably more secure, given that all memory permission
> > > > > > based protections and other page table based hardening measures
> > (e.g.,
> > > > > > BTI) are always enabled.
> > > > > >
> > > > > > In summary, I think it may be time to stop extrapolating from bad
> > > > > > experiences with buggy proprietary x86 PC firmware created by
> > > > > > traditional BIOS vendors for booting Windows (and nothing else) 15+
> > > > > > years ago. The situation is very different for non-x86 Linux
> > > > > > architectures, where we are trying hard to beat some sense into the
> > > > > > fragmented embedded ecosystem, where every SoC vendor used to have
> > its
> > > > > > own fork of u-boot that booted in a slightly different manner,
> > > > > > requiring a lot of effort on the part of the distros to track all
> > > > > > those moving targets.
> > > > >
> > > > > That's roughly where we're trying to go in RISC-V land, at least for
> > > > > most software people.  Everyone gets their own ISA, which obviously
> > > > > causes a ton of fragmentation, but not really anything we can do
> > about
> > > > > that.  At least we can avoid adding additional sources of
> > fragmentation
> > > > > from the software side of things, though.
> > > > >
> > > > > >> Then, a consensus was reached with Ard, that FFI can be applied
> > to RISC-V.
> > > > > >>
> > > > > >
> > > > > > For the record, I would not characterize this as consensus. What I
> > said was
> > > > > > - SMBIOS has very little significance to the kernel itself or
> > impact
> > > > > > on its internal operation, and so it can be exposed via DT in a
> > > > > > generic manner;
> > > > > > - ACPI without UEFI on non-x86 is a) a bad idea, and b)
> > fundamentally
> > > > > > broken on arm64. So b) is out of the question, but it is not up to
> > me
> > > > > > to decide whether or not the RISC-V maintainers should entertain
> > bad
> > > > > > ideas.
> > > > >
> > > > > IMO we have enough bad ideas in RISC-V already and thus should avoid
> > > > > adding more.
> > >
> > > > ACPI was not originally part of UEFI. ACPI works just fine on
> > > > Chromebooks, and has for 12 years, and on coreboot since 2006,
> > > > without UEFI. I've integrated support for ACPI into several
> > > > code bases, including Plan 9 on non-UEFI systems.
> > > >
> >
> > As per the section  5.2.5 of ACPI spec [1], there are only two ways
> > defined to locate the RSDP. IA-PC is not applicable to RISC-V and only
> > other method defined is via UEFI.
> >
> > [1] -
> > https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#root-system-description-pointer-rsdp
> >
> > > > I am unable to understand the claim that ACPI on non-UEFI
> > > > RISC-V is a bad idea. Clearly, I am not alone.
> > > >
> > > > But, all that said, I get the impression that the gatekeepers
> > > > are absolutely immovable on this question?
> > >
> > > > Perhaps the right way
> > > > to move forward is to find a way to extract what we need from ACPI
> > > > and move forward on systems that can function without UEFI AND ACPI?
> > > > Would that be preferable?
> > >
> > > Isn't this exactly the type of thing that has been proposed by this
> > > series, that everyone seems to be against? Or are you suggesting that we
> > > would, on a DT system, read some ACPI information, and then revert to
> > > being DT based?
> > >
> > > > Just so we're all on the same page, I just now asked Mark Himelstein
> > > > of RISC-V International if there is anything in RISC-V standards that
> > > > requires UEFI, and the answer is a solid "no."
> > >
> > > Huh? Firstly, running off to invoke RVI is not productive - they don't
> > > maintain the various operating system kernels etc.
> > > Secondly, that does not seem to be true. The platform spec mandates UEFI
> > > for the OS-A server platform, alongside ACPI:
> > >
> > https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#32-boot-process
> > > and the OS-A embedded platform needs to comply with EBBR & use DT:
> > >
> > https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#32-boot-process
> > >
> > > EBBR does say that systems must not provide both ACPI and DT to the OS
> > > loader, but I am far from an expert on these kind of things & am not
> > > sure where something like this where the DT "contains" ACPI would stand.
> > >
> > > The RISC-V ACPI spec also says "UEFI firmware is mandatory to support
> > > ACPI":
> > >
> > https://github.com/riscv-non-isa/riscv-acpi/blob/master/riscv-acpi-guidance.adoc
> > >
> > > Jess, Sunil or Ard on the EBBR front perhaps, please correct me here if I
> > > have got anything wrong.
> > >
> > IMO, if the question is generic like "Is UEFI mandatory for RISC-V?",
> > the answer will be solid "no" because we can use DT without UEFI. But if
> > you ask whether UEFI is mandatory for ACPI support on RISC-V, then the
> > answer will be "yes".
> >
> > Thanks,
> > Sunil
> >

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1991 bytes --]

Hey,

On Fri, Jul 07, 2023 at 11:56:48PM +0800, 葛士建 wrote:
> On Fri, Jul 7, 2023 at 8:55 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> 
> > On Fri, Jul 07, 2023 at 08:05:48PM +0800, 葛士建 wrote:
> > > Hi Sunil,
> > >
> > > From Sunil:
> > > IMO, if the question is generic like "Is UEFI mandatory for RISC-V?",
> > > the answer will be solid "no" because we can use DT without UEFI. But if
> > > you ask whether UEFI is mandatory for ACPI support on RISC-V, then the
> > > answer will be "yes".
> > > ---- Why UEFI is mandatory for ACPI support on RISC-V?  As we know, on X86,
> > > ACPI works well without UEFI. Is there any limitation on RISC-V
> > > architecture?
> > Yes, the limitation is RISC-V can not use IA-PC BIOS. Please see
> > section 5.2.5 and 15 in ACPI spec.
> >
> > I don't have much to add to Ard's reasons.
> >
> > https://lore.kernel.org/linux-riscv/CAMj1kXFZren0Q19DimwQaETCLz64D4bZQC5B2N=i3SAWHygkTQ@mail.gmail.com/
> >

> I don't think that's the limitation on RISC-V. BTW, how does OSPM find the
> RSDP on ARM systems? Does it meet 5.2.5?
> 
> Here are
> 1. Purpose: purpose is to provide another option on Firmware Solution; Our
> purpose is NOT to ban UEFI.
> 2. Both ARM and RISC-V starts from UBOOT solution, and that's close to
> coreboot, so we would like to enable flexible and rich ecosystem.
> 3. We don't like to push coreboot and UEFI together, so we don't plan to
> enable UEFI in coreboot(maybe from Uboot); because that makes the solution
> complex.
> 4. I think we should fix the request and problem, banning or protecting
> something is NOT the goal of us.
> 
> I think the solution is for both RISC-V and ARM, and also it works on X86
> if it's done.
> Let me know what the problem and impact is, please.

If you are going to keep arguing this, please stop sending top-posted
HTML to the mailing list. It makes it impossible for those not in the CC
list to follow along.

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2631 bytes --]

Hey,

On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
> Add the description for ffitbl subnode.
> 
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> ---
>  .../devicetree/bindings/firmware/ffitbl.txt   | 27 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
> 
> diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> new file mode 100644
> index 000000000000..c42368626199
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> @@ -0,0 +1,27 @@
> +FFI(FDT FIRMWARE INTERFACE) driver
> +
> +Required properties:
> + - entry		: acpi or smbios root pointer, u64
> + - reg			: acpi or smbios version, u32
> +
> +Some bootloaders, such as Coreboot do not support EFI,
> +only devicetree and some arches do not have a reserved
> +address segment. Add "ffitbl" subnode to obtain ACPI RSDP
> +and SMBIOS entry.

Since the conversation on this stuff all seems to be going absolutely
nowhere, the ACPI portion of this is intended for use on RISC-V in
violation of the RISC-V ACPI specs. It also goes against the
requirements of the platform spec. Quoting from [1]:

| > Just so we're all on the same page, I just now asked Mark Himelstein
| > of RISC-V International if there is anything in RISC-V standards that
| > requires UEFI, and the answer is a solid "no."
| 
| Huh? Firstly, running off to invoke RVI is not productive - they don't
| maintain the various operating system kernels etc.
| Secondly, that does not seem to be true. The platform spec mandates UEFI
| for the OS-A server platform, alongside ACPI:
| https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#32-boot-process
| and the OS-A embedded platform needs to comply with EBBR & use DT:
| https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#32-boot-process
| 
| EBBR does say that systems must not provide both ACPI and DT to the OS
| loader, but I am far from an expert on these kind of things & am not
| sure where something like this where the DT "contains" ACPI would stand.
| 
| The RISC-V ACPI spec also says "UEFI firmware is mandatory to support
| ACPI":
| https://github.com/riscv-non-isa/riscv-acpi/blob/master/riscv-acpi-guidance.adoc

NAKed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[1] - https://lore.kernel.org/linux-riscv/20230707-attach-conjuror-306d967347ce@wendy/

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[葛士建]

On Sat, Jul 8, 2023 at 12:07 AM Conor Dooley <conor@kernel.org> wrote:
>
> Hey,
>
> On Fri, Jul 07, 2023 at 11:56:48PM +0800, 葛士建 wrote:
> > On Fri, Jul 7, 2023 at 8:55 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> >
> > > On Fri, Jul 07, 2023 at 08:05:48PM +0800, 葛士建 wrote:
> > > > Hi Sunil,
> > > >
> > > > From Sunil:
> > > > IMO, if the question is generic like "Is UEFI mandatory for RISC-V?",
> > > > the answer will be solid "no" because we can use DT without UEFI. But if
> > > > you ask whether UEFI is mandatory for ACPI support on RISC-V, then the
> > > > answer will be "yes".
> > > > ---- Why UEFI is mandatory for ACPI support on RISC-V?  As we know, on X86,
> > > > ACPI works well without UEFI. Is there any limitation on RISC-V
> > > > architecture?
> > > Yes, the limitation is RISC-V can not use IA-PC BIOS. Please see
> > > section 5.2.5 and 15 in ACPI spec.
> > >
> > > I don't have much to add to Ard's reasons.
> > >
> > > https://lore.kernel.org/linux-riscv/CAMj1kXFZren0Q19DimwQaETCLz64D4bZQC5B2N=i3SAWHygkTQ@mail.gmail.com/
> > >
>
> > I don't think that's the limitation on RISC-V. BTW, how does OSPM find the
> > RSDP on ARM systems? Does it meet 5.2.5?
> >
> > Here are
> > 1. Purpose: purpose is to provide another option on Firmware Solution; Our
> > purpose is NOT to ban UEFI.
> > 2. Both ARM and RISC-V starts from UBOOT solution, and that's close to
> > coreboot, so we would like to enable flexible and rich ecosystem.
> > 3. We don't like to push coreboot and UEFI together, so we don't plan to
> > enable UEFI in coreboot(maybe from Uboot); because that makes the solution
> > complex.
> > 4. I think we should fix the request and problem, banning or protecting
> > something is NOT the goal of us.
> >
> > I think the solution is for both RISC-V and ARM, and also it works on X86
> > if it's done.
> > Let me know what the problem and impact is, please.
>
> If you are going to keep arguing this, please stop sending top-posted
> HTML to the mailing list. It makes it impossible for those not in the CC
> list to follow along.

Thanks Conor, I will follow the rules.

>
>
> Thanks,
> Conor.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

[运辉崔]

Hi Conor,

On Sat, Jul 8, 2023 at 12:53 AM 葛士建 <geshijian@bytedance.com> wrote:
>
>
>
>
> On Sat, Jul 8, 2023 at 12:16 AM Conor Dooley <conor@kernel.org> wrote:
>>
>> Hey,
>>
>> On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
>> > Add the description for ffitbl subnode.
>> >
>> > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
>> > ---
>> >  .../devicetree/bindings/firmware/ffitbl.txt   | 27 +++++++++++++++++++
>> >  MAINTAINERS                                   |  1 +
>> >  2 files changed, 28 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
>> > new file mode 100644
>> > index 000000000000..c42368626199
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
>> > @@ -0,0 +1,27 @@
>> > +FFI(FDT FIRMWARE INTERFACE) driver
>> > +
>> > +Required properties:
>> > + - entry             : acpi or smbios root pointer, u64
>> > + - reg                       : acpi or smbios version, u32
>> > +
>> > +Some bootloaders, such as Coreboot do not support EFI,
>> > +only devicetree and some arches do not have a reserved
>> > +address segment. Add "ffitbl" subnode to obtain ACPI RSDP
>> > +and SMBIOS entry.
>>
>> Since the conversation on this stuff all seems to be going absolutely
>> nowhere, the ACPI portion of this is intended for use on RISC-V in
>> violation of the RISC-V ACPI specs. It also goes against the
>> requirements of the platform spec. Quoting from [1]:
>>
>> | > Just so we're all on the same page, I just now asked Mark Himelstein
>> | > of RISC-V International if there is anything in RISC-V standards that
>> | > requires UEFI, and the answer is a solid "no."
>> |
>> | Huh? Firstly, running off to invoke RVI is not productive - they don't
>> | maintain the various operating system kernels etc.
>> | Secondly, that does not seem to be true. The platform spec mandates UEFI
>> | for the OS-A server platform, alongside ACPI:
>> | https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#32-boot-process
>> | and the OS-A embedded platform needs to comply with EBBR & use DT:
>> | https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#32-boot-process
>> |
>> | EBBR does say that systems must not provide both ACPI and DT to the OS
>> | loader, but I am far from an expert on these kind of things & am not
>> | sure where something like this where the DT "contains" ACPI would stand.
>> |
>> | The RISC-V ACPI spec also says "UEFI firmware is mandatory to support
>> | ACPI":
>> | https://github.com/riscv-non-isa/riscv-acpi/blob/master/riscv-acpi-guidance.adoc
>
>  UEFI firmware is mandatory to support ACPI and coreboot is an option to support ACPI as well. i think it works well for both, I don't think UEFI and ACPI need to be binding together, each UEFI and ACPI also works well with other solutions.

Thanks for shijian(Nill)'s suggestions.

Hi Conor,
Thank you very much for your valuable comments on this set of patch
codes, which are very helpful.

Judging from the current specifications, maybe yes, you can NACK, but
it's better not to rush to conclusions.
The so-called specifications represent the ideas of FFI opponents.
What we have to do is to discuss with them and get an effective
consensus, so as to achieve the purpose of updating the specification.

>>
>>
>>
>> NAKed-by: Conor Dooley <conor.dooley@microchip.com>
>>
>> Cheers,
>> Conor.
>>
>> [1] - https://lore.kernel.org/linux-riscv/20230707-attach-conjuror-306d967347ce@wendy/

Thanks,
Yunhui

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

[Conor Dooley]

On 8 July 2023 04:04:05 IST, "运辉崔" <cuiyunhui@bytedance.com> wrote:
>Hi Conor,
>
>On Sat, Jul 8, 2023 at 12:53 AM 葛士建 <geshijian@bytedance.com> wrote:
>>
>>
>>
>>
>> On Sat, Jul 8, 2023 at 12:16 AM Conor Dooley <conor@kernel.org> wrote:
>>>
>>> Hey,
>>>
>>> On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
>>> > Add the description for ffitbl subnode.
>>> >
>>> > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
>>> > ---
>>> >  .../devicetree/bindings/firmware/ffitbl.txt   | 27 +++++++++++++++++++
>>> >  MAINTAINERS                                   |  1 +
>>> >  2 files changed, 28 insertions(+)
>>> >  create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
>>> >
>>> > diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
>>> > new file mode 100644
>>> > index 000000000000..c42368626199
>>> > --- /dev/null
>>> > +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
>>> > @@ -0,0 +1,27 @@
>>> > +FFI(FDT FIRMWARE INTERFACE) driver
>>> > +
>>> > +Required properties:
>>> > + - entry             : acpi or smbios root pointer, u64
>>> > + - reg                       : acpi or smbios version, u32
>>> > +
>>> > +Some bootloaders, such as Coreboot do not support EFI,
>>> > +only devicetree and some arches do not have a reserved
>>> > +address segment. Add "ffitbl" subnode to obtain ACPI RSDP
>>> > +and SMBIOS entry.
>>>
>>> Since the conversation on this stuff all seems to be going absolutely
>>> nowhere, the ACPI portion of this is intended for use on RISC-V in
>>> violation of the RISC-V ACPI specs. It also goes against the
>>> requirements of the platform spec. Quoting from [1]:
>>>
>>> | > Just so we're all on the same page, I just now asked Mark Himelstein
>>> | > of RISC-V International if there is anything in RISC-V standards that
>>> | > requires UEFI, and the answer is a solid "no."
>>> |
>>> | Huh? Firstly, running off to invoke RVI is not productive - they don't
>>> | maintain the various operating system kernels etc.
>>> | Secondly, that does not seem to be true. The platform spec mandates UEFI
>>> | for the OS-A server platform, alongside ACPI:
>>> | https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#32-boot-process
>>> | and the OS-A embedded platform needs to comply with EBBR & use DT:
>>> | https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#32-boot-process
>>> |
>>> | EBBR does say that systems must not provide both ACPI and DT to the OS
>>> | loader, but I am far from an expert on these kind of things & am not
>>> | sure where something like this where the DT "contains" ACPI would stand.
>>> |
>>> | The RISC-V ACPI spec also says "UEFI firmware is mandatory to support
>>> | ACPI":
>>> | https://github.com/riscv-non-isa/riscv-acpi/blob/master/riscv-acpi-guidance.adoc
>>
>>  UEFI firmware is mandatory to support ACPI and coreboot is an option to support ACPI as well. i think it works well for both, I don't think UEFI and ACPI need to be binding together, each UEFI and ACPI also works well with other solutions.
>
>Thanks for shijian(Nill)'s suggestions.
>
>Hi Conor,
>Thank you very much for your valuable comments on this set of patch
>codes, which are very helpful.
>
>Judging from the current specifications, maybe yes, you can NACK, but
>it's better not to rush to conclusions.

Naks are not permanent, I can remove it in the future if the specs change.

>The so-called specifications represent the ideas of FFI opponents.

"So-called"? They _are_ the specs.

>What we have to do is to discuss with them and get an effective
>consensus, so as to achieve the purpose of updating the specification.

Yes, but that needs to be done on tech-brs, not lkml.

Thanks,
Conor.

>
>>>
>>>
>>>
>>> NAKed-by: Conor Dooley <conor.dooley@microchip.com>
>>>
>>> Cheers,
>>> Conor.
>>>
>>> [1] - https://lore.kernel.org/linux-riscv/20230707-attach-conjuror-306d967347ce@wendy/
>
>Thanks,
>Yunhui

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[Sunil V L]

On Sat, Jul 08, 2023 at 10:45:27AM +0200, Ard Biesheuvel wrote:
> On Fri, 7 Jul 2023 at 18:21, Dong Wei <Dong.Wei@arm.com> wrote:
> >
> > On Arm systems today, the ACPI RSDP is found using the UEFI Configuration Table. This is true for all Arm SystemReady compliant systems: 1) SystemReady LS: LBBRv1 is using a minimal UEFI FW to load LinuxBoot, that minimal UEFI FW is producing the UEFI Configuration Table. We are working on LBBRv2. LBBRv2 is based on Coreboot loading LinuxBoot. But we do not have a way today to get CoreBoot to produce this pointer to ACPI RSDP. Arm does not support x86 E820 BIOS interface. 2) SystemReady IR: this solution uses DT rather than ACPI. 3) SystemReady ES: this solution can use UBoot or EDK2, and it requires ACPI. Since both UBoot and EDK2 support UEFI now, so ACPI RSDP can be found using the UEFI Configuration Table. 4) SystemReady SR: this solution typically uses EDK2 and requires ACPI, so no issue finding RSDP via UEFI Configuration Table.
> >
> >
> >
> > So the ACPI RSDP issue only exist if we want to remove the minimum UEFI FW and go to CoreBoot completely to load LinuxBoot. We are currently exploring how to solve that issue…
> >
> 
> Hello Dong,
> 
> This fixes the RSDP issue perhaps, but that is not the only problem. I
> have mentioned this many times already, but let me mention it again
> for completeness:
> 
> ACPI does not have a memory map, and ARM is much more finicky about
> mapping memory regions with the right attributes, given that uncached
> accesses don't snoop the caches like they do on x86. This means it is
> essential that memory mappings constructed from AML code (which
> doesn't provide any context pertaining to the memory type either) are
> created with the right memory type.
> 
> Currently, the Linux/arm64 glue code for the ACPI core
> cross-references every memory mapping created from a SystemMemory
> OpRegion by AML code against the EFI memory map, and uses the EFI
> memory type and attributes to infer the memory type to program into
> the page tables. So simply providing the RSDP is *not* sufficient: on
> arm64, more work is needed and currently, booting ACPI without a EFI
> memory map results in SystemMemory OpRegions not working at all.
> 
> Of course, we might be able to work around that by providing a
> 'coreboot' memory map for doing ACPI on arm64, but that results in
> more fragmentation and an inflated validation matrix, which puts the
> burden on the Linux subsystem maintainers to make sure that all these
> different combinations remain supported.
> 
> AIUI, this memory type issue does not exist for RISC-V today, but I'd
> suggest to the RISC-V maintainers to take a careful look at arm64's
> acpi_os_ioremap() implementation and decide whether or not RISC-V
> needs similar logic.

Currently, basic ACPI support is merged for RISC-V. Still many features
including external interrupt controller support are WIP. Enhancing the
acpi_os_ioremap() similar to arm64 version is in plan for next series.
Bjorn had also provided this feedback.
https://lore.kernel.org/lkml/87leidtvn9.fsf@all.your.base.are.belong.to.us/

So, the issue will be applicable to RISC-V also even if the
implementation may differ slightly.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[Palmer Dabbelt]

On Sat, 08 Jul 2023 01:45:27 PDT (-0700), Ard Biesheuvel wrote:
> On Fri, 7 Jul 2023 at 18:21, Dong Wei <Dong.Wei@arm.com> wrote:
>>
>> On Arm systems today, the ACPI RSDP is found using the UEFI Configuration Table. This is true for all Arm SystemReady compliant systems: 1) SystemReady LS: LBBRv1 is using a minimal UEFI FW to load LinuxBoot, that minimal UEFI FW is producing the UEFI Configuration Table. We are working on LBBRv2. LBBRv2 is based on Coreboot loading LinuxBoot. But we do not have a way today to get CoreBoot to produce this pointer to ACPI RSDP. Arm does not support x86 E820 BIOS interface. 2) SystemReady IR: this solution uses DT rather than ACPI. 3) SystemReady ES: this solution can use UBoot or EDK2, and it requires ACPI. Since both UBoot and EDK2 support UEFI now, so ACPI RSDP can be found using the UEFI Configuration Table. 4) SystemReady SR: this solution typically uses EDK2 and requires ACPI, so no issue finding RSDP via UEFI Configuration Table.
>>
>>
>>
>> So the ACPI RSDP issue only exist if we want to remove the minimum UEFI FW and go to CoreBoot completely to load LinuxBoot. We are currently exploring how to solve that issue…
>>
>
> Hello Dong,
>
> This fixes the RSDP issue perhaps, but that is not the only problem. I
> have mentioned this many times already, but let me mention it again
> for completeness:
>
> ACPI does not have a memory map, and ARM is much more finicky about
> mapping memory regions with the right attributes, given that uncached
> accesses don't snoop the caches like they do on x86. This means it is
> essential that memory mappings constructed from AML code (which
> doesn't provide any context pertaining to the memory type either) are
> created with the right memory type.
>
> Currently, the Linux/arm64 glue code for the ACPI core
> cross-references every memory mapping created from a SystemMemory
> OpRegion by AML code against the EFI memory map, and uses the EFI
> memory type and attributes to infer the memory type to program into
> the page tables. So simply providing the RSDP is *not* sufficient: on
> arm64, more work is needed and currently, booting ACPI without a EFI
> memory map results in SystemMemory OpRegions not working at all.
>
> Of course, we might be able to work around that by providing a
> 'coreboot' memory map for doing ACPI on arm64, but that results in
> more fragmentation and an inflated validation matrix, which puts the
> burden on the Linux subsystem maintainers to make sure that all these
> different combinations remain supported.
>
> AIUI, this memory type issue does not exist for RISC-V today, but I'd
> suggest to the RISC-V maintainers to take a careful look at arm64's
> acpi_os_ioremap() implementation and decide whether or not RISC-V
> needs similar logic.

We've got a handful of messes around this in RISC-V already, I'd be 
surprised if we don't need anything for ACPI.  It's probably not going 
to show up until we're running on real platforms, though, and we're 
going to need some M-mode interface to get it right in the long run.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

[yunhui cui]

Hi Conor,


On Tue, Jul 11, 2023 at 12:03 AM 葛士建 <geshijian@bytedance.com> wrote:
>
> On Sat, Jul 8, 2023 at 4:45 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Fri, 7 Jul 2023 at 18:21, Dong Wei <Dong.Wei@arm.com> wrote:
> > >
> > > On Arm systems today, the ACPI RSDP is found using the UEFI Configuration Table. This is true for all Arm SystemReady compliant systems: 1) SystemReady LS: LBBRv1 is using a minimal UEFI FW to load LinuxBoot, that minimal UEFI FW is producing the UEFI Configuration Table. We are working on LBBRv2. LBBRv2 is based on Coreboot loading LinuxBoot. But we do not have a way today to get CoreBoot to produce this pointer to ACPI RSDP. Arm does not support x86 E820 BIOS interface. 2) SystemReady IR: this solution uses DT rather than ACPI. 3) SystemReady ES: this solution can use UBoot or EDK2, and it requires ACPI. Since both UBoot and EDK2 support UEFI now, so ACPI RSDP can be found using the UEFI Configuration Table. 4) SystemReady SR: this solution typically uses EDK2 and requires ACPI, so no issue finding RSDP via UEFI Configuration Table.
> > >
> > >
> > >
> > > So the ACPI RSDP issue only exist if we want to remove the minimum UEFI FW and go to CoreBoot completely to load LinuxBoot. We are currently exploring how to solve that issue…
> > >
> >
> > Hello Dong,
> >
> > This fixes the RSDP issue perhaps, but that is not the only problem. I
> > have mentioned this many times already, but let me mention it again
> > for completeness:
> >
> > ACPI does not have a memory map, and ARM is much more finicky about
> > mapping memory regions with the right attributes, given that uncached
> > accesses don't snoop the caches like they do on x86. This means it is
> > essential that memory mappings constructed from AML code (which
> > doesn't provide any context pertaining to the memory type either) are
> > created with the right memory type.
> >
> > Currently, the Linux/arm64 glue code for the ACPI core
> > cross-references every memory mapping created from a SystemMemory
> > OpRegion by AML code against the EFI memory map, and uses the EFI
> > memory type and attributes to infer the memory type to program into
> > the page tables. So simply providing the RSDP is *not* sufficient: on
> > arm64, more work is needed and currently, booting ACPI without a EFI
> > memory map results in SystemMemory OpRegions not working at all.
> >
> > Of course, we might be able to work around that by providing a
> > 'coreboot' memory map for doing ACPI on arm64, but that results in
> > more fragmentation and an inflated validation matrix, which puts the
> > burden on the Linux subsystem maintainers to make sure that all these
> > different combinations remain supported.
> >
> > AIUI, this memory type issue does not exist for RISC-V today, but I'd
> > suggest to the RISC-V maintainers to take a careful look at arm64's
> > acpi_os_ioremap() implementation and decide whether or not RISC-V
> > needs similar logic.
>
> Thanks Ard and Sunil,
>
> You are right, we need to work out a coreboot memory map for doing
> ACPI on ARM64.
> BTW, I don't suggest binding ACPI and UEFI together. On RISC-V,  the
> current solution works well based on our experience, anyway, we will
> study memory with DTS and update later.
>
> Thanks,
> -Nill

Let's move on to this patchset,
Regarding the so-called risc-v spec
(https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#32-boot-process)
that we discussed before, it is outdated , the patchset is not
constrained by the spec.
See:
https://github.com/riscv/riscv-platform-specs/pull/91#issuecomment-1709044215

So please help to review this patchset, thank you!


Thanks,
Yunhui

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv