2022-02-23 15:42:00

by Steffen Eiden

[permalink] [raw]
Subject: [PATCH v2 1/3] drivers/s390/char: Add Ultravisor io device

This patch adds a new miscdevice to expose some Ultravisor functions
to userspace. Userspace can send IOCTLs to the uvdevice that will then
emit a corresponding Ultravisor Call and hands the result over to
userspace. The uvdevice is available if the Ultravisor Call facility is
present.

Userspace is now able to call the Query Ultravisor Information
Ultravisor Command through the uvdevice.

Signed-off-by: Steffen Eiden <[email protected]>
---
MAINTAINERS | 2 +
arch/s390/include/uapi/asm/uvdevice.h | 27 +++++
drivers/s390/char/Kconfig | 11 ++
drivers/s390/char/Makefile | 1 +
drivers/s390/char/uvdevice.c | 145 ++++++++++++++++++++++++++
5 files changed, 186 insertions(+)
create mode 100644 arch/s390/include/uapi/asm/uvdevice.h
create mode 100644 drivers/s390/char/uvdevice.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 777cd6fa2b3d..f32e876f45c2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10577,9 +10577,11 @@ F: Documentation/virt/kvm/s390*
F: arch/s390/include/asm/gmap.h
F: arch/s390/include/asm/kvm*
F: arch/s390/include/uapi/asm/kvm*
+F: arch/s390/include/uapi/asm/uvdevice.h
F: arch/s390/kernel/uv.c
F: arch/s390/kvm/
F: arch/s390/mm/gmap.c
+F: drivers/s390/char/uvdevice.c
F: tools/testing/selftests/kvm/*/s390x/
F: tools/testing/selftests/kvm/s390x/

diff --git a/arch/s390/include/uapi/asm/uvdevice.h b/arch/s390/include/uapi/asm/uvdevice.h
new file mode 100644
index 000000000000..60956f8d2dc0
--- /dev/null
+++ b/arch/s390/include/uapi/asm/uvdevice.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright IBM Corp. 2022
+ * Author(s): Steffen Eiden <[email protected]>
+ */
+#ifndef __S390X_ASM_UVDEVICE_H
+#define __S390X_ASM_UVDEVICE_H
+
+#include <linux/types.h>
+
+struct uvio_ioctl_cb {
+ __u32 flags;
+ __u16 uv_rc; /* UV header rc value */
+ __u16 uv_rrc; /* UV header rrc value */
+ __u64 argument_addr; /* Userspace address of uvio argument */
+ __u32 argument_len;
+ __u8 reserved14[0x40 - 0x14]; /* must be zero */
+};
+
+#define UVIO_QUI_MAX_LEN 0x8000
+
+#define UVIO_DEVICE_NAME "uv"
+#define UVIO_TYPE_UVC 'u'
+
+#define UVIO_IOCTL_QUI _IOWR(UVIO_TYPE_UVC, 0x01, struct uvio_ioctl_cb)
+
+#endif /* __S390X_ASM_UVDEVICE_H */
diff --git a/drivers/s390/char/Kconfig b/drivers/s390/char/Kconfig
index 6cc4b19acf85..2a828274257a 100644
--- a/drivers/s390/char/Kconfig
+++ b/drivers/s390/char/Kconfig
@@ -184,3 +184,14 @@ config S390_VMUR
depends on S390
help
Character device driver for z/VM reader, puncher and printer.
+
+config S390_UV_UAPI
+ def_tristate y
+ prompt "Ultravisor userspace API"
+ depends on PROTECTED_VIRTUALIZATION_GUEST
+ help
+ Selecting exposes parts of the UV interface to userspace
+ by providing a misc character device at /dev/uv.
+ Using IOCTLs one can interact with the UV.
+ The device is available if the Ultravisor
+ Facility (158) is present.
diff --git a/drivers/s390/char/Makefile b/drivers/s390/char/Makefile
index c6fdb81a068a..ce32270082f5 100644
--- a/drivers/s390/char/Makefile
+++ b/drivers/s390/char/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_MONREADER) += monreader.o
obj-$(CONFIG_MONWRITER) += monwriter.o
obj-$(CONFIG_S390_VMUR) += vmur.o
obj-$(CONFIG_CRASH_DUMP) += sclp_sdias.o zcore.o
+obj-$(CONFIG_S390_UV_UAPI) += uvdevice.o

