-
Notifications
You must be signed in to change notification settings - Fork 93
Add Arch Linux support for xdna-driver #866
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 11 commits
24164d6
126c1a5
dd70faa
5aa2e92
d6fcf49
2951bb8
63480a4
16f0a4a
4d9eb85
88d8759
e3650e4
dfef9f3
bfd2b64
4f80d49
ea156e0
8cf1ba4
a666c05
b721228
3deb899
fb81b66
95a9dac
8d9c15d
d76a44b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| [submodule "xrt"] | ||
| path = xrt | ||
| url = https://github.com/Xilinx/XRT.git | ||
| url = https://github.com/kashif/XRT.git | ||
| branch = arch | ||
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -118,16 +118,27 @@ if("${XDNA_CPACK_LINUX_PKG_FLAVOR}" MATCHES "debian") | |||||
| set(CPACK_DEB_COMPONENT_INSTALL ON) | ||||||
| set(CPACK_DEBIAN_PACKAGE_DEPENDS "xrt-base (>= ${XDNA_CPACK_XRT_BASE_VERSION}), xrt-base (<< ${XDNA_CPACK_XRT_BASE_NEXT_VERSION})") | ||||||
| if(NOT SKIP_KMOD) | ||||||
| set(CPACK_DEBIAN_PACKAGE_CONTROL_EXTRA "${CMAKE_CURRENT_BINARY_DIR}/package/postinst" | ||||||
| set(CPACK_DEBIAN_PACKAGE_CONTROL_EXTRA "${CMAKE_CURRENT_BINARY_DIR}/package/postinst" | ||||||
| "${CMAKE_CURRENT_BINARY_DIR}/package/prerm") | ||||||
| endif() | ||||||
| elseif("${XDNA_CPACK_LINUX_PKG_FLAVOR}" MATCHES "fedora") | ||||||
| set(CPACK_GENERATOR "RPM") | ||||||
| set(CPACK_RPM_COMPONENT_INSTALL ON) | ||||||
| set(CPACK_RPM_PACKAGE_REQUIRES "xrt-base >= ${XDNA_CPACK_XRT_BASE_VERSION}, xrt-base < ${XDNA_CPACK_XRT_BASE_NEXT_VERSION}") | ||||||
| if(NOT SKIP_KMOD) | ||||||
| set(CPACK_RPM_POST_INSTALL_SCRIPT_FILE "${CMAKE_CURRENT_BINARY_DIR}/package/postinst") | ||||||
| set(CPACK_RPM_PRE_UNINSTALL_SCRIPT_FILE "${CMAKE_CURRENT_BINARY_DIR}/package/prerm") | ||||||
| set(CPACK_RPM_POST_INSTALL_SCRIPT_FILE "${CMAKE_CURRENT_BINARY_DIR}/package/postinst") | ||||||
| set(CPACK_RPM_PRE_UNINSTALL_SCRIPT_FILE "${CMAKE_CURRENT_BINARY_DIR}/package/prerm") | ||||||
| endif() | ||||||
| elseif("${XDNA_CPACK_LINUX_PKG_FLAVOR}" MATCHES "arch") | ||||||
| set(CPACK_GENERATOR "TGZ") | ||||||
| # For Arch Linux, we generate a tarball that can be used for manual installation | ||||||
| # or to create a PKGBUILD. Post-install scripts need to be run manually. | ||||||
|
||||||
| # or to create a PKGBUILD. Post-install scripts need to be run manually. | |
| # or to create a PKGBUILD. When using the provided PKGBUILD, post-install scripts are executed automatically by pacman. If installing manually from the TGZ, you need to run the post-install scripts yourself. |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error handling for unknown package flavors should include 'arch' in the conditional chain. The final else clause will incorrectly catch Arch Linux as an unknown flavor if the elseif on line 132 doesn't match. Consider changing line 143 to else() without the condition, or ensure the Arch detection pattern is comprehensive.
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -103,7 +103,7 @@ cd <root-of-source-tree> | |||||||||
| exit | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| ### Steps to create release build DEB package: | ||||||||||
| ### Steps to create release build DEB package (Ubuntu/Debian): | ||||||||||
|
|
||||||||||
| ``` bash | ||||||||||
| cd <root-of-source-tree>/build | ||||||||||
|
|
@@ -121,7 +121,53 @@ cd ../../build | |||||||||
| # To adapt according to your OS & version | ||||||||||
| sudo apt reinstall ./Release/xrt_plugin.2.19.0_ubuntu22.04-x86_64-amdxdna.deb | ||||||||||
| ``` | ||||||||||
| You will find `xrt_plugin\*-amdxdna.deb` in Release/ folder. This package includes: | ||||||||||
|
|
||||||||||
| ### Steps to create release build packages (Arch Linux): | ||||||||||
|
|
||||||||||
| ``` bash | ||||||||||
| cd <root-of-source-tree> | ||||||||||
|
|
||||||||||
| # Install dependencies (requires sudo) | ||||||||||
| sudo ./tools/amdxdna_deps.sh | ||||||||||
|
|
||||||||||
| # Get submodules | ||||||||||
| git submodule update --init --recursive | ||||||||||
|
|
||||||||||
| # Build XRT | ||||||||||
| cd xrt/build | ||||||||||
| ./build.sh -npu -opt | ||||||||||
|
|
||||||||||
| # Build and install XRT packages using pacman | ||||||||||
| # PKGBUILDs are in xrt/build/arch/ | ||||||||||
| cd arch | ||||||||||
| makepkg -p PKGBUILD-xrt-base | ||||||||||
| sudo pacman -U xrt-base-*.pkg.tar.zst | ||||||||||
|
|
||||||||||
| makepkg -p PKGBUILD-xrt-npu | ||||||||||
| sudo pacman -U xrt-npu-*.pkg.tar.zst | ||||||||||
|
|
||||||||||
| # Build XDNA driver | ||||||||||
| cd ../../../build | ||||||||||
| ./build.sh -release | ||||||||||
|
|
||||||||||
| # Build and install XDNA plugin package | ||||||||||
| makepkg -p PKGBUILD-xrt-plugin | ||||||||||
| sudo pacman -U xrt-plugin-amdxdna-*.pkg.tar.zst | ||||||||||
|
|
||||||||||
| # Configure memory limits (required for NPU access) | ||||||||||
| sudo bash -c 'echo "* soft memlock unlimited" >> /etc/security/limits.conf' | ||||||||||
| sudo bash -c 'echo "* hard memlock unlimited" >> /etc/security/limits.conf' | ||||||||||
|
Comment on lines
+158
to
+159
|
||||||||||
| sudo bash -c 'echo "* soft memlock unlimited" >> /etc/security/limits.conf' | |
| sudo bash -c 'echo "* hard memlock unlimited" >> /etc/security/limits.conf' | |
| grep -qxF '* soft memlock unlimited' /etc/security/limits.conf || echo '* soft memlock unlimited' | sudo tee -a /etc/security/limits.conf | |
| grep -qxF '* hard memlock unlimited' /etc/security/limits.conf || echo '* hard memlock unlimited' | sudo tee -a /etc/security/limits.conf |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The note mentions that XRT packages are in xrt/build/arch/, but the instructions in lines 136-147 show building XRT in xrt/build and then moving to a subdirectory arch. It would be clearer to explicitly state that the PKGBUILDs are generated during the XRT build process, or if they're provided by XRT, to mention that upfront.
| **Note for Arch Linux users**: The build system generates `.tar.gz` packages which are repackaged into proper Arch packages (`.pkg.tar.zst`) using the provided PKGBUILDs: | |
| **Note for Arch Linux users**: The build system generates `.tar.gz` packages which are repackaged into proper Arch packages (`.pkg.tar.zst`) using PKGBUILDs. | |
| The PKGBUILD files for XRT (`PKGBUILD-xrt-base`, `PKGBUILD-xrt-npu`) and the XDNA driver (`PKGBUILD-xrt-plugin`) are provided by the XRT repository and are available in the following locations after building: |
kashif marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,38 @@ | ||||||||||||||||||||||||||
| # Maintainer: Your Name <[email protected]> | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| # Maintainer: Your Name <your@email.com> | |
| # Maintainer: Jane Doe <jane.doe@example.com> |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The maintainer information is a placeholder. Replace "Your Name <[email protected]>" with actual maintainer details before distribution. This is important for package maintainability and user support.
| # Maintainer: Your Name <your@email.com> | |
| # Maintainer: Jane Doe <jane.doe@example.com> |
Outdated
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says "relative to PKGBUILD location", but $startdir in the package() function is actually the directory containing the PKGBUILD, making this slightly misleading. Consider clarifying that XDNA_BUILD_DIR is relative to the PKGBUILD's directory (i.e., $startdir).
| # Set this to your xdna-driver build output directory (relative to PKGBUILD location) | |
| # Set this to your xdna-driver build output directory (relative to the PKGBUILD's directory, i.e., $startdir) |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable assignment : ${XDNA_BUILD_DIR:="Release"} uses the colon null command which is a valid bash idiom for setting defaults. However, this pattern is unconventional in PKGBUILDs. The standard approach would be to use XDNA_BUILD_DIR=${XDNA_BUILD_DIR:-Release} which is clearer and more maintainable.
| : ${XDNA_BUILD_DIR:="Release"} | |
| XDNA_BUILD_DIR=${XDNA_BUILD_DIR:-Release} |
kashif marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
kashif marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The path $startdir/${XDNA_BUILD_DIR}/package/postinst uses unquoted variable expansion which could fail if XDNA_BUILD_DIR contains spaces or special characters. While "Release" is the default and unlikely to have spaces, it's good practice to quote variable expansions: "$startdir/${XDNA_BUILD_DIR}/package/postinst"
| tar -xzf "$tarball" -C "$pkgdir" | |
| # Copy the install scripts for reference (they'll be called from .install file) | |
| install -Dm755 "$startdir/${XDNA_BUILD_DIR}/package/postinst" \ | |
| "$pkgdir/opt/xilinx/xrt/share/amdxdna/package/postinst" | |
| install -Dm755 "$startdir/${XDNA_BUILD_DIR}/package/prerm" \ |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The install commands lack proper error handling. If the postinst or prerm files don't exist in the expected location, the install command will fail silently or cause the build to fail. Consider adding existence checks before these install commands or using || true if the files are optional.
| tar -xzf "$tarball" -C "$pkgdir" | |
| # Copy the install scripts for reference (they'll be called from .install file) | |
| mkdir -p "$pkgdir/opt/xilinx/xrt/share/amdxdna/package" | |
| if [ -f "$startdir/${XDNA_BUILD_DIR}/package/postinst" ]; then | |
| install -Dm755 "$startdir/${XDNA_BUILD_DIR}/package/postinst" \ | |
| "$pkgdir/opt/xilinx/xrt/share/amdxdna/package/postinst" | |
| fi | |
| if [ -f "$startdir/${XDNA_BUILD_DIR}/package/prerm" ]; then | |
| install -Dm755 "$startdir/${XDNA_BUILD_DIR}/package/prerm" \ | |
| "$pkgdir/opt/xilinx/xrt/share/amdxdna/package/prerm" | |
| fi |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,70 @@ | ||||||||||||||
| post_install() { | ||||||||||||||
| echo "Installing AMD XDNA driver via DKMS..." | ||||||||||||||
|
|
||||||||||||||
| install_datadir=/opt/xilinx/xrt/share/amdxdna | ||||||||||||||
| udev_rules_d=/etc/udev/rules.d | ||||||||||||||
| amdxdna_rules_file=99-amdxdna.rules | ||||||||||||||
| dracut_conf_d=/etc/dracut.conf.d | ||||||||||||||
| dracut_conf_file=amdxdna.dracut.conf | ||||||||||||||
|
|
||||||||||||||
| # On systems with dracut, exclude driver from initramfs | ||||||||||||||
| if [ -e ${dracut_conf_d} ]; then | ||||||||||||||
|
||||||||||||||
| if [ -e ${dracut_conf_d} ]; then | |
| if [ -d ${dracut_conf_d} ]; then |
kashif marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script executes dkms_driver.sh --install without checking if the script exists or if it's executable. If the script is missing or fails, the installation will fail without a clear error message. Consider adding an existence check and proper error handling.
| $install_datadir/dkms_driver.sh --install | |
| if [ ! -x "$install_datadir/dkms_driver.sh" ]; then | |
| echo "Error: $install_datadir/dkms_driver.sh not found or not executable." >&2 | |
| return 1 | |
| fi | |
| "$install_datadir/dkms_driver.sh" --install |
kashif marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting MODE="0666" grants read/write access to all users for the device. This may be overly permissive. Consider using a dedicated group (e.g., MODE="0660", GROUP="render") to restrict access to authorized users only.
| echo 'KERNEL=="accel*",DRIVERS=="amdxdna",MODE="0666"' > ${udev_rules_d}/${amdxdna_rules_file} | |
| echo 'KERNEL=="accel*",DRIVERS=="amdxdna",MODE="0660",GROUP="render"' > ${udev_rules_d}/${amdxdna_rules_file} |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rmmod amdxdna > /dev/null 2>&1 || true pattern silently ignores all errors, including cases where the module is in use and cannot be unloaded. This could lead to the subsequent modprobe loading the module while stale state exists. Consider checking if the module is loaded first with lsmod | grep -q amdxdna and handling the unload more explicitly, similar to the pre_remove function at line 57.
| rmmod amdxdna > /dev/null 2>&1 || true | |
| if lsmod | grep -q "amdxdna"; then | |
| rmmod amdxdna | |
| fi |
Outdated
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The modprobe amdxdna command can fail if the module isn't built or if there are dependency issues, but the script doesn't check its exit status. Consider adding error handling to inform the user if the module fails to load, e.g., modprobe amdxdna || echo "Warning: Failed to load amdxdna module. Check dmesg for details."
| modprobe amdxdna | |
| modprobe amdxdna || echo "Warning: Failed to load amdxdna module. Check dmesg for details." |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rmmod amdxdna command has no error handling. If the module is in use or fails to unload for any reason, the script will fail silently or abort. Consider adding error handling with a descriptive message: rmmod amdxdna || echo "Warning: Failed to unload amdxdna module. It may be in use." This is especially important during package removal when users need to understand why the removal failed.
| rmmod amdxdna | |
| rmmod amdxdna || echo "Warning: Failed to unload amdxdna module. It may be in use." |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DKMS removal script is called without error handling or existence check, unlike the installation which checks for the script's existence (line 20-23). If the script doesn't exist or fails, the removal will abort with an unclear error. Consider adding: if [ -x "$install_datadir/dkms_driver.sh" ]; then "$install_datadir/dkms_driver.sh" --remove || echo "Warning: Failed to remove DKMS module"; fi
| $install_datadir/dkms_driver.sh --remove | |
| if [ -x "$install_datadir/dkms_driver.sh" ]; then | |
| "$install_datadir/dkms_driver.sh" --remove || echo "Warning: Failed to remove DKMS module" | |
| fi |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -10,6 +10,8 @@ if [ -x "$(command -v apt-get)" ]; then | |||||
| apt-get install -y jq | ||||||
| elif [ -x "$(command -v dnf)" ]; then | ||||||
| dnf install -y jq | ||||||
| elif [ -x "$(command -v pacman)" ]; then | ||||||
| pacman -Syu --needed --noconfirm jq | ||||||
|
||||||
| pacman -Syu --needed --noconfirm jq | |
| pacman -S --needed --noconfirm jq |
Uh oh!
There was an error while loading. Please reload this page.