[U-Boot] [PATCH 18/48] efi: Add start-up library code

Bin Meng bmeng.cn at gmail.com
Thu Jul 23 10:09:44 CEST 2015


Hi Simon,

On Wed, Jul 22, 2015 at 11:49 PM, Simon Glass <sjg at chromium.org> wrote:
> When running as an EFI application, U-Boot must request memory from EFI,
> and provide access to the boot services U-Boot needs.
>
> Add library code to perform these tasks. This includes efi_main() which is
> the entry point from EFI. U-Boot is built as a shared library.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
>  arch/x86/include/asm/fsp/fsp_hob.h |  59 +-----
>  include/efi.h                      | 356 +++++++++++++++++++++++++++++++++++++
>  include/efi_api.h                  | 252 ++++++++++++++++++++++++++
>  include/part_efi.h                 |   9 +-
>  lib/Kconfig                        |   2 +
>  lib/Makefile                       |   1 +
>  lib/efi/Kconfig                    |  33 ++++
>  lib/efi/Makefile                   |   7 +
>  lib/efi/efi.c                      |  91 ++++++++++
>  lib/efi/efi_app.c                  | 125 +++++++++++++
>  10 files changed, 871 insertions(+), 64 deletions(-)
>  create mode 100644 include/efi.h
>  create mode 100644 include/efi_api.h
>  create mode 100644 lib/efi/Kconfig
>  create mode 100644 lib/efi/Makefile
>  create mode 100644 lib/efi/efi.c
>  create mode 100644 lib/efi/efi_app.c
>
> diff --git a/arch/x86/include/asm/fsp/fsp_hob.h b/arch/x86/include/asm/fsp/fsp_hob.h
> index 6cca7f5..3fb3546 100644
> --- a/arch/x86/include/asm/fsp/fsp_hob.h
> +++ b/arch/x86/include/asm/fsp/fsp_hob.h
> @@ -8,6 +8,8 @@
>  #ifndef __FSP_HOB_H__
>  #define __FSP_HOB_H__
>
> +#include <efi.h>
> +
>  /* Type of HOB Header */
>  #define HOB_TYPE_MEM_ALLOC     0x0002
>  #define HOB_TYPE_RES_DESC      0x0003
> @@ -25,63 +27,6 @@ struct hob_header {
>         u32     reserved;       /* always zero */
>  };
>
> -/* Enumeration of memory types introduced in UEFI */
> -enum efi_mem_type {
> -       EFI_RESERVED_MEMORY_TYPE,
> -       /*
> -        * The code portions of a loaded application.
> -        * (Note that UEFI OS loaders are UEFI applications.)
> -        */
> -       EFI_LOADER_CODE,
> -       /*
> -        * The data portions of a loaded application and
> -        * the default data allocation type used by an application
> -        * to allocate pool memory.
> -        */
> -       EFI_LOADER_DATA,
> -       /* The code portions of a loaded Boot Services Driver */
> -       EFI_BOOT_SERVICES_CODE,
> -       /*
> -        * The data portions of a loaded Boot Serves Driver and
> -        * the default data allocation type used by a Boot Services
> -        * Driver to allocate pool memory.
> -        */
> -       EFI_BOOT_SERVICES_DATA,
> -       /* The code portions of a loaded Runtime Services Driver */
> -       EFI_RUNTIME_SERVICES_CODE,
> -       /*
> -        * The data portions of a loaded Runtime Services Driver and
> -        * the default data allocation type used by a Runtime Services
> -        * Driver to allocate pool memory.
> -        */
> -       EFI_RUNTIME_SERVICES_DATA,
> -       /* Free (unallocated) memory */
> -       EFI_CONVENTIONAL_MEMORY,
> -       /* Memory in which errors have been detected */
> -       EFI_UNUSABLE_MEMORY,
> -       /* Memory that holds the ACPI tables */
> -       EFI_ACPI_RECLAIM_MEMORY,
> -       /* Address space reserved for use by the firmware */
> -       EFI_ACPI_MEMORY_NVS,
> -       /*
> -        * Used by system firmware to request that a memory-mapped IO region
> -        * be mapped by the OS to a virtual address so it can be accessed by
> -        * EFI runtime services.
> -        */
> -       EFI_MMAP_IO,
> -       /*
> -        * System memory-mapped IO region that is used to translate
> -        * memory cycles to IO cycles by the processor.
> -        */
> -       EFI_MMAP_IO_PORT,
> -       /*
> -        * Address space reserved by the firmware for code that is
> -        * part of the processor.
> -        */
> -       EFI_PAL_CODE,
> -       EFI_MAX_MEMORY_TYPE
> -};
> -
>  /*
>   * Describes all memory ranges used during the HOB producer phase that
>   * exist outside the HOB list. This HOB type describes how memory is used,
> diff --git a/include/efi.h b/include/efi.h
> new file mode 100644
> index 0000000..66ef6c3
> --- /dev/null
> +++ b/include/efi.h
> @@ -0,0 +1,356 @@
> +/*
> + * Extensible Firmware Interface
> + * Based on 'Extensible Firmware Interface Specification' version 0.9,
> + * April 30, 1999
> + *
> + * Copyright (C) 1999 VA Linux Systems
> + * Copyright (C) 1999 Walt Drummond <drummond at valinux.com>
> + * Copyright (C) 1999, 2002-2003 Hewlett-Packard Co.
> + *     David Mosberger-Tang <davidm at hpl.hp.com>
> + *     Stephane Eranian <eranian at hpl.hp.com>
> + *
> + * From include/linux/efi.h in kernel 4.1 with some additions/subtractions
> + */
> +
> +#ifndef _EFI_H
> +#define _EFI_H
> +
> +#include <linux/string.h>
> +#include <linux/types.h>
> +
> +#ifdef CONFIG_EFI_STUB_64BIT