hmcdrv-objs := hmcdrv_mod.o hmcdrv_dev.o hmcdrv_ftp.o hmcdrv_cache.o diag_ftp.o sclp_ftp.o
obj-$(CONFIG_HMC_DRV) += hmcdrv.o
diff --git a/drivers/s390/char/uvdevice.c b/drivers/s390/char/uvdevice.c
new file mode 100644
index 000000000000..1d90e129b570
--- /dev/null
+++ b/drivers/s390/char/uvdevice.c
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright IBM Corp. 2022
+ * Author(s): Steffen Eiden <[email protected]>
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt ".\n"
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/types.h>
+#include <linux/stddef.h>
+#include <linux/vmalloc.h>
+#include <linux/slab.h>
+
+#include <asm/uvdevice.h>
+#include <asm/uv.h>
+
+/**
+ * uvio_qui() - Perform a Query Ultravisor Information UVC.
+ *
+ * uv_ioctl: ioctl control block
+ *
+ * uvio_qui() does a Query Ultravisor Information (QUI) Ultravisor Call.
+ * It creates the uvc qui request and sends it to the Ultravisor. After that
+ * it copies the response to userspace and fills the rc and rrc of uv_ioctl
+ * uv_call with the response values of the Ultravisor.
+ *
+ * Create the UVC structure, send the UVC to UV and write the response in the ioctl struct.
+ *
+ * Return: 0 on success or a negative error code on error.
+ */
+static int uvio_qui(struct uvio_ioctl_cb *uv_ioctl)
+{
+ u8 __user *user_buf_addr = (__user u8 *)uv_ioctl->argument_addr;
+ size_t user_buf_len = uv_ioctl->argument_len;
+ struct uv_cb_header *uvcb_qui = NULL;
+ int ret;
+
+ /*
+ * Do not check for a too small buffer. If userspace provides a buffer
+ * that is too small the Ultravisor will complain.
+ */
+ ret = -EINVAL;
+ if (!user_buf_len || user_buf_len > UVIO_QUI_MAX_LEN)
+ goto out;
+ ret = -ENOMEM;
+ uvcb_qui = kvzalloc(user_buf_len, GFP_KERNEL);
+ if (!uvcb_qui)
+ goto out;
+ uvcb_qui->len = user_buf_len;
+ uvcb_qui->cmd = UVC_CMD_QUI;
+
+ uv_call(0, (u64)uvcb_qui);
+
+ ret = -EFAULT;
+ if (copy_to_user(user_buf_addr, uvcb_qui, uvcb_qui->len))
+ goto out;
+ uv_ioctl->uv_rc = uvcb_qui->rc;
+ uv_ioctl->uv_rrc = uvcb_qui->rrc;
+
+ ret = 0;
+out:
+ kvfree(uvcb_qui);
+ return ret;
+}
+
+static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp)
+{
+ if (copy_from_user(ioctl, argp, sizeof(*ioctl)))
+ return -EFAULT;
+ if (ioctl->flags != 0)
+ return -EINVAL;
+ if (memchr_inv(ioctl->reserved14, 0, sizeof(ioctl->reserved14)))
+ return -EINVAL;
+
+ return 0;
+}
+
+/*
+ * IOCTL entry point for the Ultravisor device.
+ */
+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;
+
+ ret = -ENOMEM;
+ uv_ioctl = vzalloc(sizeof(*uv_ioctl));
+ if (!uv_ioctl)
+ goto out;
+
+ switch (cmd) {
+ case UVIO_IOCTL_QUI:
+ ret = uvio_copy_and_check_ioctl(uv_ioctl, argp);
+ if (ret)
+ goto out;
+ ret = uvio_qui(uv_ioctl);
+ break;
+ default:
+ ret = -ENOIOCTLCMD;
+ break;
+ }
+ if (ret)
+ goto out;
+
+ if (copy_to_user(argp, uv_ioctl, sizeof(*uv_ioctl)))
+ ret = -EFAULT;
+
+ out:
+ vfree(uv_ioctl);
+ return ret;
+}
+
+static const struct file_operations uvio_dev_fops = {
+ .owner = THIS_MODULE,
+ .unlocked_ioctl = uvio_ioctl,
+ .llseek = no_llseek,
+};
+
+static struct miscdevice uvio_dev_miscdev = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = UVIO_DEVICE_NAME,
+ .fops = &uvio_dev_fops,
+};
+
+static void __exit uvio_dev_exit(void)
+{
+ misc_deregister(&uvio_dev_miscdev);
+}
+
+static int __init uvio_dev_init(void)
+{
+ if (!test_facility(158))
+ return -ENXIO;
+ return misc_register(&uvio_dev_miscdev);
+}
+
+module_init(uvio_dev_init);
+module_exit(uvio_dev_exit);
+
+MODULE_AUTHOR("IBM Corporation");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Ultravisor UAPI driver");
--
2.25.1


