[U-Boot] [PATCH 0/7] efi_loader: Expose SMBIOS table

[U-Boot] [PATCH 0/7] efi_loader: Expose SMBIOS table

[Alexander Graf]

We generate a few tables on x86 today that really can be used on ARM just
the same. One such example is the SMBIOS table, which people use with tools
like "dmidecode" to identify which hardware they are running on.

We're slowly growing needs to collect serial numbers from various devices
on ARM and SMBIOS seems the natural choice. So this patch set moves the
current SMBIOS generation into generic code and adds serial number exposure
to it.

I have verified that I get a correct serial number printed in dmidecode on
the RPi3.

Alexander Graf (7):
  x86: Move table csum into separate header
  x86: Move smbios generation into arch independent directory
  efi_loader: Expose efi_install_configuration_table
  smbios: Allow compilation on 64bit systems
  smbios: Expose in efi_loader as table
  efi_loader: Fix efi_install_configuration_table
  smbios: Provide serial number

 arch/x86/Kconfig                           | 27 --------------
 arch/x86/include/asm/tables.h              |  2 +
 arch/x86/lib/Makefile                      |  1 -
 arch/x86/lib/tables.c                      | 21 ++++-------
 cmd/bootefi.c                              |  3 ++
 include/efi_api.h                          |  4 ++
 include/efi_loader.h                       |  4 ++
 {arch/x86/include/asm => include}/smbios.h |  5 ++-
 include/tables_csum.h                      | 22 +++++++++++
 lib/Kconfig                                | 33 +++++++++++++++++
 lib/Makefile                               |  1 +
 lib/efi_loader/efi_boottime.c              | 24 +++++++-----
 {arch/x86/lib => lib}/smbios.c             | 59 +++++++++++++++++++++++++-----
 13 files changed, 142 insertions(+), 64 deletions(-)
 rename {arch/x86/include/asm => include}/smbios.h (96%)
 create mode 100644 include/tables_csum.h
 rename {arch/x86/lib => lib}/smbios.c (82%)

-- 
2.6.6

[U-Boot] [PATCH 1/7] x86: Move table csum into separate header

[Alexander Graf]

We need the checksum function without all the other table functionality
soon, so let's split it out into its own header file.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/x86/include/asm/tables.h |  2 ++
 arch/x86/lib/tables.c         | 12 ------------
 include/tables_csum.h         | 22 ++++++++++++++++++++++
 3 files changed, 24 insertions(+), 12 deletions(-)
 create mode 100644 include/tables_csum.h

diff --git a/arch/x86/include/asm/tables.h b/arch/x86/include/asm/tables.h
index ae9f0d0..81f98f2 100644
--- a/arch/x86/include/asm/tables.h
+++ b/arch/x86/include/asm/tables.h
@@ -7,6 +7,8 @@
 #ifndef _X86_TABLES_H_
 #define _X86_TABLES_H_
 
+#include <tables_csum.h>
+
 /*
  * All x86 tables happen to like the address range from 0xf0000 to 0x100000.
  * We use 0xf0000 as the starting address to store those tables, including
diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
index f92111e..9ee6b5e 100644
--- a/arch/x86/lib/tables.c
+++ b/arch/x86/lib/tables.c
@@ -38,18 +38,6 @@ static table_write table_write_funcs[] = {
 #endif
 };
 
-u8 table_compute_checksum(void *v, int len)
-{
-	u8 *bytes = v;
-	u8 checksum = 0;
-	int i;
-
-	for (i = 0; i < len; i++)
-		checksum -= bytes[i];
-
-	return checksum;
-}
-
 void table_fill_string(char *dest, const char *src, size_t n, char pad)
 {
 	int start, len;
diff --git a/include/tables_csum.h b/include/tables_csum.h
new file mode 100644
index 0000000..27d147b
--- /dev/null
+++ b/include/tables_csum.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2015, Bin Meng <bmeng.cn@gmail.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _TABLES_CSUM_H_
+#define _TABLES_CSUM_H_
+
+static inline u8 table_compute_checksum(void *v, int len)
+{
+	u8 *bytes = v;
+	u8 checksum = 0;
+	int i;
+
+	for (i = 0; i < len; i++)
+		checksum -= bytes[i];
+
+	return checksum;
+}
+
+#endif
-- 
2.6.6

[U-Boot] [PATCH 2/7] x86: Move smbios generation into arch independent directory

[Alexander Graf]

We will need the SMBIOS generation function on ARM as well going forward,
so let's move it into a non arch specific location.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/x86/Kconfig                           | 27 ------------------------
 arch/x86/lib/Makefile                      |  1 -
 arch/x86/lib/tables.c                      |  2 +-
 {arch/x86/include/asm => include}/smbios.h |  0
 lib/Kconfig                                | 33 ++++++++++++++++++++++++++++++
 lib/Makefile                               |  1 +
 {arch/x86/lib => lib}/smbios.c             |  4 ++--
 7 files changed, 37 insertions(+), 31 deletions(-)
 rename {arch/x86/include/asm => include}/smbios.h (100%)
 rename {arch/x86/lib => lib}/smbios.c (99%)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 29d1120..682ebb8 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -457,33 +457,6 @@ config GENERATE_ACPI_TABLE
 	  by the operating system. It defines platform-independent interfaces
 	  for configuration and power management monitoring.
 
-config GENERATE_SMBIOS_TABLE
-	bool "Generate an SMBIOS (System Management BIOS) table"
-	default y
-	help
-	  The System Management BIOS (SMBIOS) specification addresses how
-	  motherboard and system vendors present management information about
-	  their products in a standard format by extending the BIOS interface
-	  on Intel architecture systems.
-
-	  Check http://www.dmtf.org/standards/smbios for details.
-
-config SMBIOS_MANUFACTURER
-	string "SMBIOS Manufacturer"
-	depends on GENERATE_SMBIOS_TABLE
-	default SYS_VENDOR
-	help
-	  The board manufacturer to store in SMBIOS structures.
-	  Change this to override the default one (CONFIG_SYS_VENDOR).
-
-config SMBIOS_PRODUCT_NAME
-	string "SMBIOS Product Name"
-	depends on GENERATE_SMBIOS_TABLE
-	default SYS_BOARD
-	help
-	  The product name to store in SMBIOS structures.
-	  Change this to override the default one (CONFIG_SYS_BOARD).
-
 endmenu
 
 config MAX_PIRQ_LINKS
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index e17f0bb..40ea6bf 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -29,7 +29,6 @@ obj-y	+= relocate.o
 obj-y += physmem.o
 obj-$(CONFIG_X86_RAMTEST) += ramtest.o
 obj-y += sfi.o
-obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o
 obj-y	+= string.o
 ifndef CONFIG_QEMU
 obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi_table.o
diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
index 9ee6b5e..e62705a 100644
--- a/arch/x86/lib/tables.c
+++ b/arch/x86/lib/tables.c
@@ -5,9 +5,9 @@
  */
 
 #include <common.h>
+#include <smbios.h>
 #include <asm/sfi.h>
 #include <asm/mpspec.h>
-#include <asm/smbios.h>
 #include <asm/tables.h>
 #include <asm/acpi_table.h>
 #include <asm/coreboot_tables.h>
diff --git a/arch/x86/include/asm/smbios.h b/include/smbios.h
similarity index 100%
rename from arch/x86/include/asm/smbios.h
rename to include/smbios.h
diff --git a/lib/Kconfig b/lib/Kconfig
index 02ca405..5a14530 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -149,6 +149,39 @@ config SPL_OF_LIBFDT
 	  particular compatible nodes. The library operates on a flattened
 	  version of the device tree.
 
+menu "System tables"
+	depends on !EFI && !SYS_COREBOOT
+
+config GENERATE_SMBIOS_TABLE
+	bool "Generate an SMBIOS (System Management BIOS) table"
+	default y
+	depends on X86
+	help
+	  The System Management BIOS (SMBIOS) specification addresses how
+	  motherboard and system vendors present management information about
+	  their products in a standard format by extending the BIOS interface
+	  on Intel architecture systems.
+
+	  Check http://www.dmtf.org/standards/smbios for details.
+
+config SMBIOS_MANUFACTURER
+	string "SMBIOS Manufacturer"
+	depends on GENERATE_SMBIOS_TABLE
+	default SYS_VENDOR
+	help
+	  The board manufacturer to store in SMBIOS structures.
+	  Change this to override the default one (CONFIG_SYS_VENDOR).
+
+config SMBIOS_PRODUCT_NAME
+	string "SMBIOS Product Name"
+	depends on GENERATE_SMBIOS_TABLE
+	default SYS_BOARD
+	help
+	  The product name to store in SMBIOS structures.
+	  Change this to override the default one (CONFIG_SYS_BOARD).
+
+endmenu
+
 source lib/efi/Kconfig
 source lib/efi_loader/Kconfig
 
diff --git a/lib/Makefile b/lib/Makefile
index f6a8ba1..8801b8e 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_FIT) += fdtdec_common.o
 obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o
 obj-$(CONFIG_GZIP) += gunzip.o
 obj-$(CONFIG_GZIP_COMPRESSED) += gzip.o
+obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o
 obj-y += initcall.o
 obj-$(CONFIG_LMB) += lmb.o
 obj-y += ldiv.o
diff --git a/arch/x86/lib/smbios.c b/lib/smbios.c
similarity index 99%
rename from arch/x86/lib/smbios.c
rename to lib/smbios.c
index 9f30550..9808ee7 100644
--- a/arch/x86/lib/smbios.c
+++ b/lib/smbios.c
@@ -7,10 +7,10 @@
  */
 
 #include <common.h>
+#include <smbios.h>
+#include <tables_csum.h>
 #include <version.h>
 #include <asm/cpu.h>
-#include <asm/smbios.h>
-#include <asm/tables.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
-- 
2.6.6

[U-Boot] [PATCH 3/7] efi_loader: Expose efi_install_configuration_table

[Alexander Graf]

We want to be able to add configuration table entries from our own code as
well as from EFI payload code. Export the boot service function internally
too, so that we can reuse it.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 include/efi_loader.h          |  2 ++
 lib/efi_loader/efi_boottime.c | 22 +++++++++++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 91d6a84..ac8b77a 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -130,6 +130,8 @@ uint64_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type,
 			    bool overlap_only_ram);
 /* Called by board init to initialize the EFI memory map */
 int efi_memory_init(void);
+/* Adds new or overrides configuration table entry to the system table */
+efi_status_t efi_install_configuration_table(efi_guid_t *guid, void *table);
 
 #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
 extern void *efi_bounce_buffer;
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index be6f5e8..3e7a48b 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -37,7 +37,7 @@ static bool efi_is_direct_boot = true;
  * In most cases we want to pass an FDT to the payload, so reserve one slot of
  * config table space for it. The pointer gets populated by do_bootefi_exec().
  */
-static struct efi_configuration_table EFI_RUNTIME_DATA efi_conf_table[1];
+static struct efi_configuration_table EFI_RUNTIME_DATA efi_conf_table[2];
 
 /*
  * The "gd" pointer lives in a register on ARM and AArch64 that we declare
@@ -375,31 +375,35 @@ static efi_status_t EFIAPI efi_locate_device_path(efi_guid_t *protocol,
 	return EFI_EXIT(EFI_NOT_FOUND);
 }
 
-static efi_status_t EFIAPI efi_install_configuration_table(efi_guid_t *guid,
-							   void *table)
+efi_status_t efi_install_configuration_table(efi_guid_t *guid, void *table)
 {
 	int i;
 
-	EFI_ENTRY("%p, %p", guid, table);
-
 	/* Check for guid override */
 	for (i = 0; i < systab.nr_tables; i++) {
 		if (!guidcmp(guid, &efi_conf_table[i].guid)) {
 			efi_conf_table[i].table = table;
-			return EFI_EXIT(EFI_SUCCESS);
+			return EFI_SUCCESS;
 		}
 	}
 
 	/* No override, check for overflow */
 	if (i >= ARRAY_SIZE(efi_conf_table))
-		return EFI_EXIT(EFI_OUT_OF_RESOURCES);
+		return EFI_OUT_OF_RESOURCES;
 
 	/* Add a new entry */
 	memcpy(&efi_conf_table[i].guid, guid, sizeof(*guid));
 	efi_conf_table[i].table = table;
 	systab.nr_tables = i;
 
