On Sat, May 10, 2025 at 05:34:01PM -0500, Segher Boessenkool wrote:

Hi!

On Fri, May 09, 2025 at 10:33:08PM -0700, Eric Biggers wrote:

quoted

On Sat, May 10, 2025 at 01:10:22PM +0800, Herbert Xu wrote:

quoted

On Fri, May 09, 2025 at 09:44:50PM -0700, Eric Biggers wrote:

quoted

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.

Yes, the PowerPC Poly1305 code incorrectly uses VSX without first checking
crypto_simd_usable().  And PowerPC also doesn't support VSX in softirqs, or at
least it doesn't claim to (it doesn't override may_use_simd(), so it gets the
default from include/asm-generic/simd.h which returns false in softirq context).
Maybe add 'depends on BROKEN' to CRYPTO_POLY1305_P10 for now, and give the
PowerPC folks (Cc'ed) a chance to fix this before removing the code.

What doe "may_use_simd" even *mean*?  At its declaration site it says
"whether it is allowable at this time to issue SIMD instructions or
access the SIMD register file", but that is 100% meaningless, you can do
SIMD in GPRs.

On PowerPC we have two separate register files dedicated to SIMD-like
stuff, the VMX and the VSX register files.  Which of those is this
function supposed to care about?

It looks like the whole "may_use_simd" thing is a misguided abstraction
unfortunately :-(

may_use_simd() a.k.a crypto_simd_usable() is supposed to check whether vector /
SIMD registers can be used in the current context, provided that the appropriate
architecture-specific functions like kernel_fpu_begin() and kernel_fpu_end() are
used.  In the case of architectures that support the use of multiple sets of
vector / SIMD registers in kernel mode, it would have to check for the
intersection of the calling context requirements for all of them, since it
doesn't specify a particular set.

The reason that may_use_simd() a.k.a. crypto_simd_usable() got pulled out into
an abstraction shared across all architectures is that it's used by
non-architecture-specific code, such as crypto/simd.c, and also the crypto
self-tests which inject 'false' return values to test the no-SIMD code paths.

I think the users other than the self-tests are on the way out, though.  Most of
the users of crypto/simd.c just got removed, with CRYPTO_AES_GCM_P10 being the
last one.  A new non-architecture-specific user of crypto_simd_usable() just got
added in include/crypto/internal/sha2.h for some reason (despite me nacking the
patch), but that should be reverted.

So if it's really the case that VMX and VSX are both supported for kernel-mode
use but have different requirements on the calling context, you could make the
PowerPC crypto code use more precise checks like may_use_vsx().  Just the crypto
self-tests won't be able to test the no-SIMD code paths that way, unfortunately.

- Eric