From 00da4805c3e1dd843de91794662809ec2516784e Mon Sep 17 00:00:00 2001 From: Gustavo Pimentel Date: Thu, 13 Aug 2020 15:51:51 +0200 Subject: PCI: Remove unnecessary header includes Remove unnecessary includes of: linux/of_pci.h linux/pci-ats.h asm/setup.h CoverityScan CID 16442, 16443, 16444 ("Unnecessary header file (HFA)") Link: https://lore.kernel.org/r/eba4c0f2b35b1442773a722f1cf73f7240f818e3.1597325845.git.gustavo.pimentel@synopsys.com Link: https://lore.kernel.org/r/72ade1f5af35b994a7a8216ea5dc32c27cf134cd.1597325845.git.gustavo.pimentel@synopsys.com Link: https://lore.kernel.org/r/715821dc855add2565505ff8dcb9970e87996c5c.1597325845.git.gustavo.pimentel@synopsys.com [bhelgaas: squash] Signed-off-by: Gustavo Pimentel Signed-off-by: Bjorn Helgaas Cc: Joao Pinto --- drivers/pci/pci.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index a458c46d7e39..0045907cf600 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -30,8 +29,6 @@ #include #include #include -#include -#include #include #include #include "pci.h" -- cgit From e7a7499d841bfc9c7a4443f0fd7a04ae49bdbdd3 Mon Sep 17 00:00:00 2001 From: Krzysztof Wilczyński Date: Mon, 24 Aug 2020 23:39:16 +0000 Subject: PCI: Use scnprintf(), not snprintf(), in sysfs "show" functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sysfs "show" methods should return the number of bytes printed into the buffer. This is the return value of scnprintf() [1]. snprintf(buf, size, ...) prints at most "size" bytes into "buf", but returns the number of bytes that *would* be printed if "buf" were large enough. Replace use of snprintf() with scnprintf(). No functional change intended. Related: https://patchwork.kernel.org/patch/9946759/#20969333 https://lwn.net/Articles/69419 [1] Documentation/filesystems/sysfs.rst [bhelgaas: squashed, commit log] Suggested-by: Bjorn Helgaas Link: https://lore.kernel.org/r/20200824233918.26306-2-kw@linux.com Link: https://lore.kernel.org/r/20200824233918.26306-3-kw@linux.com Link: https://lore.kernel.org/r/20200824233918.26306-4-kw@linux.com Signed-off-by: Krzysztof Wilczyński Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 0045907cf600..84b993714a91 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -6347,7 +6347,7 @@ static ssize_t resource_alignment_show(struct bus_type *bus, char *buf) spin_lock(&resource_alignment_lock); if (resource_alignment_param) - count = snprintf(buf, PAGE_SIZE, "%s", resource_alignment_param); + count = scnprintf(buf, PAGE_SIZE, "%s", resource_alignment_param); spin_unlock(&resource_alignment_lock); /* -- cgit From 76fc8e854e3b0b4641acf596d4a59f36d9939083 Mon Sep 17 00:00:00 2001 From: Rajat Jain Date: Tue, 7 Jul 2020 15:46:04 -0700 Subject: PCI/ACS: Enable Translation Blocking for external devices Translation Blocking is a required feature for Downstream Ports (Root Ports or Switch Downstream Ports) that implement ACS. When enabled, the Port checks the Address Type (AT) of each upstream Memory Request it receives. The default AT (00b) means "untranslated" and the IOMMU can decide whether to treat the address as I/O virtual or physical. If AT is not the default, i.e., if the Memory Request contains an already-translated (physical) address, the Port blocks the request and reports an ACS error. When enabling ACS, enable Translation Blocking for external-facing ports and untrusted (external) devices. This is to help prevent attacks from external devices that initiate DMA with physical addresses that bypass the IOMMU. [bhelgaas: commit log, simplify setting bit and drop warning; TB is required for Downstream Ports with ACS, so we should never see the warning] Link: https://lore.kernel.org/r/20200707224604.3737893-4-rajatja@google.com Signed-off-by: Rajat Jain Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index a458c46d7e39..8b649b01aad3 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -876,6 +876,10 @@ static void pci_std_enable_acs(struct pci_dev *dev) /* Upstream Forwarding */ ctrl |= (cap & PCI_ACS_UF); + /* Enable Translation Blocking for external devices */ + if (dev->external_facing || dev->untrusted) + ctrl |= (cap & PCI_ACS_TB); + pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl); } -- cgit From 8a61449941057e9d36fe468c97d3879356fa2881 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Thu, 17 Sep 2020 16:13:20 -0500 Subject: PCI: pciehp: Reduce noisiness on hot removal When a PCIe card is hot-removed, the Presence Detect State and Data Link Layer Link Active bits often do not clear simultaneously. I've seen delays of up to 244 msec between the two events with Thunderbolt. After pciehp has brought down the slot in response to the first event, the other bit may still be set. It's not discernible whether it's set because a new card is already in the slot or if it will soon clear. So pciehp tries to bring up the slot and in the latter case fails with a bunch of messages, some of them at KERN_ERR severity. If the slot is no longer occupied, the messages are false positives and annoy users. Stuart Hayes reports the following splat on hot removal: KERN_INFO pcieport 0000:3c:06.0: pciehp: Slot(180): Link Up KERN_INFO pcieport 0000:3c:06.0: pciehp: Timeout waiting for Presence Detect KERN_ERR pcieport 0000:3c:06.0: pciehp: link training error: status 0x0001 KERN_ERR pcieport 0000:3c:06.0: pciehp: Failed to check link status Dongdong Liu complains about a similar splat: KERN_INFO pciehp 0000:80:10.0:pcie004: Slot(36): Link Down KERN_INFO iommu: Removing device 0000:87:00.0 from group 12 KERN_INFO pciehp 0000:80:10.0:pcie004: Slot(36): Card present KERN_INFO pcieport 0000:80:10.0: Data Link Layer Link Active not set in 1000 msec KERN_ERR pciehp 0000:80:10.0:pcie004: Failed to check link status Users are particularly irritated to see a bringup attempt even though the slot was explicitly brought down via sysfs. In a perfect world, we could avoid this by setting Link Disable on slot bringdown and re-enabling it upon a Presence Detect State change. In reality however, there are broken hotplug ports which hardwire Presence Detect to zero, see 80696f991424 ("PCI: pciehp: Tolerate Presence Detect hardwired to zero"). Conversely, PCIe r1.0 hotplug ports hardwire Link Active to zero because Link Active Reporting wasn't specified before PCIe r1.1. On unplug, some ports first clear Presence then Link (see Stuart Hayes' splat) whereas others use the inverse order (see Dongdong Liu's splat). To top it off, there are hotplug ports which flap the Presence and Link bits on slot bringup, see 6c35a1ac3da6 ("PCI: pciehp: Tolerate initially unstable link"). pciehp is designed to work with all of these variants. Surplus attempts at slot bringup are a lesser evil than not being able to bring up slots at all. Although we could try to perfect the behavior for specific hotplug controllers, we'd risk breaking others or increasing code complexity. But we can certainly minimize annoyance by emitting only a single message with KERN_INFO severity if bringup is unsuccessful: * Drop the "Timeout waiting for Presence Detect" message in pcie_wait_for_presence(). The sole caller of that function, pciehp_check_link_status(), ignores the timeout and carries on. It emits error messages of its own and I don't think this particular message adds much value. * There's a single error condition in pciehp_check_link_status() which does not emit a message. Adding one allows dropping the "Failed to check link status" message emitted by board_added() if pciehp_check_link_status() returns a non-zero integer. * Tone down all messages in pciehp_check_link_status() to KERN_INFO severity and rephrase them to look as innocuous as possible. To this end, move the message emitted by pcie_wait_for_link_delay() to its callers. As a result, Stuart Hayes' splat becomes: KERN_INFO pcieport 0000:3c:06.0: pciehp: Slot(180): Link Up KERN_INFO pcieport 0000:3c:06.0: pciehp: Slot(180): Cannot train link: status 0x0001 Dongdong Liu's splat becomes: KERN_INFO pciehp 0000:80:10.0:pcie004: Slot(36): Card present KERN_INFO pciehp 0000:80:10.0:pcie004: Slot(36): No link The messages now merely serve as information that presence or link bits were set a little longer than expected. Bringup failures which are not false positives are still reported, albeit no longer at KERN_ERR severity. Link: https://lore.kernel.org/linux-pci/20200310182100.102987-1-stuart.w.hayes@gmail.com/ Link: https://lore.kernel.org/linux-pci/1547649064-19019-1-git-send-email-liudongdong3@huawei.com/ Link: https://lore.kernel.org/r/b45e46fd8a6aa6930aaac9d7718c2e4b787a4e5e.1595935071.git.lukas@wunner.de Reported-by: Stuart Hayes Reported-by: Dongdong Liu Signed-off-by: Lukas Wunner Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/pci.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index a458c46d7e39..cbd3fee69f46 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4701,9 +4701,7 @@ static bool pcie_wait_for_link_delay(struct pci_dev *pdev, bool active, } if (active && ret) msleep(delay); - else if (ret != active) - pci_info(pdev, "Data Link Layer Link Active not %s in 1000 msec\n", - active ? "set" : "cleared"); + return ret == active; } @@ -4828,6 +4826,7 @@ void pci_bridge_wait_for_secondary_bus(struct pci_dev *dev) delay); if (!pcie_wait_for_link_delay(dev, true, delay)) { /* Did not train, no need to wait any further */ + pci_info(dev, "Data Link Layer Link Active not set in 1000 msec\n"); return; } } -- cgit From 10791141a6cfc96eecf578fb1240f191ac112e02 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Tue, 21 Jul 2020 13:24:51 +0200 Subject: PCI: Simplify pci_dev_reset_slot_function() pci_dev_reset_slot_function() refuses to reset a hotplug slot if it is shared by multiple pci_devs. That's the case if and only if the slot is occupied by a multifunction device. Simplify the function to check the device's multifunction flag instead of iterating over the devices on the bus. (Iterating over the devices requires holding pci_bus_sem, which the function erroneously does not acquire.) Link: https://lore.kernel.org/r/c6aab5af096f7b1b3db57f6335cebba8f0fcca89.1595330431.git.lukas@wunner.de Signed-off-by: Lukas Wunner Signed-off-by: Bjorn Helgaas Cc: Alex Williamson --- drivers/pci/pci.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 84b993714a91..b630d53e998a 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4917,16 +4917,10 @@ static int pci_reset_hotplug_slot(struct hotplug_slot *hotplug, int probe) static int pci_dev_reset_slot_function(struct pci_dev *dev, int probe) { - struct pci_dev *pdev; - - if (dev->subordinate || !dev->slot || + if (dev->multifunction || dev->subordinate || !dev->slot || dev->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET) return -ENOTTY; - list_for_each_entry(pdev, &dev->bus->devices, bus_list) - if (pdev != dev && pdev->slot == dev->slot) - return -ENOTTY; - return pci_reset_hotplug_slot(dev->slot->hotplug, probe); } -- cgit From 3789af9a13e5561738c0f2114e3a5e22c843ca3e Mon Sep 17 00:00:00 2001 From: Krzysztof Wilczyński Date: Thu, 30 Jul 2020 21:08:48 +0000 Subject: PCI/PM: Rename pci_dev.d3_delay to d3hot_delay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PCI devices support two variants of the D3 power state: D3hot (main power present) D3cold (main power removed). Previously struct pci_dev contained: unsigned int d3_delay; /* D3->D0 transition time in ms */ unsigned int d3cold_delay; /* D3cold->D0 transition time in ms */ "d3_delay" refers specifically to the D3hot state. Rename it to "d3hot_delay" to avoid ambiguity and align with the ACPI "_DSM for Specifying Device Readiness Durations" in the PCI Firmware spec r3.2, sec 4.6.9. There is no change to the functionality. Link: https://lore.kernel.org/r/20200730210848.1578826-1-kw@linux.com Signed-off-by: Krzysztof Wilczyński Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index a458c46d7e39..c4a26532a447 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -49,7 +49,7 @@ EXPORT_SYMBOL(isa_dma_bridge_buggy); int pci_pci_problems; EXPORT_SYMBOL(pci_pci_problems); -unsigned int pci_pm_d3_delay; +unsigned int pci_pm_d3hot_delay; static void pci_pme_list_scan(struct work_struct *work); @@ -66,10 +66,10 @@ struct pci_pme_device { static void pci_dev_d3_sleep(struct pci_dev *dev) { - unsigned int delay = dev->d3_delay; + unsigned int delay = dev->d3hot_delay; - if (delay < pci_pm_d3_delay) - delay = pci_pm_d3_delay; + if (delay < pci_pm_d3hot_delay) + delay = pci_pm_d3hot_delay; if (delay) msleep(delay); @@ -3013,7 +3013,7 @@ void pci_pm_init(struct pci_dev *dev) } dev->pm_cap = pm; - dev->d3_delay = PCI_PM_D3_WAIT; + dev->d3hot_delay = PCI_PM_D3HOT_WAIT; dev->d3cold_delay = PCI_PM_D3COLD_WAIT; dev->bridge_d3 = pci_bridge_d3_possible(dev); dev->d3cold_allowed = true; @@ -3038,7 +3038,7 @@ void pci_pm_init(struct pci_dev *dev) (pmc & PCI_PM_CAP_PME_D0) ? " D0" : "", (pmc & PCI_PM_CAP_PME_D1) ? " D1" : "", (pmc & PCI_PM_CAP_PME_D2) ? " D2" : "", - (pmc & PCI_PM_CAP_PME_D3) ? " D3hot" : "", + (pmc & PCI_PM_CAP_PME_D3hot) ? " D3hot" : "", (pmc & PCI_PM_CAP_PME_D3cold) ? " D3cold" : ""); dev->pme_support = pmc >> PCI_PM_CAP_PME_SHIFT; dev->pme_poll = true; @@ -4621,7 +4621,7 @@ static int pci_af_flr(struct pci_dev *dev, int probe) * * NOTE: This causes the caller to sleep for twice the device power transition * cooldown period, which for the D0->D3hot and D3hot->D0 transitions is 10 ms - * by default (i.e. unless the @dev's d3_delay field has a different value). + * by default (i.e. unless the @dev's d3hot_delay field has a different value). * Moreover, only devices in D0 can be reset by this function. */ static int pci_pm_reset(struct pci_dev *dev, int probe) -- cgit From 0a98bb98f2c7a47563f71770e458902ec211a6cf Mon Sep 17 00:00:00 2001 From: Krzysztof Wilczyński Date: Fri, 25 Sep 2020 22:45:55 +0000 Subject: PCI: Simplify bool comparisons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Take care about Coccinelle warnings: drivers/pci/pci.c:6008:6-12: WARNING: Comparison to bool drivers/pci/pci.c:6024:7-13: WARNING: Comparison to bool No change to functionality intended. Link: https://lore.kernel.org/r/20200925224555.1752460-1-kw@linux.com Signed-off-by: Krzysztof Wilczyński Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b630d53e998a..42364a1a7cb7 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5996,7 +5996,7 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode, if (flags & PCI_VGA_STATE_CHANGE_DECODES) { pci_read_config_word(dev, PCI_COMMAND, &cmd); - if (decode == true) + if (decode) cmd |= command_bits; else cmd &= ~command_bits; @@ -6012,7 +6012,7 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode, if (bridge) { pci_read_config_word(bridge, PCI_BRIDGE_CONTROL, &cmd); - if (decode == true) + if (decode) cmd |= PCI_BRIDGE_CTL_VGA; else cmd &= ~PCI_BRIDGE_CTL_VGA; -- cgit From 638c133e5f52a3793cb5494897df008f5cef0ddd Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Tue, 29 Sep 2020 14:24:11 -0500 Subject: PCI/PM: Revert "PCI/PM: Apply D2 delay as milliseconds, not microseconds" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 7e24bc347e57992d532bc2ed700209b0fc0a4bf5. 7e24bc347e57 was based on PCIe r5.0, sec 5.9, which claims we need a 200 ms delay when transitioning to or from D2. However, sec 5.3.1.3 states the delay as 200 μs (microseconds), as does the table in PCIe r4.0, sec 5.9.1. This looks like a typo in the r5.0 spec, so revert back to a 200 μs delay instead of a 200 ms delay. Fixes: 7e24bc347e57 ("PCI/PM: Apply D2 delay as milliseconds, not microseconds") Signed-off-by: Bjorn Helgaas Reviewed-by: Rafael J. Wysocki --- drivers/pci/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index c4a26532a447..d69578ad44a0 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1065,7 +1065,7 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state) if (state == PCI_D3hot || dev->current_state == PCI_D3hot) pci_dev_d3_sleep(dev); else if (state == PCI_D2 || dev->current_state == PCI_D2) - msleep(PCI_PM_D2_DELAY); + udelay(PCI_PM_D2_DELAY); pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); dev->current_state = (pmcsr & PCI_PM_CTRL_STATE_MASK); -- cgit From b0e85c3c8554944c6ec599656931b54062c38823 Mon Sep 17 00:00:00 2001 From: Jim Quinlan Date: Mon, 28 Sep 2020 15:46:51 -0400 Subject: PCI: Add Kconfig options for MPS/MRRS strategy Add Kconfig options for changing the default pcie_bus_config, i.e., the strategy for configuration MPS and MRRS, in the same manner as the CONFIG_PCIEASPM_XXXX choice. The pci_bus_config setting may still be overridden by kernel command-line parameters, e.g., "pci=pcie_bus_tune_off". [bhelgaas: depend on EXPERT, tweak help texts] Link: https://lore.kernel.org/r/20200928194651.5393-2-james.quinlan@broadcom.com Signed-off-by: Jim Quinlan Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'drivers/pci/pci.c') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index a458c46d7e39..09c5cbd5a52c 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -101,7 +101,19 @@ unsigned long pci_hotplug_mmio_pref_size = DEFAULT_HOTPLUG_MMIO_PREF_SIZE; #define DEFAULT_HOTPLUG_BUS_SIZE 1 unsigned long pci_hotplug_bus_size = DEFAULT_HOTPLUG_BUS_SIZE; + +/* PCIe MPS/MRRS strategy; can be overridden by kernel command-line param */ +#ifdef CONFIG_PCIE_BUS_TUNE_OFF +enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_TUNE_OFF; +#elif defined CONFIG_PCIE_BUS_SAFE +enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_SAFE; +#elif defined CONFIG_PCIE_BUS_PERFORMANCE +enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_PERFORMANCE; +#elif defined CONFIG_PCIE_BUS_PEER2PEER +enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_PEER2PEER; +#else enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_DEFAULT; +#endif /* * The default CLS is used if arch didn't set CLS explicitly and not -- cgit