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

[Rajat Jain]

[+Arthur]

Hello Folks,

I'm trying to get a binary / raw copy of this patch but could not find one. 
David, is it possible to please send us a binary / raw copy of this patch?

Thanks,

Rajat

> -----Original Message-----
> From: Andrei Borzenkov [mailto:address@hidden
> Sent: Friday, March 27, 2015 8:24 AM
> To: David Michael
> Cc: address@hidden; Rajat Jain; Prarit Bhargava; Leif Lindholm; Sanjay
> Jain; Raghuraman Thirumalairajan; Stu Grossman
> Subject: Re: [PATCH v3] Add a module for retrieving SMBIOS information
> 
> В Sun, 22 Mar 2015 22:01:49 -0400
> David Michael <address@hidden> пишет:
> > +struct __attribute__ ((packed)) grub_smbios_eps
> > +  {
> > +    grub_uint8_t  anchor[4]; /* "_SM_" */
> 
> any plans to implement SMBIOS 3.0 (64 bit address) support?
> 
> > +    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;  };
> > +
> > +#define eps_table_begin(eps)
> > +((grub_addr_t)((eps)->intermediate.table_address))
> > +#define eps_table_end(eps) \
> > +  ((grub_addr_t)((eps)->intermediate.table_address + \
> > +                 (eps)->intermediate.table_length))
> > +
> 
> To make adding 64 bit SMBIOS easier, may be extract entry point and size
> (and other relevant fields) instead of referring to them directly.
> Then we'd just need to add additional search for 3.0 entry point and all other
> code won't need to be changed - tables themselves remain the same as far
> as I can tell.
> ...
> 
> > +
> > +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;
> > +
> > +  grub_int32_t option;
> > +  const grub_uint8_t *structure;
> > +  grub_uint8_t accessors;
> > +  grub_uint8_t i;
> > +  char buffer[24]; /* 64-bit number -> maximum 20 decimal digits */
> > + const char *value = buffer;
> > +
> 
> Could we avoid this aliasing? It is extremely confusing to see buffer used
> everywhere and then suddenly value in the last line. What is the reason?
> > +
> > +  /* Store or print the requested value. */  if (state[8].set)
> > +    {
> > +      grub_env_set (state[8].arg, value);
> > +      grub_env_export (state[8].arg);
> 
> Why export variable here? It is up to user what to do with it later.
> 
> > +static const struct grub_arg_option options[] =
> > +  {
> > +    {"type",       't', 0, N_("Match entries with the given type."),
> > +                           N_("type"), ARG_TYPE_INT},
> > +    {"handle",     'h', 0, N_("Match entries with the given handle."),
> > +                           N_("handle"), ARG_TYPE_INT},
> > +    {"match",      'm', 0, N_("Select a structure when several match."),
> > +                           N_("match"), 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},
> > +    {"set",       '\0', 0, N_("Store the value in the given variable 
> > name."),
> > +                           N_("variable"), ARG_TYPE_STRING},
> > +    {0, 0, 0, 0, 0, 0}
> > +  };
> 
> One non-trivial structure field that is rather awkward to get otherwise is
> UUID.