-	return EFI_EXIT(EFI_SUCCESS);
+	return EFI_SUCCESS;
+}
+
+static efi_status_t EFIAPI efi_install_configuration_table_ext(efi_guid_t *guid,
+							       void *table)
+{
+	EFI_ENTRY("%p, %p", guid, table);
+	return EFI_EXIT(efi_install_configuration_table(guid, table));
 }
 
 static efi_status_t EFIAPI efi_load_image(bool boot_policy,
@@ -750,7 +754,7 @@ static const struct efi_boot_services efi_boot_services = {
 	.register_protocol_notify = efi_register_protocol_notify,
 	.locate_handle = efi_locate_handle,
 	.locate_device_path = efi_locate_device_path,
-	.install_configuration_table = efi_install_configuration_table,
+	.install_configuration_table = efi_install_configuration_table_ext,
 	.load_image = efi_load_image,
 	.start_image = efi_start_image,
 	.exit = efi_exit,
-- 
2.6.6

[U-Boot] [PATCH 4/7] smbios: Allow compilation on 64bit systems

[Alexander Graf]

The SMBIOS generation code passes pointers as u32. That causes the compiler
to warn on casts to pointers. This patch moves all address pointers to
uintptr_t instead.

Technically u32 would be enough for the current SMBIOS2 style tables, but
we may want to extend the code to SMBIOS3 in the future which is 64bit
address capable.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/x86/lib/tables.c |  7 ++++++-
 include/smbios.h      |  4 ++--
 lib/smbios.c          | 16 ++++++++--------
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
index e62705a..025b183 100644
--- a/arch/x86/lib/tables.c
+++ b/arch/x86/lib/tables.c
@@ -12,6 +12,11 @@
 #include <asm/acpi_table.h>
 #include <asm/coreboot_tables.h>
 
+static u32 write_smbios_table_wrapper(u32 addr)
+{
+	return write_smbios_table(addr);
+}
+
 /**
  * Function prototype to write a specific configuration table
  *
@@ -34,7 +39,7 @@ static table_write table_write_funcs[] = {
 	write_acpi_tables,
 #endif
 #ifdef CONFIG_GENERATE_SMBIOS_TABLE
-	write_smbios_table,
+	write_smbios_table_wrapper,
 #endif
 };
 
diff --git a/include/smbios.h b/include/smbios.h
index 623a703..5962d4c 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -221,7 +221,7 @@ static inline void fill_smbios_header(void *table, int type,
  * @handle:	the structure's handle, a unique 16-bit number
  * @return:	size of the structure
  */
-typedef int (*smbios_write_type)(u32 *addr, int handle);
+typedef int (*smbios_write_type)(uintptr_t *addr, int handle);
 
 /**
  * write_smbios_table() - Write SMBIOS table
@@ -231,6 +231,6 @@ typedef int (*smbios_write_type)(u32 *addr, int handle);
  * @addr:	start address to write SMBIOS table
  * @return:	end address of SMBIOS table
  */
-u32 write_smbios_table(u32 addr);
+uintptr_t write_smbios_table(uintptr_t addr);
 
 #endif /* _SMBIOS_H_ */
diff --git a/lib/smbios.c b/lib/smbios.c
index 9808ee7..8dfd486 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -69,7 +69,7 @@ static int smbios_string_table_len(char *start)
 	return len + 1;
 }
 
-static int smbios_write_type0(u32 *current, int handle)
+static int smbios_write_type0(uintptr_t *current, int handle)
 {
 	struct smbios_type0 *t = (struct smbios_type0 *)*current;
 	int len = sizeof(struct smbios_type0);
@@ -98,7 +98,7 @@ static int smbios_write_type0(u32 *current, int handle)
 	return len;
 }
 
-static int smbios_write_type1(u32 *current, int handle)
+static int smbios_write_type1(uintptr_t *current, int handle)
 {
 	struct smbios_type1 *t = (struct smbios_type1 *)*current;
 	int len = sizeof(struct smbios_type1);
@@ -114,7 +114,7 @@ static int smbios_write_type1(u32 *current, int handle)
 	return len;
 }
 
-static int smbios_write_type2(u32 *current, int handle)
+static int smbios_write_type2(uintptr_t *current, int handle)
 {
 	struct smbios_type2 *t = (struct smbios_type2 *)*current;
 	int len = sizeof(struct smbios_type2);
@@ -132,7 +132,7 @@ static int smbios_write_type2(u32 *current, int handle)
 	return len;
 }
 
-static int smbios_write_type3(u32 *current, int handle)
+static int smbios_write_type3(uintptr_t *current, int handle)
 {
 	struct smbios_type3 *t = (struct smbios_type3 *)*current;
 	int len = sizeof(struct smbios_type3);
@@ -152,7 +152,7 @@ static int smbios_write_type3(u32 *current, int handle)
 	return len;
 }
 
-static int smbios_write_type4(u32 *current, int handle)
+static int smbios_write_type4(uintptr_t *current, int handle)
 {
 	struct smbios_type4 *t = (struct smbios_type4 *)*current;
 	int len = sizeof(struct smbios_type4);
@@ -185,7 +185,7 @@ static int smbios_write_type4(u32 *current, int handle)
 	return len;
 }
 
-static int smbios_write_type32(u32 *current, int handle)
+static int smbios_write_type32(uintptr_t *current, int handle)
 {
 	struct smbios_type32 *t = (struct smbios_type32 *)*current;
 	int len = sizeof(struct smbios_type32);
@@ -198,7 +198,7 @@ static int smbios_write_type32(u32 *current, int handle)
 	return len;
 }
 
-static int smbios_write_type127(u32 *current, int handle)
+static int smbios_write_type127(uintptr_t *current, int handle)
 {
 	struct smbios_type127 *t = (struct smbios_type127 *)*current;
 	int len = sizeof(struct smbios_type127);
@@ -221,7 +221,7 @@ static smbios_write_type smbios_write_funcs[] = {
 	smbios_write_type127
 };
 
-u32 write_smbios_table(u32 addr)
+uintptr_t write_smbios_table(uintptr_t addr)
 {
 	struct smbios_entry *se;
 	u32 tables;
-- 
2.6.6

[U-Boot] [PATCH 5/7] smbios: Expose in efi_loader as table

[Alexander Graf]

We can pass SMBIOS easily as EFI configuration table to an EFI payload. This
patch adds enablement for that case.

While at it, we also enable SMBIOS generation for ARM systems, since they support
EFI_LOADER.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 cmd/bootefi.c        |  3 +++
 include/efi_api.h    |  4 ++++
 include/efi_loader.h |  2 ++
 include/smbios.h     |  1 +
 lib/Kconfig          |  4 ++--
 lib/smbios.c         | 36 ++++++++++++++++++++++++++++++++++++
 6 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 53a6ee3..e241b7d 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -205,6 +205,9 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
 	if (!memcmp(bootefi_device_path[0].str, "N\0e\0t", 6))
 		loaded_image_info.device_handle = nethandle;
 #endif
+#ifdef CONFIG_GENERATE_SMBIOS_TABLE
+	efi_smbios_register();
+#endif
 
 	/* Initialize EFI runtime services */
 	efi_reset_system_init();
diff --git a/include/efi_api.h b/include/efi_api.h
index f572b88..bdb600e 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -201,6 +201,10 @@ struct efi_runtime_services {
 	EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, \
 		 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0)
 
+#define SMBIOS_TABLE_GUID \
+	EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3,  \
+		 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
+
 struct efi_configuration_table
 {
 	efi_guid_t guid;
diff --git a/include/efi_loader.h b/include/efi_loader.h
index ac8b77a..b0e8a7f 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -85,6 +85,8 @@ int efi_disk_register(void);
 int efi_gop_register(void);
 /* Called by bootefi to make the network interface available */
 int efi_net_register(void **handle);
+/* Called by bootefi to make SMBIOS tables available */
+void efi_smbios_register(void);
 
 /* Called by networking code to memorize the dhcp ack package */
 void efi_net_set_dhcp_ack(void *pkt, int len);
diff --git a/include/smbios.h b/include/smbios.h
index 5962d4c..fb6396a 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -55,6 +55,7 @@ struct __packed smbios_entry {
 #define BIOS_CHARACTERISTICS_SELECTABLE_BOOT	(1 << 16)
 
 #define BIOS_CHARACTERISTICS_EXT1_ACPI		(1 << 0)
+#define BIOS_CHARACTERISTICS_EXT1_UEFI		(1 << 3)
 #define BIOS_CHARACTERISTICS_EXT2_TARGET	(1 << 2)
 
 struct __packed smbios_type0 {
diff --git a/lib/Kconfig b/lib/Kconfig
index 5a14530..d7f75fe 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -150,12 +150,12 @@ config SPL_OF_LIBFDT
 	  version of the device tree.
 
 menu "System tables"
-	depends on !EFI && !SYS_COREBOOT
+	depends on (!EFI && !SYS_COREBOOT) || (ARM && EFI_LOADER)
 
 config GENERATE_SMBIOS_TABLE
 	bool "Generate an SMBIOS (System Management BIOS) table"
 	default y
-	depends on X86
+	depends on X86 || EFI_LOADER
 	help
 	  The System Management BIOS (SMBIOS) specification addresses how
 	  motherboard and system vendors present management information about
diff --git a/lib/smbios.c b/lib/smbios.c
index 8dfd486..b9aa741 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -7,10 +7,13 @@
  */
 
 #include <common.h>
+#include <efi_loader.h>
 #include <smbios.h>
 #include <tables_csum.h>
 #include <version.h>
+#ifdef CONFIG_X86
 #include <asm/cpu.h>
+#endif
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -79,14 +82,20 @@ static int smbios_write_type0(uintptr_t *current, int handle)
 	t->vendor = smbios_add_string(t->eos, "U-Boot");
 	t->bios_ver = smbios_add_string(t->eos, PLAIN_VERSION);
 	t->bios_release_date = smbios_add_string(t->eos, U_BOOT_DMI_DATE);
+#ifdef CONFIG_ROM_SIZE
 	t->bios_rom_size = (CONFIG_ROM_SIZE / 65536) - 1;
+#endif
 	t->bios_characteristics = BIOS_CHARACTERISTICS_PCI_SUPPORTED |
 				  BIOS_CHARACTERISTICS_SELECTABLE_BOOT |
 				  BIOS_CHARACTERISTICS_UPGRADEABLE;
 #ifdef CONFIG_GENERATE_ACPI_TABLE
 	t->bios_characteristics_ext1 = BIOS_CHARACTERISTICS_EXT1_ACPI;
 #endif
+#ifdef CONFIG_EFI_LOADER
+	t->bios_characteristics_ext1 |= BIOS_CHARACTERISTICS_EXT1_UEFI;
+#endif
 	t->bios_characteristics_ext2 = BIOS_CHARACTERISTICS_EXT2_TARGET;
+
 	t->bios_major_release = 0xff;
 	t->bios_minor_release = 0xff;
 	t->ec_major_release = 0xff;
@@ -152,6 +161,7 @@ static int smbios_write_type3(uintptr_t *current, int handle)
 	return len;
 }
 
+#ifdef CONFIG_X86
 static int smbios_write_type4(uintptr_t *current, int handle)
 {
 	struct smbios_type4 *t = (struct smbios_type4 *)*current;
@@ -184,6 +194,7 @@ static int smbios_write_type4(uintptr_t *current, int handle)
 
 	return len;
 }
+#endif
 
 static int smbios_write_type32(uintptr_t *current, int handle)
 {
@@ -216,7 +227,9 @@ static smbios_write_type smbios_write_funcs[] = {
 	smbios_write_type1,
 	smbios_write_type2,
 	smbios_write_type3,
+#ifdef CONFIG_X86
 	smbios_write_type4,
+#endif
 	smbios_write_type32,
 	smbios_write_type127
 };
@@ -267,3 +280,26 @@ uintptr_t write_smbios_table(uintptr_t addr)
 
 	return addr;
 }
+
+#ifdef CONFIG_EFI_LOADER
+
+void efi_smbios_register(void)
+{
+	static efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;
+	/* Map within the low 32 bits, to allow for 32bit SMBIOS tables */
+        uint64_t dmi = 0xffffffff;
+	/* Reserve 4kb for SMBIOS */
+	uint64_t pages = 1;
+	int memtype = EFI_RUNTIME_SERVICES_DATA;
+
+	if (efi_allocate_pages(1, memtype, pages, &dmi) != EFI_SUCCESS)
+		return;
+
+	/* Generate SMBIOS tables */
+	write_smbios_table(dmi);
+
+	/* And expose them to our EFI payload */
+	efi_install_configuration_table(&smbios_guid, (void*)dmi);
+}
+
+#endif
-- 
2.6.6

[U-Boot] [PATCH 6/7] efi_loader: Fix efi_install_configuration_table

[Alexander Graf]

So far we were only installing the FDT table and didn't have space
to store any other. Hence nobody realized that our efi table allocation
was broken in that it didn't set the indicator for the number of tables
plus one.

This patch fixes it, allowing code to allocate new efi tables.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 lib/efi_loader/efi_boottime.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 3e7a48b..4d0e077 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -394,7 +394,7 @@ efi_status_t efi_install_configuration_table(efi_guid_t *guid, void *table)
 	/* Add a new entry */
 	memcpy(&efi_conf_table[i].guid, guid, sizeof(*guid));
 	efi_conf_table[i].table = table;
-	systab.nr_tables = i;
+	systab.nr_tables = i + 1;
 
 	return EFI_SUCCESS;
 }
-- 
2.6.6

[U-Boot] [PATCH 7/7] smbios: Provide serial number

[Alexander Graf]

If the system has a valid "serial#" environment variable set (which boards that
can find it out programatically set automatically), use that as input for the
serial number field in the SMBIOS tables.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 lib/smbios.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/smbios.c b/lib/smbios.c
index b9aa741..e8befdf 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -111,11 +111,14 @@ static int smbios_write_type1(uintptr_t *current, int handle)
 {
 	struct smbios_type1 *t = (struct smbios_type1 *)*current;
 	int len = sizeof(struct smbios_type1);
+	char *serial_str = getenv("serial#");
 
 	memset(t, 0, sizeof(struct smbios_type1));
 	fill_smbios_header(t, SMBIOS_SYSTEM_INFORMATION, len, handle);
 	t->manufacturer = smbios_add_string(t->eos, CONFIG_SMBIOS_MANUFACTURER);
 	t->product_name = smbios_add_string(t->eos, CONFIG_SMBIOS_PRODUCT_NAME);
+	if (serial_str)
+		t->serial_number = smbios_add_string(t->eos, serial_str);
 
 	len = t->length + smbios_string_table_len(t->eos);
 	*current += len;
-- 
2.6.6

[U-Boot] [PATCH 0/7] efi_loader: Expose SMBIOS table

[Simon Glass]

Hi Alexander,

On 8 August 2016 at 08:06, Alexander Graf <agraf@suse.de> wrote:
> We generate a few tables on x86 today that really can be used on ARM just
> the same. One such example is the SMBIOS table, which people use with tools
> like "dmidecode" to identify which hardware they are running on.
>
> We're slowly growing needs to collect serial numbers from various devices
> on ARM and SMBIOS seems the natural choice. So this patch set moves the
> current SMBIOS generation into generic code and adds serial number exposure
> to it.

Shouldn't we use device tree? Why would an ARM device use SMBIOS?

>
> I have verified that I get a correct serial number printed in dmidecode on
> the RPi3.
>
> Alexander Graf (7):
>   x86: Move table csum into separate header
>   x86: Move smbios generation into arch independent directory
>   efi_loader: Expose efi_install_configuration_table
>   smbios: Allow compilation on 64bit systems
>   smbios: Expose in efi_loader as table
>   efi_loader: Fix efi_install_configuration_table
>   smbios: Provide serial number
>
>  arch/x86/Kconfig                           | 27 --------------
>  arch/x86/include/asm/tables.h              |  2 +
>  arch/x86/lib/Makefile                      |  1 -
>  arch/x86/lib/tables.c                      | 21 ++++-------
>  cmd/bootefi.c                              |  3 ++
>  include/efi_api.h                          |  4 ++
>  include/efi_loader.h                       |  4 ++
>  {arch/x86/include/asm => include}/smbios.h |  5 ++-
>  include/tables_csum.h                      | 22 +++++++++++
>  lib/Kconfig                                | 33 +++++++++++++++++
>  lib/Makefile                               |  1 +
>  lib/efi_loader/efi_boottime.c              | 24 +++++++-----
>  {arch/x86/lib => lib}/smbios.c             | 59 +++++++++++++++++++++++++-----
>  13 files changed, 142 insertions(+), 64 deletions(-)
>  rename {arch/x86/include/asm => include}/smbios.h (96%)
>  create mode 100644 include/tables_csum.h
>  rename {arch/x86/lib => lib}/smbios.c (82%)
>
> --
> 2.6.6
>


Regards,
Simon

[U-Boot] [PATCH 0/7] efi_loader: Expose SMBIOS table

[Bin Meng]

Hi Simon,

On Tue, Aug 9, 2016 at 5:44 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Alexander,
>
> On 8 August 2016 at 08:06, Alexander Graf <agraf@suse.de> wrote:
>> We generate a few tables on x86 today that really can be used on ARM just
>> the same. One such example is the SMBIOS table, which people use with tools
>> like "dmidecode" to identify which hardware they are running on.
>>
>> We're slowly growing needs to collect serial numbers from various devices
>> on ARM and SMBIOS seems the natural choice. So this patch set moves the
>> current SMBIOS generation into generic code and adds serial number exposure
>> to it.
>
> Shouldn't we use device tree? Why would an ARM device use SMBIOS?
>

I believe the reason of using SMBIOS on ARM is with U-Boot's bootefi support.
https://wiki.linaro.org/ARM/UEFI/SMBIOS

Regards,
Bin

[U-Boot] [PATCH 0/7] efi_loader: Expose SMBIOS table

[Alexander Graf]

> Am 08.08.2016 um 23:44 schrieb Simon Glass <sjg@chromium.org>:
> 
> Hi Alexander,
> 
>> On 8 August 2016 at 08:06, Alexander Graf <agraf@suse.de> wrote:
>> We generate a few tables on x86 today that really can be used on ARM just
>> the same. One such example is the SMBIOS table, which people use with tools
>> like "dmidecode" to identify which hardware they are running on.
>> 
>> We're slowly growing needs to collect serial numbers from various devices
>> on ARM and SMBIOS seems the natural choice. So this patch set moves the
>> current SMBIOS generation into generic code and adds serial number exposure
>> to it.
> 
> Shouldn't we use device tree? Why would an ARM device use SMBIOS?

Mostly because SBBR dictates it and every ARM server platform out there provides SMBIOS tables ;).

Also, both describe very different things. At least I have never seen things like "The chassy of this server has 2 power connectors and is blue" in device tree.


Alex

[U-Boot] [PATCH 1/7] x86: Move table csum into separate header

[Bin Meng]

On Mon, Aug 8, 2016 at 10:06 PM, Alexander Graf <agraf@suse.de> wrote:
> We need the checksum function without all the other table functionality
> soon, so let's split it out into its own header file.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/x86/include/asm/tables.h |  2 ++
>  arch/x86/lib/tables.c         | 12 ------------
>  include/tables_csum.h         | 22 ++++++++++++++++++++++
>  3 files changed, 24 insertions(+), 12 deletions(-)
>  create mode 100644 include/tables_csum.h
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH 2/7] x86: Move smbios generation into arch independent directory

[Bin Meng]

On Mon, Aug 8, 2016 at 10:06 PM, Alexander Graf <agraf@suse.de> wrote:
> We will need the SMBIOS generation function on ARM as well going forward,
> so let's move it into a non arch specific location.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/x86/Kconfig                           | 27 ------------------------
>  arch/x86/lib/Makefile                      |  1 -
>  arch/x86/lib/tables.c                      |  2 +-
>  {arch/x86/include/asm => include}/smbios.h |  0
>  lib/Kconfig                                | 33 ++++++++++++++++++++++++++++++
>  lib/Makefile                               |  1 +
>  {arch/x86/lib => lib}/smbios.c             |  4 ++--
>  7 files changed, 37 insertions(+), 31 deletions(-)
>  rename {arch/x86/include/asm => include}/smbios.h (100%)
>  rename {arch/x86/lib => lib}/smbios.c (99%)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH 4/7] smbios: Allow compilation on 64bit systems

[Bin Meng]

On Mon, Aug 8, 2016 at 10:06 PM, Alexander Graf <agraf@suse.de> wrote:
> The SMBIOS generation code passes pointers as u32. That causes the compiler
> to warn on casts to pointers. This patch moves all address pointers to
> uintptr_t instead.
>
> Technically u32 would be enough for the current SMBIOS2 style tables, but
> we may want to extend the code to SMBIOS3 in the future which is 64bit
> address capable.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/x86/lib/tables.c |  7 ++++++-
>  include/smbios.h      |  4 ++--
>  lib/smbios.c          | 16 ++++++++--------
>  3 files changed, 16 insertions(+), 11 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH 5/7] smbios: Expose in efi_loader as table

[Bin Meng]

Hi Alexander,

On Mon, Aug 8, 2016 at 10:06 PM, Alexander Graf <agraf@suse.de> wrote:
> We can pass SMBIOS easily as EFI configuration table to an EFI payload. This
> patch adds enablement for that case.
>
> While at it, we also enable SMBIOS generation for ARM systems, since they support
> EFI_LOADER.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  cmd/bootefi.c        |  3 +++
>  include/efi_api.h    |  4 ++++
>  include/efi_loader.h |  2 ++
>  include/smbios.h     |  1 +
>  lib/Kconfig          |  4 ++--
>  lib/smbios.c         | 36 ++++++++++++++++++++++++++++++++++++
>  6 files changed, 48 insertions(+), 2 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

One nits below:

> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 53a6ee3..e241b7d 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -205,6 +205,9 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
>         if (!memcmp(bootefi_device_path[0].str, "N\0e\0t", 6))
>                 loaded_image_info.device_handle = nethandle;
>  #endif
> +#ifdef CONFIG_GENERATE_SMBIOS_TABLE
> +       efi_smbios_register();
> +#endif
>

[snip]

>
>  static int smbios_write_type32(uintptr_t *current, int handle)
>  {
> @@ -216,7 +227,9 @@ static smbios_write_type smbios_write_funcs[] = {
>         smbios_write_type1,
>         smbios_write_type2,
>         smbios_write_type3,
> +#ifdef CONFIG_X86
>         smbios_write_type4,
> +#endif
>         smbios_write_type32,
>         smbios_write_type127
>  };
> @@ -267,3 +280,26 @@ uintptr_t write_smbios_table(uintptr_t addr)
>
>         return addr;
>  }
> +
> +#ifdef CONFIG_EFI_LOADER
> +
> +void efi_smbios_register(void)
> +{
> +       static efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;
> +       /* Map within the low 32 bits, to allow for 32bit SMBIOS tables */
> +        uint64_t dmi = 0xffffffff;

nits: one space before uint64_t

> +       /* Reserve 4kb for SMBIOS */
> +       uint64_t pages = 1;
> +       int memtype = EFI_RUNTIME_SERVICES_DATA;
> +
> +       if (efi_allocate_pages(1, memtype, pages, &dmi) != EFI_SUCCESS)
> +               return;
> +
> +       /* Generate SMBIOS tables */
> +       write_smbios_table(dmi);
> +
> +       /* And expose them to our EFI payload */
> +       efi_install_configuration_table(&smbios_guid, (void*)dmi);
> +}
> +
> +#endif
> --

Regards,
Bin

[U-Boot] [PATCH 7/7] smbios: Provide serial number

[Bin Meng]

On Mon, Aug 8, 2016 at 10:06 PM, Alexander Graf <agraf@suse.de> wrote:
> If the system has a valid "serial#" environment variable set (which boards that
> can find it out programatically set automatically), use that as input for the
> serial number field in the SMBIOS tables.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  lib/smbios.c | 3 +++
>  1 file changed, 3 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH 0/7] efi_loader: Expose SMBIOS table

