[PATCH] crypto: powerpc/poly1305 - add depends on BROKEN for now

[PATCH] crypto: powerpc/poly1305 - add depends on BROKEN for now

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

As discussed in the thread containing
https://lore.kernel.org/linux-crypto/20250510053308.GB505731@sol/, the
Power10-optimized Poly1305 code is currently not safe to call in softirq
context.  Disable it for now.  It can be re-enabled once it is fixed.

Fixes: ba8f8624fde2 ("crypto: poly1305-p10 - Glue code for optmized Poly1305 implementation for ppc64le")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 arch/powerpc/lib/crypto/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/lib/crypto/Kconfig b/arch/powerpc/lib/crypto/Kconfig
index ffa541ad6d5da..3f9e1bbd9905b 100644
--- a/arch/powerpc/lib/crypto/Kconfig
+++ b/arch/powerpc/lib/crypto/Kconfig
@@ -8,10 +8,11 @@ config CRYPTO_CHACHA20_P10
 	select CRYPTO_ARCH_HAVE_LIB_CHACHA
 
 config CRYPTO_POLY1305_P10
 	tristate
 	depends on PPC64 && CPU_LITTLE_ENDIAN && VSX
+	depends on BROKEN # Needs to be fixed to work in softirq context
 	default CRYPTO_LIB_POLY1305
 	select CRYPTO_ARCH_HAVE_LIB_POLY1305
 	select CRYPTO_LIB_POLY1305_GENERIC
 
 config CRYPTO_SHA256_PPC_SPE

base-commit: 57999ed153ed7e651afecbabe0e998e75cf2d798
-- 
2.49.0

Re: [PATCH] crypto: powerpc/poly1305 - add depends on BROKEN for now

[Herbert Xu]

Eric Biggers <ebiggers@kernel.org> wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> As discussed in the thread containing
> https://lore.kernel.org/linux-crypto/20250510053308.GB505731@sol/, the
> Power10-optimized Poly1305 code is currently not safe to call in softirq
> context.  Disable it for now.  It can be re-enabled once it is fixed.
> 
> Fixes: ba8f8624fde2 ("crypto: poly1305-p10 - Glue code for optmized Poly1305 implementation for ppc64le")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> arch/powerpc/lib/crypto/Kconfig | 1 +
> 1 file changed, 1 insertion(+)

I thought this fix should be enough, no?

https://patchwork.kernel.org/project/linux-crypto/patch/aB8Yy0JGvoErc0ns@gondor.apana.org.au/

Cheers,
-- 
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

Re: [PATCH] crypto: powerpc/poly1305 - add depends on BROKEN for now

[Eric Biggers]

On Wed, May 14, 2025 at 05:41:39PM +0800, Herbert Xu wrote:
> Eric Biggers <ebiggers@kernel.org> wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > As discussed in the thread containing
> > https://lore.kernel.org/linux-crypto/20250510053308.GB505731@sol/, the
> > Power10-optimized Poly1305 code is currently not safe to call in softirq
> > context.  Disable it for now.  It can be re-enabled once it is fixed.
> > 
> > Fixes: ba8f8624fde2 ("crypto: poly1305-p10 - Glue code for optmized Poly1305 implementation for ppc64le")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> > arch/powerpc/lib/crypto/Kconfig | 1 +
> > 1 file changed, 1 insertion(+)
> 
> I thought this fix should be enough, no?
> 
> https://patchwork.kernel.org/project/linux-crypto/patch/aB8Yy0JGvoErc0ns@gondor.apana.org.au/

I didn't notice that.  Probably, though I don't have time to review this subtle
Poly1305 code.  Especially with all the weird unions in the code.  Would be
great if the PowerPC folks would take a look.

- Eric

Re: [PATCH] crypto: powerpc/poly1305 - add depends on BROKEN for now

[Herbert Xu]

On Wed, May 14, 2025 at 09:29:33AM -0700, Eric Biggers wrote:
>
> I didn't notice that.  Probably, though I don't have time to review this subtle
> Poly1305 code.  Especially with all the weird unions in the code.  Would be
> great if the PowerPC folks would take a look.

Of course more reviews would be great and I think they're all on
the cc list.

I did test this by manually forcing the conversion, which is how
I discovered that powerpc wasn't even using donna64.

Cheers,
-- 
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

Re: [PATCH] crypto: powerpc/poly1305 - add depends on BROKEN for now

[Christophe Leroy]

