[PATCH] Add a module for retrieving SMBIOS information

[PATCH] Add a module for retrieving SMBIOS information

[David Michael]

* grub-core/commands/i386/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.
---

Hi,

There was some interest on help-grub about supporting SMBIOS access
upstream.  I've updated the module I wrote a while ago to work with
current GRUB and added documentation.  Is this acceptable to be applied?
Are there any comments or suggestions for improvement?

I have not yet gone through any copyright assignment procedure for
GRUB.  If this patch is acceptable, can anyone advise me on what is
required on that front?

Thanks.

David

 docs/grub.texi                   |  66 +++++++
 grub-core/Makefile.core.def      |   6 +
 grub-core/commands/i386/smbios.c | 368 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 440 insertions(+)
 create mode 100644 grub-core/commands/i386/smbios.c

diff --git a/docs/grub.texi b/docs/grub.texi
index 46b9e7f..ff1028f 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -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,71 @@ 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 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
 
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 42443bc..2e1e075 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -1023,6 +1023,12 @@ module = {
 };
 
 module = {
+  name = smbios;
+  common = commands/i386/smbios.c;
+  enable = x86;
+};
+
+module = {
   name = suspend;
   ieee1275 = commands/ieee1275/suspend.c;
   enable = i386_ieee1275;
diff --git a/grub-core/commands/i386/smbios.c b/grub-core/commands/i386/smbios.c
new file mode 100644
index 0000000..25f20fe
--- /dev/null
+++ b/grub-core/commands/i386/smbios.c
@@ -0,0 +1,368 @@
+/* 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+");
+
+\f
+/* 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;
+
+\f
+/* 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;
+}
+
+\f
+/* 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;
+}
+
+\f
+/* 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_uint32_t table_address = 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;
+}
+
+\f
+/* 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;
+}
+
+\f
+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;
+}
+
+\f
+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 referenced 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.1.0

Re: [PATCH] Add a module for retrieving SMBIOS information

[Prarit Bhargava]

On 02/01/2015 09:05 PM, David Michael wrote:
> * grub-core/commands/i386/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.
> ---
> 
> Hi,
> 
> There was some interest on help-grub about supporting SMBIOS access
> upstream.  

OOC, why?  Why would you need to do this?  I'm certainly not against doing this
but just wondering exactly why you want to do this.


P.


I've updated the module I wrote a while ago to work with
> current GRUB and added documentation.  Is this acceptable to be applied?
> Are there any comments or suggestions for improvement?
> 
> I have not yet gone through any copyright assignment procedure for
> GRUB.  If this patch is acceptable, can anyone advise me on what is
> required on that front?
> 
> Thanks.
> 
> David
> 
>  docs/grub.texi                   |  66 +++++++
>  grub-core/Makefile.core.def      |   6 +
>  grub-core/commands/i386/smbios.c | 368 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 440 insertions(+)
>  create mode 100644 grub-core/commands/i386/smbios.c
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 46b9e7f..ff1028f 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -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,71 @@ 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 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
>  
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 42443bc..2e1e075 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1023,6 +1023,12 @@ module = {
>  };
>  
>  module = {
> +  name = smbios;
> +  common = commands/i386/smbios.c;
> +  enable = x86;
> +};
> +
> +module = {
>    name = suspend;
>    ieee1275 = commands/ieee1275/suspend.c;
>    enable = i386_ieee1275;
> diff --git a/grub-core/commands/i386/smbios.c b/grub-core/commands/i386/smbios.c
> new file mode 100644
> index 0000000..25f20fe
> --- /dev/null
> +++ b/grub-core/commands/i386/smbios.c
> @@ -0,0 +1,368 @@
> +/* 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+");
> +
> +\f
> +/* 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;
> +
> +\f
> +/* 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;
> +}
> +
> +\f
> +/* 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;
> +}
> +
> +\f
> +/* 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_uint32_t table_address = 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;
> +}
> +
> +\f
> +/* 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;
> +}
> +
> +\f
> +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;
> +}
> +
> +\f
> +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 referenced 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);
> +}
> 

Re: [PATCH] Add a module for retrieving SMBIOS information

[David Michael]

On Mon, Feb 2, 2015 at 6:20 AM, Prarit Bhargava <prarit@redhat.com> wrote:
> On 02/01/2015 09:05 PM, David Michael wrote:
>> * grub-core/commands/i386/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.
>> ---
>>
>> Hi,
>>
>> There was some interest on help-grub about supporting SMBIOS access
>> upstream.
>
> OOC, why?  Why would you need to do this?  I'm certainly not against doing this
> but just wondering exactly why you want to do this.

The thread on grub-help asked about booting particular kernel versions
off a hot-pluggable drive based on the detected hardware, which this
would allow.

I originally wrote it to change what options are available based on
whether a disk is being booted physically or virtually.  Since QEMU
makes it easy to add SMBIOS entries on the command line, I've also
been using it for random tweaks like showing a vga_text boot menu
instead of gfxterm when running QEMU with "-display curses".

Thanks.

David

RE: [PATCH] Add a module for retrieving SMBIOS information

[Rajat Jain]

Hello,

> -----Original Message-----
> From: David Michael [mailto:fedora.dm0@gmail.com]
> Sent: Monday, February 02, 2015 9:10 AM
> To: Prarit Bhargava
> Cc: The development of GNU GRUB; Andrei Borzenkov; Rajat Jain; Stu
> Grossman; Raghuraman Thirumalairajan; Sanjay Jain
> Subject: Re: [PATCH] Add a module for retrieving SMBIOS information
> 
> On Mon, Feb 2, 2015 at 6:20 AM, Prarit Bhargava <prarit@redhat.com>
> wrote:
> > On 02/01/2015 09:05 PM, David Michael wrote:
> >> * grub-core/commands/i386/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.
> >> ---
> >>
> >> Hi,
> >>
> >> There was some interest on help-grub about supporting SMBIOS access
> >> upstream.


I think having the capability to parse the SMBIOS data is certainly very interesting to us, and currently we have the following use cases to make use of it:

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. 

I hope that clarifies our usecase and needs where this functionality will be immensely helpful.

Thanks,

Rajat




> >
> > OOC, why?  Why would you need to do this?  I'm certainly not against
> > doing this but just wondering exactly why you want to do this.
> 
> The thread on grub-help asked about booting particular kernel versions off a
> hot-pluggable drive based on the detected hardware, which this would
> allow.
> 
> I originally wrote it to change what options are available based on whether a
> disk is being booted physically or virtually.  Since QEMU makes it easy to add
> SMBIOS entries on the command line, I've also been using it for random
> tweaks like showing a vga_text boot menu instead of gfxterm when running
> QEMU with "-display curses".
> 
> Thanks.
> 
> David

Re: [PATCH] Add a module for retrieving SMBIOS information

[Prarit Bhargava]

On 02/02/2015 01:01 PM, Rajat Jain wrote:
> Hello,
> 
>> -----Original Message-----
>> From: David Michael [mailto:fedora.dm0@gmail.com]
>> Sent: Monday, February 02, 2015 9:10 AM
>> To: Prarit Bhargava
>> Cc: The development of GNU GRUB; Andrei Borzenkov; Rajat Jain; Stu
>> Grossman; Raghuraman Thirumalairajan; Sanjay Jain
>> Subject: Re: [PATCH] Add a module for retrieving SMBIOS information
>>
>> On Mon, Feb 2, 2015 at 6:20 AM, Prarit Bhargava <prarit@redhat.com>
>> wrote:
>>> On 02/01/2015 09:05 PM, David Michael wrote:
>>>> * grub-core/commands/i386/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.
>>>> ---
>>>>
>>>> Hi,
>>>>
>>>> There was some interest on help-grub about supporting SMBIOS access
>>>> upstream.
> 
> 
> I think having the capability to parse the SMBIOS data is certainly very interesting to us, and currently we have the following use cases to make use of it:
> 
> 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. 
> 
> I hope that clarifies our usecase and needs where this functionality will be immensely helpful.
> 
> Thanks,

Thanks Rajat, that does really explain things.  I'll take a closer look in the
next few days ...

P.

> 
> Rajat
> 
> 
> 
> 
>>>
>>> OOC, why?  Why would you need to do this?  I'm certainly not against
>>> doing this but just wondering exactly why you want to do this.
>>
>> The thread on grub-help asked about booting particular kernel versions off a
>> hot-pluggable drive based on the detected hardware, which this would
>> allow.
>>
>> I originally wrote it to change what options are available based on whether a
>> disk is being booted physically or virtually.  Since QEMU makes it easy to add
>> SMBIOS entries on the command line, I've also been using it for random
>> tweaks like showing a vga_text boot menu instead of gfxterm when running
>> QEMU with "-display curses".
>>
>> Thanks.
>>
>> David

Re: [PATCH] Add a module for retrieving SMBIOS information

[Prarit Bhargava]

On 02/02/2015 12:09 PM, David Michael wrote:
> On Mon, Feb 2, 2015 at 6:20 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>> On 02/01/2015 09:05 PM, David Michael wrote:
>>> * grub-core/commands/i386/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.
>>> ---
>>>
>>> Hi,
>>>
>>> There was some interest on help-grub about supporting SMBIOS access
>>> upstream.
>>
>> OOC, why?  Why would you need to do this?  I'm certainly not against doing this
>> but just wondering exactly why you want to do this.
> 
> The thread on grub-help asked about booting particular kernel versions
> off a hot-pluggable drive based on the detected hardware, which this
> would allow.
> 
> I originally wrote it to change what options are available based on
> whether a disk is being booted physically or virtually.  Since QEMU
> makes it easy to add SMBIOS entries on the command line, I've also
> been using it for random tweaks like showing a vga_text boot menu
> instead of gfxterm when running QEMU with "-display curses".
> 

Ah interesting David -- and good job on getting the efi.smbios stuff in there
too as that's an easy thing to miss.  I'll take a closer look ...

P.

> Thanks.
> 
> David
> 

Re: [PATCH] Add a module for retrieving SMBIOS information

[Leif Lindholm]

On Sun, Feb 01, 2015 at 09:05:24PM -0500, David Michael wrote:
> * grub-core/commands/i386/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.
> ---
> 
> Hi,
> 
> There was some interest on help-grub about supporting SMBIOS access
> upstream.  I've updated the module I wrote a while ago to work with
> current GRUB and added documentation.  Is this acceptable to be applied?
> Are there any comments or suggestions for improvement?

Any particular reason for making it x86-only?
Since the code correctly does _not_ scan memory for the tables on any
EFI system, this should work without modification on arm/arm64 too.
 
> I have not yet gone through any copyright assignment procedure for
> GRUB.  If this patch is acceptable, can anyone advise me on what is
> required on that front?
> 
> Thanks.
> 
> David
> 
>  docs/grub.texi                   |  66 +++++++
>  grub-core/Makefile.core.def      |   6 +
>  grub-core/commands/i386/smbios.c | 368 +++++++++++++++++++++++++++++++++++++++

grub-core/commands/smbios.c

>  3 files changed, 440 insertions(+)
>  create mode 100644 grub-core/commands/i386/smbios.c
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 46b9e7f..ff1028f 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -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,71 @@ 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 systems.

x86 and EFI.
Is it valid on i386-of 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
>  
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 42443bc..2e1e075 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1023,6 +1023,12 @@ module = {
>  };
>  
>  module = {
> +  name = smbios;
> +  common = commands/i386/smbios.c;

commands/smbios.c

> +  enable = x86;
also
     enable = efi;

> +};
> +
> +module = {
>    name = suspend;
>    ieee1275 = commands/ieee1275/suspend.c;
>    enable = i386_ieee1275;
> diff --git a/grub-core/commands/i386/smbios.c b/grub-core/commands/i386/smbios.c
> new file mode 100644
> index 0000000..25f20fe
> --- /dev/null
> +++ b/grub-core/commands/i386/smbios.c
> @@ -0,0 +1,368 @@
> +/* 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+");
> +
> +\f
> +/* 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;
> +
> +\f
> +/* 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;
> +}
> +
> +\f
> +/* 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;
> +}
> +
> +\f
> +/* 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_uint32_t table_address = 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;
> +}
> +
> +\f
> +/* 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;
> +}
> +
> +\f
> +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;
> +}
> +
> +\f
> +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 referenced 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.1.0
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Add a module for retrieving SMBIOS information

[David Michael]

On Mon, Feb 2, 2015 at 3:06 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Sun, Feb 01, 2015 at 09:05:24PM -0500, David Michael wrote:
>> * grub-core/commands/i386/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.
>> ---
>>
>> Hi,
>>
>> There was some interest on help-grub about supporting SMBIOS access
>> upstream.  I've updated the module I wrote a while ago to work with
>> current GRUB and added documentation.  Is this acceptable to be applied?
>> Are there any comments or suggestions for improvement?
>
> Any particular reason for making it x86-only?
> Since the code correctly does _not_ scan memory for the tables on any
> EFI system, this should work without modification on arm/arm64 too.

It's marked as x86-only mostly because that's all I've tried, and I
didn't want to accidentally break builds for other configurations.  I
don't have any ARM hardware for testing the module.  Maybe an emulator
would suffice; I'll have to look into it.

Are you able to try the module with an ARM system?  I'd be happy to
update the patch with your suggested changes if it works.

Thanks.

David

Re: [PATCH] Add a module for retrieving SMBIOS information

[Leif Lindholm]

On Mon, Feb 02, 2015 at 05:01:32PM -0500, David Michael wrote:
> >> There was some interest on help-grub about supporting SMBIOS access
> >> upstream.  I've updated the module I wrote a while ago to work with
> >> current GRUB and added documentation.  Is this acceptable to be applied?
> >> Are there any comments or suggestions for improvement?
> >
> > Any particular reason for making it x86-only?
> > Since the code correctly does _not_ scan memory for the tables on any
> > EFI system, this should work without modification on arm/arm64 too.
> 
> It's marked as x86-only mostly because that's all I've tried, and I
> didn't want to accidentally break builds for other configurations.  I
> don't have any ARM hardware for testing the module.  Maybe an emulator
> would suffice; I'll have to look into it.
> 
> Are you able to try the module with an ARM system?  I'd be happy to
> update the patch with your suggested changes if it works.

I have a software model with some dummy tables, and with one
modification (below - one I think will be necessary for amd64 too),
the command seems to be functional.

I'm hoping we'll get around to implementing SMBIOS support for arm64
QEMU shortly, so that these things will be easier for others to
test. Or if you'd be interested, I could show you how to test it on
the ARM Foundation Model.

/
    Leif


diff --git a/grub-core/commands/i386/smbios.c
b/grub-core/commands/i386/smbios.c
index 25f20fe..40dae88 100644
--- a/grub-core/commands/i386/smbios.c
+++ b/grub-core/commands/i386/smbios.c
@@ -164,7 +164,7 @@ grub_smbios_match_entry (const struct
grub_smbios_eps *ep,
                          const grub_uint16_t match,
                          const grub_uint8_t print)
 {
-  grub_uint32_t table_address = ep->intermediate.table_address;
+  grub_addr_t table_address = 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;

Re: [PATCH] Add a module for retrieving SMBIOS information

[Prarit Bhargava]

On 02/02/2015 02:26 PM, Prarit Bhargava wrote:
> 
> 
> On 02/02/2015 12:09 PM, David Michael wrote:
>> On Mon, Feb 2, 2015 at 6:20 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>>> On 02/01/2015 09:05 PM, David Michael wrote:
>>>> * grub-core/commands/i386/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.
>>>> ---
>>>>
>>>> Hi,
>>>>
>>>> There was some interest on help-grub about supporting SMBIOS access
>>>> upstream.
>>>
>>> OOC, why?  Why would you need to do this?  I'm certainly not against doing this
>>> but just wondering exactly why you want to do this.
>>
>> The thread on grub-help asked about booting particular kernel versions
>> off a hot-pluggable drive based on the detected hardware, which this
>> would allow.
>>
>> I originally wrote it to change what options are available based on
>> whether a disk is being booted physically or virtually.  Since QEMU
>> makes it easy to add SMBIOS entries on the command line, I've also
>> been using it for random tweaks like showing a vga_text boot menu
>> instead of gfxterm when running QEMU with "-display curses".
>>
> 
> Ah interesting David -- and good job on getting the efi.smbios stuff in there
> too as that's an easy thing to miss.  I'll take a closer look ...
> 

FWIW, I think it looks fine and it definitely has a valid use case.  I'd suggest
that you update the description with Rajat's comment.

One odd thing in the patch (and it may be something weird on my end that I've
never seen before).  When I saved and applied your patch via 'git am', the patch
contained a few "^L" lines.  For example

^L
/* Reference: DMTF Standard DSP0134 2.7.1 Section 6.1.3 */


Not sure why that happened, but I do see them in the patch as well as the email
you sent to the list.

P.

Re: [PATCH] Add a module for retrieving SMBIOS information

[David Michael]

On Tue, Feb 3, 2015 at 10:04 AM, Prarit Bhargava <prarit@redhat.com> wrote:
> On 02/02/2015 02:26 PM, Prarit Bhargava wrote:
>>
>>
>> On 02/02/2015 12:09 PM, David Michael wrote:
>>> On Mon, Feb 2, 2015 at 6:20 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>>>> On 02/01/2015 09:05 PM, David Michael wrote:
>>>>> * grub-core/commands/i386/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.
>>>>> ---
>>>>>
>>>>> Hi,
>>>>>
>>>>> There was some interest on help-grub about supporting SMBIOS access
>>>>> upstream.
>>>>
>>>> OOC, why?  Why would you need to do this?  I'm certainly not against doing this
>>>> but just wondering exactly why you want to do this.
>>>
>>> The thread on grub-help asked about booting particular kernel versions
>>> off a hot-pluggable drive based on the detected hardware, which this
>>> would allow.
>>>
>>> I originally wrote it to change what options are available based on
>>> whether a disk is being booted physically or virtually.  Since QEMU
>>> makes it easy to add SMBIOS entries on the command line, I've also
>>> been using it for random tweaks like showing a vga_text boot menu
>>> instead of gfxterm when running QEMU with "-display curses".
>>>
>>
>> Ah interesting David -- and good job on getting the efi.smbios stuff in there
>> too as that's an easy thing to miss.  I'll take a closer look ...
>>
>
> FWIW, I think it looks fine and it definitely has a valid use case.  I'd suggest
> that you update the description with Rajat's comment.

Okay, to be clear, by "description" here do you mean to put the use
case in the commit message?

> One odd thing in the patch (and it may be something weird on my end that I've
> never seen before).  When I saved and applied your patch via 'git am', the patch
> contained a few "^L" lines.

Yes, I used formfeeds to follow the GNU coding standards document when
I first wrote the module.  I'll take them out of the updated patch.

Thanks.

David

Re: [PATCH] Add a module for retrieving SMBIOS information

[David Michael]

On Mon, Feb 2, 2015 at 5:48 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Mon, Feb 02, 2015 at 05:01:32PM -0500, David Michael wrote:
>> >> There was some interest on help-grub about supporting SMBIOS access
>> >> upstream.  I've updated the module I wrote a while ago to work with
>> >> current GRUB and added documentation.  Is this acceptable to be applied?
>> >> Are there any comments or suggestions for improvement?
>> >
>> > Any particular reason for making it x86-only?
>> > Since the code correctly does _not_ scan memory for the tables on any
>> > EFI system, this should work without modification on arm/arm64 too.
>>
>> It's marked as x86-only mostly because that's all I've tried, and I
>> didn't want to accidentally break builds for other configurations.  I
>> don't have any ARM hardware for testing the module.  Maybe an emulator
>> would suffice; I'll have to look into it.
>>
>> Are you able to try the module with an ARM system?  I'd be happy to
>> update the patch with your suggested changes if it works.
>
> I have a software model with some dummy tables, and with one
> modification (below - one I think will be necessary for amd64 too),
> the command seems to be functional.

Okay, thanks, I'll send an updated patch in the near future with your
suggested changes.

> I'm hoping we'll get around to implementing SMBIOS support for arm64
> QEMU shortly, so that these things will be easier for others to
> test. Or if you'd be interested, I could show you how to test it on
> the ARM Foundation Model.

I can check out the ARM Foundation Model until QEMU is ready.  I
haven't used it before, so if you've already got a working setup, I'd
be interested in seeing your testing instructions to get started.

Thanks.

David

Re: [PATCH] Add a module for retrieving SMBIOS information

[Prarit Bhargava]

On 02/03/2015 01:41 PM, David Michael wrote:
> On Tue, Feb 3, 2015 at 10:04 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>> On 02/02/2015 02:26 PM, Prarit Bhargava wrote:
>>>
>>>
>>> On 02/02/2015 12:09 PM, David Michael wrote:
>>>> On Mon, Feb 2, 2015 at 6:20 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>>>>> On 02/01/2015 09:05 PM, David Michael wrote:
>>>>>> * grub-core/commands/i386/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.
>>>>>> ---
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> There was some interest on help-grub about supporting SMBIOS access
>>>>>> upstream.
>>>>>
>>>>> OOC, why?  Why would you need to do this?  I'm certainly not against doing this
>>>>> but just wondering exactly why you want to do this.
>>>>
>>>> The thread on grub-help asked about booting particular kernel versions
>>>> off a hot-pluggable drive based on the detected hardware, which this
>>>> would allow.
>>>>
>>>> I originally wrote it to change what options are available based on
>>>> whether a disk is being booted physically or virtually.  Since QEMU
>>>> makes it easy to add SMBIOS entries on the command line, I've also
>>>> been using it for random tweaks like showing a vga_text boot menu
>>>> instead of gfxterm when running QEMU with "-display curses".
>>>>
>>>
>>> Ah interesting David -- and good job on getting the efi.smbios stuff in there
>>> too as that's an easy thing to miss.  I'll take a closer look ...
>>>
>>
>> FWIW, I think it looks fine and it definitely has a valid use case.  I'd suggest
>> that you update the description with Rajat's comment.
> 
> Okay, to be clear, by "description" here do you mean to put the use
> case in the commit message?

Yes, I find it useful to note the use case.  Later on if someone wants to make a
change to the module we'll know exactly why it was created.

^^^ The above is IMO and has become a sort of standard for other projects.  The
maintainer here may not like it ... but a few extra sentences in the commit
message can't hurt anything ;)

> 
>> One odd thing in the patch (and it may be something weird on my end that I've
>> never seen before).  When I saved and applied your patch via 'git am', the patch
>> contained a few "^L" lines.
> 
> Yes, I used formfeeds to follow the GNU coding standards document when
> I first wrote the module.  I'll take them out of the updated patch.

Thanks!

P.

> 
> Thanks.
> 
> David
>