[Simon Glass]

Hi Alexander,

On 9 August 2016 at 00:48, Alexander Graf <agraf@suse.de> wrote:
>
>
>> Am 08.08.2016 um 23:44 schrieb Simon Glass <sjg@chromium.org>:
>>
>> Hi Alexander,
>>
>>> On 8 August 2016 at 08:06, Alexander Graf <agraf@suse.de> wrote:
>>> We generate a few tables on x86 today that really can be used on ARM just
>>> the same. One such example is the SMBIOS table, which people use with tools
>>> like "dmidecode" to identify which hardware they are running on.
>>>
>>> We're slowly growing needs to collect serial numbers from various devices
>>> on ARM and SMBIOS seems the natural choice. So this patch set moves the
>>> current SMBIOS generation into generic code and adds serial number exposure
>>> to it.
>>
>> Shouldn't we use device tree? Why would an ARM device use SMBIOS?
>
> Mostly because SBBR dictates it and every ARM server platform out there provides SMBIOS tables ;).
>
> Also, both describe very different things. At least I have never seen things like "The chassy of this server has 2 power connectors and is blue" in device tree.

So there is no DT binding for this information? Does this mean that
U-Boot on ARM needs to pass information through just 'sitting in RAM
somewhere' like x86?

Regards,
Simon

[U-Boot] [PATCH 0/7] efi_loader: Expose SMBIOS table

[Alexander Graf]

On 08/09/2016 03:57 PM, Simon Glass wrote:
> Hi Alexander,
>
> On 9 August 2016 at 00:48, Alexander Graf <agraf@suse.de> wrote:
>>
>>> Am 08.08.2016 um 23:44 schrieb Simon Glass <sjg@chromium.org>:
>>>
>>> Hi Alexander,
>>>
>>>> On 8 August 2016 at 08:06, Alexander Graf <agraf@suse.de> wrote:
>>>> We generate a few tables on x86 today that really can be used on ARM just
>>>> the same. One such example is the SMBIOS table, which people use with tools
>>>> like "dmidecode" to identify which hardware they are running on.
>>>>
>>>> We're slowly growing needs to collect serial numbers from various devices
>>>> on ARM and SMBIOS seems the natural choice. So this patch set moves the
>>>> current SMBIOS generation into generic code and adds serial number exposure
>>>> to it.
>>> Shouldn't we use device tree? Why would an ARM device use SMBIOS?
>> Mostly because SBBR dictates it and every ARM server platform out there provides SMBIOS tables ;).
>>
>> Also, both describe very different things. At least I have never seen things like "The chassy of this server has 2 power connectors and is blue" in device tree.
> So there is no DT binding for this information? Does this mean that
> U-Boot on ARM needs to pass information through just 'sitting in RAM
> somewhere' like x86?

I don't think there's a non-EFI way on ARM to pass this through at all, 
correct. That's nothing new though - things like KASLR also depend on EFI.


Alex

[U-Boot] [PATCH 0/7] efi_loader: Expose SMBIOS table

[Simon Glass]

Hi Alexander,

On 9 August 2016 at 08:11, Alexander Graf <agraf@suse.de> wrote:
>
> On 08/09/2016 03:57 PM, Simon Glass wrote:
>>
>> Hi Alexander,
>>
>> On 9 August 2016 at 00:48, Alexander Graf <agraf@suse.de> wrote:
>>>
>>>
>>>> Am 08.08.2016 um 23:44 schrieb Simon Glass <sjg@chromium.org>:
>>>>
>>>> Hi Alexander,
>>>>
>>>>> On 8 August 2016 at 08:06, Alexander Graf <agraf@suse.de> wrote:
>>>>> We generate a few tables on x86 today that really can be used on ARM just
>>>>> the same. One such example is the SMBIOS table, which people use with tools
>>>>> like "dmidecode" to identify which hardware they are running on.
>>>>>
>>>>> We're slowly growing needs to collect serial numbers from various devices
>>>>> on ARM and SMBIOS seems the natural choice. So this patch set moves the
>>>>> current SMBIOS generation into generic code and adds serial number exposure
>>>>> to it.
>>>>
>>>> Shouldn't we use device tree? Why would an ARM device use SMBIOS?
>>>
>>> Mostly because SBBR dictates it and every ARM server platform out there provides SMBIOS tables ;).
>>>
>>> Also, both describe very different things. At least I have never seen things like "The chassy of this server has 2 power connectors and is blue" in device tree.
>>
>> So there is no DT binding for this information? Does this mean that
>> U-Boot on ARM needs to pass information through just 'sitting in RAM
>> somewhere' like x86?
>
>
> I don't think there's a non-EFI way on ARM to pass this through at all, correct. That's nothing new though - things like KASLR also depend on EFI.

That feels like it could just be done in U-Boot. The proliferation of
bootloader stuff in Linux bugs me. It's the lowest-common-denominator
problem. To me it seems that UEFI and U-Boot should implement these
features, and then Linux doesn't need to.

Anyway I do understand where you are going. Is it possible to provide
EFI tables to Linux but not EFI boot/run-time services?

Regards,
Simon

[U-Boot] [PATCH 0/7] efi_loader: Expose SMBIOS table

[Alexander Graf]

