[V4,4/5] misc: mlx5ctl: Add command rpc ioctl

Message ID 20240207072435.14182-5-saeed@kernel.org
State New
Headers
Series mlx5 ConnectX control misc driver |

Commit Message

Saeed Mahameed Feb. 7, 2024, 7:24 a.m. UTC
  From: Saeed Mahameed <saeedm@nvidia.com>

Add new IOCTL to allow user space to send device debug rpcs and
attach the user's uctx UID to each rpc.

In the mlx5 architecture the FW RPC commands are of the format of
inbox and outbox buffers. The inbox buffer contains the command
rpc layout as described in the ConnectX Programmers Reference Manual
(PRM) document and as defined in include/linux/mlx5/mlx5_ifc.h.

On success the user outbox buffer will be filled with the device's rpc
response.

For example to query device capabilities:
a user fills out an inbox buffer with the inbox layout:
   struct mlx5_ifc_query_hca_cap_in_bits
and expects an outbox buffer with the layout:
   struct mlx5_ifc_cmd_hca_cap_bits

Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/misc/mlx5ctl/main.c | 98 +++++++++++++++++++++++++++++++++++++
 include/uapi/misc/mlx5ctl.h | 12 +++++
 2 files changed, 110 insertions(+)
  

Comments

Vegard Nossum Feb. 29, 2024, 11:49 a.m. UTC | #1
On 07/02/2024 08:24, Saeed Mahameed wrote:
> @@ -328,6 +420,11 @@ static int mlx5ctl_probe(struct auxiliary_device *adev,
>   		goto abort;
>   	}
>   
> +	err = sysfs_create_link_nowarn(&mcdev->miscdev.this_device->kobj,
> +				       &mdev->device->kobj, "mdev");
> +	if (err)
> +		mlx5ctl_dbg(mcdev, "mlx5ctl: failed to create sysfs link err %d\n", err);
> +

Should this propagate the error to the caller?

What happens if/when mlx5ctl_remove()/sysfs_remove_link() gets called
later for this kobj?


Vegard
  
Saeed Mahameed March 2, 2024, 7:48 a.m. UTC | #2
On 29 Feb 12:49, Vegard Nossum wrote:
>
>On 07/02/2024 08:24, Saeed Mahameed wrote:
>>@@ -328,6 +420,11 @@ static int mlx5ctl_probe(struct auxiliary_device *adev,
>>  		goto abort;
>>  	}
>>+	err = sysfs_create_link_nowarn(&mcdev->miscdev.this_device->kobj,
>>+				       &mdev->device->kobj, "mdev");
>>+	if (err)
>>+		mlx5ctl_dbg(mcdev, "mlx5ctl: failed to create sysfs link err %d\n", err);
>>+
>
>Should this propagate the error to the caller?
>

this link is informational only and not necessary for the driver function,
it meant to help user-space apps to associate mlx5ctl driver with it parent 
mlx5_core device.

>What happens if/when mlx5ctl_remove()/sysfs_remove_link() gets called
>later for this kobj?
>

sysfs_remove_link() will eventually call kernfs_remove_by_name_ns()
and it will return -ENOENT; if not found, and it will be silently dropped.
  

Patch

diff --git a/drivers/misc/mlx5ctl/main.c b/drivers/misc/mlx5ctl/main.c
index e4e70359dbe8..c02b80efffc1 100644
--- a/drivers/misc/mlx5ctl/main.c
+++ b/drivers/misc/mlx5ctl/main.c
@@ -245,6 +245,94 @@  static int mlx5ctl_info_ioctl(struct file *file,
 	return err;
 }
 