I believe this CONFIG_EFI_STUB_64BIT is introduced in later patches,
can we remove this to later patch where you add this
CONFIG_EFI_STUB_64BIT?

> +#define EFIAPI __attribute__((ms_abi))

This is worth a comment block to describe the fact that EFI 64-bit is
using M$ ABI which is different from ours.

> +#else
> +#define EFIAPI
> +#endif
> +
> +struct efi_device_path;
> +
> +#define EFI_SUCCESS            0
> +#define EFI_LOAD_ERROR         (1 | (1UL << (BITS_PER_LONG - 1)))
> +#define EFI_INVALID_PARAMETER  (2 | (1UL << (BITS_PER_LONG - 1)))
> +#define EFI_UNSUPPORTED                (3 | (1UL << (BITS_PER_LONG - 1)))
> +#define EFI_BAD_BUFFER_SIZE    (4 | (1UL << (BITS_PER_LONG - 1)))
> +#define EFI_BUFFER_TOO_SMALL   (5 | (1UL << (BITS_PER_LONG - 1)))
> +#define EFI_NOT_READY          (6 | (1UL << (BITS_PER_LONG - 1)))
> +#define EFI_DEVICE_ERROR       (7 | (1UL << (BITS_PER_LONG - 1)))
> +#define EFI_WRITE_PROTECTED    (8 | (1UL << (BITS_PER_LONG - 1)))
> +#define EFI_OUT_OF_RESOURCES   (9 | (1UL << (BITS_PER_LONG - 1)))
> +#define EFI_NOT_FOUND          (14 | (1UL << (BITS_PER_LONG - 1)))
> +#define EFI_SECURITY_VIOLATION (26 | (1UL << (BITS_PER_LONG - 1)))
> +

Can we also remove those duplicated defines in
arch/x86/include/asm/fsp/fsp_types.h so that FSP codes can use the one
in efi.h?

> +typedef unsigned long efi_status_t;
> +typedef u64 efi_physical_addr_t;
> +typedef u64 efi_virtual_addr_t;
> +typedef void *efi_handle_t;
> +
> +#define EFI_GUID(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
> +       ((efi_guid_t) \
> +       {{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, \
> +               ((a) >> 24) & 0xff, \
> +               (b) & 0xff, ((b) >> 8) & 0xff, \
> +               (c) & 0xff, ((c) >> 8) & 0xff, \
> +               (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) } })
> +
> +/*
> + * Generic EFI table header
> + */

Please use one-line comment.

> +struct efi_table_hdr {
> +       u64 signature;
> +       u32 revision;
> +       u32 headersize;
> +       u32 crc32;
> +       u32 reserved;
> +};
> +
> +/* Enumeration of memory types introduced in UEFI */
> +enum efi_mem_type {
> +       EFI_RESERVED_MEMORY_TYPE,
> +       /*
> +        * The code portions of a loaded application.
> +        * (Note that UEFI OS loaders are UEFI applications.)
> +        */
> +       EFI_LOADER_CODE,
> +       /*
> +        * The data portions of a loaded application and
> +        * the default data allocation type used by an application
> +        * to allocate pool memory.
> +        */
> +       EFI_LOADER_DATA,
> +       /* The code portions of a loaded Boot Services Driver */
> +       EFI_BOOT_SERVICES_CODE,
> +       /*
> +        * The data portions of a loaded Boot Serves Driver and
> +        * the default data allocation type used by a Boot Services
> +        * Driver to allocate pool memory.
> +        */
> +       EFI_BOOT_SERVICES_DATA,
> +       /* The code portions of a loaded Runtime Services Driver */
> +       EFI_RUNTIME_SERVICES_CODE,
> +       /*
> +        * The data portions of a loaded Runtime Services Driver and
> +        * the default data allocation type used by a Runtime Services
> +        * Driver to allocate pool memory.
> +        */
> +       EFI_RUNTIME_SERVICES_DATA,
> +       /* Free (unallocated) memory */
> +       EFI_CONVENTIONAL_MEMORY,
> +       /* Memory in which errors have been detected */
> +       EFI_UNUSABLE_MEMORY,
> +       /* Memory that holds the ACPI tables */
> +       EFI_ACPI_RECLAIM_MEMORY,
> +       /* Address space reserved for use by the firmware */
> +       EFI_ACPI_MEMORY_NVS,
> +       /*
> +        * Used by system firmware to request that a memory-mapped IO region
> +        * be mapped by the OS to a virtual address so it can be accessed by
> +        * EFI runtime services.
> +        */
> +       EFI_MMAP_IO,
> +       /*
> +        * System memory-mapped IO region that is used to translate
> +        * memory cycles to IO cycles by the processor.
> +        */
> +       EFI_MMAP_IO_PORT,
> +       /*
> +        * Address space reserved by the firmware for code that is
> +        * part of the processor.
> +        */
> +       EFI_PAL_CODE,
> +
> +       EFI_MAX_MEMORY_TYPE,
> +       EFI_TABLE_END,  /* For efi_build_mem_table() */
> +};
> +
> +/* Attribute values */
> +enum {
> +       EFI_MEMORY_UC_SHIFT     = 0,    /* uncached */
> +       EFI_MEMORY_WC_SHIFT     = 1,    /* write-coalescing */
> +       EFI_MEMORY_WT_SHIFT     = 2,    /* write-through */
> +       EFI_MEMORY_WB_SHIFT     = 3,    /* write-back */
> +       EFI_MEMORY_UCE_SHIFT    = 4,    /* uncached, exported */
> +       EFI_MEMORY_WP_SHIFT     = 12,   /* write-protect */
> +       EFI_MEMORY_RP_SHIFT     = 13,   /* read-protect */
> +       EFI_MEMORY_XP_SHIFT     = 14,   /* execute-protect */
> +       EFI_MEMORY_RUNTIME_SHIFT = 63,  /* range requires runtime mapping */
> +
> +       EFI_MEMORY_RUNTIME = 1ULL << EFI_MEMORY_RUNTIME_SHIFT,
> +       efi_mem_desc_VERSION    = 1,

What is efi_mem_desc_VERSION? The name should be all capital letters.

> +};
> +
> +#define EFI_PAGE_SHIFT         12
> +#define EFI_PAGE_SIZE          (1UL << EFI_PAGE_SHIFT)
> +
> +struct efi_mem_desc {
> +       u32 type;
> +       u32 reserved;
> +       efi_physical_addr_t physical_start;
> +       efi_virtual_addr_t virtual_start;
> +       u64 num_pages;
> +       u64 attribute;
> +};
> +
> +/*
> + * Allocation types for calls to boottime->allocate_pages.
> + */

