Re: [PATCH v3 1/6] s390/uvdevice: Add info IOCTL

Janosch Frank
2023-06-06T13:39:02Z

On 6/6/23 13:37, Steffen Eiden wrote:
> Add an IOCTL that allows userspace to find out which IOCTLs the uvdevice
> supports without trial and error.
>
> Explicitly expose the IOCTL nr for the request types.
>
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

> ---
> arch/s390/include/uapi/asm/uvdevice.h | 42 ++++++++++++++-
> drivers/s390/char/uvdevice.c | 77 ++++++++++++++++++++++++---
> 2 files changed, 111 insertions(+), 8 deletions(-)
>
> diff --git a/arch/s390/include/uapi/asm/uvdevice.h b/arch/s390/include/uapi/asm/uvdevice.h
> index 10a5ac918e02..9d9b684836c2 100644
> --- a/arch/s390/include/uapi/asm/uvdevice.h
> +++ b/arch/s390/include/uapi/asm/uvdevice.h
> @@ -32,6 +32,33 @@ struct uvio_attest {
> __u16 reserved136; /* 0x0136 */
> };
>
> +/**
> + * uvio_uvdev_info - Information of supported functions
> + * @supp_uvio_cmds - supported IOCTLs by this device
> + * @supp_uv_cmds - supported UVCs corresponding to the IOCTL
> + *
> + * UVIO request to get information about supported request types by this
> + * uvdevice and the Ultravisor. Everything is output. Bits are in LSB0
> + * ordering. If the bit is set in both, @supp_uvio_cmds and @supp_uv_cmds, the
> + * uvdevice and the Ultravisor support that call.
> + *
> + * Note that bit 0 (UVIO_IOCTL_UVDEV_INFO_NR) is always zero for `supp_uv_cmds`
> + * as there is no corresponding UV-call.
> + */
> +struct uvio_uvdev_info {
> + /*
> + * If bit `n` is set, this device supports the IOCTL with nr `n`.
> + */
> + __u64 supp_uvio_cmds;
> + /*
> + * If bit `n` is set, the Ultravisor(UV) supports the UV-call
> + * corresponding to the IOCTL with nr `n` in the calling contextx (host
> + * or guest). The value is only valid if the corresponding bit in
> + * @supp_uvio_cmds is set as well.
> + */
> + __u64 supp_uv_cmds;
> +};
> +
> /*
> * The following max values define an upper length for the IOCTL in/out buffers.
> * However, they do not represent the maximum the Ultravisor allows which is
> @@ -46,6 +73,19 @@ struct uvio_attest {
> #define UVIO_DEVICE_NAME "uv"
> #define UVIO_TYPE_UVC 'u'
>
> -#define UVIO_IOCTL_ATT _IOWR(UVIO_TYPE_UVC, 0x01, struct uvio_ioctl_cb)
> +enum UVIO_IOCTL_NR {
> + UVIO_IOCTL_UVDEV_INFO_NR = 0x00,
> + UVIO_IOCTL_ATT_NR,
> + /* must be the last entry */
> + UVIO_IOCTL_NUM_IOCTLS
> +};
> +
> +#define UVIO_IOCTL(nr) _IOWR(UVIO_TYPE_UVC, nr, struct uvio_ioctl_cb)
> +#define UVIO_IOCTL_UVDEV_INFO UVIO_IOCTL(UVIO_IOCTL_UVDEV_INFO_NR)
> +#define UVIO_IOCTL_ATT UVIO_IOCTL(UVIO_IOCTL_ATT_NR)
> +
> +#define UVIO_SUPP_CALL(nr) (1ULL << (nr))
> +#define UVIO_SUPP_UDEV_INFO UVIO_SUPP_CALL(UVIO_IOCTL_UDEV_INFO_NR)
> +#define UVIO_SUPP_ATT UVIO_SUPP_CALL(UVIO_IOCTL_ATT_NR)
>
> #endif /* __S390_ASM_UVDEVICE_H */
> diff --git a/drivers/s390/char/uvdevice.c b/drivers/s390/char/uvdevice.c
> index 1d40457c7b10..4efeebcaf382 100644
> --- a/drivers/s390/char/uvdevice.c
> +++ b/drivers/s390/char/uvdevice.c
> @@ -32,6 +32,52 @@
> #include <asm/uvdevice.h>
> #include <asm/uv.h>
>
> +#define BIT_UVIO_INTERNAL U32_MAX
> +/* Mapping from IOCTL-nr to UVC-bit */
> +static const u32 ioctl_nr_to_uvc_bit[] __initconst = {
> + [UVIO_IOCTL_UVDEV_INFO_NR] = BIT_UVIO_INTERNAL,
> + [UVIO_IOCTL_ATT_NR] = BIT_UVC_CMD_RETR_ATTEST,
> +};
> +
> +static_assert(ARRAY_SIZE(ioctl_nr_to_uvc_bit) == UVIO_IOCTL_NUM_IOCTLS);
> +
> +static struct uvio_uvdev_info uvdev_info = {
> + .supp_uvio_cmds = GENMASK_ULL(UVIO_IOCTL_NUM_IOCTLS - 1, 0),
> +};
> +
> +static void __init set_supp_uv_cmds(unsigned long *supp_uv_cmds)
> +{
> + int i;
> +
> + for (i = 0; i < UVIO_IOCTL_NUM_IOCTLS; i++) {
> + if (ioctl_nr_to_uvc_bit[i] == BIT_UVIO_INTERNAL)
> + continue;
> + if (!test_bit_inv(ioctl_nr_to_uvc_bit[i], uv_info.inst_calls_list))
> + continue;
> + __set_bit(i, supp_uv_cmds);
> + }
> +}
> +
> +/**
> + * uvio_uvdev_info() - get information about the uvdevice
> + *
> + * @uv_ioctl: ioctl control block
> + *
> + * Lists all IOCTLs that are supported by this uvdevice
> + */
> +static int uvio_uvdev_info(struct uvio_ioctl_cb *uv_ioctl)
> +{
> + void __user *user_buf_arg = (void __user *)uv_ioctl->argument_addr;
> +
> + if (uv_ioctl->argument_len < sizeof(uvdev_info))
> + return -EINVAL;
> + if (copy_to_user(user_buf_arg, &uvdev_info, sizeof(uvdev_info)))
> + return -EFAULT;
> +
> + uv_ioctl->uv_rc = UVC_RC_EXECUTED;
> + return 0;
> +}
> +
> static int uvio_build_uvcb_attest(struct uv_cb_attest *uvcb_attest, u8 *arcb,
> u8 *meas, u8 *add_data, struct uvio_attest *uvio_attest)
> {
> @@ -185,8 +231,19 @@ static int uvio_attestation(struct uvio_ioctl_cb *uv_ioctl)
> return ret;
> }
>
> -static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp)
> +static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp,
> + unsigned long cmd)
> {
> + u8 nr = _IOC_NR(cmd);
> +
> + if (_IOC_DIR(cmd) != (_IOC_READ | _IOC_WRITE))
> + return -ENOIOCTLCMD;
> + if (_IOC_TYPE(cmd) != UVIO_TYPE_UVC)
> + return -ENOIOCTLCMD;
> + if (nr >= UVIO_IOCTL_NUM_IOCTLS)
> + return -ENOIOCTLCMD;
> + if (_IOC_SIZE(cmd) != sizeof(*ioctl))
> + return -ENOIOCTLCMD;
> if (copy_from_user(ioctl, argp, sizeof(*ioctl)))
> return -EFAULT;
> if (ioctl->flags != 0)
> @@ -194,7 +251,7 @@ static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *a
> if (memchr_inv(ioctl->reserved14, 0, sizeof(ioctl->reserved14)))
> return -EINVAL;
>
> - return 0;
> + return nr;
> }
>
> /*
> @@ -205,12 +262,17 @@ static long uvio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> void __user *argp = (void __user *)arg;
> struct uvio_ioctl_cb uv_ioctl = { };
> long ret;
> + int nr;
> +
> + nr = uvio_copy_and_check_ioctl(&uv_ioctl, argp, cmd);
> + if (nr < 0)
> + return nr;
>
> - switch (cmd) {
> - case UVIO_IOCTL_ATT:
> - ret = uvio_copy_and_check_ioctl(&uv_ioctl, argp);
> - if (ret)
> - return ret;
> + switch (nr) {
> + case UVIO_IOCTL_UVDEV_INFO_NR:
> + ret = uvio_uvdev_info(&uv_ioctl);
> + break;
> + case UVIO_IOCTL_ATT_NR:
> ret = uvio_attestation(&uv_ioctl);
> break;
> default:
> @@ -245,6 +307,7 @@ static void __exit uvio_dev_exit(void)
>
> static int __init uvio_dev_init(void)
> {
> + set_supp_uv_cmds((unsigned long *)&uvdev_info.supp_uv_cmds);
> return misc_register(&uvio_dev_miscdev);
> }
>