[ptxdist] [PATCH] tf-a: stop defining AS and other variables in cross make env

[ptxdist] [PATCH] tf-a: stop defining AS and other variables in cross make env

[Ahmad Fatoum]

The MAKE_ENV incantation results in PTXdist defining variables like AS
and LD that point at the respective tools in the cross toolchain.

This was unneeded as we already supply the TF-A built with a
CROSS_COMPILE option, which is used to derive the correct tools, but it
didn't hurt.

This will expectedly change with TF-A v2.11, which should merge commit
cc277de81692 ("build: refactor toolchain detection"), currently sitting
in the integration branch.

This changes the default value of AS to be that of the C compiler and
leads to various issues when overriding it with an assembler as was
possible before, e.g.:

  - hang during tool discovery, because `as -v` keeps waiting for input,
    unlike gcc -v

  - Assembler errors during build, because GCC-specific options (e.g.
    -DMACRO=VAL) are now passed to as(1).

While TF-A changing the semantics of the AS option is arguably
strange[2], let's take the easy way out and stop setting AS, so the PTXdist
rule can build both old and new TF-A without problems.

[1]: https://github.com/ARM-software/arm-trusted-firmware/commit/cc277de8169
[2]: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/27163

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 rules/tf-a.make | 2 --
 1 file changed, 2 deletions(-)

diff --git a/rules/tf-a.make b/rules/tf-a.make
index 54fe7d1e37d8..b8c477476c74 100644
--- a/rules/tf-a.make
+++ b/rules/tf-a.make
@@ -74,8 +74,6 @@ TF_A_CONF_TOOL	:= NO
 # Compile
 # ----------------------------------------------------------------------------
 
-TF_A_MAKE_ENV	:= $(CROSS_ENV)
-
 $(STATEDIR)/tf-a.compile:
 	@$(call targetinfo)
 
-- 
2.39.2

Re: [ptxdist] [PATCH] tf-a: stop defining AS and other variables in cross make env

[Christian Melki]

On 4/9/24 11:08 AM, Ahmad Fatoum wrote:
> The MAKE_ENV incantation results in PTXdist defining variables like AS
> and LD that point at the respective tools in the cross toolchain.
> 
> This was unneeded as we already supply the TF-A built with a
> CROSS_COMPILE option, which is used to derive the correct tools, but it
> didn't hurt.
> 
> This will expectedly change with TF-A v2.11, which should merge commit
> cc277de81692 ("build: refactor toolchain detection"), currently sitting
> in the integration branch.
> 
> This changes the default value of AS to be that of the C compiler and
> leads to various issues when overriding it with an assembler as was
> possible before, e.g.:
> 
>    - hang during tool discovery, because `as -v` keeps waiting for input,
>      unlike gcc -v
> 
>    - Assembler errors during build, because GCC-specific options (e.g.
>      -DMACRO=VAL) are now passed to as(1).
> 
> While TF-A changing the semantics of the AS option is arguably
> strange[2], let's take the easy way out and stop setting AS, so the PTXdist
> rule can build both old and new TF-A without problems.
> 
> [1]: https://github.com/ARM-software/arm-trusted-firmware/commit/cc277de8169
> [2]: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/27163
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>   rules/tf-a.make | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/rules/tf-a.make b/rules/tf-a.make
> index 54fe7d1e37d8..b8c477476c74 100644
> --- a/rules/tf-a.make
> +++ b/rules/tf-a.make
> @@ -74,8 +74,6 @@ TF_A_CONF_TOOL	:= NO
>   # Compile
>   # ----------------------------------------------------------------------------
>   
> -TF_A_MAKE_ENV	:= $(CROSS_ENV)
> -

Perhaps not in this config, but isn't the make env used for other things?
Like TLS stuff?

regards,
Christian

>   $(STATEDIR)/tf-a.compile:
>   	@$(call targetinfo)
>   

[ptxdist] [PATCH] tf-a: stop defining AS and other variables in cross make env

[Ahmad Fatoum]

The MAKE_ENV incantation results in PTXdist defining variables like AS
and LD that point at the respective tools in the cross toolchain.

This was unused as TF-A doesn't consult the AS setting in the
environment, and unneeded, because we already supply a CROSS_COMPILE
option, which is used to derive the correct tools.

This will expectedly change with TF-A v2.11, which should merge commit
cc277de81692 ("build: refactor toolchain detection"), currently sitting
in the integration branch[1].

