diff mbox

[PATCHv2,4/4] Add package cbootimage-configs

Message ID 1458227038-16383-5-git-send-email-julian@jusst.de
State Changes Requested
Headers show

Commit Message

Julian Scheel March 17, 2016, 3:03 p.m. UTC
Add package for cbootimage configurations that contains build scripts to
generate bct and image files for various tegra based boards using cbootimage
file.
Additionally the package makefile includes extra hooks to generate
self-contained flasher binaries based on u-boot, that are actually able to
flash a u-boot instance into mmc memory when loaded to tegra via tegrarcm.

Signed-off-by: Julian Scheel <julian@jusst.de>
---
Changes in v2:
--------------
- Add proper BR2_PACKAGE prefix to variable names
- Use menuconfig
- Read gpt spec from file, to avoid ugly double quotes
- Read intermediate files from UBOOT_BUILDDIR to avoid glob hack on
  UBOOT_IMAGE_NAME and keep BINARIES_DIR uncluttered
- Use github helper function
- Add hash
---
 package/Config.in                                  |   1 +
 package/cbootimage-configs/Config.in               | 122 ++++++++++++++++++++
 package/cbootimage-configs/cbootimage-configs.hash |   2 +
 package/cbootimage-configs/cbootimage-configs.mk   | 128 +++++++++++++++++++++
 package/cbootimage-configs/gpt-table               |   1 +
 5 files changed, 254 insertions(+)
 create mode 100644 package/cbootimage-configs/Config.in
 create mode 100644 package/cbootimage-configs/cbootimage-configs.hash
 create mode 100644 package/cbootimage-configs/cbootimage-configs.mk
 create mode 100644 package/cbootimage-configs/gpt-table

Comments

Thomas Petazzoni April 19, 2016, 9:08 p.m. UTC | #1
Hello,

On Thu, 17 Mar 2016 16:03:58 +0100, Julian Scheel wrote:
> Add package for cbootimage configurations that contains build scripts to
> generate bct and image files for various tegra based boards using cbootimage
> file.
> Additionally the package makefile includes extra hooks to generate
> self-contained flasher binaries based on u-boot, that are actually able to
> flash a u-boot instance into mmc memory when loaded to tegra via tegrarcm.
> 
> Signed-off-by: Julian Scheel <julian@jusst.de>

To be honest, this is too complicated. This package is doing too much
stuff in its .mk file. At some point, you mention that your .mk code is
heavily inspired by
https://github.com/NVIDIA/tegra-uboot-flasher-scripts. Why don't you
use/package those scripts instead?

Or maybe this thing should do less stuff, and simply provide the
necessary tools for a post-image script to do the job.

It would be useful if you could submit the defconfig for a Tegra based
board as part of this series so that we can see how it all fits
together.

I've added some more comments/questions below.



> +if BR2_PACKAGE_CBOOTIMAGE_CONFIGS
> +
> +config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD
> +	string "cbootimage board to build"

I think the "cbootimage" prefix in the string is useless here, and in
the other options. In menuconfig, we clearly see it's a suboption.


> +config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_SIGNED
> +	bool "sign generated image for secureboot"
> +	help
> +	  Sign the image using a provided ssl key. This makes it possible to
> +	  run the image in secureboot environments with the matching key being
> +	  burned to the tegra fuses.
> +	  Be aware that the generated image will have the extension simg
> +	  instead of img if signing is enabled.
> +
> +	  Compatible key files can be generated with openssl:
> +	  openssl genrsa -out rsa_priv.pem 2048
> +
> +config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_KEY
> +	string "pkc file name"
> +	depends on BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_SIGNED
> +	help
> +	  Key file to use for signing the loader image.

When there is such a bool to enable/disable + one string giving the
argument, I believe we generally prefer to just have the string. When
the string is empty -> feature is disabled.

> +if BR2_PACKAGE_CBOOTIMAGE_CONFIGS_GENERATE_FLASHER
> +
> +config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_CREATE_GPT
> +	bool "write partition table to mmc 0"
> +	help
> +	  Add a flashing step which is writing a custom gpt partition table
> +	  onto the target devices first mmc storage
> +
> +config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_GPT_FILE
> +	string "partition table file"
> +	depends on BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_CREATE_GPT
> +	default "package/cbootimage-configs/gpt-table"
> +	help
> +	  Provide a file that contains the paritition table to write to mmc.
> +	  Use format as specified in u-boot's README.gpt documentation file.
> +
> +	  Make sure to escape variable properly, as u-boot shall not evaluate
> +	  the variables when setting the environment variable, but only when
> +	  running the gpt command.
> +
> +	  Example file content:
> +	  uuid_disk=\$\{disk_uuid\};name=rootfs,size=2000MiB,uuid=\$\{rootfs_uuid\};name=data,size=-,uuid=\$\{data_uuid\}

Same here.

> diff --git a/package/cbootimage-configs/cbootimage-configs.hash b/package/cbootimage-configs/cbootimage-configs.hash
> new file mode 100644
> index 0000000..a4fd518
> --- /dev/null
> +++ b/package/cbootimage-configs/cbootimage-configs.hash
> @@ -0,0 +1,2 @@
> +# Locally computed
> +sha256  f9eaa2e88fa24ea348d6358f18e4bab58ac21c586b5b6cb5300e21a6e1d9de83  cbootimage-configs-9d45319.tar.gz
> diff --git a/package/cbootimage-configs/cbootimage-configs.mk b/package/cbootimage-configs/cbootimage-configs.mk
> new file mode 100644
> index 0000000..69d6d29
> --- /dev/null
> +++ b/package/cbootimage-configs/cbootimage-configs.mk
> @@ -0,0 +1,128 @@
> +################################################################################
> +#
> +# cbootimage-configs
> +#
> +################################################################################
> +
> +CBOOTIMAGE_CONFIGS_VERSION = 9d45319
> +CBOOTIMAGE_CONFIGS_SITE = $(call github,avionic-design,cbootimage-configs,$(CBOOTIMAGE_CONFIGS_VERSION))

Why do you use the Avionic Design fork and not the original NVIDIA
repository, which you reference as the upstream in your Config.in help
text?

