Skip to content

Reduce assumptions about xrt-smi path #2685

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.

Sign up for GitHub

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Reduce assumptions about xrt-smi path #2685

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b52bf95
some progress so far
hunhoffe Nov 4, 2025
cf1eb36
hardcoded paths are not ideal....
hunhoffe Nov 4, 2025
5a21b62
remove some platform dependent stuff which should not be needed
hunhoffe Nov 4, 2025
de3fca5
Update python/iron/config.py
hunhoffe Nov 4, 2025
6422470
remove changes to venv and dependencies; remove one more path from th…
hunhoffe Nov 5, 2025
eb4f52c
Merge branch 'main' into xrt-path-updates
hunhoffe Nov 5, 2025
db4e136
make error messages unified
hunhoffe Nov 5, 2025
93c7d90
Merge branch 'main' into xrt-path-updates
hunhoffe Nov 5, 2025
aa8ccfd
Merge branch 'main' into xrt-path-updates
hunhoffe Nov 6, 2025
521bba2
Merge branch 'main' into xrt-path-updates
hunhoffe Nov 7, 2025
7157115
Merge branch 'main' into xrt-path-updates
hunhoffe Nov 8, 2025
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
2 changes: 1 addition & 1 deletion docs/Building.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ Turn off SecureBoot (Allows for unsigned drivers to be installed):

1. Setup a virtual environment:
```bash
python3 -m venv ironenv
python3 -m venv --system-site-packages ironenv
source ironenv/bin/activate
python3 -m pip install --upgrade pip
```
Expand Down
24 changes: 2 additions & 22 deletions python/iron/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,12 @@
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
#
# (c) Copyright 2025 Advanced Micro Devices, Inc.

import shutil
import subprocess

from .device import NPU1, NPU2


# Detect WSL
def is_wsl() -> bool:
try:
with open("/proc/sys/kernel/osrelease", "r", encoding="utf-8") as kernel:
return "microsoft" in kernel.read().lower()
except OSError:
return False


# Prefer Windows xrt-smi when in WSL. Linux native otherwise.
def xrt_smi_path() -> str:
if is_wsl():
return "/mnt/c/Windows/System32/AMD/xrt-smi.exe"
return "/opt/xilinx/xrt/bin/xrt-smi"


