Hfi2 - #1710
Conversation
819425e to
4b9a8b2
Compare
There was a problem hiding this comment.
Pull request overview
This PR replaces the legacy ipath/qib userspace provider with a new hfi2 userspace provider, and removes truescale-related packaging bits while updating RDMA kernel header UAPI to match new ioctl/ABI requirements.
Changes:
- Remove the
providers/ipathverbsprovider (and associated truescale modprobe/dracut artifacts) and add a newproviders/hfi2verbsprovider. - Update distro packaging specs/installs to stop shipping
truescale.confandtruescale-serdes.cmds. - Refresh/install new RDMA kernel header definitions (driver IDs, buffer descriptors, mlx5/bnxt uapi additions) and update udev rule classification for
hfi2.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| suse/rdma-core.spec | Stops packaging truescale modprobe config and serdes script. |
| redhat/rdma-core.spec | Stops packaging truescale modprobe config and serdes script. |
| providers/ipathverbs/verbs.c | Removes legacy ipathverbs provider implementation. |
| providers/ipathverbs/truescale.conf.in | Removes truescale modprobe install rule. |
| providers/ipathverbs/truescale-serdes.cmds | Removes truescale serdes helper script. |
| providers/ipathverbs/ipathverbs.h | Removes legacy ipathverbs provider header. |
| providers/ipathverbs/ipathverbs.c | Removes legacy ipathverbs provider entry/registration code. |
| providers/ipathverbs/ipath-abi.h | Removes ipathverbs UAPI shim header. |
| providers/ipathverbs/dracut_kmod | Removes dracut module hook for ib_qib. |
| providers/ipathverbs/dracut_install | Removes dracut install hook for truescale assets. |
| providers/ipathverbs/dracut_check | Removes dracut hostonly detection for qib hardware. |
| providers/ipathverbs/COPYING | Removes provider-specific licensing file for ipathverbs. |
| providers/ipathverbs/CMakeLists.txt | Removes provider build/install rules for ipathverbs. |
| providers/hfi2verbs/verbs.c | Adds hfi2 verbs implementation (CQ/QP/SRQ/etc). |
| providers/hfi2verbs/hfiverbs.h | Adds hfi2 provider data structures and function prototypes. |
| providers/hfi2verbs/hfiverbs.c | Adds hfi2 provider registration/match table. |
| providers/hfi2verbs/hfi-abi.h | Adds hfi2 provider UAPI shim header. |
| providers/hfi2verbs/CMakeLists.txt | Adds build rules for the hfi2 provider. |
| MAINTAINERS | Adds a maintainer section for the new hfi2 provider. |
| libibverbs/verbs.h | Adds hfi2 provider extern and includes substantial formatting/macro changes. |
| kernel-headers/rdma/mlx5_user_ioctl_cmds.h | Adds new mlx5 CQ/QP attribute IDs for umem-backed DBR buffers. |
| kernel-headers/rdma/ib_user_verbs.h | Adds a new uverbs device capability bit. |
| kernel-headers/rdma/ib_user_ioctl_verbs.h | Adds RDMA_DRIVER_HFI2 and a new buffer descriptor UAPI type. |
| kernel-headers/rdma/ib_user_ioctl_cmds.h | Adds new uverbs CQ/QP buffer umem attribute IDs. |
| kernel-headers/rdma/hfi2-abi.h | Adds hfi2 kernel ABI header to published kernel headers set. |
| kernel-headers/rdma/bnxt_re-abi.h | Updates bnxt_re QP mask naming/fields and adds create_qp attrs. |
| kernel-headers/CMakeLists.txt | Publishes the new rdma/hfi2-abi.h header. |
| kernel-boot/rdma-description.rules | Marks hfi2 devices as OPA-capable via udev classification. |
| debian/rdma-core.lintian-overrides | Removes obsolete lintian overrides tied to truescale modprobe install usage. |
| debian/rdma-core.install | Stops installing removed truescale config/script into the Debian package. |
| CMakeLists.txt | Switches provider build from ipathverbs to hfi2verbs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #if __GNUC__ >= 3 | ||
| # define __attribute_const __attribute__((const)) | ||
| #define __attribute_const __attribute_const__ | ||
| #else | ||
| # define __attribute_const | ||
| #define __attribute_const | ||
| #endif |
| struct hfi2_cq_wc { | ||
| _Atomic(uint32_t) head; | ||
| _Atomic(uint32_t) tail; | ||
| struct hfi2_wc queue[]; | ||
| }; |
| S: Supported | ||
| F: providers/hfi1verbs/ | ||
|
|
||
| HFI2 USERSPACE PROVIDER (for hf2.ko) |
| HFI2_ATTR_SDMA_FD_RSP = (1U << UVERBS_ID_NS_SHIFT), | ||
| }; | ||
|
|
||
| #endif /* _LINIUX_HFI2_USER_H */ |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
libibverbs/verbs.h:2304
- This header still declares
verbs_provider_ipathverbs, but the ipathverbs provider has been removed from the build (andproviders/ipathverbs/is gone). Keeping the extern makes the public provider list misleading.
extern const struct verbs_device_ops verbs_provider_erdma;
extern const struct verbs_device_ops verbs_provider_hfi1verbs;
extern const struct verbs_device_ops verbs_provider_hfi2verbs;
extern const struct verbs_device_ops verbs_provider_hns;
extern const struct verbs_device_ops verbs_provider_ipathverbs;
extern const struct verbs_device_ops verbs_provider_irdma;
extern const struct verbs_device_ops verbs_provider_mana;
libibverbs/verbs.h:58
__attribute_constis defined as__attribute_const__, which is not defined here and will typically break compilation when__attribute_constis used in function prototypes.
#if __GNUC__ >= 3
#define __attribute_const __attribute_const__
#else
#define __attribute_const
#endif
MAINTAINERS:74
- Typo in kernel module name: this should reference
hfi2.ko(nothf2.ko).
HFI2 USERSPACE PROVIDER (for hf2.ko)
kernel-headers/rdma/bnxt_re-abi.h:130
- Renaming the enum constant removes the old identifier from the installed UAPI header, which can break source compatibility for out-of-tree consumers. Consider keeping the old name as an alias to the new one.
enum bnxt_re_qp_mask {
BNXT_RE_QP_REQ_MASK_FIXED_QUE_ATTR = 0x1,
};
| struct hfi2_cq_wc { | ||
| _Atomic(uint32_t) head; | ||
| _Atomic(uint32_t) tail; | ||
| struct hfi2_wc queue[]; | ||
| }; |
The ipath driver has been dead for over a decade. Qib has also been removed from the kernel. Toss this old code out of the boat. Signed-off-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.
Suppressed comments (5)
libibverbs/verbs.h:58
__attribute_constis redefined to__attribute_const__, which is not guaranteed to be provided by all libc/toolchain environments (it’s a glibc internal macro). This can break compilation on non-glibc systems. Define it directly using GCC’s attribute syntax, as before.
#if __GNUC__ >= 3
#define __attribute_const __attribute_const__
#else
#define __attribute_const
#endif
providers/hfi2verbs/hfiverbs.h:104
struct hfi2_cq_wcuses a flexible array member (queue[]), but the CQ mmap sizing and wrap logic inverbs.cassume an extra sentinel element (i.e., indices can reachcqe). Withqueue[], the computed mmap size is too small and can lead to out-of-bounds access whentailreachescqe.
struct hfi2_cq_wc {
_Atomic(uint32_t) head;
_Atomic(uint32_t) tail;
struct hfi2_wc queue[];
};
kernel-headers/rdma/hfi2-abi.h:678
- Typo in the header guard comment (
_LINIUX_->_LINUX_).
#endif /* _LINIUX_HFI2_USER_H */
providers/hfi2verbs/hfiverbs.c:69
INT_MAXis used inhfi2_dev_ops.match_max_abi_version, but this file does not include<limits.h>, which can cause build failures depending on include ordering.
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
MAINTAINERS:74
- Typo in kernel module name:
hf2.koshould behfi2.ko. Also, sinceproviders/ipathverbs/is removed in this PR, the existing IPATH provider entry later in this file (currently around line 107) should be removed/updated to avoid stale maintenance metadata.
HFI2 USERSPACE PROVIDER (for hf2.ko)
Add InfiniBand verbs support for the hfi2 driver, which drives the Cornelis Networks CN5000 adapter. Signed-off-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
To commit: ?? ("RDMA/hfi2: Support ipoib").
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
libibverbs/verbs.h:58
- __attribute_const is redefined to attribute_const, which is not defined by this header and can cause compile failures on libcs/toolchains that don't provide that macro (it turns function declarations like
int __attribute_const foo();into invalid syntax).
#if __GNUC__ >= 3
#define __attribute_const __attribute_const__
#else
#define __attribute_const
#endif
libibverbs/verbs.h:2348
- verbs_provider_ipathverbs is still declared in the public header, but this PR removes the ipathverbs provider from the build (top-level CMakeLists.txt stops adding providers/ipathverbs and the provider sources are deleted). Keeping this declaration can mislead users and can trigger link failures if someone tries to statically reference the symbol.
extern const struct verbs_device_ops verbs_provider_hfi1verbs;
extern const struct verbs_device_ops verbs_provider_hfi2verbs;
extern const struct verbs_device_ops verbs_provider_hns;
extern const struct verbs_device_ops verbs_provider_ipathverbs;
extern const struct verbs_device_ops verbs_provider_irdma;
MAINTAINERS:74
- Typo in kernel module name: this entry says "hf2.ko" but the driver name elsewhere is "hfi2" (e.g., rdma-description.rules uses DRIVERS=="hfi2").
HFI2 USERSPACE PROVIDER (for hf2.ko)
kernel-headers/rdma/hfi2-abi.h:678
- Typo in the closing include-guard comment (LINIUX vs LINUX).
#endif /* _LINIUX_HFI2_USER_H */
| struct hfi2_cq_wc { | ||
| _Atomic(uint32_t) head; | ||
| _Atomic(uint32_t) tail; | ||
| struct hfi2_wc queue[]; | ||
| }; |
|
Strange, i disabled for more than once Copilot and it still reports. |
Add support hfi2 and get rid of legacy ipath. The kernel series for this is making it's way to the list now.