> +CBOOTIMAGE_CONFIGS_DEPENDENCIES = host-cbootimage uboot
> +CBOOTIMAGE_CONFIGS_INSTALL_TARGET = NO
> +CBOOTIMAGE_CONFIGS_INSTALL_IMAGES = YES
> +
> +CONFIGS_BUILD = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD))
> +BCT_NAME = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BCT))
> +IMG_NAME = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG))
> +EXTRA_BOOTCMD = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_EXTRA_BOOTCMD))
> +GPT_FILE = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_GPT_FILE))
> +DFU_MAP = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_DFU_MAP))
> +KEY_FILE = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_KEY))

The variable namespace in Buildroot is global, so they should always be
prefixed by the name of the package.

> +ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS),y)
> +ifeq ($(CONFIGS_BUILD),)
> +	$(warning BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD=$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD))

That's debug stuff.

> +	$(warning CONFIGS_BUILD=$(CONFIGS_BUILD))

Ditto.

> +	$(error No cbootimage-configuration specififed to build. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD setting)
> +endif
> +ifeq ($(BCT_NAME),)
> +	$(error No bct filename specified. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BCT setting)
> +endif
> +ifeq ($(IMG_NAME),)
> +	$(error No img filename specified. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG setting)
> +endif
> +ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_CREATE_GPT),y)
> +ifeq ($(GPT_FILE),)
> +	$(error No gpt partitions specified. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_GPT_FILE setting)
> +endif
> +endif
> +ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_DFU),y)
> +ifeq ($(DFU_MAP),)
> +	$(error No dfu map specified. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_DFU_MAP setting)
> +endif
> +endif
> +ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_SIGNED),y)
> +ifeq ($(KEY_FILE),)
> +	$(error No key file specified. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_KEY setting)
> +endif
> +KEY_OPTS = skb=$(KEY_FILE)
> +else
> +KEY_OPTS =

Incorrect variable name. Also, I believe this should be separated from
the big list of checks.

> +endif
> +
> +TARGET_SIZE = $(TARGET_CROSS)size

To be moved to package/Makefile.in I believe, in a separate patch.

> +endif
> +
> +define CBOOTIMAGE_CONFIGS_BUILD_CMDS
> +	cp $(UBOOT_BUILDDIR)/u-boot-dtb-tegra.bin $(@D)/$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD)/u-boot.bin
> +	(cd $(@D)/$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD) && $(HOST_MAKE_ENV) $(HOST_CONFIGURE_OPTS) $(MAKE) $(KEY_OPTS))

use $(MAKE) -C $(@D)/$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD)

> +endef
> +
> +# Generate a self-contained u-boot image, flashing u-boot to mmc
> +define CBOOTIMAGE_CONFIGS_BUILD_FLASHER_CMD
> +	# u-boot bss size: ${BSS_SIZE}
> +	# u-boot-nodtb-tegra.bin size: ${UBOOT_NODTB_TEGRA_SIZE}
> +	# u-boot.dtb size: ${UBOOT_DTB_SIZE}
> +	# ${IMG_NAME} size: ${IMG_SIZE}
> +	# u-boot plus dtb size: ${UBOOT_PLUS_DTB_SIZE}
> +	# padded size: ${PADDED_SIZE}
> +	# bootcmd: $(UBOOT_FLASH_ENV)
> +	# gpt-file: $(PWD)/$(GPT_FILE)

I don't see the usefulness of this comment.

> +
> +	cp $(UBOOT_BUILDDIR)/u-boot.dtb ${DTB_FILE}
> +	# Patch environment into u-boot dtb
> +	$(HOST_MAKE_ENV) fdtput -p -t x ${DTB_FILE} '/config' 'bootdelay' '0xfffffffe'
> +	$(HOST_MAKE_ENV) fdtput -p -t s ${DTB_FILE} '/config' 'bootcmd' $(UBOOT_FLASH_ENV)

What guarantees you that fdtput is available? You need to depend on
host-dtc to be sure that it's available.

> +
> +	# Assemble u-boot-flasher.bin
> +	cp $(UBOOT_BUILDDIR)/u-boot-nodtb-tegra.bin $(BINARIES_DIR)/u-boot-flasher.bin
> +	cat ${DTB_FILE} >> $(BINARIES_DIR)/u-boot-flasher.bin

Use $() to reference variables. Why don't you do a single cat here ?

> +	truncate -s $(PADDED_SIZE) $(BINARIES_DIR)/u-boot-flasher.bin
> +	cat $(@D)/$(CONFIGS_BUILD)/$(IMG_NAME) >> $(BINARIES_DIR)/u-boot-flasher.bin
> +endef
> +
> +# Computations for assembling u-boot with custom dtb mostly stolen from
> +# https://github.com/NVIDIA/tegra-uboot-flasher-scripts
> +# All credits go to Stephen Warren
> +ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_GENERATE_FLASHER),y)
> +LOADADDR = 0x80108000 # FIXME: this is Tegra124 only
> +BSS_SIZE = $(shell $(TARGET_SIZE) -A $(UBOOT_BUILDDIR)/u-boot | grep ".bss\b" | tr -s ' ' | cut -d ' ' -f 2)
> +UBOOT_NODTB_TEGRA_SIZE = $(shell stat -c %s $(UBOOT_BUILDDIR)/u-boot-nodtb-tegra.bin)
> +UBOOT_DTB_SIZE = $(shell stat -c %s $(UBOOT_BUILDDIR)/u-boot.dtb)
> +IMG_SIZE = $(shell stat -c %s $(@D)/$(CONFIGS_BUILD)/$(IMG_NAME))
> +IMG_SIZE_SECTORS = $(shell echo $$(( $(IMG_SIZE) / 512 )) )
> +IMG_SIZE_SECTORS_HEX = $(shell printf '0x%x' $(IMG_SIZE_SECTORS) )
> +UBOOT_PLUS_DTB_SIZE = $(shell echo $$(( $(UBOOT_NODTB_TEGRA_SIZE) + $(UBOOT_DTB_SIZE) )) )
> +# Keep BSS area clear, acquire an extra 4kb for incereased dtb, align to 4k
> +# boundary
> +PADDED_SIZE = $(shell echo $$(( $(UBOOT_PLUS_DTB_SIZE) + $(BSS_SIZE) + (2 * 4 * 1024) - 1 & ~((4 * 1024) - 1) )) )
> +IMAGEADDR = $(shell echo $$(( $(LOADADDR) + $(PADDED_SIZE) )) )
> +IMAGEADDR_HEX = $(shell printf '0x%x' $(IMAGEADDR) )