> On 09 Aug 2016, at 16:35, Simon Glass <sjg@chromium.org> wrote:
> 
> Hi Alexander,
> 
> On 9 August 2016 at 08:11, Alexander Graf <agraf@suse.de> wrote:
>> 
>> On 08/09/2016 03:57 PM, Simon Glass wrote:
>>> 
>>> Hi Alexander,
>>> 
>>> On 9 August 2016 at 00:48, Alexander Graf <agraf@suse.de> wrote:
>>>> 
>>>> 
>>>>> Am 08.08.2016 um 23:44 schrieb Simon Glass <sjg@chromium.org>:
>>>>> 
>>>>> Hi Alexander,
>>>>> 
>>>>>> On 8 August 2016 at 08:06, Alexander Graf <agraf@suse.de> wrote:
>>>>>> We generate a few tables on x86 today that really can be used on ARM just
>>>>>> the same. One such example is the SMBIOS table, which people use with tools
>>>>>> like "dmidecode" to identify which hardware they are running on.
>>>>>> 
>>>>>> We're slowly growing needs to collect serial numbers from various devices
>>>>>> on ARM and SMBIOS seems the natural choice. So this patch set moves the
>>>>>> current SMBIOS generation into generic code and adds serial number exposure
>>>>>> to it.
>>>>> 
>>>>> Shouldn't we use device tree? Why would an ARM device use SMBIOS?
>>>> 
>>>> Mostly because SBBR dictates it and every ARM server platform out there provides SMBIOS tables ;).
>>>> 
>>>> Also, both describe very different things. At least I have never seen things like "The chassy of this server has 2 power connectors and is blue" in device tree.
>>> 
>>> So there is no DT binding for this information? Does this mean that
>>> U-Boot on ARM needs to pass information through just 'sitting in RAM
>>> somewhere' like x86?
>> 
>> 
>> I don't think there's a non-EFI way on ARM to pass this through at all, correct. That's nothing new though - things like KASLR also depend on EFI.
> 
> That feels like it could just be done in U-Boot. The proliferation of
> bootloader stuff in Linux bugs me. It's the lowest-common-denominator
> problem. To me it seems that UEFI and U-Boot should implement these
> features, and then Linux doesn't need to.
> 
> Anyway I do understand where you are going. Is it possible to provide
> EFI tables to Linux but not EFI boot/run-time services?

I?m not aware of any way to do that today unfortunately. I don?t see why you couldn?t add a device tree property that points to random memory locations for tables though.

As a more fundamental question though, will we really care about the non-EFI boot path 5 or 10 years from now? It doesn?t add much complexity and allows for less divergence between classic embedded and server systems. Since Linux itself is a uEFI binary, you can simply replace ?booti? with ?bootefi? in most boot cases and shouldn?t even see much of a difference.

I don?t think that the non-EFI boot path will disappear, but turns more and more into a second class citizen because a lot of work is happening in the server space which requires EFI.

But yes, please be my guest and suggest device tree properties to the Linux device tree list that allow exposure of tables (I?m not sure whether anything beyond SMBIOS makes sense for a dt based system) to Linux on non-EFI boot.


Alex

[U-Boot] [PATCH v2 5/7] smbios: Expose in efi_loader as table

[Alexander Graf]

We can pass SMBIOS easily as EFI configuration table to an EFI payload. This
patch adds enablement for that case.

While at it, we also enable SMBIOS generation for ARM systems, since they support
EFI_LOADER.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - fix whitespace
---
 cmd/bootefi.c        |  3 +++
 include/efi_api.h    |  4 ++++
 include/efi_loader.h |  2 ++
 include/smbios.h     |  1 +
 lib/Kconfig          |  4 ++--
 lib/smbios.c         | 36 ++++++++++++++++++++++++++++++++++++
 6 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 53a6ee3..e241b7d 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -205,6 +205,9 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
 	if (!memcmp(bootefi_device_path[0].str, "N\0e\0t", 6))
 		loaded_image_info.device_handle = nethandle;
 #endif
+#ifdef CONFIG_GENERATE_SMBIOS_TABLE
+	efi_smbios_register();
+#endif
 
 	/* Initialize EFI runtime services */
 	efi_reset_system_init();
diff --git a/include/efi_api.h b/include/efi_api.h
index f572b88..bdb600e 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -201,6 +201,10 @@ struct efi_runtime_services {
 	EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, \
 		 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0)
 
+#define SMBIOS_TABLE_GUID \
+	EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3,  \
+		 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
+
 struct efi_configuration_table
 {
 	efi_guid_t guid;
diff --git a/include/efi_loader.h b/include/efi_loader.h
index ac8b77a..b0e8a7f 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -85,6 +85,8 @@ int efi_disk_register(void);
 int efi_gop_register(void);
 /* Called by bootefi to make the network interface available */
 int efi_net_register(void **handle);
+/* Called by bootefi to make SMBIOS tables available */
+void efi_smbios_register(void);
 
 /* Called by networking code to memorize the dhcp ack package */
 void efi_net_set_dhcp_ack(void *pkt, int len);
diff --git a/include/smbios.h b/include/smbios.h
index 5962d4c..fb6396a 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -55,6 +55,7 @@ struct __packed smbios_entry {
 #define BIOS_CHARACTERISTICS_SELECTABLE_BOOT	(1 << 16)
 
 #define BIOS_CHARACTERISTICS_EXT1_ACPI		(1 << 0)
+#define BIOS_CHARACTERISTICS_EXT1_UEFI		(1 << 3)
 #define BIOS_CHARACTERISTICS_EXT2_TARGET	(1 << 2)
 
 struct __packed smbios_type0 {
diff --git a/lib/Kconfig b/lib/Kconfig
index 5a14530..d7f75fe 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -150,12 +150,12 @@ config SPL_OF_LIBFDT
 	  version of the device tree.
 
 menu "System tables"
-	depends on !EFI && !SYS_COREBOOT
+	depends on (!EFI && !SYS_COREBOOT) || (ARM && EFI_LOADER)
 
 config GENERATE_SMBIOS_TABLE
 	bool "Generate an SMBIOS (System Management BIOS) table"
 	default y
-	depends on X86
+	depends on X86 || EFI_LOADER
 	help
 	  The System Management BIOS (SMBIOS) specification addresses how
 	  motherboard and system vendors present management information about
diff --git a/lib/smbios.c b/lib/smbios.c
index 8dfd486..4d85155 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -7,10 +7,13 @@
  */
 
 #include <common.h>
+#include <efi_loader.h>
 #include <smbios.h>
 #include <tables_csum.h>
 #include <version.h>
+#ifdef CONFIG_X86
 #include <asm/cpu.h>
+#endif
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -79,14 +82,20 @@ static int smbios_write_type0(uintptr_t *current, int handle)
 	t->vendor = smbios_add_string(t->eos, "U-Boot");
 	t->bios_ver = smbios_add_string(t->eos, PLAIN_VERSION);
 	t->bios_release_date = smbios_add_string(t->eos, U_BOOT_DMI_DATE);
+#ifdef CONFIG_ROM_SIZE
 	t->bios_rom_size = (CONFIG_ROM_SIZE / 65536) - 1;
+#endif
 	t->bios_characteristics = BIOS_CHARACTERISTICS_PCI_SUPPORTED |
 				  BIOS_CHARACTERISTICS_SELECTABLE_BOOT |
 				  BIOS_CHARACTERISTICS_UPGRADEABLE;
 #ifdef CONFIG_GENERATE_ACPI_TABLE
 	t->bios_characteristics_ext1 = BIOS_CHARACTERISTICS_EXT1_ACPI;
 #endif
+#ifdef CONFIG_EFI_LOADER
+	t->bios_characteristics_ext1 |= BIOS_CHARACTERISTICS_EXT1_UEFI;
+#endif
 	t->bios_characteristics_ext2 = BIOS_CHARACTERISTICS_EXT2_TARGET;
+
 	t->bios_major_release = 0xff;
 	t->bios_minor_release = 0xff;
 	t->ec_major_release = 0xff;
@@ -152,6 +161,7 @@ static int smbios_write_type3(uintptr_t *current, int handle)
 	return len;
 }
 
+#ifdef CONFIG_X86
 static int smbios_write_type4(uintptr_t *current, int handle)
 {
 	struct smbios_type4 *t = (struct smbios_type4 *)*current;
@@ -184,6 +194,7 @@ static int smbios_write_type4(uintptr_t *current, int handle)
 
 	return len;
 }
+#endif
 
 static int smbios_write_type32(uintptr_t *current, int handle)
 {
@@ -216,7 +227,9 @@ static smbios_write_type smbios_write_funcs[] = {
 	smbios_write_type1,
 	smbios_write_type2,
 	smbios_write_type3,
+#ifdef CONFIG_X86
 	smbios_write_type4,
+#endif
 	smbios_write_type32,
 	smbios_write_type127
 };
@@ -267,3 +280,26 @@ uintptr_t write_smbios_table(uintptr_t addr)
 
 	return addr;
 }
+
+#ifdef CONFIG_EFI_LOADER
+
+void efi_smbios_register(void)
+{
+	static efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;
+	/* Map within the low 32 bits, to allow for 32bit SMBIOS tables */
+	uint64_t dmi = 0xffffffff;
+	/* Reserve 4kb for SMBIOS */
+	uint64_t pages = 1;
+	int memtype = EFI_RUNTIME_SERVICES_DATA;
+
+	if (efi_allocate_pages(1, memtype, pages, &dmi) != EFI_SUCCESS)
+		return;
+
+	/* Generate SMBIOS tables */
+	write_smbios_table(dmi);
+
+	/* And expose them to our EFI payload */
+	efi_install_configuration_table(&smbios_guid, (void*)dmi);
+}
+
+#endif
-- 
1.8.5.6

[U-Boot] [PATCH v2 7/7] smbios: Provide serial number

[Alexander Graf]

If the system has a valid "serial#" environment variable set (which boards that
can find it out programatically set automatically), use that as input for the
serial number and UUID fields in the SMBIOS tables.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - Also populate UUID
---
 lib/smbios.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/smbios.c b/lib/smbios.c
index 4d85155..2d0df23 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -111,11 +111,16 @@ static int smbios_write_type1(uintptr_t *current, int handle)
 {
 	struct smbios_type1 *t = (struct smbios_type1 *)*current;
 	int len = sizeof(struct smbios_type1);
+	char *serial_str = getenv("serial#");
 
 	memset(t, 0, sizeof(struct smbios_type1));
 	fill_smbios_header(t, SMBIOS_SYSTEM_INFORMATION, len, handle);
 	t->manufacturer = smbios_add_string(t->eos, CONFIG_SMBIOS_MANUFACTURER);
 	t->product_name = smbios_add_string(t->eos, CONFIG_SMBIOS_PRODUCT_NAME);
+	if (serial_str) {
+		strncpy((char*)t->uuid, serial_str, sizeof(t->uuid));
+		t->serial_number = smbios_add_string(t->eos, serial_str);
+	}
 
 	len = t->length + smbios_string_table_len(t->eos);
 	*current += len;
-- 
1.8.5.6

[U-Boot] [PATCH v2 5/7] smbios: Expose in efi_loader as table

[Bin Meng]

On Thu, Aug 11, 2016 at 5:48 PM, Alexander Graf <agraf@suse.de> wrote:
> We can pass SMBIOS easily as EFI configuration table to an EFI payload. This
> patch adds enablement for that case.
>
> While at it, we also enable SMBIOS generation for ARM systems, since they support
> EFI_LOADER.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
>
> ---
>
> v1 -> v2:
>
>   - fix whitespace
> ---
>  cmd/bootefi.c        |  3 +++
>  include/efi_api.h    |  4 ++++
>  include/efi_loader.h |  2 ++
>  include/smbios.h     |  1 +
>  lib/Kconfig          |  4 ++--
>  lib/smbios.c         | 36 ++++++++++++++++++++++++++++++++++++
>  6 files changed, 48 insertions(+), 2 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH v2 7/7] smbios: Provide serial number

[Bin Meng]

On Fri, Aug 12, 2016 at 5:45 AM, Alexander Graf <agraf@suse.de> wrote:
> If the system has a valid "serial#" environment variable set (which boards that
> can find it out programatically set automatically), use that as input for the
> serial number and UUID fields in the SMBIOS tables.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
>
> ---
>
> v1 -> v2:
>
>   - Also populate UUID
> ---
>  lib/smbios.c | 5 +++++
>  1 file changed, 5 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH 0/7] efi_loader: Expose SMBIOS table

[Simon Glass]

Hi Alex,

On 10 August 2016 at 01:29, Alexander Graf <agraf@suse.de> wrote:
>
>> On 09 Aug 2016, at 16:35, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Alexander,
>>
>> On 9 August 2016 at 08:11, Alexander Graf <agraf@suse.de> wrote:
>>>
>>> On 08/09/2016 03:57 PM, Simon Glass wrote:
>>>>
>>>> Hi Alexander,
>>>>
>>>> On 9 August 2016 at 00:48, Alexander Graf <agraf@suse.de> wrote:
>>>>>
>>>>>
>>>>>> Am 08.08.2016 um 23:44 schrieb Simon Glass <sjg@chromium.org>:
>>>>>>
>>>>>> Hi Alexander,
>>>>>>
>>>>>>> On 8 August 2016 at 08:06, Alexander Graf <agraf@suse.de> wrote:
>>>>>>> We generate a few tables on x86 today that really can be used on ARM just
>>>>>>> the same. One such example is the SMBIOS table, which people use with tools
>>>>>>> like "dmidecode" to identify which hardware they are running on.
>>>>>>>
>>>>>>> We're slowly growing needs to collect serial numbers from various devices
>>>>>>> on ARM and SMBIOS seems the natural choice. So this patch set moves the
>>>>>>> current SMBIOS generation into generic code and adds serial number exposure
>>>>>>> to it.
>>>>>>
>>>>>> Shouldn't we use device tree? Why would an ARM device use SMBIOS?
>>>>>
>>>>> Mostly because SBBR dictates it and every ARM server platform out there provides SMBIOS tables ;).
>>>>>
>>>>> Also, both describe very different things. At least I have never seen things like "The chassy of this server has 2 power connectors and is blue" in device tree.
>>>>
>>>> So there is no DT binding for this information? Does this mean that
>>>> U-Boot on ARM needs to pass information through just 'sitting in RAM
>>>> somewhere' like x86?
>>>
>>>
>>> I don't think there's a non-EFI way on ARM to pass this through at all, correct. That's nothing new though - things like KASLR also depend on EFI.
>>
>> That feels like it could just be done in U-Boot. The proliferation of
>> bootloader stuff in Linux bugs me. It's the lowest-common-denominator
>> problem. To me it seems that UEFI and U-Boot should implement these
>> features, and then Linux doesn't need to.
>>
>> Anyway I do understand where you are going. Is it possible to provide
>> EFI tables to Linux but not EFI boot/run-time services?
>
> I?m not aware of any way to do that today unfortunately. I don?t see why you couldn?t add a device tree property that points to random memory locations for tables though.

