Skip to content
Open
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions CMake/pkg.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -118,19 +118,31 @@ 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()
else("${XDNA_CPACK_LINUX_PKG_FLAVOR}" MATCHES "debian")
elseif("${XDNA_CPACK_LINUX_PKG_FLAVOR}" MATCHES "arch")
set(CPACK_GENERATOR "TGZ")
# For Arch Linux, we generate a tarball that can be repackaged into a proper
# Arch package using the provided PKGBUILD. When using the PKGBUILD, install
# hooks handle post-install/pre-remove automatically via pacman.
set(CPACK_ARCHIVE_COMPONENT_INSTALL ON)
message(STATUS "Arch Linux detected - generating TGZ package")
if(NOT SKIP_KMOD)
message(STATUS "Post-install script: ${CMAKE_CURRENT_BINARY_DIR}/package/postinst")
message(STATUS "Pre-remove script: ${CMAKE_CURRENT_BINARY_DIR}/package/prerm")
message(STATUS "Note: Use the provided PKGBUILD to create an Arch package with proper install hooks")
endif()
else()
message(FATAL_ERROR "Unknown Linux package flavor: ${XDNA_CPACK_LINUX_PKG_FLAVOR}")
Copy link

Copilot AI Nov 22, 2025

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.

Copilot uses AI. Check for mistakes.
endif("${XDNA_CPACK_LINUX_PKG_FLAVOR}" MATCHES "debian")
endif()

include(CPack)
50 changes: 48 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using bash -c with echo and >> redirection is unnecessarily complex and potentially error-prone. A more robust approach would be to use tee -a or create a dedicated script. Additionally, directly appending to /etc/security/limits.conf without checking for existing entries could create duplicates on repeated runs. Consider: echo "* soft memlock unlimited" | sudo tee -a /etc/security/limits.conf or add a check to prevent duplicates.

Suggested change
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 uses AI. Check for mistakes.

