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

[Holger Dengler <dengler@linux.ibm.com>]

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