Re: [Linaro-uefi] [PATCH] atf-build.sh: Fix path to BL32* images

On Wed, Mar 28, 2018 at 7:43 AM, Leif Lindholm leif.lindholm@linaro.org wrote:

On Tue, Mar 27, 2018 at 05:46:22PM +0100, Victor Chong wrote:

...

Since TOS_BIN* images are not copied to UEFI dir anymore, set path to the BL32 images where they are originally built.

Fixes: 0c485dab ("opteed-build.sh: Remove copy of images to UEFI dir") Signed-off-by: Victor Chong victor.chong@linaro.org

---

atf-build.sh | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/atf-build.sh b/atf-build.sh index fb80ad0..eabfb32 100755 --- a/atf-build.sh +++ b/atf-build.sh @@ -99,17 +99,24 @@ function build_platform if [ X"$TOS_DIR" != X"" ]; then SPD="`$TOOLS_DIR/parse-platforms.py $PLATFORM_CONFIG -p $1 get -o atf_spd`"

•         TOS_PLATFORM="`$TOOLS_DIR/parse-platforms.py $PLATFORM_CONFIG -p $1 get -o tos_platform`"
  

•         if [ X"$TOS_PLATFORM" = X"" ]; then
  

•                 TOS_PLATFORM="`$TOOLS_DIR/parse-platforms.py $PLATFORM_CONFIG -p $1 get -o atf_platform`"
  

I'm not sure this step makes sense. There is nothing suggesting that ATF_PLATFORM will be the correct name for TOS_PLATFORM. (If there was, we wouldn't need both.)

...

•                 if [ X"$TOS_PLATFORM" = X"" ]; then
  

And the same applies here, really.

Is there a strong reason to not just bail if TOS_PLATFORM is not set?

Copied this from opteed-build.sh, but agree, should just bail in this case.

...

•                         TOS_PLATFORM=$1
  

•                 fi
  

•         fi
          TOS_BIN="`$TOOLS_DIR/parse-platforms.py $PLATFORM_CONFIG -p $1 get -o tos_bin`"
          TOS_BIN_EXTRA1="`$TOOLS_DIR/parse-platforms.py $PLATFORM_CONFIG -p $1 get -o tos_bin_extra1`"
          TOS_BIN_EXTRA2="`$TOOLS_DIR/parse-platforms.py $PLATFORM_CONFIG -p $1 get -o tos_bin_extra2`"
          if [ X"$TOS_BIN" != X"" ]; then
  

•                 BL32=$WORKSPACE/Build/$PLATFORM_IMAGE_DIR/$BUILD_PROFILE/FV/$TOS_BIN
  

•                 BL32=$TOS_DIR/out/arm-plat-$TOS_PLATFORM/core/$TOS_BIN
  

I'm not entirely thrilled with the hardcoding of 'arm' here, since the

'arm-plat-' is actually hardcoded by optee_os itself, i.e. the name of its build output dir, but on further thoughts, this actually makes atf-build.sh dependent on opteed-build.sh instead of just tos-build.sh, which is something we don't want, so please just ignore this patch. I'll send out another patch with a different approach, i.e. restore the approach of copying optee_os build images to the UEFI build dir.

Thanks!

optee build infrastructure itself does not hardcode. We already have if [ X"$TOS_ARCH" = X"" ]; then TOS_ARCH=arm fi in atf-build.sh. Perhaps we should add the same to optee-build.sh and add a config option for TOS_ARCH as well?

/ Leif

...

          fi
          if [ X"$TOS_BIN_EXTRA1" != X"" ]; then

•                 BL32_EXTRA1=$WORKSPACE/Build/$PLATFORM_IMAGE_DIR/$BUILD_PROFILE/FV/$TOS_BIN_EXTRA1
  

•                 BL32_EXTRA1=$TOS_DIR/out/arm-plat-$TOS_PLATFORM/core/$TOS_BIN_EXTRA1
          fi
          if [ X"$TOS_BIN_EXTRA2" != X"" ]; then
  

•                 BL32_EXTRA2=$WORKSPACE/Build/$PLATFORM_IMAGE_DIR/$BUILD_PROFILE/FV/$TOS_BIN_EXTRA2
  

•                 BL32_EXTRA2=$TOS_DIR/out/arm-plat-$TOS_PLATFORM/core/$TOS_BIN_EXTRA2
          fi
  
          if [ X"$SPD" != X"" ] && [ X"$BL32" != X"" ]; then
  

-- 2.16.2