On 12/31/23 17:11, Heinrich Schuchardt wrote:
On 12/31/23 16:25, Simon Glass wrote:
When the SMBIOS table is written to an address above 4GB a 32-bit table
address is not large enough.

Use an SMBIOS3 table in that case.

Note that we cannot use efi_allocate_pages() since this function has
nothing to do with EFI. There is no equivalent function to allocate
memory below 4GB in U-Boot. One solution would be to create a separate
malloc() pool, or just always put the malloc() pool below 4GB.

- Use log_debug() for warning
- Rebase on Heinrich's smbios.h patch
- Set the checksum for SMBIOS3

Signed-off-by: Simon Glass <s...@chromium.org>
---

(no changes since v4)

Changes in v4:
- Check the start of the table rather than the end

Changes in v2:
- Check the end of the table rather than the start.

  include/smbios.h |  6 +++++-
  lib/smbios.c     | 30 +++++++++++++++++++++++++-----
  2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/include/smbios.h b/include/smbios.h
index e601283d293..77be58887a2 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -12,7 +12,7 @@

  /* SMBIOS spec version implemented */
  #define SMBIOS_MAJOR_VER    3
-#define SMBIOS_MINOR_VER    0
+#define SMBIOS_MINOR_VER    7

  enum {
      SMBIOS_STR_MAX    = 64,    /* Maximum length allowed for a
string */
@@ -80,6 +80,10 @@ struct __packed smbios3_entry {
      u64 struct_table_address;
  };

+/* These two structures should use the same amount of 16-byte-aligned
space */
+static_assert(ALIGN(16, sizeof(struct smbios_entry)) ==
+          ALIGN(16, sizeof(struct smbios3_entry)));
+
  /* BIOS characteristics */
  #define BIOS_CHARACTERISTICS_PCI_SUPPORTED    (1 << 7)
  #define BIOS_CHARACTERISTICS_UPGRADEABLE    (1 << 11)
diff --git a/lib/smbios.c b/lib/smbios.c
index eea72670bd9..7f79d969c92 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -567,7 +567,11 @@ ulong write_smbios_table(ulong addr)
      addr = ALIGN(addr, 16);
      start_addr = addr;

-    addr += sizeof(struct smbios_entry);
+    /*
+     * So far we don't know which struct will be used, but they both end
+     * up using the same amount of 16-bit-aligned space
+     */
+    addr += max(sizeof(struct smbios_entry), sizeof(struct
smbios3_entry));
      addr = ALIGN(addr, 16);
      tables = addr;

@@ -590,16 +594,32 @@ ulong write_smbios_table(ulong addr)
       * We must use a pointer here so things work correctly on
sandbox. The
       * user of this table is not aware of the mapping of addresses to
       * sandbox's DRAM buffer.
+     *
+     * Check the address of the end of the tables. If it is above 4GB
then
+     * it is sensible to use SMBIOS3 even if the start of the table
is below
+     * 4GB (this case is very unlikely to happen in practice)
       */
      table_addr = (ulong)map_sysmem(tables, 0);
-    if (sizeof(table_addr) > sizeof(u32) && table_addr >
(ulong)UINT_MAX) {
+    if (sizeof(table_addr) > sizeof(u32) && addr >= (ulong)UINT_MAX) {


All SMBIOS specifications since version 3.0.0, published 2015, allow
using a SMBIOS3 anchor in all cases. SMBIOS_MAJOR_VER == 3.

Our target is to keep the code size small. Why do you increase the code
size here?

You drop SMBIOS2 in patch 8.

Reviewed-by: Heinrich Schuchardt <xypron.g...@gmx.de>


Best regards

Heinrich

+        struct smbios3_entry *se;
          /*
           * We need to put this >32-bit pointer into the table but the
           * field is only 32 bits wide.
           */
-        printf("WARNING: SMBIOS table_address overflow %llx\n",
-               (unsigned long long)table_addr);
-        addr = 0;
+        log_debug("WARNING: Using SMBIOS3.0 due to table-address
overflow %lx\n",
+              table_addr);
+        se = map_sysmem(start_addr, sizeof(struct smbios_entry));
+        memset(se, '\0', sizeof(struct smbios_entry));
+        memcpy(se->anchor, "_SM3_", 5);
+        se->length = sizeof(struct smbios3_entry);
+        se->major_ver = SMBIOS_MAJOR_VER;
+        se->minor_ver = SMBIOS_MINOR_VER;
+        se->doc_rev = 0;
+        se->entry_point_rev = 1;
+        se->max_struct_size = len;
+        se->struct_table_address = table_addr;
+        se->checksum = table_compute_checksum(se,
+                        sizeof(struct smbios3_entry));
      } else {
          struct smbios_entry *se;



Reply via email to