| Message ID | 20250721165844.3092533-1-raj.khem@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] feature-arm-crypto: Add +nocrypto to -mcpu when crypto not in features | expand |
Hi Khem, On 7/21/25 6:58 PM, Khem Raj 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. The docs seems to be saying the opposite though? https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html#index-mcpu-2 """ Many of the supported CPUs implement optional architectural extensions. Where this is so the architectural extensions are normally enabled by default. If implementations that lack the extension exist, then the extension syntax can be used to disable those extensions that have been omitted. """ > 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 > Not entirely clear from the commit log why we apply this unconditionally to all arm cores now. I guess it could be as simple as adding """ armv8-[ar] and armv8.[1-6]a have +crypto and +nocrypto support, so let's explicitly disable crypto whenever it isn't requested. """ > [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> > Cc: Quentin Schulz <quentin.schulz@cherry.de> Looks good to me otherwise, so Acked-by: Quentin Schulz <quentin.schulz@cherry.de> Thanks! Quentin
diff --git a/meta/conf/machine/include/arm/feature-arm-crypto.inc b/meta/conf/machine/include/arm/feature-arm-crypto.inc index aade6ce08d5..8202de78e46 100644 --- a/meta/conf/machine/include/arm/feature-arm-crypto.inc +++ b/meta/conf/machine/include/arm/feature-arm-crypto.inc @@ -2,4 +2,5 @@ # armv8-a, armv8.1-a, armv8.3-a, armv8.4-a, armv8.5-a, armv8.6-a, and armv8-r TUNEVALID[crypto] = "Enable cryptographic instructions for ARMv8" -TUNE_CCARGS_MARCH_OPTS .= "${@bb.utils.contains('TUNE_FEATURES', 'crypto', '+crypto', '', d)}" +# See https://github.com/llvm/llvm-project/issues/85699 +TUNE_CCARGS_MARCH_OPTS .= "${@bb.utils.contains('TUNE_FEATURES', 'crypto', '+crypto', '+nocrypto', d)}"
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> Cc: Quentin Schulz <quentin.schulz@cherry.de> --- v2: Move the change to feature-arm-crypto.inc meta/conf/machine/include/arm/feature-arm-crypto.inc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)