Please use one-line comment.

> +#define EFI_ALLOCATE_ANY_PAGES         0
> +#define EFI_ALLOCATE_MAX_ADDRESS       1
> +#define EFI_ALLOCATE_ADDRESS           2
> +#define EFI_MAX_ALLOCATE_TYPE          3
> +
> +/*
> + * Types and defines for Time Services
> + */

Please use one-line comment.

> +#define EFI_TIME_ADJUST_DAYLIGHT 0x1
> +#define EFI_TIME_IN_DAYLIGHT     0x2
> +#define EFI_UNSPECIFIED_TIMEZONE 0x07ff
> +
> +struct efi_time {
> +       u16 year;
> +       u8 month;
> +       u8 day;
> +       u8 hour;
> +       u8 minute;
> +       u8 second;
> +       u8 pad1;
> +       u32 nanosecond;
> +       s16 timezone;
> +       u8 daylight;
> +       u8 pad2;
> +};
> +
> +struct efi_time_cap {
> +       u32 resolution;
> +       u32 accuracy;
> +       u8 sets_to_zero;
> +};
> +
> +enum efi_locate_search_type {
> +       all_handles,
> +       by_register_notify,
> +       by_protocol
> +};
> +
> +struct efi_open_protocol_info_entry {
> +       efi_handle_t agent_handle;
> +       efi_handle_t controller_handle;
> +       u32 attributes;
> +       u32 open_count;
> +};
> +
> +enum efi_entry_t {
> +       EFIET_END,      /* Signals this is the last (empty) entry */
> +       EFIET_MEMORY_MAP,
> +

Please remove this blank line.

> +       EFIET_MEMORY_COUNT,
> +};
> +
> +#define EFI_TABLE_VERSION      1
> +
> +/**
> + * struct efi_info_hdr - Header for the EFI info table
> + *
> + * @version:   EFI_TABLE_VERSION
> + * @hdr_size:  Size of this struct in bytes

Missing description for total_size and spare[5]

> + */
> +struct efi_info_hdr {
> +       u32 version;
> +       u32 hdr_size;
> +       u32 total_size;
> +       u32 spare[5];
> +};
> +
> +/**
> + * struct efi_entry_hdr - Header for a table entry
> + *
> + * @type:      enum eft_entry_t
> + * @size       size of entry bytes excluding header and padding
> + * @addr:      address of this entry (0 if it follows the header )
> + * @link:      size of entry including header and padding
> + * @

Missing description for spare1 and spare2.

> + */
> +struct efi_entry_hdr {
> +       u32 type;
> +       u32 size;
> +       u64 addr;
> +       u32 link;
> +       u32 spare1;
> +       u64 spare22;

Why 22? Should it be spare2?

> +};
> +
> +struct efi_entry_memmap {

Can we add comment block for this structure too?

> +       u32 version;
> +       u32 desc_size;
> +       u64 spare;
> +       struct efi_mem_desc desc[];

Nits: should it be "struct efi_mem_desc *desc"?

> +};
> +
> +static inline struct efi_mem_desc *efi_get_next_mem_desc(
> +               struct efi_entry_memmap *map, struct efi_mem_desc *desc)
> +{
> +       return (struct efi_mem_desc *)((ulong)desc + map->desc_size);
> +}
> +
> +struct efi_priv {
> +       efi_handle_t parent_image;
> +       struct efi_device_path *device_path;
> +       struct efi_system_table *sys_table;
> +       struct efi_boot_services *boot;
> +       struct efi_runtime_services *run;
> +       bool use_pool_for_malloc;
> +       unsigned long ram_base;
> +       unsigned int image_data_type;
> +       struct efi_info_hdr *info;
> +       unsigned int info_size;
> +       void *next_hdr;
> +};
> +
> +/* Base address of the EFI image */
> +extern char ImageBase[];

