From: Saeed Mahameed <[email protected]>
Hello Greg and Arnd,
The ConnectX HW family supported by the mlx5 drivers uses an architecture
where a FW component executes "mailbox RPCs" issued by the driver to make
changes to the device. This results in a complex debugging environment
where the FW component has information and complex low level state that
needs to be accessed to userspace for debugging purposes.
Historically a userspace program was used that accessed the PCI register
and config space directly through /sys/bus/pci/.../XXX and could operate
these debugging interfaces in parallel with the running driver.
This approach is incompatible with secure boot and kernel lockdown so this
driver provides a secure and restricted interface to that.
To solve this we add a misc driver "mlx5ctl" that would interface with
mlx5_core ConnectX driver to access the underlaying device debug
information.
1) The first patch in the series introduces the main driver file with the
implementation of a new mlx5 auxiliary device driver to run on top
mlx5_core device instances, on probe it creates a new misc device and in
this patch we implement the open and release fops, On open the driver
would allocate a special FW UID (user context ID) restricted to debug
RPCs only, where all user debug RPCs will be executed under this UID,
and on release the UID will be freed.
2) The second patch adds an info ioctl that will show the allocated UID
and the available capability masks of the device and the current UID, and
some other useful device information such as the underlying ConnectX
device
Example:
$ mlx5ctl mlx5_core.ctl.0
mlx5dev: 0000:00:04.0
UCTX UID: 1
UCTX CAP: 0x3
DEV UCTX CAP: 0x3
USER CAP: 0x1d
3) Third patch adds RPC ioctl to execute debug RPCs under the
special UID.
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 linux/include/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
4) The fourth patch adds the ability to register user memory into the
ConntectX device and create a umem object that points to that memory.
Command rpc outbox buffer is limited in size, which can be very
annoying when trying to pull large traces out of the device.
Many rpcs offer the ability to scatter output traces, contexts
and logs directly into user space buffers in a single shot.
The registered/pinned memory will be described by a device UMEM object
which has a unique umem_id, this umem_id can be later used in the rpc
inbox to tell the device where to populate the response output,
e.g HW traces and other debug object queries.
Example usecase, a ConnectX device coredump can be as large as 2MB.
Using inline rpcs will take thousands of rpcs to get the full
coredump which can consume multiple seconds.
With UMEM, it can be done in a single rpc, using 2MB of umem user buffer.
Other usecases with umem:
- dynamic HW and FW trace monitoring
- high frequency diagnostic counters sampling
- batched objects and resource dumps
See links below for information about user-space tools that use this
interface:
[1] https://github.com/saeedtx/mlx5ctl
[2] https://github.com/Mellanox/mstflint
see:
d) mstregdump utility
This utility dumps hardware registers from Mellanox hardware
for later analysis by Mellanox.
g) mstconfig
This tool sets or queries non-volatile configurable options
i) mstreg
The mlxreg utility allows users to obtain information
regarding supported access registers, such as their fields
Saeed Mahameed (5):
mlx5: Add aux dev for ctl interface
misc: mlx5ctl: Add mlx5ctl misc driver
misc: mlx5ctl: Add info ioctl
misc: mlx5ctl: Add command rpc ioctl
misc: mlx5ctl: Add umem reg/unreg ioctl
.../userspace-api/ioctl/ioctl-number.rst | 1 +
drivers/misc/Kconfig | 1 +
drivers/misc/Makefile | 1 +
drivers/misc/mlx5ctl/Kconfig | 14 +
drivers/misc/mlx5ctl/Makefile | 5 +
drivers/misc/mlx5ctl/main.c | 528 ++++++++++++++++++
drivers/misc/mlx5ctl/umem.c | 325 +++++++++++
drivers/misc/mlx5ctl/umem.h | 17 +
drivers/net/ethernet/mellanox/mlx5/core/dev.c | 8 +
include/uapi/misc/mlx5ctl.h | 51 ++
10 files changed, 951 insertions(+)
create mode 100644 drivers/misc/mlx5ctl/Kconfig
create mode 100644 drivers/misc/mlx5ctl/Makefile
create mode 100644 drivers/misc/mlx5ctl/main.c
create mode 100644 drivers/misc/mlx5ctl/umem.c
create mode 100644 drivers/misc/mlx5ctl/umem.h
create mode 100644 include/uapi/misc/mlx5ctl.h
--
2.41.0
From: Saeed Mahameed <[email protected]>
The ConnectX HW family supported by the mlx5 drivers uses an architecture
where a FW component executes "mailbox RPCs" issued by the driver to make
changes to the device. This results in a complex debugging environment
where the FW component has information and low level configuration that
needs to be accessed to userspace for debugging purposes.
Historically a userspace program was used that accessed the PCI register
and config space directly through /sys/bus/pci/.../XXX and could operate
these debugging interfaces in parallel with the running driver.
This approach is incompatible with secure boot and kernel lockdown so this
driver provides a secure and restricted interface to that same data.
On open the driver would allocate a special FW UID (user context ID)
restrected to debug RPCs only, later in this series all user RPCs will
be stamped with this UID.
Reviewed-by: Leon Romanovsky <[email protected]>
Reviewed-by: Jason Gunthorpe <[email protected]>
Signed-off-by: Saeed Mahameed <[email protected]>
---
drivers/misc/Kconfig | 1 +
drivers/misc/Makefile | 1 +
drivers/misc/mlx5ctl/Kconfig | 14 ++
drivers/misc/mlx5ctl/Makefile | 4 +
drivers/misc/mlx5ctl/main.c | 314 ++++++++++++++++++++++++++++++++++
5 files changed, 334 insertions(+)
create mode 100644 drivers/misc/mlx5ctl/Kconfig
create mode 100644 drivers/misc/mlx5ctl/Makefile
create mode 100644 drivers/misc/mlx5ctl/main.c
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index cadd4a820c03..b46bd8edc348 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -579,4 +579,5 @@ source "drivers/misc/cardreader/Kconfig"
source "drivers/misc/uacce/Kconfig"
source "drivers/misc/pvpanic/Kconfig"
source "drivers/misc/mchp_pci1xxxx/Kconfig"
+source "drivers/misc/mlx5ctl/Kconfig"
endmenu
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index f2a4d1ff65d4..49bc4697f498 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -67,3 +67,4 @@ obj-$(CONFIG_TMR_MANAGER) += xilinx_tmr_manager.o
obj-$(CONFIG_TMR_INJECT) += xilinx_tmr_inject.o
obj-$(CONFIG_TPS6594_ESM) += tps6594-esm.o
obj-$(CONFIG_TPS6594_PFSM) += tps6594-pfsm.o
+obj-$(CONFIG_MLX5CTL) += mlx5ctl/
diff --git a/drivers/misc/mlx5ctl/Kconfig b/drivers/misc/mlx5ctl/Kconfig
new file mode 100644
index 000000000000..faaa1dba2cc2
--- /dev/null
+++ b/drivers/misc/mlx5ctl/Kconfig
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+
+config MLX5CTL
+ tristate "mlx5 ConnectX control misc driver"
+ depends on MLX5_CORE
+ help
+ MLX5CTL provides interface for the user process to access the debug and
+ configuration registers of the ConnectX hardware family
+ (NICs, PCI switches and SmartNIC SoCs).
+ This will allow configuration and debug tools to work out of the box on
+ mainstream kernel.
+
+ If you don't know what to do here, say N.
diff --git a/drivers/misc/mlx5ctl/Makefile b/drivers/misc/mlx5ctl/Makefile
new file mode 100644
index 000000000000..b5c7f99e0ab6
--- /dev/null
+++ b/drivers/misc/mlx5ctl/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_MLX5CTL) += mlx5ctl.o
+mlx5ctl-y := main.o
diff --git a/drivers/misc/mlx5ctl/main.c b/drivers/misc/mlx5ctl/main.c
new file mode 100644
index 000000000000..de8d6129432c
--- /dev/null
+++ b/drivers/misc/mlx5ctl/main.c
@@ -0,0 +1,314 @@
+// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
+/* Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. */
+
+#include <linux/miscdevice.h>
+#include <linux/fs.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/auxiliary_bus.h>
+#include <linux/mlx5/device.h>
+#include <linux/mlx5/driver.h>
+#include <linux/atomic.h>
+#include <linux/refcount.h>
+
+MODULE_DESCRIPTION("mlx5 ConnectX control misc driver");
+MODULE_AUTHOR("Saeed Mahameed <[email protected]>");
+MODULE_LICENSE("Dual BSD/GPL");
+
+struct mlx5ctl_dev {
+ struct mlx5_core_dev *mdev;
+ struct miscdevice miscdev;
+ struct auxiliary_device *adev;
+ struct list_head fd_list;
+ spinlock_t fd_list_lock; /* protect list add/del */
+ struct rw_semaphore rw_lock;
+ struct kref refcount;
+};
+
+struct mlx5ctl_fd {
+ u16 uctx_uid;
+ u32 uctx_cap;
+ u32 ucap; /* user cap */
+ struct mlx5ctl_dev *mcdev;
+ struct list_head list;
+};
+
+#define mlx5ctl_err(mcdev, format, ...) \
+ dev_err(mcdev->miscdev.parent, format, ##__VA_ARGS__)
+
+#define mlx5ctl_dbg(mcdev, format, ...) \
+ dev_dbg(mcdev->miscdev.parent, "PID %d: " format, \
+ current->pid, ##__VA_ARGS__)
+
+enum {
+ MLX5_UCTX_OBJECT_CAP_RAW_TX = 0x1,
+ MLX5_UCTX_OBJECT_CAP_INTERNAL_DEVICE_RESOURCES = 0x2,
+ MLX5_UCTX_OBJECT_CAP_TOOLS_RESOURCES = 0x4,
+};
+
+static int mlx5ctl_alloc_uid(struct mlx5ctl_dev *mcdev, u32 cap)
+{
+ u32 out[MLX5_ST_SZ_DW(create_uctx_out)] = {};
+ u32 in[MLX5_ST_SZ_DW(create_uctx_in)] = {};
+ void *uctx;
+ int err;
+ u16 uid;
+
+ uctx = MLX5_ADDR_OF(create_uctx_in, in, uctx);
+
+ mlx5ctl_dbg(mcdev, "MLX5_CMD_OP_CREATE_UCTX: caps 0x%x\n", cap);
+ MLX5_SET(create_uctx_in, in, opcode, MLX5_CMD_OP_CREATE_UCTX);
+ MLX5_SET(uctx, uctx, cap, cap);
+
+ err = mlx5_cmd_exec(mcdev->mdev, in, sizeof(in), out, sizeof(out));
+ if (err)
+ return err;
+
+ uid = MLX5_GET(create_uctx_out, out, uid);
+ mlx5ctl_dbg(mcdev, "allocated uid %d with caps 0x%x\n", uid, cap);
+ return uid;
+}
+
+static void mlx5ctl_release_uid(struct mlx5ctl_dev *mcdev, u16 uid)
+{
+ u32 in[MLX5_ST_SZ_DW(destroy_uctx_in)] = {};
+ struct mlx5_core_dev *mdev = mcdev->mdev;
+ int err;
+
+ MLX5_SET(destroy_uctx_in, in, opcode, MLX5_CMD_OP_DESTROY_UCTX);
+ MLX5_SET(destroy_uctx_in, in, uid, uid);
+
+ err = mlx5_cmd_exec_in(mdev, destroy_uctx, in);
+ mlx5ctl_dbg(mcdev, "released uid %d err(%d)\n", uid, err);
+}
+
+static void mcdev_get(struct mlx5ctl_dev *mcdev);
+static void mcdev_put(struct mlx5ctl_dev *mcdev);
+
+static int mlx5ctl_open_mfd(struct mlx5ctl_fd *mfd)
+{
+ struct mlx5_core_dev *mdev = mfd->mcdev->mdev;
+ struct mlx5ctl_dev *mcdev = mfd->mcdev;
+ u32 ucap = 0, cap = 0;
+ int uid;
+
+#define MLX5_UCTX_CAP(mdev, cap) \
+ (MLX5_CAP_GEN(mdev, uctx_cap) & MLX5_UCTX_OBJECT_CAP_##cap)
+
+ if (capable(CAP_NET_RAW) && MLX5_UCTX_CAP(mdev, RAW_TX)) {
+ ucap |= CAP_NET_RAW;
+ cap |= MLX5_UCTX_OBJECT_CAP_RAW_TX;
+ }
+
+ if (capable(CAP_SYS_RAWIO) && MLX5_UCTX_CAP(mdev, INTERNAL_DEVICE_RESOURCES)) {
+ ucap |= CAP_SYS_RAWIO;
+ cap |= MLX5_UCTX_OBJECT_CAP_INTERNAL_DEVICE_RESOURCES;
+ }
+
+ if (capable(CAP_SYS_ADMIN) && MLX5_UCTX_CAP(mdev, TOOLS_RESOURCES)) {
+ ucap |= CAP_SYS_ADMIN;
+ cap |= MLX5_UCTX_OBJECT_CAP_TOOLS_RESOURCES;
+ }
+
+ uid = mlx5ctl_alloc_uid(mcdev, cap);
+ if (uid < 0)
+ return uid;
+
+ mfd->uctx_uid = uid;
+ mfd->uctx_cap = cap;
+ mfd->ucap = ucap;
+ mfd->mcdev = mcdev;
+
+ mlx5ctl_dbg(mcdev, "allocated uid %d with uctx caps 0x%x, user cap 0x%x\n",
+ uid, cap, ucap);
+ return 0;
+}
+
+static void mlx5ctl_release_mfd(struct mlx5ctl_fd *mfd)
+{
+ struct mlx5ctl_dev *mcdev = mfd->mcdev;
+
+ mlx5ctl_release_uid(mcdev, mfd->uctx_uid);
+}
+
+static int mlx5ctl_open(struct inode *inode, struct file *file)
+{
+ struct mlx5_core_dev *mdev;
+ struct mlx5ctl_dev *mcdev;
+ struct mlx5ctl_fd *mfd;
+ int err = 0;
+
+ mcdev = container_of(file->private_data, struct mlx5ctl_dev, miscdev);
+ mcdev_get(mcdev);
+ down_read(&mcdev->rw_lock);
+ mdev = mcdev->mdev;
+ if (!mdev) {
+ err = -ENODEV;
+ goto unlock;
+ }
+
+ mfd = kzalloc(sizeof(*mfd), GFP_KERNEL_ACCOUNT);
+ if (!mfd)
+ return -ENOMEM;
+
+ mfd->mcdev = mcdev;
+ err = mlx5ctl_open_mfd(mfd);
+ if (err)
+ goto unlock;
+
+ spin_lock(&mcdev->fd_list_lock);
+ list_add_tail(&mfd->list, &mcdev->fd_list);
+ spin_unlock(&mcdev->fd_list_lock);
+
+ file->private_data = mfd;
+
+unlock:
+ up_read(&mcdev->rw_lock);
+ if (err) {
+ mcdev_put(mcdev);
+ kfree(mfd);
+ }
+ return err;
+}
+
+static int mlx5ctl_release(struct inode *inode, struct file *file)
+{
+ struct mlx5ctl_fd *mfd = file->private_data;
+ struct mlx5ctl_dev *mcdev = mfd->mcdev;
+
+ down_read(&mcdev->rw_lock);
+ if (!mcdev->mdev) {
+ pr_debug("[%d] UID %d mlx5ctl: mdev is already released\n",
+ current->pid, mfd->uctx_uid);
+ /* All mfds are already released, skip ... */
+ goto unlock;
+ }
+
+ spin_lock(&mcdev->fd_list_lock);
+ list_del(&mfd->list);
+ spin_unlock(&mcdev->fd_list_lock);
+
+ mlx5ctl_release_mfd(mfd);
+
+unlock:
+ kfree(mfd);
+ up_read(&mcdev->rw_lock);
+ mcdev_put(mcdev);
+ file->private_data = NULL;
+ return 0;
+}
+
+static const struct file_operations mlx5ctl_fops = {
+ .owner = THIS_MODULE,
+ .open = mlx5ctl_open,
+ .release = mlx5ctl_release,
+};
+
+static int mlx5ctl_probe(struct auxiliary_device *adev,
+ const struct auxiliary_device_id *id)
+
+{
+ struct mlx5_adev *madev = container_of(adev, struct mlx5_adev, adev);
+ struct mlx5_core_dev *mdev = madev->mdev;
+ struct mlx5ctl_dev *mcdev;
+ char *devname = NULL;
+ int err;
+
+ mcdev = kzalloc(sizeof(*mcdev), GFP_KERNEL_ACCOUNT);
+ if (!mcdev)
+ return -ENOMEM;
+
+ kref_init(&mcdev->refcount);
+ INIT_LIST_HEAD(&mcdev->fd_list);
+ spin_lock_init(&mcdev->fd_list_lock);
+ init_rwsem(&mcdev->rw_lock);
+ mcdev->mdev = mdev;
+ mcdev->adev = adev;
+ devname = kasprintf(GFP_KERNEL_ACCOUNT, "mlx5ctl-%s",
+ dev_name(&adev->dev));
+ if (!devname) {
+ err = -ENOMEM;
+ goto abort;
+ }
+
+ mcdev->miscdev = (struct miscdevice) {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = devname,
+ .fops = &mlx5ctl_fops,
+ .parent = &adev->dev,
+ };
+
+ err = misc_register(&mcdev->miscdev);
+ if (err) {
+ mlx5ctl_err(mcdev, "mlx5ctl: failed to register misc device err %d\n", err);
+ goto abort;
+ }
+
+ mlx5ctl_dbg(mcdev, "probe mdev@%s %s\n", dev_driver_string(mdev->device), dev_name(mdev->device));
+
+ auxiliary_set_drvdata(adev, mcdev);
+
+ return 0;
+
+abort:
+ kfree(devname);
+ kfree(mcdev);
+ return err;
+}
+
+static void mlx5ctl_remove(struct auxiliary_device *adev)
+{
+ struct mlx5ctl_dev *mcdev = auxiliary_get_drvdata(adev);
+ struct mlx5_core_dev *mdev = mcdev->mdev;
+ struct mlx5ctl_fd *mfd, *n;
+
+ misc_deregister(&mcdev->miscdev);
+ down_write(&mcdev->rw_lock);
+
+ list_for_each_entry_safe(mfd, n, &mcdev->fd_list, list) {
+ mlx5ctl_dbg(mcdev, "UID %d still has open FDs\n", mfd->uctx_uid);
+ list_del(&mfd->list);
+ mlx5ctl_release_mfd(mfd);
+ }
+
+ mlx5ctl_dbg(mcdev, "removed mdev %s %s\n",
+ dev_driver_string(mdev->device), dev_name(mdev->device));
+
+ mcdev->mdev = NULL; /* prevent already open fds from accessing the device */
+ up_write(&mcdev->rw_lock);
+ mcdev_put(mcdev);
+}
+
+static void mcdev_free(struct kref *ref)
+{
+ struct mlx5ctl_dev *mcdev = container_of(ref, struct mlx5ctl_dev, refcount);
+
+ kfree(mcdev->miscdev.name);
+ kfree(mcdev);
+}
+
+static void mcdev_get(struct mlx5ctl_dev *mcdev)
+{
+ kref_get(&mcdev->refcount);
+}
+
+static void mcdev_put(struct mlx5ctl_dev *mcdev)
+{
+ kref_put(&mcdev->refcount, mcdev_free);
+}
+
+static const struct auxiliary_device_id mlx5ctl_id_table[] = {
+ { .name = MLX5_ADEV_NAME ".ctl", },
+ {},
+};
+
+MODULE_DEVICE_TABLE(auxiliary, mlx5ctl_id_table);
+
+static struct auxiliary_driver mlx5ctl_driver = {
+ .name = "ctl",
+ .probe = mlx5ctl_probe,
+ .remove = mlx5ctl_remove,
+ .id_table = mlx5ctl_id_table,
+};
+
+module_auxiliary_driver(mlx5ctl_driver);
--
2.41.0
From: Saeed Mahameed <[email protected]>
Allow ctl protocol interface auxiliary driver in mlx5.
Reviewed-by: Leon Romanovsky <[email protected]>
Reviewed-by: Jason Gunthorpe <[email protected]>
Signed-off-by: Saeed Mahameed <[email protected]>
---
drivers/net/ethernet/mellanox/mlx5/core/dev.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/dev.c b/drivers/net/ethernet/mellanox/mlx5/core/dev.c
index 7909f378dc93..f0e91793f4ad 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/dev.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/dev.c
@@ -215,8 +215,14 @@ enum {
MLX5_INTERFACE_PROTOCOL_MPIB,
MLX5_INTERFACE_PROTOCOL_VNET,
+ MLX5_INTERFACE_PROTOCOL_CTL,
};
+static bool is_ctl_supported(struct mlx5_core_dev *dev)
+{
+ return MLX5_CAP_GEN(dev, uctx_cap);
+}
+
static const struct mlx5_adev_device {
const char *suffix;
bool (*is_supported)(struct mlx5_core_dev *dev);
@@ -237,6 +243,8 @@ static const struct mlx5_adev_device {
.is_supported = &is_ib_rep_supported },
[MLX5_INTERFACE_PROTOCOL_MPIB] = { .suffix = "multiport",
.is_supported = &is_mp_supported },
+ [MLX5_INTERFACE_PROTOCOL_CTL] = { .suffix = "ctl",
+ .is_supported = &is_ctl_supported },
};
int mlx5_adev_idx_alloc(void)
--
2.41.0
On Wed, Oct 18, 2023 at 01:19:38AM -0700, Saeed Mahameed wrote:
> +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
For dual-licensed code, I need a LOT of documentation as to why this
must be dual-licensed, AND a signed-off-by from your corporate lawyer
agreeing to it so they convey an understanding of just how complex and
messy this will get over time and what you are agreeing to do here.
thanks,
greg k-h
On Wed, Oct 18, 2023 at 01:19:38AM -0700, Saeed Mahameed wrote:
> +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
<snip>
> +MODULE_LICENSE("Dual BSD/GPL");
See, it's a mess already :(
On Wed, Oct 18, 2023 at 01:19:36AM -0700, Saeed Mahameed wrote:
> Historically a userspace program was used that accessed the PCI register
> and config space directly through /sys/bus/pci/.../XXX and could operate
> these debugging interfaces in parallel with the running driver.
> This approach is incompatible with secure boot and kernel lockdown so this
> driver provides a secure and restricted interface to that.
Why not just write a UIO driver for this hardware then?
thanks,
greg k-h
On Wed, Oct 18, 2023 at 10:30:00AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Oct 18, 2023 at 01:19:38AM -0700, Saeed Mahameed wrote:
> > +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
>
> For dual-licensed code, I need a LOT of documentation as to why this
> must be dual-licensed, AND a signed-off-by from your corporate lawyer
> agreeing to it so they convey an understanding of just how complex and
> messy this will get over time and what you are agreeing to do here.
This is how we (NBU, Networking Business Unit in Nvidia, former Mellanox)
are instructed to submit all our code by default. This license is aligned
with our networking, vdpa and RDMA code.
So yes, our legal understands pros and cons of dual-license code.
Thanks
>
> thanks,
>
> greg k-h
On Wed, Oct 18, 2023 at 11:49:08AM +0300, Leon Romanovsky wrote:
> On Wed, Oct 18, 2023 at 10:30:00AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Oct 18, 2023 at 01:19:38AM -0700, Saeed Mahameed wrote:
> > > +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
> >
> > For dual-licensed code, I need a LOT of documentation as to why this
> > must be dual-licensed, AND a signed-off-by from your corporate lawyer
> > agreeing to it so they convey an understanding of just how complex and
> > messy this will get over time and what you are agreeing to do here.
>
> This is how we (NBU, Networking Business Unit in Nvidia, former Mellanox)
> are instructed to submit all our code by default. This license is aligned
> with our networking, vdpa and RDMA code.
Please don't do this without a really really good reason. Especially
for a "misc" driver that is so linux-dependant here. The "Linux-OpenIB"
license is old, obsolete, and problematic and should not be added to any
new files in the kernel tree, outside of the island of
drivers/infiniband/ as you all insist on that crazyness there.
Heck, it's even documented that you shouldn't be adding that license to
any new files, why ignore that?
thanks,
greg k-h
On Wed, Oct 18, 2023 at 10:55:35AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Oct 18, 2023 at 11:49:08AM +0300, Leon Romanovsky wrote:
> > On Wed, Oct 18, 2023 at 10:30:00AM +0200, Greg Kroah-Hartman wrote:
> > > On Wed, Oct 18, 2023 at 01:19:38AM -0700, Saeed Mahameed wrote:
> > > > +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
> > >
> > > For dual-licensed code, I need a LOT of documentation as to why this
> > > must be dual-licensed, AND a signed-off-by from your corporate lawyer
> > > agreeing to it so they convey an understanding of just how complex and
> > > messy this will get over time and what you are agreeing to do here.
> >
> > This is how we (NBU, Networking Business Unit in Nvidia, former Mellanox)
> > are instructed to submit all our code by default. This license is aligned
> > with our networking, vdpa and RDMA code.
>
> Please don't do this without a really really good reason. Especially
> for a "misc" driver that is so linux-dependant here.
Like I said, it is our default license. Saeed will change this code to be GPL-only.
> The "Linux-OpenIB" license is old, obsolete, and problematic and should not be added to any
> new files in the kernel tree, outside of the island of
> drivers/infiniband/ as you all insist on that crazyness there.
>
> Heck, it's even documented that you shouldn't be adding that license to
> any new files, why ignore that?
I don't remember being asked about it, not in this patch
https://lore.kernel.org/all/[email protected]/
and not in this later patch which changed LICENSES/other/ to be
LICENSES/deprecated/
https://lore.kernel.org/all/[email protected]/
So it is nice that someone decided to deprecate it without notifying users
about such change. It will be better to put Linux-OpenIB to LICENSES/dual/
folder instead.
Our lawyers are perfectly fine with Linux-OpenIB license.
Thanks
>
> thanks,
>
> greg k-h
On Wed, Oct 18, 2023 at 01:00:14PM +0300, Leon Romanovsky wrote:
> On Wed, Oct 18, 2023 at 10:55:35AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Oct 18, 2023 at 11:49:08AM +0300, Leon Romanovsky wrote:
> > > On Wed, Oct 18, 2023 at 10:30:00AM +0200, Greg Kroah-Hartman wrote:
> > > > On Wed, Oct 18, 2023 at 01:19:38AM -0700, Saeed Mahameed wrote:
> > > > > +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
> > > >
> > > > For dual-licensed code, I need a LOT of documentation as to why this
> > > > must be dual-licensed, AND a signed-off-by from your corporate lawyer
> > > > agreeing to it so they convey an understanding of just how complex and
> > > > messy this will get over time and what you are agreeing to do here.
> > >
> > > This is how we (NBU, Networking Business Unit in Nvidia, former Mellanox)
> > > are instructed to submit all our code by default. This license is aligned
> > > with our networking, vdpa and RDMA code.
> >
> > Please don't do this without a really really good reason. Especially
> > for a "misc" driver that is so linux-dependant here.
>
> Like I said, it is our default license. Saeed will change this code to be GPL-only.
>
> > The "Linux-OpenIB" license is old, obsolete, and problematic and should not be added to any
> > new files in the kernel tree, outside of the island of
> > drivers/infiniband/ as you all insist on that crazyness there.
> >
> > Heck, it's even documented that you shouldn't be adding that license to
> > any new files, why ignore that?
>
> I don't remember being asked about it, not in this patch
> https://lore.kernel.org/all/[email protected]/
> and not in this later patch which changed LICENSES/other/ to be
> LICENSES/deprecated/
> https://lore.kernel.org/all/[email protected]/
>
> So it is nice that someone decided to deprecate it without notifying users
> about such change. It will be better to put Linux-OpenIB to LICENSES/dual/
> folder instead.
>
> Our lawyers are perfectly fine with Linux-OpenIB license.
Almost all others are not, sorry, you will have to convince them
otherwise, so for now, let's keep this in "deprecated" as this license
is known to have issues and should not get applied to any new code
without EXPLICIT justification why it must be so.
thanks,
greg k-h
On Wed, Oct 18, 2023 at 09:00:25AM -0300, Jason Gunthorpe wrote:
> On Wed, Oct 18, 2023 at 10:31:23AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Oct 18, 2023 at 01:19:36AM -0700, Saeed Mahameed wrote:
> > > Historically a userspace program was used that accessed the PCI register
> > > and config space directly through /sys/bus/pci/.../XXX and could operate
> > > these debugging interfaces in parallel with the running driver.
> > > This approach is incompatible with secure boot and kernel lockdown so this
> > > driver provides a secure and restricted interface to that.
> >
> > Why not just write a UIO driver for this hardware then?
>
> The old mechanism relied on direct config space and sometimes mmio
> access to the PCI device. We did a security analysis and concluded
> that approach could not provide the required security for what our
> customers want from the secure boot and lockdown modes. We cannot
> allow a lockdown userspace direct access to those device registers.
>
> So, it was redesigned to be RPC driven instead of having direct HW
> access. The RPCs allow the device to provide the required level of
> security.
>
> This new misc driver does not expose any HW registers or interrupts to
> userspace, so it does not seem like a fit for UIO.
Ok, I'll not complain, I always like "real" drivers instead of UIO ones,
nice to see this happen!
thanks,
greg k-h
On Wed, Oct 18, 2023 at 10:31:23AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Oct 18, 2023 at 01:19:36AM -0700, Saeed Mahameed wrote:
> > Historically a userspace program was used that accessed the PCI register
> > and config space directly through /sys/bus/pci/.../XXX and could operate
> > these debugging interfaces in parallel with the running driver.
> > This approach is incompatible with secure boot and kernel lockdown so this
> > driver provides a secure and restricted interface to that.
>
> Why not just write a UIO driver for this hardware then?
The old mechanism relied on direct config space and sometimes mmio
access to the PCI device. We did a security analysis and concluded
that approach could not provide the required security for what our
customers want from the secure boot and lockdown modes. We cannot
allow a lockdown userspace direct access to those device registers.
So, it was redesigned to be RPC driven instead of having direct HW
access. The RPCs allow the device to provide the required level of
security.
This new misc driver does not expose any HW registers or interrupts to
userspace, so it does not seem like a fit for UIO.
Jason
On Wed, Oct 18, 2023 at 10:30:00AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Oct 18, 2023 at 01:19:38AM -0700, Saeed Mahameed wrote:
> > +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
>
> For dual-licensed code, I need a LOT of documentation as to why this
> must be dual-licensed, AND a signed-off-by from your corporate lawyer
> agreeing to it so they convey an understanding of just how complex and
> messy this will get over time and what you are agreeing to do here.
Can you provide a brief or whitepaper discussing this complexity
please? This pushback is news to me, Mellanox and the RDMA ecosystem
has been doing this for over 15 years now. I would need something
substantive to have a conversation with our legal.
However, I believe we can get an exception approval for single license
MIT or BSD-3-Clause for this code.
Thanks,
Jason
On Wed, Oct 18, 2023 at 03:01:28PM -0300, Jason Gunthorpe wrote:
> On Wed, Oct 18, 2023 at 10:30:00AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Oct 18, 2023 at 01:19:38AM -0700, Saeed Mahameed wrote:
> > > +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
> >
> > For dual-licensed code, I need a LOT of documentation as to why this
> > must be dual-licensed, AND a signed-off-by from your corporate lawyer
> > agreeing to it so they convey an understanding of just how complex and
> > messy this will get over time and what you are agreeing to do here.
>
> Can you provide a brief or whitepaper discussing this complexity
> please? This pushback is news to me, Mellanox and the RDMA ecosystem
> has been doing this for over 15 years now. I would need something
> substantive to have a conversation with our legal.
Have your legal talk to the LF legal working group, they are the ones
that told me never to mess with this license again. I'm sure that
nvidia's lawyers are part of this group, so let's let them hash it out.
> However, I believe we can get an exception approval for single license
> MIT or BSD-3-Clause for this code.
GPLv2 please, otherwise again, I'm going to demand a really really good
reason why Linux kernel code needs a non-GPL license and again, a sign
off from your lawyers explaining why it must be so.
thanks,
greg k-h
On Wed, Oct 18, 2023 at 08:22:39PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Oct 18, 2023 at 03:01:28PM -0300, Jason Gunthorpe wrote:
> > On Wed, Oct 18, 2023 at 10:30:00AM +0200, Greg Kroah-Hartman wrote:
> > > On Wed, Oct 18, 2023 at 01:19:38AM -0700, Saeed Mahameed wrote:
> > > > +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
> > >
> > > For dual-licensed code, I need a LOT of documentation as to why this
> > > must be dual-licensed, AND a signed-off-by from your corporate lawyer
> > > agreeing to it so they convey an understanding of just how complex and
> > > messy this will get over time and what you are agreeing to do here.
> >
> > Can you provide a brief or whitepaper discussing this complexity
> > please? This pushback is news to me, Mellanox and the RDMA ecosystem
> > has been doing this for over 15 years now. I would need something
> > substantive to have a conversation with our legal.
>
> Have your legal talk to the LF legal working group, they are the ones
> that told me never to mess with this license again. I'm sure that
> nvidia's lawyers are part of this group, so let's let them hash it
> out.
Are you talking about OpenIB specifically or the concept of dual
license (eg GPL/MIT) in general?
> > However, I believe we can get an exception approval for single license
> > MIT or BSD-3-Clause for this code.
>
> GPLv2 please, otherwise again, I'm going to demand a really really good
> reason why Linux kernel code needs a non-GPL license and again, a sign
> off from your lawyers explaining why it must be so.
All of the Mellanox driver stack (over 400 files now!) is dual
licensed because we have a large team of people working the Mellanox
driver for many operating systems with many different licenses. We
want the certainty of a permissive license for the driver code we
supply to Linux as the team routinely references and/or re-uses
Mellanox authored Linux driver code into other scenarios under the
permissive side of the dual license.
For instance I could easily see the work Saeed has done here finding
its way into FreeBSD. We significantly support FreeBSD employing
maintainers and develop a sophisticated Mellanox driver over
there. This would not be possible without the Linux driver being dual
licensed.
This has been the direction from our legal for a long time.
AFAIK this has also been a long time accepted Linux practice, there
are many examples in the driver tree. What has changed now that Saeed
tries to add 3 more files the giant driver? I need a bigger
explanation if we are going to revisit this practice with our legal.
To be clear, we can surely get the approvals to remove the offensive
OpenIB from these files. eg mlxsw is already approved using
"BSD-3-Clause OR GPL-2.0".
Further, as a maintainer myself, is this now the community consensus
expected review standard? When was it discussed? I do not see
evidence of a ban on dual licensing in
https://docs.kernel.org/process/license-rules.html ?
Thanks,
Jason
On Wed, Oct 18, 2023 at 03:56:29PM -0300, Jason Gunthorpe wrote:
> On Wed, Oct 18, 2023 at 08:22:39PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Oct 18, 2023 at 03:01:28PM -0300, Jason Gunthorpe wrote:
> > > On Wed, Oct 18, 2023 at 10:30:00AM +0200, Greg Kroah-Hartman wrote:
> > > > On Wed, Oct 18, 2023 at 01:19:38AM -0700, Saeed Mahameed wrote:
> > > > > +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
> > > >
> > > > For dual-licensed code, I need a LOT of documentation as to why this
> > > > must be dual-licensed, AND a signed-off-by from your corporate lawyer
> > > > agreeing to it so they convey an understanding of just how complex and
> > > > messy this will get over time and what you are agreeing to do here.
> > >
> > > Can you provide a brief or whitepaper discussing this complexity
> > > please? This pushback is news to me, Mellanox and the RDMA ecosystem
> > > has been doing this for over 15 years now. I would need something
> > > substantive to have a conversation with our legal.
> >
> > Have your legal talk to the LF legal working group, they are the ones
> > that told me never to mess with this license again. I'm sure that
> > nvidia's lawyers are part of this group, so let's let them hash it
> > out.
>
> Are you talking about OpenIB specifically or the concept of dual
> license (eg GPL/MIT) in general?
I'm talking about OpenIB specifically.
> > > However, I believe we can get an exception approval for single license
> > > MIT or BSD-3-Clause for this code.
> >
> > GPLv2 please, otherwise again, I'm going to demand a really really good
> > reason why Linux kernel code needs a non-GPL license and again, a sign
> > off from your lawyers explaining why it must be so.
>
> All of the Mellanox driver stack (over 400 files now!) is dual
> licensed because we have a large team of people working the Mellanox
> driver for many operating systems with many different licenses. We
> want the certainty of a permissive license for the driver code we
> supply to Linux as the team routinely references and/or re-uses
> Mellanox authored Linux driver code into other scenarios under the
> permissive side of the dual license.
>
> For instance I could easily see the work Saeed has done here finding
> its way into FreeBSD. We significantly support FreeBSD employing
> maintainers and develop a sophisticated Mellanox driver over
> there. This would not be possible without the Linux driver being dual
> licensed.
Yes it would, you can take the work that you all do and license it under
the BSD license and put it into FreeBSD just fine. But you are saying
you require Linux developers to help you with your FreeBSD drivers,
which is not always fair or nice to take from others that way (in my
opinion.)
> This has been the direction from our legal for a long time.
I know, but the OpenIB license is known to have issues, and so I thought
they were going to stop using it for new code.
> AFAIK this has also been a long time accepted Linux practice, there
> are many examples in the driver tree. What has changed now that Saeed
> tries to add 3 more files the giant driver? I need a bigger
> explanation if we are going to revisit this practice with our legal.
"the giant driver"? I'm confused.
> To be clear, we can surely get the approvals to remove the offensive
> OpenIB from these files. eg mlxsw is already approved using
> "BSD-3-Clause OR GPL-2.0".
For your new files, please make them single license. If you insist on
dual licensing them, I will insist on have a lawyer sign off on them so
that they understand the issues involved with dual licenses, and just
how much I hate them in the kernel tree as they are a pain over time.
Note, this isn't special to you, I do this to all new files sent to me
with this type of non-GPL-only license, see the archives for details.
> Further, as a maintainer myself, is this now the community consensus
> expected review standard? When was it discussed? I do not see
> evidence of a ban on dual licensing in
> https://docs.kernel.org/process/license-rules.html ?
It's based on my experience with existing dual licensed kernel code AND
discussing it with many many lawyers over the years. It's a pain, they
hate it, it's dubious if it actually helps anyone out in any other
operating system (as again, you can take your work and send it to
FreeBSD just fine), it is messy when dealing with gpl-only exports (like
the driver model or the USB layer), and you are taking something from
the community (free labor) to help other operating systems out when the
Linux developers might not realize it.
I think the only real place this works out is the ACPI core, for the
obvious reasons that we all want a solid ACPI core that all operating
systems can use. And Intel goes through a lot of extra effort and time
and energy to keep that going, so it is costing them real money to do
this work for this, so that makes sense. For just a hardware driver for
a specific company, this feels very selfish in my opinion.
I would be really interested in if you all actually have taken any
not-from-your-company changes to your drivers and copied that into other
operating systems for anything "real" that wasn't just tiny bugfixes.
Have you? If not, why go through this hassle?
thanks,
greg k-h
On Thu, Oct 19, 2023 at 07:21:57PM +0200, Greg Kroah-Hartman wrote:
> > Are you talking about OpenIB specifically or the concept of dual
> > license (eg GPL/MIT) in general?
>
> I'm talking about OpenIB specifically.
Let's put that aside, I think Saeed made a C&P error since he works
mostly on the historical code that is grandfathered. He will fix it to
be another our-legal approved license, probably "BSD-3-Clause & GPLv2"
> > All of the Mellanox driver stack (over 400 files now!) is dual
> > licensed because we have a large team of people working the Mellanox
> > driver for many operating systems with many different licenses. We
> > want the certainty of a permissive license for the driver code we
> > supply to Linux as the team routinely references and/or re-uses
> > Mellanox authored Linux driver code into other scenarios under the
> > permissive side of the dual license.
> >
> > For instance I could easily see the work Saeed has done here finding
> > its way into FreeBSD. We significantly support FreeBSD employing
> > maintainers and develop a sophisticated Mellanox driver over
> > there. This would not be possible without the Linux driver being dual
> > licensed.
>
> Yes it would, you can take the work that you all do and license it under
> the BSD license and put it into FreeBSD just fine.
Sure, you can do that at day 0, but mlx5 is now about 10 years old and
has tens of thousands of commits. Many non-Mellanox commits. (mostly
non-significant, IMHO, IANAL)
If Mellanox today writes a new patch for mlx5 based on that history,
can that patch be re-licensed to BSD if the file it is based on is GPL
only with a complex history? Our legal has historically said no to
this question.
We are not dumping code over a wall where there is some internal
reference that has a single copyright. The mlx5 driver team is fully
integrated with the upstream community lead processes. I'm very proud
of how Mellanox is able to work like this.
> But you are saying you require Linux developers to help you with
> your FreeBSD drivers, which is not always fair or nice to take from
> others that way (in my opinion.)
AFAIK Mellanox has never done "require". If you want to do significant
work in mlx5-land and want GPL only then put it in its own file with a
GPL only license.
This has happened in drivers/infiniband which started in 2005 with a
group of like-minded people/companies that wanted to enable a full
ecosystem across a lot of operating systems. Later someone came with
significant work and wanted GPL only so it was placed in its own files
with a GPL only license.
I agree that "require" is not really fair, but I think there is room
in Linux to support people that want their open source work shared
outside Linux along side people that don't.
> > AFAIK this has also been a long time accepted Linux practice, there
> > are many examples in the driver tree. What has changed now that Saeed
> > tries to add 3 more files the giant driver? I need a bigger
> > explanation if we are going to revisit this practice with our legal.
>
> "the giant driver"? I'm confused.
The > 500 files of approx:
$ git ls-files | egrep -i mlx5
See the other thread debating what mlx5 HW actually is.
Remember that Leon created auxiliary bus so these complex multi-system
HWs could be split up cleanly into their respective subsystems? This
is an aux device driver for the misc subsystem as part of the giant
cross-subsystem mlx5 driver. Ie Saeed is adding 3 more files to that
existing monster.
> > To be clear, we can surely get the approvals to remove the offensive
> > OpenIB from these files. eg mlxsw is already approved using
> > "BSD-3-Clause OR GPL-2.0".
>
> For your new files, please make them single license. If you insist on
> dual licensing them, I will insist on have a lawyer sign off on them so
> that they understand the issues involved with dual licenses, and just
> how much I hate them in the kernel tree as they are a pain over time.
Please share with me what words you want to see and I will get them
from our legal team.
> I think the only real place this works out is the ACPI core, for the
> obvious reasons that we all want a solid ACPI core that all operating
> systems can use. And Intel goes through a lot of extra effort and time
> and energy to keep that going, so it is costing them real money to do
> this work for this, so that makes sense. For just a hardware driver for
> a specific company, this feels very selfish in my opinion.
It costs Mellanox real money/etc too. I'm not sure I see who it is
selfish to?
> I would be really interested in if you all actually have taken any
> not-from-your-company changes to your drivers and copied that into other
> operating systems for anything "real" that wasn't just tiny bugfixes.
> Have you? If not, why go through this hassle?
The FreeBSD team references the current state of the driver in Linux
to guide their work. I don't think they carefully track the providence
of every single line. Not doing that is the main point of a dual
license.
Honestly, this has been done for 15 years now and has never been a
hassle at all. If you are asking why go through the hassle you are now
requesting - then I will have to get back to you based on how much
hassle it turns out to be :)
I believe strongly that Mellanox's efforts in FreeBSD are open source
noble and not selfish at all. I'm confident our legal will shut down
that project without a dual license, so I will go through some hassle
at your request to protect it.
Regards,
Jason
On Thu, Oct 19, 2023 at 04:00:46PM -0300, Jason Gunthorpe wrote:
> On Thu, Oct 19, 2023 at 07:21:57PM +0200, Greg Kroah-Hartman wrote:
> > > All of the Mellanox driver stack (over 400 files now!) is dual
> > > licensed because we have a large team of people working the Mellanox
> > > driver for many operating systems with many different licenses. We
> > > want the certainty of a permissive license for the driver code we
> > > supply to Linux as the team routinely references and/or re-uses
> > > Mellanox authored Linux driver code into other scenarios under the
> > > permissive side of the dual license.
> > >
> > > For instance I could easily see the work Saeed has done here finding
> > > its way into FreeBSD. We significantly support FreeBSD employing
> > > maintainers and develop a sophisticated Mellanox driver over
> > > there. This would not be possible without the Linux driver being dual
> > > licensed.
> >
> > Yes it would, you can take the work that you all do and license it under
> > the BSD license and put it into FreeBSD just fine.
>
> Sure, you can do that at day 0, but mlx5 is now about 10 years old and
> has tens of thousands of commits. Many non-Mellanox commits. (mostly
> non-significant, IMHO, IANAL)
That's not the case for this specific chunk of code, so it's not a valid
point at all, sorry.
Let's stick to just this new file, please keep it one-license, not dual,
it makes everything simpler overall.
> Remember that Leon created auxiliary bus so these complex multi-system
> HWs could be split up cleanly into their respective subsystems? This
> is an aux device driver for the misc subsystem as part of the giant
> cross-subsystem mlx5 driver. Ie Saeed is adding 3 more files to that
> existing monster.
Yes, and as the auxiliary bus code is EXPORT_SYMBOL_GPL() attempting to
license code that is a driver for that bus (i.e. this new contribution)
under anything other than just GPL is crazy. Go talk to your lawyers
about that please, it's obviously not ok.
thanks,
greg k-h
Greg Kroah-Hartman <[email protected]> writes:
> For your new files, please make them single license. If you insist on
> dual licensing them, I will insist on have a lawyer sign off on them so
> that they understand the issues involved with dual licenses, and just
> how much I hate them in the kernel tree as they are a pain over time.
Out of curiosity, is there somewhere people can look for a description
of these issues and the pain they cause? I've seen this go by enough
times to think that it would be good to set down in Documentation/
somewhere for future reference.
Thanks,
jon
On Thu, Oct 19, 2023 at 09:46:29PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Oct 19, 2023 at 04:00:46PM -0300, Jason Gunthorpe wrote:
> > On Thu, Oct 19, 2023 at 07:21:57PM +0200, Greg Kroah-Hartman wrote:
> > > > All of the Mellanox driver stack (over 400 files now!) is dual
> > > > licensed because we have a large team of people working the Mellanox
> > > > driver for many operating systems with many different licenses. We
> > > > want the certainty of a permissive license for the driver code we
> > > > supply to Linux as the team routinely references and/or re-uses
> > > > Mellanox authored Linux driver code into other scenarios under the
> > > > permissive side of the dual license.
> > > >
> > > > For instance I could easily see the work Saeed has done here finding
> > > > its way into FreeBSD. We significantly support FreeBSD employing
> > > > maintainers and develop a sophisticated Mellanox driver over
> > > > there. This would not be possible without the Linux driver being dual
> > > > licensed.
> > >
> > > Yes it would, you can take the work that you all do and license it under
> > > the BSD license and put it into FreeBSD just fine.
> >
> > Sure, you can do that at day 0, but mlx5 is now about 10 years old and
> > has tens of thousands of commits. Many non-Mellanox commits. (mostly
> > non-significant, IMHO, IANAL)
>
> That's not the case for this specific chunk of code, so it's not a valid
> point at all, sorry.
In 10 years it will be in the same situation as the rest of the
driver. You are saying we can't plan ahead now? Why?
> Let's stick to just this new file, please keep it one-license, not dual,
> it makes everything simpler overall.
Simpler for who? It seems to complicate Mellanox's situation.
More importantly it seems to represent an important philosophical
shift for Linux that touches on something we have a significant
investment in.
I would like clarity here, on a going forward basis. You do set the
tone for the whole project. I've made my case for why we are doing and
why it brings value. You are saying dual license is now effectively
banned.
Previously you said you would agree with a sign off from our legal,
please tell me what statement you want and I will go get it.
> > Remember that Leon created auxiliary bus so these complex multi-system
> > HWs could be split up cleanly into their respective subsystems? This
> > is an aux device driver for the misc subsystem as part of the giant
> > cross-subsystem mlx5 driver. Ie Saeed is adding 3 more files to that
> > existing monster.
>
> Yes, and as the auxiliary bus code is EXPORT_SYMBOL_GPL() attempting to
> license code that is a driver for that bus (i.e. this new contribution)
> under anything other than just GPL is crazy. Go talk to your lawyers
> about that please, it's obviously not ok.
The entire mlx5 driver makes free use of EXPORT_SYMBOL_GPL(). Our
legal has looked at this in the past and they continue to give
instruction to use a dual license.
You keep saying go talk to our lawyers like this hasn't been a legally
vetted approach at Mellanox for the last 15 years :( Mellanox has
300,000 lines of code in Linux with a dual license. We have a good
in-house legal and they take license obligations seriously. I don't
see any new information in this thread that makes me think they will
change their long standing position.
Tell me what you want and I will go get it from them.
Regards,
Jason
On Fri, 20 Oct 2023 at 07:51, Jonathan Corbet <[email protected]> wrote:
>
> Greg Kroah-Hartman <[email protected]> writes:
>
> > For your new files, please make them single license. If you insist on
> > dual licensing them, I will insist on have a lawyer sign off on them so
> > that they understand the issues involved with dual licenses, and just
> > how much I hate them in the kernel tree as they are a pain over time.
>
> Out of curiosity, is there somewhere people can look for a description
> of these issues and the pain they cause? I've seen this go by enough
> times to think that it would be good to set down in Documentation/
> somewhere for future reference.
I'm also very curious, I've never had any issues in 15+ years with the
dual licensed code in the DRM, and continue to prefer it.
Greg if you get new advice that you want to apply to the kernel as a
whole, you probably need to write it up for us to discuss, your
lawyers are not my lawyers etc.
Maybe something for Maintainers summit?
Dave.
On Thu, Oct 19, 2023 at 03:50:30PM -0600, Jonathan Corbet wrote:
> Greg Kroah-Hartman <[email protected]> writes:
>
> > For your new files, please make them single license. If you insist on
> > dual licensing them, I will insist on have a lawyer sign off on them so
> > that they understand the issues involved with dual licenses, and just
> > how much I hate them in the kernel tree as they are a pain over time.
>
> Out of curiosity, is there somewhere people can look for a description
> of these issues and the pain they cause? I've seen this go by enough
> times to think that it would be good to set down in Documentation/
> somewhere for future reference.
I don't have a description anywhere, sorry, this was just discussions
with many open source lawyers who focus on the kernel over the years.
I'll work to try to get something "solid" from them that I can put into
writing, but it will probably take quite a while...
thanks,
greg k-h
On Thu, Oct 19, 2023 at 08:49:47PM -0300, Jason Gunthorpe wrote:
> On Thu, Oct 19, 2023 at 09:46:29PM +0200, Greg Kroah-Hartman wrote:
> > On Thu, Oct 19, 2023 at 04:00:46PM -0300, Jason Gunthorpe wrote:
> > > On Thu, Oct 19, 2023 at 07:21:57PM +0200, Greg Kroah-Hartman wrote:
> > > > > All of the Mellanox driver stack (over 400 files now!) is dual
> > > > > licensed because we have a large team of people working the Mellanox
> > > > > driver for many operating systems with many different licenses. We
> > > > > want the certainty of a permissive license for the driver code we
> > > > > supply to Linux as the team routinely references and/or re-uses
> > > > > Mellanox authored Linux driver code into other scenarios under the
> > > > > permissive side of the dual license.
> > > > >
> > > > > For instance I could easily see the work Saeed has done here finding
> > > > > its way into FreeBSD. We significantly support FreeBSD employing
> > > > > maintainers and develop a sophisticated Mellanox driver over
> > > > > there. This would not be possible without the Linux driver being dual
> > > > > licensed.
> > > >
> > > > Yes it would, you can take the work that you all do and license it under
> > > > the BSD license and put it into FreeBSD just fine.
> > >
> > > Sure, you can do that at day 0, but mlx5 is now about 10 years old and
> > > has tens of thousands of commits. Many non-Mellanox commits. (mostly
> > > non-significant, IMHO, IANAL)
> >
> > That's not the case for this specific chunk of code, so it's not a valid
> > point at all, sorry.
>
> In 10 years it will be in the same situation as the rest of the
> driver. You are saying we can't plan ahead now? Why?
I really am confused as to what exactly the scope of this driver is
then. For some reason I thought it was just a "here's a debug driver
that we use to query the hardware and get some performance stats out of
it at times which we used to do previously by just doing raw PCI-memory
reads". Is this really going to blow up into a giant infrastructure
that you are going to use for real functionality over the next decades?
If so, it needs a lot better description of what exactly this driver is
supposed to be doing, and how it ties into existing userspace tools.
> > Let's stick to just this new file, please keep it one-license, not dual,
> > it makes everything simpler overall.
>
> Simpler for who? It seems to complicate Mellanox's situation.
How in the world is Mellanox going to take the code in a aux driver for
Linux and us that in any other operating systems WITHOUT it just being
an original contribution from a Mellanox developer themself?
> More importantly it seems to represent an important philosophical
> shift for Linux that touches on something we have a significant
> investment in.
Again, this is something that I ask of any new file that is sent to me
for my approval for many many years. There are some subsystems in Linux
(drm, IB) that have lived with dual-license code for a very long time,
but that's about it.
> I would like clarity here, on a going forward basis. You do set the
> tone for the whole project. I've made my case for why we are doing and
> why it brings value. You are saying dual license is now effectively
> banned.
I'm saying that it needs to be explicitly chosen for very good reasons.
In the past, when I have pushed back on "why are you doing this?" I have
gotten "oops, we didn't mean that or understand it at all, we'll fix it"
the majority of the times. If your company really does understand it
and knows it, great, that's what I need to confirm here. I don't keep a
list of what companies do/do-not know this type of thing as that would
be pointless (hint, companies buy other companies and change legal
policies...)
> Previously you said you would agree with a sign off from our legal,
> please tell me what statement you want and I will go get it.
A simple note in the changelog that says something like:
This file is dual licensed under XXX and YYY because of the
following reasons.... and we will be handling all contributions
to it in the following way...
and a signed-off-by in the chain by your group's lawyer so that we know
they understand the issues.
Or some other text like this, they can figure it out as they obviously
know the issues involved, and they know what they want to express to us.
> > > Remember that Leon created auxiliary bus so these complex multi-system
> > > HWs could be split up cleanly into their respective subsystems? This
> > > is an aux device driver for the misc subsystem as part of the giant
> > > cross-subsystem mlx5 driver. Ie Saeed is adding 3 more files to that
> > > existing monster.
> >
> > Yes, and as the auxiliary bus code is EXPORT_SYMBOL_GPL() attempting to
> > license code that is a driver for that bus (i.e. this new contribution)
> > under anything other than just GPL is crazy. Go talk to your lawyers
> > about that please, it's obviously not ok.
>
> The entire mlx5 driver makes free use of EXPORT_SYMBOL_GPL(). Our
> legal has looked at this in the past and they continue to give
> instruction to use a dual license.
That's odd, I wonder how you take those contributions into other
operating systems...
> You keep saying go talk to our lawyers like this hasn't been a legally
> vetted approach at Mellanox for the last 15 years :(
I keep saying this as I have no idea what company is behind this, nor
what their lawyers want, nor what has been vetted by them at all.
And this email is going to a nvidia.com address, not Mellanox, so you
can understand my "you all better get the license issues right!" wish
for me to do here, based on the past experience with "issues" from that
domain.
thanks,
greg k-h