diff mbox

[1/8] pci: Add bitmap to know if a pci device has cfg reg filters

Message ID 20170605225924.11416-1-benh@kernel.crashing.org
State Accepted
Headers show

Commit Message

Benjamin Herrenschmidt June 5, 2017, 10:59 p.m. UTC 
This avoids doing a search through the list of all devices on
every config space access to every device under a PHB.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 core/pci-iov.c |  1 +
 core/pci.c     | 10 ++++++++++
 hw/phb3.c      |  9 ++-------
 include/pci.h  |  4 ++++
 4 files changed, 17 insertions(+), 7 deletions(-)

Comments

Stewart Smith June 6, 2017, 11:22 a.m. UTC | #1 
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> This avoids doing a search through the list of all devices on
> every config space access to every device under a PHB.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Series appears to make sense, merged to master as of
41b14f9452c25f3dd74a8304763a3cc58bff019f.

On the 20ms before checking presence detect, is this something we should
backport to stable so we get into various service packs? Or more of a
new hardware thing?
Benjamin Herrenschmidt June 6, 2017, 1:04 p.m. UTC | #2 
On Tue, 2017-06-06 at 21:22 +1000, Stewart Smith wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> > This avoids doing a search through the list of all devices on
> > every config space access to every device under a PHB.
> > 
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Series appears to make sense, merged to master as of
> 41b14f9452c25f3dd74a8304763a3cc58bff019f.
> 
> On the 20ms before checking presence detect, is this something we should
> backport to stable so we get into various service packs? Or more of a
> new hardware thing?

I wouldn't bother, we never hit it in practice, I think the fact that
we init the NPU in between the PHB and the probe is "enough"... pHyp
does hit it though every now and then after refactoring their code so
I'd rather be safe.

Cheers,
Ben.
diff mbox

Patch

diff --git a/core/pci-iov.c b/core/pci-iov.c
index 14c810b..6abb85a 100644
--- a/core/pci-iov.c
+++ b/core/pci-iov.c
@@ -184,6 +184,7 @@  static void pci_iov_init_VF(struct pci_device *pd, struct pci_device *vf)
 	vf->dn			= NULL;
 	vf->slot		= NULL;
 	vf->parent		= pd;
+	vf->phb			= pd->phb;
 	list_head_init(&vf->pcrf);
 	list_head_init(&vf->children);
 }
diff --git a/core/pci.c b/core/pci.c
index 0adc6d2..5671b69 100644
--- a/core/pci.c
+++ b/core/pci.c
@@ -247,6 +247,7 @@  static struct pci_device *pci_scan_one(struct phb *phb, struct pci_device *paren
 		PCIERR(phb, bdfn,"Failed to allocate structure pci_device !\n");
 		goto fail;
 	}
+	pd->phb = phb;
 	pd->bdfn = bdfn;
 	pd->vdid = vdid;
 	pci_cfg_read32(phb, bdfn, PCI_CFG_SUBSYS_VENDOR_ID, &pd->sub_vdid);
@@ -951,6 +952,9 @@  int64_t pci_register_phb(struct phb *phb, int opal_id)
 	init_lock(&phb->lock);
 	list_head_init(&phb->devices);
 
+	phb->filter_map = zalloc(BITMAP_BYTES(0x10000));
+	assert(phb->filter_map);
+
 	return OPAL_SUCCESS;
 }
 
@@ -1764,6 +1768,11 @@  struct pci_cfg_reg_filter *pci_find_cfg_reg_filter(struct pci_device *pd,
 	return NULL;
 }
 
+bool pci_device_has_cfg_reg_filters(struct phb *phb, uint16_t bdfn)
+{
+       return bitmap_tst_bit(*phb->filter_map, bdfn);
+}
+
 struct pci_cfg_reg_filter *pci_add_cfg_reg_filter(struct pci_device *pd,
 						  uint32_t start, uint32_t len,
 						  uint32_t flags,
@@ -1793,6 +1802,7 @@  struct pci_cfg_reg_filter *pci_add_cfg_reg_filter(struct pci_device *pd,
 	if (pd->pcrf_end < (start + len))
 		pd->pcrf_end = start + len;
 	list_add_tail(&pd->pcrf, &pcrf->link);
+	bitmap_set_bit(*pd->phb->filter_map, pd->bdfn);
 
 	return pcrf;
 }
diff --git a/hw/phb3.c b/hw/phb3.c
index b33ed7b..c99f1de 100644
--- a/hw/phb3.c
+++ b/hw/phb3.c
@@ -236,13 +236,8 @@  static void phb3_pcicfg_filter(struct phb *phb, uint32_t bdfn,
 		return;
 	}
 
-	/* FIXME: It harms the performance to search the PCI
-	 * device which doesn't have any filters at all. So
-	 * it's worthy to maintain a table in PHB to indicate
-	 * the PCI devices who have filters. However, bitmap
-	 * seems not supported by skiboot yet. To implement
-	 * it after bitmap is supported.
-	 */
+	if (!pci_device_has_cfg_reg_filters(phb, bdfn))
+		return;
 	pd = pci_find_dev(phb, bdfn);
 	pcrf = pd ? pci_find_cfg_reg_filter(pd, offset, len) : NULL;
 	if (!pcrf || !pcrf->func)
diff --git a/include/pci.h b/include/pci.h
index dc418a9..1e84b51 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -20,6 +20,7 @@ 
 #include <opal.h>
 #include <device.h>
 #include <lock.h>
+#include <bitmap.h>
 #include <ccan/list/list.h>
 
 struct pci_device;
@@ -87,6 +88,7 @@  struct pci_device {
 	struct dt_node		*dn;
 	struct pci_slot		*slot;
 	struct pci_device	*parent;
+	struct phb		*phb;
 	struct list_head	children;
 	struct list_node	link;
 };
@@ -343,6 +345,7 @@  struct phb {
 	const struct phb_ops	*ops;
 	struct pci_lsi_state	lstate;
 	uint32_t		mps;
+	bitmap_t		*filter_map;
 
 	/* PCI-X only slot info, for PCI-E this is in the RC bridge */
 	struct pci_slot		*slot;
@@ -424,6 +427,7 @@  extern struct pci_device *pci_walk_dev(struct phb *phb,
 				       void *userdata);
 extern struct pci_device *pci_find_dev(struct phb *phb, uint16_t bdfn);
 extern void pci_restore_bridge_buses(struct phb *phb, struct pci_device *pd);
+extern bool pci_device_has_cfg_reg_filters(struct phb *phb, uint16_t bdfn);
 extern struct pci_cfg_reg_filter *pci_find_cfg_reg_filter(struct pci_device *pd,
 					uint32_t start, uint32_t len);
 extern struct pci_cfg_reg_filter *pci_add_cfg_reg_filter(struct pci_device *pd,