All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] s390: Ultravisor device
@ 2022-05-10 14:47 Steffen Eiden
  2022-05-10 14:47 ` [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device Steffen Eiden
  2022-05-10 14:47 ` [PATCH v4 2/2] selftests: drivers/s390x: Add uvdevice tests Steffen Eiden
  0 siblings, 2 replies; 18+ messages in thread
From: Steffen Eiden @ 2022-05-10 14:47 UTC (permalink / raw)
  To: Greg KH, Heiko Carstens, Alexander Gordeev, Shuah Khan,
	Christian Borntraeger, Janosch Frank, Claudio Imbrenda,
	David Hildenbrand, Nico Boehr, linux-kernel, linux-kselftest,
	kvm

This series adds an Ultravisor(UV) device letting the userspace send some
Ultravisor calls to the UV. Currently two calls are supported.
Query Ultravisor Information (QUI) and
Receive Attestation Measurement (Attest[ation]).

The UV device is implemented as a miscdevice accepting only IOCTLs.
The IOCTL cmd specifies the UV call and the IOCTL arg the request
and response data depending on the UV call.
The device driver writes the UV response in the ioctl argument data.

The 'uvdevice' does no checks on the request beside faulty userspace
addresses, if sizes are in a sane range before allocating in kernel space,
and other tests that prevent the system from corruption.
Especially, no checks are made, that will be performed by the UV anyway
(E.g. 'invalid command' in case of attestation on unsupported hardware).
These errors are reported back to userspace using the UV return code
field.

The first patch introduces the new device as a module configured to be
compiled directly into the kernel (y) similar to the s390 SCLP and CHSH
miscdevice modules. The second atch introduces Kselftests which verify error
paths of the ioctl.

@Greg
   There is now a userspace tool "pvattest" that uses the new device:
   https://github.com/ibm-s390-linux/s390-tools/tree/for-2.22

   We are currently discussing some nits regarding the processing
   of certificates and the user interface and add the tool to
   s390-tools/master afterwards.
   However, the interface to the kernel is fixed.
   The tool (pvattest) will be in s390-tools 2.22 that accompanies
   linux 5.18.
   You will be most interested in S390-tools/pvattest/src/uvio.{h,c}
   and S390-tools/pvattest/src/pvattest.c

v3->v4:
  * Dropped the QUI related patches, as we agreed that we do not want
    to expose the QUI-UVC to userspace for now.
  * Kconfig option was misplaced inside the S390 TAPE group ->fixed
  * Some nits fixed:
  	* dropped the X from S390X in the uvdevice.h guard
	* fixes kernel doc style issue in uvdevice.c

v2->v3:
   The main change is that QUI is now introduced after Attestation as we
   might not want pick it. Also the Kselftest patch is splitted into
   Attestation and QUI so that they can be picked without requiring
   QUI support of the uvdevice.

  * dropped the Kconfig dependency
  * reorganized the series:
    - Patch 1 now covers the introduction of the uvdevice and Attestation
    - Patch 2 adds QUI to uvdevice
    - Patch 3/4 add Kselftests for Attestation and QUI
  * fixed some nits
  * added some comments

v1->v2:
  * ioctl returns -ENOIOCTLCMD in case of a invalid ioctl command
  * streamlined reserved field test
  * default Kconfig is y instead of m
  * improved selftest documentation

Steffen Eiden (2):
  drivers/s390/char: Add Ultravisor io device
  selftests: drivers/s390x: Add uvdevice tests

 MAINTAINERS                                   |   3 +
 arch/s390/include/asm/uv.h                    |  23 +-
 arch/s390/include/uapi/asm/uvdevice.h         |  51 ++++
 drivers/s390/char/Kconfig                     |  10 +
 drivers/s390/char/Makefile                    |   1 +
 drivers/s390/char/uvdevice.c                  | 264 +++++++++++++++++
 tools/testing/selftests/Makefile              |   1 +
 tools/testing/selftests/drivers/.gitignore    |   1 +
 .../selftests/drivers/s390x/uvdevice/Makefile |  22 ++
 .../selftests/drivers/s390x/uvdevice/config   |   1 +
 .../drivers/s390x/uvdevice/test_uvdevice.c    | 276 ++++++++++++++++++
 11 files changed, 652 insertions(+), 1 deletion(-)
 create mode 100644 arch/s390/include/uapi/asm/uvdevice.h
 create mode 100644 drivers/s390/char/uvdevice.c
 create mode 100644 tools/testing/selftests/drivers/s390x/uvdevice/Makefile
 create mode 100644 tools/testing/selftests/drivers/s390x/uvdevice/config
 create mode 100644 tools/testing/selftests/drivers/s390x/uvdevice/test_uvdevice.c

-- 
2.30.2


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device
  2022-05-10 14:47 [PATCH v4 0/2] s390: Ultravisor device Steffen Eiden
@ 2022-05-10 14:47 ` Steffen Eiden
  2022-05-12 14:33   ` Claudio Imbrenda
                     ` (3 more replies)
  2022-05-10 14:47 ` [PATCH v4 2/2] selftests: drivers/s390x: Add uvdevice tests Steffen Eiden
  1 sibling, 4 replies; 18+ messages in thread
From: Steffen Eiden @ 2022-05-10 14:47 UTC (permalink / raw)
  To: Greg KH, Heiko Carstens, Alexander Gordeev, Shuah Khan,
	Christian Borntraeger, Janosch Frank, Claudio Imbrenda,
	David Hildenbrand, Nico Boehr, linux-kernel, linux-kselftest,
	kvm

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 can call the Retrieve Attestation Measurement
Ultravisor Call using IOCTLs on the uvdevice.

The uvdevice will do some sanity checks first.
Then, copy the request data to kernel space, build the UVCB,
perform the UV call, and copy the result back to userspace.

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
 MAINTAINERS                           |   2 +
 arch/s390/include/asm/uv.h            |  23 ++-
 arch/s390/include/uapi/asm/uvdevice.h |  51 +++++
 drivers/s390/char/Kconfig             |  10 +
 drivers/s390/char/Makefile            |   1 +
 drivers/s390/char/uvdevice.c          | 264 ++++++++++++++++++++++++++
 6 files changed, 350 insertions(+), 1 deletion(-)
 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 e8c52d0192a6..b42ab4a35e18 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10781,9 +10781,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/asm/uv.h b/arch/s390/include/asm/uv.h
