From 04c166713654ddb4fc05bb30b35040c1b49bcd55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 23 Feb 2025 03:26:25 +0100 Subject: [PATCH 1/3] vm: automatically filter-out "nomodeset" kernel opt if GPU is attached The "nomodeset" option is a startup time optimization for the most common case, especially for HVM. Besides saving some drivers initialization for emulated VGA, it also prevents Xorg probing the device, plymouth trying to draw something there etc. But when real GPU is attached, it is harmful. Previously one needed set no-default-kernelopts feature, and then manually set all the options that were there (skipping nomodeset). Add an automatic filtering of the nomodeset option. And add an extra control via 'no-nomodeset' feature - to allow explicitly enabling or disabling this filtering too - regardless of attaching GPU. Fixes QubesOS/qubes-issues#9792 --- qubes/tests/vm/qubesvm.py | 81 +++++++++++++++++++++++++++++++++++++++ qubes/vm/qubesvm.py | 22 +++++++++-- 2 files changed, 100 insertions(+), 3 deletions(-) diff --git a/qubes/tests/vm/qubesvm.py b/qubes/tests/vm/qubesvm.py index d6a0fbc4d..53b27fee9 100644 --- a/qubes/tests/vm/qubesvm.py +++ b/qubes/tests/vm/qubesvm.py @@ -634,6 +634,87 @@ def test_262_kernelopts(self): ) self.assertPropertyValue(vm, "kernelopts", "", "", "") + @unittest.mock.patch.dict( + qubes.config.system_path, {"qubes_kernels_base_dir": "/tmp"} + ) + def test_263_kernelopts_common(self): + d = tempfile.mkdtemp(prefix="/tmp/") + self.addCleanup(shutil.rmtree, d) + open(d + "/vmlinuz", "w").close() + open(d + "/initramfs", "w").close() + with open(d + "/default-kernelopts-common.txt", "w") as f: + f.write("some default root=/dev/sda nomodeset other") + vm = self.get_vm() + vm.kernel = os.path.basename(d) + uuid_str = str(vm.uuid).replace("-", "") + self.assertPropertyDefaultValue( + vm, + "kernelopts_common", + f"systemd.machine_id={uuid_str} " + "some default root=/dev/sda nomodeset other", + ) + vm.features["no-nomodeset"] = "1" + vm.features["os"] = "non-Linux" + self.assertPropertyDefaultValue( + vm, "kernelopts_common", "some default root=/dev/sda other" + ) + + @unittest.mock.patch.dict( + qubes.config.system_path, {"qubes_kernels_base_dir": "/tmp"} + ) + def test_264_kernelopts_common_gpu(self): + d = tempfile.mkdtemp(prefix="/tmp/") + self.addCleanup(shutil.rmtree, d) + open(d + "/vmlinuz", "w").close() + open(d + "/initramfs", "w").close() + with open(d + "/default-kernelopts-common.txt", "w") as f: + f.write("some default root=/dev/sda nomodeset other") + # required for PCI devices listing + self.app.vmm.offline_mode = False + hostdev_details = unittest.mock.Mock( + **{ + "XMLDesc.return_value": """ + + pci_0000_00_02_0 + /sys/devices/pci0000:00/0000:00:02.0 + computer + + 0x030000 + 0 + 0 + 2 + 0 + Unknown + Intel Corporation + + """, + } + ) + self.app.vmm.libvirt_mock = unittest.mock.Mock( + **{"nodeDeviceLookupByName.return_value": hostdev_details} + ) + vm = self.get_vm() + assignment = qubes.device_protocol.DeviceAssignment( + qubes.device_protocol.VirtualDevice( + qubes.device_protocol.Port( + backend_domain=vm, # this is violation of API, + # but for PCI the argument is unused + port_id="00_02.0", + devclass="pci", + ) + ), + mode="required", + ) + vm.devices["pci"]._set.add(assignment) + vm.kernel = os.path.basename(d) + uuid_str = str(vm.uuid).replace("-", "") + self.assertPropertyDefaultValue( + vm, + "kernelopts_common", + f"systemd.machine_id={uuid_str} " + "some default root=/dev/sda other", + ) + def test_270_qrexec_timeout(self): vm = self.get_vm() self.assertPropertyDefaultValue(vm, "qrexec_timeout", 60) diff --git a/qubes/vm/qubesvm.py b/qubes/vm/qubesvm.py index 4375d7121..c9c159c78 100644 --- a/qubes/vm/qubesvm.py +++ b/qubes/vm/qubesvm.py @@ -45,7 +45,7 @@ import qubes.vm import qubes.vm.adminvm import qubes.vm.mix.net -from qubes.device_protocol import DeviceInterface +from qubes.device_protocol import DeviceInterface, DeviceCategory qmemman_present = False try: @@ -2593,9 +2593,25 @@ def kernelopts_common(self): ) if os.path.exists(kernelopts_path): with open(kernelopts_path, encoding="ascii") as f_kernelopts: - return base_kernelopts + f_kernelopts.read().rstrip("\n\r") + result = base_kernelopts + f_kernelopts.read().rstrip("\n\r") else: - return base_kernelopts + qubes.config.defaults["kernelopts_common"] + result = ( + base_kernelopts + qubes.config.defaults["kernelopts_common"] + ) + no_nomodeset = self.features.check_with_template("no-nomodeset", None) + if no_nomodeset is None: + # automatically skip nomodeset if a GPU is attached + for dev_ass in self.devices["pci"].get_assigned_devices(): + for intf in dev_ass.device.interfaces: + if intf.category == DeviceCategory.Display: + no_nomodeset = True + break + if no_nomodeset: + result = " ".join( + opt for opt in result.split(" ") if opt != "nomodeset" + ) + + return result # # helper methods From d2d2ad84e8718a6cb39e85996b2cb4c122236158 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 23 Feb 2025 03:33:58 +0100 Subject: [PATCH 2/3] pci: get device class out of libvirt xml The libvirt xml already includes device class, there is no need to access sysfs for that. --- qubes/ext/pci.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/qubes/ext/pci.py b/qubes/ext/pci.py index c3938c7e4..d7e810c51 100644 --- a/qubes/ext/pci.py +++ b/qubes/ext/pci.py @@ -100,14 +100,7 @@ def pcidev_class(dev_xmldesc): def pcidev_interface(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 "000000" - + class_id = dev_xmldesc.xpath("capability[@type='pci']/class/text()")[0] if class_id.startswith("0x"): class_id = class_id[2:] return class_id From 107cc6fee4f1d27193be1d1b97b05423cd78a482 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 23 Feb 2025 03:37:28 +0100 Subject: [PATCH 3/3] libvirt-xml: set guivm backend for stubdomain's gui When using a HVM with a GUI via stubdomain, that GUI agent should connect to the appropriate guivm. It is dom0 by default, so it works in a common case, but if somebody uses non-dom0 for GUI, then the parameter is missing. Passing through the parameter is already implemented in libvirt, libxl and QEMU. But setting it in the libvirt xml in the first place was missing. Fixes QubesOS/qubes-issues#9385 --- qubes/tests/vm/qubesvm.py | 67 +++++++++++++++++++++++++++++++++++++++ templates/libvirt/xen.xml | 10 +++++- 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/qubes/tests/vm/qubesvm.py b/qubes/tests/vm/qubesvm.py index 53b27fee9..42b7fec12 100644 --- a/qubes/tests/vm/qubesvm.py +++ b/qubes/tests/vm/qubesvm.py @@ -1050,6 +1050,73 @@ def test_600_libvirt_xml_hvm(self): lxml.etree.XML(libvirt_xml), lxml.etree.XML(expected) ) + def test_600_libvirt_xml_hvm_with_guivm(self): + my_uuid = "7db78950-c467-4863-94d1-af59806384ea" + expected = """ + test-inst-test + 7db78950-c467-4863-94d1-af59806384ea + 400 + 400 + 2 + + + + + + + + + hvm + + hvmloader + + + + + + + + + + + destroy + destroy + destroy + + + + + + + + + + + + """ + guivm = self.get_vm(name="guivm") + p = unittest.mock.patch.object(guivm, "is_running", lambda: True) + p.start() + self.addCleanup(p.stop) + vm = self.get_vm(uuid=my_uuid) + vm.netvm = None + vm.virt_mode = "hvm" + vm.guivm = guivm + vm.debug = True + libvirt_xml = vm.create_config_file() + self.assertXMLEqual( + lxml.etree.XML(libvirt_xml), lxml.etree.XML(expected) + ) + def test_600_libvirt_xml_hvm_dom0_kernel(self): my_uuid = "7db78950-c467-4863-94d1-af59806384ea" expected = f""" diff --git a/templates/libvirt/xen.xml b/templates/libvirt/xen.xml index 7d8974cdd..3d18136e5 100644 --- a/templates/libvirt/xen.xml +++ b/templates/libvirt/xen.xml @@ -224,7 +224,15 @@ {% if vm.features.check_with_template('linux-stubdom', True) %} {# TODO only add qubes gui if gui-agent is not installed in HVM #} - + {% endif %} {% endif %} {% endif %}