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

Author: marmarek

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,22 +139,20 @@ def _device_desc(hostdev_xml):
 class PCIDevice(qubes.device_protocol.DeviceInfo):
     # pylint: disable=too-few-public-methods
     regex = re.compile(
-        r"\A(?P<bus>[0-9a-f]+)_(?P<device>[0-9a-f]+)\."
-        r"(?P<function>[0-9a-f]+)\Z"
+        r"\A((?P<segment>[0-9a-f]{4})[_:])?(?P<bus>[0-9a-f]{2})[_:]"
+        r"(?P<device>[0-9a-f]{2})\.(?P<function>[0-9a-f])\Z"
     )
     _libvirt_regex = re.compile(
-        r"\Apci_0000_(?P<bus>[0-9a-f]+)_(?P<device>[0-9a-f]+)_"
-        r"(?P<function>[0-9a-f]+)\Z"
+        r"\Apci_(?P<segment>[0-9a-f]{4})_(?P<bus>[0-9a-f]{2})_"
+        r"(?P<device>[0-9a-f]{2})_(?P<function>[0-9a-f])\Z"
     )
 
     def __init__(self, port: Port, libvirt_name=None):
         if libvirt_name:
             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.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,31 +389,31 @@ 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,
             )
             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.sbdf),
             result,
             flags=re.MULTILINE,
         )

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 <https://www.gnu.org/licenses/>.
+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,55 @@ 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/bus/pci/devices"
+)
+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/bus/pci/devices"
+
+
+@mock.patch("qubes.utils.SYSFS_PCI_DEVS_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()

qubes/tests/vm/qubesvm.py

@@ -47,6 +47,14 @@
 import qubes.tests
 import qubes.tests.vm
 
+# prefer location in git checkout
+tests_sysfs_path = (
+    os.path.dirname(__file__) + "/../../../tests-data/sysfs/sys/bus/pci/devices"
+)
+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/bus/pci/devices"
+
 
 class TestApp(object):
     labels = {
@@ -928,6 +936,55 @@ def test_500_property_migrate_virt_mode(self):
         with self.assertRaises(AttributeError):
             vm.hvm
 
+    @unittest.mock.patch("qubes.utils.SYSFS_PCI_DEVS_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 = """
+        <domain class="QubesVM" id="domain-1">
+            <properties>
+                <property name="qid">1</property>
+                <property name="name">testvm</property>
+                <property name="label" ref="label-1" />
+                <property name="virt_mode">hvm</property>
+            </properties>
+            <devices class="pci">
+                <device backend-domain="dom0" id="02_00.2"/>
+            </devices>
+        </domain>
+        """
+        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 = """
+        <domain class="QubesVM" id="domain-1">
+            <properties>
+                <property name="qid">1</property>
+                <property name="name">testvm</property>
+                <property name="label" ref="label-1" />
+                <property name="virt_mode">hvm</property>
+            </properties>
+            <devices class="pci">
+                <device backend-domain="dom0" id="02_00.7"/>
+            </devices>
+        </domain>
+        """
+        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"""<domain type="xen">
@@ -1527,6 +1584,7 @@ def test_600_libvirt_xml_hvm_pcidev(self):
             <hostdev type="pci" managed="yes">
                 <source>
                     <address
+                        domain="0x0000"
                         bus="0x00"
                         slot="0x00"
                         function="0x0" />
@@ -1637,6 +1695,7 @@ def test_600_libvirt_xml_hvm_pcidev_s0ix(self):
                 <source
                     powerManagementFiltering="no">
                     <address
+                        domain="0x0000"
                         bus="0x00"
                         slot="0x00"
                         function="0x0" />