On Fri, Nov 03, 2023 at 11:12:40AM -0600, Simon Glass wrote: > Hi, > > On Sat, 28 Oct 2023 at 12:41, Simon Glass <s...@chromium.org> wrote: > > > > [unfortunately I am not receiving email from the list at present] > > > > Hi Heinrich, > > > > On Wed, 25 Oct 2023 at 21:39, Heinrich Schuchardt <xypron.g...@gmx.de> > > wrote: > > > > > > On 10/25/23 04:49, Simon Glass wrote: > > > > Hi Heinrich, > > > > > > > > On Tue, 24 Oct 2023 at 18:22, Heinrich Schuchardt <xypron.g...@gmx.de> > > > > wrote: > > > >> > > > >> > > > >> > > > >> Am 25. Oktober 2023 01:31:19 MESZ schrieb Simon Glass > > > >> <s...@chromium.org>: > > > >>> U-Boot typically sets up its malloc() pool near the top of memory. On > > > >>> ARM64 systems this can result in an SMBIOS table above 4GB which is > > > >>> not supported by SMBIOSv2. > > > >>> > > > >>> Work around this problem by providing a new option to choose an > > > >>> address > > > >>> below 4GB (but as high as possible), if needed. > > > >> > > > >> You must not overwrite memory controlled by the EFI subsystem without > > > >> calling its allocator. We should provide SMBIOS 3. SMBIOS 2 is only a > > > >> fallback for outdated tools. > > > > > > > > That is not my intention and I don't believe this code does that. EFI > > > > is not running at this point, is it? > > > > > > The function install_smbios_table() only exists if CONFIG_EFI_LOADER=y. > > > > That is because ARM devices don't normally need it, right? Anyway, > > that option isn't related to this patch. If ARM devices started using > > SMBIOS and had another way to pass it to Linux (other than EFI) then > > we would want to install it. > > > > > > > > We have: > > > EVENT_SPY_SIMPLE(EVT_LAST_STAGE_INIT, install_smbios_table); > > > This is invoked after efi_memory_init(). > > > > > > The EFI specification requires that the memory area occupied by the > > > SMBIOS table uses one of a specific set of memory types where > > > EfiRuntimeServicesData is recommended. So you must call > > > > > > u64 addr = UINT_MAX; > > > ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, > > > EFI_RUNTIME_SERVICES_DATA, efi_size_in_pages(size), *addr); > > > > > > to allocate the memory. If the return code is not EFI_SUCCESS, no memory > > > below 4 GiB is available. > > > > The root problem here is that x86 and ARM used to work differently. > > When the ARM SMBIOS stuff was done, it worked by writing the SMBIOS > > table as part of the 'bootefi' command. On x86, the tables were > > written on startup, so you can examine them within U-Boot. Clearly the > > x86 approach is correct. For one thing, a previous-stage bootloader > > may set up the tables, so it simply isn't valid to write them in that > > case. So we need to separate writing the tables from telling EFI about > > them. > > > > So I have fixed that, so ARM now writes the tables at the start. But > > using an EFI allocation function is clearly not right. This is generic > > code, nothing to do with EFI, really. In fact, the SMBIOS writing > > should move out of efi_loader. The install_smbios_table() function > > should be somewhere in lib, i suppose, with just efi_smbios_register() > > sitting in lib/efi_loader > > > > Also, why is efi_memory_init() called early in init? Is there anything > > that needs that in the init sequence? Could we move it to the end, or > > perhaps skip it completely until the 'bootefi' command is used? > > > > Another point I should make is that it should be fine for U-Boot to > > put something in memory and then call efi_add_memory_map() to tell EFI > > about it. What problems does that cause? It isn't as if EFI allocates > > things in the 'conventional' memory (is that the name for memory below > > 4GB?) This is how efi_acpi_register() works. > > > > (Aside: it is bizarre to me that CONFIG_EFI_LOADER appears in > > drivers/video/rockchip_rk_vop.c and other such files) > > > > > > > > > > > > > The bit I am confused about is that we don't support SMBIOS3 in > > > > U-Boot. I am trying to fix an introduced bug... > > > > > > I would not know why we should not use SMBIOS 3. > > > > Neither do I. Perhaps there are compatibility concerns? If it is OK to > > do that then we could go back to my previous series [1]. What do you > > think? > > Tom responded but I missed it. In part it says: > > "So, can we please start by just doing the minimal changes to get the > SMBIOS table done correctly for memory above 4G, via EFI, and then start > the next steps?" > > I am OK to do an EFI hack for ARM so long as we agree that after the > release we will revert it and generate the table using generic memory > allocation, not dependent on EFI. Does that sound reasonable? > > I don't seem to have received any response from Heinrich to the > various points I made above. I cannot see any response on patchwork > either.
Drop it right after the release? No. We will replace it with what's more appropriate once we figure out what that is. And please don't call it a hack, unless I'm missing something I don't see yet how to make use of the SMBIOS table on ARM without EFI. I see something that implies MIPS can (and loongarch and ia64). -- Tom
signature.asc
Description: PGP signature