This will start accepting AS to be set from the environment and not only
from the make arguments. This will break using the default cross env as
TF-A assumes AS to point at a C compiler and not at an assembler.

This will result in build issues, like[2]:

  - hang during tool discovery, because `as -v` keeps waiting for input,
    unlike gcc -v

  - Assembler errors during build, because GCC-specific options (e.g.
    -DMACRO=VAL) are now passed to as(1).

Avoid these issues by not defining the CROSS_ENV.

[1]: https://github.com/ARM-software/arm-trusted-firmware/commit/cc277de8169
[2]: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/27163

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - reword commit message after new findings about history of AS in
    TF-A.
---
 rules/tf-a.make | 2 --
 1 file changed, 2 deletions(-)

diff --git a/rules/tf-a.make b/rules/tf-a.make
index 54fe7d1e37d8..b8c477476c74 100644
--- a/rules/tf-a.make
+++ b/rules/tf-a.make
@@ -74,8 +74,6 @@ TF_A_CONF_TOOL	:= NO
 # Compile
 # ----------------------------------------------------------------------------
 
-TF_A_MAKE_ENV	:= $(CROSS_ENV)
-
 $(STATEDIR)/tf-a.compile:
 	@$(call targetinfo)
 
-- 
2.39.2

Re: [ptxdist] [PATCH] tf-a: stop defining AS and other variables in cross make env

[Ahmad Fatoum]

Hello Christian,

On 09.04.24 12:23, Christian Melki wrote:
> 
> 
> On 4/9/24 11:08 AM, Ahmad Fatoum wrote:
>> The MAKE_ENV incantation results in PTXdist defining variables like AS
>> and LD that point at the respective tools in the cross toolchain.
>>
>> This was unneeded as we already supply the TF-A built with a
>> CROSS_COMPILE option, which is used to derive the correct tools, but it
>> didn't hurt.
>>
>> This will expectedly change with TF-A v2.11, which should merge commit
>> cc277de81692 ("build: refactor toolchain detection"), currently sitting
>> in the integration branch.
>>
>> This changes the default value of AS to be that of the C compiler and
>> leads to various issues when overriding it with an assembler as was
>> possible before, e.g.:
>>
>>    - hang during tool discovery, because `as -v` keeps waiting for input,
>>      unlike gcc -v
>>
>>    - Assembler errors during build, because GCC-specific options (e.g.
>>      -DMACRO=VAL) are now passed to as(1).
>>
>> While TF-A changing the semantics of the AS option is arguably
>> strange[2], let's take the easy way out and stop setting AS, so the PTXdist
>> rule can build both old and new TF-A without problems.
>>
>> [1]: https://github.com/ARM-software/arm-trusted-firmware/commit/cc277de8169
>> [2]: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/27163
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>   rules/tf-a.make | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/rules/tf-a.make b/rules/tf-a.make
>> index 54fe7d1e37d8..b8c477476c74 100644
>> --- a/rules/tf-a.make
>> +++ b/rules/tf-a.make
>> @@ -74,8 +74,6 @@ TF_A_CONF_TOOL    := NO
>>   # Compile
>>   # ----------------------------------------------------------------------------
>>   -TF_A_MAKE_ENV    := $(CROSS_ENV)
>> -
> 
> Perhaps not in this config, but isn't the make env used for other things?
> Like TLS stuff?

Do you have a project, where you have TLS stuff and could test this?

Anyhow, v2.11 would break the recipe, so this trades breaking all recipes
for breaking only some recipes. Once we have breakage reports for those,
we can start setting only the relevant variables in TF_A_MAKE_ENV.

Cheers,
Ahmad

> 
> regards,
> Christian
> 
>>   $(STATEDIR)/tf-a.compile:
>>       @$(call targetinfo)
>>   
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [ptxdist] [PATCH] tf-a: stop defining AS and other variables in cross make env

[Christian Melki]

