pci: pci_host_bridge_get_paddr() can not fail

Alexander Gordeev (2):
pci: Make all ones invalid translate address
pci: pci_host_bridge_get_paddr() can not fail

 lib/pci-host-generic.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

1.8.3.1

Function pci_host_bridge_get_paddr() returns zero in case no mapping for a passed PCI address is found. All ones is a better choice, since zero may be a legitimate physical address.

---
 lib/pci-host-generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index d57b47ee4c53..7f72d649144e 100644
@@ -271,7 +271,7 @@ phys_addr_t pci_host_bridge_get_paddr(u64 pci_addr)
 		as++;
 	}
 
-	return 0;
+	return ~0;
 }
 
 static void __iomem *pci_get_dev_conf(struct pci_host_bridge *host, int devfn)

1.8.3.1

Function pci_host_bridge_get_paddr() is passed a PCI bus address to translate into physical one. The PCI address is always read from a device BAR and is expected to be valid. Therefore, a failure to find the mapping for the PCI address indicates a serious implementation problem elsewhere.

---
 lib/pci-host-generic.c | 1 +
 1 file changed, 1 insertion(+)

index 7f72d649144e..433a0c8e4d15 100644
@@ -271,6 +271,7 @@ phys_addr_t pci_host_bridge_get_paddr(u64 pci_addr)
 		as++;
 	}
 
+	assert(0);
 	return ~0;
 }
 

1.8.3.1

I applied the first to arm/next, but not the other. I think returning ~0 makes more sense here, and that callers (even callers that are part of the API) should consider asserting on receiving it.

As we discussed, low-level API should return whatever it finds/doesn't find (no asserts). Higher-level API can assert on invalid inputs and unexpected results from the low-level API.

Additionally, this series doesn't solve the problem I reported, which is, if you do

pci_bar_get_addr(any_device, invalid_bar_num)

you get back the base address of the PIO region. That happens because pci_bar_get(any_device, invalid_bar_num) returns zero and then that zero gets translated correctly by pci_translate_addr to the PIO region's physical base address.

I think we should look into adding asserts to pci_bar_get/set_addr and the other higher-level API calls instead. We should add

assert(pci_bar_is_valid(bar_num))

many places, and pci_bar_get_addr should have

phys_addr = pci_translate_addr(dev, pci_addr)
assert(phys_addr != ~0)

Thanks,
drew