Skip to contentSkip to conversation

Regather scattered PCI-Code

PATCH Applied Philipp Stanner 3 participants 13 messages Jan 31, 2024 – Feb 6, 2024

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

v6 cover Regather scattered PCI-Code

@Stable-Kernel:
You receive this patch series because its first patch fixes a leak in PCI.

@Bjorn:
I decided that it's now actually possible to just embed the docu updates to the respective patches, instead of a separate patch.
Also dropped the ioport_unmap() for now.

Changes in v6:
- Remove the addition of ioport_unmap() from patch #1, since this is not really a bug, as explained by the comment above pci_iounmap. (Bjorn)
- Drop the patch unifying the two versions of pci_iounmap(). (Bjorn)
- Make patch #4's style congruent with PCI style.
- Drop (in any case empty) ioport_unmap() again from pci_iounmap()
- Add forgotten updates to Documentation/ when moving files from lib/ to drivers/pci/

Changes in v5:
- Add forgotten update to MAINTAINERS file.

Changes in v4:
- Apply Arnd's Reviewed-by's
- Add ifdef CONFIG_HAS_IOPORT_MAP guard in drivers/pci/iomap.c (build error on openrisc)
- Fix typo in patch no.5

Changes in v3:
- Create a separate patch for the leaks in lib/iomap.c. Make it the series' first patch. (Arnd)
- Turns out the aforementioned bug wasn't just accidentally removing iounmap() with the ifdef, it was also missing ioport_unmap() to begin with. Add it.
- Move the ARCH_WANTS_GENERIC_IOMEM_IS_IOPORT-mechanism from asm-generic/io.h to asm-generic/ioport.h. (Arnd)
- Adjust the implementation of iomem_is_ioport() in asm-generic/io.h so that it matches exactly what pci_iounmap() previously did in lib/pci_iomap.c. (Arnd)
- Move the CONFIG_HAS_IOPORT guard in asm-generic/io.h so that iomem_is_ioport() will always be compiled and just returns false if there are no ports.
- Add TODOs to several places informing about the generic iomem_is_ioport() in lib/iomap.c not being generic.
- Add TODO about the followup work to make drivers/pci/iomap.c's pci_iounmap() actually generic.

Changes in v2:
- Replace patch 4, previously extending the comment about pci_iounmap() in lib/iomap.c, with a patch that moves pci_iounmap() from that file to drivers/pci/iomap.c, creating a unified version there. (Arnd)
- Implement iomem_is_ioport() as a new helper in asm-generic/io.h and lib/iomap.c. (Arnd)
- Move the build rule in drivers/pci/Makefile for iomap.o under the guard of #if PCI. This had to be done because when just checking for GENERIC_PCI_IOMAP being defined, the functions don't disappear, which was the case previously in lib/pci_iomap.c, where the entire file was made empty if PCI was not set by the guard #ifdef PCI. (Intel's Bots)
- Rephares all patches' commit messages a little bit.

Sooooooooo. I reworked v1.

Please review this carefully, the IO-Ranges are obviously a bit tricky, as is the build-system / ifdef-ery.

Arnd has suggested that architectures defining a custom inb() need their own iomem_is_ioport(), as well. I've grepped for inb() and found the following list of archs that define their own:
- alpha
- arm
- m68k <--
- parisc
- powerpc
- sh
- sparc
- x86 <--

All of those have their own definitons of pci_iounmap(). Therefore, they don't need our generic version in the first place and, thus, also need no iomem_is_ioport().
The two exceptions are x86 and m68k. The former uses lib/iomap.c through CONFIG_GENERIC_IOMAP, as Arnd pointed out in the previous discussion (thus, CONFIG_GENERIC_IOMAP is not really generic in this regard).

So as I see it, only m68k WOULD need its own custom definition of iomem_is_ioport(). But as I understand it it doesn't because it uses the one from asm-generic/pci_iomap.h ??

I wasn't entirely sure how to deal with the address ranges for the generic implementation in asm-generic/io.h. It's marked with a TODO. Input appreciated.

I removed the guard around define pci_iounmap in asm-generic/io.h. An alternative would be to have it be guarded by CONFIG_GENERIC_IOMAP and CONFIG_GENERIC_PCI_IOMAP, both. Without such a guard, there is no collision however, because generic pci_iounmap() from drivers/pci/iomap.c will only get pulled in when CONFIG_GENERIC_PCI_IOMAP is actually set.

