Discussion:
[PATCH 0/2] PCI: Don't enable realloc automatically when above 4g 64bit mmio is not supported
Yinghai Lu
2013-12-08 23:54:27 UTC
Permalink
On one system (Dell) realloc auto cause ixgbe does not work,
the system does not have enough range for sriov and rom.

We could try to tighten the condition about enabling realloc auto.
Not enabling realloc auto when above 64bit mmio is not there for
root bus.
So the system does not need to take "pci=realloc=off" to have working
network as old kernel.

First patch is preparing for bus address converting with bus instead of dev.

After those two patches get into pci-next or linus-tree, I will send
other patches about 64bit resource allocation.

Thanks

Yinghai

Yinghai Lu (2):
PCI: pcibus address to resource converting take bus instead of dev
PCI: Only enable realloc auto when root bus has 64bit mmio

drivers/pci/host-bridge.c | 34 +++++++++++++++++++++-------------
drivers/pci/setup-bus.c | 32 +++++++++++++++++++++++++++-----
include/linux/pci.h | 5 +++++
3 files changed, 53 insertions(+), 18 deletions(-)

--
1.8.4
Yinghai Lu
2013-12-08 23:54:28 UTC
Permalink
For allocating resource under bus path, we do not have dev to pass along,
and we only have bus to use instead.

-v2: drop pcibios_bus_addr_to_resource().

Signed-off-by: Yinghai Lu <***@kernel.org>
Cc: ***@vger.kernel.org
---
drivers/pci/host-bridge.c | 34 +++++++++++++++++++++-------------
include/linux/pci.h | 5 +++++
2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
index a68dc61..c988a30 100644
--- a/drivers/pci/host-bridge.c
+++ b/drivers/pci/host-bridge.c
@@ -9,22 +9,19 @@

#include "pci.h"

-static struct pci_bus *find_pci_root_bus(struct pci_dev *dev)
+static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
{
- struct pci_bus *bus;
-
- bus = dev->bus;
while (bus->parent)
bus = bus->parent;

return bus;
}

-static struct pci_host_bridge *find_pci_host_bridge(struct pci_dev *dev)
+static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
{
- struct pci_bus *bus = find_pci_root_bus(dev);
+ struct pci_bus *root_bus = find_pci_root_bus(bus);

- return to_pci_host_bridge(bus->bridge);
+ return to_pci_host_bridge(root_bus->bridge);
}

void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
@@ -40,10 +37,11 @@ static bool resource_contains(struct resource *res1, struct resource *res2)
return res1->start <= res2->start && res1->end >= res2->end;
}

-void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
- struct resource *res)
+void __pcibios_resource_to_bus(struct pci_bus *bus,
+ struct pci_bus_region *region,
+ struct resource *res)
{
- struct pci_host_bridge *bridge = find_pci_host_bridge(dev);
+ struct pci_host_bridge *bridge = find_pci_host_bridge(bus);
struct pci_host_bridge_window *window;
resource_size_t offset = 0;

@@ -60,6 +58,11 @@ void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
region->start = res->start - offset;
region->end = res->end - offset;
}
+void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
+ struct resource *res)
+{
+ __pcibios_resource_to_bus(dev->bus, region, res);
+}
EXPORT_SYMBOL(pcibios_resource_to_bus);

static bool region_contains(struct pci_bus_region *region1,
@@ -68,10 +71,10 @@ static bool region_contains(struct pci_bus_region *region1,
return region1->start <= region2->start && region1->end >= region2->end;
}

-void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
- struct pci_bus_region *region)
+void __pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
+ struct pci_bus_region *region)
{
- struct pci_host_bridge *bridge = find_pci_host_bridge(dev);
+ struct pci_host_bridge *bridge = find_pci_host_bridge(bus);
struct pci_host_bridge_window *window;
resource_size_t offset = 0;

@@ -93,4 +96,9 @@ void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
res->start = region->start + offset;
res->end = region->end + offset;
}
+void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
+ struct pci_bus_region *region)
+{
+ __pcibios_bus_to_resource(dev->bus, res, region);
+}
EXPORT_SYMBOL(pcibios_bus_to_resource);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1084a15..5c2cf8e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -737,8 +737,13 @@ void pci_fixup_cardbus(struct pci_bus *);

/* Generic PCI functions used internally */

+void __pcibios_resource_to_bus(struct pci_bus *bus,
+ struct pci_bus_region *region,
+ struct resource *res);
void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
struct resource *res);
+void __pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
+ struct pci_bus_region *region);
void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
struct pci_bus_region *region);
void pcibios_scan_specific_bus(int busn);
--
1.8.4
Bjorn Helgaas
2013-12-09 17:52:15 UTC
Permalink
On Sun, Dec 8, 2013 at 4:54 PM, Yinghai Lu <***@kernel.org> wrote:
> For allocating resource under bus path, we do not have dev to pass along,
> and we only have bus to use instead.
>
> -v2: drop pcibios_bus_addr_to_resource().
>
> Signed-off-by: Yinghai Lu <***@kernel.org>
> Cc: ***@vger.kernel.org
> ---
> drivers/pci/host-bridge.c | 34 +++++++++++++++++++++-------------
> include/linux/pci.h | 5 +++++
> 2 files changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> index a68dc61..c988a30 100644
> --- a/drivers/pci/host-bridge.c
> +++ b/drivers/pci/host-bridge.c
> @@ -9,22 +9,19 @@
>
> #include "pci.h"
>
> -static struct pci_bus *find_pci_root_bus(struct pci_dev *dev)
> +static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
> {
> - struct pci_bus *bus;
> -
> - bus = dev->bus;
> while (bus->parent)
> bus = bus->parent;
>
> return bus;
> }
>
> -static struct pci_host_bridge *find_pci_host_bridge(struct pci_dev *dev)
> +static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
> {
> - struct pci_bus *bus = find_pci_root_bus(dev);
> + struct pci_bus *root_bus = find_pci_root_bus(bus);
>
> - return to_pci_host_bridge(bus->bridge);
> + return to_pci_host_bridge(root_bus->bridge);
> }
>
> void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
> @@ -40,10 +37,11 @@ static bool resource_contains(struct resource *res1, struct resource *res2)
> return res1->start <= res2->start && res1->end >= res2->end;
> }
>
> -void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
> - struct resource *res)
> +void __pcibios_resource_to_bus(struct pci_bus *bus,
> + struct pci_bus_region *region,
> + struct resource *res)
> {
> - struct pci_host_bridge *bridge = find_pci_host_bridge(dev);
> + struct pci_host_bridge *bridge = find_pci_host_bridge(bus);
> struct pci_host_bridge_window *window;
> resource_size_t offset = 0;
>
> @@ -60,6 +58,11 @@ void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
> region->start = res->start - offset;
> region->end = res->end - offset;
> }
> +void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
> + struct resource *res)
> +{
> + __pcibios_resource_to_bus(dev->bus, region, res);
> +}
> EXPORT_SYMBOL(pcibios_resource_to_bus);
>
> static bool region_contains(struct pci_bus_region *region1,
> @@ -68,10 +71,10 @@ static bool region_contains(struct pci_bus_region *region1,
> return region1->start <= region2->start && region1->end >= region2->end;
> }
>
> -void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
> - struct pci_bus_region *region)
> +void __pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
> + struct pci_bus_region *region)
> {
> - struct pci_host_bridge *bridge = find_pci_host_bridge(dev);
> + struct pci_host_bridge *bridge = find_pci_host_bridge(bus);
> struct pci_host_bridge_window *window;
> resource_size_t offset = 0;
>
> @@ -93,4 +96,9 @@ void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
> res->start = region->start + offset;
> res->end = region->end + offset;
> }
> +void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
> + struct pci_bus_region *region)
> +{
> + __pcibios_bus_to_resource(dev->bus, res, region);
> +}
> EXPORT_SYMBOL(pcibios_bus_to_resource);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 1084a15..5c2cf8e 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -737,8 +737,13 @@ void pci_fixup_cardbus(struct pci_bus *);
>
> /* Generic PCI functions used internally */
>
> +void __pcibios_resource_to_bus(struct pci_bus *bus,
> + struct pci_bus_region *region,
> + struct resource *res);
> void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
> struct resource *res);
> +void __pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
> + struct pci_bus_region *region);
> void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
> struct pci_bus_region *region);
> void pcibios_scan_specific_bus(int busn);

Ugh. Why would we add new functions with "__" prefixes? There are
only a handful of callers to pcibios_bus_to_resource() and
pcibios_resource_to_bus(). We can just change them to supply a
"struct pci_bus *" directly. That's a better match for how the
hardware works anyway.

Bjorn
Yinghai Lu
2013-12-09 19:21:53 UTC
Permalink
On Mon, Dec 9, 2013 at 9:52 AM, Bjorn Helgaas <***@google.com> wrote:
> On Sun, Dec 8, 2013 at 4:54 PM, Yinghai Lu <***@kernel.org> wrote:
>> For allocating resource under bus path, we do not have dev to pass along,
>> and we only have bus to use instead.
>>
>> -v2: drop pcibios_bus_addr_to_resource().
>>
>> Signed-off-by: Yinghai Lu <***@kernel.org>
>> Cc: ***@vger.kernel.org
>> ---
>> drivers/pci/host-bridge.c | 34 +++++++++++++++++++++-------------
>> include/linux/pci.h | 5 +++++
>> 2 files changed, 26 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
>> index a68dc61..c988a30 100644
>> --- a/drivers/pci/host-bridge.c
>> +++ b/drivers/pci/host-bridge.c
>> @@ -9,22 +9,19 @@
>>
>> #include "pci.h"
>>
>> -static struct pci_bus *find_pci_root_bus(struct pci_dev *dev)
>> +static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
>> {
>> - struct pci_bus *bus;
>> -
>> - bus = dev->bus;
>> while (bus->parent)
>> bus = bus->parent;
>>
>> return bus;
>> }
>>
>> -static struct pci_host_bridge *find_pci_host_bridge(struct pci_dev *dev)
>> +static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
>> {
>> - struct pci_bus *bus = find_pci_root_bus(dev);
>> + struct pci_bus *root_bus = find_pci_root_bus(bus);
>>
>> - return to_pci_host_bridge(bus->bridge);
>> + return to_pci_host_bridge(root_bus->bridge);
>> }
>>
>> void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
>> @@ -40,10 +37,11 @@ static bool resource_contains(struct resource *res1, struct resource *res2)
>> return res1->start <= res2->start && res1->end >= res2->end;
>> }
>>
>> -void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
>> - struct resource *res)
>> +void __pcibios_resource_to_bus(struct pci_bus *bus,
>> + struct pci_bus_region *region,
>> + struct resource *res)
>> {
>> - struct pci_host_bridge *bridge = find_pci_host_bridge(dev);
>> + struct pci_host_bridge *bridge = find_pci_host_bridge(bus);
>> struct pci_host_bridge_window *window;
>> resource_size_t offset = 0;
>>
>> @@ -60,6 +58,11 @@ void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
>> region->start = res->start - offset;
>> region->end = res->end - offset;
>> }
>> +void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
>> + struct resource *res)
>> +{
>> + __pcibios_resource_to_bus(dev->bus, region, res);
>> +}
>> EXPORT_SYMBOL(pcibios_resource_to_bus);
>>
>> static bool region_contains(struct pci_bus_region *region1,
>> @@ -68,10 +71,10 @@ static bool region_contains(struct pci_bus_region *region1,
>> return region1->start <= region2->start && region1->end >= region2->end;
>> }
>>
>> -void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
>> - struct pci_bus_region *region)
>> +void __pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
>> + struct pci_bus_region *region)
>> {
>> - struct pci_host_bridge *bridge = find_pci_host_bridge(dev);
>> + struct pci_host_bridge *bridge = find_pci_host_bridge(bus);
>> struct pci_host_bridge_window *window;
>> resource_size_t offset = 0;
>>
>> @@ -93,4 +96,9 @@ void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
>> res->start = region->start + offset;
>> res->end = region->end + offset;
>> }
>> +void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
>> + struct pci_bus_region *region)
>> +{
>> + __pcibios_bus_to_resource(dev->bus, res, region);
>> +}
>> EXPORT_SYMBOL(pcibios_bus_to_resource);
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 1084a15..5c2cf8e 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -737,8 +737,13 @@ void pci_fixup_cardbus(struct pci_bus *);
>>
>> /* Generic PCI functions used internally */
>>
>> +void __pcibios_resource_to_bus(struct pci_bus *bus,
>> + struct pci_bus_region *region,
>> + struct resource *res);
>> void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
>> struct resource *res);
>> +void __pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
>> + struct pci_bus_region *region);
>> void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
>> struct pci_bus_region *region);
>> void pcibios_scan_specific_bus(int busn);
>
> Ugh. Why would we add new functions with "__" prefixes? There are
> only a handful of callers to pcibios_bus_to_resource() and
> pcibios_resource_to_bus(). We can just change them to supply a
> "struct pci_bus *" directly. That's a better match for how the
> hardware works anyway.

