Date: Fri, 07 Aug 2026 05:01:56 +0000 From: Kevin Bowling <kbowling@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 8e9fe9996a1f - main - pci: Reconcile MPS before attaching PCIe devices Message-ID: <6a7566c4.30ecd.3c4e1f93@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by kbowling: URL: https://cgit.FreeBSD.org/src/commit/?id=8e9fe9996a1fbdb79033b082e6a96b9e1266e33f commit 8e9fe9996a1fbdb79033b082e6a96b9e1266e33f Author: Kevin Bowling <kbowling@FreeBSD.org> AuthorDate: 2026-08-06 06:40:55 +0000 Commit: Kevin Bowling <kbowling@FreeBSD.org> CommitDate: 2026-08-07 05:01:33 +0000 pci: Reconcile MPS before attaching PCIe devices Reconcile each newly enumerated link as a unit before child drivers attach. Firmware may leave Bus Master Enable set after handoff, so use the bus attachment state rather than that bit to identify the cold phase. Preserve an established hierarchy during rescan and hot-add. Refuse a reduction below a switch because recursive enumeration may already have made a sibling subtree live; lowering only the local port or Root Port would produce an inconsistent path. Report capability and active-use conflicts distinctly. Handle OFW PCI buses that clone the generic enumeration path. MFC after: 2 weeks --- share/man/man4/pci.4 | 3 + sys/dev/pci/pci.c | 293 +++++++++++++++++++++++++++++++++++++++---- sys/dev/pci/pci_private.h | 2 + sys/dev/pci/pcivar.h | 4 +- sys/powerpc/ofw/ofw_pcibus.c | 1 + 5 files changed, 279 insertions(+), 24 deletions(-) diff --git a/share/man/man4/pci.4 b/share/man/man4/pci.4 index 71ca2f50ed90..4ff03740b2c5 100644 --- a/share/man/man4/pci.4 +++ b/share/man/man4/pci.4 @@ -593,6 +593,9 @@ MSI-X interrupts can be disabled by setting this tunable to 0. Configure PCI-express Maximum Payload Size .Pq MPS during device enumeration. +A reduction required below a PCI-express switch is reported but not applied, +because recursively attached sibling subtrees may already be active and the +complete shared ancestor hierarchy cannot be safely retuned. SR-IOV Virtual Functions are excluded because their MPS fields are reserved and the Physical Function setting applies to them. Configuration-space tools may therefore display a Virtual Function's diff --git a/sys/dev/pci/pci.c b/sys/dev/pci/pci.c index 95b8e831e743..dd771c2e8fd3 100644 --- a/sys/dev/pci/pci.c +++ b/sys/dev/pci/pci.c @@ -4297,6 +4297,7 @@ pci_add_children(device_t dev, int domain, int busno) for (f = first_func; f <= pcifunchigh; f++) pci_identify_function(pcib, dev, domain, busno, s, f); } + pcie_reconcile_link_mps(dev); #undef REG } @@ -4426,16 +4427,154 @@ pci_create_iov_child_method(device_t bus, device_t pf, uint16_t rid, } #endif +static int +pcie_mps_bytes(uint16_t mps) +{ + + return (128 << (mps >> 5)); +} + +/* Return the smallest configured MPS above dev, if the walk reaches a root. */ +static bool +pcie_path_mps(device_t dev, uint16_t *mpsp) +{ + struct pci_devinfo *dinfo; + device_t bus, pcib, start; + uint16_t mps; + bool found; + + start = dev; + found = false; + for (;;) { + bus = device_get_parent(dev); + if (bus == NULL) + break; + pcib = device_get_parent(bus); + if (pcib == NULL || !is_pci_device(pcib)) + break; + dinfo = device_get_ivars(pcib); + if (dinfo->cfg.pcie.pcie_location != 0) { + mps = pcie_read_config(pcib, PCIER_DEVICE_CTL, 2) & + PCIEM_CTL_MAX_PAYLOAD; + if (!found || mps < *mpsp) + *mpsp = mps; + found = true; + if (dinfo->cfg.pcie.pcie_type == PCIEM_TYPE_ROOT_PORT) + return (true); + } + dev = pcib; + } + if (found && bootverbose) + device_printf(start, + "PCIe MPS path walk did not reach a Root Port\n"); + return (false); +} + +static bool +pcie_mps_first_warning(device_t dev) +{ + struct pci_devinfo *dinfo; + + dinfo = device_get_ivars(dev); + if ((dinfo->cfg.flags & PCICFG_MPS_WARNED) != 0) + return (false); + dinfo->cfg.flags |= PCICFG_MPS_WARNED; + return (true); +} + +static void +pcie_mps_conflict(device_t dev, uint16_t path_mps, uint16_t max_mps) +{ + + if (!pcie_mps_first_warning(dev)) + return; + device_printf(dev, + "maximum supported MPS %d is below configured path MPS %d; " + "cannot safely retune the shared ancestor hierarchy\n", + pcie_mps_bytes(max_mps), pcie_mps_bytes(path_mps)); +} + +static void +pcie_mps_active_conflict(device_t dev, uint16_t path_mps, + uint16_t device_mps) +{ + + if (!pcie_mps_first_warning(dev)) + return; + device_printf(dev, + "configured MPS %d does not match path MPS %d while bus " + "mastering is enabled; leaving device unchanged\n", + pcie_mps_bytes(device_mps), pcie_mps_bytes(path_mps)); +} + +static void +pcie_mps_mark_unreconciled(device_t dev) +{ + struct pci_devinfo *dinfo; + + dinfo = device_get_ivars(dev); + dinfo->cfg.flags |= PCICFG_MPS_UNRECONCILED; +} + +static void +pcie_mps_unreconciled(device_t dev, uint16_t path_mps, uint16_t max_mps) +{ + + pcie_mps_conflict(dev, path_mps, max_mps); + pcie_mps_mark_unreconciled(dev); +} + +static void +pcie_mps_active_unreconciled(device_t dev, uint16_t path_mps, + uint16_t device_mps) +{ + + pcie_mps_active_conflict(dev, path_mps, device_mps); + pcie_mps_mark_unreconciled(dev); +} + +static void +pcie_mps_mark_link_unreconciled(device_t *devlist, int count, + uint16_t path_mps, + bool all) +{ + struct pci_devinfo *dinfo; + device_t child; + uint16_t mmps; + int i; + + for (i = 0; i < count; i++) { + child = devlist[i]; + dinfo = device_get_ivars(child); + if ((dinfo->cfg.flags & (PCICFG_VF | + PCICFG_MPS_UNRECONCILED)) != 0 || + dinfo->cfg.pcie.pcie_location == 0) + continue; + if (all) { + pcie_mps_mark_unreconciled(child); + continue; + } + mmps = (pcie_read_config(child, PCIER_DEVICE_CAP, 2) & + PCIEM_CAP_MAX_PAYLOAD) << 5; + if (mmps < path_mps) + pcie_mps_unreconciled(child, path_mps, mmps); + } +} + /* - * For PCIe device set Max_Payload_Size to match PCIe root's. + * Tune a function discovered by rescan or hot-add against the established + * path. Never change a shared upstream port here: doing so requires + * quiescing every driver and draining all outstanding transactions in the + * hierarchy. Cold enumeration is reconciled by pcie_reconcile_link_mps(). */ static void pcie_setup_mps(device_t dev) { - struct pci_devinfo *dinfo = device_get_ivars(dev); - device_t root; - uint16_t rmps, mmps, mps; + struct pci_devinfo *dinfo; + device_t bus; + uint16_t mmps, mps, path_mps; + dinfo = device_get_ivars(dev); /* * PCIe r4.0, sec 9.3.5.4 defines the VF MPS and MRRS fields as * Reserved and Preserved, with the PF settings applying to the VF. @@ -4445,31 +4584,139 @@ pcie_setup_mps(device_t dev) return; if (dinfo->cfg.pcie.pcie_location == 0) return; - root = pci_find_pcie_root_port(dev); - if (root == NULL) + + /* Cold enumeration is reconciled one complete link at a time. */ + bus = device_get_parent(dev); + if (!device_is_attached(bus)) return; - /* Check whether the MPS is already configured. */ - rmps = pcie_read_config(root, PCIER_DEVICE_CTL, 2) & - PCIEM_CTL_MAX_PAYLOAD; + path_mps = 0; + if (!pcie_path_mps(dev, &path_mps)) + return; + + mmps = (pcie_read_config(dev, PCIER_DEVICE_CAP, 2) & + PCIEM_CAP_MAX_PAYLOAD) << 5; + if (path_mps > mmps) { + pcie_mps_unreconciled(dev, path_mps, mmps); + return; + } mps = pcie_read_config(dev, PCIER_DEVICE_CTL, 2) & PCIEM_CTL_MAX_PAYLOAD; - if (mps == rmps) + if (mps == path_mps) return; - /* Check whether the device is capable of the root's MPS. */ - mmps = (pcie_read_config(dev, PCIER_DEVICE_CAP, 2) & + if ((pci_read_config(dev, PCIR_COMMAND, 2) & + PCIM_CMD_BUSMASTEREN) != 0) { + pcie_mps_active_unreconciled(dev, path_mps, mps); + return; + } + pcie_adjust_config(dev, PCIER_DEVICE_CTL, PCIEM_CTL_MAX_PAYLOAD, + path_mps, 2); +} + +/* + * Reconcile a newly enumerated link before attaching any child drivers. A + * Root Port may be lowered because its complete downstream hierarchy is still + * idle. A late reduction below a switch is not propagated through ancestors, + * since sibling subtrees may already be active. + */ +void +pcie_reconcile_link_mps(device_t bus) +{ + struct pci_devinfo *dinfo, *upinfo; + device_t child, limiting, pcib, *devlist; + uint16_t mmps, mps, target, up_mmps, up_mps; + int count, error, i; + + if (!pci_enable_mps_tune) + return; + /* Shared-path tuning is only safe before this bus attaches children. */ + if (device_is_attached(bus)) + return; + pcib = device_get_parent(bus); + if (!is_pci_device(pcib)) + return; + upinfo = device_get_ivars(pcib); + if (upinfo->cfg.pcie.pcie_location == 0) + return; + error = device_get_children(bus, &devlist, &count); + if (error != 0) + return; + + up_mps = pcie_read_config(pcib, PCIER_DEVICE_CTL, 2) & + PCIEM_CTL_MAX_PAYLOAD; + target = up_mps; + limiting = NULL; + up_mmps = (pcie_read_config(pcib, PCIER_DEVICE_CAP, 2) & PCIEM_CAP_MAX_PAYLOAD) << 5; - if (rmps > mmps) { - /* - * The device is unable to handle root's MPS. Limit root. - * XXX: We should traverse through all the tree, applying - * it to all the devices. - */ - pcie_adjust_config(root, PCIER_DEVICE_CTL, - PCIEM_CTL_MAX_PAYLOAD, mmps, 2); - } else { - pcie_adjust_config(dev, PCIER_DEVICE_CTL, - PCIEM_CTL_MAX_PAYLOAD, rmps, 2); + if (target > up_mmps) { + target = up_mmps; + limiting = pcib; + } + /* + * Firmware may leave Bus Master Enable set after handoff. Since no + * child driver has attached during this cold pass, it is not a proxy + * for a live FreeBSD consumer. + */ + for (i = 0; i < count; i++) { + child = devlist[i]; + dinfo = device_get_ivars(child); + if ((dinfo->cfg.flags & (PCICFG_VF | + PCICFG_MPS_UNRECONCILED)) != 0 || + dinfo->cfg.pcie.pcie_location == 0) + continue; + mmps = (pcie_read_config(child, PCIER_DEVICE_CAP, 2) & + PCIEM_CAP_MAX_PAYLOAD) << 5; + if (target > mmps) { + target = mmps; + limiting = child; + } + } + + /* + * Do not lower one link below a switch without also reconciling every + * ancestor and sibling subtree. Recursive newbus attachment may already + * have made another subtree live, so leave the established path intact. + */ + if (target < up_mps && + upinfo->cfg.pcie.pcie_type != PCIEM_TYPE_ROOT_PORT) { + pcie_mps_conflict(limiting, up_mps, target); + pcie_mps_mark_link_unreconciled(devlist, count, up_mps, + up_mps > up_mmps); + /* Keep compatible functions at the established path MPS. */ + target = up_mps; + } + /* Lower downstream producers before lowering the shared Root Port. */ + for (i = 0; i < count; i++) { + child = devlist[i]; + dinfo = device_get_ivars(child); + if ((dinfo->cfg.flags & (PCICFG_VF | + PCICFG_MPS_UNRECONCILED)) != 0 || + dinfo->cfg.pcie.pcie_location == 0) + continue; + mps = pcie_read_config(child, PCIER_DEVICE_CTL, 2) & + PCIEM_CTL_MAX_PAYLOAD; + if (mps > target) + pcie_adjust_config(child, PCIER_DEVICE_CTL, + PCIEM_CTL_MAX_PAYLOAD, target, 2); + } + if (up_mps > target) + pcie_adjust_config(pcib, PCIER_DEVICE_CTL, + PCIEM_CTL_MAX_PAYLOAD, target, 2); + + /* Raise idle children only after the upstream port is configured. */ + for (i = 0; i < count; i++) { + child = devlist[i]; + dinfo = device_get_ivars(child); + if ((dinfo->cfg.flags & (PCICFG_VF | + PCICFG_MPS_UNRECONCILED)) != 0 || + dinfo->cfg.pcie.pcie_location == 0) + continue; + mps = pcie_read_config(child, PCIER_DEVICE_CTL, 2) & + PCIEM_CTL_MAX_PAYLOAD; + if (mps < target) + pcie_adjust_config(child, PCIER_DEVICE_CTL, + PCIEM_CTL_MAX_PAYLOAD, target, 2); } + free(devlist, M_TEMP); } static void diff --git a/sys/dev/pci/pci_private.h b/sys/dev/pci/pci_private.h index da8f9c02088e..8a63af046aae 100644 --- a/sys/dev/pci/pci_private.h +++ b/sys/dev/pci/pci_private.h @@ -116,6 +116,8 @@ pci_create_iov_child_t pci_create_iov_child_method; void pci_add_children(device_t dev, int domain, int busno); void pci_add_child(device_t bus, struct pci_devinfo *dinfo); +/* Call after cold enumeration and before attaching the bus's children. */ +void pcie_reconcile_link_mps(device_t bus); device_t pci_add_iov_child(device_t bus, device_t pf, uint16_t rid, uint16_t vid, uint16_t did); void pci_add_resources(device_t bus, device_t dev, int force, diff --git a/sys/dev/pci/pcivar.h b/sys/dev/pci/pcivar.h index bcd4d2d35b54..27bac22c7896 100644 --- a/sys/dev/pci/pcivar.h +++ b/sys/dev/pci/pcivar.h @@ -171,7 +171,9 @@ struct pcicfg_ea { STAILQ_HEAD(, pci_ea_entry) ea_entries; /* EA entries */ }; -#define PCICFG_VF 0x0001 /* Device is an SR-IOV Virtual Function */ +#define PCICFG_VF 0x0001 /* Device is an SR-IOV Virtual Function */ +#define PCICFG_MPS_WARNED 0x0002 /* MPS conflict already reported */ +#define PCICFG_MPS_UNRECONCILED 0x0004 /* MPS conflict left unchanged */ /* config header information common to all header types */ typedef struct pcicfg { diff --git a/sys/powerpc/ofw/ofw_pcibus.c b/sys/powerpc/ofw/ofw_pcibus.c index ff0fe39592e2..22a1634a0389 100644 --- a/sys/powerpc/ofw/ofw_pcibus.c +++ b/sys/powerpc/ofw/ofw_pcibus.c @@ -145,6 +145,7 @@ ofw_pcibus_attach(device_t dev) if (!ofw_devices_only) ofw_pcibus_enum_bus(dev, domain, busno); + pcie_reconcile_link_mps(dev); bus_attach_children(dev); return (0); }home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a7566c4.30ecd.3c4e1f93>