I cross-built this for a variety of architectures, including the usual suspects (s390, m68k). So far successfully. But let's see what Intel's robots say :O

P.

Original cover letter:

Hi!

So it seems that since ca. 2007 the PCI code has been scattered a bit. PCI's devres code, which is only ever used by users of the entire PCI-subsystem anyways, resides in lib/devres.c and is guarded by an ifdef PCI, just as the content of lib/pci_iomap.c is.

It, thus, seems reasonable to move all of that.

As I were at it, I moved as much of the devres-specific code from pci.c to devres.c, too. The only exceptions are four functions that are currently difficult to move. More information about that can be read here [1].

I noticed these scattered files while working on (new) PCI-specific devres functions. If we can get this here merged, I'll soon send another patch series that addresses some API-inconsistencies and could move the devres-part of the four remaining functions.

I don't want to do that in this series as this here is only about moving code, whereas the next series would have to actually change API behavior.

I successfully (cross-)built this for x86, x86_64, AARCH64 and ARM (allyesconfig). I booted a kernel with it on x86_64, with a Fedora desktop environment as payload. The OS came up fine

I hope this is OK. If we can get it in, we'd soon have a very consistent PCI API again.

Regards,
P.

Philipp Stanner (4):
lib/pci_iomap.c: fix cleanup bug in pci_iounmap()
lib: move pci_iomap.c to drivers/pci/
lib: move pci-specific devres code to drivers/pci/
PCI: Move devres code from pci.c to devres.c

 Documentation/driver-api/device-io.rst |   2 +-
 Documentation/driver-api/pci/pci.rst   |   6 +
 MAINTAINERS                            |   1 -
 drivers/pci/Kconfig                    |   5 +
 drivers/pci/Makefile                   |   3 +-
 drivers/pci/devres.c                   | 450 +++++++++++++++++++++++++
 lib/pci_iomap.c => drivers/pci/iomap.c |   5 +-
 drivers/pci/pci.c                      | 249 --------------
 drivers/pci/pci.h                      |  24 ++
 lib/Kconfig                            |   3 -
 lib/Makefile                           |   1 -
 lib/devres.c                           | 208 +-----------
 12 files changed, 490 insertions(+), 467 deletions(-)
 create mode 100644 drivers/pci/devres.c
 rename lib/pci_iomap.c => drivers/pci/iomap.c (99%)
Signature · 1 line
2.43.0
#1 lore raw

PATCH v6 1/4 lib/pci_iomap.c: fix cleanup bug in pci_iounmap()

The #ifdef for the ioport-ranges accidentally also guards iounmap(), potentially compiling an empty function. This would cause the mapping to be leaked.

Move the guard so that iounmap() will always be part of the function.

CC <[email protected]> # v5.15+
Fixes 316e8d79a095 ("pci_iounmap'2: Electric Boogaloo: try to make sense of it all")
---
 lib/pci_iomap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
lib/pci_iomap.c +1 -1
index ce39ce9f3526..2829ddb0e316 100644
@@ -170,8 +170,8 @@ void pci_iounmap(struct pci_dev *dev, void __iomem *p)
 
 	if (addr >= start && addr < start + IO_SPACE_LIMIT)
 		return;
-	iounmap(p);
 #endif
+	iounmap(p);
 }
 EXPORT_SYMBOL(pci_iounmap);
 
Signature · 1 line
2.43.0
#2 lore raw

Re: [PATCH v6 1/4] lib/pci_iomap.c: fix cleanup bug in pci_iounmap()

Quoted 5 lines · Philipp Stanner

The #ifdef for the ioport-ranges accidentally also guards iounmap(), potentially compiling an empty function. This would cause the mapping to be leaked.

Move the guard so that iounmap() will always be part of the function.

I tweaked the subject and commit log to be more explicit about what the bug is. Let me know if I got it wrong:

pci_iounmap(): Fix MMIO mapping leak

The #ifdef ARCH_HAS_GENERIC_IOPORT_MAP accidentally also guards iounmap(), which means MMIO mappings are leaked.

Move the guard so we call iounmap() for MMIO mappings.

Bjorn

#3 lore raw

Re: [PATCH v6 1/4] lib/pci_iomap.c: fix cleanup bug in pci_iounmap()

Quoted 7 lines · Bjorn Helgaas
Quoted 6 lines · Philipp Stanner

