Re: [PATCH for-next v2] RDMA/hfi2: Consolidate ABI files and setup uverbs access

[Leon Romanovsky <leon@kernel.org>]

On Wed, Mar 18, 2026 at 11:08:16AM -0400, Dennis Dalessandro wrote:
> hfi1 driver is being replaced eventually with an hfi2 driver. Until that
> happens rather than have all the duplicated code in header files, make hfi1
> use hfi2 variants where it can. When compatibility breaks we'll keep a
> separate hfi1 version.
> 
> This is the case for the <dev>_status struture. The hfi1 varaint is single
> port and uses a freezemsg char array while the new hfi2 chip provides
> multiple ports and thus needs and array of ports.
> 
> Likewise the tid info struct is expanded for hfi2 so we include both an
> hfi1 and hfi2 vaiant.
> 
> There is a naming conflict with the trace_hfi1_ctxt_info() call. It has been
> renamed to remove the 1 from the function name to keep the code readable
> but allow it to compile due to the #define in hfi1_ioctl.h.
> 
> The big departure from hfi1 is that we are no longer supporting access from
> users through a private character device. Instead we define two custom
> verbs ojects. dv0/1, which proivdes methods for what in hfi1 are individual
> IOCTLs. We have added an additional method to get stats related to page
> pinning done by the driver.
> 
> The hfi1_user.h is no longer needed and is removed from the uapi directory.
> There is a private compat header in hfi1 that will be deleted when hfi1 is.
> This removes driver specific content from generic RDMA UAPI headers.
> 
> Co-developed-by: Dean Luick <dean.luick@cornelisnetworks.com>
> Signed-off-by: Dean Luick <dean.luick@cornelisnetworks.com>
> Co-developed-by: Bendan Cunningham <brendan.cunningham@cornelisnetworks.com>
> Signed-off-by: Breandan Cunningham <brendan.cunningham@cornelisnetworks.com>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
> 
> ---
> 
>   Changes since v1:
>     - Remove include/uapi/rdma/hfi/hfi1_user.h; user-space libraries copy
>       UAPI headers rather than including them from the kernel tree directly,
>       so the hfi1_* compatibility aliases do not need to be exported as UAPI
>     - Move hfi1_* to hfi2_* aliases into a new private driver header
>       drivers/infiniband/hw/hfi1/hfi1_user_compat.h; hfi1 continues to use
>       hfi1_* names with no changes to driver source files
>     - Move HFI1_IOCTL_* command definitions from rdma_user_ioctl.h into
>       hfi1_ioctl.h, removing the driver-specific include from the generic
>       RDMA UAPI header
> ---
>  drivers/infiniband/hw/hfi1/common.h           |    2 
>  drivers/infiniband/hw/hfi1/file_ops.c         |    2 
>  drivers/infiniband/hw/hfi1/hfi1_user_compat.h |   93 +++
>  drivers/infiniband/hw/hfi1/trace_ctxts.h      |    2 
>  include/uapi/rdma/hfi/hfi1_ioctl.h            |  140 +----
>  include/uapi/rdma/hfi/hfi1_user.h             |  268 ---------
>  include/uapi/rdma/hfi2-abi.h                  |  726 +++++++++++++++++++++++++
>  include/uapi/rdma/ib_user_ioctl_verbs.h       |    1 
>  include/uapi/rdma/rdma_user_ioctl.h           |   47 --
>  9 files changed, 854 insertions(+), 427 deletions(-)
>  create mode 100644 drivers/infiniband/hw/hfi1/hfi1_user_compat.h
>  delete mode 100644 include/uapi/rdma/hfi/hfi1_user.h
>  create mode 100644 include/uapi/rdma/hfi2-abi.h

While I the idea looks right, I have a couple of comments.

> 
> diff --git a/drivers/infiniband/hw/hfi1/common.h b/drivers/infiniband/hw/hfi1/common.h
> index 8abc902b96f3..011e0f12cea7 100644
> --- a/drivers/infiniband/hw/hfi1/common.h
> +++ b/drivers/infiniband/hw/hfi1/common.h
> @@ -6,7 +6,7 @@
>  #ifndef _COMMON_H
>  #define _COMMON_H

<...>

> +#define HFI1_USER_SWMAJOR HFI2_USER_SWMAJOR
> +#define HFI1_USER_SWMINOR HFI2_USER_SWMINOR
> +#define HFI1_SWMAJOR_SHIFT HFI2_SWMAJOR_SHIFT

I think you need to keep these for hfi1, but hfi2 should not use this
approach. Instead, it should rely on the XXX_UVERBS_ABI_VERSION define,
as other drivers do.

<...>

