On 2/6/26 8:33 AM, Daniel P. Berrangé wrote:
On Thu, Feb 05, 2026 at 11:18:10PM +0100, Marek Marczykowski-Górecki wrote:
On Wed, Feb 04, 2026 at 09:02:47AM +0000, Daniel P. Berrangé via Devel wrote:
In addition to that, a thread in 2017 (this mailing list) seems to indicate
that this part of the code may not be necessary:
https://lists.libvirt.org/archives/list/[email protected]/thread/QRI4L5YFGBAV7NCTJ5OOJ7GBYGHQ7WZQ/#K5WRR6WTAUEPRQBWJVJSMX554DTYFTYH.
My question: can someone that knows this part of the code have a look to see
what is the best way to correct it: either propose a patch to
`virPCIDeviceIsBehindSwitchLackingACS` that is more generic or to change the
rest of the code so it's no more used?
....That thread certainly suggests we have a large set of code that can
be purged. Libvirt only needs to support the 2 most recent releases of
each distro, and since the kerenl removed legacy device assignment 8
years ago I see no reason for us to keep this.
IIUC that thread is talking about KVM only.
Yep. Since all of my usage of libvirt is with the QEMU/KVM driver, my
posts tend to speak in a "KVM-centric" manner (as if QEMU/KVM is the
center of the libvirt universe and the rest doesn't exist :-)). But
definitely what I was talking about was that the QEMU/KVM driver only
uses VFIO now. I do think that somebody (could have been me, or could
have been someone else, I have no memory) already went through and
cleaned out the KVM-specific non-VFIO code several years ago (sometime
after my message that you referenced).
Here we have Xen. I'm not
sure what "legacy assignment" is in the KVM world, but I don't think
there were any changes in this API on Xen side in the last decade or so.
Theoretically, checks like this should be done by Xen (either libxl, or
the hypervisor itself). But in practice, libvirt does it more
comprehensively and provides better error messages (when it works).
Yep, I completely overlooked that this was using Xen, and Xen does not
support VFIO like KVM does. So we do indeed need to retain this logic,
and fix any bugs.
I took a look through that code and saw that it's using a very
inefficient method of traversing the PCI topology to visit all the
parents of the device being assigned and have an idea for simplifying it
that I want to try out (unless someone else beats me to it) - basically
instead of iterating through every singe PCI device each time we look
for the parent of the current device, we can just resolve the symlink
for the device directory, e.g.
/sys/bus/pci/devices/0000:08:00.3
which will give us something like:
/sys/devices/pci0000:00/0000:00:01.2/0000:02:00.0/0000:03:08.0/0000:08:00.3
(an example from one of my machines. Sorry for any line breaks - stupid
email client :-/)
Once you have that path, you just break it down into the individual
components, and that is the entire tree of "ancestors" for the endpoint
device that need to be checked for ACS (and when you get to the
component that is "pci0000:XX", that means you're at the root complex
and so you're done.
If my ADHD brain hasn't forgotten it by the time I wake up tomorrow,
I'll try refactoring the code to do this (and then artificially
re-enable the ACS check for VFIO so I can actually test it :-).