2022-02-24 00:43:00

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drivers/s390/char: Add Ultravisor io device

On Wed, Feb 23, 2022 at 09:48:28AM -0500, Steffen Eiden wrote:
> --- /dev/null
> +++ b/drivers/s390/char/uvdevice.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright IBM Corp. 2022
> + * Author(s): Steffen Eiden <[email protected]>
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt ".\n"

Nit, this isn't needed as you do not have any pr_* calls that use it :)

thanks,

greg k-h

2022-03-02 14:03:35

by Janosch Frank

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drivers/s390/char: Add Ultravisor io device

On 2/23/22 15:48, Steffen Eiden wrote:
> This patch adds a new miscdevice to expose some Ultravisor functions
> to userspace. Userspace can send IOCTLs to the uvdevice that will then
> emit a corresponding Ultravisor Call and hands the result over to
> userspace. The uvdevice is available if the Ultravisor Call facility is
> present.
>
> Userspace is now able to call the Query Ultravisor Information
> Ultravisor Command through the uvdevice.
>
> Signed-off-by: Steffen Eiden <[email protected]>
> ---
> MAINTAINERS | 2 +
> arch/s390/include/uapi/asm/uvdevice.h | 27 +++++
> drivers/s390/char/Kconfig | 11 ++
> drivers/s390/char/Makefile | 1 +
> drivers/s390/char/uvdevice.c | 145 ++++++++++++++++++++++++++
> 5 files changed, 186 insertions(+)
> create mode 100644 arch/s390/include/uapi/asm/uvdevice.h
> create mode 100644 drivers/s390/char/uvdevice.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 777cd6fa2b3d..f32e876f45c2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10577,9 +10577,11 @@ F: Documentation/virt/kvm/s390*
> F: arch/s390/include/asm/gmap.h
> F: arch/s390/include/asm/kvm*
> F: arch/s390/include/uapi/asm/kvm*
> +F: arch/s390/include/uapi/asm/uvdevice.h
> F: arch/s390/kernel/uv.c
> F: arch/s390/kvm/
> F: arch/s390/mm/gmap.c
> +F: drivers/s390/char/uvdevice.c
> F: tools/testing/selftests/kvm/*/s390x/
> F: tools/testing/selftests/kvm/s390x/
>
> diff --git a/arch/s390/include/uapi/asm/uvdevice.h b/arch/s390/include/uapi/asm/uvdevice.h
> new file mode 100644
> index 000000000000..60956f8d2dc0
> --- /dev/null
> +++ b/arch/s390/include/uapi/asm/uvdevice.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright IBM Corp. 2022
> + * Author(s): Steffen Eiden <[email protected]>
> + */
> +#ifndef __S390X_ASM_UVDEVICE_H
> +#define __S390X_ASM_UVDEVICE_H
> +
> +#include <linux/types.h>
> +
> +struct uvio_ioctl_cb {
> + __u32 flags;
> + __u16 uv_rc; /* UV header rc value */
> + __u16 uv_rrc; /* UV header rrc value */
> + __u64 argument_addr; /* Userspace address of uvio argument */
> + __u32 argument_len;
> + __u8 reserved14[0x40 - 0x14]; /* must be zero */
> +};
> +
> +#define UVIO_QUI_MAX_LEN 0x8000
> +
> +#define UVIO_DEVICE_NAME "uv"
> +#define UVIO_TYPE_UVC 'u'
> +
> +#define UVIO_IOCTL_QUI _IOWR(UVIO_TYPE_UVC, 0x01, struct uvio_ioctl_cb)
> +
> +#endif /* __S390X_ASM_UVDEVICE_H */
> diff --git a/drivers/s390/char/Kconfig b/drivers/s390/char/Kconfig
> index 6cc4b19acf85..2a828274257a 100644
> --- a/drivers/s390/char/Kconfig
> +++ b/drivers/s390/char/Kconfig
> @@ -184,3 +184,14 @@ config S390_VMUR
> depends on S390
> help
> Character device driver for z/VM reader, puncher and printer.
> +
> +config S390_UV_UAPI
> + def_tristate y
> + prompt "Ultravisor userspace API"
> + depends on PROTECTED_VIRTUALIZATION_GUEST

