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

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

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);
> [...]