lkml.org 
[lkml]   [2014]   [Jun]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] PCI: Do not touch siblings in pci_assign_unassigned_bridge_resources
On Wed, Jun 18, 2014 at 09:41:02PM -0700, Yinghai Lu wrote:
> On Wed, Jun 18, 2014 at 3:40 PM, Andreas Noever
> <andreas.noever@gmail.com> wrote:
> > On Wed, Jun 18, 2014 at 1:39 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> > It seems to fix the testcase (no unwanted resources are released). But
> > why do you reassign bus and not just skip the top level bridge? If one
> > of the allocations below bridge failed then a resource of that device
> > will be in fail_res and bridge->subordinate will get released anyways?
> > Also by not removing fail_res from the list you trigger the code in
> > the next loop for the top level bridge (in particular the res->flags =
> > 0 line looks dangerous).
>
> Should not be dangerous, just second try.

I still don't understand this. Why do we set "res->flags = 0"? That
clears out the resource type. Where do we figure out the type of "res"
again?

> > Could you explain why this function attempts to assign resources two
> > times? In which scenario will a second attempt be successful?
>
> For example, at first mmio is assigned (by firmware), but pref mmio fails,
> then before second try, mmio get cleared, then we could separate
> mmio and mmio pref. So need to try again for pref mmio.
>
> Also I missed one MEM_64 for hotplug path.
>
> So we need two patches.
>
> Thanks
>
> Yinghai

> Subject: [PATCH] pci: Don't release sibiling bridge resources
>
> On hotplug case, we should not touch sibling bridges that is out
> side of the slots.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>
> ---
> drivers/pci/setup-bus.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 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
> @@ -1676,10 +1676,16 @@ again:
> * Try to release leaf bridge's resources that doesn't fit resource of
> * child device under that bridge
> */
> - list_for_each_entry(fail_res, &fail_head, list)
> - pci_bus_release_bridge_resources(fail_res->dev->bus,
> + list_for_each_entry(fail_res, &fail_head, list) {
> + struct pci_bus *bus = fail_res->dev->bus;
> +
> + if (fail_res->dev == bridge)
> + bus = bridge->subordinate;
> +
> + pci_bus_release_bridge_resources(bus,
> fail_res->flags & type_mask,
> whole_subtree);
> + }
>
> /* restore size and flags */
> list_for_each_entry(fail_res, &fail_head, list) {

> Subject: [PATCH] pci: Add back missing MEM_64 check for hotplug path
>
> We miss that in
> | commit 5b28541552ef5eeffc41d6936105f38c2508e566
> | PCI: Restrict 64-bit prefetchable bridge windows to 64-bit resources
> for pci hotplug path.

This changelog is useless. I don't have time to spend a few hours
figuring out why we want this change.

> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>
> ---
> drivers/pci/setup-bus.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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
> @@ -1652,7 +1652,7 @@ void pci_assign_unassigned_bridge_resour
> struct pci_dev_resource *fail_res;
> int retval;
> unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
> - IORESOURCE_PREFETCH;
> + IORESOURCE_PREFETCH | IORESOURCE_MEM_64;
>
> again:
> __pci_bus_size_bridges(parent, &add_list);



\
 
 \ /
  Last update: 2014-07-01 01:21    [W:0.070 / U:8.740 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site