[RFC,9/9] ti: omap-common: Update to generate secure FIT

From: Daniel Allred <d-allred@ti.com>


Adds commands so that when a secure device is in use and the SPL is
built to load a FIT image (with combined u-boot binary and various
DTBs), these components that get fed into the FIT are all processed to
be signed/encrypted/etc. as per the operations performed by the
secure-binary-image script of the TI SECDEV package.

Signed-off-by: Daniel Allred <d-allred@ti.com>

Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>

---
 arch/arm/cpu/armv7/omap-common/config_secure.mk | 57 ++++++++++++++++++++++++-
 arch/arm/cpu/armv7/omap5/config.mk              |  3 ++
 2 files changed, 58 insertions(+), 2 deletions(-)

-- 
2.6.4

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Hi Simon,
many thanks for your feedback on the series... I know it takes a lot of
effort to digest all that stuff. We'll see how we can tackle the
feedback one by one and send out an updated series.

Regarding this patch, please see below...

On Thu, Jun 16, 2016 at 09:52:52PM -0600, Simon Glass wrote:
> Hi Andreas,

> 

> On 15 June 2016 at 13:26, Andreas Dannenberg <dannenberg@ti.com> wrote:

> > From: Daniel Allred <d-allred@ti.com>

> >

> > Adds commands so that when a secure device is in use and the SPL is

> > built to load a FIT image (with combined u-boot binary and various

> > DTBs), these components that get fed into the FIT are all processed to

> > be signed/encrypted/etc. as per the operations performed by the

> > secure-binary-image script of the TI SECDEV package.

> >

> > Signed-off-by: Daniel Allred <d-allred@ti.com>

> > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>

> > ---

> >  arch/arm/cpu/armv7/omap-common/config_secure.mk | 57 ++++++++++++++++++++++++-

> >  arch/arm/cpu/armv7/omap5/config.mk              |  3 ++

> >  2 files changed, 58 insertions(+), 2 deletions(-)

> 

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

> 

> But please can you add a README for this somewhere?

> 

> Also, can this tool be added to U-Boot instead of being external?


Yes we will definitely enhance the readme in the PATCH version, this
wasn't quite ready yet by the time we submitted the RFC.

Also regarding the external singing/encryption tool this is only
available from TI under NDA after customers engage with TI just like the
high-secure (HS) devices themselves (you can't just buy them on
Digi-Key). Personally I hope that we eventually lower this barrier of
entry (and this has been brought up more than once) but as many things
in the corporate world there is a complex decision process behind it. So
until this strategy changes (if ever) our poor developer's hands are
tied. All we can do for now is trying to allow this external tool to get
hooked into the U-Boot build process in the easiest way possible which
is already done today by way of the $TI_SECURE_DEV_PKG environmental
variable during SPL build.

> >

> > diff --git a/arch/arm/cpu/armv7/omap-common/config_secure.mk b/arch/arm/cpu/armv7/omap-common/config_secure.mk

> > index c7bb101..c4514ad 100644

> > --- a/arch/arm/cpu/armv7/omap-common/config_secure.mk

> > +++ b/arch/arm/cpu/armv7/omap-common/config_secure.mk

> > @@ -12,8 +12,8 @@ cmd_mkomapsecimg = $(TI_SECURE_DEV_PKG)/scripts/create-boot-image.sh \

> >         $(if $(KBUILD_VERBOSE:1=), >/dev/null)

> >  else

> >  cmd_mkomapsecimg = $(TI_SECURE_DEV_PKG)/scripts/create-boot-image.sh \

> > -    $(patsubst u-boot_HS_%,%,$(@F)) $< $@ $(CONFIG_ISW_ENTRY_ADDR) \

> > -    $(if $(KBUILD_VERBOSE:1=), >/dev/null)

> > +       $(patsubst u-boot_HS_%,%,$(@F)) $< $@ $(CONFIG_ISW_ENTRY_ADDR) \

> > +       $(if $(KBUILD_VERBOSE:1=), >/dev/null)

> >  endif

> >  else

> >  cmd_mkomapsecimg = echo "WARNING:" \

> > @@ -25,6 +25,26 @@ cmd_mkomapsecimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \

> >         "variable must be defined for TI secure devices. $@ was NOT created!"

> >  endif

> >

> > +ifdef CONFIG_SPL_LOAD_FIT

> > +quiet_cmd_omapsecureimg = SECURE  $@

> > +ifneq ($(TI_SECURE_DEV_PKG),)

> > +ifneq ($(wildcard $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh),)

> > +cmd_omapsecureimg = $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh \

> > +       $< $@ \

> > +       $(if $(KBUILD_VERBOSE:1=), >/dev/null)

> > +else

> > +cmd_omapsecureimg = echo "WARNING:" \

> > +       "$(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh not found." \

> > +       "$@ was NOT created!"; cp $< $@

> > +endif

> > +else

> > +cmd_omapsecureimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \

> > +       "variable must be defined for TI secure devices." \

> > +       "$@ was NOT created!"; cp $< $@

> > +endif

> > +endif

> > +

> > +

> >  # Standard X-LOADER target (QPSI, NOR flash)

> >  u-boot-spl_HS_X-LOADER: $(obj)/u-boot-spl.bin

> >         $(call if_changed,mkomapsecimg)

> > @@ -64,3 +84,36 @@ u-boot-spl_HS_SPI_X-LOADER: $(obj)/u-boot-spl.bin

> >  # the mkomapsecimg command looks for a u-boot-HS_* prefix

> >  u-boot_HS_XIP_X-LOADER: $(obj)/u-boot.bin

> >         $(call if_changed,mkomapsecimg)

> > +

> > +# For supporting the SPL loading and interpreting

> > +# of FIT images whose components are pre-processed

> > +# before being integrated into the FIT image in order

> > +# to secure them in some way

> > +ifdef CONFIG_SPL_LOAD_FIT

> > +

> > +MKIMAGEFLAGS_u-boot_HS.img = -f auto -A $(ARCH) -T firmware -C none -O u-boot \