The #ifdef for the ioport-ranges accidentally also guards iounmap(),
potentially compiling an empty function. This would cause the mapping to
be leaked.

Quoted 5 lines
Quoted 2 lines

Move the guard so that iounmap() will always be part of the function.

I tweaked the subject and commit log to be more explicit about what the bug is.  Let me know if I got it wrong:

Mostly correct IMO

Quoted 6 lines

  pci_iounmap(): Fix MMIO mapping leak

  The #ifdef ARCH_HAS_GENERIC_IOPORT_MAP accidentally also guards iounmap(),
  which means MMIO mappings are leaked.

nit: I wasn't entirely sure when they are actually leaked, just that they _could_ be leaked. To know for sure we'd need to search who sets ARCH_WANTS_GENERIC_PCI_IOUNMAP without setting ARCH_HAS_GENERIC_IOPORT_MAP.

I think your formulation should be fine, though, since it's definitely a bug.

P.

Quoted 5 lines

  Move the guard so we call iounmap() for MMIO mappings.

Bjorn

#4 lore raw

PATCH v6 2/4 lib: move pci_iomap.c to drivers/pci/

In reply to Philipp Stanner

This file is guarded by an #ifdef CONFIG_PCI. It, consequently, does not belong to lib/ because it is not generic infrastructure.

Move the file to drivers/pci/ and implement the necessary changes to Makefiles and Kconfigs.

Update MAINTAINERS file.

Update Documentation.

---
 Documentation/driver-api/device-io.rst | 2 +-
 Documentation/driver-api/pci/pci.rst   | 3 +++
 MAINTAINERS                            | 1 -
 drivers/pci/Kconfig                    | 5 +++++
 drivers/pci/Makefile                   | 1 +
 lib/pci_iomap.c => drivers/pci/iomap.c | 3 ---
 lib/Kconfig                            | 3 ---
 lib/Makefile                           | 1 -
 8 files changed, 10 insertions(+), 9 deletions(-)
 rename lib/pci_iomap.c => drivers/pci/iomap.c (99%)
Documentation/driver-api/device-io.rst +1 -1
index d55384b106bd..d9ba2dfd1239 100644
@@ -518,5 +518,5 @@ Public Functions Provided
 .. kernel-doc:: arch/x86/include/asm/io.h
    :internal:
 
-.. kernel-doc:: lib/pci_iomap.c
+.. kernel-doc:: drivers/pci/iomap.c
    :export:
Documentation/driver-api/pci/pci.rst +3 -0
index 4843cfad4f60..bacf23bf1343 100644
@@ -4,6 +4,9 @@ PCI Support Library
 .. kernel-doc:: drivers/pci/pci.c
    :export:
 
+.. kernel-doc:: drivers/pci/iomap.c
+   :export:
+
 .. kernel-doc:: drivers/pci/pci-driver.c
    :export:
 
MAINTAINERS +0 -1
index 8d1052fa6a69..395fcaad63e7 100644
@@ -16954,7 +16954,6 @@ F:	include/asm-generic/pci*
 F:	include/linux/of_pci.h
 F:	include/linux/pci*
 F:	include/uapi/linux/pci*
-F:	lib/pci*
 
 PCIE DRIVER FOR AMAZON ANNAPURNA LABS
 M:	Jonathan Chocron <[email protected]>
drivers/pci/Kconfig +5 -0
index 74147262625b..d35001589d88 100644
@@ -13,6 +13,11 @@ config FORCE_PCI
 	select HAVE_PCI
 	select PCI
 
+# select this to provide a generic PCI iomap,
+# without PCI itself having to be defined
+config GENERIC_PCI_IOMAP
+	bool
+
 menuconfig PCI
 	bool "PCI support"
 	depends on HAVE_PCI
drivers/pci/Makefile +1 -0
index cc8b4e01e29d..64dcedccfc87 100644
@@ -14,6 +14,7 @@ ifdef CONFIG_PCI
 obj-$(CONFIG_PROC_FS)		+= proc.o
 obj-$(CONFIG_SYSFS)		+= slot.o
 obj-$(CONFIG_ACPI)		+= pci-acpi.o
+obj-$(CONFIG_GENERIC_PCI_IOMAP) += iomap.o
 endif
 
 obj-$(CONFIG_OF)		+= of.o