> index 000000000000..a3d906759730
> --- /dev/null
> +++ b/include/uapi/rdma/hfi2-abi.h
> @@ -0,0 +1,726 @@
> +/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
> +/*
> + * Copyright(c) 2025-2026 Cornelis Networks, Inc.
> + */
> +
> +#ifndef _LINUX_HFI2_USER_H
> +#define _LINUX_HFI2_USER_H
> +
> +#include <linux/types.h>
> +#include <rdma/ib_user_ioctl_cmds.h>
> +#include <rdma/rdma_user_ioctl.h>
> +
> +/*
> + * This version number is given to the driver by the user code during
> + * initialization in the spu_userversion field of hfi2_user_info, so
> + * the driver can check for compatibility with user code.

This is not how compatibility is handled in RDMA subsystem.

> + *
> + * The major version changes when data structures change in an incompatible
> + * way. The driver must be the same for initialization to succeed.
> + */
> +#define HFI2_USER_SWMAJOR 6
> +#define HFI2_RDMA_USER_SWMAJOR 10
> +
> +/*
> + * Minor version differences are always compatible
> + * a within a major version, however if user software is larger
> + * than driver software, some new features and/or structure fields
> + * may not be implemented; the user code must deal with this if it
> + * cares, or it must abort after initialization reports the difference.
> + */
> +#define HFI2_USER_SWMINOR 3
> +#define HFI2_RDMA_USER_SWMINOR 0
> +
> +/*
> + * We will encode the major/minor inside a single 32bit version number.
> + */
> +#define HFI2_SWMAJOR_SHIFT 16
> +
> +/*
> + * Set of HW and driver capability/feature bits.
> + * These bit values are used to configure enabled/disabled HW and
> + * driver features. The same set of bits are communicated to user
> + * space.
> + */
> +#define HFI2_CAP_DMA_RTAIL        (1UL <<  0) /* Use DMA'ed RTail value */
> +#define HFI2_CAP_SDMA             (1UL <<  1) /* Enable SDMA support */
> +#define HFI2_CAP_SDMA_AHG         (1UL <<  2) /* Enable SDMA AHG support */
> +#define HFI2_CAP_EXTENDED_PSN     (1UL <<  3) /* Enable Extended PSN support */
> +#define HFI2_CAP_HDRSUPP          (1UL <<  4) /* Enable Header Suppression */
> +#define HFI2_CAP_TID_RDMA         (1UL <<  5) /* Enable TID RDMA operations */
> +#define HFI2_CAP_USE_SDMA_HEAD    (1UL <<  6) /* DMA Hdr Q tail vs. use CSR */
> +#define HFI2_CAP_MULTI_PKT_EGR    (1UL <<  7) /* Enable multi-packet Egr buffs*/
> +#define HFI2_CAP_NODROP_RHQ_FULL  (1UL <<  8) /* Don't drop on Hdr Q full */
> +#define HFI2_CAP_NODROP_EGR_FULL  (1UL <<  9) /* Don't drop on EGR buffs full */
> +#define HFI2_CAP_TID_UNMAP        (1UL << 10) /* Disable Expected TID caching */
> +#define HFI2_CAP_PRINT_UNIMPL     (1UL << 11) /* Show for unimplemented feats */
> +#define HFI2_CAP_ALLOW_PERM_JKEY  (1UL << 12) /* Allow use of permissive JKEY */
> +#define HFI2_CAP_NO_INTEGRITY     (1UL << 13) /* Enable ctxt integrity checks */
> +#define HFI2_CAP_PKEY_CHECK       (1UL << 14) /* Enable ctxt PKey checking */
> +#define HFI2_CAP_STATIC_RATE_CTRL (1UL << 15) /* Allow PBC.StaticRateControl */
> +#define HFI2_CAP_OPFN             (1UL << 16) /* Enable the OPFN protocol */
> +#define HFI2_CAP_SDMA_HEAD_CHECK  (1UL << 17) /* SDMA head checking */
> +#define HFI2_CAP_EARLY_CREDIT_RETURN (1UL << 18) /* early credit return */
> +#define HFI2_CAP_AIP              (1UL << 19) /* Enable accelerated IP */

General note for whole hfi2-abi.h file, it is unclear if you really need
and use all these fields.

<...>

