Re: [RFC][PATCH v2 1/4] u-boot: basic support of dtb append for verified boot

[Yannick GICQUEL <yannick.gicquel@iot.bzh>]

[-- Attachment #1: Type: text/plain, Size: 5606 bytes --]



Le 21/04/2016 16:06, Andreas Oberritter a écrit :
> On 20.04.2016 15:50, Yannick Gicquel wrote:
>> This introduces a new uboot-sign.class to support U-Boot verified boot.
>>
>> This part delivers the new class file, with related environment variables, and
>> a basic prepend to do_install task which performs the concatenation of the
>> u-boot-nodtb.bin and the device tree blob. The 'cat' command used
>> overrides the u-boot.bin in both DEPLOYDIR & build dir to propagate the
>> changes in later tasks (do_install, do_package, etc.)
>>
>> Signed-off-by: Yannick Gicquel <yannick.gicquel@iot.bzh>
>> ---
>>   meta/classes/uboot-sign.bbclass    | 59 ++++++++++++++++++++++++++++++++++++++
>>   meta/recipes-bsp/u-boot/u-boot.inc |  2 +-
>>   2 files changed, 60 insertions(+), 1 deletion(-)
>>   create mode 100644 meta/classes/uboot-sign.bbclass
>>
>> diff --git a/meta/classes/uboot-sign.bbclass b/meta/classes/uboot-sign.bbclass
>> new file mode 100644
>> index 0000000..63a5181
>> --- /dev/null
>> +++ b/meta/classes/uboot-sign.bbclass
>> @@ -0,0 +1,59 @@
>> +# This file is part of U-Boot verified boot support and is intended to be
>> +# inherited from u-boot recipe and from kernel-fitimage.bbclass.
>> +#
>> +# The signature procedure requires the user to generate an RSA key and
>> +# certificate in a directory and to define the following variable:
>> +#
>> +#   UBOOT_SIGN_KEYDIR = "/keys/directory"
>> +#   UBOOT_SIGN_KEYNAME = "dev" # keys name in keydir (eg. "dev.crt", "dev.key")
>> +#   UBOOT_MKIMAGE_DTCOPTS = "-I dts -O dtb -p 2000"
>> +#   UBOOT_SIGN_ENABLE = "1"
>> +#
>> +# As verified boot depends on fitImage generation, following is also required:
>> +#
>> +#   KERNEL_CLASSES ?= " kernel-fitimage "
>> +#   KERNEL_IMAGETYPE ?= "fitImage"
>> +#
>> +# The signature support is limited to the use of CONFIG_OF_SEPARATE in U-Boot.
>> +#
>> +# The tasks sequence is as below, using DEPLOY_IMAGE_DIR as common place to
>> +# treat the device tree blob:
>> +#
>> +# u-boot:do_deploy -> virtual/kernel:do_assemble_fitimage -> u-boot:do_install
>> +#
>> +# For more details on signature process, please refer to U-boot documentation.
>> +
>> +# Signature activation.
>> +UBOOT_SIGN_ENABLE ?= "0"
>> +
>> +# Default value for deployment filenames.
>> +UBOOT_DTB_IMAGE ?= "u-boot-${MACHINE}-${PV}-${PR}.dtb"
>> +UBOOT_DTB_BINARY ?= "u-boot.dtb"
>> +UBOOT_DTB_SYMLINK ?= "u-boot-${MACHINE}.dtb"
>> +UBOOT_NODTB_IMAGE ?= "u-boot-nodtb-${MACHINE}-${PV}-${PR}.${UBOOT_SUFFIX}"
>> +UBOOT_NODTB_BINARY ?= "u-boot-nodtb.${UBOOT_SUFFIX}"
>> +UBOOT_NODTB_SYMLINK ?= "u-boot-nodtb-${MACHINE}.${UBOOT_SUFFIX}"
>> +
> I think you should split the file at this point and either put the lines
> above into uboot-sign-base.bbclass and include it from here or put the
> lines below into u-boot.inc directly. This way you don't add unnecessary
> code for u-boot to kernel recipes, and lose the dependency on PN =
> "u-boot" at the same time.

Thanks for the proposal, that's interesting.
I agree that splitting the file should remove the dependencies on 
'_pn-u-boot' and it also allow to keep usage of existing 'deploy' and 
'install' tasks.
But even in that case, we will still have a last reference to 'u-boot' 
recipe name, hardcoded in kernel-fitimage.bbclass:

d.appendVarFlag('do_assemble_fitimage', 'depends', ' u-boot:do_deploy')
                                                      ^^^^^^
For now, I don't know a way to avoid using PREFERRED_PROVIDER_u-boot 
variable at some point.

Also, I worked on an update (keeping one class file) and in that case it 
now introduces specifics tasks for u-boot. (cf. patch attached).
I would prefer to keep the whole thing in one single class file to avoid 
fragmentation, except if introducing new tasks it is not advised for any 
reason?
Your comments are welcome.

Anyway, I would be able to send a V3 series (maybe without RFC) early 
next week.
Thank you all for Acked, Reviewed and comments!

Best regards,
Yannick

>
> Regards,
> Andreas
>
>> +#
>> +# Following is relevant only for u-boot recipes:
>> +#
>> +
>> +do_install_prepend_pn-u-boot () {
>> +	# Concatenate U-Boot w/o DTB & DTB with public key
>> +	# (cf. kernel-fitimage.bbclass for more details)
>> +	cd ${DEPLOYDIR}
>> +	if [ "x${UBOOT_SIGN_ENABLE}" = "x1" ]; then
>> +		if [ -e "${UBOOT_NODTB_IMAGE}" -a -e "${UBOOT_DTB_IMAGE}" ]; then
>> +			cat ${UBOOT_NODTB_IMAGE} ${UBOOT_DTB_IMAGE} > ${UBOOT_IMAGE}
>> +			cat ${UBOOT_NODTB_IMAGE} ${UBOOT_DTB_IMAGE} > ${B}/${UBOOT_BINARY}
>> +		else
>> +			bbwarn "Failure while adding public key to u-boot binary. Verified boot won't be available."
>> +		fi
>> +	fi
>> +}
>> +
>> +python () {
>> +	if d.getVar('UBOOT_SIGN_ENABLE', True) == '1' and d.getVar('PN', True) == 'u-boot':
>> +		kernel_pn = d.getVar('PREFERRED_PROVIDER_virtual/kernel', True)
>> +		d.appendVarFlag('do_install', 'depends', ' %s:do_assemble_fitimage' % kernel_pn)
>> +}
>> diff --git a/meta/recipes-bsp/u-boot/u-boot.inc b/meta/recipes-bsp/u-boot/u-boot.inc
>> index 3ba866d..037f35c 100644
>> --- a/meta/recipes-bsp/u-boot/u-boot.inc
>> +++ b/meta/recipes-bsp/u-boot/u-boot.inc
>> @@ -12,7 +12,7 @@ S = "${WORKDIR}/git"
>>   
>>   PACKAGE_ARCH = "${MACHINE_ARCH}"
>>   
>> -inherit uboot-config deploy
>> +inherit uboot-config uboot-sign deploy
>>   
>>   EXTRA_OEMAKE = 'CROSS_COMPILE=${TARGET_PREFIX} CC="${TARGET_PREFIX}gcc ${TOOLCHAIN_OPTIONS}" V=1'
>>   EXTRA_OEMAKE += 'HOSTCC="${BUILD_CC} ${BUILD_CFLAGS} ${BUILD_LDFLAGS}"'
>>


[-- Attachment #2: uboot-pn.patch --]
[-- Type: text/x-patch, Size: 3227 bytes --]

diff --git a/meta/classes/kernel-fitimage.bbclass b/meta/classes/kernel-fitimage.bbclass
index dfd3306..809bd4d 100644
--- a/meta/classes/kernel-fitimage.bbclass
+++ b/meta/classes/kernel-fitimage.bbclass
@@ -20,7 +20,8 @@ python __anonymous () {
         # U-boot dtb. We ensure the U-Boot dtb is deployed before assembling
         # the fitImage:
         if d.getVar('UBOOT_SIGN_ENABLE', True):
-            d.appendVarFlag('do_assemble_fitimage', 'depends', ' u-boot:do_deploy')
+            uboot_pn = d.getVar('PREFERRED_PROVIDER_u-boot', True) or 'u-boot'
+            d.appendVarFlag('do_assemble_fitimage', 'depends', ' %s:do_deploy' % uboot_pn)
 }
 
 # Options for the device tree compiler passed to mkimage '-D' feature:
diff --git a/meta/classes/uboot-sign.bbclass b/meta/classes/uboot-sign.bbclass
index adb24d4..f3d0c73 100644
--- a/meta/classes/uboot-sign.bbclass
+++ b/meta/classes/uboot-sign.bbclass
@@ -16,10 +16,14 @@
 #
 # The signature support is limited to the use of CONFIG_OF_SEPARATE in U-Boot.
 #
-# The tasks sequence is as below, using DEPLOY_IMAGE_DIR as common place to
+# The tasks sequence is set as below, using DEPLOY_IMAGE_DIR as common place to
 # treat the device tree blob:
 #
-# u-boot:do_deploy -> virtual/kernel:do_assemble_fitimage -> u-boot:do_install
+#   u-boot:do_deploy_dtb
+#   u-boot:do_deploy
+#   virtual/kernel:do_assemble_fitimage
+#   u-boot:do_concat_dtb
+#   u-boot:do_install
 #
 # For more details on signature process, please refer to U-boot documentation.
 
@@ -38,8 +42,10 @@ UBOOT_NODTB_SYMLINK ?= "u-boot-nodtb-${MACHINE}.${UBOOT_SUFFIX}"
 # Following is relevant only for u-boot recipes:
 #
 
-do_deploy_append_pn-u-boot () {
-	# OF_SEPARATE generated files deployment
+do_deploy_dtb () {
+	mkdir -p ${DEPLOYDIR}
+	cd ${DEPLOYDIR}
+
 	if [ -f ${B}/${UBOOT_DTB_BINARY} ]; then
 		install ${B}/${UBOOT_DTB_BINARY} ${DEPLOYDIR}/${UBOOT_DTB_IMAGE}
 		rm -f ${UBOOT_DTB_BINARY} ${UBOOT_DTB_SYMLINK}
@@ -54,7 +60,7 @@ do_deploy_append_pn-u-boot () {
 	fi
 }
 
-do_install_prepend_pn-u-boot () {
+do_concat_dtb () {
 	# Concatenate U-Boot w/o DTB & DTB with public key
 	# (cf. kernel-fitimage.bbclass for more details)
 	cd ${DEPLOYDIR}
@@ -69,7 +75,16 @@ do_install_prepend_pn-u-boot () {
 }
 
 python () {
-	if d.getVar('UBOOT_SIGN_ENABLE', True) == '1' and d.getVar('PN', True) == 'u-boot':
+	uboot_pn = d.getVar('PREFERRED_PROVIDER_u-boot', True) or 'u-boot'
+	if d.getVar('UBOOT_SIGN_ENABLE', True) == '1' and d.getVar('PN', True) == uboot_pn:
 		kernel_pn = d.getVar('PREFERRED_PROVIDER_virtual/kernel', True)
-		d.appendVarFlag('do_install', 'depends', ' %s:do_assemble_fitimage' % kernel_pn)
+
+		# u-boot.dtb and u-boot-nodtb.bin are deployed _before_ do_deploy
+		# Thus, do_deploy_setscene will also populate them in DEPLOY_IMAGE_DIR
+		bb.build.addtask('do_deploy_dtb', 'do_deploy', 'do_compile', d)
+
+		# do_concat_dtb is scheduled _before_ do_install as it overwrite the
+		# u-boot.bin in both DEPLOYDIR and DEPLOY_IMAGE_DIR.
+		bb.build.addtask('do_concat_dtb', 'do_install', None, d)
+		d.appendVarFlag('do_concat_dtb', 'depends', ' %s:do_assemble_fitimage' % kernel_pn)
 }