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

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

[Philipp Tomsich]

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.

For easier readability, I kept the functional changes separate from
the update of the device-tree-bindings documentation.


Changes in v2:
 * CONFIG_EFI_PARTITION_ENTRIES_OFF is configurable via Kconfig
 * added clamping to ensure that the parititions are never put into
   the first to LBAs of a device
 * documented the new /config property (and moved the documentation
   of /config properties into a separate file per Simon's review
   comment)


Philipp Tomsich (2):
  part_efi: support padding between the GPT header and partition entries
  part_efi: document device-tree binding for part_efi configuration

 disk/Kconfig                        | 13 ++++++++++
 disk/part_efi.c                     | 50 ++++++++++++++++++++++++++++++++++---
 doc/README.fdt-control              | 16 ------------
 doc/device-tree-bindings/config.txt | 22 ++++++++++++++++
 4 files changed, 81 insertions(+), 20 deletions(-)
 create mode 100644 doc/device-tree-bindings/config.txt

-- 
1.9.1

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

[Philipp Tomsich]

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@theobroma-systems.com>
---
 disk/Kconfig    | 13 +++++++++++++
 disk/part_efi.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 59 insertions(+), 4 deletions(-)

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
+	 * 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. */
+	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

[U-Boot] [PATCH v2 2/2] part_efi: document device-tree binding for part_efi configuration

[Philipp Tomsich]

This moves the description of the /config node from README.fdt-control
into a separate file doc/device-tree-bindings/config.txt and adds the
documentation on the new u-boot,efi-partition-entries-offset property
(which overrides CONFIG_EFI_PARTITION_ENTRIES_OFF, if present).

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---
 doc/README.fdt-control              | 16 ----------------
 doc/device-tree-bindings/config.txt | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 16 deletions(-)
 create mode 100644 doc/device-tree-bindings/config.txt

diff --git a/doc/README.fdt-control b/doc/README.fdt-control
index 2913fcb..c965629 100644
--- a/doc/README.fdt-control
+++ b/doc/README.fdt-control
@@ -168,22 +168,6 @@ After board configuration is done, fdt supported u-boot can be build in two ways
     $ make DEVICE_TREE=<dts-file-name>
 
 
-Configuration Options
----------------------
-
-A number of run-time configuration options are provided in the /config node
-of the control device tree. You can access these using fdtdec_get_config_int(),
-fdtdec_get_config_bool() and fdtdec_get_config_string().
-
-Available options are:
-
-silent-console
-	If present and non-zero, the console is silenced by default on boot.
-
-no-keyboard
-	Tells U-Boot not to expect an attached keyboard with a VGA console
-
-
 Limitations
 -----------
 
diff --git a/doc/device-tree-bindings/config.txt b/doc/device-tree-bindings/config.txt
new file mode 100644
index 0000000..5640bae
--- /dev/null
+++ b/doc/device-tree-bindings/config.txt
@@ -0,0 +1,22 @@
+The /config node (Configuration Options)
+----------------------------------------
+
+A number of run-time configuration options are provided in the /config node
+of the control device tree. You can access these using fdtdec_get_config_int(),
+fdtdec_get_config_bool() and fdtdec_get_config_string().
+
+Available options are:
+
+silent-console
+	If present and non-zero, the console is silenced by default on boot.
+
+no-keyboard
+	Tells U-Boot not to expect an attached keyboard with a VGA console
+
+u-boot,efi-partition-entries-offset
+	If present, this provides an offset (in bytes, from the start of a
+	device) that should be skipped over before the partition entries.
+	This is used by the EFI/GPT partition implementation when a device
+	is formatted.
+
+	This setting will override any values configured via Kconfig.
-- 
1.9.1

[U-Boot] [PATCH v2 2/2] part_efi: document device-tree binding for part_efi configuration

[Simon Glass]

On 22 February 2017 at 11:17, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> This moves the description of the /config node from README.fdt-control
> into a separate file doc/device-tree-bindings/config.txt and adds the
> documentation on the new u-boot,efi-partition-entries-offset property
> (which overrides CONFIG_EFI_PARTITION_ENTRIES_OFF, if present).
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>  doc/README.fdt-control              | 16 ----------------
>  doc/device-tree-bindings/config.txt | 22 ++++++++++++++++++++++
>  2 files changed, 22 insertions(+), 16 deletions(-)
>  create mode 100644 doc/device-tree-bindings/config.txt

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

(if you end up resending, this would be better as two separate patches)

[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@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@theobroma-systems.com>
> ---
>  disk/Kconfig    | 13 +++++++++++++
>  disk/part_efi.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 59 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg@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