[PATCH 2/2 v2] smbios: Fallback to the default DT if sysinfo nodes are missing
Simon Glass
sjg at chromium.org
Thu Nov 30 03:45:25 CET 2023
Hi Ilias,
On Mon, 27 Nov 2023 at 10:11, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> In order to fill in the SMBIOS tables U-Boot currently relies on a
> "u-boot,sysinfo-smbios" compatible node. This is fine for the boards
> that already include such nodes. However with some recent EFI changes,
> the majority of boards can boot up distros, which usually rely on
> things like dmidecode etc for their reporting. For boards that
> lack this special node the SMBIOS output looks like:
>
> System Information
> Manufacturer: Unknown
> Product Name: Unknown
> Version: Unknown
> Serial Number: Unknown
> UUID: Not Settable
> Wake-up Type: Reserved
> SKU Number: Unknown
> Family: Unknown
>
> This looks problematic since most of the info are "Unknown". The DT spec
> specifies standard properties containing relevant information like
> 'model' and 'compatible' for which the suggested format is
> <manufacturer,model>. Unfortunately the 'model' string found in DTs is
> usually lacking the manufacturer so we can't use it for both
> 'Manufacturer' and 'Product Name' SMBIOS entries reliably.
>
> So let's add a last resort to our current smbios parsing. If none of
> the sysinfo properties are found, scan for those information in the
> root node of the device tree. Use the 'model' to fill the 'Product
> Name' and the first value of 'compatible' for the 'Manufacturer', since
> that always contains one.
>
> pre-patch:
> Handle 0x0001, DMI type 1, 27 bytes
> System Information
> Manufacturer: Unknown
> Product Name: Unknown
> Version: Unknown
> Serial Number: 100000000bb24ceb
> UUID: 30303031-3030-3030-3061-613234636435
> Wake-up Type: Reserved
> SKU Number: Unknown
> Family: Unknown
> [...]
>
> and post patch:
> Handle 0x0001, DMI type 1, 27 bytes
> System Information
> Manufacturer: raspberrypi
> Product Name: Raspberry Pi 4 Model B Rev 1.1
> Version: Unknown
> Serial Number: 100000000bb24ceb
> UUID: 30303031-3030-3030-3061-613234636435
> Wake-up Type: Reserved
> SKU Number: Unknown
> Family: Unknown
> [...]
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
> Changes since v1:
> - Tokenize the DT node entry and use the appropriate value instead of
> the entire string
> - Removed Peters tested/reviewed-by tags due to the above
> lib/smbios.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 90 insertions(+), 4 deletions(-)
>
Can this be put behind a Kconfig? It adds quite a bit of code which
punishes those boards which do the right thing.
> diff --git a/lib/smbios.c b/lib/smbios.c
> index fcc8686993ef..423893639ff0 100644
> --- a/lib/smbios.c
> +++ b/lib/smbios.c
> @@ -9,11 +9,14 @@
> #include <dm.h>
> #include <env.h>
> #include <linux/stringify.h>
> +#include <linux/string.h>
> #include <mapmem.h>
> #include <smbios.h>
> #include <sysinfo.h>
> #include <tables_csum.h>
> #include <version.h>
> +#include <malloc.h>
> +#include <dm/ofnode.h>
> #ifdef CONFIG_CPU
> #include <cpu.h>
> #include <dm/uclass-internal.h>
> @@ -43,6 +46,25 @@
>
> DECLARE_GLOBAL_DATA_PTR;
>
> +/**
> + * struct map_sysinfo - Mapping of sysinfo strings to DT
> + *
> + * @sysinfo_str: sysinfo string
> + * @dt_str: DT string
> + * @max: Max index of the tokenized string to pick. Counting starts from 0
> + *
> + */
> +struct map_sysinfo {
> + const char *sysinfo_str;
> + const char *dt_str;
> + int max;
> +};
> +
> +static const struct map_sysinfo sysinfo_to_dt[] = {
> + { .sysinfo_str = "product", .dt_str = "model", 2 },
> + { .sysinfo_str = "manufacturer", .dt_str = "compatible", 1 },
> +};
> +
> /**
> * struct smbios_ctx - context for writing SMBIOS tables
> *
> @@ -87,6 +109,18 @@ struct smbios_write_method {
> const char *subnode_name;
> };
>
> +static const struct map_sysinfo *convert_sysinfo_to_dt(const char *sysinfo_str)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(sysinfo_to_dt); i++) {
> + if (!strcmp(sysinfo_str, sysinfo_to_dt[i].sysinfo_str))
> + return &sysinfo_to_dt[i];
> + }
> +
> + return NULL;
> +}
> +
> /**
> * smbios_add_string() - add a string to the string area
> *
> @@ -127,6 +161,42 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str)
> }
> }
>
> +/**
> + * get_str_from_dt - Get a substring from a DT property.
> + * After finding the property in the DT, the function
> + * will parse comma separted values and return the value.
spelling
> + * If nprop->max exceeds the number of comma separated
comma-separated
> + * elements the last non NULL value will be returned.
elements, the
> + * Counting starts from zero.
> + *
> + * @nprop: sysinfo property to use
> + * @str: pointer to fill with data
> + * @size: str buffer length
> + */
> +static
> +void get_str_from_dt(const struct map_sysinfo *nprop, char *str, size_t size)
> +{
> + const char *dt_str;
> + int cnt = 0;
> + char *token;
> +
> + memset(str, 0, size);
> + if (!nprop || !nprop->max)
> + return;
> +
> + dt_str = ofnode_read_string(ofnode_root(), nprop->dt_str);
Could this use ofnode_read_string_index() ?
> + if (!dt_str)
> + return;
> +
> + memcpy(str, dt_str, size);
> + token = strtok(str, ",");
> + while (token && cnt < nprop->max) {
> + strlcpy(str, token, strlen(token) + 1);
> + token = strtok(NULL, ",");
> + cnt++;
> + }
> +}
> +
> /**
> * smbios_add_prop_si() - Add a property from the devicetree or sysinfo
> *
> @@ -139,19 +209,35 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str)
> static int smbios_add_prop_si(struct smbios_ctx *ctx, const char *prop,
> int sysinfo_id)
> {
> + int ret = 0;
> +
> if (sysinfo_id && ctx->dev) {
> char val[SMBIOS_STR_MAX];
> - int ret;
>
> ret = sysinfo_get_str(ctx->dev, sysinfo_id, sizeof(val), val);
> if (!ret)
> return smbios_add_string(ctx, val);
> }
> +
> if (IS_ENABLED(CONFIG_OF_CONTROL)) {
> - const char *str;
> + const char *str = NULL;
> + char str_dt[128] = { 0 };
> + /*
> + * If the node is not valid fallback and try the entire DT
> + * so we can at least fill in maufacturer and board type
spelling
> + */
> + if (ofnode_valid(ctx->node)) {
> + str = ofnode_read_string(ctx->node, prop);
> + } else {
> + const struct map_sysinfo *nprop;
> +
> + nprop = convert_sysinfo_to_dt(prop);
> + get_str_from_dt(nprop, str_dt, sizeof(str_dt));
> + }
>
> - str = ofnode_read_string(ctx->node, prop);
> - return smbios_add_string(ctx, str);
> + ret = smbios_add_string(ctx, ofnode_valid(ctx->node) ?
> + str : (const char *)str_dt);
> + return ret;
> }
>
> return 0;
> --
> 2.40.1
>
Any chance of a test for this code?
Regards,
Simon
More information about the U-Boot
mailing list