diff mbox series

tune-cortexa53,tune-cortexa72: Add +nocrypto to -mcpu

Message ID 20250721054721.261911-1-raj.khem@gmail.com
State New
Headers show
Series tune-cortexa53,tune-cortexa72: Add +nocrypto to -mcpu | expand

Commit Message

Khem Raj July 21, 2025, 5:47 a.m. UTC
When crypto is not in tune features then add +nocryto to
-mcpu explicitly. This makes the behavior between clang
and gcc match. Currently -mcpu=cortex-a72 has different
behavior in clang and gcc in terms of what features are
considered default. Clang's philosophy to enable common
features but maximally, gcc on other hand takes minimal.
Clang therefore enables crypto with default set but gcc
does not. crypto is optional on cortext-a53 and cortex-a72
while, this is not as common but Broadcom SOCs in
raspberrypi3/4 have dropped crypto for cost reasons [1]
This results in illegal instruction traps [2] [3] when building
components e.g. chromium, qtwebengine, weston etc
with clang using -mcpu=cortex-a72 for rpi4 target.

Adding +nocrypto makes it behave like gcc does today. We
do have separate tune if crypto enabled cortex-a72 cores
are to be targeted (cortexa72-cryto) as DEFAULTTUNE

[1] https://forums.raspberrypi.com/viewtopic.php?f=63&t=207888#p1332960
[2] https://github.com/llvm/llvm-project/issues/85699
[3] https://github.com/llvm/llvm-project/issues/90365

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 meta/conf/machine/include/arm/armv8a/tune-cortexa53.inc | 2 ++
 meta/conf/machine/include/arm/armv8a/tune-cortexa72.inc | 2 ++
 2 files changed, 4 insertions(+)

Comments

Quentin Schulz July 21, 2025, 8:25 a.m. UTC | #1
Hi Khem,

On 7/21/25 7:47 AM, Khem Raj via lists.openembedded.org wrote:
> When crypto is not in tune features then add +nocryto to
> -mcpu explicitly. This makes the behavior between clang
> and gcc match. Currently -mcpu=cortex-a72 has different
> behavior in clang and gcc in terms of what features are
> considered default. Clang's philosophy to enable common
> features but maximally, gcc on other hand takes minimal.
> Clang therefore enables crypto with default set but gcc
> does not. crypto is optional on cortext-a53 and cortex-a72
> while, this is not as common but Broadcom SOCs in
> raspberrypi3/4 have dropped crypto for cost reasons [1]
> This results in illegal instruction traps [2] [3] when building
> components e.g. chromium, qtwebengine, weston etc
> with clang using -mcpu=cortex-a72 for rpi4 target.
> 
> Adding +nocrypto makes it behave like gcc does today. We
> do have separate tune if crypto enabled cortex-a72 cores
> are to be targeted (cortexa72-cryto) as DEFAULTTUNE
> 
> [1] https://forums.raspberrypi.com/viewtopic.php?f=63&t=207888#p1332960
> [2] https://github.com/llvm/llvm-project/issues/85699
> [3] https://github.com/llvm/llvm-project/issues/90365
> 
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---
>   meta/conf/machine/include/arm/armv8a/tune-cortexa53.inc | 2 ++
>   meta/conf/machine/include/arm/armv8a/tune-cortexa72.inc | 2 ++
>   2 files changed, 4 insertions(+)
> 
> diff --git a/meta/conf/machine/include/arm/armv8a/tune-cortexa53.inc b/meta/conf/machine/include/arm/armv8a/tune-cortexa53.inc
> index a88575eb156..65fd6333198 100644
> --- a/meta/conf/machine/include/arm/armv8a/tune-cortexa53.inc
> +++ b/meta/conf/machine/include/arm/armv8a/tune-cortexa53.inc
> @@ -2,6 +2,8 @@ DEFAULTTUNE ?= "cortexa53"
>   
>   TUNEVALID[cortexa53] = "Enable Cortex-A53 specific processor optimizations"
>   TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'cortexa53', ' -mcpu=cortex-a53', '', d)}"
> +# See  https://github.com/llvm/llvm-project/issues/85699
> +TUNE_CCARGS_MARCH_OPTS .= "${@bb.utils.contains('TUNE_FEATURES', 'crypto', '', '+nocrypto', d)}"
>   
>   require conf/machine/include/arm/arch-armv8a.inc
>   
> diff --git a/meta/conf/machine/include/arm/armv8a/tune-cortexa72.inc b/meta/conf/machine/include/arm/armv8a/tune-cortexa72.inc
> index cbb6418c069..cf09db00664 100644
> --- a/meta/conf/machine/include/arm/armv8a/tune-cortexa72.inc
> +++ b/meta/conf/machine/include/arm/armv8a/tune-cortexa72.inc
> @@ -2,6 +2,8 @@ DEFAULTTUNE ?= "cortexa72"
>   
>   TUNEVALID[cortexa72] = "Enable Cortex-A72 specific processor optimizations"
>   TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'cortexa72', ' -mcpu=cortex-a72', '', d)}"
> +# See  https://github.com/llvm/llvm-project/issues/85699
> +TUNE_CCARGS_MARCH_OPTS .= "${@bb.utils.contains('TUNE_FEATURES', 'crypto', '', '+nocrypto', d)}"
>   

