Re: [PATCH v3 07/21] s390/zcrypt: Rework zcrypt function zcrypt_device_status_mask_ext

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

On 27/03/2025 15:39, Harald Freudenberger wrote:
> Rework the existing function zcrypt_device_status_mask_ext():
> Add two new parameters to provide upper limits for
> cards and queues. The existing implementation needed an
> array of 256 * 256 * 4 = 256 KB which is really huge. The
> reworked function is more flexible in the sense that the
> caller can decide the upper limit for cards and domains to
> be stored into the status array. So for example a caller may
> decide to only query for cards 0...127 and queues 0...127
> and thus only an array of size 128 * 128 * 4 = 64 KB is needed.
> 
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>

Looks good now. With the minor change mentioned below
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>

> ---
>  drivers/s390/crypto/zcrypt_api.c      | 14 +++++++++++---
>  drivers/s390/crypto/zcrypt_api.h      |  3 ++-
>  drivers/s390/crypto/zcrypt_ccamisc.c  | 17 ++++++++++++-----
>  drivers/s390/crypto/zcrypt_ep11misc.c | 12 ++++++++----
>  4 files changed, 33 insertions(+), 13 deletions(-)
> 
[...]
> diff --git a/drivers/s390/crypto/zcrypt_ep11misc.c b/drivers/s390/crypto/zcrypt_ep11misc.c
> index 12efcf1b54fc..504064c27a8b 100644
> --- a/drivers/s390/crypto/zcrypt_ep11misc.c
> +++ b/drivers/s390/crypto/zcrypt_ep11misc.c
> @@ -1599,7 +1599,7 @@ int ep11_findcard2(u32 **apqns, u32 *nr_apqns, u16 cardnr, u16 domain,
>  {
>  	struct zcrypt_device_status_ext *device_status;
>  	u32 *_apqns = NULL, _nr_apqns = 0;
> -	int i, card, dom, rc = -ENOMEM;
> +	int i, card, dom, rc;

If you initialize rc with -ENOMEM here ....

>  	struct ep11_domain_info edi;
>  	struct ep11_card_info eci;
>  
> @@ -1609,13 +1609,16 @@ int ep11_findcard2(u32 **apqns, u32 *nr_apqns, u16 cardnr, u16 domain,
>  				 GFP_KERNEL);
>  	if (!device_status)
>  		return -ENOMEM;
> -	zcrypt_device_status_mask_ext(device_status);
> +
> +	zcrypt_device_status_mask_ext(device_status,
> +				      MAX_ZDEV_CARDIDS_EXT,
> +				      MAX_ZDEV_DOMAINS_EXT);
>  
>  	/* allocate 1k space for up to 256 apqns */
>  	_apqns = kmalloc_array(256, sizeof(u32), GFP_KERNEL);
>  	if (!_apqns) {
> -		kvfree(device_status);
> -		return -ENOMEM;
> +		rc = -ENOMEM;
> +		goto out;

... you do not need to set it here. Just jump to out.

if (!_apqns)
	goto out;

>  	}
>  
>  	/* walk through all the crypto apqnss */
[...]

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