Please drop the dependency.
We want this to be available to both guest and host as QUI is available
in both environments and more calls like this could follow.

We could put an option around the attestation but the savings are not
worth the effort.

2022-03-02 22:08:22

by Steffen Eiden

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drivers/s390/char: Add Ultravisor io device



On 3/2/22 11:34, Janosch Frank wrote:
> On 2/23/22 15:48, Steffen Eiden wrote:
>> This patch adds a new miscdevice to expose some Ultravisor functions
>> to userspace. Userspace can send IOCTLs to the uvdevice that will then
>> emit a corresponding Ultravisor Call and hands the result over to
>> userspace. The uvdevice is available if the Ultravisor Call facility is
>> present.
>>
>> Userspace is now able to call the Query Ultravisor Information
>> Ultravisor Command through the uvdevice.
>>
>> Signed-off-by: Steffen Eiden <[email protected]>
>> ---
[...]
>> +
>> +config S390_UV_UAPI
>> +    def_tristate y
>> +    prompt "Ultravisor userspace API"
>> +    depends on PROTECTED_VIRTUALIZATION_GUEST
>
> Please drop the dependency.
> We want this to be available to both guest and host as QUI is available
> in both environments and more calls like this could follow.
>
> We could put an option around the attestation but the savings are not
> worth the effort.

Makes sense. I will drop the dependency in v3.

Steffen

2022-03-03 11:16:01

by Janosch Frank

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drivers/s390/char: Add Ultravisor io device

