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

Message ID 20180327164623.32739-1-victor.chong@linaro.org
State New
Headers show
Series
  • [Linaro-uefi] atf-build.sh: Fix path to BL32* images
Related show

Commit Message

Victor Chong March 27, 2018, 4:46 p.m.
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(-)

--
2.16.2

Comments

Fathi Boudra March 27, 2018, 5:54 p.m. | #1
Reviewed-by: Fathi Boudra <fathi.boudra@linaro.org>

On 27 March 2018 at 19:46, Victor Chong <victor.chong@linaro.org> 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`"
> +                       if [ X"$TOS_PLATFORM" = X"" ]; then
> +                               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
>                 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
>
Leif Lindholm March 27, 2018, 10:43 p.m. | #2
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?

> +				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
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
>
Victor Chong March 28, 2018, 6:19 a.m. | #3
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
>>

Patch

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`"
+			if [ X"$TOS_PLATFORM" = X"" ]; then
+				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
 		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