On Tue, Jun 02, 2015 at 02:11:24PM +1000, Gavin Shan wrote:

On Tue, Jun 02, 2015 at 11:51:15AM +0800, Wei Yang wrote:

quoted

On Mon, Jun 01, 2015 at 06:32:33PM -0500, Bjorn Helgaas wrote:

quoted

The subject says "Trace first 7 BARs..."  I think maybe you meant "Track
first 7 BARs" or maybe "Cache only BARs, not windows or IOV BARs"

Agree, Track is more accurate.

Gavin,

Which subject you prefer?

"Cache only BARs, not windows or IOV BARs" is better.

quoted

quoted

On Tue, May 19, 2015 at 06:50:06PM +0800, Wei Yang wrote:

quoted

EEH address cache, which helps to locate the PCI device according to
the given (physical) MMIO address, didn't cover PCI bridges. 

"doesn't contain PCI bridge windows"?

I see that eeh_addr_cache_insert_dev() ignores bridges because it never
calls __eeh_addr_cache_insert_dev() when "(dev->class >> 16) ==
PCI_BASE_CLASS_BRIDGE".  I think it would be more technically correct if
you removed that test and relied on the "i <= PCI_ROM_RESOURCE" test in
this patch, because it is legal (though rare) for bridge devices to have
two BARs, and I assume you would want to put those in your cache if they
exist.

I think this is fine to remove the test "(dev->class >> 16) ==
PCI_BASE_CLASS_BRIDGE" for a bridge and rely on the "i <= PCI_ROM_RESOURCE"

Gavin,

Do you thinks this is fine?

Fine to me.

Thanks, I will change accordingly and do some tests.

quoted

quoted

quoted

Also, it
shouldn't return PF with address in PF's IOV BARs. Instead, the VFs
should be returned.
The patch restricts the address cache to cover first 7 BARs for the
above purposes.

[gwshan: changelog]
Signed-off-by: Wei Yang <redacted>
Acked-by: Gavin Shan <redacted>
---
 arch/powerpc/kernel/eeh_cache.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
index eeabeab..f6c5f05 100644
--- a/arch/powerpc/kernel/eeh_cache.c
+++ b/arch/powerpc/kernel/eeh_cache.c

@@ -196,7 +196,7 @@ static void __eeh_addr_cache_insert_dev(struct pci_dev *dev)
 	}
 
 	/* Walk resources on this device, poke them into the tree */
-	for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
+	for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
 		unsigned long start = pci_resource_start(dev,i);
 		unsigned long end = pci_resource_end(dev,i);
 		unsigned int flags = pci_resource_flags(dev,i);

-- 
1.7.9.5

-- 
Richard Yang
Help you, Help me

-- 
Richard Yang
Help you, Help me