Repository navigation
Conversation
The Linux driver discovery (v1.28.x) gained a stat()-based pre-filter (getLibrarySearchPaths) that only checks LD_LIBRARY_PATH, the system locations (/lib, /usr/lib, multiarch), and /etc/ld.so.conf. A driver installed in a relocatable prefix that is not on any of those (e.g. a conda environment) is filtered out before the dlopen that would otherwise load it -- even though its soname is already in the default knownDriverNames list. Add an optional compile-time macro ZE_LOADER_EXTRA_DRIVER_DIR. When defined by the build system to a directory (a package can set it to its own install prefix's lib dir), that directory is appended to the search paths immediately after LD_LIBRARY_PATH -- so a user's LD_LIBRARY_PATH still wins -- and before the system locations. Undefined by default, so upstream/system builds are unchanged. CMake injects the macro as a quoted string literal via target_compile_definitions when ZE_LOADER_EXTRA_DRIVER_DIR is passed at configure time.
There was a problem hiding this comment.
🟢 Approval recommended
The narrow, opt-in change preserves default behavior and has no unresolved findings.
No open issues 🎉
What changed in this PR
Adds an optional build-time driver directory to Linux discovery, allowing drivers in custom installation prefixes to pass the discovery pre-filter.
Changes:
- Passes nonempty
ZE_LOADER_EXTRA_DRIVER_DIRvalues as a private compile definition. - Searches that directory after
LD_LIBRARY_PATHand before system locations, preserving default behavior when unset.
| File | Description |
|---|---|
| source/loader/linux/driver_discovery_lin.cpp | Adds the configured directory to discovery search paths. |
| source/loader/CMakeLists.txt | Conditionally defines the extra driver directory. |
Review effort: Balanced
|
This is great thx |
<details><summary>Claude's draft</summary> Replace recipe/patches/0001-loader-build-time-driver-dir.patch with the format-patch of the commit in oneapi-src/level-zero#541 (b2097e838aee2205b110f3314283f563f5c4f146), so the feedstock carries exactly what is under upstream review. The code hunks are identical to the old patch; only the file name and the mail header (diffstat, full index hashes) change. meta.yaml now links the upstream PR next to the patch so it can be dropped once #541 is in a release. Verified the patch applies to the v1.34.0 tarball with both `patch -p1 --dry-run` and `git apply --check -p1`. build.sh still passes -DZE_LOADER_EXTRA_DRIVER_DIR, the same variable the upstream version reads. Resume this Claude session: ``` cd /home/mark/git/feedstock claude --resume fdaba65c-5a43-4f26-a32f-d49b747fdbd7 ``` </details> Claude-Session: https://claude.ai/code/session_01R9WZp6UVaiXYyC2hWTNWZY
| auto split = splitPaths(ldLibPath); | ||
| paths.insert(paths.end(), split.begin(), split.end()); | ||
| } | ||
| #ifdef ZE_LOADER_EXTRA_DRIVER_DIR |
There was a problem hiding this comment.
Recommend changing the name for conformity, the ZE_* nomenclature is heavily standardized for driver and loader runtime variables, can we go with something like "L0_LOADER_EXTRA_DRIVER_DIR" ?
|
Some history, we need to keep this patch for Linux only, and I realize that is the target. So just cautionary advice that Windows systems get real touchy on introducing additional search pathing due to DLL hijacking attacks. LD_LIBRARY_PATH easily already allows this, so a Linux edit like this doesn't introduce a wider attack vector. |
|
This brings up a second question, a legacy topic which we could address here, or I can move to a second PR (perhaps better). By enabling more search paths the logic today that finds a file vs loads a file, may actually allow loading a different file than intended. i.e. there is a time of detection and time of use race (can be changed between lookup and load).. But also the additional path is not being passed into the dlopen() call logic, only the pre-filtering detection logic. The fact this works in Konda today is likely due to the RPATH being compiled in... I think we can change the detection logic to load the full path and save it, and thus pass the full path into the dlopen() call solving both issues... Also may eliminate the need for the RPATH addition. If you feel this is to much for this PR we can move that enhancement into a different PR, or if you want to take a crack it. If not, just ensure this PR is targeted Linux only and doesn't introduce the users own folder option. |
…path Rename the build-time option to L0_LOADER_EXTRA_DRIVER_DIR so it does not look like a ZE_* runtime environment variable. Discovery on Linux used to check that a known driver existed in one of the search paths and then dlopen() the bare soname, leaving the dynamic linker to pick the file. The file that was checked and the file that was loaded could therefore differ, and a directory only on the search-path list (such as the build-time extra directory) passed the check without being reachable by dlopen(). Return the full path of the first match and load that path instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks @rwmcguir. Both are addressed in 0e8587a:
One behavior change to check: the search-path order now decides which copy wins, not the dynamic linker. That order is Tested on Ubuntu 24.04 / g++ 13:
|
<details><summary>Claude's draft</summary> Upstream reworked oneapi-src/level-zero#541 after review: the CMake option is now L0_LOADER_EXTRA_DRIVER_DIR, and Linux discovery returns the full path of the driver it found and dlopen()s that path, instead of dlopen()ing the bare soname. - 0001: upstream b2097e8, unchanged. - 0002: upstream 0e8587a, with line endings normalized to LF. That commit converts both files to CRLF, so as published it rewrites every line and would not apply cleanly to the LF 1.34.0 tarball. Ignoring CR, the result is byte-identical to the upstream branch. - 0003: new. L0_LOADER_EXTRA_DRIVER_DIR is a string literal, so GCC folds the std::string length at compile time. conda relocates the prefix by rewriting the string in the binary and padding it with NULs, so the NULs stayed inside the path and dlopen() failed with "Is a directory". The old patch only appeared to work because stat() stopped at the first NUL, found the lib directory, and the bare soname then resolved via the RPATH. Reading the literal through a volatile pointer takes the length at run time. - build.sh passes -DL0_LOADER_EXTRA_DRIVER_DIR. Verified with build-locally.py linux_64_ (recipe tests pass), then installed the package with intel-level-zero-npu into a fresh env and ran zeInit with ZE_ENABLE_LOADER_DEBUG_TRACE=1 and without ZE_ENABLE_ALT_DRIVERS or LD_LIBRARY_PATH on an Arrow Lake host: before 0003 every driver failed to load ("Is a directory"); with it, $PREFIX/lib/libze_intel_npu.so.1 loads by full path and zeDriverGet returns 1 driver. Resume this Claude session: ``` cd /home/mark/git/feedstock claude --resume fdaba65c-5a43-4f26-a32f-d49b747fdbd7 ``` </details> Claude-Session: https://claude.ai/code/session_01R9WZp6UVaiXYyC2hWTNWZY
Since 1.28.x, Linux driver discovery runs a
stat()pre-filter (getLibrarySearchPaths) that only checksLD_LIBRARY_PATH, the system lib dirs and/etc/ld.so.conf. A driver installed in a relocatable prefix such as a conda environment (e.g.libze_intel_npu.so.1, already inknownDriverNames) is dropped before thedlopenthat would have loaded it. Today the only workaround isZE_ENABLE_ALT_DRIVERS.This adds an optional CMake option,
ZE_LOADER_EXTRA_DRIVER_DIR. When it is set, that directory is searched right afterLD_LIBRARY_PATHand before the system locations. It is unset by default, so existing builds behave the same.Context: conda-forge currently carries this as a patch (conda-forge/level-zero-feedstock#92). The reviewers there asked for it to go upstream instead. The patch was written by @hmaarrfk and keeps his authorship.
One alternative is to also search the loader's own directory (resolved via
dladdr, i.e.$ORIGIN). That would cover relocatable installs without a build-time setting. We're happy to switch to that if you prefer it. cc @nrspruit🤖 Generated with Claude Code