Skip to contentSkip to conversation

Add a module for retrieving SMBIOS information

PATCH Superseded David Michael 4 participants 11 messages Feb 8, 2015 – Mar 18, 2015

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 Add a module for retrieving SMBIOS information

The following are two use cases from Rajat Jain <[email protected]>:

1) We have a board that boots Linux and this board itself can be plugged into one of different chassis types. We need to pass different parameters to the kernel based on the "CHASSIS_TYPE" information that is passed by the bios in the DMI / SMBIOS tables.

2) We may have a USB stick that can go into multiple boards, and the exact kernel to be loaded depends on the machine information (PRODUCT_NAME etc) passed via the DMI.

* grub-core/commands/smbios.c: New file.
* grub-core/Makefile.core.def (smbios): New module.
* docs/grub.texi (smbios): New node.
(Command-line and menu entry commands): Add a menu entry for smbios.
---

Changes since v1:

* Removed formfeeds and quoted use cases in the commit messages as suggested by Prarit Bhargava.

* Enabled building the module on all EFI platforms as suggested by Leif Lindholm.

 docs/grub.texi              |  67 ++++++++
 grub-core/Makefile.core.def |   7 +
 grub-core/commands/smbios.c | 361 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 435 insertions(+)
 create mode 100644 grub-core/commands/smbios.c
docs/grub.texi +67 -0
index 46b9e7f..f4e187f 100644
@@ -3830,6 +3830,7 @@ you forget a command, you can run the command @command{help}
 * sha256sum::                   Compute or check SHA256 hash
 * sha512sum::                   Compute or check SHA512 hash
 * sleep::                       Wait for a specified number of seconds
+* smbios::                      Retrieve SMBIOS information
 * source::                      Read a configuration file in same context
 * test::                        Check file types and compare values
 * true::                        Do nothing, successfully
@@ -4944,6 +4945,72 @@ if timeout was interrupted by @key{ESC}.
 @end deffn
 
 
+@node smbios
+@subsection smbios
+
+@deffn Command smbios @
+ [@option{-t} @var{type}] @
+ [@option{-h} @var{handle}] @
+ [@option{-m} @var{match}] @
+ [ (@option{-b} | @option{-w} | @option{-d} | @option{-q} | @option{-s}) @
+   @var{offset} [@option{-v} @var{variable}] ]
+Retrieve SMBIOS information.  This command is only available on x86 and EFI
+systems.
+
+When run with no options, @command{smbios} will print the SMBIOS table entries
+to the console as the header values, a hex dump, and a string list.  The
+following options can filter which table entries are printed.
+
+@itemize @bullet
+@item
+Specifying @option{-t} will only print entries with a matching @var{type}.  The
+type can be any value from 0 to 255.  Specify a value outside this range to
+match entries with any type.  The default is -1.
+@item
+Specifying @option{-h} will only print entries with a matching @var{handle}.
+The handle can be any value from 0 to 65535.  Specify a value outside this
+range to match entries with any handle.  The default is -1.
+@item
+Specifying @option{-m} will only print number @var{match} in the list of all
+filtered entries; e.g. @code{smbios -m 3} will print the third entry.  The list
+is always ordered the same as the hardware's SMBIOS table.  The match number
+can be any positive value.  Specify 0 to match all entries.  The default is 0.
+@end itemize
+
+The remaining options print the value of a field in the matched SMBIOS table
+entry.  Only one of these options may be specified.  When they are used, the
+option @code{-m 0} is synonymous with @code{-m 1} to match the first entry.
+
+@itemize @bullet
+@item
+When given @option{-b}, print the value of the byte
+at @var{offset} bytes into the matched SMBIOS table entry.
+@item
+When given @option{-w}, print the value of the word (two bytes)
+at @var{offset} bytes into the matched SMBIOS table entry.
+@item
+When given @option{-d}, print the value of the dword (four bytes)
+at @var{offset} bytes into the matched SMBIOS table entry.
+@item
+When given @option{-q}, print the value of the qword (eight bytes)
+at @var{offset} bytes into the matched SMBIOS table entry.
+@item
+When given @option{-s}, print the string table entry with its index found
+at @var{offset} bytes into the matched SMBIOS table entry.
+@end itemize
+
+If any of the options in the above list were given, a variable name can be
+specified with @option{-v} to store the value instead of printing it.
+
+For example, this will store and display the system manufacturer string.
+
+@example
+smbios -t 1 -s 4 -v system_manufacturer
+echo $system_manufacturer
+@end example
+@end deffn
+
+
 @node source
 @subsection source
 
grub-core/Makefile.core.def +7 -0
index 42443bc..f87de70 100644
@@ -1023,6 +1023,13 @@ module = {
 };
 
 module = {
+  name = smbios;
+  common = commands/smbios.c;
+  enable = efi;
+  enable = x86;
+};
+
+module = {
   name = suspend;
   ieee1275 = commands/ieee1275/suspend.c;
   enable = i386_ieee1275;
grub-core/commands/smbios.c new file +361 -0

361 lines not on this page · raw message

Signature · 1 line
2.1.0
#1 lore raw
Quoted 4 lines · David Michael

The following are two use cases from Rajat Jain <[email protected]>:

1) We have a board that boots Linux and this board itself can be plugged into one of different chassis types. We need to pass different parameters to the kernel based on the "CHASSIS_TYPE" information that is passed by the bios in the DMI / SMBIOS tables.

why isn't 1) already in the kernel itself?

P.

Quoted 493 lines

2) We may have a USB stick that can go into multiple boards, and the exact kernel to be loaded depends on the machine information (PRODUCT_NAME etc) passed via the DMI.

* grub-core/commands/smbios.c: New file.
* grub-core/Makefile.core.def (smbios): New module.
* docs/grub.texi (smbios): New node.
(Command-line and menu entry commands): Add a menu entry for smbios.
---

Changes since v1:

* Removed formfeeds and quoted use cases in the commit messages as suggested by Prarit Bhargava.

* Enabled building the module on all EFI platforms as suggested by Leif Lindholm.

 docs/grub.texi              |  67 ++++++++
 grub-core/Makefile.core.def |   7 +
 grub-core/commands/smbios.c | 361 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 435 insertions(+)
 create mode 100644 grub-core/commands/smbios.c
docs/grub.texi +67 -0
index 46b9e7f..f4e187f 100644
@@ -3830,6 +3830,7 @@ you forget a command, you can run the command @command{help}
 * sha256sum::                   Compute or check SHA256 hash
 * sha512sum::                   Compute or check SHA512 hash
 * sleep::                       Wait for a specified number of seconds
+* smbios::                      Retrieve SMBIOS information
 * source::                      Read a configuration file in same context
 * test::                        Check file types and compare values
 * true::                        Do nothing, successfully
@@ -4944,6 +4945,72 @@ if timeout was interrupted by @key{ESC}.
 @end deffn
 
 
