[U-Boot] [PATCH] rpi: Do not use dtb loaded by firmware

[U-Boot] [PATCH] rpi: Do not use dtb loaded by firmware

[Jun Nie]

Do not use dtb loaded by firmware if fit image signature is enabled.
So that u-boot.dtb can be used. The u-boot.dtb contains the pulibc key
that is to verify Linux kernel FIT image blob.

The u-boot.dtb can be loaded by Arm Trusted Firmware(ATF) together
with u-boot.bin to make sure the key is protected by ATF.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 board/raspberrypi/rpi/rpi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index 649127c..f7041e3 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -283,6 +283,7 @@ int dram_init(void)
 	return 0;
 }
 
+#ifndef CONFIG_FIT_SIGNATURE
 static void set_fdtfile(void)
 {
 	const char *fdtfile;
@@ -318,6 +319,7 @@ unsigned long board_get_usable_ram_top(unsigned long total_size)
 		return gd->ram_top;
 	return fw_dtb_pointer & ~0xffff;
 }
+#endif
 
 static void set_usbethaddr(void)
 {
@@ -390,8 +392,10 @@ static void set_serial_number(void)
 
 int misc_init_r(void)
 {
+#ifndef CONFIG_FIT_SIGNATURE
 	set_fdt_addr();
 	set_fdtfile();
+#endif
 	set_usbethaddr();
 #ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
 	set_board_info();
@@ -467,6 +471,7 @@ int board_init(void)
 	return bcm2835_power_on_module(BCM2835_MBOX_POWER_DEVID_USB_HCD);
 }
 
+#ifndef CONFIG_FIT_SIGNATURE
 /*
  * If the firmware passed a device tree use it for U-Boot.
  */
@@ -476,6 +481,7 @@ void *board_fdt_blob_setup(void)
 		return NULL;
 	return (void *)fw_dtb_pointer;
 }