Le 15/05/2025 à 04:10, Herbert Xu a écrit :
> On Wed, May 14, 2025 at 09:29:33AM -0700, Eric Biggers wrote:
>>
>> I didn't notice that.  Probably, though I don't have time to review this subtle
>> Poly1305 code.  Especially with all the weird unions in the code.  Would be
>> great if the PowerPC folks would take a look.
> 
> Of course more reviews would be great and I think they're all on
> the cc list.
> 
> I did test this by manually forcing the conversion, which is how
> I discovered that powerpc wasn't even using donna64.
> 

As far as I can see related patches found in linux-next tree were not 
sent to linuxppc-dev@lists.ozlabs.org.

Could you resend them, and split out the introduction of 
CONFIG_ARCH_SUPPORTS_INT128 from other parts of patch "crypto: 
powerpc/poly1305 - Add SIMD fallback" and add the lib/tishift.S in the 
patch which adds CONFIG_ARCH_SUPPORTS_INT128 ?

Thanks
Christophe

Re: [PATCH] crypto: powerpc/poly1305 - add depends on BROKEN for now

[Herbert Xu]

On Mon, May 19, 2025 at 03:55:16PM +0200, Christophe Leroy wrote:
> 
> As far as I can see related patches found in linux-next tree were not sent
> to linuxppc-dev@lists.ozlabs.org.

I just checked and it was definitely sent to linuxppc-dev:

Cc: oe-kbuild-all@lists.linux.dev, Linux Crypto Mailing List <linux-crypto@vger.kernel.org>, Venkat Rao Bagalkote <venkat88@linux.ibm.com>, Madhavan Srinivasan <maddy@linux.ibm.com>, Stephen Rothwell <sfr@canb.auug.org.au>, Danny Tsen <dtsen@linux.ibm.com>, linuxppc-dev@lists.ozlabs.org, Michael Ellerman <mpe@ellerman.id.au>

> Could you resend them, and split out the introduction of
> CONFIG_ARCH_SUPPORTS_INT128 from other parts of patch "crypto:
> powerpc/poly1305 - Add SIMD fallback" and add the lib/tishift.S in the patch
> which adds CONFIG_ARCH_SUPPORTS_INT128 ?

I'll just revert them and mark powerpc/poly1305 as broken.  You
guys can sort it out later.

Thanks,
-- 
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

Re: [PATCH] crypto: powerpc/poly1305 - add depends on BROKEN for now

[Herbert Xu]

On Wed, May 14, 2025 at 05:18:47AM +0000, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> As discussed in the thread containing
> https://lore.kernel.org/linux-crypto/20250510053308.GB505731@sol/, the
> Power10-optimized Poly1305 code is currently not safe to call in softirq
> context.  Disable it for now.  It can be re-enabled once it is fixed.
> 
> Fixes: ba8f8624fde2 ("crypto: poly1305-p10 - Glue code for optmized Poly1305 implementation for ppc64le")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  arch/powerpc/lib/crypto/Kconfig | 1 +
>  1 file changed, 1 insertion(+)

Patch applied.  Thanks.
-- 
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

Re: [PATCH] crypto: powerpc/poly1305 - add depends on BROKEN for now

[Christophe Leroy]

Le 20/05/2025 à 04:32, Herbert Xu a écrit :
> On Mon, May 19, 2025 at 03:55:16PM +0200, Christophe Leroy wrote:
>>
>> As far as I can see related patches found in linux-next tree were not sent
>> to linuxppc-dev@lists.ozlabs.org.
> 
> I just checked and it was definitely sent to linuxppc-dev:

Oops sorry, my mistake.

I see them in patchwork, they still have status 'New'.

> 
> Cc: oe-kbuild-all@lists.linux.dev, Linux Crypto Mailing List <linux-crypto@vger.kernel.org>, Venkat Rao Bagalkote <venkat88@linux.ibm.com>, Madhavan Srinivasan <maddy@linux.ibm.com>, Stephen Rothwell <sfr@canb.auug.org.au>, Danny Tsen <dtsen@linux.ibm.com>, linuxppc-dev@lists.ozlabs.org, Michael Ellerman <mpe@ellerman.id.au>
> 
>> Could you resend them, and split out the introduction of
>> CONFIG_ARCH_SUPPORTS_INT128 from other parts of patch "crypto:
>> powerpc/poly1305 - Add SIMD fallback" and add the lib/tishift.S in the patch
>> which adds CONFIG_ARCH_SUPPORTS_INT128 ?
> 
> I'll just revert them and mark powerpc/poly1305 as broken.  You
> guys can sort it out later.

Fine, lets do that and make sure all necessary bits are there when 
enabling CONFIG_ARCH_SUPPORTS_INT128

Thanks
Christophe