ok, will update that if you prefer that.

Yinghai
Yinghai Lu
2013-12-09 20:03:18 UTC
Permalink
On Mon, Dec 9, 2013 at 11:21 AM, Yinghai Lu <***@kernel.org> wrote:
> On Mon, Dec 9, 2013 at 9:52 AM, Bjorn Helgaas <***@google.com> wrote:
>>
>> Ugh. Why would we add new functions with "__" prefixes? There are
>> only a handful of callers to pcibios_bus_to_resource() and
>> pcibios_resource_to_bus(). We can just change them to supply a
>> "struct pci_bus *" directly. That's a better match for how the
>> hardware works anyway.
>

Please double check if you are happy with the change.
Hope that will not update stable tree maintainer guys.

Subject: [PATCH] PCI: pcibus address to resource converting take bus
instead of dev

For allocating resource under bus path, we do not have dev to pass along,
and we only have bus to use instead.

-v2: drop pcibios_bus_addr_to_resource().
-v3: drop __* change requested by Bjorn.

Signed-off-by: Yinghai Lu <***@kernel.org>
Cc: ***@vger.kernel.org

---
arch/alpha/kernel/pci-sysfs.c | 4 ++--
arch/powerpc/kernel/pci-common.c | 4 ++--
arch/powerpc/kernel/pci_of_scan.c | 4 ++--
arch/sparc/kernel/pci.c | 6 +++---
drivers/pci/host-bridge.c | 24 +++++++++++-------------
drivers/pci/probe.c | 18 +++++++++---------
drivers/pci/quirks.c | 2 +-
drivers/pci/rom.c | 2 +-
drivers/pci/setup-bus.c | 16 ++++++++--------
drivers/pci/setup-res.c | 2 +-
drivers/pcmcia/i82092.c | 2 +-
drivers/pcmcia/yenta_socket.c | 6 +++---
drivers/scsi/sym53c8xx_2/sym_glue.c | 5 +++--
drivers/video/arkfb.c | 2 +-
drivers/video/s3fb.c | 2 +-
drivers/video/vt8623fb.c | 2 +-
include/linux/pci.h | 4 ++--
17 files changed, 52 insertions(+), 53 deletions(-)

Index: linux-2.6/drivers/pci/host-bridge.c
===================================================================
--- linux-2.6.orig/drivers/pci/host-bridge.c
+++ linux-2.6/drivers/pci/host-bridge.c
@@ -9,22 +9,19 @@

#include "pci.h"

-static struct pci_bus *find_pci_root_bus(struct pci_dev *dev)
+static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
{
- struct pci_bus *bus;
-
- bus = dev->bus;
while (bus->parent)
bus = bus->parent;

return bus;
}

-static struct pci_host_bridge *find_pci_host_bridge(struct pci_dev *dev)
+static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
{
- struct pci_bus *bus = find_pci_root_bus(dev);
+ struct pci_bus *root_bus = find_pci_root_bus(bus);

- return to_pci_host_bridge(bus->bridge);
+ return to_pci_host_bridge(root_bus->bridge);
}

void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
@@ -40,10 +37,11 @@ static bool resource_contains(struct res
return res1->start <= res2->start && res1->end >= res2->end;
}