+struct mlx5_ifc_mbox_in_hdr_bits {
+	u8         opcode[0x10];
+	u8         uid[0x10];
+
+	u8         reserved_at_20[0x10];
+	u8         op_mod[0x10];
+
+	u8         reserved_at_40[0x40];
+};
+
+struct mlx5_ifc_mbox_out_hdr_bits {
+	u8         status[0x8];
+	u8         reserved_at_8[0x18];
+
+	u8         syndrome[0x20];
+
+	u8         reserved_at_40[0x40];
+};
+
+static int mlx5ctl_cmdrpc_ioctl(struct file *file,
+				struct mlx5ctl_cmdrpc __user *arg,
+				size_t usize)
+{
+	size_t ksize = sizeof(struct mlx5ctl_cmdrpc);
+	struct mlx5ctl_fd *mfd = file->private_data;
+	struct mlx5ctl_dev *mcdev = mfd->mcdev;
+	struct mlx5ctl_cmdrpc *rpc = NULL;
+	void *in = NULL, *out = NULL;
+	int err;
+
+	if (usize < ksize)
+		return -EINVAL;
+
+	rpc = kzalloc(ksize, GFP_KERNEL);
+	if (!rpc)
+		return -ENOMEM;
+
+	err = copy_from_user(rpc, arg, usize);
+	if (err)
+		goto out;
+
+	mlx5ctl_dbg(mcdev, "[UID %d] cmdrpc: inlen %d outlen %d\n",
+		    mfd->uctx_uid, rpc->inlen, rpc->outlen);
+
+	if (rpc->inlen < MLX5_ST_SZ_BYTES(mbox_in_hdr) ||
+	    rpc->outlen < MLX5_ST_SZ_BYTES(mbox_out_hdr) ||
+	    rpc->inlen > MLX5CTL_MAX_RPC_SIZE ||
+	    rpc->outlen > MLX5CTL_MAX_RPC_SIZE) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	in = memdup_user(u64_to_user_ptr(rpc->in), rpc->inlen);
+	if (IS_ERR(in)) {
+		err = PTR_ERR(in);
+		goto out;
+	}
+
+	out = kvzalloc(rpc->outlen, GFP_KERNEL);
+	if (!out) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	mlx5ctl_dbg(mcdev, "[UID %d] cmdif: opcode 0x%x inlen %d outlen %d\n",
+		    mfd->uctx_uid,
+		    MLX5_GET(mbox_in_hdr, in, opcode), rpc->inlen, rpc->outlen);
+
+	MLX5_SET(mbox_in_hdr, in, uid, mfd->uctx_uid);
+	err = mlx5_cmd_do(mcdev->mdev, in, rpc->inlen, out, rpc->outlen);
+	mlx5ctl_dbg(mcdev, "[UID %d] cmdif: opcode 0x%x retval %d\n",
+		    mfd->uctx_uid,
+		    MLX5_GET(mbox_in_hdr, in, opcode), err);
+
+	/* -EREMOTEIO means outbox is valid, but out.status is not */
+	if (!err || err == -EREMOTEIO) {
+		err = 0;
+		if (copy_to_user(u64_to_user_ptr(rpc->out), out, rpc->outlen))
+			err = -EFAULT;
+	}
+
+out:
+	kvfree(out);
+	kfree(in);
+	kfree(rpc);
+	return err;
+}
+
 static long mlx5ctl_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	struct mlx5ctl_fd *mfd = file->private_data;
@@ -270,6 +358,10 @@  static long mlx5ctl_ioctl(struct file *file, unsigned int cmd, unsigned long arg
 		err = mlx5ctl_info_ioctl(file, argp, size);
 		break;
 
+	case MLX5CTL_IOCTL_CMDRPC:
+		err = mlx5ctl_cmdrpc_ioctl(file, argp, size);
+		break;
+
 	default:
 		mlx5ctl_dbg(mcdev, "Unknown ioctl %x\n", cmd);
 		err = -ENOTTY;
@@ -328,6 +420,11 @@  static int mlx5ctl_probe(struct auxiliary_device *adev,
 		goto abort;
 	}
 
+	err = sysfs_create_link_nowarn(&mcdev->miscdev.this_device->kobj,
+				       &mdev->device->kobj, "mdev");
+	if (err)
+		mlx5ctl_dbg(mcdev, "mlx5ctl: failed to create sysfs link err %d\n", err);
+
 	mlx5ctl_dbg(mcdev, "probe mdev@%s %s\n",
 		    dev_driver_string(mdev->device),
 		    dev_name(mdev->device));
@@ -348,6 +445,7 @@  static void mlx5ctl_remove(struct auxiliary_device *adev)
 	struct mlx5_core_dev *mdev = mcdev->mdev;
 	struct mlx5ctl_fd *mfd, *n;
 
+	sysfs_remove_link(&mcdev->miscdev.this_device->kobj, "mdev");
 	misc_deregister(&mcdev->miscdev);
 	down_write(&mcdev->rw_lock);
 
diff --git a/include/uapi/misc/mlx5ctl.h b/include/uapi/misc/mlx5ctl.h
index 9be944128025..1e4622c5979f 100644
--- a/include/uapi/misc/mlx5ctl.h
+++ b/include/uapi/misc/mlx5ctl.h
@@ -12,9 +12,21 @@  struct mlx5ctl_info {
 	__u32 ucap; /* process user capability */
 };
 
+struct mlx5ctl_cmdrpc {
+	__aligned_u64 in; /* RPC inbox buffer user address */
+	__aligned_u64 out; /* RPC outbox buffer user address */
+	__u32 inlen; /* inbox buffer length */
+	__u32 outlen; /* outbox buffer length */
+};
+
+#define MLX5CTL_MAX_RPC_SIZE (512 * 512) /* max FW RPC buffer size 512 blocks of 512 bytes */
+
 #define MLX5CTL_IOCTL_MAGIC 0x5c
 
 #define MLX5CTL_IOCTL_INFO \
 	_IOR(MLX5CTL_IOCTL_MAGIC, 0x0, struct mlx5ctl_info)
 
+#define MLX5CTL_IOCTL_CMDRPC \
+	_IOWR(MLX5CTL_IOCTL_MAGIC, 0x1, struct mlx5ctl_cmdrpc)
+
 #endif /* __MLX5CTL_IOCTL_H__ */