drivers/pci/iomap.c renamed from lib/pci_iomap.c +0 -3
similarity index 99%
rename from lib/pci_iomap.c
rename to drivers/pci/iomap.c
index 2829ddb0e316..c9725428e387 100644
@@ -9,7 +9,6 @@
 
 #include <linux/export.h>
 
-#ifdef CONFIG_PCI
 /**
  * pci_iomap_range - create a virtual mapping cookie for a PCI BAR
  * @dev: PCI device that owns the BAR
@@ -176,5 +175,3 @@ void pci_iounmap(struct pci_dev *dev, void __iomem *p)
 EXPORT_SYMBOL(pci_iounmap);
 
 #endif /* ARCH_WANTS_GENERIC_PCI_IOUNMAP */
-
-#endif /* CONFIG_PCI */
lib/Kconfig +0 -3
index 5ddda7c2ed9b..4557bb8a5256 100644
@@ -70,9 +70,6 @@ source "lib/math/Kconfig"
 config NO_GENERIC_PCI_IOPORT_MAP
 	bool
 
-config GENERIC_PCI_IOMAP
-	bool
-
 config GENERIC_IOMAP
 	bool
 	select GENERIC_PCI_IOMAP
lib/Makefile +0 -1
index 6b09731d8e61..0800289ec6c5 100644
@@ -153,7 +153,6 @@ CFLAGS_debug_info.o += $(call cc-option, -femit-struct-debug-detailed=any)
 obj-y += math/ crypto/
 
 obj-$(CONFIG_GENERIC_IOMAP) += iomap.o
-obj-$(CONFIG_GENERIC_PCI_IOMAP) += pci_iomap.o
 obj-$(CONFIG_HAS_IOMEM) += iomap_copy.o devres.o
 obj-$(CONFIG_CHECK_SIGNATURE) += check_signature.o
 obj-$(CONFIG_DEBUG_LOCKING_API_SELFTESTS) += locking-selftest.o
Signature · 1 line
2.43.0
#5 lore raw

Re: [PATCH v6 2/4] lib: move pci_iomap.c to drivers/pci/

FYI: kernel test robot notices the stable kernel rule is not satisfied.

Hi,

Thanks for your patch.

FYI: kernel test robot notices the stable kernel rule is not satisfied.

The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-1

Rule: add the tag "Cc: [email protected]" in the sign-off area to have the patch automatically included in the stable tree.
Subject: [PATCH v6 2/4] lib: move pci_iomap.c to drivers/pci/

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
#6 lore raw

PATCH v6 3/4 lib: move pci-specific devres code to drivers/pci/

In reply to Philipp Stanner

The pcim_*() functions in lib/devres.c are guarded by an #ifdef CONFIG_PCI and, thus, don't belong to this file. They are only ever used for pci and are not generic infrastructure.

Move all pcim_*() functions in lib/devres.c to drivers/pci/devres.c. Adjust the Makefile.

Add drivers/pci/devres.c to Documentation.

---
 Documentation/driver-api/pci/pci.rst |   3 +
 drivers/pci/Makefile                 |   2 +-
 drivers/pci/devres.c                 | 207 ++++++++++++++++++++++++++
 lib/devres.c                         | 208 +--------------------------
 4 files changed, 212 insertions(+), 208 deletions(-)
 create mode 100644 drivers/pci/devres.c
Documentation/driver-api/pci/pci.rst +3 -0
index bacf23bf1343..aa40b1cc243b 100644
@@ -7,6 +7,9 @@ PCI Support Library
 .. kernel-doc:: drivers/pci/iomap.c
    :export:
 
+.. kernel-doc:: drivers/pci/devres.c
+   :export:
+
 .. kernel-doc:: drivers/pci/pci-driver.c
    :export:
 
drivers/pci/Makefile +1 -1
index 64dcedccfc87..ed65299b42b5 100644
@@ -5,7 +5,7 @@
 obj-$(CONFIG_PCI)		+= access.o bus.o probe.o host-bridge.o \
 				   remove.o pci.o pci-driver.o search.o \
 				   pci-sysfs.o rom.o setup-res.o irq.o vpd.o \
-				   setup-bus.o vc.o mmap.o setup-irq.o
+				   setup-bus.o vc.o mmap.o setup-irq.o devres.o
 
 obj-$(CONFIG_PCI)		+= msi/
 obj-$(CONFIG_PCI)		+= pcie/
drivers/pci/devres.c new file +207 -0

207 lines not on this page · raw message

