Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <dc7e7448-eed0-591c-0c64-eba84b5a8e92@linux.ibm.com>
Date:   Mon, 5 Jun 2023 15:34:17 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH v2 4/6] s390/uvdevice: Add 'Lock Secret Store' UVC
Content-Language: en-US
To:     Steffen Eiden <seiden@linux.ibm.com>, kvm@vger.kernel.org,
        linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org,
        Viktor Mihajlovski <mihajlov@linux.ibm.com>
Cc:     Claudio Imbrenda <imbrenda@linux.ibm.com>,
        Nico Boehr <nrb@linux.ibm.com>,
        Christian Borntraeger <borntraeger@linux.ibm.com>,
        Heiko Carstens <hca@linux.ibm.com>,
        Hendrik Brueckner <brueckner@linux.ibm.com>
References: <20230519093708.810957-1-seiden@linux.ibm.com>
 <20230519093708.810957-5-seiden@linux.ibm.com>
From:   Janosch Frank <frankja@linux.ibm.com>
In-Reply-To: <20230519093708.810957-5-seiden@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 5/19/23 11:37, Steffen Eiden wrote:
> Userspace can call the Lock Secret Store Ultravisor Call
> using IOCTLs on the uvdevice.
> During the handling of the new IOCTL nr the uvdevice will do some sanity
> checks first. Then, perform the Ultravisor command, and copy the
> return codes to userspace.
> If the Lock Secrets UV facility is not present, UV will return
> invalid command rc. This won't be fenced in the driver and does not
> result in a negative return value. This is also true for any other
> possible error code the UV can return.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>

While the add and list secret calls work on data structures that are 
opaque to the kernel I'd describe the effects of this call here. Namely 
that any further add secret calls will fail with return code 0x102 once 
the store has been locked.

> ---
>   arch/s390/include/asm/uv.h            |  2 ++
>   arch/s390/include/uapi/asm/uvdevice.h |  3 +++
>   drivers/s390/char/uvdevice.c          | 39 +++++++++++++++++++++++++++
>   3 files changed, 44 insertions(+)
> 
> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
> index 1e4f0f6d4923..6180ac8909d5 100644
> --- a/arch/s390/include/asm/uv.h
> +++ b/arch/s390/include/asm/uv.h
> @@ -60,6 +60,7 @@
[...]
>   
> +/** uvio_lock_secrets() - perform a Lock Secret Store UVC
> + *
> + * @uv_ioctl: ioctl control block
> + *
> + * uvio_lock_secrets() performs the Lock Secret Store Ultravisor Call.
> + * It performs the UV-call and copies the return codes to the
> + * ioctl control block.
> + *
> + * The argument address and size must be 0.
> + *
> + * If the List Secrets UV facility is not present,
> + * UV will return invalid command rc. This won't be fenced in the driver
> + * and does not result in a negative return value.

This has weird indenting. The others often have it too but this one is 
especially strange. Did you do that yourself or is that from your editor?

> + *
> + * Context: might sleep
> + *
> + * Return: 0 on success or a negative error code on error.
> + */
> +static int uvio_lock_secrets(struct uvio_ioctl_cb *ioctl)
> +{
> +	struct uv_cb_nodata uvcb = {
> +		.header.len = sizeof(uvcb),
> +		.header.cmd = UVC_CMD_LOCK_SECRETS,
> +	};
> +
> +	if (ioctl->argument_addr || ioctl->argument_len)
> +		return -EINVAL;
> +
> +	uv_call(0, (u64)&uvcb);
> +	ioctl->uv_rc = uvcb.header.rc;
> +	ioctl->uv_rrc = uvcb.header.rrc;
> +
> +	return 0;
> +}
> +
>   static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp,
>   				     unsigned long cmd)
>   {
> @@ -388,6 +424,9 @@ static long uvio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>   	case UVIO_IOCTL_LIST_SECRETS_NR:
>   		ret = uvio_list_secrets(&uv_ioctl);
>   		break;
> +	case UVIO_IOCTL_LOCK_SECRETS_NR:
> +		ret = uvio_lock_secrets(&uv_ioctl);
> +		break;
>   	default:
>   		ret = -ENOIOCTLCMD;
>   		break;