+@node smbios
+@subsection smbios
+
+@deffn Command smbios @
+ [@option{-t} @var{type}] @
+ [@option{-h} @var{handle}] @
+ [@option{-m} @var{match}] @
+ [ (@option{-b} | @option{-w} | @option{-d} | @option{-q} | @option{-s}) @
+   @var{offset} [@option{-v} @var{variable}] ]
+Retrieve SMBIOS information.  This command is only available on x86 and EFI
+systems.
+
+When run with no options, @command{smbios} will print the SMBIOS table entries
+to the console as the header values, a hex dump, and a string list.  The
+following options can filter which table entries are printed.
+
+@itemize @bullet
+@item
+Specifying @option{-t} will only print entries with a matching @var{type}.  The
+type can be any value from 0 to 255.  Specify a value outside this range to
+match entries with any type.  The default is -1.
+@item
+Specifying @option{-h} will only print entries with a matching @var{handle}.
+The handle can be any value from 0 to 65535.  Specify a value outside this
+range to match entries with any handle.  The default is -1.
+@item
+Specifying @option{-m} will only print number @var{match} in the list of all
+filtered entries; e.g. @code{smbios -m 3} will print the third entry.  The list
+is always ordered the same as the hardware's SMBIOS table.  The match number
+can be any positive value.  Specify 0 to match all entries.  The default is 0.
+@end itemize
+
+The remaining options print the value of a field in the matched SMBIOS table
+entry.  Only one of these options may be specified.  When they are used, the
+option @code{-m 0} is synonymous with @code{-m 1} to match the first entry.
+
+@itemize @bullet
+@item
+When given @option{-b}, print the value of the byte
+at @var{offset} bytes into the matched SMBIOS table entry.
+@item
+When given @option{-w}, print the value of the word (two bytes)
+at @var{offset} bytes into the matched SMBIOS table entry.
+@item
+When given @option{-d}, print the value of the dword (four bytes)
+at @var{offset} bytes into the matched SMBIOS table entry.
+@item
+When given @option{-q}, print the value of the qword (eight bytes)
+at @var{offset} bytes into the matched SMBIOS table entry.
+@item
+When given @option{-s}, print the string table entry with its index found
+at @var{offset} bytes into the matched SMBIOS table entry.
+@end itemize
+
+If any of the options in the above list were given, a variable name can be
+specified with @option{-v} to store the value instead of printing it.
+
+For example, this will store and display the system manufacturer string.
+
+@example
+smbios -t 1 -s 4 -v system_manufacturer
+echo $system_manufacturer
+@end example
+@end deffn
+
+
 @node source
 @subsection source
 