> > +       -a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \

> > +       -n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \

> > +       $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))

> > +

> > +OF_LIST_TARGETS = $(patsubst %,arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))

> > +$(OF_LIST_TARGETS): dtbs

> > +

> > +%_HS.dtb: %.dtb

> > +       $(call if_changed,omapsecureimg)

> > +       $(Q)if [ -f $@ ]; then \

> > +               cp -f $@ $<; \

> > +       fi

> > +

> > +u-boot-nodtb_HS.bin: u-boot-nodtb.bin

> > +       $(call if_changed,omapsecureimg)

> > +

> > +u-boot_HS.img: u-boot-nodtb_HS.bin u-boot.img $(patsubst %.dtb,%_HS.dtb,$(OF_LIST_TARGETS))

> > +       $(call if_changed,mkimage)

> > +       $(Q)if [ -f $@ ]; then \

> > +               cp -f $@ u-boot.img; \

> > +       fi

> > +

> > +.NOTPARALLEL: dtbs

> 

> Why is that needed?


Good catch! This issue actually caused me a fair amount of grief when
running into it. During development we found the build process works
with make -j1 but would fail for parallel builds (like make -j8) with
the following error....

----------------------->8---------------------------
$ make mrproper
$ make dra7xx_hs_evm_defconfig
$ make -j8
....
  LD      u-boot
  OBJCOPY u-boot-nodtb.bin
  OBJCOPY u-boot.srec
  SYM     u-boot.sym
  DTC     arch/arm/dts/dra72-evm.dtb
  DTC     arch/arm/dts/dra7-evm.dtb
  DTC     arch/arm/dts/dra72-evm.dtb
  DTC     arch/arm/dts/dra7-evm.dtb
Error: arch/arm/dts/dra7-evm.dts:492.15-16 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [arch/arm/dts/dra7-evm.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
Error: arch/arm/dts/dra72-evm.dts:149.26-150.2 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [arch/arm/dts/dra72-evm.dtb] Error 1
make[1]: *** [arch-dtbs] Error 2
make: *** [dtbs] Error 2
make: *** Waiting for unfinished jobs....
  SHIPPED dts/dt.dtb
----------------------->8---------------------------

However with the .NOTPARALLEL: dtbs line included it works. Also when
doing make a second time it works as well (clearly that's not a
solution:)

On my quest trying to digest/root cause this I concluded that this may
be caused by the DTB files somehow getting compiled twice (see snippet).
This in combination with the fact that we post-process files like
dta7-evm.dtb (signing them) and creating new DTB files with the same
name (like dra7-evm.dtb) replacing the old ones causes issues during the
DTB compile step.

Now I'm not a make guru but I tried to clean up the dependency chain as
good as I can but could not get it to the point where each DTB file is
only build once which most likely would allow us getting rid of
NOPARALLEL.

On a related note we wanted to keep the same names for the DTB files
(rather than turning dra7-evm.dtb into dra7-evm-sec.dtb for example, as
I had it initially) since it's important as they get bundled into the
u-boot.img FIT blob so that they can be later matched by the SPL to a
board. But it appears like that trying to keep the same names despite
the additional signing step seems to complicate things from a make
perspective.

If you or anybody else has any smart suggestion how to address this in
a better way I'd love to hear about it.

Thanks,
Andreas

> > +

> > +endif

> > diff --git a/arch/arm/cpu/armv7/omap5/config.mk b/arch/arm/cpu/armv7/omap5/config.mk

> > index a7e55a5..503f31c 100644

> > --- a/arch/arm/cpu/armv7/omap5/config.mk

> > +++ b/arch/arm/cpu/armv7/omap5/config.mk

> > @@ -15,5 +15,8 @@ else

> >  ALL-y  += MLO

> >  endif

> >  else

> > +ifeq ($(CONFIG_TI_SECURE_DEVICE)$(CONFIG_SECURE_BOOT),yy)

> > +ALL-y   += u-boot_HS.img

> > +endif

> >  ALL-y  += u-boot.img

> >  endif

> > --

> > 2.6.4

> >

> 

> Regards,

> Simon

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Hi Simon,
thanks for the continued feedback. Comments below...

On Mon, Jun 20, 2016 at 04:40:10PM -0600, Simon Glass wrote:
> +Masahiro for the Makefile question

> 

> Hi Andreas,

> 

> On 17 June 2016 at 10:13, Andreas Dannenberg <dannenberg@ti.com> wrote:

> > Hi Simon,

> > many thanks for your feedback on the series... I know it takes a lot of

> > effort to digest all that stuff. We'll see how we can tackle the

> > feedback one by one and send out an updated series.

> >

> > Regarding this patch, please see below...

> >

> > On Thu, Jun 16, 2016 at 09:52:52PM -0600, Simon Glass wrote:

> >> Hi Andreas,

> >>

> >> On 15 June 2016 at 13:26, Andreas Dannenberg <dannenberg@ti.com> wrote:

> >> > From: Daniel Allred <d-allred@ti.com>

> >> >

> >> > Adds commands so that when a secure device is in use and the SPL is

> >> > built to load a FIT image (with combined u-boot binary and various

> >> > DTBs), these components that get fed into the FIT are all processed to

> >> > be signed/encrypted/etc. as per the operations performed by the

> >> > secure-binary-image script of the TI SECDEV package.

> >> >

> >> > Signed-off-by: Daniel Allred <d-allred@ti.com>

> >> > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>

> >> > ---

> >> >  arch/arm/cpu/armv7/omap-common/config_secure.mk | 57 ++++++++++++++++++++++++-

> >> >  arch/arm/cpu/armv7/omap5/config.mk              |  3 ++

> >> >  2 files changed, 58 insertions(+), 2 deletions(-)

> >>

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

> >>

> >> But please can you add a README for this somewhere?

> >>

> >> Also, can this tool be added to U-Boot instead of being external?

> >

> > Yes we will definitely enhance the readme in the PATCH version, this

> > wasn't quite ready yet by the time we submitted the RFC.

> >

> > Also regarding the external singing/encryption tool this is only

> > available from TI under NDA after customers engage with TI just like the

> > high-secure (HS) devices themselves (you can't just buy them on

> > Digi-Key). Personally I hope that we eventually lower this barrier of

> > entry (and this has been brought up more than once) but as many things

> > in the corporate world there is a complex decision process behind it. So

> > until this strategy changes (if ever) our poor developer's hands are

> > tied. All we can do for now is trying to allow this external tool to get

> > hooked into the U-Boot build process in the easiest way possible which

> > is already done today by way of the $TI_SECURE_DEV_PKG environmental

> > variable during SPL build.

> 

> OK. That's odd because it should be the keys that are secure, not the

> tool! If anything, the tool should have as many eyes on it as

> possible.


Yes that's the ideal-world case I was also arguing we should follow...
But as indicated decision processes in the corporate world sometimes are
complex :)