This is really the messy part. That's clearly way too much stuff
encoded into a Buildroot package.

> +ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_CREATE_GPT),y)
> +PWD = $(shell pwd)

Debugging stuff?

> +GPT_SPEC = $(shell cat $(GPT_FILE))

If it's just a string, why is it the path to a file rather than having
direcly the value in the Config.in option?

> +GPT_BOOTCMD = echo >>> Write partition table to MMC; gpt write mmc 0 '$(GPT_SPEC)'; mmc rescan
> +else
> +GPT_BOOTCMD =

Not needed, the default for a variable is to be empty.

Also, remember: all variables should be prefixed by the name of the
package.

> +endif
> +
> +ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_DFU),y)
> +DFU_BOOTCMD = echo >>> Enter DFU mode; setenv dfu_alt_info '$(DFU_MAP)'; dfu 0 mmc 0;
> +else
> +DFU_BOOTCMD =
> +endif
> +
> +UBOOT_FLASH_ENV = "echo >>> Write image to MMC; mmc dev 0 1; mmc write ${IMAGEADDR_HEX} 0 ${IMG_SIZE_SECTORS_HEX}; echo >>> Configure environment; env default -f -a; saveenv; $(GPT_BOOTCMD) $(EXTRA_BOOTCMD); $(DFU_BOOTCMD) echo >>> Resetting system; reset"

All this U-Boot command construction seem really project-specific, and
I don't see why we should hardcode all that stuff in a Buildroot
package. Buildroot should give the tools to generate such a U-Boot
image, but should not hardcode so much logic IMO.

> +DTB_FILE = $(@D)/u-boot-flasher.dtb
> +
> +# Add the actual assembly hook
> +CBOOTIMAGE_CONFIGS_POST_INSTALL_IMAGES_HOOKS += CBOOTIMAGE_CONFIGS_BUILD_FLASHER_CMD
> +endif

Thomas
Arnout Vandecappelle April 19, 2016, 10:36 p.m. UTC | #2
On 04/19/16 23:08, Thomas Petazzoni wrote:
> Hello,
>
> On Thu, 17 Mar 2016 16:03:58 +0100, Julian Scheel wrote:
[snip]
>> >+TARGET_SIZE = $(TARGET_CROSS)size
> To be moved to package/Makefile.in I believe, in a separate patch.

  Why do you need a cross-size anyway? size, like readelf, should be universal, no?


  Regards,
  Arnout
Julian Scheel April 20, 2016, 6:18 a.m. UTC | #3
Hi Thomas,

thanks for the review, see comments below.

On 19.04.2016 23:08, Thomas Petazzoni wrote:
> Hello,
>
> On Thu, 17 Mar 2016 16:03:58 +0100, Julian Scheel wrote:
>> Add package for cbootimage configurations that contains build scripts to
>> generate bct and image files for various tegra based boards using cbootimage
>> file.
>> Additionally the package makefile includes extra hooks to generate
>> self-contained flasher binaries based on u-boot, that are actually able to
>> flash a u-boot instance into mmc memory when loaded to tegra via tegrarcm.
>>
>> Signed-off-by: Julian Scheel <julian@jusst.de>
>
> To be honest, this is too complicated. This package is doing too much
> stuff in its .mk file. At some point, you mention that your .mk code is
> heavily inspired by
> https://github.com/NVIDIA/tegra-uboot-flasher-scripts. Why don't you
> use/package those scripts instead?

The problem with tegra-uboot-flasher-scripts is, that it does a lot more 
than what this package does. In fact it includes build logic for all the 
required tools (like dtc, tegrarcm, cbootimage, u-boot), which wouldn't 
work out well in the context of buildroot. Using externally provided 
builds isn't easily possible without major abuse of the scripts, which I 
don't think would be merged upstream anyway.

> Or maybe this thing should do less stuff, and simply provide the
> necessary tools for a post-image script to do the job.

If in the end it's decided that this is too complicated for the package 
makefile I'd rather create a dedicated package that handles this. So, 
that the magic would be in done in some independently maintained script 
and we only need a buildroot package that heads the options to that package.
It just felt a bit like overkill for what it does in the end.

> It would be useful if you could submit the defconfig for a Tegra based
> board as part of this series so that we can see how it all fits
> together.

Sure, I'll prepare a defconfig.

> I've added some more comments/questions below.
>
>
>
>> +if BR2_PACKAGE_CBOOTIMAGE_CONFIGS
>> +
>> +config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD
>> +	string "cbootimage board to build"
>
> I think the "cbootimage" prefix in the string is useless here, and in
> the other options. In menuconfig, we clearly see it's a suboption.

Ack.

>> +config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_SIGNED
>> +	bool "sign generated image for secureboot"
>> +	help
>> +	  Sign the image using a provided ssl key. This makes it possible to
>> +	  run the image in secureboot environments with the matching key being
>> +	  burned to the tegra fuses.
>> +	  Be aware that the generated image will have the extension simg
>> +	  instead of img if signing is enabled.
>> +
>> +	  Compatible key files can be generated with openssl:
>> +	  openssl genrsa -out rsa_priv.pem 2048
>> +
>> +config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_KEY
>> +	string "pkc file name"
>> +	depends on BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_SIGNED
>> +	help
>> +	  Key file to use for signing the loader image.
>
> When there is such a bool to enable/disable + one string giving the
> argument, I believe we generally prefer to just have the string. When
> the string is empty -> feature is disabled.

Makes sense, thanks.

