diff --git a/.github/scripts/tests/test_gpu_passthrough_entries.py b/.github/scripts/tests/test_gpu_passthrough_entries.py new file mode 100644 index 00000000..1f037aa7 --- /dev/null +++ b/.github/scripts/tests/test_gpu_passthrough_entries.py @@ -0,0 +1,92 @@ +"""The passthrough entries of a VM that belong to a GPU are the GPU, the audio +at its slot and the devices of its IOMMU group. Another function at the same +slot is a device of its own: on an AMD APU the USB controllers share the slot +of the integrated graphics, and a VM that only has one of them does not use +the GPU.""" + +from pathlib import Path +import shutil +import subprocess +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[3] +HELPERS = ROOT / "scripts/global/pci_passthrough_helpers.sh" +SCRIPTS = [ROOT / "scripts/gpu_tpu" / name + for name in ("switch_gpu_mode.sh", "switch_gpu_mode_direct.sh", "add_gpu_vm.sh")] +IGPU = "0000:05:00.0" +# The functions of the slot of an AMD APU, and a discrete card. +CLASSES = {"0000:05:00.0": "0x030000", "0000:05:00.1": "0x040300", "0000:05:00.3": "0x0c0330", + "0000:05:00.4": "0x0c0330", "0000:05:00.6": "0x040300", + "0000:01:00.0": "0x030000", "0000:01:00.1": "0x040300"} + + +@unittest.skipUnless(shutil.which("bash"), "bash is needed") +class GpuEntriesTests(unittest.TestCase): + def setUp(self): + folder = tempfile.TemporaryDirectory() + self.addCleanup(folder.cleanup) + self.folder = Path(folder.name) + for bdf, value in CLASSES.items(): + (self.folder / "sys" / bdf).mkdir(parents=True) + (self.folder / "sys" / bdf / "class").write_text(value + "\n") + + def entries(self, config, *group, gpu=IGPU): + conf = self.folder / "vm.conf" + conf.write_text(config) + script = f'source "{HELPERS}"; _vm_gpu_hostpci_indexes "$1" "$2" "${{@:3}}"' + done = subprocess.run(["bash", "-c", script, "bash", str(conf), gpu, *group], capture_output=True, text=True, + env={"PMX_PCI_SYSFS": str(self.folder / "sys"), "PATH": "/usr/bin:/bin"}, check=False) + self.assertEqual(done.stderr, "") + return done.stdout.split() + + def test_a_vm_with_only_a_usb_controller_of_that_slot_does_not_have_the_gpu(self): + self.assertEqual(self.entries("hostpci0: 0000:01:00.0,pcie=1,x-vga=1\nhostpci1: 0000:01:00.1,pcie=1\n" + "hostpci2: 0000:05:00.4,pcie=1\n"), []) + + def test_the_gpu_goes_with_the_audio_of_its_slot_and_the_usb_stays(self): + self.assertEqual(self.entries("hostpci0: 0000:05:00.0,pcie=1,x-vga=1\nhostpci1: 0000:05:00.1,pcie=1\n" + "hostpci2: 0000:05:00.4,pcie=1\nhostpci3: 0000:05:00.6\n"), ["0", "1", "3"]) + + def test_a_device_of_the_same_iommu_group_goes_with_the_gpu(self): + self.assertEqual(self.entries("hostpci0: 0000:05:00.0\nhostpci1: 0000:05:00.4\n", "0000:05:00.4"), ["0", "1"]) + + def test_the_whole_slot_carries_the_gpu(self): + self.assertEqual(self.entries("hostpci0: 05:00,pcie=1\nhostpci1: 0000:01:00.0\n"), ["0"]) + + def test_several_devices_in_one_entry(self): + self.assertEqual(self.entries("hostpci4: 0000:05:00.0;0000:05:00.1,pcie=1\n"), ["4"]) + + def test_addresses_are_read_with_or_without_domain_and_in_any_case(self): + self.assertEqual(self.entries("hostpci0: 05:00.0,pcie=1\n", gpu="05:00.0"), ["0"]) + self.assertEqual(self.entries("hostpci0: 0000:0A:00.0\n", gpu="0000:0a:00.0"), ["0"]) + + def test_another_gpu_and_a_slot_that_only_looks_alike_are_left(self): + # 00:02 inside 0000:02:00.0 is not the same device. + self.assertEqual(self.entries("hostpci0: 0000:02:00.0\n", gpu="0000:00:02.0"), []) + self.assertEqual(self.entries("hostpci0: 0000:01:00.0\nhostpci1: 0000:05:00.0\n", gpu="0000:01:00.0"), ["0"]) + + def test_snapshots_and_resource_mappings_are_not_read(self): + self.assertEqual(self.entries("hostpci0: 0000:01:00.0\n[snap1]\nhostpci0: 0000:05:00.0\n"), []) + self.assertEqual(self.entries("hostpci0: mapping=gpu1,pcie=1\nhostpci1: 0000:05:00.1\n"), []) + + def test_the_gpu_scripts_decide_through_it(self): + for script in SCRIPTS: + with self.subTest(script=script.name): + source = script.read_text() + self.assertIn("_vm_gpu_hostpci_indexes", source) + # No entry is deleted, or taken for the GPU, by its slot alone. + self.assertNotIn("sed -E -i \"/^hostpci", source) + self.assertNotRegex(source, r'grep -qE "hostpci\[0-9\]\+:\.\*\$\{pci_slot\}"') + self.assertNotRegex(source, r"\$\{slot\}\(\\\\\.\[0-7\]\)\?") + + def test_the_device_check_does_not_match_a_neighbour(self): + script = (f'qm() {{ printf "hostpci0: 0000:05:00.4,pcie=1\\n"; }}; source "{HELPERS}"; ' + '_pci_slot_assigned_to_vm 0000:05:00.0 100 && echo gpu; ' + '_pci_slot_assigned_to_vm 0000:05:00.4 100 && echo usb; true') + done = subprocess.run(["bash", "-c", script], capture_output=True, text=True, check=False) + self.assertEqual(done.stdout.split(), ["usb"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/global/pci_passthrough_helpers.sh b/scripts/global/pci_passthrough_helpers.sh index 60b45630..17b11601 100644 --- a/scripts/global/pci_passthrough_helpers.sh +++ b/scripts/global/pci_passthrough_helpers.sh @@ -156,6 +156,59 @@ function _vm_remove_hostpci_index() { qm set "$vmid" --delete "hostpci${idx}" >>"$log" 2>&1 } +# The hostpciN entries of a VM that carry a GPU, one index per line. +# +# A GPU is its own function, the audio functions at its slot (the HDMI codec +# it is added with) and the other devices of its IOMMU group, which is what +# the passthrough setup gives a VM. Any other function at the same slot is a +# device of its own: on an AMD APU the USB controllers share the slot of the +# integrated graphics and each one sits in its own IOMMU group. +# +# Nothing is printed when the VM does not have the GPU itself, so a VM that +# only uses one of those neighbours is not taken for one that uses the GPU. +# Only the current configuration is read, not its snapshots. +# +# Usage: _vm_gpu_hostpci_indexes [iommu_group_bdf]... +function _vm_gpu_hostpci_indexes() { + local conf="$1" gpu="$2" + shift 2 + [[ -f "$conf" && -n "$gpu" ]] || return 1 + [[ "$gpu" == 0000:* ]] || gpu="0000:${gpu}" + gpu=$(printf '%s' "$gpu" | tr 'A-F' 'a-f') + local slot="${gpu%.*}" sysfs="${PMX_PCI_SYSFS:-/sys/bus/pci/devices}" + local line idx value item bdf class member kind has_gpu=0 + local -a mine=() + while IFS= read -r line; do + [[ "$line" == \[* ]] && break + [[ "$line" =~ ^hostpci([0-9]+):[[:space:]]*([^,[:space:]]+) ]] || continue + idx="${BASH_REMATCH[1]}" + value="${BASH_REMATCH[2]}" + kind="" + for item in ${value//;/ }; do + [[ "$item" =~ ^(0000:)?([0-9a-fA-F]{2}:[0-9a-fA-F]{2})(\.[0-7])?$ ]] || continue + bdf=$(printf '0000:%s%s' "${BASH_REMATCH[2]}" "${BASH_REMATCH[3]}" | tr 'A-F' 'a-f') + if [[ "$bdf" == "$gpu" || "$bdf" == "$slot" ]]; then + # The GPU, by its function or by its whole slot. + kind="gpu" + continue + fi + [[ -n "$kind" ]] && continue + if [[ "${bdf%.*}" == "$slot" ]]; then + class=$(cat "${sysfs}/${bdf}/class" 2>/dev/null) + [[ "${class:2:2}" == "04" ]] && kind="with" + fi + for member in "$@"; do + [[ "$member" == 0000:* ]] || member="0000:${member}" + [[ "$(printf '%s' "$member" | tr 'A-F' 'a-f')" == "$bdf" ]] && kind="with" + done + done + [[ "$kind" == "gpu" ]] && has_gpu=1 + [[ -n "$kind" ]] && mine+=("$idx") + done < "$conf" + (( has_gpu == 1 )) || return 0 + printf '%s\n' "${mine[@]}" +} + # Robust LXC stop for switch-mode / passthrough flows. # # A plain `pct stop` can hang indefinitely when: @@ -226,15 +279,18 @@ function _pci_next_hostpci_index() { echo "$idx" } +# Whether the VM has this device, by its function or by its whole slot. +# Another function at the same slot is a different device: on an AMD APU the +# USB controllers share the slot of the integrated graphics. function _pci_slot_assigned_to_vm() { local pci_full="$1" local vmid="$2" - local slot_base - slot_base="${pci_full#0000:}" - slot_base="${slot_base%.*}" + local bdf slot_base + bdf="${pci_full#0000:}" + slot_base="${bdf%.*}" qm config "$vmid" 2>/dev/null \ - | grep -qE "^hostpci[0-9]+:.*(0000:)?${slot_base}(\\.[0-7])?([,[:space:]]|$)" + | grep -qE "^hostpci[0-9]+:.*(0000:)?(${bdf}|${slot_base})([,;[:space:]]|$)" } function _pci_function_assigned_to_vm() { diff --git a/scripts/gpu_tpu/add_gpu_vm.sh b/scripts/gpu_tpu/add_gpu_vm.sh index 87a777e1..a1b44238 100644 --- a/scripts/gpu_tpu/add_gpu_vm.sh +++ b/scripts/gpu_tpu/add_gpu_vm.sh @@ -458,12 +458,14 @@ _is_pci_slot_assigned_to_vm() { local pci_full="$1" local vmid="$2" - local slot_base - slot_base="${pci_full#0000:}" - slot_base="${slot_base%.*}" # 01:00 + local bdf slot_base + bdf="${pci_full#0000:}" # 01:00.0 + slot_base="${bdf%.*}" # 01:00 + # The device itself or its whole slot; another function at the same slot + # is a different device. qm config "$vmid" 2>/dev/null \ - | grep -qE "^hostpci[0-9]+:.*(0000:)?${slot_base}(\\.[0-7])?([,[:space:]]|$)" + | grep -qE "^hostpci[0-9]+:.*(0000:)?(${bdf}|${slot_base})([,;[:space:]]|$)" } # Match a specific PCI function when possible. @@ -1453,7 +1455,9 @@ check_switch_mode() { local vmid vmid=$(basename "$conf" .conf) [[ "$vmid" == "$SELECTED_VMID" ]] && continue # same target VM, no conflict - if grep -qE "hostpci[0-9]+:.*${pci_slot}" "$conf"; then + # Another VM has the GPU when it has the function of the GPU itself; + # a neighbour at the same slot, as a USB controller, is not the GPU. + if [[ -n "$(_vm_gpu_hostpci_indexes "$conf" "$SELECTED_GPU_PCI" "${IOMMU_DEVICES[@]}")" ]]; then vm_src_id="$vmid" vm_src_name=$(grep "^name:" "$conf" 2>/dev/null | awk '{print $2}') break @@ -2008,12 +2012,14 @@ cleanup_vm_config() { local src_conf="/etc/pve/qemu-server/${SWITCH_VM_SRC}.conf" if [[ -f "$src_conf" ]]; then msg_info "$(translate 'Removing GPU from VM') ${SWITCH_VM_SRC}..." - # Precise regex: slot must be followed by "." and a - # delimiter. Kept in sync with switch_gpu_mode.sh. A looser - # ".*${pci_slot}" would match the slot as a substring and wipe - # unrelated hostpci entries (e.g. slot "00:02" matching inside - # a dGPU BDF 0000:02:00.0). - sed -E -i "/^hostpci[0-9]+:[[:space:]]*(0000:)?${pci_slot}\.[0-7]([,[:space:]]|$)/d" "$src_conf" + # Only the entries that carry the GPU are removed: its own function, + # the audio at its slot and the devices of its IOMMU group. Other + # functions at the same slot stay in the source VM. + local _gpu_idx + while IFS= read -r _gpu_idx; do + [[ -n "$_gpu_idx" ]] || continue + _vm_remove_hostpci_index "$SWITCH_VM_SRC" "$_gpu_idx" "$LOG_FILE" + done < <(_vm_gpu_hostpci_indexes "$src_conf" "$SELECTED_GPU_PCI" "${IOMMU_DEVICES[@]}") msg_ok "$(translate 'GPU removed from VM') ${SWITCH_VM_SRC}" | tee -a "$screen_capture" # Cascade cleanup: detect audio companions orphaned in the diff --git a/scripts/gpu_tpu/switch_gpu_mode.sh b/scripts/gpu_tpu/switch_gpu_mode.sh index 6f2d30af..30ba44d5 100644 --- a/scripts/gpu_tpu/switch_gpu_mode.sh +++ b/scripts/gpu_tpu/switch_gpu_mode.sh @@ -85,6 +85,7 @@ declare -a SELECTED_GPU_IDX=() declare -a SELECTED_IOMMU_IDS=() declare -a SELECTED_PCI_SLOTS=() +declare -a SELECTED_GPU_BDFS=() declare -a SELECTED_NVIDIA_BDFS=() declare -a SELECTED_LEGACY_IOMMU_IDS=() @@ -688,6 +689,7 @@ check_sriov_and_block_if_needed() { collect_selected_iommu_ids() { SELECTED_IOMMU_IDS=() SELECTED_PCI_SLOTS=() + SELECTED_GPU_BDFS=() SELECTED_NVIDIA_BDFS=() SELECTED_LEGACY_IOMMU_IDS=() @@ -699,6 +701,7 @@ collect_selected_iommu_ids() { slot="${pci#0000:}" slot="${slot%.*}" SELECTED_PCI_SLOTS+=("$slot") + SELECTED_GPU_BDFS+=("$pci") local -a group_bdfs=() mapfile -t group_bdfs < <(_get_iommu_group_bdfs "$pci") @@ -894,9 +897,12 @@ detect_affected_vms_for_selected() { local conf for conf in /etc/pve/qemu-server/*.conf; do [[ -f "$conf" ]] || continue - local matched=false slot - for slot in "${SELECTED_PCI_SLOTS[@]}"; do - if grep -qE "hostpci[0-9]+:.*(0000:)?${slot}(\\.[0-7])?([,[:space:]]|$)" "$conf"; then + # A VM has the GPU when it has the function of the GPU itself. Another + # function at the same slot, as a USB controller, is a device of its own. + local matched=false gpu + for gpu in "${SELECTED_GPU_BDFS[@]}"; do + # shellcheck disable=SC2046 + if [[ -n "$(_vm_gpu_hostpci_indexes "$conf" "$gpu" $(_get_iommu_group_bdfs "$gpu"))" ]]; then matched=true break fi @@ -978,19 +984,16 @@ apply_vm_action_for_lxc_mode() { fi if [[ "$VM_ACTION" == "remove_gpu_keep_onboot" && -f "$conf" ]]; then - # Primary cleanup: strip hostpci lines whose BDF matches any of - # the GPU's selected slots. Matches both the PF function (.0) and - # any sibling audio or HDMI codec that shares the slot (typical - # for discrete NVIDIA/AMD cards where .1 is the HDMI audio). - # - # Precise regex: the slot must be followed by "." and - # either a delimiter or end-of-line. A looser ".*${slot}" would - # match by pure substring and delete unrelated hostpci entries — - # e.g. slot "00:02" would match inside "0000:02:00.0" (a dGPU at - # 02:00) and wipe both the iGPU and the unrelated dGPU. - local slot - for slot in "${SELECTED_PCI_SLOTS[@]}"; do - sed -E -i "/^hostpci[0-9]+:[[:space:]]*(0000:)?${slot}\.[0-7]([,[:space:]]|$)/d" "$conf" + # Primary cleanup: remove the entries that carry the GPU — its own + # function, the audio at its slot and the devices of its IOMMU + # group. Other functions at the same slot stay in the VM. + local gpu _gpu_idx + for gpu in "${SELECTED_GPU_BDFS[@]}"; do + while IFS= read -r _gpu_idx; do + [[ -n "$_gpu_idx" ]] || continue + _vm_remove_hostpci_index "$vmid" "$_gpu_idx" "$LOG_FILE" + # shellcheck disable=SC2046 + done < <(_vm_gpu_hostpci_indexes "$conf" "$gpu" $(_get_iommu_group_bdfs "$gpu")) done msg_ok "$(translate 'GPU removed from VM config') ${vmid}" | tee -a "$screen_capture" diff --git a/scripts/gpu_tpu/switch_gpu_mode_direct.sh b/scripts/gpu_tpu/switch_gpu_mode_direct.sh index 1a98c0a3..b42b9764 100644 --- a/scripts/gpu_tpu/switch_gpu_mode_direct.sh +++ b/scripts/gpu_tpu/switch_gpu_mode_direct.sh @@ -58,6 +58,7 @@ declare -a SELECTED_GPU_IDX=() declare -a SELECTED_IOMMU_IDS=() declare -a SELECTED_PCI_SLOTS=() +declare -a SELECTED_GPU_BDFS=() declare -a SELECTED_NVIDIA_BDFS=() declare -a SELECTED_LEGACY_IOMMU_IDS=() @@ -510,6 +511,7 @@ validate_vm_mode_blocked_ids() { collect_selected_iommu_ids() { SELECTED_IOMMU_IDS=() SELECTED_PCI_SLOTS=() + SELECTED_GPU_BDFS=() SELECTED_NVIDIA_BDFS=() SELECTED_LEGACY_IOMMU_IDS=() @@ -521,6 +523,7 @@ collect_selected_iommu_ids() { slot="${pci#0000:}" slot="${slot%.*}" SELECTED_PCI_SLOTS+=("$slot") + SELECTED_GPU_BDFS+=("$pci") local -a group_bdfs=() mapfile -t group_bdfs < <(_get_iommu_group_bdfs "$pci") @@ -721,9 +724,12 @@ detect_affected_vms_for_selected() { local conf for conf in /etc/pve/qemu-server/*.conf; do [[ -f "$conf" ]] || continue - local matched=false slot - for slot in "${SELECTED_PCI_SLOTS[@]}"; do - if grep -qE "hostpci[0-9]+:.*(0000:)?${slot}(\\.[0-7])?([,[:space:]]|$)" "$conf"; then + # A VM has the GPU when it has the function of the GPU itself. Another + # function at the same slot, as a USB controller, is a device of its own. + local matched=false gpu + for gpu in "${SELECTED_GPU_BDFS[@]}"; do + # shellcheck disable=SC2046 + if [[ -n "$(_vm_gpu_hostpci_indexes "$conf" "$gpu" $(_get_iommu_group_bdfs "$gpu"))" ]]; then matched=true break fi @@ -806,17 +812,16 @@ apply_vm_action_for_lxc_mode() { fi if [[ "$VM_ACTION" == "remove_gpu_keep_onboot" && -f "$conf" ]]; then - # Primary cleanup: strip hostpci lines whose BDF matches any of - # the GPU's selected slots. Matches both the PF function (.0) and - # sibling audio/HDMI codecs (.1, typical for discrete cards). - # - # Precise regex: the slot must be followed by "." and a - # delimiter. Kept in sync with switch_gpu_mode.sh — a looser - # substring match would wipe unrelated hostpci entries (e.g. slot - # "00:02" matching as a substring inside a dGPU BDF 0000:02:00.0). - local slot - for slot in "${SELECTED_PCI_SLOTS[@]}"; do - sed -E -i "/^hostpci[0-9]+:[[:space:]]*(0000:)?${slot}\.[0-7]([,[:space:]]|$)/d" "$conf" + # Primary cleanup: remove the entries that carry the GPU — its own + # function, the audio at its slot and the devices of its IOMMU + # group. Other functions at the same slot stay in the VM. + local gpu _gpu_idx + for gpu in "${SELECTED_GPU_BDFS[@]}"; do + while IFS= read -r _gpu_idx; do + [[ -n "$_gpu_idx" ]] || continue + _vm_remove_hostpci_index "$vmid" "$_gpu_idx" "$LOG_FILE" + # shellcheck disable=SC2046 + done < <(_vm_gpu_hostpci_indexes "$conf" "$gpu" $(_get_iommu_group_bdfs "$gpu")) done msg_ok "$(translate 'GPU removed from VM config') ${vmid}" | tee -a "$screen_capture"