> >> >

> >> > diff --git a/arch/arm/cpu/armv7/omap-common/config_secure.mk b/arch/arm/cpu/armv7/omap-common/config_secure.mk

> >> > index c7bb101..c4514ad 100644

> >> > --- a/arch/arm/cpu/armv7/omap-common/config_secure.mk

> >> > +++ b/arch/arm/cpu/armv7/omap-common/config_secure.mk

> >> > @@ -12,8 +12,8 @@ cmd_mkomapsecimg = $(TI_SECURE_DEV_PKG)/scripts/create-boot-image.sh \

> >> >         $(if $(KBUILD_VERBOSE:1=), >/dev/null)

> >> >  else

> >> >  cmd_mkomapsecimg = $(TI_SECURE_DEV_PKG)/scripts/create-boot-image.sh \

> >> > -    $(patsubst u-boot_HS_%,%,$(@F)) $< $@ $(CONFIG_ISW_ENTRY_ADDR) \

> >> > -    $(if $(KBUILD_VERBOSE:1=), >/dev/null)

> >> > +       $(patsubst u-boot_HS_%,%,$(@F)) $< $@ $(CONFIG_ISW_ENTRY_ADDR) \

> >> > +       $(if $(KBUILD_VERBOSE:1=), >/dev/null)

> >> >  endif

> >> >  else

> >> >  cmd_mkomapsecimg = echo "WARNING:" \

> >> > @@ -25,6 +25,26 @@ cmd_mkomapsecimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \

> >> >         "variable must be defined for TI secure devices. $@ was NOT created!"

> >> >  endif

> >> >

> >> > +ifdef CONFIG_SPL_LOAD_FIT

> >> > +quiet_cmd_omapsecureimg = SECURE  $@

> >> > +ifneq ($(TI_SECURE_DEV_PKG),)

> >> > +ifneq ($(wildcard $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh),)

> >> > +cmd_omapsecureimg = $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh \

> >> > +       $< $@ \

> >> > +       $(if $(KBUILD_VERBOSE:1=), >/dev/null)

> >> > +else

> >> > +cmd_omapsecureimg = echo "WARNING:" \

> >> > +       "$(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh not found." \

> >> > +       "$@ was NOT created!"; cp $< $@

> >> > +endif

> >> > +else

> >> > +cmd_omapsecureimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \

> >> > +       "variable must be defined for TI secure devices." \

> >> > +       "$@ was NOT created!"; cp $< $@

> >> > +endif

> >> > +endif

> >> > +

> >> > +

> >> >  # Standard X-LOADER target (QPSI, NOR flash)

> >> >  u-boot-spl_HS_X-LOADER: $(obj)/u-boot-spl.bin

> >> >         $(call if_changed,mkomapsecimg)

> >> > @@ -64,3 +84,36 @@ u-boot-spl_HS_SPI_X-LOADER: $(obj)/u-boot-spl.bin

> >> >  # the mkomapsecimg command looks for a u-boot-HS_* prefix

> >> >  u-boot_HS_XIP_X-LOADER: $(obj)/u-boot.bin

> >> >         $(call if_changed,mkomapsecimg)

> >> > +

> >> > +# For supporting the SPL loading and interpreting

> >> > +# of FIT images whose components are pre-processed

> >> > +# before being integrated into the FIT image in order

> >> > +# to secure them in some way

> >> > +ifdef CONFIG_SPL_LOAD_FIT

> >> > +

> >> > +MKIMAGEFLAGS_u-boot_HS.img = -f auto -A $(ARCH) -T firmware -C none -O u-boot \

> >> > +       -a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \

> >> > +       -n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \

> >> > +       $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))

> >> > +

> >> > +OF_LIST_TARGETS = $(patsubst %,arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))

> >> > +$(OF_LIST_TARGETS): dtbs

> >> > +

> >> > +%_HS.dtb: %.dtb

> >> > +       $(call if_changed,omapsecureimg)

> >> > +       $(Q)if [ -f $@ ]; then \

> >> > +               cp -f $@ $<; \

> >> > +       fi

> >> > +

> >> > +u-boot-nodtb_HS.bin: u-boot-nodtb.bin

> >> > +       $(call if_changed,omapsecureimg)

> >> > +

> >> > +u-boot_HS.img: u-boot-nodtb_HS.bin u-boot.img $(patsubst %.dtb,%_HS.dtb,$(OF_LIST_TARGETS))

> >> > +       $(call if_changed,mkimage)

> >> > +       $(Q)if [ -f $@ ]; then \

> >> > +               cp -f $@ u-boot.img; \

> >> > +       fi

> >> > +

> >> > +.NOTPARALLEL: dtbs

> >>

> >> Why is that needed?

> >

> > Good catch! This issue actually caused me a fair amount of grief when

> > running into it. During development we found the build process works

> > with make -j1 but would fail for parallel builds (like make -j8) with

> > the following error....

> >

> > ----------------------->8---------------------------

> > $ make mrproper