>> +if BR2_PACKAGE_CBOOTIMAGE_CONFIGS_GENERATE_FLASHER
>> +
>> +config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_CREATE_GPT
>> +	bool "write partition table to mmc 0"
>> +	help
>> +	  Add a flashing step which is writing a custom gpt partition table
>> +	  onto the target devices first mmc storage
>> +
>> +config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_GPT_FILE
>> +	string "partition table file"
>> +	depends on BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_CREATE_GPT
>> +	default "package/cbootimage-configs/gpt-table"
>> +	help
>> +	  Provide a file that contains the paritition table to write to mmc.
>> +	  Use format as specified in u-boot's README.gpt documentation file.
>> +
>> +	  Make sure to escape variable properly, as u-boot shall not evaluate
>> +	  the variables when setting the environment variable, but only when
>> +	  running the gpt command.
>> +
>> +	  Example file content:
>> +	  uuid_disk=\$\{disk_uuid\};name=rootfs,size=2000MiB,uuid=\$\{rootfs_uuid\};name=data,size=-,uuid=\$\{data_uuid\}
>
> Same here.
>
>> diff --git a/package/cbootimage-configs/cbootimage-configs.hash b/package/cbootimage-configs/cbootimage-configs.hash
>> new file mode 100644
>> index 0000000..a4fd518
>> --- /dev/null
>> +++ b/package/cbootimage-configs/cbootimage-configs.hash
>> @@ -0,0 +1,2 @@
>> +# Locally computed
>> +sha256  f9eaa2e88fa24ea348d6358f18e4bab58ac21c586b5b6cb5300e21a6e1d9de83  cbootimage-configs-9d45319.tar.gz
>> diff --git a/package/cbootimage-configs/cbootimage-configs.mk b/package/cbootimage-configs/cbootimage-configs.mk
>> new file mode 100644
>> index 0000000..69d6d29
>> --- /dev/null
>> +++ b/package/cbootimage-configs/cbootimage-configs.mk
>> @@ -0,0 +1,128 @@
>> +################################################################################
>> +#
>> +# cbootimage-configs
>> +#
>> +################################################################################
>> +
>> +CBOOTIMAGE_CONFIGS_VERSION = 9d45319
>> +CBOOTIMAGE_CONFIGS_SITE = $(call github,avionic-design,cbootimage-configs,$(CBOOTIMAGE_CONFIGS_VERSION))
>
> Why do you use the Avionic Design fork and not the original NVIDIA
> repository, which you reference as the upstream in your Config.in help
> text?

Just because our configurations are not yet merged to the upstream 
repository. I really need to push on that, as this package really shall 
use upstream. Although it might be wise to have the usual 
custom-repository option like it's done for kernel and u-boot. It's not 
that unlikely that people would have configs in custom repositories 
which are not merged upstream.

>> +CBOOTIMAGE_CONFIGS_DEPENDENCIES = host-cbootimage uboot
>> +CBOOTIMAGE_CONFIGS_INSTALL_TARGET = NO
>> +CBOOTIMAGE_CONFIGS_INSTALL_IMAGES = YES
>> +
>> +CONFIGS_BUILD = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD))
>> +BCT_NAME = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BCT))
>> +IMG_NAME = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG))
>> +EXTRA_BOOTCMD = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_EXTRA_BOOTCMD))
>> +GPT_FILE = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_GPT_FILE))
>> +DFU_MAP = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_DFU_MAP))
>> +KEY_FILE = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_KEY))
>
> The variable namespace in Buildroot is global, so they should always be
> prefixed by the name of the package.

Ok.

>> +ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS),y)
>> +ifeq ($(CONFIGS_BUILD),)
>> +	$(warning BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD=$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD))
>
> That's debug stuff.

Yep, already removed in my tree :)

>> +	$(warning CONFIGS_BUILD=$(CONFIGS_BUILD))
>
> Ditto.
>
>> +	$(error No cbootimage-configuration specififed to build. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD setting)
>> +endif
>> +ifeq ($(BCT_NAME),)
>> +	$(error No bct filename specified. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BCT setting)
>> +endif
>> +ifeq ($(IMG_NAME),)
>> +	$(error No img filename specified. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG setting)
>> +endif
>> +ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_CREATE_GPT),y)
>> +ifeq ($(GPT_FILE),)
>> +	$(error No gpt partitions specified. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_GPT_FILE setting)
>> +endif
>> +endif
>> +ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_DFU),y)
>> +ifeq ($(DFU_MAP),)
>> +	$(error No dfu map specified. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_DFU_MAP setting)
>> +endif
>> +endif
>> +ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_SIGNED),y)
>> +ifeq ($(KEY_FILE),)
>> +	$(error No key file specified. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_KEY setting)
>> +endif
>> +KEY_OPTS = skb=$(KEY_FILE)
>> +else
>> +KEY_OPTS =
>
> Incorrect variable name. Also, I believe this should be separated from
> the big list of checks.

Yeah, you're probably right, that should be separated.

>> +endif
>> +
>> +TARGET_SIZE = $(TARGET_CROSS)size
>
> To be moved to package/Makefile.in I believe, in a separate patch.

Ok.

>> +endif
>> +
>> +define CBOOTIMAGE_CONFIGS_BUILD_CMDS
>> +	cp $(UBOOT_BUILDDIR)/u-boot-dtb-tegra.bin $(@D)/$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD)/u-boot.bin
>> +	(cd $(@D)/$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD) && $(HOST_MAKE_ENV) $(HOST_CONFIGURE_OPTS) $(MAKE) $(KEY_OPTS))
>
> use $(MAKE) -C $(@D)/$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD)

Ack.

>> +endef
>> +
>> +# Generate a self-contained u-boot image, flashing u-boot to mmc
>> +define CBOOTIMAGE_CONFIGS_BUILD_FLASHER_CMD
>> +	# u-boot bss size: ${BSS_SIZE}
>> +	# u-boot-nodtb-tegra.bin size: ${UBOOT_NODTB_TEGRA_SIZE}
>> +	# u-boot.dtb size: ${UBOOT_DTB_SIZE}
>> +	# ${IMG_NAME} size: ${IMG_SIZE}
>> +	# u-boot plus dtb size: ${UBOOT_PLUS_DTB_SIZE}
>> +	# padded size: ${PADDED_SIZE}
>> +	# bootcmd: $(UBOOT_FLASH_ENV)
>> +	# gpt-file: $(PWD)/$(GPT_FILE)
>
> I don't see the usefulness of this comment.

