From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Date: Tue, 16 Aug 2016 10:38:56 +0200 Subject: [U-Boot] [PATCH 5/7] smbios: Expose in efi_loader as table In-Reply-To: References: <1470665194-19619-1-git-send-email-agraf@suse.de> <1470665194-19619-6-git-send-email-agraf@suse.de> Message-ID: <57B2D120.3050709@suse.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 08/12/2016 10:07 PM, Simon Glass wrote: > Hi Alex, > > On 12 August 2016 at 12:36, Alexander Graf wrote: >> >> On 12.08.16 19:20, Simon Glass wrote: >>> Hi Alex, >>> >>> On 8 August 2016 at 08:06, Alexander Graf wrote: >>>> We can pass SMBIOS easily as EFI configuration table to an EFI payload. This >>>> patch adds enablement for that case. >>>> >>>> While at it, we also enable SMBIOS generation for ARM systems, since they support >>>> EFI_LOADER. >>>> >>>> Signed-off-by: Alexander Graf >>>> --- >>>> cmd/bootefi.c | 3 +++ >>>> include/efi_api.h | 4 ++++ >>>> include/efi_loader.h | 2 ++ >>>> include/smbios.h | 1 + >>>> lib/Kconfig | 4 ++-- >>>> lib/smbios.c | 36 ++++++++++++++++++++++++++++++++++++ >>>> 6 files changed, 48 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c >>>> index 53a6ee3..e241b7d 100644 >>>> --- a/cmd/bootefi.c >>>> +++ b/cmd/bootefi.c >>>> @@ -205,6 +205,9 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt) >>>> if (!memcmp(bootefi_device_path[0].str, "N\0e\0t", 6)) >>>> loaded_image_info.device_handle = nethandle; >>>> #endif >>>> +#ifdef CONFIG_GENERATE_SMBIOS_TABLE >>>> + efi_smbios_register(); >>>> +#endif >>>> >>>> /* Initialize EFI runtime services */ >>>> efi_reset_system_init(); >>>> diff --git a/include/efi_api.h b/include/efi_api.h >>>> index f572b88..bdb600e 100644 >>>> --- a/include/efi_api.h >>>> +++ b/include/efi_api.h >>>> @@ -201,6 +201,10 @@ struct efi_runtime_services { >>>> EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, \ >>>> 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0) >>>> >>>> +#define SMBIOS_TABLE_GUID \ >>>> + EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3, \ >>>> + 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d) >>>> + >>>> struct efi_configuration_table >>>> { >>>> efi_guid_t guid; >>>> diff --git a/include/efi_loader.h b/include/efi_loader.h >>>> index ac8b77a..b0e8a7f 100644 >>>> --- a/include/efi_loader.h >>>> +++ b/include/efi_loader.h >>>> @@ -85,6 +85,8 @@ int efi_disk_register(void); >>>> int efi_gop_register(void); >>>> /* Called by bootefi to make the network interface available */ >>>> int efi_net_register(void **handle); >>>> +/* Called by bootefi to make SMBIOS tables available */ >>>> +void efi_smbios_register(void); >>>> >>>> /* Called by networking code to memorize the dhcp ack package */ >>>> void efi_net_set_dhcp_ack(void *pkt, int len); >>>> diff --git a/include/smbios.h b/include/smbios.h >>>> index 5962d4c..fb6396a 100644 >>>> --- a/include/smbios.h >>>> +++ b/include/smbios.h >>>> @@ -55,6 +55,7 @@ struct __packed smbios_entry { >>>> #define BIOS_CHARACTERISTICS_SELECTABLE_BOOT (1 << 16) >>>> >>>> #define BIOS_CHARACTERISTICS_EXT1_ACPI (1 << 0) >>>> +#define BIOS_CHARACTERISTICS_EXT1_UEFI (1 << 3) >>>> #define BIOS_CHARACTERISTICS_EXT2_TARGET (1 << 2) >>>> >>>> struct __packed smbios_type0 { >>>> diff --git a/lib/Kconfig b/lib/Kconfig >>>> index 5a14530..d7f75fe 100644 >>>> --- a/lib/Kconfig >>>> +++ b/lib/Kconfig >>>> @@ -150,12 +150,12 @@ config SPL_OF_LIBFDT >>>> version of the device tree. >>>> >>>> menu "System tables" >>>> - depends on !EFI && !SYS_COREBOOT >>>> + depends on (!EFI && !SYS_COREBOOT) || (ARM && EFI_LOADER) >>>> >>>> config GENERATE_SMBIOS_TABLE >>>> bool "Generate an SMBIOS (System Management BIOS) table" >>>> default y >>>> - depends on X86 >>>> + depends on X86 || EFI_LOADER >>>> help >>>> The System Management BIOS (SMBIOS) specification addresses how >>>> motherboard and system vendors present management information about >>>> diff --git a/lib/smbios.c b/lib/smbios.c >>>> index 8dfd486..b9aa741 100644 >>>> --- a/lib/smbios.c >>>> +++ b/lib/smbios.c >>>> @@ -7,10 +7,13 @@ >>>> */ >>>> >>>> #include >>>> +#include >>>> #include >>>> #include >>>> #include >>>> +#ifdef CONFIG_X86 >>>> #include >>>> +#endif >>>> >>>> DECLARE_GLOBAL_DATA_PTR; >>>> >>>> @@ -79,14 +82,20 @@ static int smbios_write_type0(uintptr_t *current, int handle) >>>> t->vendor = smbios_add_string(t->eos, "U-Boot"); >>>> t->bios_ver = smbios_add_string(t->eos, PLAIN_VERSION); >>>> t->bios_release_date = smbios_add_string(t->eos, U_BOOT_DMI_DATE); >>>> +#ifdef CONFIG_ROM_SIZE >>>> t->bios_rom_size = (CONFIG_ROM_SIZE / 65536) - 1; >>>> +#endif >>>> t->bios_characteristics = BIOS_CHARACTERISTICS_PCI_SUPPORTED | >>>> BIOS_CHARACTERISTICS_SELECTABLE_BOOT | >>>> BIOS_CHARACTERISTICS_UPGRADEABLE; >>>> #ifdef CONFIG_GENERATE_ACPI_TABLE >>>> t->bios_characteristics_ext1 = BIOS_CHARACTERISTICS_EXT1_ACPI; >>>> #endif >>>> +#ifdef CONFIG_EFI_LOADER >>>> + t->bios_characteristics_ext1 |= BIOS_CHARACTERISTICS_EXT1_UEFI; >>>> +#endif >>>> t->bios_characteristics_ext2 = BIOS_CHARACTERISTICS_EXT2_TARGET; >>>> + >>>> t->bios_major_release = 0xff; >>>> t->bios_minor_release = 0xff; >>>> t->ec_major_release = 0xff; >>>> @@ -152,6 +161,7 @@ static int smbios_write_type3(uintptr_t *current, int handle) >>>> return len; >>>> } >>>> >>>> +#ifdef CONFIG_X86 >>>> static int smbios_write_type4(uintptr_t *current, int handle) >>>> { >>>> struct smbios_type4 *t = (struct smbios_type4 *)*current; >>>> @@ -184,6 +194,7 @@ static int smbios_write_type4(uintptr_t *current, int handle) >>>> >>>> return len; >>>> } >>>> +#endif >>>> >>>> static int smbios_write_type32(uintptr_t *current, int handle) >>>> { >>>> @@ -216,7 +227,9 @@ static smbios_write_type smbios_write_funcs[] = { >>>> smbios_write_type1, >>>> smbios_write_type2, >>>> smbios_write_type3, >>>> +#ifdef CONFIG_X86 >>> This should become a parameter I think. Since you are making this into >>> generic code, it should avoid arch-specific #ifdefs. >> The type4 table really contains x86 cpu specific information, so I >> figured an #ifdef CONFIG_X86 makes the most sense here. >> >> Looking at >> >> >> https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.0.0.pdf >> >> I guess you *could* put one in that addresses ARM as well. Looking at >> example output from a ThunderX system, that seems to be what other >> vendors do: >> >> Handle 0x0004, DMI type 4, 48 bytes >> Processor Information >> Socket Designation: CPU 0 >> Type: Central Processor >> Family: Other >> Manufacturer: www.cavium.com >> ID: 00 00 00 00 00 00 00 00 >> Version: 2.0 >> Voltage: 3.3 V >> External Clock: 156 MHz >> Max Speed: 2000 MHz >> Current Speed: 2000 MHz >> Status: Populated, Enabled >> Upgrade: Other >> L1 Cache Handle: 0x0080 >> L2 Cache Handle: 0x0082 >> L3 Cache Handle: 0x0007 >> Serial Number: 1.0 >> Asset Tag: 1.0 >> Part Number: [...] >> Core Count: 48 >> Core Enabled: 48 >> Thread Count: 1 >> Characteristics: None >> >> So what we really need is a rename for smbios_write_type4 to >> smbios_write_type4_x86 which then is still surrounded by the #ifdef >> CONFIG_X86. We could just simply add another one for arm if we want to ;). > From what I can see, the problem is the cpu_get_name() call. Is that > right? If so, that could move to the CPU uclass and use cpu_get_info() > (with the name added as a new field) or perhaps a new cpu_get_name() > call. I see what you're aiming for, but I just fail to see how we could properly abstract away CPU specific information - and I don't think it's terribly useful either. I've refactored the code to be slightly more pretty now, but I really don't think that we should depend on CONFIG_CPU or introduce new calls for every bit: static void smbios_write_type4_arch(struct smbios_type4 *t) { u16 processor_family = SMBIOS_PROCESSOR_FAMILY_UNKNOWN; const char *vendor = "Unknown"; char *name = "Unknown"; #ifdef CONFIG_X86 char processor_name[CPU_MAX_NAME_LEN]; struct cpuid_result res; processor_family = gd->arch.x86; vendor = cpu_vendor_name(gd->arch.x86_vendor); res = cpuid(1); t->processor_id[0] = res.eax; t->processor_id[1] = res.edx; name = cpu_get_name(processor_name); #elif defined(CONFIG_ARM) processor_family = SMBIOS_PROCESSOR_FAMILY_OTHER; vendor = "ARM"; #endif t->processor_family = processor_family; t->processor_manufacturer = smbios_add_string(t->eos, vendor); t->processor_version = smbios_add_string(t->eos, name); } Alex