Re: [PATCH v1 2/6] s390/uv: Retrieve UV secrets support

[Christoph Schlameuss]

On Mon Sep 30, 2024 at 3:19 PM CEST, Steffen Eiden wrote:
> Provide a kernel API to retrieve secrets from the UV secret store.
> Add two new functions:
> * `uv_get_secret_metadata` - get metadata for a given secret identifier
> * `uv_retrieve_secret` - get the secret value for the secret index
>
> With those two functions one can extract the secret for a given secret
> id, if the secret is retrievable.
>
> Signed-off-by: Steffen Eiden <seiden@xxxxxxxxxxxxx>
> ---
>  arch/s390/include/asm/uv.h | 131 ++++++++++++++++++++++++++++++++++++-
>  arch/s390/kernel/uv.c      | 124 +++++++++++++++++++++++++++++++++++
>  2 files changed, 254 insertions(+), 1 deletion(-)

[...]

>  /* Bits in installed uv calls */
>  enum uv_cmds_inst {
> @@ -95,6 +96,7 @@ enum uv_cmds_inst {
>  	BIT_UVC_CMD_ADD_SECRET = 29,
>  	BIT_UVC_CMD_LIST_SECRETS = 30,
>  	BIT_UVC_CMD_LOCK_SECRETS = 31,

Is 32 skipped intentionally? Should there be a comment here that it is reserved?

> +	BIT_UVC_CMD_RETR_SECRETS = 33,
>  };

[...]

> +/**
> + * uv_secret_list - UV secret-metadata list
> + * @num_secr_stored: Number of secrets stored in this list
> + * @total_num_secrets: Number of secrets stored in the UV for this guest
> + * @next_valid_idx: positive number if there are more secrets available or zero

s/next_valid_idx/next_secret_idx/

> + * @secrets: Up to 85 UV-secret metadata entries.
> + */
> +struct uv_secret_list {
> +	u16 num_secr_stored;
> +	u16 total_num_secrets;
> +	u16 next_secret_idx;
> +	u16 reserved_06;
> +	u64 reserved_08;
> +	struct uv_secret_list_item secrets[85];
> +} __packed __aligned(8);
> +static_assert(sizeof(struct uv_secret_list) == PAGE_SIZE);
> +
>  static inline int __uv_call(unsigned long r1, unsigned long r2)
>  {
>  	int cc;
> @@ -383,6 +469,45 @@ static inline int uv_cmd_nodata(u64 handle, u16 cmd, u16 *rc, u16 *rrc)
>  	return cc ? -EINVAL : 0;
>  }
>  
> +/** uv_list_secrets() - Do a List Secrets UVC
> + *  @buf: Buffer to write list into; size of one page
> + *  @start_idx: The smallest index that should be included in the list.
> + *		For the fist invocation use 0.
> + *  @rc: Pointer to store the return code or NULL.
> + *  @rrc: Pointer to store the return reason code or NULL.
> + *
> + *  This function calls the List Secrets UVC. The result is written into `buf`,
> + *  that needs to be at least one page of writable memory.
> + *  `buf` consists of:
> + *  * %struct uv_secret_list_hdr
> + *  * %struct uv_secret_list_item (multiple)
> + *
> + *  For `start_idx` use _0_ for the first call. If there are more secrets available
> + *  but could not fit into the page then `rc` is `UVC_RC_MORE_DATA`.
> + *  In this case use `uv_secret_list_hdr.next_valid_idx` for `start_idx`.

s/next_valid_idx/next_secret_idx/

> + *
> + *  Context: might sleep
> + *
> + *  Return: The UVC condition code
> + */
> +static inline int uv_list_secrets(u8 *buf, u16 start_idx, u16 *rc, u16 *rrc)
> +{
> +	struct uv_cb_list_secrets uvcb = {
> +		.header.len = sizeof(uvcb),
> +		.header.cmd = UVC_CMD_LIST_SECRETS,
> +		.start_idx = start_idx,
> +		.list_addr = (u64)buf,
> +	};
> +	int cc = uv_call_sched(0, (u64)&uvcb);
> +
> +	if (rc)
> +		*rc = uvcb.header.rc;
> +	if (rrc)
> +		*rrc = uvcb.header.rrc;
> +
> +	return cc;
> +}
> +
>  struct uv_info {
>  	unsigned long inst_calls_list[4];
>  	unsigned long uv_base_stor_len;
> @@ -469,6 +594,10 @@ static inline int uv_remove_shared(unsigned long addr)
>  	return share(addr, UVC_CMD_REMOVE_SHARED_ACCESS);
>  }
>  
> +int uv_get_secret_metadata(const u8 secret_id[UV_SECRET_ID_LEN],
> +			   struct uv_secret_list_item_hdr *secret);
> +int uv_retrieve_secret(u16 secret_idx, u8 *buf, size_t buf_size);
> +
>  extern int prot_virt_host;
>  
>  static inline int is_prot_virt_host(void)
> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index 36db065c7cf7..090246efc1fa 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c
> @@ -786,3 +786,127 @@ static int __init uv_info_init(void)
>  	return rc;
>  }
>  device_initcall(uv_info_init);
> +
> +/*
> + * Find the secret with the secret_id in the provided list.
> + *
> + * Context: might sleep
> + */

The method comment formatting is in general a bit inconsistent. Sometimes with,
sometimes without newlines / dots.

> +static int find_secret_in_page(const u8 secret_id[UV_SECRET_ID_LEN],
> +			       const struct uv_secret_list *list,
> +			       struct uv_secret_list_item_hdr *secret)
> +{
> +	u16 i;
> +
> +	for (i = 0; i < list->total_num_secrets; i++) {
> +		if (memcmp(secret_id, list->secrets[i].id, UV_SECRET_ID_LEN) == 0) {
> +			*secret = list->secrets[i].hdr;
> +			return 0;
> +		}
> +	}
> +	return -ENOENT;
> +}

[...]

> +/**
> + * uv_get_secret_metadata() - get secret metadata for a given secret id
> + * @secret_id: search pattern
> + * @secret: output data, containing the secret's metadata
> + *
> + * Search for a secret with the given secret_id in the Ultravisor secret store.
> + *
> + * Context: might sleep
> + *
> + * Return:
> + * * %0:	- Found entry; secret_idx and secret type are valid

You mean secret->index and secret->type?

> + * * %ENOENT	- No entry found
> + * * %ENODEV:	- Not supported: UV not available or command not available
> + * * %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 *buf;
> +	int rc;
> +
> +	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> +	rc = find_secret(secret_id, buf, secret);
> +	kfree(buf);
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(uv_get_secret_metadata);

[...]