grub-core/Makefile.core.def +7 -0
index 42443bc..f87de70 100644
@@ -1023,6 +1023,13 @@ module = {
 };
 
 module = {
+  name = smbios;
+  common = commands/smbios.c;
+  enable = efi;
+  enable = x86;
+};
+
+module = {
   name = suspend;
   ieee1275 = commands/ieee1275/suspend.c;
   enable = i386_ieee1275;
grub-core/commands/smbios.c new file +361 -0
new file mode 100644
index 0000000..d8edeea
@@ -0,0 +1,361 @@
+/* Expose SMBIOS data to the console and configuration files */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2013,2014,2015  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <grub/dl.h>
+#include <grub/extcmd.h>
+#include <grub/env.h>
+#include <grub/i18n.h>
+#include <grub/misc.h>
+#include <grub/mm.h>
+
+#ifdef GRUB_MACHINE_EFI
+#include <grub/efi/efi.h>
+#else
+#include <grub/acpi.h>
+#endif
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+/* Reference: DMTF Standard DSP0134 2.7.1 Table 1 */
+
+struct __attribute__ ((packed)) grub_smbios_ieps
+  {
+    grub_uint8_t  anchor[5]; /* "_DMI_" */
+    grub_uint8_t  checksum;
+    grub_uint16_t table_length;
+    grub_uint32_t table_address;
+    grub_uint16_t structures;
+    grub_uint8_t  revision;
+  };
+
+struct __attribute__ ((packed)) grub_smbios_eps
+  {
+    grub_uint8_t  anchor[4]; /* "_SM_" */
+    grub_uint8_t  checksum;
+    grub_uint8_t  length;
+    grub_uint8_t  version_major;
+    grub_uint8_t  version_minor;
+    grub_uint16_t maximum_structure_size;
+    grub_uint8_t  revision;
+    grub_uint8_t  formatted[5];
+    struct grub_smbios_ieps intermediate;
+  };
+
+static const struct grub_smbios_eps *eps = NULL;
+
+/* Reference: DMTF Standard DSP0134 2.7.1 Section 5.2.1 */
+
+/*
+ * In order for any of this module to function, it needs to find an entry point
+ * structure.  This returns a pointer to a struct that identifies the fields of
+ * the EPS, or NULL if it cannot be located.
+ */
+static const struct grub_smbios_eps *
+grub_smbios_locate_eps (void)
+{
+#ifdef GRUB_MACHINE_EFI
+  static const grub_efi_guid_t smbios_guid = GRUB_EFI_SMBIOS_TABLE_GUID;
+  const grub_efi_system_table_t *st = grub_efi_system_table;
+  const grub_efi_configuration_table_t *t = st->configuration_table;
+  unsigned int i;
+
+  for (i = 0; i < st->num_table_entries; i++)
+    if (grub_memcmp (&smbios_guid, &t->vendor_guid, sizeof (smbios_guid)) == 0)
+      {
+        grub_dprintf ("smbios", "Found entry point structure at %p\n",
+                      t->vendor_table);
+        return (const struct grub_smbios_eps *)t->vendor_table;
+      }
+    else
+      t++;
+#else
+  grub_uint8_t *ptr;
+
+  for (ptr = (grub_uint8_t *)0x000F0000;
+       ptr < (grub_uint8_t *)0x00100000;
+       ptr += 16)
+    if (grub_memcmp (ptr, "_SM_", 4) == 0
+        && grub_byte_checksum (ptr, ptr[5]) == 0)
+      {
+        grub_dprintf ("smbios", "Found entry point structure at %p\n", ptr);
+        return (const struct grub_smbios_eps *)ptr;
+      }
+#endif
+
+  grub_dprintf ("smbios", "Failed to locate entry point structure\n");
+  return NULL;
+}
+
+/* Reference: DMTF Standard DSP0134 2.7.1 Sections 6.1.2-6.1.3 */
+
+/*
+ * Given a pointer to the first bytes of an entry, print its contents in a
+ * semi-human-readable format.  Return the size of the entry printed.
+ */
+static grub_uint16_t
+grub_smbios_dump_entry (const grub_uint8_t *entry)
+{
+  const grub_uint8_t *ptr = entry;
+  grub_uint8_t newstr = 1;
+  grub_uint8_t length;
+
+  /* Write the entry's mandatory four header bytes. */
+  length = ptr[1];
+  grub_printf ("Entry: Type=0x%02x Length=0x%02x Handle=0x%04x\n",
+               ptr[0], length, *(1 + (grub_uint16_t *)ptr));
+
+  /* Dump of the formatted area (including the header) in hex. */
+  grub_printf (" Hex Dump: ");
+  while (length-- > 0)
+    grub_printf ("%02x", *ptr++);
+  grub_printf ("\n");
+
+  /* Print each string found in the appended string list. */
+  while (ptr[0] != 0 || ptr[1] != 0)
+    {
+      if (newstr)
+        grub_printf (" String: %s\n", ptr);
+      newstr = *ptr++ == 0;
+    }
+  ptr += 2;
+
+  /* Return the total number of bytes covered. */
+  return ptr - entry;
+}
+
+/* Reference: DMTF Standard DSP0134 2.7.1 Sections 6.1.2-6.1.3 */
+
+/*
+ * Return or print a matched table entry.  Multiple entries can be matched if
+ * they are being printed.
+ *
+ * This method can handle up to three criteria for selecting an entry:
+ *   - The entry's "type" field             (use outside [0,0xFF] to ignore)
+ *   - The entry's "handle" field           (use outside [0,0xFFFF] to ignore)
+ *   - The entry to return if several match (use 0 to print all matches)
+ *
+ * The parameter "print" was added for verbose debugging.  When non-zero, the
+ * matched entries are printed to the console instead of returned.
+ */
+static const grub_uint8_t *
+grub_smbios_match_entry (const struct grub_smbios_eps *ep,
+                         const grub_int16_t type,
+                         const grub_int32_t handle,
+                         const grub_uint16_t match,
+                         const grub_uint8_t print)
+{
+  grub_addr_t table_address = (grub_addr_t)ep->intermediate.table_address;
+  grub_uint16_t table_length = ep->intermediate.table_length;
+  grub_uint16_t structures = ep->intermediate.structures;
+  grub_uint16_t structure = 0;
+  grub_uint16_t matches = 0;
+  const grub_uint8_t *table = (const grub_uint8_t *)table_address;
+  const grub_uint8_t *ptr = table;
+
+  while (ptr - table < table_length && structure++ < structures)
+
+    /* Test if the current entry matches the given parameters. */
+    if ((handle < 0 || 65535 < handle || handle == *(1 + (grub_uint16_t *)ptr))
+        && (type < 0 || 255 < type || type == ptr[0])
+        && (match == 0 || match == ++matches))
+      {
+        if (print)
+          {
+            ptr += grub_smbios_dump_entry (ptr);
+            if (match > 0)
+              break;
+          }
+        else
+          return ptr;
+      }
+
+    /* If the entry didn't match, skip it (formatted area and string list). */
+    else
+      {
+        ptr += ptr[1];
+        while (*ptr++ != 0 || *ptr++ != 0);
+      }
+
+  return NULL;
+}
+
+/* Reference: DMTF Standard DSP0134 2.7.1 Section 6.1.3 */
+
+/*
+ * Given a pointer to a string list and a number, iterate over each of the
+ * strings until the desired item is reached.  (The value of 1 indicates the
+ * first string, etc.)
+ */
+static const char *
+grub_smbios_get_string (const grub_uint8_t *strings, const grub_uint8_t number)
+{
+  const char *ptr = (const char *)strings;
+  grub_uint8_t newstr = 1;
+  grub_uint8_t index = 1;
+
+  /* A string referenced with zero is interpreted as unset. */
+  if (number == 0)
+    return NULL;
+
+  /* Search the string table, incrementing the counter as each null passes. */
+  while (ptr[0] != 0 || ptr[1] != 0)
+    {
+      if (newstr && number == index++)
+        return ptr;
+      newstr = *ptr++ == 0;
+    }
+
+  /* The requested string index is greater than the number of table entries. */
+  return NULL;
+}
+
+static grub_err_t
+grub_cmd_smbios (grub_extcmd_context_t ctxt,
+                 int argc __attribute__ ((unused)),
+                 char **argv __attribute__ ((unused)))
+{
+  struct grub_arg_list *state = ctxt->state;
+
+  grub_int16_t type = -1;
+  grub_int32_t handle = -1;
+  grub_uint16_t match = 0;
+  grub_uint8_t offset = 0;
+
+  const grub_uint8_t *entry;
+  grub_uint8_t accessors;
+  grub_uint8_t i;
+  char buffer[24]; /* 64-bit number -> maximum 20 decimal digits */
+  const char *value = buffer;
+
+  if (eps == NULL)
+    return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("missing entry point"));
+
+  /* Only one value can be returned at a time; reject multiple selections. */
+  accessors = !!state[3].set + !!state[4].set + !!state[5].set +
+              !!state[6].set + !!state[7].set;
+  if (accessors > 1)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("too many -b|-w|-d|-q|-s"));
+
+  /* Reject the environment variable if no value was selected. */
+  if (accessors == 0 && state[8].set)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("-v without -b|-w|-d|-q|-s"));
+
+  /* Read the given matching parameters. */
+  if (state[0].set)
+    type = grub_strtol (state[0].arg, NULL, 0);
+  if (state[1].set)
+    handle = grub_strtol (state[1].arg, NULL, 0);
+  if (state[2].set)
+    match = grub_strtoul (state[2].arg, NULL, 0);
+
+  /* When not selecting a value, print all matching entries and quit. */
+  if (accessors == 0)
+    {
+      grub_smbios_match_entry (eps, type, handle, match, 1);
+      return GRUB_ERR_NONE;
+    }
+
+  /* Select a single entry from the matching parameters. */
+  entry = grub_smbios_match_entry (eps, type, handle, match, 0);
+  if (entry == NULL)
+    return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("no such entry"));
+
+  /* Ensure the specified offset+length is within the matched entry. */
+  for (i = 3; i <= 7; i++)
+    if (state[i].set)
+      {
+        offset = grub_strtoul (state[i].arg, NULL, 0);
+        if (offset + (1 << (i == 7 ? 0 : i - 3)) > entry[1])
+          return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("value outside entry"));
+        break;
+      }
+
+  /* If a string was requested, try to find its pointer. */
+  if (state[7].set)
+    {
+      value = grub_smbios_get_string (entry + entry[1], entry[offset]);
+      if (value == NULL)
+        return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("string not defined"));
+    }
+
+  /* Create a string from a numeric value suitable for printing. */
+  else if (state[3].set)
+    grub_snprintf (buffer, sizeof (buffer), "%u", entry[offset]);
+  else if (state[4].set)
+    grub_snprintf (buffer, sizeof (buffer), "%u",
+                   *(grub_uint16_t *)(entry + offset));
+  else if (state[5].set)
+    grub_snprintf (buffer, sizeof (buffer), "%" PRIuGRUB_UINT32_T,
+                   *(grub_uint32_t *)(entry + offset));
+  else if (state[6].set)
+    grub_snprintf (buffer, sizeof (buffer), "%" PRIuGRUB_UINT64_T,
+                   *(grub_uint64_t *)(entry + offset));
+
+  /* Store or print the requested value. */
+  if (state[8].set)
+    {
+      grub_env_set (state[8].arg, value);
+      grub_env_export (state[8].arg);
+    }
+  else
+    grub_printf ("%s\n", value);
+
+  return GRUB_ERR_NONE;
+}
+
+static grub_extcmd_t cmd;
+
+static const struct grub_arg_option options[] =
+  {
+    {"type",       't', 0, N_("Match entries with the given type."),
+                           N_("byte"), ARG_TYPE_INT},
+    {"handle",     'h', 0, N_("Match entries with the given handle."),
+                           N_("word"), ARG_TYPE_INT},
+    {"match",      'm', 0, N_("Select the entry to use when several match."),
+                           N_("number"), ARG_TYPE_INT},
+    {"get-byte",   'b', 0, N_("Get the byte's value at the given offset."),
+                           N_("offset"), ARG_TYPE_INT},
+    {"get-word",   'w', 0, N_("Get two bytes' value at the given offset."),
+                           N_("offset"), ARG_TYPE_INT},
+    {"get-dword",  'd', 0, N_("Get four bytes' value at the given offset."),
+                           N_("offset"), ARG_TYPE_INT},
+    {"get-qword",  'q', 0, N_("Get eight bytes' value at the given offset."),
+                           N_("offset"), ARG_TYPE_INT},
+    {"get-string", 's', 0, N_("Get the string specified at the given offset."),
+                           N_("offset"), ARG_TYPE_INT},
+    {"variable",   'v', 0, N_("Store the value in the given variable name."),
+                           N_("name"), ARG_TYPE_STRING},
+    {0, 0, 0, 0, 0, 0}
+  };
+
+GRUB_MOD_INIT(smbios)
+{
+  /* SMBIOS data is supposed to be static, so find it only once during init. */
+  eps = grub_smbios_locate_eps ();
+
+  cmd = grub_register_extcmd ("smbios", grub_cmd_smbios, 0,
+                              N_("[-t byte] [-h word] [-m number] "
+                                 "[(-b|-w|-d|-q|-s) offset [-v name]]"),
+                              N_("Retrieve SMBIOS information."), options);
+}
+
+GRUB_MOD_FINI(smbios)
+{
+  grub_unregister_extcmd (cmd);
+}
#2 lore raw
In reply to David Michael
Quoted 4 lines · David Michael

The following are two use cases from Rajat Jain <[email protected]>:

1) We have a board that boots Linux and this board itself can be plugged into one of different chassis types. We need to pass different parameters to the kernel based on the "CHASSIS_TYPE" information that is passed by the bios in the DMI / SMBIOS tables.

Whups ... somehow stripped all the cc's on my original reply.

why isn't 1) already in the kernel itself?

P.

#3 lore raw

Hello,

Quoted 4 lines
Quoted 1 line

