Messages in this thread | | | Date | Mon, 25 Mar 2019 12:16:16 +0100 | From | Lukas Wunner <> | Subject | Re: [PATCH v2 17/28] thunderbolt: Add support for full PCIe daisy chains |
| |
On Mon, Mar 25, 2019 at 11:57:33AM +0200, Mika Westerberg wrote: > On Sun, Mar 24, 2019 at 12:31:44PM +0100, Lukas Wunner wrote: > > On Wed, Feb 06, 2019 at 04:17:27PM +0300, Mika Westerberg wrote: > > > -static void tb_activate_pcie_devices(struct tb *tb) > > > +static int tb_tunnel_pci(struct tb *tb, struct tb_switch *sw) > > > { > > [...] > > > + /* > > > + * Look up available down port. Since we are chaining, it is > > > + * typically found right above this switch. > > > + */ > > > + down = NULL; > > > + parent_sw = tb_to_switch(sw->dev.parent); > > > + while (parent_sw) { > > > + down = tb_find_unused_down_port(parent_sw); > > > + if (down) > > > + break; > > > + parent_sw = tb_to_switch(parent_sw->dev.parent); > > > + } > > > > The problem I see here is that there's no guarantee that the switch > > on which you're selecting a down port is actually itself connected > > with a PCI tunnel. E.g., allocation of a tunnel to that parent > > switch may have failed. In that case you end up establishing a > > tunnel between that parent switch and the newly connected switch > > but the tunnel is of no use. > > Since this is going through tb_domain_approve_switch() it does not allow > PCIe tunnel creation if the parent is not authorized first.
Yes, but my point is that it doesn't make much sense to establish a tunnel between a switch and one of its parent switches unless that parent switch is reachable from the root switch over a PCI tunnel or a series of PCI tunnels.
It may be worth checking that condition, or, if new tunnels are established top-down in the daisy-chain and tunnel establishment has failed for a switch, to not establish tunnels for switches beyond that one.
Thanks,
Lukas
| |