> +#endif /* _LINIUX_HFI2_USER_H */
> diff --git a/include/uapi/rdma/ib_user_ioctl_verbs.h b/include/uapi/rdma/ib_user_ioctl_verbs.h
> index 89e6a3f13191..c7573131c862 100644
> --- a/include/uapi/rdma/ib_user_ioctl_verbs.h
> +++ b/include/uapi/rdma/ib_user_ioctl_verbs.h
> @@ -256,6 +256,7 @@ enum rdma_driver_id {
>  	RDMA_DRIVER_ERDMA,
>  	RDMA_DRIVER_MANA,
>  	RDMA_DRIVER_IONIC,
> +	RDMA_DRIVER_HFI2,

This hunk should be separated and submitted as part of hfi2 addition.

>  };
>  
>  enum ib_uverbs_gid_type {
> diff --git a/include/uapi/rdma/rdma_user_ioctl.h b/include/uapi/rdma/rdma_user_ioctl.h
> index 53c55188dd2a..263cace86f8f 100644
> --- a/include/uapi/rdma/rdma_user_ioctl.h
> +++ b/include/uapi/rdma/rdma_user_ioctl.h
> @@ -39,47 +39,14 @@
>  #include <rdma/rdma_user_ioctl_cmds.h>
>  
>  /* Legacy name, for user space application which already use it */
> -#define IB_IOCTL_MAGIC		RDMA_IOCTL_MAGIC
> -
> -/*
> - * General blocks assignments
> - * It is closed on purpose - do not expose it to user space
> - * #define MAD_CMD_BASE		0x00
> - * #define HFI1_CMD_BAS		0xE0
> - */
> +#define IB_IOCTL_MAGIC RDMA_IOCTL_MAGIC
>  
>  /* MAD specific section */
> -#define IB_USER_MAD_REGISTER_AGENT	_IOWR(RDMA_IOCTL_MAGIC, 0x01, struct ib_user_mad_reg_req)
> -#define IB_USER_MAD_UNREGISTER_AGENT	_IOW(RDMA_IOCTL_MAGIC,  0x02, __u32)
> -#define IB_USER_MAD_ENABLE_PKEY		_IO(RDMA_IOCTL_MAGIC,   0x03)
> -#define IB_USER_MAD_REGISTER_AGENT2	_IOWR(RDMA_IOCTL_MAGIC, 0x04, struct ib_user_mad_reg_req2)
> -
> -/* HFI specific section */
> -/* allocate HFI and context */
> -#define HFI1_IOCTL_ASSIGN_CTXT		_IOWR(RDMA_IOCTL_MAGIC, 0xE1, struct hfi1_user_info)
> -/* find out what resources we got */
> -#define HFI1_IOCTL_CTXT_INFO		_IOW(RDMA_IOCTL_MAGIC,  0xE2, struct hfi1_ctxt_info)
> -/* set up userspace */
> -#define HFI1_IOCTL_USER_INFO		_IOW(RDMA_IOCTL_MAGIC,  0xE3, struct hfi1_base_info)
> -/* update expected TID entries */
> -#define HFI1_IOCTL_TID_UPDATE		_IOWR(RDMA_IOCTL_MAGIC, 0xE4, struct hfi1_tid_info)
> -/* free expected TID entries */
> -#define HFI1_IOCTL_TID_FREE		_IOWR(RDMA_IOCTL_MAGIC, 0xE5, struct hfi1_tid_info)
> -/* force an update of PIO credit */
> -#define HFI1_IOCTL_CREDIT_UPD		_IO(RDMA_IOCTL_MAGIC,   0xE6)
> -/* control receipt of packets */
> -#define HFI1_IOCTL_RECV_CTRL		_IOW(RDMA_IOCTL_MAGIC,  0xE8, int)
> -/* set the kind of polling we want */
> -#define HFI1_IOCTL_POLL_TYPE		_IOW(RDMA_IOCTL_MAGIC,  0xE9, int)
> -/* ack & clear user status bits */
> -#define HFI1_IOCTL_ACK_EVENT		_IOW(RDMA_IOCTL_MAGIC,  0xEA, unsigned long)
> -/* set context's pkey */
> -#define HFI1_IOCTL_SET_PKEY		_IOW(RDMA_IOCTL_MAGIC,  0xEB, __u16)
> -/* reset context's HW send context */
> -#define HFI1_IOCTL_CTXT_RESET		_IO(RDMA_IOCTL_MAGIC,   0xEC)
> -/* read TID cache invalidations */
> -#define HFI1_IOCTL_TID_INVAL_READ	_IOWR(RDMA_IOCTL_MAGIC, 0xED, struct hfi1_tid_info)
> -/* get the version of the user cdev */
> -#define HFI1_IOCTL_GET_VERS		_IOR(RDMA_IOCTL_MAGIC,  0xEE, int)
> +#define IB_USER_MAD_REGISTER_AGENT \
> +	_IOWR(RDMA_IOCTL_MAGIC, 0x01, struct ib_user_mad_reg_req)
> +#define IB_USER_MAD_UNREGISTER_AGENT _IOW(RDMA_IOCTL_MAGIC, 0x02, __u32)
> +#define IB_USER_MAD_ENABLE_PKEY _IO(RDMA_IOCTL_MAGIC, 0x03)
> +#define IB_USER_MAD_REGISTER_AGENT2 \
> +	_IOWR(RDMA_IOCTL_MAGIC, 0x04, struct ib_user_mad_reg_req2)

These changes are unrelated.

Thanks

>  
>  #endif /* RDMA_USER_IOCTL_H */
> 
>