On 4/9/24 12:37 PM, Ahmad Fatoum wrote:
> Hello Christian,
> 
> On 09.04.24 12:23, Christian Melki wrote:
>>
>>
>> On 4/9/24 11:08 AM, Ahmad Fatoum wrote:
>>> The MAKE_ENV incantation results in PTXdist defining variables like AS
>>> and LD that point at the respective tools in the cross toolchain.
>>>
>>> This was unneeded as we already supply the TF-A built with a
>>> CROSS_COMPILE option, which is used to derive the correct tools, but it
>>> didn't hurt.
>>>
>>> This will expectedly change with TF-A v2.11, which should merge commit
>>> cc277de81692 ("build: refactor toolchain detection"), currently sitting
>>> in the integration branch.
>>>
>>> This changes the default value of AS to be that of the C compiler and
>>> leads to various issues when overriding it with an assembler as was
>>> possible before, e.g.:
>>>
>>>     - hang during tool discovery, because `as -v` keeps waiting for input,
>>>       unlike gcc -v
>>>
>>>     - Assembler errors during build, because GCC-specific options (e.g.
>>>       -DMACRO=VAL) are now passed to as(1).
>>>
>>> While TF-A changing the semantics of the AS option is arguably
>>> strange[2], let's take the easy way out and stop setting AS, so the PTXdist
>>> rule can build both old and new TF-A without problems.
>>>
>>> [1]: https://github.com/ARM-software/arm-trusted-firmware/commit/cc277de8169
>>> [2]: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/27163
>>>
>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>> ---
>>>    rules/tf-a.make | 2 --
>>>    1 file changed, 2 deletions(-)
>>>
>>> diff --git a/rules/tf-a.make b/rules/tf-a.make
>>> index 54fe7d1e37d8..b8c477476c74 100644
>>> --- a/rules/tf-a.make
>>> +++ b/rules/tf-a.make
>>> @@ -74,8 +74,6 @@ TF_A_CONF_TOOL    := NO
>>>    # Compile
>>>    # ----------------------------------------------------------------------------
>>>    -TF_A_MAKE_ENV    := $(CROSS_ENV)
>>> -
>>
>> Perhaps not in this config, but isn't the make env used for other things?
>> Like TLS stuff?
> 
> Do you have a project, where you have TLS stuff and could test this?
> 
> Anyhow, v2.11 would break the recipe, so this trades breaking all recipes
> for breaking only some recipes. Once we have breakage reports for those,
> we can start setting only the relevant variables in TF_A_MAKE_ENV.
> 
> Cheers,
> Ahmad
> 

I have one with an older release, not actively rebuilding it.
And I'm fine with the changes, just highlighting that it's maybe not an entirely useless variable.
So please, go ahead with the changes.

>>
>> regards,
>> Christian
>>
>>>    $(STATEDIR)/tf-a.compile:
>>>        @$(call targetinfo)
>>>    
>>
> 

Re: [ptxdist] [APPLIED] tf-a: stop defining AS and other variables in cross make env

[Michael Olbrich]

Thanks, applied as 341428f780542f718348dd9138c50a577f2998cd.

Michael

[sent from post-receive hook]

On Thu, 11 Apr 2024 17:04:57 +0200, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> The MAKE_ENV incantation results in PTXdist defining variables like AS
> and LD that point at the respective tools in the cross toolchain.
> 
> This was unused as TF-A doesn't consult the AS setting in the
> environment, and unneeded, because we already supply a CROSS_COMPILE
> option, which is used to derive the correct tools.
> 
> This will expectedly change with TF-A v2.11, which should merge commit
> cc277de81692 ("build: refactor toolchain detection"), currently sitting
> in the integration branch[1].
> 
> This will start accepting AS to be set from the environment and not only
> from the make arguments. This will break using the default cross env as
> TF-A assumes AS to point at a C compiler and not at an assembler.
> 
> This will result in build issues, like[2]:
> 
>   - hang during tool discovery, because `as -v` keeps waiting for input,
>     unlike gcc -v
> 
>   - Assembler errors during build, because GCC-specific options (e.g.
>     -DMACRO=VAL) are now passed to as(1).
> 
> Avoid these issues by not defining the CROSS_ENV.
> 
> [1]: https://github.com/ARM-software/arm-trusted-firmware/commit/cc277de8169
> [2]: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/27163
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Message-Id: <20240409102429.3143482-1-a.fatoum@pengutronix.de>
> Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
> 
> diff --git a/rules/tf-a.make b/rules/tf-a.make
> index 54fe7d1e37d8..b8c477476c74 100644
> --- a/rules/tf-a.make
> +++ b/rules/tf-a.make
> @@ -74,8 +74,6 @@ TF_A_CONF_TOOL	:= NO
>  # Compile
>  # ----------------------------------------------------------------------------
>  
> -TF_A_MAKE_ENV	:= $(CROSS_ENV)
> -
>  $(STATEDIR)/tf-a.compile:
>  	@$(call targetinfo)
>