> > $ make dra7xx_hs_evm_defconfig

> > $ make -j8

> > ....

> >   LD      u-boot

> >   OBJCOPY u-boot-nodtb.bin

> >   OBJCOPY u-boot.srec

> >   SYM     u-boot.sym

> >   DTC     arch/arm/dts/dra72-evm.dtb

> >   DTC     arch/arm/dts/dra7-evm.dtb

> >   DTC     arch/arm/dts/dra72-evm.dtb

> >   DTC     arch/arm/dts/dra7-evm.dtb

> > Error: arch/arm/dts/dra7-evm.dts:492.15-16 syntax error

> > FATAL ERROR: Unable to parse input tree

> > make[2]: *** [arch/arm/dts/dra7-evm.dtb] Error 1

> > make[2]: *** Waiting for unfinished jobs....

> > Error: arch/arm/dts/dra72-evm.dts:149.26-150.2 syntax error

> > FATAL ERROR: Unable to parse input tree

> > make[2]: *** [arch/arm/dts/dra72-evm.dtb] Error 1

> > make[1]: *** [arch-dtbs] Error 2

> > make: *** [dtbs] Error 2

> > make: *** Waiting for unfinished jobs....

> >   SHIPPED dts/dt.dtb

> > ----------------------->8---------------------------

> 

> Probably you are missing a dependency somewhere - but the failure to

> parse is odd. What is it missing?


This failure in parsing is not the real issue. What I found (at least
this is how interpreted it) two concurrent DTC builds on the same file
are stomping over each other in terms of temp files. Besides this I'd
also have a very hard time explaining this error in any other way since
the dts file itself is perfectly fine.

> >

> > However with the .NOTPARALLEL: dtbs line included it works. Also when

> > doing make a second time it works as well (clearly that's not a

> > solution:)

> >

> > On my quest trying to digest/root cause this I concluded that this may

> > be caused by the DTB files somehow getting compiled twice (see snippet).

> > This in combination with the fact that we post-process files like

> > dta7-evm.dtb (signing them) and creating new DTB files with the same

> > name (like dra7-evm.dtb) replacing the old ones causes issues during the

> > DTB compile step.

> 

> Yes I think it is better to create new files, since otherwise the

> build system can't determine which version of the file it should

> target...it assumes a single version as I understand it.


Yes this is how I had it initially. But as indicated the issue was that
the mkimage step that packages this newly-named DTB files would put them
into the u-boot.img FIT blob also with this odd new name. And then the
board match would not work anymore... And that was when I decided to
stop hacking stuff to get it to work.

> >

> > Now I'm not a make guru but I tried to clean up the dependency chain as

> > good as I can but could not get it to the point where each DTB file is

> > only build once which most likely would allow us getting rid of

> > NOPARALLEL.

> >

> > On a related note we wanted to keep the same names for the DTB files

> > (rather than turning dra7-evm.dtb into dra7-evm-sec.dtb for example, as

> > I had it initially) since it's important as they get bundled into the

> > u-boot.img FIT blob so that they can be later matched by the SPL to a

> > board. But it appears like that trying to keep the same names despite

> > the additional signing step seems to complicate things from a make

> > perspective.

> 

> Yes - best to create a new u-boot-sec.img I think.


U-Boot itself is not really the issue here - in fact we are already
creating an u-boot_HS.img output artifact. The issue is with the DTBs...

> > If you or anybody else has any smart suggestion how to address this in

> > a better way I'd love to hear about it.

> 

> Masahiro is the expert, but my ideas are above.


Awesome. Now that the kids are asleep I'm actually in the process
preparing/testing an optimized patch series based on your, Lokesh's, and
Tom's feedback. I should be able to send it out soon hopefully tonight.
It won't have 100% of feedback implemented (Make stuff is the same) but
it'll be a better base for continued discussion.

Best Regards,


--
Andreas Dannenberg
Texas Instruments Inc
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

2016-06-21 11:35 GMT+09:00 Andreas Dannenberg <dannenberg@ti.com>:
> Hi Simon,

> thanks for the continued feedback. Comments below...

>

> On Mon, Jun 20, 2016 at 04:40:10PM -0600, Simon Glass wrote:

>> +Masahiro for the Makefile question

>>

>> Hi Andreas,

>>

>> On 17 June 2016 at 10:13, Andreas Dannenberg <dannenberg@ti.com> wrote:

>> > Hi Simon,

>> > many thanks for your feedback on the series... I know it takes a lot of

>> > effort to digest all that stuff. We'll see how we can tackle the

>> > feedback one by one and send out an updated series.

>> >

>> > Regarding this patch, please see below...

>> >

>> > On Thu, Jun 16, 2016 at 09:52:52PM -0600, Simon Glass wrote:

>> >> Hi Andreas,

>> >>

>> >> On 15 June 2016 at 13:26, Andreas Dannenberg <dannenberg@ti.com> wrote:

>> >> > From: Daniel Allred <d-allred@ti.com>

>> >> >

>> >> > Adds commands so that when a secure device is in use and the SPL is

>> >> > built to load a FIT image (with combined u-boot binary and various

>> >> > DTBs), these components that get fed into the FIT are all processed to

>> >> > be signed/encrypted/etc. as per the operations performed by the

>> >> > secure-binary-image script of the TI SECDEV package.

>> >> >

>> >> > Signed-off-by: Daniel Allred <d-allred@ti.com>

>> >> > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>

>> >> > ---

>> >> >  arch/arm/cpu/armv7/omap-common/config_secure.mk | 57 ++++++++++++++++++++++++-

>> >> >  arch/arm/cpu/armv7/omap5/config.mk              |  3 ++

>> >> >  2 files changed, 58 insertions(+), 2 deletions(-)

>> >>

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

>> >>

>> >> But please can you add a README for this somewhere?

>> >>

>> >> Also, can this tool be added to U-Boot instead of being external?

>> >

>> > Yes we will definitely enhance the readme in the PATCH version, this