1) We have a board that boots Linux and this board itself can be plugged

into one of different chassis types. We need to pass different parameters to the kernel based on the "CHASSIS_TYPE" information that is passed by the bios in the DMI / SMBIOS tables.

Actually, I had simplified the problem statement, to avoid complicating the discussion. Basically, we have enhanced the grub with a "devicetree" command that supplies a flattened device tree to the kernel (just like u-boot or others do in embedded world).

http://www.denx.de/wiki/view/DULG/LinuxFDTBlob

However, we need to pass different flattened device tree to the kernel based on different CHASSIS_TYPE. (because different chassis have different hardware components, slots etc that need to be handled differently).

Quoted 5 lines
Quoted 1 line

Whups ... somehow stripped all the cc's on my original reply.

why isn't 1) already in the kernel itself?

I hope my problem statement clarification above makes it clearer. However, I think the problem is more generic - essentially depending on the current machine type, we may have to do different things on different platforms, such as:
* Loading different kernels / initrd / device tree etc.
* Passing different kernel parameters for:
- different root fs mount points (root=/dev/sda[a/b/c/d]......)
- enabling disabling different kernel features / drivers / subsystem (pci=[nocrs/use_crs]...)
- etc.

Thanks,

Rajat

Quoted 22 lines

-----Original Message-----
From: Prarit Bhargava [mailto:[email protected]]
Sent: Monday, February 09, 2015 4:44 AM
To: David Michael
Cc: [email protected]; Leif Lindholm; Andrei Borzenkov; Rajat Jain; Sanjay Jain; Raghuraman Thirumalairajan; Stu Grossman
Subject: Re: [PATCH v2] Add a module for retrieving SMBIOS information

Quoted 3 lines · David Michael

The following are two use cases from Rajat Jain <[email protected]>:

1) We have a board that boots Linux and this board itself can be plugged

into one of different chassis types. We need to pass different parameters to the kernel based on the "CHASSIS_TYPE" information that is passed by the bios in the DMI / SMBIOS tables.

Quoted 1 line

Whups ... somehow stripped all the cc's on my original reply.

why isn't 1) already in the kernel itself?

P.

#4 lore raw
Quoted 17 lines · Rajat Jain

Hello,

Quoted 4 lines
Quoted 1 line

1) We have a board that boots Linux and this board itself can be plugged

into one of different chassis types. We need to pass different parameters to the kernel based on the "CHASSIS_TYPE" information that is passed by the bios in the DMI / SMBIOS tables.

Actually, I had simplified the problem statement, to avoid complicating the discussion. Basically, we have enhanced the grub with a "devicetree" command that supplies a flattened device tree to the kernel (just like u-boot or others do in embedded world).

http://www.denx.de/wiki/view/DULG/LinuxFDTBlob

However, we need to pass different flattened device tree to the kernel based on different CHASSIS_TYPE. (because different chassis have different hardware components, slots etc that need to be handled differently).

Quoted 4 lines
Quoted 1 line

Whups ... somehow stripped all the cc's on my original reply.

Quoted 9 lines
Quoted 1 line

why isn't 1) already in the kernel itself?

I hope my problem statement clarification above makes it clearer. However, I think the problem is more generic - essentially depending on the current machine type, we may have to do different things on different platforms, such as:
* Loading different kernels / initrd / device tree etc.
* Passing different kernel parameters for:
- different root fs mount points (root=/dev/sda[a/b/c/d]......)
- enabling disabling different kernel features / drivers / subsystem (pci=[nocrs/use_crs]...)
- etc.

Well I understand the request for different kernels, but 1) implies that you're adding additional kernel parameters based on the CHASSIS type? So I'm wondering why you're not making some boot time decision based on the CHASSIS type instead of a bootloader time decision.

/me is not against the patchset. I think the idea of booting different kernels based on the HW sounds reasonable from a administration perspective.

P.

Quoted 29 lines

Thanks,

Rajat

Quoted 22 lines

-----Original Message-----
From: Prarit Bhargava [mailto:[email protected]]
Sent: Monday, February 09, 2015 4:44 AM
To: David Michael
Cc: [email protected]; Leif Lindholm; Andrei Borzenkov; Rajat Jain; Sanjay Jain; Raghuraman Thirumalairajan; Stu Grossman
Subject: Re: [PATCH v2] Add a module for retrieving SMBIOS information

Quoted 3 lines · David Michael

The following are two use cases from Rajat Jain <[email protected]>:

1) We have a board that boots Linux and this board itself can be plugged

into one of different chassis types. We need to pass different parameters to the kernel based on the "CHASSIS_TYPE" information that is passed by the bios in the DMI / SMBIOS tables.

Quoted 1 line

Whups ... somehow stripped all the cc's on my original reply.

why isn't 1) already in the kernel itself?

P.

#5 lore raw

Hello Prarit,

Quoted 44 lines

-----Original Message-----
From: Prarit Bhargava [mailto:[email protected]]
Sent: Monday, February 09, 2015 8:29 AM
To: Rajat Jain
Cc: David Michael; [email protected]; Leif Lindholm; Andrei Borzenkov; Sanjay Jain; Raghuraman Thirumalairajan; Stu Grossman
Subject: Re: [PATCH v2] Add a module for retrieving SMBIOS information

Quoted 9 lines · Rajat Jain

Hello,

Quoted 5 lines
Quoted 2 lines

1) We have a board that boots Linux and this board itself can be plugged

into one of different chassis types. We need to pass different parameters to the kernel based on the "CHASSIS_TYPE" information that is passed by the bios in the DMI / SMBIOS tables.

Actually, I had simplified the problem statement, to avoid complicating the

discussion. Basically, we have enhanced the grub with a "devicetree" command that supplies a flattened device tree to the kernel (just like u-boot or others do in embedded world).

Quoted 4 lines

http://www.denx.de/wiki/view/DULG/LinuxFDTBlob

However, we need to pass different flattened device tree to the kernel

based on different CHASSIS_TYPE. (because different chassis have different hardware components, slots etc that need to be handled differently).

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

Whups ... somehow stripped all the cc's on my original reply.

why isn't 1) already in the kernel itself?

I hope my problem statement clarification above makes it clearer.

However, I think the problem is more generic - essentially depending on the current machine type, we may have to do different things on different platforms, such as:

Quoted 4 lines

* Loading different kernels / initrd / device tree etc.
* Passing different kernel parameters for:
- different root fs mount points (root=/dev/sda[a/b/c/d]......)
- enabling disabling different kernel features / drivers / subsystem

Quoted 8 lines

(pci=[nocrs/use_crs]...)

Quoted 2 lines

- etc.

Well I understand the request for different kernels, but 1) implies that you're adding additional kernel parameters based on the CHASSIS type? So I'm wondering why you're not making some boot time decision based on the CHASSIS type instead of a bootloader time decision.

Oh OK. I guess you are saying why not read the CHASSIS_TYPE in the kernel and take different paths depending on the value? The reason is because we may want to pass different kernel options that *exist today*. (E.g. different root file systems, or hw tuning parametes etc). Ideally once deployed, we neither want to change the bootloader, nor the kernel since that would require a recompilation which is best avoided. If something changes, it is much easier to change the grub config file which can be changed anytime, and thus we are requesting the *capability* to do it in a grub script or config file.

Quoted 3 lines

/me is not against the patchset. I think the idea of booting different kernels based on the HW sounds reasonable from a administration perspective.

Yup, understood.

Also from admin perspective, I'm suggesting that we may need tune the kernel differently depending on the HW.

Thanks,

Rajat

Quoted 33 lines

P.

Quoted 31 lines

Thanks,

Rajat

Quoted 24 lines

-----Original Message-----
From: Prarit Bhargava [mailto:[email protected]]
Sent: Monday, February 09, 2015 4:44 AM
To: David Michael
Cc: [email protected]; Leif Lindholm; Andrei Borzenkov; Rajat Jain; Sanjay Jain; Raghuraman Thirumalairajan; Stu Grossman
Subject: Re: [PATCH v2] Add a module for retrieving SMBIOS information

