* [PATCH] PCI: Reset failed bridge resources in PCI realloc
@ 2014-07-30 8:00 Guo Chao
2014-07-30 19:59 ` Yinghai Lu
0 siblings, 1 reply; 4+ messages in thread
From: Guo Chao @ 2014-07-30 8:00 UTC (permalink / raw)
To: linux-pci; +Cc: Guo Chao
We observed a problem on at least two POWER machines with PCIe
devices with very large SR-IOV BARs. The total size is larger than
host bridge MMIO window. We expect normal BARs of this device get address
successfuly, and SR-IOV BARs deserted. However, this only happens if PCI
realloc is disabled (pci=realloc=off or not config at all). Otherwise we
fail to assign both BARs.
Here is what happened when realloc is enabled:
* SR-IOV resources will NOT be considered as optional until the last
round of realloc. Both normal BARs and SR-IOV BARs of this device
will be sized and recorded in upstream bridge as initial size.
* Realloc never works because this bridge is too big, at end of
each round, the bridge is restored to the value before
assigning, i.e. it keeps its size.
* At last round, we finally consider SR-IOV resources optional
and size them seperately. We should get a small bridge this
time.
* However, in pbus_size_mem() we call calculate_memsize() to
calculate the final size. calculate_memsize() never shrinks
window, i.e. if old size is larger than newly sized result,
old size will be honored. Unfortunately, the old size at this
time is the sum of both normal BARs and SR-IOV BARs when doing
realloc.
* The bridge window failed to get address just like previous rounds.
Without PCI realloc, SR-IOV resources are considered as optional
from the beginning and sized seperately. The old size would be 0. We get
a small bridge window and assinged successfully. Everything work as
expected.
I actually don't quite understand what realloc does. Apparently this is
not the right fix ...
Cc: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com>
---
drivers/pci/setup-bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index a5a63ec..39837d7 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1623,7 +1623,7 @@ again:
res->end = fail_res->end;
res->flags = fail_res->flags;
if (fail_res->dev->subordinate)
- res->flags = 0;
+ res->flags = res->start = res->end = 0;
}
free_list(&fail_head);
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: Reset failed bridge resources in PCI realloc
2014-07-30 8:00 [PATCH] PCI: Reset failed bridge resources in PCI realloc Guo Chao
@ 2014-07-30 19:59 ` Yinghai Lu
2014-07-31 8:48 ` Guo Chao
2014-09-04 4:23 ` Bjorn Helgaas
0 siblings, 2 replies; 4+ messages in thread
From: Yinghai Lu @ 2014-07-30 19:59 UTC (permalink / raw)
To: Guo Chao; +Cc: linux-pci
[-- Attachment #1: Type: text/plain, Size: 2178 bytes --]
On Wed, Jul 30, 2014 at 1:00 AM, Guo Chao <yan@linux.vnet.ibm.com> wrote:
> * However, in pbus_size_mem() we call calculate_memsize() to
> calculate the final size. calculate_memsize() never shrinks
> window, i.e. if old size is larger than newly sized result,
> old size will be honored. Unfortunately, the old size at this
> time is the sum of both normal BARs and SR-IOV BARs when doing
> realloc.
>
that checking about old_size is added by:
| commit d65245c3297ac63abc51a976d92f45f2195d2854
| Author: Yinghai Lu <yinghai@kernel.org>
| Date: Fri Jan 22 01:02:23 2010 -0800
|
| PCI: don't shrink bridge resources
|
| When clearing leaf bridge resources, trying to get a big enough one, we
| could shrink the bridge if there is no resource under it. Confirm
| against the old resource side to make sure we're increasing the
| allocation.
so to make both cases ok, may need to check if there is any children devices
on the dev->subordinate. Please check attached.
Thanks
Yinghai
---
drivers/pci/setup-bus.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -1622,8 +1622,11 @@ again:
res->start = fail_res->start;
res->end = fail_res->end;
res->flags = fail_res->flags;
- if (fail_res->dev->subordinate)
+ if (fail_res->dev->subordinate) {
res->flags = 0;
+ if (!list_empty(&fail_res->dev->subordinate->devices))
+ res->start = res->end = 0;
+ }
}
free_list(&fail_head);
@@ -1688,8 +1691,11 @@ again:
res->start = fail_res->start;
res->end = fail_res->end;
res->flags = fail_res->flags;
- if (fail_res->dev->subordinate)
+ if (fail_res->dev->subordinate) {
res->flags = 0;
+ if (!list_empty(&fail_res->dev->subordinate->devices))
+ res->start = res->end = 0;
+ }
}
free_list(&fail_head);
[-- Attachment #2: shrink_realloc_bridge_resource.patch --]
[-- Type: text/x-patch, Size: 975 bytes --]
---
drivers/pci/setup-bus.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -1622,8 +1622,11 @@ again:
res->start = fail_res->start;
res->end = fail_res->end;
res->flags = fail_res->flags;
- if (fail_res->dev->subordinate)
+ if (fail_res->dev->subordinate) {
res->flags = 0;
+ if (!list_empty(&fail_res->dev->subordinate->devices))
+ res->start = res->end = 0;
+ }
}
free_list(&fail_head);
@@ -1688,8 +1691,11 @@ again:
res->start = fail_res->start;
res->end = fail_res->end;
res->flags = fail_res->flags;
- if (fail_res->dev->subordinate)
+ if (fail_res->dev->subordinate) {
res->flags = 0;
+ if (!list_empty(&fail_res->dev->subordinate->devices))
+ res->start = res->end = 0;
+ }
}
free_list(&fail_head);
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] PCI: Reset failed bridge resources in PCI realloc
2014-07-30 19:59 ` Yinghai Lu
@ 2014-07-31 8:48 ` Guo Chao
2014-09-04 4:23 ` Bjorn Helgaas
1 sibling, 0 replies; 4+ messages in thread
From: Guo Chao @ 2014-07-31 8:48 UTC (permalink / raw)
To: Yinghai Lu; +Cc: linux-pci
On Wed, Jul 30, 2014 at 12:59:35PM -0700, Yinghai Lu wrote:
> On Wed, Jul 30, 2014 at 1:00 AM, Guo Chao <yan@linux.vnet.ibm.com> wrote:
> > * However, in pbus_size_mem() we call calculate_memsize() to
> > calculate the final size. calculate_memsize() never shrinks
> > window, i.e. if old size is larger than newly sized result,
> > old size will be honored. Unfortunately, the old size at this
> > time is the sum of both normal BARs and SR-IOV BARs when doing
> > realloc.
> >
>
> that checking about old_size is added by:
>
> | commit d65245c3297ac63abc51a976d92f45f2195d2854
> | Author: Yinghai Lu <yinghai@kernel.org>
> | Date: Fri Jan 22 01:02:23 2010 -0800
> |
> | PCI: don't shrink bridge resources
> |
> | When clearing leaf bridge resources, trying to get a big enough one, we
> | could shrink the bridge if there is no resource under it. Confirm
> | against the old resource side to make sure we're increasing the
> | allocation.
>
> so to make both cases ok, may need to check if there is any children devices
> on the dev->subordinate. Please check attached.
>
Works well on one of the machines. Thanks.
Guo Chao
> Thanks
>
> Yinghai
>
>
> ---
> drivers/pci/setup-bus.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -1622,8 +1622,11 @@ again:
> res->start = fail_res->start;
> res->end = fail_res->end;
> res->flags = fail_res->flags;
> - if (fail_res->dev->subordinate)
> + if (fail_res->dev->subordinate) {
> res->flags = 0;
> + if (!list_empty(&fail_res->dev->subordinate->devices))
> + res->start = res->end = 0;
> + }
> }
> free_list(&fail_head);
>
> @@ -1688,8 +1691,11 @@ again:
> res->start = fail_res->start;
> res->end = fail_res->end;
> res->flags = fail_res->flags;
> - if (fail_res->dev->subordinate)
> + if (fail_res->dev->subordinate) {
> res->flags = 0;
> + if (!list_empty(&fail_res->dev->subordinate->devices))
> + res->start = res->end = 0;
> + }
> }
> free_list(&fail_head);
> ---
> drivers/pci/setup-bus.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -1622,8 +1622,11 @@ again:
> res->start = fail_res->start;
> res->end = fail_res->end;
> res->flags = fail_res->flags;
> - if (fail_res->dev->subordinate)
> + if (fail_res->dev->subordinate) {
> res->flags = 0;
> + if (!list_empty(&fail_res->dev->subordinate->devices))
> + res->start = res->end = 0;
> + }
> }
> free_list(&fail_head);
>
> @@ -1688,8 +1691,11 @@ again:
> res->start = fail_res->start;
> res->end = fail_res->end;
> res->flags = fail_res->flags;
> - if (fail_res->dev->subordinate)
> + if (fail_res->dev->subordinate) {
> res->flags = 0;
> + if (!list_empty(&fail_res->dev->subordinate->devices))
> + res->start = res->end = 0;
> + }
> }
> free_list(&fail_head);
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] PCI: Reset failed bridge resources in PCI realloc
2014-07-30 19:59 ` Yinghai Lu
2014-07-31 8:48 ` Guo Chao
@ 2014-09-04 4:23 ` Bjorn Helgaas
1 sibling, 0 replies; 4+ messages in thread
From: Bjorn Helgaas @ 2014-09-04 4:23 UTC (permalink / raw)
To: Yinghai Lu; +Cc: Guo Chao, linux-pci
On Wed, Jul 30, 2014 at 12:59:35PM -0700, Yinghai Lu wrote:
> On Wed, Jul 30, 2014 at 1:00 AM, Guo Chao <yan@linux.vnet.ibm.com> wrote:
> > * However, in pbus_size_mem() we call calculate_memsize() to
> > calculate the final size. calculate_memsize() never shrinks
> > window, i.e. if old size is larger than newly sized result,
> > old size will be honored. Unfortunately, the old size at this
> > time is the sum of both normal BARs and SR-IOV BARs when doing
> > realloc.
> >
>
> that checking about old_size is added by:
>
> | commit d65245c3297ac63abc51a976d92f45f2195d2854
> | Author: Yinghai Lu <yinghai@kernel.org>
> | Date: Fri Jan 22 01:02:23 2010 -0800
> |
> | PCI: don't shrink bridge resources
> |
> | When clearing leaf bridge resources, trying to get a big enough one, we
> | could shrink the bridge if there is no resource under it. Confirm
> | against the old resource side to make sure we're increasing the
> | allocation.
>
> so to make both cases ok, may need to check if there is any children devices
> on the dev->subordinate. Please check attached.
I think we should postpone this until we figure out how we're going to
manage resource start/end/flags, i.e., the IORESOURCE_UNSET and
IORESOURCE_DISABLED question.
> Thanks
>
> Yinghai
>
>
> ---
> drivers/pci/setup-bus.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -1622,8 +1622,11 @@ again:
> res->start = fail_res->start;
> res->end = fail_res->end;
> res->flags = fail_res->flags;
> - if (fail_res->dev->subordinate)
> + if (fail_res->dev->subordinate) {
> res->flags = 0;
> + if (!list_empty(&fail_res->dev->subordinate->devices))
> + res->start = res->end = 0;
> + }
> }
> free_list(&fail_head);
>
> @@ -1688,8 +1691,11 @@ again:
> res->start = fail_res->start;
> res->end = fail_res->end;
> res->flags = fail_res->flags;
> - if (fail_res->dev->subordinate)
> + if (fail_res->dev->subordinate) {
> res->flags = 0;
> + if (!list_empty(&fail_res->dev->subordinate->devices))
> + res->start = res->end = 0;
> + }
> }
> free_list(&fail_head);
> ---
> drivers/pci/setup-bus.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -1622,8 +1622,11 @@ again:
> res->start = fail_res->start;
> res->end = fail_res->end;
> res->flags = fail_res->flags;
> - if (fail_res->dev->subordinate)
> + if (fail_res->dev->subordinate) {
> res->flags = 0;
> + if (!list_empty(&fail_res->dev->subordinate->devices))
> + res->start = res->end = 0;
> + }
> }
> free_list(&fail_head);
>
> @@ -1688,8 +1691,11 @@ again:
> res->start = fail_res->start;
> res->end = fail_res->end;
> res->flags = fail_res->flags;
> - if (fail_res->dev->subordinate)
> + if (fail_res->dev->subordinate) {
> res->flags = 0;
> + if (!list_empty(&fail_res->dev->subordinate->devices))
> + res->start = res->end = 0;
> + }
> }
> free_list(&fail_head);
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-09-04 4:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-30 8:00 [PATCH] PCI: Reset failed bridge resources in PCI realloc Guo Chao
2014-07-30 19:59 ` Yinghai Lu
2014-07-31 8:48 ` Guo Chao
2014-09-04 4:23 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).