[U-Boot] [PATCH v2 1/2] part_efi: support padding between the GPT header and partition entries

[Simon Glass]

Hi Philipp,

On 22 February 2017 at 11:17, Philipp Tomsich
<philipp.tomsich at theobroma-systems.com> wrote:
> Some architectures require their SPL loader at a fixed address within
> the first 16KB of the disk. To avoid an overlap with the partition
> entries of the EFI partition table, the first safe offset (in bytes,
> from the start of the device) for the entries can be set through
> CONFIG_EFI_PARTITION_ENTRIES_OFF (via Kconfig)
>
> When formatting a device with an EFI partition table, we may need to
> leave a gap between the GPT header (always in LBA 1) and the partition
> entries. The GPT header already contains a field to specify the
> on-disk location, which has so far always been set to LBA 2. With this
> change, a configurable offset will be translated into a LBA address
> indicating where to put the entries.
>
> Now also allows an override via device-tree using a config-node (see
> doc/device-tree-bindings/config.txt for documentation).
>
> Tested (exporting an internal MMC formatted with this) against Linux,
> MacOS X and Windows.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
> ---
>  disk/Kconfig    | 13 +++++++++++++
>  disk/part_efi.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 59 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

But please fix comment style below.

>
> diff --git a/disk/Kconfig b/disk/Kconfig
> index 16ff52d..8e328b4 100644
> --- a/disk/Kconfig
> +++ b/disk/Kconfig
> @@ -79,6 +79,19 @@ config EFI_PARTITION
>           common when EFI is the bootloader.  Note 2TB partition limit;
>           see disk/part_efi.c
>
> +config EFI_PARTITION_ENTRIES_OFF
> +        int "Offset (in bytes) of the EFI partition entries"
> +       depends on EFI_PARTITION
> +       default 0
> +       help
> +         Specify an earliest location (in bytes) where the partition
> +         entries may be located. This is meant to allow "punching a
> +         hole into a device" to create a gap for an SPL, its payload
> +         and the U-Boot environment.
> +
> +         If unsure, leave at 0 (which will locate the partition
> +         entries at the first possible LBA following the GPT header).
> +
>  config SPL_EFI_PARTITION
>         bool "Enable EFI GPT partition table for SPL"
>         depends on  SPL && PARTITIONS
> diff --git a/disk/part_efi.c b/disk/part_efi.c
> index 893cbbd..fafca25 100644
> --- a/disk/part_efi.c
> +++ b/disk/part_efi.c
> @@ -13,6 +13,7 @@
>  #include <asm/unaligned.h>
>  #include <common.h>
>  #include <command.h>
> +#include <fdtdec.h>
>  #include <ide.h>
>  #include <inttypes.h>
>  #include <malloc.h>
> @@ -373,8 +374,8 @@ int write_gpt_table(struct blk_desc *dev_desc,
>         if (blk_dwrite(dev_desc, 1, 1, gpt_h) != 1)
>                 goto err;
>
> -       if (blk_dwrite(dev_desc, 2, pte_blk_cnt, gpt_e)
> -           != pte_blk_cnt)
> +       if (blk_dwrite(dev_desc, le64_to_cpu(gpt_h->partition_entry_lba),
> +                      pte_blk_cnt, gpt_e) != pte_blk_cnt)
>                 goto err;
>
>         prepare_backup_gpt_header(gpt_h);
> @@ -498,6 +499,45 @@ int gpt_fill_pte(gpt_header *gpt_h, gpt_entry *gpt_e,
>         return 0;
>  }
>
> +static uint32_t partition_entries_offset(struct blk_desc *dev_desc)
> +{
> +       uint32_t offset_blks = 2;
> +       int config_offset;
> +
> +#if defined(CONFIG_EFI_PARTITION_ENTRIES_OFF)
> +       /* Some architectures require their SPL loader at a fixed

First and last line of multi-line comment should not have any text:

/*
 * Some arch...
 * ...
 * ...
 */

> +        * address within the first 16KB of the disk.  To avoid an
> +        * overlap with the partition entries of the EFI partition
> +        * table, the first safe offset (in bytes, from the start of
> +        * the disk) for the entries can be set in
> +        * CONFIG_EFI_PARTITION_ENTRIES_OFF.
> +        */
> +       offset_blks =
> +               PAD_TO_BLOCKSIZE(CONFIG_EFI_PARTITION_ENTRIES_OFF, dev_desc);
> +#endif
> +
> +#if defined(CONFIG_OF_CONTROL)
> +       /* Allow the offset of the first partition entires (in bytes
> +          from the start of the device) to be specified as a property
> +          of the device tree '/config' node. */

Also here

> +       config_offset = fdtdec_get_config_int(gd->fdt_blob,
> +                                             "u-boot,efi-partition-entries-offset",
> +                                             -EINVAL);
> +       if (config_offset != -EINVAL)
> +               offset_blks = PAD_TO_BLOCKSIZE(config_offset, dev_desc);
> +#endif
> +
> +       debug("efi: partition entries offset (in blocks): %d\n", offset_blks);
> +
> +       /* The earliest LBA this can be at is LBA#2 (i.e. right behind
> +        * the (protective) MBR and the GPT header.
> +        */
> +       if (offset_blks < 2)
> +               offset_blks = 2;
> +
> +       return offset_blks;
> +}
> +
>  int gpt_fill_header(struct blk_desc *dev_desc, gpt_header *gpt_h,
>                 char *str_guid, int parts_count)
>  {
> @@ -506,9 +546,11 @@ int gpt_fill_header(struct blk_desc *dev_desc, gpt_header *gpt_h,
>         gpt_h->header_size = cpu_to_le32(sizeof(gpt_header));
>         gpt_h->my_lba = cpu_to_le64(1);
>         gpt_h->alternate_lba = cpu_to_le64(dev_desc->lba - 1);
> -       gpt_h->first_usable_lba = cpu_to_le64(34);
>         gpt_h->last_usable_lba = cpu_to_le64(dev_desc->lba - 34);
> -       gpt_h->partition_entry_lba = cpu_to_le64(2);
> +       gpt_h->partition_entry_lba =
> +               cpu_to_le64(partition_entries_offset(dev_desc));
> +       gpt_h->first_usable_lba =
> +               cpu_to_le64(le64_to_cpu(gpt_h->partition_entry_lba) + 32);
>         gpt_h->num_partition_entries = cpu_to_le32(GPT_ENTRY_NUMBERS);
>         gpt_h->sizeof_partition_entry = cpu_to_le32(sizeof(gpt_entry));
>         gpt_h->header_crc32 = 0;
> --
> 1.9.1
>

Regards,
Simon