[PATCH 03/35] x86: Show some EFI info with the bdinfo command

[Simon Glass]

Hi Heinrich,

On Wed, 8 Sept 2021 at 11:34, Heinrich Schuchardt <xypron.glpk at 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 <sjg at 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.
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.

Regards,
Simon