Re: [PATCH v2 03/20] s390/ap: Introduce ap message buffer pool

Holger Dengler
2025-03-17T16:14:23Z

On 04/03/2025 18:20, Harald Freudenberger wrote:
> There is a need for a do-not-allocate-memory path through the
> ap bus layer. When ap_init_apmsg() with use_mempool set to true
> is called, instead of kmalloc() the ap message buffer is
> allocated from the ap_msg_pool. This pool only holds a limited
> amount of buffers: ap_msg_pool_min_items with the item size
> AP_DEFAULT_MAX_MSG_SIZE and exactly one of these items (if available)
> is returned if ap_init_apmsg() with the use_mempool arg set to true
> is called. When this pool is exhausted and use_mempool is set true,
> ap_init_apmsg() returns -ENOMEM without any attempt to allocate
> memory.
>
> Default values for this mempool of ap messages is:
> * Each buffer is 12KB (that is the default AP bus size
> and all the urgend messages should fit into this space).

typo: urgent

> * Minimum items held in the pool is 8. This value is adjustable
> via module parameter ap.msgpool_min_items.
>
> The zcrypt layer may use this flag to indicate to the ap bus
> that the processing path for this message should not allocate
> memory but should use pre-allocated memory buffer instead.
> This is to prevent deadlocks with crypto and io for example
> with encrypted swap volumes.
>
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>

With the typo and the changed indent (see comment below)
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>

> ---
> drivers/s390/crypto/ap_bus.c | 57 ++++++++++++++++++++++++++++----
> drivers/s390/crypto/ap_bus.h | 9 ++---
> drivers/s390/crypto/zcrypt_api.c | 10 +++---
> 3 files changed, 60 insertions(+), 16 deletions(-)
>
[...]
> diff --git a/drivers/s390/crypto/ap_bus.h b/drivers/s390/crypto/ap_bus.h
> index 483231bcdea6..a7bd44e5cc76 100644
> --- a/drivers/s390/crypto/ap_bus.h
> +++ b/drivers/s390/crypto/ap_bus.h
> @@ -233,11 +233,12 @@ struct ap_message {
> struct ap_message *);
> };
>
> -#define AP_MSG_FLAG_SPECIAL 0x0001 /* flag msg as 'special' with NQAP */
> -#define AP_MSG_FLAG_USAGE 0x0002 /* CCA, EP11: usage (no admin) msg */
> -#define AP_MSG_FLAG_ADMIN 0x0004 /* CCA, EP11: admin (=control) msg */
> +#define AP_MSG_FLAG_SPECIAL 0x0001 /* flag msg as 'special' with NQAP */
> +#define AP_MSG_FLAG_USAGE 0x0002 /* CCA, EP11: usage (no admin) msg */
> +#define AP_MSG_FLAG_ADMIN 0x0004 /* CCA, EP11: admin (=control) msg */
> +#define AP_MSG_FLAG_MEMPOOL 0x0008 /* ap msg buffer allocated from mempool */

Please do not change the indent here. The change is not required, but it makes the life of reviewers much harder.

>
> -int ap_init_apmsg(struct ap_message *ap_msg);
> +int ap_init_apmsg(struct ap_message *ap_msg, bool use_mempool);
> void ap_release_apmsg(struct ap_message *ap_msg);
>
> enum ap_sm_wait ap_sm_event(struct ap_queue *aq, enum ap_sm_event event);
[...]

--
Mit freundlichen Grüßen / Kind regards
Holger Dengler
--
IBM Systems, Linux on IBM Z Development
dengler@linux.ibm.com