Hi Heinrich, On Mon, Jan 1, 2024 at 4:23 PM Heinrich Schuchardt <heinrich.schucha...@canonical.com> wrote: > > On 1/1/24 23:41, Simon Glass wrote: > > Hi Heinrich, > > > > On Mon, Jan 1, 2024 at 11:50 AM Heinrich Schuchardt > > <heinrich.schucha...@canonical.com> wrote: > >> > >> An EFI binary dmidump.efi is provided that can be used to check the SMBIOS > >> table for consistency and to dump it as a file. > >> > >> The tool provides the following commands: > >> > >> check > >> Check the SMBIOS table for consistency. > >> > >> exit > >> Leave the tool. > >> > >> help > >> Show available commands. > >> > >> save > >> Save the SMBIOS table to a file on the EFI system partition. The file > >> can be further analyzed with the dmidecode command line tool:: > >> > >> dmidecode --from-dump <filename> > >> > >> Specifying 'nocolor' as load option data suppresses colored output and > >> clearing of the screen. > >> > >> Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com> > >> --- > >> v2: > >> fix an incorrect variable usage when checking the result of memcmp > >> --- > >> lib/efi_loader/Makefile | 7 + > >> lib/efi_loader/dmidump.c | 595 +++++++++++++++++++++++++++++++++++++++ > >> 2 files changed, 602 insertions(+) > >> create mode 100644 lib/efi_loader/dmidump.c > > > > Does this tool need tests? > > Once we have all the SMBIOS stuff in, we could use the tool in a test to > verify that the SMBIOS tables are correctly installed.
Yes that is useful for the EFI side of things, along with your new test for smbios within U-Boot. > > > > > Can you please split up do_save() a bit and also move the command loop > > into its own function? Otherwise it looks good to me. > > > >> > >> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile > >> index 24d33d5409..71880d330e 100644 > >> --- a/lib/efi_loader/Makefile > >> +++ b/lib/efi_loader/Makefile > >> @@ -16,6 +16,8 @@ CFLAGS_boothart.o := $(CFLAGS_EFI) -Os -ffreestanding > >> CFLAGS_REMOVE_boothart.o := $(CFLAGS_NON_EFI) > >> CFLAGS_helloworld.o := $(CFLAGS_EFI) -Os -ffreestanding > >> CFLAGS_REMOVE_helloworld.o := $(CFLAGS_NON_EFI) > >> +CFLAGS_dmidump.o := $(CFLAGS_EFI) -Os -ffreestanding > >> +CFLAGS_REMOVE_dmidump.o := $(CFLAGS_NON_EFI) > >> CFLAGS_dtbdump.o := $(CFLAGS_EFI) -Os -ffreestanding > >> CFLAGS_REMOVE_dtbdump.o := $(CFLAGS_NON_EFI) > >> CFLAGS_initrddump.o := $(CFLAGS_EFI) -Os -ffreestanding > > > > Is there a way to have a list of these tools such that the flags stuff > > is done automatically and doesn't need to be repeated each time? > > Looking at > > scripts/Makefile.lib:102: > _c_flags = $(filter-out $(CFLAGS_REMOVE_$(basetarget).o), $(orig_c_flags)) > > this is not possible. Not with that rule, but perhaps another could be added. Anyway, I suppose we can put up with it for now. > > > > >> @@ -31,6 +33,11 @@ always += helloworld.efi > >> targets += helloworld.o > >> endif > >> > >> +ifneq ($(CONFIG_GENERATE_SMBIOS_TABLE),) > >> +always += dmidump.efi > >> +targets += dmidump.o > > > > How about smbios_tool ? I think 'dmi' is a bit of an obfuscation. > > We can call it smbiosdump.efi. Yes that is better. > > Thanks for reviewing. > > Best regards > > Heinrich > > > > >> +endif > >> + > >> ifeq ($(CONFIG_GENERATE_ACPI_TABLE),) > >> always += dtbdump.efi > >> targets += dtbdump.o > > > > [..] Regards, Simon