Debugging. I'll remove it.

>> +
>> +	cp $(UBOOT_BUILDDIR)/u-boot.dtb ${DTB_FILE}
>> +	# Patch environment into u-boot dtb
>> +	$(HOST_MAKE_ENV) fdtput -p -t x ${DTB_FILE} '/config' 'bootdelay' '0xfffffffe'
>> +	$(HOST_MAKE_ENV) fdtput -p -t s ${DTB_FILE} '/config' 'bootcmd' $(UBOOT_FLASH_ENV)
>
> What guarantees you that fdtput is available? You need to depend on
> host-dtc to be sure that it's available.

Ack, dependency is needed.

>> +
>> +	# Assemble u-boot-flasher.bin
>> +	cp $(UBOOT_BUILDDIR)/u-boot-nodtb-tegra.bin $(BINARIES_DIR)/u-boot-flasher.bin
>> +	cat ${DTB_FILE} >> $(BINARIES_DIR)/u-boot-flasher.bin
>
> Use $() to reference variables. Why don't you do a single cat here ?

Good question. I'll change it to single cat.

>> +	truncate -s $(PADDED_SIZE) $(BINARIES_DIR)/u-boot-flasher.bin
>> +	cat $(@D)/$(CONFIGS_BUILD)/$(IMG_NAME) >> $(BINARIES_DIR)/u-boot-flasher.bin
>> +endef
>> +
>> +# Computations for assembling u-boot with custom dtb mostly stolen from
>> +# https://github.com/NVIDIA/tegra-uboot-flasher-scripts
>> +# All credits go to Stephen Warren
>> +ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_GENERATE_FLASHER),y)
>> +LOADADDR = 0x80108000 # FIXME: this is Tegra124 only
>> +BSS_SIZE = $(shell $(TARGET_SIZE) -A $(UBOOT_BUILDDIR)/u-boot | grep ".bss\b" | tr -s ' ' | cut -d ' ' -f 2)
>> +UBOOT_NODTB_TEGRA_SIZE = $(shell stat -c %s $(UBOOT_BUILDDIR)/u-boot-nodtb-tegra.bin)
>> +UBOOT_DTB_SIZE = $(shell stat -c %s $(UBOOT_BUILDDIR)/u-boot.dtb)
>> +IMG_SIZE = $(shell stat -c %s $(@D)/$(CONFIGS_BUILD)/$(IMG_NAME))
>> +IMG_SIZE_SECTORS = $(shell echo $$(( $(IMG_SIZE) / 512 )) )
>> +IMG_SIZE_SECTORS_HEX = $(shell printf '0x%x' $(IMG_SIZE_SECTORS) )
>> +UBOOT_PLUS_DTB_SIZE = $(shell echo $$(( $(UBOOT_NODTB_TEGRA_SIZE) + $(UBOOT_DTB_SIZE) )) )
>> +# Keep BSS area clear, acquire an extra 4kb for incereased dtb, align to 4k
>> +# boundary
>> +PADDED_SIZE = $(shell echo $$(( $(UBOOT_PLUS_DTB_SIZE) + $(BSS_SIZE) + (2 * 4 * 1024) - 1 & ~((4 * 1024) - 1) )) )
>> +IMAGEADDR = $(shell echo $$(( $(LOADADDR) + $(PADDED_SIZE) )) )
>> +IMAGEADDR_HEX = $(shell printf '0x%x' $(IMAGEADDR) )
>
> This is really the messy part. That's clearly way too much stuff
> encoded into a Buildroot package.

Well, yes, it's the messy part...
Just for the record, this is what it does:
- Take compiled u-boot
- Add a customized dtb providing environment for the actual flashing of 
cbootimage processed u-boot into mmc
- Append the cbootimage processed u-boot to the binary, so it's self 
contained

>> +ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_CREATE_GPT),y)
>> +PWD = $(shell pwd)
>
> Debugging stuff?

Yes, sorry, leftover.

>> +GPT_SPEC = $(shell cat $(GPT_FILE))
>
> If it's just a string, why is it the path to a file rather than having
> direcly the value in the Config.in option?

Yann asked me to move it to a file in his review of the initial patch, 
as quoting gets rather ugly if provided as Config.in option.
This was the previous default:
default 
"uuid_disk=\\$$\\{disk_uuid\\};name=rootfs,size=-,uuid=\\$$\\{rootfs_uuid\\}"

And that's just for a single partition, so it becomes really messy if 
you add some more.

>> +GPT_BOOTCMD = echo >>> Write partition table to MMC; gpt write mmc 0 '$(GPT_SPEC)'; mmc rescan
>> +else
>> +GPT_BOOTCMD =
>
> Not needed, the default for a variable is to be empty.
>
> Also, remember: all variables should be prefixed by the name of the
> package.

Ack.

>> +endif
>> +
>> +ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_DFU),y)
>> +DFU_BOOTCMD = echo >>> Enter DFU mode; setenv dfu_alt_info '$(DFU_MAP)'; dfu 0 mmc 0;
>> +else
>> +DFU_BOOTCMD =
>> +endif
>> +
>> +UBOOT_FLASH_ENV = "echo >>> Write image to MMC; mmc dev 0 1; mmc write ${IMAGEADDR_HEX} 0 ${IMG_SIZE_SECTORS_HEX}; echo >>> Configure environment; env default -f -a; saveenv; $(GPT_BOOTCMD) $(EXTRA_BOOTCMD); $(DFU_BOOTCMD) echo >>> Resetting system; reset"
>
> All this U-Boot command construction seem really project-specific, and
> I don't see why we should hardcode all that stuff in a Buildroot
> package. Buildroot should give the tools to generate such a U-Boot
> image, but should not hardcode so much logic IMO.

The hardcoded part of this is not really board specific. All it does, is 
writing the bootloader to the emmc. While the tegra supports booting 
from SPI as well, I've never seen a board doing it. So writing to the 
emmc is what's needed on virtually any tegra based board.

In the end this uboot environment is just the tooling to generate an 
automated flash writer, as the core tegra flashing tools do only support 
loading images for execution, but not writing anything directly to memory.

