| Message ID | 20220906134426.53748-2-ilias.apalodimas@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] smbios: Simplify reporting of unknown values | expand |
On Tue, Sep 6, 2022 at 2:44 PM Ilias Apalodimas <ilias.apalodimas@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>. So let's add a last resort to our current > smbios parsing. If none of the sysinfo properties are found, we can > scan the root node for 'model' and 'compatible'. I don't think the information below all needs to go in the commit, maybe in the cover letter? > pre-patch dmidecode: > <snip> > Handle 0x0001, DMI type 1, 27 bytes > System Information > Manufacturer: Unknown > Product Name: Unknown > Version: Unknown > Serial Number: Unknown > UUID: Not Settable > Wake-up Type: Reserved > SKU Number: Unknown > Family: Unknown > > Handle 0x0002, DMI type 2, 14 bytes > Base Board Information > Manufacturer: Unknown > Product Name: Unknown > Version: Unknown > Serial Number: Not Specified > Asset Tag: Unknown > Features: > Board is a hosting board > Location In Chassis: Not Specified > Chassis Handle: 0x0000 > Type: Motherboard > > Handle 0x0003, DMI type 3, 21 bytes > Chassis Information > Manufacturer: Unknown > Type: Desktop > Lock: Not Present > Version: Not Specified > Serial Number: Not Specified > Asset Tag: Not Specified > Boot-up State: Safe > Power Supply State: Safe > Thermal State: Safe > Security Status: None > OEM Information: 0x00000000 > Height: Unspecified > Number Of Power Cords: Unspecified > Contained Elements: 0 > <snip> > > post-pastch dmidecode: > <snip> > Handle 0x0001, DMI type 1, 27 bytes > System Information > Manufacturer: socionext,developer-box > Product Name: Socionext Developer Box > Version: Unknown > Serial Number: Unknown > UUID: Not Settable > Wake-up Type: Reserved > SKU Number: Unknown > Family: Unknown > > Handle 0x0002, DMI type 2, 14 bytes > Base Board Information > Manufacturer: socionext,developer-box > Product Name: Socionext Developer Box > Version: Unknown > Serial Number: Not Specified > Asset Tag: Unknown > Features: > Board is a hosting board > Location In Chassis: Not Specified > Chassis Handle: 0x0000 > Type: Motherboard > > Handle 0x0003, DMI type 3, 21 bytes > Chassis Information > Manufacturer: socionext,developer-box > Type: Desktop > Lock: Not Present > Version: Not Specified > Serial Number: Not Specified > Asset Tag: Not Specified > Boot-up State: Safe > Power Supply State: Safe > Thermal State: Safe > Security Status: None > OEM Information: 0x00000000 > Height: Unspecified > Number Of Power Cords: Unspecified > Contained Elements: 0 > <snip> > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> Reviewed-by: Peter Robinson <pbrobinson@gmail.com> Tested-by: Peter Robinson <pbrobinson@gmail.com> > --- > lib/smbios.c | 41 +++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 39 insertions(+), 2 deletions(-) > > diff --git a/lib/smbios.c b/lib/smbios.c > index fcc8686993ef..f2eb961f514b 100644 > --- a/lib/smbios.c > +++ b/lib/smbios.c > @@ -43,6 +43,20 @@ > > DECLARE_GLOBAL_DATA_PTR; > > +/** > + * struct sysifno_to_dt - Mapping of sysinfo strings to DT > + * > + * @sysinfo_str: sysinfo string > + * @dt_str: DT string > + */ > +static const struct { > + const char *sysinfo_str; > + const char *dt_str; > +} sysifno_to_dt[] = { > + { .sysinfo_str = "product", .dt_str = "model" }, > + { .sysinfo_str = "manufacturer", .dt_str = "compatible" }, > +}; > + > /** > * struct smbios_ctx - context for writing SMBIOS tables > * > @@ -87,6 +101,18 @@ struct smbios_write_method { > const char *subnode_name; > }; > > +static const char *convert_sysinfo_to_dt(const char *sysinfo_str) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(sysifno_to_dt); i++) { > + if (!strcmp(sysinfo_str, sysifno_to_dt[i].sysinfo_str)) > + return sysifno_to_dt[i].dt_str; > + } > + > + return NULL; > +} > + > /** > * smbios_add_string() - add a string to the string area > * > @@ -148,9 +174,20 @@ static int smbios_add_prop_si(struct smbios_ctx *ctx, const char *prop, > return smbios_add_string(ctx, val); > } > if (IS_ENABLED(CONFIG_OF_CONTROL)) { > - const char *str; > + const char *str = NULL; > > - str = ofnode_read_string(ctx->node, prop); > + /* > + * If the node is not valid fallback and try the entire DT > + * so we can at least fill in maufacturer and board type > + */ > + if (!ofnode_valid(ctx->node)) { > + const char *nprop = convert_sysinfo_to_dt(prop); > + > + if (nprop) > + str = ofnode_read_string(ofnode_root(), nprop); > + } else { > + str = ofnode_read_string(ctx->node, prop); > + } > return smbios_add_string(ctx, str); > } > > -- > 2.37.2 >
Hi, On Tue, 20 Sept 2022 at 05:10, Peter Robinson <pbrobinson@gmail.com> wrote: > > On Tue, Sep 6, 2022 at 2:44 PM Ilias Apalodimas > <ilias.apalodimas@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>. So let's add a last resort to our current > > smbios parsing. If none of the sysinfo properties are found, we can > > scan the root node for 'model' and 'compatible'. > > I don't think the information below all needs to go in the commit, > maybe in the cover letter? > > > pre-patch dmidecode: > > <snip> > > Handle 0x0001, DMI type 1, 27 bytes > > System Information > > Manufacturer: Unknown > > Product Name: Unknown > > Version: Unknown > > Serial Number: Unknown > > UUID: Not Settable > > Wake-up Type: Reserved > > SKU Number: Unknown > > Family: Unknown > > > > Handle 0x0002, DMI type 2, 14 bytes > > Base Board Information > > Manufacturer: Unknown > > Product Name: Unknown > > Version: Unknown > > Serial Number: Not Specified > > Asset Tag: Unknown > > Features: > > Board is a hosting board > > Location In Chassis: Not Specified > > Chassis Handle: 0x0000 > > Type: Motherboard > > > > Handle 0x0003, DMI type 3, 21 bytes > > Chassis Information > > Manufacturer: Unknown > > Type: Desktop > > Lock: Not Present > > Version: Not Specified > > Serial Number: Not Specified > > Asset Tag: Not Specified > > Boot-up State: Safe > > Power Supply State: Safe > > Thermal State: Safe > > Security Status: None > > OEM Information: 0x00000000 > > Height: Unspecified > > Number Of Power Cords: Unspecified > > Contained Elements: 0 > > <snip> > > > > post-pastch dmidecode: > > <snip> > > Handle 0x0001, DMI type 1, 27 bytes > > System Information > > Manufacturer: socionext,developer-box > > Product Name: Socionext Developer Box > > Version: Unknown > > Serial Number: Unknown > > UUID: Not Settable > > Wake-up Type: Reserved > > SKU Number: Unknown > > Family: Unknown > > > > Handle 0x0002, DMI type 2, 14 bytes > > Base Board Information > > Manufacturer: socionext,developer-box > > Product Name: Socionext Developer Box > > Version: Unknown > > Serial Number: Not Specified > > Asset Tag: Unknown > > Features: > > Board is a hosting board > > Location In Chassis: Not Specified > > Chassis Handle: 0x0000 > > Type: Motherboard > > > > Handle 0x0003, DMI type 3, 21 bytes > > Chassis Information > > Manufacturer: socionext,developer-box > > Type: Desktop > > Lock: Not Present > > Version: Not Specified > > Serial Number: Not Specified > > Asset Tag: Not Specified > > Boot-up State: Safe > > Power Supply State: Safe > > Thermal State: Safe > > Security Status: None > > OEM Information: 0x00000000 > > Height: Unspecified > > Number Of Power Cords: Unspecified > > Contained Elements: 0 > > <snip> > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > Reviewed-by: Peter Robinson <pbrobinson@gmail.com> > Tested-by: Peter Robinson <pbrobinson@gmail.com> > > > --- > > lib/smbios.c | 41 +++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 39 insertions(+), 2 deletions(-) I've thought about this a lot. As I mentioned earlier, we should require boards to add this information when they enable GENERATE_SMBIOS_TABLE It is a simple patch for each board vendor and it solves the problem. What we have here just masks it. Regards, Simon
Hi Simon, On Thu, Sep 29, 2022 at 03:59:51AM -0600, Simon Glass wrote: > Hi, > > On Tue, 20 Sept 2022 at 05:10, Peter Robinson <pbrobinson@gmail.com> wrote: > > > > On Tue, Sep 6, 2022 at 2:44 PM Ilias Apalodimas > > <ilias.apalodimas@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>. So let's add a last resort to our current > > > smbios parsing. If none of the sysinfo properties are found, we can > > > scan the root node for 'model' and 'compatible'. > > > > I don't think the information below all needs to go in the commit, > > maybe in the cover letter? > > > > > pre-patch dmidecode: > > > <snip> > > > Handle 0x0001, DMI type 1, 27 bytes > > > System Information > > > Manufacturer: Unknown > > > Product Name: Unknown > > > Version: Unknown > > > Serial Number: Unknown > > > UUID: Not Settable > > > Wake-up Type: Reserved > > > SKU Number: Unknown > > > Family: Unknown > > > > > > Handle 0x0002, DMI type 2, 14 bytes > > > Base Board Information > > > Manufacturer: Unknown > > > Product Name: Unknown > > > Version: Unknown > > > Serial Number: Not Specified > > > Asset Tag: Unknown > > > Features: > > > Board is a hosting board > > > Location In Chassis: Not Specified > > > Chassis Handle: 0x0000 > > > Type: Motherboard > > > > > > Handle 0x0003, DMI type 3, 21 bytes > > > Chassis Information > > > Manufacturer: Unknown > > > Type: Desktop > > > Lock: Not Present > > > Version: Not Specified > > > Serial Number: Not Specified > > > Asset Tag: Not Specified > > > Boot-up State: Safe > > > Power Supply State: Safe > > > Thermal State: Safe > > > Security Status: None > > > OEM Information: 0x00000000 > > > Height: Unspecified > > > Number Of Power Cords: Unspecified > > > Contained Elements: 0 > > > <snip> > > > > > > post-pastch dmidecode: > > > <snip> > > > Handle 0x0001, DMI type 1, 27 bytes > > > System Information > > > Manufacturer: socionext,developer-box > > > Product Name: Socionext Developer Box > > > Version: Unknown > > > Serial Number: Unknown > > > UUID: Not Settable > > > Wake-up Type: Reserved > > > SKU Number: Unknown > > > Family: Unknown > > > > > > Handle 0x0002, DMI type 2, 14 bytes > > > Base Board Information > > > Manufacturer: socionext,developer-box > > > Product Name: Socionext Developer Box > > > Version: Unknown > > > Serial Number: Not Specified > > > Asset Tag: Unknown > > > Features: > > > Board is a hosting board > > > Location In Chassis: Not Specified > > > Chassis Handle: 0x0000 > > > Type: Motherboard > > > > > > Handle 0x0003, DMI type 3, 21 bytes > > > Chassis Information > > > Manufacturer: socionext,developer-box > > > Type: Desktop > > > Lock: Not Present > > > Version: Not Specified > > > Serial Number: Not Specified > > > Asset Tag: Not Specified > > > Boot-up State: Safe > > > Power Supply State: Safe > > > Thermal State: Safe > > > Security Status: None > > > OEM Information: 0x00000000 > > > Height: Unspecified > > > Number Of Power Cords: Unspecified > > > Contained Elements: 0 > > > <snip> > > > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > > Reviewed-by: Peter Robinson <pbrobinson@gmail.com> > > Tested-by: Peter Robinson <pbrobinson@gmail.com> > > > > > --- > > > lib/smbios.c | 41 +++++++++++++++++++++++++++++++++++++++-- > > > 1 file changed, 39 insertions(+), 2 deletions(-) > > I've thought about this a lot. > > As I mentioned earlier, we should require boards to add this > information when they enable GENERATE_SMBIOS_TABLE > > It is a simple patch for each board vendor and it solves the problem. > What we have here just masks it. Not entirely. I think we just see the problem differently here. I agree that the code here masks a problem (but only for *some* boards) and ideally we should go and add smbios nodes on the boards that miss it. However we conveniently keep ignoring OF_BOARD here. Until those things are documented in a spec and you can *demand* a previous bootloader to include it, we'll have boards that display "Unknown" all over the place. Personally I don't think that's acceptable, hence the last resort solution. I'd be much happier if we popped a warning on boards that miss the smbios node and are not compiling with OF_BOARD, explaining smbios will be disabled for them in the future, while having the flexibility to not display values that make no sense. Thanks /Ilias > > Regards, > Simon
Hi Ilias, On Thu, 29 Sept 2022 at 04:23, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Simon, > > On Thu, Sep 29, 2022 at 03:59:51AM -0600, Simon Glass wrote: > > Hi, > > > > On Tue, 20 Sept 2022 at 05:10, Peter Robinson <pbrobinson@gmail.com> wrote: > > > > > > On Tue, Sep 6, 2022 at 2:44 PM Ilias Apalodimas > > > <ilias.apalodimas@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>. So let's add a last resort to our current > > > > smbios parsing. If none of the sysinfo properties are found, we can > > > > scan the root node for 'model' and 'compatible'. > > > > > > I don't think the information below all needs to go in the commit, > > > maybe in the cover letter? > > > > > > > pre-patch dmidecode: > > > > <snip> > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > System Information > > > > Manufacturer: Unknown > > > > Product Name: Unknown > > > > Version: Unknown > > > > Serial Number: Unknown > > > > UUID: Not Settable > > > > Wake-up Type: Reserved > > > > SKU Number: Unknown > > > > Family: Unknown > > > > > > > > Handle 0x0002, DMI type 2, 14 bytes > > > > Base Board Information > > > > Manufacturer: Unknown > > > > Product Name: Unknown > > > > Version: Unknown > > > > Serial Number: Not Specified > > > > Asset Tag: Unknown > > > > Features: > > > > Board is a hosting board > > > > Location In Chassis: Not Specified > > > > Chassis Handle: 0x0000 > > > > Type: Motherboard > > > > > > > > Handle 0x0003, DMI type 3, 21 bytes > > > > Chassis Information > > > > Manufacturer: Unknown > > > > Type: Desktop > > > > Lock: Not Present > > > > Version: Not Specified > > > > Serial Number: Not Specified > > > > Asset Tag: Not Specified > > > > Boot-up State: Safe > > > > Power Supply State: Safe > > > > Thermal State: Safe > > > > Security Status: None > > > > OEM Information: 0x00000000 > > > > Height: Unspecified > > > > Number Of Power Cords: Unspecified > > > > Contained Elements: 0 > > > > <snip> > > > > > > > > post-pastch dmidecode: > > > > <snip> > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > System Information > > > > Manufacturer: socionext,developer-box > > > > Product Name: Socionext Developer Box > > > > Version: Unknown > > > > Serial Number: Unknown > > > > UUID: Not Settable > > > > Wake-up Type: Reserved > > > > SKU Number: Unknown > > > > Family: Unknown > > > > > > > > Handle 0x0002, DMI type 2, 14 bytes > > > > Base Board Information > > > > Manufacturer: socionext,developer-box > > > > Product Name: Socionext Developer Box > > > > Version: Unknown > > > > Serial Number: Not Specified > > > > Asset Tag: Unknown > > > > Features: > > > > Board is a hosting board > > > > Location In Chassis: Not Specified > > > > Chassis Handle: 0x0000 > > > > Type: Motherboard > > > > > > > > Handle 0x0003, DMI type 3, 21 bytes > > > > Chassis Information > > > > Manufacturer: socionext,developer-box > > > > Type: Desktop > > > > Lock: Not Present > > > > Version: Not Specified > > > > Serial Number: Not Specified > > > > Asset Tag: Not Specified > > > > Boot-up State: Safe > > > > Power Supply State: Safe > > > > Thermal State: Safe > > > > Security Status: None > > > > OEM Information: 0x00000000 > > > > Height: Unspecified > > > > Number Of Power Cords: Unspecified > > > > Contained Elements: 0 > > > > <snip> > > > > > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > > > > Reviewed-by: Peter Robinson <pbrobinson@gmail.com> > > > Tested-by: Peter Robinson <pbrobinson@gmail.com> > > > > > > > --- > > > > lib/smbios.c | 41 +++++++++++++++++++++++++++++++++++++++-- > > > > 1 file changed, 39 insertions(+), 2 deletions(-) > > > > I've thought about this a lot. > > > > As I mentioned earlier, we should require boards to add this > > information when they enable GENERATE_SMBIOS_TABLE > > > > It is a simple patch for each board vendor and it solves the problem. > > What we have here just masks it. > > > Not entirely. I think we just see the problem differently here. I agree > that the code here masks a problem (but only for *some* boards) and ideally > we should go and add smbios nodes on the boards that miss it. However we > conveniently keep ignoring OF_BOARD here. Until those things are documented > in a spec and you can *demand* a previous bootloader to include it, we'll have > boards that display "Unknown" all over the place. Personally I don't > think that's acceptable, hence the last resort solution. I think you mean OF_HAS_PRIOR_STAGE - we have an explicit Kconfig now. We can easily make U-Boot halt if the info is not there but it is needed. That will cause people to fix it for their board. > > I'd be much happier if we popped a warning on boards that miss the smbios > node and are not compiling with OF_BOARD, explaining smbios will be > disabled for them in the future, while having the flexibility to not > display values that make no sense. How about just failing the build and producing an error, if people enable the SMBIOS tables without the data? We could run with a warning for a while if you like, then change it to an error. Regards, Simon
> From: Simon Glass <sjg@chromium.org> > Date: Thu, 29 Sep 2022 17:55:43 -0600 > > Hi Ilias, > > On Thu, 29 Sept 2022 at 04:23, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > Hi Simon, > > > > On Thu, Sep 29, 2022 at 03:59:51AM -0600, Simon Glass wrote: > > > Hi, > > > > > > On Tue, 20 Sept 2022 at 05:10, Peter Robinson <pbrobinson@gmail.com> wrote: > > > > > > > > On Tue, Sep 6, 2022 at 2:44 PM Ilias Apalodimas > > > > <ilias.apalodimas@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>. So let's add a last resort to our current > > > > > smbios parsing. If none of the sysinfo properties are found, we can > > > > > scan the root node for 'model' and 'compatible'. > > > > > > > > I don't think the information below all needs to go in the commit, > > > > maybe in the cover letter? > > > > > > > > > pre-patch dmidecode: > > > > > <snip> > > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > > System Information > > > > > Manufacturer: Unknown > > > > > Product Name: Unknown > > > > > Version: Unknown > > > > > Serial Number: Unknown > > > > > UUID: Not Settable > > > > > Wake-up Type: Reserved > > > > > SKU Number: Unknown > > > > > Family: Unknown > > > > > > > > > > Handle 0x0002, DMI type 2, 14 bytes > > > > > Base Board Information > > > > > Manufacturer: Unknown > > > > > Product Name: Unknown > > > > > Version: Unknown > > > > > Serial Number: Not Specified > > > > > Asset Tag: Unknown > > > > > Features: > > > > > Board is a hosting board > > > > > Location In Chassis: Not Specified > > > > > Chassis Handle: 0x0000 > > > > > Type: Motherboard > > > > > > > > > > Handle 0x0003, DMI type 3, 21 bytes > > > > > Chassis Information > > > > > Manufacturer: Unknown > > > > > Type: Desktop > > > > > Lock: Not Present > > > > > Version: Not Specified > > > > > Serial Number: Not Specified > > > > > Asset Tag: Not Specified > > > > > Boot-up State: Safe > > > > > Power Supply State: Safe > > > > > Thermal State: Safe > > > > > Security Status: None > > > > > OEM Information: 0x00000000 > > > > > Height: Unspecified > > > > > Number Of Power Cords: Unspecified > > > > > Contained Elements: 0 > > > > > <snip> > > > > > > > > > > post-pastch dmidecode: > > > > > <snip> > > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > > System Information > > > > > Manufacturer: socionext,developer-box > > > > > Product Name: Socionext Developer Box > > > > > Version: Unknown > > > > > Serial Number: Unknown > > > > > UUID: Not Settable > > > > > Wake-up Type: Reserved > > > > > SKU Number: Unknown > > > > > Family: Unknown > > > > > > > > > > Handle 0x0002, DMI type 2, 14 bytes > > > > > Base Board Information > > > > > Manufacturer: socionext,developer-box > > > > > Product Name: Socionext Developer Box > > > > > Version: Unknown > > > > > Serial Number: Not Specified > > > > > Asset Tag: Unknown > > > > > Features: > > > > > Board is a hosting board > > > > > Location In Chassis: Not Specified > > > > > Chassis Handle: 0x0000 > > > > > Type: Motherboard > > > > > > > > > > Handle 0x0003, DMI type 3, 21 bytes > > > > > Chassis Information > > > > > Manufacturer: socionext,developer-box > > > > > Type: Desktop > > > > > Lock: Not Present > > > > > Version: Not Specified > > > > > Serial Number: Not Specified > > > > > Asset Tag: Not Specified > > > > > Boot-up State: Safe > > > > > Power Supply State: Safe > > > > > Thermal State: Safe > > > > > Security Status: None > > > > > OEM Information: 0x00000000 > > > > > Height: Unspecified > > > > > Number Of Power Cords: Unspecified > > > > > Contained Elements: 0 > > > > > <snip> > > > > > > > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > > > > > > Reviewed-by: Peter Robinson <pbrobinson@gmail.com> > > > > Tested-by: Peter Robinson <pbrobinson@gmail.com> > > > > > > > > > --- > > > > > lib/smbios.c | 41 +++++++++++++++++++++++++++++++++++++++-- > > > > > 1 file changed, 39 insertions(+), 2 deletions(-) > > > > > > I've thought about this a lot. > > > > > > As I mentioned earlier, we should require boards to add this > > > information when they enable GENERATE_SMBIOS_TABLE > > > > > > It is a simple patch for each board vendor and it solves the problem. > > > What we have here just masks it. > > > > > > Not entirely. I think we just see the problem differently here. I agree > > that the code here masks a problem (but only for *some* boards) and ideally > > we should go and add smbios nodes on the boards that miss it. However we > > conveniently keep ignoring OF_BOARD here. Until those things are documented > > in a spec and you can *demand* a previous bootloader to include it, we'll have > > boards that display "Unknown" all over the place. Personally I don't > > think that's acceptable, hence the last resort solution. > > I think you mean OF_HAS_PRIOR_STAGE - we have an explicit Kconfig now. > > We can easily make U-Boot halt if the info is not there but it is > needed. That will cause people to fix it for their board. That seems unecessarily harsh... The smbios stuff is by no means essential to run an OS on a board. On many low-end (or user assembled) x86 machines it is full of lies as well (gotta love all those machines with serial number 123456789) and a lot of the information in the tables doesn't make sense for "embedded" boards anyway. At best the smbios tables are a "nice to have" feature. But it seems to be mostly a box ticking excercise to me. > > I'd be much happier if we popped a warning on boards that miss the smbios > > node and are not compiling with OF_BOARD, explaining smbios will be > > disabled for them in the future, while having the flexibility to not > > display values that make no sense. > > How about just failing the build and producing an error, if people > enable the SMBIOS tables without the data? We could run with a warning > for a while if you like, then change it to an error. Again, that seems unecessarily harsh. If foks are really bothered about the correctness of the information we supply, we should either just not offer the tables if essential information is missing from the device tree, or maybe require boards to explicitly request the smbios feature by dropping the "|| EFI_LOADER" from its Kconfig entry.
On Fri, Sep 30, 2022 at 11:56:53AM +0200, Mark Kettenis wrote: > > From: Simon Glass <sjg@chromium.org> > > Date: Thu, 29 Sep 2022 17:55:43 -0600 > > > > Hi Ilias, > > > > On Thu, 29 Sept 2022 at 04:23, Ilias Apalodimas > > <ilias.apalodimas@linaro.org> wrote: > > > > > > Hi Simon, > > > > > > On Thu, Sep 29, 2022 at 03:59:51AM -0600, Simon Glass wrote: > > > > Hi, > > > > > > > > On Tue, 20 Sept 2022 at 05:10, Peter Robinson <pbrobinson@gmail.com> wrote: > > > > > > > > > > On Tue, Sep 6, 2022 at 2:44 PM Ilias Apalodimas > > > > > <ilias.apalodimas@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>. So let's add a last resort to our current > > > > > > smbios parsing. If none of the sysinfo properties are found, we can > > > > > > scan the root node for 'model' and 'compatible'. > > > > > > > > > > I don't think the information below all needs to go in the commit, > > > > > maybe in the cover letter? > > > > > > > > > > > pre-patch dmidecode: > > > > > > <snip> > > > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > > > System Information > > > > > > Manufacturer: Unknown > > > > > > Product Name: Unknown > > > > > > Version: Unknown > > > > > > Serial Number: Unknown > > > > > > UUID: Not Settable > > > > > > Wake-up Type: Reserved > > > > > > SKU Number: Unknown > > > > > > Family: Unknown > > > > > > > > > > > > Handle 0x0002, DMI type 2, 14 bytes > > > > > > Base Board Information > > > > > > Manufacturer: Unknown > > > > > > Product Name: Unknown > > > > > > Version: Unknown > > > > > > Serial Number: Not Specified > > > > > > Asset Tag: Unknown > > > > > > Features: > > > > > > Board is a hosting board > > > > > > Location In Chassis: Not Specified > > > > > > Chassis Handle: 0x0000 > > > > > > Type: Motherboard > > > > > > > > > > > > Handle 0x0003, DMI type 3, 21 bytes > > > > > > Chassis Information > > > > > > Manufacturer: Unknown > > > > > > Type: Desktop > > > > > > Lock: Not Present > > > > > > Version: Not Specified > > > > > > Serial Number: Not Specified > > > > > > Asset Tag: Not Specified > > > > > > Boot-up State: Safe > > > > > > Power Supply State: Safe > > > > > > Thermal State: Safe > > > > > > Security Status: None > > > > > > OEM Information: 0x00000000 > > > > > > Height: Unspecified > > > > > > Number Of Power Cords: Unspecified > > > > > > Contained Elements: 0 > > > > > > <snip> > > > > > > > > > > > > post-pastch dmidecode: > > > > > > <snip> > > > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > > > System Information > > > > > > Manufacturer: socionext,developer-box > > > > > > Product Name: Socionext Developer Box > > > > > > Version: Unknown > > > > > > Serial Number: Unknown > > > > > > UUID: Not Settable > > > > > > Wake-up Type: Reserved > > > > > > SKU Number: Unknown > > > > > > Family: Unknown > > > > > > > > > > > > Handle 0x0002, DMI type 2, 14 bytes > > > > > > Base Board Information > > > > > > Manufacturer: socionext,developer-box > > > > > > Product Name: Socionext Developer Box > > > > > > Version: Unknown > > > > > > Serial Number: Not Specified > > > > > > Asset Tag: Unknown > > > > > > Features: > > > > > > Board is a hosting board > > > > > > Location In Chassis: Not Specified > > > > > > Chassis Handle: 0x0000 > > > > > > Type: Motherboard > > > > > > > > > > > > Handle 0x0003, DMI type 3, 21 bytes > > > > > > Chassis Information > > > > > > Manufacturer: socionext,developer-box > > > > > > Type: Desktop > > > > > > Lock: Not Present > > > > > > Version: Not Specified > > > > > > Serial Number: Not Specified > > > > > > Asset Tag: Not Specified > > > > > > Boot-up State: Safe > > > > > > Power Supply State: Safe > > > > > > Thermal State: Safe > > > > > > Security Status: None > > > > > > OEM Information: 0x00000000 > > > > > > Height: Unspecified > > > > > > Number Of Power Cords: Unspecified > > > > > > Contained Elements: 0 > > > > > > <snip> > > > > > > > > > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > > > > > > > > Reviewed-by: Peter Robinson <pbrobinson@gmail.com> > > > > > Tested-by: Peter Robinson <pbrobinson@gmail.com> > > > > > > > > > > > --- > > > > > > lib/smbios.c | 41 +++++++++++++++++++++++++++++++++++++++-- > > > > > > 1 file changed, 39 insertions(+), 2 deletions(-) > > > > > > > > I've thought about this a lot. > > > > > > > > As I mentioned earlier, we should require boards to add this > > > > information when they enable GENERATE_SMBIOS_TABLE > > > > > > > > It is a simple patch for each board vendor and it solves the problem. > > > > What we have here just masks it. > > > > > > > > > Not entirely. I think we just see the problem differently here. I agree > > > that the code here masks a problem (but only for *some* boards) and ideally > > > we should go and add smbios nodes on the boards that miss it. However we > > > conveniently keep ignoring OF_BOARD here. Until those things are documented > > > in a spec and you can *demand* a previous bootloader to include it, we'll have > > > boards that display "Unknown" all over the place. Personally I don't > > > think that's acceptable, hence the last resort solution. > > > > I think you mean OF_HAS_PRIOR_STAGE - we have an explicit Kconfig now. > > > > We can easily make U-Boot halt if the info is not there but it is > > needed. That will cause people to fix it for their board. > > That seems unecessarily harsh... > > The smbios stuff is by no means essential to run an OS on a board. On > many low-end (or user assembled) x86 machines it is full of lies as > well (gotta love all those machines with serial number 123456789) and > a lot of the information in the tables doesn't make sense for > "embedded" boards anyway. At best the smbios tables are a "nice to > have" feature. But it seems to be mostly a box ticking excercise to > me. This is another point I'd been trying to think how to best bring up. The majority of x86 HW I've ever used is full of useless values. Even my laptop has some to be filled in values. So it's entirely reasonable I think to populate some default values and document how and where to put something more correct, when it's possible / useful.
diff --git a/lib/smbios.c b/lib/smbios.c index fcc8686993ef..f2eb961f514b 100644 --- a/lib/smbios.c +++ b/lib/smbios.c @@ -43,6 +43,20 @@ DECLARE_GLOBAL_DATA_PTR; +/** + * struct sysifno_to_dt - Mapping of sysinfo strings to DT + * + * @sysinfo_str: sysinfo string + * @dt_str: DT string + */ +static const struct { + const char *sysinfo_str; + const char *dt_str; +} sysifno_to_dt[] = { + { .sysinfo_str = "product", .dt_str = "model" }, + { .sysinfo_str = "manufacturer", .dt_str = "compatible" }, +}; + /** * struct smbios_ctx - context for writing SMBIOS tables * @@ -87,6 +101,18 @@ struct smbios_write_method { const char *subnode_name; }; +static const char *convert_sysinfo_to_dt(const char *sysinfo_str) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(sysifno_to_dt); i++) { + if (!strcmp(sysinfo_str, sysifno_to_dt[i].sysinfo_str)) + return sysifno_to_dt[i].dt_str; + } + + return NULL; +} + /** * smbios_add_string() - add a string to the string area * @@ -148,9 +174,20 @@ static int smbios_add_prop_si(struct smbios_ctx *ctx, const char *prop, return smbios_add_string(ctx, val); } if (IS_ENABLED(CONFIG_OF_CONTROL)) { - const char *str; + const char *str = NULL; - str = ofnode_read_string(ctx->node, prop); + /* + * If the node is not valid fallback and try the entire DT + * so we can at least fill in maufacturer and board type + */ + if (!ofnode_valid(ctx->node)) { + const char *nprop = convert_sysinfo_to_dt(prop); + + if (nprop) + str = ofnode_read_string(ofnode_root(), nprop); + } else { + str = ofnode_read_string(ctx->node, prop); + } return smbios_add_string(ctx, str); }
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>. So let's add a last resort to our current smbios parsing. If none of the sysinfo properties are found, we can scan the root node for 'model' and 'compatible'. pre-patch dmidecode: <snip> Handle 0x0001, DMI type 1, 27 bytes System Information Manufacturer: Unknown Product Name: Unknown Version: Unknown Serial Number: Unknown UUID: Not Settable Wake-up Type: Reserved SKU Number: Unknown Family: Unknown Handle 0x0002, DMI type 2, 14 bytes Base Board Information Manufacturer: Unknown Product Name: Unknown Version: Unknown Serial Number: Not Specified Asset Tag: Unknown Features: Board is a hosting board Location In Chassis: Not Specified Chassis Handle: 0x0000 Type: Motherboard Handle 0x0003, DMI type 3, 21 bytes Chassis Information Manufacturer: Unknown Type: Desktop Lock: Not Present Version: Not Specified Serial Number: Not Specified Asset Tag: Not Specified Boot-up State: Safe Power Supply State: Safe Thermal State: Safe Security Status: None OEM Information: 0x00000000 Height: Unspecified Number Of Power Cords: Unspecified Contained Elements: 0 <snip> post-pastch dmidecode: <snip> Handle 0x0001, DMI type 1, 27 bytes System Information Manufacturer: socionext,developer-box Product Name: Socionext Developer Box Version: Unknown Serial Number: Unknown UUID: Not Settable Wake-up Type: Reserved SKU Number: Unknown Family: Unknown Handle 0x0002, DMI type 2, 14 bytes Base Board Information Manufacturer: socionext,developer-box Product Name: Socionext Developer Box Version: Unknown Serial Number: Not Specified Asset Tag: Unknown Features: Board is a hosting board Location In Chassis: Not Specified Chassis Handle: 0x0000 Type: Motherboard Handle 0x0003, DMI type 3, 21 bytes Chassis Information Manufacturer: socionext,developer-box Type: Desktop Lock: Not Present Version: Not Specified Serial Number: Not Specified Asset Tag: Not Specified Boot-up State: Safe Power Supply State: Safe Thermal State: Safe Security Status: None OEM Information: 0x00000000 Height: Unspecified Number Of Power Cords: Unspecified Contained Elements: 0 <snip> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- lib/smbios.c | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-)