Quoted 4 lines · David Michael

The following are two use cases from Rajat Jain <[email protected]>:

1) We have a board that boots Linux and this board itself can be plugged

into one of different chassis types. We need to pass different parameters to the kernel based on the "CHASSIS_TYPE" information that is passed by the bios in the DMI / SMBIOS tables.

Quoted 1 line

Whups ... somehow stripped all the cc's on my original reply.

why isn't 1) already in the kernel itself?

P.

#6 lore raw
Quoted 50 lines · Rajat Jain

Hello Prarit,

Quoted 48 lines

-----Original Message-----
From: Prarit Bhargava [mailto:[email protected]]
Sent: Monday, February 09, 2015 8:29 AM
To: Rajat Jain
Cc: David Michael; [email protected]; Leif Lindholm; Andrei Borzenkov; Sanjay Jain; Raghuraman Thirumalairajan; Stu Grossman
Subject: Re: [PATCH v2] Add a module for retrieving SMBIOS information

Quoted 9 lines · Rajat Jain

Hello,

Quoted 5 lines
Quoted 2 lines

1) We have a board that boots Linux and this board itself can be plugged

into one of different chassis types. We need to pass different parameters to the kernel based on the "CHASSIS_TYPE" information that is passed by the bios in the DMI / SMBIOS tables.

Actually, I had simplified the problem statement, to avoid complicating the

discussion. Basically, we have enhanced the grub with a "devicetree" command that supplies a flattened device tree to the kernel (just like u-boot or others do in embedded world).

Quoted 4 lines

http://www.denx.de/wiki/view/DULG/LinuxFDTBlob

However, we need to pass different flattened device tree to the kernel

based on different CHASSIS_TYPE. (because different chassis have different hardware components, slots etc that need to be handled differently).

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

Whups ... somehow stripped all the cc's on my original reply.

why isn't 1) already in the kernel itself?

I hope my problem statement clarification above makes it clearer.

However, I think the problem is more generic - essentially depending on the current machine type, we may have to do different things on different platforms, such as:

Quoted 4 lines

* Loading different kernels / initrd / device tree etc.
* Passing different kernel parameters for:
- different root fs mount points (root=/dev/sda[a/b/c/d]......)
- enabling disabling different kernel features / drivers / subsystem

(pci=[nocrs/use_crs]...)

Quoted 2 lines

- etc.

Quoted 7 lines
Quoted 4 lines

Well I understand the request for different kernels, but 1) implies that you're adding additional kernel parameters based on the CHASSIS type? So I'm wondering why you're not making some boot time decision based on the CHASSIS type instead of a bootloader time decision.

Oh OK. I guess you are saying why not read the CHASSIS_TYPE in the kernel and take different paths depending on the value? The reason is because we may want to pass different kernel options that *exist today*. (E.g. different root file systems, or hw tuning parametes etc). Ideally once deployed, we neither want to change the bootloader, nor the kernel since that would require a recompilation which is best avoided. If something changes, it is much easier to change the grub config file which can be changed anytime, and thus we are requesting the *capability* to do it in a grub script or config file.

Ah, I get it. That certainly makes sense. I just thought there was some specific HW command that you always wanted executed on these boards.

Quoted 7 lines
Quoted 3 lines

/me is not against the patchset. I think the idea of booting different kernels based on the HW sounds reasonable from a administration perspective.

Yup, understood.

Also from admin perspective, I'm suggesting that we may need tune the kernel differently depending on the HW.

Thanks for the info :)

P.

Quoted 44 lines

Thanks,

Rajat

Quoted 33 lines

P.

Quoted 31 lines

Thanks,

Rajat

Quoted 24 lines

-----Original Message-----
From: Prarit Bhargava [mailto:[email protected]]
Sent: Monday, February 09, 2015 4:44 AM
To: David Michael
Cc: [email protected]; Leif Lindholm; Andrei Borzenkov; Rajat Jain; Sanjay Jain; Raghuraman Thirumalairajan; Stu Grossman
Subject: Re: [PATCH v2] Add a module for retrieving SMBIOS information

Quoted 4 lines · David Michael

The following are two use cases from Rajat Jain <[email protected]>:

1) We have a board that boots Linux and this board itself can be plugged

into one of different chassis types. We need to pass different parameters to the kernel based on the "CHASSIS_TYPE" information that is passed by the bios in the DMI / SMBIOS tables.

Quoted 1 line

Whups ... somehow stripped all the cc's on my original reply.

why isn't 1) already in the kernel itself?

P.

_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel

#7 lore raw
In reply to David Michael

В Sun, 08 Feb 2015 13:54:46 -0500
David Michael <[email protected]> пишет:

Quoted 10 lines

The following are two use cases from Rajat Jain <[email protected]>:

1) We have a board that boots Linux and this board itself can be plugged into one of different chassis types. We need to pass different parameters to the kernel based on the "CHASSIS_TYPE" information that is passed by the bios in the DMI / SMBIOS tables.

2) We may have a USB stick that can go into multiple boards, and the exact kernel to be loaded depends on the machine information (PRODUCT_NAME etc) passed via the DMI.

* grub-core/commands/smbios.c: New file.
* grub-core/Makefile.core.def (smbios): New module.
* docs/grub.texi (smbios): New node.
(Command-line and menu entry commands): Add a menu entry for smbios.

That's not needed. ChangeLog is now generated from GIT commits.

Quoted 31 lines

---

Changes since v1:

* Removed formfeeds and quoted use cases in the commit messages as suggested by Prarit Bhargava.

* Enabled building the module on all EFI platforms as suggested by Leif Lindholm.

 docs/grub.texi              |  67 ++++++++
 grub-core/Makefile.core.def |   7 +
 grub-core/commands/smbios.c | 361 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 435 insertions(+)
 create mode 100644 grub-core/commands/smbios.c
docs/grub.texi +1 -0
index 46b9e7f..f4e187f 100644
@@ -3830,6 +3830,7 @@ you forget a command, you can run the command @command{help}
 * sha256sum::                   Compute or check SHA256 hash
 * sha512sum::                   Compute or check SHA512 hash
 * sleep::                       Wait for a specified number of seconds
+* smbios::                      Retrieve SMBIOS information
 * source::                      Read a configuration file in same context
 * test::                        Check file types and compare values
 * true::                        Do nothing, successfully
@@ -4944,6 +4945,72 @@ if timeout was interrupted by @key{ESC}.
 @end deffn
 

docs/grub.texi if timeout was interrupted by @key{ESC}. · line 4956

Quoted 10 lines
docs/grub.texi +9 -0
 
+@node smbios
+@subsection smbios
+
+@deffn Command smbios @
+ [@option{-t} @var{type}] @
+ [@option{-h} @var{handle}] @
+ [@option{-m} @var{match}] @
+ [ (@option{-b} | @option{-w} | @option{-d} | @option{-q} | @option{-s}) @
+   @var{offset} [@option{-v} @var{variable}] ]

I think that in general we should endorse and advertise long options - they make scripts much more readable - and mention single letter options in description (if at all) or help output. Look e.g. at search command description.

In other places we use "--set var" to set variable. Let's remain consistent with this usage. If you want to spare some typing, make it optional, like

smbios offset [[--set] var]

docs/grub.texi if timeout was interrupted by @key{ESC}. · line 4968

Quoted 12 lines
docs/grub.texi +12 -0
+Retrieve SMBIOS information.  This command is only available on x86 and EFI
+systems.
+
+When run with no options, @command{smbios} will print the SMBIOS table entries
+to the console as the header values, a hex dump, and a string list.  The
+following options can filter which table entries are printed.
+
+@itemize @bullet
+@item
+Specifying @option{-t} will only print entries with a matching @var{type}.  The
+type can be any value from 0 to 255.  Specify a value outside this range to
+match entries with any type.  The default is -1.