Can we avoid CamelCase?

> +
> +/**
> + * efi_get_sys_table() - Get access to the main EFI system table
> + *
> + * @return pointer to EFI system table
> + */
> +struct efi_system_table *efi_get_sys_table(void);
> +
> +/**
> + * efi_get_ram_base() - Find the base of RAM
> + *
> + * This is used when U-Boot is built as an EFI application.
> + *
> + * @return the base of RAM as know to U-Boot

as 'known'?

> + */
> +unsigned long efi_get_ram_base(void);
> +
> +/**
> + * efi_init() - Set up ready for use of EFI boot services
> + *
> + * @priv:      Pointer to our private EFI structure to fill in
> + * @banner:    Banner to display when starting
> + * @image:     The image handle passed to efi_main()
> + * @sys_table: The EFI system table pointer passed to efi_main()
> + */
> +int efi_init(struct efi_priv *priv, const char *banner, efi_handle_t image,
> +            struct efi_system_table *sys_table);
> +
> +/**
> + * efi_malloc() - Allocate some memory from EFI
> + *
> + * @priv:      Pointer to private EFI structure
> + * @size:      Number of bytes to allocate
> + * @retp:      Return EFI status resuilt

Typo of 'result'

> + * @return pointer to memory allocated, or NULL on error
> + */
> +void *efi_malloc(struct efi_priv *priv, int size, efi_status_t *retp);
> +
> +/**
> + * efi_free() - Free memory allocated from EFI
> + *
> + * @priv:      Pointer to private EFI structure
> + * @ptr:       Pointer to memory to free
> + */
> +void efi_free(struct efi_priv *priv, void *ptr);
> +
> +/**
> + * efi_puts() - Write out a string to the EFI console
> + *
> + * @priv:      Pointer to private EFI structure
> + * @str:       String to write (note this is ASCII, not unicode)
> + */
> +void efi_puts(struct efi_priv *priv, const char *str);
> +
> +/**
> + * efi_putc() - Write out a character to the EFI console
> + *
> + * @priv:      Pointer to private EFI structure
> + * @ch:                Character to write (note this is ASCII, not unicode)

A character can only be ASCII.

> + */
> +void efi_putc(struct efi_priv *priv, const char ch);
> +
> +/**
> + * efi_info_get() - get an entry from an EFI table
> + *
> + * @type:      Entry type to search for
> + * @datap:     Returns pointer to entry data
> + * @sizep:     Returns pointer to entry size
> + * @return 0 if OK, -ENODATA if there is no table, -ENOENT if there is no entry
> + * of the requested type, -EPROTONOSUPPORT if the table has the wrong version
> + */
> +int efi_info_get(enum efi_entry_t type, void **datap, int *sizep);
> +
> +/**
> + * efi_build_mem_table() - make a sorted copy of the memory table
> + *
> + * @map:       Pointer to EFI memory map table
> + * @size:      Size of table in bytes
> + * @skip_bs:   True to skip boot-time memory and merge it with conventional
> + *             memory. This will significantly reduce the number of table
> + *             entries.
> + * @return pointer to the new table. It should be freed with free() by the
> + *        caller
> + */
> +void *efi_build_mem_table(struct efi_entry_memmap *map, int size, bool skip_bs);
> +
> +#endif /* _LINUX_EFI_H */
> diff --git a/include/efi_api.h b/include/efi_api.h
> new file mode 100644
> index 0000000..d4528f6
> --- /dev/null
> +++ b/include/efi_api.h
> @@ -0,0 +1,252 @@
> +/*
> + * Extensible Firmware Interface
> + * Based on 'Extensible Firmware Interface Specification' version 0.9,
> + * April 30, 1999
> + *
> + * Copyright (C) 1999 VA Linux Systems
> + * Copyright (C) 1999 Walt Drummond <drummond at valinux.com>
> + * Copyright (C) 1999, 2002-2003 Hewlett-Packard Co.
> + *     David Mosberger-Tang <davidm at hpl.hp.com>
> + *     Stephane Eranian <eranian at hpl.hp.com>
> + *
> + * From include/linux/efi.h in kernel 4.1 with some additions/subtractions
> + */
> +
> +#ifndef _EFI_API_H
> +#define _EFI_API_H
> +
> +#include <efi.h>
> +
> +/*
> + * EFI Boot Services table
> + */

Please use one-line comment.

