2019-03-28 12:37:36

by Mika Westerberg

Subject: [PATCH v3 00/36] thunderbolt: Software connection manager improvements

Hi,

This is third iteration of the patch series intending to bring same kind of
functionality for older Apple systems than we have in PCs. Software
connection manager is used on Apple hardware with Light Ridge, Cactus Ridge
or Falcon Ridge controllers to create PCIe tunnels when a Thunderbolt
device is connected. Currently only one PCIe tunnel is supported. On newer
Alpine Ridge based Apple systems the driver starts the firmware which then
takes care creating tunnels.

This series improves the software connection manager so that it will
support:

- Full PCIe daisy chains (up to 6 devices)
- Display Port tunneling
- P2P networking

We also add support for Titan Ridge based Apple systems where we can use
the same flows than with Alpine Ridge to start the firmware.

This applies on top of thunderbolt.git/next.

Christian, Mario do you see any issues with patch [05/36] regarding bolt
and fwupd? The kernel is supposed to restart the syscall automatically so
userspace should not be affected but wanted to check with you.

Previous version of the patch series can be viewed here:

v2: https://lkml.org/lkml/2019/2/6/347
v1: https://lkml.org/lkml/2019/1/29/924

Making v3 took longer than I anticipated mostly due to some issues I run
during testing the new changes. There are quite many changes so I dropped
the reviewed-by tags I got for v2. Below is the list of major changes from
the previous version:

* Always set port->remote even in case of dual link connection.

* Leave (DP, PCIe) tunnels up when the driver is unloaded. When loaded
back, it discovers the existing tunnels and updated data structures
accordingly. I noticed that the code in v2 did not support cases
properly when you unplug something before the driver gets loaded back.
This version tears down partial paths during discovery.

* Do not automatically create PCIe tunnels. Instead we implement "user"
security level in the software connection manager as well taking
advantage of the existing sysfs interfaces. This allows user to disable
PCIe tunneling completely or implement different white listing
policies. Major distros include bolt system daemon that takes care of
this.

* When testing on two-port Falcon Ridge based system I realized
that we always just pick the first available PCIe downstream adapter
regardless of which Thunderbolt port you plug the device which is not
consistent. To solve this we add mapping between host PCIe downstream
adapter and the Thunderbolt port for Cactus Ridge and Falcon Ridge
based systems.

* Take domain lock in switch sysfs callbacks. This is needed because the
software connection manager needs to walk over the topology during
tunnel creation so switch_lock is not enough anymore.

Changes from v1:

* Added ACK from David

* Add constant (TMU_ACCESS_EN) for BIT(20) when TMU access is enabled. We
keep it in cap.c close to the LR/ER workaround. Also we enable/disable
only during capability walk. If it turns we need to have it enabled
elsewhere we can move it to switch.c and enable just once during
switch enumeration.

* Use 0 to mean no cap_adap instead of negative value. This follows
cap_phy.

* Use correct PCI IDs (_BRIDGE) in the last patch where we start firmware
on Titan Ridge. It wrongly used NHI PCI IDs in v1.

Mika Westerberg (36):
net: thunderbolt: Unregister ThunderboltIP protocol handler when suspending
thunderbolt: Remove unused work field in struct tb_switch
thunderbolt: Drop duplicated get_switch_by_route()
thunderbolt: Block reads and writes if switch is unplugged
thunderbolt: Take domain lock in switch sysfs attribute callbacks
thunderbolt: Do not allocate switch if depth is greater than 6
thunderbolt: Enable TMU access when accessing port space on legacy devices
thunderbolt: Add dummy read after port capability list walk on Light Ridge
thunderbolt: Move LC specific functionality into a separate file
thunderbolt: Configure lanes when switch is initialized
thunderbolt: Set sleep bit when suspending switch
thunderbolt: Properly disable path
thunderbolt: Cache adapter specific capability offset into struct port
thunderbolt: Rename tunnel_pci to tunnel
thunderbolt: Generalize tunnel creation functionality
thunderbolt: Add functions for allocating and releasing HopIDs
thunderbolt: Assign remote for both ports in case of dual link
thunderbolt: Add helper function to iterate from one port to another
thunderbolt: Extend tunnel creation to more than 2 adjacent switches
thunderbolt: Deactivate all paths before restarting them
thunderbolt: Discover preboot PCIe paths the boot firmware established
thunderbolt: Add support for full PCIe daisy chains
thunderbolt: Scan only valid NULL adapter ports in hotplug
thunderbolt: Generalize port finding routines to support all port types
thunderbolt: Rework NFC credits handling
thunderbolt: Add support for Display Port tunnels
thunderbolt: Do not tear down tunnels when driver is unloaded
thunderbolt: Run tb_xdp_handle_request() in system workqueue
thunderbolt: Add XDomain UUID exchange support
thunderbolt: Add support for DMA tunnels
thunderbolt: Make tb_switch_alloc() return ERR_PTR()
thunderbolt: Add support for XDomain connections
thunderbolt: Make __TB_[SW|PORT]_PRINT take const parameters
thunderbolt: Make rest of the logging to happen at debug level
thunderbolt: Reword output of tb_dump_hop()
thunderbolt: Start firmware on Titan Ridge Apple systems

drivers/net/thunderbolt.c | 3 +
drivers/thunderbolt/Makefile | 4 +-
drivers/thunderbolt/cap.c | 85 +++-
drivers/thunderbolt/ctl.c | 2 +-
drivers/thunderbolt/icm.c | 60 ++-
drivers/thunderbolt/lc.c | 179 ++++++++
drivers/thunderbolt/nhi.c | 3 +-
drivers/thunderbolt/path.c | 421 ++++++++++++++++---
drivers/thunderbolt/switch.c | 551 +++++++++++++++++++-----
drivers/thunderbolt/tb.c | 608 ++++++++++++++++++++-------
drivers/thunderbolt/tb.h | 173 +++++++-
drivers/thunderbolt/tb_msgs.h | 11 +
drivers/thunderbolt/tb_regs.h | 50 ++-
drivers/thunderbolt/tunnel.c | 691 +++++++++++++++++++++++++++++++
drivers/thunderbolt/tunnel.h | 78 ++++
drivers/thunderbolt/tunnel_pci.c | 226 ----------
drivers/thunderbolt/tunnel_pci.h | 31 --
drivers/thunderbolt/xdomain.c | 147 ++++++-
include/linux/thunderbolt.h | 8 +
19 files changed, 2680 insertions(+), 651 deletions(-)
create mode 100644 drivers/thunderbolt/lc.c
create mode 100644 drivers/thunderbolt/tunnel.c
create mode 100644 drivers/thunderbolt/tunnel.h
delete mode 100644 drivers/thunderbolt/tunnel_pci.c
delete mode 100644 drivers/thunderbolt/tunnel_pci.h

--
2.20.1

2019-03-28 12:37:47

by Mika Westerberg

Subject: [PATCH v3 20/36] thunderbolt: Deactivate all paths before restarting them

State of the connected devices and tunnel configuration is not known
during resume. For example some paths may not be complete anymore if the
user has unplugged the related devices. So instead of marking all paths
as inactive we go ahead and deactivate them explicitly before we restart
them.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/tunnel.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index 91d7e00516b4..e109578da175 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -185,8 +185,18 @@ int tb_tunnel_restart(struct tb_tunnel *tunnel)

tb_tunnel_info(tunnel, "activating\n");

+ /*
+ * Make sure all paths are properly disabled before enabling
+ * them again.
+ */
+ for (i = 0; i < tunnel->npaths; i++) {
+ if (tunnel->paths[i]->activated) {
+ tb_path_deactivate(tunnel->paths[i]);
+ tunnel->paths[i]->activated = false;
+ }
+ }
+
for (i = 0; i < tunnel->npaths; i++) {
- tunnel->paths[i]->activated = false;
res = tb_path_activate(tunnel->paths[i]);
if (res)
goto err;
--
2.20.1

2019-03-28 12:37:49

by Mika Westerberg

Subject: [PATCH v3 35/36] thunderbolt: Reword output of tb_dump_hop()

While tb_dump_hop() prints out necessary information it is in format
that is quite hard to read from the logs especially when one needs to
follow the path to see that the setup is correct.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/path.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/thunderbolt/path.c b/drivers/thunderbolt/path.c
index 2dbfb3e1c3fc..44bdf08f3f68 100644
--- a/drivers/thunderbolt/path.c
+++ b/drivers/thunderbolt/path.c
@@ -13,21 +13,22 @@

#include "tb.h"

-static void tb_dump_hop(struct tb_port *port, struct tb_regs_hop *hop)
+static void tb_dump_hop(const struct tb_path_hop *hop, const struct tb_regs_hop *regs)
{
- tb_port_dbg(port, " Hop through port %d to hop %d (%s)\n",
- hop->out_port, hop->next_hop,
- hop->enable ? "enabled" : "disabled");
+ const struct tb_port *port = hop->in_port;
+
+ tb_port_dbg(port, " In HopID: %d => Out port: %d Out HopID: %d\n",
+ hop->in_hop_index, regs->out_port, regs->next_hop);
tb_port_dbg(port, " Weight: %d Priority: %d Credits: %d Drop: %d\n",
- hop->weight, hop->priority,
- hop->initial_credits, hop->drop_packages);
+ regs->weight, regs->priority,
+ regs->initial_credits, regs->drop_packages);
tb_port_dbg(port, " Counter enabled: %d Counter index: %d\n",
- hop->counter_enable, hop->counter);
+ regs->counter_enable, regs->counter);
tb_port_dbg(port, " Flow Control (In/Eg): %d/%d Shared Buffer (In/Eg): %d/%d\n",
- hop->ingress_fc, hop->egress_fc,
- hop->ingress_shared_buffer, hop->egress_shared_buffer);
+ regs->ingress_fc, regs->egress_fc,
+ regs->ingress_shared_buffer, regs->egress_shared_buffer);
tb_port_dbg(port, " Unknown1: %#x Unknown2: %#x Unknown3: %#x\n",
- hop->unknown1, hop->unknown2, hop->unknown3);
+ regs->unknown1, regs->unknown2, regs->unknown3);
}

static struct tb_port *tb_path_find_dst_port(struct tb_port *src, int src_hopid,
@@ -503,9 +504,8 @@ int tb_path_activate(struct tb_path *path)
& out_mask;
hop.unknown3 = 0;

- tb_port_dbg(path->hops[i].in_port, "Writing hop %d, index %d",
- i, path->hops[i].in_hop_index);
- tb_dump_hop(path->hops[i].in_port, &hop);
+ tb_port_dbg(path->hops[i].in_port, "Writing hop %d\n", i);
+ tb_dump_hop(&path->hops[i], &hop);
res = tb_port_write(path->hops[i].in_port, &hop, TB_CFG_HOPS,
2 * path->hops[i].in_hop_index, 2);
if (res) {
--
2.20.1

2019-03-28 12:37:57

by Mika Westerberg

Subject: [PATCH v3 33/36] thunderbolt: Make __TB_[SW|PORT]_PRINT take const parameters

The printing macros do not modify the passed object so make them
const. While there make tb_route() to take const parameter as well.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/tb.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index ca53ec145e27..535ddd77138c 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -287,7 +287,7 @@ static inline bool tb_is_upstream_port(const struct tb_port *port)
return port == upstream_port || port->dual_link_port == upstream_port;
}

-static inline u64 tb_route(struct tb_switch *sw)
+static inline u64 tb_route(const struct tb_switch *sw)
{
return ((u64) sw->config.route_hi) << 32 | sw->config.route_lo;
}
@@ -410,7 +410,7 @@ static inline int tb_port_write(struct tb_port *port, const void *buffer,

#define __TB_SW_PRINT(level, sw, fmt, arg...) \
do { \
- struct tb_switch *__sw = (sw); \
+ const struct tb_switch *__sw = (sw); \
level(__sw->tb, "%llx: " fmt, \
tb_route(__sw), ## arg); \
} while (0)
@@ -421,7 +421,7 @@ static inline int tb_port_write(struct tb_port *port, const void *buffer,

#define __TB_PORT_PRINT(level, _port, fmt, arg...) \
do { \
- struct tb_port *__port = (_port); \
+ const struct tb_port *__port = (_port); \
level(__port->sw->tb, "%llx:%x: " fmt, \
tb_route(__port->sw), __port->port, ## arg); \
} while (0)
--
2.20.1

2019-03-28 12:38:25

by Mika Westerberg

Subject: [PATCH v3 21/36] thunderbolt: Discover preboot PCIe paths the boot firmware established

In Apple Macs the boot firmware (EFI) connects all devices automatically
when the system is started, before it hands over to the OS. Instead of
ignoring we discover all those PCIe tunnels and record them using our
internal structures, just like we do when a device is connected after
the OS is already up.

By doing this we can properly tear down tunnels when devices are
disconnected. Also this allows us to resume the existing tunnels after
system suspend/resume cycle.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/path.c | 204 +++++++++++++++++++++++++++++++----
drivers/thunderbolt/switch.c | 14 +++
drivers/thunderbolt/tb.c | 39 +++++++
drivers/thunderbolt/tb.h | 16 +++
drivers/thunderbolt/tunnel.c | 86 ++++++++++++++-
drivers/thunderbolt/tunnel.h | 4 +-
6 files changed, 338 insertions(+), 25 deletions(-)

diff --git a/drivers/thunderbolt/path.c b/drivers/thunderbolt/path.c
index d3f5df779d8d..daef74548197 100644
--- a/drivers/thunderbolt/path.c
+++ b/drivers/thunderbolt/path.c
@@ -1,8 +1,9 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Thunderbolt Cactus Ridge driver - path/tunnel functionality
+ * Thunderbolt driver - path/tunnel functionality
*
* Copyright (c) 2014 Andreas Noever <[email protected]>
+ * Copyright (C) 2019, Intel Corporation
*/

#include <linux/slab.h>
@@ -12,7 +13,6 @@

#include "tb.h"

-
static void tb_dump_hop(struct tb_port *port, struct tb_regs_hop *hop)
{
tb_port_dbg(port, " Hop through port %d to hop %d (%s)\n",
@@ -30,6 +30,182 @@ static void tb_dump_hop(struct tb_port *port, struct tb_regs_hop *hop)
hop->unknown1, hop->unknown2, hop->unknown3);
}

+static struct tb_port *tb_path_find_dst_port(struct tb_port *src, int src_hopid,
+ int dst_hopid)
+{
+ struct tb_port *port, *out_port = NULL;
+ struct tb_regs_hop hop;
+ struct tb_switch *sw;
+ int i, ret, hopid;
+
+ hopid = src_hopid;
+ port = src;
+
+ for (i = 0; port && i < TB_PATH_MAX_HOPS; i++) {
+ sw = port->sw;
+
+ ret = tb_port_read(port, &hop, TB_CFG_HOPS, 2 * hopid, 2);
+ if (ret) {
+ tb_port_warn(port, "failed to read path at %d\n", hopid);
+ return NULL;
+ }
+
+ if (!hop.enable)
+ return NULL;
+
+ out_port = &sw->ports[hop.out_port];
+ hopid = hop.next_hop;
+ port = out_port->remote;
+ }
+
+ return out_port && hopid == dst_hopid ? out_port : NULL;
+}
+
+static int tb_path_find_src_hopid(struct tb_port *src,
+ const struct tb_port *dst, int dst_hopid)
+{
+ struct tb_port *out;
+ int i;
+
+ for (i = TB_PATH_MIN_HOPID; i <= src->config.max_in_hop_id; i++) {
+ out = tb_path_find_dst_port(src, i, dst_hopid);
+ if (out == dst)
+ return i;
+ }
+
+ return 0;
+}
+
+/**
+ * tb_path_discover() - Discover a path
+ * @src: First input port of a path
+ * @src_hopid: Starting HopID of a path (%-1 if don't care)
+ * @dst: Expected destination port of the path (%NULL if don't care)
+ * @dst_hopid: HopID to the @dst (%-1 if don't care)
+ * @last: Last port is filled here if not %NULL
+ * @name: Name of the path
+ *
+ * Follows a path starting from @src and @src_hopid to the last output
+ * port of the path. Allocates HopIDs for the visited ports. Call
+ * tb_path_free() to release the path and allocated HopIDs when the path
+ * is not needed anymore.
+ *
+ * Note function discovers also incomplete paths so caller should check
+ * that the @dst port is the expected one. If it is not, the path can be
+ * cleaned up by calling tb_path_deactivate() before tb_path_free().
+ *
+ * Return: Discovered path on success, %NULL in case of failure
+ */
+struct tb_path *tb_path_discover(struct tb_port *src, int src_hopid,
+ struct tb_port *dst, int dst_hopid,
+ struct tb_port **last, const char *name)
+{
+ struct tb_port *out_port;
+ struct tb_regs_hop hop;
+ struct tb_path *path;
+ struct tb_switch *sw;
+ struct tb_port *p;
+ size_t num_hops;
+ int ret, i, h;
+
+ if (src_hopid < 0 && dst) {
+ /*
+ * For incomplete paths the intermediate HopID can be
+ * different from the one used by the protocol adapter
+ * so in that case find a path that ends on @dst with
+ * matching @dst_hopid. That should give us the correct
+ * HopID for the @src.
+ */
+ src_hopid = tb_path_find_src_hopid(src, dst, dst_hopid);
+ if (!src_hopid)
+ return NULL;
+ }
+
+ p = src;
+ h = src_hopid;
+ num_hops = 0;
+
+ for (i = 0; p && i < TB_PATH_MAX_HOPS; i++) {
+ sw = p->sw;
+
+ ret = tb_port_read(p, &hop, TB_CFG_HOPS, 2 * h, 2);
+ if (ret) {
+ tb_port_warn(p, "failed to read path at %d\n", h);
+ return NULL;
+ }
+
+ /* If the hop is not enabled we got an incomplete path */
+ if (!hop.enable)
+ break;
+
+ out_port = &sw->ports[hop.out_port];
+ if (last)
+ *last = out_port;
+
+ h = hop.next_hop;
+ p = out_port->remote;
+ num_hops++;
+ }
+
+ path = kzalloc(sizeof(*path), GFP_KERNEL);
+ if (!path)
+ return NULL;
+
+ path->name = name;
+ path->tb = src->sw->tb;
+ path->path_length = num_hops;
+ path->activated = true;
+
+ path->hops = kcalloc(num_hops, sizeof(*path->hops), GFP_KERNEL);
+ if (!path->hops) {
+ kfree(path);
+ return NULL;
+ }
+
+ p = src;
+ h = src_hopid;
+
+ for (i = 0; i < num_hops; i++) {
+ int next_hop;
+
+ sw = p->sw;
+
+ ret = tb_port_read(p, &hop, TB_CFG_HOPS, 2 * h, 2);
+ if (ret) {
+ tb_port_warn(p, "failed to read path at %d\n", h);
+ goto err;
+ }
+
+ if (tb_port_alloc_in_hopid(p, h, h) < 0)
+ goto err;
+
+ out_port = &sw->ports[hop.out_port];
+ next_hop = hop.next_hop;
+
+ if (tb_port_alloc_out_hopid(out_port, next_hop, next_hop) < 0) {
+ tb_port_release_in_hopid(p, h);
+ goto err;
+ }
+
+ path->hops[i].in_port = p;
+ path->hops[i].in_hop_index = h;
+ path->hops[i].in_counter_index = -1;
+ path->hops[i].out_port = out_port;
+ path->hops[i].next_hop_index = next_hop;
+
+ h = next_hop;
+ p = out_port->remote;
+ }
+
+ return path;
+
+err:
+ tb_port_warn(src, "failed to discover path starting at HopID %d\n",
+ src_hopid);
+ tb_path_free(path);
+ return NULL;
+}
+
/**
* tb_path_alloc() - allocate a thunderbolt path between two ports
* @tb: Domain pointer
@@ -286,30 +462,14 @@ int tb_path_activate(struct tb_path *path)
for (i = path->path_length - 1; i >= 0; i--) {
struct tb_regs_hop hop = { 0 };

- /*
- * We do (currently) not tear down paths setup by the firmeware.
- * If a firmware device is unplugged and plugged in again then
- * it can happen that we reuse some of the hops from the (now
- * defunct) firmeware path. This causes the hotplug operation to
- * fail (the pci device does not show up). Clearing the hop
- * before overwriting it fixes the problem.
- *
- * Should be removed once we discover and tear down firmeware
- * paths.
- */
- res = tb_port_write(path->hops[i].in_port, &hop, TB_CFG_HOPS,
- 2 * path->hops[i].in_hop_index, 2);
- if (res) {
- __tb_path_deactivate_hops(path, i);
- __tb_path_deallocate_nfc(path, 0);
- goto err;
- }
+ /* If it is left active deactivate it first */
+ __tb_path_deactivate_hop(path->hops[i].in_port,
+ path->hops[i].in_hop_index);

/* dword 0 */
hop.next_hop = path->hops[i].next_hop_index;
hop.out_port = path->hops[i].out_port->port;
- /* TODO: figure out why these are good values */
- hop.initial_credits = (i == path->path_length - 1) ? 16 : 7;
+ hop.initial_credits = path->hops[i].initial_credits;
hop.unknown1 = 0;
hop.enable = 1;

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index ecd41f7b7649..00aec2124f79 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -730,6 +730,20 @@ struct tb_port *tb_next_port_on_path(struct tb_port *start, struct tb_port *end,
return next;
}

+/**
+ * tb_pci_port_is_enabled() - Is the PCIe adapter port enabled
+ * @port: PCIe port to check
+ */
+bool tb_pci_port_is_enabled(struct tb_port *port)
+{
+ u32 data;
+
+ if (tb_port_read(port, &data, TB_CFG_PORT, port->cap_adap, 1))
+ return false;
+
+ return !!(data & TB_PCI_EN);
+}
+
/**
* tb_pci_port_enable() - Enable PCIe adapter port
* @port: PCIe port to enable
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 0485f4ef9a62..a62695a99835 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -29,6 +29,43 @@ struct tb_cm {

/* enumeration & hot plug handling */

+static void tb_discover_tunnels(struct tb_switch *sw)
+{
+ struct tb *tb = sw->tb;
+ struct tb_cm *tcm = tb_priv(tb);
+ struct tb_port *port;
+ int i;
+
+ for (i = 1; i <= sw->config.max_port_number; i++) {
+ struct tb_tunnel *tunnel = NULL;
+
+ port = &sw->ports[i];
+ switch (port->config.type) {
+ case TB_TYPE_PCIE_DOWN:
+ tunnel = tb_tunnel_discover_pci(tb, port);
+ break;
+
+ default:
+ break;
+ }
+
+ if (tunnel) {
+ struct tb_switch *parent = tunnel->dst_port->sw;
+
+ while (parent != tunnel->src_port->sw) {
+ parent->boot = true;
+ parent = tb_switch_parent(parent);
+ }
+
+ list_add_tail(&tunnel->list, &tcm->tunnel_list);
+ }
+ }
+
+ for (i = 1; i <= sw->config.max_port_number; i++) {
+ if (tb_port_has_remote(&sw->ports[i]))
+ tb_discover_tunnels(sw->ports[i].remote->sw);
+ }
+}

static void tb_scan_port(struct tb_port *port);

@@ -408,6 +445,8 @@ static int tb_start(struct tb *tb)

/* Full scan to discover devices added before the driver was loaded. */
tb_scan_switch(tb->root_switch);
+ /* Find out tunnels created by the boot firmware */
+ tb_discover_tunnels(tb->root_switch);
tb_activate_pcie_devices(tb);

/* Allow tb_handle_hotplug to progress events */
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 1c7d67a829d3..6ce3832b6c99 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -162,6 +162,7 @@ struct tb_path_hop {
int in_hop_index;
int in_counter_index; /* write -1 to disable counters for this hop. */
int next_hop_index;
+ unsigned int initial_credits;
};

/**
@@ -201,6 +202,7 @@ struct tb_path {

/* HopIDs 0-7 are reserved by the Thunderbolt protocol */
#define TB_PATH_MIN_HOPID 8
+#define TB_PATH_MAX_HOPS 7

/**
* struct tb_cm_ops - Connection manager specific operations vector
@@ -318,6 +320,11 @@ static inline bool tb_port_has_remote(const struct tb_port *port)
return true;
}

+static inline bool tb_port_is_pcie_up(const struct tb_port *port)
+{
+ return port && port->config.type == TB_TYPE_PCIE_UP;
+}
+
static inline int tb_sw_read(struct tb_switch *sw, void *buffer,
enum tb_cfg_space space, u32 offset, u32 length)
{
@@ -480,6 +487,11 @@ static inline struct tb_switch *tb_to_switch(struct device *dev)
return NULL;
}

+static inline struct tb_switch *tb_switch_parent(struct tb_switch *sw)
+{
+ return tb_to_switch(sw->dev.parent);
+}
+
static inline bool tb_switch_is_lr(const struct tb_switch *sw)
{
return sw->config.device_id == PCI_DEVICE_ID_INTEL_LIGHT_RIDGE;
@@ -503,8 +515,12 @@ struct tb_port *tb_next_port_on_path(struct tb_port *start, struct tb_port *end,
int tb_switch_find_vse_cap(struct tb_switch *sw, enum tb_switch_vse_cap vsec);
int tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap);

+bool tb_pci_port_is_enabled(struct tb_port *port);
int tb_pci_port_enable(struct tb_port *port, bool enable);

+struct tb_path *tb_path_discover(struct tb_port *src, int src_hopid,
+ struct tb_port *dst, int dst_hopid,
+ struct tb_port **last, const char *name);
struct tb_path *tb_path_alloc(struct tb *tb, struct tb_port *src, int src_hopid,
struct tb_port *dst, int dst_hopid, int link_nr,
const char *name);
diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index e109578da175..71c712300326 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -35,6 +35,8 @@
__TB_TUNNEL_PRINT(tb_warn, tunnel, fmt, ##arg)
#define tb_tunnel_info(tunnel, fmt, arg...) \
__TB_TUNNEL_PRINT(tb_info, tunnel, fmt, ##arg)
+#define tb_tunnel_dbg(tunnel, fmt, arg...) \
+ __TB_TUNNEL_PRINT(tb_dbg, tunnel, fmt, ##arg)

static struct tb_tunnel *tb_tunnel_alloc(struct tb *tb, size_t npaths)
{
@@ -65,7 +67,10 @@ static int tb_pci_activate(struct tb_tunnel *tunnel, bool activate)
if (res)
return res;

- return tb_pci_port_enable(tunnel->dst_port, activate);
+ if (tb_port_is_pcie_up(tunnel->dst_port))
+ return tb_pci_port_enable(tunnel->dst_port, activate);
+
+ return 0;
}

static void tb_pci_init_path(struct tb_path *path)
@@ -78,6 +83,83 @@ static void tb_pci_init_path(struct tb_path *path)
path->weight = 1;
path->drop_packages = 0;
path->nfc_credits = 0;
+ path->hops[0].initial_credits = 7;
+ path->hops[1].initial_credits = 16;
+}
+
+/**
+ * tb_tunnel_discover_pci() - Discover existing PCIe tunnels
+ * @tb: Pointer to the domain structure
+ * @down: PCIe downstream adapter
+ *
+ * If @down adapter is active, follows the tunnel to the PCIe upstream
+ * adapter and back. Returns the discovered tunnel or %NULL if there was
+ * no tunnel.
+ */
+struct tb_tunnel *tb_tunnel_discover_pci(struct tb *tb, struct tb_port *down)
+{
+ struct tb_tunnel *tunnel;
+ struct tb_path *path;
+
+ if (!tb_pci_port_is_enabled(down))
+ return NULL;
+
+ tunnel = tb_tunnel_alloc(tb, 2);
+ if (!tunnel)
+ return NULL;
+
+ tunnel->activate = tb_pci_activate;
+ tunnel->src_port = down;
+
+ /*
+ * Discover both paths even if they are not complete. We will
+ * clean them up by calling tb_tunnel_deactivate() below in that
+ * case.
+ */
+ path = tb_path_discover(down, TB_PCI_HOPID, NULL, -1,
+ &tunnel->dst_port, "PCIe Up");
+ if (!path) {
+ /* Just disable the downstream port */
+ tb_pci_port_enable(down, false);
+ goto err_free;
+ }
+ tunnel->paths[TB_PCI_PATH_UP] = path;
+ tb_pci_init_path(tunnel->paths[TB_PCI_PATH_UP]);
+
+ path = tb_path_discover(tunnel->dst_port, -1, down, TB_PCI_HOPID, NULL,
+ "PCIe Down");
+ if (!path)
+ goto err_deactivate;
+ tunnel->paths[TB_PCI_PATH_DOWN] = path;
+ tb_pci_init_path(tunnel->paths[TB_PCI_PATH_DOWN]);
+
+ /* Validate that the tunnel is complete */
+ if (!tb_port_is_pcie_up(tunnel->dst_port)) {
+ tb_port_warn(tunnel->dst_port,
+ "path does not end on a PCIe adapter, cleaning up\n");
+ goto err_deactivate;
+ }
+
+ if (down != tunnel->src_port) {
+ tb_tunnel_warn(tunnel, "path is not complete, cleaning up\n");
+ goto err_deactivate;
+ }
+
+ if (!tb_pci_port_is_enabled(tunnel->dst_port)) {
+ tb_tunnel_warn(tunnel,
+ "tunnel is not fully activated, cleaning up\n");
+ goto err_deactivate;
+ }
+
+ tb_tunnel_dbg(tunnel, "discovered\n");
+ return tunnel;
+
+err_deactivate:
+ tb_tunnel_deactivate(tunnel);
+err_free:
+ tb_tunnel_free(tunnel);
+
+ return NULL;
}

/**
@@ -253,7 +335,7 @@ void tb_tunnel_deactivate(struct tb_tunnel *tunnel)
tunnel->activate(tunnel, false);

for (i = 0; i < tunnel->npaths; i++) {
- if (tunnel->paths[i]->activated)
+ if (tunnel->paths[i] && tunnel->paths[i]->activated)
tb_path_deactivate(tunnel->paths[i]);
}
}
diff --git a/drivers/thunderbolt/tunnel.h b/drivers/thunderbolt/tunnel.h
index b4e992165e56..07bf587bed80 100644
--- a/drivers/thunderbolt/tunnel.h
+++ b/drivers/thunderbolt/tunnel.h
@@ -15,7 +15,8 @@
* struct tb_tunnel - Tunnel between two ports
* @tb: Pointer to the domain
* @src_port: Source port of the tunnel
- * @dst_port: Destination port of the tunnel
+ * @dst_port: Destination port of the tunnel. For discovered incomplete
+ * tunnels may be %NULL or null adapter port instead.
* @paths: All paths required by the tunnel
* @npaths: Number of paths in @paths
* @activate: Optional tunnel specific activation/deactivation
@@ -31,6 +32,7 @@ struct tb_tunnel {
struct list_head list;
};

+struct tb_tunnel *tb_tunnel_discover_pci(struct tb *tb, struct tb_port *down);
struct tb_tunnel *tb_tunnel_alloc_pci(struct tb *tb, struct tb_port *up,
struct tb_port *down);
void tb_tunnel_free(struct tb_tunnel *tunnel);
--
2.20.1

2019-03-28 12:38:34

by Mika Westerberg

Subject: [PATCH v3 28/36] thunderbolt: Run tb_xdp_handle_request() in system workqueue

We run all XDomain requests during discovery in tb->wq and since it only
runs one work at the time it means that sending back reply to the other
domain may be delayed too much depending whether there is an active
XDomain discovery request running.

To make sure we can send reply to the other domain as soon as possible
run tb_xdp_handle_request() in system workqueue instead. Since the
device can be hot-removed in the middle we need to make sure the domain
structure is still around when the function is run so increase reference
count before we schedule the reply work.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/tb.h | 7 +++++++
drivers/thunderbolt/xdomain.c | 6 ++++--
2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index dd7808f9de39..19bdeeaf1d18 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -462,6 +462,13 @@ int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd);
int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd);
int tb_domain_disconnect_all_paths(struct tb *tb);

+static inline struct tb *tb_domain_get(struct tb *tb)
+{
+ if (tb)
+ get_device(&tb->dev);
+ return tb;
+}
+
static inline void tb_domain_put(struct tb *tb)
{
put_device(&tb->dev);
diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c
index 7bae92bc486f..44d3b2e486cd 100644
--- a/drivers/thunderbolt/xdomain.c
+++ b/drivers/thunderbolt/xdomain.c
@@ -524,6 +524,8 @@ static void tb_xdp_handle_request(struct work_struct *work)
out:
kfree(xw->pkg);
kfree(xw);
+
+ tb_domain_put(tb);
}

static bool
@@ -542,9 +544,9 @@ tb_xdp_schedule_request(struct tb *tb, const struct tb_xdp_header *hdr,
kfree(xw);
return false;
}
- xw->tb = tb;
+ xw->tb = tb_domain_get(tb);

- queue_work(tb->wq, &xw->work);
+ schedule_work(&xw->work);
return true;
}

--
2.20.1

2019-03-28 12:38:42

by Mika Westerberg

Subject: [PATCH v3 32/36] thunderbolt: Add support for XDomain connections

Two domains (hosts) can be connected through a Thunderbolt cable and in
that case they can start software services such as networking over the
high-speed DMA paths. Now that we have all the basic building blocks in
place to create DMA tunnels over the Thunderbolt fabric we can add this
support to the software connection manager as well.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/switch.c | 29 +++++-
drivers/thunderbolt/tb.c | 167 ++++++++++++++++++++++++++++++++++-
2 files changed, 188 insertions(+), 8 deletions(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 460f2bcad40a..a5345a6225bd 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -1845,6 +1845,8 @@ void tb_sw_set_unplugged(struct tb_switch *sw)
for (i = 0; i <= sw->config.max_port_number; i++) {
if (tb_port_has_remote(&sw->ports[i]))
tb_sw_set_unplugged(sw->ports[i].remote->sw);
+ else if (sw->ports[i].xdomain)
+ sw->ports[i].xdomain->is_unplugged = true;
}
}

@@ -1860,6 +1862,17 @@ int tb_switch_resume(struct tb_switch *sw)
if (tb_route(sw)) {
u64 uid;

+ /*
+ * Check first that we can still read the switch config
+ * space. It may be that there is now another domain
+ * connected.
+ */
+ err = tb_cfg_get_upstream_port(sw->tb->ctl, tb_route(sw));
+ if (err < 0) {
+ tb_sw_info(sw, "switch not present anymore\n");
+ return err;
+ }
+
err = tb_drom_read_uid_only(sw, &uid);
if (err) {
tb_sw_warn(sw, "uid read failed\n");
@@ -1890,14 +1903,22 @@ int tb_switch_resume(struct tb_switch *sw)
for (i = 1; i <= sw->config.max_port_number; i++) {
struct tb_port *port = &sw->ports[i];

- if (!tb_port_has_remote(port))
+ if (!tb_port_has_remote(port) && !port->xdomain)
continue;

- if (tb_wait_for_port(port, true) <= 0
- || tb_switch_resume(port->remote->sw)) {
+ if (tb_wait_for_port(port, true) <= 0) {
tb_port_warn(port,
"lost during suspend, disconnecting\n");
- tb_sw_set_unplugged(port->remote->sw);
+ if (tb_port_has_remote(port))
+ tb_sw_set_unplugged(port->remote->sw);
+ else if (port->xdomain)
+ port->xdomain->is_unplugged = true;
+ } else if (tb_port_has_remote(port)) {
+ if (tb_switch_resume(port->remote->sw)) {
+ tb_port_warn(port,
+ "lost during suspend, disconnecting\n");
+ tb_sw_set_unplugged(port->remote->sw);
+ }
}
}
return 0;
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index c5e82c4dcb64..e39fc1e35e6b 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -101,6 +101,28 @@ static void tb_discover_tunnels(struct tb_switch *sw)
}
}

+static void tb_scan_xdomain(struct tb_port *port)
+{
+ struct tb_switch *sw = port->sw;
+ struct tb *tb = sw->tb;
+ struct tb_xdomain *xd;
+ u64 route;
+
+ route = tb_downstream_route(port);
+ xd = tb_xdomain_find_by_route(tb, route);
+ if (xd) {
+ tb_xdomain_put(xd);
+ return;
+ }
+
+ xd = tb_xdomain_alloc(tb, &sw->dev, route, tb->root_switch->uuid,
+ NULL);
+ if (xd) {
+ tb_port_at(route, sw)->xdomain = xd;
+ tb_xdomain_add(xd);
+ }
+}
+
static void tb_scan_port(struct tb_port *port);

/**
@@ -143,19 +165,36 @@ static void tb_scan_port(struct tb_port *port)
if (tb_wait_for_port(port, false) <= 0)
return;
if (port->remote) {
- tb_port_WARN(port, "port already has a remote!\n");
+ tb_port_dbg(port, "port already has a remote\n");
return;
}
sw = tb_switch_alloc(port->sw->tb, &port->sw->dev,
tb_downstream_route(port));
- if (IS_ERR(sw))
+ if (IS_ERR(sw)) {
+ /*
+ * If there is an error accessing the connected switch
+ * it may be connected to another domain. Also we allow
+ * the other domain to be connected to a max depth switch.
+ */
+ if (PTR_ERR(sw) == -EIO || PTR_ERR(sw) == -EADDRNOTAVAIL)
+ tb_scan_xdomain(port);
return;
+ }

if (tb_switch_configure(sw)) {
tb_switch_put(sw);
return;
}

+ /*
+ * If there was previously another domain connected remove it
+ * first.
+ */
+ if (port->xdomain) {
+ tb_xdomain_remove(port->xdomain);
+ port->xdomain = NULL;
+ }
+
/*
* Do not send uevents until we have discovered all existing
* tunnels and know which switches were authorized already by
@@ -393,6 +432,65 @@ static int tb_tunnel_pci(struct tb *tb, struct tb_switch *sw)
return 0;
}

+static int tb_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd)
+{
+ struct tb_cm *tcm = tb_priv(tb);
+ struct tb_port *nhi_port, *dst_port;
+ struct tb_tunnel *tunnel;
+ struct tb_switch *sw;
+
+ sw = tb_to_switch(xd->dev.parent);
+ dst_port = tb_port_at(xd->route, sw);
+ nhi_port = tb_find_port(tb->root_switch, TB_TYPE_NHI);
+
+ mutex_lock(&tb->lock);
+ tunnel = tb_tunnel_alloc_dma(tb, nhi_port, dst_port, xd->transmit_ring,
+ xd->transmit_path, xd->receive_ring,
+ xd->receive_path);
+ if (!tunnel) {
+ mutex_unlock(&tb->lock);
+ return -ENOMEM;
+ }
+
+ if (tb_tunnel_activate(tunnel)) {
+ tb_port_info(nhi_port,
+ "DMA tunnel activation failed, aborting\n");
+ tb_tunnel_free(tunnel);
+ mutex_unlock(&tb->lock);
+ return -EIO;
+ }
+
+ list_add_tail(&tunnel->list, &tcm->tunnel_list);
+ mutex_unlock(&tb->lock);
+ return 0;
+}
+
+static void __tb_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd)
+{
+ struct tb_port *dst_port;
+ struct tb_switch *sw;
+
+ sw = tb_to_switch(xd->dev.parent);
+ dst_port = tb_port_at(xd->route, sw);
+
+ /*
+ * It is possible that the tunnel was already teared down (in
+ * case of cable disconnect) so it is fine if we cannot find it
+ * here anymore.
+ */
+ tb_free_tunnel(tb, TB_TUNNEL_DMA, NULL, dst_port);
+}
+
+static int tb_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd)
+{
+ if (!xd->is_unplugged) {
+ mutex_lock(&tb->lock);
+ __tb_disconnect_xdomain_paths(tb, xd);
+ mutex_unlock(&tb->lock);
+ }
+ return 0;
+}
+
/* hotplug handling */

/**
@@ -432,13 +530,29 @@ static void tb_handle_hotplug(struct work_struct *work)
}
if (ev->unplug) {
if (tb_port_has_remote(port)) {
- tb_port_info(port, "unplugged\n");
+ tb_port_dbg(port, "switch unplugged\n");
tb_sw_set_unplugged(port->remote->sw);
tb_free_invalid_tunnels(tb);
tb_switch_remove(port->remote->sw);
port->remote = NULL;
if (port->dual_link_port)
port->dual_link_port->remote = NULL;
+ } else if (port->xdomain) {
+ struct tb_xdomain *xd = tb_xdomain_get(port->xdomain);
+
+ tb_port_dbg(port, "xdomain unplugged\n");
+ /*
+ * Service drivers are unbound during
+ * tb_xdomain_remove() so setting XDomain as
+ * unplugged here prevents deadlock if they call
+ * tb_xdomain_disable_paths(). We will tear down
+ * the path below.
+ */
+ xd->is_unplugged = true;
+ tb_xdomain_remove(xd);
+ port->xdomain = NULL;
+ __tb_disconnect_xdomain_paths(tb, xd);
+ tb_xdomain_put(xd);
} else if (tb_port_is_dpout(port)) {
tb_teardown_dp(tb, port);
} else {
@@ -500,8 +614,16 @@ static void tb_stop(struct tb *tb)
struct tb_tunnel *n;

/* tunnels are only present after everything has been initialized */
- list_for_each_entry_safe(tunnel, n, &tcm->tunnel_list, list)
+ list_for_each_entry_safe(tunnel, n, &tcm->tunnel_list, list) {
+ /*
+ * DMA tunnels require the driver to be functional so we
+ * tear them down. Other protocol tunnels can be left
+ * intact.
+ */
+ if (tb_tunnel_is_dma(tunnel))
+ tb_tunnel_deactivate(tunnel);
tb_tunnel_free(tunnel);
+ }
tb_switch_remove(tb->root_switch);
tcm->hotplug_active = false; /* signal tb_handle_hotplug to quit */
}
@@ -611,13 +733,50 @@ static int tb_resume_noirq(struct tb *tb)
return 0;
}