I think out of range value should result in error.

docs/grub.texi if timeout was interrupted by @key{ESC}. · line 4972

Quoted 4 lines
docs/grub.texi +4 -0
+@item
+Specifying @option{-h} will only print entries with a matching @var{handle}.
+The handle can be any value from 0 to 65535.  Specify a value outside this
+range to match entries with any handle.  The default is -1.

ditto.

docs/grub.texi if timeout was interrupted by @key{ESC}. · line 4982

Quoted 10 lines
docs/grub.texi +10 -0
+@item
+Specifying @option{-m} will only print number @var{match} in the list of all
+filtered entries; e.g. @code{smbios -m 3} will print the third entry.  The list
+is always ordered the same as the hardware's SMBIOS table.  The match number
+can be any positive value.  Specify 0 to match all entries.  The default is 0.
+@end itemize
+
+The remaining options print the value of a field in the matched SMBIOS table
+entry.  Only one of these options may be specified.  When they are used, the
+option @code{-m 0} is synonymous with @code{-m 1} to match the first entry.

If user provides conflicting options this should really be an error.

Quoted 169 lines
docs/grub.texi +31 -0
+
+@itemize @bullet
+@item
+When given @option{-b}, print the value of the byte
+at @var{offset} bytes into the matched SMBIOS table entry.
+@item
+When given @option{-w}, print the value of the word (two bytes)
+at @var{offset} bytes into the matched SMBIOS table entry.
+@item
+When given @option{-d}, print the value of the dword (four bytes)
+at @var{offset} bytes into the matched SMBIOS table entry.
+@item
+When given @option{-q}, print the value of the qword (eight bytes)
+at @var{offset} bytes into the matched SMBIOS table entry.
+@item
+When given @option{-s}, print the string table entry with its index found
+at @var{offset} bytes into the matched SMBIOS table entry.
+@end itemize
+
+If any of the options in the above list were given, a variable name can be
+specified with @option{-v} to store the value instead of printing it.
+
+For example, this will store and display the system manufacturer string.
+
+@example
+smbios -t 1 -s 4 -v system_manufacturer
+echo $system_manufacturer
+@end example
+@end deffn
+
+
 @node source
 @subsection source
 
grub-core/Makefile.core.def +7 -0
index 42443bc..f87de70 100644
@@ -1023,6 +1023,13 @@ module = {
 };
 
 module = {
+  name = smbios;
+  common = commands/smbios.c;
+  enable = efi;
+  enable = x86;
+};
+
+module = {
   name = suspend;
   ieee1275 = commands/ieee1275/suspend.c;
   enable = i386_ieee1275;
grub-core/commands/smbios.c new file +111 -0
new file mode 100644
index 0000000..d8edeea
@@ -0,0 +1,361 @@
+/* Expose SMBIOS data to the console and configuration files */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2013,2014,2015  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <grub/dl.h>
+#include <grub/extcmd.h>
+#include <grub/env.h>
+#include <grub/i18n.h>
+#include <grub/misc.h>
+#include <grub/mm.h>
+
+#ifdef GRUB_MACHINE_EFI
+#include <grub/efi/efi.h>
+#else
+#include <grub/acpi.h>
+#endif
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+/* Reference: DMTF Standard DSP0134 2.7.1 Table 1 */
+
+struct __attribute__ ((packed)) grub_smbios_ieps
+  {
+    grub_uint8_t  anchor[5]; /* "_DMI_" */
+    grub_uint8_t  checksum;
+    grub_uint16_t table_length;
+    grub_uint32_t table_address;
+    grub_uint16_t structures;
+    grub_uint8_t  revision;
+  };
+
+struct __attribute__ ((packed)) grub_smbios_eps
+  {
+    grub_uint8_t  anchor[4]; /* "_SM_" */
+    grub_uint8_t  checksum;
+    grub_uint8_t  length;
+    grub_uint8_t  version_major;
+    grub_uint8_t  version_minor;
+    grub_uint16_t maximum_structure_size;
+    grub_uint8_t  revision;
+    grub_uint8_t  formatted[5];
+    struct grub_smbios_ieps intermediate;
+  };
+
+static const struct grub_smbios_eps *eps = NULL;
+
+/* Reference: DMTF Standard DSP0134 2.7.1 Section 5.2.1 */
+
+/*
+ * In order for any of this module to function, it needs to find an entry point
+ * structure.  This returns a pointer to a struct that identifies the fields of
+ * the EPS, or NULL if it cannot be located.
+ */
+static const struct grub_smbios_eps *
+grub_smbios_locate_eps (void)
+{
+#ifdef GRUB_MACHINE_EFI
+  static const grub_efi_guid_t smbios_guid = GRUB_EFI_SMBIOS_TABLE_GUID;
+  const grub_efi_system_table_t *st = grub_efi_system_table;
+  const grub_efi_configuration_table_t *t = st->configuration_table;
+  unsigned int i;
+
+  for (i = 0; i < st->num_table_entries; i++)
+    if (grub_memcmp (&smbios_guid, &t->vendor_guid, sizeof (smbios_guid)) == 0)
+      {
+        grub_dprintf ("smbios", "Found entry point structure at %p\n",
+                      t->vendor_table);
+        return (const struct grub_smbios_eps *)t->vendor_table;
+      }
+    else
+      t++;
+#else
+  grub_uint8_t *ptr;
+
+  for (ptr = (grub_uint8_t *)0x000F0000;
+       ptr < (grub_uint8_t *)0x00100000;
+       ptr += 16)
+    if (grub_memcmp (ptr, "_SM_", 4) == 0
+        && grub_byte_checksum (ptr, ptr[5]) == 0)
+      {
+        grub_dprintf ("smbios", "Found entry point structure at %p\n", ptr);
+        return (const struct grub_smbios_eps *)ptr;
+      }
+#endif
+
+  grub_dprintf ("smbios", "Failed to locate entry point structure\n");
+  return NULL;
+}
+
+/* Reference: DMTF Standard DSP0134 2.7.1 Sections 6.1.2-6.1.3 */
+
+/*
+ * Given a pointer to the first bytes of an entry, print its contents in a
+ * semi-human-readable format.  Return the size of the entry printed.
+ */
+static grub_uint16_t

grub-core/commands/smbios.c line 121

Quoted 10 lines
grub-core/commands/smbios.c new file +10 -0
+grub_smbios_dump_entry (const grub_uint8_t *entry)
+{
+  const grub_uint8_t *ptr = entry;
+  grub_uint8_t newstr = 1;
+  grub_uint8_t length;
+
+  /* Write the entry's mandatory four header bytes. */
+  length = ptr[1];
+  grub_printf ("Entry: Type=0x%02x Length=0x%02x Handle=0x%04x\n",
+               ptr[0], length, *(1 + (grub_uint16_t *)ptr));

Could you reverse the order in 1 + (grub_uint16_t *)ptr and in other places too?

grub-core/commands/smbios.c line 130

Quoted 9 lines
grub-core/commands/smbios.c new file +9 -0
+
+  /* Dump of the formatted area (including the header) in hex. */
+  grub_printf (" Hex Dump: ");
+  while (length-- > 0)
+    grub_printf ("%02x", *ptr++);
+  grub_printf ("\n");
+
+  /* Print each string found in the appended string list. */
+  while (ptr[0] != 0 || ptr[1] != 0)

I wonder do we have any ad hoc way to limit it? It looks like Linux kernel does more sanity checks and does not blindly trust data.

Quoted 36 lines
grub-core/commands/smbios.c new file +36 -0
+    {
+      if (newstr)
+        grub_printf (" String: %s\n", ptr);
+      newstr = *ptr++ == 0;
+    }
+  ptr += 2;
+
+  /* Return the total number of bytes covered. */
+  return ptr - entry;
+}
+
+/* Reference: DMTF Standard DSP0134 2.7.1 Sections 6.1.2-6.1.3 */
+
+/*
+ * Return or print a matched table entry.  Multiple entries can be matched if
+ * they are being printed.
+ *
+ * This method can handle up to three criteria for selecting an entry:
+ *   - The entry's "type" field             (use outside [0,0xFF] to ignore)
+ *   - The entry's "handle" field           (use outside [0,0xFFFF] to ignore)
+ *   - The entry to return if several match (use 0 to print all matches)
+ *
+ * The parameter "print" was added for verbose debugging.  When non-zero, the
+ * matched entries are printed to the console instead of returned.
+ */
+static const grub_uint8_t *
+grub_smbios_match_entry (const struct grub_smbios_eps *ep,
+                         const grub_int16_t type,
+                         const grub_int32_t handle,
+                         const grub_uint16_t match,
+                         const grub_uint8_t print)
+{
+  grub_addr_t table_address = (grub_addr_t)ep->intermediate.table_address;
+  grub_uint16_t table_length = ep->intermediate.table_length;
+  grub_uint16_t structures = ep->intermediate.structures;
+  grub_uint16_t structure = 0;

grub-core/commands/smbios.c line 176

Quoted 10 lines
grub-core/commands/smbios.c new file +10 -0
+  grub_uint16_t matches = 0;
+  const grub_uint8_t *table = (const grub_uint8_t *)table_address;
+  const grub_uint8_t *ptr = table;
+
+  while (ptr - table < table_length && structure++ < structures)
+
+    /* Test if the current entry matches the given parameters. */
+    if ((handle < 0 || 65535 < handle || handle == *(1 + (grub_uint16_t *)ptr))
+        && (type < 0 || 255 < type || type == ptr[0])
+        && (match == 0 || match == ++matches))

As said it is better to validate on input, so handle > 65535 etc can be omitted.

Quoted 6 lines
grub-core/commands/smbios.c new file +6 -0
+      {
+        if (print)
+          {
+            ptr += grub_smbios_dump_entry (ptr);
+            if (match > 0)
+              break;

grub-core/commands/smbios.c line 192

Quoted 10 lines
grub-core/commands/smbios.c new file +10 -0
+          }
+        else
+          return ptr;
+      }
+
+    /* If the entry didn't match, skip it (formatted area and string list). */
+    else
+      {
+        ptr += ptr[1];
+        while (*ptr++ != 0 || *ptr++ != 0);

Same here about more checks for table end.

Quoted 15 lines
grub-core/commands/smbios.c new file +15 -0
+      }
+
+  return NULL;
+}
+
+/* Reference: DMTF Standard DSP0134 2.7.1 Section 6.1.3 */
+
+/*
+ * Given a pointer to a string list and a number, iterate over each of the
+ * strings until the desired item is reached.  (The value of 1 indicates the
+ * first string, etc.)
+ */
+static const char *
+grub_smbios_get_string (const grub_uint8_t *strings, const grub_uint8_t number)
+{

grub-core/commands/smbios.c line 217

Quoted 10 lines
grub-core/commands/smbios.c new file +10 -0
+  const char *ptr = (const char *)strings;
+  grub_uint8_t newstr = 1;
+  grub_uint8_t index = 1;
+
+  /* A string referenced with zero is interpreted as unset. */
+  if (number == 0)
+    return NULL;
+
+  /* Search the string table, incrementing the counter as each null passes. */
+  while (ptr[0] != 0 || ptr[1] != 0)

And here.

Quoted 20 lines
grub-core/commands/smbios.c new file +20 -0
+    {
+      if (newstr && number == index++)
+        return ptr;
+      newstr = *ptr++ == 0;
+    }
+
+  /* The requested string index is greater than the number of table entries. */
+  return NULL;
+}
+
+static grub_err_t
+grub_cmd_smbios (grub_extcmd_context_t ctxt,
+                 int argc __attribute__ ((unused)),
+                 char **argv __attribute__ ((unused)))
+{
+  struct grub_arg_list *state = ctxt->state;
+
+  grub_int16_t type = -1;
+  grub_int32_t handle = -1;
+  grub_uint16_t match = 0;

grub-core/commands/smbios.c line 247

Quoted 10 lines
grub-core/commands/smbios.c new file +10 -0
+  grub_uint8_t offset = 0;
+
+  const grub_uint8_t *entry;
+  grub_uint8_t accessors;
+  grub_uint8_t i;
+  char buffer[24]; /* 64-bit number -> maximum 20 decimal digits */
+  const char *value = buffer;
+
+  if (eps == NULL)
+    return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("missing entry point"));

"SMBIOS table not found" or something like this, so it is more understandable.

grub-core/commands/smbios.c line 253

Quoted 6 lines
grub-core/commands/smbios.c new file +6 -0
+
+  /* Only one value can be returned at a time; reject multiple selections. */
+  accessors = !!state[3].set + !!state[4].set + !!state[5].set +
+              !!state[6].set + !!state[7].set;
+  if (accessors > 1)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("too many -b|-w|-d|-q|-s"));