# Log out and log back in (or reboot) for memory limit changes to take effect
```

**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:
Copy link

Copilot AI Nov 28, 2025

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.

Suggested change
**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:

Copilot uses AI. Check for mistakes.
- XRT packages: `xrt/build/arch/` (PKGBUILD-xrt-base, PKGBUILD-xrt-npu)
- XDNA driver: `build/` directory (PKGBUILD-xrt-plugin)

This ensures proper integration with pacman for installation, upgrades, and removal.

You will find `xrt_plugin*-amdxdna.deb` (Ubuntu/Debian) or `xrt_plugin*.tar.gz` (Arch Linux) in Release/ folder. This package includes:
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation states that the package will be in the "Release/" folder, but for Arch Linux builds, the actual .pkg.tar.zst packages are created in the respective build directories (xrt/build/arch/ and build/). Only the intermediate .tar.gz file is in Release/. Consider clarifying: "You will find the intermediate xrt_plugin*.tar.gz in Release/ folder (which is repackaged into xrt-plugin-amdxdna-*.pkg.tar.zst using the PKGBUILD)."

Suggested change
You will find `xrt_plugin*-amdxdna.deb` (Ubuntu/Debian) or `xrt_plugin*.tar.gz` (Arch Linux) in Release/ folder. This package includes:
You will find the intermediate `xrt_plugin*.tar.gz` (Arch Linux) or `xrt_plugin*-amdxdna.deb` (Ubuntu/Debian) in the Release/ folder. For Arch Linux, this `.tar.gz` is repackaged into the final installable package `xrt-plugin-amdxdna-*.pkg.tar.zst` using the provided PKGBUILD, which can be found in the respective build directories (`xrt/build/arch/` or `build/`). These packages include:

Copilot uses AI. Check for mistakes.
* The `.so` library files, which will be installed into `/opt/xilinx/xrt/lib` folder
* The XDNA driver and DKMS script, which build, install and load
`amdxdna.ko` driver when installing the .DEB package on target machine
Expand Down
39 changes: 39 additions & 0 deletions build/PKGBUILD-xrt-plugin
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Maintainer: Your Name <[email protected]>
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The maintainer information should be updated with actual contact details before distribution.

Suggested change
# Maintainer: Your Name <your@email.com>
# Maintainer: Jane Doe <jane.doe@example.com>

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Nov 28, 2025

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.

Suggested change
# Maintainer: Your Name <your@email.com>
# Maintainer: Jane Doe <jane.doe@example.com>

Copilot uses AI. Check for mistakes.
pkgname=xrt-plugin-amdxdna
pkgver=2.21.0
pkgrel=1
pkgdesc="AMD XDNA Driver plugin for Xilinx RunTime"
arch=('x86_64')
url="https://github.com/amd/xdna-driver"
license=('Apache-2.0')
depends=('xrt-base' 'xrt-npu' 'dkms' 'linux-headers')
provides=('xrt-plugin-amdxdna')
conflicts=('amdxdna-driver' 'amdxdna-driver-bin')
install=xrt-plugin-amdxdna.install

# Set this to your xdna-driver build output directory (relative to PKGBUILD location)
Copy link

Copilot AI Nov 28, 2025

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).

Suggested change
# 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 uses AI. Check for mistakes.
: ${XDNA_BUILD_DIR:="Release"}
Copy link

Copilot AI Dec 3, 2025

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.

Suggested change
: ${XDNA_BUILD_DIR:="Release"}
XDNA_BUILD_DIR=${XDNA_BUILD_DIR:-Release}

Copilot uses AI. Check for mistakes.

package() {
cd "$srcdir"

# Extract the tarball directly into the package directory
# The tarball should be at $startdir/$XDNA_BUILD_DIR/xrt_plugin*.tar.gz
local tarballs=("$startdir/${XDNA_BUILD_DIR}"/xrt_plugin*-amdxdna.tar.gz)
Comment on lines 21 to 25
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The array check [ ${#tarballs[@]} -eq 0 ] will not work as expected because bash glob expansion that finds no matches will result in an array with one element containing the literal pattern. The correct check should be [ ! -f "${tarballs[0]}" ] only, or use shopt -s nullglob before the glob to handle no matches properly.

Suggested change
# The tarball should be at $startdir/$XDNA_BUILD_DIR/xrt_plugin*.tar.gz
local tarballs=("$startdir/${XDNA_BUILD_DIR}"/xrt_plugin*-amdxdna.tar.gz)
# The tarball should be at $startdir/$XDNA_BUILD_DIR/xrt_plugin*.tar.gz
local _nullglob_set=$(shopt -p nullglob)
shopt -s nullglob
local tarballs=("$startdir/${XDNA_BUILD_DIR}"/xrt_plugin*-amdxdna.tar.gz)
$_nullglob_set

Copilot uses AI. Check for mistakes.
if [ ${#tarballs[@]} -eq 0 ] || [ ! -f "${tarballs[0]}" ]; then
error "XDNA plugin tarball not found in ${XDNA_BUILD_DIR}"
error "Please build XDNA driver first: cd build && ./build.sh -release"
return 1
fi

local tarball="${tarballs[0]}"
Comment on lines 25 to 33
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The glob pattern uses a generic wildcard that could match multiple tarballs if different versions exist. Consider using a more specific pattern or add logic to handle multiple matches, selecting the most recent one if needed.

Suggested change
local tarballs=("$startdir/${XDNA_BUILD_DIR}"/xrt_plugin*-amdxdna.tar.gz)
if [ ${#tarballs[@]} -eq 0 ] || [ ! -f "${tarballs[0]}" ]; then
error "XDNA plugin tarball not found in ${XDNA_BUILD_DIR}"
error "Please build XDNA driver first: cd build && ./build.sh -release"
return 1
fi
local tarball="${tarballs[0]}"
# Find all matching tarballs and select the most recently modified one
local tarball
tarball=$(ls -1t "$startdir/${XDNA_BUILD_DIR}"/xrt_plugin*-amdxdna.tar.gz 2>/dev/null | head -n 1)
if [ -z "$tarball" ] || [ ! -f "$tarball" ]; then
error "XDNA plugin tarball not found in ${XDNA_BUILD_DIR}"
error "Please build XDNA driver first: cd build && ./build.sh -release"
return 1
fi

Copilot uses AI. Check for mistakes.
msg2 "Extracting $tarball"
tar -xzf "$tarball" -C "$pkgdir"

# Copy the install scripts for reference (they'll be called from .install file)
Comment on lines +35 to +37
Copy link

Copilot AI Dec 3, 2025

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"

Suggested change
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 uses AI. Check for mistakes.
mkdir -p "$pkgdir/opt/xilinx/xrt/share/amdxdna/package"
Comment on lines +35 to +38
Copy link

Copilot AI Dec 3, 2025

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.

Suggested change
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

Copilot uses AI. Check for mistakes.
install -Dm755 $startdir/${XDNA_BUILD_DIR}/package/postinst \
"$pkgdir/opt/xilinx/xrt/share/amdxdna/package/postinst"
install -Dm755 $startdir/${XDNA_BUILD_DIR}/package/prerm \
"$pkgdir/opt/xilinx/xrt/share/amdxdna/package/prerm"
}
71 changes: 71 additions & 0 deletions build/xrt-plugin-amdxdna.install
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
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
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conditional check uses -e to test for directory existence, but should use -d instead. The -e flag tests for file existence (any type), while -d specifically tests for directories. This could lead to unexpected behavior if a file with the same name exists.

Suggested change
if [ -e ${dracut_conf_d} ]; then
if [ -d ${dracut_conf_d} ]; then

Copilot uses AI. Check for mistakes.
if [ ! -f ${dracut_conf_d}/${dracut_conf_file} ]; then
touch ${dracut_conf_d}/${dracut_conf_file}
fi
grep -q '^omit_drivers\+=\" amdxdna \"' "${dracut_conf_d}/${dracut_conf_file}" || echo 'omit_drivers+=" amdxdna "' >> "${dracut_conf_d}/${dracut_conf_file}"
fi

# Install DKMS module
echo "Installing amdxdna Linux kernel module via DKMS..."
$install_datadir/dkms_driver.sh --install
Copy link

Copilot AI Nov 28, 2025

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.

Suggested change
$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

Copilot uses AI. Check for mistakes.

# Setup udev rules
echo "Setting up udev rules..."
if [ ! -f ${udev_rules_d}/${amdxdna_rules_file} ]; then
touch ${udev_rules_d}/${amdxdna_rules_file}
fi
grep -q '^KERNEL=="accel\*",DRIVERS=="amdxdna",MODE="0666"$' ${udev_rules_d}/${amdxdna_rules_file} || \
echo 'KERNEL=="accel*",DRIVERS=="amdxdna",MODE="0666"' >> ${udev_rules_d}/${amdxdna_rules_file}
Comment on lines +31 to +32
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The udev rule sets MODE="0666" which grants read and write access to all users. While this may be intentional for device access, it could pose a security risk by allowing any user to access the NPU device. Consider using GROUP-based permissions with a dedicated group (e.g., "render" or "video") and MODE="0660" instead.

Suggested change
grep -q '^KERNEL=="accel\*",DRIVERS=="amdxdna",MODE="0666"$' ${udev_rules_d}/${amdxdna_rules_file} || \
echo 'KERNEL=="accel*",DRIVERS=="amdxdna",MODE="0666"' >> ${udev_rules_d}/${amdxdna_rules_file}
grep -q '^KERNEL=="accel\*",DRIVERS=="amdxdna",GROUP="video",MODE="0660"$' ${udev_rules_d}/${amdxdna_rules_file} || \
echo 'KERNEL=="accel*",DRIVERS=="amdxdna",GROUP="video",MODE="0660"' >> ${udev_rules_d}/${amdxdna_rules_file}

Copilot uses AI. Check for mistakes.

# Load the kernel module
echo "Loading amdxdna kernel module..."
rmmod amdxdna > /dev/null 2>&1 || true
Copy link

Copilot AI Dec 3, 2025

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.

Suggested change
rmmod amdxdna > /dev/null 2>&1 || true
if lsmod | grep -q "amdxdna"; then
rmmod amdxdna
fi

Copilot uses AI. Check for mistakes.
modprobe amdxdna
Copy link

Copilot AI Nov 28, 2025

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."

Suggested change
modprobe amdxdna
modprobe amdxdna || echo "Warning: Failed to load amdxdna module. Check dmesg for details."

Copilot uses AI. Check for mistakes.

echo "AMD XDNA driver installation complete!"
echo "You can test it with: source /opt/xilinx/xrt/setup.sh && xrt-smi validate"
}

post_upgrade() {
post_install
}

pre_remove() {
echo "Removing AMD XDNA driver..."

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

# Unload kernel module
if lsmod | grep -q "amdxdna"; then
rmmod amdxdna
Copy link

Copilot AI Dec 3, 2025

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.

Suggested change
rmmod amdxdna
rmmod amdxdna || echo "Warning: Failed to unload amdxdna module. It may be in use."

Copilot uses AI. Check for mistakes.
fi

# Remove dracut config
if [ -f ${dracut_conf_d}/${dracut_conf_file} ]; then
rm -f ${dracut_conf_d}/${dracut_conf_file}
fi

# Remove udev rules
if [ -f ${udev_rules_d}/${amdxdna_rules_file} ]; then
rm -f ${udev_rules_d}/${amdxdna_rules_file}
fi

# Remove DKMS module
$install_datadir/dkms_driver.sh --remove
Copy link

Copilot AI Dec 3, 2025

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

Suggested change
$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

Copilot uses AI. Check for mistakes.

echo "AMD XDNA driver removed!"
}
2 changes: 2 additions & 0 deletions tools/amdxdna_deps.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The -Syu flag performs a full system upgrade in addition to installing jq, which is inconsistent with the behavior of apt-get install and dnf install on lines 10 and 12. For consistency, use pacman -S --needed --noconfirm jq instead, which only installs the package without upgrading the entire system.

Suggested change
pacman -Syu --needed --noconfirm jq
pacman -S --needed --noconfirm jq

Copilot uses AI. Check for mistakes.
fi

$SCRIPT_DIR/../xrt/src/runtime_src/tools/scripts/xrtdeps.sh