On 2/23/22 15:48, Steffen Eiden wrote:
> This patch adds a new miscdevice to expose some Ultravisor functions
> to userspace. Userspace can send IOCTLs to the uvdevice that will then
> emit a corresponding Ultravisor Call and hands the result over to
> userspace. The uvdevice is available if the Ultravisor Call facility is
> present.
>
> Userspace is now able to call the Query Ultravisor Information
> Ultravisor Command through the uvdevice.
>
> Signed-off-by: Steffen Eiden <[email protected]>
> ---
> MAINTAINERS | 2 +
> arch/s390/include/uapi/asm/uvdevice.h | 27 +++++
> drivers/s390/char/Kconfig | 11 ++
> drivers/s390/char/Makefile | 1 +
> drivers/s390/char/uvdevice.c | 145 ++++++++++++++++++++++++++
> 5 files changed, 186 insertions(+)
> create mode 100644 arch/s390/include/uapi/asm/uvdevice.h
> create mode 100644 drivers/s390/char/uvdevice.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 777cd6fa2b3d..f32e876f45c2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10577,9 +10577,11 @@ F: Documentation/virt/kvm/s390*
> F: arch/s390/include/asm/gmap.h
> F: arch/s390/include/asm/kvm*
> F: arch/s390/include/uapi/asm/kvm*
> +F: arch/s390/include/uapi/asm/uvdevice.h
> F: arch/s390/kernel/uv.c
> F: arch/s390/kvm/
> F: arch/s390/mm/gmap.c
> +F: drivers/s390/char/uvdevice.c
> F: tools/testing/selftests/kvm/*/s390x/
> F: tools/testing/selftests/kvm/s390x/
>
> diff --git a/arch/s390/include/uapi/asm/uvdevice.h b/arch/s390/include/uapi/asm/uvdevice.h
> new file mode 100644
> index 000000000000..60956f8d2dc0
> --- /dev/null
> +++ b/arch/s390/include/uapi/asm/uvdevice.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright IBM Corp. 2022
> + * Author(s): Steffen Eiden <[email protected]>
> + */
> +#ifndef __S390X_ASM_UVDEVICE_H
> +#define __S390X_ASM_UVDEVICE_H
> +
> +#include <linux/types.h>
> +
> +struct uvio_ioctl_cb {
> + __u32 flags;
> + __u16 uv_rc; /* UV header rc value */
> + __u16 uv_rrc; /* UV header rrc value */
> + __u64 argument_addr; /* Userspace address of uvio argument */
> + __u32 argument_len;
> + __u8 reserved14[0x40 - 0x14]; /* must be zero */
> +};
> +
> +#define UVIO_QUI_MAX_LEN 0x8000
> +
> +#define UVIO_DEVICE_NAME "uv"
> +#define UVIO_TYPE_UVC 'u'
> +
> +#define UVIO_IOCTL_QUI _IOWR(UVIO_TYPE_UVC, 0x01, struct uvio_ioctl_cb)
> +
> +#endif /* __S390X_ASM_UVDEVICE_H */
> diff --git a/drivers/s390/char/Kconfig b/drivers/s390/char/Kconfig
> index 6cc4b19acf85..2a828274257a 100644
> --- a/drivers/s390/char/Kconfig
> +++ b/drivers/s390/char/Kconfig
> @@ -184,3 +184,14 @@ config S390_VMUR
> depends on S390
> help
> Character device driver for z/VM reader, puncher and printer.
> +
> +config S390_UV_UAPI
> + def_tristate y
> + prompt "Ultravisor userspace API"
> + depends on PROTECTED_VIRTUALIZATION_GUEST
> + help
> + Selecting exposes parts of the UV interface to userspace
> + by providing a misc character device at /dev/uv.
> + Using IOCTLs one can interact with the UV.
> + The device is available if the Ultravisor
> + Facility (158) is present.
> diff --git a/drivers/s390/char/Makefile b/drivers/s390/char/Makefile
> index c6fdb81a068a..ce32270082f5 100644
> --- a/drivers/s390/char/Makefile
> +++ b/drivers/s390/char/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_MONREADER) += monreader.o
> obj-$(CONFIG_MONWRITER) += monwriter.o
> obj-$(CONFIG_S390_VMUR) += vmur.o
> obj-$(CONFIG_CRASH_DUMP) += sclp_sdias.o zcore.o
> +obj-$(CONFIG_S390_UV_UAPI) += uvdevice.o
>
> hmcdrv-objs := hmcdrv_mod.o hmcdrv_dev.o hmcdrv_ftp.o hmcdrv_cache.o diag_ftp.o sclp_ftp.o
> obj-$(CONFIG_HMC_DRV) += hmcdrv.o
> diff --git a/drivers/s390/char/uvdevice.c b/drivers/s390/char/uvdevice.c
> new file mode 100644
> index 000000000000..1d90e129b570
> --- /dev/null
> +++ b/drivers/s390/char/uvdevice.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright IBM Corp. 2022
> + * Author(s): Steffen Eiden <[email protected]>
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt ".\n"
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/miscdevice.h>
> +#include <linux/types.h>
> +#include <linux/stddef.h>
> +#include <linux/vmalloc.h>
> +#include <linux/slab.h>
> +
> +#include <asm/uvdevice.h>
> +#include <asm/uv.h>
> +
> +/**
> + * uvio_qui() - Perform a Query Ultravisor Information UVC.
> + *
> + * uv_ioctl: ioctl control block
> + *
> + * uvio_qui() does a Query Ultravisor Information (QUI) Ultravisor Call.
> + * It creates the uvc qui request and sends it to the Ultravisor. After that
> + * it copies the response to userspace and fills the rc and rrc of uv_ioctl
> + * uv_call with the response values of the Ultravisor.
> + *
> + * Create the UVC structure, send the UVC to UV and write the response in the ioctl struct.
> + *
> + * Return: 0 on success or a negative error code on error.
> + */
> +static int uvio_qui(struct uvio_ioctl_cb *uv_ioctl)
> +{
> + u8 __user *user_buf_addr = (__user u8 *)uv_ioctl->argument_addr;
> + size_t user_buf_len = uv_ioctl->argument_len;
> + struct uv_cb_header *uvcb_qui = NULL;
> + int ret;
> +
> + /*
> + * Do not check for a too small buffer. If userspace provides a buffer
> + * that is too small the Ultravisor will complain.
> + */
> + ret = -EINVAL;
> + if (!user_buf_len || user_buf_len > UVIO_QUI_MAX_LEN)
> + goto out;
> + ret = -ENOMEM;
> + uvcb_qui = kvzalloc(user_buf_len, GFP_KERNEL);
> + if (!uvcb_qui)
> + goto out;
> + uvcb_qui->len = user_buf_len;
> + uvcb_qui->cmd = UVC_CMD_QUI;
> +
> + uv_call(0, (u64)uvcb_qui);

You'll copy zeroes to userspace if the rc is anything other than 0x1 or
0x100. It's not a mistake in itself, it just looks weird to me.

Also you don't consider the UV rc an error so userspace will always have
to check the uv rc and rrc even if the ioctl returns 0.

And it pretty much makes the rc and rrc copying below completely useless
since they'll always be in the actual uvcb data, no?

> +
> + ret = -EFAULT;
> + if (copy_to_user(user_buf_addr, uvcb_qui, uvcb_qui->len))
> + goto out;
> + uv_ioctl->uv_rc = uvcb_qui->rc;
> + uv_ioctl->uv_rrc = uvcb_qui->rrc;
> +
> + ret = 0;
> +out:
> + kvfree(uvcb_qui);
> + return ret;
> +}
> +
> +static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp)
> +{
> + if (copy_from_user(ioctl, argp, sizeof(*ioctl)))
> + return -EFAULT;
> + if (ioctl->flags != 0)
> + return -EINVAL;
> + if (memchr_inv(ioctl->reserved14, 0, sizeof(ioctl->reserved14)))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +/*
> + * IOCTL entry point for the Ultravisor device.
> + */
> +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;
> +
> + ret = -ENOMEM;
> + uv_ioctl = vzalloc(sizeof(*uv_ioctl));
> + if (!uv_ioctl)
> + goto out;
> +
> + switch (cmd) {
> + case UVIO_IOCTL_QUI:
> + ret = uvio_copy_and_check_ioctl(uv_ioctl, argp);
> + if (ret)
> + goto out;

You'll have to repeat this for every command making the

> + ret = uvio_qui(uv_ioctl);
> + break;
> + default:
> + ret = -ENOIOCTLCMD;
> + break;
> + }
> + if (ret)
> + goto out;
> +
> + if (copy_to_user(argp, uv_ioctl, sizeof(*uv_ioctl)))
> + ret = -EFAULT;
> +
> + out:
> + vfree(uv_ioctl);
> + return ret;
> +}
> +
> +static const struct file_operations uvio_dev_fops = {
> + .owner = THIS_MODULE,
> + .unlocked_ioctl = uvio_ioctl,
> + .llseek = no_llseek,
> +};
> +
> +static struct miscdevice uvio_dev_miscdev = {
> + .minor = MISC_DYNAMIC_MINOR,
> + .name = UVIO_DEVICE_NAME,
> + .fops = &uvio_dev_fops,
> +};
> +
> +static void __exit uvio_dev_exit(void)
> +{
> + misc_deregister(&uvio_dev_miscdev);
> +}
> +
> +static int __init uvio_dev_init(void)
> +{
> + if (!test_facility(158))
> + return -ENXIO;
> + return misc_register(&uvio_dev_miscdev);
> +}
> +
> +module_init(uvio_dev_init);
> +module_exit(uvio_dev_exit);
> +
> +MODULE_AUTHOR("IBM Corporation");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Ultravisor UAPI driver");