>> > wasn't quite ready yet by the time we submitted the RFC.

>> >

>> > Also regarding the external singing/encryption tool this is only

>> > available from TI under NDA after customers engage with TI just like the

>> > high-secure (HS) devices themselves (you can't just buy them on

>> > Digi-Key). Personally I hope that we eventually lower this barrier of

>> > entry (and this has been brought up more than once) but as many things

>> > in the corporate world there is a complex decision process behind it. So

>> > until this strategy changes (if ever) our poor developer's hands are

>> > tied. All we can do for now is trying to allow this external tool to get

>> > hooked into the U-Boot build process in the easiest way possible which

>> > is already done today by way of the $TI_SECURE_DEV_PKG environmental

>> > variable during SPL build.

>>

>> OK. That's odd because it should be the keys that are secure, not the

>> tool! If anything, the tool should have as many eyes on it as

>> possible.

>

> Yes that's the ideal-world case I was also arguing we should follow...

> But as indicated decision processes in the corporate world sometimes are

> complex :)

>

>> >> >

>> >> > diff --git a/arch/arm/cpu/armv7/omap-common/config_secure.mk b/arch/arm/cpu/armv7/omap-common/config_secure.mk

>> >> > index c7bb101..c4514ad 100644

>> >> > --- a/arch/arm/cpu/armv7/omap-common/config_secure.mk

>> >> > +++ b/arch/arm/cpu/armv7/omap-common/config_secure.mk

>> >> > @@ -12,8 +12,8 @@ cmd_mkomapsecimg = $(TI_SECURE_DEV_PKG)/scripts/create-boot-image.sh \

>> >> >         $(if $(KBUILD_VERBOSE:1=), >/dev/null)

>> >> >  else

>> >> >  cmd_mkomapsecimg = $(TI_SECURE_DEV_PKG)/scripts/create-boot-image.sh \

>> >> > -    $(patsubst u-boot_HS_%,%,$(@F)) $< $@ $(CONFIG_ISW_ENTRY_ADDR) \

>> >> > -    $(if $(KBUILD_VERBOSE:1=), >/dev/null)

>> >> > +       $(patsubst u-boot_HS_%,%,$(@F)) $< $@ $(CONFIG_ISW_ENTRY_ADDR) \

>> >> > +       $(if $(KBUILD_VERBOSE:1=), >/dev/null)

>> >> >  endif

>> >> >  else

>> >> >  cmd_mkomapsecimg = echo "WARNING:" \

>> >> > @@ -25,6 +25,26 @@ cmd_mkomapsecimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \

>> >> >         "variable must be defined for TI secure devices. $@ was NOT created!"

>> >> >  endif

>> >> >

>> >> > +ifdef CONFIG_SPL_LOAD_FIT

>> >> > +quiet_cmd_omapsecureimg = SECURE  $@

>> >> > +ifneq ($(TI_SECURE_DEV_PKG),)

>> >> > +ifneq ($(wildcard $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh),)

>> >> > +cmd_omapsecureimg = $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh \

>> >> > +       $< $@ \

>> >> > +       $(if $(KBUILD_VERBOSE:1=), >/dev/null)

>> >> > +else

>> >> > +cmd_omapsecureimg = echo "WARNING:" \

>> >> > +       "$(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh not found." \

>> >> > +       "$@ was NOT created!"; cp $< $@

>> >> > +endif

>> >> > +else

>> >> > +cmd_omapsecureimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \

>> >> > +       "variable must be defined for TI secure devices." \

>> >> > +       "$@ was NOT created!"; cp $< $@

>> >> > +endif

>> >> > +endif

>> >> > +

>> >> > +

>> >> >  # Standard X-LOADER target (QPSI, NOR flash)

>> >> >  u-boot-spl_HS_X-LOADER: $(obj)/u-boot-spl.bin

>> >> >         $(call if_changed,mkomapsecimg)

>> >> > @@ -64,3 +84,36 @@ u-boot-spl_HS_SPI_X-LOADER: $(obj)/u-boot-spl.bin

>> >> >  # the mkomapsecimg command looks for a u-boot-HS_* prefix

>> >> >  u-boot_HS_XIP_X-LOADER: $(obj)/u-boot.bin

>> >> >         $(call if_changed,mkomapsecimg)

>> >> > +

>> >> > +# For supporting the SPL loading and interpreting

>> >> > +# of FIT images whose components are pre-processed

>> >> > +# before being integrated into the FIT image in order

>> >> > +# to secure them in some way

>> >> > +ifdef CONFIG_SPL_LOAD_FIT

>> >> > +

>> >> > +MKIMAGEFLAGS_u-boot_HS.img = -f auto -A $(ARCH) -T firmware -C none -O u-boot \

>> >> > +       -a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \

>> >> > +       -n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \

>> >> > +       $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))

>> >> > +

>> >> > +OF_LIST_TARGETS = $(patsubst %,arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))

>> >> > +$(OF_LIST_TARGETS): dtbs

>> >> > +

>> >> > +%_HS.dtb: %.dtb

>> >> > +       $(call if_changed,omapsecureimg)

>> >> > +       $(Q)if [ -f $@ ]; then \

>> >> > +               cp -f $@ $<; \

>> >> > +       fi

>> >> > +

>> >> > +u-boot-nodtb_HS.bin: u-boot-nodtb.bin

>> >> > +       $(call if_changed,omapsecureimg)

>> >> > +

>> >> > +u-boot_HS.img: u-boot-nodtb_HS.bin u-boot.img $(patsubst %.dtb,%_HS.dtb,$(OF_LIST_TARGETS))

>> >> > +       $(call if_changed,mkimage)

>> >> > +       $(Q)if [ -f $@ ]; then \

>> >> > +               cp -f $@ u-boot.img; \

>> >> > +       fi

>> >> > +

>> >> > +.NOTPARALLEL: dtbs

>> >>

>> >> Why is that needed?

>> >

>> > Good catch! This issue actually caused me a fair amount of grief when

