Re: [PATCH v2 06/20] s390/zcrypt: Introduce cprb mempool for ep11 misc functions

Harald Freudenberger
2025-03-25T08:36:22Z

On 2025-03-18 16:16, Holger Dengler wrote:
> On 04/03/2025 18:21, Harald Freudenberger wrote:
>> Introduce a cprb mempool for the zcrypt ep11 misc functions
>> (zcrypt_ep11misc.*) do some preparation rework to support
>> a do-not-allocate path through some zcrypt cca misc functions.
>>
>> The mempool is by default space for 8 cprbs with each 8KB.
>> For EP11 a CPRB either holds the request or the reply. So for
>> a request/reply there is always a couple of cprb buffers
>> needed. The minimal number of items in the mempool can get
>> adjusted via module parameter zcrypt.ep11_cprbpool_min_items
>> on module load.
>>
>> This is only part of an rework to support a new xflag
>> ZCRYPT_XFLAG_NOMEMALLOC but not yet complete.
>>
>> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
>
> With the minor change (see comment below)
> Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
>
>> ---
>> drivers/s390/crypto/zcrypt_api.c | 10 ++
>> drivers/s390/crypto/zcrypt_api.h | 1 +
>> drivers/s390/crypto/zcrypt_ep11misc.c | 158
>> ++++++++++++++++----------
>> drivers/s390/crypto/zcrypt_ep11misc.h | 1 +
>> 4 files changed, 109 insertions(+), 61 deletions(-)
>>
> [...]
>> diff --git a/drivers/s390/crypto/zcrypt_ep11misc.c
>> b/drivers/s390/crypto/zcrypt_ep11misc.c
>> index b60e262bcaa3..86578a95b140 100644
>> --- a/drivers/s390/crypto/zcrypt_ep11misc.c
>> +++ b/drivers/s390/crypto/zcrypt_ep11misc.c
> [...]
>> @@ -621,22 +652,20 @@ static int ep11_query_info(u16 cardnr, u16
>> domain, u32 query_type,
>> req_pl->query_subtype_len = sizeof(u32);
>>
>> /* reply cprb and payload */
>> - rep = alloc_cprb(sizeof(struct ep11_info_rep_pl) + buflen);
>> + rep = alloc_cprbmem(sizeof(struct ep11_info_rep_pl) + buflen,
>> xflags);
>> if (!rep)
>> goto out;
>> rep_pl = (struct ep11_info_rep_pl *)(((u8 *)rep) + sizeof(*rep));
>>
>> /* urb and target */
>> - urb = kmalloc(sizeof(*urb), GFP_KERNEL);
>> - if (!urb)
>> - goto out;
>> + memset(&urb, 0, sizeof(urb));
>
> The memset is done each time before a prep_urb() call. Please do the
> memset inside the prep_urb() function and remove the other ones.
>

done -> v3

>> target.ap_id = cardnr;
>> target.dom_id = domain;
>> - prep_urb(urb, &target, 1,
>> + prep_urb(&urb, &target, 1,
>> req, sizeof(*req) + sizeof(*req_pl),
>> rep, sizeof(*rep) + sizeof(*rep_pl) + buflen);
>>
>> - rc = zcrypt_send_ep11_cprb(urb, 0);
>> + rc = zcrypt_send_ep11_cprb(&urb, xflags);
>> if (rc) {
>> ZCRYPT_DBF_ERR("%s zcrypt_send_ep11_cprb(card=%d dom=%d) failed,
>> rc=%d\n",
>> __func__, (int)cardnr, (int)domain, rc);
> …