2022-03-03 15:39:20

by Janosch Frank

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drivers/s390/char: Add Ultravisor io device

On 2/23/22 15:48, Steffen Eiden wrote:
> This patch adds a new miscdevice to expose some Ultravisor functions
> to userspace. Userspace can send IOCTLs to the uvdevice that will then
> emit a corresponding Ultravisor Call and hands the result over to
> userspace. The uvdevice is available if the Ultravisor Call facility is
> present.
>
> Userspace is now able to call the Query Ultravisor Information
> Ultravisor Command through the uvdevice.
>
> Signed-off-by: Steffen Eiden <[email protected]>
> ---
> MAINTAINERS | 2 +
> arch/s390/include/uapi/asm/uvdevice.h | 27 +++++
> drivers/s390/char/Kconfig | 11 ++
> drivers/s390/char/Makefile | 1 +
> drivers/s390/char/uvdevice.c | 145 ++++++++++++++++++++++++++
> 5 files changed, 186 insertions(+)
> create mode 100644 arch/s390/include/uapi/asm/uvdevice.h
> create mode 100644 drivers/s390/char/uvdevice.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 777cd6fa2b3d..f32e876f45c2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10577,9 +10577,11 @@ F: Documentation/virt/kvm/s390*
> F: arch/s390/include/asm/gmap.h
> F: arch/s390/include/asm/kvm*
> F: arch/s390/include/uapi/asm/kvm*
> +F: arch/s390/include/uapi/asm/uvdevice.h
> F: arch/s390/kernel/uv.c
> F: arch/s390/kvm/
> F: arch/s390/mm/gmap.c
> +F: drivers/s390/char/uvdevice.c
> F: tools/testing/selftests/kvm/*/s390x/
> F: tools/testing/selftests/kvm/s390x/
>
> diff --git a/arch/s390/include/uapi/asm/uvdevice.h b/arch/s390/include/uapi/asm/uvdevice.h
> new file mode 100644
> index 000000000000..60956f8d2dc0
> --- /dev/null
> +++ b/arch/s390/include/uapi/asm/uvdevice.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright IBM Corp. 2022
> + * Author(s): Steffen Eiden <[email protected]>
> + */
> +#ifndef __S390X_ASM_UVDEVICE_H
> +#define __S390X_ASM_UVDEVICE_H
> +
> +#include <linux/types.h>
> +
> +struct uvio_ioctl_cb {
> + __u32 flags;
> + __u16 uv_rc; /* UV header rc value */
> + __u16 uv_rrc; /* UV header rrc value */
> + __u64 argument_addr; /* Userspace address of uvio argument */
> + __u32 argument_len;
> + __u8 reserved14[0x40 - 0x14]; /* must be zero */
> +};
> +