+#endif
 
 int ft_board_setup(void *blob, bd_t *bd)
 {
-- 
2.7.4

[U-Boot] [PATCH] rpi: Do not use dtb loaded by firmware

[Alexander Graf]

On 11/08/2018 09:36 AM, Jun Nie wrote:
> Do not use dtb loaded by firmware if fit image signature is enabled.
> So that u-boot.dtb can be used. The u-boot.dtb contains the pulibc key
> that is to verify Linux kernel FIT image blob.
>
> The u-boot.dtb can be loaded by Arm Trusted Firmware(ATF) together
> with u-boot.bin to make sure the key is protected by ATF.

I don't think I fully understand what you're trying to do here. If ATF 
loads U-Boot as well as the DT, ATF can pass the DT to U-Boot which then 
ends up as $fdtaddr. If you enable CONFIG_OF_BOARD, it even becomes the 
input DT for U-Boot.

Hiding the fdtfile variable name doesn't sound like it solves anything 
to me?


Alex

>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>   board/raspberrypi/rpi/rpi.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> index 649127c..f7041e3 100644
> --- a/board/raspberrypi/rpi/rpi.c
> +++ b/board/raspberrypi/rpi/rpi.c
> @@ -283,6 +283,7 @@ int dram_init(void)
>   	return 0;
>   }
>   
> +#ifndef CONFIG_FIT_SIGNATURE
>   static void set_fdtfile(void)
>   {
>   	const char *fdtfile;
> @@ -318,6 +319,7 @@ unsigned long board_get_usable_ram_top(unsigned long total_size)
>   		return gd->ram_top;
>   	return fw_dtb_pointer & ~0xffff;
>   }
> +#endif
>   
>   static void set_usbethaddr(void)
>   {
> @@ -390,8 +392,10 @@ static void set_serial_number(void)
>   
>   int misc_init_r(void)
>   {
> +#ifndef CONFIG_FIT_SIGNATURE
>   	set_fdt_addr();
>   	set_fdtfile();
> +#endif
>   	set_usbethaddr();
>   #ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
>   	set_board_info();
> @@ -467,6 +471,7 @@ int board_init(void)
>   	return bcm2835_power_on_module(BCM2835_MBOX_POWER_DEVID_USB_HCD);
>   }
>   
> +#ifndef CONFIG_FIT_SIGNATURE
>   /*
>    * If the firmware passed a device tree use it for U-Boot.
>    */
> @@ -476,6 +481,7 @@ void *board_fdt_blob_setup(void)
>   		return NULL;
>   	return (void *)fw_dtb_pointer;
>   }
> +#endif
>   
>   int ft_board_setup(void *blob, bd_t *bd)
>   {

[U-Boot] [PATCH] rpi: Do not use dtb loaded by firmware

[Jun Nie]

Alexander Graf <agraf@suse.de> 于2018年11月8日周四 下午4:59写道：
>
> On 11/08/2018 09:36 AM, Jun Nie wrote:
> > Do not use dtb loaded by firmware if fit image signature is enabled.
> > So that u-boot.dtb can be used. The u-boot.dtb contains the pulibc key
> > that is to verify Linux kernel FIT image blob.
> >
> > The u-boot.dtb can be loaded by Arm Trusted Firmware(ATF) together
> > with u-boot.bin to make sure the key is protected by ATF.
>
> I don't think I fully understand what you're trying to do here. If ATF
> loads U-Boot as well as the DT, ATF can pass the DT to U-Boot which then
> ends up as $fdtaddr. If you enable CONFIG_OF_BOARD, it even becomes the
> input DT for U-Boot.

Current usage is that pack u-boot.dtb to u-boot-nodtb.bin so that
u-boot can find
device tree in the end if u-boot binary. This saves separate signing
to u-boot.dtb in
ATF. Do you see any benefit to load u-boot.dtb separately and feed
$fdtaddr to u-boot?

Jun

>
> Hiding the fdtfile variable name doesn't sound like it solves anything
> to me?
>
>
> Alex
>
> >
> > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > ---
> >   board/raspberrypi/rpi/rpi.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> > index 649127c..f7041e3 100644
> > --- a/board/raspberrypi/rpi/rpi.c
> > +++ b/board/raspberrypi/rpi/rpi.c
> > @@ -283,6 +283,7 @@ int dram_init(void)
> >       return 0;
> >   }
> >
> > +#ifndef CONFIG_FIT_SIGNATURE
> >   static void set_fdtfile(void)
> >   {
> >       const char *fdtfile;
> > @@ -318,6 +319,7 @@ unsigned long board_get_usable_ram_top(unsigned long total_size)
> >               return gd->ram_top;
> >       return fw_dtb_pointer & ~0xffff;
> >   }
> > +#endif
> >
> >   static void set_usbethaddr(void)
> >   {
> > @@ -390,8 +392,10 @@ static void set_serial_number(void)
> >
> >   int misc_init_r(void)
> >   {
> > +#ifndef CONFIG_FIT_SIGNATURE
> >       set_fdt_addr();
> >       set_fdtfile();
> > +#endif
> >       set_usbethaddr();
> >   #ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
> >       set_board_info();
> > @@ -467,6 +471,7 @@ int board_init(void)
> >       return bcm2835_power_on_module(BCM2835_MBOX_POWER_DEVID_USB_HCD);
> >   }
> >
> > +#ifndef CONFIG_FIT_SIGNATURE
> >   /*
> >    * If the firmware passed a device tree use it for U-Boot.
> >    */
> > @@ -476,6 +481,7 @@ void *board_fdt_blob_setup(void)
> >               return NULL;
> >       return (void *)fw_dtb_pointer;
> >   }
> > +#endif
> >
> >   int ft_board_setup(void *blob, bd_t *bd)
> >   {
>
>

[U-Boot] [PATCH] rpi: Do not use dtb loaded by firmware

[Alexander Graf]

On 11/08/2018 10:05 AM, Jun Nie wrote:
> Alexander Graf <agraf@suse.de> 于2018年11月8日周四 下午4:59写道：
>> On 11/08/2018 09:36 AM, Jun Nie wrote:
>>> Do not use dtb loaded by firmware if fit image signature is enabled.
>>> So that u-boot.dtb can be used. The u-boot.dtb contains the pulibc key
>>> that is to verify Linux kernel FIT image blob.
>>>
>>> The u-boot.dtb can be loaded by Arm Trusted Firmware(ATF) together
>>> with u-boot.bin to make sure the key is protected by ATF.
>> I don't think I fully understand what you're trying to do here. If ATF
>> loads U-Boot as well as the DT, ATF can pass the DT to U-Boot which then
>> ends up as $fdtaddr. If you enable CONFIG_OF_BOARD, it even becomes the
>> input DT for U-Boot.
> Current usage is that pack u-boot.dtb to u-boot-nodtb.bin so that
> u-boot can find
> device tree in the end if u-boot binary. This saves separate signing
> to u-boot.dtb in
> ATF. Do you see any benefit to load u-boot.dtb separately and feed
> $fdtaddr to u-boot?

The main reason that is a useful scenario is that you can do 
modifications in upper layers that propagate. We use it for example to 
allow people to add dt overlays via config.txt, but you could as well 
use it to expose changes from ATF into U-Boot (and Linux from there).


Alex