Re: [dmidecode] [PATCH RFC] dmidecode: Add a --dumb option

[lijiang]

Hi, Jean

在 2020年11月10日 17:39, Jean Delvare 写道:
> This new option is aimed at firmware developers to help them validate
> their work. When this option is used, quirks and fixups are disabled
> in dmidecode, which will dumbly decode everything found in the table

Seems this should be a raw data dump, not sure if it is more reasonable
to use the --dump-raw option, what do you think?

> even if it is known to be incorrect.
> 
> Signed-off-by: Jean Delvare <jdelvare@suse.de>
> ---
> I think that would be useful functionality, which can be extended over
> time. The only thing I'm slightly worried about is the name. "Dumb" is
> pretty close to "dump", which could cause some confusion. But "dump"
> describes really well what the option is doing and I couldn't come up
> with a better name. Suggestions welcome.
> 
>  dmidecode.c     |   50 ++++++++++++++++++++++++++++----------------------
>  dmiopt.c        |    5 +++++
>  dmiopt.h        |    1 +
>  man/dmidecode.8 |    5 +++++
>  4 files changed, 39 insertions(+), 22 deletions(-)
> 
> --- dmidecode.orig/dmidecode.c        2020-10-30 15:53:40.995588100 +0100
> +++ dmidecode/dmidecode.c     2020-10-30 16:04:28.977359891 +0100
> @@ -2541,7 +2541,7 @@ static void dmi_memory_device_width(cons
>       /*
>        * If no memory module is present, width may be 0
>        */
> -     if (code == 0xFFFF || code == 0)
> +     if (code == 0xFFFF || (code == 0 && !(opt.flags & FLAG_DUMB)))
>               pr_attr(attr, "Unknown");
>       else
>               pr_attr(attr, "%u bits", code);
> @@ -4485,7 +4485,7 @@ static void dmi_decode(const struct dmi_
>                       dmi_memory_device_type_detail(WORD(data + 0x13));
>                       if (h->length < 0x17) break;
>                       /* If no module is present, the remaining fields are 
> irrelevant */
> -                     if (WORD(data + 0x0C) == 0)
> +                     if (WORD(data + 0x0C) == 0 && !(opt.flags & FLAG_DUMB))
>                               break;
>                       dmi_memory_device_speed("Speed", WORD(data + 0x15),
>                                               h->length >= 0x5C ?
> @@ -5246,7 +5246,7 @@ static void dmi_table_decode(u8 *buf, u3
>                       dmi_set_vendor(dmi_string(&h, data[0x04]));
>  
>               /* Fixup a common mistake */
> -             if (h.type == 34)
> +             if (h.type == 34 && !(opt.flags & FLAG_DUMB))
>                       dmi_fixup_type_34(&h, display);
>  
>               if (display)
> @@ -5436,6 +5436,29 @@ static int smbios3_decode(u8 *buf, const
>       return 1;
>  }
>  
> +static void dmi_fixup_version(u16 *ver)
> +{
> +     /* Some BIOS report weird SMBIOS version, fix that up */
> +     switch (*ver)
> +     {
> +             case 0x021F:
> +             case 0x0221:
> +                     if (!(opt.flags & FLAG_QUIET))
> +                             fprintf(stderr,
> +                                     "SMBIOS version fixup (2.%d -> 
> 2.%d).\n",
> +                                     *ver & 0xFF, 3);
> +                     *ver = 0x0203;
> +                     break;
> +             case 0x0233:
> +                     if (!(opt.flags & FLAG_QUIET))
> +                             fprintf(stderr,
> +                                     "SMBIOS version fixup (2.%d -> 
> 2.%d).\n",
> +                                     51, 6);
> +                     *ver = 0x0206;
> +                     break;
> +     }
> +}
> +
>  static int smbios_decode(u8 *buf, const char *devmem, u32 flags)
>  {
>       u16 ver;
> @@ -5455,25 +5478,8 @@ static int smbios_decode(u8 *buf, const
>               return 0;
>  
>       ver = (buf[0x06] << 8) + buf[0x07];
> -     /* Some BIOS report weird SMBIOS version, fix that up */
> -     switch (ver)
> -     {
> -             case 0x021F:
> -             case 0x0221:
> -                     if (!(opt.flags & FLAG_QUIET))
> -                             fprintf(stderr,
> -                                     "SMBIOS version fixup (2.%d -> 
> 2.%d).\n",
> -                                     ver & 0xFF, 3);
> -                     ver = 0x0203;
> -                     break;
> -             case 0x0233:
> -                     if (!(opt.flags & FLAG_QUIET))
> -                             fprintf(stderr,
> -                                     "SMBIOS version fixup (2.%d -> 
> 2.%d).\n",
> -                                     51, 6);
> -                     ver = 0x0206;
> -                     break;
> -     }
> +     if (!(opt.flags & FLAG_DUMB))
> +             dmi_fixup_version(&ver);
>       if (!(opt.flags & FLAG_QUIET))
>               pr_info("SMBIOS %u.%u present.",
>                       ver >> 8, ver & 0xFF);
> --- dmidecode.orig/dmiopt.c   2020-10-30 15:53:40.995588100 +0100
> +++ dmidecode/dmiopt.c        2020-10-30 16:05:16.242931796 +0100
> @@ -270,6 +270,7 @@ int parse_command_line(int argc, char *
>               { "dev-mem", required_argument, NULL, 'd' },
>               { "help", no_argument, NULL, 'h' },
>               { "quiet", no_argument, NULL, 'q' },
> +             { "dumb", no_argument, NULL, 'D' },
>               { "string", required_argument, NULL, 's' },
>               { "type", required_argument, NULL, 't' },
>               { "dump", no_argument, NULL, 'u' },
> @@ -302,6 +303,9 @@ int parse_command_line(int argc, char *
>                       case 'q':
>                               opt.flags |= FLAG_QUIET;
>                               break;
> +                     case 'D':
> +                             opt.flags |= FLAG_DUMB;
> +                             break;
>                       case 's':
>                               if (parse_opt_string(optarg) < 0)
>                                       return -1;
> @@ -371,6 +375,7 @@ void print_help(void)
>               " -d, --dev-mem FILE     Read memory from device FILE (default: 
> " DEFAULT_MEM_DEV ")\n"
>               " -h, --help             Display this help text and exit\n"
>               " -q, --quiet            Less verbose output\n"
> +             "     --dumb             Decode everything without quirks\n"
>               " -s, --string KEYWORD   Only display the value of the given 
> DMI string\n"
>               " -t, --type TYPE        Only display the entries of given 
> type\n"
>               " -H, --handle HANDLE    Only display the entry of given 
> handle\n"
> --- dmidecode.orig/dmiopt.h   2020-10-30 15:53:40.995588100 +0100
> +++ dmidecode/dmiopt.h        2020-10-30 15:53:52.772729848 +0100
> @@ -46,6 +46,7 @@ extern struct opt opt;
>  #define FLAG_DUMP_BIN           (1 << 4)
>  #define FLAG_FROM_DUMP          (1 << 5)
>  #define FLAG_NO_SYSFS           (1 << 6)
> +#define FLAG_DUMB               (1 << 7)

#define FLAG_DUMP_RAW              (1 << 7)

Maybe all codes can be changed like the above.

Thanks.
Lianbo

>  
>  int parse_command_line(int argc, char * const argv[]);
>  void print_help(void);
> --- dmidecode.orig/man/dmidecode.8    2020-10-30 15:53:40.995588100 +0100
> +++ dmidecode/man/dmidecode.8 2020-10-30 16:08:00.403918117 +0100
> @@ -70,6 +70,11 @@ Read memory from device \fBFILE\fR (defa
>  Be less verbose. Unknown, inactive and \s-1OEM\s0-specific entries are not
>  displayed. Meta-data and handle references are hidden.
>  .TP
> +.BR "  " "  " "--dumb"
> +Decode everything exactly as it is in the table, without trying to fix up
> +common mistakes or hide irrelevant fields.
> +This mode is primarily aimed at firmware developers.
> +.TP
>  .BR "-s" ", " "--string KEYWORD"
>  Only display the value of the \s-1DMI\s0 string identified by \fBKEYWORD\fR.
>  \fBKEYWORD\fR must be a keyword from the following list: \fBbios-vendor\fR,
> 
>