[PATCH v2 0/6] s390/uvdevice: Expose secret UVCs

[PATCH v2 0/6] s390/uvdevice: Expose secret UVCs

[Steffen Eiden]

IBM Secure Execution guests may want to inject secrets into the Ultravisor(UV).
Also they should be able to know which secrets the UV possesses and prevent the
further addition of more secrets.

Therefore, add three new Ultravisor-Calls and expose them via the uvdevice:
Add Secret, List Secrets, and Lock Secrets.
The uvdevice still acts as the messenger only and does not inspect or modify
the requests. Only some sanity checks are made to prevent the kernel from corruption.
Also add a new IOCTL to get information about the supported UV-calls of the uvdevice.
As userspace wants to know which secrets, types, etc. are supported expose the corresponding
UV Query info data to userspace via sysfs.

The series contains:
  * A new info IOCTL, giving information about the capabilities of the uvdevice and UV
  * 3 patches adding new Ultravisor-Calls and expose them to userspace
  * A patch replacing scnprintf with sysfs_emit in arch/s390/kernel/uv.c
  * A patch with an Ultravisor Query Info update for the new secret related information


Changes for v2:
  * use __set_bit instead of the atomic set_bit (Heiko)
  * add a patch for replacing scnprintf with sysfs_emit in arch/s390/kernel/uv.c (Heiko)
  * use scnprintf instead of sysfs_emit for the new sysfs entries in the last patch (Heiko)
  * use hex values in struct definitions (Claudio)



Steffen

Steffen Eiden (6):
  s390/uvdevice: Add info IOCTL
  s390/uvdevice: Add 'Add Secret' UVC
  s390/uvdevice: Add 'List Secrets' UVC
  s390/uvdevice: Add 'Lock Secret Store' UVC
  s390/uv: replace scnprintf with sysfs_emit
  s390/uv: Update query for secret-UVCs

 arch/s390/boot/uv.c                   |   4 +
 arch/s390/include/asm/uv.h            |  32 +++-
 arch/s390/include/uapi/asm/uvdevice.h |  56 ++++++-
 arch/s390/kernel/uv.c                 |  76 ++++++---
 drivers/s390/char/uvdevice.c          | 229 +++++++++++++++++++++++++-
 5 files changed, 366 insertions(+), 31 deletions(-)

-- 
2.40.1

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

[Steffen Eiden]

Add an IOCTL that allows userspace to find out which IOCTLs the uvdevice
supports without trial and error.

Explicitly expose the IOCTL nr for the request types.

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
---
 arch/s390/include/uapi/asm/uvdevice.h | 45 +++++++++++++++-
 drivers/s390/char/uvdevice.c          | 77 ++++++++++++++++++++++++---
 2 files changed, 114 insertions(+), 8 deletions(-)

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

[PATCH v2 2/6] s390/uvdevice: Add 'Add Secret' UVC

[Steffen Eiden]

Userspace can call the Add Secret Ultravisor Call
using IOCTLs on the uvdevice.
During the handling of the new IOCTL nr the uvdevice will do some sanity
checks first. Then, copy the request data to kernel space, perform the
Ultravisor command, and copy the return codes to userspace.
If the Add Secret UV facility is not present,
UV will return invalid command rc. This won't be fenced in the driver
and does not result in a negative return value. This is also true for
any other possible error code the UV can return.

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
---
 arch/s390/include/asm/uv.h            | 14 +++++++
 arch/s390/include/uapi/asm/uvdevice.h |  4 ++
 drivers/s390/char/uvdevice.c          | 58 +++++++++++++++++++++++++++
 3 files changed, 76 insertions(+)

diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
index 28a9ad57b6f1..a7dff64e1e24 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -58,6 +58,7 @@
 #define UVC_CMD_SET_SHARED_ACCESS	0x1000
 #define UVC_CMD_REMOVE_SHARED_ACCESS	0x1001
 #define UVC_CMD_RETR_ATTEST		0x1020
