Re: [PATCH v12 0/6] New s390 specific protected key hmac

[Harald Freudenberger <freude@linux.ibm.com>]

On 2025-06-17 15:44, Harald Freudenberger wrote:
> Add support for protected key hmac ("phmac") for s390 arch.
> 
> With the latest machine generation there is now support for
> protected key (that is a key wrapped by a master key stored
> in firmware) hmac for sha2 (sha224, sha256, sha384 and sha512)
> for the s390 specific CPACF instruction kmac.
> 
> This patch adds support via 4 new hashes registered as
> phmac(sha224), phmac(sha256), phmac(sha384) and phmac(sha512).
> 
> Changelog:
> v1: Initial version
> v2: Increase HASH_MAX_DESCSIZE generic (not just for arch s390).
>     Fix one finding to use kmemdup instead of kmalloc/memcpy from test
>     robot. Remove unneeded cpacf subfunctions checks. Simplify
>     clone_tfm() function. Rebased to s390/features.
> v3: Feedback from Herbert: Use GFP_ATOMIC in setkey function.
>     Feedback from Holger: rework tfm clone function, move convert key
>     invocation from setkey to init function. Rebased to updated
>     s390/features from 11/7/2024. Ready for integration if there are
>     no complains on v3.
> v4: Rewind back more or less to v2. Add code to check for non-sleeping
>     context. Non-sleeping context during attempt to derive the
>     protected key from raw key material is not accepted and
>     -EOPNOTSUPP is returned (also currently all derivation pathes
>     would in fact never sleep). In general the phmac implementation is
>     not to be used within non-sleeping context and the code header
>     mentions this. Tested with (patched) dm-integrity - works fine.
> v5: As suggested by Herbert now the shashes have been marked as
>     'internal' and wrapped by ahashes which use the cryptd if an
>     atomic context is detected. So the visible phmac algorithms are
>     now ahashes. Unfortunately the dm-integrity implementation
>     currently requests and deals only with shashes and this phmac
>     implementation is not fitting to the original goal any more...
> v6: As suggested by Herbert now a pure async phmac implementation.
>     Tested via AF_ALG interface. Untested via dm-integrity as this 
> layer
>     only supports shashes. Maybe I'll develop a patch to switch the
>     dm-integrity to ahash as it is anyway the more flexible interface.
> v7: Total rework of the implementation. Now uses workqueues and 
> triggers
>     asynch requests for key convert, init, update, final and digest.
>     Tested with instrumented code and with a reworked version of
>     dm-integrity which uses asynchronous hashes. A patch for 
> dm-integrity
>     is on the way but yet needs some last hone work.
> v8: Added selftest. With the selftest comes some code which wraps the
>     clear key into a "clear key token" digestible by PKEY. The
>     selftest also uses import() and export(), so these are now also
>     implemented. Furthermore a finup() implementation is now also
>     available. Tested with AF_ALG testcases and dm-integrity, also
>     tested with some instrumented code to check that the asynch
>     workqueue functions do their job correctly. Coding is complete!
> v9: As suggested by Herbert use ahash_request_complete() and surround 
> it
>     with local_bh_disable().
> v10: Split the pkey selftest patch into 3 patches. Slight rework of the
>      setkey function as suggested by Holger: When selftest is running
>      as much as possible of the production code should run. So now the
>      key prep with selftest is one additional if/then block instead of
>      an if/then/else construct.
>      Code is ready for integration and well tested.
> v11: Utterly rework with the insights collected with the paes rework
>      and the basic work done with the pkey rework over the last 5 
> month.
>      Note that patch #1 effectively reverts commit 7fa481734016
>      ("crypto: ahash - make hash walk functions private to ahash.c")
>      from Eric Biggers.
> v12: Fixed some typos, adaptions to 128 bit total counter,
>      misc_register() invocation was missing in the patches series,
>      added Herbert's proposal for a new function crypto_ahash_tested().
> 
> Harald Freudenberger (5):
>   crypto: ahash - make hash walk functions from ahash.c  public
>   s390/crypto: New s390 specific protected key hash phmac
>   crypto: api - Add crypto_ahash_tested() helper function
>   s390/crypto: Add selftest support for phmac
>   crypto: testmgr - Enable phmac selftest
> 
> Holger Dengler (1):
>   s390/crypto: Add protected key hmac subfunctions for KMAC
> 
>  arch/s390/configs/debug_defconfig |    1 +
>  arch/s390/configs/defconfig       |    1 +
>  arch/s390/crypto/Makefile         |    1 +
>  arch/s390/crypto/phmac_s390.c     | 1048 +++++++++++++++++++++++++++++
>  arch/s390/include/asm/cpacf.h     |    4 +
>  crypto/ahash.c                    |   26 +-
>  crypto/testmgr.c                  |   30 +
>  drivers/crypto/Kconfig            |   13 +
>  include/crypto/internal/hash.h    |   30 +
>  9 files changed, 1133 insertions(+), 21 deletions(-)
>  create mode 100644 arch/s390/crypto/phmac_s390.c
> 
> 
> base-commit: 1029436218e50168812dbc44b16bca6d35721b0b
> --
> 2.43.0

Hi Herbert,

as the phmac implementation uses the newly introduced 
CRYPTO_ALG_NO_FALLBACK
flag, we can't deliver this patch series via s390. I talked with Heiko
about that and there are two options:
1) We (s390) pick your patch 4ccd065a69df ("crypto: ahash - Add support 
for
    drivers with no fallback") together with my patch series for the next
    kernel's merge window.
2) You (crypto) pick my patch series into your cryptodev-2.6 for next
    kernel's merge window.
I would prefer option 2 as most of the patches anyway deal with crypto
and Heiko and I do not expect unsolvable merge conflicts with the next
kernel's merge.
So what is your opinion?

Thanks,
Harald Freudenberger