+static int tb_free_unplugged_xdomains(struct tb_switch *sw)
+{
+ int i, ret = 0;
+
+ for (i = 1; i <= sw->config.max_port_number; i++) {
+ struct tb_port *port = &sw->ports[i];
+
+ if (tb_is_upstream_port(port))
+ continue;
+ if (port->xdomain && port->xdomain->is_unplugged) {
+ tb_xdomain_remove(port->xdomain);
+ port->xdomain = NULL;
+ ret++;
+ } else if (port->remote) {
+ ret += tb_free_unplugged_xdomains(port->remote->sw);
+ }
+ }
+
+ return ret;
+}
+
+static void tb_complete(struct tb *tb)
+{
+ /*
+ * Release any unplugged XDomains and if there is a case where
+ * another domain is swapped in place of unplugged XDomain we
+ * need to run another rescan.
+ */
+ mutex_lock(&tb->lock);
+ if (tb_free_unplugged_xdomains(tb->root_switch))
+ tb_scan_switch(tb->root_switch);
+ mutex_unlock(&tb->lock);
+}
+
static const struct tb_cm_ops tb_cm_ops = {
.start = tb_start,
.stop = tb_stop,
.suspend_noirq = tb_suspend_noirq,
.resume_noirq = tb_resume_noirq,
+ .complete = tb_complete,
.handle_event = tb_handle_event,
.approve_switch = tb_tunnel_pci,
+ .approve_xdomain_paths = tb_approve_xdomain_paths,
+ .disconnect_xdomain_paths = tb_disconnect_xdomain_paths,
};

struct tb *tb_probe(struct tb_nhi *nhi)
--
2.20.1

2019-03-28 12:38:51

by Mika Westerberg

Subject: [PATCH v3 18/36] thunderbolt: Add helper function to iterate from one port to another

We need to be able to walk from one port to another when we are creating
paths where there are multiple switches between two ports. For this
reason introduce a new function tb_next_port_on_path().

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/switch.c | 54 ++++++++++++++++++++++++++++++++++++
drivers/thunderbolt/tb.h | 2 ++
2 files changed, 56 insertions(+)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 6f98b3d6eb2a..ecd41f7b7649 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -676,6 +676,60 @@ void tb_port_release_out_hopid(struct tb_port *port, int hopid)
ida_simple_remove(&port->out_hopids, hopid);
}

+/**
+ * tb_next_port_on_path() - Return next port for given port on a path
+ * @start: Start port of the walk
+ * @end: End port of the walk
+ * @prev: Previous port (%NULL if this is the first)
+ *
+ * This function can be used to walk from one port to another if they
+ * are connected through zero or more switches. If the @prev is dual
+ * link port, the function follows that link and returns another end on
+ * that same link.
+ *
+ * If the @end port has been reached, return %NULL.
+ *
+ * Domain tb->lock must be held when this function is called.
+ */
+struct tb_port *tb_next_port_on_path(struct tb_port *start, struct tb_port *end,
+ struct tb_port *prev)
+{
+ struct tb_port *next;
+
+ if (!prev)
+ return start;
+
+ if (prev->sw == end->sw) {
+ if (prev == end)
+ return NULL;
+ return end;
+ }
+
+ if (start->sw->config.depth < end->sw->config.depth) {
+ if (prev->remote &&
+ prev->remote->sw->config.depth > prev->sw->config.depth)
+ next = prev->remote;
+ else
+ next = tb_port_at(tb_route(end->sw), prev->sw);
+ } else {
+ if (tb_is_upstream_port(prev)) {
+ next = prev->remote;
+ } else {
+ next = tb_upstream_port(prev->sw);
+ /*
+ * Keep the same link if prev and next are both
+ * dual link ports.
+ */
+ if (next->dual_link_port &&
+ next->link_nr != prev->link_nr) {
+ next = next->dual_link_port;
+ }
+ }
+ }
+
+ return next;
+}
+
/**
* tb_pci_port_enable() - Enable PCIe adapter port
* @port: PCIe port to enable
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 5e317ca87786..c797512dfb05 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -496,6 +496,8 @@ int tb_port_alloc_in_hopid(struct tb_port *port, int hopid, int max_hopid);
void tb_port_release_in_hopid(struct tb_port *port, int hopid);
int tb_port_alloc_out_hopid(struct tb_port *port, int hopid, int max_hopid);
void tb_port_release_out_hopid(struct tb_port *port, int hopid);
+struct tb_port *tb_next_port_on_path(struct tb_port *start, struct tb_port *end,
+ struct tb_port *prev);

int tb_switch_find_vse_cap(struct tb_switch *sw, enum tb_switch_vse_cap vsec);
int tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap);
--
2.20.1

2019-03-28 12:39:05

by Mika Westerberg

Subject: [PATCH v3 30/36] thunderbolt: Add support for DMA tunnels

In addition to PCIe and Display Port tunnels it is also possible to
create tunnels that forward DMA traffic from the host interface adapter
(NHI) to a NULL port that is connected to another domain through a
Thunderbolt cable. These tunnels can be used to carry software messages
such as networking packets.

To support this we introduce another tunnel type (TB_TUNNEL_DMA) that
supports paths from NHI to NULL port and back.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/path.c | 22 +++++++--
drivers/thunderbolt/switch.c | 22 +++++++++
drivers/thunderbolt/tb.h | 2 +
drivers/thunderbolt/tb_regs.h | 3 ++
drivers/thunderbolt/tunnel.c | 93 ++++++++++++++++++++++++++++++++++-
drivers/thunderbolt/tunnel.h | 10 ++++
6 files changed, 147 insertions(+), 5 deletions(-)

diff --git a/drivers/thunderbolt/path.c b/drivers/thunderbolt/path.c
index 9df79548a941..d06fe8bf8872 100644
--- a/drivers/thunderbolt/path.c
+++ b/drivers/thunderbolt/path.c
@@ -341,7 +341,8 @@ static void __tb_path_deallocate_nfc(struct tb_path *path, int first_hop)
}
}

-static int __tb_path_deactivate_hop(struct tb_port *port, int hop_index)
+static int __tb_path_deactivate_hop(struct tb_port *port, int hop_index,
+ bool clear_fc)
{
struct tb_regs_hop hop;
ktime_t timeout;
@@ -372,8 +373,20 @@ static int __tb_path_deactivate_hop(struct tb_port *port, int hop_index)
if (ret)
return ret;

- if (!hop.pending)
+ if (!hop.pending) {
+ if (clear_fc) {
+ /* Clear flow control */
+ hop.ingress_fc = 0;
+ hop.egress_fc = 0;
+ hop.ingress_shared_buffer = 0;
+ hop.egress_shared_buffer = 0;
+
+ return tb_port_write(port, &hop, TB_CFG_HOPS,
+ 2 * hop_index, 2);
+ }
+
return 0;
+ }

usleep_range(10, 20);
} while (ktime_before(ktime_get(), timeout));
@@ -387,7 +400,8 @@ static void __tb_path_deactivate_hops(struct tb_path *path, int first_hop)

for (i = first_hop; i < path->path_length; i++) {
res = __tb_path_deactivate_hop(path->hops[i].in_port,
- path->hops[i].in_hop_index);
+ path->hops[i].in_hop_index,
+ path->clear_fc);
if (res)
tb_port_warn(path->hops[i].in_port,
"hop deactivation failed for hop %d, index %d\n",
@@ -462,7 +476,7 @@ int tb_path_activate(struct tb_path *path)

/* If it is left active deactivate it first */
__tb_path_deactivate_hop(path->hops[i].in_port,
- path->hops[i].in_hop_index);
+ path->hops[i].in_hop_index, path->clear_fc);

/* dword 0 */
hop.next_hop = path->hops[i].next_hop_index;
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 1de1afa24182..ecf53d986a5c 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -555,6 +555,28 @@ int tb_port_add_nfc_credits(struct tb_port *port, int credits)
TB_CFG_PORT, 4, 1);
}

+/**
+ * tb_port_set_initial_credits() - Set initial port link credits allocated
+ * @port: Port to set the initial credits
+ * @credits: Number of credits to to allocate
+ *
+ * Set initial credits value to be used for ingress shared buffering.
+ */
+int tb_port_set_initial_credits(struct tb_port *port, u32 credits)
+{
+ u32 data;
+ int ret;
+
+ ret = tb_port_read(port, &data, TB_CFG_PORT, 5, 1);
+ if (ret)
+ return ret;
+
+ data &= ~TB_PORT_LCA_MASK;
+ data |= (credits << TB_PORT_LCA_SHIFT) & TB_PORT_LCA_MASK;
+
+ return tb_port_write(port, &data, TB_CFG_PORT, 5, 1);
+}
+
/**
* tb_port_clear_counter() - clear a counter in TB_CFG_COUNTER
*
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 19bdeeaf1d18..ca53ec145e27 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -196,6 +196,7 @@ struct tb_path {
int weight:4;
bool drop_packages;
bool activated;
+ bool clear_fc;
struct tb_path_hop *hops;
int path_length; /* number of hops */
};
@@ -553,6 +554,7 @@ static inline bool tb_switch_is_fr(const struct tb_switch *sw)

int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged);
int tb_port_add_nfc_credits(struct tb_port *port, int credits);
+int tb_port_set_initial_credits(struct tb_port *port, u32 credits);
int tb_port_clear_counter(struct tb_port *port, int counter);
int tb_port_alloc_in_hopid(struct tb_port *port, int hopid, int max_hopid);
void tb_port_release_in_hopid(struct tb_port *port, int hopid);
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index 3ce705184e2c..deb9d4a977b9 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -215,6 +215,9 @@ struct tb_regs_port_header {
#define TB_PORT_NFC_CREDITS_MASK GENMASK(19, 0)
#define TB_PORT_MAX_CREDITS_SHIFT 20
#define TB_PORT_MAX_CREDITS_MASK GENMASK(26, 20)
+/* DWORD 5 */
+#define TB_PORT_LCA_SHIFT 22
+#define TB_PORT_LCA_MASK GENMASK(28, 22)

/* Display Port adapter registers */

diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index 0bc6639c6e74..9f9b26b12d0a 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -27,7 +27,10 @@
#define TB_DP_AUX_PATH_OUT 1
#define TB_DP_AUX_PATH_IN 2

-static const char * const tb_tunnel_names[] = { "PCI", "DP" };
+#define TB_DMA_PATH_OUT 0
+#define TB_DMA_PATH_IN 1
+
+static const char * const tb_tunnel_names[] = { "PCI", "DP", "DMA" };

#define __TB_TUNNEL_PRINT(level, tunnel, fmt, arg...) \
do { \
@@ -471,6 +474,94 @@ struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,
return NULL;
}

+static u32 tb_dma_credits(struct tb_port *nhi)
+{
+ u32 max_credits;
+
+ max_credits = (nhi->config.nfc_credits & TB_PORT_MAX_CREDITS_MASK) >>
+ TB_PORT_MAX_CREDITS_SHIFT;
+ return min(max_credits, 13U);
+}
+
+static int tb_dma_activate(struct tb_tunnel *tunnel, bool active)
+{
+ struct tb_port *nhi = tunnel->src_port;
+ u32 credits;
+
+ credits = active ? tb_dma_credits(nhi) : 0;
+ return tb_port_set_initial_credits(nhi, credits);
+}
+
+static void tb_dma_init_path(struct tb_path *path, unsigned int isb,
+ unsigned int efc, u32 credits)
+{
+ int i;
+
+ path->egress_fc_enable = efc;
+ path->ingress_fc_enable = TB_PATH_ALL;
+ path->egress_shared_buffer = TB_PATH_NONE;
+ path->ingress_shared_buffer = isb;
+ path->priority = 5;
+ path->weight = 1;
+ path->clear_fc = true;
+
+ for (i = 0; i < path->path_length; i++)
+ path->hops[i].initial_credits = credits;
+}
+
+/**
+ * tb_tunnel_alloc_dma() - allocate a DMA tunnel
+ * @tb: Pointer to the domain structure
+ * @nhi: Host controller port
+ * @dst: Destination null port which the other domain is connected to
+ * @transmit_ring: NHI ring number used to send packets towards the
+ * other domain
+ * @transmit_path: HopID used for transmitting packets
+ * @receive_ring: NHI ring number used to receive packets from the
+ * other domain
+ * @reveive_path: HopID used for receiving packets
+ *
+ * Return: Returns a tb_tunnel on success or NULL on failure.
+ */
+struct tb_tunnel *tb_tunnel_alloc_dma(struct tb *tb, struct tb_port *nhi,
+ struct tb_port *dst, int transmit_ring,
+ int transmit_path, int receive_ring,
+ int receive_path)
+{
+ struct tb_tunnel *tunnel;
+ struct tb_path *path;
+ u32 credits;
+
+ tunnel = tb_tunnel_alloc(tb, 2, TB_TUNNEL_DMA);
+ if (!tunnel)
+ return NULL;
+
+ tunnel->activate = tb_dma_activate;
+ tunnel->src_port = nhi;
+ tunnel->dst_port = dst;
+
+ credits = tb_dma_credits(nhi);
+
+ path = tb_path_alloc(tb, dst, receive_path, nhi, receive_ring, 0, "DMA RX");
+ if (!path) {
+ tb_tunnel_free(tunnel);
+ return NULL;
+ }
+ tb_dma_init_path(path, TB_PATH_NONE, TB_PATH_SOURCE | TB_PATH_INTERNAL,
+ credits);
+ tunnel->paths[TB_DMA_PATH_IN] = path;
+
+ path = tb_path_alloc(tb, nhi, transmit_ring, dst, transmit_path, 0, "DMA TX");
+ if (!path) {
+ tb_tunnel_free(tunnel);
+ return NULL;
+ }
+ tb_dma_init_path(path, TB_PATH_SOURCE, TB_PATH_ALL, credits);
+ tunnel->paths[TB_DMA_PATH_OUT] = path;
+
+ return tunnel;
+}
+
/**
* tb_tunnel_free() - free a tunnel
* @tunnel: Tunnel to be freed
diff --git a/drivers/thunderbolt/tunnel.h b/drivers/thunderbolt/tunnel.h
index 0373779f43ba..c68bbcd3a62c 100644
--- a/drivers/thunderbolt/tunnel.h
+++ b/drivers/thunderbolt/tunnel.h
@@ -14,6 +14,7 @@
enum tb_tunnel_type {
TB_TUNNEL_PCI,
TB_TUNNEL_DP,
+ TB_TUNNEL_DMA,
};

/**
@@ -47,6 +48,10 @@ struct tb_tunnel *tb_tunnel_alloc_pci(struct tb *tb, struct tb_port *up,
struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in);
struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,
struct tb_port *out);
+struct tb_tunnel *tb_tunnel_alloc_dma(struct tb *tb, struct tb_port *nhi,
+ struct tb_port *dst, int transmit_ring,
+ int transmit_path, int receive_ring,
+ int receive_path);

void tb_tunnel_free(struct tb_tunnel *tunnel);
int tb_tunnel_activate(struct tb_tunnel *tunnel);
@@ -64,5 +69,10 @@ static inline bool tb_tunnel_is_dp(const struct tb_tunnel *tunnel)
return tunnel->type == TB_TUNNEL_DP;
}

+static inline bool tb_tunnel_is_dma(const struct tb_tunnel *tunnel)
+{
+ return tunnel->type == TB_TUNNEL_DMA;
+}
+
#endif

--
2.20.1

2019-03-28 12:39:12

by Mika Westerberg

Subject: [PATCH v3 36/36] thunderbolt: Start firmware on Titan Ridge Apple systems

Titan Ridge flow to start the firmware is the same as Alpine Ridge so we
can do the same on Titan Ridge based Apple systems.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/icm.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
index 597507bbd8ed..f1c10378fa3e 100644
--- a/drivers/thunderbolt/icm.c
+++ b/drivers/thunderbolt/icm.c
@@ -1199,6 +1199,8 @@ static struct pci_dev *get_upstream_port(struct pci_dev *pdev)
case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_BRIDGE:
case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_BRIDGE:
case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_BRIDGE:
+ case PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_BRIDGE:
+ case PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_BRIDGE:
return parent;
}

--
2.20.1

2019-03-28 12:39:25

by Mika Westerberg

Subject: [PATCH v3 29/36] thunderbolt: Add XDomain UUID exchange support

Currently ICM has been handling XDomain UUID exchange so there was no
need to have it in the driver yet. However, since now we are going to
add the same capabilities to the software connection manager it needs to
be handled properly.

For this reason modify the driver XDomain protocol handling so that if
the remote domain UUID is not filled in the core will query it first and
only then start the normal property exchange flow.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/tb_msgs.h | 11 +++
drivers/thunderbolt/xdomain.c | 136 +++++++++++++++++++++++++++++++---
include/linux/thunderbolt.h | 8 ++
3 files changed, 145 insertions(+), 10 deletions(-)

diff --git a/drivers/thunderbolt/tb_msgs.h b/drivers/thunderbolt/tb_msgs.h
index 02c84aa3d018..afbe1d29bb03 100644
--- a/drivers/thunderbolt/tb_msgs.h
+++ b/drivers/thunderbolt/tb_msgs.h
@@ -492,6 +492,17 @@ struct tb_xdp_header {
u32 type;
};

+struct tb_xdp_uuid {
+ struct tb_xdp_header hdr;
+};
+
+struct tb_xdp_uuid_response {
+ struct tb_xdp_header hdr;
+ uuid_t src_uuid;
+ u32 src_route_hi;
+ u32 src_route_lo;
+};
+
struct tb_xdp_properties {
struct tb_xdp_header hdr;
uuid_t src_uuid;
diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c
index 44d3b2e486cd..5118d46702d5 100644
--- a/drivers/thunderbolt/xdomain.c
+++ b/drivers/thunderbolt/xdomain.c
@@ -18,6 +18,7 @@
#include "tb.h"

#define XDOMAIN_DEFAULT_TIMEOUT 5000 /* ms */
+#define XDOMAIN_UUID_RETRIES 10
#define XDOMAIN_PROPERTIES_RETRIES 60
#define XDOMAIN_PROPERTIES_CHANGED_RETRIES 10

@@ -222,6 +223,50 @@ static int tb_xdp_handle_error(const struct tb_xdp_header *hdr)
return 0;
}

+static int tb_xdp_uuid_request(struct tb_ctl *ctl, u64 route, int retry,
+ uuid_t *uuid)
+{
+ struct tb_xdp_uuid_response res;
+ struct tb_xdp_uuid req;
+ int ret;
+
+ memset(&req, 0, sizeof(req));
+ tb_xdp_fill_header(&req.hdr, route, retry % 4, UUID_REQUEST,
+ sizeof(req));
+
+ memset(&res, 0, sizeof(res));
+ ret = __tb_xdomain_request(ctl, &req, sizeof(req),
+ TB_CFG_PKG_XDOMAIN_REQ, &res, sizeof(res),
+ TB_CFG_PKG_XDOMAIN_RESP,
+ XDOMAIN_DEFAULT_TIMEOUT);
+ if (ret)
+ return ret;
+
+ ret = tb_xdp_handle_error(&res.hdr);
+ if (ret)
+ return ret;
+
+ uuid_copy(uuid, &res.src_uuid);
+ return 0;
+}
+
+static int tb_xdp_uuid_response(struct tb_ctl *ctl, u64 route, u8 sequence,
+ const uuid_t *uuid)
+{
+ struct tb_xdp_uuid_response res;
+
+ memset(&res, 0, sizeof(res));
+ tb_xdp_fill_header(&res.hdr, route, sequence, UUID_RESPONSE,
+ sizeof(res));
+
+ uuid_copy(&res.src_uuid, uuid);
+ res.src_route_hi = upper_32_bits(route);
+ res.src_route_lo = lower_32_bits(route);
+
+ return __tb_xdomain_response(ctl, &res, sizeof(res),
+ TB_CFG_PKG_XDOMAIN_RESP);
+}
+
static int tb_xdp_error_response(struct tb_ctl *ctl, u64 route, u8 sequence,
enum tb_xdp_error error)
{
@@ -512,7 +557,14 @@ static void tb_xdp_handle_request(struct work_struct *work)
break;
}

+ case UUID_REQUEST_OLD:
+ case UUID_REQUEST:
+ ret = tb_xdp_uuid_response(ctl, route, sequence, uuid);
+ break;
+
default:
+ tb_xdp_error_response(ctl, route, sequence,
+ ERROR_NOT_SUPPORTED);
break;
}

@@ -839,6 +891,55 @@ static void tb_xdomain_restore_paths(struct tb_xdomain *xd)
}
}

