Hi Ilias, On Thu, 29 Sept 2022 at 04:23, Ilias Apalodimas <ilias.apalodi...@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 <pbrobin...@gmail.com> wrote: > > > > > > On Tue, Sep 6, 2022 at 2:44 PM Ilias Apalodimas > > > <ilias.apalodi...@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.apalodi...@linaro.org> > > > > > > Reviewed-by: Peter Robinson <pbrobin...@gmail.com> > > > Tested-by: Peter Robinson <pbrobin...@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