lib/devres.c +1 -207

219 lines not on this page · raw message

Signature · 1 line
2.43.0
#7 lore raw

PATCH v6 4/4 PCI: Move devres code from pci.c to devres.c

In reply to Philipp Stanner

The file pci.c is very large and contains a number of devres-functions. These functions should now reside in devres.c

There are a few callers left in pci.c that do devres operations. These should be ported in the future. Corresponding TODOs are added by this commit.

The reason they are not moved right now in this commit is that pci's devres currently implements a sort of "hybrid-mode":
pci_request_region(), for instance, does not have a corresponding pcim_ equivalent, yet. Instead, the function can be made managed by previously calling pcim_enable_device() (instead of pci_enable_device()). This makes it unreasonable to move pci_request_region() to devres.c Moving the functions would require changes to pci's API and is, therefore, left for future work.

In summary, this commit serves as a preparation step for a following patch-series that will cleanly separate the PCI's managed and unmanaged API.

Move as much devres-specific code from pci.c to devres.c as possible.

---
 drivers/pci/devres.c | 243 +++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.c    | 249 -------------------------------------------
 drivers/pci/pci.h    |  24 +++++
 3 files changed, 267 insertions(+), 249 deletions(-)
drivers/pci/devres.c +243 -0
index a3fd0d65cef1..4bd1e125bca1 100644
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <linux/device.h>
 #include <linux/pci.h>
 #include "pci.h"
 
@@ -11,6 +12,248 @@ struct pcim_iomap_devres {
 	void __iomem *table[PCIM_IOMAP_MAX];
 };
 
