On 9/9/21 10:57 AM, Simon Glass wrote:
Hi Heinrich,

On Wed, 8 Sept 2021 at 11:34, Heinrich Schuchardt <xypron.g...@gmx.de> wrote:



On 9/8/21 3:33 PM, Simon Glass wrote:
It is useful to see some basic EFI info with the command as it forms part
of the information about a board.

Add a hook for this and show the table address as a start.

While here, fix an invalid cast in setup_efi_info().

Signed-off-by: Simon Glass <s...@chromium.org>
---

   arch/x86/cpu/efi/payload.c | 13 +++++++++++--
   arch/x86/include/asm/efi.h |  7 +++++++
   arch/x86/lib/Makefile      |  1 +
   arch/x86/lib/bdinfo.c      | 22 ++++++++++++++++++++++
   4 files changed, 41 insertions(+), 2 deletions(-)
   create mode 100644 arch/x86/lib/bdinfo.c

diff --git a/arch/x86/cpu/efi/payload.c b/arch/x86/cpu/efi/payload.c
index 9a73b768e9b..3a9f7d72868 100644
--- a/arch/x86/cpu/efi/payload.c
+++ b/arch/x86/cpu/efi/payload.c
@@ -280,15 +280,24 @@ void setup_efi_info(struct efi_info *efi_info)
       }
       efi_info->efi_memdesc_size = map->desc_size;
       efi_info->efi_memdesc_version = map->version;
-     efi_info->efi_memmap = (u32)(map->desc);
+     efi_info->efi_memmap = (ulong)(map->desc);

When converting a pointer to integer we use (uintptr_t).

Generally in U-Boot it is ulong so I try to be consistent here.

Currently ulong and uintprt_t are the same technically. But for the sake
of readability uintptr_t is much clearer.

We use uintptr_t in 935 code locations already. Just grep for it.

Anyway, u32 is clearly not right for a 64-bit build as it gives
warnings.


arch/x86/include/asm/bootparam.h defines efi_info->efi_memmap as u32.
This type is too small to hold a pointer.

       efi_info->efi_memmap_size = size - sizeof(struct efi_entry_memmap);

   #ifdef CONFIG_EFI_STUB_64BIT
       efi_info->efi_systab_hi = table->sys_table >> 32;
-     efi_info->efi_memmap_hi = (u64)(u32)(map->desc) >> 32;
+     efi_info->efi_memmap_hi = (u64)(ulong)map->desc >> 32;

You should not use two fields to hold a single pointer.

Please, replace all of these duplicate fields.

We do need to be compatible with what the kernel expects, otherwise
there is no point in filling out this information.

Why should we have separate fields for the low and high 32 bits of a
pointer?

Best regards

Heinrich

Reply via email to