From b06451acb3b195ec694ab484384795a7214d0d0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 16 Mar 2025 02:44:20 +0100 Subject: [PATCH 1/6] ext/pci: get PCI class from libvirt xml This goes further in the direction of d2d2ad84 "pci: get device class out of libvirt xml". --- qubes/ext/pci.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/qubes/ext/pci.py b/qubes/ext/pci.py index d7e810c51..9769bcbe5 100644 --- a/qubes/ext/pci.py +++ b/qubes/ext/pci.py @@ -80,18 +80,9 @@ def load_pci_classes(): def pcidev_class(dev_xmldesc): - sysfs_path = dev_xmldesc.findtext("path") - assert sysfs_path - try: - with open(sysfs_path + "/class", encoding="ascii") as f_class: - class_id = f_class.read().strip() - except OSError: - return "unknown" - + class_id = pcidev_interface(dev_xmldesc) if not qubes.ext.pci.pci_classes: qubes.ext.pci.pci_classes = load_pci_classes() - if class_id.startswith("0x"): - class_id = class_id[2:] try: # ignore prog-if return qubes.ext.pci.pci_classes[class_id[0:4]] From c02059889ae8de3b0f4c7b943520ab47b561a18a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 16 Mar 2025 15:36:06 +0100 Subject: [PATCH 2/6] tests: fix invalid device BDF PCI device function is just one hex digit. --- doc/example.xml | 2 +- qubes/tests/vm/init.py | 4 ++-- relaxng/qubes.rng | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/example.xml b/doc/example.xml index c33c232cf..83711910e 100644 --- a/doc/example.xml +++ b/doc/example.xml @@ -22,7 +22,7 @@ - + diff --git a/qubes/tests/vm/init.py b/qubes/tests/vm/init.py index 3ad3fb4ff..158f37316 100644 --- a/qubes/tests/vm/init.py +++ b/qubes/tests/vm/init.py @@ -84,7 +84,7 @@ def setUp(self): - + @@ -125,7 +125,7 @@ def test_000_load(self): self.assertTrue( list(vm.devices["pci"].get_assigned_devices())[0].matches( - qubes.ext.pci.PCIDevice(Port(vm, "00_11.22", "pci")) + qubes.ext.pci.PCIDevice(Port(vm, "00_11.2", "pci")) ) ) diff --git a/relaxng/qubes.rng b/relaxng/qubes.rng index edf54b8a2..55da203fd 100644 --- a/relaxng/qubes.rng +++ b/relaxng/qubes.rng @@ -219,7 +219,7 @@ the parser will complain about missing combine= attribute on the second . - [0-9a-f]{2}_[0-9a-f]{2}.[0-9a-f]{2} + [0-9a-f]{2}_[0-9a-f]{2}.[0-9a-f]{1} From f8ffb897c99cc529046ffd11a088d90427a6e412 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 16 Mar 2025 16:56:18 +0100 Subject: [PATCH 3/6] tests: adjust test for unsupported device The issue was about device with segment part exceeding 16 bits, which technically is not spec compliant. Adjust the test to be about a device that was problematic initially. QubesOS/qubes-issues#6932 --- qubes/tests/devices_pci.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qubes/tests/devices_pci.py b/qubes/tests/devices_pci.py index 0d8be0f47..4311e948e 100644 --- a/qubes/tests/devices_pci.py +++ b/qubes/tests/devices_pci.py @@ -143,7 +143,7 @@ def test_000_unsupported_device(self): mock.Mock( **{ "XMLDesc.return_value": PCI_XML.format( - *["1000"] * 3 + *["10000"] * 3 ), "listCaps.return_value": ["pci"], } From a1778ad96bdfbee9f1ac5b53b33868e33689bbf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 17 Mar 2025 02:19:41 +0100 Subject: [PATCH 4/6] Make black happy about qubes/utils.py --- qubes/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qubes/utils.py b/qubes/utils.py index d1f5e3645..ff4dfcdd7 100644 --- a/qubes/utils.py +++ b/qubes/utils.py @@ -236,7 +236,7 @@ def replace_file( permissions, close_on_success=True, logger=LOGGER, - log_level=logging.DEBUG + log_level=logging.DEBUG, ): """Yield a tempfile whose name starts with dst. If the block does not raise an exception, apply permissions and persist the From d0ce8a627e3672fce35ec1b5973d04f1dece8388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 16 Mar 2025 19:57:07 +0100 Subject: [PATCH 5/6] ext/pci: implement PCI path for device identification PCI SBDF may change if _another_ device is added/removed from the system. Use PCI device path to avoid this issue. The PCI device path is more or less sysfs path, but with bus number behind bridges replaced with relative bus number to bridge's secondary bus number. Each component of the path is separated with '-', as that is allowed in qrexec arguments. For example device 0000:c1:00.0 behind bridge at 0000:00:08.1 will get path 0000_00_08.1-00_00.0 (assuming bridge 0000:00:08.1 has secondary bus at 0xc1). Resolving device path requires access to the system's sysfs. As a side effect, this adds also support for multi-segment systems, PCI segment is no longer hardcoded to 0000. PCIDevice object will automatically resolve old device identifiers, which partially covers migration path. The other part is converting BDF to device path when loading device assignments. Tests include subset of sysfs from a real system that has multiple bridges. TODO: - consider still allowing attaching via SBDF, or otherwise expose function to translate SBDF to PCI path (an Admin API call?) - fix salt listing/attaching devices to sys-net/sys-usb - depending on the above Fixes QubesOS/qubes-issues#8681 --- qubes/ext/pci.py | 47 +++--- qubes/tests/devices_pci.py | 49 +++++- qubes/tests/vm/qubesvm.py | 57 +++++++ qubes/utils.py | 140 ++++++++++++++++++ qubes/vm/__init__.py | 11 +- rpm_spec/core-dom0.spec.in | 1 + templates/libvirt/devices/pci.xml | 1 + .../sysfs/sys/bus/pci/devices/0000:00:08.1 | 1 + .../sysfs/sys/bus/pci/devices/0000:00:18.0 | 1 + .../sysfs/sys/bus/pci/devices/0000:00:18.1 | 1 + .../sysfs/sys/bus/pci/devices/0000:00:18.2 | 1 + .../sysfs/sys/bus/pci/devices/0000:00:18.3 | 1 + .../sysfs/sys/bus/pci/devices/0000:00:18.4 | 1 + .../sysfs/sys/bus/pci/devices/0000:02:00.0 | 1 + .../sysfs/sys/bus/pci/devices/0000:02:00.2 | 1 + .../sysfs/sys/bus/pci/devices/0000:c0:03.5 | 1 + .../sysfs/sys/bus/pci/devices/0000:c5:00.0 | 1 + .../sysfs/sys/bus/pci/devices/0000:c6:00.0 | 1 + .../0000:00:08.1/secondary_bus_number | 1 + .../0000:00:08.1/subordinate_bus_number | 1 + .../0000:c5:00.0/secondary_bus_number | 1 + .../0000:c5:00.0/subordinate_bus_number | 1 + .../0000:c0:03.5/secondary_bus_number | 1 + .../0000:c0:03.5/subordinate_bus_number | 1 + 24 files changed, 299 insertions(+), 24 deletions(-) create mode 120000 tests-data/sysfs/sys/bus/pci/devices/0000:00:08.1 create mode 120000 tests-data/sysfs/sys/bus/pci/devices/0000:00:18.0 create mode 120000 tests-data/sysfs/sys/bus/pci/devices/0000:00:18.1 create mode 120000 tests-data/sysfs/sys/bus/pci/devices/0000:00:18.2 create mode 120000 tests-data/sysfs/sys/bus/pci/devices/0000:00:18.3 create mode 120000 tests-data/sysfs/sys/bus/pci/devices/0000:00:18.4 create mode 120000 tests-data/sysfs/sys/bus/pci/devices/0000:02:00.0 create mode 120000 tests-data/sysfs/sys/bus/pci/devices/0000:02:00.2 create mode 120000 tests-data/sysfs/sys/bus/pci/devices/0000:c0:03.5 create mode 120000 tests-data/sysfs/sys/bus/pci/devices/0000:c5:00.0 create mode 120000 tests-data/sysfs/sys/bus/pci/devices/0000:c6:00.0 create mode 100644 tests-data/sysfs/sys/devices/pci0000:00/0000:00:08.1/secondary_bus_number create mode 100644 tests-data/sysfs/sys/devices/pci0000:00/0000:00:08.1/subordinate_bus_number create mode 100644 tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/0000:c5:00.0/secondary_bus_number create mode 100644 tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/0000:c5:00.0/subordinate_bus_number create mode 100644 tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/secondary_bus_number create mode 100644 tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/subordinate_bus_number diff --git a/qubes/ext/pci.py b/qubes/ext/pci.py index 9769bcbe5..0571da337 100644 --- a/qubes/ext/pci.py +++ b/qubes/ext/pci.py @@ -35,6 +35,7 @@ import qubes.devices import qubes.ext from qubes.device_protocol import Port +from qubes.utils import sbdf_to_path, path_to_sbdf, is_pci_path #: cache of PCI device classes pci_classes = None @@ -138,12 +139,12 @@ def _device_desc(hostdev_xml): class PCIDevice(qubes.device_protocol.DeviceInfo): # pylint: disable=too-few-public-methods regex = re.compile( - r"\A(?P[0-9a-f]+)_(?P[0-9a-f]+)\." - r"(?P[0-9a-f]+)\Z" + r"\A((?P[0-9a-f]{4})[_:])?(?P[0-9a-f]{2})[_:]" + r"(?P[0-9a-f]{2})\.(?P[0-9a-f])\Z" ) _libvirt_regex = re.compile( - r"\Apci_0000_(?P[0-9a-f]+)_(?P[0-9a-f]+)_" - r"(?P[0-9a-f]+)\Z" + r"\Apci_(?P[0-9a-f]{4})_(?P[0-9a-f]{2})_" + r"(?P[0-9a-f]{2})_(?P[0-9a-f])\Z" ) def __init__(self, port: Port, libvirt_name=None): @@ -151,9 +152,7 @@ def __init__(self, port: Port, libvirt_name=None): dev_match = self._libvirt_regex.match(libvirt_name) if not dev_match: raise UnsupportedDevice(libvirt_name) - port_id = "{bus}_{device}.{function}".format( - **dev_match.groupdict() - ) + port_id = sbdf_to_path(libvirt_name) port = Port( backend_domain=port.backend_domain, port_id=port_id, @@ -162,14 +161,24 @@ def __init__(self, port: Port, libvirt_name=None): super().__init__(port) - dev_match = self.regex.match(port.port_id) + if is_pci_path(port.port_id): + sbdf = path_to_sbdf(port.port_id) + else: + sbdf = port.port_id + dev_match = self.regex.match(sbdf) if not dev_match: raise ValueError( - "Invalid device identifier: {!r}".format(port.port_id) + "Invalid device identifier: {!r} (sbdf: {!r})".format( + port.port_id, sbdf + ) ) + self.data["sbdf"] = sbdf + for group in self.regex.groupindex: setattr(self, group, dev_match.group(group)) + if getattr(self, "segment") is None: + self.segment = "0000" # lazy loading self._description: Optional[str] = None @@ -249,7 +258,7 @@ def parent_device(self) -> Optional[qubes.device_protocol.DeviceInfo]: def libvirt_name(self): # pylint: disable=no-member # noinspection PyUnresolvedReferences - return f"pci_0000_{self.bus}_{self.device}_{self.function}" + return f"pci_{self.segment}_{self.bus}_{self.device}_{self.function}" @property def description(self): @@ -380,11 +389,13 @@ def on_device_list_attached(self, vm, event, **kwargs): if hostdev.get("type") != "pci": continue address = hostdev.find("source/address") + segment = address.get("domain")[2:] bus = address.get("bus")[2:] device = address.get("slot")[2:] function = address.get("function")[2:] - port_id = "{bus}_{device}.{function}".format( + libvirt_name = "pci_{segment}_{bus}_{device}_{function}".format( + segment=segment, bus=bus, device=device, function=function, @@ -392,19 +403,17 @@ def on_device_list_attached(self, vm, event, **kwargs): yield PCIDevice( Port( backend_domain=vm.app.domains[0], - port_id=port_id, + port_id=None, devclass="pci", - ) + ), + libvirt_name=libvirt_name, ), {} @qubes.ext.handler("device-pre-attach:pci") def on_device_pre_attached_pci(self, vm, event, device, options): # pylint: disable=unused-argument - if not os.path.exists( - "/sys/bus/pci/devices/0000:{}".format( - device.port_id.replace("_", ":") - ) - ): + sbdf = path_to_sbdf(device.port_id) + if sbdf is None or not os.path.exists(f"/sys/bus/pci/devices/{sbdf}"): raise qubes.exc.QubesException( "Invalid PCI device: {}".format(device.port_id) ) @@ -458,7 +467,7 @@ def on_device_pre_detached_pci(self, vm, event, port): ) as p: result = p.communicate()[0].decode() m = re.search( - r"^(\d+.\d+)\s+0000:{}$".format(device.port_id.replace("_", ":")), + r"^(\d+.\d+)\s+{}$".format(device.data["sbdf"]), result, flags=re.MULTILINE, ) diff --git a/qubes/tests/devices_pci.py b/qubes/tests/devices_pci.py index 4311e948e..10704c4cd 100644 --- a/qubes/tests/devices_pci.py +++ b/qubes/tests/devices_pci.py @@ -17,11 +17,16 @@ # # You should have received a copy of the GNU Lesser General Public # License along with this library; if not, see . +import os.path +import unittest from unittest import mock import qubes.tests import qubes.ext.pci from qubes.device_protocol import DeviceInterface +from qubes.utils import sbdf_to_path, path_to_sbdf + +orig_open = open class TestVM(object): @@ -107,17 +112,53 @@ def mock_file_open(filename: str, *_args, **_kwargs): \t09 CANBUS \t80 Serial bus controller """ - elif filename.startswith("/sys/devices/pci"): - content = "0x0c0330" else: - raise OSError() + return orig_open(filename, *_args, **_kwargs) file_object = mock.mock_open(read_data=content).return_value file_object.__iter__.return_value = content return file_object -class TC_00_Block(qubes.tests.QubesTestCase): +# prefer location in git checkout +tests_sysfs_path = os.path.dirname(__file__) + "/../../tests-data/sysfs/sys" +if not os.path.exists(tests_sysfs_path): + # but if not there, look for package installed one + tests_sysfs_path = "/usr/share/qubes/tests-data/sysfs/sys" + + +@mock.patch("qubes.utils.SYSFS_BASE", tests_sysfs_path) +class TC_00_helpers(qubes.tests.QubesTestCase): + def test_000_sbdf_to_path1(self): + path = sbdf_to_path("0000:c6:00.0") + self.assertEqual(path, "c0_03.5-00_00.0-00_00.0") + + def test_001_sbdf_to_path2(self): + path = sbdf_to_path("0000:00:18.4") + self.assertEqual(path, "00_18.4") + + def test_002_sbdf_to_path_libvirt(self): + path = sbdf_to_path("pci_0000_00_18_4") + self.assertEqual(path, "00_18.4") + + def test_003_sbdf_to_path_default_segment1(self): + path = sbdf_to_path("00:18.4") + self.assertEqual(path, "00_18.4") + + def test_004_sbdf_to_path_default_segment2(self): + path = sbdf_to_path("0000:00:18.4") + self.assertEqual(path, "00_18.4") + + def test_010_path_to_sbdf1(self): + path = path_to_sbdf("0000_c0_03.5-00_00.0-00_00.0") + self.assertEqual(path, "0000:c6:00.0") + + def test_011_path_to_sbdf2(self): + path = path_to_sbdf("0000_00_18.4") + self.assertEqual(path, "0000:00:18.4") + + +class TC_10_PCI(qubes.tests.QubesTestCase): def setUp(self): super().setUp() self.ext = qubes.ext.pci.PCIDeviceExtension() diff --git a/qubes/tests/vm/qubesvm.py b/qubes/tests/vm/qubesvm.py index 09bc4f36f..eccd2e503 100644 --- a/qubes/tests/vm/qubesvm.py +++ b/qubes/tests/vm/qubesvm.py @@ -47,6 +47,12 @@ import qubes.tests import qubes.tests.vm +# prefer location in git checkout +tests_sysfs_path = os.path.dirname(__file__) + "/../../../tests-data/sysfs/sys" +if not os.path.exists(tests_sysfs_path): + # but if not there, look for package installed one + tests_sysfs_path = "/usr/share/qubes/tests-data/sysfs/sys" + class TestApp(object): labels = { @@ -928,6 +934,55 @@ def test_500_property_migrate_virt_mode(self): with self.assertRaises(AttributeError): vm.hvm + @unittest.mock.patch("qubes.utils.SYSFS_BASE", tests_sysfs_path) + def test_510_migrate_pci_assignments(self): + vm = qubes.vm.adminvm.AdminVM(self.app, None) + dom0 = self.get_vm(vm=vm) + xml_template = """ + + + 1 + testvm + + hvm + + + + + + """ + xml = lxml.etree.XML(xml_template) + vm = qubes.vm.qubesvm.QubesVM(self.app, xml) + vm.load_properties() + vm.load_extras() + dev_ass = list(vm.devices["pci"].get_assigned_devices()) + self.assertEqual(len(dev_ass), 1) + self.assertEqual(dev_ass[0].port_id, "00_08.1-00_00.2") + + def test_511_migrate_pci_assignments_non_existing(self): + vm = qubes.vm.adminvm.AdminVM(self.app, None) + dom0 = self.get_vm(vm=vm) + xml_template = """ + + + 1 + testvm + + hvm + + + + + + """ + xml = lxml.etree.XML(xml_template) + vm = qubes.vm.qubesvm.QubesVM(self.app, xml) + vm.load_properties() + vm.load_extras() + dev_ass = list(vm.devices["pci"].get_assigned_devices()) + self.assertEqual(len(dev_ass), 1) + self.assertEqual(dev_ass[0].port_id, "02_00.7") + def test_600_libvirt_xml_pv(self): my_uuid = "7db78950-c467-4863-94d1-af59806384ea" expected = f""" @@ -1527,6 +1582,7 @@ def test_600_libvirt_xml_hvm_pcidev(self):
@@ -1637,6 +1693,7 @@ def test_600_libvirt_xml_hvm_pcidev_s0ix(self):
diff --git a/qubes/utils.py b/qubes/utils.py index ff4dfcdd7..1ddc53798 100644 --- a/qubes/utils.py +++ b/qubes/utils.py @@ -24,6 +24,7 @@ import hashlib import logging import random +import re import string import os import os.path @@ -353,3 +354,142 @@ def sanitize_stderr_for_log(untrusted_stderr: bytes) -> str: b if b in allowed_bytes else b"_"[0] for b in untrusted_stderr ) return stderr.decode("ascii") + + +SYSFS_BASE = "/sys" + + +def sbdf_to_path(device_id: str): + """ + Lookup full path for a given device + + :param device_id: sbdf, for example 0000:02:03.0; accepts also libvirt + format like 0000_02_03_0 + :return: converted identifier of None if device is not found + """ + regex = re.compile( + r"\A(?:pci_)?((?P[0-9a-f]{4})[_:])?(?P[0-9a-f]{2})[_:]" + r"(?P[0-9a-f]{2})[._](?P[0-9a-f])\Z" + ) + sysfs_pci_devs_base = f"{SYSFS_BASE}/bus/pci/devices" + + dev_match = regex.match(device_id) + if not dev_match: + raise ValueError("Invalid device identifier: {!r}".format(device_id)) + if dev_match["segment"] is not None: + segment = dev_match["segment"] + else: + segment = "0000" + if dev_match["bus"] == "00": + return (f"{segment}_" if segment != "0000" else "") + ( + f"{dev_match['bus']}_" + f"{dev_match['device']}.{dev_match['function']}" + ) + sbdf = ( + f"{segment}:{dev_match['bus']}:" + f"{dev_match['device']}.{dev_match['function']}" + ) + try: + sysfs_path = os.readlink(f"{sysfs_pci_devs_base}/{sbdf}") + except FileNotFoundError: + return None + # example: ../../../devices/pci0000:00/0000:00:1a.0/0000:02:00.0 + rel_links, _, path = sysfs_path.partition(f"/pci{segment}:") + assert os.path.normpath( + os.path.join(sysfs_pci_devs_base, rel_links) + ) == os.path.normpath(f"{SYSFS_BASE}/devices") + # drop also 00/ part, which may be also 40/, 80/ etc + path = path[3:] + bus_offset = 0 + result_list = [] + for path_part in path.split("/"): + assert bus_offset != -1 + bridge_match = regex.match(path_part) + if not bridge_match: + raise ValueError("Invalid bridge found: {!r}".format(path_part)) + assert int(bridge_match["bus"], 16) >= bus_offset + bus_num = int(bridge_match["bus"], 16) - bus_offset + bridge_str = ( + f"{bus_num:02x}_{bridge_match['device']}." + f"{bridge_match['function']}" + ) + result_list.append(bridge_str) + try: + with open( + f"{sysfs_pci_devs_base}/{path_part}/secondary_bus_number", + encoding="ascii", + ) as f_bus_num: + # this one is in decimal + # this can raise ValueError, propagate it + bus_offset = int(f_bus_num.read()) + except FileNotFoundError: + # last device in chain + bus_offset = -1 + + if segment == "0000": + return "-".join(result_list) + return segment + "_" + "-".join(result_list) + + +def path_to_sbdf(path: str): + """ + Convert device path as done by *sbdf_to_path* back to SBDF + :param path: + :return: + """ + + regex = re.compile( + r"\A(?P[0-9a-f]+)[_:]" + r"(?P[0-9a-f]+)[._](?P[0-9a-f]+)\Z" + ) + segment_re = re.compile(r"\A(?P[0-9a-f]{4})[_:](?P.*)\Z") + + # default segment + segment = "0000" + bus_offset = 0 + current_dev = "" + for path_part in path.split("-"): + assert bus_offset != -1 + # first part may include segment + if bus_offset == 0: + segment_match = segment_re.match(path_part) + if segment_match: + segment = segment_match["segment"] + path_part = segment_match["rest"] + part_match = regex.match(path_part) + if not part_match: + raise ValueError( + "Invalid PCI device path at {!r}".format(path_part) + ) + bus_num = int(part_match["bus"], 16) + bus_offset + current_dev = ( + f"{segment}:{bus_num:02x}:{part_match['device']}." + f"{part_match['function']}" + ) + try: + with open( + f"{SYSFS_BASE}/bus/pci/devices/" + f"{current_dev}/secondary_bus_number", + encoding="ascii", + ) as f_bus_num: + # this one is in decimal + # this can raise ValueError, propagate it + bus_offset = int(f_bus_num.read()) + except FileNotFoundError: + # last device in chain + bus_offset = -1 + + return current_dev + + +def is_pci_path(device_id: str): + """Check if given device id is already a device path. + + :param device_id: device id to check + :return: + """ + path_re = re.compile( + r"\A([0-9a-f]{4}_)?00_[0-9a-f]{2}\.[0-9a-f]" + r"(-[0-9a-f]{2}_[0-9a-f]{2}\.[0-9a-f])*\Z" + ) + return bool(path_re.match(device_id)) diff --git a/qubes/vm/__init__.py b/qubes/vm/__init__.py index 840944ea8..20e6d0507 100644 --- a/qubes/vm/__init__.py +++ b/qubes/vm/__init__.py @@ -37,6 +37,7 @@ import qubes.events import qubes.features import qubes.log +from qubes.utils import is_pci_path, sbdf_to_path VM_ENTRY_POINT = "qubes.vm" @@ -350,11 +351,19 @@ def load_extras(self): backend = ( self.app.domains[backend_name] if backend_name else None ) + port_id = node.get("id", "*") + # migration from plain BDF to PCI path + if ( + devclass == "pci" + and port_id != "*" + and not is_pci_path(port_id) + ): + port_id = sbdf_to_path(port_id) or port_id device_assignment = qubes.device_protocol.DeviceAssignment( qubes.device_protocol.VirtualDevice( qubes.device_protocol.Port( backend_domain=backend, - port_id=node.get("id", "*"), + port_id=port_id, devclass=devclass, ), device_id=identity, diff --git a/rpm_spec/core-dom0.spec.in b/rpm_spec/core-dom0.spec.in index 6453f00be..f2949c594 100644 --- a/rpm_spec/core-dom0.spec.in +++ b/rpm_spec/core-dom0.spec.in @@ -568,6 +568,7 @@ done /usr/share/qubes/templates/libvirt/devices/pci.xml /usr/share/qubes/templates/libvirt/devices/net.xml /usr/share/qubes/tests-data/dispvm-open-thunderbird-attachment +/usr/share/qubes/tests-data/sysfs /usr/lib/tmpfiles.d/qubes.conf %attr(0755,root,root) /usr/lib/qubes/create-snapshot %attr(0755,root,root) /usr/lib/qubes/destroy-snapshot diff --git a/templates/libvirt/devices/pci.xml b/templates/libvirt/devices/pci.xml index 90d0c45c2..160ec193d 100644 --- a/templates/libvirt/devices/pci.xml +++ b/templates/libvirt/devices/pci.xml @@ -12,6 +12,7 @@ {% endif %} >
diff --git a/tests-data/sysfs/sys/bus/pci/devices/0000:00:08.1 b/tests-data/sysfs/sys/bus/pci/devices/0000:00:08.1 new file mode 120000 index 000000000..b3c2d2213 --- /dev/null +++ b/tests-data/sysfs/sys/bus/pci/devices/0000:00:08.1 @@ -0,0 +1 @@ +../../../devices/pci0000:00/0000:00:08.1 \ No newline at end of file diff --git a/tests-data/sysfs/sys/bus/pci/devices/0000:00:18.0 b/tests-data/sysfs/sys/bus/pci/devices/0000:00:18.0 new file mode 120000 index 000000000..8979233d2 --- /dev/null +++ b/tests-data/sysfs/sys/bus/pci/devices/0000:00:18.0 @@ -0,0 +1 @@ +../../../devices/pci0000:00/0000:00:18.0 \ No newline at end of file diff --git a/tests-data/sysfs/sys/bus/pci/devices/0000:00:18.1 b/tests-data/sysfs/sys/bus/pci/devices/0000:00:18.1 new file mode 120000 index 000000000..765e021a5 --- /dev/null +++ b/tests-data/sysfs/sys/bus/pci/devices/0000:00:18.1 @@ -0,0 +1 @@ +../../../devices/pci0000:00/0000:00:18.1 \ No newline at end of file diff --git a/tests-data/sysfs/sys/bus/pci/devices/0000:00:18.2 b/tests-data/sysfs/sys/bus/pci/devices/0000:00:18.2 new file mode 120000 index 000000000..5485bc7f4 --- /dev/null +++ b/tests-data/sysfs/sys/bus/pci/devices/0000:00:18.2 @@ -0,0 +1 @@ +../../../devices/pci0000:00/0000:00:18.2 \ No newline at end of file diff --git a/tests-data/sysfs/sys/bus/pci/devices/0000:00:18.3 b/tests-data/sysfs/sys/bus/pci/devices/0000:00:18.3 new file mode 120000 index 000000000..01573b1d4 --- /dev/null +++ b/tests-data/sysfs/sys/bus/pci/devices/0000:00:18.3 @@ -0,0 +1 @@ +../../../devices/pci0000:00/0000:00:18.3 \ No newline at end of file diff --git a/tests-data/sysfs/sys/bus/pci/devices/0000:00:18.4 b/tests-data/sysfs/sys/bus/pci/devices/0000:00:18.4 new file mode 120000 index 000000000..c9c4134a7 --- /dev/null +++ b/tests-data/sysfs/sys/bus/pci/devices/0000:00:18.4 @@ -0,0 +1 @@ +../../../devices/pci0000:00/0000:00:18.4 \ No newline at end of file diff --git a/tests-data/sysfs/sys/bus/pci/devices/0000:02:00.0 b/tests-data/sysfs/sys/bus/pci/devices/0000:02:00.0 new file mode 120000 index 000000000..ae75cda7b --- /dev/null +++ b/tests-data/sysfs/sys/bus/pci/devices/0000:02:00.0 @@ -0,0 +1 @@ +../../../devices/pci0000:00/0000:00:08.1/0000:02:00.0 \ No newline at end of file diff --git a/tests-data/sysfs/sys/bus/pci/devices/0000:02:00.2 b/tests-data/sysfs/sys/bus/pci/devices/0000:02:00.2 new file mode 120000 index 000000000..cdea98dd2 --- /dev/null +++ b/tests-data/sysfs/sys/bus/pci/devices/0000:02:00.2 @@ -0,0 +1 @@ +../../../devices/pci0000:00/0000:00:08.1/0000:02:00.2 \ No newline at end of file diff --git a/tests-data/sysfs/sys/bus/pci/devices/0000:c0:03.5 b/tests-data/sysfs/sys/bus/pci/devices/0000:c0:03.5 new file mode 120000 index 000000000..179c98bef --- /dev/null +++ b/tests-data/sysfs/sys/bus/pci/devices/0000:c0:03.5 @@ -0,0 +1 @@ +../../../devices/pci0000:c0/0000:c0:03.5 \ No newline at end of file diff --git a/tests-data/sysfs/sys/bus/pci/devices/0000:c5:00.0 b/tests-data/sysfs/sys/bus/pci/devices/0000:c5:00.0 new file mode 120000 index 000000000..bcc4bedc1 --- /dev/null +++ b/tests-data/sysfs/sys/bus/pci/devices/0000:c5:00.0 @@ -0,0 +1 @@ +../../../devices/pci0000:c0/0000:c0:03.5/0000:c5:00.0 \ No newline at end of file diff --git a/tests-data/sysfs/sys/bus/pci/devices/0000:c6:00.0 b/tests-data/sysfs/sys/bus/pci/devices/0000:c6:00.0 new file mode 120000 index 000000000..90ee17a60 --- /dev/null +++ b/tests-data/sysfs/sys/bus/pci/devices/0000:c6:00.0 @@ -0,0 +1 @@ +../../../devices/pci0000:c0/0000:c0:03.5/0000:c5:00.0/0000:c6:00.0 \ No newline at end of file diff --git a/tests-data/sysfs/sys/devices/pci0000:00/0000:00:08.1/secondary_bus_number b/tests-data/sysfs/sys/devices/pci0000:00/0000:00:08.1/secondary_bus_number new file mode 100644 index 000000000..0cfbf0888 --- /dev/null +++ b/tests-data/sysfs/sys/devices/pci0000:00/0000:00:08.1/secondary_bus_number @@ -0,0 +1 @@ +2 diff --git a/tests-data/sysfs/sys/devices/pci0000:00/0000:00:08.1/subordinate_bus_number b/tests-data/sysfs/sys/devices/pci0000:00/0000:00:08.1/subordinate_bus_number new file mode 100644 index 000000000..0cfbf0888 --- /dev/null +++ b/tests-data/sysfs/sys/devices/pci0000:00/0000:00:08.1/subordinate_bus_number @@ -0,0 +1 @@ +2 diff --git a/tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/0000:c5:00.0/secondary_bus_number b/tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/0000:c5:00.0/secondary_bus_number new file mode 100644 index 000000000..ca55a6c59 --- /dev/null +++ b/tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/0000:c5:00.0/secondary_bus_number @@ -0,0 +1 @@ +198 diff --git a/tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/0000:c5:00.0/subordinate_bus_number b/tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/0000:c5:00.0/subordinate_bus_number new file mode 100644 index 000000000..ca55a6c59 --- /dev/null +++ b/tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/0000:c5:00.0/subordinate_bus_number @@ -0,0 +1 @@ +198 diff --git a/tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/secondary_bus_number b/tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/secondary_bus_number new file mode 100644 index 000000000..538165229 --- /dev/null +++ b/tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/secondary_bus_number @@ -0,0 +1 @@ +197 diff --git a/tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/subordinate_bus_number b/tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/subordinate_bus_number new file mode 100644 index 000000000..ca55a6c59 --- /dev/null +++ b/tests-data/sysfs/sys/devices/pci0000:c0/0000:c0:03.5/subordinate_bus_number @@ -0,0 +1 @@ +198 From 20e3c20b3fe7c37a27f5344837b69afacf559145 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Wed, 16 Apr 2025 03:36:00 +0200 Subject: [PATCH 6/6] doc: document new PCI devices addressing QubesOS/qubes-issues#8681 --- doc/qubes-devices.rst | 69 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/doc/qubes-devices.rst b/doc/qubes-devices.rst index 9018fec8d..9c382945f 100644 --- a/doc/qubes-devices.rst +++ b/doc/qubes-devices.rst @@ -189,6 +189,75 @@ We must first create an assignment (`assign`) as required attached upon each VM startup. However, if a PCI device is currently in use by another VM, the startup of the second VM will fail. +PCI devices addressing +^^^^^^^^^^^^^^^^^^^^^^ + +Qubes identifies PCI devices using a PCI path, instead of just +segment/bus/device/function (aka SBDF). The latter is used by many tools (like +`lspci`) but it's not guaranteed to remain stable across (seemingly unrelated) +hardware or firmware changes. The PCI path is built by following PCIe bridges +from the root port down to the target device. The path is constructed as +follows: + +1. It starts with the root port address written as +`[_]_.` (the segment part can be omitted if +`0000`). All numbers written in hex, with `` (if present) padded to +4 digits, `` and `` padded to 2 digits and `` as 1 digit. +2. Subsequent bridges are added after a dash (`-`) in form of +`_.`, where `` is a bus number +relative to the parent bridge's secondary bus number. In a simple case where +the parent bridge has only one bus, the `` will be `00`. +3. Final device is added the same way as the bridges described in the second +step. + +The path uses `` instead of bridge's BUS number directly, to +remain stable across adding/removing *other* bridges. + +For example, given the following PCI devices layout: + +.. code-block:: + + # lspci -t + -[0000:00]-+-00.0 + +-00.2 + +-01.0 + +-02.0 + +-02.2-[01]----00.0 + +-02.4-[02]----00.0 + +-03.0 + +-03.1-[03-61]-- + +-04.0 + +-04.1-[62-c0]-- + +-08.0 + +-08.1-[c1]--+-00.0 + | +-00.1 + | +-00.2 + | +-00.3 + | +-00.4 + | +-00.5 + | \-00.6 + ... + + +The device 0000:c1:00.0 is behind bridge at 0000:00:08.1. In some cases there +may be more bridges. There may be also more devices behind a single bridge - in +the example above 0000:c1:00.0 is just a single multi-function device, but +bridge 0000:00:03.1 can have multiple devices (covering buses `03` up to `61`). +You can get bus ranges handled by a given bridge by looking at "secondary" +and "subordinate" attributes on lspci (and similarly in sysfs): + +.. code-block:: + + 00:08.1 PCI bridge: Advanced Micro Devices, Inc. [AMD] Phoenix Internal GPP Bridge to Bus [C:A] (prog-if 00 [Normal decode]) + Subsystem: Device 0006:f111 + Flags: bus master, fast devsel, latency 0, IRQ 106 + Bus: primary=00, secondary=c1, subordinate=c1, sec-latency=0 + ... + +The path for the 0000:c1:00.0 device is: `0000_00_08.1-00_00.0`, where the -00 +part means "the first bus behind this bridge". Since the segment is `0000`, +this path can be written also as `00_08.1-00_00.0`. + Microphone ----------