[PATCH] util: Fix secondary bus reset check for multi-root PCI systems
The virPCIDeviceReset() function was using a hardcoded check for bus != 0 to determine if a device is on a root bus before attempting a secondary bus reset. This breaks on multi-root systems like Intel Arrow Lake where devices can be on non-zero root buses (e.g., Bus 0x80 for the PCH root complex). Update the logic to use virPCIDeviceIsOnRootBus() which detects root bus attachment via sysfs topology, making it work correctly on any PCI-compliant system regardless of bus numbers. This ensures secondary bus reset is never attempted on ANY root bus (0x00, 0x80, etc.), only on devices behind PCI bridges where it's safe. Signed-off-by: Bruno Martins <ehanoc@protonmail.com> --- src/util/virpci.c | 106 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 99 insertions(+), 7 deletions(-) diff --git a/src/util/virpci.c b/src/util/virpci.c index 2348a98003..f2ce24e6aa 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1081,6 +1081,81 @@ virPCIDeviceInit(virPCIDevice *dev, int cfgfd) return 0; } +/* + * Check if a PCI device is directly attached to a root bus (no parent bridge). + * + * Modern systems like Intel Arrow Lake (Core Ultra 200) can have multiple + * root buses (e.g., bus 0x00 for CPU devices and bus 0x80 for PCH devices). + * We detect root bus attachment by examining the sysfs topology: a device + * is on a root bus if its sysfs path is a direct child of a pciXXXX:YY + * directory (e.g., /sys/devices/pci0000:80/0000:80:14.0). + * + * Returns 1 if on root bus, 0 if not, -1 on error. + */ +static int +virPCIDeviceIsOnRootBus(virPCIDevice *dev) +{ + unsigned int domain, bus; + g_autofree char *devPath = NULL; + g_autofree char *realPath = NULL; + g_autofree char *parentDir = NULL; + const char *parentName; + + /* Get the sysfs symlink path for this device */ + devPath = g_strdup_printf(PCI_SYSFS "devices/%s", dev->name); + + /* Resolve the symlink to get the real path in /sys/devices/... */ + if (virFileResolveLink(devPath, &realPath) < 0) { + VIR_DEBUG("%s %s: failed to resolve sysfs path %s", + dev->id, dev->name, devPath); + return -1; + } + + parentDir = g_path_get_dirname(realPath); + if (!parentDir) + return -1; + + parentName = strrchr(parentDir, '/'); + if (!parentName) + return -1; + parentName++; // skip the '/' + + /* + * Detect root bus by parsing the parent directory name using sscanf. + * + * PCI root buses in sysfs follow the naming convention: + * pci<domain>:<bus> + * + * - <domain>: number (0000-ffff, hex) + * - <bus>: number within the domain (00-ff, hex) + * + * This format is defined in: + * https://www.kernel.org/doc/html/latest/PCI/sysfs-pci.html + * + * Examples from Intel Arrow Lake systems: + * - pci0000:00 (CPU root bus) + * - pci0000:80 (PCH root bus - the problematic case on Arrow Lake) + * + * sscanf pattern "pci%x:%x" matches: + * - "pci" : literal prefix + * - "%x" : hex domain (stops at ':') + * - ":" : literal separator + * - "%x" : hex bus (must reach end of string) + * + * Returns 2 if both domain and bus were successfully parsed, + * confirming the device is a direct child of a root bus. + */ + if (sscanf(parentName, "pci%x:%x", &domain, &bus) == 2) { + VIR_DEBUG("%s %s: device is on root bus (parent=%s)", + dev->id, dev->name, parentName); + return 1; + } + + VIR_DEBUG("%s %s: device is not on root bus (parent=%s)", + dev->id, dev->name, parentName); + return 0; +} + int virPCIDeviceReset(virPCIDevice *dev, virPCIDeviceList *activeDevs, @@ -1146,9 +1221,17 @@ virPCIDeviceReset(virPCIDevice *dev, if (dev->has_pm_reset) ret = virPCIDeviceTryPowerManagementReset(dev, fd); - /* Bus reset is not an option with the root bus */ - if (ret < 0 && dev->address.bus != 0) - ret = virPCIDeviceTrySecondaryBusReset(dev, fd, inactiveDevs); + /* Bus reset is not an option with the root bus. On multi-root + * systems like Intel Arrow Lake, root buses can be non-zero (e.g., 0x80), + * so we use sysfs topology detection instead of checking bus == 0. + */ + if (ret < 0) { + int onRootBus = virPCIDeviceIsOnRootBus(dev); + if (onRootBus < 0) + goto cleanup; + if (!onRootBus) + ret = virPCIDeviceTrySecondaryBusReset(dev, fd, inactiveDevs); + } if (ret < 0) { virErrorPtr err = virGetLastError(); @@ -2531,15 +2614,24 @@ static int virPCIDeviceIsBehindSwitchLackingACS(virPCIDevice *dev) { g_autoptr(virPCIDevice) parent = NULL; + int onRootBus; if (virPCIDeviceGetParent(dev, &parent) < 0) return -1; if (!parent) { - /* if we have no parent, and this is the root bus, ACS doesn't come - * into play since devices on the root bus can't P2P without going - * through the root IOMMU. + /* If we have no parent bridge, check if the device is directly + * attached to a root bus. Systems like Intel Arrow Lake can have + * multiple root buses (e.g., 0x00 and 0x80), so we can't just + * check for bus == 0. Instead, use sysfs topology to detect + * root bus attachment. + * + * ACS doesn't come into play for devices on root buses since + * they can't P2P without going through the root IOMMU. */ - if (dev->address.bus == 0) { + onRootBus = virPCIDeviceIsOnRootBus(dev); + if (onRootBus < 0) + return -1; + if (onRootBus) { return 0; } else { virReportError(VIR_ERR_INTERNAL_ERROR, -- 2.47.3
participants (1)
-
ehanoc