Hi Alex,
On 12 August 2016 at 12:36, Alexander Graf <ag...@suse.de> wrote:
>
>
> On 12.08.16 19:20, Simon Glass wrote:
>> Hi Alex,
>>
>> On 8 August 2016 at 08:06, Alexander Graf <ag...@suse.de> 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 <ag...@suse.de>
>>> ---
>>> 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 <common.h>
>>> +#include <efi_loader.h>
>>> #include <smbios.h>
>>> #include <tables_csum.h>
>>> #include <version.h>
>>> +#ifdef CONFIG_X86
>>> #include <asm/cpu.h>
>>> +#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.
>
>>
>>> smbios_write_type4,
>>> +#endif
>>> smbios_write_type32,
>>> smbios_write_type127
>>> };
>>> @@ -267,3 +280,26 @@ uintptr_t write_smbios_table(uintptr_t addr)
>>>
>>> return addr;
>>> }
>>> +
>>> +#ifdef CONFIG_EFI_LOADER
>>
>> Can this function go into the efi_loader directory instead?
>
> I guess we could have a small file in the efi_loader directory that
> bridges the gap between the smbios lib and the generation, yeah.
OK good. I think this would make the #ifdef go away (sometimes a sign
that code is in the wrong place).
>
>
> Alex
[...]
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