+#define UVC_CMD_ADD_SECRET		0x1031
 
 /* Bits in installed uv calls */
 enum uv_cmds_inst {
@@ -88,6 +89,7 @@ enum uv_cmds_inst {
 	BIT_UVC_CMD_DUMP_CPU = 26,
 	BIT_UVC_CMD_DUMP_COMPLETE = 27,
 	BIT_UVC_CMD_RETR_ATTEST = 28,
+	BIT_UVC_CMD_ADD_SECRET = 29,
 };
 
 enum uv_feat_ind {
@@ -292,6 +294,18 @@ struct uv_cb_dump_complete {
 	u64 reserved30[5];
 } __packed __aligned(8);
 
+/*
+ * A common call for pv guests that contains a single address
+ * Examples:
+ * Add Secret
+ */
+struct uv_cb_guest_addr {
+	struct uv_cb_header header;
+	u64 reserved08[3];
+	u64 addr;
+	u64 reserved28[4];
+} __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
index ed58979b8438..e919b802213c 100644
--- a/arch/s390/include/uapi/asm/uvdevice.h
+++ b/arch/s390/include/uapi/asm/uvdevice.h
@@ -72,6 +72,7 @@ struct uvio_uvdev_info {
 #define UVIO_ATT_ARCB_MAX_LEN		0x100000
 #define UVIO_ATT_MEASUREMENT_MAX_LEN	0x8000
 #define UVIO_ATT_ADDITIONAL_MAX_LEN	0x8000
+#define UVIO_ADD_SECRET_MAX_LEN		0x100000
 
 #define UVIO_DEVICE_NAME "uv"
 #define UVIO_TYPE_UVC 'u'
@@ -79,6 +80,7 @@ struct uvio_uvdev_info {
 enum UVIO_IOCTL_NR {
 	UVIO_IOCTL_UVDEV_INFO_NR = 0x00,
 	UVIO_IOCTL_ATT_NR,
+	UVIO_IOCTL_ADD_SECRET_NR,
 	/* must be the last entry */
 	UVIO_IOCTL_NUM_IOCTLS
 };
@@ -86,9 +88,11 @@ enum UVIO_IOCTL_NR {
 #define UVIO_IOCTL(nr)		_IOWR(UVIO_TYPE_UVC, nr, struct uvio_ioctl_cb)
 #define UVIO_IOCTL_UVDEV_INFO	UVIO_IOCTL(UVIO_IOCTL_UVDEV_INFO_NR)
 #define UVIO_IOCTL_ATT		UVIO_IOCTL(UVIO_IOCTL_ATT_NR)
+#define UVIO_IOCTL_ADD_SECRET	UVIO_IOCTL(UVIO_IOCTL_ADD_SECRET_NR)
 
 #define UVIO_SUPP_CALL(nr)	(1ULL << (nr))
 #define UVIO_SUPP_UDEV_INFO	UVIO_SUPP_CALL(UVIO_IOCTL_UDEV_INFO_NR)
 #define UVIO_SUPP_ATT		UVIO_SUPP_CALL(UVIO_IOCTL_ATT_NR)
+#define UVIO_SUPP_ADD_SECRET	UVIO_SUPP_CALL(UVIO_IOCTL_ADD_SECRET_NR)
 
 #endif /* __S390_ASM_UVDEVICE_H */
diff --git a/drivers/s390/char/uvdevice.c b/drivers/s390/char/uvdevice.c
index 61c7f284c5c5..c9bcd1b63d3c 100644
--- a/drivers/s390/char/uvdevice.c
+++ b/drivers/s390/char/uvdevice.c
@@ -37,6 +37,7 @@
 static const u64 ioctl_nr_to_uvc_bit[] __initconst = {
 	[UVIO_IOCTL_UVDEV_INFO_NR] = -1UL,
 	[UVIO_IOCTL_ATT_NR] = BIT_UVC_CMD_RETR_ATTEST,
+	[UVIO_IOCTL_ADD_SECRET_NR] = BIT_UVC_CMD_ADD_SECRET,
 };
 
 static_assert(ARRAY_SIZE(ioctl_nr_to_uvc_bit) == UVIO_IOCTL_NUM_IOCTLS);
@@ -231,6 +232,60 @@ static int uvio_attestation(struct uvio_ioctl_cb *uv_ioctl)
 	return ret;
 }
 
+/** uvio_add_secret() - perform an Add Secret UVC
+ *
+ * @uv_ioctl: ioctl control block
+ *
+ * uvio_add_secret() performs the Add Secret Ultravisor Call.
+ * It verifies that the given userspace argument address is valid and its size
+ * is sane. Every other check is made by the Ultravisor (UV) and won't result
+ * in a negative return value. It copies the request to kernelspace,
+ * performs the UV-call, and copies the return codes to the ioctl control block.
+ * The argument has to point to an Add Secret Request Control Block.
+ * It is an encrypted and  cryptographically verified request generated by
+ * userspace to insert the actual secret into the UV.
+ * If the Add Secret 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_add_secret(struct uvio_ioctl_cb *uv_ioctl)
+{
+	void __user *user_buf_arg = (void __user *)uv_ioctl->argument_addr;
+	struct uv_cb_guest_addr uvcb = {
+		.header.len = sizeof(uvcb),
+		.header.cmd = UVC_CMD_ADD_SECRET,
+	};
+	void *asrcb = NULL;
+	int ret;
+
+	if (uv_ioctl->argument_len > UVIO_ADD_SECRET_MAX_LEN)
+		return -EINVAL;
+	if (uv_ioctl->argument_len == 0)
+		return -EINVAL;
+
+	asrcb = kvzalloc(uv_ioctl->argument_len, GFP_KERNEL);
+	if (!asrcb)
+		return -EINVAL;
+
+	ret = -EFAULT;
+	if (copy_from_user(asrcb, user_buf_arg, uv_ioctl->argument_len))
+		goto out;
+
+	ret = 0;
+	uvcb.addr = (u64)asrcb;
+	uv_call_sched(0, (u64)&uvcb);
+	uv_ioctl->uv_rc = uvcb.header.rc;
+	uv_ioctl->uv_rrc = uvcb.header.rrc;
+
+out:
+	kvfree(asrcb);
+	return ret;
+}
+
 static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp,
 				     unsigned long cmd)
 {
@@ -275,6 +330,9 @@ static long uvio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	case UVIO_IOCTL_ATT_NR:
 		ret = uvio_attestation(&uv_ioctl);
 		break;
+	case UVIO_IOCTL_ADD_SECRET_NR:
+		ret = uvio_add_secret(&uv_ioctl);
+		break;
 	default:
 		ret = -ENOIOCTLCMD;
 		break;
-- 
2.40.1

[PATCH v2 3/6] s390/uvdevice: Add 'List Secrets' UVC

[Steffen Eiden]

Userspace can call the List Secrets Ultravisor Call
using IOCTLs on the uvdevice.
During the handling of the new IOCTL nr the uvdevice will do some sanity
checks first. Then, perform the Ultravisor command, and copy the answer
to userspace.
If the List Secrets UV facility is not present, UV will return
invalid command rc. This won't be fenced in the driver and does not
result in a negative return value. This is also true for any other
possible error code the UV can return.

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
---
 arch/s390/include/asm/uv.h            |  3 ++
 arch/s390/include/uapi/asm/uvdevice.h |  4 ++
 drivers/s390/char/uvdevice.c          | 55 +++++++++++++++++++++++++++
 3 files changed, 62 insertions(+)

diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
index a7dff64e1e24..1e4f0f6d4923 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -59,6 +59,7 @@
 #define UVC_CMD_REMOVE_SHARED_ACCESS	0x1001
 #define UVC_CMD_RETR_ATTEST		0x1020
 #define UVC_CMD_ADD_SECRET		0x1031
+#define UVC_CMD_LIST_SECRETS		0x1033
 
 /* Bits in installed uv calls */
 enum uv_cmds_inst {
@@ -90,6 +91,7 @@ enum uv_cmds_inst {
 	BIT_UVC_CMD_DUMP_COMPLETE = 27,
 	BIT_UVC_CMD_RETR_ATTEST = 28,
 	BIT_UVC_CMD_ADD_SECRET = 29,
+	BIT_UVC_CMD_LIST_SECRETS = 30,
 };
 
 enum uv_feat_ind {
@@ -298,6 +300,7 @@ struct uv_cb_dump_complete {
  * A common call for pv guests that contains a single address
  * Examples:
  * Add Secret
+ * List Secrets
  */
 struct uv_cb_guest_addr {
 	struct uv_cb_header header;
diff --git a/arch/s390/include/uapi/asm/uvdevice.h b/arch/s390/include/uapi/asm/uvdevice.h
index e919b802213c..cf12d6b8d8d8 100644
--- a/arch/s390/include/uapi/asm/uvdevice.h
+++ b/arch/s390/include/uapi/asm/uvdevice.h
@@ -73,6 +73,7 @@ struct uvio_uvdev_info {
 #define UVIO_ATT_MEASUREMENT_MAX_LEN	0x8000
 #define UVIO_ATT_ADDITIONAL_MAX_LEN	0x8000
 #define UVIO_ADD_SECRET_MAX_LEN		0x100000
+#define UVIO_LIST_SECRETS_LEN		0x1000
 
 #define UVIO_DEVICE_NAME "uv"
 #define UVIO_TYPE_UVC 'u'
@@ -81,6 +82,7 @@ enum UVIO_IOCTL_NR {
 	UVIO_IOCTL_UVDEV_INFO_NR = 0x00,
 	UVIO_IOCTL_ATT_NR,
 	UVIO_IOCTL_ADD_SECRET_NR,
+	UVIO_IOCTL_LIST_SECRETS_NR,
 	/* must be the last entry */
 	UVIO_IOCTL_NUM_IOCTLS
 };
@@ -89,10 +91,12 @@ enum UVIO_IOCTL_NR {
 #define UVIO_IOCTL_UVDEV_INFO	UVIO_IOCTL(UVIO_IOCTL_UVDEV_INFO_NR)
 #define UVIO_IOCTL_ATT		UVIO_IOCTL(UVIO_IOCTL_ATT_NR)
 #define UVIO_IOCTL_ADD_SECRET	UVIO_IOCTL(UVIO_IOCTL_ADD_SECRET_NR)
+#define UVIO_IOCTL_LIST_SECRETS	UVIO_IOCTL(UVIO_IOCTL_LIST_SECRETS_NR)
 
 #define UVIO_SUPP_CALL(nr)	(1ULL << (nr))
 #define UVIO_SUPP_UDEV_INFO	UVIO_SUPP_CALL(UVIO_IOCTL_UDEV_INFO_NR)
 #define UVIO_SUPP_ATT		UVIO_SUPP_CALL(UVIO_IOCTL_ATT_NR)
 #define UVIO_SUPP_ADD_SECRET	UVIO_SUPP_CALL(UVIO_IOCTL_ADD_SECRET_NR)
+#define UVIO_SUPP_LIST_SECRETS	UVIO_SUPP_CALL(UVIO_IOCTL_LIST_SECRETS_NR)
 
 #endif /* __S390_ASM_UVDEVICE_H */
diff --git a/drivers/s390/char/uvdevice.c b/drivers/s390/char/uvdevice.c
index c9bcd1b63d3c..e5ced8b5a59a 100644
--- a/drivers/s390/char/uvdevice.c
+++ b/drivers/s390/char/uvdevice.c
@@ -38,6 +38,7 @@ static const u64 ioctl_nr_to_uvc_bit[] __initconst = {
 	[UVIO_IOCTL_UVDEV_INFO_NR] = -1UL,
 	[UVIO_IOCTL_ATT_NR] = BIT_UVC_CMD_RETR_ATTEST,
 	[UVIO_IOCTL_ADD_SECRET_NR] = BIT_UVC_CMD_ADD_SECRET,
+	[UVIO_IOCTL_LIST_SECRETS_NR] = BIT_UVC_CMD_LIST_SECRETS,
 };
 
 static_assert(ARRAY_SIZE(ioctl_nr_to_uvc_bit) == UVIO_IOCTL_NUM_IOCTLS);
@@ -286,6 +287,57 @@ static int uvio_add_secret(struct uvio_ioctl_cb *uv_ioctl)
 	return ret;
 }
 
+/** uvio_list_secrets() - perform a List Secret UVC
+ *
+ * @uv_ioctl: ioctl control block
+ *
+ * uvio_list_secrets() performs the List Secret Ultravisor Call.
+ * It verifies that the given userspace argument address is valid and its size
+ * is sane. Every other check is made by the Ultravisor (UV) and won't result
+ * in a negative return value. It builds the request, performs the UV-call,
+ * and copies the result to userspace.
+ *
+ * The argument specifies the location for the result of the UV-Call.
+ *
+ * If the List Secrets UV facility is not present,
+ * UV will return invalid command rc. This won't be fenced in the driver
+ * and does not result in a negative return value.
+ *
+ * Context: might sleep
+ *
+ * Return: 0 on success or a negative error code on error.
+ */
+static int uvio_list_secrets(struct uvio_ioctl_cb *uv_ioctl)
+{
+	void __user *user_buf_arg = (void __user *)uv_ioctl->argument_addr;
+	struct uv_cb_guest_addr uvcb = {
+		.header.len = sizeof(uvcb),
+		.header.cmd = UVC_CMD_LIST_SECRETS,
+	};
+	void *secrets = NULL;
+	int ret;
+
+	if (uv_ioctl->argument_len != UVIO_LIST_SECRETS_LEN)
+		return -EINVAL;
+
+	secrets = kvzalloc(uv_ioctl->argument_len, GFP_KERNEL);
+	if (!secrets)
+		return -ENOMEM;
+
+	uvcb.addr = (u64)secrets;
+	uv_call_sched(0, (u64)&uvcb);
+	uv_ioctl->uv_rc = uvcb.header.rc;
+	uv_ioctl->uv_rrc = uvcb.header.rrc;
+
+	if (copy_to_user(user_buf_arg, secrets, uv_ioctl->argument_len))
+		ret = -EFAULT;
+	else
+		ret = 0;
+
+	kvfree(secrets);
+	return ret;
+}
+
 static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp,
 				     unsigned long cmd)
 {
@@ -333,6 +385,9 @@ static long uvio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	case UVIO_IOCTL_ADD_SECRET_NR:
 		ret = uvio_add_secret(&uv_ioctl);
 		break;
+	case UVIO_IOCTL_LIST_SECRETS_NR:
+		ret = uvio_list_secrets(&uv_ioctl);
+		break;
 	default:
 		ret = -ENOIOCTLCMD;
 		break;
-- 
2.40.1

[PATCH v2 4/6] s390/uvdevice: Add 'Lock Secret Store' UVC

[Steffen Eiden]

Userspace can call the Lock Secret Store Ultravisor Call
using IOCTLs on the uvdevice.
During the handling of the new IOCTL nr the uvdevice will do some sanity
checks first. Then, perform the Ultravisor command, and copy the
return codes to userspace.
If the Lock Secrets UV facility is not present, UV will return
invalid command rc. This won't be fenced in the driver and does not
result in a negative return value. This is also true for any other
possible error code the UV can return.

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
---
 arch/s390/include/asm/uv.h            |  2 ++
 arch/s390/include/uapi/asm/uvdevice.h |  3 +++
 drivers/s390/char/uvdevice.c          | 39 +++++++++++++++++++++++++++
 3 files changed, 44 insertions(+)

diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
index 1e4f0f6d4923..6180ac8909d5 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -60,6 +60,7 @@
 #define UVC_CMD_RETR_ATTEST		0x1020
 #define UVC_CMD_ADD_SECRET		0x1031
 #define UVC_CMD_LIST_SECRETS		0x1033
+#define UVC_CMD_LOCK_SECRETS		0x1034
 
 /* Bits in installed uv calls */
 enum uv_cmds_inst {
@@ -92,6 +93,7 @@ enum uv_cmds_inst {
 	BIT_UVC_CMD_RETR_ATTEST = 28,
 	BIT_UVC_CMD_ADD_SECRET = 29,
 	BIT_UVC_CMD_LIST_SECRETS = 30,
+	BIT_UVC_CMD_LOCK_SECRETS = 31,
 };
 
 enum uv_feat_ind {
diff --git a/arch/s390/include/uapi/asm/uvdevice.h b/arch/s390/include/uapi/asm/uvdevice.h
index cf12d6b8d8d8..b6e8d47dd589 100644
--- a/arch/s390/include/uapi/asm/uvdevice.h
+++ b/arch/s390/include/uapi/asm/uvdevice.h
@@ -83,6 +83,7 @@ enum UVIO_IOCTL_NR {
 	UVIO_IOCTL_ATT_NR,
 	UVIO_IOCTL_ADD_SECRET_NR,
 	UVIO_IOCTL_LIST_SECRETS_NR,
+	UVIO_IOCTL_LOCK_SECRETS_NR,
 	/* must be the last entry */
 	UVIO_IOCTL_NUM_IOCTLS
 };
@@ -92,11 +93,13 @@ enum UVIO_IOCTL_NR {
 #define UVIO_IOCTL_ATT		UVIO_IOCTL(UVIO_IOCTL_ATT_NR)
 #define UVIO_IOCTL_ADD_SECRET	UVIO_IOCTL(UVIO_IOCTL_ADD_SECRET_NR)
 #define UVIO_IOCTL_LIST_SECRETS	UVIO_IOCTL(UVIO_IOCTL_LIST_SECRETS_NR)
+#define UVIO_IOCTL_LOCK_SECRETS UVIO_IOCTL(UVIO_IOCTL_LOCK_SECRETS_NR)
 
 #define UVIO_SUPP_CALL(nr)	(1ULL << (nr))
 #define UVIO_SUPP_UDEV_INFO	UVIO_SUPP_CALL(UVIO_IOCTL_UDEV_INFO_NR)
 #define UVIO_SUPP_ATT		UVIO_SUPP_CALL(UVIO_IOCTL_ATT_NR)
 #define UVIO_SUPP_ADD_SECRET	UVIO_SUPP_CALL(UVIO_IOCTL_ADD_SECRET_NR)
 #define UVIO_SUPP_LIST_SECRETS	UVIO_SUPP_CALL(UVIO_IOCTL_LIST_SECRETS_NR)
+#define UVIO_SUPP_LOCK_SECRETS	UVIO_SUPP_CALL(UVIO_IOCTL_LOCK_SECRETS_NR)
 
 #endif /* __S390_ASM_UVDEVICE_H */
diff --git a/drivers/s390/char/uvdevice.c b/drivers/s390/char/uvdevice.c
index e5ced8b5a59a..6c0b749e94b4 100644
--- a/drivers/s390/char/uvdevice.c
+++ b/drivers/s390/char/uvdevice.c
@@ -39,6 +39,7 @@ static const u64 ioctl_nr_to_uvc_bit[] __initconst = {
 	[UVIO_IOCTL_ATT_NR] = BIT_UVC_CMD_RETR_ATTEST,
 	[UVIO_IOCTL_ADD_SECRET_NR] = BIT_UVC_CMD_ADD_SECRET,
 	[UVIO_IOCTL_LIST_SECRETS_NR] = BIT_UVC_CMD_LIST_SECRETS,
+	[UVIO_IOCTL_LOCK_SECRETS_NR] = BIT_UVC_CMD_LOCK_SECRETS,
 };
 
 static_assert(ARRAY_SIZE(ioctl_nr_to_uvc_bit) == UVIO_IOCTL_NUM_IOCTLS);
@@ -338,6 +339,41 @@ static int uvio_list_secrets(struct uvio_ioctl_cb *uv_ioctl)
 	return ret;
 }
 
+/** uvio_lock_secrets() - perform a Lock Secret Store UVC
+ *
+ * @uv_ioctl: ioctl control block
+ *
+ * uvio_lock_secrets() performs the Lock Secret Store Ultravisor Call.
+ * It performs the UV-call and copies the return codes to the
+ * ioctl control block.
+ *
+ * The argument address and size must be 0.
+ *
+ * If the List Secrets UV facility is not present,
+ * UV will return invalid command rc. This won't be fenced in the driver
+ * and does not result in a negative return value.
+ *
+ * Context: might sleep
+ *
+ * Return: 0 on success or a negative error code on error.
+ */
+static int uvio_lock_secrets(struct uvio_ioctl_cb *ioctl)
+{
+	struct uv_cb_nodata uvcb = {
+		.header.len = sizeof(uvcb),
+		.header.cmd = UVC_CMD_LOCK_SECRETS,
+	};
+
+	if (ioctl->argument_addr || ioctl->argument_len)
+		return -EINVAL;
+
+	uv_call(0, (u64)&uvcb);
+	ioctl->uv_rc = uvcb.header.rc;
+	ioctl->uv_rrc = uvcb.header.rrc;
+
+	return 0;
+}
+
 static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp,
 				     unsigned long cmd)
 {
@@ -388,6 +424,9 @@ static long uvio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	case UVIO_IOCTL_LIST_SECRETS_NR:
 		ret = uvio_list_secrets(&uv_ioctl);
 		break;
+	case UVIO_IOCTL_LOCK_SECRETS_NR:
+		ret = uvio_lock_secrets(&uv_ioctl);
+		break;
 	default:
 		ret = -ENOIOCTLCMD;
 		break;
-- 
2.40.1

[PATCH v2 5/6] s390/uv: replace scnprintf with sysfs_emit

[Steffen Eiden]

Replace scnprintf(page, PAGE_SIZE, ...) with the page size aware
sysfs_emit(page, ...) which adds some sanity checks.

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
---
 arch/s390/kernel/uv.c | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
index cb2ee06df286..d5b5d7e83c17 100644
--- a/arch/s390/kernel/uv.c
+++ b/arch/s390/kernel/uv.c
@@ -462,11 +462,11 @@ EXPORT_SYMBOL_GPL(arch_make_page_accessible);
 static ssize_t uv_query_facilities(struct kobject *kobj,
 				   struct kobj_attribute *attr, char *page)
 {
-	return scnprintf(page, PAGE_SIZE, "%lx\n%lx\n%lx\n%lx\n",
-			uv_info.inst_calls_list[0],
-			uv_info.inst_calls_list[1],
-			uv_info.inst_calls_list[2],
-			uv_info.inst_calls_list[3]);
+	return sysfs_emit(page, "%lx\n%lx\n%lx\n%lx\n",
+			  uv_info.inst_calls_list[0],
+			  uv_info.inst_calls_list[1],
+			  uv_info.inst_calls_list[2],
+			  uv_info.inst_calls_list[3]);
 }
 
 static struct kobj_attribute uv_query_facilities_attr =
@@ -493,8 +493,7 @@ static struct kobj_attribute uv_query_supp_se_hdr_pcf_attr =
 static ssize_t uv_query_dump_cpu_len(struct kobject *kobj,
 				     struct kobj_attribute *attr, char *page)
 {
-	return scnprintf(page, PAGE_SIZE, "%lx\n",
-			uv_info.guest_cpu_stor_len);
+	return sysfs_emit(page, "%lx\n", uv_info.guest_cpu_stor_len);
 }
 
 static struct kobj_attribute uv_query_dump_cpu_len_attr =
@@ -503,8 +502,7 @@ static struct kobj_attribute uv_query_dump_cpu_len_attr =
 static ssize_t uv_query_dump_storage_state_len(struct kobject *kobj,
 					       struct kobj_attribute *attr, char *page)
 {
-	return scnprintf(page, PAGE_SIZE, "%lx\n",
-			uv_info.conf_dump_storage_state_len);
+	return sysfs_emit(page, "%lx\n", uv_info.conf_dump_storage_state_len);
 }
 
 static struct kobj_attribute uv_query_dump_storage_state_len_attr =
@@ -513,8 +511,7 @@ static struct kobj_attribute uv_query_dump_storage_state_len_attr =
 static ssize_t uv_query_dump_finalize_len(struct kobject *kobj,
 					  struct kobj_attribute *attr, char *page)
 {
-	return scnprintf(page, PAGE_SIZE, "%lx\n",
-			uv_info.conf_dump_finalize_len);
+	return sysfs_emit(page, "%lx\n", uv_info.conf_dump_finalize_len);
 }
 
 static struct kobj_attribute uv_query_dump_finalize_len_attr =
@@ -532,8 +529,7 @@ static struct kobj_attribute uv_query_feature_indications_attr =
 static ssize_t uv_query_max_guest_cpus(struct kobject *kobj,
 				       struct kobj_attribute *attr, char *page)
 {
-	return scnprintf(page, PAGE_SIZE, "%d\n",
-			uv_info.max_guest_cpu_id + 1);
+	return sysfs_emit(page, "%d\n", uv_info.max_guest_cpu_id + 1);
 }
 
 static struct kobj_attribute uv_query_max_guest_cpus_attr =
@@ -542,8 +538,7 @@ static struct kobj_attribute uv_query_max_guest_cpus_attr =
 static ssize_t uv_query_max_guest_vms(struct kobject *kobj,
 				      struct kobj_attribute *attr, char *page)
 {
-	return scnprintf(page, PAGE_SIZE, "%d\n",
-			uv_info.max_num_sec_conf);
+	return sysfs_emit(page, "%d\n", uv_info.max_num_sec_conf);
 }
 
 static struct kobj_attribute uv_query_max_guest_vms_attr =
@@ -552,8 +547,7 @@ static struct kobj_attribute uv_query_max_guest_vms_attr =
 static ssize_t uv_query_max_guest_addr(struct kobject *kobj,
 				       struct kobj_attribute *attr, char *page)
 {
-	return scnprintf(page, PAGE_SIZE, "%lx\n",
-			uv_info.max_sec_stor_addr);
+	return sysfs_emit(page, "%lx\n", uv_info.max_sec_stor_addr);
 }
 
 static struct kobj_attribute uv_query_max_guest_addr_attr =
@@ -562,7 +556,7 @@ static struct kobj_attribute uv_query_max_guest_addr_attr =
 static ssize_t uv_query_supp_att_req_hdr_ver(struct kobject *kobj,
 					     struct kobj_attribute *attr, char *page)
 {
-	return scnprintf(page, PAGE_SIZE, "%lx\n", uv_info.supp_att_req_hdr_ver);
+	return sysfs_emit(page, "%lx\n", uv_info.supp_att_req_hdr_ver);
 }
 
 static struct kobj_attribute uv_query_supp_att_req_hdr_ver_attr =
@@ -571,7 +565,7 @@ static struct kobj_attribute uv_query_supp_att_req_hdr_ver_attr =
 static ssize_t uv_query_supp_att_pflags(struct kobject *kobj,
 					struct kobj_attribute *attr, char *page)
 {
-	return scnprintf(page, PAGE_SIZE, "%lx\n", uv_info.supp_att_pflags);
+	return sysfs_emit(page, "%lx\n", uv_info.supp_att_pflags);
 }
 
 static struct kobj_attribute uv_query_supp_att_pflags_attr =
@@ -605,7 +599,7 @@ static ssize_t uv_is_prot_virt_guest(struct kobject *kobj,
 #ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
 	val = prot_virt_guest;
 #endif
-	return scnprintf(page, PAGE_SIZE, "%d\n", val);
+	return sysfs_emit(page, "%d\n", val);
 }
 
 static ssize_t uv_is_prot_virt_host(struct kobject *kobj,
@@ -617,7 +611,7 @@ static ssize_t uv_is_prot_virt_host(struct kobject *kobj,
 	val = prot_virt_host;
 #endif
 
-	return scnprintf(page, PAGE_SIZE, "%d\n", val);
+	return sysfs_emit(page, "%d\n", val);
 }
 
 static struct kobj_attribute uv_prot_virt_guest =
-- 
2.40.1

[PATCH v2 6/6] s390/uv: Update query for secret-UVCs

[Steffen Eiden]

Update the query struct such that secret-UVC related
information can be parsed.
Add sysfs files for these new values.

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
---
 arch/s390/boot/uv.c        |  4 ++++
 arch/s390/include/asm/uv.h | 13 +++++++++++--
 arch/s390/kernel/uv.c      | 40 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/arch/s390/boot/uv.c b/arch/s390/boot/uv.c
index 0a077c0a2056..323b5cae3cf1 100644
--- a/arch/s390/boot/uv.c
+++ b/arch/s390/boot/uv.c
@@ -47,6 +47,10 @@ void uv_query_info(void)
 		uv_info.conf_dump_finalize_len = uvcb.conf_dump_finalize_len;
 		uv_info.supp_att_req_hdr_ver = uvcb.supp_att_req_hdr_ver;
 		uv_info.supp_att_pflags = uvcb.supp_att_pflags;
+		uv_info.supp_add_secret_req_ver = uvcb.supp_add_secret_req_ver;
+		uv_info.supp_add_secret_pcf = uvcb.supp_add_secret_pcf;
+		uv_info.supp_secret_types = uvcb.supp_secret_types;
+		uv_info.max_secrets = uvcb.max_num_secrets;
 	}
 
 #ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
index 6180ac8909d5..0f3dee3fce71 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -123,7 +123,7 @@ struct uv_cb_qui {
 	u32 reserved70[3];			/* 0x0070 */
 	u32 max_num_sec_conf;			/* 0x007c */
 	u64 max_guest_stor_addr;		/* 0x0080 */
-	u8  reserved88[158 - 136];		/* 0x0088 */
+	u8  reserved88[0x9e - 0x88];		/* 0x0088 */
 	u16 max_guest_cpu_id;			/* 0x009e */
 	u64 uv_feature_indications;		/* 0x00a0 */
 	u64 reserveda8;				/* 0x00a8 */
@@ -135,7 +135,12 @@ struct uv_cb_qui {
 	u64 reservedd8;				/* 0x00d8 */
 	u64 supp_att_req_hdr_ver;		/* 0x00e0 */
 	u64 supp_att_pflags;			/* 0x00e8 */
-	u8 reservedf0[256 - 240];		/* 0x00f0 */
+	u64 reservedf0;				/* 0x00f0 */
+	u64 supp_add_secret_req_ver;		/* 0x00f8 */
+	u64 supp_add_secret_pcf;		/* 0x0100 */
+	u64 supp_secret_types;			/* 0x0180 */
+	u16 max_num_secrets;			/* 0x0110 */
+	u8 reserved112[0x120 - 0x112];		/* 0x0112 */
 } __packed __aligned(8);
 
 /* Initialize Ultravisor */
@@ -384,6 +389,10 @@ struct uv_info {
 	unsigned long conf_dump_finalize_len;
 	unsigned long supp_att_req_hdr_ver;
 	unsigned long supp_att_pflags;
+	unsigned long supp_add_secret_req_ver;
+	unsigned long supp_add_secret_pcf;
+	unsigned long supp_secret_types;
+	unsigned short max_secrets;
 };
 
 extern struct uv_info uv_info;
diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
index d5b5d7e83c17..c0a1bfb01135 100644
--- a/arch/s390/kernel/uv.c
+++ b/arch/s390/kernel/uv.c
@@ -571,6 +571,42 @@ static ssize_t uv_query_supp_att_pflags(struct kobject *kobj,
 static struct kobj_attribute uv_query_supp_att_pflags_attr =
 	__ATTR(supp_att_pflags, 0444, uv_query_supp_att_pflags, NULL);
 
+static ssize_t uv_query_supp_add_secret_req_ver(struct kobject *kobj,
+						struct kobj_attribute *attr, char *page)
+{
+	return sysfs_emit(page, "%lx\n", uv_info.supp_add_secret_req_ver);
+}
+
+static struct kobj_attribute uv_query_supp_add_secret_req_ver_attr =
+	__ATTR(supp_add_secret_req_ver, 0444, uv_query_supp_add_secret_req_ver, NULL);
+
+static ssize_t uv_query_supp_add_secret_pcf(struct kobject *kobj,
+					    struct kobj_attribute *attr, char *page)
+{
+	return sysfs_emit(page, "%lx\n", uv_info.supp_add_secret_pcf);
+}
+
+static struct kobj_attribute uv_query_supp_add_secret_pcf_attr =
+	__ATTR(supp_add_secret_pcf, 0444, uv_query_supp_add_secret_pcf, NULL);
+
+static ssize_t uv_query_supp_secret_types(struct kobject *kobj,
+					  struct kobj_attribute *attr, char *page)
+{
+	return sysfs_emit(page, "%lx\n", uv_info.supp_secret_types);
+}
+
+static struct kobj_attribute uv_query_supp_secret_types_attr =
+	__ATTR(supp_secret_types, 0444, uv_query_supp_secret_types, NULL);
+
+static ssize_t uv_query_max_secrets(struct kobject *kobj,
+				    struct kobj_attribute *attr, char *page)
+{
+	return sysfs_emit(page, "%d\n", uv_info.max_secrets);
+}
+
+static struct kobj_attribute uv_query_max_num_secrets_attr =
+	__ATTR(max_secrets, 0444, uv_query_max_secrets, NULL);
+
 static struct attribute *uv_query_attrs[] = {
 	&uv_query_facilities_attr.attr,
 	&uv_query_feature_indications_attr.attr,
@@ -584,6 +620,10 @@ static struct attribute *uv_query_attrs[] = {
 	&uv_query_dump_cpu_len_attr.attr,
 	&uv_query_supp_att_req_hdr_ver_attr.attr,
 	&uv_query_supp_att_pflags_attr.attr,
+	&uv_query_supp_add_secret_req_ver_attr.attr,
+	&uv_query_supp_add_secret_pcf_attr.attr,
+	&uv_query_supp_secret_types_attr.attr,
+	&uv_query_max_num_secrets_attr.attr,
 	NULL,
 };
 
-- 
2.40.1

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

[Janosch Frank]

On 5/19/23 11:37, Steffen Eiden wrote:
> Add an IOCTL that allows userspace to find out which IOCTLs the uvdevice
> supports without trial and error.
> 
> Explicitly expose the IOCTL nr for the request types.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
[...]
>   #endif /* __S390_ASM_UVDEVICE_H */
> diff --git a/drivers/s390/char/uvdevice.c b/drivers/s390/char/uvdevice.c
> index 1d40457c7b10..61c7f284c5c5 100644
> --- a/drivers/s390/char/uvdevice.c
> +++ b/drivers/s390/char/uvdevice.c
> @@ -20,6 +20,7 @@
>    *  channel for userspace to the Ultravisor.
>    */
>   
> +#include "asm-generic/ioctl.h"
>   #include <linux/module.h>
>   #include <linux/kernel.h>
>   #include <linux/miscdevice.h>
> @@ -32,6 +33,51 @@
>   #include <asm/uvdevice.h>
>   #include <asm/uv.h>
>   
> +/* Mapping from IOCTL-nr to UVC-bit */
> +static const u64 ioctl_nr_to_uvc_bit[] __initconst = {

This can be u32 since cmds are u16.

> +	[UVIO_IOCTL_UVDEV_INFO_NR] = -1UL,

#define BIT_UVIO_INTERNAL U32_MAX

> +	[UVIO_IOCTL_ATT_NR] = BIT_UVC_CMD_RETR_ATTEST,
> +};
> +
> +static_assert(ARRAY_SIZE(ioctl_nr_to_uvc_bit) == UVIO_IOCTL_NUM_IOCTLS);
> +
> +static struct uvio_uvdev_info uvdev_info = {
> +	.supp_uvio_cmds = GENMASK_ULL(UVIO_IOCTL_NUM_IOCTLS - 1, 0),
> +};
> +
> +static void __init set_supp_uv_cmds(struct uvio_uvdev_info *info)
> +{
> +	int i;
> +
> +	for (i = 0; i < UVIO_IOCTL_NUM_IOCTLS; i++) {
> +		if (ioctl_nr_to_uvc_bit[i] == -1UL)

BIT_UVIO_INTERNAL

> +			continue;
> +		if (!test_bit_inv(ioctl_nr_to_uvc_bit[i], uv_info.inst_calls_list))
> +			continue;
> +		__set_bit(i, (unsigned long *)&info->supp_uv_cmds);

Move the cast to the caller and make this function take a "unsigned long 
*)" argument.

> +	}
> +}
> +
> +/**
> + * uvio_uvdev_info() - get information about the uvdevice
> + *
> + * @uv_ioctl: ioctl control block
> + *
> + * Lists all supported IOCTLs by this uvdevice

Lists all IOCTLs that are supported by this uvdevice

[...]

Rest seems fine to me.

Re: [PATCH v2 2/6] s390/uvdevice: Add 'Add Secret' UVC

[Janosch Frank]

On 5/19/23 11:37, Steffen Eiden wrote:
> Userspace can call the Add Secret Ultravisor Call
> using IOCTLs on the uvdevice.
> During the handling of the new IOCTL nr the uvdevice will do some sanity
> checks first. Then, copy the request data to kernel space, perform the
> Ultravisor command, and copy the return codes to userspace.
> If the Add Secret UV facility is not present,
> UV will return invalid command rc. This won't be fenced in the driver
> and does not result in a negative return value. This is also true for
> any other possible error code the UV can return.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
>   arch/s390/include/asm/uv.h            | 14 +++++++
>   arch/s390/include/uapi/asm/uvdevice.h |  4 ++
>   drivers/s390/char/uvdevice.c          | 58 +++++++++++++++++++++++++++
>   3 files changed, 76 insertions(+)
> 
> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
> index 28a9ad57b6f1..a7dff64e1e24 100644
> --- a/arch/s390/include/asm/uv.h
> +++ b/arch/s390/include/asm/uv.h
> @@ -58,6 +58,7 @@
>   #define UVC_CMD_SET_SHARED_ACCESS	0x1000
>   #define UVC_CMD_REMOVE_SHARED_ACCESS	0x1001
>   #define UVC_CMD_RETR_ATTEST		0x1020
> +#define UVC_CMD_ADD_SECRET		0x1031
>   
>   /* Bits in installed uv calls */
>   enum uv_cmds_inst {
> @@ -88,6 +89,7 @@ enum uv_cmds_inst {
>   	BIT_UVC_CMD_DUMP_CPU = 26,
>   	BIT_UVC_CMD_DUMP_COMPLETE = 27,
>   	BIT_UVC_CMD_RETR_ATTEST = 28,
> +	BIT_UVC_CMD_ADD_SECRET = 29,
>   };
>   
>   enum uv_feat_ind {
> @@ -292,6 +294,18 @@ struct uv_cb_dump_complete {
>   	u64 reserved30[5];
>   } __packed __aligned(8);
>   
> +/*
> + * A common call for pv guests that contains a single address

control block struct

> + * Examples:
> + * Add Secret
> + */
> +struct uv_cb_guest_addr {
> +	struct uv_cb_header header;
> +	u64 reserved08[3];
> +	u64 addr;
> +	u64 reserved28[4];
> +} __packed __aligned(8);
> +
[...]

> +static int uvio_add_secret(struct uvio_ioctl_cb *uv_ioctl)
> +{
> +	void __user *user_buf_arg = (void __user *)uv_ioctl->argument_addr;
> +	struct uv_cb_guest_addr uvcb = {
> +		.header.len = sizeof(uvcb),
> +		.header.cmd = UVC_CMD_ADD_SECRET,
> +	};
> +	void *asrcb = NULL;
> +	int ret;
> +
> +	if (uv_ioctl->argument_len > UVIO_ADD_SECRET_MAX_LEN)
> +		return -EINVAL;
> +	if (uv_ioctl->argument_len == 0)
> +		return -EINVAL;
> +
> +	asrcb = kvzalloc(uv_ioctl->argument_len, GFP_KERNEL);
> +	if (!asrcb)
> +		return -EINVAL;

-ENOMEM

> +
> +	ret = -EFAULT;
> +	if (copy_from_user(asrcb, user_buf_arg, uv_ioctl->argument_len))
> +		goto out;
> +
> +	ret = 0;
> +	uvcb.addr = (u64)asrcb;
> +	uv_call_sched(0, (u64)&uvcb);
> +	uv_ioctl->uv_rc = uvcb.header.rc;
> +	uv_ioctl->uv_rrc = uvcb.header.rrc;
> +
> +out:
> +	kvfree(asrcb);
> +	return ret;
> +}
> +
>   static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp,
>   				     unsigned long cmd)
>   {
> @@ -275,6 +330,9 @@ static long uvio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>   	case UVIO_IOCTL_ATT_NR:
>   		ret = uvio_attestation(&uv_ioctl);
>   		break;
> +	case UVIO_IOCTL_ADD_SECRET_NR:
> +		ret = uvio_add_secret(&uv_ioctl);
> +		break;
>   	default:
>   		ret = -ENOIOCTLCMD;
>   		break;

Re: [PATCH v2 3/6] s390/uvdevice: Add 'List Secrets' UVC

[Janosch Frank]

On 5/19/23 11:37, Steffen Eiden wrote:
> Userspace can call the List Secrets Ultravisor Call
> using IOCTLs on the uvdevice.
> During the handling of the new IOCTL nr the uvdevice will do some sanity
> checks first. Then, perform the Ultravisor command, and copy the answer
> to userspace.
> If the List Secrets UV facility is not present, UV will return
> invalid command rc. This won't be fenced in the driver and does not
> result in a negative return value. This is also true for any other
> possible error code the UV can return.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
>[...]
> +/** uvio_list_secrets() - perform a List Secret UVC
> + *
> + * @uv_ioctl: ioctl control block
> + *
> + * uvio_list_secrets() performs the List Secret Ultravisor Call.
> + * It verifies that the given userspace argument address is valid and its size
> + * is sane. Every other check is made by the Ultravisor (UV) and won't result
> + * in a negative return value. It builds the request, performs the UV-call,
> + * and copies the result to userspace.
> + *
> + * The argument specifies the location for the result of the UV-Call.
> + *
> + * If the List Secrets UV facility is not present,
> + * UV will return invalid command rc. This won't be fenced in the driver
> + * and does not result in a negative return value.
> + *
> + * Context: might sleep
> + *
> + * Return: 0 on success or a negative error code on error.
> + */
> +static int uvio_list_secrets(struct uvio_ioctl_cb *uv_ioctl)
> +{
> +	void __user *user_buf_arg = (void __user *)uv_ioctl->argument_addr;
> +	struct uv_cb_guest_addr uvcb = {
> +		.header.len = sizeof(uvcb),
> +		.header.cmd = UVC_CMD_LIST_SECRETS,
> +	};
> +	void *secrets = NULL;
> +	int ret;

int ret = 0;

> +
> +	if (uv_ioctl->argument_len != UVIO_LIST_SECRETS_LEN)
> +		return -EINVAL;

I'd be less uneasy if you 
s/uv_ioctl->argument_len/UVIO_LIST_SECRETS_LEN/ below. Yes, you check 
the length above but it still feels weird to use a variable when we have 
a perfectly fine constant just waiting to be used.

> +
> +	secrets = kvzalloc(uv_ioctl->argument_len, GFP_KERNEL);
> +	if (!secrets)
> +		return -ENOMEM;
> +
> +	uvcb.addr = (u64)secrets;
> +	uv_call_sched(0, (u64)&uvcb);
> +	uv_ioctl->uv_rc = uvcb.header.rc;
> +	uv_ioctl->uv_rrc = uvcb.header.rrc;
> +
> +	if (copy_to_user(user_buf_arg, secrets, uv_ioctl->argument_len))
> +		ret = -EFAULT;

and remove the else

> +	else
> +		ret = 0;
> +
> +	kvfree(secrets);
> +	return ret;
> +}
> +
>   static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp,
>   				     unsigned long cmd)
>   {
> @@ -333,6 +385,9 @@ static long uvio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>   	case UVIO_IOCTL_ADD_SECRET_NR:
>   		ret = uvio_add_secret(&uv_ioctl);
>   		break;
> +	case UVIO_IOCTL_LIST_SECRETS_NR:
> +		ret = uvio_list_secrets(&uv_ioctl);
> +		break;
>   	default:
>   		ret = -ENOIOCTLCMD;
>   		break;

Re: [PATCH v2 4/6] s390/uvdevice: Add 'Lock Secret Store' UVC

[Janosch Frank]

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

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

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

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

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

Re: [PATCH v2 5/6] s390/uv: replace scnprintf with sysfs_emit

[Janosch Frank]

On 5/19/23 11:37, Steffen Eiden wrote:
> Replace scnprintf(page, PAGE_SIZE, ...) with the page size aware
> sysfs_emit(page, ...) which adds some sanity checks.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
>   arch/s390/kernel/uv.c | 36 +++++++++++++++---------------------
>   1 file changed, 15 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index cb2ee06df286..d5b5d7e83c17 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c
> @@ -462,11 +462,11 @@ EXPORT_SYMBOL_GPL(arch_make_page_accessible);
>   static ssize_t uv_query_facilities(struct kobject *kobj,
>   				   struct kobj_attribute *attr, char *page)

The last argument is called buf so let's also change that over to make 
all of the functions look the same.

>   {
> -	return scnprintf(page, PAGE_SIZE, "%lx\n%lx\n%lx\n%lx\n",
> -			uv_info.inst_calls_list[0],
> -			uv_info.inst_calls_list[1],
> -			uv_info.inst_calls_list[2],
> -			uv_info.inst_calls_list[3]);
> +	return sysfs_emit(page, "%lx\n%lx\n%lx\n%lx\n",
> +			  uv_info.inst_calls_list[0],
> +			  uv_info.inst_calls_list[1],
> +			  uv_info.inst_calls_list[2],
> +			  uv_info.inst_calls_list[3]);
>   }
>   
>   static struct kobj_attribute uv_query_facilities_attr =
> @@ -493,8 +493,7 @@ static struct kobj_attribute uv_query_supp_se_hdr_pcf_attr =
>   static ssize_t uv_query_dump_cpu_len(struct kobject *kobj,
>   				     struct kobj_attribute *attr, char *page)
>   {
> -	return scnprintf(page, PAGE_SIZE, "%lx\n",
> -			uv_info.guest_cpu_stor_len);
> +	return sysfs_emit(page, "%lx\n", uv_info.guest_cpu_stor_len);
>   }
>   
>   static struct kobj_attribute uv_query_dump_cpu_len_attr =
> @@ -503,8 +502,7 @@ static struct kobj_attribute uv_query_dump_cpu_len_attr =
>   static ssize_t uv_query_dump_storage_state_len(struct kobject *kobj,
>   					       struct kobj_attribute *attr, char *page)
>   {
> -	return scnprintf(page, PAGE_SIZE, "%lx\n",
> -			uv_info.conf_dump_storage_state_len);
> +	return sysfs_emit(page, "%lx\n", uv_info.conf_dump_storage_state_len);
>   }
>   
>   static struct kobj_attribute uv_query_dump_storage_state_len_attr =
> @@ -513,8 +511,7 @@ static struct kobj_attribute uv_query_dump_storage_state_len_attr =
>   static ssize_t uv_query_dump_finalize_len(struct kobject *kobj,
>   					  struct kobj_attribute *attr, char *page)
>   {
> -	return scnprintf(page, PAGE_SIZE, "%lx\n",
> -			uv_info.conf_dump_finalize_len);
> +	return sysfs_emit(page, "%lx\n", uv_info.conf_dump_finalize_len);
>   }
>   
>   static struct kobj_attribute uv_query_dump_finalize_len_attr =
> @@ -532,8 +529,7 @@ static struct kobj_attribute uv_query_feature_indications_attr =
>   static ssize_t uv_query_max_guest_cpus(struct kobject *kobj,
>   				       struct kobj_attribute *attr, char *page)
>   {
> -	return scnprintf(page, PAGE_SIZE, "%d\n",
> -			uv_info.max_guest_cpu_id + 1);
> +	return sysfs_emit(page, "%d\n", uv_info.max_guest_cpu_id + 1);
>   }
>   
>   static struct kobj_attribute uv_query_max_guest_cpus_attr =
> @@ -542,8 +538,7 @@ static struct kobj_attribute uv_query_max_guest_cpus_attr =
>   static ssize_t uv_query_max_guest_vms(struct kobject *kobj,
>   				      struct kobj_attribute *attr, char *page)
>   {
> -	return scnprintf(page, PAGE_SIZE, "%d\n",
> -			uv_info.max_num_sec_conf);
> +	return sysfs_emit(page, "%d\n", uv_info.max_num_sec_conf);
>   }
>   
>   static struct kobj_attribute uv_query_max_guest_vms_attr =
> @@ -552,8 +547,7 @@ static struct kobj_attribute uv_query_max_guest_vms_attr =
>   static ssize_t uv_query_max_guest_addr(struct kobject *kobj,
>   				       struct kobj_attribute *attr, char *page)
>   {
> -	return scnprintf(page, PAGE_SIZE, "%lx\n",
> -			uv_info.max_sec_stor_addr);
> +	return sysfs_emit(page, "%lx\n", uv_info.max_sec_stor_addr);
>   }
>   
>   static struct kobj_attribute uv_query_max_guest_addr_attr =
> @@ -562,7 +556,7 @@ static struct kobj_attribute uv_query_max_guest_addr_attr =
>   static ssize_t uv_query_supp_att_req_hdr_ver(struct kobject *kobj,
>   					     struct kobj_attribute *attr, char *page)
>   {
> -	return scnprintf(page, PAGE_SIZE, "%lx\n", uv_info.supp_att_req_hdr_ver);
> +	return sysfs_emit(page, "%lx\n", uv_info.supp_att_req_hdr_ver);
>   }
>   
>   static struct kobj_attribute uv_query_supp_att_req_hdr_ver_attr =
> @@ -571,7 +565,7 @@ static struct kobj_attribute uv_query_supp_att_req_hdr_ver_attr =
>   static ssize_t uv_query_supp_att_pflags(struct kobject *kobj,
>   					struct kobj_attribute *attr, char *page)
>   {
> -	return scnprintf(page, PAGE_SIZE, "%lx\n", uv_info.supp_att_pflags);
> +	return sysfs_emit(page, "%lx\n", uv_info.supp_att_pflags);
>   }
>   
>   static struct kobj_attribute uv_query_supp_att_pflags_attr =
> @@ -605,7 +599,7 @@ static ssize_t uv_is_prot_virt_guest(struct kobject *kobj,
>   #ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
>   	val = prot_virt_guest;
>   #endif
> -	return scnprintf(page, PAGE_SIZE, "%d\n", val);
> +	return sysfs_emit(page, "%d\n", val);
>   }
>   
>   static ssize_t uv_is_prot_virt_host(struct kobject *kobj,
> @@ -617,7 +611,7 @@ static ssize_t uv_is_prot_virt_host(struct kobject *kobj,
>   	val = prot_virt_host;
>   #endif
>   
> -	return scnprintf(page, PAGE_SIZE, "%d\n", val);
> +	return sysfs_emit(page, "%d\n", val);
>   }
>   
>   static struct kobj_attribute uv_prot_virt_guest =

Re: [PATCH v2 6/6] s390/uv: Update query for secret-UVCs

[Janosch Frank]

On 5/19/23 11:37, Steffen Eiden wrote:
> Update the query struct such that secret-UVC related
> information can be parsed.
> Add sysfs files for these new values.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>

You should add a bit of information about the fields.
Not a large amount but having no info doesn't help anyone without access 
to our design documents.

> ---

[...]

>   extern struct uv_info uv_info;
> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index d5b5d7e83c17..c0a1bfb01135 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c
> @@ -571,6 +571,42 @@ static ssize_t uv_query_supp_att_pflags(struct kobject *kobj,
>   static struct kobj_attribute uv_query_supp_att_pflags_attr =
>   	__ATTR(supp_att_pflags, 0444, uv_query_supp_att_pflags, NULL);
>   
> +static ssize_t uv_query_supp_add_secret_req_ver(struct kobject *kobj,
> +						struct kobj_attribute *attr, char *page)

s/page/buf/ for the whole lot.

> +{
> +	return sysfs_emit(page, "%lx\n", uv_info.supp_add_secret_req_ver);
> +}
> +
> +static struct kobj_attribute uv_query_supp_add_secret_req_ver_attr =
> +	__ATTR(supp_add_secret_req_ver, 0444, uv_query_supp_add_secret_req_ver, NULL);
> +
> +static ssize_t uv_query_supp_add_secret_pcf(struct kobject *kobj,
> +					    struct kobj_attribute *attr, char *page)
> +{
> +	return sysfs_emit(page, "%lx\n", uv_info.supp_add_secret_pcf);
> +}
> +
> +static struct kobj_attribute uv_query_supp_add_secret_pcf_attr =
> +	__ATTR(supp_add_secret_pcf, 0444, uv_query_supp_add_secret_pcf, NULL);
> +
> +static ssize_t uv_query_supp_secret_types(struct kobject *kobj,
> +					  struct kobj_attribute *attr, char *page)
> +{
> +	return sysfs_emit(page, "%lx\n", uv_info.supp_secret_types);
> +}
> +
> +static struct kobj_attribute uv_query_supp_secret_types_attr =
> +	__ATTR(supp_secret_types, 0444, uv_query_supp_secret_types, NULL);
> +

> +static ssize_t uv_query_max_secrets(struct kobject *kobj,
> +				    struct kobj_attribute *attr, char *page)
> +{
> +	return sysfs_emit(page, "%d\n", uv_info.max_secrets);
> +}

uv_query_max_secrets
but then
uv_query_max_num_secrets_attr

To num or not to num. That's the question

> +
> +static struct kobj_attribute uv_query_max_num_secrets_attr =
> +	__ATTR(max_secrets, 0444, uv_query_max_secrets, NULL);

And here the num is gone again.

> +
>   static struct attribute *uv_query_attrs[] = {
>   	&uv_query_facilities_attr.attr,
>   	&uv_query_feature_indications_attr.attr,
> @@ -584,6 +620,10 @@ static struct attribute *uv_query_attrs[] = {
>   	&uv_query_dump_cpu_len_attr.attr,
>   	&uv_query_supp_att_req_hdr_ver_attr.attr,
>   	&uv_query_supp_att_pflags_attr.attr,
> +	&uv_query_supp_add_secret_req_ver_attr.attr,
> +	&uv_query_supp_add_secret_pcf_attr.attr,
> +	&uv_query_supp_secret_types_attr.attr,
> +	&uv_query_max_num_secrets_attr.attr,
>   	NULL,
>   };
>