On Wed, Feb 25, 2026 at 10:35:37AM +0000, ehanoc via Devel wrote: > 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 <[email protected]>
Hi,
Any (re)views on this patch? Is there something to change to get it in?
> ---
> 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
>
>
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
signature.asc
Description: PGP signature