def detect_npu_device():
"""Detects the current device in the system.
This assumes XRT and XDNA driver is installed
Expand All @@ -37,7 +21,7 @@ def detect_npu_device():
"""
try:
# Run `xrt-smi examine` and capture output
xrt_smi = xrt_smi_path()
xrt_smi = shutil.which("xrt-smi")
result = subprocess.run(
[xrt_smi, "examine"],
check=True,
Expand Down Expand Up @@ -73,10 +57,6 @@ def detect_npu_device():
raise RuntimeError("No supported NPU device found.")

except FileNotFoundError:
if is_wsl():
raise RuntimeError(
"WSL detected but Windows xrt-smi.exe not found. Install AMD Ryzen AI Software."
)
raise RuntimeError("xrt-smi not found. Make sure XRT is installed.")
except subprocess.CalledProcessError:
raise RuntimeError("Failed to run xrt-smi examine.")
Expand Down
3 changes: 2 additions & 1 deletion utils/env_setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if [ "$#" -ge 1 ]; then
export MLIR_AIE_INSTALL_DIR=`realpath $1`
export PATH=${MLIR_AIE_INSTALL_DIR}/bin:${PATH}
export PYTHONPATH=${MLIR_AIE_INSTALL_DIR}/python:${PYTHONPATH}
export LD_LIBRARY_PATH=${MLIR_AIE_INSTALL_DIR}/lib:${LD_LIBRARY_PATH}
export LD_LIBRARY_PATH=${MLIR_AIE_INSTALL_DIR}/lib:${LD_LIBRARY_PATH}:/usr/lib/python3
Copy link
Collaborator

@jgmelber jgmelber Nov 4, 2025

Choose a reason for hiding this comment

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

Why?

Copy link
Collaborator Author

@hunhoffe hunhoffe Nov 4, 2025

Choose a reason for hiding this comment

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

When you install xrt from the debs, a .so gets put in site packages. Python needs the site packages in the LD_LIBRARY_PATH to find this shared object file, as far as I could tell.

Copy link
Collaborator

@ypapadop-amd ypapadop-amd Nov 5, 2025

Choose a reason for hiding this comment

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

I can see this one backfiring with venvs.

Copy link
Collaborator Author

@hunhoffe hunhoffe Nov 5, 2025

Choose a reason for hiding this comment

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

Yes, I had to change the command for creating the venv in the README to allow the venv to see/access site packages. I don't think this is a great solution, so I'm happy to hear other suggestions!

Copy link
Collaborator

@fifield fifield Nov 5, 2025

Choose a reason for hiding this comment

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

This makes me want to never use the debs and continue building it myself.

The ideal solution would be to fix the upstream packaging so that one could 'apt install xdna-driver' then 'pip install pyxrt' for their venv and python version, with a looser coupling between the driver and xrt layers. The next best solution might be for mlir-aie to include the bindings as part of its build/wheel.

Copy link
Collaborator

@ypapadop-amd ypapadop-amd Nov 5, 2025

Choose a reason for hiding this comment

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

Yes, I had to change the command for creating the venv in the README to allow the venv to see/access site packages. I don't think this is a great solution, so I'm happy to hear other suggestions!

This can break a lot of other things. mlir-aie is just a part of a toolchain and assuming site-packages AFAIK can force specific package versions which other parts of the toolchain may not like. That was the case in the past with cmake being part of the requirements.txt for example.

I think the the right thing here is to allow local package installation in editable mode via pip install -e . but I haven't used it when libs are needed.

Copy link
Collaborator Author

@hunhoffe hunhoffe Nov 5, 2025

Choose a reason for hiding this comment

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

@ypapadop-amd I didn't pursue the pip install method because I'm not sure we assume a user would know the location of the debs? I also haven't used debs in this manner, so I'd need to check it. I could look into how we could migrate just this one site package into the venv... but that still sounds a little messy.

@fifield I think those solutions both sound better. I'm not sure what steps need to be taken for either one.

Copy link
Collaborator

@ypapadop-amd ypapadop-amd Nov 5, 2025

Choose a reason for hiding this comment

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

Maybe I'm missing something. Why would the user need to know the location? The XRT setup script is already taking care of PYTHONPATH and LD_LIBRARY_PATH for XRT and pyxrt.

env_setup.sh only manipulates LD_LIBRARY_PATH and PYTHONPATH because mlir-aie is not installed in the vevn; if you pip install mlir-aie there's no need for either.

FORCE_INSTALL=0
else
export MLIR_AIE_INSTALL_DIR="$(pip show mlir_aie 2>/dev/null | grep ^Location: | awk '{print $2}')/mlir_aie"
Expand All @@ -54,6 +54,7 @@ fi

XRTSMI=`which xrt-smi`
if ! test -f "$XRTSMI"; then
# TODO(erika): should this be removed?
Copy link
Collaborator

@jgmelber jgmelber Nov 4, 2025

Choose a reason for hiding this comment

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

yes. as long as it is clear in the README that xrt-smi needs to be available on the user's PATH

Copy link
Collaborator Author

@hunhoffe hunhoffe Nov 5, 2025

Choose a reason for hiding this comment

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

The test command for xrt-smi is already in both the main README and the building from scratch page. I tried to make it look a little more explicit too!

source /opt/xilinx/xrt/setup.sh
fi
NPU=`xrt-smi examine | grep -E "NPU Phoenix|NPU Strix|NPU Strix Halo|NPU Krackan|RyzenAI-npu[1456]"`
Expand Down
2 changes: 1 addition & 1 deletion utils/quick_setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ if [ -z "${WSL_DISTRO_NAME-}" ]; then
echo "XRT is not installed"
return 1
fi
NPU=`/opt/xilinx/xrt/bin/xrt-smi examine | grep -E "NPU Phoenix|NPU Strix|NPU Strix Halo|NPU Krackan|RyzenAI-npu[1456]"`
NPU=`$XRTSMI examine | grep -E "NPU Phoenix|NPU Strix|NPU Strix Halo|NPU Krackan|RyzenAI-npu[1456]"`
if echo "$NPU" | grep -qE "NPU Phoenix|NPU Strix|NPU Strix Halo|NPU Krackan|RyzenAI-npu[1456]"; then
echo "AMD XDNA NPU found: "
echo $NPU
Expand Down
4 changes: 0 additions & 4 deletions utils/reset_npu.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ if [ x"$NUMBER" != x"" ]; then
sudo modprobe -r amdxdna
sudo modprobe drm_shmem_helper
sudo modprobe amdxdna

# if [ -f "/opt/xilinx/xrt/test/example_noop_test" ]; then
# /opt/xilinx/xrt/test/example_noop_test /lib/firmware/amdipu/1502/validate.xclbin
# fi
else
echo "couldn't find npu"
fi
Expand Down
Loading