>> > running into it. During development we found the build process works

>> > with make -j1 but would fail for parallel builds (like make -j8) with

>> > the following error....

>> >

>> > ----------------------->8---------------------------

>> > $ make mrproper

>> > $ make dra7xx_hs_evm_defconfig

>> > $ make -j8

>> > ....

>> >   LD      u-boot

>> >   OBJCOPY u-boot-nodtb.bin

>> >   OBJCOPY u-boot.srec

>> >   SYM     u-boot.sym

>> >   DTC     arch/arm/dts/dra72-evm.dtb

>> >   DTC     arch/arm/dts/dra7-evm.dtb

>> >   DTC     arch/arm/dts/dra72-evm.dtb

>> >   DTC     arch/arm/dts/dra7-evm.dtb

>> > Error: arch/arm/dts/dra7-evm.dts:492.15-16 syntax error

>> > FATAL ERROR: Unable to parse input tree

>> > make[2]: *** [arch/arm/dts/dra7-evm.dtb] Error 1

>> > make[2]: *** Waiting for unfinished jobs....

>> > Error: arch/arm/dts/dra72-evm.dts:149.26-150.2 syntax error

>> > FATAL ERROR: Unable to parse input tree

>> > make[2]: *** [arch/arm/dts/dra72-evm.dtb] Error 1

>> > make[1]: *** [arch-dtbs] Error 2

>> > make: *** [dtbs] Error 2

>> > make: *** Waiting for unfinished jobs....

>> >   SHIPPED dts/dt.dtb

>> > ----------------------->8---------------------------

>>

>> Probably you are missing a dependency somewhere - but the failure to

>> parse is odd. What is it missing?

>

> This failure in parsing is not the real issue. What I found (at least

> this is how interpreted it) two concurrent DTC builds on the same file

> are stomping over each other in terms of temp files. Besides this I'd

> also have a very hard time explaining this error in any other way since

> the dts file itself is perfectly fine.

>

>> >

>> > However with the .NOTPARALLEL: dtbs line included it works. Also when

>> > doing make a second time it works as well (clearly that's not a

>> > solution:)

>> >

>> > On my quest trying to digest/root cause this I concluded that this may

>> > be caused by the DTB files somehow getting compiled twice (see snippet).

>> > This in combination with the fact that we post-process files like

>> > dta7-evm.dtb (signing them) and creating new DTB files with the same

>> > name (like dra7-evm.dtb) replacing the old ones causes issues during the

>> > DTB compile step.

>>

>> Yes I think it is better to create new files, since otherwise the

>> build system can't determine which version of the file it should

>> target...it assumes a single version as I understand it.

>

> Yes this is how I had it initially. But as indicated the issue was that

> the mkimage step that packages this newly-named DTB files would put them

> into the u-boot.img FIT blob also with this odd new name. And then the

> board match would not work anymore... And that was when I decided to

> stop hacking stuff to get it to work.

>

>> >

>> > Now I'm not a make guru but I tried to clean up the dependency chain as

>> > good as I can but could not get it to the point where each DTB file is

>> > only build once which most likely would allow us getting rid of

>> > NOPARALLEL.

>> >

>> > On a related note we wanted to keep the same names for the DTB files

>> > (rather than turning dra7-evm.dtb into dra7-evm-sec.dtb for example, as

>> > I had it initially) since it's important as they get bundled into the

>> > u-boot.img FIT blob so that they can be later matched by the SPL to a

>> > board. But it appears like that trying to keep the same names despite

>> > the additional signing step seems to complicate things from a make

>> > perspective.

>>

>> Yes - best to create a new u-boot-sec.img I think.

>

> U-Boot itself is not really the issue here - in fact we are already

> creating an u-boot_HS.img output artifact. The issue is with the DTBs...

>

>> > If you or anybody else has any smart suggestion how to address this in

>> > a better way I'd love to hear about it.

>>

>> Masahiro is the expert, but my ideas are above.

>

> Awesome. Now that the kids are asleep I'm actually in the process

> preparing/testing an optimized patch series based on your, Lokesh's, and

> Tom's feedback. I should be able to send it out soon hopefully tonight.

> It won't have 100% of feedback implemented (Make stuff is the same) but

> it'll be a better base for continued discussion.

>

> Best Regards,

>



Hi.

Could you try this?
http://patchwork.ozlabs.org/patch/639478/

I think you can remove the ".NOTPARALLEL" line.




-- 
Best Regards
Masahiro Yamada
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

On Thu, Jun 23, 2016 at 01:59:40PM +0900, Masahiro Yamada wrote:
> 2016-06-21 11:35 GMT+09:00 Andreas Dannenberg <dannenberg@ti.com>:

> > Hi Simon,

> > thanks for the continued feedback. Comments below...

> >

> > On Mon, Jun 20, 2016 at 04:40:10PM -0600, Simon Glass wrote:

> >> +Masahiro for the Makefile question

> >>

> >> Hi Andreas,

> >>

> >> On 17 June 2016 at 10:13, Andreas Dannenberg <dannenberg@ti.com> wrote:

> >> > Hi Simon,

> >> > many thanks for your feedback on the series... I know it takes a lot of

> >> > effort to digest all that stuff. We'll see how we can tackle the

> >> > feedback one by one and send out an updated series.

> >> >

> >> > Regarding this patch, please see below...

> >> >

> >> > On Thu, Jun 16, 2016 at 09:52:52PM -0600, Simon Glass wrote:

> >> >> Hi Andreas,

> >> >>

> >> >> On 15 June 2016 at 13:26, Andreas Dannenberg <dannenberg@ti.com> wrote:

> >> >> > From: Daniel Allred <d-allred@ti.com>

> >> >> >

> >> >> > Adds commands so that when a secure device is in use and the SPL is

> >> >> > built to load a FIT image (with combined u-boot binary and various

> >> >> > DTBs), these components that get fed into the FIT are all processed to

> >> >> > be signed/encrypted/etc. as per the operations performed by the