I was more thinking of whether this is defined in the device tree,  or
only exists as a binary table in SMBIOS? I probably know the answer
:-)

>
> As a more fundamental question though, will we really care about the non-EFI boot path 5 or 10 years from now? It doesn?t add much complexity and allows for less divergence between classic embedded and server systems. Since Linux itself is a uEFI binary, you can simply replace ?booti? with ?bootefi? in most boot cases and shouldn?t even see much of a difference.

Linux can certainly be built as an EFI binary. I can't predict the
future, but I'm not sure that 'classic embedded' (i.e. most devices in
the world) want to become more like server systems. We'll see.

>
> I don?t think that the non-EFI boot path will disappear, but turns more and more into a second class citizen because a lot of work is happening in the server space which requires EFI.

OK, well let's see. Most of my objection to EFI is the
complexity/size.obfuscation of the code. Maybe someone should work on
that, or are you suggesting that UEFI dies and people use U-Boot with
the EFI interface?

>
> But yes, please be my guest and suggest device tree properties to the Linux device tree list that allow exposure of tables (I?m not sure whether anything beyond SMBIOS makes sense for a dt based system) to Linux on non-EFI boot.

If I get a server system one day I might do that. I don't have one yet.

Regards,
Simon

[U-Boot] [PATCH 5/7] smbios: Expose in efi_loader as table

[Simon Glass]

Hi Alex,

On 8 August 2016 at 08:06, Alexander Graf <agraf@suse.de> wrote:
> We can pass SMBIOS easily as EFI configuration table to an EFI payload. This
> patch adds enablement for that case.
>
> While at it, we also enable SMBIOS generation for ARM systems, since they support
> EFI_LOADER.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  cmd/bootefi.c        |  3 +++
>  include/efi_api.h    |  4 ++++
>  include/efi_loader.h |  2 ++
>  include/smbios.h     |  1 +
>  lib/Kconfig          |  4 ++--
>  lib/smbios.c         | 36 ++++++++++++++++++++++++++++++++++++
>  6 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 53a6ee3..e241b7d 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -205,6 +205,9 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
>         if (!memcmp(bootefi_device_path[0].str, "N\0e\0t", 6))
>                 loaded_image_info.device_handle = nethandle;
>  #endif
> +#ifdef CONFIG_GENERATE_SMBIOS_TABLE
> +       efi_smbios_register();
> +#endif
>
>         /* Initialize EFI runtime services */
>         efi_reset_system_init();
> diff --git a/include/efi_api.h b/include/efi_api.h
> index f572b88..bdb600e 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -201,6 +201,10 @@ struct efi_runtime_services {
>         EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, \
>                  0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0)
>
> +#define SMBIOS_TABLE_GUID \
> +       EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3,  \
> +                0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
> +
>  struct efi_configuration_table
>  {
>         efi_guid_t guid;
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index ac8b77a..b0e8a7f 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -85,6 +85,8 @@ int efi_disk_register(void);
>  int efi_gop_register(void);
>  /* Called by bootefi to make the network interface available */
>  int efi_net_register(void **handle);
> +/* Called by bootefi to make SMBIOS tables available */
> +void efi_smbios_register(void);
>
>  /* Called by networking code to memorize the dhcp ack package */
>  void efi_net_set_dhcp_ack(void *pkt, int len);
> diff --git a/include/smbios.h b/include/smbios.h
> index 5962d4c..fb6396a 100644
> --- a/include/smbios.h
> +++ b/include/smbios.h
> @@ -55,6 +55,7 @@ struct __packed smbios_entry {
>  #define BIOS_CHARACTERISTICS_SELECTABLE_BOOT   (1 << 16)
>
>  #define BIOS_CHARACTERISTICS_EXT1_ACPI         (1 << 0)
> +#define BIOS_CHARACTERISTICS_EXT1_UEFI         (1 << 3)
>  #define BIOS_CHARACTERISTICS_EXT2_TARGET       (1 << 2)
>
>  struct __packed smbios_type0 {
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 5a14530..d7f75fe 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -150,12 +150,12 @@ config SPL_OF_LIBFDT
>           version of the device tree.
>
>  menu "System tables"
> -       depends on !EFI && !SYS_COREBOOT
> +       depends on (!EFI && !SYS_COREBOOT) || (ARM && EFI_LOADER)
>
>  config GENERATE_SMBIOS_TABLE
>         bool "Generate an SMBIOS (System Management BIOS) table"
>         default y
> -       depends on X86
> +       depends on X86 || EFI_LOADER
>         help
>           The System Management BIOS (SMBIOS) specification addresses how
>           motherboard and system vendors present management information about
> diff --git a/lib/smbios.c b/lib/smbios.c
> index 8dfd486..b9aa741 100644
> --- a/lib/smbios.c
> +++ b/lib/smbios.c
> @@ -7,10 +7,13 @@
>   */
>
>  #include <common.h>
> +#include <efi_loader.h>
>  #include <smbios.h>
>  #include <tables_csum.h>
>  #include <version.h>
> +#ifdef CONFIG_X86
>  #include <asm/cpu.h>
> +#endif
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -79,14 +82,20 @@ static int smbios_write_type0(uintptr_t *current, int handle)
>         t->vendor = smbios_add_string(t->eos, "U-Boot");
>         t->bios_ver = smbios_add_string(t->eos, PLAIN_VERSION);
>         t->bios_release_date = smbios_add_string(t->eos, U_BOOT_DMI_DATE);
> +#ifdef CONFIG_ROM_SIZE
>         t->bios_rom_size = (CONFIG_ROM_SIZE / 65536) - 1;
> +#endif
>         t->bios_characteristics = BIOS_CHARACTERISTICS_PCI_SUPPORTED |
>                                   BIOS_CHARACTERISTICS_SELECTABLE_BOOT |
>                                   BIOS_CHARACTERISTICS_UPGRADEABLE;
>  #ifdef CONFIG_GENERATE_ACPI_TABLE
>         t->bios_characteristics_ext1 = BIOS_CHARACTERISTICS_EXT1_ACPI;
>  #endif
> +#ifdef CONFIG_EFI_LOADER
> +       t->bios_characteristics_ext1 |= BIOS_CHARACTERISTICS_EXT1_UEFI;
> +#endif
>         t->bios_characteristics_ext2 = BIOS_CHARACTERISTICS_EXT2_TARGET;
> +
>         t->bios_major_release = 0xff;
>         t->bios_minor_release = 0xff;
>         t->ec_major_release = 0xff;
> @@ -152,6 +161,7 @@ static int smbios_write_type3(uintptr_t *current, int handle)
>         return len;
>  }
>
> +#ifdef CONFIG_X86
>  static int smbios_write_type4(uintptr_t *current, int handle)
>  {
>         struct smbios_type4 *t = (struct smbios_type4 *)*current;
> @@ -184,6 +194,7 @@ static int smbios_write_type4(uintptr_t *current, int handle)
>
>         return len;
>  }
> +#endif
>
>  static int smbios_write_type32(uintptr_t *current, int handle)
>  {
> @@ -216,7 +227,9 @@ static smbios_write_type smbios_write_funcs[] = {
>         smbios_write_type1,
>         smbios_write_type2,
>         smbios_write_type3,
> +#ifdef CONFIG_X86

This should become a parameter I think. Since you are making this into
generic code, it should avoid arch-specific #ifdefs.

>         smbios_write_type4,
> +#endif
>         smbios_write_type32,
>         smbios_write_type127
>  };
> @@ -267,3 +280,26 @@ uintptr_t write_smbios_table(uintptr_t addr)
>
>         return addr;
>  }
> +
> +#ifdef CONFIG_EFI_LOADER

Can this function go into the efi_loader directory instead?

> +
> +void efi_smbios_register(void)
> +{
> +       static efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;
> +       /* Map within the low 32 bits, to allow for 32bit SMBIOS tables */
> +        uint64_t dmi = 0xffffffff;
> +       /* Reserve 4kb for SMBIOS */
> +       uint64_t pages = 1;
> +       int memtype = EFI_RUNTIME_SERVICES_DATA;
> +
> +       if (efi_allocate_pages(1, memtype, pages, &dmi) != EFI_SUCCESS)
> +               return;
> +
> +       /* Generate SMBIOS tables */
> +       write_smbios_table(dmi);
> +
> +       /* And expose them to our EFI payload */
> +       efi_install_configuration_table(&smbios_guid, (void*)dmi);
> +}
> +
> +#endif
> --
> 2.6.6
>

Regards,
Simon

[U-Boot] [PATCH v2 7/7] smbios: Provide serial number

[Simon Glass]

On 11 August 2016 at 19:31, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Fri, Aug 12, 2016 at 5:45 AM, Alexander Graf <agraf@suse.de> wrote:
>> If the system has a valid "serial#" environment variable set (which boards that
>> can find it out programatically set automatically), use that as input for the
>> serial number and UUID fields in the SMBIOS tables.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>
>> ---
>>
>> v1 -> v2:
>>
>>   - Also populate UUID
>> ---
>>  lib/smbios.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 0/7] efi_loader: Expose SMBIOS table

[Alexander Graf]

On 12.08.16 19:20, Simon Glass wrote:
> Hi Alex,
> 
> On 10 August 2016 at 01:29, Alexander Graf <agraf@suse.de> wrote:
>>
>>> On 09 Aug 2016, at 16:35, Simon Glass <sjg@chromium.org> wrote:
>>>
>>> Hi Alexander,
>>>
>>> On 9 August 2016 at 08:11, Alexander Graf <agraf@suse.de> wrote:
>>>>
>>>> On 08/09/2016 03:57 PM, Simon Glass wrote:
>>>>>
>>>>> Hi Alexander,
>>>>>
>>>>> On 9 August 2016 at 00:48, Alexander Graf <agraf@suse.de> wrote:
>>>>>>
>>>>>>
>>>>>>> Am 08.08.2016 um 23:44 schrieb Simon Glass <sjg@chromium.org>:
>>>>>>>
>>>>>>> Hi Alexander,
>>>>>>>
>>>>>>>> On 8 August 2016 at 08:06, Alexander Graf <agraf@suse.de> wrote:
>>>>>>>> We generate a few tables on x86 today that really can be used on ARM just
>>>>>>>> the same. One such example is the SMBIOS table, which people use with tools
>>>>>>>> like "dmidecode" to identify which hardware they are running on.
>>>>>>>>
>>>>>>>> We're slowly growing needs to collect serial numbers from various devices
>>>>>>>> on ARM and SMBIOS seems the natural choice. So this patch set moves the
>>>>>>>> current SMBIOS generation into generic code and adds serial number exposure
>>>>>>>> to it.
>>>>>>>
>>>>>>> Shouldn't we use device tree? Why would an ARM device use SMBIOS?
>>>>>>
>>>>>> Mostly because SBBR dictates it and every ARM server platform out there provides SMBIOS tables ;).
>>>>>>
>>>>>> Also, both describe very different things. At least I have never seen things like "The chassy of this server has 2 power connectors and is blue" in device tree.
>>>>>
>>>>> So there is no DT binding for this information? Does this mean that
>>>>> U-Boot on ARM needs to pass information through just 'sitting in RAM
>>>>> somewhere' like x86?
>>>>
>>>>
>>>> I don't think there's a non-EFI way on ARM to pass this through at all, correct. That's nothing new though - things like KASLR also depend on EFI.
>>>
>>> That feels like it could just be done in U-Boot. The proliferation of
>>> bootloader stuff in Linux bugs me. It's the lowest-common-denominator
>>> problem. To me it seems that UEFI and U-Boot should implement these
>>> features, and then Linux doesn't need to.
>>>
>>> Anyway I do understand where you are going. Is it possible to provide
>>> EFI tables to Linux but not EFI boot/run-time services?
>>
>> I?m not aware of any way to do that today unfortunately. I don?t see why you couldn?t add a device tree property that points to random memory locations for tables though.
> 
> I was more thinking of whether this is defined in the device tree,  or
> only exists as a binary table in SMBIOS? I probably know the answer
> :-)
> 
>>
>> As a more fundamental question though, will we really care about the non-EFI boot path 5 or 10 years from now? It doesn?t add much complexity and allows for less divergence between classic embedded and server systems. Since Linux itself is a uEFI binary, you can simply replace ?booti? with ?bootefi? in most boot cases and shouldn?t even see much of a difference.
> 
> Linux can certainly be built as an EFI binary. I can't predict the

It does get built as EFI binary by default, no?

> future, but I'm not sure that 'classic embedded' (i.e. most devices in
> the world) want to become more like server systems. We'll see.

The nice thing about the EFI_LOADER support is that they really don't
have to. They can stay embedded in pretty much every aspect they are today.

> 
>>
>> I don?t think that the non-EFI boot path will disappear, but turns more and more into a second class citizen because a lot of work is happening in the server space which requires EFI.
> 
> OK, well let's see. Most of my objection to EFI is the
> complexity/size.obfuscation of the code. Maybe someone should work on
> that, or are you suggesting that UEFI dies and people use U-Boot with
> the EFI interface?

To get the naming right, uEFI is the specification as provided here:

  http://www.uefi.org/sites/default/files/resources/UEFI%20Spec%202_6.pdf

while the main pain point that people (including me) have, is the edk2
code base which is Intel's reference implementation of the uEFI spec:

  https://github.com/tianocore/edk2

I don't think either of them will die. But I do believe that there is no
real disadvantage of running U-Boot + uEFI Linux entry point over U-Boot
+ native booti entry point.

Since some work (like kASLR) depends on the uEFI entry point, I don't
think there's much need for the booti in the future. But I like to be
proven wrong :).

> 
>>
>> But yes, please be my guest and suggest device tree properties to the Linux device tree list that allow exposure of tables (I?m not sure whether anything beyond SMBIOS makes sense for a dt based system) to Linux on non-EFI boot.
> 
> If I get a server system one day I might do that. I don't have one yet.

