Patchwork [v7] PCI: Try best to allocate pref mmio 64bit above 4g

login
register
mail settings
Submitter Yinghai Lu
Date April 16, 2014, 2:29 a.m.
Message ID <CAE9FiQUv7yfUDsTa_N=FFARRYFmrCxRVN-vCp3NikSf+ARkPdA@mail.gmail.com>
Download mbox | patch
Permalink /patch/339415/
State Superseded
Headers show

Comments

Yinghai Lu - April 16, 2014, 2:29 a.m.
On Tue, Apr 15, 2014 at 5:09 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> It went wrong at the beginning. Note the error message never considers
>> 64-bit or not, but BAR 15 here has it MEM_64 flag cleared.
>
> BAR 15 is a bridge window.  I think its resource flags should reflect
> the capability of the *window*, even if we disable the window or we
> happen to assign addresses that are under 4GB.  So I think it's wrong
> that we clear the MEM_64 flag  in pbus_size_mem() and the IO flag in
> pbus_size_io().

We keep PCI_PREF_RANGE_TYPE_64 in the flag.

We should check that before we touch PCI_PREF_LIMIT_UPPER32 BAR in
pci_setup_bridge_mmio_pref()

like following:

Subject: [PATCH] PCI: only touch upper 32bit for pref64bit bridge bar

If the bridge pref mmio bar does not support 64bit, we should touch
upper 32bit.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/pci/setup-bus.c |   14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
Bjorn Helgaas - April 16, 2014, 5:06 p.m.
On Tue, Apr 15, 2014 at 8:29 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Tue, Apr 15, 2014 at 5:09 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> It went wrong at the beginning. Note the error message never considers
>>> 64-bit or not, but BAR 15 here has it MEM_64 flag cleared.
>>
>> BAR 15 is a bridge window.  I think its resource flags should reflect
>> the capability of the *window*, even if we disable the window or we
>> happen to assign addresses that are under 4GB.  So I think it's wrong
>> that we clear the MEM_64 flag  in pbus_size_mem() and the IO flag in
>> pbus_size_io().
>
> We keep PCI_PREF_RANGE_TYPE_64 in the flag.

I think we should stop putting PCI_PREF_RANGE_TYPE_64 in res->flags.
I don't think we ever test for PCI_PREF_RANGE_TYPE_64 today, except
for the trivial usage in pci_read_bridge_mmio_pref(), which can be
easily removed.

Everywhere else, we use IORESOURCE_MEM, IORESOURCE_MEM_64, and
IORESOURCE_PREFETCH to keep track of the properties of the hardware
underlying the struct resource, i.e., the BAR.  Why shouldn't we do
the same for bridge windows?

The struct resource flags should tell us the *possible* properties of
the window, e.g., what the bridge can support.  These properties are
fixed by the bridge hardware, regardless of what devices happen to be
below the bridge.  We discover those properties at enumeration-time,
and we shouldn't change them afterwards.

> We should check that before we touch PCI_PREF_LIMIT_UPPER32 BAR in
> pci_setup_bridge_mmio_pref()
>
> like following:
>
> Subject: [PATCH] PCI: only touch upper 32bit for pref64bit bridge bar
>
> If the bridge pref mmio bar does not support 64bit, we should touch
> upper 32bit.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>
> ---
>  drivers/pci/setup-bus.c |   14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -592,23 +592,23 @@ static void pci_setup_bridge_mmio(struct
>  static void pci_setup_bridge_mmio_pref(struct pci_bus *bus)
>  {
>      struct pci_dev *bridge = bus->self;
> -    struct resource *res;
> +    struct resource *res = bus->resource[2];
>      struct pci_bus_region region;
>      u32 l, bu, lu;
>
>      /* Clear out the upper 32 bits of PREF limit.
>         If PCI_PREF_BASE_UPPER32 was non-zero, this temporarily
>         disables PREF range, which is ok. */
> -    pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, 0);
> +    if (res->flags & PCI_PREF_RANGE_TYPE_64)
> +        pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, 0);
>
>      /* Set up PREF base/limit. */
>      bu = lu = 0;
> -    res = bus->resource[2];
>      pcibios_resource_to_bus(bridge->bus, &region, res);
>      if (res->flags & IORESOURCE_PREFETCH) {
>          l = (region.start >> 16) & 0xfff0;
>          l |= region.end & 0xfff00000;
> -        if (res->flags & IORESOURCE_MEM_64) {
> +        if (res->flags & PCI_PREF_RANGE_TYPE_64) {
>              bu = upper_32_bits(region.start);
>              lu = upper_32_bits(region.end);
>          }
> @@ -619,8 +619,10 @@ static void pci_setup_bridge_mmio_pref(s
>      pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, l);
>
>      /* Set the upper 32 bits of PREF base & limit. */
> -    pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, bu);
> -    pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu);
> +    if (res->flags & PCI_PREF_RANGE_TYPE_64) {
> +        pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, bu);
> +        pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu);
> +    }
>  }
>
>  static void __pci_setup_bridge(struct pci_bus *bus, unsigned long type)
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

Subject: [PATCH] PCI: only touch upper 32bit for pref64bit bridge bar

If the bridge pref mmio bar does not support 64bit, we should touch
upper 32bit.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/pci/setup-bus.c |   14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -592,23 +592,23 @@  static void pci_setup_bridge_mmio(struct
 static void pci_setup_bridge_mmio_pref(struct pci_bus *bus)
 {
 	struct pci_dev *bridge = bus->self;
-	struct resource *res;
+	struct resource *res = bus->resource[2];
 	struct pci_bus_region region;
 	u32 l, bu, lu;
 
 	/* Clear out the upper 32 bits of PREF limit.
 	   If PCI_PREF_BASE_UPPER32 was non-zero, this temporarily
 	   disables PREF range, which is ok. */
-	pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, 0);
+	if (res->flags & PCI_PREF_RANGE_TYPE_64)
+		pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, 0);
 
 	/* Set up PREF base/limit. */
 	bu = lu = 0;
-	res = bus->resource[2];
 	pcibios_resource_to_bus(bridge->bus, &region, res);
 	if (res->flags & IORESOURCE_PREFETCH) {
 		l = (region.start >> 16) & 0xfff0;
 		l |= region.end & 0xfff00000;
-		if (res->flags & IORESOURCE_MEM_64) {
+		if (res->flags & PCI_PREF_RANGE_TYPE_64) {
 			bu = upper_32_bits(region.start);
 			lu = upper_32_bits(region.end);
 		}
@@ -619,8 +619,10 @@  static void pci_setup_bridge_mmio_pref(s
 	pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, l);
 
 	/* Set the upper 32 bits of PREF base & limit. */
-	pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, bu);
-	pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu);
+	if (res->flags & PCI_PREF_RANGE_TYPE_64) {
+		pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, bu);
+		pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu);
+	}
 }
 
 static void __pci_setup_bridge(struct pci_bus *bus, unsigned long type)