* ERROR: alg: shash: ghash-neon test failed (wrong result) on test vector 1, cfg="init+update+final aligned buffer"
@ 2025-12-09 19:36 Diederik de Haas
2025-12-09 22:34 ` [PATCH] crypto: arm64/ghash - Fix incorrect output from ghash-neon Eric Biggers
0 siblings, 1 reply; 10+ messages in thread
From: Diederik de Haas @ 2025-12-09 19:36 UTC (permalink / raw)
To: linux-crypto
Hi,
With either 6.18-rc1 or -rc2 I saw the above error in dmesg:
https://paste.sr.ht/~diederik/54ed00a4cdcf5f9285674cf940d53a1ffd7ecc4f
and then the stack trace in the dmesg warnings.
This was on a (Rockchip based) PineTab2 (rk3566) on arm64.
Didn't have time to report it then and then it seems like the error was
gone with 6.18-rc5, so I assumed it was fixed*.
Then I started up an Raspberry Pi 3B+ (also arm64) with Debian's 6.17.9
kernel ... and I saw that error again. I also tried Debian's 6.17.2 and
my own 6.18(.0) kernel and they also showed that error.
So I guess there is an actual problem?
For completeness, the error and warnings (incl stack trace) on 6.17.9:
```
[ 21.438404] alg: shash: ghash-neon test failed (wrong result) on test vector 1, cfg="init+update+final aligned buffer"
[ 21.448906] alg: self-tests for ghash using ghash-neon failed (rc=-22)
[ 21.448927] ------------[ cut here ]------------
[ 21.460059] alg: self-tests for ghash using ghash-neon failed (rc=-22)
[ 21.460231] WARNING: CPU: 0 PID: 650 at crypto/testmgr.c:5827 alg_test+0x92c/0x950
[ 21.474266] Modules linked in: ghash_ce gf128mul gcm ccm algif_aead des_generic libdes rfcomm algif_skcipher bnep aes_neon_blk md4 algif_hash af_alg evdev vc4 snd_soc_hdmi_codec brcmfmac_wcc snd_soc_core btsdio nls_ascii snd_compress brcmfmac microchip nls_cp437 bcm2835_v4l2(C) snd_pcm_dmaengine drm_exec brcmutil hci_uart bcm2835_mmal_vchiq(C) drm_display_helper vfat btqca fat videobuf2_vmalloc lan78xx btrtl cfg80211 videobuf2_memops cec videobuf2_v4l2 phylink btintel rc_core of_mdio btbcm videodev drm_client_lib fixed_phy fwnode_mdio drm_dma_helper snd_bcm2835(C) libphy videobuf2_common snd_pcm bluetooth mdio_bus mc drm_kms_helper snd_timer snd cpufreq_dt soundcore ecdh_generic rfkill raspberrypi_cpufreq pwrseq_core ledtrig_default_on bcm2835_thermal vchiq(C) pwm_bcm2835 bcm2835_rng leds_gpio pkcs8_key_parser drm efi_pstore configfs nfnetlink autofs4 ext4 crc16 mbcache jbd2 crc32c_cryptoapi onboard_usb_dev dwc2 udc_core usbcore sdhci_iproc sdhci_pltfm sdhci bcm2835_wdt usb_common bcm2835 phy_generic i2c_bcm2835
[ 21.565523] CPU: 0 UID: 0 PID: 650 Comm: cryptomgr_test Tainted: G C 6.17.9+deb14-arm64 #1 PREEMPTLAZY Debian 6.17.9-1
[ 21.577700] Tainted: [C]=CRAP
[ 21.580655] Hardware name: Raspberry Pi 3 Model B Plus Rev 1.3 (DT)
[ 21.587014] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 21.594062] pc : alg_test+0x92c/0x950
[ 21.597746] lr : alg_test+0x92c/0x950
[ 21.601446] sp : ffff800081413d20
[ 21.604793] x29: ffff800081413dd0 x28: 0000000000000111 x27: 00000000ffffffea
[ 21.612030] x26: 0000000000000088 x25: ffffb3112dfa7d48 x24: 00000000ffffffff
[ 21.621241] x23: 0000000000000089 x22: 000000000104000e x21: ffff000006320e00
[ 21.629874] x20: ffffb3112f6d0000 x19: ffffb3112dfa9f48 x18: 0000000000000000
[ 21.638500] x17: 726f746365762074 x16: ffffb3112d52c1a8 x15: ffffb3112f1b45e0
[ 21.647115] x14: ffffb3112f0d7480 x13: 00000000000000c0 x12: 00000004ff201560
[ 21.655729] x11: 00000000000000c0 x10: 0000000000000d40 x9 : ffffb3112cf28078
[ 21.664327] x8 : ffff00000575dda0 x7 : 0000000000000004 x6 : 0000000000000000
[ 21.672936] x5 : 0000000000000000 x4 : ffff00000575d000 x3 : ffff000004ad2800
[ 21.681520] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00000575d000
[ 21.690116] Call trace:
[ 21.693925] alg_test+0x92c/0x950 (P)
[ 21.698952] cryptomgr_test+0x2c/0x58
[ 21.703972] kthread+0x148/0x240
[ 21.708535] ret_from_fork+0x10/0x20
[ 21.713440] ---[ end trace 0000000000000000 ]---
```
Hopefully this'll tell you where the (likely) problem is at.
Cheers,
Diederik
*) Reading my own comment from that paste ... I possibly switched to an
'rkbin' based U-Boot and that's why I didn't see it anymore
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] crypto: arm64/ghash - Fix incorrect output from ghash-neon
2025-12-09 19:36 ERROR: alg: shash: ghash-neon test failed (wrong result) on test vector 1, cfg="init+update+final aligned buffer" Diederik de Haas
@ 2025-12-09 22:34 ` Eric Biggers
2025-12-09 23:24 ` Herbert Xu
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Eric Biggers @ 2025-12-09 22:34 UTC (permalink / raw)
To: linux-crypto
Cc: linux-kernel, Ard Biesheuvel, Jason A . Donenfeld, Herbert Xu,
linux-arm-kernel, Eric Biggers, stable, Diederik de Haas
Commit 9a7c987fb92b ("crypto: arm64/ghash - Use API partial block
handling") made ghash_finup() pass the wrong buffer to
ghash_do_simd_update(). As a result, ghash-neon now produces incorrect
outputs when the message length isn't divisible by 16 bytes. Fix this.
(I didn't notice this earlier because this code is reached only on CPUs
that support NEON but not PMULL. I haven't yet found a way to get
qemu-system-aarch64 to emulate that configuration.)
Fixes: 9a7c987fb92b ("crypto: arm64/ghash - Use API partial block handling")
Cc: stable@vger.kernel.org
Reported-by: Diederik de Haas <diederik@cknow-tech.com>
Closes: https://lore.kernel.org/linux-crypto/DETXT7QI62KE.F3CGH2VWX1SC@cknow-tech.com/
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
If it's okay, I'd like to just take this via libcrypto-fixes.
arch/arm64/crypto/ghash-ce-glue.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/crypto/ghash-ce-glue.c b/arch/arm64/crypto/ghash-ce-glue.c
index 7951557a285a..ef249d06c92c 100644
--- a/arch/arm64/crypto/ghash-ce-glue.c
+++ b/arch/arm64/crypto/ghash-ce-glue.c
@@ -131,11 +131,11 @@ static int ghash_finup(struct shash_desc *desc, const u8 *src,
if (len) {
u8 buf[GHASH_BLOCK_SIZE] = {};
memcpy(buf, src, len);
- ghash_do_simd_update(1, ctx->digest, src, key, NULL,
+ ghash_do_simd_update(1, ctx->digest, buf, key, NULL,
pmull_ghash_update_p8);
memzero_explicit(buf, sizeof(buf));
}
return ghash_export(desc, dst);
}
base-commit: 7a3984bbd69055898add0fe22445f99435f33450
--
2.52.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] crypto: arm64/ghash - Fix incorrect output from ghash-neon
2025-12-09 22:34 ` [PATCH] crypto: arm64/ghash - Fix incorrect output from ghash-neon Eric Biggers
@ 2025-12-09 23:24 ` Herbert Xu
2025-12-10 0:30 ` Eric Biggers
2025-12-10 9:22 ` Diederik de Haas
2 siblings, 0 replies; 10+ messages in thread
From: Herbert Xu @ 2025-12-09 23:24 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-crypto, linux-kernel, Ard Biesheuvel, Jason A . Donenfeld,
linux-arm-kernel, stable, Diederik de Haas
On Tue, Dec 09, 2025 at 02:34:17PM -0800, Eric Biggers wrote:
> Commit 9a7c987fb92b ("crypto: arm64/ghash - Use API partial block
> handling") made ghash_finup() pass the wrong buffer to
> ghash_do_simd_update(). As a result, ghash-neon now produces incorrect
> outputs when the message length isn't divisible by 16 bytes. Fix this.
>
> (I didn't notice this earlier because this code is reached only on CPUs
> that support NEON but not PMULL. I haven't yet found a way to get
> qemu-system-aarch64 to emulate that configuration.)
>
> Fixes: 9a7c987fb92b ("crypto: arm64/ghash - Use API partial block handling")
> Cc: stable@vger.kernel.org
> Reported-by: Diederik de Haas <diederik@cknow-tech.com>
> Closes: https://lore.kernel.org/linux-crypto/DETXT7QI62KE.F3CGH2VWX1SC@cknow-tech.com/
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
>
> If it's okay, I'd like to just take this via libcrypto-fixes.
>
> arch/arm64/crypto/ghash-ce-glue.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks for catching this!
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] crypto: arm64/ghash - Fix incorrect output from ghash-neon
2025-12-09 22:34 ` [PATCH] crypto: arm64/ghash - Fix incorrect output from ghash-neon Eric Biggers
2025-12-09 23:24 ` Herbert Xu
@ 2025-12-10 0:30 ` Eric Biggers
2025-12-10 9:22 ` Diederik de Haas
2 siblings, 0 replies; 10+ messages in thread
From: Eric Biggers @ 2025-12-10 0:30 UTC (permalink / raw)
To: linux-crypto
Cc: linux-kernel, Ard Biesheuvel, Jason A . Donenfeld, Herbert Xu,
linux-arm-kernel, stable, Diederik de Haas
On Tue, Dec 09, 2025 at 02:34:17PM -0800, Eric Biggers wrote:
> Commit 9a7c987fb92b ("crypto: arm64/ghash - Use API partial block
> handling") made ghash_finup() pass the wrong buffer to
> ghash_do_simd_update(). As a result, ghash-neon now produces incorrect
> outputs when the message length isn't divisible by 16 bytes. Fix this.
>
> (I didn't notice this earlier because this code is reached only on CPUs
> that support NEON but not PMULL. I haven't yet found a way to get
> qemu-system-aarch64 to emulate that configuration.)
>
> Fixes: 9a7c987fb92b ("crypto: arm64/ghash - Use API partial block handling")
> Cc: stable@vger.kernel.org
> Reported-by: Diederik de Haas <diederik@cknow-tech.com>
> Closes: https://lore.kernel.org/linux-crypto/DETXT7QI62KE.F3CGH2VWX1SC@cknow-tech.com/
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
>
> If it's okay, I'd like to just take this via libcrypto-fixes.
>
> arch/arm64/crypto/ghash-ce-glue.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=libcrypto-fixes
(As always, additional reviews/acks still appreciated!)
- Eric
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] crypto: arm64/ghash - Fix incorrect output from ghash-neon
2025-12-09 22:34 ` [PATCH] crypto: arm64/ghash - Fix incorrect output from ghash-neon Eric Biggers
2025-12-09 23:24 ` Herbert Xu
2025-12-10 0:30 ` Eric Biggers
@ 2025-12-10 9:22 ` Diederik de Haas
2025-12-10 9:31 ` Ard Biesheuvel
2 siblings, 1 reply; 10+ messages in thread
From: Diederik de Haas @ 2025-12-10 9:22 UTC (permalink / raw)
To: Eric Biggers, linux-crypto
Cc: linux-kernel, Ard Biesheuvel, Jason A . Donenfeld, Herbert Xu,
linux-arm-kernel, stable, Diederik de Haas
On Tue Dec 9, 2025 at 11:34 PM CET, Eric Biggers wrote:
> Commit 9a7c987fb92b ("crypto: arm64/ghash - Use API partial block
> handling") made ghash_finup() pass the wrong buffer to
> ghash_do_simd_update(). As a result, ghash-neon now produces incorrect
> outputs when the message length isn't divisible by 16 bytes. Fix this.
I was hoping to not have to do a 'git bisect', but this is much better
:-D I can confirm that this patch fixes the error I was seeing, so
Tested-by: Diederik de Haas <diederik@cknow-tech.com>
> (I didn't notice this earlier because this code is reached only on CPUs
> that support NEON but not PMULL. I haven't yet found a way to get
> qemu-system-aarch64 to emulate that configuration.)
https://www.qemu.org/docs/master/system/arm/raspi.html indicates it can
emulate various Raspberry Pi models. I've only tested it with RPi 3B+
(bc of its wifi+bt chip), but I wouldn't be surprised if all RPi models
would have this problem? Dunno if QEMU emulates that though.
Thanks for the quick fix!
Cheers,
Diederik
> Fixes: 9a7c987fb92b ("crypto: arm64/ghash - Use API partial block handling")
> Cc: stable@vger.kernel.org
> Reported-by: Diederik de Haas <diederik@cknow-tech.com>
> Closes: https://lore.kernel.org/linux-crypto/DETXT7QI62KE.F3CGH2VWX1SC@cknow-tech.com/
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
>
> If it's okay, I'd like to just take this via libcrypto-fixes.
>
> arch/arm64/crypto/ghash-ce-glue.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/crypto/ghash-ce-glue.c b/arch/arm64/crypto/ghash-ce-glue.c
> index 7951557a285a..ef249d06c92c 100644
> --- a/arch/arm64/crypto/ghash-ce-glue.c
> +++ b/arch/arm64/crypto/ghash-ce-glue.c
> @@ -131,11 +131,11 @@ static int ghash_finup(struct shash_desc *desc, const u8 *src,
>
> if (len) {
> u8 buf[GHASH_BLOCK_SIZE] = {};
>
> memcpy(buf, src, len);
> - ghash_do_simd_update(1, ctx->digest, src, key, NULL,
> + ghash_do_simd_update(1, ctx->digest, buf, key, NULL,
> pmull_ghash_update_p8);
> memzero_explicit(buf, sizeof(buf));
> }
> return ghash_export(desc, dst);
> }
>
> base-commit: 7a3984bbd69055898add0fe22445f99435f33450
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] crypto: arm64/ghash - Fix incorrect output from ghash-neon
2025-12-10 9:22 ` Diederik de Haas
@ 2025-12-10 9:31 ` Ard Biesheuvel
2025-12-12 5:40 ` Eric Biggers
0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2025-12-10 9:31 UTC (permalink / raw)
To: Diederik de Haas
Cc: Eric Biggers, linux-crypto, linux-kernel, Jason A . Donenfeld,
Herbert Xu, linux-arm-kernel, stable
On Wed, 10 Dec 2025 at 18:22, Diederik de Haas <diederik@cknow-tech.com> wrote:
>
> On Tue Dec 9, 2025 at 11:34 PM CET, Eric Biggers wrote:
> > Commit 9a7c987fb92b ("crypto: arm64/ghash - Use API partial block
> > handling") made ghash_finup() pass the wrong buffer to
> > ghash_do_simd_update(). As a result, ghash-neon now produces incorrect
> > outputs when the message length isn't divisible by 16 bytes. Fix this.
>
> I was hoping to not have to do a 'git bisect', but this is much better
> :-D I can confirm that this patch fixes the error I was seeing, so
>
> Tested-by: Diederik de Haas <diederik@cknow-tech.com>
>
> > (I didn't notice this earlier because this code is reached only on CPUs
> > that support NEON but not PMULL. I haven't yet found a way to get
> > qemu-system-aarch64 to emulate that configuration.)
>
> https://www.qemu.org/docs/master/system/arm/raspi.html indicates it can
> emulate various Raspberry Pi models. I've only tested it with RPi 3B+
> (bc of its wifi+bt chip), but I wouldn't be surprised if all RPi models
> would have this problem? Dunno if QEMU emulates that though.
>
All 64-bit RPi models except the RPi5 are affected by this, as those
do not implement the crypto extensions. So I would expect QEMU to do
the same.
It would be nice, though, if we could emulate this on the mach-virt
machine model too. It should be fairly trivial to do, so if there is
demand for this I can look into it.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] crypto: arm64/ghash - Fix incorrect output from ghash-neon
2025-12-10 9:31 ` Ard Biesheuvel
@ 2025-12-12 5:40 ` Eric Biggers
2025-12-15 7:54 ` Ard Biesheuvel
0 siblings, 1 reply; 10+ messages in thread
From: Eric Biggers @ 2025-12-12 5:40 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Diederik de Haas, linux-crypto, linux-kernel, Jason A . Donenfeld,
Herbert Xu, linux-arm-kernel, stable
On Wed, Dec 10, 2025 at 06:31:44PM +0900, Ard Biesheuvel wrote:
> On Wed, 10 Dec 2025 at 18:22, Diederik de Haas <diederik@cknow-tech.com> wrote:
> >
> > On Tue Dec 9, 2025 at 11:34 PM CET, Eric Biggers wrote:
> > > Commit 9a7c987fb92b ("crypto: arm64/ghash - Use API partial block
> > > handling") made ghash_finup() pass the wrong buffer to
> > > ghash_do_simd_update(). As a result, ghash-neon now produces incorrect
> > > outputs when the message length isn't divisible by 16 bytes. Fix this.
> >
> > I was hoping to not have to do a 'git bisect', but this is much better
> > :-D I can confirm that this patch fixes the error I was seeing, so
> >
> > Tested-by: Diederik de Haas <diederik@cknow-tech.com>
> >
> > > (I didn't notice this earlier because this code is reached only on CPUs
> > > that support NEON but not PMULL. I haven't yet found a way to get
> > > qemu-system-aarch64 to emulate that configuration.)
> >
> > https://www.qemu.org/docs/master/system/arm/raspi.html indicates it can
> > emulate various Raspberry Pi models. I've only tested it with RPi 3B+
> > (bc of its wifi+bt chip), but I wouldn't be surprised if all RPi models
> > would have this problem? Dunno if QEMU emulates that though.
> >
>
> All 64-bit RPi models except the RPi5 are affected by this, as those
> do not implement the crypto extensions. So I would expect QEMU to do
> the same.
>
> It would be nice, though, if we could emulate this on the mach-virt
> machine model too. It should be fairly trivial to do, so if there is
> demand for this I can look into it.
I'm definitely interested in it. I'm already testing multiple "-cpu"
options, and it's easy to add more.
With qemu-system-aarch64 I'm currently only using "-M virt", since the
other machine models I've tried don't boot with arm64 defconfig,
including "-M raspi3b" and "-M raspi4b".
There may be some tricks I'm missing. Regardless, expanding the
selection of available CPUs for "-M virt" would be helpful. Either by
adding "real" CPUs that have "interesting" combinations of features, or
by just allowing turning features off like
"-cpu max,aes=off,pmull=off,sha256=off". (Certain features like sve can
already be turned off in that way, but not the ones relevant to us.)
- Eric
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] crypto: arm64/ghash - Fix incorrect output from ghash-neon
2025-12-12 5:40 ` Eric Biggers
@ 2025-12-15 7:54 ` Ard Biesheuvel
2025-12-15 20:16 ` Eric Biggers
0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2025-12-15 7:54 UTC (permalink / raw)
To: Eric Biggers
Cc: Diederik de Haas, linux-crypto, linux-kernel, Jason A . Donenfeld,
Herbert Xu, linux-arm-kernel, stable
On Fri, 12 Dec 2025 at 06:40, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Wed, Dec 10, 2025 at 06:31:44PM +0900, Ard Biesheuvel wrote:
> > On Wed, 10 Dec 2025 at 18:22, Diederik de Haas <diederik@cknow-tech.com> wrote:
> > >
> > > On Tue Dec 9, 2025 at 11:34 PM CET, Eric Biggers wrote:
> > > > Commit 9a7c987fb92b ("crypto: arm64/ghash - Use API partial block
> > > > handling") made ghash_finup() pass the wrong buffer to
> > > > ghash_do_simd_update(). As a result, ghash-neon now produces incorrect
> > > > outputs when the message length isn't divisible by 16 bytes. Fix this.
> > >
> > > I was hoping to not have to do a 'git bisect', but this is much better
> > > :-D I can confirm that this patch fixes the error I was seeing, so
> > >
> > > Tested-by: Diederik de Haas <diederik@cknow-tech.com>
> > >
> > > > (I didn't notice this earlier because this code is reached only on CPUs
> > > > that support NEON but not PMULL. I haven't yet found a way to get
> > > > qemu-system-aarch64 to emulate that configuration.)
> > >
> > > https://www.qemu.org/docs/master/system/arm/raspi.html indicates it can
> > > emulate various Raspberry Pi models. I've only tested it with RPi 3B+
> > > (bc of its wifi+bt chip), but I wouldn't be surprised if all RPi models
> > > would have this problem? Dunno if QEMU emulates that though.
> > >
> >
> > All 64-bit RPi models except the RPi5 are affected by this, as those
> > do not implement the crypto extensions. So I would expect QEMU to do
> > the same.
> >
> > It would be nice, though, if we could emulate this on the mach-virt
> > machine model too. It should be fairly trivial to do, so if there is
> > demand for this I can look into it.
>
> I'm definitely interested in it. I'm already testing multiple "-cpu"
> options, and it's easy to add more.
>
> With qemu-system-aarch64 I'm currently only using "-M virt", since the
> other machine models I've tried don't boot with arm64 defconfig,
> including "-M raspi3b" and "-M raspi4b".
>
> There may be some tricks I'm missing. Regardless, expanding the
> selection of available CPUs for "-M virt" would be helpful. Either by
> adding "real" CPUs that have "interesting" combinations of features, or
> by just allowing turning features off like
> "-cpu max,aes=off,pmull=off,sha256=off". (Certain features like sve can
> already be turned off in that way, but not the ones relevant to us.)
>
There are some architectural rules around which combinations of crypto
extensions are permitted:
- PMULL implies AES, and there is no way for the ID registers to
describe a CPU that has PMULL but not AES
- SHA256 implies SHA1 (but the ID register fields are independent)
- SHA3 and SHA512 both imply SHA256+SHA1
- SVE versions are not allowed to be implemented unless the plain NEON
version is implemented as well
- FEAT_Crypto has different meanings for v8.0, v8.2 and v9.x
So it would be much easier, also in terms of future maintenance, to
have a simple 'crypto=off' setting that applies to all emulated CPU
models, given that disabling all crypto on any given compliant CPU
will never result in something that the architecture does not permit.
Would that work for you?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] crypto: arm64/ghash - Fix incorrect output from ghash-neon
2025-12-15 7:54 ` Ard Biesheuvel
@ 2025-12-15 20:16 ` Eric Biggers
2025-12-16 8:15 ` Ard Biesheuvel
0 siblings, 1 reply; 10+ messages in thread
From: Eric Biggers @ 2025-12-15 20:16 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Diederik de Haas, linux-crypto, linux-kernel, Jason A . Donenfeld,
Herbert Xu, linux-arm-kernel, stable
On Mon, Dec 15, 2025 at 04:54:34PM +0900, Ard Biesheuvel wrote:
> > > All 64-bit RPi models except the RPi5 are affected by this, as those
> > > do not implement the crypto extensions. So I would expect QEMU to do
> > > the same.
> > >
> > > It would be nice, though, if we could emulate this on the mach-virt
> > > machine model too. It should be fairly trivial to do, so if there is
> > > demand for this I can look into it.
> >
> > I'm definitely interested in it. I'm already testing multiple "-cpu"
> > options, and it's easy to add more.
> >
> > With qemu-system-aarch64 I'm currently only using "-M virt", since the
> > other machine models I've tried don't boot with arm64 defconfig,
> > including "-M raspi3b" and "-M raspi4b".
> >
> > There may be some tricks I'm missing. Regardless, expanding the
> > selection of available CPUs for "-M virt" would be helpful. Either by
> > adding "real" CPUs that have "interesting" combinations of features, or
> > by just allowing turning features off like
> > "-cpu max,aes=off,pmull=off,sha256=off". (Certain features like sve can
> > already be turned off in that way, but not the ones relevant to us.)
> >
>
> There are some architectural rules around which combinations of crypto
> extensions are permitted:
> - PMULL implies AES, and there is no way for the ID registers to
> describe a CPU that has PMULL but not AES
> - SHA256 implies SHA1 (but the ID register fields are independent)
> - SHA3 and SHA512 both imply SHA256+SHA1
> - SVE versions are not allowed to be implemented unless the plain NEON
> version is implemented as well
> - FEAT_Crypto has different meanings for v8.0, v8.2 and v9.x
>
> So it would be much easier, also in terms of future maintenance, to
> have a simple 'crypto=off' setting that applies to all emulated CPU
> models, given that disabling all crypto on any given compliant CPU
> will never result in something that the architecture does not permit.
>
> Would that work for you?
I thought it had been established that the "crypto" grouping of features
(as implemented by gcc and clang) doesn't reflect the actual hardware
feature fields and is misleading because additional crypto extensions
continue to be added.
I'm not sure that applies here, but just something to consider.
There's certainly no need to support emulating combinations of features
that no hardware actually implements. So yes, if that means "crypto" is
the right choice, that sounds fine.
- Eric
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] crypto: arm64/ghash - Fix incorrect output from ghash-neon
2025-12-15 20:16 ` Eric Biggers
@ 2025-12-16 8:15 ` Ard Biesheuvel
0 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2025-12-16 8:15 UTC (permalink / raw)
To: Eric Biggers
Cc: Diederik de Haas, linux-crypto, linux-kernel, Jason A . Donenfeld,
Herbert Xu, linux-arm-kernel, stable
On Mon, 15 Dec 2025 at 21:16, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Mon, Dec 15, 2025 at 04:54:34PM +0900, Ard Biesheuvel wrote:
> > > > All 64-bit RPi models except the RPi5 are affected by this, as those
> > > > do not implement the crypto extensions. So I would expect QEMU to do
> > > > the same.
> > > >
> > > > It would be nice, though, if we could emulate this on the mach-virt
> > > > machine model too. It should be fairly trivial to do, so if there is
> > > > demand for this I can look into it.
> > >
> > > I'm definitely interested in it. I'm already testing multiple "-cpu"
> > > options, and it's easy to add more.
> > >
> > > With qemu-system-aarch64 I'm currently only using "-M virt", since the
> > > other machine models I've tried don't boot with arm64 defconfig,
> > > including "-M raspi3b" and "-M raspi4b".
> > >
> > > There may be some tricks I'm missing. Regardless, expanding the
> > > selection of available CPUs for "-M virt" would be helpful. Either by
> > > adding "real" CPUs that have "interesting" combinations of features, or
> > > by just allowing turning features off like
> > > "-cpu max,aes=off,pmull=off,sha256=off". (Certain features like sve can
> > > already be turned off in that way, but not the ones relevant to us.)
> > >
> >
> > There are some architectural rules around which combinations of crypto
> > extensions are permitted:
> > - PMULL implies AES, and there is no way for the ID registers to
> > describe a CPU that has PMULL but not AES
> > - SHA256 implies SHA1 (but the ID register fields are independent)
> > - SHA3 and SHA512 both imply SHA256+SHA1
> > - SVE versions are not allowed to be implemented unless the plain NEON
> > version is implemented as well
> > - FEAT_Crypto has different meanings for v8.0, v8.2 and v9.x
> >
> > So it would be much easier, also in terms of future maintenance, to
> > have a simple 'crypto=off' setting that applies to all emulated CPU
> > models, given that disabling all crypto on any given compliant CPU
> > will never result in something that the architecture does not permit.
> >
> > Would that work for you?
>
> I thought it had been established that the "crypto" grouping of features
> (as implemented by gcc and clang) doesn't reflect the actual hardware
> feature fields and is misleading because additional crypto extensions
> continue to be added.
>
> I'm not sure that applies here, but just something to consider.
>
You are right, this is why 'crypto=on' can never mean anything other
than 'do not disable the crypto extensions that this particular CPU
type provides' But that does not mean 'crypto=off' is equally
problematic.
> There's certainly no need to support emulating combinations of features
> that no hardware actually implements. So yes, if that means "crypto" is
> the right choice, that sounds fine.
>
OK, I'll have a stab at that and cc you on the patches.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-12-16 8:15 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-09 19:36 ERROR: alg: shash: ghash-neon test failed (wrong result) on test vector 1, cfg="init+update+final aligned buffer" Diederik de Haas
2025-12-09 22:34 ` [PATCH] crypto: arm64/ghash - Fix incorrect output from ghash-neon Eric Biggers
2025-12-09 23:24 ` Herbert Xu
2025-12-10 0:30 ` Eric Biggers
2025-12-10 9:22 ` Diederik de Haas
2025-12-10 9:31 ` Ard Biesheuvel
2025-12-12 5:40 ` Eric Biggers
2025-12-15 7:54 ` Ard Biesheuvel
2025-12-15 20:16 ` Eric Biggers
2025-12-16 8:15 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).