> >> >> > secure-binary-image script of the TI SECDEV package.

> >> >> >

> >> >> > Signed-off-by: Daniel Allred <d-allred@ti.com>

> >> >> > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>

> >> >> > ---

> >> >> >  arch/arm/cpu/armv7/omap-common/config_secure.mk | 57 ++++++++++++++++++++++++-

> >> >> >  arch/arm/cpu/armv7/omap5/config.mk              |  3 ++

> >> >> >  2 files changed, 58 insertions(+), 2 deletions(-)

> >> >>

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

> >> >>

> >> >> But please can you add a README for this somewhere?

> >> >>

> >> >> Also, can this tool be added to U-Boot instead of being external?

> >> >

> >> > Yes we will definitely enhance the readme in the PATCH version, this

> >> > wasn't quite ready yet by the time we submitted the RFC.

> >> >

> >> > Also regarding the external singing/encryption tool this is only

> >> > available from TI under NDA after customers engage with TI just like the

> >> > high-secure (HS) devices themselves (you can't just buy them on

> >> > Digi-Key). Personally I hope that we eventually lower this barrier of

> >> > entry (and this has been brought up more than once) but as many things

> >> > in the corporate world there is a complex decision process behind it. So

> >> > until this strategy changes (if ever) our poor developer's hands are

> >> > tied. All we can do for now is trying to allow this external tool to get

> >> > hooked into the U-Boot build process in the easiest way possible which

> >> > is already done today by way of the $TI_SECURE_DEV_PKG environmental

> >> > variable during SPL build.

> >>

> >> OK. That's odd because it should be the keys that are secure, not the

> >> tool! If anything, the tool should have as many eyes on it as

> >> possible.

> >

> > Yes that's the ideal-world case I was also arguing we should follow...

> > But as indicated decision processes in the corporate world sometimes are

> > complex :)

> >

> >> >> >

> >> >> > diff --git a/arch/arm/cpu/armv7/omap-common/config_secure.mk b/arch/arm/cpu/armv7/omap-common/config_secure.mk

> >> >> > index c7bb101..c4514ad 100644

> >> >> > --- a/arch/arm/cpu/armv7/omap-common/config_secure.mk

> >> >> > +++ b/arch/arm/cpu/armv7/omap-common/config_secure.mk

> >> >> > @@ -12,8 +12,8 @@ cmd_mkomapsecimg = $(TI_SECURE_DEV_PKG)/scripts/create-boot-image.sh \

> >> >> >         $(if $(KBUILD_VERBOSE:1=), >/dev/null)

> >> >> >  else

> >> >> >  cmd_mkomapsecimg = $(TI_SECURE_DEV_PKG)/scripts/create-boot-image.sh \

> >> >> > -    $(patsubst u-boot_HS_%,%,$(@F)) $< $@ $(CONFIG_ISW_ENTRY_ADDR) \

> >> >> > -    $(if $(KBUILD_VERBOSE:1=), >/dev/null)

> >> >> > +       $(patsubst u-boot_HS_%,%,$(@F)) $< $@ $(CONFIG_ISW_ENTRY_ADDR) \

> >> >> > +       $(if $(KBUILD_VERBOSE:1=), >/dev/null)

> >> >> >  endif

> >> >> >  else

> >> >> >  cmd_mkomapsecimg = echo "WARNING:" \

> >> >> > @@ -25,6 +25,26 @@ cmd_mkomapsecimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \

> >> >> >         "variable must be defined for TI secure devices. $@ was NOT created!"

> >> >> >  endif

> >> >> >

> >> >> > +ifdef CONFIG_SPL_LOAD_FIT

> >> >> > +quiet_cmd_omapsecureimg = SECURE  $@

> >> >> > +ifneq ($(TI_SECURE_DEV_PKG),)

> >> >> > +ifneq ($(wildcard $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh),)

> >> >> > +cmd_omapsecureimg = $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh \

> >> >> > +       $< $@ \

> >> >> > +       $(if $(KBUILD_VERBOSE:1=), >/dev/null)

> >> >> > +else

> >> >> > +cmd_omapsecureimg = echo "WARNING:" \

> >> >> > +       "$(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh not found." \

> >> >> > +       "$@ was NOT created!"; cp $< $@

> >> >> > +endif

> >> >> > +else

> >> >> > +cmd_omapsecureimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \

> >> >> > +       "variable must be defined for TI secure devices." \

> >> >> > +       "$@ was NOT created!"; cp $< $@

> >> >> > +endif

> >> >> > +endif

> >> >> > +

> >> >> > +

> >> >> >  # Standard X-LOADER target (QPSI, NOR flash)

> >> >> >  u-boot-spl_HS_X-LOADER: $(obj)/u-boot-spl.bin

> >> >> >         $(call if_changed,mkomapsecimg)

> >> >> > @@ -64,3 +84,36 @@ u-boot-spl_HS_SPI_X-LOADER: $(obj)/u-boot-spl.bin

> >> >> >  # the mkomapsecimg command looks for a u-boot-HS_* prefix

> >> >> >  u-boot_HS_XIP_X-LOADER: $(obj)/u-boot.bin

> >> >> >         $(call if_changed,mkomapsecimg)

> >> >> > +

> >> >> > +# For supporting the SPL loading and interpreting

> >> >> > +# of FIT images whose components are pre-processed

> >> >> > +# before being integrated into the FIT image in order

> >> >> > +# to secure them in some way

> >> >> > +ifdef CONFIG_SPL_LOAD_FIT

> >> >> > +

> >> >> > +MKIMAGEFLAGS_u-boot_HS.img = -f auto -A $(ARCH) -T firmware -C none -O u-boot \

> >> >> > +       -a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \

> >> >> > +       -n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \

> >> >> > +       $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))

> >> >> > +

> >> >> > +OF_LIST_TARGETS = $(patsubst %,arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))

> >> >> > +$(OF_LIST_TARGETS): dtbs

> >> >> > +