Only one of ... can be specified

grub-core/commands/smbios.c line 257

Quoted 4 lines
grub-core/commands/smbios.c new file +4 -0
+
+  /* Reject the environment variable if no value was selected. */
+  if (accessors == 0 && state[8].set)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("-v without -b|-w|-d|-q|-s"));

--set requires explicit value type.

Quoted 17 lines
grub-core/commands/smbios.c new file +17 -0
+
+  /* Read the given matching parameters. */
+  if (state[0].set)
+    type = grub_strtol (state[0].arg, NULL, 0);
+  if (state[1].set)
+    handle = grub_strtol (state[1].arg, NULL, 0);
+  if (state[2].set)
+    match = grub_strtoul (state[2].arg, NULL, 0);
+
+  /* When not selecting a value, print all matching entries and quit. */
+  if (accessors == 0)
+    {
+      grub_smbios_match_entry (eps, type, handle, match, 1);
+      return GRUB_ERR_NONE;
+    }
+
+  /* Select a single entry from the matching parameters. */

grub-core/commands/smbios.c line 284

Quoted 10 lines
grub-core/commands/smbios.c new file +10 -0
+  entry = grub_smbios_match_entry (eps, type, handle, match, 0);
+  if (entry == NULL)
+    return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("no such entry"));
+
+  /* Ensure the specified offset+length is within the matched entry. */
+  for (i = 3; i <= 7; i++)
+    if (state[i].set)
+      {
+        offset = grub_strtoul (state[i].arg, NULL, 0);
+        if (offset + (1 << (i == 7 ? 0 : i - 3)) > entry[1])

Such optimizations will really strike back in future, as soon as options are changed and make code confusing. Just explicitly set entry size when parsing arguments.

Quoted 8 lines
grub-core/commands/smbios.c new file +8 -0
+          return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("value outside entry"));
+        break;
+      }
+
+  /* If a string was requested, try to find its pointer. */
+  if (state[7].set)
+    {
+      value = grub_smbios_get_string (entry + entry[1], entry[offset]);

grub-core/commands/smbios.c line 302

Quoted 10 lines
grub-core/commands/smbios.c new file +10 -0
+      if (value == NULL)
+        return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("string not defined"));
+    }
+
+  /* Create a string from a numeric value suitable for printing. */
+  else if (state[3].set)
+    grub_snprintf (buffer, sizeof (buffer), "%u", entry[offset]);
+  else if (state[4].set)
+    grub_snprintf (buffer, sizeof (buffer), "%u",
+                   *(grub_uint16_t *)(entry + offset));

Will it also work on other platforms (alignment)?

grub-core/commands/smbios.c line 305

Quoted 3 lines
grub-core/commands/smbios.c new file +3 -0
+  else if (state[5].set)
+    grub_snprintf (buffer, sizeof (buffer), "%" PRIuGRUB_UINT32_T,
+                   *(grub_uint32_t *)(entry + offset));