I'm very unfamiliar with the tuning part of the machine configuration 
files, but isn't that something we could add in

meta/conf/machine/include/arm/feature-arm-crypto.inc

in the counterpart of +crypto?

I'm just a bit confused by the decision to only touch cortex a53 and a72 
tuning files? Reading the DTs and the commit log it seems those are the 
cores in the RPi 3 and 4 which do not have crypto, but I assume this 
would be applicable to any SoC without the crypto block anyway no? So 
should be generic instead? Are you aware of the RPi5 having done the 
same cost-cutting measure? It's sporting a76 cores so maybe we need to 
do something with that too?

Cheers,
Quentin
Khem Raj July 21, 2025, 3:54 p.m. UTC | #2
On Mon, Jul 21, 2025 at 1:25 AM Quentin Schulz <quentin.schulz@cherry.de> wrote:
>
> Hi Khem,
>
> On 7/21/25 7:47 AM, Khem Raj via lists.openembedded.org wrote:
> > When crypto is not in tune features then add +nocryto to
> > -mcpu explicitly. This makes the behavior between clang
> > and gcc match. Currently -mcpu=cortex-a72 has different
> > behavior in clang and gcc in terms of what features are
> > considered default. Clang's philosophy to enable common
> > features but maximally, gcc on other hand takes minimal.
> > Clang therefore enables crypto with default set but gcc
> > does not. crypto is optional on cortext-a53 and cortex-a72
> > while, this is not as common but Broadcom SOCs in
> > raspberrypi3/4 have dropped crypto for cost reasons [1]
> > This results in illegal instruction traps [2] [3] when building
> > components e.g. chromium, qtwebengine, weston etc
> > with clang using -mcpu=cortex-a72 for rpi4 target.
> >
> > Adding +nocrypto makes it behave like gcc does today. We
> > do have separate tune if crypto enabled cortex-a72 cores
> > are to be targeted (cortexa72-cryto) as DEFAULTTUNE
> >
> > [1] https://forums.raspberrypi.com/viewtopic.php?f=63&t=207888#p1332960
> > [2] https://github.com/llvm/llvm-project/issues/85699
> > [3] https://github.com/llvm/llvm-project/issues/90365
> >
> > Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > ---
> >   meta/conf/machine/include/arm/armv8a/tune-cortexa53.inc | 2 ++
> >   meta/conf/machine/include/arm/armv8a/tune-cortexa72.inc | 2 ++
> >   2 files changed, 4 insertions(+)
> >
> > diff --git a/meta/conf/machine/include/arm/armv8a/tune-cortexa53.inc b/meta/conf/machine/include/arm/armv8a/tune-cortexa53.inc
> > index a88575eb156..65fd6333198 100644
> > --- a/meta/conf/machine/include/arm/armv8a/tune-cortexa53.inc
> > +++ b/meta/conf/machine/include/arm/armv8a/tune-cortexa53.inc
> > @@ -2,6 +2,8 @@ DEFAULTTUNE ?= "cortexa53"
> >
> >   TUNEVALID[cortexa53] = "Enable Cortex-A53 specific processor optimizations"
> >   TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'cortexa53', ' -mcpu=cortex-a53', '', d)}"
> > +# See  https://github.com/llvm/llvm-project/issues/85699
> > +TUNE_CCARGS_MARCH_OPTS .= "${@bb.utils.contains('TUNE_FEATURES', 'crypto', '', '+nocrypto', d)}"
> >
> >   require conf/machine/include/arm/arch-armv8a.inc
> >
> > diff --git a/meta/conf/machine/include/arm/armv8a/tune-cortexa72.inc b/meta/conf/machine/include/arm/armv8a/tune-cortexa72.inc
> > index cbb6418c069..cf09db00664 100644
> > --- a/meta/conf/machine/include/arm/armv8a/tune-cortexa72.inc
> > +++ b/meta/conf/machine/include/arm/armv8a/tune-cortexa72.inc
> > @@ -2,6 +2,8 @@ DEFAULTTUNE ?= "cortexa72"
> >
> >   TUNEVALID[cortexa72] = "Enable Cortex-A72 specific processor optimizations"
> >   TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'cortexa72', ' -mcpu=cortex-a72', '', d)}"
> > +# See  https://github.com/llvm/llvm-project/issues/85699
> > +TUNE_CCARGS_MARCH_OPTS .= "${@bb.utils.contains('TUNE_FEATURES', 'crypto', '', '+nocrypto', d)}"
> >
>
> I'm very unfamiliar with the tuning part of the machine configuration
> files, but isn't that something we could add in
>
> meta/conf/machine/include/arm/feature-arm-crypto.inc
>
> in the counterpart of +crypto?
>
> I'm just a bit confused by the decision to only touch cortex a53 and a72
> tuning files? Reading the DTs and the commit log it seems those are the
> cores in the RPi 3 and 4 which do not have crypto, but I assume this
> would be applicable to any SoC without the crypto block anyway no? So
> should be generic instead?

