Re: [v2 PATCH] crypto: powerpc/poly1305 - Add poly1305_emit_arch wrapper

Venkat Rao Bagalkote
2025-05-12T05:14:19Z

On 10/05/25 10:40 am, Herbert Xu wrote:
> On Fri, May 09, 2025 at 09:44:50PM -0700, Eric Biggers wrote:
>> This fixes "-cpu Power10", but older CPUs (e.g. "-cpu POWER9") are still
>> failing.
> You're right. I'll revert this and apply the following patch
> instead.
>
> BTW this thing is still hopelessly broken if it's called from
> softirq context because there is no SIMD fallback. Yes I removed
> the SIMD check but it was already broken before that as it simply
> switched from the 4-block version to the 1-block version if SIMD
> is not available rather than actually doing something that is
> safe in softirq context.
>
> Perhaps we should just remove this altogether until it's fixed.
>
> ---8<---
> Add poly1305_emit_arch with fallback instead of calling assembly
> directly. This is because the state format differs between p10
> and that of the generic implementation.
>
> Reported-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
> Reported-by: Eric Biggers <ebiggers@google.com>
> Fixes: 14d31979145d ("crypto: powerpc/poly1305 - Add block-only interface")
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/arch/powerpc/lib/crypto/poly1305-p10-glue.c b/arch/powerpc/lib/crypto/poly1305-p10-glue.c
> index 7cea0ebcc6bc..3f1664a724b6 100644
> --- a/arch/powerpc/lib/crypto/poly1305-p10-glue.c
> +++ b/arch/powerpc/lib/crypto/poly1305-p10-glue.c
> @@ -14,10 +14,7 @@
>
> asmlinkage void poly1305_p10le_4blocks(struct poly1305_block_state *state, const u8 *m, u32 mlen);
> asmlinkage void poly1305_64s(struct poly1305_block_state *state, const u8 *m, u32 mlen, int highbit);
> -asmlinkage void poly1305_emit_arch(const struct poly1305_state *state,
> - u8 digest[POLY1305_DIGEST_SIZE],
> - const u32 nonce[4]);
> -EXPORT_SYMBOL_GPL(poly1305_emit_arch);
> +asmlinkage void poly1305_emit_64(const struct poly1305_state *state, const u32 nonce[4], u8 digest[POLY1305_DIGEST_SIZE]);
>
> static __ro_after_init DEFINE_STATIC_KEY_FALSE(have_p10);
>
> @@ -65,6 +62,16 @@ void poly1305_blocks_arch(struct poly1305_block_state *state, const u8 *src,
> }
> EXPORT_SYMBOL_GPL(poly1305_blocks_arch);
>
> +void poly1305_emit_arch(const struct poly1305_state *state,
> + u8 digest[POLY1305_DIGEST_SIZE],
> + const u32 nonce[4])
> +{
> + if (!static_key_enabled(&have_p10))
> + return poly1305_emit_generic(state, digest, nonce);
> + poly1305_emit_64(state, nonce, digest);
> +}
> +EXPORT_SYMBOL_GPL(poly1305_emit_arch);
> +
> bool poly1305_is_arch_optimized(void)
> {
> return static_key_enabled(&have_p10);
> diff --git a/arch/powerpc/lib/crypto/poly1305-p10le_64.S b/arch/powerpc/lib/crypto/poly1305-p10le_64.S
> index 2ba2911b8038..a3c1987f1ecd 100644
> --- a/arch/powerpc/lib/crypto/poly1305-p10le_64.S
> +++ b/arch/powerpc/lib/crypto/poly1305-p10le_64.S
> @@ -1030,7 +1030,7 @@ SYM_FUNC_END(poly1305_64s)
> # Input: r3 = h, r4 = s, r5 = mac
> # mac = h + s
> #
> -SYM_FUNC_START(poly1305_emit_arch)
> +SYM_FUNC_START(poly1305_emit_64)
> ld 10, 0(3)
> ld 11, 8(3)
> ld 12, 16(3)
> @@ -1060,7 +1060,7 @@ Skip_h64:
> std 10, 0(5)
> std 11, 8(5)
> blr
> -SYM_FUNC_END(poly1305_emit_arch)
> +SYM_FUNC_END(poly1305_emit_64)
>
> SYM_DATA_START_LOCAL(RMASK)
> .align 5

Tested this patch, by applying on top of next-20250508 on IBM Power9
system and it fixes the reported boot warnings. Hence,

Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>

Regards,

Venkat.