On 29 Feb 12:47, Vegard Nossum wrote:
On 07/02/2024 08:24, Saeed Mahameed wrote:
+static int mlx5ctl_info_ioctl(struct file *file,
+ struct mlx5ctl_info __user *arg,
+ size_t usize)
+{
+ struct mlx5ctl_fd *mfd = file->private_data;
+ size_t ksize = sizeof(struct mlx5ctl_info);
+ struct mlx5ctl_dev *mcdev = mfd->mcdev;
+ struct mlx5_core_dev *mdev = mcdev->mdev;
+ struct mlx5ctl_info *info;
+ int err = 0;
+
+ if (usize < ksize)
+ return -EINVAL;
+
+ info = kzalloc(ksize, GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
struct mlx5ctl_info is small, why not put it on the stack or even copy
it directly from the original object, assuming it has no holes/padding?
There's no original object, but yes storing it on the stack should work.
+
+ info->dev_uctx_cap = MLX5_CAP_GEN(mdev, uctx_cap);
+ info->uctx_cap = mfd->uctx_cap;
+ info->uctx_uid = mfd->uctx_uid;
+ info->ucap = mfd->ucap;
+
+ if (copy_to_user(arg, info, ksize))
+ err = -EFAULT;
+
+ kfree(info);
+ return err;
+}
Is there even a remote possibility of extending this structure in the
future? If so the size check will not allow you to be backwards
compatible. Should there be a version field in there or would you
just add a new ioctl altogether? Adding linux-api@xxxxxxxxxxxxxxx to Cc.
This was my original implementation, but Greg's preference is to allow no
extension to the ioctl structures, in case of extension required, new IOCTL
and structure should be introduced.
diff --git a/include/uapi/misc/mlx5ctl.h b/include/uapi/misc/mlx5ctl.h
new file mode 100644
index 000000000000..9be944128025
--- /dev/null
+++ b/include/uapi/misc/mlx5ctl.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 WITH Linux-syscall-note */
+/* Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. */
+
+#ifndef __MLX5CTL_IOCTL_H__
+#define __MLX5CTL_IOCTL_H__
+
+struct mlx5ctl_info {
+ __u16 uctx_uid; /* current process allocated UCTX UID */
+ __u16 reserved1; /* explicit padding must be zero */
+ __u32 uctx_cap; /* current process effective UCTX cap */
+ __u32 dev_uctx_cap; /* device's UCTX capabilities */
+ __u32 ucap; /* process user capability */
+};
+
+#define MLX5CTL_IOCTL_MAGIC 0x5c
+
+#define MLX5CTL_IOCTL_INFO \
+ _IOR(MLX5CTL_IOCTL_MAGIC, 0x0, struct mlx5ctl_info)
+
+#endif /* __MLX5CTL_IOCTL_H__ */
Should you add anything to Documentation/ABI/ ? (Or add other
documentation for this driver?)
The driver doesn't expose any sysfs other than the IOCTLs, but yes
a documentation might be useful to make sure ABI is stable, most of the
other drivers point out to the uapi header for documentation.