-void pcibios_resource_to_bus(struct pci_dev *dev, struct
pci_bus_region *region,
- struct resource *res)
+void pcibios_resource_to_bus(struct pci_bus *bus,
+ struct pci_bus_region *region,
+ struct resource *res)
{
- struct pci_host_bridge *bridge = find_pci_host_bridge(dev);
+ struct pci_host_bridge *bridge = find_pci_host_bridge(bus);
struct pci_host_bridge_window *window;
resource_size_t offset = 0;

@@ -68,10 +66,10 @@ static bool region_contains(struct pci_b
return region1->start <= region2->start && region1->end >= region2->end;
}

-void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
- struct pci_bus_region *region)
+void pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
+ struct pci_bus_region *region)
{
- struct pci_host_bridge *bridge = find_pci_host_bridge(dev);
+ struct pci_host_bridge *bridge = find_pci_host_bridge(bus);
struct pci_host_bridge_window *window;
resource_size_t offset = 0;

Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -738,9 +738,9 @@ void pci_fixup_cardbus(struct pci_bus *)

/* Generic PCI functions used internally */

-void pcibios_resource_to_bus(struct pci_dev *dev, struct
pci_bus_region *region,
+void pcibios_resource_to_bus(struct pci_bus *bus, struct
pci_bus_region *region,
struct resource *res);
-void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
+void pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
struct pci_bus_region *region);
void pcibios_scan_specific_bus(int busn);
struct pci_bus *pci_find_bus(int domain, int busnr);
Index: linux-2.6/arch/alpha/kernel/pci-sysfs.c
===================================================================
--- linux-2.6.orig/arch/alpha/kernel/pci-sysfs.c
+++ linux-2.6/arch/alpha/kernel/pci-sysfs.c
@@ -83,7 +83,7 @@ static int pci_mmap_resource(struct kobj
if (iomem_is_exclusive(res->start))
return -EINVAL;

- pcibios_resource_to_bus(pdev, &bar, res);
+ pcibios_resource_to_bus(pdev->bus, &bar, res);
vma->vm_pgoff += bar.start >> (PAGE_SHIFT - (sparse ? 5 : 0));
mmap_type = res->flags & IORESOURCE_MEM ? pci_mmap_mem : pci_mmap_io;

@@ -139,7 +139,7 @@ static int sparse_mem_mmap_fits(struct p
long dense_offset;
unsigned long sparse_size;

- pcibios_resource_to_bus(pdev, &bar, &pdev->resource[num]);
+ pcibios_resource_to_bus(pdev->bus, &bar, &pdev->resource[num]);

/* All core logic chips have 4G sparse address space, except
CIA which has 16G (see xxx_SPARSE_MEM and xxx_DENSE_MEM
Index: linux-2.6/arch/powerpc/kernel/pci-common.c
===================================================================
--- linux-2.6.orig/arch/powerpc/kernel/pci-common.c
+++ linux-2.6/arch/powerpc/kernel/pci-common.c
@@ -835,7 +835,7 @@ static void pcibios_fixup_resources(stru
* at 0 as unset as well, except if PCI_PROBE_ONLY is also set
* since in that case, we don't want to re-assign anything
*/
- pcibios_resource_to_bus(dev, &reg, res);
+ pcibios_resource_to_bus(dev->bus, &reg, res);
if (pci_has_flag(PCI_REASSIGN_ALL_RSRC) ||
(reg.start == 0 && !pci_has_flag(PCI_PROBE_ONLY))) {
/* Only print message if not re-assigning */
@@ -886,7 +886,7 @@ static int pcibios_uninitialized_bridge_

/* Job is a bit different between memory and IO */
if (res->flags & IORESOURCE_MEM) {
- pcibios_resource_to_bus(dev, &region, res);
+ pcibios_resource_to_bus(dev->bus, &region, res);

/* If the BAR is non-0 then it's probably been initialized */
if (region.start != 0)
Index: linux-2.6/drivers/pci/rom.c
===================================================================
--- linux-2.6.orig/drivers/pci/rom.c
+++ linux-2.6/drivers/pci/rom.c
@@ -31,7 +31,7 @@ int pci_enable_rom(struct pci_dev *pdev)
if (!res->flags)
return -1;

- pcibios_resource_to_bus(pdev, &region, res);
+ pcibios_resource_to_bus(pdev->bus, &region, res);
pci_read_config_dword(pdev, pdev->rom_base_reg, &rom_addr);
rom_addr &= ~PCI_ROM_ADDRESS_MASK;
rom_addr |= region.start | PCI_ROM_ADDRESS_ENABLE;
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
@@ -475,7 +475,7 @@ void pci_setup_cardbus(struct pci_bus *b
&bus->busn_res);

res = bus->resource[0];
- pcibios_resource_to_bus(bridge, &region, res);
+ pcibios_resource_to_bus(bridge->bus, &region, res);
if (res->flags & IORESOURCE_IO) {
/*
* The IO resource is allocated a range twice as large as it
@@ -489,7 +489,7 @@ void pci_setup_cardbus(struct pci_bus *b
}

res = bus->resource[1];
- pcibios_resource_to_bus(bridge, &region, res);
+ pcibios_resource_to_bus(bridge->bus, &region, res);
if (res->flags & IORESOURCE_IO) {
dev_info(&bridge->dev, " bridge window %pR\n", res);
pci_write_config_dword(bridge, PCI_CB_IO_BASE_1,
@@ -499,7 +499,7 @@ void pci_setup_cardbus(struct pci_bus *b
}

res = bus->resource[2];
- pcibios_resource_to_bus(bridge, &region, res);
+ pcibios_resource_to_bus(bridge->bus, &region, res);
if (res->flags & IORESOURCE_MEM) {
dev_info(&bridge->dev, " bridge window %pR\n", res);
pci_write_config_dword(bridge, PCI_CB_MEMORY_BASE_0,
@@ -509,7 +509,7 @@ void pci_setup_cardbus(struct pci_bus *b
}

res = bus->resource[3];
- pcibios_resource_to_bus(bridge, &region, res);
+ pcibios_resource_to_bus(bridge->bus, &region, res);
if (res->flags & IORESOURCE_MEM) {
dev_info(&bridge->dev, " bridge window %pR\n", res);
pci_write_config_dword(bridge, PCI_CB_MEMORY_BASE_1,
@@ -546,7 +546,7 @@ static void pci_setup_bridge_io(struct p

/* Set up the top and bottom of the PCI I/O segment for this bus. */
res = bus->resource[0];
- pcibios_resource_to_bus(bridge, &region, res);
+ pcibios_resource_to_bus(bridge->bus, &region, res);
if (res->flags & IORESOURCE_IO) {
pci_read_config_dword(bridge, PCI_IO_BASE, &l);
l &= 0xffff0000;
@@ -578,7 +578,7 @@ static void pci_setup_bridge_mmio(struct

/* Set up the top and bottom of the PCI Memory segment for this bus. */
res = bus->resource[1];
- pcibios_resource_to_bus(bridge, &region, res);
+ pcibios_resource_to_bus(bridge->bus, &region, res);
if (res->flags & IORESOURCE_MEM) {
l = (region.start >> 16) & 0xfff0;
l |= region.end & 0xfff00000;
@@ -604,7 +604,7 @@ static void pci_setup_bridge_mmio_pref(s
/* Set up PREF base/limit. */
bu = lu = 0;
res = bus->resource[2];
- pcibios_resource_to_bus(bridge, &region, res);
+ pcibios_resource_to_bus(bridge->bus, &region, res);
if (res->flags & IORESOURCE_PREFETCH) {
l = (region.start >> 16) & 0xfff0;
l |= region.end & 0xfff00000;
@@ -1422,7 +1422,7 @@ static int iov_resources_unassigned(stru
if (!r->flags)
continue;

- pcibios_resource_to_bus(dev, &region, r);
+ pcibios_resource_to_bus(dev->bus, &region, r);
if (!region.start) {
*unassigned = true;
return 1; /* return early from pci_walk_bus() */
Index: linux-2.6/drivers/pci/setup-res.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-res.c
+++ linux-2.6/drivers/pci/setup-res.c
@@ -52,7 +52,7 @@ void pci_update_resource(struct pci_dev
if (res->flags & IORESOURCE_PCI_FIXED)
return;

- pcibios_resource_to_bus(dev, &region, res);
+ pcibios_resource_to_bus(dev->bus, &region, res);

new = region.start | (res->flags & PCI_REGION_FLAG_MASK);
if (res->flags & IORESOURCE_IO)
Index: linux-2.6/drivers/pcmcia/i82092.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/i82092.c
+++ linux-2.6/drivers/pcmcia/i82092.c
@@ -608,7 +608,7 @@ static int i82092aa_set_mem_map(struct p

enter("i82092aa_set_mem_map");

- pcibios_resource_to_bus(sock_info->dev, &region, mem->res);
+ pcibios_resource_to_bus(sock_info->dev->bus, &region, mem->res);

map = mem->map;
if (map > 4) {
Index: linux-2.6/drivers/pcmcia/yenta_socket.c
===================================================================
--- linux-2.6.orig/drivers/pcmcia/yenta_socket.c
+++ linux-2.6/drivers/pcmcia/yenta_socket.c
@@ -445,7 +445,7 @@ static int yenta_set_mem_map(struct pcmc
unsigned int start, stop, card_start;
unsigned short word;

- pcibios_resource_to_bus(socket->dev, &region, mem->res);
+ pcibios_resource_to_bus(socket->dev->bus, &region, mem->res);

map = mem->map;
start = region.start;
@@ -709,7 +709,7 @@ static int yenta_allocate_res(struct yen
region.start = config_readl(socket, addr_start) & mask;
region.end = config_readl(socket, addr_end) | ~mask;
if (region.start && region.end > region.start && !override_bios) {
- pcibios_bus_to_resource(dev, res, &region);
+ pcibios_bus_to_resource(dev->bus, res, &region);
if (pci_claim_resource(dev, PCI_BRIDGE_RESOURCES + nr) == 0)
return 0;
dev_printk(KERN_INFO, &dev->dev,
@@ -1033,7 +1033,7 @@ static void yenta_config_init(struct yen
struct pci_dev *dev = socket->dev;
struct pci_bus_region region;

- pcibios_resource_to_bus(socket->dev, &region, &dev->resource[0]);
+ pcibios_resource_to_bus(socket->dev->bus, &region, &dev->resource[0]);

config_writel(socket, CB_LEGACY_MODE_BASE, 0);
config_writel(socket, PCI_BASE_ADDRESS_0, region.start);
Index: linux-2.6/drivers/scsi/sym53c8xx_2/sym_glue.c
===================================================================
--- linux-2.6.orig/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ linux-2.6/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -1531,7 +1531,7 @@ static int sym_iomap_device(struct sym_d
struct pci_bus_region bus_addr;
int i = 2;

- pcibios_resource_to_bus(pdev, &bus_addr, &pdev->resource[1]);
+ pcibios_resource_to_bus(pdev->bus, &bus_addr, &pdev->resource[1]);
device->mmio_base = bus_addr.start;

if (device->chip.features & FE_RAM) {
@@ -1541,7 +1541,8 @@ static int sym_iomap_device(struct sym_d
*/
if (!pdev->resource[i].flags)
i++;
- pcibios_resource_to_bus(pdev, &bus_addr, &pdev->resource[i]);
+ pcibios_resource_to_bus(pdev->bus, &bus_addr,
+ &pdev->resource[i]);
device->ram_base = bus_addr.start;
}

Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ linux-2.6/drivers/pci/probe.c
@@ -269,8 +269,8 @@ int __pci_read_base(struct pci_dev *dev,
region.end = l + sz;
}

- pcibios_bus_to_resource(dev, res, &region);
- pcibios_resource_to_bus(dev, &inverted_region, res);
+ pcibios_bus_to_resource(dev->bus, res, &region);
+ pcibios_resource_to_bus(dev->bus, &inverted_region, res);

/*
* If "A" is a BAR value (a bus address), "bus_to_resource(A)" is
@@ -364,7 +364,7 @@ static void pci_read_bridge_io(struct pc
res->flags = (io_base_lo & PCI_IO_RANGE_TYPE_MASK) | IORESOURCE_IO;
region.start = base;
region.end = limit + io_granularity - 1;
- pcibios_bus_to_resource(dev, res, &region);
+ pcibios_bus_to_resource(dev->bus, res, &region);
dev_printk(KERN_DEBUG, &dev->dev, " bridge window %pR\n", res);
}
}
@@ -386,7 +386,7 @@ static void pci_read_bridge_mmio(struct
res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) |
IORESOURCE_MEM;
region.start = base;
region.end = limit + 0xfffff;
- pcibios_bus_to_resource(dev, res, &region);
+ pcibios_bus_to_resource(dev->bus, res, &region);
dev_printk(KERN_DEBUG, &dev->dev, " bridge window %pR\n", res);
}
}
@@ -436,7 +436,7 @@ static void pci_read_bridge_mmio_pref(st
res->flags |= IORESOURCE_MEM_64;
region.start = base;
region.end = limit + 0xfffff;
- pcibios_bus_to_resource(dev, res, &region);
+ pcibios_bus_to_resource(dev->bus, res, &region);
dev_printk(KERN_DEBUG, &dev->dev, " bridge window %pR\n", res);
}
}
@@ -1084,24 +1084,24 @@ int pci_setup_device(struct pci_dev *dev
region.end = 0x1F7;
res = &dev->resource[0];
res->flags = LEGACY_IO_RESOURCE;
- pcibios_bus_to_resource(dev, res, &region);
+ pcibios_bus_to_resource(dev->bus, res, &region);
region.start = 0x3F6;
region.end = 0x3F6;
res = &dev->resource[1];
res->flags = LEGACY_IO_RESOURCE;
- pcibios_bus_to_resource(dev, res, &region);
+ pcibios_bus_to_resource(dev->bus, res, &region);
}
if ((progif & 4) == 0) {
region.start = 0x170;
region.end = 0x177;
res = &dev->resource[2];
res->flags = LEGACY_IO_RESOURCE;
- pcibios_bus_to_resource(dev, res, &region);
+ pcibios_bus_to_resource(dev->bus, res, &region);
region.start = 0x376;
region.end = 0x376;
res = &dev->resource[3];
res->flags = LEGACY_IO_RESOURCE;
- pcibios_bus_to_resource(dev, res, &region);
+ pcibios_bus_to_resource(dev->bus, res, &region);
}
}
break;
Index: linux-2.6/arch/powerpc/kernel/pci_of_scan.c
===================================================================
--- linux-2.6.orig/arch/powerpc/kernel/pci_of_scan.c
+++ linux-2.6/arch/powerpc/kernel/pci_of_scan.c
@@ -111,7 +111,7 @@ static void of_pci_parse_addrs(struct de
res->name = pci_name(dev);
region.start = base;
region.end = base + size - 1;
- pcibios_bus_to_resource(dev, res, &region);
+ pcibios_bus_to_resource(dev->bus, res, &region);
}
}

@@ -280,7 +280,7 @@ void of_scan_pci_bridge(struct pci_dev *
res->flags = flags;
region.start = of_read_number(&ranges[1], 2);
region.end = region.start + size - 1;
- pcibios_bus_to_resource(dev, res, &region);
+ pcibios_bus_to_resource(dev->bus, res, &region);
}
sprintf(bus->name, "PCI Bus %04x:%02x", pci_domain_nr(bus),
bus->number);
Index: linux-2.6/arch/sparc/kernel/pci.c
===================================================================
--- linux-2.6.orig/arch/sparc/kernel/pci.c
+++ linux-2.6/arch/sparc/kernel/pci.c
@@ -392,7 +392,7 @@ static void apb_fake_ranges(struct pci_d
res->flags = IORESOURCE_IO;
region.start = (first << 21);
region.end = (last << 21) + ((1 << 21) - 1);
- pcibios_bus_to_resource(dev, res, &region);
+ pcibios_bus_to_resource(dev->bus, res, &region);

pci_read_config_byte(dev, APB_MEM_ADDRESS_MAP, &map);
apb_calc_first_last(map, &first, &last);
@@ -400,7 +400,7 @@ static void apb_fake_ranges(struct pci_d
res->flags = IORESOURCE_MEM;
region.start = (first << 29);
region.end = (last << 29) + ((1 << 29) - 1);
- pcibios_bus_to_resource(dev, res, &region);
+ pcibios_bus_to_resource(dev->bus, res, &region);
}

static void pci_of_scan_bus(struct pci_pbm_info *pbm,
@@ -491,7 +491,7 @@ static void of_scan_pci_bridge(struct pc
res->flags = flags;
region.start = GET_64BIT(ranges, 1);
region.end = region.start + size - 1;
- pcibios_bus_to_resource(dev, res, &region);
+ pcibios_bus_to_resource(dev->bus, res, &region);
}
after_ranges:
sprintf(bus->name, "PCI Bus %04x:%02x", pci_domain_nr(bus),
Index: linux-2.6/drivers/pci/quirks.c
===================================================================
--- linux-2.6.orig/drivers/pci/quirks.c
+++ linux-2.6/drivers/pci/quirks.c
@@ -339,7 +339,7 @@ static void quirk_io_region(struct pci_d
/* Convert from PCI bus to resource space */
bus_region.start = region;
bus_region.end = region + size - 1;
- pcibios_bus_to_resource(dev, res, &bus_region);
+ pcibios_bus_to_resource(dev->bus, res, &bus_region);

if (!pci_claim_resource(dev, nr))
dev_info(&dev->dev, "quirk: %pR claimed by %s\n", res, name);
Index: linux-2.6/drivers/video/arkfb.c
===================================================================
--- linux-2.6.orig/drivers/video/arkfb.c
+++ linux-2.6/drivers/video/arkfb.c
@@ -1014,7 +1014,7 @@ static int ark_pci_probe(struct pci_dev

vga_res.flags = IORESOURCE_IO;

- pcibios_bus_to_resource(dev, &vga_res, &bus_reg);
+ pcibios_bus_to_resource(dev->bus, &vga_res, &bus_reg);

par->state.vgabase = (void __iomem *) vga_res.start;

Index: linux-2.6/drivers/video/s3fb.c
===================================================================
--- linux-2.6.orig/drivers/video/s3fb.c
+++ linux-2.6/drivers/video/s3fb.c
@@ -1180,7 +1180,7 @@ static int s3_pci_probe(struct pci_dev *

vga_res.flags = IORESOURCE_IO;

- pcibios_bus_to_resource(dev, &vga_res, &bus_reg);
+ pcibios_bus_to_resource(dev->bus, &vga_res, &bus_reg);

par->state.vgabase = (void __iomem *) vga_res.start;

Index: linux-2.6/drivers/video/vt8623fb.c
===================================================================
--- linux-2.6.orig/drivers/video/vt8623fb.c
+++ linux-2.6/drivers/video/vt8623fb.c
@@ -729,7 +729,7 @@ static int vt8623_pci_probe(struct pci_d

vga_res.flags = IORESOURCE_IO;

- pcibios_bus_to_resource(dev, &vga_res, &bus_reg);
+ pcibios_bus_to_resource(dev->bus, &vga_res, &bus_reg);

par->state.vgabase = (void __iomem *) vga_res.start;
Greg KH
2013-12-09 20:43:23 UTC
Permalink
On Mon, Dec 09, 2013 at 12:03:18PM -0800, Yinghai Lu wrote:
> On Mon, Dec 9, 2013 at 11:21 AM, Yinghai Lu <***@kernel.org> wrote:
> > On Mon, Dec 9, 2013 at 9:52 AM, Bjorn Helgaas <***@google.com> wrote:
> >>
> >> Ugh. Why would we add new functions with "__" prefixes? There are
> >> only a handful of callers to pcibios_bus_to_resource() and
> >> pcibios_resource_to_bus(). We can just change them to supply a
> >> "struct pci_bus *" directly. That's a better match for how the
> >> hardware works anyway.
> >
>
> Please double check if you are happy with the change.
> Hope that will not update stable tree maintainer guys.

What do you mean by this?

How is this a -stable kernel patch?
Yinghai Lu
2013-12-09 20:47:24 UTC
Permalink
On Mon, Dec 9, 2013 at 12:43 PM, Greg KH <***@linuxfoundation.org> wrote:
> On Mon, Dec 09, 2013 at 12:03:18PM -0800, Yinghai Lu wrote:
>> On Mon, Dec 9, 2013 at 11:21 AM, Yinghai Lu <***@kernel.org> wrote:
>> > On Mon, Dec 9, 2013 at 9:52 AM, Bjorn Helgaas <***@google.com> wrote:
>> >>
>> >> Ugh. Why would we add new functions with "__" prefixes? There are
>> >> only a handful of callers to pcibios_bus_to_resource() and
>> >> pcibios_resource_to_bus(). We can just change them to supply a
>> >> "struct pci_bus *" directly. That's a better match for how the
>> >> hardware works anyway.
>> >
>>
>> Please double check if you are happy with the change.
>> Hope that will not update stable tree maintainer guys.
>
> What do you mean by this?

Do not want to change too much code, to upset stable tree maintainers.

>
> How is this a -stable kernel patch?

We have stable patch that will need use the new function.

[PATCH 2/2] PCI: Only enable realloc auto when root bus has 64bit mmio

Thanks

Yinghai
Bjorn Helgaas
2013-12-09 21:06:59 UTC
Permalink
On Mon, Dec 9, 2013 at 1:47 PM, Yinghai Lu <***@kernel.org> wrote:
> On Mon, Dec 9, 2013 at 12:43 PM, Greg KH <***@linuxfoundation.org> wrote:
>> On Mon, Dec 09, 2013 at 12:03:18PM -0800, Yinghai Lu wrote:
>>> On Mon, Dec 9, 2013 at 11:21 AM, Yinghai Lu <***@kernel.org> wrote:
>>> > On Mon, Dec 9, 2013 at 9:52 AM, Bjorn Helgaas <***@google.com> wrote:
>>> >>
>>> >> Ugh. Why would we add new functions with "__" prefixes? There are
>>> >> only a handful of callers to pcibios_bus_to_resource() and
>>> >> pcibios_resource_to_bus(). We can just change them to supply a
>>> >> "struct pci_bus *" directly. That's a better match for how the
>>> >> hardware works anyway.
>>> >
>>>
>>> Please double check if you are happy with the change.
>>> Hope that will not update stable tree maintainer guys.
>>
>> What do you mean by this?
>
> Do not want to change too much code, to upset stable tree maintainers.
>
>>
>> How is this a -stable kernel patch?
>
> We have stable patch that will need use the new function.
>
> [PATCH 2/2] PCI: Only enable realloc auto when root bus has 64bit mmio

Let's not get too far ahead of ourselves here. I don't like this
patch even for upstream, so we don't need to worry about -stable quite
yet.

Bjorn
Yinghai Lu
2013-12-08 23:54:29 UTC
Permalink
Joseph found
| commit b07f2ebc109b607789f648dedcff4b125f9afec6
| Date: Thu Feb 23 19:23:32 2012 -0800
|
| PCI: add a PCI resource reallocation config option

cause one system can not load driver for Intel x520 NIC's.

The root resource:
[ 1.212470] PCI host bridge to bus 0000:20
[ 1.212475] pci_bus 0000:20: root bus resource [bus 20-3e]
[ 1.212479] pci_bus 0000:20: root bus resource [io 0xc000-0xdfff]
[ 1.212483] pci_bus 0000:20: root bus resource [mem 0xfecc0000-0xfecfffff]
[ 1.212487] pci_bus 0000:20: root bus resource [mem 0xe9400000-0xe97fffff]

and bios does not assign sriov, also have two function ROM bar point to same
position.
[ 1.213197] pci 0000:22:00.0: [8086:10fb] type 00 class 0x020000
...
[ 1.213240] pci 0000:22:00.0: reg 0x30: [mem 0xe9500000-0xe957ffff pref]
[ 1.213303] pci 0000:22:00.0: reg 0x184: [mem 0x00000000-0x00003fff 64bit]
[ 1.213317] pci 0000:22:00.0: reg 0x190: [mem 0x00000000-0x00003fff 64bit]
[ 1.213366] pci 0000:22:00.1: [8086:10fb] type 00 class 0x020000
...
[ 1.213408] pci 0000:22:00.1: reg 0x30: [mem 0xe9500000-0xe957ffff pref]
[ 1.213468] pci 0000:22:00.1: reg 0x184: [mem 0x00000000-0x00003fff 64bit]
[ 1.213481] pci 0000:22:00.1: reg 0x190: [mem 0x00000000-0x00003fff 64bit]
[ 1.218527] pci 0000:20:03.0: PCI bridge to [bus 22]
[ 1.218534] pci 0000:20:03.0: bridge window [io 0xd000-0xdfff]
[ 1.218537] pci 0000:20:03.0: bridge window [mem 0xe9400000-0xe95fffff]
...
[ 1.254103] pci 0000:22:00.1: address space collision: [mem 0xe9500000-0xe957ffff pref] conflicts with 0000:22:00.0 [mem 0xe9500000-0xe957ffff pref]
[ 1.254111] pci 0000:23:00.1: address space collision: [mem 0xe9700000-0xe977ffff pref] conflicts with 0000:23:00.0 [mem 0xe9700000-0xe977ffff pref]

We don't need to enable realloc for this case, as we can not alter root bus mmio
range to get big one to hold two rom bar, and sriov under 4G.

Add checking if pci root bus have 4G above mmio res, and don't enable
realloc auto accordingly.

bug report at: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1245938

Reported-by: Joseph Salisbury <***@canonical.com>
Signed-off-by: Yinghai Lu <***@kernel.org>
Cc: ***@vger.kernel.org
---
drivers/pci/setup-bus.c | 32 +++++++++++++++++++++++++++-----
1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 219a410..f9e6efb 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1432,17 +1432,39 @@ static int iov_resources_unassigned(struct pci_dev *dev, void *data)
return 0;
}

+static bool pci_bus_mem_above_4g(struct pci_bus *bus)
+{
+ int i;
+ struct resource *res;
+
+ pci_bus_for_each_resource(bus, res, i) {
+ struct pci_bus_region region;
+
+ if (!res || !(res->flags & IORESOURCE_MEM))
+ continue;
+
+ __pcibios_resource_to_bus(bus, &region, res);
+ if (region.end > 0xffffffff)
+ return true;
+ }
+
+ return false;
+}
+
static enum enable_type pci_realloc_detect(struct pci_bus *bus,
enum enable_type enable_local)
{
- bool unassigned = false;
-
if (enable_local != undefined)
return enable_local;

- pci_walk_bus(bus, iov_resources_unassigned, &unassigned);
- if (unassigned)
- return auto_enabled;
+ /* only enable auto when root bus does support 64bit mmio */
+ if (pci_bus_mem_above_4g(bus)) {
+ bool unassigned = false;
+
+ pci_walk_bus(bus, iov_resources_unassigned, &unassigned);
+ if (unassigned)
+ return auto_enabled;
+ }

return enable_local;
}
--
1.8.4
Bjorn Helgaas
2013-12-09 17:54:20 UTC
Permalink
On Sun, Dec 8, 2013 at 4:54 PM, Yinghai Lu <***@kernel.org> wrote:
> Joseph found
> | commit b07f2ebc109b607789f648dedcff4b125f9afec6
> | Date: Thu Feb 23 19:23:32 2012 -0800
> |
> | PCI: add a PCI resource reallocation config option
>
> cause one system can not load driver for Intel x520 NIC's.
>
> The root resource:
> [ 1.212470] PCI host bridge to bus 0000:20
> [ 1.212475] pci_bus 0000:20: root bus resource [bus 20-3e]
> [ 1.212479] pci_bus 0000:20: root bus resource [io 0xc000-0xdfff]
> [ 1.212483] pci_bus 0000:20: root bus resource [mem 0xfecc0000-0xfecfffff]
> [ 1.212487] pci_bus 0000:20: root bus resource [mem 0xe9400000-0xe97fffff]
>
> and bios does not assign sriov, also have two function ROM bar point to same
> position.
> [ 1.213197] pci 0000:22:00.0: [8086:10fb] type 00 class 0x020000
> ...
> [ 1.213240] pci 0000:22:00.0: reg 0x30: [mem 0xe9500000-0xe957ffff pref]
> [ 1.213303] pci 0000:22:00.0: reg 0x184: [mem 0x00000000-0x00003fff 64bit]
> [ 1.213317] pci 0000:22:00.0: reg 0x190: [mem 0x00000000-0x00003fff 64bit]
> [ 1.213366] pci 0000:22:00.1: [8086:10fb] type 00 class 0x020000
> ...
> [ 1.213408] pci 0000:22:00.1: reg 0x30: [mem 0xe9500000-0xe957ffff pref]
> [ 1.213468] pci 0000:22:00.1: reg 0x184: [mem 0x00000000-0x00003fff 64bit]
> [ 1.213481] pci 0000:22:00.1: reg 0x190: [mem 0x00000000-0x00003fff 64bit]
> [ 1.218527] pci 0000:20:03.0: PCI bridge to [bus 22]
> [ 1.218534] pci 0000:20:03.0: bridge window [io 0xd000-0xdfff]
> [ 1.218537] pci 0000:20:03.0: bridge window [mem 0xe9400000-0xe95fffff]
> ...
> [ 1.254103] pci 0000:22:00.1: address space collision: [mem 0xe9500000-0xe957ffff pref] conflicts with 0000:22:00.0 [mem 0xe9500000-0xe957ffff pref]
> [ 1.254111] pci 0000:23:00.1: address space collision: [mem 0xe9700000-0xe977ffff pref] conflicts with 0000:23:00.0 [mem 0xe9700000-0xe977ffff pref]
>
> We don't need to enable realloc for this case, as we can not alter root bus mmio
> range to get big one to hold two rom bar, and sriov under 4G.
>
> Add checking if pci root bus have 4G above mmio res, and don't enable
> realloc auto accordingly.
>
> bug report at: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1245938
>
> Reported-by: Joseph Salisbury <***@canonical.com>
> Signed-off-by: Yinghai Lu <***@kernel.org>
> Cc: ***@vger.kernel.org
> ---
> drivers/pci/setup-bus.c | 32 +++++++++++++++++++++++++++-----
> 1 file changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 219a410..f9e6efb 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1432,17 +1432,39 @@ static int iov_resources_unassigned(struct pci_dev *dev, void *data)
> return 0;
> }
>
> +static bool pci_bus_mem_above_4g(struct pci_bus *bus)
> +{
> + int i;
> + struct resource *res;
> +
> + pci_bus_for_each_resource(bus, res, i) {
> + struct pci_bus_region region;
> +
> + if (!res || !(res->flags & IORESOURCE_MEM))
> + continue;
> +
> + __pcibios_resource_to_bus(bus, &region, res);
> + if (region.end > 0xffffffff)
> + return true;
> + }
> +
> + return false;
> +}
> +
> static enum enable_type pci_realloc_detect(struct pci_bus *bus,
> enum enable_type enable_local)
> {
> - bool unassigned = false;
> -
> if (enable_local != undefined)
> return enable_local;
>
> - pci_walk_bus(bus, iov_resources_unassigned, &unassigned);
> - if (unassigned)
> - return auto_enabled;
> + /* only enable auto when root bus does support 64bit mmio */
> + if (pci_bus_mem_above_4g(bus)) {
> + bool unassigned = false;
> +
> + pci_walk_bus(bus, iov_resources_unassigned, &unassigned);
> + if (unassigned)
> + return auto_enabled;

I don't see how the question of whether the host bridge has an
aperture above 4G is related to whether we should automatically
reassign resources. They seem orthogonal to me. No doubt it "fixes"
the current problem, but it doesn't make sense conceptually.

Bjorn

> + }
>
> return enable_local;
> }
> --
> 1.8.4
>
Yinghai Lu
2013-12-09 19:20:55 UTC
Permalink
On Mon, Dec 9, 2013 at 9:54 AM, Bjorn Helgaas <***@google.com> wrote:
> I don't see how the question of whether the host bridge has an
> aperture above 4G is related to whether we should automatically
> reassign resources. They seem orthogonal to me. No doubt it "fixes"
> the current problem, but it doesn't make sense conceptually.

the BIOS has problem to have two functions point to same position.

During realloc first try: standard+two rom_bar+sriov will be fail at first as
pci root bus does not have enough mmio range,
later try will standard+two rom_bar it will fail too as root bus mmio range will
still only fit standard+one rom_bar.

My thoughts is limit use realloc auto in those systems that does not have mmio64
above 4g...
so old system will never have to specify "pci=realloc=off" to disable it.

Thanks

Yinghai
Bjorn Helgaas
2013-12-09 19:42:29 UTC
Permalink
On Mon, Dec 9, 2013 at 12:20 PM, Yinghai Lu <***@kernel.org> wrote:
> On Mon, Dec 9, 2013 at 9:54 AM, Bjorn Helgaas <***@google.com> wrote:
>> I don't see how the question of whether the host bridge has an
>> aperture above 4G is related to whether we should automatically
>> reassign resources. They seem orthogonal to me. No doubt it "fixes"
>> the current problem, but it doesn't make sense conceptually.
>
> the BIOS has problem to have two functions point to same position.
>
> During realloc first try: standard+two rom_bar+sriov will be fail at first as
> pci root bus does not have enough mmio range,
> later try will standard+two rom_bar it will fail too as root bus mmio range will
> still only fit standard+one rom_bar.
>
> My thoughts is limit use realloc auto in those systems that does not have mmio64
> above 4g...
> so old system will never have to specify "pci=realloc=off" to disable it.

That doesn't answer my question at all.

I understand that this change makes it so Joseph doesn't have to use
"pci=realloc=off". But why should auto-reallocation be limited to
buses that have resources above 4GB? That doesn't make any sense.

We should fix the reallocation code so it can deal with this case. If
there's not enough space for everything, obviously we have to leave
something unassigned. A ROM BAR is a good candidate for leaving
unassigned, because most of the time we can get along without it.

Bjorn
Yinghai Lu
2013-12-09 20:10:58 UTC
Permalink
On Mon, Dec 9, 2013 at 11:42 AM, Bjorn Helgaas <***@google.com> wrote:
> That doesn't answer my question at all.
>
> I understand that this change makes it so Joseph doesn't have to use
> "pci=realloc=off". But why should auto-reallocation be limited to
> buses that have resources above 4GB? That doesn't make any sense.
>
> We should fix the reallocation code so it can deal with this case. If
> there's not enough space for everything, obviously we have to leave
> something unassigned. A ROM BAR is a good candidate for leaving
> unassigned, because most of the time we can get along without it.

Yes, that is ideal and not that simple.
but that would be hard to backport to old kernels.

BTW, Joseph, can you try
git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
for-pci-3.14
with pci=realloc=on

on that system?

Thanks

Yinghai
Bjorn Helgaas
2013-12-09 20:20:59 UTC
Permalink
On Mon, Dec 9, 2013 at 1:10 PM, Yinghai Lu <***@kernel.org> wrote:
> On Mon, Dec 9, 2013 at 11:42 AM, Bjorn Helgaas <***@google.com> wrote:
>> That doesn't answer my question at all.
>>
>> I understand that this change makes it so Joseph doesn't have to use
>> "pci=realloc=off". But why should auto-reallocation be limited to
>> buses that have resources above 4GB? That doesn't make any sense.
>>
>> We should fix the reallocation code so it can deal with this case. If
>> there's not enough space for everything, obviously we have to leave
>> something unassigned. A ROM BAR is a good candidate for leaving
>> unassigned, because most of the time we can get along without it.
>
> Yes, that is ideal and not that simple.
> but that would be hard to backport to old kernels.

Yes, I agree. I didn't say it would be simple. The quick fix would
be easy for now, but adding nonsensical code makes our lives harder
long into the future.

Bjorn
Yinghai Lu
2013-12-09 20:44:58 UTC
Permalink
On Mon, Dec 9, 2013 at 12:20 PM, Bjorn Helgaas <***@google.com> wrote:

> Yes, I agree. I didn't say it would be simple. The quick fix would
> be easy for now, but adding nonsensical code makes our lives harder
> long into the future.

If the old kernel is working, and user update kernel then we should
not request them to append "pci=realloc=off" for booting.

other user that would have "pci=realloc=on" works, they could always
keep that.

For now, we just reduce auto enable scope, as very beginning we are doing
that esp for SRIOV..., and now SRIOV would require huge range esp when
num_VF could be 64. we are easy to hit the case that realloc code mess up
the bios allocation when there is not above 4G 64bit mmio.

Yinghai
Bjorn Helgaas
2013-12-09 21:05:49 UTC
Permalink
On Mon, Dec 9, 2013 at 1:44 PM, Yinghai Lu <***@kernel.org> wrote:
> On Mon, Dec 9, 2013 at 12:20 PM, Bjorn Helgaas <***@google.com> wrote:
>
>> Yes, I agree. I didn't say it would be simple. The quick fix would
>> be easy for now, but adding nonsensical code makes our lives harder
>> long into the future.
>
> If the old kernel is working, and user update kernel then we should
> not request them to append "pci=realloc=off" for booting.

I agree. I would just prefer a better fix.

Bjorn
Joseph Salisbury
2013-12-11 19:19:33 UTC
Permalink
On 12/09/2013 03:10 PM, Yinghai Lu wrote:
> On Mon, Dec 9, 2013 at 11:42 AM, Bjorn Helgaas <***@google.com> wrote:
>> That doesn't answer my question at all.
>>
>> I understand that this change makes it so Joseph doesn't have to use
>> "pci=realloc=off". But why should auto-reallocation be limited to
>> buses that have resources above 4GB? That doesn't make any sense.
>>
>> We should fix the reallocation code so it can deal with this case. If
>> there's not enough space for everything, obviously we have to leave
>> something unassigned. A ROM BAR is a good candidate for leaving
>> unassigned, because most of the time we can get along without it.
> Yes, that is ideal and not that simple.
> but that would be hard to backport to old kernels.
>
> BTW, Joseph, can you try
> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
> for-pci-3.14
> with pci=realloc=on
>
> on that system?
>
> Thanks
>
> Yinghai
I noticed there was some back and forth on this thread. Do you still
want me to test this version, Yinghai?
Yinghai Lu
2013-12-11 19:55:42 UTC
Permalink
On Wed, Dec 11, 2013 at 11:19 AM, Joseph Salisbury
<***@canonical.com> wrote:
> On 12/09/2013 03:10 PM, Yinghai Lu wrote:
>> On Mon, Dec 9, 2013 at 11:42 AM, Bjorn Helgaas <***@google.com> wrote:
>>> That doesn't answer my question at all.
>>>
>>> I understand that this change makes it so Joseph doesn't have to use
>>> "pci=realloc=off". But why should auto-reallocation be limited to
>>> buses that have resources above 4GB? That doesn't make any sense.
>>>
>>> We should fix the reallocation code so it can deal with this case. If
>>> there's not enough space for everything, obviously we have to leave
>>> something unassigned. A ROM BAR is a good candidate for leaving
>>> unassigned, because most of the time we can get along without it.
>> Yes, that is ideal and not that simple.
>> but that would be hard to backport to old kernels.
>>
>> BTW, Joseph, can you try
>> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
>> for-pci-3.14
>> with pci=realloc=on
>>
>> on that system?
>>
>> Thanks
>>
>> Yinghai
> I noticed there was some back and forth on this thread. Do you still
> want me to test this version, Yinghai?

Yes, if that works, we would not need to put the patch in upstream for limiting
realloc auto scope.

Thanks

Yinghai
Joseph Salisbury
2014-01-08 16:38:39 UTC
Permalink
On 12/11/2013 02:55 PM, Yinghai Lu wrote:
> On Wed, Dec 11, 2013 at 11:19 AM, Joseph Salisbury
> <***@canonical.com> wrote:
>> On 12/09/2013 03:10 PM, Yinghai Lu wrote:
>>> On Mon, Dec 9, 2013 at 11:42 AM, Bjorn Helgaas <***@google.com> wrote:
>>>> That doesn't answer my question at all.
>>>>
>>>> I understand that this change makes it so Joseph doesn't have to use
>>>> "pci=realloc=off". But why should auto-reallocation be limited to
>>>> buses that have resources above 4GB? That doesn't make any sense.
>>>>
>>>> We should fix the reallocation code so it can deal with this case. If
>>>> there's not enough space for everything, obviously we have to leave
>>>> something unassigned. A ROM BAR is a good candidate for leaving
>>>> unassigned, because most of the time we can get along without it.
>>> Yes, that is ideal and not that simple.
>>> but that would be hard to backport to old kernels.
>>>
>>> BTW, Joseph, can you try
>>> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
>>> for-pci-3.14
>>> with pci=realloc=on
>>>
>>> on that system?
>>>
>>> Thanks
>>>
>>> Yinghai
>> I noticed there was some back and forth on this thread. Do you still
>> want me to test this version, Yinghai?
> Yes, if that works, we would not need to put the patch in upstream for limiting
> realloc auto scope.
>
> Thanks
>
> Yinghai
Hi Yinghai,

Sorry for the delay. The bug reporter was finally able to test your
patch. He reports that this version of the patch does in fact fix the
bug. See comment #72 here:

http://pad.lv/1245938

Thanks again for all your help!

Joe
Joseph Salisbury
2014-01-10 16:19:14 UTC
Permalink
On 12/11/2013 02:55 PM, Yinghai Lu wrote:
> On Wed, Dec 11, 2013 at 11:19 AM, Joseph Salisbury
> <***@canonical.com> wrote:
>> On 12/09/2013 03:10 PM, Yinghai Lu wrote:
>>> On Mon, Dec 9, 2013 at 11:42 AM, Bjorn Helgaas <***@google.com> wrote:
>>>> That doesn't answer my question at all.
>>>>
>>>> I understand that this change makes it so Joseph doesn't have to use
>>>> "pci=realloc=off". But why should auto-reallocation be limited to
>>>> buses that have resources above 4GB? That doesn't make any sense.
>>>>
>>>> We should fix the reallocation code so it can deal with this case. If
>>>> there's not enough space for everything, obviously we have to leave
>>>> something unassigned. A ROM BAR is a good candidate for leaving
>>>> unassigned, because most of the time we can get along without it.
>>> Yes, that is ideal and not that simple.
>>> but that would be hard to backport to old kernels.
>>>
>>> BTW, Joseph, can you try
>>> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
>>> for-pci-3.14
>>> with pci=realloc=on
>>>
>>> on that system?
>>>
>>> Thanks
>>>
>>> Yinghai
>> I noticed there was some back and forth on this thread. Do you still
>> want me to test this version, Yinghai?
> Yes, if that works, we would not need to put the patch in upstream for limiting
> realloc auto scope.
>
> Thanks
>
> Yinghai

Another user has confirmed that at test kernel from your branch[0] does
resolve the bug.

Thanks,

Joe

[0]

git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
Yinghai Lu
2014-01-10 17:13:37 UTC
Permalink
On Fri, Jan 10, 2014 at 8:19 AM, Joseph Salisbury
<***@canonical.com> wrote:
> On 12/11/2013 02:55 PM, Yinghai Lu wrote:
>> On Wed, Dec 11, 2013 at 11:19 AM, Joseph Salisbury
>> <***@canonical.com> wrote:
>>> On 12/09/2013 03:10 PM, Yinghai Lu wrote:
>>>> On Mon, Dec 9, 2013 at 11:42 AM, Bjorn Helgaas <***@google.com> wrote:
>>>>> That doesn't answer my question at all.
>>>>>
>>>>> I understand that this change makes it so Joseph doesn't have to use
>>>>> "pci=realloc=off". But why should auto-reallocation be limited to
>>>>> buses that have resources above 4GB? That doesn't make any sense.
>>>>>
>>>>> We should fix the reallocation code so it can deal with this case. If
>>>>> there's not enough space for everything, obviously we have to leave
>>>>> something unassigned. A ROM BAR is a good candidate for leaving
>>>>> unassigned, because most of the time we can get along without it.
>>>> Yes, that is ideal and not that simple.
>>>> but that would be hard to backport to old kernels.
>>>>
>>>> BTW, Joseph, can you try
>>>> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
>>>> for-pci-3.14
>>>> with pci=realloc=on
>>>>
>>>> on that system?
>>>>
>>>> Thanks
>>>>
>>>> Yinghai
>>> I noticed there was some back and forth on this thread. Do you still
>>> want me to test this version, Yinghai?
>> Yes, if that works, we would not need to put the patch in upstream for limiting
>> realloc auto scope.
>>
>
> Another user has confirmed that at test kernel from your branch[0] does
> resolve the bug.

Hi, Joe,

Bjorn does not want to limit auto realloc, so this patch can not make
to upstream at this point.

so we may have to ask user to append "pci=realloc=off" before we find
more smart way to realloc
resource.

Thanks

Yinghai
Joseph Salisbury
2014-01-10 21:54:20 UTC
Permalink
On 01/10/2014 12:13 PM, Yinghai Lu wrote:
> On Fri, Jan 10, 2014 at 8:19 AM, Joseph Salisbury
> <***@canonical.com> wrote:
>> On 12/11/2013 02:55 PM, Yinghai Lu wrote:
>>> On Wed, Dec 11, 2013 at 11:19 AM, Joseph Salisbury
>>> <***@canonical.com> wrote:
>>>> On 12/09/2013 03:10 PM, Yinghai Lu wrote:
>>>>> On Mon, Dec 9, 2013 at 11:42 AM, Bjorn Helgaas <***@google.com> wrote:
>>>>>> That doesn't answer my question at all.
>>>>>>
>>>>>> I understand that this change makes it so Joseph doesn't have to use
>>>>>> "pci=realloc=off". But why should auto-reallocation be limited to
>>>>>> buses that have resources above 4GB? That doesn't make any sense.
>>>>>>
>>>>>> We should fix the reallocation code so it can deal with this case. If
>>>>>> there's not enough space for everything, obviously we have to leave
>>>>>> something unassigned. A ROM BAR is a good candidate for leaving
>>>>>> unassigned, because most of the time we can get along without it.
>>>>> Yes, that is ideal and not that simple.
>>>>> but that would be hard to backport to old kernels.
>>>>>
>>>>> BTW, Joseph, can you try
>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
>>>>> for-pci-3.14
>>>>> with pci=realloc=on
>>>>>
>>>>> on that system?
>>>>>
>>>>> Thanks
>>>>>
>>>>> Yinghai
>>>> I noticed there was some back and forth on this thread. Do you still
>>>> want me to test this version, Yinghai?
>>> Yes, if that works, we would not need to put the patch in upstream for limiting
>>> realloc auto scope.
>>>
>> Another user has confirmed that at test kernel from your branch[0] does
>> resolve the bug.
> Hi, Joe,
>
> Bjorn does not want to limit auto realloc, so this patch can not make
> to upstream at this point.
>
> so we may have to ask user to append "pci=realloc=off" before we find
> more smart way to realloc
> resource.
>
> Thanks
>
> Yinghai
Hi Yinghai,

In a prior email you mentioned: "Yes, if that works, we would not need
to put the patch in upstream for limiting realloc auto scope." Is the
git tree at
git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
somehow different now?

Thanks,

Joe
Yinghai Lu
2014-01-10 23:12:48 UTC
Permalink
On Fri, Jan 10, 2014 at 1:54 PM, Joseph Salisbury
<***@canonical.com> wrote:
> On 01/10/2014 12:13 PM, Yinghai Lu wrote:
>
> In a prior email you mentioned: "Yes, if that works, we would not need
> to put the patch in upstream for limiting realloc auto scope." Is the
> git tree at
> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
> somehow different now?

but you boot with "pci=realloc=off", right?

Anyway please try pci/next to see if there is any help.

git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/next
Joseph Salisbury
2014-09-16 20:21:34 UTC
Permalink
On 01/10/2014 12:13 PM, Yinghai Lu wrote:
> On Fri, Jan 10, 2014 at 8:19 AM, Joseph Salisbury
> <***@canonical.com> wrote:
>> On 12/11/2013 02:55 PM, Yinghai Lu wrote:
>>> On Wed, Dec 11, 2013 at 11:19 AM, Joseph Salisbury
>>> <***@canonical.com> wrote:
>>>> On 12/09/2013 03:10 PM, Yinghai Lu wrote:
>>>>> On Mon, Dec 9, 2013 at 11:42 AM, Bjorn Helgaas <***@google.com> wrote:
>>>>>> That doesn't answer my question at all.
>>>>>>
>>>>>> I understand that this change makes it so Joseph doesn't have to use
>>>>>> "pci=realloc=off". But why should auto-reallocation be limited to
>>>>>> buses that have resources above 4GB? That doesn't make any sense.
>>>>>>
>>>>>> We should fix the reallocation code so it can deal with this case. If
>>>>>> there's not enough space for everything, obviously we have to leave
>>>>>> something unassigned. A ROM BAR is a good candidate for leaving
>>>>>> unassigned, because most of the time we can get along without it.
>>>>> Yes, that is ideal and not that simple.
>>>>> but that would be hard to backport to old kernels.
>>>>>
>>>>> BTW, Joseph, can you try
>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
>>>>> for-pci-3.14
>>>>> with pci=realloc=on
>>>>>
>>>>> on that system?
>>>>>
>>>>> Thanks
>>>>>
>>>>> Yinghai
>>>> I noticed there was some back and forth on this thread. Do you still
>>>> want me to test this version, Yinghai?
>>> Yes, if that works, we would not need to put the patch in upstream for limiting
>>> realloc auto scope.
>>>
>> Another user has confirmed that at test kernel from your branch[0] does
>> resolve the bug.
> Hi, Joe,
>
> Bjorn does not want to limit auto realloc, so this patch can not make
> to upstream at this point.
>
> so we may have to ask user to append "pci=realloc=off" before we find
> more smart way to realloc
> resource.
>
> Thanks
>
> Yinghai

Hi Yinghai,

A new bug[0] was opened due to enabling PCI_REALLOC_ENABLE_AUTO, which
is similar to the original bug[1] we discussed.

Just wondering if there have been any additional ideas on realloc since
this was last discussed?

Thanks,

Joe


[0] http://pad.lv/1363313
[1] http://pad.lv/1245938
Yinghai Lu
2014-09-16 22:27:32 UTC
Permalink
On Tue, Sep 16, 2014 at 1:21 PM, Joseph Salisbury
<***@canonical.com> wrote:
>
> A new bug[0] was opened due to enabling PCI_REALLOC_ENABLE_AUTO, which
> is similar to the original bug[1] we discussed.
>
> Just wondering if there have been any additional ideas on realloc since
> this was last discussed?
>
> [0] http://pad.lv/1363313
> [1] http://pad.lv/1245938

This one looks different, that LSI card support SRIOV, but BIOS does not
allocate resource to SRIOV bar.
We release old resource and ...reallocate resource to them with two retries.

[ 0.321983] pci_bus 0000:00: max bus depth: 1 pci_try_num: 2
[ 0.322010] pci 0000:00:03.0: BAR 15: assigned [mem
0xef800000-0xef8fffff pref]
[ 0.322025] pci 0000:01:00.0: reg 0x174: [mem 0x00000000-0x00003fff 64bit]
[ 0.322033] pci 0000:01:00.0: reg 0x17c: [mem 0x00000000-0x0003ffff 64bit]
[ 0.322040] pci 0000:01:00.0: reg 0x174: [mem 0x00000000-0x00003fff 64bit]
[ 0.322042] pci 0000:01:00.0: BAR 6: assigned [mem
0xef800000-0xef87ffff pref]
[ 0.322051] pci 0000:01:00.0: reg 0x17c: [mem 0x00000000-0x0003ffff 64bit]
[ 0.322053] pci 0000:01:00.0: BAR 9: can't assign mem (size 0x400000)
[ 0.322061] pci 0000:01:00.0: reg 0x174: [mem 0x00000000-0x00003fff 64bit]
[ 0.322063] pci 0000:01:00.0: BAR 7: assigned [mem
0xfbd00000-0xfbd3ffff 64bit]
[ 0.322070] pci 0000:00:03.0: PCI bridge to [bus 01]
[ 0.322073] pci 0000:00:03.0: bridge window [io 0xc000-0xcfff]
[ 0.322077] pci 0000:00:03.0: bridge window [mem 0xfbd00000-0xfbdfffff]
[ 0.322080] pci 0000:00:03.0: bridge window [mem
0xef800000-0xef8fffff pref]
[ 0.322142] pci_bus 0000:00: No. 2 try to assign unassigned res
[ 0.322143] release child resource [mem 0xfbd00000-0xfbd3ffff 64bit]
[ 0.322144] release child resource [mem 0xfbd80000-0xfbdbffff 64bit]
[ 0.322145] release child resource [mem 0xfbdfc000-0xfbdfffff 64bit]
[ 0.322147] pci 0000:00:03.0: resource 14 [mem
0xfbd00000-0xfbdfffff] released
[ 0.322148] pci 0000:00:03.0: PCI bridge to [bus 01]
[ 0.322156] pci 0000:00:03.0: bridge window [mem
0x00100000-0x001fffff] to [bus 01] add_size 500000
[ 0.322174] pci 0000:00:03.0: res[14]=[mem 0x00100000-0x001fffff]
get_res_add_size add_size 500000
[ 0.322176] pci 0000:00:03.0: BAR 14: assigned [mem 0xefb00000-0xf00fffff]
[ 0.322185] pci 0000:01:00.0: reg 0x174: [mem 0xfbd00000-0xfbd03fff 64bit]
[ 0.322192] pci 0000:01:00.0: reg 0x17c: [mem 0x00000000-0x0003ffff 64bit]
[ 0.322194] pci 0000:01:00.0: res[9]=[mem
0x00000000-0xffffffffffffffff 64bit] get_res_add_size add_size 400000
[ 0.322196] pci 0000:01:00.0: res[7]=[mem
0x00000000-0xffffffffffffffff 64bit] get_res_add_size add_size 40000
[ 0.322198] pci 0000:01:00.0: BAR 3: assigned [mem
0xefb00000-0xefb3ffff 64bit]
[ 0.322211] pci 0000:01:00.0: reg 0x17c: [mem 0x00000000-0x0003ffff 64bit]
[ 0.322212] pci 0000:01:00.0: BAR 9: assigned [mem
0xefb40000-0xeff3ffff 64bit]
[ 0.322219] pci 0000:01:00.0: BAR 1: assigned [mem
0xeff40000-0xeff43fff 64bit]
[ 0.322232] pci 0000:01:00.0: reg 0x174: [mem 0xfbd00000-0xfbd03fff 64bit]
[ 0.322234] pci 0000:01:00.0: BAR 7: assigned [mem
0xeff44000-0xeff83fff 64bit]
[ 0.322241] pci 0000:00:03.0: PCI bridge to [bus 01]
[ 0.322244] pci 0000:00:03.0: bridge window [io 0xc000-0xcfff]
[ 0.322248] pci 0000:00:03.0: bridge window [mem 0xefb00000-0xf00fffff]
[ 0.322251] pci 0000:00:03.0: bridge window [mem
0xef800000-0xef8fffff 64bit pref]

So the resource realloc do work as expected. but LSI firmware has some
problem again?

We may need to add command after the reset the bridge resource.

Can you get bootlog with "pci=earlydump"?

Thanks

Yinghai
Yinghai Lu
2014-09-17 07:46:24 UTC
Permalink
On Tue, Sep 16, 2014 at 3:27 PM, Yinghai Lu <***@kernel.org> wrote:
> On Tue, Sep 16, 2014 at 1:21 PM, Joseph Salisbury
> <***@canonical.com> wrote:
>>
>> A new bug[0] was opened due to enabling PCI_REALLOC_ENABLE_AUTO, which
>> is similar to the original bug[1] we discussed.
>>
>> Just wondering if there have been any additional ideas on realloc since
>> this was last discussed?
>>
>> [0] http://pad.lv/1363313
>
> So the resource realloc do work as expected. but LSI firmware has some
> problem again?
>
> We may need to add command after the reset the bridge resource.

Can you try attached patch?

Thanks

Yinghai
Bjorn Helgaas
2014-09-24 23:48:19 UTC
Permalink
On Wed, Sep 17, 2014 at 12:46:24AM -0700, Yinghai Lu wrote:
> On Tue, Sep 16, 2014 at 3:27 PM, Yinghai Lu <***@kernel.org> wrote:
> > On Tue, Sep 16, 2014 at 1:21 PM, Joseph Salisbury
> > <***@canonical.com> wrote:
> >>
> >> A new bug[0] was opened due to enabling PCI_REALLOC_ENABLE_AUTO, which
> >> is similar to the original bug[1] we discussed.
> >>
> >> Just wondering if there have been any additional ideas on realloc since
> >> this was last discussed?
> >>
> >> [0] http://pad.lv/1363313
> >
> > So the resource realloc do work as expected. but LSI firmware has some
> > problem again?
> >
> > We may need to add command after the reset the bridge resource.
>
> Can you try attached patch?

This sitting in patchwork, but I'm dropping it because there's no changelog
or signed-off-by. I don't think the underlying bug has been resolved,
though.

Bjorn

> ---
> drivers/pci/setup-bus.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> 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
> @@ -1615,6 +1615,22 @@ again:
> fail_res->flags & type_mask,
> rel_type);
>
> + /* reset LSI device */
> + list_for_each_entry(fail_res, &fail_head, list) {
> + struct pci_dev *pdev;
> + struct pci_bus *pbus = fail_res->dev->bus;
> +
> + if (pci_is_root_bus(pbus) || !pbus->self)
> + continue;
> +
> + /* LSI device firmware is not happy with changing BAR value */
> + list_for_each_entry(pdev, &pbus->devices, bus_list)
> + if (pdev->vendor == PCI_VENDOR_ID_LSI_LOGIC) {
> + pci_reset_secondary_bus(pbus->self);
> + break;
> + }
> + }
> +
> /* restore size and flags */
> list_for_each_entry(fail_res, &fail_head, list) {
> struct resource *res = fail_res->res;
Joseph Salisbury
2014-09-25 18:19:00 UTC
Permalink
On 09/17/2014 03:46 AM, Yinghai Lu wrote:
> On Tue, Sep 16, 2014 at 3:27 PM, Yinghai Lu <***@kernel.org> wrote:
>> On Tue, Sep 16, 2014 at 1:21 PM, Joseph Salisbury
>> <***@canonical.com> wrote:
>>> A new bug[0] was opened due to enabling PCI_REALLOC_ENABLE_AUTO, which
>>> is similar to the original bug[1] we discussed.
>>>
>>> Just wondering if there have been any additional ideas on realloc since
>>> this was last discussed?
>>>
>>> [0] http://pad.lv/1363313
>> So the resource realloc do work as expected. but LSI firmware has some
>> problem again?
>>
>> We may need to add command after the reset the bridge resource.
> Can you try attached patch?
>
> Thanks
>
> Yinghai
Sorry, this message was marked as SPAM for some reason. Do you still
want this patch tested?
Yinghai Lu
2014-09-25 18:24:01 UTC
Permalink
On Thu, Sep 25, 2014 at 11:19 AM, Joseph Salisbury
<***@canonical.com> wrote:
> On 09/17/2014 03:46 AM, Yinghai Lu wrote:
>> On Tue, Sep 16, 2014 at 3:27 PM, Yinghai Lu <***@kernel.org> wrote:
>>> On Tue, Sep 16, 2014 at 1:21 PM, Joseph Salisbury
>>> <***@canonical.com> wrote:
>>>> A new bug[0] was opened due to enabling PCI_REALLOC_ENABLE_AUTO, which
>>>> is similar to the original bug[1] we discussed.
>>>>
>>>> Just wondering if there have been any additional ideas on realloc since
>>>> this was last discussed?
>>>>
>>>> [0] http://pad.lv/1363313
>>> So the resource realloc do work as expected. but LSI firmware has some
>>> problem again?
>>>
>>> We may need to add command after the reset the bridge resource.
>> Can you try attached patch?
>>
>> Thanks
>>
>> Yinghai
> Sorry, this message was marked as SPAM for some reason. Do you still
> want this patch tested?

Yes, Please.

We need special handling for LSI devices.

Thanks

Yinghai
Joseph Salisbury
2014-09-25 15:49:12 UTC
Permalink
On 09/16/2014 06:27 PM, Yinghai Lu wrote:
> On Tue, Sep 16, 2014 at 1:21 PM, Joseph Salisbury
> <***@canonical.com> wrote:
>> A new bug[0] was opened due to enabling PCI_REALLOC_ENABLE_AUTO, which
>> is similar to the original bug[1] we discussed.
>>
>> Just wondering if there have been any additional ideas on realloc since
>> this was last discussed?
>>
>> [0] http://pad.lv/1363313
>> [1] http://pad.lv/1245938
> This one looks different, that LSI card support SRIOV, but BIOS does not
> allocate resource to SRIOV bar.
> We release old resource and ...reallocate resource to them with two retries.
>
> [ 0.321983] pci_bus 0000:00: max bus depth: 1 pci_try_num: 2
> [ 0.322010] pci 0000:00:03.0: BAR 15: assigned [mem
> 0xef800000-0xef8fffff pref]
> [ 0.322025] pci 0000:01:00.0: reg 0x174: [mem 0x00000000-0x00003fff 64bit]
> [ 0.322033] pci 0000:01:00.0: reg 0x17c: [mem 0x00000000-0x0003ffff 64bit]
> [ 0.322040] pci 0000:01:00.0: reg 0x174: [mem 0x00000000-0x00003fff 64bit]
> [ 0.322042] pci 0000:01:00.0: BAR 6: assigned [mem
> 0xef800000-0xef87ffff pref]
> [ 0.322051] pci 0000:01:00.0: reg 0x17c: [mem 0x00000000-0x0003ffff 64bit]
> [ 0.322053] pci 0000:01:00.0: BAR 9: can't assign mem (size 0x400000)
> [ 0.322061] pci 0000:01:00.0: reg 0x174: [mem 0x00000000-0x00003fff 64bit]
> [ 0.322063] pci 0000:01:00.0: BAR 7: assigned [mem
> 0xfbd00000-0xfbd3ffff 64bit]
> [ 0.322070] pci 0000:00:03.0: PCI bridge to [bus 01]
> [ 0.322073] pci 0000:00:03.0: bridge window [io 0xc000-0xcfff]
> [ 0.322077] pci 0000:00:03.0: bridge window [mem 0xfbd00000-0xfbdfffff]
> [ 0.322080] pci 0000:00:03.0: bridge window [mem
> 0xef800000-0xef8fffff pref]
> [ 0.322142] pci_bus 0000:00: No. 2 try to assign unassigned res
> [ 0.322143] release child resource [mem 0xfbd00000-0xfbd3ffff 64bit]
> [ 0.322144] release child resource [mem 0xfbd80000-0xfbdbffff 64bit]
> [ 0.322145] release child resource [mem 0xfbdfc000-0xfbdfffff 64bit]
> [ 0.322147] pci 0000:00:03.0: resource 14 [mem
> 0xfbd00000-0xfbdfffff] released
> [ 0.322148] pci 0000:00:03.0: PCI bridge to [bus 01]
> [ 0.322156] pci 0000:00:03.0: bridge window [mem
> 0x00100000-0x001fffff] to [bus 01] add_size 500000
> [ 0.322174] pci 0000:00:03.0: res[14]=[mem 0x00100000-0x001fffff]
> get_res_add_size add_size 500000
> [ 0.322176] pci 0000:00:03.0: BAR 14: assigned [mem 0xefb00000-0xf00fffff]
> [ 0.322185] pci 0000:01:00.0: reg 0x174: [mem 0xfbd00000-0xfbd03fff 64bit]
> [ 0.322192] pci 0000:01:00.0: reg 0x17c: [mem 0x00000000-0x0003ffff 64bit]
> [ 0.322194] pci 0000:01:00.0: res[9]=[mem
> 0x00000000-0xffffffffffffffff 64bit] get_res_add_size add_size 400000
> [ 0.322196] pci 0000:01:00.0: res[7]=[mem
> 0x00000000-0xffffffffffffffff 64bit] get_res_add_size add_size 40000
> [ 0.322198] pci 0000:01:00.0: BAR 3: assigned [mem
> 0xefb00000-0xefb3ffff 64bit]
> [ 0.322211] pci 0000:01:00.0: reg 0x17c: [mem 0x00000000-0x0003ffff 64bit]
> [ 0.322212] pci 0000:01:00.0: BAR 9: assigned [mem
> 0xefb40000-0xeff3ffff 64bit]
> [ 0.322219] pci 0000:01:00.0: BAR 1: assigned [mem
> 0xeff40000-0xeff43fff 64bit]
> [ 0.322232] pci 0000:01:00.0: reg 0x174: [mem 0xfbd00000-0xfbd03fff 64bit]
> [ 0.322234] pci 0000:01:00.0: BAR 7: assigned [mem
> 0xeff44000-0xeff83fff 64bit]
> [ 0.322241] pci 0000:00:03.0: PCI bridge to [bus 01]
> [ 0.322244] pci 0000:00:03.0: bridge window [io 0xc000-0xcfff]
> [ 0.322248] pci 0000:00:03.0: bridge window [mem 0xefb00000-0xf00fffff]
> [ 0.322251] pci 0000:00:03.0: bridge window [mem
> 0xef800000-0xef8fffff 64bit pref]
>
> So the resource realloc do work as expected. but LSI firmware has some
> problem again?
>
> We may need to add command after the reset the bridge resource.
>
> Can you get bootlog with "pci=earlydump"?
>
> Thanks
>
> Yinghai
Hi Yinghai,

The user collected earlydump data, which can be found here:
https://launchpadlibrarian.net/185141163/dmesg-pci%3Dearlydump
Bjorn Helgaas
2014-09-25 16:04:44 UTC
Permalink
On Thu, Sep 25, 2014 at 9:49 AM, Joseph Salisbury
<***@canonical.com> wrote:
> On 09/16/2014 06:27 PM, Yinghai Lu wrote:
>> On Tue, Sep 16, 2014 at 1:21 PM, Joseph Salisbury
>> <***@canonical.com> wrote:
>>> A new bug[0] was opened due to enabling PCI_REALLOC_ENABLE_AUTO, which
>>> is similar to the original bug[1] we discussed.
>>>
>>> Just wondering if there have been any additional ideas on realloc since
>>> this was last discussed?
>>>
>>> [0] http://pad.lv/1363313
>>> [1] http://pad.lv/1245938

As a side-note, this looks like a regression and I should have given
it more attention, but I don't track all the vendor bug databases. If
you open a kernel.bug bugzilla and mark it as a regression, I *do*
track that regularly. For regressions, it's helpful if you can attach
dmesg logs from working and non-working kernels that are as close
together as possible. It'd be ideal if those were upstream kernels,
but I doubt you apply any patches that would be relevant, so I think
logs from Ubuntu kernels would be enough.

Bjorn
Joseph Salisbury
2014-09-25 18:04:31 UTC
Permalink
On 09/25/2014 12:04 PM, Bjorn Helgaas wrote:
> On Thu, Sep 25, 2014 at 9:49 AM, Joseph Salisbury
> <***@canonical.com> wrote:
>> On 09/16/2014 06:27 PM, Yinghai Lu wrote:
>>> On Tue, Sep 16, 2014 at 1:21 PM, Joseph Salisbury
>>> <***@canonical.com> wrote:
>>>> A new bug[0] was opened due to enabling PCI_REALLOC_ENABLE_AUTO, which
>>>> is similar to the original bug[1] we discussed.
>>>>
>>>> Just wondering if there have been any additional ideas on realloc since
>>>> this was last discussed?
>>>>
>>>> [0] http://pad.lv/1363313
>>>> [1] http://pad.lv/1245938
> As a side-note, this looks like a regression and I should have given
> it more attention, but I don't track all the vendor bug databases. If
> you open a kernel.bug bugzilla and mark it as a regression, I *do*
> track that regularly. For regressions, it's helpful if you can attach
> dmesg logs from working and non-working kernels that are as close
> together as possible. It'd be ideal if those were upstream kernels,
> but I doubt you apply any patches that would be relevant, so I think
> logs from Ubuntu kernels would be enough.
>
> Bjorn

Thanks, Bjorn. I'll open a bugzilla bug and ask the bug reporters for
the requested dmesg data. Thanks again for the feedback.

Joe
Yinghai Lu
2014-09-25 18:23:13 UTC
Permalink
On Thu, Sep 25, 2014 at 8:49 AM, Joseph Salisbury
<***@canonical.com> wrote:
> On 09/16/2014 06:27 PM, Yinghai Lu wrote:
>> On Tue, Sep 16, 2014 at 1:21 PM, Joseph Salisbury
>> <***@canonical.com> wrote:
>>> A new bug[0] was opened due to enabling PCI_REALLOC_ENABLE_AUTO, which
>>> is similar to the original bug[1] we discussed.
>>>
>>> Just wondering if there have been any additional ideas on realloc since
>>> this was last discussed?
>>>
>>> [0] http://pad.lv/1363313
>>> [1] http://pad.lv/1245938
>> This one looks different, that LSI card support SRIOV, but BIOS does not
>> allocate resource to SRIOV bar.
>> We release old resource and ...reallocate resource to them with two retries.
>
> The user collected earlydump data, which can be found here:
> https://launchpadlibrarian.net/185141163/dmesg-pci%3Dearlydump

Did you ask the user to test reset_lsi_pci_devices.patch?

> ---
> drivers/pci/setup-bus.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> 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
> @@ -1615,6 +1615,22 @@ again:
> fail_res->flags & type_mask,
> rel_type);
>
> + /* reset LSI device */
> + list_for_each_entry(fail_res, &fail_head, list) {
> + struct pci_dev *pdev;
> + struct pci_bus *pbus = fail_res->dev->bus;
> +
> + if (pci_is_root_bus(pbus) || !pbus->self)
> + continue;
> +
> + /* LSI device firmware is not happy with changing BAR value */
> + list_for_each_entry(pdev, &pbus->devices, bus_list)
> + if (pdev->vendor == PCI_VENDOR_ID_LSI_LOGIC) {
> + pci_reset_secondary_bus(pbus->self);
> + break;
> + }
> + }
> +
> /* restore size and flags */
> list_for_each_entry(fail_res, &fail_head, list) {
> struct resource *res = fail_res->res;

Thanks

Yinghai
Loading...