> +struct efi_boot_services {
> +       struct efi_table_hdr hdr;
> +       void *raise_tpl;
> +       void *restore_tpl;
> +
> +       efi_status_t (EFIAPI *allocate_pages)(int, int, unsigned long,
> +                                             efi_physical_addr_t *);
> +       efi_status_t (EFIAPI *free_pages)(efi_physical_addr_t, unsigned long);
> +       efi_status_t (EFIAPI *get_memory_map)(unsigned long *memory_map_size,
> +                       struct efi_mem_desc *desc, unsigned long *key,
> +                       unsigned long *desc_size, u32 *desc_version);
> +       efi_status_t (EFIAPI *allocate_pool)(int, unsigned long, void **);
> +       efi_status_t (EFIAPI *free_pool)(void *);
> +
> +       void *create_event;
> +       void *set_timer;
> +       efi_status_t(EFIAPI *wait_for_event)(unsigned long number_of_events,
> +                                            void *event, unsigned long *index);
> +       void *signal_event;
> +       void *close_event;
> +       void *check_event;
> +
> +       void *install_protocol_interface;
> +       void *reinstall_protocol_interface;
> +       void *uninstall_protocol_interface;
> +       efi_status_t (EFIAPI *handle_protocol)(efi_handle_t, efi_guid_t *,
> +                                              void **);
> +       void *__reserved;

Can it be just 'reserved'?

> +       void *register_protocol_notify;
> +       efi_status_t (EFIAPI *locate_handle)(
> +                       enum efi_locate_search_type search_type,
> +                       efi_guid_t *protocol, void *search_key,
> +                       unsigned long *buffer_size, efi_handle_t *buffer);
> +       efi_status_t (EFIAPI *locate_device_path)(efi_guid_t *protocol,
> +                       struct efi_device_path **device_path,
> +                       efi_handle_t *device);
> +       void *install_configuration_table;
> +
> +       efi_status_t (EFIAPI *load_image)(bool boot_policiy,
> +                       efi_handle_t parent_image,
> +                       struct efi_device_path *file_path, void *source_buffer,
> +                       unsigned long source_size, efi_handle_t *image);
> +       efi_status_t (EFIAPI *start_image)(efi_handle_t handle,
> +                                          unsigned long *exitdata_size,
> +                                          s16 **exitdata);
> +       efi_status_t (EFIAPI *exit)(efi_handle_t handle,
> +                                   efi_status_t exit_status,
> +                                   unsigned long exitdata_size, s16 *exitdata);
> +       void *unload_image;
> +       efi_status_t (EFIAPI *exit_boot_services)(efi_handle_t, unsigned long);
> +
> +       efi_status_t (EFIAPI *get_next_monotonic_count)(u64 *count);
> +       efi_status_t (EFIAPI *stall)(unsigned long usecs);
> +       void *set_watchdog_timer;
> +       efi_status_t(EFIAPI *connect_controller)(efi_handle_t controller_handle,
> +                       efi_handle_t *driver_image_handle,
> +                       struct efi_device_path *remaining_device_path,
> +                       bool recursive);
> +       void *disconnect_controller;
> +#define EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL  0x00000001
> +#define EFI_OPEN_PROTOCOL_GET_PROTOCOL        0x00000002
> +#define EFI_OPEN_PROTOCOL_TEST_PROTOCOL       0x00000004
> +#define EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER 0x00000008
> +#define EFI_OPEN_PROTOCOL_BY_DRIVER           0x00000010
> +#define EFI_OPEN_PROTOCOL_EXCLUSIVE           0x00000020

Can we move these to efi.h?

> +       efi_status_t (EFIAPI *open_protocol)(efi_handle_t handle,
> +                       efi_guid_t *protocol, void **interface,
> +                       efi_handle_t agent_handle,
> +                       efi_handle_t controller_handle, u32 attributes);
> +       void *close_protocol;
> +       efi_status_t(EFIAPI *open_protocol_information)(efi_handle_t handle,
> +                       efi_guid_t *protocol,
> +                       struct efi_open_protocol_info_entry **entry_buffer,
> +                       unsigned long *entry_count);
> +       efi_status_t (EFIAPI *protocols_per_handle)(efi_handle_t handle,
> +                       efi_guid_t ***protocol_buffer,
> +                       unsigned long *protocols_buffer_count);
> +       efi_status_t (EFIAPI *locate_handle_buffer) (
> +                       enum efi_locate_search_type search_type,
> +                       efi_guid_t *protocol, void *search_key,
> +                       unsigned long *no_handles, efi_handle_t **buffer);
> +       void *locate_protocol;
> +       void *install_multiple_protocol_interfaces;
> +       void *uninstall_multiple_protocol_interfaces;
> +       void *calculate_crc32;
> +       void *copy_mem;
> +       void *set_mem;
> +       void *create_event_ex;
> +};
> +
> +/*
> + * Types and defines for EFI ResetSystem
> + */

Please use one-line comment.

> +enum efi_reset_type {
> +       EFI_RESET_COLD = 0,
> +       EFI_RESET_WARM = 1,
> +       EFI_RESET_SHUTDOWN = 2
> +};

Nits: can we move this to efi.h?

> +
> +/*
> + * EFI Runtime Services table
> + */

Please use one-line comment.

> +#define EFI_RUNTIME_SERVICES_SIGNATURE ((u64)0x5652453544e5552ULL)

Is the (u64) cast necessary?

> +#define EFI_RUNTIME_SERVICES_REVISION  0x00010000

Nits: can we move the above two to efi.h?

> +struct efi_runtime_services {
> +       struct efi_table_hdr hdr;
> +       void *get_time;
> +       void *set_time;
> +       void *get_wakeup_time;
> +       void *set_wakeup_time;
> +       void *set_virtual_address_map;
> +       void *convert_pointer;
> +       efi_status_t (EFIAPI *get_variable)(s16 *variable_name,
> +                       efi_guid_t *vendor, u32 *attributes,
> +                       unsigned long *data_size, void *data);
> +       efi_status_t (EFIAPI *get_next_variable)(
> +                       unsigned long *variable_name_size,
> +                       s16 *variable_name, efi_guid_t *vendor);
> +       efi_status_t (EFIAPI *set_variable)(s16 *variable_name,
> +                       efi_guid_t *vendor, u32 attributes,
> +                       unsigned long data_size, void *data);
> +       void *get_next_high_mono_count;
> +       void (EFIAPI *reset_system)(enum efi_reset_type reset_type,
> +                                   efi_status_t reset_status,
> +                                   unsigned long data_size, void *reset_data);
> +       void *update_capsule;
> +       void *query_capsule_caps;
> +       void *query_variable_info;
> +};
> +
> +/*
> + *  EFI Configuration Table and GUID definitions
> + */