index a2d376b8bce3..cfea7b77a5b8 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -2,7 +2,7 @@
 /*
  * Ultravisor Interfaces
  *
- * Copyright IBM Corp. 2019
+ * Copyright IBM Corp. 2019, 2022
  *
  * Author(s):
  *	Vasily Gorbik <gor@linux.ibm.com>
@@ -52,6 +52,7 @@
 #define UVC_CMD_UNPIN_PAGE_SHARED	0x0342
 #define UVC_CMD_SET_SHARED_ACCESS	0x1000
 #define UVC_CMD_REMOVE_SHARED_ACCESS	0x1001
+#define UVC_CMD_RETR_ATTEST		0x1020
 
 /* Bits in installed uv calls */
 enum uv_cmds_inst {
@@ -76,6 +77,7 @@ enum uv_cmds_inst {
 	BIT_UVC_CMD_UNSHARE_ALL = 20,
 	BIT_UVC_CMD_PIN_PAGE_SHARED = 21,
 	BIT_UVC_CMD_UNPIN_PAGE_SHARED = 22,
+	BIT_UVC_CMD_RETR_ATTEST = 28,
 };
 
 enum uv_feat_ind {
@@ -219,6 +221,25 @@ struct uv_cb_share {
 	u64 reserved28;
 } __packed __aligned(8);
 
+/* Retrieve Attestation Measurement */
+struct uv_cb_attest {
+	struct uv_cb_header header;	/* 0x0000 */
+	u64 reserved08[2];		/* 0x0008 */
+	u64 arcb_addr;			/* 0x0018 */
+	u64 cont_token;			/* 0x0020 */
+	u8  reserved28[6];		/* 0x0028 */
+	u16 user_data_len;		/* 0x002e */
+	u8  user_data[256];		/* 0x0030 */
+	u32 reserved130[3];		/* 0x0130 */
+	u32 meas_len;			/* 0x013c */
+	u64 meas_addr;			/* 0x0140 */
+	u8  config_uid[16];		/* 0x0148 */
+	u32 reserved158;		/* 0x0158 */
+	u32 add_data_len;		/* 0x015c */
+	u64 add_data_addr;		/* 0x0160 */
+	u64 reserved168[4];		/* 0x0168 */
+} __packed __aligned(8);
+
 static inline int __uv_call(unsigned long r1, unsigned long r2)
 {
 	int cc;
diff --git a/arch/s390/include/uapi/asm/uvdevice.h b/arch/s390/include/uapi/asm/uvdevice.h
new file mode 100644
index 000000000000..d0bdde4969a1
--- /dev/null
+++ b/arch/s390/include/uapi/asm/uvdevice.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ *  Copyright IBM Corp. 2022
+ *  Author(s): Steffen Eiden <seiden@linux.ibm.com>
+ */
+#ifndef __S390_ASM_UVDEVICE_H
+#define __S390_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_ATT_USER_DATA_LEN		0x100
+#define UVIO_ATT_UID_LEN		0x10
+struct uvio_attest {
+	__u64 arcb_addr;				/* 0x0000 */
+	__u64 meas_addr;				/* 0x0008 */
+	__u64 add_data_addr;				/* 0x0010 */
+	__u8  user_data[UVIO_ATT_USER_DATA_LEN];	/* 0x0018 */
+	__u8  config_uid[UVIO_ATT_UID_LEN];		/* 0x0118 */
+	__u32 arcb_len;					/* 0x0128 */
+	__u32 meas_len;					/* 0x012c */
+	__u32 add_data_len;				/* 0x0130 */
+	__u16 user_data_len;				/* 0x0134 */
+	__u16 reserved136;				/* 0x0136 */
+};
+
+/*
+ * 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
+ * often way smaller. By allowing larger buffer sizes we hopefully do not need
+ * to update the code with every machine update. It is therefore possible for
+ * userspace to request more memory than actually used by kernel/UV.
+ */
+#define UVIO_ATT_ARCB_MAX_LEN		0x100000
+#define UVIO_ATT_MEASUREMENT_MAX_LEN	0x8000
+#define UVIO_ATT_ADDITIONAL_MAX_LEN	0x8000
+
+#define UVIO_DEVICE_NAME "uv"
+#define UVIO_TYPE_UVC 'u'
+
+#define UVIO_IOCTL_ATT _IOWR(UVIO_TYPE_UVC, 0x01, struct uvio_ioctl_cb)
+
+#endif  /* __S390_ASM_UVDEVICE_H */
diff --git a/drivers/s390/char/Kconfig b/drivers/s390/char/Kconfig
index 6cc4b19acf85..e9b9902abbaf 100644
--- a/drivers/s390/char/Kconfig
+++ b/drivers/s390/char/Kconfig
@@ -100,6 +100,16 @@ config SCLP_OFB
 	  This option enables the Open-for-Business interface to the s390
 	  Service Element.
 
+config S390_UV_UAPI
+	def_tristate y
+	prompt "Ultravisor userspace API"
+	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 only available if the Ultravisor
+	  Facility (158) is present.
+
 config S390_TAPE
 	def_tristate m
 	prompt "S/390 tape device support"
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..bcada86e8fc4
--- /dev/null
+++ b/drivers/s390/char/uvdevice.c
@@ -0,0 +1,264 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Copyright IBM Corp. 2022
+ *  Author(s): Steffen Eiden <seiden@linux.ibm.com>
+ *
+ *  This file provides a Linux misc device to give userspace access to some
+ *  Ultravisor (UV) functions. The device only accepts IOCTLs and will only
+ *  be present if the Ultravisor facility (158) is present.
+ *
+ *  When userspace sends a valid IOCTL uvdevice will copy the input data to
+ *  kernel space, do some basic validity checks to avoid kernel/system
+ *  corruption. Any other check that the Ultravisor does will not be done by
+ *  the uvdevice to keep changes minimal when adding new functionalities
+ *  to existing UV-calls.
+ *  After the checks uvdevice builds a corresponding
+ *  Ultravisor Call Control Block, and sends the request to the Ultravisor.
+ *  Then, it copies the response, including the return codes, back to userspace.
+ *  It is the responsibility of the userspace to check for any error issued
+ *  by UV and to interpret the UV response. The uvdevice acts as a communication
+ *  channel for userspace to the Ultravisor.
+ */
+
+#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>
+
+static int uvio_build_uvcb_attest(struct uv_cb_attest *uvcb_attest, u8 *arcb,
+				  u8 *meas, u8 *add_data, struct uvio_attest *uvio_attest)
+{
+	void __user *user_buf_arcb = (void __user *)uvio_attest->arcb_addr;
+
+	if (copy_from_user(arcb, user_buf_arcb, uvio_attest->arcb_len))
+		return -EFAULT;
+
+	uvcb_attest->header.len = sizeof(*uvcb_attest);
+	uvcb_attest->header.cmd = UVC_CMD_RETR_ATTEST;
+	uvcb_attest->arcb_addr = (u64)arcb;
+	uvcb_attest->cont_token = 0;
+	uvcb_attest->user_data_len = uvio_attest->user_data_len;
+	memcpy(uvcb_attest->user_data, uvio_attest->user_data, sizeof(uvcb_attest->user_data));
+	uvcb_attest->meas_len = uvio_attest->meas_len;
+	uvcb_attest->meas_addr = (u64)meas;
+	uvcb_attest->add_data_len = uvio_attest->add_data_len;
+	uvcb_attest->add_data_addr = (u64)add_data;
+
+	return 0;
+}
+
+static int uvio_copy_attest_result_to_user(struct uv_cb_attest *uvcb_attest,
+					   struct uvio_ioctl_cb *uv_ioctl,
+					   u8 *measurement, u8 *add_data,
+					   struct uvio_attest *uvio_attest)
+{
+	struct uvio_attest __user *user_uvio_attest = (void __user *)uv_ioctl->argument_addr;
+	void __user *user_buf_add = (void __user *)uvio_attest->add_data_addr;
+	void __user *user_buf_meas = (void __user *)uvio_attest->meas_addr;
+	void __user *user_buf_uid = &user_uvio_attest->config_uid;
+
+	if (copy_to_user(user_buf_meas, measurement, uvio_attest->meas_len))
+		return -EFAULT;
+	if (add_data && copy_to_user(user_buf_add, add_data, uvio_attest->add_data_len))
+		return -EFAULT;
+	if (copy_to_user(user_buf_uid, uvcb_attest->config_uid, sizeof(uvcb_attest->config_uid)))
+		return -EFAULT;
+	return 0;
+}
+
+static int get_uvio_attest(struct uvio_ioctl_cb *uv_ioctl, struct uvio_attest *uvio_attest)
+{
+	u8 __user *user_arg_buf = (u8 __user *)uv_ioctl->argument_addr;
+
+	if (copy_from_user(uvio_attest, user_arg_buf, sizeof(*uvio_attest)))
+		return -EFAULT;
+
+	if (uvio_attest->arcb_len > UVIO_ATT_ARCB_MAX_LEN)
+		return -EINVAL;
+	if (uvio_attest->arcb_len == 0)
+		return -EINVAL;
+	if (uvio_attest->meas_len > UVIO_ATT_MEASUREMENT_MAX_LEN)
+		return -EINVAL;
+	if (uvio_attest->meas_len == 0)
+		return -EINVAL;
+	if (uvio_attest->add_data_len > UVIO_ATT_ADDITIONAL_MAX_LEN)
+		return -EINVAL;
+	if (uvio_attest->reserved136)
+		return -EINVAL;
+	return 0;
+}
+
+/**
+ * uvio_attestation() - Perform a Retrieve Attestation Measurement UVC.
+ *
+ * @uv_ioctl: ioctl control block
+ *
+ * uvio_attestation() does a  Retrieve Attestation Measurement Ultravisor Call.
+ * It verifies that the given userspace addresses are valid and request sizes
+ * are sane. Every other check is made by the Ultravisor (UV) and won't result
+ * in a negative return value. It copies the input to kernelspace, builds the
+ * request, sends the UV-call, and copies the result to userspace.
+ *
+ * The Attestation Request has two input and two outputs.
+ * ARCB and User Data are inputs for the UV generated by userspace.
+ * Measurement and Additional Data are outputs for userspace generated by UV.
+ *
+ * The Attestation Request Control Block (ARCB) is a cryptographically verified
+ * and secured request to UV and User Data is some plaintext data which is
+ * going to be included in the Attestation Measurement calculation.
+ *
+ * Measurement is a cryptographic measurement of the callers properties,
+ * optional data configured by the ARCB and the user data. If specified by the
+ * ARCB, UV will add some Additional Data to the measurement calculation.
+ * This Additional Data is then returned as well.
+ *
+ * If the Retrieve Attestation Measurement 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.
+ *
+ * Context: might sleep
+ *
+ * Return: 0 on success or a negative error code on error.
+ */
+static int uvio_attestation(struct uvio_ioctl_cb *uv_ioctl)
+{
+	struct uv_cb_attest *uvcb_attest = NULL;
+	struct uvio_attest *uvio_attest = NULL;
+	u8 *measurement = NULL;
+	u8 *add_data = NULL;
+	u8 *arcb = NULL;
+	int ret;
+
+	ret = -EINVAL;
+	if (uv_ioctl->argument_len != sizeof(*uvio_attest))
+		goto out;
+
+	ret = -ENOMEM;
+	uvio_attest = kzalloc(sizeof(*uvio_attest), GFP_KERNEL);
+	if (!uvio_attest)
+		goto out;
+
+	ret = get_uvio_attest(uv_ioctl, uvio_attest);
+	if (ret)
+		goto out;
+
+	ret = -ENOMEM;
+	arcb = kvzalloc(uvio_attest->arcb_len, GFP_KERNEL);
+	measurement = kvzalloc(uvio_attest->meas_len, GFP_KERNEL);
+	if (!arcb || !measurement)
+		goto out;
+
+	if (uvio_attest->add_data_len) {
+		add_data = kvzalloc(uvio_attest->add_data_len, GFP_KERNEL);
+		if (!add_data)
+			goto out;
+	}
+
+	uvcb_attest = kzalloc(sizeof(*uvcb_attest), GFP_KERNEL);
+	if (!uvcb_attest)
+		goto out;
+
+	ret = uvio_build_uvcb_attest(uvcb_attest, arcb,  measurement, add_data, uvio_attest);
+	if (ret)
+		goto out;
+
+	uv_call_sched(0, (u64)uvcb_attest);
+
+	uv_ioctl->uv_rc = uvcb_attest->header.rc;
+	uv_ioctl->uv_rrc = uvcb_attest->header.rrc;
+
+	ret = uvio_copy_attest_result_to_user(uvcb_attest, uv_ioctl, measurement, add_data,
+					      uvio_attest);
+out:
+	kvfree(arcb);
+	kvfree(measurement);
+	kvfree(add_data);
+	kfree(uvio_attest);
+	kfree(uvcb_attest);
+	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_ATT:
+		ret = uvio_copy_and_check_ioctl(uv_ioctl, argp);
+		if (ret)
+			goto out;
+		ret = uvio_attestation(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.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 2/2] selftests: drivers/s390x: Add uvdevice tests
  2022-05-10 14:47 [PATCH v4 0/2] s390: Ultravisor device Steffen Eiden
  2022-05-10 14:47 ` [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device Steffen Eiden
@ 2022-05-10 14:47 ` Steffen Eiden
  2022-05-21 13:53   ` Muhammad Usama Anjum
  1 sibling, 1 reply; 18+ messages in thread
From: Steffen Eiden @ 2022-05-10 14:47 UTC (permalink / raw)
  To: Greg KH, Heiko Carstens, Alexander Gordeev, Shuah Khan,
	Christian Borntraeger, Janosch Frank, Claudio Imbrenda,
	David Hildenbrand, Nico Boehr, linux-kernel, linux-kselftest,
	kvm

Adds some selftests to test ioctl error paths of the uv-uapi.
The Kconfig S390_UV_UAPI must be selected and the Ultravisor facility
must be available. The test can be executed by non-root, however, the
uvdevice special file /dev/uv must be accessible for reading and
writing which may imply root privileges.

  ./test-uv-device
  TAP version 13
  1..6
  # Starting 6 tests from 3 test cases.
  #  RUN           uvio_fixture.att.fault_ioctl_arg ...
  #            OK  uvio_fixture.att.fault_ioctl_arg
  ok 1 uvio_fixture.att.fault_ioctl_arg
  #  RUN           uvio_fixture.att.fault_uvio_arg ...
  #            OK  uvio_fixture.att.fault_uvio_arg
  ok 2 uvio_fixture.att.fault_uvio_arg
  #  RUN           uvio_fixture.att.inval_ioctl_cb ...
  #            OK  uvio_fixture.att.inval_ioctl_cb
  ok 3 uvio_fixture.att.inval_ioctl_cb
  #  RUN           uvio_fixture.att.inval_ioctl_cmd ...
  #            OK  uvio_fixture.att.inval_ioctl_cmd
  ok 4 uvio_fixture.att.inval_ioctl_cmd
  #  RUN           attest_fixture.att_inval_request ...
  #            OK  attest_fixture.att_inval_request
  ok 5 attest_fixture.att_inval_request
  #  RUN           attest_fixture.att_inval_addr ...
  #            OK  attest_fixture.att_inval_addr
  ok 6 attest_fixture.att_inval_addr
  # PASSED: 6 / 6 tests passed.
  # Totals: pass:6 fail:0 xfail:0 xpass:0 skip:0 error:0

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
Acked-by: Janosch Frank <frankja@linux.ibm.com>
---
 MAINTAINERS                                   |   1 +
 tools/testing/selftests/Makefile              |   1 +
 tools/testing/selftests/drivers/.gitignore    |   1 +
 .../selftests/drivers/s390x/uvdevice/Makefile |  22 ++
 .../selftests/drivers/s390x/uvdevice/config   |   1 +
 .../drivers/s390x/uvdevice/test_uvdevice.c    | 276 ++++++++++++++++++
 6 files changed, 302 insertions(+)
 create mode 100644 tools/testing/selftests/drivers/s390x/uvdevice/Makefile
 create mode 100644 tools/testing/selftests/drivers/s390x/uvdevice/config
 create mode 100644 tools/testing/selftests/drivers/s390x/uvdevice/test_uvdevice.c

diff --git a/MAINTAINERS b/MAINTAINERS
index b42ab4a35e18..46a9b1467380 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10786,6 +10786,7 @@ 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/drivers/s390x/uvdevice/
 F:	tools/testing/selftests/kvm/*/s390x/
 F:	tools/testing/selftests/kvm/s390x/
 
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 2319ec87f53d..d6b307371ef7 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -10,6 +10,7 @@ TARGETS += core
 TARGETS += cpufreq
 TARGETS += cpu-hotplug
 TARGETS += drivers/dma-buf
+TARGETS += drivers/s390x/uvdevice
 TARGETS += efivarfs
 TARGETS += exec
 TARGETS += filesystems
diff --git a/tools/testing/selftests/drivers/.gitignore b/tools/testing/selftests/drivers/.gitignore
index ca74f2e1c719..09e23b5afa96 100644
--- a/tools/testing/selftests/drivers/.gitignore
+++ b/tools/testing/selftests/drivers/.gitignore
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 /dma-buf/udmabuf
+/s390x/uvdevice/test_uvdevice
diff --git a/tools/testing/selftests/drivers/s390x/uvdevice/Makefile b/tools/testing/selftests/drivers/s390x/uvdevice/Makefile
new file mode 100644
index 000000000000..5e701d2708d4
--- /dev/null
+++ b/tools/testing/selftests/drivers/s390x/uvdevice/Makefile
@@ -0,0 +1,22 @@
+include ../../../../../build/Build.include
+
+UNAME_M := $(shell uname -m)
+
+ifneq ($(UNAME_M),s390x)
+nothing:
+.PHONY: all clean run_tests install
+.SILENT:
+else
+
+TEST_GEN_PROGS := test_uvdevice
+
+top_srcdir ?= ../../../../../..
+KSFT_KHDR_INSTALL := 1
+khdr_dir = $(top_srcdir)/usr/include
+LINUX_TOOL_ARCH_INCLUDE = $(top_srcdir)/tools/arch/$(ARCH)/include
+
+CFLAGS += -Wall -Werror -static -I$(khdr_dir) -I$(LINUX_TOOL_ARCH_INCLUDE)
+
+include ../../../lib.mk
+
+endif
diff --git a/tools/testing/selftests/drivers/s390x/uvdevice/config b/tools/testing/selftests/drivers/s390x/uvdevice/config
new file mode 100644
index 000000000000..f28a04b99eff
--- /dev/null
+++ b/tools/testing/selftests/drivers/s390x/uvdevice/config
@@ -0,0 +1 @@
+CONFIG_S390_UV_UAPI=y
diff --git a/tools/testing/selftests/drivers/s390x/uvdevice/test_uvdevice.c b/tools/testing/selftests/drivers/s390x/uvdevice/test_uvdevice.c
new file mode 100644
index 000000000000..ea0cdc37b44f
--- /dev/null
+++ b/tools/testing/selftests/drivers/s390x/uvdevice/test_uvdevice.c
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  selftest for the Ultravisor UAPI device
+ *
+ *  Copyright IBM Corp. 2022
+ *  Author(s): Steffen Eiden <seiden@linux.ibm.com>
+ */
+
+#include <stdint.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+
+#include <asm/uvdevice.h>
+
+#include "../../../kselftest_harness.h"
+
+#define UV_PATH  "/dev/uv"
+#define BUFFER_SIZE 0x200
+FIXTURE(uvio_fixture) {
+	int uv_fd;
+	struct uvio_ioctl_cb uvio_ioctl;
+	uint8_t buffer[BUFFER_SIZE];
+	__u64 fault_page;
+};
+
+FIXTURE_VARIANT(uvio_fixture) {
+	unsigned long ioctl_cmd;
+	uint32_t arg_size;
+};
+
+FIXTURE_VARIANT_ADD(uvio_fixture, att) {
+	.ioctl_cmd = UVIO_IOCTL_ATT,
+	.arg_size = sizeof(struct uvio_attest),
+};
+
+FIXTURE_SETUP(uvio_fixture)
+{
+	self->uv_fd = open(UV_PATH, O_ACCMODE);
+
+	self->uvio_ioctl.argument_addr = (__u64)self->buffer;
+	self->uvio_ioctl.argument_len = variant->arg_size;
+	self->fault_page =
+		(__u64)mmap(NULL, (size_t)getpagesize(), PROT_NONE, MAP_ANONYMOUS, -1, 0);
+}
+
+FIXTURE_TEARDOWN(uvio_fixture)
+{
+	if (self->uv_fd)
+		close(self->uv_fd);
+	munmap((void *)self->fault_page, (size_t)getpagesize());
+}
+
+TEST_F(uvio_fixture, fault_ioctl_arg)
+{
+	int rc, errno_cache;
+
+	rc = ioctl(self->uv_fd, variant->ioctl_cmd, NULL);
+	errno_cache = errno;
+	ASSERT_EQ(rc, -1);
+	ASSERT_EQ(errno_cache, EFAULT);
+
+	rc = ioctl(self->uv_fd, variant->ioctl_cmd, self->fault_page);
+	errno_cache = errno;
+	ASSERT_EQ(rc, -1);
+	ASSERT_EQ(errno_cache, EFAULT);
+}
+
+TEST_F(uvio_fixture, fault_uvio_arg)
+{
+	int rc, errno_cache;
+
+	self->uvio_ioctl.argument_addr = 0;
+	rc = ioctl(self->uv_fd, variant->ioctl_cmd, &self->uvio_ioctl);
+	errno_cache = errno;
+	ASSERT_EQ(rc, -1);
+	ASSERT_EQ(errno_cache, EFAULT);
+
+	self->uvio_ioctl.argument_addr = self->fault_page;
+	rc = ioctl(self->uv_fd, variant->ioctl_cmd, &self->uvio_ioctl);
+	errno_cache = errno;
+	ASSERT_EQ(rc, -1);
+	ASSERT_EQ(errno_cache, EFAULT);
+}
+
+/*
+ * Test to verify that IOCTLs with invalid values in the ioctl_control block
+ * are rejected.
+ */
+TEST_F(uvio_fixture, inval_ioctl_cb)
+{
+	int rc, errno_cache;
+
+	self->uvio_ioctl.argument_len = 0;
+	rc = ioctl(self->uv_fd, variant->ioctl_cmd, &self->uvio_ioctl);
+	errno_cache = errno;
+	ASSERT_EQ(rc, -1);
+	ASSERT_EQ(errno_cache, EINVAL);
+
+	self->uvio_ioctl.argument_len = (uint32_t)-1;
+	rc = ioctl(self->uv_fd, variant->ioctl_cmd, &self->uvio_ioctl);
+	errno_cache = errno;
+	ASSERT_EQ(rc, -1);
+	ASSERT_EQ(errno_cache, EINVAL);
+	self->uvio_ioctl.argument_len = variant->arg_size;
+
+	self->uvio_ioctl.flags = (uint32_t)-1;
+	rc = ioctl(self->uv_fd, variant->ioctl_cmd, &self->uvio_ioctl);
+	errno_cache = errno;
+	ASSERT_EQ(rc, -1);
+	ASSERT_EQ(errno_cache, EINVAL);
+	self->uvio_ioctl.flags = 0;
+
+	memset(self->uvio_ioctl.reserved14, 0xff, sizeof(self->uvio_ioctl.reserved14));
+	rc = ioctl(self->uv_fd, variant->ioctl_cmd, &self->uvio_ioctl);
+	errno_cache = errno;
+	ASSERT_EQ(rc, -1);
+	ASSERT_EQ(errno_cache, EINVAL);
+
+	memset(&self->uvio_ioctl, 0x11, sizeof(self->uvio_ioctl));
+	rc = ioctl(self->uv_fd, variant->ioctl_cmd, &self->uvio_ioctl);
+	ASSERT_EQ(rc, -1);
+}
+
+TEST_F(uvio_fixture, inval_ioctl_cmd)
+{
+	int rc, errno_cache;
+	uint8_t nr = _IOC_NR(variant->ioctl_cmd);
+	unsigned long cmds[] = {
+		_IOWR('a', nr, struct uvio_ioctl_cb),
+		_IOWR(UVIO_TYPE_UVC, nr, int),
+		_IO(UVIO_TYPE_UVC, nr),
+		_IOR(UVIO_TYPE_UVC, nr, struct uvio_ioctl_cb),
+		_IOW(UVIO_TYPE_UVC, nr, struct uvio_ioctl_cb),
+	};
+
+	for (size_t i = 0; i < ARRAY_SIZE(cmds); i++) {
+		rc = ioctl(self->uv_fd, cmds[i], &self->uvio_ioctl);
+		errno_cache = errno;
+		ASSERT_EQ(rc, -1);
+		ASSERT_EQ(errno_cache, ENOTTY);
+	}
+}
+
+struct test_attest_buffer {
+	uint8_t arcb[0x180];
+	uint8_t meas[64];
+	uint8_t add[32];
+};
+
+FIXTURE(attest_fixture) {
+	int uv_fd;
+	struct uvio_ioctl_cb uvio_ioctl;
+	struct uvio_attest uvio_attest;
+	struct test_attest_buffer attest_buffer;
+	__u64 fault_page;
+};
+
+FIXTURE_SETUP(attest_fixture)
+{
+	self->uv_fd = open(UV_PATH, O_ACCMODE);
+
+	self->uvio_ioctl.argument_addr = (__u64)&self->uvio_attest;
+	self->uvio_ioctl.argument_len = sizeof(self->uvio_attest);
+
+	self->uvio_attest.arcb_addr = (__u64)&self->attest_buffer.arcb;
+	self->uvio_attest.arcb_len = sizeof(self->attest_buffer.arcb);
+
+	self->uvio_attest.meas_addr = (__u64)&self->attest_buffer.meas;
+	self->uvio_attest.meas_len = sizeof(self->attest_buffer.meas);
+
+	self->uvio_attest.add_data_addr = (__u64)&self->attest_buffer.add;
+	self->uvio_attest.add_data_len = sizeof(self->attest_buffer.add);
+	self->fault_page =
+		(__u64)mmap(NULL, (size_t)getpagesize(), PROT_NONE, MAP_ANONYMOUS, -1, 0);
+}
+
+FIXTURE_TEARDOWN(attest_fixture)
+{
+	if (self->uv_fd)
+		close(self->uv_fd);
+	munmap((void *)self->fault_page, (size_t)getpagesize());
+}
+
+static void att_inval_sizes_test(uint32_t *size, uint32_t max_size, bool test_zero,
+				 struct __test_metadata *_metadata,
+				 FIXTURE_DATA(attest_fixture) *self)
+{
+	int rc, errno_cache;
+	uint32_t tmp = *size;
+
+	if (test_zero) {
+		*size = 0;
+		rc = ioctl(self->uv_fd, UVIO_IOCTL_ATT, &self->uvio_ioctl);
+		errno_cache = errno;
+		ASSERT_EQ(rc, -1);
+		ASSERT_EQ(errno_cache, EINVAL);
+	}
+	*size = max_size + 1;
+	rc = ioctl(self->uv_fd, UVIO_IOCTL_ATT, &self->uvio_ioctl);
+	errno_cache = errno;
+	ASSERT_EQ(rc, -1);
+	ASSERT_EQ(errno_cache, EINVAL);
+	*size = tmp;
+}
+
+/*
+ * Test to verify that attestation IOCTLs with invalid values in the UVIO
+ * attestation control block are rejected.
+ */
+TEST_F(attest_fixture, att_inval_request)
+{
+	int rc, errno_cache;
+
+	att_inval_sizes_test(&self->uvio_attest.add_data_len, UVIO_ATT_ADDITIONAL_MAX_LEN,
+			     false, _metadata, self);
+	att_inval_sizes_test(&self->uvio_attest.meas_len, UVIO_ATT_MEASUREMENT_MAX_LEN,
+			     true, _metadata, self);
+	att_inval_sizes_test(&self->uvio_attest.arcb_len, UVIO_ATT_ARCB_MAX_LEN,
+			     true, _metadata, self);
+
+	self->uvio_attest.reserved136 = (uint16_t)-1;
+	rc = ioctl(self->uv_fd, UVIO_IOCTL_ATT, &self->uvio_ioctl);
+	errno_cache = errno;
+	ASSERT_EQ(rc, -1);
+	ASSERT_EQ(errno_cache, EINVAL);
+
+	memset(&self->uvio_attest, 0x11, sizeof(self->uvio_attest));
+	rc = ioctl(self->uv_fd, UVIO_IOCTL_ATT, &self->uvio_ioctl);
+	ASSERT_EQ(rc, -1);
+}
+
+static void att_inval_addr_test(__u64 *addr, struct __test_metadata *_metadata,
+				FIXTURE_DATA(attest_fixture) *self)
+{
+	int rc, errno_cache;
+	__u64 tmp = *addr;
+
+	*addr = 0;
+	rc = ioctl(self->uv_fd, UVIO_IOCTL_ATT, &self->uvio_ioctl);
+	errno_cache = errno;
+	ASSERT_EQ(rc, -1);
+	ASSERT_EQ(errno_cache, EFAULT);
+	*addr = self->fault_page;
+	rc = ioctl(self->uv_fd, UVIO_IOCTL_ATT, &self->uvio_ioctl);
+	errno_cache = errno;
+	ASSERT_EQ(rc, -1);
+	ASSERT_EQ(errno_cache, EFAULT);
+	*addr = tmp;
+}
+
+TEST_F(attest_fixture, att_inval_addr)
+{
+	att_inval_addr_test(&self->uvio_attest.arcb_addr, _metadata, self);
+	att_inval_addr_test(&self->uvio_attest.add_data_addr, _metadata, self);
+	att_inval_addr_test(&self->uvio_attest.meas_addr, _metadata, self);
+}
+
+static void __attribute__((constructor)) __constructor_order_last(void)
+{
+	if (!__constructor_order)
+		__constructor_order = _CONSTRUCTOR_ORDER_BACKWARD;
+}
+
+int main(int argc, char **argv)
+{
+	int fd = open(UV_PATH, O_ACCMODE);
+
+	if (fd < 0)
+		ksft_exit_skip("No uv-device or cannot access " UV_PATH  "\n"
+			       "Enable CONFIG_S390_UV_UAPI and check the access rights on "
+			       UV_PATH ".\n");
+	close(fd);
+	return test_harness_run(argc, argv);
+}
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device
  2022-05-10 14:47 ` [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device Steffen Eiden
@ 2022-05-12 14:33   ` Claudio Imbrenda
  2022-05-13  7:45     ` Steffen Eiden
  2022-05-16 11:33   ` Steffen Eiden
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Claudio Imbrenda @ 2022-05-12 14:33 UTC (permalink / raw)
  To: Steffen Eiden
  Cc: Greg KH, Heiko Carstens, Alexander Gordeev, Shuah Khan,
	Christian Borntraeger, Janosch Frank, David Hildenbrand,
	Nico Boehr, linux-kernel, linux-kselftest, kvm

On Tue, 10 May 2022 14:47:23 +0000
Steffen Eiden <seiden@linux.ibm.com> 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 can call the Retrieve Attestation Measurement
> Ultravisor Call using IOCTLs on the uvdevice.
> 
> The uvdevice will do some sanity checks first.
> Then, copy the request data to kernel space, build the UVCB,
> perform the UV call, and copy the result back to userspace.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  MAINTAINERS                           |   2 +
>  arch/s390/include/asm/uv.h            |  23 ++-
>  arch/s390/include/uapi/asm/uvdevice.h |  51 +++++
>  drivers/s390/char/Kconfig             |  10 +
>  drivers/s390/char/Makefile            |   1 +
>  drivers/s390/char/uvdevice.c          | 264 ++++++++++++++++++++++++++
>  6 files changed, 350 insertions(+), 1 deletion(-)
>  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 e8c52d0192a6..b42ab4a35e18 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10781,9 +10781,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/asm/uv.h b/arch/s390/include/asm/uv.h
> index a2d376b8bce3..cfea7b77a5b8 100644
> --- a/arch/s390/include/asm/uv.h
> +++ b/arch/s390/include/asm/uv.h
> @@ -2,7 +2,7 @@
>  /*
>   * Ultravisor Interfaces
>   *
> - * Copyright IBM Corp. 2019
> + * Copyright IBM Corp. 2019, 2022
>   *
>   * Author(s):
>   *	Vasily Gorbik <gor@linux.ibm.com>
> @@ -52,6 +52,7 @@
>  #define UVC_CMD_UNPIN_PAGE_SHARED	0x0342
>  #define UVC_CMD_SET_SHARED_ACCESS	0x1000
>  #define UVC_CMD_REMOVE_SHARED_ACCESS	0x1001
> +#define UVC_CMD_RETR_ATTEST		0x1020
>  
>  /* Bits in installed uv calls */
>  enum uv_cmds_inst {
> @@ -76,6 +77,7 @@ enum uv_cmds_inst {
>  	BIT_UVC_CMD_UNSHARE_ALL = 20,
>  	BIT_UVC_CMD_PIN_PAGE_SHARED = 21,
>  	BIT_UVC_CMD_UNPIN_PAGE_SHARED = 22,
> +	BIT_UVC_CMD_RETR_ATTEST = 28,
>  };
>  
>  enum uv_feat_ind {
> @@ -219,6 +221,25 @@ struct uv_cb_share {
>  	u64 reserved28;
>  } __packed __aligned(8);
>  
> +/* Retrieve Attestation Measurement */
> +struct uv_cb_attest {
> +	struct uv_cb_header header;	/* 0x0000 */
> +	u64 reserved08[2];		/* 0x0008 */
> +	u64 arcb_addr;			/* 0x0018 */
> +	u64 cont_token;			/* 0x0020 */
> +	u8  reserved28[6];		/* 0x0028 */
> +	u16 user_data_len;		/* 0x002e */
> +	u8  user_data[256];		/* 0x0030 */
> +	u32 reserved130[3];		/* 0x0130 */
> +	u32 meas_len;			/* 0x013c */
> +	u64 meas_addr;			/* 0x0140 */
> +	u8  config_uid[16];		/* 0x0148 */
> +	u32 reserved158;		/* 0x0158 */
> +	u32 add_data_len;		/* 0x015c */
> +	u64 add_data_addr;		/* 0x0160 */
> +	u64 reserved168[4];		/* 0x0168 */
> +} __packed __aligned(8);
> +
>  static inline int __uv_call(unsigned long r1, unsigned long r2)
>  {
>  	int cc;
> diff --git a/arch/s390/include/uapi/asm/uvdevice.h b/arch/s390/include/uapi/asm/uvdevice.h
> new file mode 100644
> index 000000000000..d0bdde4969a1
> --- /dev/null
> +++ b/arch/s390/include/uapi/asm/uvdevice.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + *  Copyright IBM Corp. 2022
> + *  Author(s): Steffen Eiden <seiden@linux.ibm.com>
> + */
> +#ifndef __S390_ASM_UVDEVICE_H
> +#define __S390_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_ATT_USER_DATA_LEN		0x100
> +#define UVIO_ATT_UID_LEN		0x10
> +struct uvio_attest {
> +	__u64 arcb_addr;				/* 0x0000 */
> +	__u64 meas_addr;				/* 0x0008 */
> +	__u64 add_data_addr;				/* 0x0010 */
> +	__u8  user_data[UVIO_ATT_USER_DATA_LEN];	/* 0x0018 */
> +	__u8  config_uid[UVIO_ATT_UID_LEN];		/* 0x0118 */
> +	__u32 arcb_len;					/* 0x0128 */
> +	__u32 meas_len;					/* 0x012c */
> +	__u32 add_data_len;				/* 0x0130 */
> +	__u16 user_data_len;				/* 0x0134 */
> +	__u16 reserved136;				/* 0x0136 */
> +};
> +
> +/*
> + * 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
> + * often way smaller. By allowing larger buffer sizes we hopefully do not need
> + * to update the code with every machine update. It is therefore possible for
> + * userspace to request more memory than actually used by kernel/UV.
> + */
> +#define UVIO_ATT_ARCB_MAX_LEN		0x100000
> +#define UVIO_ATT_MEASUREMENT_MAX_LEN	0x8000
> +#define UVIO_ATT_ADDITIONAL_MAX_LEN	0x8000
> +
> +#define UVIO_DEVICE_NAME "uv"
> +#define UVIO_TYPE_UVC 'u'
> +
> +#define UVIO_IOCTL_ATT _IOWR(UVIO_TYPE_UVC, 0x01, struct uvio_ioctl_cb)
> +
> +#endif  /* __S390_ASM_UVDEVICE_H */
> diff --git a/drivers/s390/char/Kconfig b/drivers/s390/char/Kconfig
> index 6cc4b19acf85..e9b9902abbaf 100644
> --- a/drivers/s390/char/Kconfig
> +++ b/drivers/s390/char/Kconfig
> @@ -100,6 +100,16 @@ config SCLP_OFB
>  	  This option enables the Open-for-Business interface to the s390
>  	  Service Element.
>  
> +config S390_UV_UAPI
> +	def_tristate y
> +	prompt "Ultravisor userspace API"
> +	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 only available if the Ultravisor
> +	  Facility (158) is present.
> +
>  config S390_TAPE
>  	def_tristate m
>  	prompt "S/390 tape device support"
> 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..bcada86e8fc4
> --- /dev/null
> +++ b/drivers/s390/char/uvdevice.c
> @@ -0,0 +1,264 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  Copyright IBM Corp. 2022
> + *  Author(s): Steffen Eiden <seiden@linux.ibm.com>
> + *
> + *  This file provides a Linux misc device to give userspace access to some
> + *  Ultravisor (UV) functions. The device only accepts IOCTLs and will only
> + *  be present if the Ultravisor facility (158) is present.
> + *
> + *  When userspace sends a valid IOCTL uvdevice will copy the input data to
> + *  kernel space, do some basic validity checks to avoid kernel/system
> + *  corruption. Any other check that the Ultravisor does will not be done by
> + *  the uvdevice to keep changes minimal when adding new functionalities
> + *  to existing UV-calls.
> + *  After the checks uvdevice builds a corresponding
> + *  Ultravisor Call Control Block, and sends the request to the Ultravisor.
> + *  Then, it copies the response, including the return codes, back to userspace.
> + *  It is the responsibility of the userspace to check for any error issued
> + *  by UV and to interpret the UV response. The uvdevice acts as a communication
> + *  channel for userspace to the Ultravisor.
> + */
> +
> +#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>
> +
> +static int uvio_build_uvcb_attest(struct uv_cb_attest *uvcb_attest, u8 *arcb,
> +				  u8 *meas, u8 *add_data, struct uvio_attest *uvio_attest)
> +{
> +	void __user *user_buf_arcb = (void __user *)uvio_attest->arcb_addr;
> +
> +	if (copy_from_user(arcb, user_buf_arcb, uvio_attest->arcb_len))
> +		return -EFAULT;
> +
> +	uvcb_attest->header.len = sizeof(*uvcb_attest);
> +	uvcb_attest->header.cmd = UVC_CMD_RETR_ATTEST;
> +	uvcb_attest->arcb_addr = (u64)arcb;
> +	uvcb_attest->cont_token = 0;
> +	uvcb_attest->user_data_len = uvio_attest->user_data_len;
> +	memcpy(uvcb_attest->user_data, uvio_attest->user_data, sizeof(uvcb_attest->user_data));
> +	uvcb_attest->meas_len = uvio_attest->meas_len;
> +	uvcb_attest->meas_addr = (u64)meas;
> +	uvcb_attest->add_data_len = uvio_attest->add_data_len;
> +	uvcb_attest->add_data_addr = (u64)add_data;
> +
> +	return 0;
> +}
> +
> +static int uvio_copy_attest_result_to_user(struct uv_cb_attest *uvcb_attest,
> +					   struct uvio_ioctl_cb *uv_ioctl,
> +					   u8 *measurement, u8 *add_data,
> +					   struct uvio_attest *uvio_attest)
> +{
> +	struct uvio_attest __user *user_uvio_attest = (void __user *)uv_ioctl->argument_addr;
> +	void __user *user_buf_add = (void __user *)uvio_attest->add_data_addr;
> +	void __user *user_buf_meas = (void __user *)uvio_attest->meas_addr;
> +	void __user *user_buf_uid = &user_uvio_attest->config_uid;
> +
> +	if (copy_to_user(user_buf_meas, measurement, uvio_attest->meas_len))
> +		return -EFAULT;
> +	if (add_data && copy_to_user(user_buf_add, add_data, uvio_attest->add_data_len))
> +		return -EFAULT;
> +	if (copy_to_user(user_buf_uid, uvcb_attest->config_uid, sizeof(uvcb_attest->config_uid)))
> +		return -EFAULT;
> +	return 0;
> +}
> +
> +static int get_uvio_attest(struct uvio_ioctl_cb *uv_ioctl, struct uvio_attest *uvio_attest)
> +{
> +	u8 __user *user_arg_buf = (u8 __user *)uv_ioctl->argument_addr;
> +
> +	if (copy_from_user(uvio_attest, user_arg_buf, sizeof(*uvio_attest)))
> +		return -EFAULT;
> +
> +	if (uvio_attest->arcb_len > UVIO_ATT_ARCB_MAX_LEN)
> +		return -EINVAL;
> +	if (uvio_attest->arcb_len == 0)
> +		return -EINVAL;
> +	if (uvio_attest->meas_len > UVIO_ATT_MEASUREMENT_MAX_LEN)
> +		return -EINVAL;
> +	if (uvio_attest->meas_len == 0)
> +		return -EINVAL;
> +	if (uvio_attest->add_data_len > UVIO_ATT_ADDITIONAL_MAX_LEN)
> +		return -EINVAL;
> +	if (uvio_attest->reserved136)
> +		return -EINVAL;
> +	return 0;
> +}
> +
> +/**
> + * uvio_attestation() - Perform a Retrieve Attestation Measurement UVC.
> + *
> + * @uv_ioctl: ioctl control block
> + *
> + * uvio_attestation() does a  Retrieve Attestation Measurement Ultravisor Call.
> + * It verifies that the given userspace addresses are valid and request sizes
> + * are sane. Every other check is made by the Ultravisor (UV) and won't result
> + * in a negative return value. It copies the input to kernelspace, builds the
> + * request, sends the UV-call, and copies the result to userspace.
> + *
> + * The Attestation Request has two input and two outputs.
> + * ARCB and User Data are inputs for the UV generated by userspace.
> + * Measurement and Additional Data are outputs for userspace generated by UV.
> + *
> + * The Attestation Request Control Block (ARCB) is a cryptographically verified
> + * and secured request to UV and User Data is some plaintext data which is
> + * going to be included in the Attestation Measurement calculation.
> + *
> + * Measurement is a cryptographic measurement of the callers properties,
> + * optional data configured by the ARCB and the user data. If specified by the
> + * ARCB, UV will add some Additional Data to the measurement calculation.
> + * This Additional Data is then returned as well.
> + *
> + * If the Retrieve Attestation Measurement 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.
> + *
> + * Context: might sleep
> + *
> + * Return: 0 on success or a negative error code on error.
> + */
> +static int uvio_attestation(struct uvio_ioctl_cb *uv_ioctl)
> +{
> +	struct uv_cb_attest *uvcb_attest = NULL;
> +	struct uvio_attest *uvio_attest = NULL;
> +	u8 *measurement = NULL;
> +	u8 *add_data = NULL;
> +	u8 *arcb = NULL;
> +	int ret;
> +
> +	ret = -EINVAL;
> +	if (uv_ioctl->argument_len != sizeof(*uvio_attest))
> +		goto out;
> +
> +	ret = -ENOMEM;
> +	uvio_attest = kzalloc(sizeof(*uvio_attest), GFP_KERNEL);
> +	if (!uvio_attest)
> +		goto out;
> +
> +	ret = get_uvio_attest(uv_ioctl, uvio_attest);
> +	if (ret)
> +		goto out;
> +
> +	ret = -ENOMEM;
> +	arcb = kvzalloc(uvio_attest->arcb_len, GFP_KERNEL);
> +	measurement = kvzalloc(uvio_attest->meas_len, GFP_KERNEL);
> +	if (!arcb || !measurement)
> +		goto out;
> +
> +	if (uvio_attest->add_data_len) {
> +		add_data = kvzalloc(uvio_attest->add_data_len, GFP_KERNEL);
> +		if (!add_data)
> +			goto out;
> +	}
> +
> +	uvcb_attest = kzalloc(sizeof(*uvcb_attest), GFP_KERNEL);
> +	if (!uvcb_attest)
> +		goto out;
> +
> +	ret = uvio_build_uvcb_attest(uvcb_attest, arcb,  measurement, add_data, uvio_attest);
> +	if (ret)
> +		goto out;
> +
> +	uv_call_sched(0, (u64)uvcb_attest);
> +
> +	uv_ioctl->uv_rc = uvcb_attest->header.rc;
> +	uv_ioctl->uv_rrc = uvcb_attest->header.rrc;
> +
> +	ret = uvio_copy_attest_result_to_user(uvcb_attest, uv_ioctl, measurement, add_data,
> +					      uvio_attest);
> +out:
> +	kvfree(arcb);
> +	kvfree(measurement);
> +	kvfree(add_data);
> +	kfree(uvio_attest);
> +	kfree(uvcb_attest);
> +	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));

struct uvio_ioctl_cb is rather small, couldn't you just allocate it on
the stack?

> +	if (!uv_ioctl)
> +		goto out;
> +
> +	switch (cmd) {
> +	case UVIO_IOCTL_ATT:
> +		ret = uvio_copy_and_check_ioctl(uv_ioctl, argp);
> +		if (ret)
> +			goto out;
> +		ret = uvio_attestation(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");


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device
  2022-05-12 14:33   ` Claudio Imbrenda
@ 2022-05-13  7:45     ` Steffen Eiden
  2022-05-13  8:37       ` Claudio Imbrenda
  0 siblings, 1 reply; 18+ messages in thread
From: Steffen Eiden @ 2022-05-13  7:45 UTC (permalink / raw)
  To: Claudio Imbrenda
  Cc: Greg KH, Heiko Carstens, Alexander Gordeev, Shuah Khan,
	Christian Borntraeger, Janosch Frank, David Hildenbrand,
	Nico Boehr, linux-kernel, linux-kselftest, kvm



On 5/12/22 16:33, Claudio Imbrenda wrote:

[snip]

>> +/*
>> + * 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));
> struct uvio_ioctl_cb is rather small, couldn't you just allocate it on
> the stack?
> 
IIRC it was on stack in some previous version. We then had a discussion
earlier about this triggered by the inverse comment and decided to not 
use the stack.

[snip]


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device
  2022-05-13  7:45     ` Steffen Eiden
@ 2022-05-13  8:37       ` Claudio Imbrenda
  2022-05-13 12:35         ` Steffen Eiden
  0 siblings, 1 reply; 18+ messages in thread
From: Claudio Imbrenda @ 2022-05-13  8:37 UTC (permalink / raw)
  To: Steffen Eiden
  Cc: Greg KH, Heiko Carstens, Alexander Gordeev, Shuah Khan,
	Christian Borntraeger, Janosch Frank, David Hildenbrand,
	Nico Boehr, linux-kernel, linux-kselftest, kvm

On Fri, 13 May 2022 09:45:39 +0200
Steffen Eiden <seiden@linux.ibm.com> wrote:

> On 5/12/22 16:33, Claudio Imbrenda wrote:
> 
> [snip]
> 
> >> +/*
> >> + * 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));  
> > struct uvio_ioctl_cb is rather small, couldn't you just allocate it on
> > the stack?
> >   
> IIRC it was on stack in some previous version. We then had a discussion
> earlier about this triggered by the inverse comment and decided to not 
> use the stack.

ok fair enough

but what's the reason for a vzalloc instead of a kzalloc, when the
allocation is surely going to be small?

> 
> [snip]
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device
  2022-05-13  8:37       ` Claudio Imbrenda
@ 2022-05-13 12:35         ` Steffen Eiden
  0 siblings, 0 replies; 18+ messages in thread
From: Steffen Eiden @ 2022-05-13 12:35 UTC (permalink / raw)
  To: Claudio Imbrenda
  Cc: Greg KH, Heiko Carstens, Alexander Gordeev, Shuah Khan,
	Christian Borntraeger, Janosch Frank, David Hildenbrand,
	Nico Boehr, linux-kernel, linux-kselftest, kvm



On 5/13/22 10:37, Claudio Imbrenda wrote:
> On Fri, 13 May 2022 09:45:39 +0200
> Steffen Eiden <seiden@linux.ibm.com> wrote:
> 
>> On 5/12/22 16:33, Claudio Imbrenda wrote:
>>
>> [snip]
>>
>>>> +/*
>>>> + * 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));
>>> struct uvio_ioctl_cb is rather small, couldn't you just allocate it on
>>> the stack?
>>>    
>> IIRC it was on stack in some previous version. We then had a discussion
>> earlier about this triggered by the inverse comment and decided to not
>> use the stack.
> 
> ok fair enough
> 
> but what's the reason for a vzalloc instead of a kzalloc, when the
> allocation is surely going to be small?
> 
We had no strong reasons against or for vzalloc/kzalloc.
If you want me to change it to kzalloc I can do it. I still
have no strong opinion on that.

>> [snip]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device
  2022-05-10 14:47 ` [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device Steffen Eiden
  2022-05-12 14:33   ` Claudio Imbrenda
@ 2022-05-16 11:33   ` Steffen Eiden
  2022-05-16 14:51     ` Claudio Imbrenda
  2022-05-17  8:38   ` Janosch Frank
  2022-05-18 11:47   ` Heiko Carstens
  3 siblings, 1 reply; 18+ messages in thread
From: Steffen Eiden @ 2022-05-16 11:33 UTC (permalink / raw)
  To: Greg KH, Heiko Carstens, Alexander Gordeev, Shuah Khan,
	Christian Borntraeger, Janosch Frank, Claudio Imbrenda,
	David Hildenbrand, Nico Boehr, linux-kernel, linux-kselftest,
	kvm

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 can call the Retrieve Attestation Measurement
Ultravisor Call using IOCTLs on the uvdevice.

The uvdevice will do some sanity checks first.
Then, copy the request data to kernel space, build the UVCB,
perform the UV call, and copy the result back to userspace.

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
After some private discussion "struct uvio_ioctl_cb uv_ioctl"
in "uvio_ioctl()" is now in the stack.

 MAINTAINERS                           |   2 +
 arch/s390/include/asm/uv.h            |  23 ++-
 arch/s390/include/uapi/asm/uvdevice.h |  51 +++++
 drivers/s390/char/Kconfig             |  10 +
 drivers/s390/char/Makefile            |   1 +
 drivers/s390/char/uvdevice.c          | 257 ++++++++++++++++++++++++++
 6 files changed, 343 insertions(+), 1 deletion(-)
 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 e8c52d0192a6..b42ab4a35e18 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10781,9 +10781,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/asm/uv.h b/arch/s390/include/asm/uv.h
index a2d376b8bce3..cfea7b77a5b8 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -2,7 +2,7 @@
 /*
  * Ultravisor Interfaces
  *
- * Copyright IBM Corp. 2019
+ * Copyright IBM Corp. 2019, 2022
  *
  * Author(s):
  *	Vasily Gorbik <gor@linux.ibm.com>
@@ -52,6 +52,7 @@
 #define UVC_CMD_UNPIN_PAGE_SHARED	0x0342
 #define UVC_CMD_SET_SHARED_ACCESS	0x1000
 #define UVC_CMD_REMOVE_SHARED_ACCESS	0x1001
+#define UVC_CMD_RETR_ATTEST		0x1020
 
 /* Bits in installed uv calls */
 enum uv_cmds_inst {
@@ -76,6 +77,7 @@ enum uv_cmds_inst {
 	BIT_UVC_CMD_UNSHARE_ALL = 20,
 	BIT_UVC_CMD_PIN_PAGE_SHARED = 21,
 	BIT_UVC_CMD_UNPIN_PAGE_SHARED = 22,
+	BIT_UVC_CMD_RETR_ATTEST = 28,
 };
 
 enum uv_feat_ind {
@@ -219,6 +221,25 @@ struct uv_cb_share {
 	u64 reserved28;
 } __packed __aligned(8);
 
+/* Retrieve Attestation Measurement */
+struct uv_cb_attest {
+	struct uv_cb_header header;	/* 0x0000 */
+	u64 reserved08[2];		/* 0x0008 */
+	u64 arcb_addr;			/* 0x0018 */
+	u64 cont_token;			/* 0x0020 */
+	u8  reserved28[6];		/* 0x0028 */
+	u16 user_data_len;		/* 0x002e */
+	u8  user_data[256];		/* 0x0030 */
+	u32 reserved130[3];		/* 0x0130 */
+	u32 meas_len;			/* 0x013c */
+	u64 meas_addr;			/* 0x0140 */
+	u8  config_uid[16];		/* 0x0148 */
+	u32 reserved158;		/* 0x0158 */
+	u32 add_data_len;		/* 0x015c */
+	u64 add_data_addr;		/* 0x0160 */
+	u64 reserved168[4];		/* 0x0168 */
+} __packed __aligned(8);
+
 static inline int __uv_call(unsigned long r1, unsigned long r2)
 {
 	int cc;
diff --git a/arch/s390/include/uapi/asm/uvdevice.h b/arch/s390/include/uapi/asm/uvdevice.h
new file mode 100644
index 000000000000..d0bdde4969a1
--- /dev/null
+++ b/arch/s390/include/uapi/asm/uvdevice.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ *  Copyright IBM Corp. 2022
+ *  Author(s): Steffen Eiden <seiden@linux.ibm.com>
+ */
+#ifndef __S390_ASM_UVDEVICE_H
+#define __S390_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_ATT_USER_DATA_LEN		0x100
+#define UVIO_ATT_UID_LEN		0x10
+struct uvio_attest {
+	__u64 arcb_addr;				/* 0x0000 */
+	__u64 meas_addr;				/* 0x0008 */
+	__u64 add_data_addr;				/* 0x0010 */
+	__u8  user_data[UVIO_ATT_USER_DATA_LEN];	/* 0x0018 */
+	__u8  config_uid[UVIO_ATT_UID_LEN];		/* 0x0118 */
+	__u32 arcb_len;					/* 0x0128 */
+	__u32 meas_len;					/* 0x012c */
+	__u32 add_data_len;				/* 0x0130 */
+	__u16 user_data_len;				/* 0x0134 */
+	__u16 reserved136;				/* 0x0136 */
+};
+
+/*
+ * 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
+ * often way smaller. By allowing larger buffer sizes we hopefully do not need
+ * to update the code with every machine update. It is therefore possible for
+ * userspace to request more memory than actually used by kernel/UV.
+ */
+#define UVIO_ATT_ARCB_MAX_LEN		0x100000
+#define UVIO_ATT_MEASUREMENT_MAX_LEN	0x8000
+#define UVIO_ATT_ADDITIONAL_MAX_LEN	0x8000
+
+#define UVIO_DEVICE_NAME "uv"
+#define UVIO_TYPE_UVC 'u'
+
+#define UVIO_IOCTL_ATT _IOWR(UVIO_TYPE_UVC, 0x01, struct uvio_ioctl_cb)
+
+#endif  /* __S390_ASM_UVDEVICE_H */
diff --git a/drivers/s390/char/Kconfig b/drivers/s390/char/Kconfig
index 6cc4b19acf85..e9b9902abbaf 100644
--- a/drivers/s390/char/Kconfig
+++ b/drivers/s390/char/Kconfig
@@ -100,6 +100,16 @@ config SCLP_OFB
 	  This option enables the Open-for-Business interface to the s390
 	  Service Element.
 
+config S390_UV_UAPI
+	def_tristate y
+	prompt "Ultravisor userspace API"
+	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 only available if the Ultravisor
+	  Facility (158) is present.
+
 config S390_TAPE
 	def_tristate m
 	prompt "S/390 tape device support"
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..e8e1409df015
--- /dev/null
+++ b/drivers/s390/char/uvdevice.c
@@ -0,0 +1,257 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Copyright IBM Corp. 2022
+ *  Author(s): Steffen Eiden <seiden@linux.ibm.com>
+ *
+ *  This file provides a Linux misc device to give userspace access to some
+ *  Ultravisor (UV) functions. The device only accepts IOCTLs and will only
+ *  be present if the Ultravisor facility (158) is present.
+ *
+ *  When userspace sends a valid IOCTL uvdevice will copy the input data to
+ *  kernel space, do some basic validity checks to avoid kernel/system
+ *  corruption. Any other check that the Ultravisor does will not be done by
+ *  the uvdevice to keep changes minimal when adding new functionalities
+ *  to existing UV-calls.
+ *  After the checks uvdevice builds a corresponding
+ *  Ultravisor Call Control Block, and sends the request to the Ultravisor.
+ *  Then, it copies the response, including the return codes, back to userspace.
+ *  It is the responsibility of the userspace to check for any error issued
+ *  by UV and to interpret the UV response. The uvdevice acts as a communication
+ *  channel for userspace to the Ultravisor.
+ */
+
+#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>
+
+static int uvio_build_uvcb_attest(struct uv_cb_attest *uvcb_attest, u8 *arcb,
+				  u8 *meas, u8 *add_data, struct uvio_attest *uvio_attest)
+{
+	void __user *user_buf_arcb = (void __user *)uvio_attest->arcb_addr;
+
+	if (copy_from_user(arcb, user_buf_arcb, uvio_attest->arcb_len))
+		return -EFAULT;
+
+	uvcb_attest->header.len = sizeof(*uvcb_attest);
+	uvcb_attest->header.cmd = UVC_CMD_RETR_ATTEST;
+	uvcb_attest->arcb_addr = (u64)arcb;
+	uvcb_attest->cont_token = 0;
+	uvcb_attest->user_data_len = uvio_attest->user_data_len;
+	memcpy(uvcb_attest->user_data, uvio_attest->user_data, sizeof(uvcb_attest->user_data));
+	uvcb_attest->meas_len = uvio_attest->meas_len;
+	uvcb_attest->meas_addr = (u64)meas;
+	uvcb_attest->add_data_len = uvio_attest->add_data_len;
+	uvcb_attest->add_data_addr = (u64)add_data;
+
+	return 0;
+}
+
+static int uvio_copy_attest_result_to_user(struct uv_cb_attest *uvcb_attest,
+					   struct uvio_ioctl_cb *uv_ioctl,
+					   u8 *measurement, u8 *add_data,
+					   struct uvio_attest *uvio_attest)
+{
+	struct uvio_attest __user *user_uvio_attest = (void __user *)uv_ioctl->argument_addr;
+	void __user *user_buf_add = (void __user *)uvio_attest->add_data_addr;
+	void __user *user_buf_meas = (void __user *)uvio_attest->meas_addr;
+	void __user *user_buf_uid = &user_uvio_attest->config_uid;
+
+	if (copy_to_user(user_buf_meas, measurement, uvio_attest->meas_len))
+		return -EFAULT;
+	if (add_data && copy_to_user(user_buf_add, add_data, uvio_attest->add_data_len))
+		return -EFAULT;
+	if (copy_to_user(user_buf_uid, uvcb_attest->config_uid, sizeof(uvcb_attest->config_uid)))
+		return -EFAULT;
+	return 0;
+}
+
+static int get_uvio_attest(struct uvio_ioctl_cb *uv_ioctl, struct uvio_attest *uvio_attest)
+{
+	u8 __user *user_arg_buf = (u8 __user *)uv_ioctl->argument_addr;
+
+	if (copy_from_user(uvio_attest, user_arg_buf, sizeof(*uvio_attest)))
+		return -EFAULT;
+
+	if (uvio_attest->arcb_len > UVIO_ATT_ARCB_MAX_LEN)
+		return -EINVAL;
+	if (uvio_attest->arcb_len == 0)
+		return -EINVAL;
+	if (uvio_attest->meas_len > UVIO_ATT_MEASUREMENT_MAX_LEN)
+		return -EINVAL;
+	if (uvio_attest->meas_len == 0)
+		return -EINVAL;
+	if (uvio_attest->add_data_len > UVIO_ATT_ADDITIONAL_MAX_LEN)
+		return -EINVAL;
+	if (uvio_attest->reserved136)
+		return -EINVAL;
+	return 0;
+}
+
+/**
+ * uvio_attestation() - Perform a Retrieve Attestation Measurement UVC.
+ *
+ * @uv_ioctl: ioctl control block
+ *
+ * uvio_attestation() does a  Retrieve Attestation Measurement Ultravisor Call.
+ * It verifies that the given userspace addresses are valid and request sizes
+ * are sane. Every other check is made by the Ultravisor (UV) and won't result
+ * in a negative return value. It copies the input to kernelspace, builds the
+ * request, sends the UV-call, and copies the result to userspace.
+ *
+ * The Attestation Request has two input and two outputs.
+ * ARCB and User Data are inputs for the UV generated by userspace.
+ * Measurement and Additional Data are outputs for userspace generated by UV.
+ *
+ * The Attestation Request Control Block (ARCB) is a cryptographically verified
+ * and secured request to UV and User Data is some plaintext data which is
+ * going to be included in the Attestation Measurement calculation.
+ *
+ * Measurement is a cryptographic measurement of the callers properties,
+ * optional data configured by the ARCB and the user data. If specified by the
+ * ARCB, UV will add some Additional Data to the measurement calculation.
+ * This Additional Data is then returned as well.
+ *
+ * If the Retrieve Attestation Measurement 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.
+ *
+ * Context: might sleep
+ *
+ * Return: 0 on success or a negative error code on error.
+ */
+static int uvio_attestation(struct uvio_ioctl_cb *uv_ioctl)
+{
+	struct uv_cb_attest *uvcb_attest = NULL;
+	struct uvio_attest *uvio_attest = NULL;
+	u8 *measurement = NULL;
+	u8 *add_data = NULL;
+	u8 *arcb = NULL;
+	int ret;
+
+	ret = -EINVAL;
+	if (uv_ioctl->argument_len != sizeof(*uvio_attest))
+		goto out;
+
+	ret = -ENOMEM;
+	uvio_attest = kzalloc(sizeof(*uvio_attest), GFP_KERNEL);
+	if (!uvio_attest)
+		goto out;
+
+	ret = get_uvio_attest(uv_ioctl, uvio_attest);
+	if (ret)
+		goto out;
+
+	ret = -ENOMEM;
+	arcb = kvzalloc(uvio_attest->arcb_len, GFP_KERNEL);
+	measurement = kvzalloc(uvio_attest->meas_len, GFP_KERNEL);
+	if (!arcb || !measurement)
+		goto out;
+
+	if (uvio_attest->add_data_len) {
+		add_data = kvzalloc(uvio_attest->add_data_len, GFP_KERNEL);
+		if (!add_data)
+			goto out;
+	}
+
+	uvcb_attest = kzalloc(sizeof(*uvcb_attest), GFP_KERNEL);
+	if (!uvcb_attest)
+		goto out;
+
+	ret = uvio_build_uvcb_attest(uvcb_attest, arcb,  measurement, add_data, uvio_attest);
+	if (ret)
+		goto out;
+
+	uv_call_sched(0, (u64)uvcb_attest);
+
+	uv_ioctl->uv_rc = uvcb_attest->header.rc;
+	uv_ioctl->uv_rrc = uvcb_attest->header.rrc;
+
+	ret = uvio_copy_attest_result_to_user(uvcb_attest, uv_ioctl, measurement, add_data,
+					      uvio_attest);
+out:
+	kvfree(arcb);
+	kvfree(measurement);
+	kvfree(add_data);
+	kfree(uvio_attest);
+	kfree(uvcb_attest);
+	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;
+
+	switch (cmd) {
+	case UVIO_IOCTL_ATT:
+		ret = uvio_copy_and_check_ioctl(&uv_ioctl, argp);
+		if (ret)
+			return ret;
+		ret = uvio_attestation(&uv_ioctl);
+		break;
+	default:
+		ret = -ENOIOCTLCMD;
+		break;
+	}
+	if (ret)
+		return ret;
+
+	if (copy_to_user(argp, &uv_ioctl, sizeof(uv_ioctl)))
+		ret = -EFAULT;
+
+	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.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device
  2022-05-16 11:33   ` Steffen Eiden
@ 2022-05-16 14:51     ` Claudio Imbrenda
  0 siblings, 0 replies; 18+ messages in thread
From: Claudio Imbrenda @ 2022-05-16 14:51 UTC (permalink / raw)
  To: Steffen Eiden
  Cc: Greg KH, Heiko Carstens, Alexander Gordeev, Shuah Khan,
	Christian Borntraeger, Janosch Frank, David Hildenbrand,
	Nico Boehr, linux-kernel, linux-kselftest, kvm

On Mon, 16 May 2022 11:33:35 +0000
Steffen Eiden <seiden@linux.ibm.com> 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 can call the Retrieve Attestation Measurement
> Ultravisor Call using IOCTLs on the uvdevice.
> 
> The uvdevice will do some sanity checks first.
> Then, copy the request data to kernel space, build the UVCB,
> perform the UV call, and copy the result back to userspace.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
> After some private discussion "struct uvio_ioctl_cb uv_ioctl"
> in "uvio_ioctl()" is now in the stack.
> 
>  MAINTAINERS                           |   2 +
>  arch/s390/include/asm/uv.h            |  23 ++-
>  arch/s390/include/uapi/asm/uvdevice.h |  51 +++++
>  drivers/s390/char/Kconfig             |  10 +
>  drivers/s390/char/Makefile            |   1 +
>  drivers/s390/char/uvdevice.c          | 257 ++++++++++++++++++++++++++
>  6 files changed, 343 insertions(+), 1 deletion(-)
>  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 e8c52d0192a6..b42ab4a35e18 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10781,9 +10781,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/asm/uv.h b/arch/s390/include/asm/uv.h
> index a2d376b8bce3..cfea7b77a5b8 100644
> --- a/arch/s390/include/asm/uv.h
> +++ b/arch/s390/include/asm/uv.h
> @@ -2,7 +2,7 @@
>  /*
>   * Ultravisor Interfaces
>   *
> - * Copyright IBM Corp. 2019
> + * Copyright IBM Corp. 2019, 2022
>   *
>   * Author(s):
>   *	Vasily Gorbik <gor@linux.ibm.com>
> @@ -52,6 +52,7 @@
>  #define UVC_CMD_UNPIN_PAGE_SHARED	0x0342
>  #define UVC_CMD_SET_SHARED_ACCESS	0x1000
>  #define UVC_CMD_REMOVE_SHARED_ACCESS	0x1001
> +#define UVC_CMD_RETR_ATTEST		0x1020
>  
>  /* Bits in installed uv calls */
>  enum uv_cmds_inst {
> @@ -76,6 +77,7 @@ enum uv_cmds_inst {
>  	BIT_UVC_CMD_UNSHARE_ALL = 20,
>  	BIT_UVC_CMD_PIN_PAGE_SHARED = 21,
>  	BIT_UVC_CMD_UNPIN_PAGE_SHARED = 22,
> +	BIT_UVC_CMD_RETR_ATTEST = 28,
>  };
>  
>  enum uv_feat_ind {
> @@ -219,6 +221,25 @@ struct uv_cb_share {
>  	u64 reserved28;
>  } __packed __aligned(8);
>  
> +/* Retrieve Attestation Measurement */
> +struct uv_cb_attest {
> +	struct uv_cb_header header;	/* 0x0000 */
> +	u64 reserved08[2];		/* 0x0008 */
> +	u64 arcb_addr;			/* 0x0018 */
> +	u64 cont_token;			/* 0x0020 */
> +	u8  reserved28[6];		/* 0x0028 */
> +	u16 user_data_len;		/* 0x002e */
> +	u8  user_data[256];		/* 0x0030 */
> +	u32 reserved130[3];		/* 0x0130 */
> +	u32 meas_len;			/* 0x013c */
> +	u64 meas_addr;			/* 0x0140 */
> +	u8  config_uid[16];		/* 0x0148 */
> +	u32 reserved158;		/* 0x0158 */
> +	u32 add_data_len;		/* 0x015c */
> +	u64 add_data_addr;		/* 0x0160 */
> +	u64 reserved168[4];		/* 0x0168 */
> +} __packed __aligned(8);
> +
>  static inline int __uv_call(unsigned long r1, unsigned long r2)
>  {
>  	int cc;
> diff --git a/arch/s390/include/uapi/asm/uvdevice.h b/arch/s390/include/uapi/asm/uvdevice.h
> new file mode 100644
> index 000000000000..d0bdde4969a1
> --- /dev/null
> +++ b/arch/s390/include/uapi/asm/uvdevice.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + *  Copyright IBM Corp. 2022
> + *  Author(s): Steffen Eiden <seiden@linux.ibm.com>
> + */
> +#ifndef __S390_ASM_UVDEVICE_H
> +#define __S390_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_ATT_USER_DATA_LEN		0x100
> +#define UVIO_ATT_UID_LEN		0x10
> +struct uvio_attest {
> +	__u64 arcb_addr;				/* 0x0000 */
> +	__u64 meas_addr;				/* 0x0008 */
> +	__u64 add_data_addr;				/* 0x0010 */
> +	__u8  user_data[UVIO_ATT_USER_DATA_LEN];	/* 0x0018 */
> +	__u8  config_uid[UVIO_ATT_UID_LEN];		/* 0x0118 */
> +	__u32 arcb_len;					/* 0x0128 */
> +	__u32 meas_len;					/* 0x012c */
> +	__u32 add_data_len;				/* 0x0130 */
> +	__u16 user_data_len;				/* 0x0134 */
> +	__u16 reserved136;				/* 0x0136 */
> +};
> +
> +/*
> + * 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
> + * often way smaller. By allowing larger buffer sizes we hopefully do not need
> + * to update the code with every machine update. It is therefore possible for
> + * userspace to request more memory than actually used by kernel/UV.
> + */
> +#define UVIO_ATT_ARCB_MAX_LEN		0x100000
> +#define UVIO_ATT_MEASUREMENT_MAX_LEN	0x8000
> +#define UVIO_ATT_ADDITIONAL_MAX_LEN	0x8000
> +
> +#define UVIO_DEVICE_NAME "uv"
> +#define UVIO_TYPE_UVC 'u'
> +
> +#define UVIO_IOCTL_ATT _IOWR(UVIO_TYPE_UVC, 0x01, struct uvio_ioctl_cb)
> +
> +#endif  /* __S390_ASM_UVDEVICE_H */
> diff --git a/drivers/s390/char/Kconfig b/drivers/s390/char/Kconfig
> index 6cc4b19acf85..e9b9902abbaf 100644
> --- a/drivers/s390/char/Kconfig
> +++ b/drivers/s390/char/Kconfig
> @@ -100,6 +100,16 @@ config SCLP_OFB
>  	  This option enables the Open-for-Business interface to the s390
>  	  Service Element.
>  
> +config S390_UV_UAPI
> +	def_tristate y
> +	prompt "Ultravisor userspace API"
> +	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 only available if the Ultravisor
> +	  Facility (158) is present.
> +
>  config S390_TAPE
>  	def_tristate m
>  	prompt "S/390 tape device support"
> 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..e8e1409df015
> --- /dev/null
> +++ b/drivers/s390/char/uvdevice.c
> @@ -0,0 +1,257 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  Copyright IBM Corp. 2022
> + *  Author(s): Steffen Eiden <seiden@linux.ibm.com>
> + *
> + *  This file provides a Linux misc device to give userspace access to some
> + *  Ultravisor (UV) functions. The device only accepts IOCTLs and will only
> + *  be present if the Ultravisor facility (158) is present.
> + *
> + *  When userspace sends a valid IOCTL uvdevice will copy the input data to
> + *  kernel space, do some basic validity checks to avoid kernel/system
> + *  corruption. Any other check that the Ultravisor does will not be done by
> + *  the uvdevice to keep changes minimal when adding new functionalities
> + *  to existing UV-calls.
> + *  After the checks uvdevice builds a corresponding
> + *  Ultravisor Call Control Block, and sends the request to the Ultravisor.
> + *  Then, it copies the response, including the return codes, back to userspace.
> + *  It is the responsibility of the userspace to check for any error issued
> + *  by UV and to interpret the UV response. The uvdevice acts as a communication
> + *  channel for userspace to the Ultravisor.
> + */
> +
> +#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>
> +
> +static int uvio_build_uvcb_attest(struct uv_cb_attest *uvcb_attest, u8 *arcb,
> +				  u8 *meas, u8 *add_data, struct uvio_attest *uvio_attest)
> +{
> +	void __user *user_buf_arcb = (void __user *)uvio_attest->arcb_addr;
> +
> +	if (copy_from_user(arcb, user_buf_arcb, uvio_attest->arcb_len))
> +		return -EFAULT;
> +
> +	uvcb_attest->header.len = sizeof(*uvcb_attest);
> +	uvcb_attest->header.cmd = UVC_CMD_RETR_ATTEST;
> +	uvcb_attest->arcb_addr = (u64)arcb;
> +	uvcb_attest->cont_token = 0;
> +	uvcb_attest->user_data_len = uvio_attest->user_data_len;
> +	memcpy(uvcb_attest->user_data, uvio_attest->user_data, sizeof(uvcb_attest->user_data));
> +	uvcb_attest->meas_len = uvio_attest->meas_len;
> +	uvcb_attest->meas_addr = (u64)meas;
> +	uvcb_attest->add_data_len = uvio_attest->add_data_len;
> +	uvcb_attest->add_data_addr = (u64)add_data;
> +
> +	return 0;
> +}
> +
> +static int uvio_copy_attest_result_to_user(struct uv_cb_attest *uvcb_attest,
> +					   struct uvio_ioctl_cb *uv_ioctl,
> +					   u8 *measurement, u8 *add_data,
> +					   struct uvio_attest *uvio_attest)
> +{
> +	struct uvio_attest __user *user_uvio_attest = (void __user *)uv_ioctl->argument_addr;
> +	void __user *user_buf_add = (void __user *)uvio_attest->add_data_addr;
> +	void __user *user_buf_meas = (void __user *)uvio_attest->meas_addr;
> +	void __user *user_buf_uid = &user_uvio_attest->config_uid;
> +
> +	if (copy_to_user(user_buf_meas, measurement, uvio_attest->meas_len))
> +		return -EFAULT;
> +	if (add_data && copy_to_user(user_buf_add, add_data, uvio_attest->add_data_len))
> +		return -EFAULT;
> +	if (copy_to_user(user_buf_uid, uvcb_attest->config_uid, sizeof(uvcb_attest->config_uid)))
> +		return -EFAULT;
> +	return 0;
> +}
> +
> +static int get_uvio_attest(struct uvio_ioctl_cb *uv_ioctl, struct uvio_attest *uvio_attest)
> +{
> +	u8 __user *user_arg_buf = (u8 __user *)uv_ioctl->argument_addr;
> +
> +	if (copy_from_user(uvio_attest, user_arg_buf, sizeof(*uvio_attest)))
> +		return -EFAULT;
> +
> +	if (uvio_attest->arcb_len > UVIO_ATT_ARCB_MAX_LEN)
> +		return -EINVAL;
> +	if (uvio_attest->arcb_len == 0)
> +		return -EINVAL;
> +	if (uvio_attest->meas_len > UVIO_ATT_MEASUREMENT_MAX_LEN)
> +		return -EINVAL;
> +	if (uvio_attest->meas_len == 0)
> +		return -EINVAL;
> +	if (uvio_attest->add_data_len > UVIO_ATT_ADDITIONAL_MAX_LEN)
> +		return -EINVAL;
> +	if (uvio_attest->reserved136)
> +		return -EINVAL;
> +	return 0;
> +}
> +
> +/**
> + * uvio_attestation() - Perform a Retrieve Attestation Measurement UVC.
> + *
> + * @uv_ioctl: ioctl control block
> + *
> + * uvio_attestation() does a  Retrieve Attestation Measurement Ultravisor Call.
> + * It verifies that the given userspace addresses are valid and request sizes
> + * are sane. Every other check is made by the Ultravisor (UV) and won't result
> + * in a negative return value. It copies the input to kernelspace, builds the
> + * request, sends the UV-call, and copies the result to userspace.
> + *
> + * The Attestation Request has two input and two outputs.
> + * ARCB and User Data are inputs for the UV generated by userspace.
> + * Measurement and Additional Data are outputs for userspace generated by UV.
> + *
> + * The Attestation Request Control Block (ARCB) is a cryptographically verified
> + * and secured request to UV and User Data is some plaintext data which is
> + * going to be included in the Attestation Measurement calculation.
> + *
> + * Measurement is a cryptographic measurement of the callers properties,
> + * optional data configured by the ARCB and the user data. If specified by the
> + * ARCB, UV will add some Additional Data to the measurement calculation.
> + * This Additional Data is then returned as well.
> + *
> + * If the Retrieve Attestation Measurement 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.
> + *
> + * Context: might sleep
> + *
> + * Return: 0 on success or a negative error code on error.
> + */
> +static int uvio_attestation(struct uvio_ioctl_cb *uv_ioctl)
> +{
> +	struct uv_cb_attest *uvcb_attest = NULL;
> +	struct uvio_attest *uvio_attest = NULL;
> +	u8 *measurement = NULL;
> +	u8 *add_data = NULL;
> +	u8 *arcb = NULL;
> +	int ret;
> +
> +	ret = -EINVAL;
> +	if (uv_ioctl->argument_len != sizeof(*uvio_attest))
> +		goto out;
> +
> +	ret = -ENOMEM;
> +	uvio_attest = kzalloc(sizeof(*uvio_attest), GFP_KERNEL);
> +	if (!uvio_attest)
> +		goto out;
> +
> +	ret = get_uvio_attest(uv_ioctl, uvio_attest);
> +	if (ret)
> +		goto out;
> +
> +	ret = -ENOMEM;
> +	arcb = kvzalloc(uvio_attest->arcb_len, GFP_KERNEL);
> +	measurement = kvzalloc(uvio_attest->meas_len, GFP_KERNEL);
> +	if (!arcb || !measurement)
> +		goto out;
> +
> +	if (uvio_attest->add_data_len) {
> +		add_data = kvzalloc(uvio_attest->add_data_len, GFP_KERNEL);
> +		if (!add_data)
> +			goto out;
> +	}
> +
> +	uvcb_attest = kzalloc(sizeof(*uvcb_attest), GFP_KERNEL);
> +	if (!uvcb_attest)
> +		goto out;
> +
> +	ret = uvio_build_uvcb_attest(uvcb_attest, arcb,  measurement, add_data, uvio_attest);
> +	if (ret)
> +		goto out;
> +
> +	uv_call_sched(0, (u64)uvcb_attest);
> +
> +	uv_ioctl->uv_rc = uvcb_attest->header.rc;
> +	uv_ioctl->uv_rrc = uvcb_attest->header.rrc;
> +
> +	ret = uvio_copy_attest_result_to_user(uvcb_attest, uv_ioctl, measurement, add_data,
> +					      uvio_attest);
> +out:
> +	kvfree(arcb);
> +	kvfree(measurement);
> +	kvfree(add_data);
> +	kfree(uvio_attest);
> +	kfree(uvcb_attest);
> +	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;
> +
> +	switch (cmd) {
> +	case UVIO_IOCTL_ATT:
> +		ret = uvio_copy_and_check_ioctl(&uv_ioctl, argp);
> +		if (ret)
> +			return ret;
> +		ret = uvio_attestation(&uv_ioctl);
> +		break;
> +	default:
> +		ret = -ENOIOCTLCMD;
> +		break;
> +	}
> +	if (ret)
> +		return ret;
> +
> +	if (copy_to_user(argp, &uv_ioctl, sizeof(uv_ioctl)))
> +		ret = -EFAULT;
> +
> +	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");


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device
  2022-05-10 14:47 ` [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device Steffen Eiden
  2022-05-12 14:33   ` Claudio Imbrenda
  2022-05-16 11:33   ` Steffen Eiden
@ 2022-05-17  8:38   ` Janosch Frank
  2022-05-17  8:42     ` Steffen Eiden
  2022-05-18 11:47   ` Heiko Carstens
  3 siblings, 1 reply; 18+ messages in thread
From: Janosch Frank @ 2022-05-17  8:38 UTC (permalink / raw)
  To: Steffen Eiden, Greg KH, Heiko Carstens, Alexander Gordeev,
	Shuah Khan, Christian Borntraeger, Claudio Imbrenda,
	David Hildenbrand, Nico Boehr, linux-kernel, linux-kselftest,
	kvm

On 5/10/22 16:47, 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 can call the Retrieve Attestation Measurement
> Ultravisor Call using IOCTLs on the uvdevice.
> 
> The uvdevice will do some sanity checks first.
> Then, copy the request data to kernel space, build the UVCB,
> perform the UV call, and copy the result back to userspace.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

I'd like to pick this if I'm allowed to fix the two white space problems 
below.

> ---
>   MAINTAINERS                           |   2 +
>   arch/s390/include/asm/uv.h            |  23 ++-
>   arch/s390/include/uapi/asm/uvdevice.h |  51 +++++
>   drivers/s390/char/Kconfig             |  10 +
>   drivers/s390/char/Makefile            |   1 +
>   drivers/s390/char/uvdevice.c          | 264 ++++++++++++++++++++++++++
>   6 files changed, 350 insertions(+), 1 deletion(-)
>   create mode 100644 arch/s390/include/uapi/asm/uvdevice.h
>   create mode 100644 drivers/s390/char/uvdevice.c
> 

> +#endif  /* __S390_ASM_UVDEVICE_H */

There are two spaces between the "endif" and the "/"

> diff --git a/drivers/s390/char/Kconfig b/drivers/s390/char/Kconfig
> index 6cc4b19acf85..e9b9902abbaf 100644
> --- a/drivers/s390/char/Kconfig
> +++ b/drivers/s390/char/Kconfig
> @@ -100,6 +100,16 @@ config SCLP_OFB
>   	  This option enables the Open-for-Business interface to the s390
>   	  Service Element.
>   
[...]
> + * uvio_attestation() does a  Retrieve Attestation Measurement Ultravisor Call.

Double space

> + * It verifies that the given userspace addresses are valid and request sizes
> + * are sane. Every other check is made by the Ultravisor (UV) and won't result
> + * in a negative return value. It copies the input to kernelspace, builds the
> + * request, sends the UV-call, and copies the result to userspace.
> + *


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device
  2022-05-17  8:38   ` Janosch Frank
@ 2022-05-17  8:42     ` Steffen Eiden
  2022-05-17 12:41       ` Janosch Frank
  0 siblings, 1 reply; 18+ messages in thread
From: Steffen Eiden @ 2022-05-17  8:42 UTC (permalink / raw)
  To: Janosch Frank, Greg KH, Heiko Carstens, Alexander Gordeev,
	Shuah Khan, Christian Borntraeger, Claudio Imbrenda,
	David Hildenbrand, Nico Boehr, linux-kernel, linux-kselftest,
	kvm



On 5/17/22 10:38, Janosch Frank wrote:
> On 5/10/22 16:47, 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 can call the Retrieve Attestation Measurement
>> Ultravisor Call using IOCTLs on the uvdevice.
>>
>> The uvdevice will do some sanity checks first.
>> Then, copy the request data to kernel space, build the UVCB,
>> perform the UV call, and copy the result back to userspace.
>>
>> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> 
> I'd like to pick this if I'm allowed to fix the two white space problems 
> below.
I am fine with that.

Thank you.

> 
>> ---
>>   MAINTAINERS                           |   2 +
>>   arch/s390/include/asm/uv.h            |  23 ++-
>>   arch/s390/include/uapi/asm/uvdevice.h |  51 +++++
>>   drivers/s390/char/Kconfig             |  10 +
>>   drivers/s390/char/Makefile            |   1 +
>>   drivers/s390/char/uvdevice.c          | 264 ++++++++++++++++++++++++++
>>   6 files changed, 350 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/s390/include/uapi/asm/uvdevice.h
>>   create mode 100644 drivers/s390/char/uvdevice.c
>>
> 
>> +#endif  /* __S390_ASM_UVDEVICE_H */
> 
> There are two spaces between the "endif" and the "/"
> 
>> diff --git a/drivers/s390/char/Kconfig b/drivers/s390/char/Kconfig
>> index 6cc4b19acf85..e9b9902abbaf 100644
>> --- a/drivers/s390/char/Kconfig
>> +++ b/drivers/s390/char/Kconfig
>> @@ -100,6 +100,16 @@ config SCLP_OFB
>>         This option enables the Open-for-Business interface to the s390
>>         Service Element.
> [...]
>> + * uvio_attestation() does a  Retrieve Attestation Measurement 
>> Ultravisor Call.
> 
> Double space
> 
>> + * It verifies that the given userspace addresses are valid and 
>> request sizes
>> + * are sane. Every other check is made by the Ultravisor (UV) and 
>> won't result
>> + * in a negative return value. It copies the input to kernelspace, 
>> builds the
>> + * request, sends the UV-call, and copies the result to userspace.
>> + *
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device
  2022-05-17  8:42     ` Steffen Eiden
@ 2022-05-17 12:41       ` Janosch Frank
  0 siblings, 0 replies; 18+ messages in thread
From: Janosch Frank @ 2022-05-17 12:41 UTC (permalink / raw)
  To: Steffen Eiden, Greg KH, Heiko Carstens, Alexander Gordeev,
	Shuah Khan, Christian Borntraeger, Claudio Imbrenda,
	David Hildenbrand, Nico Boehr, linux-kernel, linux-kselftest,
	kvm

On 5/17/22 10:42, Steffen Eiden wrote:
> 
> 
> On 5/17/22 10:38, Janosch Frank wrote:
>> On 5/10/22 16:47, 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 can call the Retrieve Attestation Measurement
>>> Ultravisor Call using IOCTLs on the uvdevice.
>>>
>>> The uvdevice will do some sanity checks first.
>>> Then, copy the request data to kernel space, build the UVCB,
>>> perform the UV call, and copy the result back to userspace.
>>>
>>> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
>>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
>>
>> I'd like to pick this if I'm allowed to fix the two white space problems
>> below.
> I am fine with that.

Thanks, picked

> 
> Thank you.
> 
>>
>>> ---
>>>    MAINTAINERS                           |   2 +
>>>    arch/s390/include/asm/uv.h            |  23 ++-
>>>    arch/s390/include/uapi/asm/uvdevice.h |  51 +++++
>>>    drivers/s390/char/Kconfig             |  10 +
>>>    drivers/s390/char/Makefile            |   1 +
>>>    drivers/s390/char/uvdevice.c          | 264 ++++++++++++++++++++++++++
>>>    6 files changed, 350 insertions(+), 1 deletion(-)
>>>    create mode 100644 arch/s390/include/uapi/asm/uvdevice.h
>>>    create mode 100644 drivers/s390/char/uvdevice.c
>>>
>>
>>> +#endif  /* __S390_ASM_UVDEVICE_H */
>>
>> There are two spaces between the "endif" and the "/"
>>
>>> diff --git a/drivers/s390/char/Kconfig b/drivers/s390/char/Kconfig
>>> index 6cc4b19acf85..e9b9902abbaf 100644
>>> --- a/drivers/s390/char/Kconfig
>>> +++ b/drivers/s390/char/Kconfig
>>> @@ -100,6 +100,16 @@ config SCLP_OFB
>>>          This option enables the Open-for-Business interface to the s390
>>>          Service Element.
>> [...]
>>> + * uvio_attestation() does a  Retrieve Attestation Measurement
>>> Ultravisor Call.
>>
>> Double space
>>
>>> + * It verifies that the given userspace addresses are valid and
>>> request sizes
>>> + * are sane. Every other check is made by the Ultravisor (UV) and
>>> won't result
>>> + * in a negative return value. It copies the input to kernelspace,
>>> builds the
>>> + * request, sends the UV-call, and copies the result to userspace.
>>> + *
>>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device
  2022-05-10 14:47 ` [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device Steffen Eiden
                     ` (2 preceding siblings ...)
  2022-05-17  8:38   ` Janosch Frank
@ 2022-05-18 11:47   ` Heiko Carstens
  2022-05-18 13:45     ` Janosch Frank
  3 siblings, 1 reply; 18+ messages in thread
From: Heiko Carstens @ 2022-05-18 11:47 UTC (permalink / raw)
  To: Steffen Eiden
  Cc: Greg KH, Alexander Gordeev, Shuah Khan, Christian Borntraeger,
	Janosch Frank, Claudio Imbrenda, David Hildenbrand, Nico Boehr,
	linux-kernel, linux-kselftest, kvm

On Tue, May 10, 2022 at 02:47:23PM +0000, Steffen Eiden wrote:
> diff --git a/drivers/s390/char/Kconfig b/drivers/s390/char/Kconfig
> index 6cc4b19acf85..e9b9902abbaf 100644
> --- a/drivers/s390/char/Kconfig
> +++ b/drivers/s390/char/Kconfig
> @@ -100,6 +100,16 @@ config SCLP_OFB
>  	  This option enables the Open-for-Business interface to the s390
>  	  Service Element.
>  
> +config S390_UV_UAPI
> +	def_tristate y
> +	prompt "Ultravisor userspace API"
> +	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 only available if the Ultravisor
> +	  Facility (158) is present.

Is there a reason why this is default "y"? If you think this should be
compiled into the kernel if used, then why allow to make it a module
at all?
Instead you could get rid of a couple if lines of code.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device
  2022-05-18 11:47   ` Heiko Carstens
@ 2022-05-18 13:45     ` Janosch Frank
  2022-05-18 13:49       ` Steffen Eiden
  2022-05-19  5:37       ` Heiko Carstens
  0 siblings, 2 replies; 18+ messages in thread
From: Janosch Frank @ 2022-05-18 13:45 UTC (permalink / raw)
  To: Heiko Carstens, Steffen Eiden
  Cc: Greg KH, Alexander Gordeev, Shuah Khan, Christian Borntraeger,
	Claudio Imbrenda, David Hildenbrand, Nico Boehr, linux-kernel,
	linux-kselftest, kvm

On 5/18/22 13:47, Heiko Carstens wrote:
> On Tue, May 10, 2022 at 02:47:23PM +0000, Steffen Eiden wrote:
>> diff --git a/drivers/s390/char/Kconfig b/drivers/s390/char/Kconfig
>> index 6cc4b19acf85..e9b9902abbaf 100644
>> --- a/drivers/s390/char/Kconfig
>> +++ b/drivers/s390/char/Kconfig
>> @@ -100,6 +100,16 @@ config SCLP_OFB
>>   	  This option enables the Open-for-Business interface to the s390
>>   	  Service Element.
>>   
>> +config S390_UV_UAPI
>> +	def_tristate y
>> +	prompt "Ultravisor userspace API"
>> +	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 only available if the Ultravisor
>> +	  Facility (158) is present.
> 
> Is there a reason why this is default "y"? If you think this should be
> compiled into the kernel if used, then why allow to make it a module
> at all?
> Instead you could get rid of a couple if lines of code.

There was a lot of discussion around this already and the "Y" was chosen 
as auto-loading this is a pain and therefore the SCLP and CHSC-Misc set 
it to Y and we took that as an example (Steffen spoke to Peter to get 
guidance).

I'm sure that we want the possibility to have this as a module. 
Personally I'd choose "m" over "y" since the module is only useful for a 
very small amount of users.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device
  2022-05-18 13:45     ` Janosch Frank
@ 2022-05-18 13:49       ` Steffen Eiden
  2022-05-19  5:37       ` Heiko Carstens
  1 sibling, 0 replies; 18+ messages in thread
From: Steffen Eiden @ 2022-05-18 13:49 UTC (permalink / raw)
  To: Janosch Frank, Heiko Carstens
  Cc: Greg KH, Alexander Gordeev, Shuah Khan, Christian Borntraeger,
	Claudio Imbrenda, David Hildenbrand, Nico Boehr, linux-kernel,
	linux-kselftest, kvm



On 5/18/22 15:45, Janosch Frank wrote:
> On 5/18/22 13:47, Heiko Carstens wrote:
>> On Tue, May 10, 2022 at 02:47:23PM +0000, Steffen Eiden wrote:
>>> diff --git a/drivers/s390/char/Kconfig b/drivers/s390/char/Kconfig
>>> index 6cc4b19acf85..e9b9902abbaf 100644
>>> --- a/drivers/s390/char/Kconfig
>>> +++ b/drivers/s390/char/Kconfig
>>> @@ -100,6 +100,16 @@ config SCLP_OFB
>>>         This option enables the Open-for-Business interface to the s390
>>>         Service Element.
>>> +config S390_UV_UAPI
>>> +    def_tristate y
>>> +    prompt "Ultravisor userspace API"
>>> +    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 only available if the Ultravisor
>>> +      Facility (158) is present.
>>
>> Is there a reason why this is default "y"? If you think this should be
>> compiled into the kernel if used, then why allow to make it a module
>> at all?
>> Instead you could get rid of a couple if lines of code.
> 
> There was a lot of discussion around this already and the "Y" was chosen 
> as auto-loading this is a pain and therefore the SCLP and CHSC-Misc set 
> it to Y and we took that as an example (Steffen spoke to Peter to get 
> guidance).
> 
> I'm sure that we want the possibility to have this as a module. 
> Personally I'd choose "m" over "y" since the module is only useful for a 
> very small amount of users.

I am fine with changing "y" to "m".

Steffen


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device
  2022-05-18 13:45     ` Janosch Frank
  2022-05-18 13:49       ` Steffen Eiden
@ 2022-05-19  5:37       ` Heiko Carstens
  2022-05-19  9:13         ` Janosch Frank
  1 sibling, 1 reply; 18+ messages in thread
From: Heiko Carstens @ 2022-05-19  5:37 UTC (permalink / raw)
  To: Janosch Frank
  Cc: Steffen Eiden, Greg KH, Alexander Gordeev, Shuah Khan,
	Christian Borntraeger, Claudio Imbrenda, David Hildenbrand,
	Nico Boehr, linux-kernel, linux-kselftest, kvm

On Wed, May 18, 2022 at 03:45:27PM +0200, Janosch Frank wrote:
> > > +	  The device is only available if the Ultravisor
> > > +	  Facility (158) is present.
> > 
> > Is there a reason why this is default "y"? If you think this should be
> > compiled into the kernel if used, then why allow to make it a module
> > at all?
> > Instead you could get rid of a couple if lines of code.
> 
> There was a lot of discussion around this already and the "Y" was chosen as
> auto-loading this is a pain and therefore the SCLP and CHSC-Misc set it to Y
> and we took that as an example (Steffen spoke to Peter to get guidance).
> 
> I'm sure that we want the possibility to have this as a module. Personally
> I'd choose "m" over "y" since the module is only useful for a very small
> amount of users.

Why not simply use module_cpu_feature_match() to implement auto module
loading like we do it for the crypto modules? That would require that
either the uv facility is represented within elf hwcaps, or
alternatively the s390 implementation of cpu_feature() needs to be
changed to work with cpu facilities instead of hwcap bits.
(see arch/s390/include/asm/cpufeature.h)

This doesn't look too difficult. Or was there a reason not to go this route?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device
  2022-05-19  5:37       ` Heiko Carstens
@ 2022-05-19  9:13         ` Janosch Frank
  0 siblings, 0 replies; 18+ messages in thread
From: Janosch Frank @ 2022-05-19  9:13 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Steffen Eiden, Greg KH, Alexander Gordeev, Shuah Khan,
	Christian Borntraeger, Claudio Imbrenda, David Hildenbrand,
	Nico Boehr, linux-kernel, linux-kselftest, kvm

On 5/19/22 07:37, Heiko Carstens wrote:
> On Wed, May 18, 2022 at 03:45:27PM +0200, Janosch Frank wrote:
>>>> +	  The device is only available if the Ultravisor
>>>> +	  Facility (158) is present.
>>>
>>> Is there a reason why this is default "y"? If you think this should be
>>> compiled into the kernel if used, then why allow to make it a module
>>> at all?
>>> Instead you could get rid of a couple if lines of code.
>>
>> There was a lot of discussion around this already and the "Y" was chosen as
>> auto-loading this is a pain and therefore the SCLP and CHSC-Misc set it to Y
>> and we took that as an example (Steffen spoke to Peter to get guidance).
>>
>> I'm sure that we want the possibility to have this as a module. Personally
>> I'd choose "m" over "y" since the module is only useful for a very small
>> amount of users.
> 
> Why not simply use module_cpu_feature_match() to implement auto module
> loading like we do it for the crypto modules? That would require that
> either the uv facility is represented within elf hwcaps, or
> alternatively the s390 implementation of cpu_feature() needs to be
> changed to work with cpu facilities instead of hwcap bits.
> (see arch/s390/include/asm/cpufeature.h)
> 
> This doesn't look too difficult. Or was there a reason not to go this route?

I'd guess we looked into the wrong direction for auto-load.

We'll look into that for 5.20 but we'll take this patch with the "m" 
tristate for 5.19.

Thanks for the pointer and review.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 2/2] selftests: drivers/s390x: Add uvdevice tests
  2022-05-10 14:47 ` [PATCH v4 2/2] selftests: drivers/s390x: Add uvdevice tests Steffen Eiden
@ 2022-05-21 13:53   ` Muhammad Usama Anjum
  0 siblings, 0 replies; 18+ messages in thread
From: Muhammad Usama Anjum @ 2022-05-21 13:53 UTC (permalink / raw)
  To: Steffen Eiden, Greg KH, Heiko Carstens, Alexander Gordeev,
	Shuah Khan, Christian Borntraeger, Janosch Frank,
	Claudio Imbrenda, David Hildenbrand, Nico Boehr, linux-kernel,
	linux-kselftest, kvm
  Cc: usama.anjum

On 5/10/22 7:47 PM, Steffen Eiden wrote:
> Adds some selftests to test ioctl error paths of the uv-uapi.
> The Kconfig S390_UV_UAPI must be selected and the Ultravisor facility
> must be available. The test can be executed by non-root, however, the
> uvdevice special file /dev/uv must be accessible for reading and
> writing which may imply root privileges.
> 
>   ./test-uv-device
>   TAP version 13
>   1..6
>   # Starting 6 tests from 3 test cases.
>   #  RUN           uvio_fixture.att.fault_ioctl_arg ...
>   #            OK  uvio_fixture.att.fault_ioctl_arg
>   ok 1 uvio_fixture.att.fault_ioctl_arg
>   #  RUN           uvio_fixture.att.fault_uvio_arg ...
>   #            OK  uvio_fixture.att.fault_uvio_arg
>   ok 2 uvio_fixture.att.fault_uvio_arg
>   #  RUN           uvio_fixture.att.inval_ioctl_cb ...
>   #            OK  uvio_fixture.att.inval_ioctl_cb
>   ok 3 uvio_fixture.att.inval_ioctl_cb
>   #  RUN           uvio_fixture.att.inval_ioctl_cmd ...
>   #            OK  uvio_fixture.att.inval_ioctl_cmd
>   ok 4 uvio_fixture.att.inval_ioctl_cmd
>   #  RUN           attest_fixture.att_inval_request ...
>   #            OK  attest_fixture.att_inval_request
>   ok 5 attest_fixture.att_inval_request
>   #  RUN           attest_fixture.att_inval_addr ...
>   #            OK  attest_fixture.att_inval_addr
>   ok 6 attest_fixture.att_inval_addr
>   # PASSED: 6 / 6 tests passed.
>   # Totals: pass:6 fail:0 xfail:0 xpass:0 skip:0 error:0
Maybe we shouldn't add the result like this in the commit message. The
test order may change and it may become invalid soon. Just put it after
the commit message.

> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> Acked-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  MAINTAINERS                                   |   1 +
>  tools/testing/selftests/Makefile              |   1 +
>  tools/testing/selftests/drivers/.gitignore    |   1 +
>  .../selftests/drivers/s390x/uvdevice/Makefile |  22 ++
>  .../selftests/drivers/s390x/uvdevice/config   |   1 +
>  .../drivers/s390x/uvdevice/test_uvdevice.c    | 276 ++++++++++++++++++
>  6 files changed, 302 insertions(+)
>  create mode 100644 tools/testing/selftests/drivers/s390x/uvdevice/Makefile
>  create mode 100644 tools/testing/selftests/drivers/s390x/uvdevice/config
>  create mode 100644 tools/testing/selftests/drivers/s390x/uvdevice/test_uvdevice.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b42ab4a35e18..46a9b1467380 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10786,6 +10786,7 @@ 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/drivers/s390x/uvdevice/
>  F:	tools/testing/selftests/kvm/*/s390x/
>  F:	tools/testing/selftests/kvm/s390x/
>  
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 2319ec87f53d..d6b307371ef7 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -10,6 +10,7 @@ TARGETS += core
>  TARGETS += cpufreq
>  TARGETS += cpu-hotplug
>  TARGETS += drivers/dma-buf
> +TARGETS += drivers/s390x/uvdevice
>  TARGETS += efivarfs
>  TARGETS += exec
>  TARGETS += filesystems
> diff --git a/tools/testing/selftests/drivers/.gitignore b/tools/testing/selftests/drivers/.gitignore
> index ca74f2e1c719..09e23b5afa96 100644
> --- a/tools/testing/selftests/drivers/.gitignore
> +++ b/tools/testing/selftests/drivers/.gitignore
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  /dma-buf/udmabuf
> +/s390x/uvdevice/test_uvdevice
> diff --git a/tools/testing/selftests/drivers/s390x/uvdevice/Makefile b/tools/testing/selftests/drivers/s390x/uvdevice/Makefile
> new file mode 100644
> index 000000000000..5e701d2708d4
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/s390x/uvdevice/Makefile
> @@ -0,0 +1,22 @@
> +include ../../../../../build/Build.include
> +
> +UNAME_M := $(shell uname -m)
> +
> +ifneq ($(UNAME_M),s390x)
> +nothing:
> +.PHONY: all clean run_tests install
> +.SILENT:
> +else
> +
> +TEST_GEN_PROGS := test_uvdevice
> +
> +top_srcdir ?= ../../../../../..
> +KSFT_KHDR_INSTALL := 1
> +khdr_dir = $(top_srcdir)/usr/include
This doesn't work in different build cases. Please use $(KHDR_INCLUDES)
instead.

> +LINUX_TOOL_ARCH_INCLUDE = $(top_srcdir)/tools/arch/$(ARCH)/include
Which header files do you need from here? I'm not sure what is the
purpose of the header files in $(top_srcdir)/tools/arch and where they
are used.

> +
> +CFLAGS += -Wall -Werror -static -I$(khdr_dir) -I$(LINUX_TOOL_ARCH_INCLUDE)
> +
> +include ../../../lib.mk
> +
> +endif
-- 
Muhammad Usama Anjum

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-05-21 13:53 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-10 14:47 [PATCH v4 0/2] s390: Ultravisor device Steffen Eiden
2022-05-10 14:47 ` [PATCH v4 1/2] drivers/s390/char: Add Ultravisor io device Steffen Eiden
2022-05-12 14:33   ` Claudio Imbrenda
2022-05-13  7:45     ` Steffen Eiden
2022-05-13  8:37       ` Claudio Imbrenda
2022-05-13 12:35         ` Steffen Eiden
2022-05-16 11:33   ` Steffen Eiden
2022-05-16 14:51     ` Claudio Imbrenda
2022-05-17  8:38   ` Janosch Frank
2022-05-17  8:42     ` Steffen Eiden
2022-05-17 12:41       ` Janosch Frank
2022-05-18 11:47   ` Heiko Carstens
2022-05-18 13:45     ` Janosch Frank
2022-05-18 13:49       ` Steffen Eiden
2022-05-19  5:37       ` Heiko Carstens
2022-05-19  9:13         ` Janosch Frank
2022-05-10 14:47 ` [PATCH v4 2/2] selftests: drivers/s390x: Add uvdevice tests Steffen Eiden
2022-05-21 13:53   ` Muhammad Usama Anjum

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.