> >> >> > +%_HS.dtb: %.dtb

> >> >> > +       $(call if_changed,omapsecureimg)

> >> >> > +       $(Q)if [ -f $@ ]; then \

> >> >> > +               cp -f $@ $<; \

> >> >> > +       fi

> >> >> > +

> >> >> > +u-boot-nodtb_HS.bin: u-boot-nodtb.bin

> >> >> > +       $(call if_changed,omapsecureimg)

> >> >> > +

> >> >> > +u-boot_HS.img: u-boot-nodtb_HS.bin u-boot.img $(patsubst %.dtb,%_HS.dtb,$(OF_LIST_TARGETS))

> >> >> > +       $(call if_changed,mkimage)

> >> >> > +       $(Q)if [ -f $@ ]; then \

> >> >> > +               cp -f $@ u-boot.img; \

> >> >> > +       fi

> >> >> > +

> >> >> > +.NOTPARALLEL: dtbs

> >> >>

> >> >> Why is that needed?

> >> >

> >> > Good catch! This issue actually caused me a fair amount of grief when

> >> > running into it. During development we found the build process works

> >> > with make -j1 but would fail for parallel builds (like make -j8) with

> >> > the following error....

> >> >

> >> > ----------------------->8---------------------------

> >> > $ make mrproper

> >> > $ make dra7xx_hs_evm_defconfig

> >> > $ make -j8

> >> > ....

> >> >   LD      u-boot

> >> >   OBJCOPY u-boot-nodtb.bin

> >> >   OBJCOPY u-boot.srec

> >> >   SYM     u-boot.sym

> >> >   DTC     arch/arm/dts/dra72-evm.dtb

> >> >   DTC     arch/arm/dts/dra7-evm.dtb

> >> >   DTC     arch/arm/dts/dra72-evm.dtb

> >> >   DTC     arch/arm/dts/dra7-evm.dtb

> >> > Error: arch/arm/dts/dra7-evm.dts:492.15-16 syntax error

> >> > FATAL ERROR: Unable to parse input tree

> >> > make[2]: *** [arch/arm/dts/dra7-evm.dtb] Error 1

> >> > make[2]: *** Waiting for unfinished jobs....

> >> > Error: arch/arm/dts/dra72-evm.dts:149.26-150.2 syntax error

> >> > FATAL ERROR: Unable to parse input tree

> >> > make[2]: *** [arch/arm/dts/dra72-evm.dtb] Error 1

> >> > make[1]: *** [arch-dtbs] Error 2

> >> > make: *** [dtbs] Error 2

> >> > make: *** Waiting for unfinished jobs....

> >> >   SHIPPED dts/dt.dtb

> >> > ----------------------->8---------------------------

> >>

> >> Probably you are missing a dependency somewhere - but the failure to

> >> parse is odd. What is it missing?

> >

> > This failure in parsing is not the real issue. What I found (at least

> > this is how interpreted it) two concurrent DTC builds on the same file

> > are stomping over each other in terms of temp files. Besides this I'd

> > also have a very hard time explaining this error in any other way since

> > the dts file itself is perfectly fine.

> >

> >> >

> >> > However with the .NOTPARALLEL: dtbs line included it works. Also when

> >> > doing make a second time it works as well (clearly that's not a

> >> > solution:)

> >> >

> >> > On my quest trying to digest/root cause this I concluded that this may

> >> > be caused by the DTB files somehow getting compiled twice (see snippet).

> >> > This in combination with the fact that we post-process files like

> >> > dta7-evm.dtb (signing them) and creating new DTB files with the same

> >> > name (like dra7-evm.dtb) replacing the old ones causes issues during the

> >> > DTB compile step.

> >>

> >> Yes I think it is better to create new files, since otherwise the

> >> build system can't determine which version of the file it should

> >> target...it assumes a single version as I understand it.

> >

> > Yes this is how I had it initially. But as indicated the issue was that

> > the mkimage step that packages this newly-named DTB files would put them

> > into the u-boot.img FIT blob also with this odd new name. And then the

> > board match would not work anymore... And that was when I decided to

> > stop hacking stuff to get it to work.

> >

> >> >

> >> > Now I'm not a make guru but I tried to clean up the dependency chain as

> >> > good as I can but could not get it to the point where each DTB file is

> >> > only build once which most likely would allow us getting rid of

> >> > NOPARALLEL.

> >> >

> >> > On a related note we wanted to keep the same names for the DTB files

> >> > (rather than turning dra7-evm.dtb into dra7-evm-sec.dtb for example, as

> >> > I had it initially) since it's important as they get bundled into the

> >> > u-boot.img FIT blob so that they can be later matched by the SPL to a

> >> > board. But it appears like that trying to keep the same names despite

> >> > the additional signing step seems to complicate things from a make

> >> > perspective.

> >>

> >> Yes - best to create a new u-boot-sec.img I think.

> >

> > U-Boot itself is not really the issue here - in fact we are already

> > creating an u-boot_HS.img output artifact. The issue is with the DTBs...

> >

> >> > If you or anybody else has any smart suggestion how to address this in

> >> > a better way I'd love to hear about it.

> >>

> >> Masahiro is the expert, but my ideas are above.

> >

> > Awesome. Now that the kids are asleep I'm actually in the process

> > preparing/testing an optimized patch series based on your, Lokesh's, and

> > Tom's feedback. I should be able to send it out soon hopefully tonight.

> > It won't have 100% of feedback implemented (Make stuff is the same) but

> > it'll be a better base for continued discussion.

> >

> > Best Regards,

> >

> 

> 

> Hi.

> 

> Could you try this?

> http://patchwork.ozlabs.org/patch/639478/

> 

> I think you can remove the ".NOTPARALLEL" line.


Yamada-san, yes your patch fixes this. Thanks for spending time thinking
about this issue. Will remove the .NOTPARALLEL hack in the next revision
of this patch series.

Regards,

--
Andreas Dannenberg
Texas Instruments Inc
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot