diff mbox

[10/13] PCI: Only treat non-pref mmio64 as pref if all bridges have MEM_64

Message ID 20170421050500.13957-11-yinghai@kernel.org (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Yinghai Lu April 21, 2017, 5:04 a.m. UTC 
If any bridge up to root only have 32bit pref mmio, We don't need to
treat device non-pref mmio64 as as pref mmio64.

We need to move pci_bridge_check_ranges calling early.
For parent bridges pref mmio BAR may not allocated by BIOS, res flags
is still 0, we need to have it correct set before we check them for
child device resources.

-v2: check all bus resources instead of just res[15].

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Tested-by: Khalid Aziz <khalid.aziz@oracle.com>
---
 drivers/pci/setup-bus.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

Comments

Bjorn Helgaas May 4, 2017, 9:43 p.m. UTC | #1 
On Thu, Apr 20, 2017 at 10:04:57PM -0700, Yinghai Lu wrote:
> If any bridge up to root only have 32bit pref mmio, We don't need to
> treat device non-pref mmio64 as as pref mmio64.

I don't understand the reasoning here.  

Apparently it is only safe to put a non-prefetchable PCIe BAR in a
prefetchable window if all upstream bridges have a 64-bit prefetchable
window?  Why is that?  Why is it important that the prefetchable
window be 64-bit?  I don't see this mentioned in the implementation
note.

Does this mean "PCI: Check pref compatible bit for mem64 resource of
PCIe device" can make unsafe assignments until we apply this patch?
If so, I don't want that sort of bisection hole.

What bad thing happens without this patch?

> We need to move pci_bridge_check_ranges calling early.
> For parent bridges pref mmio BAR may not allocated by BIOS, res flags
> is still 0, we need to have it correct set before we check them for
> child device resources.
> 
> -v2: check all bus resources instead of just res[15].
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Tested-by: Khalid Aziz <khalid.aziz@oracle.com>
> ---
>  drivers/pci/setup-bus.c | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 3de66e6..b3fd314 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -735,6 +735,29 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i)
>  	return -EINVAL;
>  }
>  
> +static bool pci_up_path_over_pref_mem64(struct pci_bus *bus)
> +{
> +	if (pci_is_root_bus(bus))
> +		return true;
> +
> +	if (bus->self) {
> +		int i;
> +		bool found = false;
> +		struct resource *res;
> +
> +		pci_bus_for_each_resource(bus, res, i)
> +			if (res->flags & IORESOURCE_MEM_64) {
> +				found = true;
> +				break;
> +			}
> +
> +		if (!found)
> +			return false;
> +	}
> +
> +	return pci_up_path_over_pref_mem64(bus->parent);
> +}
> +
>  int pci_resource_pref_compatible(const struct pci_dev *dev,
>  				 struct resource *res)
>  {
> @@ -743,7 +766,8 @@ int pci_resource_pref_compatible(const struct pci_dev *dev,
>  
>  	if ((res->flags & IORESOURCE_MEM) &&
>  	    (res->flags & IORESOURCE_MEM_64) &&
> -	    dev->on_all_pcie_path)
> +	    dev->on_all_pcie_path &&
> +	    pci_up_path_over_pref_mem64(dev->bus))
>  		return res->flags | IORESOURCE_PREFETCH;
>  
>  	return res->flags;
> @@ -1236,6 +1260,10 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
>  	struct resource *b_res;
>  	int ret;
>  
> +	if (!pci_is_root_bus(bus) &&
> +	    (bus->self->class >> 8) == PCI_CLASS_BRIDGE_PCI)
> +		pci_bridge_check_ranges(bus);
> +
>  	list_for_each_entry(dev, &bus->devices, bus_list) {
>  		struct pci_bus *b = dev->subordinate;
>  		if (!b)
> @@ -1263,7 +1291,6 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
>  		break;
>  
>  	case PCI_CLASS_BRIDGE_PCI:
> -		pci_bridge_check_ranges(bus);
>  		if (bus->self->is_hotplug_bridge) {
>  			additional_io_size  = pci_hotplug_io_size;
>  			additional_mem_size = pci_hotplug_mem_size;
> -- 
> 2.9.3
>
diff mbox

Patch

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 3de66e6..b3fd314 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -735,6 +735,29 @@  int pci_claim_bridge_resource(struct pci_dev *bridge, int i)
 	return -EINVAL;
 }
 
+static bool pci_up_path_over_pref_mem64(struct pci_bus *bus)
+{
+	if (pci_is_root_bus(bus))
+		return true;
+
+	if (bus->self) {
+		int i;
+		bool found = false;
+		struct resource *res;
+
+		pci_bus_for_each_resource(bus, res, i)
+			if (res->flags & IORESOURCE_MEM_64) {
+				found = true;
+				break;
+			}
+
+		if (!found)
+			return false;
+	}
+
+	return pci_up_path_over_pref_mem64(bus->parent);
+}
+
 int pci_resource_pref_compatible(const struct pci_dev *dev,
 				 struct resource *res)
 {
@@ -743,7 +766,8 @@  int pci_resource_pref_compatible(const struct pci_dev *dev,
 
 	if ((res->flags & IORESOURCE_MEM) &&
 	    (res->flags & IORESOURCE_MEM_64) &&
-	    dev->on_all_pcie_path)
+	    dev->on_all_pcie_path &&
+	    pci_up_path_over_pref_mem64(dev->bus))
 		return res->flags | IORESOURCE_PREFETCH;
 
 	return res->flags;
@@ -1236,6 +1260,10 @@  void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 	struct resource *b_res;
 	int ret;
 
+	if (!pci_is_root_bus(bus) &&
+	    (bus->self->class >> 8) == PCI_CLASS_BRIDGE_PCI)
+		pci_bridge_check_ranges(bus);
+
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		struct pci_bus *b = dev->subordinate;
 		if (!b)
@@ -1263,7 +1291,6 @@  void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 		break;
 
 	case PCI_CLASS_BRIDGE_PCI:
-		pci_bridge_check_ranges(bus);
 		if (bus->self->is_hotplug_bridge) {
 			additional_io_size  = pci_hotplug_io_size;
 			additional_mem_size = pci_hotplug_mem_size;