Please use one-line comment.

> +#define NULL_GUID \
> +       EFI_GUID(0x00000000, 0x0000, 0x0000, 0x00, 0x00, \
> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00)
> +
> +#define LOADED_IMAGE_PROTOCOL_GUID \
> +       EFI_GUID(0x5b1b31a1, 0x9562, 0x11d2, 0x8e, 0x3f, \
> +                0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
> +

Nits: can we move the above two to efi.h?

> +struct efi_system_table {
> +       struct efi_table_hdr hdr;
> +       unsigned long fw_vendor;   /* physical addr of CHAR16 vendor string */

What's CHAR16? Is it wchar_t?

> +       u32 fw_revision;
> +       unsigned long con_in_handle;
> +       struct efi_simple_input_interface *con_in;
> +       unsigned long con_out_handle;
> +       struct efi_simple_text_output_protocol *con_out;
> +       unsigned long stderr_handle;
> +       unsigned long std_err;
> +       struct efi_runtime_services *runtime;
> +       struct efi_boot_services *boottime;
> +       unsigned long nr_tables;
> +       unsigned long tables;
> +};
> +
> +struct efi_loaded_image {
> +       u32 revision;
> +       void *parent_handle;
> +       struct efi_system_table *system_table;
> +       void *device_handle;
> +       void *file_path;
> +       void *reserved;
> +       u32 load_options_size;
> +       void *load_options;
> +       void *image_base;
> +       aligned_u64 image_size;
> +       unsigned int image_code_type;
> +       unsigned int image_data_type;
> +       unsigned long unload;
> +};
> +
> +struct __packed efi_device_path {

__packed is not needed here.

> +       u8 type;
> +       u8 sub_type;
> +       u16 length;
> +};
> +
> +struct simple_text_output_mode {
> +       s32 max_mode;
> +       s32 mode;
> +       s32 attribute;
> +       s32 cursor_column;
> +       s32 cursor_row;
> +       bool cursor_visible;
> +};
> +
> +struct efi_simple_text_output_protocol {
> +       void *reset;
> +       efi_status_t (EFIAPI *output_string)(
> +                       struct efi_simple_text_output_protocol *this,
> +                       const unsigned short *str);
> +       void *test_string;
> +
> +       efi_status_t(EFIAPI *query_mode)(
> +                       struct efi_simple_text_output_protocol *this,
> +                       unsigned long mode_number, unsigned long *columns,
> +                       unsigned long *rows);
> +       efi_status_t(EFIAPI *set_mode)(
> +                       struct efi_simple_text_output_protocol *this,
> +                       unsigned long mode_number);
> +       efi_status_t(EFIAPI *set_attribute)(
> +                       struct efi_simple_text_output_protocol *this,
> +                       unsigned long attribute);
> +       efi_status_t(EFIAPI *clear_screen) (
> +                       struct efi_simple_text_output_protocol *this);
> +       efi_status_t(EFIAPI *set_cursor_position) (
> +                       struct efi_simple_text_output_protocol *this,
> +                       unsigned long column, unsigned long row);
> +       efi_status_t(EFIAPI *enable_cursor)(void *, bool enable);
> +       struct simple_text_output_mode *mode;
> +};
> +
> +struct efi_input_key {
> +       u16 scan_code;
> +       s16 unicode_char;
> +};
> +
> +struct efi_simple_input_interface {
> +       efi_status_t(EFIAPI *reset)(struct efi_simple_input_interface *this,
> +                       bool ExtendedVerification);
> +       efi_status_t(EFIAPI *read_key_stroke)(
> +                       struct efi_simple_input_interface *this,
> +                       struct efi_input_key *key);
> +       void *wait_for_key;
> +};
> +
> +#endif
> diff --git a/include/part_efi.h b/include/part_efi.h
> index d68ef3b..3012b91 100644
> --- a/include/part_efi.h
> +++ b/include/part_efi.h
> @@ -18,6 +18,8 @@
>  #ifndef _DISK_PART_EFI_H
>  #define _DISK_PART_EFI_H
>
> +#include <efi.h>
> +
>  #define MSDOS_MBR_SIGNATURE 0xAA55
>  #define EFI_PMBR_OSTYPE_EFI 0xEF
>  #define EFI_PMBR_OSTYPE_EFI_GPT 0xEE
> @@ -29,13 +31,6 @@
>  #define GPT_ENTRY_NUMBERS              128
>  #define GPT_ENTRY_SIZE                 128
>
> -#define EFI_GUID(a,b,c,d0,d1,d2,d3,d4,d5,d6,d7) \
> -       ((efi_guid_t) \
> -       {{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \
> -               (b) & 0xff, ((b) >> 8) & 0xff, \
> -               (c) & 0xff, ((c) >> 8) & 0xff, \
> -               (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
> -
>  #define PARTITION_SYSTEM_GUID \
>         EFI_GUID( 0xC12A7328, 0xF81F, 0x11d2, \
>                 0xBA, 0x4B, 0x00, 0xA0, 0xC9, 0x3E, 0xC9, 0x3B)
> diff --git a/lib/Kconfig b/lib/Kconfig
> index c98d399..4c8d38e 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -96,4 +96,6 @@ config ERRNO_STR
>           - if errno is null or positive number - a pointer to "Success" message
>           - if errno is negative - a pointer to errno related message
>
> +source lib/efi/Kconfig
> +
>  endmenu
> diff --git a/lib/Makefile b/lib/Makefile
> index 97ed398..4821779 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -7,6 +7,7 @@
>
>  ifndef CONFIG_SPL_BUILD
>
> +obj-$(CONFIG_EFI) += efi/
>  obj-$(CONFIG_RSA) += rsa/
>  obj-$(CONFIG_LZMA) += lzma/
>  obj-$(CONFIG_LZO) += lzo/
> diff --git a/lib/efi/Kconfig b/lib/efi/Kconfig
> new file mode 100644
> index 0000000..2b3dbd4
> --- /dev/null
> +++ b/lib/efi/Kconfig
> @@ -0,0 +1,33 @@
> +config EFI
> +       bool "Support running U-Boot from EFI"
> +       depends on X86
> +       help
> +         U-Boot can be started from EFI on certain platforms. This allows
> +         EFI to perform most of the system init and then jump to U-Boot for
> +         final system boot. Another option is to run U-Boot as an EFI
> +         application, with U-Boot using EFI's drivers instead of its own.
> +
> +choice
> +       prompt "Select EFI mode to use"
> +       depends on X86 && EFI
> +
> +config ARCH_EFI

Can we change this to EFI_APP?

> +       bool "Support running U-Boot from EFI"

Nits: better to be "Support running as an EFI application".

> +       help
> +         Build U-Boot as an application which can be started from EFI. This
> +         is useful for examining a platform in the early stages of porting
> +         U-Boot to it. It allows only very basic functionality, such as a
> +         command problem and memory and I/O functions. Use 'reset' to return

I don't understand what is 'a command problem'.

> +         to EFI.
> +
> +config EFI_RAM_SIZE
> +       hex "Amount of EFI RAM for U-Boot"
> +       depends on ARCH_EFI
> +       default 0x2000000
> +       help
> +         Set the amount of EFI RAM which is claimed by U-Boot for its own
> +         use. U-Boot allocates this from EFI on start-up (along with a few
> +         other smaller amounts) and it can never be increased after that.
> +         It is used as the RAM size in withU-Boot.

Missing space between 'with' and 'U-Boot'

> +
> +endchoice
> diff --git a/lib/efi/Makefile b/lib/efi/Makefile
> new file mode 100644
> index 0000000..137d2f9
> --- /dev/null
> +++ b/lib/efi/Makefile
> @@ -0,0 +1,7 @@
> +#
> +# (C) Copyright 2015 Google, Inc
> +#
> +# SPDX-License-Identifier:     GPL-2.0+
> +#
> +
> +obj-$(CONFIG_ARCH_EFI) += efi_app.o efi.o
> diff --git a/lib/efi/efi.c b/lib/efi/efi.c
> new file mode 100644
> index 0000000..acd276c
> --- /dev/null
> +++ b/lib/efi/efi.c
> @@ -0,0 +1,91 @@
> +/*
> + * Copyright (c) 2015 Google, Inc
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + *
> + * EFI information obtained here:
> + * http://wiki.phoenix.com/wiki/index.php/EFI_BOOT_SERVICES
> + *
> + * Common EFI functions
> + */
> +
> +#include <common.h>
> +#include <debug_uart.h>
> +#include <errno.h>
> +#include <linux/err.h>
> +#include <linux/types.h>
> +#include <efi.h>
> +#include <efi_api.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static void efi_memset(void *ptr, int ch, int size)
> +{
> +       char *dest = ptr;
> +
> +       while (size-- > 0)
> +               *dest++ = ch;
> +}

Why do we need this? Can we use memset() here?

> +
> +void efi_putc(struct efi_priv *priv, const char ch)
> +{
> +       struct efi_simple_text_output_protocol *con = priv->sys_table->con_out;
> +       uint16_t ucode[2];
> +
> +       ucode[0] = ch;
> +       ucode[1] = '\0';
> +       con->output_string(con, ucode);
> +}
> +
> +void efi_puts(struct efi_priv *priv, const char *str)
> +{
> +       while (*str)
> +               efi_putc(priv, *str++);
> +}
> +
> +int efi_init(struct efi_priv *priv, const char *banner, efi_handle_t image,
> +            struct efi_system_table *sys_table)
> +{
> +       efi_guid_t loaded_image_guid = LOADED_IMAGE_PROTOCOL_GUID;
> +       struct efi_boot_services *boot = sys_table->boottime;
> +       struct efi_loaded_image *loaded_image;
> +       int ret;
> +
> +       efi_memset(priv, '\0', sizeof(*priv));
> +       priv->sys_table = sys_table;
> +       priv->boot = sys_table->boottime;
> +       priv->parent_image = image;
> +       priv->run = sys_table->runtime;
> +
> +       efi_puts(priv, "U-Boot EFI ");
> +       efi_puts(priv, banner);
> +       efi_putc(priv, ' ');
> +
> +       ret = boot->open_protocol(priv->parent_image, &loaded_image_guid,
> +                                 (void **)&loaded_image, &priv->parent_image,
> +                                 NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> +       if (ret) {
> +               efi_puts(priv, "Failed to get loaded image protocol\n");
> +               return ret;
> +       }
> +       priv->image_data_type = loaded_image->image_data_type;
> +
> +       return 0;
> +}
> +
> +void *efi_malloc(struct efi_priv *priv, int size, efi_status_t *retp)
> +{
> +       struct efi_boot_services *boot = priv->boot;
> +       void *buf = NULL;
> +
> +       *retp = boot->allocate_pool(priv->image_data_type, size, &buf);
> +
> +       return buf;
> +}
> +
> +void efi_free(struct efi_priv *priv, void *ptr)
> +{
> +       struct efi_boot_services *boot = priv->boot;
> +
> +       boot->free_pool(ptr);
> +}
> diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c
> new file mode 100644
> index 0000000..b71669c
> --- /dev/null
> +++ b/lib/efi/efi_app.c
> @@ -0,0 +1,125 @@
> +/*
> + * Copyright (c) 2015 Google, Inc
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + *
> + * EFI information obtained here:
> + * http://wiki.phoenix.com/wiki/index.php/EFI_BOOT_SERVICES
> + *
> + * This file implements U-Boot running as an EFI application.
> + */
> +
> +#include <common.h>
> +#include <debug_uart.h>
> +#include <errno.h>
> +#include <linux/err.h>
> +#include <linux/types.h>
> +#include <efi.h>
> +#include <efi_api.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static struct efi_priv *global_priv;
> +
> +struct efi_system_table *efi_get_sys_table(void)
> +{
> +       return global_priv->sys_table;
> +}
> +
> +unsigned long efi_get_ram_base(void)
> +{
> +       return global_priv->ram_base;
> +}
> +
> +static efi_status_t setup_memory(struct efi_priv *priv)
> +{
> +       struct efi_boot_services *boot = priv->boot;
> +       efi_physical_addr_t addr;
> +       efi_status_t ret;
> +       int pages;
> +
> +       global_data_ptr = efi_malloc(priv, sizeof(struct global_data), &ret);

Where is this global_data_ptr defined? I guess this is introduced in
later patches. The introduction of global_data_ptr should be moved to
this patch.

> +       if (!global_data_ptr)
> +               return ret;
> +       memset(gd, '\0', sizeof(*gd));
> +

And gd here? If gd == global_data_ptr, why no just use gd without
adding global_data_ptr?

> +       gd->malloc_base = (ulong)efi_malloc(priv, CONFIG_SYS_MALLOC_F_LEN,
> +                                           &ret);
> +       if (!gd->malloc_base)
> +               return ret;
> +       pages = CONFIG_EFI_RAM_SIZE >> 12;
> +       addr = 1ULL << 32;

Please add a comment to describe what we are trying to do here, eg:
addr points to 4GiB, that means the allocated address should not
exceed that.

> +       ret = boot->allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
> +                                  priv->image_data_type, pages, &addr);
> +       if (ret) {
> +               printf("(using pool %lx) ", ret);
> +               priv->ram_base = (ulong)efi_malloc(priv, CONFIG_EFI_RAM_SIZE,
> +                                                  &ret);
> +               if (!priv->ram_base)
> +                       return ret;
> +               priv->use_pool_for_malloc = true;
> +       } else {
> +               priv->ram_base = addr;
> +       }
> +       gd->ram_size = pages << 12;
> +
> +       return 0;
> +}
> +
> +static void free_memory(struct efi_priv *priv)
> +{
> +       struct efi_boot_services *boot = priv->boot;
> +
> +       if (priv->use_pool_for_malloc)
> +               efi_free(priv, (void *)priv->ram_base);
> +       else
> +               boot->free_pages(priv->ram_base, gd->ram_size >> 12);
> +
> +       efi_free(priv, (void *)gd->malloc_base);
> +       efi_free(priv, gd);
> +       global_data_ptr = NULL;
> +}
> +
> +/**
> + * efi_main() - Start an EFI image
> + *
> + * This function is called by our EFI start-up code. It handles running
> + * U-Boot. If it returns, EFI will continue. Another way to get back to EFI
> + * is via reset_cpu().
> + */
> +efi_status_t efi_main(efi_handle_t image, struct efi_system_table *sys_table)
> +{
> +       struct efi_priv local_priv, *priv = &local_priv;
> +       efi_status_t ret;
> +
> +       /* Set up access to EFI data structures */
> +       efi_init(priv, "App", image, sys_table);
> +
> +       global_priv = priv;
> +
> +       /* Set up the EFI debug UART so that printf() works */
> +       debug_uart_init();

Where is this implemented for efi app?

> +
> +       ret = setup_memory(priv);
> +       if (ret) {
> +               printf("Failed to set up memory: ret=%lx\n", ret);

Is printf usable here? If yes, why do we create efi_puts() and efi_putc()?

> +               return ret;
> +       }
> +
> +       printf("starting\n");
> +
> +       board_init_f(GD_FLG_SKIP_RELOC);
> +       board_init_r(NULL, 0);
> +       free_memory(priv);
> +
> +       return EFI_SUCCESS;
> +}
> +
> +void reset_cpu(ulong addr)
> +{
> +       struct efi_priv *priv = global_priv;
> +
> +       free_memory(priv);
> +       printf("U-Boot EFI exiting\n");
> +       priv->boot->exit(priv->parent_image, EFI_SUCCESS, 0, NULL);
> +}
> --

Regards,
Bin



More information about the U-Boot mailing list