Board specific are only the options to write a partition table and 
automatically expose it via dfu for flashing root filesystems. I added 
those as they appear to be rather useful to generate customized, 
automated flashing solution for the tegra.

Thanks,
Julian

>> +DTB_FILE = $(@D)/u-boot-flasher.dtb
>> +
>> +# Add the actual assembly hook
>> +CBOOTIMAGE_CONFIGS_POST_INSTALL_IMAGES_HOOKS += CBOOTIMAGE_CONFIGS_BUILD_FLASHER_CMD
>> +endif
>
> Thomas
>
Thomas Petazzoni April 20, 2016, 7:11 a.m. UTC | #4
Hello,

On Wed, 20 Apr 2016 08:18:00 +0200, Julian Scheel wrote:

> The problem with tegra-uboot-flasher-scripts is, that it does a lot more 
> than what this package does. In fact it includes build logic for all the 
> required tools (like dtc, tegrarcm, cbootimage, u-boot), which wouldn't 
> work out well in the context of buildroot. Using externally provided 
> builds isn't easily possible without major abuse of the scripts, which I 
> don't think would be merged upstream anyway.

Is this something that you can discuss with Stephen Warren? I believe
he is generally quite open to suggestions, so maybe he'll be inclined
to changing a bit the tegra-uboot-flasher-scripts to make it usable by
build systems?

> > Or maybe this thing should do less stuff, and simply provide the
> > necessary tools for a post-image script to do the job.
> 
> If in the end it's decided that this is too complicated for the package 
> makefile I'd rather create a dedicated package that handles this. So, 
> that the magic would be in done in some independently maintained script 
> and we only need a buildroot package that heads the options to that package.
> It just felt a bit like overkill for what it does in the end.

Alternatively, you could have a shell script the package directory
itself.

> > Why do you use the Avionic Design fork and not the original NVIDIA
> > repository, which you reference as the upstream in your Config.in help
> > text?
> 
> Just because our configurations are not yet merged to the upstream 
> repository. I really need to push on that, as this package really shall 
> use upstream. Although it might be wise to have the usual 
> custom-repository option like it's done for kernel and u-boot. It's not 
> that unlikely that people would have configs in custom repositories 
> which are not merged upstream.

We generally try to not add such options for too many packages, but
that remain to be discussed specifically for this package because it
indeed contains board-specific stuff.


> >> +GPT_SPEC = $(shell cat $(GPT_FILE))
> >
> > If it's just a string, why is it the path to a file rather than having
> > direcly the value in the Config.in option?
> 
> Yann asked me to move it to a file in his review of the initial patch, 
> as quoting gets rather ugly if provided as Config.in option.
> This was the previous default:
> default 
> "uuid_disk=\\$$\\{disk_uuid\\};name=rootfs,size=-,uuid=\\$$\\{rootfs_uuid\\}"
> 
> And that's just for a single partition, so it becomes really messy if 
> you add some more.

OK, makes sense indeed. Please keep the file then. Sorry for not
looking at the previous review.

> > All this U-Boot command construction seem really project-specific, and
> > I don't see why we should hardcode all that stuff in a Buildroot
> > package. Buildroot should give the tools to generate such a U-Boot
> > image, but should not hardcode so much logic IMO.
> 
> The hardcoded part of this is not really board specific. All it does, is 
> writing the bootloader to the emmc. While the tegra supports booting 
> from SPI as well, I've never seen a board doing it. So writing to the 
> emmc is what's needed on virtually any tegra based board.
> 
> In the end this uboot environment is just the tooling to generate an 
> automated flash writer, as the core tegra flashing tools do only support 
> loading images for execution, but not writing anything directly to memory.
> 
> Board specific are only the options to write a partition table and 
> automatically expose it via dfu for flashing root filesystems. I added 
> those as they appear to be rather useful to generate customized, 
> automated flashing solution for the tegra.

Hum, ok. I guess a couple of comments above this logic would be quite
useful.

Thomas
diff mbox

Patch

diff --git a/package/Config.in b/package/Config.in
index bdc3063..9977190 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -150,6 +150,7 @@  endmenu
 
 menu "Filesystem and flash utilities"
 	source "package/btrfs-progs/Config.in"
+	source "package/cbootimage-configs/Config.in"
 	source "package/cifs-utils/Config.in"
 	source "package/cpio/Config.in"
 	source "package/cramfs/Config.in"