Add comment like this:
These max length constants define an upper length limit for UV control
blocks but they do not represent the actual maximum enforced by the
Ultravisor which is often way lower. By allowing these larger lengths we
hopefully won't need to update the code as often when a new machine
extends the control block length.

Userspace can therefore request more data than the kernel is usually
requesting for its own purposes.


> +#define UVIO_QUI_MAX_LEN 0x8000
> +
> +#define UVIO_DEVICE_NAME "uv"
> +#define UVIO_TYPE_UVC 'u'
> +
> +#define UVIO_IOCTL_QUI _IOWR(UVIO_TYPE_UVC, 0x01, struct uvio_ioctl_cb)
> +
> +#endif /* __S390X_ASM_UVDEVICE_H */
> diff --git a/drivers/s390/char/Kconfig b/drivers/s390/char/Kconfig
> index 6cc4b19acf85..2a828274257a 100644
> --- a/drivers/s390/char/Kconfig
> +++ b/drivers/s390/char/Kconfig
> @@ -184,3 +184,14 @@ config S390_VMUR
> depends on S390
> help
> Character device driver for z/VM reader, puncher and printer.
> +
> +config S390_UV_UAPI
> + def_tristate y
> + prompt "Ultravisor userspace API"
> + depends on PROTECTED_VIRTUALIZATION_GUEST
> + help
> + Selecting exposes parts of the UV interface to userspace
> + by providing a misc character device at /dev/uv.
> + Using IOCTLs one can interact with the UV.
> + The device is available if the Ultravisor
> + Facility (158) is present.
> diff --git a/drivers/s390/char/Makefile b/drivers/s390/char/Makefile
> index c6fdb81a068a..ce32270082f5 100644
> --- a/drivers/s390/char/Makefile
> +++ b/drivers/s390/char/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_MONREADER) += monreader.o
> obj-$(CONFIG_MONWRITER) += monwriter.o
> obj-$(CONFIG_S390_VMUR) += vmur.o
> obj-$(CONFIG_CRASH_DUMP) += sclp_sdias.o zcore.o
> +obj-$(CONFIG_S390_UV_UAPI) += uvdevice.o
>
> hmcdrv-objs := hmcdrv_mod.o hmcdrv_dev.o hmcdrv_ftp.o hmcdrv_cache.o diag_ftp.o sclp_ftp.o
> obj-$(CONFIG_HMC_DRV) += hmcdrv.o
> diff --git a/drivers/s390/char/uvdevice.c b/drivers/s390/char/uvdevice.c
> new file mode 100644
> index 000000000000..1d90e129b570
> --- /dev/null
> +++ b/drivers/s390/char/uvdevice.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright IBM Corp. 2022
> + * Author(s): Steffen Eiden <[email protected]>
> + */

You'll need to add a few words here to explain what the device does and
why we only do cursory checks.

Apart from these two comments and the line below I'm pretty happy with
the patch.

> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt ".\n"
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/miscdevice.h>
> +#include <linux/types.h>
> +#include <linux/stddef.h>
> +#include <linux/vmalloc.h>
> +#include <linux/slab.h>
> +
> +#include <asm/uvdevice.h>
> +#include <asm/uv.h>
> +
> +/**
> + * uvio_qui() - Perform a Query Ultravisor Information UVC.
> + *
> + * uv_ioctl: ioctl control block
> + *
> + * uvio_qui() does a Query Ultravisor Information (QUI) Ultravisor Call.
> + * It creates the uvc qui request and sends it to the Ultravisor. After that
> + * it copies the response to userspace and fills the rc and rrc of uv_ioctl
> + * uv_call with the response values of the Ultravisor.
> + *
> + * Create the UVC structure, send the UVC to UV and write the response in the ioctl struct.
> + *
> + * Return: 0 on success or a negative error code on error.
> + */
> +static int uvio_qui(struct uvio_ioctl_cb *uv_ioctl)
> +{
> + u8 __user *user_buf_addr = (__user u8 *)uv_ioctl->argument_addr;
> + size_t user_buf_len = uv_ioctl->argument_len;
> + struct uv_cb_header *uvcb_qui = NULL;
> + int ret;
> +
> + /*
> + * Do not check for a too small buffer. If userspace provides a buffer
> + * that is too small the Ultravisor will complain.
> + */
> + ret = -EINVAL;
> + if (!user_buf_len || user_buf_len > UVIO_QUI_MAX_LEN)
> + goto out;
> + ret = -ENOMEM;
> + uvcb_qui = kvzalloc(user_buf_len, GFP_KERNEL);
> + if (!uvcb_qui)
> + goto out;
> + uvcb_qui->len = user_buf_len;
> + uvcb_qui->cmd = UVC_CMD_QUI;
> +
> + uv_call(0, (u64)uvcb_qui);
> +
> + ret = -EFAULT;
> + if (copy_to_user(user_buf_addr, uvcb_qui, uvcb_qui->len))
> + goto out;
> + uv_ioctl->uv_rc = uvcb_qui->rc;
> + uv_ioctl->uv_rrc = uvcb_qui->rrc;
> +
> + ret = 0;
> +out:
> + kvfree(uvcb_qui);
> + return ret;
> +}
> +
> +static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp)
> +{
> + if (copy_from_user(ioctl, argp, sizeof(*ioctl)))
> + return -EFAULT;
> + if (ioctl->flags != 0)
> + return -EINVAL;
> + if (memchr_inv(ioctl->reserved14, 0, sizeof(ioctl->reserved14)))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +/*
> + * IOCTL entry point for the Ultravisor device.
> + */
> +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;
> +
> + ret = -ENOMEM;
> + uv_ioctl = vzalloc(sizeof(*uv_ioctl));
> + if (!uv_ioctl)
> + goto out;
> +
> + switch (cmd) {
> + case UVIO_IOCTL_QUI:
> + ret = uvio_copy_and_check_ioctl(uv_ioctl, argp);
> + if (ret)
> + goto out;
> + ret = uvio_qui(uv_ioctl);
> + break;
> + default:
> + ret = -ENOIOCTLCMD;
> + break;
> + }
> + if (ret)
> + goto out;
> +
> + if (copy_to_user(argp, uv_ioctl, sizeof(*uv_ioctl)))
> + ret = -EFAULT;
> +
> + out:
> + vfree(uv_ioctl);
> + return ret;
> +}
> +
> +static const struct file_operations uvio_dev_fops = {
> + .owner = THIS_MODULE,
> + .unlocked_ioctl = uvio_ioctl,
> + .llseek = no_llseek,
> +};
> +
> +static struct miscdevice uvio_dev_miscdev = {
> + .minor = MISC_DYNAMIC_MINOR,
> + .name = UVIO_DEVICE_NAME,
> + .fops = &uvio_dev_fops,
> +};
> +
> +static void __exit uvio_dev_exit(void)
> +{
> + misc_deregister(&uvio_dev_miscdev);
> +}
> +
> +static int __init uvio_dev_init(void)
> +{
> + if (!test_facility(158))
> + return -ENXIO;
> + return misc_register(&uvio_dev_miscdev);
> +}
> +
> +module_init(uvio_dev_init);
> +module_exit(uvio_dev_exit);
> +
> +MODULE_AUTHOR("IBM Corporation");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Ultravisor UAPI driver");