+
+static void devm_pci_unmap_iospace(struct device *dev, void *ptr)
+{
+	struct resource **res = ptr;
+
+	pci_unmap_iospace(*res);
+}
+
+/**
+ * devm_pci_remap_iospace - Managed pci_remap_iospace()
+ * @dev: Generic device to remap IO address for
+ * @res: Resource describing the I/O space
+ * @phys_addr: physical address of range to be mapped
+ *
+ * Managed pci_remap_iospace().  Map is automatically unmapped on driver
+ * detach.
+ */
+int devm_pci_remap_iospace(struct device *dev, const struct resource *res,
+			   phys_addr_t phys_addr)
+{
+	const struct resource **ptr;
+	int error;
+
+	ptr = devres_alloc(devm_pci_unmap_iospace, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return -ENOMEM;
+
+	error = pci_remap_iospace(res, phys_addr);
+	if (error) {
+		devres_free(ptr);
+	} else	{
+		*ptr = res;
+		devres_add(dev, ptr);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(devm_pci_remap_iospace);
+
+/**
+ * devm_pci_remap_cfgspace - Managed pci_remap_cfgspace()
+ * @dev: Generic device to remap IO address for
+ * @offset: Resource address to map
+ * @size: Size of map
+ *
+ * Managed pci_remap_cfgspace().  Map is automatically unmapped on driver
+ * detach.
+ */
+void __iomem *devm_pci_remap_cfgspace(struct device *dev,
+				      resource_size_t offset,
+				      resource_size_t size)
+{
+	void __iomem **ptr, *addr;
+
+	ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return NULL;
+
+	addr = pci_remap_cfgspace(offset, size);
+	if (addr) {
+		*ptr = addr;
+		devres_add(dev, ptr);
+	} else
+		devres_free(ptr);
+
+	return addr;
+}
+EXPORT_SYMBOL(devm_pci_remap_cfgspace);
+
+/**
+ * devm_pci_remap_cfg_resource - check, request region and ioremap cfg resource
+ * @dev: generic device to handle the resource for
+ * @res: configuration space resource to be handled
+ *
+ * Checks that a resource is a valid memory region, requests the memory
+ * region and ioremaps with pci_remap_cfgspace() API that ensures the
+ * proper PCI configuration space memory attributes are guaranteed.
+ *
+ * All operations are managed and will be undone on driver detach.
+ *
+ * Returns a pointer to the remapped memory or an ERR_PTR() encoded error code
+ * on failure. Usage example::
+ *
+ *	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ *	base = devm_pci_remap_cfg_resource(&pdev->dev, res);
+ *	if (IS_ERR(base))
+ *		return PTR_ERR(base);
+ */
+void __iomem *devm_pci_remap_cfg_resource(struct device *dev,
+					  struct resource *res)
+{
+	resource_size_t size;
+	const char *name;
+	void __iomem *dest_ptr;
+
+	BUG_ON(!dev);
+
+	if (!res || resource_type(res) != IORESOURCE_MEM) {
+		dev_err(dev, "invalid resource\n");
+		return IOMEM_ERR_PTR(-EINVAL);
+	}
+
+	size = resource_size(res);
+
+	if (res->name)
+		name = devm_kasprintf(dev, GFP_KERNEL, "%s %s", dev_name(dev),
+				      res->name);
+	else
+		name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL);
+	if (!name)
+		return IOMEM_ERR_PTR(-ENOMEM);
+
+	if (!devm_request_mem_region(dev, res->start, size, name)) {
+		dev_err(dev, "can't request region for resource %pR\n", res);
+		return IOMEM_ERR_PTR(-EBUSY);
+	}
+
+	dest_ptr = devm_pci_remap_cfgspace(dev, res->start, size);
+	if (!dest_ptr) {
+		dev_err(dev, "ioremap failed for resource %pR\n", res);
+		devm_release_mem_region(dev, res->start, size);
+		dest_ptr = IOMEM_ERR_PTR(-ENOMEM);
+	}
+
+	return dest_ptr;
+}
+EXPORT_SYMBOL(devm_pci_remap_cfg_resource);
+
+/**
+ * pcim_set_mwi - a device-managed pci_set_mwi()
+ * @dev: the PCI device for which MWI is enabled
+ *
+ * Managed pci_set_mwi().
+ *
+ * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
+ */
+int pcim_set_mwi(struct pci_dev *dev)
+{
+	struct pci_devres *dr;
+
+	dr = find_pci_dr(dev);
+	if (!dr)
+		return -ENOMEM;
+
+	dr->mwi = 1;
+	return pci_set_mwi(dev);
+}
+EXPORT_SYMBOL(pcim_set_mwi);
+
+
+static void pcim_release(struct device *gendev, void *res)
+{
+	struct pci_dev *dev = to_pci_dev(gendev);
+	struct pci_devres *this = res;
+	int i;
+
+	for (i = 0; i < DEVICE_COUNT_RESOURCE; i++)
+		if (this->region_mask & (1 << i))
+			pci_release_region(dev, i);
+
+	if (this->mwi)
+		pci_clear_mwi(dev);
+
+	if (this->restore_intx)
+		pci_intx(dev, this->orig_intx);
+
+	if (this->enabled && !this->pinned)
+		pci_disable_device(dev);
+}
+
+/*
+ * TODO:
+ * Once the last four callers in pci.c are ported, this function here needs to
+ * be made static again.
+ */
+struct pci_devres *find_pci_dr(struct pci_dev *pdev)
+{
+	if (pci_is_managed(pdev))
+		return devres_find(&pdev->dev, pcim_release, NULL, NULL);
+	return NULL;
+}
+EXPORT_SYMBOL(find_pci_dr);
+
+static struct pci_devres *get_pci_dr(struct pci_dev *pdev)
+{
+	struct pci_devres *dr, *new_dr;
+
+	dr = devres_find(&pdev->dev, pcim_release, NULL, NULL);
+	if (dr)
+		return dr;
+
+	new_dr = devres_alloc(pcim_release, sizeof(*new_dr), GFP_KERNEL);
+	if (!new_dr)
+		return NULL;
+	return devres_get(&pdev->dev, new_dr, NULL, NULL);
+}
+
+/**
+ * pcim_enable_device - Managed pci_enable_device()
+ * @pdev: PCI device to be initialized
+ *
+ * Managed pci_enable_device().
+ */
+int pcim_enable_device(struct pci_dev *pdev)
+{
+	struct pci_devres *dr;
+	int rc;
+
+	dr = get_pci_dr(pdev);
+	if (unlikely(!dr))
+		return -ENOMEM;
+	if (dr->enabled)
+		return 0;
+
+	rc = pci_enable_device(pdev);
+	if (!rc) {
+		pdev->is_managed = 1;
+		dr->enabled = 1;
+	}
+	return rc;
+}
+EXPORT_SYMBOL(pcim_enable_device);
+
+/**
+ * pcim_pin_device - Pin managed PCI device
+ * @pdev: PCI device to pin
+ *
+ * Pin managed PCI device @pdev.  Pinned device won't be disabled on
+ * driver detach.  @pdev must have been enabled with
+ * pcim_enable_device().
+ */
+void pcim_pin_device(struct pci_dev *pdev)
+{
+	struct pci_devres *dr;
+
+	dr = find_pci_dr(pdev);
+	WARN_ON(!dr || !dr->enabled);
+	if (dr)
+		dr->pinned = 1;
+}
+EXPORT_SYMBOL(pcim_pin_device);
+
 static void pcim_iomap_release(struct device *gendev, void *res)
 {
 	struct pci_dev *dev = to_pci_dev(gendev);
drivers/pci/pci.c +0 -249

267 lines not on this page · raw message

drivers/pci/pci.h +24 -0
index 2336a8d1edab..2215858b2584 100644
@@ -797,6 +797,30 @@ static inline pci_power_t mid_pci_get_power_state(struct pci_dev *pdev)
 }
 #endif
 
+/*
+ * TODO:
+ * The following two components wouldn't need to be here if they weren't used at
+ * four last places in pci.c
+ * Port or move these functions to devres.c and then remove the
+ * pci_devres-components from this header file here.
+ */
+/*
+ * Managed PCI resources.  This manages device on/off, INTx/MSI/MSI-X
+ * on/off and BAR regions.  pci_dev itself records MSI/MSI-X status, so
+ * there's no need to track it separately.  pci_devres is initialized
+ * when a device is enabled using managed PCI device enable interface.
+ */
+struct pci_devres {
+	unsigned int enabled:1;
+	unsigned int pinned:1;
+	unsigned int orig_intx:1;
+	unsigned int restore_intx:1;
+	unsigned int mwi:1;
+	u32 region_mask;
+};
+
+struct pci_devres *find_pci_dr(struct pci_dev *pdev);
+
 /*
  * Config Address for PCI Configuration Mechanism #1
  *
Signature · 1 line
2.43.0
#8 lore raw

Re: [PATCH v6 4/4] PCI: Move devres code from pci.c to devres.c

drivers/pci/devres.c struct pcim_iomap_devres { · line 196

Quoted 11 lines · Philipp Stanner

The file pci.c is very large and contains a number of devres-functions. These functions should now reside in devres.c
...

drivers/pci/devres.c +7 -0
+struct pci_devres *find_pci_dr(struct pci_dev *pdev)
+{
+	if (pci_is_managed(pdev))
+		return devres_find(&pdev->dev, pcim_release, NULL, NULL);
+	return NULL;
+}
+EXPORT_SYMBOL(find_pci_dr);

find_pci_dr() was not previously exported, and I don't think it needs to be exported now, so I dropped the EXPORT_SYMBOL. It's still usable inside drivers/pci since it's declared in drivers/pci/pci.h; it's just not usable from modules. Let me know if I missed something.

Quoted 6 lines
drivers/pci/pci.c +0 -6
-static struct pci_devres *find_pci_dr(struct pci_dev *pdev)
-{
-	if (pci_is_managed(pdev))
-		return devres_find(&pdev->dev, pcim_release, NULL, NULL);
-	return NULL;
-}
#9 lore raw

Re: [PATCH v6 4/4] PCI: Move devres code from pci.c to devres.c

Quoted 6 lines · Bjorn Helgaas
Quoted 4 lines · Philipp Stanner

The file pci.c is very large and contains a number of devres- functions.
These functions should now reside in devres.c
...

Quoted 15 lines

Review

Quoted 8 lines
drivers/pci/devres.c +4 -0
+struct pci_devres *find_pci_dr(struct pci_dev *pdev)
+{
+       if (pci_is_managed(pdev))
+               return devres_find(&pdev->dev, pcim_release, NULL,

NULL);
+       return NULL;

(quoted diff) +2 -0
+}
+EXPORT_SYMBOL(find_pci_dr);

find_pci_dr() was not previously exported, and I don't think it needs to be exported now, so I dropped the EXPORT_SYMBOL.  It's still usable
inside drivers/pci since it's declared in drivers/pci/pci.h; it's just
not usable from modules.  Let me know if I missed something.

No, ACK, you are right.
I forgot this since find_pci_dr() is removed later anyways.

P.

Quoted 9 lines
Quoted 8 lines
drivers/pci/pci.c +0 -4
-static struct pci_devres *find_pci_dr(struct pci_dev *pdev)
-{
-       if (pci_is_managed(pdev))
-               return devres_find(&pdev->dev, pcim_release, NULL,

NULL);
-       return NULL;
-}

#10 lore raw
In reply to Philipp Stanner
Quoted 4 lines · Philipp Stanner

@Bjorn:
I decided that it's now actually possible to just embed the docu updates to the respective patches, instead of a separate patch.
Also dropped the ioport_unmap() for now.

Thanks. I didn't see any documentation updates (other than those related to the changed path names) in this series, so I assume the updates you mention would be in a future series.

Quoted 7 lines

...
Philipp Stanner (4):
lib/pci_iomap.c: fix cleanup bug in pci_iounmap()
lib: move pci_iomap.c to drivers/pci/
lib: move pci-specific devres code to drivers/pci/
PCI: Move devres code from pci.c to devres.c

Quoted 15 lines
 Documentation/driver-api/device-io.rst |   2 +-
 Documentation/driver-api/pci/pci.rst   |   6 +
 MAINTAINERS                            |   1 -
 drivers/pci/Kconfig                    |   5 +
 drivers/pci/Makefile                   |   3 +-
 drivers/pci/devres.c                   | 450 +++++++++++++++++++++++++
 lib/pci_iomap.c => drivers/pci/iomap.c |   5 +-
 drivers/pci/pci.c                      | 249 --------------
 drivers/pci/pci.h                      |  24 ++
 lib/Kconfig                            |   3 -
 lib/Makefile                           |   1 -
 lib/devres.c                           | 208 +-----------
 12 files changed, 490 insertions(+), 467 deletions(-)
 create mode 100644 drivers/pci/devres.c
 rename lib/pci_iomap.c => drivers/pci/iomap.c (99%)

Applied to pci/devres for v6.9, thanks!

#11 lore raw
Quoted 10 lines · Bjorn Helgaas
Quoted 5 lines · Philipp Stanner

@Bjorn:
I decided that it's now actually possible to just embed the docu updates
to the respective patches, instead of a separate patch. Also dropped the ioport_unmap() for now.

Thanks.  I didn't see any documentation updates (other than those related to the changed path names) in this series, so I assume the updates you mention would be in a future series.

No, I actually meant the changed path names.

The next series (new devres functions) just adds more docstrings to iomap.c, devres.c and pci.c in drivers/pci/, which, after this series here is applied, are all already added to the Docu.

Quoted 25 lines
Quoted 21 lines

...
Philipp Stanner (4):
  lib/pci_iomap.c: fix cleanup bug in pci_iounmap()
  lib: move pci_iomap.c to drivers/pci/
  lib: move pci-specific devres code to drivers/pci/
  PCI: Move devres code from pci.c to devres.c

 Documentation/driver-api/device-io.rst |   2 +- Documentation/driver-api/pci/pci.rst   |   6 + MAINTAINERS                            |   1 - drivers/pci/Kconfig                    |   5 + drivers/pci/Makefile                   |   3 +- drivers/pci/devres.c                   | 450
+++++++++++++++++++++++++
 lib/pci_iomap.c => drivers/pci/iomap.c |   5 +- drivers/pci/pci.c                      | 249 -------------- drivers/pci/pci.h                      |  24 ++ lib/Kconfig                            |   3 - lib/Makefile                           |   1 - lib/devres.c                           | 208 +-----------

Quoted 3 lines
 12 files changed, 490 insertions(+), 467 deletions(-)

 create mode 100644 drivers/pci/devres.c rename lib/pci_iomap.c => drivers/pci/iomap.c (99%)

Quoted 1 line

Applied to pci/devres for v6.9, thanks!

Thx!

#12 lore raw
Quoted 12 lines · Philipp Stanner
Quoted 10 lines · Bjorn Helgaas
Quoted 5 lines · Philipp Stanner

@Bjorn:
I decided that it's now actually possible to just embed the docu updates
to the respective patches, instead of a separate patch. Also dropped the ioport_unmap() for now.

Thanks.  I didn't see any documentation updates (other than those related to the changed path names) in this series, so I assume the updates you mention would be in a future series.

Quoted 5 lines

No, I actually meant the changed path names.

The next series (new devres functions) just adds more docstrings to iomap.c, devres.c and pci.c in drivers/pci/, which, after this series here is applied, are all already added to the Docu.

OK. Other doc issues, I'm sure you've seen already:
https://lore.kernel.org/r/[email protected]

I'll squash the fixes into this series when they're ready.

Bjorn