Yes we could and I think its a better way as well. I was just focussed on rpi
but there might be more implementations of these cortex cpus without crypto
will send a v2.

Are you aware of the RPi5 having done the
> same cost-cutting measure? It's sporting a76 cores so maybe we need to
> do something with that too?

pi5 uses BCM2712 SoC which has Armv8 crypto extensions

>
> Cheers,
> Quentin
diff mbox series

Patch

diff --git a/meta/conf/machine/include/arm/armv8a/tune-cortexa53.inc b/meta/conf/machine/include/arm/armv8a/tune-cortexa53.inc
index a88575eb156..65fd6333198 100644
--- a/meta/conf/machine/include/arm/armv8a/tune-cortexa53.inc
+++ b/meta/conf/machine/include/arm/armv8a/tune-cortexa53.inc
@@ -2,6 +2,8 @@  DEFAULTTUNE ?= "cortexa53"
 
 TUNEVALID[cortexa53] = "Enable Cortex-A53 specific processor optimizations"
 TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'cortexa53', ' -mcpu=cortex-a53', '', d)}"
+# See  https://github.com/llvm/llvm-project/issues/85699
+TUNE_CCARGS_MARCH_OPTS .= "${@bb.utils.contains('TUNE_FEATURES', 'crypto', '', '+nocrypto', d)}"
 
 require conf/machine/include/arm/arch-armv8a.inc
 
diff --git a/meta/conf/machine/include/arm/armv8a/tune-cortexa72.inc b/meta/conf/machine/include/arm/armv8a/tune-cortexa72.inc
index cbb6418c069..cf09db00664 100644
--- a/meta/conf/machine/include/arm/armv8a/tune-cortexa72.inc
+++ b/meta/conf/machine/include/arm/armv8a/tune-cortexa72.inc
@@ -2,6 +2,8 @@  DEFAULTTUNE ?= "cortexa72"
 
 TUNEVALID[cortexa72] = "Enable Cortex-A72 specific processor optimizations"
 TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'cortexa72', ' -mcpu=cortex-a72', '', d)}"
+# See  https://github.com/llvm/llvm-project/issues/85699
+TUNE_CCARGS_MARCH_OPTS .= "${@bb.utils.contains('TUNE_FEATURES', 'crypto', '', '+nocrypto', d)}"
 
 require conf/machine/include/arm/arch-armv8a.inc