The easiest way to get one is to run QEMU! :)


Alex

[U-Boot] [PATCH 5/7] smbios: Expose in efi_loader as table

[Alexander Graf]

On 12.08.16 19:20, Simon Glass wrote:
> Hi Alex,
> 
> On 8 August 2016 at 08:06, Alexander Graf <agraf@suse.de> wrote:
>> We can pass SMBIOS easily as EFI configuration table to an EFI payload. This
>> patch adds enablement for that case.
>>
>> While at it, we also enable SMBIOS generation for ARM systems, since they support
>> EFI_LOADER.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>  cmd/bootefi.c        |  3 +++
>>  include/efi_api.h    |  4 ++++
>>  include/efi_loader.h |  2 ++
>>  include/smbios.h     |  1 +
>>  lib/Kconfig          |  4 ++--
>>  lib/smbios.c         | 36 ++++++++++++++++++++++++++++++++++++
>>  6 files changed, 48 insertions(+), 2 deletions(-)
>>
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index 53a6ee3..e241b7d 100644
>> --- a/cmd/bootefi.c
>> +++ b/cmd/bootefi.c
>> @@ -205,6 +205,9 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
>>         if (!memcmp(bootefi_device_path[0].str, "N\0e\0t", 6))
>>                 loaded_image_info.device_handle = nethandle;
>>  #endif
>> +#ifdef CONFIG_GENERATE_SMBIOS_TABLE
>> +       efi_smbios_register();
>> +#endif
>>
>>         /* Initialize EFI runtime services */
>>         efi_reset_system_init();
>> diff --git a/include/efi_api.h b/include/efi_api.h
>> index f572b88..bdb600e 100644
>> --- a/include/efi_api.h
>> +++ b/include/efi_api.h
>> @@ -201,6 +201,10 @@ struct efi_runtime_services {
>>         EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, \
>>                  0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0)
>>
>> +#define SMBIOS_TABLE_GUID \
>> +       EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3,  \
>> +                0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
>> +
>>  struct efi_configuration_table
>>  {
>>         efi_guid_t guid;
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index ac8b77a..b0e8a7f 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -85,6 +85,8 @@ int efi_disk_register(void);
>>  int efi_gop_register(void);
>>  /* Called by bootefi to make the network interface available */
>>  int efi_net_register(void **handle);
>> +/* Called by bootefi to make SMBIOS tables available */
>> +void efi_smbios_register(void);
>>
>>  /* Called by networking code to memorize the dhcp ack package */
>>  void efi_net_set_dhcp_ack(void *pkt, int len);
>> diff --git a/include/smbios.h b/include/smbios.h
>> index 5962d4c..fb6396a 100644
>> --- a/include/smbios.h
>> +++ b/include/smbios.h
>> @@ -55,6 +55,7 @@ struct __packed smbios_entry {
>>  #define BIOS_CHARACTERISTICS_SELECTABLE_BOOT   (1 << 16)
>>
>>  #define BIOS_CHARACTERISTICS_EXT1_ACPI         (1 << 0)
>> +#define BIOS_CHARACTERISTICS_EXT1_UEFI         (1 << 3)
>>  #define BIOS_CHARACTERISTICS_EXT2_TARGET       (1 << 2)
>>
>>  struct __packed smbios_type0 {
>> diff --git a/lib/Kconfig b/lib/Kconfig
>> index 5a14530..d7f75fe 100644
>> --- a/lib/Kconfig
>> +++ b/lib/Kconfig
>> @@ -150,12 +150,12 @@ config SPL_OF_LIBFDT
>>           version of the device tree.
>>
>>  menu "System tables"
>> -       depends on !EFI && !SYS_COREBOOT
>> +       depends on (!EFI && !SYS_COREBOOT) || (ARM && EFI_LOADER)
>>
>>  config GENERATE_SMBIOS_TABLE
>>         bool "Generate an SMBIOS (System Management BIOS) table"
>>         default y
>> -       depends on X86
>> +       depends on X86 || EFI_LOADER
>>         help
>>           The System Management BIOS (SMBIOS) specification addresses how
>>           motherboard and system vendors present management information about
>> diff --git a/lib/smbios.c b/lib/smbios.c
>> index 8dfd486..b9aa741 100644
>> --- a/lib/smbios.c
>> +++ b/lib/smbios.c
>> @@ -7,10 +7,13 @@
>>   */
>>
>>  #include <common.h>
>> +#include <efi_loader.h>
>>  #include <smbios.h>
>>  #include <tables_csum.h>
>>  #include <version.h>
>> +#ifdef CONFIG_X86
>>  #include <asm/cpu.h>
>> +#endif
>>
>>  DECLARE_GLOBAL_DATA_PTR;
>>
>> @@ -79,14 +82,20 @@ static int smbios_write_type0(uintptr_t *current, int handle)
>>         t->vendor = smbios_add_string(t->eos, "U-Boot");
>>         t->bios_ver = smbios_add_string(t->eos, PLAIN_VERSION);
>>         t->bios_release_date = smbios_add_string(t->eos, U_BOOT_DMI_DATE);
>> +#ifdef CONFIG_ROM_SIZE
>>         t->bios_rom_size = (CONFIG_ROM_SIZE / 65536) - 1;
>> +#endif
>>         t->bios_characteristics = BIOS_CHARACTERISTICS_PCI_SUPPORTED |
>>                                   BIOS_CHARACTERISTICS_SELECTABLE_BOOT |
>>                                   BIOS_CHARACTERISTICS_UPGRADEABLE;
>>  #ifdef CONFIG_GENERATE_ACPI_TABLE
>>         t->bios_characteristics_ext1 = BIOS_CHARACTERISTICS_EXT1_ACPI;
>>  #endif
>> +#ifdef CONFIG_EFI_LOADER
>> +       t->bios_characteristics_ext1 |= BIOS_CHARACTERISTICS_EXT1_UEFI;
>> +#endif
>>         t->bios_characteristics_ext2 = BIOS_CHARACTERISTICS_EXT2_TARGET;
>> +
>>         t->bios_major_release = 0xff;
>>         t->bios_minor_release = 0xff;
>>         t->ec_major_release = 0xff;
>> @@ -152,6 +161,7 @@ static int smbios_write_type3(uintptr_t *current, int handle)
>>         return len;
>>  }
>>
>> +#ifdef CONFIG_X86
>>  static int smbios_write_type4(uintptr_t *current, int handle)
>>  {
>>         struct smbios_type4 *t = (struct smbios_type4 *)*current;
>> @@ -184,6 +194,7 @@ static int smbios_write_type4(uintptr_t *current, int handle)
>>
>>         return len;
>>  }
>> +#endif
>>
>>  static int smbios_write_type32(uintptr_t *current, int handle)
>>  {
>> @@ -216,7 +227,9 @@ static smbios_write_type smbios_write_funcs[] = {
>>         smbios_write_type1,
>>         smbios_write_type2,
>>         smbios_write_type3,
>> +#ifdef CONFIG_X86
> 
> This should become a parameter I think. Since you are making this into
> generic code, it should avoid arch-specific #ifdefs.

The type4 table really contains x86 cpu specific information, so I
figured an #ifdef CONFIG_X86 makes the most sense here.

Looking at


https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.0.0.pdf

I guess you *could* put one in that addresses ARM as well. Looking at
example output from a ThunderX system, that seems to be what other
vendors do:

Handle 0x0004, DMI type 4, 48 bytes
Processor Information
        Socket Designation: CPU 0
        Type: Central Processor
        Family: Other
        Manufacturer: www.cavium.com
        ID: 00 00 00 00 00 00 00 00
        Version: 2.0
        Voltage: 3.3 V
        External Clock: 156 MHz
        Max Speed: 2000 MHz
        Current Speed: 2000 MHz
        Status: Populated, Enabled
        Upgrade: Other
        L1 Cache Handle: 0x0080
        L2 Cache Handle: 0x0082
        L3 Cache Handle: 0x0007
        Serial Number: 1.0
        Asset Tag: 1.0
        Part Number: [...]
        Core Count: 48
        Core Enabled: 48
        Thread Count: 1
        Characteristics: None

So what we really need is a rename for smbios_write_type4 to
smbios_write_type4_x86 which then is still surrounded by the #ifdef
CONFIG_X86. We could just simply add another one for arm if we want to ;).

> 
>>         smbios_write_type4,
>> +#endif
>>         smbios_write_type32,
>>         smbios_write_type127
>>  };
>> @@ -267,3 +280,26 @@ uintptr_t write_smbios_table(uintptr_t addr)
>>
>>         return addr;
>>  }
>> +
>> +#ifdef CONFIG_EFI_LOADER
> 
> Can this function go into the efi_loader directory instead?

I guess we could have a small file in the efi_loader directory that
bridges the gap between the smbios lib and the generation, yeah.


Alex

> 
>> +
>> +void efi_smbios_register(void)
>> +{
>> +       static efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;
>> +       /* Map within the low 32 bits, to allow for 32bit SMBIOS tables */
>> +        uint64_t dmi = 0xffffffff;
>> +       /* Reserve 4kb for SMBIOS */
>> +       uint64_t pages = 1;
>> +       int memtype = EFI_RUNTIME_SERVICES_DATA;
>> +
>> +       if (efi_allocate_pages(1, memtype, pages, &dmi) != EFI_SUCCESS)
>> +               return;
>> +
>> +       /* Generate SMBIOS tables */
>> +       write_smbios_table(dmi);
>> +
>> +       /* And expose them to our EFI payload */
>> +       efi_install_configuration_table(&smbios_guid, (void*)dmi);
>> +}
>> +
>> +#endif
>> --
>> 2.6.6
>>
> 
> Regards,
> Simon
> 

[U-Boot] [PATCH 5/7] smbios: Expose in efi_loader as table

[Simon Glass]

Hi Alex,

On 12 August 2016 at 12:36, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 12.08.16 19:20, Simon Glass wrote:
>> Hi Alex,
>>
>> On 8 August 2016 at 08:06, Alexander Graf <agraf@suse.de> wrote:
>>> We can pass SMBIOS easily as EFI configuration table to an EFI payload. This
>>> patch adds enablement for that case.
>>>
>>> While at it, we also enable SMBIOS generation for ARM systems, since they support
>>> EFI_LOADER.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>>  cmd/bootefi.c        |  3 +++
>>>  include/efi_api.h    |  4 ++++
>>>  include/efi_loader.h |  2 ++
>>>  include/smbios.h     |  1 +
>>>  lib/Kconfig          |  4 ++--
>>>  lib/smbios.c         | 36 ++++++++++++++++++++++++++++++++++++
>>>  6 files changed, 48 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>> index 53a6ee3..e241b7d 100644
>>> --- a/cmd/bootefi.c
>>> +++ b/cmd/bootefi.c
>>> @@ -205,6 +205,9 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
>>>         if (!memcmp(bootefi_device_path[0].str, "N\0e\0t", 6))
>>>                 loaded_image_info.device_handle = nethandle;
>>>  #endif
>>> +#ifdef CONFIG_GENERATE_SMBIOS_TABLE
>>> +       efi_smbios_register();
>>> +#endif
>>>
>>>         /* Initialize EFI runtime services */
>>>         efi_reset_system_init();
>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>> index f572b88..bdb600e 100644
>>> --- a/include/efi_api.h
>>> +++ b/include/efi_api.h
>>> @@ -201,6 +201,10 @@ struct efi_runtime_services {
>>>         EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, \
>>>                  0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0)
>>>
>>> +#define SMBIOS_TABLE_GUID \
>>> +       EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3,  \
>>> +                0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
>>> +
>>>  struct efi_configuration_table
>>>  {
>>>         efi_guid_t guid;
>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>> index ac8b77a..b0e8a7f 100644
>>> --- a/include/efi_loader.h
>>> +++ b/include/efi_loader.h
>>> @@ -85,6 +85,8 @@ int efi_disk_register(void);
>>>  int efi_gop_register(void);
>>>  /* Called by bootefi to make the network interface available */
>>>  int efi_net_register(void **handle);
>>> +/* Called by bootefi to make SMBIOS tables available */
>>> +void efi_smbios_register(void);
>>>
>>>  /* Called by networking code to memorize the dhcp ack package */
>>>  void efi_net_set_dhcp_ack(void *pkt, int len);
>>> diff --git a/include/smbios.h b/include/smbios.h
>>> index 5962d4c..fb6396a 100644
>>> --- a/include/smbios.h
>>> +++ b/include/smbios.h
>>> @@ -55,6 +55,7 @@ struct __packed smbios_entry {
>>>  #define BIOS_CHARACTERISTICS_SELECTABLE_BOOT   (1 << 16)
>>>
>>>  #define BIOS_CHARACTERISTICS_EXT1_ACPI         (1 << 0)
>>> +#define BIOS_CHARACTERISTICS_EXT1_UEFI         (1 << 3)
>>>  #define BIOS_CHARACTERISTICS_EXT2_TARGET       (1 << 2)
>>>
>>>  struct __packed smbios_type0 {
>>> diff --git a/lib/Kconfig b/lib/Kconfig
>>> index 5a14530..d7f75fe 100644
>>> --- a/lib/Kconfig
>>> +++ b/lib/Kconfig
>>> @@ -150,12 +150,12 @@ config SPL_OF_LIBFDT
>>>           version of the device tree.
>>>
>>>  menu "System tables"
>>> -       depends on !EFI && !SYS_COREBOOT
>>> +       depends on (!EFI && !SYS_COREBOOT) || (ARM && EFI_LOADER)
>>>
>>>  config GENERATE_SMBIOS_TABLE
>>>         bool "Generate an SMBIOS (System Management BIOS) table"
>>>         default y
>>> -       depends on X86
>>> +       depends on X86 || EFI_LOADER
>>>         help
>>>           The System Management BIOS (SMBIOS) specification addresses how
>>>           motherboard and system vendors present management information about
>>> diff --git a/lib/smbios.c b/lib/smbios.c
>>> index 8dfd486..b9aa741 100644
>>> --- a/lib/smbios.c
>>> +++ b/lib/smbios.c
>>> @@ -7,10 +7,13 @@
>>>   */
>>>
>>>  #include <common.h>
>>> +#include <efi_loader.h>
>>>  #include <smbios.h>
>>>  #include <tables_csum.h>
>>>  #include <version.h>
>>> +#ifdef CONFIG_X86
>>>  #include <asm/cpu.h>
>>> +#endif
>>>
>>>  DECLARE_GLOBAL_DATA_PTR;
>>>
>>> @@ -79,14 +82,20 @@ static int smbios_write_type0(uintptr_t *current, int handle)
>>>         t->vendor = smbios_add_string(t->eos, "U-Boot");
>>>         t->bios_ver = smbios_add_string(t->eos, PLAIN_VERSION);
>>>         t->bios_release_date = smbios_add_string(t->eos, U_BOOT_DMI_DATE);
>>> +#ifdef CONFIG_ROM_SIZE
>>>         t->bios_rom_size = (CONFIG_ROM_SIZE / 65536) - 1;
>>> +#endif
>>>         t->bios_characteristics = BIOS_CHARACTERISTICS_PCI_SUPPORTED |
>>>                                   BIOS_CHARACTERISTICS_SELECTABLE_BOOT |
>>>                                   BIOS_CHARACTERISTICS_UPGRADEABLE;
>>>  #ifdef CONFIG_GENERATE_ACPI_TABLE
>>>         t->bios_characteristics_ext1 = BIOS_CHARACTERISTICS_EXT1_ACPI;
>>>  #endif
>>> +#ifdef CONFIG_EFI_LOADER
>>> +       t->bios_characteristics_ext1 |= BIOS_CHARACTERISTICS_EXT1_UEFI;
>>> +#endif
>>>         t->bios_characteristics_ext2 = BIOS_CHARACTERISTICS_EXT2_TARGET;
>>> +
>>>         t->bios_major_release = 0xff;
>>>         t->bios_minor_release = 0xff;
>>>         t->ec_major_release = 0xff;
>>> @@ -152,6 +161,7 @@ static int smbios_write_type3(uintptr_t *current, int handle)
>>>         return len;
>>>  }
>>>
>>> +#ifdef CONFIG_X86
>>>  static int smbios_write_type4(uintptr_t *current, int handle)
>>>  {
>>>         struct smbios_type4 *t = (struct smbios_type4 *)*current;
>>> @@ -184,6 +194,7 @@ static int smbios_write_type4(uintptr_t *current, int handle)
>>>
>>>         return len;
>>>  }
>>> +#endif
>>>
>>>  static int smbios_write_type32(uintptr_t *current, int handle)
>>>  {
>>> @@ -216,7 +227,9 @@ static smbios_write_type smbios_write_funcs[] = {
>>>         smbios_write_type1,
>>>         smbios_write_type2,
>>>         smbios_write_type3,
>>> +#ifdef CONFIG_X86
>>
>> This should become a parameter I think. Since you are making this into
>> generic code, it should avoid arch-specific #ifdefs.
>
> The type4 table really contains x86 cpu specific information, so I
> figured an #ifdef CONFIG_X86 makes the most sense here.
>
> Looking at
>
>
> https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.0.0.pdf
>
> I guess you *could* put one in that addresses ARM as well. Looking at
> example output from a ThunderX system, that seems to be what other
> vendors do:
>
> Handle 0x0004, DMI type 4, 48 bytes
> Processor Information
>         Socket Designation: CPU 0
>         Type: Central Processor
>         Family: Other
>         Manufacturer: www.cavium.com
>         ID: 00 00 00 00 00 00 00 00
>         Version: 2.0
>         Voltage: 3.3 V
>         External Clock: 156 MHz
>         Max Speed: 2000 MHz
>         Current Speed: 2000 MHz
>         Status: Populated, Enabled
>         Upgrade: Other
>         L1 Cache Handle: 0x0080
>         L2 Cache Handle: 0x0082
>         L3 Cache Handle: 0x0007
>         Serial Number: 1.0
>         Asset Tag: 1.0
>         Part Number: [...]
>         Core Count: 48
>         Core Enabled: 48
>         Thread Count: 1
>         Characteristics: None
>
> So what we really need is a rename for smbios_write_type4 to
> smbios_write_type4_x86 which then is still surrounded by the #ifdef
> CONFIG_X86. We could just simply add another one for arm if we want to ;).

From what I can see, the problem is the cpu_get_name() call. Is that
right? If so, that could move to the CPU uclass and use cpu_get_info()
(with the name added as a new field) or perhaps a new cpu_get_name()
call.

>
>>
>>>         smbios_write_type4,
>>> +#endif
>>>         smbios_write_type32,
>>>         smbios_write_type127
>>>  };
>>> @@ -267,3 +280,26 @@ uintptr_t write_smbios_table(uintptr_t addr)
>>>
>>>         return addr;
>>>  }
>>> +
>>> +#ifdef CONFIG_EFI_LOADER
>>
>> Can this function go into the efi_loader directory instead?
>
> I guess we could have a small file in the efi_loader directory that
> bridges the gap between the smbios lib and the generation, yeah.

OK good. I think this would make the #ifdef go away (sometimes a sign
that code is in the wrong place).

>
>
> Alex

[...]

Regards,
Simon

[U-Boot] [PATCH 5/7] smbios: Expose in efi_loader as table

[Alexander Graf]

On 08/12/2016 10:07 PM, Simon Glass wrote:
> Hi Alex,
>
> On 12 August 2016 at 12:36, Alexander Graf <agraf@suse.de> wrote:
>>
>> On 12.08.16 19:20, Simon Glass wrote:
>>> Hi Alex,
>>>
>>> On 8 August 2016 at 08:06, Alexander Graf <agraf@suse.de> wrote:
>>>> We can pass SMBIOS easily as EFI configuration table to an EFI payload. This
>>>> patch adds enablement for that case.
>>>>
>>>> While at it, we also enable SMBIOS generation for ARM systems, since they support
>>>> EFI_LOADER.
>>>>
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> ---
>>>>   cmd/bootefi.c        |  3 +++
>>>>   include/efi_api.h    |  4 ++++
>>>>   include/efi_loader.h |  2 ++
>>>>   include/smbios.h     |  1 +
>>>>   lib/Kconfig          |  4 ++--
>>>>   lib/smbios.c         | 36 ++++++++++++++++++++++++++++++++++++
>>>>   6 files changed, 48 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>>> index 53a6ee3..e241b7d 100644
>>>> --- a/cmd/bootefi.c
>>>> +++ b/cmd/bootefi.c
>>>> @@ -205,6 +205,9 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
>>>>          if (!memcmp(bootefi_device_path[0].str, "N\0e\0t", 6))
>>>>                  loaded_image_info.device_handle = nethandle;
>>>>   #endif
>>>> +#ifdef CONFIG_GENERATE_SMBIOS_TABLE
>>>> +       efi_smbios_register();
>>>> +#endif
>>>>
>>>>          /* Initialize EFI runtime services */
>>>>          efi_reset_system_init();
>>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>>> index f572b88..bdb600e 100644
>>>> --- a/include/efi_api.h
>>>> +++ b/include/efi_api.h
>>>> @@ -201,6 +201,10 @@ struct efi_runtime_services {
>>>>          EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, \
>>>>                   0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0)
>>>>
>>>> +#define SMBIOS_TABLE_GUID \
>>>> +       EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3,  \
>>>> +                0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
>>>> +
>>>>   struct efi_configuration_table
>>>>   {
>>>>          efi_guid_t guid;
>>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>>> index ac8b77a..b0e8a7f 100644
>>>> --- a/include/efi_loader.h
>>>> +++ b/include/efi_loader.h
>>>> @@ -85,6 +85,8 @@ int efi_disk_register(void);
>>>>   int efi_gop_register(void);
>>>>   /* Called by bootefi to make the network interface available */
>>>>   int efi_net_register(void **handle);
>>>> +/* Called by bootefi to make SMBIOS tables available */
>>>> +void efi_smbios_register(void);
>>>>
>>>>   /* Called by networking code to memorize the dhcp ack package */
>>>>   void efi_net_set_dhcp_ack(void *pkt, int len);
>>>> diff --git a/include/smbios.h b/include/smbios.h
>>>> index 5962d4c..fb6396a 100644
>>>> --- a/include/smbios.h
>>>> +++ b/include/smbios.h
>>>> @@ -55,6 +55,7 @@ struct __packed smbios_entry {
>>>>   #define BIOS_CHARACTERISTICS_SELECTABLE_BOOT   (1 << 16)
>>>>
>>>>   #define BIOS_CHARACTERISTICS_EXT1_ACPI         (1 << 0)
>>>> +#define BIOS_CHARACTERISTICS_EXT1_UEFI         (1 << 3)
>>>>   #define BIOS_CHARACTERISTICS_EXT2_TARGET       (1 << 2)
>>>>
>>>>   struct __packed smbios_type0 {
>>>> diff --git a/lib/Kconfig b/lib/Kconfig
>>>> index 5a14530..d7f75fe 100644
>>>> --- a/lib/Kconfig
>>>> +++ b/lib/Kconfig
>>>> @@ -150,12 +150,12 @@ config SPL_OF_LIBFDT
>>>>            version of the device tree.
>>>>
>>>>   menu "System tables"
>>>> -       depends on !EFI && !SYS_COREBOOT
>>>> +       depends on (!EFI && !SYS_COREBOOT) || (ARM && EFI_LOADER)
>>>>
>>>>   config GENERATE_SMBIOS_TABLE
>>>>          bool "Generate an SMBIOS (System Management BIOS) table"
>>>>          default y
>>>> -       depends on X86
>>>> +       depends on X86 || EFI_LOADER
>>>>          help
>>>>            The System Management BIOS (SMBIOS) specification addresses how
>>>>            motherboard and system vendors present management information about
>>>> diff --git a/lib/smbios.c b/lib/smbios.c
>>>> index 8dfd486..b9aa741 100644
>>>> --- a/lib/smbios.c
>>>> +++ b/lib/smbios.c
>>>> @@ -7,10 +7,13 @@
>>>>    */
>>>>
>>>>   #include <common.h>
>>>> +#include <efi_loader.h>
>>>>   #include <smbios.h>
>>>>   #include <tables_csum.h>
>>>>   #include <version.h>
>>>> +#ifdef CONFIG_X86
>>>>   #include <asm/cpu.h>
>>>> +#endif
>>>>
>>>>   DECLARE_GLOBAL_DATA_PTR;
>>>>
>>>> @@ -79,14 +82,20 @@ static int smbios_write_type0(uintptr_t *current, int handle)
>>>>          t->vendor = smbios_add_string(t->eos, "U-Boot");
>>>>          t->bios_ver = smbios_add_string(t->eos, PLAIN_VERSION);
>>>>          t->bios_release_date = smbios_add_string(t->eos, U_BOOT_DMI_DATE);
>>>> +#ifdef CONFIG_ROM_SIZE
>>>>          t->bios_rom_size = (CONFIG_ROM_SIZE / 65536) - 1;
>>>> +#endif
>>>>          t->bios_characteristics = BIOS_CHARACTERISTICS_PCI_SUPPORTED |
>>>>                                    BIOS_CHARACTERISTICS_SELECTABLE_BOOT |
>>>>                                    BIOS_CHARACTERISTICS_UPGRADEABLE;
>>>>   #ifdef CONFIG_GENERATE_ACPI_TABLE
>>>>          t->bios_characteristics_ext1 = BIOS_CHARACTERISTICS_EXT1_ACPI;
>>>>   #endif
>>>> +#ifdef CONFIG_EFI_LOADER
>>>> +       t->bios_characteristics_ext1 |= BIOS_CHARACTERISTICS_EXT1_UEFI;
>>>> +#endif
>>>>          t->bios_characteristics_ext2 = BIOS_CHARACTERISTICS_EXT2_TARGET;
>>>> +
>>>>          t->bios_major_release = 0xff;
>>>>          t->bios_minor_release = 0xff;
>>>>          t->ec_major_release = 0xff;
>>>> @@ -152,6 +161,7 @@ static int smbios_write_type3(uintptr_t *current, int handle)
>>>>          return len;
>>>>   }
>>>>
>>>> +#ifdef CONFIG_X86
>>>>   static int smbios_write_type4(uintptr_t *current, int handle)
>>>>   {
>>>>          struct smbios_type4 *t = (struct smbios_type4 *)*current;
>>>> @@ -184,6 +194,7 @@ static int smbios_write_type4(uintptr_t *current, int handle)
>>>>
>>>>          return len;
>>>>   }
>>>> +#endif
>>>>
>>>>   static int smbios_write_type32(uintptr_t *current, int handle)
>>>>   {
>>>> @@ -216,7 +227,9 @@ static smbios_write_type smbios_write_funcs[] = {
>>>>          smbios_write_type1,
>>>>          smbios_write_type2,
>>>>          smbios_write_type3,
>>>> +#ifdef CONFIG_X86
>>> This should become a parameter I think. Since you are making this into
>>> generic code, it should avoid arch-specific #ifdefs.
>> The type4 table really contains x86 cpu specific information, so I
>> figured an #ifdef CONFIG_X86 makes the most sense here.
>>
>> Looking at
>>
>>
>> https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.0.0.pdf
>>
>> I guess you *could* put one in that addresses ARM as well. Looking at
>> example output from a ThunderX system, that seems to be what other
>> vendors do:
>>
>> Handle 0x0004, DMI type 4, 48 bytes
>> Processor Information
>>          Socket Designation: CPU 0
>>          Type: Central Processor
>>          Family: Other
>>          Manufacturer: www.cavium.com
>>          ID: 00 00 00 00 00 00 00 00
>>          Version: 2.0
>>          Voltage: 3.3 V
>>          External Clock: 156 MHz
>>          Max Speed: 2000 MHz
>>          Current Speed: 2000 MHz
>>          Status: Populated, Enabled
>>          Upgrade: Other
>>          L1 Cache Handle: 0x0080
>>          L2 Cache Handle: 0x0082
>>          L3 Cache Handle: 0x0007
>>          Serial Number: 1.0
>>          Asset Tag: 1.0
>>          Part Number: [...]
>>          Core Count: 48
>>          Core Enabled: 48
>>          Thread Count: 1
>>          Characteristics: None
>>
>> So what we really need is a rename for smbios_write_type4 to
>> smbios_write_type4_x86 which then is still surrounded by the #ifdef
>> CONFIG_X86. We could just simply add another one for arm if we want to ;).
>  From what I can see, the problem is the cpu_get_name() call. Is that
> right? If so, that could move to the CPU uclass and use cpu_get_info()
> (with the name added as a new field) or perhaps a new cpu_get_name()
> call.

I see what you're aiming for, but I just fail to see how we could 
properly abstract away CPU specific information - and I don't think it's 
terribly useful either. I've refactored the code to be slightly more 
pretty now, but I really don't think that we should depend on CONFIG_CPU 
or introduce new calls for every bit:

static void smbios_write_type4_arch(struct smbios_type4 *t)
{
         u16 processor_family = SMBIOS_PROCESSOR_FAMILY_UNKNOWN;
         const char *vendor = "Unknown";
         char *name = "Unknown";

#ifdef CONFIG_X86
         char processor_name[CPU_MAX_NAME_LEN];
         struct cpuid_result res;

         processor_family = gd->arch.x86;
         vendor = cpu_vendor_name(gd->arch.x86_vendor);
         res = cpuid(1);
         t->processor_id[0] = res.eax;
         t->processor_id[1] = res.edx;
         name = cpu_get_name(processor_name);
#elif defined(CONFIG_ARM)
         processor_family = SMBIOS_PROCESSOR_FAMILY_OTHER;
         vendor = "ARM";
#endif

         t->processor_family = processor_family;
         t->processor_manufacturer = smbios_add_string(t->eos, vendor);
         t->processor_version = smbios_add_string(t->eos, name);
}


Alex

[U-Boot] [PATCH 5/7] smbios: Expose in efi_loader as table

[Simon Glass]

Hi Alex,

On 16 August 2016 at 02:38, Alexander Graf <agraf@suse.de> wrote:
> On 08/12/2016 10:07 PM, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 12 August 2016 at 12:36, Alexander Graf <agraf@suse.de> wrote:
>>>
>>>
>>> On 12.08.16 19:20, Simon Glass wrote:
>>>>
>>>> Hi Alex,
>>>>
>>>> On 8 August 2016 at 08:06, Alexander Graf <agraf@suse.de> wrote:
>>>>>
>>>>> We can pass SMBIOS easily as EFI configuration table to an EFI payload.
>>>>> This
>>>>> patch adds enablement for that case.
>>>>>
>>>>> While at it, we also enable SMBIOS generation for ARM systems, since
>>>>> they support
>>>>> EFI_LOADER.
>>>>>
>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>> ---
>>>>>   cmd/bootefi.c        |  3 +++
>>>>>   include/efi_api.h    |  4 ++++
>>>>>   include/efi_loader.h |  2 ++
>>>>>   include/smbios.h     |  1 +
>>>>>   lib/Kconfig          |  4 ++--
>>>>>   lib/smbios.c         | 36 ++++++++++++++++++++++++++++++++++++
>>>>>   6 files changed, 48 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>>>> index 53a6ee3..e241b7d 100644
>>>>> --- a/cmd/bootefi.c
>>>>> +++ b/cmd/bootefi.c
>>>>> @@ -205,6 +205,9 @@ static unsigned long do_bootefi_exec(void *efi,
>>>>> void *fdt)
>>>>>          if (!memcmp(bootefi_device_path[0].str, "N\0e\0t", 6))
>>>>>                  loaded_image_info.device_handle = nethandle;
>>>>>   #endif
>>>>> +#ifdef CONFIG_GENERATE_SMBIOS_TABLE
>>>>> +       efi_smbios_register();
>>>>> +#endif
>>>>>
>>>>>          /* Initialize EFI runtime services */
>>>>>          efi_reset_system_init();
>>>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>>>> index f572b88..bdb600e 100644
>>>>> --- a/include/efi_api.h
>>>>> +++ b/include/efi_api.h
>>>>> @@ -201,6 +201,10 @@ struct efi_runtime_services {
>>>>>          EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, \
>>>>>                   0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0)
>>>>>
>>>>> +#define SMBIOS_TABLE_GUID \
>>>>> +       EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3,  \
>>>>> +                0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
>>>>> +
>>>>>   struct efi_configuration_table
>>>>>   {
>>>>>          efi_guid_t guid;
>>>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>>>> index ac8b77a..b0e8a7f 100644
>>>>> --- a/include/efi_loader.h
>>>>> +++ b/include/efi_loader.h
>>>>> @@ -85,6 +85,8 @@ int efi_disk_register(void);
>>>>>   int efi_gop_register(void);
>>>>>   /* Called by bootefi to make the network interface available */
>>>>>   int efi_net_register(void **handle);
>>>>> +/* Called by bootefi to make SMBIOS tables available */
>>>>> +void efi_smbios_register(void);
>>>>>
>>>>>   /* Called by networking code to memorize the dhcp ack package */
>>>>>   void efi_net_set_dhcp_ack(void *pkt, int len);
>>>>> diff --git a/include/smbios.h b/include/smbios.h
>>>>> index 5962d4c..fb6396a 100644
>>>>> --- a/include/smbios.h
>>>>> +++ b/include/smbios.h
>>>>> @@ -55,6 +55,7 @@ struct __packed smbios_entry {
>>>>>   #define BIOS_CHARACTERISTICS_SELECTABLE_BOOT   (1 << 16)
>>>>>
>>>>>   #define BIOS_CHARACTERISTICS_EXT1_ACPI         (1 << 0)
>>>>> +#define BIOS_CHARACTERISTICS_EXT1_UEFI         (1 << 3)
>>>>>   #define BIOS_CHARACTERISTICS_EXT2_TARGET       (1 << 2)
>>>>>
>>>>>   struct __packed smbios_type0 {
>>>>> diff --git a/lib/Kconfig b/lib/Kconfig
>>>>> index 5a14530..d7f75fe 100644
>>>>> --- a/lib/Kconfig
>>>>> +++ b/lib/Kconfig
>>>>> @@ -150,12 +150,12 @@ config SPL_OF_LIBFDT
>>>>>            version of the device tree.
>>>>>
>>>>>   menu "System tables"
>>>>> -       depends on !EFI && !SYS_COREBOOT
>>>>> +       depends on (!EFI && !SYS_COREBOOT) || (ARM && EFI_LOADER)
>>>>>
>>>>>   config GENERATE_SMBIOS_TABLE
>>>>>          bool "Generate an SMBIOS (System Management BIOS) table"
>>>>>          default y
>>>>> -       depends on X86
>>>>> +       depends on X86 || EFI_LOADER
>>>>>          help
>>>>>            The System Management BIOS (SMBIOS) specification addresses
>>>>> how
>>>>>            motherboard and system vendors present management
>>>>> information about
>>>>> diff --git a/lib/smbios.c b/lib/smbios.c
>>>>> index 8dfd486..b9aa741 100644
>>>>> --- a/lib/smbios.c
>>>>> +++ b/lib/smbios.c
>>>>> @@ -7,10 +7,13 @@
>>>>>    */
>>>>>
>>>>>   #include <common.h>
>>>>> +#include <efi_loader.h>
>>>>>   #include <smbios.h>
>>>>>   #include <tables_csum.h>
>>>>>   #include <version.h>
>>>>> +#ifdef CONFIG_X86
>>>>>   #include <asm/cpu.h>
>>>>> +#endif
>>>>>
>>>>>   DECLARE_GLOBAL_DATA_PTR;
>>>>>
>>>>> @@ -79,14 +82,20 @@ static int smbios_write_type0(uintptr_t *current,
>>>>> int handle)
>>>>>          t->vendor = smbios_add_string(t->eos, "U-Boot");
>>>>>          t->bios_ver = smbios_add_string(t->eos, PLAIN_VERSION);
>>>>>          t->bios_release_date = smbios_add_string(t->eos,
>>>>> U_BOOT_DMI_DATE);
>>>>> +#ifdef CONFIG_ROM_SIZE
>>>>>          t->bios_rom_size = (CONFIG_ROM_SIZE / 65536) - 1;
>>>>> +#endif
>>>>>          t->bios_characteristics = BIOS_CHARACTERISTICS_PCI_SUPPORTED |
>>>>>                                    BIOS_CHARACTERISTICS_SELECTABLE_BOOT
>>>>> |
>>>>>                                    BIOS_CHARACTERISTICS_UPGRADEABLE;
>>>>>   #ifdef CONFIG_GENERATE_ACPI_TABLE
>>>>>          t->bios_characteristics_ext1 = BIOS_CHARACTERISTICS_EXT1_ACPI;
>>>>>   #endif
>>>>> +#ifdef CONFIG_EFI_LOADER
>>>>> +       t->bios_characteristics_ext1 |= BIOS_CHARACTERISTICS_EXT1_UEFI;
>>>>> +#endif
>>>>>          t->bios_characteristics_ext2 =
>>>>> BIOS_CHARACTERISTICS_EXT2_TARGET;
>>>>> +
>>>>>          t->bios_major_release = 0xff;
>>>>>          t->bios_minor_release = 0xff;
>>>>>          t->ec_major_release = 0xff;
>>>>> @@ -152,6 +161,7 @@ static int smbios_write_type3(uintptr_t *current,
>>>>> int handle)
>>>>>          return len;
>>>>>   }
>>>>>
>>>>> +#ifdef CONFIG_X86
>>>>>   static int smbios_write_type4(uintptr_t *current, int handle)
>>>>>   {
>>>>>          struct smbios_type4 *t = (struct smbios_type4 *)*current;
>>>>> @@ -184,6 +194,7 @@ static int smbios_write_type4(uintptr_t *current,
>>>>> int handle)
>>>>>
>>>>>          return len;
>>>>>   }
>>>>> +#endif
>>>>>
>>>>>   static int smbios_write_type32(uintptr_t *current, int handle)
>>>>>   {
>>>>> @@ -216,7 +227,9 @@ static smbios_write_type smbios_write_funcs[] = {
>>>>>          smbios_write_type1,
>>>>>          smbios_write_type2,
>>>>>          smbios_write_type3,
>>>>> +#ifdef CONFIG_X86
>>>>
>>>> This should become a parameter I think. Since you are making this into
>>>> generic code, it should avoid arch-specific #ifdefs.
>>>
>>> The type4 table really contains x86 cpu specific information, so I
>>> figured an #ifdef CONFIG_X86 makes the most sense here.
>>>
>>> Looking at
>>>
>>>
>>>
>>> https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.0.0.pdf
>>>
>>> I guess you *could* put one in that addresses ARM as well. Looking at
>>> example output from a ThunderX system, that seems to be what other
>>> vendors do:
>>>
>>> Handle 0x0004, DMI type 4, 48 bytes
>>> Processor Information
>>>          Socket Designation: CPU 0
>>>          Type: Central Processor
>>>          Family: Other
>>>          Manufacturer: www.cavium.com
>>>          ID: 00 00 00 00 00 00 00 00
>>>          Version: 2.0
>>>          Voltage: 3.3 V
>>>          External Clock: 156 MHz
>>>          Max Speed: 2000 MHz
>>>          Current Speed: 2000 MHz
>>>          Status: Populated, Enabled
>>>          Upgrade: Other
>>>          L1 Cache Handle: 0x0080
>>>          L2 Cache Handle: 0x0082
>>>          L3 Cache Handle: 0x0007
>>>          Serial Number: 1.0
>>>          Asset Tag: 1.0
>>>          Part Number: [...]
>>>          Core Count: 48
>>>          Core Enabled: 48
>>>          Thread Count: 1
>>>          Characteristics: None
>>>
>>> So what we really need is a rename for smbios_write_type4 to
>>> smbios_write_type4_x86 which then is still surrounded by the #ifdef
>>> CONFIG_X86. We could just simply add another one for arm if we want to
>>> ;).
>>
>>  From what I can see, the problem is the cpu_get_name() call. Is that
>> right? If so, that could move to the CPU uclass and use cpu_get_info()
>> (with the name added as a new field) or perhaps a new cpu_get_name()
>> call.
>
>
> I see what you're aiming for, but I just fail to see how we could properly
> abstract away CPU specific information - and I don't think it's terribly
> useful either. I've refactored the code to be slightly more pretty now, but
> I really don't think that we should depend on CONFIG_CPU or introduce new

All you are getting here is the same and the processor ID, I think. It
is not hard to do what you need in the cpu uclass.

Please don't introduce this sort of cruft in what is now generic code.
You can make EFI_LOADER depend on CPU or you can put an #ifdef
CONFIG_CPU here, but what you have is just going to create a mess as
more things are added to it. Given that we have a defined CPU
interface, it is also unnecessary. With a little more effort you can
come to a nice solution.

> calls for every bit:
>
> static void smbios_write_type4_arch(struct smbios_type4 *t)
> {
>         u16 processor_family = SMBIOS_PROCESSOR_FAMILY_UNKNOWN;
>         const char *vendor = "Unknown";
>         char *name = "Unknown";
>
> #ifdef CONFIG_X86
>         char processor_name[CPU_MAX_NAME_LEN];
>         struct cpuid_result res;
>
>         processor_family = gd->arch.x86;
>         vendor = cpu_vendor_name(gd->arch.x86_vendor);
>         res = cpuid(1);
>         t->processor_id[0] = res.eax;
>         t->processor_id[1] = res.edx;
>         name = cpu_get_name(processor_name);
> #elif defined(CONFIG_ARM)
>         processor_family = SMBIOS_PROCESSOR_FAMILY_OTHER;
>         vendor = "ARM";
> #endif
>
>         t->processor_family = processor_family;
>         t->processor_manufacturer = smbios_add_string(t->eos, vendor);
>         t->processor_version = smbios_add_string(t->eos, name);
> }
>
>
> Alex
>

Regards,
Simon