c16c722: PCI: only enable pci realloc when SRIOV bar is not assigned
868faa5: PCI: print out suggestion about using pci=realloc
ace6e3c: PCI: Make pci bridge reallocating enabled/disabled
3cf1011: PCI: Retry on type IORESOURCE_IO allocation.
b6d30ee: PCI: Skip reset cardbus assigned resource during pci bus rescan
2bc1cf0: PCI: Fix cardbus bridge resources as optional size handling
e6c495b: PCI: Disable cardbus bridge MEM1 pref CTL
d1e2d75: PCI: Fix /sys warning when sriov enabled card is hot removed
29834f2: pci: Fix pci cardbus removal
are left over after Jesse pickup more of them.
it will try to auto detect if need to use pci=realloc, and print out suggestion.
including fixing some pci carbus handling.
could get from
git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci3
-v2: add two patches before, it will fix pci cardbus removal, and srov removal
warning
the first one should be with v3.3.
Thanks
Yinghai
Documentation/kernel-parameters.txt | 8 ++-
drivers/pci/iov.c | 7 ++-
drivers/pci/pci.c | 4 +-
drivers/pci/pci.h | 2 +-
drivers/pci/remove.c | 28 +++++--
drivers/pci/setup-bus.c | 159 +++++++++++++++++++++++++----------
6 files changed, 153 insertions(+), 55 deletions(-)
During reenabling pci reallocating for pci bridge by clean the small size in
bridge and assign with requested + optional size for first several try,
Ram mention could have problem with one case
https://bugzilla.kernel.org/show_bug.cgi?id=15960
After checking the booting log in
https://lkml.org/lkml/2010/4/19/44
[regression, bisected] Xonar DX invalid PCI I/O range since 977d17bb174
We should not stop too early for io ports.
Apr 19 10:19:38 [kernel] pci 0000:04:00.0: BAR 7: can't assign io (size 0x4000)
Apr 19 10:19:38 [kernel] pci 0000:05:01.0: BAR 8: assigned [mem 0x80400000-0x805fffff]
Apr 19 10:19:38 [kernel] pci 0000:05:01.0: BAR 7: can't assign io (size 0x2000)
Apr 19 10:19:38 [kernel] pci 0000:05:02.0: BAR 7: can't assign io (size 0x1000)
Apr 19 10:19:38 [kernel] pci 0000:05:03.0: BAR 7: can't assign io (size 0x1000)
Apr 19 10:19:38 [kernel] pci 0000:08:00.0: BAR 7: can't assign io (size 0x1000)
Apr 19 10:19:38 [kernel] pci 0000:09:04.0: BAR 0: can't assign io (size 0x100)
and clear 00:1c.0 to retry again.
The patch remove the IORESOUCE_IO checking, and try one more time.
and we will have chance to get allocation for 00:1c.0 io port range because
from 0x4000 to 0x8000 could be used.
Signed-off-by: Yinghai Lu <[email protected]>
---
drivers/pci/setup-bus.c | 12 +-----------
1 files changed, 1 insertions(+), 11 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 2991a89..162edfb 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1292,7 +1292,6 @@ pci_assign_unassigned_resources(void)
struct pci_dev_resource *fail_res;
unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
IORESOURCE_PREFETCH;
- unsigned long failed_type;
int pci_try_num = 1;
/* don't realloc if asked to do so */
@@ -1327,16 +1326,7 @@ again:
if (list_empty(&fail_head))
goto enable_and_dump;
- failed_type = 0;
- list_for_each_entry(fail_res, &fail_head, list)
- failed_type |= fail_res->flags;
-
- /*
- * io port are tight, don't try extra
- * or if reach the limit, don't want to try more
- */
- failed_type &= type_mask;
- if ((failed_type == IORESOURCE_IO) || (tried_times >= pci_try_num)) {
+ if (tried_times >= pci_try_num) {
free_list(&fail_head);
goto enable_and_dump;
}
--
1.7.7
let user know they could try if pci=realloc could help.
Suggested-by: Jesse Barnes <[email protected]>
Signed-off-by: Yinghai Lu <[email protected]>
---
drivers/pci/setup-bus.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 22454c5..9526038 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1346,6 +1346,9 @@ again:
goto enable_and_dump;
if (tried_times >= pci_try_num) {
+ if (pci_realloc_enable == enable_not_set)
+ printk(KERN_INFO "Some pci devices resources are not assigned, please try to boot with pci=realloc\n");
+
free_list(&fail_head);
goto enable_and_dump;
}
--
1.7.7
If bios does not assign those BAR or wrong address, then kernel will
try to do pci realloc.
in that case, user still can use pci=realloc=off to override it.
Signed-off-by: Yinghai Lu <[email protected]>
---
drivers/pci/setup-bus.c | 28 ++++++++++++++++++++++++++++
1 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 9526038..520f256 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1293,6 +1293,31 @@ static bool __init pci_realloc_enabled(void)
return pci_realloc_enable >= enable_yes_user;
}
+static void __init pci_realloc_detect(void)
+{
+ struct pci_dev *dev = NULL;
+
+ if (pci_realloc_enable != enable_not_set)
+ return;
+
+#ifdef CONFIG_PCI_IOV
+ for_each_pci_dev(dev) {
+ int i;
+
+ for (i = PCI_IOV_RESOURCES; i <= PCI_IOV_RESOURCE_END; i++) {
+ struct resource *r = &dev->resource[i];
+
+ /* Not assigned, or rejected by kernel */
+ if (r->flags && !r->start) {
+ pci_realloc_enable = enable_yes_detected;
+
+ return;
+ }
+ }
+ }
+#endif
+}
+
/*
* first try will not touch pci bridge res
* second and later try will clear small leaf bridge res
@@ -1314,6 +1339,7 @@ pci_assign_unassigned_resources(void)
int pci_try_num = 1;
/* don't realloc if asked to do so */
+ pci_realloc_detect();
if (pci_realloc_enabled()) {
int max_depth = pci_get_max_depth();
@@ -1348,6 +1374,8 @@ again:
if (tried_times >= pci_try_num) {
if (pci_realloc_enable == enable_not_set)
printk(KERN_INFO "Some pci devices resources are not assigned, please try to boot with pci=realloc\n");
+ else if (pci_realloc_enable == enable_yes_detected)
+ printk(KERN_INFO "Automatically enabled pci realloc, if you have problem, please try to boot with pci=realloc=off\n");
free_list(&fail_head);
goto enable_and_dump;
--
1.7.7
Let the user could enable and disable with pci=realloc=on or pci=realloc=off
Also
1. move variable and functions near the place they are used.
2. change macro to function
3. change related functions and variable to static and _init
4. update parameter description accordingly.
-v2: still honor pci=realloc, and treat it as pci=realloc=on
also use enum instead of ...
Signed-off-by: Yinghai Lu <[email protected]>
---
Documentation/kernel-parameters.txt | 8 ++++++--
drivers/pci/pci.c | 4 +++-
drivers/pci/pci.h | 2 +-
drivers/pci/setup-bus.c | 33 ++++++++++++++++++++++++++-------
4 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 033d4e6..e11e5dd 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2109,8 +2109,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
the default.
off: Turn ECRC off
on: Turn ECRC on.
- realloc reallocate PCI resources if allocations done by BIOS
- are erroneous.
+ realloc= Enable/disable reallocating PCI bridge resources
+ if allocations done by BIOS are too small to fit
+ resources required by children devices.
+ off: Turn realloc off
+ on: Turn realloc on
+ realloc same as realloc=on
pcie_aspm= [PCIE] Forcibly enable or disable PCIe Active State Power
Management.
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 9c89447..09cb1f2 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3744,8 +3744,10 @@ static int __init pci_setup(char *str)
pci_no_msi();
} else if (!strcmp(str, "noaer")) {
pci_no_aer();
+ } else if (!strncmp(str, "realloc=", 8)) {
+ pci_realloc_get_opt(str + 8);
} else if (!strncmp(str, "realloc", 7)) {
- pci_realloc();
+ pci_realloc_get_opt("on");
} else if (!strcmp(str, "nodomains")) {
pci_no_domains();
} else if (!strncmp(str, "cbiosize=", 9)) {
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index aaf7ff8..f862a5a 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -148,7 +148,7 @@ static inline void pci_no_msi(void) { }
static inline void pci_msi_init_pci_dev(struct pci_dev *dev) { }
#endif
-extern void pci_realloc(void);
+void pci_realloc_get_opt(char *);
static inline int pci_no_d1d2(struct pci_dev *dev)
{
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 162edfb..22454c5 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -48,13 +48,6 @@ static void free_list(struct list_head *head)
}
}
-int pci_realloc_enable = 0;
-#define pci_realloc_enabled() pci_realloc_enable
-void pci_realloc(void)
-{
- pci_realloc_enable = 1;
-}
-
/**
* add_to_list() - add a new resource tracker to the list
* @head: Head of the list
@@ -1273,6 +1266,32 @@ static int __init pci_get_max_depth(void)
return depth;
}
+/*
+ * -1: undefined, will auto detect later
+ * 0: disabled by user
+ * 1: enabled by user
+ * 2: enabled by auto detect
+ */
+enum enable_type {
+ enable_not_set = -1,
+ enable_no_user,
+ enable_no_detected,
+ enable_yes_user,
+ enable_yes_detected,
+};
+
+static enum enable_type pci_realloc_enable __initdata = enable_not_set;
+void __init pci_realloc_get_opt(char *str)
+{
+ if (!strncmp(str, "off", 3))
+ pci_realloc_enable = enable_no_user;
+ else if (!strncmp(str, "on", 2))
+ pci_realloc_enable = enable_yes_user;
+}
+static bool __init pci_realloc_enabled(void)
+{
+ return pci_realloc_enable >= enable_yes_user;
+}
/*
* first try will not touch pci bridge res
--
1.7.7
otherwise when rescan is used for cardbus, assigned resource will get
cleared.
Signed-off-by: Yinghai Lu <[email protected]>
---
drivers/pci/setup-bus.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 3b3932a..2991a89 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -901,6 +901,8 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
resource_size_t b_res_3_size = pci_cardbus_mem_size * 2;
u16 ctrl;
+ if (b_res[0].parent)
+ goto handle_b_res_1;
/*
* Reserve some resources for CardBus. We reserve
* a fixed amount of bus space for CardBus bridges.
@@ -914,6 +916,9 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
pci_cardbus_io_size);
}
+handle_b_res_1:
+ if (b_res[1].parent)
+ goto handle_b_res_2;
b_res[1].start = pci_cardbus_io_size;
b_res[1].end = b_res[1].start + pci_cardbus_io_size - 1;
b_res[1].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
@@ -923,6 +928,7 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
pci_cardbus_io_size);
}
+handle_b_res_2:
/* MEM1 must not be pref mmio */
pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM1) {
@@ -942,6 +948,8 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
}
+ if (b_res[2].parent)
+ goto handle_b_res_3;
/*
* If we have prefetchable memory support, allocate
* two regions. Otherwise, allocate one region of
@@ -962,6 +970,9 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
b_res_3_size = pci_cardbus_mem_size;
}
+handle_b_res_3:
+ if (b_res[3].parent)
+ goto handle_done;
b_res[3].start = pci_cardbus_mem_size;
b_res[3].end = b_res[3].start + b_res_3_size - 1;
b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_STARTALIGN;
@@ -970,6 +981,9 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
add_to_list(realloc_head, bridge, b_res+3, b_res_3_size,
pci_cardbus_mem_size);
}
+
+handle_done:
+ ;
}
void __ref __pci_bus_size_bridges(struct pci_bus *bus,
--
1.7.7
We should not set the requested size to -2.
that will confuse the resource list sorting with align when SIZEALIGN is used.
Change to STARTALIGN and pass align from start.
We are safe to do that just as we do that regular pci bridge.
In the long run, we should just treat cardbus like regular pci bridge.
Also fix when realloc is not passed, should keep the requested size.
Signed-off-by: Yinghai Lu <[email protected]>
---
drivers/pci/setup-bus.c | 63 ++++++++++++++++++++++++++--------------------
1 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index d5897c3..3b3932a 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -898,21 +898,30 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
{
struct pci_dev *bridge = bus->self;
struct resource *b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
+ resource_size_t b_res_3_size = pci_cardbus_mem_size * 2;
u16 ctrl;
/*
* Reserve some resources for CardBus. We reserve
* a fixed amount of bus space for CardBus bridges.
*/
- b_res[0].start = 0;
- b_res[0].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
- if (realloc_head)
- add_to_list(realloc_head, bridge, b_res, pci_cardbus_io_size, 0 /* dont care */);
+ b_res[0].start = pci_cardbus_io_size;
+ b_res[0].end = b_res[0].start + pci_cardbus_io_size - 1;
+ b_res[0].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
+ if (realloc_head) {
+ b_res[0].end -= pci_cardbus_io_size;
+ add_to_list(realloc_head, bridge, b_res, pci_cardbus_io_size,
+ pci_cardbus_io_size);
+ }
- b_res[1].start = 0;
- b_res[1].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
- if (realloc_head)
- add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size, 0 /* dont care */);
+ b_res[1].start = pci_cardbus_io_size;
+ b_res[1].end = b_res[1].start + pci_cardbus_io_size - 1;
+ b_res[1].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
+ if (realloc_head) {
+ b_res[1].end -= pci_cardbus_io_size;
+ add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size,
+ pci_cardbus_io_size);
+ }
/* MEM1 must not be pref mmio */
pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
@@ -939,28 +948,28 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
* twice the size.
*/
if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
- b_res[2].start = 0;
- b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_SIZEALIGN;
- if (realloc_head)
- add_to_list(realloc_head, bridge, b_res+2, pci_cardbus_mem_size, 0 /* dont care */);
+ b_res[2].start = pci_cardbus_mem_size;
+ b_res[2].end = b_res[2].start + pci_cardbus_mem_size - 1;
+ b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH |
+ IORESOURCE_STARTALIGN;
+ if (realloc_head) {
+ b_res[2].end -= pci_cardbus_mem_size;
+ add_to_list(realloc_head, bridge, b_res+2,
+ pci_cardbus_mem_size, pci_cardbus_mem_size);
+ }
- b_res[3].start = 0;
- b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
- if (realloc_head)
- add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size, 0 /* dont care */);
- } else {
- b_res[3].start = 0;
- b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
- if (realloc_head)
- add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size * 2, 0 /* dont care */);
+ /* reduce that to half */
+ b_res_3_size = pci_cardbus_mem_size;
}
- /* set the size of the resource to zero, so that the resource does not
- * get assigned during required-resource allocation cycle but gets assigned
- * during the optional-resource allocation cycle.
- */
- b_res[0].start = b_res[1].start = b_res[2].start = b_res[3].start = 1;
- b_res[0].end = b_res[1].end = b_res[2].end = b_res[3].end = 0;
+ b_res[3].start = pci_cardbus_mem_size;
+ b_res[3].end = b_res[3].start + b_res_3_size - 1;
+ b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_STARTALIGN;
+ if (realloc_head) {
+ b_res[3].end -= b_res_3_size;
+ add_to_list(realloc_head, bridge, b_res+3, b_res_3_size,
+ pci_cardbus_mem_size);
+ }
}
void __ref __pci_bus_size_bridges(struct pci_bus *bus,
--
1.7.7
Some BIOS enable both pref for MEM0 and MEM1.
but we assume MEM1 is non-pref...
Signed-off-by: Yinghai Lu <[email protected]>
---
drivers/pci/setup-bus.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 090217a..d5897c3 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -914,6 +914,14 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
if (realloc_head)
add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size, 0 /* dont care */);
+ /* MEM1 must not be pref mmio */
+ pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
+ if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM1) {
+ ctrl &= ~PCI_CB_BRIDGE_CTL_PREFETCH_MEM1;
+ pci_write_config_word(bridge, PCI_CB_BRIDGE_CONTROL, ctrl);
+ pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
+ }
+
/*
* Check whether prefetchable memory is supported
* by this bridge.
--
1.7.7
During recent strick checking about sysfs_remove from Eric.
it will spit more bitter warning.
For SRIOV hotplug, we are calling pci_stop_dev() for VF at first.
(after we update pci_stop_bus_devices).
that pci_stop_dev will calling device_unregiste for that VF, so that directory
in VF is removed already.
We double checking if that VF dir in /sys is there, before try removing that
physfn link.
Signed-of-by: Yinghai Lu <[email protected]>
---
drivers/pci/iov.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 0321fa3..dfc7d65 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -173,7 +173,12 @@ static void virtfn_remove(struct pci_dev *dev, int id, int reset)
sprintf(buf, "virtfn%u", id);
sysfs_remove_link(&dev->dev.kobj, buf);
- sysfs_remove_link(&virtfn->dev.kobj, "physfn");
+ /*
+ * pci_stop_dev() could be called for this virtfn before already
+ * so directory for the virtfn is removed before.
+ */
+ if (virtfn->dev.kobj.sd)
+ sysfs_remove_link(&virtfn->dev.kobj, "physfn");
mutex_lock(&iov->dev->sriov->lock);
pci_remove_bus_device(virtfn);
--
1.7.7
During test busn_res allocation with cardbus, found pci card removal is not
working anymore, and it turns out it is broken by:
|commit 79cc9601c3e42b4f0650fe7e69132ebce7ab48f9
|Date: Tue Nov 22 21:06:53 2011 -0800
|
| PCI: Only call pci_stop_bus_device() one time for child devices at remove
that patch changed pci_remove_behind_bridge behavoir that yenta_carbus depends.
Restore the old behavoir of pci_remove_behind_bridge by:
1. rename pci_remove_behind_bridge to __pci_remove_behind_bridge, and let
__pci_remove_bus_device() call it instead.
2. add pci-stop_befind_bridge that will stop device under bridge
3. add back pci_remove_behind_bridge that will stop and remove device
under bridge.
This one is for v3.3
-v2: update commit description a little bit.
Signed-off-by: Yinghai Lu <[email protected]>
---
drivers/pci/remove.c | 28 ++++++++++++++++++++++------
1 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index bd70f23..82f8ae5 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -77,6 +77,7 @@ void pci_remove_bus(struct pci_bus *pci_bus)
}
EXPORT_SYMBOL(pci_remove_bus);
+static void __pci_remove_behind_bridge(struct pci_dev *dev);
/**
* pci_remove_bus_device - remove a PCI device and any children
* @dev: the device to remove
@@ -94,7 +95,7 @@ static void __pci_remove_bus_device(struct pci_dev *dev)
if (dev->subordinate) {
struct pci_bus *b = dev->subordinate;
- pci_remove_behind_bridge(dev);
+ __pci_remove_behind_bridge(dev);
pci_remove_bus(b);
dev->subordinate = NULL;
}
@@ -107,6 +108,24 @@ void pci_remove_bus_device(struct pci_dev *dev)
__pci_remove_bus_device(dev);
}
+static void __pci_remove_behind_bridge(struct pci_dev *dev)
+{
+ struct list_head *l, *n;
+
+ if (dev->subordinate)
+ list_for_each_safe(l, n, &dev->subordinate->devices)
+ __pci_remove_bus_device(pci_dev_b(l));
+}
+
+static void pci_stop_behind_bridge(struct pci_dev *dev)
+{
+ struct list_head *l, *n;
+
+ if (dev->subordinate)
+ list_for_each_safe(l, n, &dev->subordinate->devices)
+ pci_stop_bus_device(pci_dev_b(l));
+}
+
/**
* pci_remove_behind_bridge - remove all devices behind a PCI bridge
* @dev: PCI bridge device
@@ -117,11 +136,8 @@ void pci_remove_bus_device(struct pci_dev *dev)
*/
void pci_remove_behind_bridge(struct pci_dev *dev)
{
- struct list_head *l, *n;
-
- if (dev->subordinate)
- list_for_each_safe(l, n, &dev->subordinate->devices)
- __pci_remove_bus_device(pci_dev_b(l));
+ pci_stop_behind_bridge(dev);
+ __pci_remove_behind_bridge(dev);
}
static void pci_stop_bus_devices(struct pci_bus *bus)
--
1.7.7
On Sat, Feb 04, 2012 at 10:55:03PM -0800, Yinghai Lu wrote:
> We should not set the requested size to -2.
>
> that will confuse the resource list sorting with align when SIZEALIGN is used.
>
> Change to STARTALIGN and pass align from start.
>
> We are safe to do that just as we do that regular pci bridge.
>
> In the long run, we should just treat cardbus like regular pci bridge.
>
> Also fix when realloc is not passed, should keep the requested size.
>
> Signed-off-by: Yinghai Lu <[email protected]>
> ---
> drivers/pci/setup-bus.c | 63 ++++++++++++++++++++++++++--------------------
> 1 files changed, 36 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index d5897c3..3b3932a 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -898,21 +898,30 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
> {
> struct pci_dev *bridge = bus->self;
> struct resource *b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
> + resource_size_t b_res_3_size = pci_cardbus_mem_size * 2;
> u16 ctrl;
>
> /*
> * Reserve some resources for CardBus. We reserve
> * a fixed amount of bus space for CardBus bridges.
> */
> - b_res[0].start = 0;
> - b_res[0].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
> - if (realloc_head)
> - add_to_list(realloc_head, bridge, b_res, pci_cardbus_io_size, 0 /* dont care */);
> + b_res[0].start = pci_cardbus_io_size;
> + b_res[0].end = b_res[0].start + pci_cardbus_io_size - 1;
> + b_res[0].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
> + if (realloc_head) {
> + b_res[0].end -= pci_cardbus_io_size;
> + add_to_list(realloc_head, bridge, b_res, pci_cardbus_io_size,
> + pci_cardbus_io_size);
> + }
>
> - b_res[1].start = 0;
> - b_res[1].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
> - if (realloc_head)
> - add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size, 0 /* dont care */);
> + b_res[1].start = pci_cardbus_io_size;
> + b_res[1].end = b_res[1].start + pci_cardbus_io_size - 1;
> + b_res[1].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
> + if (realloc_head) {
> + b_res[1].end -= pci_cardbus_io_size;
> + add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size,
> + pci_cardbus_io_size);
> + }
>
> /* MEM1 must not be pref mmio */
> pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
> @@ -939,28 +948,28 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
> * twice the size.
> */
> if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
> - b_res[2].start = 0;
> - b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_SIZEALIGN;
> - if (realloc_head)
> - add_to_list(realloc_head, bridge, b_res+2, pci_cardbus_mem_size, 0 /* dont care */);
> + b_res[2].start = pci_cardbus_mem_size;
> + b_res[2].end = b_res[2].start + pci_cardbus_mem_size - 1;
> + b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH |
> + IORESOURCE_STARTALIGN;
> + if (realloc_head) {
> + b_res[2].end -= pci_cardbus_mem_size;
> + add_to_list(realloc_head, bridge, b_res+2,
> + pci_cardbus_mem_size, pci_cardbus_mem_size);
> + }
>
> - b_res[3].start = 0;
> - b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
> - if (realloc_head)
> - add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size, 0 /* dont care */);
The b_res_3_size has to be reduced to half here.
Otherwise it will try allocate 2*pci_cardbus_mem_size to the BAR3..
b_res_3_size = pci_cardbus_mem_size;
> - } else {
> - b_res[3].start = 0;
> - b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
> - if (realloc_head)
> - add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size * 2, 0 /* dont care */);
> + /* reduce that to half */
> + b_res_3_size = pci_cardbus_mem_size;
b_res3_3_size should not be updated here, since BAR3 has to be allocated 2*pci_cardbus_mem_size
resource.
RP
> }
>
> - /* set the size of the resource to zero, so that the resource does not
> - * get assigned during required-resource allocation cycle but gets assigned
> - * during the optional-resource allocation cycle.
> - */
> - b_res[0].start = b_res[1].start = b_res[2].start = b_res[3].start = 1;
> - b_res[0].end = b_res[1].end = b_res[2].end = b_res[3].end = 0;
> + b_res[3].start = pci_cardbus_mem_size;
> + b_res[3].end = b_res[3].start + b_res_3_size - 1;
> + b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_STARTALIGN;
> + if (realloc_head) {
> + b_res[3].end -= b_res_3_size;
> + add_to_list(realloc_head, bridge, b_res+3, b_res_3_size,
> + pci_cardbus_mem_size);
> + }
> }
>
> void __ref __pci_bus_size_bridges(struct pci_bus *bus,
> --
> 1.7.7
On Tue, Feb 7, 2012 at 8:35 PM, Ram Pai <[email protected]> wrote:
> On Sat, Feb 04, 2012 at 10:55:03PM -0800, Yinghai Lu wrote:
>> We should not set the requested size to -2.
>>
>> that will confuse the resource list sorting with align when SIZEALIGN is used.
>>
>> Change to STARTALIGN and pass align from start.
>>
>> We are safe to do that just as we do that regular pci bridge.
>>
>> In the long run, we should just treat cardbus like regular pci bridge.
>>
>> Also fix when realloc is not passed, should keep the requested size.
>>
>> Signed-off-by: Yinghai Lu <[email protected]>
>> ---
>> ?drivers/pci/setup-bus.c | ? 63 ++++++++++++++++++++++++++--------------------
>> ?1 files changed, 36 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>> index d5897c3..3b3932a 100644
>> --- a/drivers/pci/setup-bus.c
>> +++ b/drivers/pci/setup-bus.c
>> @@ -898,21 +898,30 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
>> ?{
>> ? ? ? struct pci_dev *bridge = bus->self;
>> ? ? ? struct resource *b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
>> + ? ? resource_size_t b_res_3_size = pci_cardbus_mem_size * 2;
>> ? ? ? u16 ctrl;
>>
>> ? ? ? /*
>> ? ? ? ?* Reserve some resources for CardBus. ?We reserve
>> ? ? ? ?* a fixed amount of bus space for CardBus bridges.
>> ? ? ? ?*/
>> - ? ? b_res[0].start = 0;
>> - ? ? b_res[0].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
>> - ? ? if (realloc_head)
>> - ? ? ? ? ? ? add_to_list(realloc_head, bridge, b_res, pci_cardbus_io_size, 0 /* dont care */);
>> + ? ? b_res[0].start = pci_cardbus_io_size;
>> + ? ? b_res[0].end = b_res[0].start + pci_cardbus_io_size - 1;
>> + ? ? b_res[0].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
>> + ? ? if (realloc_head) {
>> + ? ? ? ? ? ? b_res[0].end -= pci_cardbus_io_size;
>> + ? ? ? ? ? ? add_to_list(realloc_head, bridge, b_res, pci_cardbus_io_size,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? pci_cardbus_io_size);
>> + ? ? }
>>
>> - ? ? b_res[1].start = 0;
>> - ? ? b_res[1].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
>> - ? ? if (realloc_head)
>> - ? ? ? ? ? ? add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size, 0 /* dont care */);
>> + ? ? b_res[1].start = pci_cardbus_io_size;
>> + ? ? b_res[1].end = b_res[1].start + pci_cardbus_io_size - 1;
>> + ? ? b_res[1].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
>> + ? ? if (realloc_head) {
>> + ? ? ? ? ? ? b_res[1].end -= pci_cardbus_io_size;
>> + ? ? ? ? ? ? add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pci_cardbus_io_size);
>> + ? ? }
>>
>> ? ? ? /* MEM1 must not be pref mmio */
>> ? ? ? pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
>> @@ -939,28 +948,28 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
>> ? ? ? ?* twice the size.
>> ? ? ? ?*/
>> ? ? ? if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
>> - ? ? ? ? ? ? b_res[2].start = 0;
>> - ? ? ? ? ? ? b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_SIZEALIGN;
>> - ? ? ? ? ? ? if (realloc_head)
>> - ? ? ? ? ? ? ? ? ? ? add_to_list(realloc_head, bridge, b_res+2, pci_cardbus_mem_size, 0 /* dont care */);
>> + ? ? ? ? ? ? b_res[2].start = pci_cardbus_mem_size;
>> + ? ? ? ? ? ? b_res[2].end = b_res[2].start + pci_cardbus_mem_size - 1;
>> + ? ? ? ? ? ? b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH |
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? IORESOURCE_STARTALIGN;
>> + ? ? ? ? ? ? if (realloc_head) {
>> + ? ? ? ? ? ? ? ? ? ? b_res[2].end -= pci_cardbus_mem_size;
>> + ? ? ? ? ? ? ? ? ? ? add_to_list(realloc_head, bridge, b_res+2,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pci_cardbus_mem_size, pci_cardbus_mem_size);
>> + ? ? ? ? ? ? }
>>
>> - ? ? ? ? ? ? b_res[3].start = 0;
>> - ? ? ? ? ? ? b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
>> - ? ? ? ? ? ? if (realloc_head)
>> - ? ? ? ? ? ? ? ? ? ? add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size, 0 /* dont care */);
>
> The b_res_3_size has to be reduced to half here.
> Otherwise it will try allocate 2*pci_cardbus_mem_size to the BAR3..
>
> ? ? ? ? ? ? ? ?b_res_3_size = pci_cardbus_mem_size;
>
>
>> - ? ? } else {
>> - ? ? ? ? ? ? b_res[3].start = 0;
>> - ? ? ? ? ? ? b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
>> - ? ? ? ? ? ? if (realloc_head)
>> - ? ? ? ? ? ? ? ? ? ? add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size * 2, 0 /* dont care */);
>> + ? ? ? ? ? ? /* reduce that to half */
>> + ? ? ? ? ? ? b_res_3_size = pci_cardbus_mem_size;
>
> b_res3_3_size should not be updated here, since BAR3 has to be allocated 2*pci_cardbus_mem_size
> resource.
>
looks like you did not read the patch correctly
that else {} get removed already in this patch
please check segment after patch...
static void pci_bus_size_cardbus(struct pci_bus *bus,
struct list_head *realloc_head)
{
struct pci_dev *bridge = bus->self;
struct resource *b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
resource_size_t b_res_3_size = pci_cardbus_mem_size * 2;
u16 ctrl;
/*
* Reserve some resources for CardBus. We reserve
* a fixed amount of bus space for CardBus bridges.
*/
b_res[0].start = pci_cardbus_io_size;
b_res[0].end = b_res[0].start + pci_cardbus_io_size - 1;
b_res[0].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
if (realloc_head) {
b_res[0].end -= pci_cardbus_io_size;
add_to_list(realloc_head, bridge, b_res, pci_cardbus_io_size,
pci_cardbus_io_size);
}
b_res[1].start = pci_cardbus_io_size;
b_res[1].end = b_res[1].start + pci_cardbus_io_size - 1;
b_res[1].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
if (realloc_head) {
b_res[1].end -= pci_cardbus_io_size;
add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size,
pci_cardbus_io_size);
}
/* MEM1 must not be pref mmio */
pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM1) {
ctrl &= ~PCI_CB_BRIDGE_CTL_PREFETCH_MEM1;
pci_write_config_word(bridge, PCI_CB_BRIDGE_CONTROL, ctrl);
pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
}
/*
* Check whether prefetchable memory is supported
* by this bridge.
*/
pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
if (!(ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0)) {
ctrl |= PCI_CB_BRIDGE_CTL_PREFETCH_MEM0;
pci_write_config_word(bridge, PCI_CB_BRIDGE_CONTROL, ctrl);
pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
}
/*
* If we have prefetchable memory support, allocate
* two regions. Otherwise, allocate one region of
* twice the size.
*/
if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
b_res[2].start = pci_cardbus_mem_size;
b_res[2].end = b_res[2].start + pci_cardbus_mem_size - 1;
b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH |
IORESOURCE_STARTALIGN;
if (realloc_head) {
b_res[2].end -= pci_cardbus_mem_size;
add_to_list(realloc_head, bridge, b_res+2,
pci_cardbus_mem_size, pci_cardbus_mem_size);
}
/* reduce that to half */
b_res_3_size = pci_cardbus_mem_size;
}
b_res[3].start = pci_cardbus_mem_size;
b_res[3].end = b_res[3].start + b_res_3_size - 1;
b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_STARTALIGN;
if (realloc_head) {
b_res[3].end -= b_res_3_size;
add_to_list(realloc_head, bridge, b_res+3, b_res_3_size,
pci_cardbus_mem_size);
}
}
On Tue, Feb 07, 2012 at 08:48:36PM -0800, Yinghai Lu wrote:
> On Tue, Feb 7, 2012 at 8:35 PM, Ram Pai <[email protected]> wrote:
> > On Sat, Feb 04, 2012 at 10:55:03PM -0800, Yinghai Lu wrote:
> >> We should not set the requested size to -2.
> >>
> >> that will confuse the resource list sorting with align when SIZEALIGN is used.
> >>
> >> Change to STARTALIGN and pass align from start.
> >>
> >> We are safe to do that just as we do that regular pci bridge.
> >>
> >> In the long run, we should just treat cardbus like regular pci bridge.
> >>
> >> Also fix when realloc is not passed, should keep the requested size.
> >>
> >> Signed-off-by: Yinghai Lu <[email protected]>
> >> ---
> >> ?drivers/pci/setup-bus.c | ? 63 ++++++++++++++++++++++++++--------------------
> >> ?1 files changed, 36 insertions(+), 27 deletions(-)
> >>
> >> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> >> index d5897c3..3b3932a 100644
> >> --- a/drivers/pci/setup-bus.c
> >> +++ b/drivers/pci/setup-bus.c
> >> @@ -898,21 +898,30 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
..snip
> >> - ? ? ? ? ? ? b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
> >> - ? ? ? ? ? ? if (realloc_head)
> >> - ? ? ? ? ? ? ? ? ? ? add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size, 0 /* dont care */);
> >
> > The b_res_3_size has to be reduced to half here.
> > Otherwise it will try allocate 2*pci_cardbus_mem_size to the BAR3..
> >
> > ? ? ? ? ? ? ? ?b_res_3_size = pci_cardbus_mem_size;
> >
> >
> >> - ? ? } else {
> >> - ? ? ? ? ? ? b_res[3].start = 0;
> >> - ? ? ? ? ? ? b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
> >> - ? ? ? ? ? ? if (realloc_head)
> >> - ? ? ? ? ? ? ? ? ? ? add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size * 2, 0 /* dont care */);
> >> + ? ? ? ? ? ? /* reduce that to half */
> >> + ? ? ? ? ? ? b_res_3_size = pci_cardbus_mem_size;
> >
> > b_res3_3_size should not be updated here, since BAR3 has to be allocated 2*pci_cardbus_mem_size
> > resource.
> >
>
> looks like you did not read the patch correctly
>
> that else {} get removed already in this patch
>
> please check segment after patch...
> /*
..snip..
> * If we have prefetchable memory support, allocate
> * two regions. Otherwise, allocate one region of
> * twice the size.
> */
> if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
> b_res[2].start = pci_cardbus_mem_size;
> b_res[2].end = b_res[2].start + pci_cardbus_mem_size - 1;
> b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH |
> IORESOURCE_STARTALIGN;
> if (realloc_head) {
> b_res[2].end -= pci_cardbus_mem_size;
> add_to_list(realloc_head, bridge, b_res+2,
> pci_cardbus_mem_size, pci_cardbus_mem_size);
> }
>
> /* reduce that to half */
> b_res_3_size = pci_cardbus_mem_size;
> }
ACK. Ok. got it. This looks correct.
RP
On Tue, Feb 7, 2012 at 9:01 PM, Ram Pai <[email protected]> wrote:
>
> ..snip..
>> ? ? ? ?* If we have prefetchable memory support, allocate
>> ? ? ? ?* two regions. ?Otherwise, allocate one region of
>> ? ? ? ?* twice the size.
>> ? ? ? ?*/
>> ? ? ? if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
>> ? ? ? ? ? ? ? b_res[2].start = pci_cardbus_mem_size;
>> ? ? ? ? ? ? ? b_res[2].end = b_res[2].start + pci_cardbus_mem_size - 1;
>> ? ? ? ? ? ? ? b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH |
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? IORESOURCE_STARTALIGN;
>> ? ? ? ? ? ? ? if (realloc_head) {
>> ? ? ? ? ? ? ? ? ? ? ? b_res[2].end -= pci_cardbus_mem_size;
>> ? ? ? ? ? ? ? ? ? ? ? add_to_list(realloc_head, bridge, b_res+2,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pci_cardbus_mem_size, pci_cardbus_mem_size);
>> ? ? ? ? ? ? ? }
>>
>> ? ? ? ? ? ? ? /* reduce that to half */
>> ? ? ? ? ? ? ? b_res_3_size = pci_cardbus_mem_size;
>> ? ? ? }
>
> ACK. Ok. got it. This looks correct.
Good, So we have
Acked-by: Ram Pai <[email protected]>
Tested-by: Dominik Brodowski <[email protected]>
Hope Jesse will not miss to add them
Thanks
Yinghai
On Sat, 4 Feb 2012 22:55:00 -0800
Yinghai Lu <[email protected]> wrote:
> During test busn_res allocation with cardbus, found pci card removal is not
> working anymore, and it turns out it is broken by:
>
> |commit 79cc9601c3e42b4f0650fe7e69132ebce7ab48f9
> |Date: Tue Nov 22 21:06:53 2011 -0800
> |
> | PCI: Only call pci_stop_bus_device() one time for child devices at remove
>
> that patch changed pci_remove_behind_bridge behavoir that yenta_carbus depends.
>
> Restore the old behavoir of pci_remove_behind_bridge by:
> 1. rename pci_remove_behind_bridge to __pci_remove_behind_bridge, and let
> __pci_remove_bus_device() call it instead.
> 2. add pci-stop_befind_bridge that will stop device under bridge
> 3. add back pci_remove_behind_bridge that will stop and remove device
> under bridge.
>
> This one is for v3.3
Applied to -fixes with Dominik's tested-by. A nice cleanup on top
might be to rename pci_remove_behind_bridge to
pci_stop_remove_behind_bridge or something to indicate that it's
calling stop each time as well...
--
Jesse Barnes, Intel Open Source Technology Center
On Sat, 4 Feb 2012 22:55:01 -0800
Yinghai Lu <[email protected]> wrote:
> During recent strick checking about sysfs_remove from Eric.
> it will spit more bitter warning.
>
> For SRIOV hotplug, we are calling pci_stop_dev() for VF at first.
> (after we update pci_stop_bus_devices).
>
> that pci_stop_dev will calling device_unregiste for that VF, so that directory
> in VF is removed already.
>
> We double checking if that VF dir in /sys is there, before try removing that
> physfn link.
>
> Signed-of-by: Yinghai Lu <[email protected]>
> ---
> drivers/pci/iov.c | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 0321fa3..dfc7d65 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -173,7 +173,12 @@ static void virtfn_remove(struct pci_dev *dev, int id, int reset)
>
> sprintf(buf, "virtfn%u", id);
> sysfs_remove_link(&dev->dev.kobj, buf);
> - sysfs_remove_link(&virtfn->dev.kobj, "physfn");
> + /*
> + * pci_stop_dev() could be called for this virtfn before already
> + * so directory for the virtfn is removed before.
> + */
> + if (virtfn->dev.kobj.sd)
> + sysfs_remove_link(&virtfn->dev.kobj, "physfn");
>
> mutex_lock(&iov->dev->sriov->lock);
> pci_remove_bus_device(virtfn);
Cleaned up the commit & comment text a bit and put into -next. Please
check it out.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
On Sat, 4 Feb 2012 22:55:02 -0800
Yinghai Lu <[email protected]> wrote:
> Some BIOS enable both pref for MEM0 and MEM1.
>
> but we assume MEM1 is non-pref...
>
> Signed-off-by: Yinghai Lu <[email protected]>
> ---
> drivers/pci/setup-bus.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 090217a..d5897c3 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -914,6 +914,14 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
> if (realloc_head)
> add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size, 0 /* dont care */);
>
> + /* MEM1 must not be pref mmio */
> + pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
> + if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM1) {
> + ctrl &= ~PCI_CB_BRIDGE_CTL_PREFETCH_MEM1;
> + pci_write_config_word(bridge, PCI_CB_BRIDGE_CONTROL, ctrl);
> + pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
> + }
> +
> /*
> * Check whether prefetchable memory is supported
> * by this bridge.
Does this actually fix any bugs? Dominik, have you tested it?
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
On Tue, 7 Feb 2012 22:11:22 -0800
Yinghai Lu <[email protected]> wrote:
> On Tue, Feb 7, 2012 at 9:01 PM, Ram Pai <[email protected]> wrote:
> >
> > ..snip..
> >> * If we have prefetchable memory support, allocate
> >> * two regions. Otherwise, allocate one region of
> >> * twice the size.
> >> */
> >> if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
> >> b_res[2].start = pci_cardbus_mem_size;
> >> b_res[2].end = b_res[2].start + pci_cardbus_mem_size - 1;
> >> b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH |
> >> IORESOURCE_STARTALIGN;
> >> if (realloc_head) {
> >> b_res[2].end -= pci_cardbus_mem_size;
> >> add_to_list(realloc_head, bridge, b_res+2,
> >> pci_cardbus_mem_size, pci_cardbus_mem_size);
> >> }
> >>
> >> /* reduce that to half */
> >> b_res_3_size = pci_cardbus_mem_size;
> >> }
> >
> > ACK. Ok. got it. This looks correct.
>
> Good, So we have
>
> Acked-by: Ram Pai <[email protected]>
> Tested-by: Dominik Brodowski <[email protected]>
>
> Hope Jesse will not miss to add them
Yeah looks fine. I'd like to see an ack from Dominik on 5/9 as well
though. Can you include the acks and tested-bys when you re-post 3-9
with bug references and any other comments included?
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
On Fri, Feb 10, 2012 at 12:46 PM, Jesse Barnes <[email protected]> wrote:
> On Sat, ?4 Feb 2012 22:55:02 -0800
> Yinghai Lu <[email protected]> wrote:
>
>> Some BIOS enable both pref for MEM0 and MEM1.
>>
>> but we assume MEM1 is non-pref...
>>
>> Signed-off-by: Yinghai Lu <[email protected]>
>> ---
>> ?drivers/pci/setup-bus.c | ? ?8 ++++++++
>> ?1 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>> index 090217a..d5897c3 100644
>> --- a/drivers/pci/setup-bus.c
>> +++ b/drivers/pci/setup-bus.c
>> @@ -914,6 +914,14 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
>> ? ? ? if (realloc_head)
>> ? ? ? ? ? ? ? add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size, 0 /* dont care */);
>>
>> + ? ? /* MEM1 must not be pref mmio */
>> + ? ? pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
>> + ? ? if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM1) {
>> + ? ? ? ? ? ? ctrl &= ~PCI_CB_BRIDGE_CTL_PREFETCH_MEM1;
>> + ? ? ? ? ? ? pci_write_config_word(bridge, PCI_CB_BRIDGE_CONTROL, ctrl);
>> + ? ? ? ? ? ? pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
>> + ? ? }
>> +
>> ? ? ? /*
>> ? ? ? ?* Check whether prefetchable memory is supported
>> ? ? ? ?* by this bridge.
>
> Does this actually fix any bugs?
at least, one of laptop have this problem.
> Dominik, have you tested it?
he tested my for-pci3 and for-pci-busn-alloc branches, both branches
have this patch.
and it does not cause problem to him.
Yinghai
On Fri, Feb 10, 2012 at 12:52 PM, Jesse Barnes <[email protected]> wrote:
>> > ACK. Ok. got it. This looks correct.
>>
>> Good, So we have
>>
>> Acked-by: Ram Pai <[email protected]>
>> Tested-by: Dominik Brodowski <[email protected]>
>>
>> Hope Jesse will not miss to add them
>
> Yeah looks fine. ?I'd like to see an ack from Dominik on 5/9 as well
> though. ?Can you include the acks and tested-bys when you re-post 3-9
> with bug references and any other comments included?
sure. Will send them updated acks and etc.
Yinghai
On Sat, 4 Feb 2012 22:55:06 -0800
Yinghai Lu <[email protected]> wrote:
> Let the user could enable and disable with pci=realloc=on or pci=realloc=off
>
> Also
> 1. move variable and functions near the place they are used.
> 2. change macro to function
> 3. change related functions and variable to static and _init
> 4. update parameter description accordingly.
>
> -v2: still honor pci=realloc, and treat it as pci=realloc=on
> also use enum instead of ...
>
> Signed-off-by: Yinghai Lu <[email protected]>
> ---
> Documentation/kernel-parameters.txt | 8 ++++++--
> drivers/pci/pci.c | 4 +++-
> drivers/pci/pci.h | 2 +-
> drivers/pci/setup-bus.c | 33 ++++++++++++++++++++++++++-------
> 4 files changed, 36 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 033d4e6..e11e5dd 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -2109,8 +2109,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> the default.
> off: Turn ECRC off
> on: Turn ECRC on.
> - realloc reallocate PCI resources if allocations done by BIOS
> - are erroneous.
> + realloc= Enable/disable reallocating PCI bridge resources
> + if allocations done by BIOS are too small to fit
> + resources required by children devices.
> + off: Turn realloc off
> + on: Turn realloc on
> + realloc same as realloc=on
"too small to accommodate resources required by all child devices"
Looks good otherwise.
--
Jesse Barnes, Intel Open Source Technology Center
On Sat, 4 Feb 2012 22:55:07 -0800
Yinghai Lu <[email protected]> wrote:
> let user know they could try if pci=realloc could help.
>
> Suggested-by: Jesse Barnes <[email protected]>
> Signed-off-by: Yinghai Lu <[email protected]>
> ---
> drivers/pci/setup-bus.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 22454c5..9526038 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1346,6 +1346,9 @@ again:
> goto enable_and_dump;
>
> if (tried_times >= pci_try_num) {
> + if (pci_realloc_enable == enable_not_set)
> + printk(KERN_INFO "Some pci devices resources are not assigned, please try to boot with pci=realloc\n");
> +
> free_list(&fail_head);
> goto enable_and_dump;
> }
"Some PCI device resources are unassigned, try booting with pci=realloc"
That said, I wonder if this will cause more problems than it solves.
In some cases the BIOS may not allocate all resources, but this will be
harmless since the user won't actually use the device, or the driver
for the device won't even use the BARs in question anyway (as happens a
lot for IO regions).
--
Jesse Barnes, Intel Open Source Technology Center
On Sat, 4 Feb 2012 22:55:08 -0800
Yinghai Lu <[email protected]> wrote:
> If bios does not assign those BAR or wrong address, then kernel will
> try to do pci realloc.
>
> in that case, user still can use pci=realloc=off to override it.
>
> Signed-off-by: Yinghai Lu <[email protected]>
> ---
> drivers/pci/setup-bus.c | 28 ++++++++++++++++++++++++++++
> 1 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 9526038..520f256 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1293,6 +1293,31 @@ static bool __init pci_realloc_enabled(void)
> return pci_realloc_enable >= enable_yes_user;
> }
>
> +static void __init pci_realloc_detect(void)
> +{
> + struct pci_dev *dev = NULL;
> +
> + if (pci_realloc_enable != enable_not_set)
> + return;
> +
> +#ifdef CONFIG_PCI_IOV
> + for_each_pci_dev(dev) {
> + int i;
> +
> + for (i = PCI_IOV_RESOURCES; i <= PCI_IOV_RESOURCE_END; i++) {
> + struct resource *r = &dev->resource[i];
> +
> + /* Not assigned, or rejected by kernel */
> + if (r->flags && !r->start) {
> + pci_realloc_enable = enable_yes_detected;
> +
> + return;
> + }
> + }
> + }
> +#endif
> +}
> +
> /*
> * first try will not touch pci bridge res
> * second and later try will clear small leaf bridge res
> @@ -1314,6 +1339,7 @@ pci_assign_unassigned_resources(void)
> int pci_try_num = 1;
>
> /* don't realloc if asked to do so */
> + pci_realloc_detect();
> if (pci_realloc_enabled()) {
> int max_depth = pci_get_max_depth();
>
> @@ -1348,6 +1374,8 @@ again:
> if (tried_times >= pci_try_num) {
> if (pci_realloc_enable == enable_not_set)
> printk(KERN_INFO "Some pci devices resources are not assigned, please try to boot with pci=realloc\n");
> + else if (pci_realloc_enable == enable_yes_detected)
> + printk(KERN_INFO "Automatically enabled pci realloc, if you have problem, please try to boot with pci=realloc=off\n");
>
> free_list(&fail_head);
> goto enable_and_dump;
Not sure I like this one either. Distros will want to enable IOV by
default, but in most configs users still won't use it, and many BIOSes
won't bother to allocate IOV space. So defaulting to trying to do so
will likely cause a lot of unnecessary re-allocation.
So distro guidance would be appreciated here for the default behavior.
--
Jesse Barnes, Intel Open Source Technology Center
On Fri, Feb 10, 2012 at 1:00 PM, Jesse Barnes <[email protected]> wrote:
> On Sat, ?4 Feb 2012 22:55:08 -0800
> Yinghai Lu <[email protected]> wrote:
>
>> If bios does not assign those BAR or wrong address, then kernel will
>> try to do pci realloc.
>>
>> in that case, user still can use pci=realloc=off to override it.
>>
>> Signed-off-by: Yinghai Lu <[email protected]>
>> ---
>> ?drivers/pci/setup-bus.c | ? 28 ++++++++++++++++++++++++++++
>> ?1 files changed, 28 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>> index 9526038..520f256 100644
>> --- a/drivers/pci/setup-bus.c
>> +++ b/drivers/pci/setup-bus.c
>> @@ -1293,6 +1293,31 @@ static bool __init pci_realloc_enabled(void)
>> ? ? ? return pci_realloc_enable >= enable_yes_user;
>> ?}
>>
>> +static void __init pci_realloc_detect(void)
>> +{
>> + ? ? struct pci_dev *dev = NULL;
>> +
>> + ? ? if (pci_realloc_enable != enable_not_set)
>> + ? ? ? ? ? ? return;
>> +
>> +#ifdef CONFIG_PCI_IOV
>> + ? ? for_each_pci_dev(dev) {
>> + ? ? ? ? ? ? int i;
>> +
>> + ? ? ? ? ? ? for (i = PCI_IOV_RESOURCES; i <= PCI_IOV_RESOURCE_END; i++) {
>> + ? ? ? ? ? ? ? ? ? ? struct resource *r = &dev->resource[i];
>> +
>> + ? ? ? ? ? ? ? ? ? ? /* Not assigned, or rejected by kernel */
>> + ? ? ? ? ? ? ? ? ? ? if (r->flags && !r->start) {
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? pci_realloc_enable = enable_yes_detected;
>> +
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? return;
>> + ? ? ? ? ? ? ? ? ? ? }
>> + ? ? ? ? ? ? }
>> + ? ? }
>> +#endif
>> +}
>> +
>> ?/*
>> ? * first try will not touch pci bridge res
>> ? * second ?and later try will clear small leaf bridge res
>> @@ -1314,6 +1339,7 @@ pci_assign_unassigned_resources(void)
>> ? ? ? int pci_try_num = 1;
>>
>> ? ? ? /* don't realloc if asked to do so */
>> + ? ? pci_realloc_detect();
>> ? ? ? if (pci_realloc_enabled()) {
>> ? ? ? ? ? ? ? int max_depth = pci_get_max_depth();
>>
>> @@ -1348,6 +1374,8 @@ again:
>> ? ? ? if (tried_times >= pci_try_num) {
>> ? ? ? ? ? ? ? if (pci_realloc_enable == enable_not_set)
>> ? ? ? ? ? ? ? ? ? ? ? printk(KERN_INFO "Some pci devices resources are not assigned, please try to boot with pci=realloc\n");
>> + ? ? ? ? ? ? else if (pci_realloc_enable == enable_yes_detected)
>> + ? ? ? ? ? ? ? ? ? ? printk(KERN_INFO "Automatically enabled pci realloc, if you have problem, please try to boot with pci=realloc=off\n");
>>
>> ? ? ? ? ? ? ? free_list(&fail_head);
>> ? ? ? ? ? ? ? goto enable_and_dump;
>
> Not sure I like this one either. ?Distros will want to enable IOV by
> default, but in most configs users still won't use it, and many BIOSes
> won't bother to allocate IOV space. ?So defaulting to trying to do so
> will likely cause a lot of unnecessary re-allocation.
>
> So distro guidance would be appreciated here for the default behavior.
Or do you prefer to:
if all SRIOV bar are not assigned, We will not enable SRIOV feature even?
Yinghai
On Fri, Feb 10, 2012 at 12:54 PM, Yinghai Lu <[email protected]> wrote:
> On Fri, Feb 10, 2012 at 12:46 PM, Jesse Barnes <[email protected]> wrote:
>> On Sat, ?4 Feb 2012 22:55:02 -0800
>> Yinghai Lu <[email protected]> wrote:
>>
>>> Some BIOS enable both pref for MEM0 and MEM1.
>>>
>>> but we assume MEM1 is non-pref...
>>>
>>> Signed-off-by: Yinghai Lu <[email protected]>
>>> ---
>>> ?drivers/pci/setup-bus.c | ? ?8 ++++++++
>>> ?1 files changed, 8 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>>> index 090217a..d5897c3 100644
>>> --- a/drivers/pci/setup-bus.c
>>> +++ b/drivers/pci/setup-bus.c
>>> @@ -914,6 +914,14 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
>>> ? ? ? if (realloc_head)
>>> ? ? ? ? ? ? ? add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size, 0 /* dont care */);
>>>
>>> + ? ? /* MEM1 must not be pref mmio */
>>> + ? ? pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
>>> + ? ? if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM1) {
>>> + ? ? ? ? ? ? ctrl &= ~PCI_CB_BRIDGE_CTL_PREFETCH_MEM1;
>>> + ? ? ? ? ? ? pci_write_config_word(bridge, PCI_CB_BRIDGE_CONTROL, ctrl);
>>> + ? ? ? ? ? ? pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
>>> + ? ? }
>>> +
>>> ? ? ? /*
>>> ? ? ? ?* Check whether prefetchable memory is supported
>>> ? ? ? ?* by this bridge.
>>
>> Does this actually fix any bugs?
>
> at least, one of laptop have this problem.
Rog?rio Brito's Clevo M5X0JE laptop seems to boot with both CardBus
mem apertures set to prefetchable. I think there's evidence of this
in one of the video boot logs Rog?rio mentioned in this thread:
https://lkml.org/lkml/2012/1/6/343 because I noticed the same thing
and fixed a nearby bug in lspci.
I looked for it, but didn't find it. I think it would be useful if
Yinghai could locate it, transcribe it into a bugzilla report, and
mention the bugzilla URL in this commit log.
I think Rog?rio did try the patch, but I don't know whether he
observed different behavior. I think it *does* fix a bug; it's just
that his box has more serious problems that we haven't figured out
yet.
Bjorn