diff --git a/package/cbootimage-configs/Config.in b/package/cbootimage-configs/Config.in
new file mode 100644
index 0000000..6268740
--- /dev/null
+++ b/package/cbootimage-configs/Config.in
@@ -0,0 +1,122 @@ 
+menuconfig BR2_PACKAGE_CBOOTIMAGE_CONFIGS
+	bool "cbootimage-configs"
+	select BR2_PACKAGE_HOST_CBOOTIMAGE
+	depends on BR2_TARGET_UBOOT
+	help
+	  The cbootimage-configs project contains cbootimage configuration
+	  files for many Tegra boards, both those designed by NVIDIA, and
+	  various third-parties.
+
+	  Needs BR2_TARGET_UBOOT_FORMAT_CUSTOM_NAME to be u-boot-dtb-tegra.bin
+	  or {u-boot,u-boot-dtb-tegra.bin,u-boot-nodtb-tegra.bin,u-boot.dtb}"
+
+	  https://github.com/NVIDIA/cbootimage-configs
+
+
+if BR2_PACKAGE_CBOOTIMAGE_CONFIGS
+
+config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD
+	string "cbootimage board to build"
+	help
+	  The full path of a configuration inside cbootimage-configs that is
+	  to be built for the board you want to run. File directory structure
+	  usually is <soc>/<vendor>/<board>
+	  Example for Jetson TK1:
+	  tegra124/nvidia/jetson-tk1
+
+config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BCT
+	string "cbootimage bct filename"
+	help
+	  The name of the bct generated by cbootimage for the selected
+	  cbootimage configuration. If unsure look it up from the Makefile in
+	  the specified board directory.
+
+config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG
+	string "cbootimage img filename"
+	help
+	  The name of the image generated by cbootimage for the selected
+	  cbootimage configuration. If unsure look it up from the Makefile in
+	  the specified board directory.
+
+config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_SIGNED
+	bool "sign generated image for secureboot"
+	help
+	  Sign the image using a provided ssl key. This makes it possible to
+	  run the image in secureboot environments with the matching key being
+	  burned to the tegra fuses.
+	  Be aware that the generated image will have the extension simg
+	  instead of img if signing is enabled.
+
+	  Compatible key files can be generated with openssl:
+	  openssl genrsa -out rsa_priv.pem 2048
+
+config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_KEY
+	string "pkc file name"
+	depends on BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_SIGNED
+	help
+	  Key file to use for signing the loader image.
+
+config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_GENERATE_FLASHER
+	bool "generate self-contained u-boot flasher"
+	help
+	  Generate an image consisting of a u-boot to flash the actual target
+	  u-boot. For this a u-boot with a customized environment is generated
+	  that contains a bootcmd to copy a target image into mmc. The actual
+	  target u-boot, which is specified in BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG is
+	  appended to that image so that one self-contained image is generated
+	  that can be flashed with tegrarcm.
+
+	  Needs BR2_TARGET_UBOOT_FORMAT_CUSTOM_NAME to be
+	  {u-boot,u-boot-dtb-tegra.bin,u-boot-nodtb-tegra.bin,u-boot.dtb}
+
+if BR2_PACKAGE_CBOOTIMAGE_CONFIGS_GENERATE_FLASHER
+
+config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_CREATE_GPT
+	bool "write partition table to mmc 0"
+	help
+	  Add a flashing step which is writing a custom gpt partition table
+	  onto the target devices first mmc storage
+
+config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_GPT_FILE
+	string "partition table file"
+	depends on BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_CREATE_GPT
+	default "package/cbootimage-configs/gpt-table"
+	help
+	  Provide a file that contains the paritition table to write to mmc.
+	  Use format as specified in u-boot's README.gpt documentation file.
+
+	  Make sure to escape variable properly, as u-boot shall not evaluate
+	  the variables when setting the environment variable, but only when
+	  running the gpt command.
+
+	  Example file content:
+	  uuid_disk=\$\{disk_uuid\};name=rootfs,size=2000MiB,uuid=\$\{rootfs_uuid\};name=data,size=-,uuid=\$\{data_uuid\}
+
+config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_DFU
+	bool "start dfu mode before reset"
+	help
+	  Put the device in DFU mode on usb 0 and mmc 0, so that filesystem
+	  images can be flashed to the device instantly.
+
+config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_DFU_MAP
+	string "dfu map"
+	depends on BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_DFU
+	default "rootfs part 0 1"
+	help
+	  Mapping scheme for partitions to DFU unit names according to u-boot
+	  documentation README.dfutftp variable dfu_alt_info.
+
+	  Example:
+	  rootfs part 0 1;data part 0 2
+
+config BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_EXTRA_BOOTCMD
+	string "u-boot flasher bootcmd extras"
+	help
+	  Specify extra bootcmds you want the u-boot flasher instance to run
+	  before resetting the board. For example this could be writing a
+	  partition table and enter DFU flashing mode to write filesystem
+	  images.
+
+endif
+
+endif
diff --git a/package/cbootimage-configs/cbootimage-configs.hash b/package/cbootimage-configs/cbootimage-configs.hash
new file mode 100644
index 0000000..a4fd518
--- /dev/null
+++ b/package/cbootimage-configs/cbootimage-configs.hash
@@ -0,0 +1,2 @@ 
+# Locally computed
+sha256  f9eaa2e88fa24ea348d6358f18e4bab58ac21c586b5b6cb5300e21a6e1d9de83  cbootimage-configs-9d45319.tar.gz
diff --git a/package/cbootimage-configs/cbootimage-configs.mk b/package/cbootimage-configs/cbootimage-configs.mk
new file mode 100644
index 0000000..69d6d29
--- /dev/null
+++ b/package/cbootimage-configs/cbootimage-configs.mk
@@ -0,0 +1,128 @@ 
+################################################################################
+#
+# cbootimage-configs
+#
+################################################################################
+
+CBOOTIMAGE_CONFIGS_VERSION = 9d45319
+CBOOTIMAGE_CONFIGS_SITE = $(call github,avionic-design,cbootimage-configs,$(CBOOTIMAGE_CONFIGS_VERSION))
+CBOOTIMAGE_CONFIGS_DEPENDENCIES = host-cbootimage uboot
+CBOOTIMAGE_CONFIGS_INSTALL_TARGET = NO
+CBOOTIMAGE_CONFIGS_INSTALL_IMAGES = YES
+
+CONFIGS_BUILD = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD))
+BCT_NAME = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BCT))
+IMG_NAME = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG))
+EXTRA_BOOTCMD = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_EXTRA_BOOTCMD))
+GPT_FILE = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_GPT_FILE))
+DFU_MAP = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_DFU_MAP))
+KEY_FILE = $(call qstrip,$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_KEY))
+
+ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS),y)
+ifeq ($(CONFIGS_BUILD),)
+	$(warning BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD=$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD))
+	$(warning CONFIGS_BUILD=$(CONFIGS_BUILD))
+	$(error No cbootimage-configuration specififed to build. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD setting)
+endif
+ifeq ($(BCT_NAME),)
+	$(error No bct filename specified. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BCT setting)
+endif
+ifeq ($(IMG_NAME),)
+	$(error No img filename specified. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG setting)
+endif
+ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_CREATE_GPT),y)
+ifeq ($(GPT_FILE),)
+	$(error No gpt partitions specified. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_GPT_FILE setting)
+endif
+endif
+ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_DFU),y)
+ifeq ($(DFU_MAP),)
+	$(error No dfu map specified. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_DFU_MAP setting)
+endif
+endif
+ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_SIGNED),y)
+ifeq ($(KEY_FILE),)
+	$(error No key file specified. Check your BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG_KEY setting)
+endif
+KEY_OPTS = skb=$(KEY_FILE)
+else
+KEY_OPTS =
+endif
+
+TARGET_SIZE = $(TARGET_CROSS)size
+endif
+
+define CBOOTIMAGE_CONFIGS_BUILD_CMDS
+	cp $(UBOOT_BUILDDIR)/u-boot-dtb-tegra.bin $(@D)/$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD)/u-boot.bin
+	(cd $(@D)/$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD) && $(HOST_MAKE_ENV) $(HOST_CONFIGURE_OPTS) $(MAKE) $(KEY_OPTS))
+endef
+
+# Generate a self-contained u-boot image, flashing u-boot to mmc
+define CBOOTIMAGE_CONFIGS_BUILD_FLASHER_CMD
+	# u-boot bss size: ${BSS_SIZE}
+	# u-boot-nodtb-tegra.bin size: ${UBOOT_NODTB_TEGRA_SIZE}
+	# u-boot.dtb size: ${UBOOT_DTB_SIZE}
+	# ${IMG_NAME} size: ${IMG_SIZE}
+	# u-boot plus dtb size: ${UBOOT_PLUS_DTB_SIZE}
+	# padded size: ${PADDED_SIZE}
+	# bootcmd: $(UBOOT_FLASH_ENV)
+	# gpt-file: $(PWD)/$(GPT_FILE)
+
+	cp $(UBOOT_BUILDDIR)/u-boot.dtb ${DTB_FILE}
+	# Patch environment into u-boot dtb
+	$(HOST_MAKE_ENV) fdtput -p -t x ${DTB_FILE} '/config' 'bootdelay' '0xfffffffe'
+	$(HOST_MAKE_ENV) fdtput -p -t s ${DTB_FILE} '/config' 'bootcmd' $(UBOOT_FLASH_ENV)
+
+	# Assemble u-boot-flasher.bin
+	cp $(UBOOT_BUILDDIR)/u-boot-nodtb-tegra.bin $(BINARIES_DIR)/u-boot-flasher.bin
+	cat ${DTB_FILE} >> $(BINARIES_DIR)/u-boot-flasher.bin
+	truncate -s $(PADDED_SIZE) $(BINARIES_DIR)/u-boot-flasher.bin
+	cat $(@D)/$(CONFIGS_BUILD)/$(IMG_NAME) >> $(BINARIES_DIR)/u-boot-flasher.bin
+endef
+
+# Computations for assembling u-boot with custom dtb mostly stolen from
+# https://github.com/NVIDIA/tegra-uboot-flasher-scripts
+# All credits go to Stephen Warren
+ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_GENERATE_FLASHER),y)
+LOADADDR = 0x80108000 # FIXME: this is Tegra124 only
+BSS_SIZE = $(shell $(TARGET_SIZE) -A $(UBOOT_BUILDDIR)/u-boot | grep ".bss\b" | tr -s ' ' | cut -d ' ' -f 2)
+UBOOT_NODTB_TEGRA_SIZE = $(shell stat -c %s $(UBOOT_BUILDDIR)/u-boot-nodtb-tegra.bin)
+UBOOT_DTB_SIZE = $(shell stat -c %s $(UBOOT_BUILDDIR)/u-boot.dtb)
+IMG_SIZE = $(shell stat -c %s $(@D)/$(CONFIGS_BUILD)/$(IMG_NAME))
+IMG_SIZE_SECTORS = $(shell echo $$(( $(IMG_SIZE) / 512 )) )
+IMG_SIZE_SECTORS_HEX = $(shell printf '0x%x' $(IMG_SIZE_SECTORS) )
+UBOOT_PLUS_DTB_SIZE = $(shell echo $$(( $(UBOOT_NODTB_TEGRA_SIZE) + $(UBOOT_DTB_SIZE) )) )
+# Keep BSS area clear, acquire an extra 4kb for incereased dtb, align to 4k
+# boundary
+PADDED_SIZE = $(shell echo $$(( $(UBOOT_PLUS_DTB_SIZE) + $(BSS_SIZE) + (2 * 4 * 1024) - 1 & ~((4 * 1024) - 1) )) )
+IMAGEADDR = $(shell echo $$(( $(LOADADDR) + $(PADDED_SIZE) )) )
+IMAGEADDR_HEX = $(shell printf '0x%x' $(IMAGEADDR) )
+
+ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_CREATE_GPT),y)
+PWD = $(shell pwd)
+GPT_SPEC = $(shell cat $(GPT_FILE))
+GPT_BOOTCMD = echo >>> Write partition table to MMC; gpt write mmc 0 '$(GPT_SPEC)'; mmc rescan
+else
+GPT_BOOTCMD =
+endif
+
+ifeq ($(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_FLASHER_DFU),y)
+DFU_BOOTCMD = echo >>> Enter DFU mode; setenv dfu_alt_info '$(DFU_MAP)'; dfu 0 mmc 0;
+else
+DFU_BOOTCMD =
+endif
+
+UBOOT_FLASH_ENV = "echo >>> Write image to MMC; mmc dev 0 1; mmc write ${IMAGEADDR_HEX} 0 ${IMG_SIZE_SECTORS_HEX}; echo >>> Configure environment; env default -f -a; saveenv; $(GPT_BOOTCMD) $(EXTRA_BOOTCMD); $(DFU_BOOTCMD) echo >>> Resetting system; reset"
+
+DTB_FILE = $(@D)/u-boot-flasher.dtb
+
+# Add the actual assembly hook
+CBOOTIMAGE_CONFIGS_POST_INSTALL_IMAGES_HOOKS += CBOOTIMAGE_CONFIGS_BUILD_FLASHER_CMD
+endif
+
+define CBOOTIMAGE_CONFIGS_INSTALL_IMAGES_CMDS
+	cp -dpf $(@D)/$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD)/$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BCT) $(BINARIES_DIR)
+	cp -dpf $(@D)/$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_BUILD)/$(BR2_PACKAGE_CBOOTIMAGE_CONFIGS_IMG) $(BINARIES_DIR)
+endef
+
+$(eval $(generic-package))
diff --git a/package/cbootimage-configs/gpt-table b/package/cbootimage-configs/gpt-table
new file mode 100644
index 0000000..9a2dc18
--- /dev/null
+++ b/package/cbootimage-configs/gpt-table
@@ -0,0 +1 @@ 
+uuid_disk=\$\{disk_uuid\};name=rootfs,size=-,uuid=\$\{rootfs_uuid\}