Ditto.

grub-core/commands/smbios.c line 309

Quoted 4 lines
grub-core/commands/smbios.c new file +4 -0
+  else if (state[6].set)
+    grub_snprintf (buffer, sizeof (buffer), "%" PRIuGRUB_UINT64_T,
+                   *(grub_uint64_t *)(entry + offset));
+

Ditto.

Quoted 52 lines
grub-core/commands/smbios.c new file +52 -0
+  /* Store or print the requested value. */
+  if (state[8].set)
+    {
+      grub_env_set (state[8].arg, value);
+      grub_env_export (state[8].arg);
+    }
+  else
+    grub_printf ("%s\n", value);
+
+  return GRUB_ERR_NONE;
+}
+
+static grub_extcmd_t cmd;
+
+static const struct grub_arg_option options[] =
+  {
+    {"type",       't', 0, N_("Match entries with the given type."),
+                           N_("byte"), ARG_TYPE_INT},
+    {"handle",     'h', 0, N_("Match entries with the given handle."),
+                           N_("word"), ARG_TYPE_INT},
+    {"match",      'm', 0, N_("Select the entry to use when several match."),
+                           N_("number"), ARG_TYPE_INT},
+    {"get-byte",   'b', 0, N_("Get the byte's value at the given offset."),
+                           N_("offset"), ARG_TYPE_INT},
+    {"get-word",   'w', 0, N_("Get two bytes' value at the given offset."),
+                           N_("offset"), ARG_TYPE_INT},
+    {"get-dword",  'd', 0, N_("Get four bytes' value at the given offset."),
+                           N_("offset"), ARG_TYPE_INT},
+    {"get-qword",  'q', 0, N_("Get eight bytes' value at the given offset."),
+                           N_("offset"), ARG_TYPE_INT},
+    {"get-string", 's', 0, N_("Get the string specified at the given offset."),
+                           N_("offset"), ARG_TYPE_INT},
+    {"variable",   'v', 0, N_("Store the value in the given variable name."),
+                           N_("name"), ARG_TYPE_STRING},
+    {0, 0, 0, 0, 0, 0}
+  };
+
+GRUB_MOD_INIT(smbios)
+{
+  /* SMBIOS data is supposed to be static, so find it only once during init. */
+  eps = grub_smbios_locate_eps ();
+
+  cmd = grub_register_extcmd ("smbios", grub_cmd_smbios, 0,
+                              N_("[-t byte] [-h word] [-m number] "
+                                 "[(-b|-w|-d|-q|-s) offset [-v name]]"),
+                              N_("Retrieve SMBIOS information."), options);
+}
+
+GRUB_MOD_FINI(smbios)
+{
+  grub_unregister_extcmd (cmd);
+}
#8 lore raw
Quoted 8 lines · Andrei Borzenkov

В Sun, 08 Feb 2015 13:54:46 -0500
David Michael <[email protected]> пишет:

docs/grub.texi if timeout was interrupted by @key{ESC}. · line 4968

Quoted 4 lines

+@item

docs/grub.texi +3 -0
+Specifying @option{-t} will only print entries with a matching @var{type}.  The
+type can be any value from 0 to 255.  Specify a value outside this range to
+match entries with any type.  The default is -1.

I think out of range value should result in error.

Okay, but I think the user should still be able to specify a value that can remove a previously specified type. Maybe a separate long option "--no-type" or a magic value like "-t any"?

Quoted 7 lines

grub-core/commands/smbios.c line 121

Quoted 4 lines
grub-core/commands/smbios.c new file +4 -0
+  /* Write the entry's mandatory four header bytes. */
+  length = ptr[1];
+  grub_printf ("Entry: Type=0x%02x Length=0x%02x Handle=0x%04x\n",
+               ptr[0], length, *(1 + (grub_uint16_t *)ptr));

Could you reverse the order in 1 + (grub_uint16_t *)ptr and in other places too?

I'll replace all these pointer casts+dereferences with functions that avoid alignment issues.

Quoted 11 lines

grub-core/commands/smbios.c line 130

Quoted 8 lines
grub-core/commands/smbios.c new file +8 -0
+  /* Dump of the formatted area (including the header) in hex. */
+  grub_printf (" Hex Dump: ");
+  while (length-- > 0)
+    grub_printf ("%02x", *ptr++);
+  grub_printf ("\n");
+
+  /* Print each string found in the appended string list. */
+  while (ptr[0] != 0 || ptr[1] != 0)

I wonder do we have any ad hoc way to limit it? It looks like Linux kernel does more sanity checks and does not blindly trust data.

The entry point structure has a two-byte field for total SMBIOS table length, so that can put an upper limit on the string sets.

Quoted 1 line

Thanks for the detailed review. I hope to be able to be able to send a new revision later in the week.

Side note: A new major version of the SMBIOS specification was posted last week which includes 64-bit addresses and such. I'm not planning to try to add this at the moment, unless anyone has a need for it.

Thanks.

David

#9 lore raw

Hello,

Just wanted to check on the status of this patch. Has it been accepted / merged?

Thanks,

Rajat

Quoted 61 lines

-----Original Message-----
From: David Michael [mailto:[email protected]]
Sent: Sunday, February 15, 2015 7:11 PM
To: Andrei Borzenkov
Cc: The development of GNU GRUB; Prarit Bhargava; Leif Lindholm; Rajat Jain; Sanjay Jain; Raghuraman Thirumalairajan; Stu Grossman
Subject: Re: [PATCH v2] Add a module for retrieving SMBIOS information

Quoted 6 lines · Andrei Borzenkov

В Sun, 08 Feb 2015 13:54:46 -0500
David Michael <[email protected]> пишет:

Review

Quoted 4 lines

+@item

(quoted diff) +3 -0
+Specifying @option{-t} will only print entries with a matching
+@var{type}.  The type can be any value from 0 to 255.  Specify a
+value outside this range to match entries with any type.  The default is -

1.

Quoted 2 lines

I think out of range value should result in error.

Okay, but I think the user should still be able to specify a value that can remove a previously specified type. Maybe a separate long option "--no- type" or a magic value like "-t any"?

Quoted 7 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+  /* Write the entry's mandatory four header bytes. */  length =
+ ptr[1];  grub_printf ("Entry: Type=0x%02x Length=0x%02x
+ Handle=0x%04x\n",
+               ptr[0], length, *(1 + (grub_uint16_t *)ptr));

Could you reverse the order in 1 + (grub_uint16_t *)ptr and in other places too?

I'll replace all these pointer casts+dereferences with functions that avoid alignment issues.

Quoted 10 lines

grub-core/commands/smbios.c

Quoted 7 lines
grub-core/commands/smbios.c new file +7 -0
+  /* Dump of the formatted area (including the header) in hex. */
+ grub_printf (" Hex Dump: ");  while (length-- > 0)
+    grub_printf ("%02x", *ptr++);
+  grub_printf ("\n");
+
+  /* Print each string found in the appended string list. */  while
+ (ptr[0] != 0 || ptr[1] != 0)

I wonder do we have any ad hoc way to limit it? It looks like Linux kernel does more sanity checks and does not blindly trust data.

The entry point structure has a two-byte field for total SMBIOS table length, so that can put an upper limit on the string sets.

Quoted 1 line

Thanks for the detailed review. I hope to be able to be able to send a new revision later in the week.

Side note: A new major version of the SMBIOS specification was posted last week which includes 64-bit addresses and such. I'm not planning to try to add this at the moment, unless anyone has a need for it.

Thanks.

David

#10 lore raw
Quoted 3 lines · Rajat Jain

Hello,

Just wanted to check on the status of this patch. Has it been accepted / merged?

No, sorry, I lost track of it. There are still a few points I need to change from Andrei's review before sending a new revision.

David