+static void tb_xdomain_get_uuid(struct work_struct *work)
+{
+ struct tb_xdomain *xd = container_of(work, typeof(*xd),
+ get_uuid_work.work);
+ struct tb *tb = xd->tb;
+ uuid_t uuid;
+ int ret;
+
+ ret = tb_xdp_uuid_request(tb->ctl, xd->route, xd->uuid_retries, &uuid);
+ if (ret < 0) {
+ if (xd->uuid_retries-- > 0) {
+ queue_delayed_work(xd->tb->wq, &xd->get_uuid_work,
+ msecs_to_jiffies(100));
+ } else {
+ dev_dbg(&xd->dev, "failed to read remote UUID\n");
+ }
+ return;
+ }
+
+ if (uuid_equal(&uuid, xd->local_uuid)) {
+ dev_dbg(&xd->dev, "intra-domain loop detected\n");
+ return;
+ }
+
+ /*
+ * If the UUID is different, there is another domain connected
+ * so mark this one unplugged and wait for the connection
+ * manager to replace it.
+ */
+ if (xd->remote_uuid && !uuid_equal(&uuid, xd->remote_uuid)) {
+ dev_dbg(&xd->dev, "remote UUID is different, unplugging\n");
+ xd->is_unplugged = true;
+ return;
+ }
+
+ /* First time fill in the missing UUID */
+ if (!xd->remote_uuid) {
+ xd->remote_uuid = kmemdup(&uuid, sizeof(uuid_t), GFP_KERNEL);
+ if (!xd->remote_uuid)
+ return;
+ }
+
+ /* Now we can start the normal properties exchange */
+ queue_delayed_work(xd->tb->wq, &xd->properties_changed_work,
+ msecs_to_jiffies(100));
+ queue_delayed_work(xd->tb->wq, &xd->get_properties_work,
+ msecs_to_jiffies(1000));
+}
+
static void tb_xdomain_get_properties(struct work_struct *work)
{
struct tb_xdomain *xd = container_of(work, typeof(*xd),
@@ -1045,21 +1146,29 @@ static void tb_xdomain_release(struct device *dev)

static void start_handshake(struct tb_xdomain *xd)
{
+ xd->uuid_retries = XDOMAIN_UUID_RETRIES;
xd->properties_retries = XDOMAIN_PROPERTIES_RETRIES;
xd->properties_changed_retries = XDOMAIN_PROPERTIES_CHANGED_RETRIES;

- /* Start exchanging properties with the other host */
- queue_delayed_work(xd->tb->wq, &xd->properties_changed_work,
- msecs_to_jiffies(100));
- queue_delayed_work(xd->tb->wq, &xd->get_properties_work,
- msecs_to_jiffies(1000));
+ if (xd->needs_uuid) {
+ queue_delayed_work(xd->tb->wq, &xd->get_uuid_work,
+ msecs_to_jiffies(100));
+ } else {
+ /* Start exchanging properties with the other host */
+ queue_delayed_work(xd->tb->wq, &xd->properties_changed_work,
+ msecs_to_jiffies(100));
+ queue_delayed_work(xd->tb->wq, &xd->get_properties_work,
+ msecs_to_jiffies(1000));
+ }
}

static void stop_handshake(struct tb_xdomain *xd)
{
+ xd->uuid_retries = 0;
xd->properties_retries = 0;
xd->properties_changed_retries = 0;

+ cancel_delayed_work_sync(&xd->get_uuid_work);
cancel_delayed_work_sync(&xd->get_properties_work);
cancel_delayed_work_sync(&xd->properties_changed_work);
}
@@ -1102,7 +1211,7 @@ EXPORT_SYMBOL_GPL(tb_xdomain_type);
* other domain is reached).
* @route: Route string used to reach the other domain
* @local_uuid: Our local domain UUID
- * @remote_uuid: UUID of the other domain
+ * @remote_uuid: UUID of the other domain (optional)
*
* Allocates new XDomain structure and returns pointer to that. The
* object must be released by calling tb_xdomain_put().
@@ -1121,6 +1230,7 @@ struct tb_xdomain *tb_xdomain_alloc(struct tb *tb, struct device *parent,
xd->route = route;
ida_init(&xd->service_ids);
mutex_init(&xd->lock);
+ INIT_DELAYED_WORK(&xd->get_uuid_work, tb_xdomain_get_uuid);
INIT_DELAYED_WORK(&xd->get_properties_work, tb_xdomain_get_properties);
INIT_DELAYED_WORK(&xd->properties_changed_work,
tb_xdomain_properties_changed);
@@ -1129,9 +1239,14 @@ struct tb_xdomain *tb_xdomain_alloc(struct tb *tb, struct device *parent,
if (!xd->local_uuid)
goto err_free;

- xd->remote_uuid = kmemdup(remote_uuid, sizeof(uuid_t), GFP_KERNEL);
- if (!xd->remote_uuid)
- goto err_free_local_uuid;
+ if (remote_uuid) {
+ xd->remote_uuid = kmemdup(remote_uuid, sizeof(uuid_t),
+ GFP_KERNEL);
+ if (!xd->remote_uuid)
+ goto err_free_local_uuid;
+ } else {
+ xd->needs_uuid = true;
+ }

device_initialize(&xd->dev);
xd->dev.parent = get_device(parent);
@@ -1299,7 +1414,8 @@ static struct tb_xdomain *switch_find_xdomain(struct tb_switch *sw,
xd = port->xdomain;

if (lookup->uuid) {
- if (uuid_equal(xd->remote_uuid, lookup->uuid))
+ if (xd->remote_uuid &&
+ uuid_equal(xd->remote_uuid, lookup->uuid))
return xd;
} else if (lookup->link &&
lookup->link == xd->link &&
diff --git a/include/linux/thunderbolt.h b/include/linux/thunderbolt.h
index bf6ec83e60ee..2d7e012db03f 100644
--- a/include/linux/thunderbolt.h
+++ b/include/linux/thunderbolt.h
@@ -181,6 +181,8 @@ void tb_unregister_property_dir(const char *key, struct tb_property_dir *dir);
* @device_name: Name of the device (or %NULL if not known)
* @is_unplugged: The XDomain is unplugged
* @resume: The XDomain is being resumed
+ * @needs_uuid: If the XDomain does not have @remote_uuid it will be
+ * queried first
* @transmit_path: HopID which the remote end expects us to transmit
* @transmit_ring: Local ring (hop) where outgoing packets are pushed
* @receive_path: HopID which we expect the remote end to transmit
@@ -189,6 +191,9 @@ void tb_unregister_property_dir(const char *key, struct tb_property_dir *dir);
* @properties: Properties exported by the remote domain
* @property_block_gen: Generation of @properties
* @properties_lock: Lock protecting @properties.
+ * @get_uuid_work: Work used to retrieve @remote_uuid
+ * @uuid_retries: Number of times left @remote_uuid is requested before
+ * giving up
* @get_properties_work: Work used to get remote domain properties
* @properties_retries: Number of times left to read properties
* @properties_changed_work: Work used to notify the remote domain that
@@ -220,6 +225,7 @@ struct tb_xdomain {
const char *device_name;
bool is_unplugged;
bool resume;
+ bool needs_uuid;
u16 transmit_path;
u16 transmit_ring;
u16 receive_path;
@@ -227,6 +233,8 @@ struct tb_xdomain {
struct ida service_ids;
struct tb_property_dir *properties;
u32 property_block_gen;
+ struct delayed_work get_uuid_work;
+ int uuid_retries;
struct delayed_work get_properties_work;
int properties_retries;
struct delayed_work properties_changed_work;
--
2.20.1

2019-03-28 12:39:26

by Mika Westerberg

Subject: [PATCH v3 25/36] thunderbolt: Rework NFC credits handling

NFC (non flow control) credits is actually 20-bit field so update
tb_port_add_nfc_credits() to handle this properly. This allows us to set
NFC credits for Display Port path in subsequent patches.

Also make sure the function does not update the hardware if the
underlying switch is already unplugged.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/switch.c | 20 +++++++++++++-------
drivers/thunderbolt/tb_regs.h | 3 +++
2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 028e9a293382..9462e6982061 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -537,14 +537,20 @@ int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged)
*/
int tb_port_add_nfc_credits(struct tb_port *port, int credits)
{
- if (credits == 0)
+ u32 nfc_credits;
+
+ if (credits == 0 || port->sw->is_unplugged)
return 0;
- tb_port_info(port,
- "adding %#x NFC credits (%#x -> %#x)",
- credits,
- port->config.nfc_credits,
- port->config.nfc_credits + credits);
- port->config.nfc_credits += credits;
+
+ nfc_credits = port->config.nfc_credits & TB_PORT_NFC_CREDITS_MASK;
+ nfc_credits += credits;
+
+ tb_port_dbg(port, "adding %d NFC credits to %lu",
+ credits, port->config.nfc_credits & TB_PORT_NFC_CREDITS_MASK);
+
+ port->config.nfc_credits &= ~TB_PORT_NFC_CREDITS_MASK;
+ port->config.nfc_credits |= nfc_credits;
+
return tb_port_write(port, &port->config.nfc_credits,
TB_CFG_PORT, 4, 1);
}
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index 75e935acade5..74c0f4a5606d 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -211,6 +211,9 @@ struct tb_regs_port_header {

} __packed;

+/* DWORD 4 */
+#define TB_PORT_NFC_CREDITS_MASK GENMASK(19, 0)
+
/* PCIe adapter registers */

#define TB_PCI_EN BIT(31)
--
2.20.1

2019-03-28 12:39:34

by Mika Westerberg

Subject: [PATCH v3 34/36] thunderbolt: Make rest of the logging to happen at debug level

Now that the driver can handle every possible tunnel types there is no
point to log everything as info level so turn these to happen at debug
level instead.

While at it remove duplicated tunnel activation log message
(tb_tunnel_activate() calls tb_tunnel_restart() which print the same
message).

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/ctl.c | 2 +-
drivers/thunderbolt/icm.c | 2 +-
drivers/thunderbolt/path.c | 6 +++---
drivers/thunderbolt/switch.c | 19 +++++++++----------
drivers/thunderbolt/tb.c | 11 +++++------
drivers/thunderbolt/tunnel.c | 6 ++----
6 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/thunderbolt/ctl.c b/drivers/thunderbolt/ctl.c
index 73b386de4d15..2427d73be731 100644
--- a/drivers/thunderbolt/ctl.c
+++ b/drivers/thunderbolt/ctl.c
@@ -720,7 +720,7 @@ int tb_cfg_error(struct tb_ctl *ctl, u64 route, u32 port,
.port = port,
.error = error,
};
- tb_ctl_info(ctl, "resetting error on %llx:%x.\n", route, port);
+ tb_ctl_dbg(ctl, "resetting error on %llx:%x.\n", route, port);
return tb_ctl_tx(ctl, &pkg, sizeof(pkg), TB_CFG_PKG_ERROR);
}

diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
index d1f6ec89763c..597507bbd8ed 100644
--- a/drivers/thunderbolt/icm.c
+++ b/drivers/thunderbolt/icm.c
@@ -1568,7 +1568,7 @@ static int icm_firmware_start(struct tb *tb, struct tb_nhi *nhi)
if (val & REG_FW_STS_ICM_EN)
return 0;

- dev_info(&nhi->pdev->dev, "starting ICM firmware\n");
+ dev_dbg(&nhi->pdev->dev, "starting ICM firmware\n");

ret = icm_firmware_reset(tb, nhi);
if (ret)
diff --git a/drivers/thunderbolt/path.c b/drivers/thunderbolt/path.c
index d06fe8bf8872..2dbfb3e1c3fc 100644
--- a/drivers/thunderbolt/path.c
+++ b/drivers/thunderbolt/path.c
@@ -503,8 +503,8 @@ int tb_path_activate(struct tb_path *path)
& out_mask;
hop.unknown3 = 0;

- tb_port_info(path->hops[i].in_port, "Writing hop %d, index %d",
- i, path->hops[i].in_hop_index);
+ tb_port_dbg(path->hops[i].in_port, "Writing hop %d, index %d",
+ i, path->hops[i].in_hop_index);
tb_dump_hop(path->hops[i].in_port, &hop);
res = tb_port_write(path->hops[i].in_port, &hop, TB_CFG_HOPS,
2 * path->hops[i].in_hop_index, 2);
@@ -515,7 +515,7 @@ int tb_path_activate(struct tb_path *path)
}
}
path->activated = true;
- tb_info(path->tb, "path activation complete\n");
+ tb_dbg(path->tb, "path activation complete\n");
return 0;
err:
tb_WARN(path->tb, "path activation failed\n");
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index a5345a6225bd..c1b016574fb4 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -493,23 +493,22 @@ int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged)
if (state < 0)
return state;
if (state == TB_PORT_DISABLED) {
- tb_port_info(port, "is disabled (state: 0)\n");
+ tb_port_dbg(port, "is disabled (state: 0)\n");
return 0;
}
if (state == TB_PORT_UNPLUGGED) {
if (wait_if_unplugged) {
/* used during resume */
- tb_port_info(port,
- "is unplugged (state: 7), retrying...\n");
+ tb_port_dbg(port,
+ "is unplugged (state: 7), retrying...\n");
msleep(100);
continue;
}
- tb_port_info(port, "is unplugged (state: 7)\n");
+ tb_port_dbg(port, "is unplugged (state: 7)\n");
return 0;
}
if (state == TB_PORT_UP) {
- tb_port_info(port,
- "is connected, link is up (state: 2)\n");
+ tb_port_dbg(port, "is connected, link is up (state: 2)\n");
return 1;
}

@@ -517,9 +516,9 @@ int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged)
* After plug-in the state is TB_PORT_CONNECTING. Give it some
* time.
*/
- tb_port_info(port,
- "is connected, link is not up (state: %d), retrying...\n",
- state);
+ tb_port_dbg(port,
+ "is connected, link is not up (state: %d), retrying...\n",
+ state);
msleep(100);
}
tb_port_warn(port,
@@ -585,7 +584,7 @@ int tb_port_set_initial_credits(struct tb_port *port, u32 credits)
int tb_port_clear_counter(struct tb_port *port, int counter)
{
u32 zero[3] = { 0, 0, 0 };
- tb_port_info(port, "clearing counter %d\n", counter);
+ tb_port_dbg(port, "clearing counter %d\n", counter);
return tb_port_write(port, zero, TB_CFG_COUNTERS, 3 * counter, 3);
}

diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index e39fc1e35e6b..1f7a9e1cc09c 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -556,18 +556,17 @@ static void tb_handle_hotplug(struct work_struct *work)
} else if (tb_port_is_dpout(port)) {
tb_teardown_dp(tb, port);
} else {
- tb_port_info(port,
- "got unplug event for disconnected port, ignoring\n");
+ tb_port_dbg(port,
+ "got unplug event for disconnected port, ignoring\n");
}
} else if (port->remote) {
- tb_port_info(port,
- "got plug event for connected port, ignoring\n");
+ tb_port_dbg(port, "got plug event for connected port, ignoring\n");
} else {
if (tb_port_is_null(port)) {
- tb_port_info(port, "hotplug: scanning\n");
+ tb_port_dbg(port, "hotplug: scanning\n");
tb_scan_port(port);
if (!port->remote)
- tb_port_info(port, "hotplug: no switch found\n");
+ tb_port_dbg(port, "hotplug: no switch found\n");
} else if (tb_port_is_dpout(port)) {
tb_tunnel_dp(tb, port);
}
diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index 9f9b26b12d0a..31d0234837e4 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -611,7 +611,7 @@ int tb_tunnel_restart(struct tb_tunnel *tunnel)
{
int res, i;

- tb_tunnel_info(tunnel, "activating\n");
+ tb_tunnel_dbg(tunnel, "activating\n");

/*
* Make sure all paths are properly disabled before enabling
@@ -660,8 +660,6 @@ int tb_tunnel_activate(struct tb_tunnel *tunnel)
{
int i;

- tb_tunnel_info(tunnel, "activating\n");
-
for (i = 0; i < tunnel->npaths; i++) {
if (tunnel->paths[i]->activated) {
tb_tunnel_WARN(tunnel,
@@ -681,7 +679,7 @@ void tb_tunnel_deactivate(struct tb_tunnel *tunnel)
{
int i;

- tb_tunnel_info(tunnel, "deactivating\n");
+ tb_tunnel_dbg(tunnel, "deactivating\n");

if (tunnel->activate)
tunnel->activate(tunnel, false);
--
2.20.1

2019-03-28 12:40:15

by Mika Westerberg

Subject: [PATCH v3 24/36] thunderbolt: Generalize port finding routines to support all port types

We will be needing these routines to find Display Port adapters as well
so modify them to take port type as the second parameter.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/switch.c | 16 ++++++++++++++++
drivers/thunderbolt/tb.c | 35 +++++++++++++++++------------------
drivers/thunderbolt/tb.h | 1 +
3 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 00aec2124f79..028e9a293382 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -730,6 +730,22 @@ struct tb_port *tb_next_port_on_path(struct tb_port *start, struct tb_port *end,
return next;
}

+/**
+ * tb_port_is_enabled() - Is the adapter port enabled
+ * @port: Port to check
+ */
+bool tb_port_is_enabled(struct tb_port *port)
+{
+ switch (port->config.type) {
+ case TB_TYPE_PCIE_UP:
+ case TB_TYPE_PCIE_DOWN:
+ return tb_pci_port_is_enabled(port);
+
+ default:
+ return false;
+ }
+}
+
/**
* tb_pci_port_is_enabled() - Is the PCIe adapter port enabled
* @port: PCIe port to check
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index fb01396a62a9..903922a16d64 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -180,40 +180,39 @@ static void tb_free_unplugged_children(struct tb_switch *sw)
}
}

-
/**
- * find_pci_up_port() - return the first PCIe up port on @sw or NULL
+ * tb_find_port() - return the first port of @type on @sw or NULL
+ * @sw: Switch to find the port from
+ * @type: Port type to look for
*/
-static struct tb_port *tb_find_pci_up_port(struct tb_switch *sw)
+static struct tb_port *tb_find_port(struct tb_switch *sw,
+ enum tb_port_type type)
{
int i;
for (i = 1; i <= sw->config.max_port_number; i++)
- if (sw->ports[i].config.type == TB_TYPE_PCIE_UP)
+ if (sw->ports[i].config.type == type)
return &sw->ports[i];
return NULL;
}

/**
- * find_unused_down_port() - return the first inactive PCIe down port on @sw
+ * tb_find_unused_port() - return the first inactive port on @sw
+ * @sw: Switch to find the port on
+ * @type: Port type to look for
*/
-static struct tb_port *tb_find_unused_down_port(struct tb_switch *sw)
+static struct tb_port *tb_find_unused_port(struct tb_switch *sw,
+ enum tb_port_type type)
{
int i;
- int cap;
- int res;
- int data;
+
for (i = 1; i <= sw->config.max_port_number; i++) {
if (tb_is_upstream_port(&sw->ports[i]))
continue;
- if (sw->ports[i].config.type != TB_TYPE_PCIE_DOWN)
- continue;
- cap = sw->ports[i].cap_adap;
- if (!cap)
+ if (sw->ports[i].config.type != type)
continue;
- res = tb_port_read(&sw->ports[i], &data, TB_CFG_PORT, cap, 1);
- if (res < 0)
+ if (!sw->ports[i].cap_adap)
continue;
- if (data & 0x80000000)
+ if (tb_port_is_enabled(&sw->ports[i]))
continue;
return &sw->ports[i];
}
@@ -255,7 +254,7 @@ static struct tb_port *tb_find_pcie_down(struct tb_switch *sw,
}

out:
- return tb_find_unused_down_port(sw);
+ return tb_find_unused_port(sw, TB_TYPE_PCIE_DOWN);
}

static int tb_tunnel_pci(struct tb *tb, struct tb_switch *sw)
@@ -265,7 +264,7 @@ static int tb_tunnel_pci(struct tb *tb, struct tb_switch *sw)
struct tb_switch *parent_sw;
struct tb_tunnel *tunnel;

- up = tb_find_pci_up_port(sw);
+ up = tb_find_port(sw, TB_TYPE_PCIE_UP);
if (!up)
return 0;

diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 5ca8be0db310..d802d8c5a953 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -546,6 +546,7 @@ struct tb_port *tb_next_port_on_path(struct tb_port *start, struct tb_port *end,

int tb_switch_find_vse_cap(struct tb_switch *sw, enum tb_switch_vse_cap vsec);
int tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap);
+bool tb_port_is_enabled(struct tb_port *port);

bool tb_pci_port_is_enabled(struct tb_port *port);
int tb_pci_port_enable(struct tb_port *port, bool enable);
--
2.20.1

2019-03-28 12:40:23

by Mika Westerberg

Subject: [PATCH v3 31/36] thunderbolt: Make tb_switch_alloc() return ERR_PTR()

In order to detect possible connections to other domains we need to be
able to find out why tb_switch_alloc() fails so make it return ERR_PTR()
instead. This allows the caller to differentiate between errors such as
-ENOMEM which comes from the kernel and for instance -EIO which comes
from the hardware when trying to access the possible switch.

Convert all the current call sites to handle this properly.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/icm.c | 6 +++---
drivers/thunderbolt/switch.c | 36 ++++++++++++++++++++----------------
drivers/thunderbolt/tb.c | 6 +++---
3 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
index 805958e53c58..d1f6ec89763c 100644
--- a/drivers/thunderbolt/icm.c
+++ b/drivers/thunderbolt/icm.c
@@ -468,7 +468,7 @@ static void add_switch(struct tb_switch *parent_sw, u64 route,
pm_runtime_get_sync(&parent_sw->dev);

sw = tb_switch_alloc(parent_sw->tb, &parent_sw->dev, route);
- if (!sw)
+ if (IS_ERR(sw))
goto out;

sw->uuid = kmemdup(uuid, sizeof(*uuid), GFP_KERNEL);
@@ -1848,8 +1848,8 @@ static int icm_start(struct tb *tb)
tb->root_switch = tb_switch_alloc_safe_mode(tb, &tb->dev, 0);
else
tb->root_switch = tb_switch_alloc(tb, &tb->dev, 0);
- if (!tb->root_switch)
- return -ENODEV;
+ if (IS_ERR(tb->root_switch))
+ return PTR_ERR(tb->root_switch);

/*
* NVM upgrade has not been tested on Apple systems and they
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index ecf53d986a5c..460f2bcad40a 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -1450,30 +1450,32 @@ static int tb_switch_get_generation(struct tb_switch *sw)
* separately. The returned switch should be released by calling
* tb_switch_put().
*
- * Return: Pointer to the allocated switch or %NULL in case of failure
+ * Return: Pointer to the allocated switch or ERR_PTR() in case of
+ * failure.
*/
struct tb_switch *tb_switch_alloc(struct tb *tb, struct device *parent,
u64 route)
{
struct tb_switch *sw;
int upstream_port;
- int i, cap, depth;
+ int i, ret, depth;

/* Make sure we do not exceed maximum topology limit */
depth = tb_route_length(route);
if (depth > TB_SWITCH_MAX_DEPTH)
- return NULL;
+ return ERR_PTR(-EADDRNOTAVAIL);

upstream_port = tb_cfg_get_upstream_port(tb->ctl, route);
if (upstream_port < 0)
- return NULL;
+ return ERR_PTR(upstream_port);

sw = kzalloc(sizeof(*sw), GFP_KERNEL);
if (!sw)
- return NULL;
+ return ERR_PTR(-ENOMEM);

sw->tb = tb;
- if (tb_cfg_read(tb->ctl, &sw->config, route, 0, TB_CFG_SWITCH, 0, 5))
+ ret = tb_cfg_read(tb->ctl, &sw->config, route, 0, TB_CFG_SWITCH, 0, 5);
+ if (ret)
goto err_free_sw_ports;

tb_dbg(tb, "current switch config:\n");
@@ -1489,8 +1491,10 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, struct device *parent,
/* initialize ports */
sw->ports = kcalloc(sw->config.max_port_number + 1, sizeof(*sw->ports),
GFP_KERNEL);
- if (!sw->ports)
+ if (!sw->ports) {
+ ret = -ENOMEM;
goto err_free_sw_ports;
+ }

for (i = 0; i <= sw->config.max_port_number; i++) {
/* minimum setup for tb_find_cap and tb_drom_read to work */
@@ -1500,16 +1504,16 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, struct device *parent,

sw->generation = tb_switch_get_generation(sw);

- cap = tb_switch_find_vse_cap(sw, TB_VSE_CAP_PLUG_EVENTS);
- if (cap < 0) {
+ ret = tb_switch_find_vse_cap(sw, TB_VSE_CAP_PLUG_EVENTS);
+ if (ret < 0) {
tb_sw_warn(sw, "cannot find TB_VSE_CAP_PLUG_EVENTS aborting\n");
goto err_free_sw_ports;
}
- sw->cap_plug_events = cap;
+ sw->cap_plug_events = ret;

- cap = tb_switch_find_vse_cap(sw, TB_VSE_CAP_LINK_CONTROLLER);
- if (cap > 0)
- sw->cap_lc = cap;
+ ret = tb_switch_find_vse_cap(sw, TB_VSE_CAP_LINK_CONTROLLER);
+ if (ret > 0)
+ sw->cap_lc = ret;

/* Root switch is always authorized */
if (!route)
@@ -1528,7 +1532,7 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, struct device *parent,
kfree(sw->ports);
kfree(sw);

- return NULL;
+ return ERR_PTR(ret);
}

/**
@@ -1543,7 +1547,7 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, struct device *parent,
*
* The returned switch must be released by calling tb_switch_put().
*
- * Return: Pointer to the allocated switch or %NULL in case of failure
+ * Return: Pointer to the allocated switch or ERR_PTR() in case of failure
*/
struct tb_switch *
tb_switch_alloc_safe_mode(struct tb *tb, struct device *parent, u64 route)
@@ -1552,7 +1556,7 @@ tb_switch_alloc_safe_mode(struct tb *tb, struct device *parent, u64 route)

sw = kzalloc(sizeof(*sw), GFP_KERNEL);
if (!sw)
- return NULL;
+ return ERR_PTR(-ENOMEM);

sw->tb = tb;
sw->config.depth = tb_route_length(route);
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 8a97a4e19638..c5e82c4dcb64 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -148,7 +148,7 @@ static void tb_scan_port(struct tb_port *port)
}
sw = tb_switch_alloc(port->sw->tb, &port->sw->dev,
tb_downstream_route(port));
- if (!sw)
+ if (IS_ERR(sw))
return;

if (tb_switch_configure(sw)) {
@@ -533,8 +533,8 @@ static int tb_start(struct tb *tb)
int ret;

tb->root_switch = tb_switch_alloc(tb, &tb->dev, 0);
- if (!tb->root_switch)
- return -ENOMEM;
+ if (IS_ERR(tb->root_switch))
+ return PTR_ERR(tb->root_switch);

/*
* ICM firmware upgrade needs running firmware and in native
--
2.20.1

2019-03-28 12:40:28

by Mika Westerberg

Subject: [PATCH v3 14/36] thunderbolt: Rename tunnel_pci to tunnel

In order to tunnel non-PCIe traffic as well rename tunnel_pci.[ch] to
tunnel.[ch] to reflect this fact. No functional changes.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/Makefile | 2 +-
drivers/thunderbolt/tb.c | 2 +-
drivers/thunderbolt/{tunnel_pci.c => tunnel.c} | 4 ++--
drivers/thunderbolt/{tunnel_pci.h => tunnel.h} | 6 +++---
4 files changed, 7 insertions(+), 7 deletions(-)
rename drivers/thunderbolt/{tunnel_pci.c => tunnel.c} (98%)
rename drivers/thunderbolt/{tunnel_pci.h => tunnel.h} (87%)

diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
index 8531f15d3b3c..833bdee3cec7 100644
--- a/drivers/thunderbolt/Makefile
+++ b/drivers/thunderbolt/Makefile
@@ -1,3 +1,3 @@
obj-${CONFIG_THUNDERBOLT} := thunderbolt.o
-thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o tunnel_pci.o eeprom.o
+thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o tunnel.o eeprom.o
thunderbolt-objs += domain.o dma_port.o icm.o property.o xdomain.o lc.o
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index e71530d0af65..8de43a2ab205 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -12,7 +12,7 @@

#include "tb.h"
#include "tb_regs.h"
-#include "tunnel_pci.h"
+#include "tunnel.h"

/**
* struct tb_cm - Simple Thunderbolt connection manager
diff --git a/drivers/thunderbolt/tunnel_pci.c b/drivers/thunderbolt/tunnel.c
similarity index 98%
rename from drivers/thunderbolt/tunnel_pci.c
rename to drivers/thunderbolt/tunnel.c
index 2de4edccbd6d..1e470564e99d 100644
--- a/drivers/thunderbolt/tunnel_pci.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Thunderbolt Cactus Ridge driver - PCIe tunnel
+ * Thunderbolt Cactus Ridge driver - Tunneling support
*
* Copyright (c) 2014 Andreas Noever <[email protected]>
*/
@@ -8,7 +8,7 @@
#include <linux/slab.h>
#include <linux/list.h>

-#include "tunnel_pci.h"
+#include "tunnel.h"
#include "tb.h"

#define __TB_TUNNEL_PRINT(level, tunnel, fmt, arg...) \
diff --git a/drivers/thunderbolt/tunnel_pci.h b/drivers/thunderbolt/tunnel.h
similarity index 87%
rename from drivers/thunderbolt/tunnel_pci.h
rename to drivers/thunderbolt/tunnel.h
index f9b65fa1fd4d..dff0f27d6ab5 100644
--- a/drivers/thunderbolt/tunnel_pci.h
+++ b/drivers/thunderbolt/tunnel.h
@@ -1,12 +1,12 @@
/* SPDX-License-Identifier: GPL-2.0 */
/*
- * Thunderbolt Cactus Ridge driver - PCIe tunnel
+ * Thunderbolt Cactus Ridge driver - Tunneling support
*
* Copyright (c) 2014 Andreas Noever <[email protected]>
*/

-#ifndef TB_PCI_H_
-#define TB_PCI_H_
+#ifndef TB_TUNNEL_H_
+#define TB_TUNNEL_H_

#include "tb.h"

--
2.20.1

2019-03-28 12:40:37

by Mika Westerberg

Subject: [PATCH v3 16/36] thunderbolt: Add functions for allocating and releasing HopIDs

Each port has a separate path configuration space that is used for
finding the next hop (switch) in the path. HopID is an index to this
configuration space. HopIDs 0 - 7 are reserved by the protocol.

In order to get next available HopID for each direction we provide two
pairs of helper functions that can be used to allocate and release
HopIDs for a given port.

While there remove obsolete TODO comment.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/nhi.c | 3 +-
drivers/thunderbolt/switch.c | 87 +++++++++++++++++++++++++++++++++++-
drivers/thunderbolt/tb.h | 11 +++++
3 files changed, 98 insertions(+), 3 deletions(-)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index 9aa44f9762a3..cac1ead5e302 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -27,8 +27,7 @@
* use this ring for anything else.
*/
#define RING_E2E_UNUSED_HOPID 2
-/* HopIDs 0-7 are reserved by the Thunderbolt protocol */
-#define RING_FIRST_USABLE_HOPID 8
+#define RING_FIRST_USABLE_HOPID TB_PATH_MIN_HOPID

/*
* Minimal number of vectors when we use MSI-X. Two for control channel
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index b132439618da..30f3d6a9fe90 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -594,11 +594,88 @@ static int tb_init_port(struct tb_port *port)

tb_dump_port(port->sw->tb, &port->config);

- /* TODO: Read dual link port, DP port and more from EEPROM. */
+ /* Control port does not need HopID allocation */
+ if (port->port) {
+ ida_init(&port->in_hopids);
+ ida_init(&port->out_hopids);
+ }
+
return 0;

}

+static int tb_port_alloc_hopid(struct tb_port *port, bool in, int min_hopid,
+ int max_hopid)
+{
+ int port_max_hopid;
+ struct ida *ida;
+
+ if (in) {
+ port_max_hopid = port->config.max_in_hop_id;
+ ida = &port->in_hopids;
+ } else {
+ port_max_hopid = port->config.max_out_hop_id;
+ ida = &port->out_hopids;
+ }
+
+ /* HopIDs 0-7 are reserved */
+ if (min_hopid < TB_PATH_MIN_HOPID)
+ min_hopid = TB_PATH_MIN_HOPID;
+
+ if (max_hopid < 0 || max_hopid > port_max_hopid)
+ max_hopid = port_max_hopid;
+
+ return ida_simple_get(ida, min_hopid, max_hopid + 1, GFP_KERNEL);
+}
+
+/**
+ * tb_port_alloc_in_hopid() - Allocate input HopID from port
+ * @port: Port to allocate HopID for
+ * @min_hopid: Minimum acceptable input HopID
+ * @max_hopid: Maximum acceptable input HopID
+ *
+ * Return: HopID between @min_hopid and @max_hopid or negative errno in
+ * case of error.
+ */
+int tb_port_alloc_in_hopid(struct tb_port *port, int min_hopid, int max_hopid)
+{
+ return tb_port_alloc_hopid(port, true, min_hopid, max_hopid);
+}
+
+/**
+ * tb_port_alloc_out_hopid() - Allocate output HopID from port
+ * @port: Port to allocate HopID for
+ * @min_hopid: Minimum acceptable output HopID
+ * @max_hopid: Maximum acceptable output HopID
+ *
+ * Return: HopID between @min_hopid and @max_hopid or negative errno in
+ * case of error.
+ */
+int tb_port_alloc_out_hopid(struct tb_port *port, int min_hopid, int max_hopid)
+{
+ return tb_port_alloc_hopid(port, false, min_hopid, max_hopid);
+}
+
+/**
+ * tb_port_release_in_hopid() - Release allocated input HopID from port
+ * @port: Port whose HopID to release
+ * @hopid: HopID to release
+ */
+void tb_port_release_in_hopid(struct tb_port *port, int hopid)
+{
+ ida_simple_remove(&port->in_hopids, hopid);
+}
+
+/**
+ * tb_port_release_out_hopid() - Release allocated output HopID from port
+ * @port: Port whose HopID to release
+ * @hopid: HopID to release
+ */
+void tb_port_release_out_hopid(struct tb_port *port, int hopid)
+{
+ ida_simple_remove(&port->out_hopids, hopid);
+}
+
/**
* tb_pci_port_enable() - Enable PCIe adapter port
* @port: PCIe port to enable
@@ -1055,9 +1132,17 @@ static const struct attribute_group *switch_groups[] = {
static void tb_switch_release(struct device *dev)
{
struct tb_switch *sw = tb_to_switch(dev);
+ int i;

dma_port_free(sw->dma_port);

+ for (i = 1; i <= sw->config.max_port_number; i++) {
+ if (!sw->ports[i].disabled) {
+ ida_destroy(&sw->ports[i].in_hopids);
+ ida_destroy(&sw->ports[i].out_hopids);
+ }
+ }
+
kfree(sw->uuid);
kfree(sw->device_name);
kfree(sw->vendor_name);
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index d1f8e9722f33..1cb5009195f9 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -127,6 +127,8 @@ struct tb_switch {
* @dual_link_port: If the switch is connected using two ports, points
* to the other port.
* @link_nr: Is this primary or secondary port on the dual_link.
+ * @in_hopids: Currently allocated input HopIDs
+ * @out_hopids: Currently allocated output HopIDs
*/
struct tb_port {
struct tb_regs_port_header config;
@@ -139,6 +141,8 @@ struct tb_port {
bool disabled;
struct tb_port *dual_link_port;
u8 link_nr:1;
+ struct ida in_hopids;
+ struct ida out_hopids;
};

/**
@@ -194,6 +198,9 @@ struct tb_path {
int path_length; /* number of hops */
};

+/* HopIDs 0-7 are reserved by the Thunderbolt protocol */
+#define TB_PATH_MIN_HOPID 8
+
/**
* struct tb_cm_ops - Connection manager specific operations vector
* @driver_ready: Called right after control channel is started. Used by
@@ -453,6 +460,10 @@ static inline bool tb_switch_is_er(const struct tb_switch *sw)
int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged);
int tb_port_add_nfc_credits(struct tb_port *port, int credits);
int tb_port_clear_counter(struct tb_port *port, int counter);
+int tb_port_alloc_in_hopid(struct tb_port *port, int hopid, int max_hopid);
+void tb_port_release_in_hopid(struct tb_port *port, int hopid);
+int tb_port_alloc_out_hopid(struct tb_port *port, int hopid, int max_hopid);
+void tb_port_release_out_hopid(struct tb_port *port, int hopid);

int tb_switch_find_vse_cap(struct tb_switch *sw, enum tb_switch_vse_cap vsec);
int tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap);
--
2.20.1

2019-03-28 12:40:51

by Mika Westerberg

Subject: [PATCH v3 23/36] thunderbolt: Scan only valid NULL adapter ports in hotplug

The only way to expand Thunderbolt topology is through the NULL adapter
ports (typically ports 1, 2, 3 and 4). There is no point handling
Thunderbolt hotplug events on any other port.

Add a helper function (tb_port_is_null()) that can be used to determine
if the port is NULL port, and use it in software connection manager code
when hotplug event is handled.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/tb.c | 10 ++++++----
drivers/thunderbolt/tb.h | 5 +++++
2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index cfc451c938fd..fb01396a62a9 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -356,10 +356,12 @@ static void tb_handle_hotplug(struct work_struct *work)
tb_port_info(port,
"got plug event for connected port, ignoring\n");
} else {
- tb_port_info(port, "hotplug: scanning\n");
- tb_scan_port(port);
- if (!port->remote)
- tb_port_info(port, "hotplug: no switch found\n");
+ if (tb_port_is_null(port)) {
+ tb_port_info(port, "hotplug: scanning\n");
+ tb_scan_port(port);
+ if (!port->remote)
+ tb_port_info(port, "hotplug: no switch found\n");
+ }
}

put_sw:
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index d810fb188937..5ca8be0db310 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -320,6 +320,11 @@ static inline bool tb_port_has_remote(const struct tb_port *port)
return true;
}

+static inline bool tb_port_is_null(const struct tb_port *port)
+{
+ return port && port->port && port->config.type == TB_TYPE_PORT;
+}
+
static inline bool tb_port_is_pcie_down(const struct tb_port *port)
{
return port && port->config.type == TB_TYPE_PCIE_DOWN;
--
2.20.1

2019-03-28 12:40:54

by Mika Westerberg

Subject: [PATCH v3 26/36] thunderbolt: Add support for Display Port tunnels

Display Port tunnels are somewhat more complex than PCIe tunnels as it
requires 3 tunnels (AUX Rx/Tx and Video). In addition we are not
supposed to create the tunnels immediately when a DP OUT is enumerated.
Instead we need to wait until we get hotplug event to that adapter port
or check if the port has HPD set before tunnels can be established. This
adds Display Port tunneling support to the software connection manager.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/switch.c | 111 ++++++++++++++
drivers/thunderbolt/tb.c | 129 +++++++++++++---
drivers/thunderbolt/tb.h | 17 +++
drivers/thunderbolt/tb_regs.h | 22 +++
drivers/thunderbolt/tunnel.c | 277 +++++++++++++++++++++++++++++++++-
drivers/thunderbolt/tunnel.h | 23 +++
6 files changed, 554 insertions(+), 25 deletions(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 9462e6982061..1de1afa24182 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -747,6 +747,10 @@ bool tb_port_is_enabled(struct tb_port *port)
case TB_TYPE_PCIE_DOWN:
return tb_pci_port_is_enabled(port);

+ case TB_TYPE_DP_HDMI_IN:
+ case TB_TYPE_DP_HDMI_OUT:
+ return tb_dp_port_is_enabled(port);
+
default:
return false;
}
@@ -779,6 +783,113 @@ int tb_pci_port_enable(struct tb_port *port, bool enable)
return tb_port_write(port, &word, TB_CFG_PORT, port->cap_adap, 1);
}

+/**
+ * tb_dp_port_hpd_is_active() - Is HPD already active
+ * @port: DP out port to check
+ *
+ * Checks if the DP OUT adapter port has HDP bit already set.
+ */
+int tb_dp_port_hpd_is_active(struct tb_port *port)
+{
+ u32 data;
+ int ret;
+
+ ret = tb_port_read(port, &data, TB_CFG_PORT, port->cap_adap + 2, 1);
+ if (ret)
+ return ret;
+
+ return !!(data & TB_DP_HDP);
+}
+
+/**
+ * tb_dp_port_hpd_clear() - Clear HPD from DP IN port
+ * @port: Port to clear HPD
+ *
+ * If the DP IN port has HDP set, this function can be used to clear it.
+ */
+int tb_dp_port_hpd_clear(struct tb_port *port)
+{
+ u32 data;
+ int ret;
+
+ ret = tb_port_read(port, &data, TB_CFG_PORT, port->cap_adap + 3, 1);
+ if (ret)
+ return ret;
+
+ data |= TB_DP_HPDC;
+ return tb_port_write(port, &data, TB_CFG_PORT, port->cap_adap + 3, 1);
+}
+
+/**
+ * tb_dp_port_set_hops() - Set video/aux Hop IDs for DP port
+ * @port: DP IN/OUT port to set hops
+ * @video: Video Hop ID
+ * @aux_tx: AUX TX Hop ID
+ * @aux_rx: AUX RX Hop ID
+ *
+ * Programs specified Hop IDs for DP IN/OUT port.
+ */
+int tb_dp_port_set_hops(struct tb_port *port, unsigned int video,
+ unsigned int aux_tx, unsigned int aux_rx)
+{
+ u32 data[2];
+ int ret;
+
+ ret = tb_port_read(port, data, TB_CFG_PORT, port->cap_adap,
+ ARRAY_SIZE(data));
+ if (ret)
+ return ret;
+
+ data[0] &= ~TB_DP_VIDEO_HOPID_MASK;
+ data[1] &= ~(TB_DP_AUX_RX_HOPID_MASK | TB_DP_AUX_TX_HOPID_MASK);
+
+ data[0] |= (video << TB_DP_VIDEO_HOPID_SHIFT) & TB_DP_VIDEO_HOPID_MASK;
+ data[1] |= aux_tx & TB_DP_AUX_TX_HOPID_MASK;
+ data[1] |= (aux_rx << TB_DP_AUX_RX_HOPID_SHIFT) & TB_DP_AUX_RX_HOPID_MASK;
+
+ return tb_port_write(port, data, TB_CFG_PORT, port->cap_adap,
+ ARRAY_SIZE(data));
+}
+
+/**
+ * tb_dp_port_is_enabled() - Is DP adapter port enabled
+ * @port: DP adapter port to check
+ */
+bool tb_dp_port_is_enabled(struct tb_port *port)
+{
+ u32 data;
+
+ if (tb_port_read(port, &data, TB_CFG_PORT, port->cap_adap, 1))
+ return false;
+
+ return !!(data & (TB_DP_VIDEO_EN | TB_DP_AUX_EN));
+}
+
+/**
+ * tb_dp_port_enable() - Enables/disables DP paths of a port
+ * @port: DP IN/OUT port
+ * @enable: Enable/disable DP path
+ *
+ * Once Hop IDs are programmed DP paths can be enabled or disabled by
+ * calling this function.
+ */
+int tb_dp_port_enable(struct tb_port *port, bool enable)
+{
+ u32 data;
+ int ret;
+
+ ret = tb_port_read(port, &data, TB_CFG_PORT, port->cap_adap, 1);
+ if (ret)
+ return ret;
+
+ if (enable)
+ data |= TB_DP_VIDEO_EN | TB_DP_AUX_EN;
+ else
+ data &= ~(TB_DP_VIDEO_EN | TB_DP_AUX_EN);
+
+ return tb_port_write(port, &data, TB_CFG_PORT, port->cap_adap, 1);
+}
+
/* switch utility functions */

static void tb_dump_switch(struct tb *tb, struct tb_regs_switch_header *sw)
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 903922a16d64..c5e96e7ac37a 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -28,6 +28,32 @@ struct tb_cm {
bool hotplug_active;
};

+struct tb_hotplug_event {
+ struct work_struct work;
+ struct tb *tb;
+ u64 route;
+ u8 port;
+ bool unplug;
+};
+
+static void tb_handle_hotplug(struct work_struct *work);
+
+static void tb_queue_hotplug(struct tb *tb, u64 route, u8 port, bool unplug)
+{
+ struct tb_hotplug_event *ev;
+
+ ev = kmalloc(sizeof(*ev), GFP_KERNEL);
+ if (!ev)
+ return;
+
+ ev->tb = tb;
+ ev->route = route;
+ ev->port = port;
+ ev->unplug = unplug;
+ INIT_WORK(&ev->work, tb_handle_hotplug);
+ queue_work(tb->wq, &ev->work);
+}
+
/* enumeration & hot plug handling */

static void tb_discover_tunnels(struct tb_switch *sw)
@@ -42,6 +68,10 @@ static void tb_discover_tunnels(struct tb_switch *sw)

port = &sw->ports[i];
switch (port->config.type) {
+ case TB_TYPE_DP_HDMI_IN:
+ tunnel = tb_tunnel_discover_dp(tb, port);
+ break;
+
case TB_TYPE_PCIE_DOWN:
tunnel = tb_tunnel_discover_pci(tb, port);
break;
@@ -50,16 +80,19 @@ static void tb_discover_tunnels(struct tb_switch *sw)
break;
}

- if (tunnel) {
+ if (!tunnel)
+ continue;
+
+ if (tb_tunnel_is_pci(tunnel)) {
struct tb_switch *parent = tunnel->dst_port->sw;

while (parent != tunnel->src_port->sw) {
parent->boot = true;
parent = tb_switch_parent(parent);
}
-
- list_add_tail(&tunnel->list, &tcm->tunnel_list);
}
+
+ list_add_tail(&tunnel->list, &tcm->tunnel_list);
}

for (i = 1; i <= sw->config.max_port_number; i++) {
@@ -91,6 +124,15 @@ static void tb_scan_port(struct tb_port *port)

if (tb_is_upstream_port(port))
return;
+
+ if (tb_port_is_dpout(port) && tb_dp_port_hpd_is_active(port) == 1 &&
+ !tb_dp_port_is_enabled(port)) {
+ tb_port_dbg(port, "DP adapter HPD set, queuing hotplug\n");
+ tb_queue_hotplug(port->sw->tb, tb_route(port->sw), port->port,
+ false);
+ return;
+ }
+
if (port->config.type != TB_TYPE_PORT)
return;
if (port->dual_link_port && port->link_nr)
@@ -139,6 +181,26 @@ static void tb_scan_port(struct tb_port *port)
tb_scan_switch(sw);
}

+static int tb_free_tunnel(struct tb *tb, enum tb_tunnel_type type,
+ struct tb_port *src_port, struct tb_port *dst_port)
+{
+ struct tb_cm *tcm = tb_priv(tb);
+ struct tb_tunnel *tunnel;
+
+ list_for_each_entry(tunnel, &tcm->tunnel_list, list) {
+ if (tunnel->type == type &&
+ ((src_port && src_port == tunnel->src_port) ||
+ (dst_port && dst_port == tunnel->dst_port))) {
+ tb_tunnel_deactivate(tunnel);
+ list_del(&tunnel->list);
+ tb_tunnel_free(tunnel);
+ return 0;
+ }
+ }
+
+ return -ENODEV;
+}
+
/**
* tb_free_invalid_tunnels() - destroy tunnels of devices that have gone away
*/
@@ -257,6 +319,44 @@ static struct tb_port *tb_find_pcie_down(struct tb_switch *sw,
return tb_find_unused_port(sw, TB_TYPE_PCIE_DOWN);
}

+static int tb_tunnel_dp(struct tb *tb, struct tb_port *out)
+{
+ struct tb_cm *tcm = tb_priv(tb);
+ struct tb_switch *sw = out->sw;
+ struct tb_tunnel *tunnel;
+ struct tb_port *in;
+
+ if (tb_port_is_enabled(out))
+ return 0;
+
+ do {
+ sw = tb_to_switch(sw->dev.parent);
+ if (!sw)
+ return 0;
+ in = tb_find_unused_port(sw, TB_TYPE_DP_HDMI_IN);
+ } while (!in);
+
+ tunnel = tb_tunnel_alloc_dp(tb, in, out);
+ if (!tunnel) {
+ tb_port_dbg(out, "DP tunnel allocation failed\n");
+ return -ENOMEM;
+ }
+
+ if (tb_tunnel_activate(tunnel)) {
+ tb_port_info(out, "DP tunnel activation failed, aborting\n");
+ tb_tunnel_free(tunnel);
+ return -EIO;
+ }
+
+ list_add_tail(&tunnel->list, &tcm->tunnel_list);
+ return 0;
+}
+
+static void tb_teardown_dp(struct tb *tb, struct tb_port *out)
+{
+ tb_free_tunnel(tb, TB_TUNNEL_DP, NULL, out);
+}
+
static int tb_tunnel_pci(struct tb *tb, struct tb_switch *sw)
{
struct tb_port *up, *down, *port;
@@ -295,14 +395,6 @@ static int tb_tunnel_pci(struct tb *tb, struct tb_switch *sw)

/* hotplug handling */

-struct tb_hotplug_event {
- struct work_struct work;
- struct tb *tb;
- u64 route;
- u8 port;
- bool unplug;
-};
-
/**
* tb_handle_hotplug() - handle hotplug event
*
@@ -347,6 +439,8 @@ static void tb_handle_hotplug(struct work_struct *work)
port->remote = NULL;
if (port->dual_link_port)
port->dual_link_port->remote = NULL;
+ } else if (tb_port_is_dpout(port)) {
+ tb_teardown_dp(tb, port);
} else {
tb_port_info(port,
"got unplug event for disconnected port, ignoring\n");
@@ -360,6 +454,8 @@ static void tb_handle_hotplug(struct work_struct *work)
tb_scan_port(port);
if (!port->remote)
tb_port_info(port, "hotplug: no switch found\n");
+ } else if (tb_port_is_dpout(port)) {
+ tb_tunnel_dp(tb, port);
}
}

@@ -379,7 +475,6 @@ static void tb_handle_event(struct tb *tb, enum tb_cfg_pkg_type type,
const void *buf, size_t size)
{
const struct cfg_event_pkg *pkg = buf;
- struct tb_hotplug_event *ev;
u64 route;

if (type != TB_CFG_PKG_EVENT) {
@@ -395,15 +490,7 @@ static void tb_handle_event(struct tb *tb, enum tb_cfg_pkg_type type,
pkg->port);
}

- ev = kmalloc(sizeof(*ev), GFP_KERNEL);
- if (!ev)
- return;
- INIT_WORK(&ev->work, tb_handle_hotplug);
- ev->tb = tb;
- ev->route = route;
- ev->port = pkg->port;
- ev->unplug = pkg->unplug;
- queue_work(tb->wq, &ev->work);
+ tb_queue_hotplug(tb, route, pkg->port, pkg->unplug);
}

static void tb_stop(struct tb *tb)
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index d802d8c5a953..dd7808f9de39 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -335,6 +335,16 @@ static inline bool tb_port_is_pcie_up(const struct tb_port *port)
return port && port->config.type == TB_TYPE_PCIE_UP;
}

+static inline bool tb_port_is_dpin(const struct tb_port *port)
+{
+ return port && port->config.type == TB_TYPE_DP_HDMI_IN;
+}
+
+static inline bool tb_port_is_dpout(const struct tb_port *port)
+{
+ return port && port->config.type == TB_TYPE_DP_HDMI_OUT;
+}
+
static inline int tb_sw_read(struct tb_switch *sw, void *buffer,
enum tb_cfg_space space, u32 offset, u32 length)
{
@@ -551,6 +561,13 @@ bool tb_port_is_enabled(struct tb_port *port);
bool tb_pci_port_is_enabled(struct tb_port *port);
int tb_pci_port_enable(struct tb_port *port, bool enable);

+int tb_dp_port_hpd_is_active(struct tb_port *port);
+int tb_dp_port_hpd_clear(struct tb_port *port);
+int tb_dp_port_set_hops(struct tb_port *port, unsigned int video,
+ unsigned int aux_tx, unsigned int aux_rx);
+bool tb_dp_port_is_enabled(struct tb_port *port);
+int tb_dp_port_enable(struct tb_port *port, bool enable);
+
struct tb_path *tb_path_discover(struct tb_port *src, int src_hopid,
struct tb_port *dst, int dst_hopid,
struct tb_port **last, const char *name);
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index 74c0f4a5606d..3ce705184e2c 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -213,6 +213,28 @@ struct tb_regs_port_header {

/* DWORD 4 */
#define TB_PORT_NFC_CREDITS_MASK GENMASK(19, 0)
+#define TB_PORT_MAX_CREDITS_SHIFT 20
+#define TB_PORT_MAX_CREDITS_MASK GENMASK(26, 20)
+
+/* Display Port adapter registers */
+
+/* DWORD 0 */
+#define TB_DP_VIDEO_HOPID_SHIFT 16
+#define TB_DP_VIDEO_HOPID_MASK GENMASK(26, 16)
+#define TB_DP_AUX_EN BIT(30)
+#define TB_DP_VIDEO_EN BIT(31)
+/* DWORD 1 */
+#define TB_DP_AUX_TX_HOPID_MASK GENMASK(10, 0)
+#define TB_DP_AUX_RX_HOPID_SHIFT 11
+#define TB_DP_AUX_RX_HOPID_MASK GENMASK(21, 11)
+/* DWORD 2 */
+#define TB_DP_HDP BIT(6)
+/* DWORD 3 */
+#define TB_DP_HPDC BIT(9)
+/* DWORD 4 */
+#define TB_DP_LOCAL_CAP 0x4
+/* DWORD 5 */
+#define TB_DP_REMOTE_CAP 0x5

/* PCIe adapter registers */

diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index 71c712300326..21d3393c6e9c 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -18,14 +18,26 @@
#define TB_PCI_PATH_DOWN 0
#define TB_PCI_PATH_UP 1

+/* DP adapters use HopID 8 for AUX and 9 for Video */
+#define TB_DP_AUX_TX_HOPID 8
+#define TB_DP_AUX_RX_HOPID 8
+#define TB_DP_VIDEO_HOPID 9
+
+#define TB_DP_VIDEO_PATH_OUT 0
+#define TB_DP_AUX_PATH_OUT 1
+#define TB_DP_AUX_PATH_IN 2
+
+static const char * const tb_tunnel_names[] = { "PCI", "DP" };
+
#define __TB_TUNNEL_PRINT(level, tunnel, fmt, arg...) \
do { \
struct tb_tunnel *__tunnel = (tunnel); \
- level(__tunnel->tb, "%llx:%x <-> %llx:%x (PCI): " fmt, \
+ level(__tunnel->tb, "%llx:%x <-> %llx:%x (%s): " fmt, \
tb_route(__tunnel->src_port->sw), \
__tunnel->src_port->port, \
tb_route(__tunnel->dst_port->sw), \
__tunnel->dst_port->port, \
+ tb_tunnel_names[__tunnel->type], \
## arg); \
} while (0)

@@ -38,7 +50,8 @@
#define tb_tunnel_dbg(tunnel, fmt, arg...) \
__TB_TUNNEL_PRINT(tb_dbg, tunnel, fmt, ##arg)

-static struct tb_tunnel *tb_tunnel_alloc(struct tb *tb, size_t npaths)
+static struct tb_tunnel *tb_tunnel_alloc(struct tb *tb, size_t npaths,
+ enum tb_tunnel_type type)
{
struct tb_tunnel *tunnel;

@@ -55,6 +68,7 @@ static struct tb_tunnel *tb_tunnel_alloc(struct tb *tb, size_t npaths)
INIT_LIST_HEAD(&tunnel->list);
tunnel->tb = tb;
tunnel->npaths = npaths;
+ tunnel->type = type;

return tunnel;
}
@@ -104,7 +118,7 @@ struct tb_tunnel *tb_tunnel_discover_pci(struct tb *tb, struct tb_port *down)
if (!tb_pci_port_is_enabled(down))
return NULL;

- tunnel = tb_tunnel_alloc(tb, 2);
+ tunnel = tb_tunnel_alloc(tb, 2, TB_TUNNEL_PCI);
if (!tunnel)
return NULL;

@@ -179,7 +193,7 @@ struct tb_tunnel *tb_tunnel_alloc_pci(struct tb *tb, struct tb_port *up,
struct tb_tunnel *tunnel;
struct tb_path *path;

- tunnel = tb_tunnel_alloc(tb, 2);
+ tunnel = tb_tunnel_alloc(tb, 2, TB_TUNNEL_PCI);
if (!tunnel)
return NULL;

@@ -208,6 +222,255 @@ struct tb_tunnel *tb_tunnel_alloc_pci(struct tb *tb, struct tb_port *up,
return tunnel;
}

+static int tb_dp_xchg_caps(struct tb_tunnel *tunnel)
+{
+ struct tb_port *out = tunnel->dst_port;
+ struct tb_port *in = tunnel->src_port;
+ u32 in_dp_cap, out_dp_cap;
+ int ret;
+
+ /*
+ * Copy DP_LOCAL_CAP register to DP_REMOTE_CAP register for
+ * newer generation hardware.
+ */
+ if (in->sw->generation < 2 || out->sw->generation < 2)
+ return 0;
+
+ /* Read both DP_LOCAL_CAP registers */
+ ret = tb_port_read(in, &in_dp_cap, TB_CFG_PORT,
+ in->cap_adap + TB_DP_LOCAL_CAP, 1);
+ if (ret)
+ return ret;
+
+ ret = tb_port_read(out, &out_dp_cap, TB_CFG_PORT,
+ out->cap_adap + TB_DP_LOCAL_CAP, 1);
+ if (ret)
+ return ret;
+
+ /* Write IN local caps to OUT remote caps */
+ ret = tb_port_write(out, &in_dp_cap, TB_CFG_PORT,
+ out->cap_adap + TB_DP_REMOTE_CAP, 1);
+ if (ret)
+ return ret;
+
+ return tb_port_write(in, &out_dp_cap, TB_CFG_PORT,
+ in->cap_adap + TB_DP_REMOTE_CAP, 1);
+}
+
+static int tb_dp_activate(struct tb_tunnel *tunnel, bool active)
+{
+ int ret;
+
+ if (active) {
+ struct tb_path **paths;
+ int last;
+
+ paths = tunnel->paths;
+ last = paths[TB_DP_VIDEO_PATH_OUT]->path_length - 1;
+
+ tb_dp_port_set_hops(tunnel->src_port,
+ paths[TB_DP_VIDEO_PATH_OUT]->hops[0].in_hop_index,
+ paths[TB_DP_AUX_PATH_OUT]->hops[0].in_hop_index,
+ paths[TB_DP_AUX_PATH_IN]->hops[last].next_hop_index);
+
+ tb_dp_port_set_hops(tunnel->dst_port,
+ paths[TB_DP_VIDEO_PATH_OUT]->hops[last].next_hop_index,
+ paths[TB_DP_AUX_PATH_IN]->hops[0].in_hop_index,
+ paths[TB_DP_AUX_PATH_OUT]->hops[last].next_hop_index);
+ } else {
+ tb_dp_port_hpd_clear(tunnel->src_port);
+ tb_dp_port_set_hops(tunnel->src_port, 0, 0, 0);
+ if (tb_port_is_dpout(tunnel->dst_port))
+ tb_dp_port_set_hops(tunnel->dst_port, 0, 0, 0);
+ }
+
+ ret = tb_dp_port_enable(tunnel->src_port, active);
+ if (ret)
+ return ret;
+
+ if (tb_port_is_dpout(tunnel->dst_port))
+ return tb_dp_port_enable(tunnel->dst_port, active);
+
+ return 0;
+}
+
+static void tb_dp_init_aux_path(struct tb_path *path)
+{
+ int i;
+
+ path->egress_fc_enable = TB_PATH_SOURCE | TB_PATH_INTERNAL;
+ path->egress_shared_buffer = TB_PATH_NONE;
+ path->ingress_fc_enable = TB_PATH_ALL;
+ path->ingress_shared_buffer = TB_PATH_NONE;
+ path->priority = 2;
+ path->weight = 1;
+
+ for (i = 0; i < path->path_length; i++)
+ path->hops[i].initial_credits = 1;
+}
+
+static void tb_dp_init_video_path(struct tb_path *path, bool discover)
+{
+ u32 nfc_credits = path->hops[0].in_port->config.nfc_credits;
+
+ path->egress_fc_enable = TB_PATH_NONE;
+ path->egress_shared_buffer = TB_PATH_NONE;
+ path->ingress_fc_enable = TB_PATH_NONE;
+ path->ingress_shared_buffer = TB_PATH_NONE;
+ path->priority = 1;
+ path->weight = 1;
+
+ if (discover) {
+ path->nfc_credits = nfc_credits & TB_PORT_NFC_CREDITS_MASK;
+ } else {
+ u32 max_credits;
+
+ max_credits = (nfc_credits & TB_PORT_MAX_CREDITS_MASK) >>
+ TB_PORT_MAX_CREDITS_SHIFT;
+ /* Leave some credits for AUX path */
+ path->nfc_credits = min(max_credits - 2, 12U);
+ }
+}
+
+/**
+ * tb_tunnel_discover_dp() - Discover existing Display Port tunnels
+ * @tb: Pointer to the domain structure
+ * @in: DP in adapter
+ *
+ * If @in adapter is active, follows the tunnel to the DP out adapter
+ * and back. Returns the discovered tunnel or %NULL if there was no
+ * tunnel.
+ *
+ * Return: DP tunnel or %NULL if no tunnel found.
+ */
+struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in)
+{
+ struct tb_tunnel *tunnel;
+ struct tb_port *port;
+ struct tb_path *path;
+
+ if (!tb_dp_port_is_enabled(in))
+ return NULL;
+
+ tunnel = tb_tunnel_alloc(tb, 3, TB_TUNNEL_DP);
+ if (!tunnel)
+ return NULL;
+
+ tunnel->init = tb_dp_xchg_caps;
+ tunnel->activate = tb_dp_activate;
+ tunnel->src_port = in;
+
+ path = tb_path_discover(in, TB_DP_VIDEO_HOPID, NULL, -1,
+ &tunnel->dst_port, "Video");
+ if (!path) {
+ /* Just disable the DP IN port */
+ tb_dp_port_enable(in, false);
+ goto err_free;
+ }
+ tunnel->paths[TB_DP_VIDEO_PATH_OUT] = path;
+ tb_dp_init_video_path(tunnel->paths[TB_DP_VIDEO_PATH_OUT], true);
+
+ path = tb_path_discover(in, TB_DP_AUX_TX_HOPID, NULL, -1, NULL, "AUX TX");
+ if (!path)
+ goto err_deactivate;
+ tunnel->paths[TB_DP_AUX_PATH_OUT] = path;
+ tb_dp_init_aux_path(tunnel->paths[TB_DP_AUX_PATH_OUT]);
+
+ path = tb_path_discover(tunnel->dst_port, -1, in, TB_DP_AUX_RX_HOPID,
+ &port, "AUX RX");
+ if (!path)
+ goto err_deactivate;
+ tunnel->paths[TB_DP_AUX_PATH_IN] = path;
+ tb_dp_init_aux_path(tunnel->paths[TB_DP_AUX_PATH_IN]);
+
+ /* Validate that the tunnel is complete */
+ if (!tb_port_is_dpout(tunnel->dst_port)) {
+ tb_port_warn(in, "path does not end on a DP adapter, cleaning up\n");
+ goto err_deactivate;
+ }
+
+ if (!tb_dp_port_is_enabled(tunnel->dst_port))
+ goto err_deactivate;
+
+ if (!tb_dp_port_hpd_is_active(tunnel->dst_port))
+ goto err_deactivate;
+
+ if (port != tunnel->src_port) {
+ tb_tunnel_warn(tunnel, "path is not complete, cleaning up\n");
+ goto err_deactivate;
+ }
+
+ tb_tunnel_dbg(tunnel, "discovered\n");
+ return tunnel;
+
+err_deactivate:
+ tb_tunnel_deactivate(tunnel);
+err_free:
+ tb_tunnel_free(tunnel);
+
+ return NULL;
+}
+
+/**
+ * tb_tunnel_alloc_dp() - allocate a Display Port tunnel
+ * @tb: Pointer to the domain structure
+ * @in: DP in adapter port
+ * @out: DP out adapter port
+ *
+ * Allocates a tunnel between @in and @out that is capable of tunneling
+ * Display Port traffic.
+ *
+ * Return: Returns a tb_tunnel on success or NULL on failure.
+ */
+struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,
+ struct tb_port *out)
+{
+ struct tb_tunnel *tunnel;
+ struct tb_path **paths;
+ struct tb_path *path;
+
+ if (WARN_ON(!in->cap_adap || !out->cap_adap))
+ return NULL;
+
+ tunnel = tb_tunnel_alloc(tb, 3, TB_TUNNEL_DP);
+ if (!tunnel)
+ return NULL;
+
+ tunnel->init = tb_dp_xchg_caps;
+ tunnel->activate = tb_dp_activate;
+ tunnel->src_port = in;
+ tunnel->dst_port = out;
+
+ paths = tunnel->paths;
+
+ path = tb_path_alloc(tb, in, TB_DP_VIDEO_HOPID, out, TB_DP_VIDEO_HOPID,
+ 1, "Video");
+ if (!path)
+ goto err_free;
+ tb_dp_init_video_path(path, false);
+ paths[TB_DP_VIDEO_PATH_OUT] = path;
+
+ path = tb_path_alloc(tb, in, TB_DP_AUX_TX_HOPID, out,
+ TB_DP_AUX_TX_HOPID, 1, "AUX TX");
+ if (!path)
+ goto err_free;
+ tb_dp_init_aux_path(path);
+ paths[TB_DP_AUX_PATH_OUT] = path;
+
+ path = tb_path_alloc(tb, out, TB_DP_AUX_RX_HOPID, in,
+ TB_DP_AUX_RX_HOPID, 1, "AUX RX");
+ if (!path)
+ goto err_free;
+ tb_dp_init_aux_path(path);
+ paths[TB_DP_AUX_PATH_IN] = path;
+
+ return tunnel;
+
+err_free:
+ tb_tunnel_free(tunnel);
+ return NULL;
+}
+
/**
* tb_tunnel_free() - free a tunnel
* @tunnel: Tunnel to be freed
@@ -278,6 +541,12 @@ int tb_tunnel_restart(struct tb_tunnel *tunnel)
}
}

+ if (tunnel->init) {
+ res = tunnel->init(tunnel);
+ if (res)
+ return res;
+ }
+
for (i = 0; i < tunnel->npaths; i++) {
res = tb_path_activate(tunnel->paths[i]);
if (res)
diff --git a/drivers/thunderbolt/tunnel.h b/drivers/thunderbolt/tunnel.h
index 07bf587bed80..0373779f43ba 100644
--- a/drivers/thunderbolt/tunnel.h
+++ b/drivers/thunderbolt/tunnel.h
@@ -11,6 +11,11 @@

#include "tb.h"

+enum tb_tunnel_type {
+ TB_TUNNEL_PCI,
+ TB_TUNNEL_DP,
+};
+
/**
* struct tb_tunnel - Tunnel between two ports
* @tb: Pointer to the domain
@@ -19,8 +24,10 @@
* tunnels may be %NULL or null adapter port instead.
* @paths: All paths required by the tunnel
* @npaths: Number of paths in @paths
+ * @init: Optional tunnel specific initialization
* @activate: Optional tunnel specific activation/deactivation
* @list: Tunnels are linked using this field
+ * @type: Type of the tunnel
*/
struct tb_tunnel {
struct tb *tb;
@@ -28,18 +35,34 @@ struct tb_tunnel {
struct tb_port *dst_port;
struct tb_path **paths;
size_t npaths;
+ int (*init)(struct tb_tunnel *tunnel);
int (*activate)(struct tb_tunnel *tunnel, bool activate);
struct list_head list;
+ enum tb_tunnel_type type;
};

struct tb_tunnel *tb_tunnel_discover_pci(struct tb *tb, struct tb_port *down);
struct tb_tunnel *tb_tunnel_alloc_pci(struct tb *tb, struct tb_port *up,
struct tb_port *down);
+struct tb_tunnel *tb_tunnel_discover_dp(struct tb *tb, struct tb_port *in);
+struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,
+ struct tb_port *out);
+
void tb_tunnel_free(struct tb_tunnel *tunnel);
int tb_tunnel_activate(struct tb_tunnel *tunnel);
int tb_tunnel_restart(struct tb_tunnel *tunnel);
void tb_tunnel_deactivate(struct tb_tunnel *tunnel);
bool tb_tunnel_is_invalid(struct tb_tunnel *tunnel);

+static inline bool tb_tunnel_is_pci(const struct tb_tunnel *tunnel)
+{
+ return tunnel->type == TB_TUNNEL_PCI;
+}
+
+static inline bool tb_tunnel_is_dp(const struct tb_tunnel *tunnel)
+{
+ return tunnel->type == TB_TUNNEL_DP;
+}
+
#endif

--
2.20.1

2019-03-28 12:41:04

by Mika Westerberg

Subject: [PATCH v3 27/36] thunderbolt: Do not tear down tunnels when driver is unloaded

Now that we have capability to discover existing tunnels during driver
load there is no point tearing down tunnels when the driver gets
unloaded. Instead we can just leave them running. If user disconnects
devices while there is no Thunderbolt driver loaded, tunneled protocol
hotplug happens and is handled by the corresponding driver (pciehp in
case of PCIe tunnel, GFX driver in case of DP tunnel).

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/path.c | 10 ++++------
drivers/thunderbolt/tb.c | 4 +---
drivers/thunderbolt/tunnel.c | 10 +---------
3 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/drivers/thunderbolt/path.c b/drivers/thunderbolt/path.c
index daef74548197..9df79548a941 100644
--- a/drivers/thunderbolt/path.c
+++ b/drivers/thunderbolt/path.c
@@ -304,17 +304,15 @@ struct tb_path *tb_path_alloc(struct tb *tb, struct tb_port *src, int src_hopid,
}

/**
- * tb_path_free() - free a deactivated path
+ * tb_path_free() - free a path
+ * @path: Path to free
+ *
+ * Frees a path. The path does not need to be deactivated.
*/
void tb_path_free(struct tb_path *path)
{
int i;

- if (path->activated) {
- tb_WARN(path->tb, "trying to free an activated path\n")
- return;
- }
-
for (i = 0; i < path->path_length; i++) {
const struct tb_path_hop *hop = &path->hops[i];

diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index c5e96e7ac37a..8a97a4e19638 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -500,10 +500,8 @@ static void tb_stop(struct tb *tb)
struct tb_tunnel *n;

/* tunnels are only present after everything has been initialized */
- list_for_each_entry_safe(tunnel, n, &tcm->tunnel_list, list) {
- tb_tunnel_deactivate(tunnel);
+ list_for_each_entry_safe(tunnel, n, &tcm->tunnel_list, list)
tb_tunnel_free(tunnel);
- }
tb_switch_remove(tb->root_switch);
tcm->hotplug_active = false; /* signal tb_handle_hotplug to quit */
}
diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index 21d3393c6e9c..0bc6639c6e74 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -475,7 +475,7 @@ struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,
* tb_tunnel_free() - free a tunnel
* @tunnel: Tunnel to be freed
*
- * The tunnel must have been deactivated.
+ * Frees a tunnel. The tunnel does not need to be deactivated.
*/
void tb_tunnel_free(struct tb_tunnel *tunnel)
{
@@ -484,14 +484,6 @@ void tb_tunnel_free(struct tb_tunnel *tunnel)
if (!tunnel)
return;

- for (i = 0; i < tunnel->npaths; i++) {
- if (tunnel->paths[i] && tunnel->paths[i]->activated) {
- tb_tunnel_WARN(tunnel,
- "trying to free an activated tunnel\n");
- return;
- }
- }
-
for (i = 0; i < tunnel->npaths; i++) {
if (tunnel->paths[i])
tb_path_free(tunnel->paths[i]);
--
2.20.1

2019-03-28 12:41:09

by Mika Westerberg

Subject: [PATCH v3 17/36] thunderbolt: Assign remote for both ports in case of dual link

Currently the driver only assigns remote port for the primary port if in
case of dual link. This makes things such as walking from one port to
another more complex than necessary because the code needs to change
from secondary to primary port if the path that is established is
created using secondary links.

In order to always assign both remote pointers we need to prevent the
scanning code from following the secondary link. Failing to do that
might cause problems as the same switch may be enumerated twice (or
removed in case of unplug). Handle that properly by introducing a new
function tb_port_has_remote() that returns true only for the primary
port. We also update tb_is_upstream_port() to support both dual link
ports, make it take const port pointer and move it below
tb_upstream_port() to keep similar functions close.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/icm.c | 33 ++++++++++---------------------
drivers/thunderbolt/switch.c | 21 ++++++++++----------
drivers/thunderbolt/tb.c | 30 +++++++++++++++++++---------
drivers/thunderbolt/tb.h | 37 ++++++++++++++++++++++++++++++-----
drivers/thunderbolt/xdomain.c | 5 +----
5 files changed, 74 insertions(+), 52 deletions(-)

diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
index bec360eef6cf..805958e53c58 100644
--- a/drivers/thunderbolt/icm.c
+++ b/drivers/thunderbolt/icm.c
@@ -1761,16 +1761,10 @@ static void icm_unplug_children(struct tb_switch *sw)
for (i = 1; i <= sw->config.max_port_number; i++) {
struct tb_port *port = &sw->ports[i];

- if (tb_is_upstream_port(port))
- continue;
- if (port->xdomain) {
+ if (port->xdomain)
port->xdomain->is_unplugged = true;
- continue;
- }
- if (!port->remote)
- continue;
-
- icm_unplug_children(port->remote->sw);
+ else if (tb_port_has_remote(port))
+ icm_unplug_children(port->remote->sw);
}
}

@@ -1781,23 +1775,16 @@ static void icm_free_unplugged_children(struct tb_switch *sw)
for (i = 1; i <= sw->config.max_port_number; i++) {
struct tb_port *port = &sw->ports[i];

- if (tb_is_upstream_port(port))
- continue;
-
if (port->xdomain && port->xdomain->is_unplugged) {
tb_xdomain_remove(port->xdomain);
port->xdomain = NULL;
- continue;
- }
-
- if (!port->remote)
- continue;
-
- if (port->remote->sw->is_unplugged) {
- tb_switch_remove(port->remote->sw);
- port->remote = NULL;
- } else {
- icm_free_unplugged_children(port->remote->sw);
+ } else if (tb_port_has_remote(port)) {
+ if (port->remote->sw->is_unplugged) {
+ tb_switch_remove(port->remote->sw);
+ port->remote = NULL;
+ } else {
+ icm_free_unplugged_children(port->remote->sw);
+ }
}
}
}
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 30f3d6a9fe90..6f98b3d6eb2a 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -1580,14 +1580,13 @@ void tb_switch_remove(struct tb_switch *sw)

/* port 0 is the switch itself and never has a remote */
for (i = 1; i <= sw->config.max_port_number; i++) {
- if (tb_is_upstream_port(&sw->ports[i]))
- continue;
- if (sw->ports[i].remote)
+ if (tb_port_has_remote(&sw->ports[i])) {
tb_switch_remove(sw->ports[i].remote->sw);
- sw->ports[i].remote = NULL;
- if (sw->ports[i].xdomain)
+ sw->ports[i].remote = NULL;
+ } else if (sw->ports[i].xdomain) {
tb_xdomain_remove(sw->ports[i].xdomain);
- sw->ports[i].xdomain = NULL;
+ sw->ports[i].xdomain = NULL;
+ }
}

if (!sw->is_unplugged)
@@ -1617,7 +1616,7 @@ void tb_sw_set_unplugged(struct tb_switch *sw)
}
sw->is_unplugged = true;
for (i = 0; i <= sw->config.max_port_number; i++) {
- if (!tb_is_upstream_port(&sw->ports[i]) && sw->ports[i].remote)
+ if (tb_port_has_remote(&sw->ports[i]))
tb_sw_set_unplugged(sw->ports[i].remote->sw);
}
}
@@ -1663,10 +1662,10 @@ int tb_switch_resume(struct tb_switch *sw)
/* check for surviving downstream switches */
for (i = 1; i <= sw->config.max_port_number; i++) {
struct tb_port *port = &sw->ports[i];
- if (tb_is_upstream_port(port))
- continue;
- if (!port->remote)
+
+ if (!tb_port_has_remote(port))
continue;
+
if (tb_wait_for_port(port, true) <= 0
|| tb_switch_resume(port->remote->sw)) {
tb_port_warn(port,
@@ -1685,7 +1684,7 @@ void tb_switch_suspend(struct tb_switch *sw)
return;

for (i = 1; i <= sw->config.max_port_number; i++) {
- if (!tb_is_upstream_port(&sw->ports[i]) && sw->ports[i].remote)
+ if (tb_port_has_remote(&sw->ports[i]))
tb_switch_suspend(sw->ports[i].remote->sw);
}

diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 36dad0a00ac2..0485f4ef9a62 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -47,7 +47,9 @@ static void tb_scan_switch(struct tb_switch *sw)
*/
static void tb_scan_port(struct tb_port *port)
{
+ struct tb_port *upstream_port;
struct tb_switch *sw;
+
if (tb_is_upstream_port(port))
return;
if (port->config.type != TB_TYPE_PORT)
@@ -80,8 +82,15 @@ static void tb_scan_port(struct tb_port *port)
return;
}

- port->remote = tb_upstream_port(sw);
- tb_upstream_port(sw)->remote = port;
+ /* Link the switches using both links if available */
+ upstream_port = tb_upstream_port(sw);
+ port->remote = upstream_port;
+ upstream_port->remote = port;
+ if (port->dual_link_port && upstream_port->dual_link_port) {
+ port->dual_link_port->remote = upstream_port->dual_link_port;
+ upstream_port->dual_link_port->remote = port->dual_link_port;
+ }
+
tb_scan_switch(sw);
}

@@ -111,13 +120,15 @@ static void tb_free_unplugged_children(struct tb_switch *sw)
int i;
for (i = 1; i <= sw->config.max_port_number; i++) {
struct tb_port *port = &sw->ports[i];
- if (tb_is_upstream_port(port))
- continue;
- if (!port->remote)
+
+ if (!tb_port_has_remote(port))
continue;
+
if (port->remote->sw->is_unplugged) {
tb_switch_remove(port->remote->sw);
port->remote = NULL;
+ if (port->dual_link_port)
+ port->dual_link_port->remote = NULL;
} else {
tb_free_unplugged_children(port->remote->sw);
}
@@ -273,18 +284,19 @@ static void tb_handle_hotplug(struct work_struct *work)
}
port = &sw->ports[ev->port];
if (tb_is_upstream_port(port)) {
- tb_warn(tb,
- "hotplug event for upstream port %llx:%x (unplug: %d)\n",
- ev->route, ev->port, ev->unplug);
+ tb_dbg(tb, "hotplug event for upstream port %llx:%x (unplug: %d)\n",
+ ev->route, ev->port, ev->unplug);
goto put_sw;
}
if (ev->unplug) {
- if (port->remote) {
+ if (tb_port_has_remote(port)) {
tb_port_info(port, "unplugged\n");
tb_sw_set_unplugged(port->remote->sw);
tb_free_invalid_tunnels(tb);
tb_switch_remove(port->remote->sw);
port->remote = NULL;
+ if (port->dual_link_port)
+ port->dual_link_port->remote = NULL;
} else {
tb_port_info(port,
"got unplug event for disconnected port, ignoring\n");
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 1cb5009195f9..5e317ca87786 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -270,6 +270,19 @@ static inline struct tb_port *tb_upstream_port(struct tb_switch *sw)
return &sw->ports[sw->config.upstream_port_number];
}

+/**
+ * tb_is_upstream_port() - Is the port upstream facing
+ * @port: Port to check
+ *
+ * Returns true if @port is upstream facing port. In case of dual link
+ * ports both return true.
+ */
+static inline bool tb_is_upstream_port(const struct tb_port *port)
+{
+ const struct tb_port *upstream_port = tb_upstream_port(port->sw);
+ return port == upstream_port || port->dual_link_port == upstream_port;
+}
+
static inline u64 tb_route(struct tb_switch *sw)
{
return ((u64) sw->config.route_hi) << 32 | sw->config.route_lo;
@@ -285,6 +298,25 @@ static inline struct tb_port *tb_port_at(u64 route, struct tb_switch *sw)
return &sw->ports[port];
}

+/**
+ * tb_port_has_remote() - Does the port have switch connected downstream
+ * @port: Port to check
+ *
+ * Returns true only when the port is primary port and that it has
+ * remote set.
+ */
+static inline bool tb_port_has_remote(const struct tb_port *port)
+{
+ if (tb_is_upstream_port(port))
+ return false;
+ if (!port->remote)
+ return false;
+ if (port->dual_link_port && port->link_nr)
+ return false;
+
+ return true;
+}
+
static inline int tb_sw_read(struct tb_switch *sw, void *buffer,
enum tb_cfg_space space, u32 offset, u32 length)
{
@@ -489,11 +521,6 @@ static inline int tb_route_length(u64 route)
return (fls64(route) + TB_ROUTE_SHIFT - 1) / TB_ROUTE_SHIFT;
}

-static inline bool tb_is_upstream_port(struct tb_port *port)
-{
- return port == tb_upstream_port(port->sw);
-}
-
/**
* tb_downstream_route() - get route to downstream switch
*
diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c
index e2fc4543142d..7bae92bc486f 100644
--- a/drivers/thunderbolt/xdomain.c
+++ b/drivers/thunderbolt/xdomain.c
@@ -1293,9 +1293,6 @@ static struct tb_xdomain *switch_find_xdomain(struct tb_switch *sw,
struct tb_port *port = &sw->ports[i];
struct tb_xdomain *xd;

- if (tb_is_upstream_port(port))
- continue;
-
if (port->xdomain) {
xd = port->xdomain;

@@ -1310,7 +1307,7 @@ static struct tb_xdomain *switch_find_xdomain(struct tb_switch *sw,
lookup->route == xd->route) {
return xd;
}
- } else if (port->remote) {
+ } else if (tb_port_has_remote(port)) {
xd = switch_find_xdomain(port->remote->sw, lookup);
if (xd)
return xd;
--
2.20.1

2019-03-28 12:41:11

by Mika Westerberg

Subject: [PATCH v3 19/36] thunderbolt: Extend tunnel creation to more than 2 adjacent switches

Now that we can allocate hop IDs per port on a path, we can take
advantage of this and create tunnels covering longer paths than just
between two adjacent switches. PCIe actually does not need this as it
is typically a daisy chain between two adjacent switches but this way we
do not need to hard-code creation of the tunnel.

While there add name to struct tb_path to make debugging easier.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/path.c | 121 ++++++++++++++++++++++++++++++-----
drivers/thunderbolt/tb.h | 5 +-
drivers/thunderbolt/tunnel.c | 56 ++++------------
3 files changed, 124 insertions(+), 58 deletions(-)

diff --git a/drivers/thunderbolt/path.c b/drivers/thunderbolt/path.c
index 97f37f86fb75..d3f5df779d8d 100644
--- a/drivers/thunderbolt/path.c
+++ b/drivers/thunderbolt/path.c
@@ -31,23 +31,100 @@ static void tb_dump_hop(struct tb_port *port, struct tb_regs_hop *hop)
}

/**
- * tb_path_alloc() - allocate a thunderbolt path
+ * tb_path_alloc() - allocate a thunderbolt path between two ports
+ * @tb: Domain pointer
+ * @src: Source port of the path
+ * @src_hopid: HopID used for the first ingress port in the path
+ * @dst: Destination port of the path
+ * @dst_hopid: HopID used for the last egress port in the path
+ * @link_nr: Preferred link if there are dual links on the path
+ * @name: Name of the path
+ *
+ * Creates path between two ports starting with given @src_hopid. Reserves
+ * HopIDs for each port (they can be different from @src_hopid depending on
+ * how many HopIDs each port already have reserved). If there are dual
+ * links on the path, prioritizes using @link_nr.
*
* Return: Returns a tb_path on success or NULL on failure.
*/
-struct tb_path *tb_path_alloc(struct tb *tb, int num_hops)
+struct tb_path *tb_path_alloc(struct tb *tb, struct tb_port *src, int src_hopid,
+ struct tb_port *dst, int dst_hopid, int link_nr,
+ const char *name)
{
- struct tb_path *path = kzalloc(sizeof(*path), GFP_KERNEL);
+ struct tb_port *in_port, *out_port;
+ int in_hopid, out_hopid;
+ struct tb_path *path;
+ size_t num_hops;
+ int i, ret;
+
+ path = kzalloc(sizeof(*path), GFP_KERNEL);
if (!path)
return NULL;
+
+ /*
+ * Number of hops on a path is the distance between the two
+ * switches plus the source adapter port.
+ */
+ num_hops = abs(tb_route_length(tb_route(src->sw)) -
+ tb_route_length(tb_route(dst->sw))) + 1;
+
path->hops = kcalloc(num_hops, sizeof(*path->hops), GFP_KERNEL);
if (!path->hops) {
kfree(path);
return NULL;
}
+
+ in_hopid = src_hopid;
+ out_port = NULL;
+
+ for (i = 0; i < num_hops; i++) {
+ in_port = tb_next_port_on_path(src, dst, out_port);
+ if (!in_port)
+ goto err;
+
+ if (in_port->dual_link_port && in_port->link_nr != link_nr)
+ in_port = in_port->dual_link_port;
+
+ ret = tb_port_alloc_in_hopid(in_port, in_hopid, -1);
+ if (ret < 0)
+ goto err;
+ in_hopid = ret;
+
+ out_port = tb_next_port_on_path(src, dst, in_port);
+ if (!out_port)
+ goto err;
+
+ if (out_port->dual_link_port && out_port->link_nr != link_nr)
+ out_port = out_port->dual_link_port;
+
+ if (i == num_hops - 1)
+ ret = tb_port_alloc_out_hopid(out_port, dst_hopid,
+ dst_hopid);
+ else
+ ret = tb_port_alloc_out_hopid(out_port, -1, -1);
+
+ if (ret < 0)
+ goto err;
+ out_hopid = ret;
+
+ path->hops[i].in_hop_index = in_hopid;
+ path->hops[i].in_port = in_port;
+ path->hops[i].in_counter_index = -1;
+ path->hops[i].out_port = out_port;
+ path->hops[i].next_hop_index = out_hopid;
+
+ in_hopid = out_hopid;
+ }
+
path->tb = tb;
path->path_length = num_hops;
+ path->name = name;
+
return path;
+
+err:
+ tb_path_free(path);
+ return NULL;
}

/**
@@ -55,10 +132,24 @@ struct tb_path *tb_path_alloc(struct tb *tb, int num_hops)
*/
void tb_path_free(struct tb_path *path)
{
+ int i;
+
if (path->activated) {
tb_WARN(path->tb, "trying to free an activated path\n")
return;
}
+
+ for (i = 0; i < path->path_length; i++) {
+ const struct tb_path_hop *hop = &path->hops[i];
+
+ if (hop->in_port)
+ tb_port_release_in_hopid(hop->in_port,
+ hop->in_hop_index);
+ if (hop->out_port)
+ tb_port_release_out_hopid(hop->out_port,
+ hop->next_hop_index);
+ }
+
kfree(path->hops);
kfree(path);
}
@@ -136,12 +227,12 @@ void tb_path_deactivate(struct tb_path *path)
tb_WARN(path->tb, "trying to deactivate an inactive path\n");
return;
}
- tb_info(path->tb,
- "deactivating path from %llx:%x to %llx:%x\n",
- tb_route(path->hops[0].in_port->sw),
- path->hops[0].in_port->port,
- tb_route(path->hops[path->path_length - 1].out_port->sw),
- path->hops[path->path_length - 1].out_port->port);
+ tb_dbg(path->tb,
+ "deactivating %s path from %llx:%x to %llx:%x\n",
+ path->name, tb_route(path->hops[0].in_port->sw),
+ path->hops[0].in_port->port,
+ tb_route(path->hops[path->path_length - 1].out_port->sw),
+ path->hops[path->path_length - 1].out_port->port);
__tb_path_deactivate_hops(path, 0);
__tb_path_deallocate_nfc(path, 0);
path->activated = false;
@@ -164,12 +255,12 @@ int tb_path_activate(struct tb_path *path)
return -EINVAL;
}

- tb_info(path->tb,
- "activating path from %llx:%x to %llx:%x\n",
- tb_route(path->hops[0].in_port->sw),
- path->hops[0].in_port->port,
- tb_route(path->hops[path->path_length - 1].out_port->sw),
- path->hops[path->path_length - 1].out_port->port);
+ tb_dbg(path->tb,
+ "activating %s path from %llx:%x to %llx:%x\n",
+ path->name, tb_route(path->hops[0].in_port->sw),
+ path->hops[0].in_port->port,
+ tb_route(path->hops[path->path_length - 1].out_port->sw),
+ path->hops[path->path_length - 1].out_port->port);

/* Clear counters. */
for (i = path->path_length - 1; i >= 0; i--) {
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index c797512dfb05..1c7d67a829d3 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -184,6 +184,7 @@ enum tb_path_port {
*/
struct tb_path {
struct tb *tb;
+ const char *name;
int nfc_credits; /* non flow controlled credits */
enum tb_path_port ingress_shared_buffer;
enum tb_path_port egress_shared_buffer;
@@ -504,7 +505,9 @@ int tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap);

int tb_pci_port_enable(struct tb_port *port, bool enable);

-struct tb_path *tb_path_alloc(struct tb *tb, int num_hops);
+struct tb_path *tb_path_alloc(struct tb *tb, struct tb_port *src, int src_hopid,
+ struct tb_port *dst, int dst_hopid, int link_nr,
+ const char *name);
void tb_path_free(struct tb_path *path);
int tb_path_activate(struct tb_path *path);
void tb_path_deactivate(struct tb_path *path);
diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index 20ce28276f7a..91d7e00516b4 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -12,6 +12,9 @@
#include "tunnel.h"
#include "tb.h"

+/* PCIe adapters use always HopID of 8 for both directions */
+#define TB_PCI_HOPID 8
+
#define TB_PCI_PATH_DOWN 0
#define TB_PCI_PATH_UP 1

@@ -86,21 +89,13 @@ static void tb_pci_init_path(struct tb_path *path)
* Allocate a PCI tunnel. The ports must be of type TB_TYPE_PCIE_UP and
* TB_TYPE_PCIE_DOWN.
*
- * Currently only paths consisting of two hops are supported (that is the
- * ports must be on "adjacent" switches).
- *
- * The paths are hard-coded to use hop 8 (the only working hop id available on
- * my thunderbolt devices). Therefore at most ONE path per device may be
- * activated.
- *
* Return: Returns a tb_tunnel on success or NULL on failure.
*/
struct tb_tunnel *tb_tunnel_alloc_pci(struct tb *tb, struct tb_port *up,
struct tb_port *down)
{
- struct tb_path *path_to_up;
- struct tb_path *path_to_down;
struct tb_tunnel *tunnel;
+ struct tb_path *path;

tunnel = tb_tunnel_alloc(tb, 2);
if (!tunnel)
@@ -110,46 +105,23 @@ struct tb_tunnel *tb_tunnel_alloc_pci(struct tb *tb, struct tb_port *up,
tunnel->src_port = down;
tunnel->dst_port = up;

- path_to_up = tb_path_alloc(tb, 2);
- if (!path_to_up) {
+ path = tb_path_alloc(tb, down, TB_PCI_HOPID, up, TB_PCI_HOPID, 0,
+ "PCIe Down");
+ if (!path) {
tb_tunnel_free(tunnel);
return NULL;
}
- tunnel->paths[TB_PCI_PATH_UP] = path_to_up;
+ tb_pci_init_path(path);
+ tunnel->paths[TB_PCI_PATH_UP] = path;

- path_to_down = tb_path_alloc(tb, 2);
- if (!path_to_down) {
+ path = tb_path_alloc(tb, up, TB_PCI_HOPID, down, TB_PCI_HOPID, 0,
+ "PCIe Up");
+ if (!path) {
tb_tunnel_free(tunnel);
return NULL;
}
- tunnel->paths[TB_PCI_PATH_DOWN] = path_to_down;
-
- tb_pci_init_path(path_to_up);
- tb_pci_init_path(path_to_down);
-
- path_to_up->hops[0].in_port = down;
- path_to_up->hops[0].in_hop_index = 8;
- path_to_up->hops[0].in_counter_index = -1;
- path_to_up->hops[0].out_port = tb_upstream_port(up->sw)->remote;
- path_to_up->hops[0].next_hop_index = 8;
-
- path_to_up->hops[1].in_port = tb_upstream_port(up->sw);
- path_to_up->hops[1].in_hop_index = 8;
- path_to_up->hops[1].in_counter_index = -1;
- path_to_up->hops[1].out_port = up;
- path_to_up->hops[1].next_hop_index = 8;
-
- path_to_down->hops[0].in_port = up;
- path_to_down->hops[0].in_hop_index = 8;
- path_to_down->hops[0].in_counter_index = -1;
- path_to_down->hops[0].out_port = tb_upstream_port(up->sw);
- path_to_down->hops[0].next_hop_index = 8;
-
- path_to_down->hops[1].in_port = tb_upstream_port(up->sw)->remote;
- path_to_down->hops[1].in_hop_index = 8;
- path_to_down->hops[1].in_counter_index = -1;
- path_to_down->hops[1].out_port = down;
- path_to_down->hops[1].next_hop_index = 8;
+ tb_pci_init_path(path);
+ tunnel->paths[TB_PCI_PATH_DOWN] = path;

return tunnel;
}
--
2.20.1

2019-03-28 12:41:30

by Mika Westerberg

Subject: [PATCH v3 15/36] thunderbolt: Generalize tunnel creation functionality

To be able to tunnel non-PCIe traffic, separate tunnel functionality
into generic and PCIe specific parts. Rename struct tb_pci_tunnel to
tb_tunnel, and make it hold an array of paths instead of just two.
Update all the tunneling functions to take this structure as parameter.

We also move tb_pci_port_active() to switch.c (and rename it) where we
will be keeping all port and switch related functions.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/switch.c | 13 ++
drivers/thunderbolt/tb.c | 30 ++--
drivers/thunderbolt/tb.h | 2 +
drivers/thunderbolt/tb_regs.h | 4 +
drivers/thunderbolt/tunnel.c | 298 ++++++++++++++++++++--------------
drivers/thunderbolt/tunnel.h | 38 +++--
6 files changed, 235 insertions(+), 150 deletions(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 9756e6279dc9..b132439618da 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -599,6 +599,19 @@ static int tb_init_port(struct tb_port *port)

}

+/**
+ * tb_pci_port_enable() - Enable PCIe adapter port
+ * @port: PCIe port to enable
+ * @enable: Enable/disable the PCIe adapter
+ */
+int tb_pci_port_enable(struct tb_port *port, bool enable)
+{
+ u32 word = enable ? TB_PCI_EN : 0x0;
+ if (!port->cap_adap)
+ return -ENXIO;
+ return tb_port_write(port, &word, TB_CFG_PORT, port->cap_adap, 1);
+}
+
/* switch utility functions */

static void tb_dump_switch(struct tb *tb, struct tb_regs_switch_header *sw)
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 8de43a2ab205..36dad0a00ac2 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -91,14 +91,14 @@ static void tb_scan_port(struct tb_port *port)
static void tb_free_invalid_tunnels(struct tb *tb)
{
struct tb_cm *tcm = tb_priv(tb);
- struct tb_pci_tunnel *tunnel;
- struct tb_pci_tunnel *n;
+ struct tb_tunnel *tunnel;
+ struct tb_tunnel *n;

list_for_each_entry_safe(tunnel, n, &tcm->tunnel_list, list) {
- if (tb_pci_is_invalid(tunnel)) {
- tb_pci_deactivate(tunnel);
+ if (tb_tunnel_is_invalid(tunnel)) {
+ tb_tunnel_deactivate(tunnel);
list_del(&tunnel->list);
- tb_pci_free(tunnel);
+ tb_tunnel_free(tunnel);
}
}
}
@@ -178,7 +178,7 @@ static void tb_activate_pcie_devices(struct tb *tb)
struct tb_switch *sw;
struct tb_port *up_port;
struct tb_port *down_port;
- struct tb_pci_tunnel *tunnel;
+ struct tb_tunnel *tunnel;
struct tb_cm *tcm = tb_priv(tb);

/* scan for pcie devices at depth 1*/
@@ -214,17 +214,17 @@ static void tb_activate_pcie_devices(struct tb *tb)
"All PCIe down ports are occupied, aborting\n");
continue;
}
- tunnel = tb_pci_alloc(tb, up_port, down_port);
+ tunnel = tb_tunnel_alloc_pci(tb, up_port, down_port);
if (!tunnel) {
tb_port_info(up_port,
"PCIe tunnel allocation failed, aborting\n");
continue;
}

- if (tb_pci_activate(tunnel)) {
+ if (tb_tunnel_activate(tunnel)) {
tb_port_info(up_port,
"PCIe tunnel activation failed, aborting\n");
- tb_pci_free(tunnel);
+ tb_tunnel_free(tunnel);
continue;
}

@@ -353,13 +353,13 @@ static void tb_handle_event(struct tb *tb, enum tb_cfg_pkg_type type,
static void tb_stop(struct tb *tb)
{
struct tb_cm *tcm = tb_priv(tb);
- struct tb_pci_tunnel *tunnel;
- struct tb_pci_tunnel *n;
+ struct tb_tunnel *tunnel;
+ struct tb_tunnel *n;

/* tunnels are only present after everything has been initialized */
list_for_each_entry_safe(tunnel, n, &tcm->tunnel_list, list) {
- tb_pci_deactivate(tunnel);
- tb_pci_free(tunnel);
+ tb_tunnel_deactivate(tunnel);
+ tb_tunnel_free(tunnel);
}
tb_switch_remove(tb->root_switch);
tcm->hotplug_active = false; /* signal tb_handle_hotplug to quit */
@@ -418,7 +418,7 @@ static int tb_suspend_noirq(struct tb *tb)
static int tb_resume_noirq(struct tb *tb)
{
struct tb_cm *tcm = tb_priv(tb);
- struct tb_pci_tunnel *tunnel, *n;
+ struct tb_tunnel *tunnel, *n;

tb_dbg(tb, "resuming...\n");

@@ -429,7 +429,7 @@ static int tb_resume_noirq(struct tb *tb)
tb_free_invalid_tunnels(tb);
tb_free_unplugged_children(tb->root_switch);
list_for_each_entry_safe(tunnel, n, &tcm->tunnel_list, list)
- tb_pci_restart(tunnel);
+ tb_tunnel_restart(tunnel);
if (!list_empty(&tcm->tunnel_list)) {
/*
* the pcie links need some time to get going.
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index b4d7c4d408bd..d1f8e9722f33 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -457,6 +457,8 @@ int tb_port_clear_counter(struct tb_port *port, int counter);
int tb_switch_find_vse_cap(struct tb_switch *sw, enum tb_switch_vse_cap vsec);
int tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap);

+int tb_pci_port_enable(struct tb_port *port, bool enable);
+
struct tb_path *tb_path_alloc(struct tb *tb, int num_hops);
void tb_path_free(struct tb_path *path);
int tb_path_activate(struct tb_path *path);
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index 82ac4ec8757f..75e935acade5 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -211,6 +211,10 @@ struct tb_regs_port_header {

} __packed;

+/* PCIe adapter registers */
+
+#define TB_PCI_EN BIT(31)
+
/* Hop register from TB_CFG_HOPS. 8 byte per entry. */
struct tb_regs_hop {
/* DWORD 0 */
diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index 1e470564e99d..20ce28276f7a 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -1,8 +1,9 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Thunderbolt Cactus Ridge driver - Tunneling support
+ * Thunderbolt driver - Tunneling support
*
* Copyright (c) 2014 Andreas Noever <[email protected]>
+ * Copyright (C) 2019, Intel Corporation
*/

#include <linux/slab.h>
@@ -11,14 +12,17 @@
#include "tunnel.h"
#include "tb.h"

+#define TB_PCI_PATH_DOWN 0
+#define TB_PCI_PATH_UP 1
+
#define __TB_TUNNEL_PRINT(level, tunnel, fmt, arg...) \
do { \
- struct tb_pci_tunnel *__tunnel = (tunnel); \
+ struct tb_tunnel *__tunnel = (tunnel); \
level(__tunnel->tb, "%llx:%x <-> %llx:%x (PCI): " fmt, \
- tb_route(__tunnel->down_port->sw), \
- __tunnel->down_port->port, \
- tb_route(__tunnel->up_port->sw), \
- __tunnel->up_port->port, \
+ tb_route(__tunnel->src_port->sw), \
+ __tunnel->src_port->port, \
+ tb_route(__tunnel->dst_port->sw), \
+ __tunnel->dst_port->port, \
## arg); \
} while (0)

@@ -29,6 +33,38 @@
#define tb_tunnel_info(tunnel, fmt, arg...) \
__TB_TUNNEL_PRINT(tb_info, tunnel, fmt, ##arg)

+static struct tb_tunnel *tb_tunnel_alloc(struct tb *tb, size_t npaths)
+{
+ struct tb_tunnel *tunnel;
+
+ tunnel = kzalloc(sizeof(*tunnel), GFP_KERNEL);
+ if (!tunnel)
+ return NULL;
+
+ tunnel->paths = kcalloc(npaths, sizeof(tunnel->paths[0]), GFP_KERNEL);
+ if (!tunnel->paths) {
+ tb_tunnel_free(tunnel);
+ return NULL;
+ }
+
+ INIT_LIST_HEAD(&tunnel->list);
+ tunnel->tb = tb;
+ tunnel->npaths = npaths;
+
+ return tunnel;
+}
+
+static int tb_pci_activate(struct tb_tunnel *tunnel, bool activate)
+{
+ int res;
+
+ res = tb_pci_port_enable(tunnel->src_port, activate);
+ if (res)
+ return res;
+
+ return tb_pci_port_enable(tunnel->dst_port, activate);
+}
+
static void tb_pci_init_path(struct tb_path *path)
{
path->egress_fc_enable = TB_PATH_SOURCE | TB_PATH_INTERNAL;
@@ -42,7 +78,10 @@ static void tb_pci_init_path(struct tb_path *path)
}

/**
- * tb_pci_alloc() - allocate a pci tunnel
+ * tb_tunnel_alloc_pci() - allocate a pci tunnel
+ * @tb: Pointer to the domain structure
+ * @up: PCIe upstream adapter port
+ * @down: PCIe downstream adapter port
*
* Allocate a PCI tunnel. The ports must be of type TB_TYPE_PCIE_UP and
* TB_TYPE_PCIE_DOWN.
@@ -54,170 +93,185 @@ static void tb_pci_init_path(struct tb_path *path)
* my thunderbolt devices). Therefore at most ONE path per device may be
* activated.
*
- * Return: Returns a tb_pci_tunnel on success or NULL on failure.
+ * Return: Returns a tb_tunnel on success or NULL on failure.
*/
-struct tb_pci_tunnel *tb_pci_alloc(struct tb *tb, struct tb_port *up,
- struct tb_port *down)
+struct tb_tunnel *tb_tunnel_alloc_pci(struct tb *tb, struct tb_port *up,
+ struct tb_port *down)
{
- struct tb_pci_tunnel *tunnel = kzalloc(sizeof(*tunnel), GFP_KERNEL);
+ struct tb_path *path_to_up;
+ struct tb_path *path_to_down;
+ struct tb_tunnel *tunnel;
+
+ tunnel = tb_tunnel_alloc(tb, 2);
if (!tunnel)
- goto err;
- tunnel->tb = tb;
- tunnel->down_port = down;
- tunnel->up_port = up;
- INIT_LIST_HEAD(&tunnel->list);
- tunnel->path_to_up = tb_path_alloc(up->sw->tb, 2);
- if (!tunnel->path_to_up)
- goto err;
- tunnel->path_to_down = tb_path_alloc(up->sw->tb, 2);
- if (!tunnel->path_to_down)
- goto err;
- tb_pci_init_path(tunnel->path_to_up);
- tb_pci_init_path(tunnel->path_to_down);
-
- tunnel->path_to_up->hops[0].in_port = down;
- tunnel->path_to_up->hops[0].in_hop_index = 8;
- tunnel->path_to_up->hops[0].in_counter_index = -1;
- tunnel->path_to_up->hops[0].out_port = tb_upstream_port(up->sw)->remote;
- tunnel->path_to_up->hops[0].next_hop_index = 8;
-
- tunnel->path_to_up->hops[1].in_port = tb_upstream_port(up->sw);
- tunnel->path_to_up->hops[1].in_hop_index = 8;
- tunnel->path_to_up->hops[1].in_counter_index = -1;
- tunnel->path_to_up->hops[1].out_port = up;
- tunnel->path_to_up->hops[1].next_hop_index = 8;
-
- tunnel->path_to_down->hops[0].in_port = up;
- tunnel->path_to_down->hops[0].in_hop_index = 8;
- tunnel->path_to_down->hops[0].in_counter_index = -1;
- tunnel->path_to_down->hops[0].out_port = tb_upstream_port(up->sw);
- tunnel->path_to_down->hops[0].next_hop_index = 8;
-
- tunnel->path_to_down->hops[1].in_port =
- tb_upstream_port(up->sw)->remote;
- tunnel->path_to_down->hops[1].in_hop_index = 8;
- tunnel->path_to_down->hops[1].in_counter_index = -1;
- tunnel->path_to_down->hops[1].out_port = down;
- tunnel->path_to_down->hops[1].next_hop_index = 8;
- return tunnel;
+ return NULL;

-err:
- if (tunnel) {
- if (tunnel->path_to_down)
- tb_path_free(tunnel->path_to_down);
- if (tunnel->path_to_up)
- tb_path_free(tunnel->path_to_up);
- kfree(tunnel);
+ tunnel->activate = tb_pci_activate;
+ tunnel->src_port = down;
+ tunnel->dst_port = up;
+
+ path_to_up = tb_path_alloc(tb, 2);
+ if (!path_to_up) {
+ tb_tunnel_free(tunnel);
+ return NULL;
}
- return NULL;
+ tunnel->paths[TB_PCI_PATH_UP] = path_to_up;
+
+ path_to_down = tb_path_alloc(tb, 2);
+ if (!path_to_down) {
+ tb_tunnel_free(tunnel);
+ return NULL;
+ }
+ tunnel->paths[TB_PCI_PATH_DOWN] = path_to_down;
+
+ tb_pci_init_path(path_to_up);
+ tb_pci_init_path(path_to_down);
+
+ path_to_up->hops[0].in_port = down;
+ path_to_up->hops[0].in_hop_index = 8;
+ path_to_up->hops[0].in_counter_index = -1;
+ path_to_up->hops[0].out_port = tb_upstream_port(up->sw)->remote;
+ path_to_up->hops[0].next_hop_index = 8;
+
+ path_to_up->hops[1].in_port = tb_upstream_port(up->sw);
+ path_to_up->hops[1].in_hop_index = 8;
+ path_to_up->hops[1].in_counter_index = -1;
+ path_to_up->hops[1].out_port = up;
+ path_to_up->hops[1].next_hop_index = 8;
+
+ path_to_down->hops[0].in_port = up;
+ path_to_down->hops[0].in_hop_index = 8;
+ path_to_down->hops[0].in_counter_index = -1;
+ path_to_down->hops[0].out_port = tb_upstream_port(up->sw);
+ path_to_down->hops[0].next_hop_index = 8;
+
+ path_to_down->hops[1].in_port = tb_upstream_port(up->sw)->remote;
+ path_to_down->hops[1].in_hop_index = 8;
+ path_to_down->hops[1].in_counter_index = -1;
+ path_to_down->hops[1].out_port = down;
+ path_to_down->hops[1].next_hop_index = 8;
+
+ return tunnel;
}

/**
- * tb_pci_free() - free a tunnel
+ * tb_tunnel_free() - free a tunnel
+ * @tunnel: Tunnel to be freed
*
* The tunnel must have been deactivated.
*/
-void tb_pci_free(struct tb_pci_tunnel *tunnel)
+void tb_tunnel_free(struct tb_tunnel *tunnel)
{
- if (tunnel->path_to_up->activated || tunnel->path_to_down->activated) {
- tb_tunnel_WARN(tunnel, "trying to free an activated tunnel\n");
+ int i;
+
+ if (!tunnel)
return;
+
+ for (i = 0; i < tunnel->npaths; i++) {
+ if (tunnel->paths[i] && tunnel->paths[i]->activated) {
+ tb_tunnel_WARN(tunnel,
+ "trying to free an activated tunnel\n");
+ return;
+ }
}
- tb_path_free(tunnel->path_to_up);
- tb_path_free(tunnel->path_to_down);
+
+ for (i = 0; i < tunnel->npaths; i++) {
+ if (tunnel->paths[i])
+ tb_path_free(tunnel->paths[i]);
+ }
+
+ kfree(tunnel->paths);
kfree(tunnel);
}

/**
- * tb_pci_is_invalid - check whether an activated path is still valid
+ * tb_tunnel_is_invalid - check whether an activated path is still valid
+ * @tunnel: Tunnel to check
*/
-bool tb_pci_is_invalid(struct tb_pci_tunnel *tunnel)
+bool tb_tunnel_is_invalid(struct tb_tunnel *tunnel)
{
- WARN_ON(!tunnel->path_to_up->activated);
- WARN_ON(!tunnel->path_to_down->activated);
+ int i;

- return tb_path_is_invalid(tunnel->path_to_up)
- || tb_path_is_invalid(tunnel->path_to_down);
-}
+ for (i = 0; i < tunnel->npaths; i++) {
+ WARN_ON(!tunnel->paths[i]->activated);
+ if (tb_path_is_invalid(tunnel->paths[i]))
+ return true;
+ }

-/**
- * tb_pci_port_active() - activate/deactivate PCI capability
- *
- * Return: Returns 0 on success or an error code on failure.
- */
-static int tb_pci_port_active(struct tb_port *port, bool active)
-{
- u32 word = active ? 0x80000000 : 0x0;
- if (!port->cap_adap)
- return -ENXIO;
- return tb_port_write(port, &word, TB_CFG_PORT, port->cap_adap, 1);
+ return false;
}

/**
- * tb_pci_restart() - activate a tunnel after a hardware reset
+ * tb_tunnel_restart() - activate a tunnel after a hardware reset
+ * @tunnel: Tunnel to restart
+ *
+ * Return: 0 on success and negative errno in case if failure
*/
-int tb_pci_restart(struct tb_pci_tunnel *tunnel)
+int tb_tunnel_restart(struct tb_tunnel *tunnel)
{
- int res;
- tunnel->path_to_up->activated = false;
- tunnel->path_to_down->activated = false;
+ int res, i;

tb_tunnel_info(tunnel, "activating\n");

- res = tb_path_activate(tunnel->path_to_up);
- if (res)
- goto err;
- res = tb_path_activate(tunnel->path_to_down);
- if (res)
- goto err;
+ for (i = 0; i < tunnel->npaths; i++) {
+ tunnel->paths[i]->activated = false;
+ res = tb_path_activate(tunnel->paths[i]);
+ if (res)
+ goto err;
+ }

- res = tb_pci_port_active(tunnel->down_port, true);
- if (res)
- goto err;
+ if (tunnel->activate) {
+ res = tunnel->activate(tunnel, true);
+ if (res)
+ goto err;
+ }

- res = tb_pci_port_active(tunnel->up_port, true);
- if (res)
- goto err;
return 0;
+
err:
tb_tunnel_warn(tunnel, "activation failed\n");
- tb_pci_deactivate(tunnel);
+ tb_tunnel_deactivate(tunnel);
return res;
}

/**
- * tb_pci_activate() - activate a tunnel
+ * tb_tunnel_activate() - activate a tunnel
+ * @tunnel: Tunnel to activate
*
* Return: Returns 0 on success or an error code on failure.
*/
-int tb_pci_activate(struct tb_pci_tunnel *tunnel)
+int tb_tunnel_activate(struct tb_tunnel *tunnel)
{
- if (tunnel->path_to_up->activated || tunnel->path_to_down->activated) {
- tb_tunnel_WARN(tunnel,
- "trying to activate an already activated tunnel\n");
- return -EINVAL;
- }
+ int i;

- return tb_pci_restart(tunnel);
-}
+ tb_tunnel_info(tunnel, "activating\n");

+ for (i = 0; i < tunnel->npaths; i++) {
+ if (tunnel->paths[i]->activated) {
+ tb_tunnel_WARN(tunnel,
+ "trying to activate an already activated tunnel\n");
+ return -EINVAL;
+ }
+ }

+ return tb_tunnel_restart(tunnel);
+}

/**
- * tb_pci_deactivate() - deactivate a tunnel
+ * tb_tunnel_deactivate() - deactivate a tunnel
+ * @tunnel: Tunnel to deactivate
*/
-void tb_pci_deactivate(struct tb_pci_tunnel *tunnel)
+void tb_tunnel_deactivate(struct tb_tunnel *tunnel)
{
+ int i;
+
tb_tunnel_info(tunnel, "deactivating\n");
- /*
- * TODO: enable reset by writing 0x04000000 to TB_CAP_PCIE + 1 on up
- * port. Seems to have no effect?
- */
- tb_pci_port_active(tunnel->up_port, false);
- tb_pci_port_active(tunnel->down_port, false);
- if (tunnel->path_to_down->activated)
- tb_path_deactivate(tunnel->path_to_down);
- if (tunnel->path_to_up->activated)
- tb_path_deactivate(tunnel->path_to_up);
-}

+ if (tunnel->activate)
+ tunnel->activate(tunnel, false);
+
+ for (i = 0; i < tunnel->npaths; i++) {
+ if (tunnel->paths[i]->activated)
+ tb_path_deactivate(tunnel->paths[i]);
+ }
+}
diff --git a/drivers/thunderbolt/tunnel.h b/drivers/thunderbolt/tunnel.h
index dff0f27d6ab5..b4e992165e56 100644
--- a/drivers/thunderbolt/tunnel.h
+++ b/drivers/thunderbolt/tunnel.h
@@ -1,8 +1,9 @@
/* SPDX-License-Identifier: GPL-2.0 */
/*
- * Thunderbolt Cactus Ridge driver - Tunneling support
+ * Thunderbolt driver - Tunneling support
*
* Copyright (c) 2014 Andreas Noever <[email protected]>
+ * Copyright (C) 2019, Intel Corporation
*/

#ifndef TB_TUNNEL_H_
@@ -10,22 +11,33 @@

#include "tb.h"

-struct tb_pci_tunnel {
+/**
+ * struct tb_tunnel - Tunnel between two ports
+ * @tb: Pointer to the domain
+ * @src_port: Source port of the tunnel
+ * @dst_port: Destination port of the tunnel
+ * @paths: All paths required by the tunnel
+ * @npaths: Number of paths in @paths
+ * @activate: Optional tunnel specific activation/deactivation
+ * @list: Tunnels are linked using this field
+ */
+struct tb_tunnel {
struct tb *tb;
- struct tb_port *up_port;
- struct tb_port *down_port;
- struct tb_path *path_to_up;
- struct tb_path *path_to_down;
+ struct tb_port *src_port;
+ struct tb_port *dst_port;
+ struct tb_path **paths;
+ size_t npaths;
+ int (*activate)(struct tb_tunnel *tunnel, bool activate);
struct list_head list;
};

-struct tb_pci_tunnel *tb_pci_alloc(struct tb *tb, struct tb_port *up,
- struct tb_port *down);
-void tb_pci_free(struct tb_pci_tunnel *tunnel);
-int tb_pci_activate(struct tb_pci_tunnel *tunnel);
-int tb_pci_restart(struct tb_pci_tunnel *tunnel);
-void tb_pci_deactivate(struct tb_pci_tunnel *tunnel);
-bool tb_pci_is_invalid(struct tb_pci_tunnel *tunnel);
+struct tb_tunnel *tb_tunnel_alloc_pci(struct tb *tb, struct tb_port *up,
+ struct tb_port *down);
+void tb_tunnel_free(struct tb_tunnel *tunnel);
+int tb_tunnel_activate(struct tb_tunnel *tunnel);
+int tb_tunnel_restart(struct tb_tunnel *tunnel);
+void tb_tunnel_deactivate(struct tb_tunnel *tunnel);
+bool tb_tunnel_is_invalid(struct tb_tunnel *tunnel);

#endif

--
2.20.1

2019-03-28 12:41:58

by Mika Westerberg

Subject: [PATCH v3 22/36] thunderbolt: Add support for full PCIe daisy chains

Currently the software connection manager (tb.c) has only supported
creating a single PCIe tunnel, no PCIe device daisy chaining has been
supported so far. This updates the software connection manager so that
it now can create PCIe tunnels for full chain of six devices.

Because PCIe allows DMA and opens possibility for DMA attacks we change
security level to "user" meaning that PCIe tunneling requires that the
userspace authorizes the devices first. This makes it possible to block
PCIe tunneling completely while still allowing other types of tunnels to
be automatically created.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/tb.c | 174 +++++++++++++++++++++++----------------
drivers/thunderbolt/tb.h | 27 ++++++
2 files changed, 129 insertions(+), 72 deletions(-)

diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index a62695a99835..cfc451c938fd 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -1,8 +1,9 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Thunderbolt Cactus Ridge driver - bus logic (NHI independent)
+ * Thunderbolt driver - bus logic (NHI independent)
*
* Copyright (c) 2014 Andreas Noever <[email protected]>
+ * Copyright (C) 2019, Intel Corporation
*/

#include <linux/slab.h>
@@ -84,6 +85,7 @@ static void tb_scan_switch(struct tb_switch *sw)
*/
static void tb_scan_port(struct tb_port *port)
{
+ struct tb_cm *tcm = tb_priv(port->sw->tb);
struct tb_port *upstream_port;
struct tb_switch *sw;

@@ -112,7 +114,13 @@ static void tb_scan_port(struct tb_port *port)
return;
}

- sw->authorized = true;
+ /*
+ * Do not send uevents until we have discovered all existing
+ * tunnels and know which switches were authorized already by
+ * the boot firmware.
+ */
+ if (!tcm->hotplug_active)
+ dev_set_uevent_suppress(&sw->dev, true);

if (tb_switch_add(sw)) {
tb_switch_put(sw);
@@ -212,72 +220,78 @@ static struct tb_port *tb_find_unused_down_port(struct tb_switch *sw)
return NULL;
}

-/**
- * tb_activate_pcie_devices() - scan for and activate PCIe devices
- *
- * This method is somewhat ad hoc. For now it only supports one device
- * per port and only devices at depth 1.
- */
-static void tb_activate_pcie_devices(struct tb *tb)
+static struct tb_port *tb_find_pcie_down(struct tb_switch *sw,
+ const struct tb_port *port)
{
- int i;
- int cap;
- u32 data;
- struct tb_switch *sw;
- struct tb_port *up_port;
- struct tb_port *down_port;
- struct tb_tunnel *tunnel;
- struct tb_cm *tcm = tb_priv(tb);
+ /*
+ * To keep plugging devices consistently in the same PCIe
+ * hierarchy, do mapping here for root switch downstream PCIe
+ * ports.
+ */
+ if (!tb_route(sw)) {
+ int phy_port = tb_phy_port_from_link(port->port);
+ int index;

- /* scan for pcie devices at depth 1*/
- for (i = 1; i <= tb->root_switch->config.max_port_number; i++) {
- if (tb_is_upstream_port(&tb->root_switch->ports[i]))
- continue;
- if (tb->root_switch->ports[i].config.type != TB_TYPE_PORT)
- continue;
- if (!tb->root_switch->ports[i].remote)
- continue;
- sw = tb->root_switch->ports[i].remote->sw;
- up_port = tb_find_pci_up_port(sw);
- if (!up_port) {
- tb_sw_info(sw, "no PCIe devices found, aborting\n");
- continue;
- }
+ /*
+ * Hard-coded Thunderbolt port to PCIe down port mapping
+ * per controller.
+ */
+ if (tb_switch_is_cr(sw))
+ index = !phy_port ? 6 : 7;
+ else if (tb_switch_is_fr(sw))
+ index = !phy_port ? 6 : 8;
+ else
+ goto out;
+
+ /* Validate the hard-coding */
+ if (WARN_ON(index > sw->config.max_port_number))
+ goto out;
+ if (WARN_ON(!tb_port_is_pcie_down(&sw->ports[index])))
+ goto out;
+ if (WARN_ON(tb_pci_port_is_enabled(&sw->ports[index])))
+ goto out;
+
+ return &sw->ports[index];
+ }

- /* check whether port is already activated */
- cap = up_port->cap_adap;
- if (!cap)
- continue;
- if (tb_port_read(up_port, &data, TB_CFG_PORT, cap, 1))
- continue;
- if (data & 0x80000000) {
- tb_port_info(up_port,
- "PCIe port already activated, aborting\n");
- continue;
- }
+out:
+ return tb_find_unused_down_port(sw);
+}

- down_port = tb_find_unused_down_port(tb->root_switch);
- if (!down_port) {
- tb_port_info(up_port,
- "All PCIe down ports are occupied, aborting\n");
- continue;
- }
- tunnel = tb_tunnel_alloc_pci(tb, up_port, down_port);
- if (!tunnel) {
- tb_port_info(up_port,
- "PCIe tunnel allocation failed, aborting\n");
- continue;
- }
+static int tb_tunnel_pci(struct tb *tb, struct tb_switch *sw)
+{
+ struct tb_port *up, *down, *port;
+ struct tb_cm *tcm = tb_priv(tb);
+ struct tb_switch *parent_sw;
+ struct tb_tunnel *tunnel;

- if (tb_tunnel_activate(tunnel)) {
- tb_port_info(up_port,
- "PCIe tunnel activation failed, aborting\n");
- tb_tunnel_free(tunnel);
- continue;
- }
+ up = tb_find_pci_up_port(sw);
+ if (!up)
+ return 0;

- list_add(&tunnel->list, &tcm->tunnel_list);
+ /*
+ * Look up available down port. Since we are chaining it should
+ * be found right above this switch.
+ */
+ parent_sw = tb_to_switch(sw->dev.parent);
+ port = tb_port_at(tb_route(sw), parent_sw);
+ down = tb_find_pcie_down(parent_sw, port);
+ if (!down)
+ return 0;
+
+ tunnel = tb_tunnel_alloc_pci(tb, up, down);
+ if (!tunnel)
+ return -ENOMEM;
+
+ if (tb_tunnel_activate(tunnel)) {
+ tb_port_info(up,
+ "PCIe tunnel activation failed, aborting\n");
+ tb_tunnel_free(tunnel);
+ return -EIO;
}
+
+ list_add_tail(&tunnel->list, &tcm->tunnel_list);
+ return 0;
}

/* hotplug handling */
@@ -344,16 +358,8 @@ static void tb_handle_hotplug(struct work_struct *work)
} else {
tb_port_info(port, "hotplug: scanning\n");
tb_scan_port(port);
- if (!port->remote) {
+ if (!port->remote)
tb_port_info(port, "hotplug: no switch found\n");
- } else if (port->remote->sw->config.depth > 1) {
- tb_sw_warn(port->remote->sw,
- "hotplug: chaining not supported\n");
- } else {
- tb_sw_info(port->remote->sw,
- "hotplug: activating pcie devices\n");
- tb_activate_pcie_devices(tb);
- }
}

put_sw:
@@ -414,6 +420,27 @@ static void tb_stop(struct tb *tb)
tcm->hotplug_active = false; /* signal tb_handle_hotplug to quit */
}

+static int tb_scan_finalize_switch(struct device *dev, void *data)
+{
+ if (tb_is_switch(dev)) {
+ struct tb_switch *sw = tb_to_switch(dev);
+
+ /*
+ * If we found that the switch was already setup by the
+ * boot firmware, mark it as authorized now before we
+ * send uevent to userspace.
+ */
+ if (sw->boot)
+ sw->authorized = 1;
+
+ dev_set_uevent_suppress(dev, false);
+ kobject_uevent(&dev->kobj, KOBJ_ADD);
+ device_for_each_child(dev, NULL, tb_scan_finalize_switch);
+ }
+
+ return 0;
+}
+
static int tb_start(struct tb *tb)
{
struct tb_cm *tcm = tb_priv(tb);
@@ -447,7 +474,9 @@ static int tb_start(struct tb *tb)
tb_scan_switch(tb->root_switch);
/* Find out tunnels created by the boot firmware */
tb_discover_tunnels(tb->root_switch);
- tb_activate_pcie_devices(tb);
+ /* Make the discovered switches available to the userspace */
+ device_for_each_child(&tb->root_switch->dev, NULL,
+ tb_scan_finalize_switch);

/* Allow tb_handle_hotplug to progress events */
tcm->hotplug_active = true;
@@ -502,6 +531,7 @@ static const struct tb_cm_ops tb_cm_ops = {
.suspend_noirq = tb_suspend_noirq,
.resume_noirq = tb_resume_noirq,
.handle_event = tb_handle_event,
+ .approve_switch = tb_tunnel_pci,
};

struct tb *tb_probe(struct tb_nhi *nhi)
@@ -516,7 +546,7 @@ struct tb *tb_probe(struct tb_nhi *nhi)
if (!tb)
return NULL;

- tb->security_level = TB_SECURITY_NONE;
+ tb->security_level = TB_SECURITY_USER;
tb->cm_ops = &tb_cm_ops;

tcm = tb_priv(tb);
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 6ce3832b6c99..d810fb188937 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -320,6 +320,11 @@ static inline bool tb_port_has_remote(const struct tb_port *port)
return true;
}

+static inline bool tb_port_is_pcie_down(const struct tb_port *port)
+{
+ return port && port->config.type == TB_TYPE_PCIE_DOWN;
+}
+
static inline bool tb_port_is_pcie_up(const struct tb_port *port)
{
return port && port->config.type == TB_TYPE_PCIE_UP;
@@ -502,6 +507,28 @@ static inline bool tb_switch_is_er(const struct tb_switch *sw)
return sw->config.device_id == PCI_DEVICE_ID_INTEL_EAGLE_RIDGE;
}

+static inline bool tb_switch_is_cr(const struct tb_switch *sw)
+{
+ switch (sw->config.device_id) {
+ case PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_2C:
+ case PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static inline bool tb_switch_is_fr(const struct tb_switch *sw)
+{
+ switch (sw->config.device_id) {
+ case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_BRIDGE:
+ case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_BRIDGE:
+ return true;
+ default:
+ return false;
+ }
+}
+
int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged);
int tb_port_add_nfc_credits(struct tb_port *port, int credits);
int tb_port_clear_counter(struct tb_port *port, int counter);
--
2.20.1

2019-03-28 12:42:06

by Mika Westerberg

Subject: [PATCH v3 10/36] thunderbolt: Configure lanes when switch is initialized

Thunderbolt 2 devices and beyond need to have additional bits set in
link controller specific registers. This includes two bits in LC_SX_CTRL
that tell the link controller which lane is connected and whether it is
upstream facing or not.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/lc.c | 114 ++++++++++++++++++++++++++++++++++
drivers/thunderbolt/switch.c | 9 +++
drivers/thunderbolt/tb.h | 2 +
drivers/thunderbolt/tb_regs.h | 11 ++++
4 files changed, 136 insertions(+)

diff --git a/drivers/thunderbolt/lc.c b/drivers/thunderbolt/lc.c
index 2134a55ed837..a5dddf176546 100644
--- a/drivers/thunderbolt/lc.c
+++ b/drivers/thunderbolt/lc.c
@@ -19,3 +19,117 @@ int tb_lc_read_uuid(struct tb_switch *sw, u32 *uuid)
return -EINVAL;
return tb_sw_read(sw, uuid, TB_CFG_SWITCH, sw->cap_lc + TB_LC_FUSE, 4);
}
+
+static int read_lc_desc(struct tb_switch *sw, u32 *desc)
+{
+ if (!sw->cap_lc)
+ return -EINVAL;
+ return tb_sw_read(sw, desc, TB_CFG_SWITCH, sw->cap_lc + TB_LC_DESC, 1);
+}
+
+static int find_port_lc_cap(struct tb_port *port)
+{
+ struct tb_switch *sw = port->sw;
+ int start, phys, ret, size;
+ u32 desc;
+
+ ret = read_lc_desc(sw, &desc);
+ if (ret)
+ return ret;
+
+ /* Start of port LC registers */
+ start = (desc & TB_LC_DESC_SIZE_MASK) >> TB_LC_DESC_SIZE_SHIFT;
+ size = (desc & TB_LC_DESC_PORT_SIZE_MASK) >> TB_LC_DESC_PORT_SIZE_SHIFT;
+ phys = tb_phy_port_from_link(port->port);
+
+ return sw->cap_lc + start + phys * size;
+}
+
+static int tb_lc_configure_lane(struct tb_port *port, bool configure)
+{
+ bool upstream = tb_is_upstream_port(port);
+ struct tb_switch *sw = port->sw;
+ u32 ctrl, lane;
+ int cap, ret;
+
+ if (sw->generation < 2)
+ return 0;
+
+ cap = find_port_lc_cap(port);
+ if (cap < 0)
+ return cap;
+
+ ret = tb_sw_read(sw, &ctrl, TB_CFG_SWITCH, cap + TB_LC_SX_CTRL, 1);
+ if (ret)
+ return ret;
+
+ /* Resolve correct lane */
+ if (port->port % 2)
+ lane = TB_LC_SX_CTRL_L1C;
+ else
+ lane = TB_LC_SX_CTRL_L2C;
+
+ if (configure) {
+ ctrl |= lane;
+ if (upstream)
+ ctrl |= TB_LC_SX_CTRL_UPSTREAM;
+ } else {
+ ctrl &= ~lane;
+ if (upstream)
+ ctrl &= ~TB_LC_SX_CTRL_UPSTREAM;
+ }
+
+ return tb_sw_write(sw, &ctrl, TB_CFG_SWITCH, cap + TB_LC_SX_CTRL, 1);
+}
+
+/**
+ * tb_lc_configure_link() - Let LC know about configured link
+ * @sw: Switch that is being added
+ *
+ * Informs LC of both parent switch and @sw that there is established
+ * link between the two.
+ */
+int tb_lc_configure_link(struct tb_switch *sw)
+{
+ struct tb_port *up, *down;
+ int ret;
+
+ if (!sw->config.enabled || !tb_route(sw))
+ return 0;
+
+ up = tb_upstream_port(sw);
+ down = tb_port_at(tb_route(sw), tb_to_switch(sw->dev.parent));
+
+ /* Configure parent link toward this switch */
+ ret = tb_lc_configure_lane(down, true);
+ if (ret)
+ return ret;
+
+ /* Configure upstream link from this switch to the parent */
+ ret = tb_lc_configure_lane(up, true);
+ if (ret)
+ tb_lc_configure_lane(down, false);
+
+ return ret;
+}
+
+/**
+ * tb_lc_unconfigure_link() - Let LC know about unconfigured link
+ * @sw: Switch to unconfigure
+ *
+ * Informs LC of both parent switch and @sw that the link between the
+ * two does not exist anymore.
+ */
+void tb_lc_unconfigure_link(struct tb_switch *sw)
+{
+ struct tb_port *up, *down;
+
+ if (sw->is_unplugged || !sw->config.enabled || !tb_route(sw))
+ return;
+
+ up = tb_upstream_port(sw);
+ down = tb_port_at(tb_route(sw), tb_to_switch(sw->dev.parent));
+
+ tb_lc_configure_lane(up, false);
+ tb_lc_configure_lane(down, false);
+}
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 63ff4c753d89..dd218dc4781b 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -1276,6 +1276,10 @@ int tb_switch_configure(struct tb_switch *sw)
if (ret)
return ret;

+ ret = tb_lc_configure_link(sw);
+ if (ret)
+ return ret;
+
return tb_plug_events_active(sw, true);
}

@@ -1486,6 +1490,7 @@ void tb_switch_remove(struct tb_switch *sw)

if (!sw->is_unplugged)
tb_plug_events_active(sw, false);
+ tb_lc_unconfigure_link(sw);

tb_switch_nvm_remove(sw);

@@ -1545,6 +1550,10 @@ int tb_switch_resume(struct tb_switch *sw)
if (err)
return err;

+ err = tb_lc_configure_link(sw);
+ if (err)
+ return err;
+
err = tb_plug_events_active(sw, true);
if (err)
return err;
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index e52d39b25266..69e0534224d8 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -465,6 +465,8 @@ int tb_drom_read(struct tb_switch *sw);
int tb_drom_read_uid_only(struct tb_switch *sw, u64 *uid);

int tb_lc_read_uuid(struct tb_switch *sw, u32 *uuid);
+int tb_lc_configure_link(struct tb_switch *sw);
+void tb_lc_unconfigure_link(struct tb_switch *sw);

static inline int tb_route_length(u64 route)
{
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index 4895ae9f0b40..e0f867dad5cf 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -238,6 +238,17 @@ struct tb_regs_hop {
} __packed;

/* Common link controller registers */
+#define TB_LC_DESC 0x02
+#define TB_LC_DESC_SIZE_SHIFT 8
+#define TB_LC_DESC_SIZE_MASK GENMASK(15, 8)
+#define TB_LC_DESC_PORT_SIZE_SHIFT 16
+#define TB_LC_DESC_PORT_SIZE_MASK GENMASK(27, 16)
#define TB_LC_FUSE 0x03

+/* Link controller registers */
+#define TB_LC_SX_CTRL 0x96
+#define TB_LC_SX_CTRL_L1C BIT(16)
+#define TB_LC_SX_CTRL_L2C BIT(20)
+#define TB_LC_SX_CTRL_UPSTREAM BIT(30)
+
#endif
--
2.20.1

2019-03-28 12:42:08

by Mika Westerberg

Subject: [PATCH v3 09/36] thunderbolt: Move LC specific functionality into a separate file

We will be adding more link controller functionality in subsequent
patches and it does not make sense to keep all that in switch.c, so
separate LC functionality into its own file.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/Makefile | 2 +-
drivers/thunderbolt/lc.c | 21 +++++++++++++++++++++
drivers/thunderbolt/switch.c | 21 ++++++++++-----------
drivers/thunderbolt/tb.h | 3 +++
drivers/thunderbolt/tb_regs.h | 2 ++
5 files changed, 37 insertions(+), 12 deletions(-)
create mode 100644 drivers/thunderbolt/lc.c

diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
index f2f0de27252b..8531f15d3b3c 100644
--- a/drivers/thunderbolt/Makefile
+++ b/drivers/thunderbolt/Makefile
@@ -1,3 +1,3 @@
obj-${CONFIG_THUNDERBOLT} := thunderbolt.o
thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o tunnel_pci.o eeprom.o
-thunderbolt-objs += domain.o dma_port.o icm.o property.o xdomain.o
+thunderbolt-objs += domain.o dma_port.o icm.o property.o xdomain.o lc.o
diff --git a/drivers/thunderbolt/lc.c b/drivers/thunderbolt/lc.c
new file mode 100644
index 000000000000..2134a55ed837
--- /dev/null
+++ b/drivers/thunderbolt/lc.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Thunderbolt link controller support
+ *
+ * Copyright (C) 2019, Intel Corporation
+ * Author: Mika Westerberg <[email protected]>
+ */
+
+#include "tb.h"
+
+/**
+ * tb_lc_read_uuid() - Read switch UUID from link controller common register
+ * @sw: Switch whose UUID is read
+ * @uuid: UUID is placed here
+ */
+int tb_lc_read_uuid(struct tb_switch *sw, u32 *uuid)
+{
+ if (!sw->cap_lc)
+ return -EINVAL;
+ return tb_sw_read(sw, uuid, TB_CFG_SWITCH, sw->cap_lc + TB_LC_FUSE, 4);
+}
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 1e29c06947af..63ff4c753d89 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -1182,6 +1182,10 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, struct device *parent,
}
sw->cap_plug_events = cap;

+ cap = tb_switch_find_vse_cap(sw, TB_VSE_CAP_LINK_CONTROLLER);
+ if (cap > 0)
+ sw->cap_lc = cap;
+
/* Root switch is always authorized */
if (!route)
sw->authorized = true;
@@ -1278,22 +1282,17 @@ int tb_switch_configure(struct tb_switch *sw)
static int tb_switch_set_uuid(struct tb_switch *sw)
{
u32 uuid[4];
- int cap, ret;
+ int ret;

- ret = 0;
if (sw->uuid)
- return ret;
+ return 0;

/*
* The newer controllers include fused UUID as part of link
* controller specific registers
*/
- cap = tb_switch_find_vse_cap(sw, TB_VSE_CAP_LINK_CONTROLLER);
- if (cap > 0) {
- ret = tb_sw_read(sw, uuid, TB_CFG_SWITCH, cap + 3, 4);
- if (ret)
- return ret;
- } else {
+ ret = tb_lc_read_uuid(sw, uuid);
+ if (ret) {
/*
* ICM generates UUID based on UID and fills the upper
* two words with ones. This is not strictly following
@@ -1308,8 +1307,8 @@ static int tb_switch_set_uuid(struct tb_switch *sw)

sw->uuid = kmemdup(uuid, sizeof(uuid), GFP_KERNEL);
if (!sw->uuid)
- ret = -ENOMEM;
- return ret;
+ return -ENOMEM;
+ return 0;
}

static int tb_switch_add_dma_port(struct tb_switch *sw)
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index a166265dfcf9..e52d39b25266 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -63,6 +63,7 @@ struct tb_switch_nvm {
* @device_name: Name of the device (or %NULL if not known)
* @generation: Switch Thunderbolt generation
* @cap_plug_events: Offset to the plug events capability (%0 if not found)
+ * @cap_lc: Offset to the link controller capability (%0 if not found)
* @is_unplugged: The switch is going away
* @drom: DROM of the switch (%NULL if not found)
* @nvm: Pointer to the NVM if the switch has one (%NULL otherwise)
@@ -96,6 +97,7 @@ struct tb_switch {
const char *device_name;
unsigned int generation;
int cap_plug_events;
+ int cap_lc;
bool is_unplugged;
u8 *drom;
struct tb_switch_nvm *nvm;
@@ -462,6 +464,7 @@ bool tb_path_is_invalid(struct tb_path *path);
int tb_drom_read(struct tb_switch *sw);
int tb_drom_read_uid_only(struct tb_switch *sw, u64 *uid);

+int tb_lc_read_uuid(struct tb_switch *sw, u32 *uuid);

static inline int tb_route_length(u64 route)
{
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index 6f1ff04ee195..4895ae9f0b40 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -237,5 +237,7 @@ struct tb_regs_hop {
u32 unknown3:4; /* set to zero */
} __packed;

+/* Common link controller registers */
+#define TB_LC_FUSE 0x03

#endif
--
2.20.1

2019-03-28 12:42:25

by Mika Westerberg

Subject: [PATCH v3 06/36] thunderbolt: Do not allocate switch if depth is greater than 6

Maximum depth in Thunderbolt topology is 6 so make sure it is not
possible to allocate switches that exceed the depth limit.

While at it update tb_switch_alloc() to use upper/lower_32_bits()
following tb_switch_alloc_safe_mode().

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/icm.c | 5 ++---
drivers/thunderbolt/switch.c | 18 ++++++++++++------
drivers/thunderbolt/tb.h | 1 +
3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
index 7c923e16a7d8..bec360eef6cf 100644
--- a/drivers/thunderbolt/icm.c
+++ b/drivers/thunderbolt/icm.c
@@ -42,7 +42,6 @@
#define ICM_TIMEOUT 5000 /* ms */
#define ICM_APPROVE_TIMEOUT 10000 /* ms */
#define ICM_MAX_LINK 4
-#define ICM_MAX_DEPTH 6

/**
* struct icm - Internal connection manager private data
@@ -714,7 +713,7 @@ icm_fr_device_disconnected(struct tb *tb, const struct icm_pkg_header *hdr)
depth = (pkg->link_info & ICM_LINK_INFO_DEPTH_MASK) >>
ICM_LINK_INFO_DEPTH_SHIFT;

- if (link > ICM_MAX_LINK || depth > ICM_MAX_DEPTH) {
+ if (link > ICM_MAX_LINK || depth > TB_SWITCH_MAX_DEPTH) {
tb_warn(tb, "invalid topology %u.%u, ignoring\n", link, depth);
return;
}
@@ -744,7 +743,7 @@ icm_fr_xdomain_connected(struct tb *tb, const struct icm_pkg_header *hdr)
depth = (pkg->link_info & ICM_LINK_INFO_DEPTH_MASK) >>
ICM_LINK_INFO_DEPTH_SHIFT;

- if (link > ICM_MAX_LINK || depth > ICM_MAX_DEPTH) {
+ if (link > ICM_MAX_LINK || depth > TB_SWITCH_MAX_DEPTH) {
tb_warn(tb, "invalid topology %u.%u, ignoring\n", link, depth);
return;
}
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 7fa4ab076404..1e29c06947af 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -1130,10 +1130,16 @@ static int tb_switch_get_generation(struct tb_switch *sw)
struct tb_switch *tb_switch_alloc(struct tb *tb, struct device *parent,
u64 route)
{
- int i;
- int cap;
struct tb_switch *sw;
- int upstream_port = tb_cfg_get_upstream_port(tb->ctl, route);
+ int upstream_port;
+ int i, cap, depth;
+
+ /* Make sure we do not exceed maximum topology limit */
+ depth = tb_route_length(route);
+ if (depth > TB_SWITCH_MAX_DEPTH)
+ return NULL;
+
+ upstream_port = tb_cfg_get_upstream_port(tb->ctl, route);
if (upstream_port < 0)
return NULL;

@@ -1150,9 +1156,9 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, struct device *parent,

/* configure switch */
sw->config.upstream_port_number = upstream_port;
- sw->config.depth = tb_route_length(route);
- sw->config.route_lo = route;
- sw->config.route_hi = route >> 32;
+ sw->config.depth = depth;
+ sw->config.route_hi = upper_32_bits(route);
+ sw->config.route_lo = lower_32_bits(route);
sw->config.enabled = 0;

/* initialize ports */
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index f7b0c43c29a7..93c1ea21feeb 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -43,6 +43,7 @@ struct tb_switch_nvm {
};

#define TB_SWITCH_KEY_SIZE 32
+#define TB_SWITCH_MAX_DEPTH 6

/**
* struct tb_switch - a thunderbolt switch
--
2.20.1

2019-03-28 12:42:34

by Mika Westerberg

Subject: [PATCH v3 05/36] thunderbolt: Take domain lock in switch sysfs attribute callbacks

switch_lock was introduced because it allowed serialization of device
authorization requests from userspace without need to take the big
domain lock (tb->lock). This was fine because device authorization with
ICM is just one command that is sent to the firmware. Now that we start
to handle all tunneling in the driver switch_lock is not enough because
we need to walk over the topology to establish paths.

For this reason drop switch_lock from the driver completely in favour of
big domain lock.

There is one complication, though. If userspace is waiting for the lock
in tb_switch_set_authorized(), it keeps the device_del() from removing
the sysfs attribute because it waits for active users to release the
attribute first which leads into following splat:

INFO: task kworker/u8:3:73 blocked for more than 61 seconds.
Tainted: G W 5.1.0-rc1+ #244
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
kworker/u8:3 D12976 73 2 0x80000000
Workqueue: thunderbolt0 tb_handle_hotplug [thunderbolt]
Call Trace:
? __schedule+0x2e5/0x740
? _raw_spin_lock_irqsave+0x12/0x40
? prepare_to_wait_event+0xc5/0x160
schedule+0x2d/0x80
__kernfs_remove.part.17+0x183/0x1f0
? finish_wait+0x80/0x80
kernfs_remove_by_name_ns+0x4a/0x90
remove_files.isra.1+0x2b/0x60
sysfs_remove_group+0x38/0x80
sysfs_remove_groups+0x24/0x40
device_remove_attrs+0x3d/0x70
device_del+0x14c/0x360
device_unregister+0x15/0x50
tb_switch_remove+0x9e/0x1d0 [thunderbolt]
tb_handle_hotplug+0x119/0x5a0 [thunderbolt]
? process_one_work+0x1b7/0x420
process_one_work+0x1b7/0x420
worker_thread+0x37/0x380
? _raw_spin_unlock_irqrestore+0xf/0x30
? process_one_work+0x420/0x420
kthread+0x118/0x130
? kthread_create_on_node+0x60/0x60
ret_from_fork+0x35/0x40

We deal this by following what network stack did for some of their
attributes and use mutex_trylock() with restart_syscall(). This makes
userspace release the attribute allowing sysfs attribute removal to
progress before the write is restarted and eventually fail when the
attribute is removed.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/switch.c | 45 +++++++++++++++---------------------
drivers/thunderbolt/tb.h | 3 +--
2 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 5c2c0201ae7f..7fa4ab076404 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -10,15 +10,13 @@
#include <linux/idr.h>
#include <linux/nvmem-provider.h>
#include <linux/pm_runtime.h>
+#include <linux/sched/signal.h>
#include <linux/sizes.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>

#include "tb.h"

-/* Switch authorization from userspace is serialized by this lock */
-static DEFINE_MUTEX(switch_lock);
-
/* Switch NVM support */

#define NVM_DEVID 0x05
@@ -254,8 +252,8 @@ static int tb_switch_nvm_write(void *priv, unsigned int offset, void *val,
struct tb_switch *sw = priv;
int ret = 0;

- if (mutex_lock_interruptible(&switch_lock))
- return -ERESTARTSYS;
+ if (!mutex_trylock(&sw->tb->lock))
+ return restart_syscall();

/*
* Since writing the NVM image might require some special steps,
@@ -275,7 +273,7 @@ static int tb_switch_nvm_write(void *priv, unsigned int offset, void *val,
memcpy(sw->nvm->buf + offset, val, bytes);

unlock:
- mutex_unlock(&switch_lock);
+ mutex_unlock(&sw->tb->lock);

return ret;
}
@@ -364,10 +362,7 @@ static int tb_switch_nvm_add(struct tb_switch *sw)
}
nvm->non_active = nvm_dev;

- mutex_lock(&switch_lock);
sw->nvm = nvm;
- mutex_unlock(&switch_lock);
-
return 0;

err_nvm_active:
@@ -384,10 +379,8 @@ static void tb_switch_nvm_remove(struct tb_switch *sw)
{
struct tb_switch_nvm *nvm;

- mutex_lock(&switch_lock);
nvm = sw->nvm;
sw->nvm = NULL;
- mutex_unlock(&switch_lock);

if (!nvm)
return;
@@ -698,8 +691,8 @@ static int tb_switch_set_authorized(struct tb_switch *sw, unsigned int val)
{
int ret = -EINVAL;

- if (mutex_lock_interruptible(&switch_lock))
- return -ERESTARTSYS;
+ if (!mutex_trylock(&sw->tb->lock))
+ return restart_syscall();

if (sw->authorized)
goto unlock;
@@ -742,7 +735,7 @@ static int tb_switch_set_authorized(struct tb_switch *sw, unsigned int val)
}

unlock:
- mutex_unlock(&switch_lock);
+ mutex_unlock(&sw->tb->lock);
return ret;
}

@@ -799,15 +792,15 @@ static ssize_t key_show(struct device *dev, struct device_attribute *attr,
struct tb_switch *sw = tb_to_switch(dev);
ssize_t ret;

- if (mutex_lock_interruptible(&switch_lock))
- return -ERESTARTSYS;
+ if (!mutex_trylock(&sw->tb->lock))
+ return restart_syscall();

if (sw->key)
ret = sprintf(buf, "%*phN\n", TB_SWITCH_KEY_SIZE, sw->key);
else
ret = sprintf(buf, "\n");

- mutex_unlock(&switch_lock);
+ mutex_unlock(&sw->tb->lock);
return ret;
}

@@ -824,8 +817,8 @@ static ssize_t key_store(struct device *dev, struct device_attribute *attr,
else if (hex2bin(key, buf, sizeof(key)))
return -EINVAL;

- if (mutex_lock_interruptible(&switch_lock))
- return -ERESTARTSYS;
+ if (!mutex_trylock(&sw->tb->lock))
+ return restart_syscall();

if (sw->authorized) {
ret = -EBUSY;
@@ -840,7 +833,7 @@ static ssize_t key_store(struct device *dev, struct device_attribute *attr,
}
}

- mutex_unlock(&switch_lock);
+ mutex_unlock(&sw->tb->lock);
return ret;
}
static DEVICE_ATTR(key, 0600, key_show, key_store);
@@ -886,8 +879,8 @@ static ssize_t nvm_authenticate_store(struct device *dev,
bool val;
int ret;

- if (mutex_lock_interruptible(&switch_lock))
- return -ERESTARTSYS;
+ if (!mutex_trylock(&sw->tb->lock))
+ return restart_syscall();

/* If NVMem devices are not yet added */
if (!sw->nvm) {
@@ -935,7 +928,7 @@ static ssize_t nvm_authenticate_store(struct device *dev,
}

exit_unlock:
- mutex_unlock(&switch_lock);
+ mutex_unlock(&sw->tb->lock);

if (ret)
return ret;
@@ -949,8 +942,8 @@ static ssize_t nvm_version_show(struct device *dev,
struct tb_switch *sw = tb_to_switch(dev);
int ret;

- if (mutex_lock_interruptible(&switch_lock))
- return -ERESTARTSYS;
+ if (!mutex_trylock(&sw->tb->lock))
+ return restart_syscall();

if (sw->safe_mode)
ret = -ENODATA;
@@ -959,7 +952,7 @@ static ssize_t nvm_version_show(struct device *dev,
else
ret = sprintf(buf, "%x.%x\n", sw->nvm->major, sw->nvm->minor);

- mutex_unlock(&switch_lock);
+ mutex_unlock(&sw->tb->lock);

return ret;
}
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 496dcd03ede1..f7b0c43c29a7 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -79,8 +79,7 @@ struct tb_switch_nvm {
* @depth: Depth in the chain this switch is connected (ICM only)
*
* When the switch is being added or removed to the domain (other
- * switches) you need to have domain lock held. For switch authorization
- * internal switch_lock is enough.
+ * switches) you need to have domain lock held.
*/
struct tb_switch {
struct device dev;
--
2.20.1

2019-03-28 12:42:36

by Mika Westerberg

Subject: [PATCH v3 07/36] thunderbolt: Enable TMU access when accessing port space on legacy devices

Light Ridge and Eagle Ridge both need to have TMU access enabled before
port space can be fully accessed so make sure it happens on those. This
allows us to get rid of the offset quirk in tb_port_find_cap().

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/cap.c | 69 +++++++++++++++++++++++++++++----------
drivers/thunderbolt/tb.h | 10 ++++++
2 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/drivers/thunderbolt/cap.c b/drivers/thunderbolt/cap.c
index 9553305c63ea..a58585b4e6d9 100644
--- a/drivers/thunderbolt/cap.c
+++ b/drivers/thunderbolt/cap.c
@@ -13,6 +13,7 @@

#define CAP_OFFSET_MAX 0xff
#define VSE_CAP_OFFSET_MAX 0xffff
+#define TMU_ACCESS_EN BIT(20)

struct tb_cap_any {
union {
@@ -22,28 +23,38 @@ struct tb_cap_any {
};
} __packed;

-/**
- * tb_port_find_cap() - Find port capability
- * @port: Port to find the capability for
- * @cap: Capability to look
- *
- * Returns offset to start of capability or %-ENOENT if no such
- * capability was found. Negative errno is returned if there was an
- * error.
- */
-int tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap)
+static int tb_port_enable_tmu(struct tb_port *port, bool enable)
{
- u32 offset;
+ struct tb_switch *sw = port->sw;
+ u32 value, offset;
+ int ret;

/*
- * DP out adapters claim to implement TMU capability but in
- * reality they do not so we hard code the adapter specific
- * capability offset here.
+ * Legacy devices need to have TMU access enabled before port
+ * space can be fully accessed.
*/
- if (port->config.type == TB_TYPE_DP_HDMI_OUT)
- offset = 0x39;
+ if (tb_switch_is_lr(sw))
+ offset = 0x26;
+ else if (tb_switch_is_er(sw))
+ offset = 0x2a;
+ else
+ return 0;
+
+ ret = tb_sw_read(sw, &value, TB_CFG_SWITCH, offset, 1);
+ if (ret)
+ return ret;
+
+ if (enable)
+ value |= TMU_ACCESS_EN;
else
- offset = 0x1;
+ value &= ~TMU_ACCESS_EN;
+
+ return tb_sw_write(sw, &value, TB_CFG_SWITCH, offset, 1);
+}
+
+static int __tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap)
+{
+ u32 offset = 1;

do {
struct tb_cap_any header;
@@ -62,6 +73,30 @@ int tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap)
return -ENOENT;
}

+/**
+ * tb_port_find_cap() - Find port capability
+ * @port: Port to find the capability for
+ * @cap: Capability to look
+ *
+ * Returns offset to start of capability or %-ENOENT if no such
+ * capability was found. Negative errno is returned if there was an
+ * error.
+ */
+int tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap)
+{
+ int ret;
+
+ ret = tb_port_enable_tmu(port, true);
+ if (ret)
+ return ret;
+
+ ret = __tb_port_find_cap(port, cap);
+
+ tb_port_enable_tmu(port, false);
+
+ return ret;
+}
+
static int tb_switch_find_cap(struct tb_switch *sw, enum tb_switch_cap cap)
{
int offset = sw->config.first_cap_offset;
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 93c1ea21feeb..a166265dfcf9 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -436,6 +436,16 @@ static inline struct tb_switch *tb_to_switch(struct device *dev)
return NULL;
}

+static inline bool tb_switch_is_lr(const struct tb_switch *sw)
+{
+ return sw->config.device_id == PCI_DEVICE_ID_INTEL_LIGHT_RIDGE;
+}
+
+static inline bool tb_switch_is_er(const struct tb_switch *sw)
+{
+ return sw->config.device_id == PCI_DEVICE_ID_INTEL_EAGLE_RIDGE;
+}
+
int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged);
int tb_port_add_nfc_credits(struct tb_port *port, int credits);
int tb_port_clear_counter(struct tb_port *port, int counter);
--
2.20.1

2019-03-28 12:42:43

by Mika Westerberg

Subject: [PATCH v3 08/36] thunderbolt: Add dummy read after port capability list walk on Light Ridge

Light Ridge has an issue where reading the next capability pointer
location in port config space the read data is not cleared. It is fine
to read capabilities each after another so only thing we need to do is
to make sure we issue dummy read after tb_port_find_cap() is finished to
avoid the issue in next read.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/cap.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/drivers/thunderbolt/cap.c b/drivers/thunderbolt/cap.c
index a58585b4e6d9..8bf8e031f0bc 100644
--- a/drivers/thunderbolt/cap.c
+++ b/drivers/thunderbolt/cap.c
@@ -52,6 +52,21 @@ static int tb_port_enable_tmu(struct tb_port *port, bool enable)
return tb_sw_write(sw, &value, TB_CFG_SWITCH, offset, 1);
}

+static void tb_port_dummy_read(struct tb_port *port)
+{
+ /*
+ * When reading from next capability pointer location in port
+ * config space the read data is not cleared on LR. To avoid
+ * reading stale data on next read perform one dummy read after
+ * port capabilities are walked.
+ */
+ if (tb_switch_is_lr(port->sw)) {
+ u32 dummy;
+
+ tb_port_read(port, &dummy, TB_CFG_PORT, 0, 1);
+ }
+}
+
static int __tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap)
{
u32 offset = 1;
@@ -92,6 +107,7 @@ int tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap)

ret = __tb_port_find_cap(port, cap);

+ tb_port_dummy_read(port);
tb_port_enable_tmu(port, false);

return ret;
--
2.20.1

2019-03-28 12:42:45

by Mika Westerberg

Subject: [PATCH v3 11/36] thunderbolt: Set sleep bit when suspending switch

Thunderbolt 2 devices and beyond link controller needs to be notified
when a switch is going to be suspended by setting bit 31 in LC_SX_CTRL
register. Add this functionality to the software connection manager.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/lc.c | 44 +++++++++++++++++++++++++++++++++++
drivers/thunderbolt/switch.c | 6 ++---
drivers/thunderbolt/tb.h | 1 +
drivers/thunderbolt/tb_regs.h | 2 ++
4 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/drivers/thunderbolt/lc.c b/drivers/thunderbolt/lc.c
index a5dddf176546..ae1e92611c3e 100644
--- a/drivers/thunderbolt/lc.c
+++ b/drivers/thunderbolt/lc.c
@@ -133,3 +133,47 @@ void tb_lc_unconfigure_link(struct tb_switch *sw)
tb_lc_configure_lane(up, false);
tb_lc_configure_lane(down, false);
}
+
+/**
+ * tb_lc_set_sleep() - Inform LC that the switch is going to sleep
+ * @sw: Switch to set sleep
+ *
+ * Let the switch link controllers know that the switch is going to
+ * sleep.
+ */
+int tb_lc_set_sleep(struct tb_switch *sw)
+{
+ int start, size, nlc, ret, i;
+ u32 desc;
+
+ if (sw->generation < 2)
+ return 0;
+
+ ret = read_lc_desc(sw, &desc);
+ if (ret)
+ return ret;
+
+ /* Figure out number of link controllers */
+ nlc = desc & TB_LC_DESC_NLC_MASK;
+ start = (desc & TB_LC_DESC_SIZE_MASK) >> TB_LC_DESC_SIZE_SHIFT;
+ size = (desc & TB_LC_DESC_PORT_SIZE_MASK) >> TB_LC_DESC_PORT_SIZE_SHIFT;
+
+ /* For each link controller set sleep bit */
+ for (i = 0; i < nlc; i++) {
+ unsigned int offset = sw->cap_lc + start + i * size;
+ u32 ctrl;
+
+ ret = tb_sw_read(sw, &ctrl, TB_CFG_SWITCH,
+ offset + TB_LC_SX_CTRL, 1);
+ if (ret)
+ return ret;
+
+ ctrl |= TB_LC_SX_CTRL_SLP;
+ ret = tb_sw_write(sw, &ctrl, TB_CFG_SWITCH,
+ offset + TB_LC_SX_CTRL, 1);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index dd218dc4781b..b3f93ebe6e39 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -1586,10 +1586,8 @@ void tb_switch_suspend(struct tb_switch *sw)
if (!tb_is_upstream_port(&sw->ports[i]) && sw->ports[i].remote)
tb_switch_suspend(sw->ports[i].remote->sw);
}
- /*
- * TODO: invoke tb_cfg_prepare_to_sleep here? does not seem to have any
- * effect?
- */
+
+ tb_lc_set_sleep(sw);
}

struct tb_sw_lookup {
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 69e0534224d8..985a48a67a43 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -467,6 +467,7 @@ int tb_drom_read_uid_only(struct tb_switch *sw, u64 *uid);
int tb_lc_read_uuid(struct tb_switch *sw, u32 *uuid);
int tb_lc_configure_link(struct tb_switch *sw);
void tb_lc_unconfigure_link(struct tb_switch *sw);
+int tb_lc_set_sleep(struct tb_switch *sw);

static inline int tb_route_length(u64 route)
{
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index e0f867dad5cf..1ab6e0fb31c0 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -239,6 +239,7 @@ struct tb_regs_hop {

/* Common link controller registers */
#define TB_LC_DESC 0x02
+#define TB_LC_DESC_NLC_MASK GENMASK(3, 0)
#define TB_LC_DESC_SIZE_SHIFT 8
#define TB_LC_DESC_SIZE_MASK GENMASK(15, 8)
#define TB_LC_DESC_PORT_SIZE_SHIFT 16
@@ -250,5 +251,6 @@ struct tb_regs_hop {
#define TB_LC_SX_CTRL_L1C BIT(16)
#define TB_LC_SX_CTRL_L2C BIT(20)
#define TB_LC_SX_CTRL_UPSTREAM BIT(30)
+#define TB_LC_SX_CTRL_SLP BIT(31)

#endif
--
2.20.1

2019-03-28 12:43:00

by Mika Westerberg

Subject: [PATCH v3 04/36] thunderbolt: Block reads and writes if switch is unplugged

If switch is already disconnected there is no point sending it commands
and waiting for timeout. Instead in that case return error immediately.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/tb.h | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index aea668c40d27..496dcd03ede1 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -277,6 +277,8 @@ static inline struct tb_port *tb_port_at(u64 route, struct tb_switch *sw)
static inline int tb_sw_read(struct tb_switch *sw, void *buffer,
enum tb_cfg_space space, u32 offset, u32 length)
{
+ if (sw->is_unplugged)
+ return -ENODEV;
return tb_cfg_read(sw->tb->ctl,
buffer,
tb_route(sw),
@@ -289,6 +291,8 @@ static inline int tb_sw_read(struct tb_switch *sw, void *buffer,
static inline int tb_sw_write(struct tb_switch *sw, void *buffer,
enum tb_cfg_space space, u32 offset, u32 length)
{
+ if (sw->is_unplugged)
+ return -ENODEV;
return tb_cfg_write(sw->tb->ctl,
buffer,
tb_route(sw),
@@ -301,6 +305,8 @@ static inline int tb_sw_write(struct tb_switch *sw, void *buffer,
static inline int tb_port_read(struct tb_port *port, void *buffer,
enum tb_cfg_space space, u32 offset, u32 length)
{
+ if (port->sw->is_unplugged)
+ return -ENODEV;
return tb_cfg_read(port->sw->tb->ctl,
buffer,
tb_route(port->sw),
@@ -313,6 +319,8 @@ static inline int tb_port_read(struct tb_port *port, void *buffer,
static inline int tb_port_write(struct tb_port *port, const void *buffer,
enum tb_cfg_space space, u32 offset, u32 length)
{
+ if (port->sw->is_unplugged)
+ return -ENODEV;
return tb_cfg_write(port->sw->tb->ctl,
buffer,
tb_route(port->sw),
--
2.20.1

2019-03-28 12:43:02

by Mika Westerberg

Subject: [PATCH v3 12/36] thunderbolt: Properly disable path

We need to wait until all buffers have been drained before the path can
be considered disabled. Do this for every hop in a path. Also if the
switch is physically disconnected, do not bother disabling it anymore
(it is not present anyway).

This adds another bit field to struct tb_regs_hop even if we are trying
to get rid of them but we can clean them up another day.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/path.c | 48 ++++++++++++++++++++++++++++++++---
drivers/thunderbolt/tb_regs.h | 3 ++-
2 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/drivers/thunderbolt/path.c b/drivers/thunderbolt/path.c
index a11956522bac..97f37f86fb75 100644
--- a/drivers/thunderbolt/path.c
+++ b/drivers/thunderbolt/path.c
@@ -7,6 +7,8 @@

#include <linux/slab.h>
#include <linux/errno.h>
+#include <linux/delay.h>
+#include <linux/ktime.h>

#include "tb.h"

@@ -74,13 +76,53 @@ static void __tb_path_deallocate_nfc(struct tb_path *path, int first_hop)
}
}

+static int __tb_path_deactivate_hop(struct tb_port *port, int hop_index)
+{
+ struct tb_regs_hop hop;
+ ktime_t timeout;
+ int ret;
+
+ if (port->sw->is_unplugged)
+ return 0;
+
+ /* Disable the path */
+ ret = tb_port_read(port, &hop, TB_CFG_HOPS, 2 * hop_index, 2);
+ if (ret)
+ return ret;
+
+ /* Already disabled */
+ if (!hop.enable)
+ return 0;
+
+ hop.enable = 0;
+
+ ret = tb_port_write(port, &hop, TB_CFG_HOPS, 2 * hop_index, 2);
+ if (ret)
+ return ret;
+
+ /* Wait until it is drained */
+ timeout = ktime_add_ms(ktime_get(), 500);
+ do {
+ ret = tb_port_read(port, &hop, TB_CFG_HOPS, 2 * hop_index, 2);
+ if (ret)
+ return ret;
+
+ if (!hop.pending)
+ return 0;
+
+ usleep_range(10, 20);
+ } while (ktime_before(ktime_get(), timeout));
+
+ return -ETIMEDOUT;
+}
+
static void __tb_path_deactivate_hops(struct tb_path *path, int first_hop)
{
int i, res;
- struct tb_regs_hop hop = { };
+
for (i = first_hop; i < path->path_length; i++) {
- res = tb_port_write(path->hops[i].in_port, &hop, TB_CFG_HOPS,
- 2 * path->hops[i].in_hop_index, 2);
+ res = __tb_path_deactivate_hop(path->hops[i].in_port,
+ path->hops[i].in_hop_index);
if (res)
tb_port_warn(path->hops[i].in_port,
"hop deactivation failed for hop %d, index %d\n",
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index 1ab6e0fb31c0..82ac4ec8757f 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -234,7 +234,8 @@ struct tb_regs_hop {
bool egress_fc:1;
bool ingress_shared_buffer:1;
bool egress_shared_buffer:1;
- u32 unknown3:4; /* set to zero */
+ bool pending:1;
+ u32 unknown3:3; /* set to zero */
} __packed;

/* Common link controller registers */
--
2.20.1

2019-03-28 12:43:03

by Mika Westerberg

Subject: [PATCH v3 03/36] thunderbolt: Drop duplicated get_switch_by_route()

tb_switch_find_by_route() does the same already so use it instead and
remove duplicated get_switch_by_route().

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/icm.c | 12 ++++++++----
drivers/thunderbolt/switch.c | 18 ------------------
drivers/thunderbolt/tb.c | 9 ++++++---
drivers/thunderbolt/tb.h | 1 -
4 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
index 8b7f9131e9d1..7c923e16a7d8 100644
--- a/drivers/thunderbolt/icm.c
+++ b/drivers/thunderbolt/icm.c
@@ -798,9 +798,11 @@ icm_fr_xdomain_connected(struct tb *tb, const struct icm_pkg_header *hdr)
* connected another host to the same port, remove the switch
* first.
*/
- sw = get_switch_at_route(tb->root_switch, route);
- if (sw)
+ sw = tb_switch_find_by_route(tb, route);
+ if (sw) {
remove_switch(sw);
+ tb_switch_put(sw);
+ }

sw = tb_switch_find_by_link_depth(tb, link, depth);
if (!sw) {
@@ -1143,9 +1145,11 @@ icm_tr_xdomain_connected(struct tb *tb, const struct icm_pkg_header *hdr)
* connected another host to the same port, remove the switch
* first.
*/
- sw = get_switch_at_route(tb->root_switch, route);
- if (sw)
+ sw = tb_switch_find_by_route(tb, route);
+ if (sw) {
remove_switch(sw);
+ tb_switch_put(sw);
+ }

sw = tb_switch_find_by_route(tb, get_parent_route(route));
if (!sw) {
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 504365d46827..5c2c0201ae7f 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -644,24 +644,6 @@ int tb_switch_reset(struct tb *tb, u64 route)
return res.err;
}

-struct tb_switch *get_switch_at_route(struct tb_switch *sw, u64 route)
-{
- u8 next_port = route; /*
- * Routes use a stride of 8 bits,
- * eventhough a port index has 6 bits at most.
- * */
- if (route == 0)
- return sw;
- if (next_port > sw->config.max_port_number)
- return NULL;
- if (tb_is_upstream_port(&sw->ports[next_port]))
- return NULL;
- if (!sw->ports[next_port].remote)
- return NULL;
- return get_switch_at_route(sw->ports[next_port].remote->sw,
- route >> TB_ROUTE_SHIFT);
-}
-
/**
* tb_plug_events_active() - enable/disable plug events on a switch
*
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 30e02c716f6c..d8f4ed0f2ef8 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -258,7 +258,7 @@ static void tb_handle_hotplug(struct work_struct *work)
if (!tcm->hotplug_active)
goto out; /* during init, suspend or shutdown */

- sw = get_switch_at_route(tb->root_switch, ev->route);
+ sw = tb_switch_find_by_route(tb, ev->route);
if (!sw) {
tb_warn(tb,
"hotplug event from non existent switch %llx:%x (unplug: %d)\n",
@@ -269,14 +269,14 @@ static void tb_handle_hotplug(struct work_struct *work)
tb_warn(tb,
"hotplug event from non existent port %llx:%x (unplug: %d)\n",
ev->route, ev->port, ev->unplug);
- goto out;
+ goto put_sw;
}
port = &sw->ports[ev->port];
if (tb_is_upstream_port(port)) {
tb_warn(tb,
"hotplug event for upstream port %llx:%x (unplug: %d)\n",
ev->route, ev->port, ev->unplug);
- goto out;
+ goto put_sw;
}
if (ev->unplug) {
if (port->remote) {
@@ -306,6 +306,9 @@ static void tb_handle_hotplug(struct work_struct *work)
tb_activate_pcie_devices(tb);
}
}
+
+put_sw:
+ tb_switch_put(sw);
out:
mutex_unlock(&tb->lock);
kfree(ev);
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 8058ea02d572..aea668c40d27 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -399,7 +399,6 @@ void tb_switch_suspend(struct tb_switch *sw);
int tb_switch_resume(struct tb_switch *sw);
int tb_switch_reset(struct tb *tb, u64 route);
void tb_sw_set_unplugged(struct tb_switch *sw);
-struct tb_switch *get_switch_at_route(struct tb_switch *sw, u64 route);
struct tb_switch *tb_switch_find_by_link_depth(struct tb *tb, u8 link,
u8 depth);
struct tb_switch *tb_switch_find_by_uuid(struct tb *tb, const uuid_t *uuid);
--
2.20.1

2019-03-28 12:43:10

by Mika Westerberg

Subject: [PATCH v3 02/36] thunderbolt: Remove unused work field in struct tb_switch

This field is not used anywhere so remove it.

Reported-by: Lukas Wunner <[email protected]>
Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/tb.h | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 52584c4003e3..8058ea02d572 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -70,7 +70,6 @@ struct tb_switch_nvm {
* @boot: Whether the switch was already authorized on boot or not
* @rpm: The switch supports runtime PM
* @authorized: Whether the switch is authorized by user or policy
- * @work: Work used to automatically authorize a switch
* @security_level: Switch supported security level
* @key: Contains the key used to challenge the device or %NULL if not
* supported. Size of the key is %TB_SWITCH_KEY_SIZE.
@@ -105,7 +104,6 @@ struct tb_switch {
bool boot;
bool rpm;
unsigned int authorized;
- struct work_struct work;
enum tb_security_level security_level;
u8 *key;
u8 connection_id;
--
2.20.1

2019-03-28 12:43:54

by Mika Westerberg

Subject: [PATCH v3 01/36] net: thunderbolt: Unregister ThunderboltIP protocol handler when suspending

The XDomain protocol messages may start as soon as Thunderbolt control
channel is started. This means that if the other host starts sending
ThunderboltIP packets early enough they will be passed to the network
driver which then gets confused because its resume hook is not called
yet.

Fix this by unregistering the ThunderboltIP protocol handler when
suspending and registering it back on resume.

Signed-off-by: Mika Westerberg <[email protected]>
Acked-by: David S. Miller <[email protected]>
---
drivers/net/thunderbolt.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/net/thunderbolt.c b/drivers/net/thunderbolt.c
index c48c3a1eb1f8..fcf31335a8b6 100644
--- a/drivers/net/thunderbolt.c
+++ b/drivers/net/thunderbolt.c
@@ -1282,6 +1282,7 @@ static int __maybe_unused tbnet_suspend(struct device *dev)
tbnet_tear_down(net, true);
}

+ tb_unregister_protocol_handler(&net->handler);
return 0;
}

@@ -1290,6 +1291,8 @@ static int __maybe_unused tbnet_resume(struct device *dev)
struct tb_service *svc = tb_to_service(dev);
struct tbnet *net = tb_service_get_drvdata(svc);

+ tb_register_protocol_handler(&net->handler);
+
netif_carrier_off(net->dev);
if (netif_running(net->dev)) {
netif_device_attach(net->dev);
--
2.20.1

2019-03-28 12:43:56

by Mika Westerberg

Subject: [PATCH v3 13/36] thunderbolt: Cache adapter specific capability offset into struct port

The adapter specific capability either is there or not if the port does
not hold an adapter. Instead of always finding it on-demand we read the
offset just once when the port is initialized.

While there we update the struct port documentation to follow kernel-doc
format.

Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/switch.c | 4 ++++
drivers/thunderbolt/tb.c | 8 ++++----
drivers/thunderbolt/tb.h | 2 ++
drivers/thunderbolt/tunnel_pci.c | 9 +++------
4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index b3f93ebe6e39..9756e6279dc9 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -586,6 +586,10 @@ static int tb_init_port(struct tb_port *port)
port->cap_phy = cap;
else
tb_port_WARN(port, "non switch port without a PHY\n");
+ } else if (port->port != 0) {
+ cap = tb_port_find_cap(port, TB_PORT_CAP_ADAP);
+ if (cap > 0)
+ port->cap_adap = cap;
}

tb_dump_port(port->sw->tb, &port->config);
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index d8f4ed0f2ef8..e71530d0af65 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -151,8 +151,8 @@ static struct tb_port *tb_find_unused_down_port(struct tb_switch *sw)
continue;
if (sw->ports[i].config.type != TB_TYPE_PCIE_DOWN)
continue;
- cap = tb_port_find_cap(&sw->ports[i], TB_PORT_CAP_ADAP);
- if (cap < 0)
+ cap = sw->ports[i].cap_adap;
+ if (!cap)
continue;
res = tb_port_read(&sw->ports[i], &data, TB_CFG_PORT, cap, 1);
if (res < 0)
@@ -197,8 +197,8 @@ static void tb_activate_pcie_devices(struct tb *tb)
}

/* check whether port is already activated */
- cap = tb_port_find_cap(up_port, TB_PORT_CAP_ADAP);
- if (cap < 0)
+ cap = up_port->cap_adap;
+ if (!cap)
continue;
if (tb_port_read(up_port, &data, TB_CFG_PORT, cap, 1))
continue;
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 985a48a67a43..b4d7c4d408bd 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -121,6 +121,7 @@ struct tb_switch {
* @remote: Remote port (%NULL if not connected)
* @xdomain: Remote host (%NULL if not connected)
* @cap_phy: Offset, zero if not found
+ * @cap_adap: Offset of the adapter specific capability (%0 if not present)
* @port: Port number on switch
* @disabled: Disabled by eeprom
* @dual_link_port: If the switch is connected using two ports, points
@@ -133,6 +134,7 @@ struct tb_port {
struct tb_port *remote;
struct tb_xdomain *xdomain;
int cap_phy;
+ int cap_adap;
u8 port;
bool disabled;
struct tb_port *dual_link_port;
diff --git a/drivers/thunderbolt/tunnel_pci.c b/drivers/thunderbolt/tunnel_pci.c
index 0637537ea53f..2de4edccbd6d 100644
--- a/drivers/thunderbolt/tunnel_pci.c
+++ b/drivers/thunderbolt/tunnel_pci.c
@@ -148,12 +148,9 @@ bool tb_pci_is_invalid(struct tb_pci_tunnel *tunnel)
static int tb_pci_port_active(struct tb_port *port, bool active)
{
u32 word = active ? 0x80000000 : 0x0;
- int cap = tb_port_find_cap(port, TB_PORT_CAP_ADAP);
- if (cap < 0) {
- tb_port_warn(port, "TB_PORT_CAP_ADAP not found: %d\n", cap);
- return cap;
- }
- return tb_port_write(port, &word, TB_CFG_PORT, cap, 1);
+ if (!port->cap_adap)
+ return -ENXIO;
+ return tb_port_write(port, &word, TB_CFG_PORT, port->cap_adap, 1);
}

/**
--
2.20.1

2019-03-28 15:20:35

by Mario Limonciello

Subject: RE: [PATCH v3 00/36] thunderbolt: Software connection manager improvements

> -----Original Message-----
> From: Mika Westerberg <[email protected]>
> Sent: Thursday, March 28, 2019 7:36 AM
> To: [email protected]
> Cc: Michael Jamet; Yehezkel Bernat; Andreas Noever; Lukas Wunner; David S .
> Miller; Andy Shevchenko; Christian Kellner; Limonciello, Mario; Mika Westerberg;
> [email protected]
> Subject: [PATCH v3 00/36] thunderbolt: Software connection manager
> improvements
>
>
> [EXTERNAL EMAIL]
>
> Hi,
>
> This is third iteration of the patch series intending to bring same kind of
> functionality for older Apple systems than we have in PCs. Software
> connection manager is used on Apple hardware with Light Ridge, Cactus Ridge
> or Falcon Ridge controllers to create PCIe tunnels when a Thunderbolt
> device is connected. Currently only one PCIe tunnel is supported. On newer
> Alpine Ridge based Apple systems the driver starts the firmware which then
> takes care creating tunnels.
>
> This series improves the software connection manager so that it will
> support:
>
> - Full PCIe daisy chains (up to 6 devices)
> - Display Port tunneling
> - P2P networking
>
> We also add support for Titan Ridge based Apple systems where we can use
> the same flows than with Alpine Ridge to start the firmware.

It seems to me that there would be an expectation that PC system firmware and TBT controller
firmware is configured to behave like Apple systems to use this SW connection manager
instead of the ICM in AR/TR FW.

Is there an intent to eventually offer a way to "side-step" the TBT ICM and try to use this instead
without firmware support?

>
> This applies on top of thunderbolt.git/next.
>
> Christian, Mario do you see any issues with patch [05/36] regarding bolt
> and fwupd? The kernel is supposed to restart the syscall automatically so
> userspace should not be affected but wanted to check with you.

I don't see a problem for fwupd in this area.

>
> Previous version of the patch series can be viewed here:
>
> v2: https://lkml.org/lkml/2019/2/6/347
> v1: https://lkml.org/lkml/2019/1/29/924
>
> Making v3 took longer than I anticipated mostly due to some issues I run
> during testing the new changes. There are quite many changes so I dropped
> the reviewed-by tags I got for v2. Below is the list of major changes from
> the previous version:
>
> * Always set port->remote even in case of dual link connection.
>
> * Leave (DP, PCIe) tunnels up when the driver is unloaded. When loaded
> back, it discovers the existing tunnels and updated data structures
> accordingly. I noticed that the code in v2 did not support cases
> properly when you unplug something before the driver gets loaded back.
> This version tears down partial paths during discovery.
>
> * Do not automatically create PCIe tunnels. Instead we implement "user"
> security level in the software connection manager as well taking
> advantage of the existing sysfs interfaces. This allows user to disable
> PCIe tunneling completely or implement different white listing
> policies. Major distros include bolt system daemon that takes care of
> this.

This is a bit unfortunate. Is this because of IOMMU limitations in working
with devices down the chain?

>
> * When testing on two-port Falcon Ridge based system I realized
> that we always just pick the first available PCIe downstream adapter
> regardless of which Thunderbolt port you plug the device which is not
> consistent. To solve this we add mapping between host PCIe downstream
> adapter and the Thunderbolt port for Cactus Ridge and Falcon Ridge
> based systems.
>
> * Take domain lock in switch sysfs callbacks. This is needed because the
> software connection manager needs to walk over the topology during
> tunnel creation so switch_lock is not enough anymore.
>
> Changes from v1:
>
> * Added ACK from David
>
> * Add constant (TMU_ACCESS_EN) for BIT(20) when TMU access is enabled. We
> keep it in cap.c close to the LR/ER workaround. Also we enable/disable
> only during capability walk. If it turns we need to have it enabled
> elsewhere we can move it to switch.c and enable just once during
> switch enumeration.
>
> * Use 0 to mean no cap_adap instead of negative value. This follows
> cap_phy.
>
> * Use correct PCI IDs (_BRIDGE) in the last patch where we start firmware
> on Titan Ridge. It wrongly used NHI PCI IDs in v1.
>
> Mika Westerberg (36):
> net: thunderbolt: Unregister ThunderboltIP protocol handler when suspending
> thunderbolt: Remove unused work field in struct tb_switch
> thunderbolt: Drop duplicated get_switch_by_route()
> thunderbolt: Block reads and writes if switch is unplugged
> thunderbolt: Take domain lock in switch sysfs attribute callbacks
> thunderbolt: Do not allocate switch if depth is greater than 6
> thunderbolt: Enable TMU access when accessing port space on legacy devices
> thunderbolt: Add dummy read after port capability list walk on Light Ridge
> thunderbolt: Move LC specific functionality into a separate file
> thunderbolt: Configure lanes when switch is initialized
> thunderbolt: Set sleep bit when suspending switch
> thunderbolt: Properly disable path
> thunderbolt: Cache adapter specific capability offset into struct port
> thunderbolt: Rename tunnel_pci to tunnel
> thunderbolt: Generalize tunnel creation functionality
> thunderbolt: Add functions for allocating and releasing HopIDs
> thunderbolt: Assign remote for both ports in case of dual link
> thunderbolt: Add helper function to iterate from one port to another
> thunderbolt: Extend tunnel creation to more than 2 adjacent switches
> thunderbolt: Deactivate all paths before restarting them
> thunderbolt: Discover preboot PCIe paths the boot firmware established
> thunderbolt: Add support for full PCIe daisy chains
> thunderbolt: Scan only valid NULL adapter ports in hotplug
> thunderbolt: Generalize port finding routines to support all port types
> thunderbolt: Rework NFC credits handling
> thunderbolt: Add support for Display Port tunnels
> thunderbolt: Do not tear down tunnels when driver is unloaded
> thunderbolt: Run tb_xdp_handle_request() in system workqueue
> thunderbolt: Add XDomain UUID exchange support
> thunderbolt: Add support for DMA tunnels
> thunderbolt: Make tb_switch_alloc() return ERR_PTR()
> thunderbolt: Add support for XDomain connections
> thunderbolt: Make __TB_[SW|PORT]_PRINT take const parameters
> thunderbolt: Make rest of the logging to happen at debug level
> thunderbolt: Reword output of tb_dump_hop()
> thunderbolt: Start firmware on Titan Ridge Apple systems
>
> drivers/net/thunderbolt.c | 3 +
> drivers/thunderbolt/Makefile | 4 +-
> drivers/thunderbolt/cap.c | 85 +++-
> drivers/thunderbolt/ctl.c | 2 +-
> drivers/thunderbolt/icm.c | 60 ++-
> drivers/thunderbolt/lc.c | 179 ++++++++
> drivers/thunderbolt/nhi.c | 3 +-
> drivers/thunderbolt/path.c | 421 ++++++++++++++++---
> drivers/thunderbolt/switch.c | 551 +++++++++++++++++++-----
> drivers/thunderbolt/tb.c | 608 ++++++++++++++++++++-------
> drivers/thunderbolt/tb.h | 173 +++++++-
> drivers/thunderbolt/tb_msgs.h | 11 +
> drivers/thunderbolt/tb_regs.h | 50 ++-
> drivers/thunderbolt/tunnel.c | 691 +++++++++++++++++++++++++++++++
> drivers/thunderbolt/tunnel.h | 78 ++++
> drivers/thunderbolt/tunnel_pci.c | 226 ----------
> drivers/thunderbolt/tunnel_pci.h | 31 --
> drivers/thunderbolt/xdomain.c | 147 ++++++-
> include/linux/thunderbolt.h | 8 +
> 19 files changed, 2680 insertions(+), 651 deletions(-)
> create mode 100644 drivers/thunderbolt/lc.c
> create mode 100644 drivers/thunderbolt/tunnel.c
> create mode 100644 drivers/thunderbolt/tunnel.h
> delete mode 100644 drivers/thunderbolt/tunnel_pci.c
> delete mode 100644 drivers/thunderbolt/tunnel_pci.h
>
> --
> 2.20.1

2019-03-28 15:41:34

by Joe Perches

Subject: Re: [PATCH v3 34/36] thunderbolt: Make rest of the logging to happen at debug level

On Thu, 2019-03-28 at 15:36 +0300, Mika Westerberg wrote:
> Now that the driver can handle every possible tunnel types there is no
> point to log everything as info level so turn these to happen at debug
> level instead.

trivia:

> diff --git a/drivers/thunderbolt/path.c b/drivers/thunderbolt/path.c
[]
> @@ -503,8 +503,8 @@ int tb_path_activate(struct tb_path *path)
> & out_mask;
> hop.unknown3 = 0;
>
> - tb_port_info(path->hops[i].in_port, "Writing hop %d, index %d",
> - i, path->hops[i].in_hop_index);
> + tb_port_dbg(path->hops[i].in_port, "Writing hop %d, index %d",
> + i, path->hops[i].in_hop_index);

Appears to miss a '\n' termination

2019-03-28 16:57:23

by Mika Westerberg

Subject: Re: [PATCH v3 00/36] thunderbolt: Software connection manager improvements

On Thu, Mar 28, 2019 at 03:17:57PM +0000, [email protected] wrote:
> > -----Original Message-----
> > From: Mika Westerberg <[email protected]>
> > Sent: Thursday, March 28, 2019 7:36 AM
> > To: [email protected]
> > Cc: Michael Jamet; Yehezkel Bernat; Andreas Noever; Lukas Wunner; David S .
> > Miller; Andy Shevchenko; Christian Kellner; Limonciello, Mario; Mika Westerberg;
> > [email protected]
> > Subject: [PATCH v3 00/36] thunderbolt: Software connection manager
> > improvements
> >
> >
> > [EXTERNAL EMAIL]
> >
> > Hi,
> >
> > This is third iteration of the patch series intending to bring same kind of
> > functionality for older Apple systems than we have in PCs. Software
> > connection manager is used on Apple hardware with Light Ridge, Cactus Ridge
> > or Falcon Ridge controllers to create PCIe tunnels when a Thunderbolt
> > device is connected. Currently only one PCIe tunnel is supported. On newer
> > Alpine Ridge based Apple systems the driver starts the firmware which then
> > takes care creating tunnels.
> >
> > This series improves the software connection manager so that it will
> > support:
> >
> > - Full PCIe daisy chains (up to 6 devices)
> > - Display Port tunneling
> > - P2P networking
> >
> > We also add support for Titan Ridge based Apple systems where we can use
> > the same flows than with Alpine Ridge to start the firmware.
>
> It seems to me that there would be an expectation that PC system firmware and TBT controller
> firmware is configured to behave like Apple systems to use this SW connection manager
> instead of the ICM in AR/TR FW.
>
> Is there an intent to eventually offer a way to "side-step" the TBT ICM and try to use this instead
> without firmware support?

Yes, that's the intention.

> >
> > This applies on top of thunderbolt.git/next.
> >
> > Christian, Mario do you see any issues with patch [05/36] regarding bolt
> > and fwupd? The kernel is supposed to restart the syscall automatically so
> > userspace should not be affected but wanted to check with you.
>
> I don't see a problem for fwupd in this area.

OK, thanks for checking.

> > Previous version of the patch series can be viewed here:
> >
> > v2: https://lkml.org/lkml/2019/2/6/347
> > v1: https://lkml.org/lkml/2019/1/29/924
> >
> > Making v3 took longer than I anticipated mostly due to some issues I run
> > during testing the new changes. There are quite many changes so I dropped
> > the reviewed-by tags I got for v2. Below is the list of major changes from
> > the previous version:
> >
> > * Always set port->remote even in case of dual link connection.
> >
> > * Leave (DP, PCIe) tunnels up when the driver is unloaded. When loaded
> > back, it discovers the existing tunnels and updated data structures
> > accordingly. I noticed that the code in v2 did not support cases
> > properly when you unplug something before the driver gets loaded back.
> > This version tears down partial paths during discovery.
> >
> > * Do not automatically create PCIe tunnels. Instead we implement "user"
> > security level in the software connection manager as well taking
> > advantage of the existing sysfs interfaces. This allows user to disable
> > PCIe tunneling completely or implement different white listing
> > policies. Major distros include bolt system daemon that takes care of
> > this.
>
> This is a bit unfortunate. Is this because of IOMMU limitations in working
> with devices down the chain?

No, it just makes it possible to do things such as "disable all PCIe
tunneling", like the master switch we have in GNOME UI. Even if you have
full IOMMU support it still does not prevent misbehaving devices.

This also allows other kind of whitelisting like supporting devices from
certain "known" vendor only.

IOMMU is still the primary protection against DMA attacks.

2019-03-30 09:56:53

by Lukas Wunner

Subject: Re: [PATCH v3 03/36] thunderbolt: Drop duplicated get_switch_by_route()

On Thu, Mar 28, 2019 at 03:36:00PM +0300, Mika Westerberg wrote:
> tb_switch_find_by_route() does the same already so use it instead and
> remove duplicated get_switch_by_route().
^^
"at" route (also in subject)

Apart from that,
Reviewed-by: Lukas Wunner <[email protected]>

2019-03-31 15:25:59

by Lukas Wunner

Subject: Re: [PATCH v3 12/36] thunderbolt: Properly disable path

On Thu, Mar 28, 2019 at 03:36:09PM +0300, Mika Westerberg wrote:
> +static int __tb_path_deactivate_hop(struct tb_port *port, int hop_index)
> +{
> + struct tb_regs_hop hop;
> + ktime_t timeout;
> + int ret;
> +
> + if (port->sw->is_unplugged)
> + return 0;

This check is basically a duplication of the checks added in patch 4 ...


> +
> + /* Disable the path */
> + ret = tb_port_read(port, &hop, TB_CFG_HOPS, 2 * hop_index, 2);
> + if (ret)
> + return ret;
[...]
> static void __tb_path_deactivate_hops(struct tb_path *path, int first_hop)
> {
> int i, res;
> - struct tb_regs_hop hop = { };
> +
> for (i = first_hop; i < path->path_length; i++) {
> - res = tb_port_write(path->hops[i].in_port, &hop, TB_CFG_HOPS,
> - 2 * path->hops[i].in_hop_index, 2);
> + res = __tb_path_deactivate_hop(path->hops[i].in_port,
> + path->hops[i].in_hop_index);
> if (res)
> tb_port_warn(path->hops[i].in_port,
> "hop deactivation failed for hop %d, index %d\n",

... however you return 0 above to avoid the warning here whereas the checks
added in patch 4 return -ENODEV. You may want to change "if (res)" to
"if (res && res != -ENODEV)" and drop the unplugged check in
__tb_path_deactivate_hop().

Thanks,

Lukas

2019-04-01 04:32:04

by Lukas Wunner

Subject: Re: [PATCH v3 00/36] thunderbolt: Software connection manager improvements

On Thu, Mar 28, 2019 at 06:56:21PM +0200, Mika Westerberg wrote:
> On Thu, Mar 28, 2019 at 03:17:57PM +0000, [email protected] wrote:
> > > From: Mika Westerberg <[email protected]>
> > > Sent: Thursday, March 28, 2019 7:36 AM
> > > * Do not automatically create PCIe tunnels. Instead we implement "user"
> > > security level in the software connection manager as well taking
> > > advantage of the existing sysfs interfaces. This allows user to disable
> > > PCIe tunneling completely or implement different white listing
> > > policies. Major distros include bolt system daemon that takes care of
> > > this.
> >
> > This is a bit unfortunate. Is this because of IOMMU limitations in working
> > with devices down the chain?
>
> No, it just makes it possible to do things such as "disable all PCIe
> tunneling", like the master switch we have in GNOME UI.

It appears to be a change in behavior though as PCIe tunnels are
currently established automatically on Macs which don't use ICM.
The change might be considered breaking userspace, not sure.

Also on macOS it's just plug and play without any need to configure
whitelists or anything.

Thanks,

Lukas

2019-04-01 07:46:31

by Mika Westerberg

Subject: Re: [PATCH v3 00/36] thunderbolt: Software connection manager improvements

On Mon, Apr 01, 2019 at 06:31:11AM +0200, Lukas Wunner wrote:
> On Thu, Mar 28, 2019 at 06:56:21PM +0200, Mika Westerberg wrote:
> > On Thu, Mar 28, 2019 at 03:17:57PM +0000, [email protected] wrote:
> > > > From: Mika Westerberg <[email protected]>
> > > > Sent: Thursday, March 28, 2019 7:36 AM
> > > > * Do not automatically create PCIe tunnels. Instead we implement "user"
> > > > security level in the software connection manager as well taking
> > > > advantage of the existing sysfs interfaces. This allows user to disable
> > > > PCIe tunneling completely or implement different white listing
> > > > policies. Major distros include bolt system daemon that takes care of
> > > > this.
> > >
> > > This is a bit unfortunate. Is this because of IOMMU limitations in working
> > > with devices down the chain?
> >
> > No, it just makes it possible to do things such as "disable all PCIe
> > tunneling", like the master switch we have in GNOME UI.
>
> It appears to be a change in behavior though as PCIe tunnels are
> currently established automatically on Macs which don't use ICM.
> The change might be considered breaking userspace, not sure.

I don't think userspace is breaking here. The userspace interface we
provide is through sysfs and only change we do in this series that could
affect is in the patch 05. I Cc'd Mario and Christian just to make sure
fwupd and bolt still keep working (these are the two userspace
applications using the interface AFAIK).

I think you mean user experience instead. It should not affect because
if you run any major distro all the components including GNOME UI are
prepared to handle this.

> Also on macOS it's just plug and play without any need to configure
> whitelists or anything.

macOS does have whitelist too. If you plug in unsupported device you
don't get to use it and it is listed as "unsupported" in the system
report. I'm not sure if end users can tune the list.

In Linux side I would really like to make it possible to implement
whitelisting and disabling PCIe tunneling, and since we already have the
interface for that (the sysfs ABI) it only makes sense to use it. That
interface allows userspace to implement different kinds of policies from
disabling all PCIe tunneling to allowing everything (it can be done
using simple udev rule).

2019-04-04 19:29:50

by Lukas Wunner

Subject: Re: [PATCH v3 17/36] thunderbolt: Assign remote for both ports in case of dual link

On Thu, Mar 28, 2019 at 03:36:14PM +0300, Mika Westerberg wrote:
> +/**
> + * tb_port_has_remote() - Does the port have switch connected downstream
> + * @port: Port to check
> + *
> + * Returns true only when the port is primary port and that it has
> + * remote set.
> + */

Just a nit: s/that it//

Otherwise this patch LGTM.

Thanks,

Lukas

2019-04-04 19:40:57

by Lukas Wunner

Subject: Re: [PATCH v3 18/36] thunderbolt: Add helper function to iterate from one port to another

On Thu, Mar 28, 2019 at 03:36:15PM +0300, Mika Westerberg wrote:
> We need to be able to walk from one port to another when we are creating
> paths where there are multiple switches between two ports. For this
> reason introduce a new function tb_next_port_on_path().
>
> Signed-off-by: Mika Westerberg <[email protected]>

Reviewed-by: Lukas Wunner <[email protected]>

The algorithm in this patch looks clear and concise now.

Thanks,

Lukas

2019-04-07 16:55:55

by Lukas Wunner

Subject: Re: [PATCH v3 19/36] thunderbolt: Extend tunnel creation to more than 2 adjacent switches

On Thu, Mar 28, 2019 at 03:36:16PM +0300, Mika Westerberg wrote:
> +struct tb_path *tb_path_alloc(struct tb *tb, struct tb_port *src, int src_hopid,
> + struct tb_port *dst, int dst_hopid, int link_nr,
> + const char *name)
> {
[...]
> + in_hopid = src_hopid;
> + out_port = NULL;
> +
> + for (i = 0; i < num_hops; i++) {
> + in_port = tb_next_port_on_path(src, dst, out_port);
> + if (!in_port)
> + goto err;
> +
> + if (in_port->dual_link_port && in_port->link_nr != link_nr)
> + in_port = in_port->dual_link_port;
> +
> + ret = tb_port_alloc_in_hopid(in_port, in_hopid, -1);
> + if (ret < 0)
> + goto err;
> + in_hopid = ret;
> +
> + out_port = tb_next_port_on_path(src, dst, in_port);
> + if (!out_port)
> + goto err;
> +
> + if (out_port->dual_link_port && out_port->link_nr != link_nr)
> + out_port = out_port->dual_link_port;
> +
> + if (i == num_hops - 1)
> + ret = tb_port_alloc_out_hopid(out_port, dst_hopid,
> + dst_hopid);
> + else
> + ret = tb_port_alloc_out_hopid(out_port, -1, -1);
> +
> + if (ret < 0)
> + goto err;
> + out_hopid = ret;
> +
> + path->hops[i].in_hop_index = in_hopid;
> + path->hops[i].in_port = in_port;
> + path->hops[i].in_counter_index = -1;
> + path->hops[i].out_port = out_port;
> + path->hops[i].next_hop_index = out_hopid;
> +
> + in_hopid = out_hopid;
> + }

According to the code comment in struct tb_regs_hop (in tb_regs.h),
the out_hopid ("next_hop" in struct tb_regs_hop) denotes the
"hop to take after sending the packet through out_port (on the
incoming port of the next switch)".

So intuitively, the hop config space is like a routing table and
the entry in in_port's hop config space specifies through which
out_port the packets shall be routed, and which entry to look up
on the remote port reachable through out_port.

This means that the out_hopid must always be identical to the in_hopid
of out_port->remote. Otherwise the routing wouldn't work.

And yet, you've introduced *two* struct ida for each port in
patch 16. This doesn't seem to make sense: The out_hopids ida
of a port always has to be identical to the in_hopids ida of that
port's remote. But if it's identical, why does it have to exist
twice?

Also, the above algorithm fails to ensure that the two struct ida
are always identical: It uses the out_hopid on the previous switch
as *minimum* for the in_hopid on the current switch. If that hopid
is already taken by an existing tunnel, tb_port_alloc_in_hopid()
will allocate a *different* hopid and thereby break the routing.

So either the code comment in struct tb_regs_hop is wrong, or this
algorithm and the duplicate struct ida in patch 16 are wrong, or I'm
missing something.

Thanks,

Lukas

2019-04-08 07:37:23

by Mika Westerberg

Subject: Re: [PATCH v3 19/36] thunderbolt: Extend tunnel creation to more than 2 adjacent switches

On Sun, Apr 07, 2019 at 06:54:25PM +0200, Lukas Wunner wrote:
> On Thu, Mar 28, 2019 at 03:36:16PM +0300, Mika Westerberg wrote:
> > +struct tb_path *tb_path_alloc(struct tb *tb, struct tb_port *src, int src_hopid,
> > + struct tb_port *dst, int dst_hopid, int link_nr,
> > + const char *name)
> > {
> [...]
> > + in_hopid = src_hopid;
> > + out_port = NULL;
> > +
> > + for (i = 0; i < num_hops; i++) {
> > + in_port = tb_next_port_on_path(src, dst, out_port);
> > + if (!in_port)
> > + goto err;
> > +
> > + if (in_port->dual_link_port && in_port->link_nr != link_nr)
> > + in_port = in_port->dual_link_port;
> > +
> > + ret = tb_port_alloc_in_hopid(in_port, in_hopid, -1);
> > + if (ret < 0)
> > + goto err;
> > + in_hopid = ret;
> > +
> > + out_port = tb_next_port_on_path(src, dst, in_port);
> > + if (!out_port)
> > + goto err;
> > +
> > + if (out_port->dual_link_port && out_port->link_nr != link_nr)
> > + out_port = out_port->dual_link_port;
> > +
> > + if (i == num_hops - 1)
> > + ret = tb_port_alloc_out_hopid(out_port, dst_hopid,
> > + dst_hopid);
> > + else
> > + ret = tb_port_alloc_out_hopid(out_port, -1, -1);
> > +
> > + if (ret < 0)
> > + goto err;
> > + out_hopid = ret;
> > +
> > + path->hops[i].in_hop_index = in_hopid;
> > + path->hops[i].in_port = in_port;
> > + path->hops[i].in_counter_index = -1;
> > + path->hops[i].out_port = out_port;
> > + path->hops[i].next_hop_index = out_hopid;
> > +
> > + in_hopid = out_hopid;
> > + }
>
> According to the code comment in struct tb_regs_hop (in tb_regs.h),
> the out_hopid ("next_hop" in struct tb_regs_hop) denotes the
> "hop to take after sending the packet through out_port (on the
> incoming port of the next switch)".
>
> So intuitively, the hop config space is like a routing table and
> the entry in in_port's hop config space specifies through which
> out_port the packets shall be routed, and which entry to look up
> on the remote port reachable through out_port.
>
> This means that the out_hopid must always be identical to the in_hopid
> of out_port->remote. Otherwise the routing wouldn't work.
>
> And yet, you've introduced *two* struct ida for each port in
> patch 16. This doesn't seem to make sense: The out_hopids ida
> of a port always has to be identical to the in_hopids ida of that
> port's remote. But if it's identical, why does it have to exist
> twice?

The reason for two HopID allocators (struct idas) is to make it possible
to track HopIDs to each direction. The same port can be output for one
path and input for another. I'm not sure how that can be done without
having two struct idas per port.

You are right, in case of out port HopID connecter to remote in port,
they should use the same HopID.

> Also, the above algorithm fails to ensure that the two struct ida
> are always identical: It uses the out_hopid on the previous switch
> as *minimum* for the in_hopid on the current switch. If that hopid
> is already taken by an existing tunnel, tb_port_alloc_in_hopid()
> will allocate a *different* hopid and thereby break the routing.
>
> So either the code comment in struct tb_regs_hop is wrong, or this
> algorithm and the duplicate struct ida in patch 16 are wrong, or I'm
> missing something.

No you are right. I think the above code should look like:

ret = tb_port_alloc_in_hopid(in_port, in_hopid, in_hopid);

instead of

ret = tb_port_alloc_in_hopid(in_port, in_hopid, -1);

to make sure out port and in port of a remote use the same HopID. Will
fix.