Re: [PATCH v2 1/1] s390/uv: New param workpage for the uv_get_secret_metadata() function

[Heiko Carstens <hca@linux.ibm.com>]

On Mon, Mar 31, 2025 at 12:35:05PM +0200, Harald Freudenberger wrote:
> The pkey uv handler may be called in a do-not-allocate memory
> situation. For example when an encrypted swap file is used and the
> encryption is done via UV retrievable secrets with protected keys.

This doesn't answer the question if the context is process, bottom halve,
or interrupt context. If it is process context, is it sleepable?

> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index 9f05df2da2f7..0a8a6bc19c49 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c
> @@ -713,6 +713,9 @@ static int find_secret(const u8 secret_id[UV_SECRET_ID_LEN],
>   * uv_get_secret_metadata() - get secret metadata for a given secret id.
>   * @secret_id: search pattern.
>   * @secret: output data, containing the secret's metadata.
> + * @workpage: ephemeral working page. Caller may give a ptr to one page
> + *            here as ephemeral working buffer. If NULL, kmalloc is used
> + *            to alloc a working buffer.
>   *
>   * Search for a secret with the given secret_id in the Ultravisor secret store.
>   *
> @@ -725,16 +728,19 @@ static int find_secret(const u8 secret_id[UV_SECRET_ID_LEN],
>   * * %EIO:	- Other unexpected UV error.
>   */
>  int uv_get_secret_metadata(const u8 secret_id[UV_SECRET_ID_LEN],
> -			   struct uv_secret_list_item_hdr *secret)
> +			   struct uv_secret_list_item_hdr *secret,
> +			   u8 *workpage)
>  {
>  	struct uv_secret_list *buf;
>  	int rc;
>  
> -	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> +	buf = workpage ? (struct uv_secret_list *)workpage :
> +		kzalloc(sizeof(*buf), GFP_KERNEL);
>  	if (!buf)
>  		return -ENOMEM;
>  	rc = find_secret(secret_id, buf, secret);
> -	kfree(buf);
> +	if (!workpage)
> +		kfree(buf);
>  	return rc;
>  }
>  EXPORT_SYMBOL_GPL(uv_get_secret_metadata);

Please don't do this ugly workpage interface. Just rename find_secret() to
e.g. uv_find_secret() and make it globally visable. Then you have the
unchanged uv_get_secret_metadata() + find_secret() interfaces, and in addition
can call find_secret() / uv_find_secret() with a custom buffer.

Given that pkey_uv.c is the only user of this interface: how would the changes
to pkey_uv.c look like?  Or in other words: wouldn't it make more sense to get
rid of uv_get_secret_metadata(), and just keep uv_find_secret() and push the
allocation to the pkey code?