thunderbolt: Add support for Intel Titan Ridge

PATCH In review Mika Westerberg 1 participant 20 messages Mar 2, 2018 – Mar 9, 2018

Subsystems (2)

Documentation Thunderbolt driver

lore

Share & tools

lore

Mika Westerberg

author

2018-03-02 12:43 UTC #0 lore raw

v3 cover thunderbolt: Add support for Intel Titan Ridge

Hi,

This series adds support for Intel Titan Ridge Thunderbolt controller. Titan Ridge is the next generation Thunderbolt 3 controller and successor of Alpine Ridge.

In addition to fixes and Titan Ridge support this series adds following:

- USB only security level (SL4).

- A new attribute for devices telling whether they were connected automatically during boot.

- Preboot ACL allows userspace to specify a list of devices (based on device unique_id) that the firmware automatically connects during boot.

The previous versions of the patch series can be seen here:

v2: https://lwn.net/Articles/748030/
v1: https://www.spinics.net/lists/kernel/msg2721243.html

Changes from v2:

- Change wording of the message when Thunderbolt device is rejected by the firmware to be more clear what the reason for the rejection might be. Patches [7/18 and 18/18].
- Use GENMASK() also in ICM_AR_INFO_SLEVEL_MASK in patch [18/18].
- Added Andy's tag to patch [18/18].

Changes from v1:

- Add stable tag to patch [6/18].
- Use tb_xdomain_get() instead of open coding it in patch [12/18].
- If ->get_boot_acl() fails unlock and goto out immediately (follow more

    standard pattern) in patch [12/18].

- Make newline optional in boot_acl_store() in patch [12/18].
- Update Documentation/admin-guide/thunderbolt.rst to show the new "usbonly" security level in patch [17/18].
- Order DMA ports from 3 to 7 in patch [18/18].
- Use 0xffULL instead in get_parent_route() in patch [18/18].
- Sort Titan Ridge PCI IDs numerically in patch [18/18].
- Use GENMASK() in patch [18/18].
- Added Andy's tag to patches 1-17.

Mika Westerberg (13):
thunderbolt: Resume control channel after hibernation image is created
thunderbolt: Serialize PCIe tunnel creation with PCI rescan
thunderbolt: Handle connecting device in place of host properly
thunderbolt: Do not overwrite error code when domain adding fails
thunderbolt: Wait a bit longer for root switch config space
thunderbolt: Wait a bit longer for ICM to authenticate the active NVM
thunderbolt: Handle rejected Thunderbolt devices
thunderbolt: Factor common ICM add and update operations out
thunderbolt: Add tb_switch_get()
thunderbolt: Add constant for approval timeout
thunderbolt: Move driver ready handling to struct icm
thunderbolt: Add support for preboot ACL
thunderbolt: Introduce USB only (SL4) security level

Radion Mirchevsky (4):
thunderbolt: Correct function name in kernel-doc comment
thunderbolt: Add tb_switch_find_by_route()
thunderbolt: Add tb_xdomain_find_by_route()
thunderbolt: Add support for Intel Titan Ridge

Yehezkel Bernat (1):
thunderbolt: Add 'boot' attribute for devices

 Documentation/ABI/testing/sysfs-bus-thunderbolt |  33 +
 Documentation/admin-guide/thunderbolt.rst       |  15 +-
 drivers/thunderbolt/dma_port.c                  |  28 +-
 drivers/thunderbolt/domain.c                    | 130 +++-
 drivers/thunderbolt/icm.c                       | 761 +++++++++++++++++++++---
 drivers/thunderbolt/nhi.c                       |   5 +-
 drivers/thunderbolt/nhi.h                       |   5 +
 drivers/thunderbolt/switch.c                    |  61 +-
 drivers/thunderbolt/tb.h                        |  14 +
 drivers/thunderbolt/tb_msgs.h                   | 180 +++++-
 drivers/thunderbolt/xdomain.c                   |  47 +-
 include/linux/thunderbolt.h                     |  19 +
 12 files changed, 1183 insertions(+), 115 deletions(-)

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:39 UTC #1 lore raw

PATCH v3 1/18 thunderbolt: Resume control channel after hibernation image is created

The driver misses implementation of PM hook that undoes what ->freeze_noirq() does after the hibernation image is created. This means the control channel is not resumed properly and the Thunderbolt bus becomes useless in later stages of hibernation (when the image is stored or if the operation fails).

Fix this by pointing ->thaw_noirq to driver nhi_resume_noirq(). This makes sure the control channel is resumed properly.

Fixes 23dd5bb49d98 ("thunderbolt: Add suspend/hibernate support")

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

Cc [email protected]

---
 drivers/thunderbolt/nhi.c | 1 +
 1 file changed, 1 insertion(+)

drivers/thunderbolt/nhi.c +1 -0

index f45bcbc63738..80c33c7404f5 100644
@@ -1064,6 +1064,7 @@ static const struct dev_pm_ops nhi_pm_ops = {
 					    * we just disable hotplug, the
 					    * pci-tunnels stay alive.
 					    */
+	.thaw_noirq = nhi_resume_noirq,
 	.restore_noirq = nhi_resume_noirq,
 	.suspend = nhi_suspend,
 	.freeze = nhi_suspend,

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:44 UTC #2 lore raw

PATCH v3 2/18 thunderbolt: Serialize PCIe tunnel creation with PCI rescan

In reply to Mika Westerberg

We need to make sure a new PCIe tunnel is not created in a middle of previous PCI rescan because otherwise the rescan code might find too much and fail to reconfigure devices properly. This is important when native PCIe hotplug is used. In BIOS assisted hotplug there should be no such issue.

Fixes f67cf491175a ("thunderbolt: Add support for Internal Connection Manager (ICM)")

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

Cc Bjorn Helgaas <[email protected]>

Cc [email protected]

---
 drivers/thunderbolt/switch.c | 9 +++++++++
 1 file changed, 9 insertions(+)

drivers/thunderbolt/switch.c +9 -0

index da54ace4dd2f..1cc79785ce42 100644
@@ -716,6 +716,13 @@ static int tb_switch_set_authorized(struct tb_switch *sw, unsigned int val)
 	if (sw->authorized)
 		goto unlock;
 
+	/*
+	 * Make sure there is no PCIe rescan ongoing when a new PCIe
+	 * tunnel is created. Otherwise the PCIe rescan code might find
+	 * the new tunnel too early.
+	 */
+	pci_lock_rescan_remove();
+
 	switch (val) {
 	/* Approve switch */
 	case 1:

@@ -735,6 +742,8 @@ static int tb_switch_set_authorized(struct tb_switch *sw, unsigned int val)
 		break;
 	}
 
+	pci_unlock_rescan_remove();
+
 	if (!ret) {
 		sw->authorized = val;
 		/* Notify status change to the userspace */

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:43 UTC #3 lore raw

PATCH v3 3/18 thunderbolt: Handle connecting device in place of host properly

In reply to Mika Westerberg

If the system is suspended and user disconnects cable to another host and connects it to a Thunderbolt device instead we get a warning from driver core about adding duplicate sysfs attribute and adding the new device fails.

Handle this properly so that we first remove the existing XDomain connection before adding new devices.

Fixes d1ff70241a27 ("thunderbolt: Add support for XDomain discovery protocol")

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

Cc [email protected]

---
 drivers/thunderbolt/icm.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

drivers/thunderbolt/icm.c +17 -9

index ab02d13f40b7..182226023acb 100644
@@ -383,6 +383,15 @@ static void remove_switch(struct tb_switch *sw)
 	tb_switch_remove(sw);
 }
 
+static void remove_xdomain(struct tb_xdomain *xd)
+{
+	struct tb_switch *sw;
+
+	sw = tb_to_switch(xd->dev.parent);
+	tb_port_at(xd->route, sw)->xdomain = NULL;
+	tb_xdomain_remove(xd);
+}
+
 static void
 icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
 {

@@ -391,6 +400,7 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
 	struct tb_switch *sw, *parent_sw;
 	struct icm *icm = tb_priv(tb);
 	bool authorized = false;
+	struct tb_xdomain *xd;
 	u8 link, depth;
 	u64 route;
 	int ret;

@@ -467,6 +477,13 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
 		tb_switch_put(sw);
 	}
 
+	/* Remove existing XDomain connection if found */
+	xd = tb_xdomain_find_by_link_depth(tb, link, depth);
+	if (xd) {
+		remove_xdomain(xd);
+		tb_xdomain_put(xd);
+	}
+
 	parent_sw = tb_switch_find_by_link_depth(tb, link, depth - 1);
 	if (!parent_sw) {
 		tb_err(tb, "failed to find parent switch for %u.%u\n",

@@ -529,15 +546,6 @@ icm_fr_device_disconnected(struct tb *tb, const struct icm_pkg_header *hdr)
 	tb_switch_put(sw);
 }
 
-static void remove_xdomain(struct tb_xdomain *xd)
-{
-	struct tb_switch *sw;
-
-	sw = tb_to_switch(xd->dev.parent);
-	tb_port_at(xd->route, sw)->xdomain = NULL;
-	tb_xdomain_remove(xd);
-}
-
 static void
 icm_fr_xdomain_connected(struct tb *tb, const struct icm_pkg_header *hdr)
 {

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:38 UTC #4 lore raw

PATCH v3 4/18 thunderbolt: Do not overwrite error code when domain adding fails

In reply to Mika Westerberg

If the Thunderbolt domain adding fails for some reason we currently always return -EIO instead of the real error code. To make debugging easier return the actual error code instead.

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/thunderbolt/nhi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/thunderbolt/nhi.c +1 -1

index 80c33c7404f5..9e58d09f6029 100644
@@ -1036,7 +1036,7 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		 */
 		tb_domain_put(tb);
 		nhi_shutdown(nhi);
-		return -EIO;
+		return res;
 	}
 	pci_set_drvdata(pdev, tb);

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:39 UTC #5 lore raw

PATCH v3 5/18 thunderbolt: Wait a bit longer for root switch config space

In reply to Mika Westerberg

In some case reading root switch config space takes longer than what we are currently waiting in the driver resulting timeout and failure. Increase number of retries to allow some more time for the root switch config space to become accesssible.

Also log an error if the timeout is exceeded so we know why the driver probe failed.

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/thunderbolt/icm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/thunderbolt/icm.c +2 -1

index 182226023acb..1183321586c5 100644
@@ -828,7 +828,7 @@ __icm_driver_ready(struct tb *tb, enum tb_security_level *security_level)
 	struct icm_pkg_driver_ready request = {
 		.hdr.code = ICM_DRIVER_READY,
 	};
-	unsigned int retries = 10;
+	unsigned int retries = 50;
 	int ret;
 
 	memset(&reply, 0, sizeof(reply));

@@ -856,6 +856,7 @@ __icm_driver_ready(struct tb *tb, enum tb_security_level *security_level)
 		msleep(50);
 	} while (--retries);
 
+	tb_err(tb, "failed to read root switch config space, giving up\n");
 	return -ETIMEDOUT;
 }

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:39 UTC #6 lore raw

PATCH v3 6/18 thunderbolt: Wait a bit longer for ICM to authenticate the active NVM

In reply to Mika Westerberg

Sometimes during cold boot ICM has not yet authenticated the active NVM image leading to timeout and failing the driver probe. Allow ICM to take some more time and increase the timeout to 3 seconds before we give up.

While there fix icm_firmware_init() to return the real error code without overwriting it with -ENODEV.

Fixes f67cf491175a ("thunderbolt: Add support for Internal Connection Manager (ICM)")

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

Cc [email protected]

---
 drivers/thunderbolt/icm.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/thunderbolt/icm.c +5 -2

index 1183321586c5..611d28e8e5f2 100644
@@ -736,14 +736,14 @@ static bool icm_ar_is_supported(struct tb *tb)
 static int icm_ar_get_mode(struct tb *tb)
 {
 	struct tb_nhi *nhi = tb->nhi;
-	int retries = 5;
+	int retries = 60;
 	u32 val;
 
 	do {
 		val = ioread32(nhi->iobase + REG_FW_STS);
 		if (val & REG_FW_STS_NVM_AUTH_DONE)
 			break;
-		msleep(30);
+		msleep(50);
 	} while (--retries);
 
 	if (!retries) {

@@ -1063,6 +1063,9 @@ static int icm_firmware_init(struct tb *tb)
 			break;
 
 		default:
+			if (ret < 0)
+				return ret;
+
 			tb_err(tb, "ICM firmware is in wrong mode: %u\n", ret);
 			return -ENODEV;
 		}

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:39 UTC #7 lore raw

PATCH v3 7/18 thunderbolt: Handle rejected Thunderbolt devices

In reply to Mika Westerberg

The ICM firmware rejects devices if the maximum topology limit is exceeded (more than 6 devices are connected). If that happens just log a message to the kernel message buffer and bail out.

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/thunderbolt/icm.c     | 6 ++++++
 drivers/thunderbolt/tb_msgs.h | 1 +
 2 files changed, 7 insertions(+)

drivers/thunderbolt/icm.c +6 -0

index 611d28e8e5f2..6d7f8262f864 100644
@@ -410,6 +410,12 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
 		ICM_LINK_INFO_DEPTH_SHIFT;
 	authorized = pkg->link_info & ICM_LINK_INFO_APPROVED;
 
+	if (pkg->link_info & ICM_LINK_INFO_REJECTED) {
+		tb_info(tb, "switch at %u.%u was rejected by ICM firmware because topology limit exceeded\n",
+			link, depth);
+		return;
+	}
+
 	ret = icm->get_route(tb, link, depth, &route);
 	if (ret) {
 		tb_err(tb, "failed to find route string for switch at %u.%u\n",

drivers/thunderbolt/tb_msgs.h +1 -0

index b0a092baa605..476bc04cac6c 100644
@@ -176,6 +176,7 @@ struct icm_fr_event_device_connected {
 #define ICM_LINK_INFO_DEPTH_SHIFT	4
 #define ICM_LINK_INFO_DEPTH_MASK	GENMASK(7, 4)
 #define ICM_LINK_INFO_APPROVED		BIT(8)
+#define ICM_LINK_INFO_REJECTED		BIT(9)
 
 struct icm_fr_pkg_approve_device {
 	struct icm_pkg_header hdr;

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:39 UTC #8 lore raw

PATCH v3 8/18 thunderbolt: Factor common ICM add and update operations out

In reply to Mika Westerberg

The newer ICM will not use link and depth to address devices. Instead it uses route strings. In order to take advantage of the existing code factor out common operations so that we can use the same functions with the new ICM as well.

No functional changes intended.

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/thunderbolt/icm.c | 136 +++++++++++++++++++++++++++++-----------------
 1 file changed, 87 insertions(+), 49 deletions(-)

drivers/thunderbolt/icm.c +87 ~~-49~~

index 6d7f8262f864..6798516d6474 100644
@@ -374,6 +374,57 @@ static int icm_fr_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd)
 	return 0;
 }
 
+static void add_switch(struct tb_switch *parent_sw, u64 route,
+		       const uuid_t *uuid, u8 connection_id, u8 connection_key,
+		       u8 link, u8 depth, enum tb_security_level security_level,
+		       bool authorized)
+{
+	struct tb_switch *sw;
+
+	sw = tb_switch_alloc(parent_sw->tb, &parent_sw->dev, route);
+	if (!sw)
+		return;
+
+	sw->uuid = kmemdup(uuid, sizeof(*uuid), GFP_KERNEL);
+	sw->connection_id = connection_id;
+	sw->connection_key = connection_key;
+	sw->link = link;
+	sw->depth = depth;
+	sw->authorized = authorized;
+	sw->security_level = security_level;
+
+	/* Link the two switches now */
+	tb_port_at(route, parent_sw)->remote = tb_upstream_port(sw);
+	tb_upstream_port(sw)->remote = tb_port_at(route, parent_sw);
+
+	if (tb_switch_add(sw)) {
+		tb_port_at(tb_route(sw), parent_sw)->remote = NULL;
+		tb_switch_put(sw);
+		return;
+	}
+}
+
+static void update_switch(struct tb_switch *parent_sw, struct tb_switch *sw,
+			  u64 route, u8 connection_id, u8 connection_key,
+			  u8 link, u8 depth)
+{
+	/* Disconnect from parent */
+	tb_port_at(tb_route(sw), parent_sw)->remote = NULL;
+	/* Re-connect via updated port*/
+	tb_port_at(route, parent_sw)->remote = tb_upstream_port(sw);
+
+	/* Update with the new addressing information */
+	sw->config.route_hi = upper_32_bits(route);
+	sw->config.route_lo = lower_32_bits(route);
+	sw->connection_id = connection_id;
+	sw->connection_key = connection_key;
+	sw->link = link;
+	sw->depth = depth;
+
+	/* This switch still exists */
+	sw->is_unplugged = false;
+}
+
 static void remove_switch(struct tb_switch *sw)
 {
 	struct tb_switch *parent_sw;

@@ -383,6 +434,31 @@ static void remove_switch(struct tb_switch *sw)
 	tb_switch_remove(sw);
 }
 
+static void add_xdomain(struct tb_switch *sw, u64 route,
+			const uuid_t *local_uuid, const uuid_t *remote_uuid,
+			u8 link, u8 depth)
+{
+	struct tb_xdomain *xd;
+
+	xd = tb_xdomain_alloc(sw->tb, &sw->dev, route, local_uuid, remote_uuid);
+	if (!xd)
+		return;
+
+	xd->link = link;
+	xd->depth = depth;
+
+	tb_port_at(route, sw)->xdomain = xd;
+
+	tb_xdomain_add(xd);
+}
+
+static void update_xdomain(struct tb_xdomain *xd, u64 route, u8 link)
+{
+	xd->link = link;
+	xd->route = route;
+	xd->is_unplugged = false;
+}
+
 static void remove_xdomain(struct tb_xdomain *xd)
 {
 	struct tb_switch *sw;

@@ -397,6 +473,7 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
 {
 	const struct icm_fr_event_device_connected *pkg =
 		(const struct icm_fr_event_device_connected *)hdr;
+	enum tb_security_level security_level;
 	struct tb_switch *sw, *parent_sw;
 	struct icm *icm = tb_priv(tb);
 	bool authorized = false;

@@ -409,6 +486,8 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
 	depth = (pkg->link_info & ICM_LINK_INFO_DEPTH_MASK) >>
 		ICM_LINK_INFO_DEPTH_SHIFT;
 	authorized = pkg->link_info & ICM_LINK_INFO_APPROVED;
+	security_level = (pkg->hdr.flags & ICM_FLAGS_SLEVEL_MASK) >>
+			 ICM_FLAGS_SLEVEL_SHIFT;
 
 	if (pkg->link_info & ICM_LINK_INFO_REJECTED) {
 		tb_info(tb, "switch at %u.%u was rejected by ICM firmware because topology limit exceeded\n",

@@ -441,16 +520,8 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
 		 */
 		if (sw->depth == depth && sw_phy_port == phy_port &&
 		    !!sw->authorized == authorized) {
-			tb_port_at(tb_route(sw), parent_sw)->remote = NULL;
-			tb_port_at(route, parent_sw)->remote =
-				   tb_upstream_port(sw);
-			sw->config.route_hi = upper_32_bits(route);
-			sw->config.route_lo = lower_32_bits(route);
-			sw->connection_id = pkg->connection_id;
-			sw->connection_key = pkg->connection_key;
-			sw->link = link;
-			sw->depth = depth;
-			sw->is_unplugged = false;
+			update_switch(parent_sw, sw, route, pkg->connection_id,
+				      pkg->connection_key, link, depth);
 			tb_switch_put(sw);
 			return;
 		}

@@ -497,30 +568,10 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
 		return;
 	}
 
-	sw = tb_switch_alloc(tb, &parent_sw->dev, route);
-	if (!sw) {
-		tb_switch_put(parent_sw);
-		return;
-	}
+	add_switch(parent_sw, route, &pkg->ep_uuid, pkg->connection_id,
+		   pkg->connection_key, link, depth, security_level,
+		   authorized);
 
-	sw->uuid = kmemdup(&pkg->ep_uuid, sizeof(pkg->ep_uuid), GFP_KERNEL);
-	sw->connection_id = pkg->connection_id;
-	sw->connection_key = pkg->connection_key;
-	sw->link = link;
-	sw->depth = depth;
-	sw->authorized = authorized;
-	sw->security_level = (pkg->hdr.flags & ICM_FLAGS_SLEVEL_MASK) >>
-				ICM_FLAGS_SLEVEL_SHIFT;
-
-	/* Link the two switches now */
-	tb_port_at(route, parent_sw)->remote = tb_upstream_port(sw);
-	tb_upstream_port(sw)->remote = tb_port_at(route, parent_sw);
-
-	ret = tb_switch_add(sw);
-	if (ret) {
-		tb_port_at(tb_route(sw), parent_sw)->remote = NULL;
-		tb_switch_put(sw);
-	}
 	tb_switch_put(parent_sw);
 }

@@ -591,9 +642,7 @@ icm_fr_xdomain_connected(struct tb *tb, const struct icm_pkg_header *hdr)
 		phy_port = phy_port_from_route(route, depth);
 
 		if (xd->depth == depth && xd_phy_port == phy_port) {
-			xd->link = link;
-			xd->route = route;
-			xd->is_unplugged = false;
+			update_xdomain(xd, route, link);
 			tb_xdomain_put(xd);
 			return;
 		}

@@ -643,19 +692,8 @@ icm_fr_xdomain_connected(struct tb *tb, const struct icm_pkg_header *hdr)
 		return;
 	}
 
-	xd = tb_xdomain_alloc(sw->tb, &sw->dev, route,
-			      &pkg->local_uuid, &pkg->remote_uuid);
-	if (!xd) {
-		tb_switch_put(sw);
-		return;
-	}
-
-	xd->link = link;
-	xd->depth = depth;
-
-	tb_port_at(route, sw)->xdomain = xd;
-
-	tb_xdomain_add(xd);
+	add_xdomain(sw, route, &pkg->local_uuid, &pkg->remote_uuid, link,
+		    depth);
 	tb_switch_put(sw);
 }

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:41 UTC #9 lore raw

PATCH v3 9/18 thunderbolt: Correct function name in kernel-doc comment

In reply to Mika Westerberg

From: Radion Mirchevsky <[email protected]>

Use correct name in kernel-doc of tb_switch_find_by_uuid().

Signed-off-by Radion Mirchevsky <[email protected]>

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/thunderbolt/switch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/thunderbolt/switch.c +1 -1

index 1cc79785ce42..2a0bded44922 100644
@@ -1528,7 +1528,7 @@ struct tb_switch *tb_switch_find_by_link_depth(struct tb *tb, u8 link, u8 depth)
 }
 
 /**
- * tb_switch_find_by_link_depth() - Find switch by UUID
+ * tb_switch_find_by_uuid() - Find switch by UUID
  * @tb: Domain the switch belongs
  * @uuid: UUID to look for
  *

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:41 UTC #10 lore raw

PATCH v3 10/18 thunderbolt: Add tb_switch_get()

In reply to Mika Westerberg

Sometimes there is need for increasing reference count of a switch as well. This also follows what we have for xdomains.

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/thunderbolt/tb.h | 7 +++++++
 1 file changed, 7 insertions(+)

drivers/thunderbolt/tb.h +7 -0

index 895c57a0a090..c7bd77e9c2c3 100644
@@ -387,6 +387,13 @@ 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);
 
+static inline struct tb_switch *tb_switch_get(struct tb_switch *sw)
+{
+	if (sw)
+		get_device(&sw->dev);
+	return sw;
+}
+
 static inline void tb_switch_put(struct tb_switch *sw)
 {
 	put_device(&sw->dev);

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:39 UTC #11 lore raw

PATCH v3 11/18 thunderbolt: Add tb_switch_find_by_route()

In reply to Mika Westerberg

From: Radion Mirchevsky <[email protected]>

With the new ICM messaging there is need for find switch by route string instead of link and depth. Add new function that makes it possible.

Signed-off-by Radion Mirchevsky <[email protected]>

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/thunderbolt/switch.c | 33 +++++++++++++++++++++++++++++++++
 drivers/thunderbolt/tb.h     |  1 +
 2 files changed, 34 insertions(+)

drivers/thunderbolt/switch.c +33 -0

index 2a0bded44922..4e2b2097bbfc 100644
@@ -1479,6 +1479,7 @@ struct tb_sw_lookup {
 	u8 link;
 	u8 depth;
 	const uuid_t *uuid;
+	u64 route;
 };
 
 static int tb_switch_match(struct device *dev, void *data)

@@ -1494,6 +1495,11 @@ static int tb_switch_match(struct device *dev, void *data)
 	if (lookup->uuid)
 		return !memcmp(sw->uuid, lookup->uuid, sizeof(*lookup->uuid));
 
+	if (lookup->route) {
+		return sw->config.route_lo == lower_32_bits(lookup->route) &&
+		       sw->config.route_hi == upper_32_bits(lookup->route);
+	}
+
 	/* Root switch is matched only by depth */
 	if (!lookup->depth)
 		return !sw->depth;

@@ -1551,6 +1557,33 @@ struct tb_switch *tb_switch_find_by_uuid(struct tb *tb, const uuid_t *uuid)
 	return NULL;
 }
 
+/**
+ * tb_switch_find_by_route() - Find switch by route string
+ * @tb: Domain the switch belongs
+ * @route: Route string to look for
+ *
+ * Returned switch has reference count increased so the caller needs to
+ * call tb_switch_put() when done with the switch.
+ */
+struct tb_switch *tb_switch_find_by_route(struct tb *tb, u64 route)
+{
+	struct tb_sw_lookup lookup;
+	struct device *dev;
+
+	if (!route)
+		return tb_switch_get(tb->root_switch);
+
+	memset(&lookup, 0, sizeof(lookup));
+	lookup.tb = tb;
+	lookup.route = route;
+
+	dev = bus_find_device(&tb_bus_type, NULL, &lookup, tb_switch_match);
+	if (dev)
+		return tb_to_switch(dev);
+
+	return NULL;
+}
+
 void tb_switch_exit(void)
 {
 	ida_destroy(&nvm_ida);

drivers/thunderbolt/tb.h +1 -0

index c7bd77e9c2c3..2cd6085a6e10 100644
@@ -386,6 +386,7 @@ 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);
+struct tb_switch *tb_switch_find_by_route(struct tb *tb, u64 route);
 
 static inline struct tb_switch *tb_switch_get(struct tb_switch *sw)
 {

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:41 UTC #12 lore raw

PATCH v3 12/18 thunderbolt: Add tb_xdomain_find_by_route()

In reply to Mika Westerberg

From: Radion Mirchevsky <[email protected]>

This is needed by the new ICM interface to find xdomains by route string instead of link and depth.

While there update existing tb_xdomain_find_* functions to use tb_xdomain_get() instead of open-coding the same.

Signed-off-by Radion Mirchevsky <[email protected]>

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/thunderbolt/xdomain.c | 47 ++++++++++++++++++++++++++++++++-----------
 include/linux/thunderbolt.h   | 13 ++++++++++++
 2 files changed, 48 insertions(+), 12 deletions(-)

drivers/thunderbolt/xdomain.c +35 ~~-12~~

index f25d88d4552b..8abb4e843085 100644
@@ -1255,6 +1255,7 @@ struct tb_xdomain_lookup {
 	const uuid_t *uuid;
 	u8 link;
 	u8 depth;
+	u64 route;
 };
 
 static struct tb_xdomain *switch_find_xdomain(struct tb_switch *sw,

@@ -1275,9 +1276,13 @@ static struct tb_xdomain *switch_find_xdomain(struct tb_switch *sw,
 			if (lookup->uuid) {
 				if (uuid_equal(xd->remote_uuid, lookup->uuid))
 					return xd;
-			} else if (lookup->link == xd->link &&
+			} else if (lookup->link &&
+				   lookup->link == xd->link &&
 				   lookup->depth == xd->depth) {
 				return xd;
+			} else if (lookup->route &&
+				   lookup->route == xd->route) {
+				return xd;
 			}
 		} else if (port->remote) {
 			xd = switch_find_xdomain(port->remote->sw, lookup);

@@ -1313,12 +1318,7 @@ struct tb_xdomain *tb_xdomain_find_by_uuid(struct tb *tb, const uuid_t *uuid)
 	lookup.uuid = uuid;
 
 	xd = switch_find_xdomain(tb->root_switch, &lookup);
-	if (xd) {
-		get_device(&xd->dev);
-		return xd;
-	}
-
-	return NULL;
+	return tb_xdomain_get(xd);
 }
 EXPORT_SYMBOL_GPL(tb_xdomain_find_by_uuid);

@@ -1349,13 +1349,36 @@ struct tb_xdomain *tb_xdomain_find_by_link_depth(struct tb *tb, u8 link,
 	lookup.depth = depth;
 
 	xd = switch_find_xdomain(tb->root_switch, &lookup);
-	if (xd) {
-		get_device(&xd->dev);
-		return xd;
-	}
+	return tb_xdomain_get(xd);
+}
 
-	return NULL;
+/**
+ * tb_xdomain_find_by_route() - Find an XDomain by route string
+ * @tb: Domain where the XDomain belongs to
+ * @route: XDomain route string
+ *
+ * Finds XDomain by walking through the Thunderbolt topology below @tb.
+ * The returned XDomain will have its reference count increased so the
+ * caller needs to call tb_xdomain_put() when it is done with the
+ * object.
+ *
+ * This will find all XDomains including the ones that are not yet added
+ * to the bus (handshake is still in progress).
+ *
+ * The caller needs to hold @tb->lock.
+ */
+struct tb_xdomain *tb_xdomain_find_by_route(struct tb *tb, u64 route)
+{
+	struct tb_xdomain_lookup lookup;
+	struct tb_xdomain *xd;
+
+	memset(&lookup, 0, sizeof(lookup));
+	lookup.route = route;
+
+	xd = switch_find_xdomain(tb->root_switch, &lookup);
+	return tb_xdomain_get(xd);
 }
+EXPORT_SYMBOL_GPL(tb_xdomain_find_by_route);
 
 bool tb_xdomain_handle_request(struct tb *tb, enum tb_cfg_pkg_type type,
 			       const void *buf, size_t size)

include/linux/thunderbolt.h +13 -0

index 7b69853188b1..27b9be34d4b9 100644
@@ -237,6 +237,7 @@ int tb_xdomain_enable_paths(struct tb_xdomain *xd, u16 transmit_path,
 			    u16 receive_ring);
 int tb_xdomain_disable_paths(struct tb_xdomain *xd);
 struct tb_xdomain *tb_xdomain_find_by_uuid(struct tb *tb, const uuid_t *uuid);
+struct tb_xdomain *tb_xdomain_find_by_route(struct tb *tb, u64 route);
 
 static inline struct tb_xdomain *
 tb_xdomain_find_by_uuid_locked(struct tb *tb, const uuid_t *uuid)

@@ -250,6 +251,18 @@ tb_xdomain_find_by_uuid_locked(struct tb *tb, const uuid_t *uuid)
 	return xd;
 }
 
+static inline struct tb_xdomain *
+tb_xdomain_find_by_route_locked(struct tb *tb, u64 route)
+{
+	struct tb_xdomain *xd;
+
+	mutex_lock(&tb->lock);
+	xd = tb_xdomain_find_by_route(tb, route);
+	mutex_unlock(&tb->lock);
+
+	return xd;
+}
+
 static inline struct tb_xdomain *tb_xdomain_get(struct tb_xdomain *xd)
 {
 	if (xd)

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:41 UTC #13 lore raw

PATCH v3 13/18 thunderbolt: Add constant for approval timeout

In reply to Mika Westerberg

We will be using this from Titan Ridge support code as well so make it constant.

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/thunderbolt/icm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/thunderbolt/icm.c +3 -2

index 6798516d6474..1c10aa8d224d 100644
@@ -41,7 +41,8 @@
 #define PHY_PORT_CS1_LINK_STATE_MASK	GENMASK(29, 26)
 #define PHY_PORT_CS1_LINK_STATE_SHIFT	26
 
-#define ICM_TIMEOUT			5000 /* ms */
+#define ICM_TIMEOUT			5000	/* ms */
+#define ICM_APPROVE_TIMEOUT		10000	/* ms */
 #define ICM_MAX_LINK			4
 #define ICM_MAX_DEPTH			6

@@ -260,7 +261,7 @@ static int icm_fr_approve_switch(struct tb *tb, struct tb_switch *sw)
 	memset(&reply, 0, sizeof(reply));
 	/* Use larger timeout as establishing tunnels can take some time */
 	ret = icm_request(tb, &request, sizeof(request), &reply, sizeof(reply),
-			  1, 10000);
+			  1, ICM_APPROVE_TIMEOUT);
 	if (ret)
 		return ret;

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:39 UTC #14 lore raw

PATCH v3 14/18 thunderbolt: Move driver ready handling to struct icm

In reply to Mika Westerberg

Intel Titan Ridge uses slightly different format for ICM driver ready response, so add a new ->driver_ready() callback to struct icm and move the existing handling to a separate function which we then use in Falcon Ridge and Alpine Ridge.

No functional changes intended.

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/thunderbolt/icm.c     | 42 +++++++++++++++++++++++++++++++-----------
 drivers/thunderbolt/tb_msgs.h |  6 ++++--
 2 files changed, 35 insertions(+), 13 deletions(-)

drivers/thunderbolt/icm.c +31 ~~-11~~

index 1c10aa8d224d..5d3cd740b71f 100644
@@ -59,6 +59,7 @@
  * @is_supported: Checks if we can support ICM on this controller
  * @get_mode: Read and return the ICM firmware mode (optional)
  * @get_route: Find a route string for given switch
+ * @driver_ready: Send driver ready message to ICM
  * @device_connected: Handle device connected ICM message
  * @device_disconnected: Handle device disconnected ICM message
  * @xdomain_connected - Handle XDomain connected ICM message

@@ -73,6 +74,8 @@ struct icm {
 	bool (*is_supported)(struct tb *tb);
 	int (*get_mode)(struct tb *tb);
 	int (*get_route)(struct tb *tb, u8 link, u8 depth, u64 *route);
+	int (*driver_ready)(struct tb *tb,
+			    enum tb_security_level *security_level);
 	void (*device_connected)(struct tb *tb,
 				 const struct icm_pkg_header *hdr);
 	void (*device_disconnected)(struct tb *tb,

@@ -246,6 +249,27 @@ static int icm_fr_get_route(struct tb *tb, u8 link, u8 depth, u64 *route)
 	return ret;
 }
 
+static int
+icm_fr_driver_ready(struct tb *tb, enum tb_security_level *security_level)
+{
+	struct icm_fr_pkg_driver_ready_response reply;
+	struct icm_pkg_driver_ready request = {
+		.hdr.code = ICM_DRIVER_READY,
+	};
+	int ret;
+
+	memset(&reply, 0, sizeof(reply));
+	ret = icm_request(tb, &request, sizeof(request), &reply, sizeof(reply),
+			  1, ICM_TIMEOUT);
+	if (ret)
+		return ret;
+
+	if (security_level)
+		*security_level = reply.security_level & ICM_FR_SLEVEL_MASK;
+
+	return 0;
+}
+
 static int icm_fr_approve_switch(struct tb *tb, struct tb_switch *sw)
 {
 	struct icm_fr_pkg_approve_device request;

@@ -869,21 +893,15 @@ static void icm_handle_event(struct tb *tb, enum tb_cfg_pkg_type type,
 static int
 __icm_driver_ready(struct tb *tb, enum tb_security_level *security_level)
 {
-	struct icm_pkg_driver_ready_response reply;
-	struct icm_pkg_driver_ready request = {
-		.hdr.code = ICM_DRIVER_READY,
-	};
+	struct icm *icm = tb_priv(tb);
 	unsigned int retries = 50;
 	int ret;
 
-	memset(&reply, 0, sizeof(reply));
-	ret = icm_request(tb, &request, sizeof(request), &reply, sizeof(reply),
-			  1, ICM_TIMEOUT);
-	if (ret)
+	ret = icm->driver_ready(tb, security_level);
+	if (ret) {
+		tb_err(tb, "failed to send driver ready to ICM\n");
 		return ret;
-
-	if (security_level)
-		*security_level = reply.security_level & 0xf;
+	}
 
 	/*
 	 * Hold on here until the switch config space is accessible so

@@ -1329,6 +1347,7 @@ struct tb *icm_probe(struct tb_nhi *nhi)
 	case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_NHI:
 		icm->is_supported = icm_fr_is_supported;
 		icm->get_route = icm_fr_get_route;
+		icm->driver_ready = icm_fr_driver_ready;
 		icm->device_connected = icm_fr_device_connected;
 		icm->device_disconnected = icm_fr_device_disconnected;
 		icm->xdomain_connected = icm_fr_xdomain_connected;

@@ -1344,6 +1363,7 @@ struct tb *icm_probe(struct tb_nhi *nhi)
 		icm->is_supported = icm_ar_is_supported;
 		icm->get_mode = icm_ar_get_mode;
 		icm->get_route = icm_ar_get_route;
+		icm->driver_ready = icm_fr_driver_ready;
 		icm->device_connected = icm_fr_device_connected;
 		icm->device_disconnected = icm_fr_device_disconnected;
 		icm->xdomain_connected = icm_fr_xdomain_connected;

drivers/thunderbolt/tb_msgs.h +4 -2

index 476bc04cac6c..931db2a7c7b3 100644
@@ -127,14 +127,16 @@ struct icm_pkg_driver_ready {
 	struct icm_pkg_header hdr;
 };
 
-struct icm_pkg_driver_ready_response {
+/* Falcon Ridge & Alpine Ridge common messages */
+
+struct icm_fr_pkg_driver_ready_response {
 	struct icm_pkg_header hdr;
 	u8 romver;
 	u8 ramver;
 	u16 security_level;
 };
 
-/* Falcon Ridge & Alpine Ridge common messages */
+#define ICM_FR_SLEVEL_MASK		0xf
 
 struct icm_fr_pkg_get_topology {
 	struct icm_pkg_header hdr;

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:41 UTC #15 lore raw

PATCH v3 15/18 thunderbolt: Add 'boot' attribute for devices

In reply to Mika Westerberg

From: Yehezkel Bernat <[email protected]>

In various cases, Thunderbolt device can be connected by ICM on boot without waiting for approval from user. Most cases are related to OEM-specific BIOS configurations. This information is interesting for user-space as if the device isn't in SW ACL, it may create a friction in the user experience where the device is automatically authorized if it's connected on boot but requires an explicit user action if connected after OS is up. User-space can use this information to suggest adding the device to SW ACL for auto-authorization on later connections.

Signed-off-by Yehezkel Bernat <[email protected]>

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-thunderbolt |  7 +++++++
 drivers/thunderbolt/icm.c                       | 12 ++++++++----
 drivers/thunderbolt/switch.c                    | 14 ++++++++++++++
 drivers/thunderbolt/tb.h                        |  2 ++
 drivers/thunderbolt/tb_msgs.h                   |  1 +
 5 files changed, 32 insertions(+), 4 deletions(-)

Documentation/ABI/testing/sysfs-bus-thunderbolt +7 -0

index 93798c02e28b..1f145b727d76 100644
@@ -38,6 +38,13 @@ Description:	This attribute is used to authorize Thunderbolt devices
 		   the device did not contain a key at all, and
 		   EKEYREJECTED if the challenge response did not match.
 
+What: /sys/bus/thunderbolt/devices/.../boot
+Date:		Jun 2018
+KernelVersion:	4.17
+Contact:	[email protected]
+Description:	This attribute contains 1 if Thunderbolt device was already
+		authorized on boot and 0 otherwise.
+
 What: /sys/bus/thunderbolt/devices/.../key
 Date:		Sep 2017
 KernelVersion:	4.13

drivers/thunderbolt/icm.c +8 -4

index 5d3cd740b71f..bece5540b06b 100644
@@ -402,7 +402,7 @@ static int icm_fr_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd)
 static void add_switch(struct tb_switch *parent_sw, u64 route,
 		       const uuid_t *uuid, u8 connection_id, u8 connection_key,
 		       u8 link, u8 depth, enum tb_security_level security_level,
-		       bool authorized)
+		       bool authorized, bool boot)
 {
 	struct tb_switch *sw;

@@ -417,6 +417,7 @@ static void add_switch(struct tb_switch *parent_sw, u64 route,
 	sw->depth = depth;
 	sw->authorized = authorized;
 	sw->security_level = security_level;
+	sw->boot = boot;
 
 	/* Link the two switches now */
 	tb_port_at(route, parent_sw)->remote = tb_upstream_port(sw);

@@ -431,7 +432,7 @@ static void add_switch(struct tb_switch *parent_sw, u64 route,
 
 static void update_switch(struct tb_switch *parent_sw, struct tb_switch *sw,
 			  u64 route, u8 connection_id, u8 connection_key,
-			  u8 link, u8 depth)
+			  u8 link, u8 depth, bool boot)
 {
 	/* Disconnect from parent */
 	tb_port_at(tb_route(sw), parent_sw)->remote = NULL;

@@ -445,6 +446,7 @@ static void update_switch(struct tb_switch *parent_sw, struct tb_switch *sw,
 	sw->connection_key = connection_key;
 	sw->link = link;
 	sw->depth = depth;
+	sw->boot = boot;
 
 	/* This switch still exists */
 	sw->is_unplugged = false;

@@ -504,6 +506,7 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
 	bool authorized = false;
 	struct tb_xdomain *xd;
 	u8 link, depth;
+	bool boot;
 	u64 route;
 	int ret;

@@ -513,6 +516,7 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
 	authorized = pkg->link_info & ICM_LINK_INFO_APPROVED;
 	security_level = (pkg->hdr.flags & ICM_FLAGS_SLEVEL_MASK) >>
 			 ICM_FLAGS_SLEVEL_SHIFT;
+	boot = pkg->link_info & ICM_LINK_INFO_BOOT;
 
 	if (pkg->link_info & ICM_LINK_INFO_REJECTED) {
 		tb_info(tb, "switch at %u.%u was rejected by ICM firmware because topology limit exceeded\n",

@@ -546,7 +550,7 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
 		if (sw->depth == depth && sw_phy_port == phy_port &&
 		    !!sw->authorized == authorized) {
 			update_switch(parent_sw, sw, route, pkg->connection_id,
-				      pkg->connection_key, link, depth);
+				      pkg->connection_key, link, depth, boot);
 			tb_switch_put(sw);
 			return;
 		}

@@ -595,7 +599,7 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
 
 	add_switch(parent_sw, route, &pkg->ep_uuid, pkg->connection_id,
 		   pkg->connection_key, link, depth, security_level,
-		   authorized);
+		   authorized, boot);
 
 	tb_switch_put(parent_sw);
 }

drivers/thunderbolt/switch.c +14 -0

index 4e2b2097bbfc..e9e30aaab2a3 100644
@@ -775,6 +775,15 @@ static ssize_t authorized_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(authorized);
 
+static ssize_t boot_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct tb_switch *sw = tb_to_switch(dev);
+
+	return sprintf(buf, "%u\n", sw->boot);
+}
+static DEVICE_ATTR_RO(boot);
+
 static ssize_t device_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)
 {

@@ -951,6 +960,7 @@ static DEVICE_ATTR_RO(unique_id);
 
 static struct attribute *switch_attrs[] = {
 	&dev_attr_authorized.attr,
+	&dev_attr_boot.attr,
 	&dev_attr_device.attr,
 	&dev_attr_device_name.attr,
 	&dev_attr_key.attr,

@@ -979,6 +989,10 @@ static umode_t switch_attr_is_visible(struct kobject *kobj,
 		if (sw->dma_port)
 			return attr->mode;
 		return 0;
+	} else if (attr == &dev_attr_boot.attr) {
+		if (tb_route(sw))
+			return attr->mode;
+		return 0;
 	}
 
 	return sw->safe_mode ? 0 : attr->mode;

drivers/thunderbolt/tb.h +2 -0

index 2cd6085a6e10..9c9cef875ca8 100644
@@ -66,6 +66,7 @@ struct tb_switch_nvm {
  * @nvm: Pointer to the NVM if the switch has one (%NULL otherwise)
  * @no_nvm_upgrade: Prevent NVM upgrade of this switch
  * @safe_mode: The switch is in safe-mode
+ * @boot: Whether the switch was already authorized on boot or not
  * @authorized: Whether the switch is authorized by user or policy
  * @work: Work used to automatically authorize a switch
  * @security_level: Switch supported security level

@@ -99,6 +100,7 @@ struct tb_switch {
 	struct tb_switch_nvm *nvm;
 	bool no_nvm_upgrade;
 	bool safe_mode;
+	bool boot;
 	unsigned int authorized;
 	struct work_struct work;
 	enum tb_security_level security_level;

drivers/thunderbolt/tb_msgs.h +1 -0

index 931db2a7c7b3..9f52f842257a 100644
@@ -179,6 +179,7 @@ struct icm_fr_event_device_connected {
 #define ICM_LINK_INFO_DEPTH_MASK	GENMASK(7, 4)
 #define ICM_LINK_INFO_APPROVED		BIT(8)
 #define ICM_LINK_INFO_REJECTED		BIT(9)
+#define ICM_LINK_INFO_BOOT		BIT(10)
 
 struct icm_fr_pkg_approve_device {
 	struct icm_pkg_header hdr;

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:41 UTC #16 lore raw

PATCH v3 16/18 thunderbolt: Add support for preboot ACL

In reply to Mika Westerberg

Preboot ACL is a mechanism that allows connecting Thunderbolt devices boot time in more secure way than the legacy Thunderbolt boot support. As with the legacy boot option, this also needs to be enabled from the BIOS before booting is allowed. Difference to the legacy mode is that the userspace software explicitly adds device UUIDs by sending a special message to the ICM firmware. Only the devices listed in the boot ACL are connected automatically during the boot. This works in both "user" and "secure" security levels.

We implement this in Linux by exposing a new sysfs attribute (boot_acl) below each Thunderbolt domain. The userspace software can then update the full list as needed.

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-thunderbolt |  23 ++++
 drivers/thunderbolt/domain.c                    | 123 ++++++++++++++++++
 drivers/thunderbolt/icm.c                       | 158 ++++++++++++++++++++++--
 drivers/thunderbolt/tb.h                        |   4 +
 drivers/thunderbolt/tb_msgs.h                   |  35 +++++-
 include/linux/thunderbolt.h                     |   2 +
 6 files changed, 335 insertions(+), 10 deletions(-)

Documentation/ABI/testing/sysfs-bus-thunderbolt +23 -0

index 1f145b727d76..4ed229789852 100644
@@ -1,3 +1,26 @@
+What: /sys/bus/thunderbolt/devices/.../domainX/boot_acl
+Date:		Jun 2018
+KernelVersion:	4.17
+Contact:	[email protected]
+Description:	Holds a comma separated list of device unique_ids that
+		are allowed to be connected automatically during system
+		startup (e.g boot devices). The list always contains
+		maximum supported number of unique_ids where unused
+		entries are empty. This allows the userspace software
+		to determine how many entries the controller supports.
+		If there are multiple controllers, each controller has
+		its own ACL list and size may be different between the
+		controllers.
+
+		System BIOS may have an option "Preboot ACL" or similar
+		that needs to be selected before this list is taken into
+		consideration.
+
+		Software always updates a full list in each write.
+
+		If a device is authorized automatically during boot its
+		boot attribute is set to 1.
+
 What: /sys/bus/thunderbolt/devices/.../domainX/security
 Date:		Sep 2017
 KernelVersion:	4.13

drivers/thunderbolt/domain.c +123 -0

140 lines not on this page · raw message

drivers/thunderbolt/icm.c +149 -9

242 lines not on this page · raw message

drivers/thunderbolt/tb.h +4 -0

index 9c9cef875ca8..9d9f0ca16bfb 100644
@@ -200,6 +200,8 @@ struct tb_path {
  * @suspend: Connection manager specific suspend
  * @complete: Connection manager specific complete
  * @handle_event: Handle thunderbolt event
+ * @get_boot_acl: Get boot ACL list
+ * @set_boot_acl: Set boot ACL list
  * @approve_switch: Approve switch
  * @add_switch_key: Add key to switch
  * @challenge_switch_key: Challenge switch using key

@@ -217,6 +219,8 @@ struct tb_cm_ops {
 	void (*complete)(struct tb *tb);
 	void (*handle_event)(struct tb *tb, enum tb_cfg_pkg_type,
 			     const void *buf, size_t size);
+	int (*get_boot_acl)(struct tb *tb, uuid_t *uuids, size_t nuuids);
+	int (*set_boot_acl)(struct tb *tb, const uuid_t *uuids, size_t nuuids);
 	int (*approve_switch)(struct tb *tb, struct tb_switch *sw);
 	int (*add_switch_key)(struct tb *tb, struct tb_switch *sw);
 	int (*challenge_switch_key)(struct tb *tb, struct tb_switch *sw,

drivers/thunderbolt/tb_msgs.h +34 -1

index 9f52f842257a..496b91f3b579 100644
@@ -102,6 +102,7 @@ enum icm_pkg_code {
 	ICM_ADD_DEVICE_KEY = 0x6,
 	ICM_GET_ROUTE = 0xa,
 	ICM_APPROVE_XDOMAIN = 0x10,
+	ICM_PREBOOT_ACL = 0x18,
 };
 
 enum icm_event_code {

@@ -122,12 +123,13 @@ struct icm_pkg_header {
 #define ICM_FLAGS_NO_KEY		BIT(1)
 #define ICM_FLAGS_SLEVEL_SHIFT		3
 #define ICM_FLAGS_SLEVEL_MASK		GENMASK(4, 3)
+#define ICM_FLAGS_WRITE			BIT(7)
 
 struct icm_pkg_driver_ready {
 	struct icm_pkg_header hdr;
 };
 
-/* Falcon Ridge & Alpine Ridge common messages */
+/* Falcon Ridge only messages */
 
 struct icm_fr_pkg_driver_ready_response {
 	struct icm_pkg_header hdr;

@@ -138,6 +140,8 @@ struct icm_fr_pkg_driver_ready_response {
 
 #define ICM_FR_SLEVEL_MASK		0xf
 
+/* Falcon Ridge & Alpine Ridge common messages */
+
 struct icm_fr_pkg_get_topology {
 	struct icm_pkg_header hdr;
 };

@@ -274,6 +278,18 @@ struct icm_fr_pkg_approve_xdomain_response {
 
 /* Alpine Ridge only messages */
 
+struct icm_ar_pkg_driver_ready_response {
+	struct icm_pkg_header hdr;
+	u8 romver;
+	u8 ramver;
+	u16 info;
+};
+
+#define ICM_AR_INFO_SLEVEL_MASK		GENMASK(3, 0)
+#define ICM_AR_INFO_BOOT_ACL_SHIFT	7
+#define ICM_AR_INFO_BOOT_ACL_MASK	GENMASK(11, 7)
+#define ICM_AR_INFO_BOOT_ACL_SUPPORTED	BIT(13)
+
 struct icm_ar_pkg_get_route {
 	struct icm_pkg_header hdr;
 	u16 reserved;

@@ -288,6 +304,23 @@ struct icm_ar_pkg_get_route_response {
 	u32 route_lo;
 };
 
+struct icm_ar_boot_acl_entry {
+	u32 uuid_lo;
+	u32 uuid_hi;
+};
+
+#define ICM_AR_PREBOOT_ACL_ENTRIES	16
+
+struct icm_ar_pkg_preboot_acl {
+	struct icm_pkg_header hdr;
+	struct icm_ar_boot_acl_entry acl[ICM_AR_PREBOOT_ACL_ENTRIES];
+};
+
+struct icm_ar_pkg_preboot_acl_response {
+	struct icm_pkg_header hdr;
+	struct icm_ar_boot_acl_entry acl[ICM_AR_PREBOOT_ACL_ENTRIES];
+};
+
 /* XDomain messages */
 
 struct tb_xdomain_header {

include/linux/thunderbolt.h +2 -0

index 27b9be34d4b9..47251844d064 100644
@@ -65,6 +65,7 @@ enum tb_security_level {
  * @cm_ops: Connection manager specific operations vector
  * @index: Linux assigned domain number
  * @security_level: Current security level
+ * @nboot_acl: Number of boot ACLs the domain supports
  * @privdata: Private connection manager specific data
  */
 struct tb {

@@ -77,6 +78,7 @@ struct tb {
 	const struct tb_cm_ops *cm_ops;
 	int index;
 	enum tb_security_level security_level;
+	size_t nboot_acl;
 	unsigned long privdata[0];
 };

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:41 UTC #17 lore raw

PATCH v3 17/18 thunderbolt: Introduce USB only (SL4) security level

In reply to Mika Westerberg

This new security level works so that it creates one PCIe tunnel to the connected Thunderbolt dock, removing PCIe links downstream of the dock. This leaves only the internal USB controller visible.

Display Port tunnels are created normally.

While there make sure security sysfs attribute returns "unknown" for any future security level.

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-thunderbolt |  3 +++
 Documentation/admin-guide/thunderbolt.rst       | 15 ++++++++++-----
 drivers/thunderbolt/domain.c                    |  7 ++++++-
 include/linux/thunderbolt.h                     |  4 ++++
 4 files changed, 23 insertions(+), 6 deletions(-)

Documentation/ABI/testing/sysfs-bus-thunderbolt +3 -0

index 4ed229789852..151584a1f950 100644
@@ -35,6 +35,9 @@ Description:	This attribute holds current Thunderbolt security level
 			minimum. User needs to authorize each device.
 		dponly: Automatically tunnel Display port (and USB). No
 			PCIe tunnels are created.
+		usbonly: Automatically tunnel USB controller of the
+			 connected Thunderbolt dock (and Display Port). All
+			 PCIe links downstream of the dock are removed.
 
 What: /sys/bus/thunderbolt/devices/.../authorized
 Date:		Sep 2017

Documentation/admin-guide/thunderbolt.rst +10 -5

index 9948ec36a204..35fccba6a9a6 100644
@@ -21,11 +21,11 @@ vulnerable to DMA attacks.
 Security levels and how to use them
 -----------------------------------
 Starting with Intel Falcon Ridge Thunderbolt controller there are 4
-security levels available. The reason for these is the fact that the
-connected devices can be DMA masters and thus read contents of the host
-memory without CPU and OS knowing about it. There are ways to prevent
-this by setting up an IOMMU but it is not always available for various
-reasons.
+security levels available. Intel Titan Ridge added one more security level
+(usbonly). The reason for these is the fact that the connected devices can
+be DMA masters and thus read contents of the host memory without CPU and OS
+knowing about it. There are ways to prevent this by setting up an IOMMU but
+it is not always available for various reasons.
 
 The security levels are as follows:

@@ -52,6 +52,11 @@ The security levels are as follows:
     USB. No PCIe tunneling is done. In BIOS settings this is
     typically called *Display Port Only*.
 
+  usbonly
+    The firmware automatically creates tunnels for the USB controller and
+    Display Port in a dock. All PCIe links downstream of the dock are
+    removed.
+
 The current security level can be read from
 ``/sys/bus/thunderbolt/devices/domainX/security`` where ``domainX`` is
 the Thunderbolt domain the host controller manages. There is typically

drivers/thunderbolt/domain.c +6 -1

index ab4b304306f7..6281266b8ec0 100644
@@ -117,6 +117,7 @@ static const char * const tb_security_names[] = {
 	[TB_SECURITY_USER] = "user",
 	[TB_SECURITY_SECURE] = "secure",
 	[TB_SECURITY_DPONLY] = "dponly",
+	[TB_SECURITY_USBONLY] = "usbonly",
 };
 
 static ssize_t boot_acl_show(struct device *dev, struct device_attribute *attr,

@@ -227,8 +228,12 @@ static ssize_t security_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
 	struct tb *tb = container_of(dev, struct tb, dev);
+	const char *name = "unknown";
 
-	return sprintf(buf, "%s\n", tb_security_names[tb->security_level]);
+	if (tb->security_level < ARRAY_SIZE(tb_security_names))
+		name = tb_security_names[tb->security_level];
+
+	return sprintf(buf, "%s\n", name);
 }
 static DEVICE_ATTR_RO(security);

include/linux/thunderbolt.h +4 -0

index 47251844d064..a3ed26082bc1 100644
@@ -45,12 +45,16 @@ enum tb_cfg_pkg_type {
  * @TB_SECURITY_USER: User approval required at minimum
  * @TB_SECURITY_SECURE: One time saved key required at minimum
  * @TB_SECURITY_DPONLY: Only tunnel Display port (and USB)
+ * @TB_SECURITY_USBONLY: Only tunnel USB controller of the connected
+ *			 Thunderbolt dock (and Display Port). All PCIe
+ *			 links downstream of the dock are removed.
  */
 enum tb_security_level {
 	TB_SECURITY_NONE,
 	TB_SECURITY_USER,
 	TB_SECURITY_SECURE,
 	TB_SECURITY_DPONLY,
+	TB_SECURITY_USBONLY,
 };
 
 /**

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-02 12:40 UTC #18 lore raw

PATCH v3 18/18 thunderbolt: Add support for Intel Titan Ridge

In reply to Mika Westerberg

From: Radion Mirchevsky <[email protected]>

Intel Titan Ridge is the next Thunderbolt 3 controller. The ICM firmware message format in Titan Ridge differs from Falcon Ridge and Alpine Ridge somewhat because it is using route strings addressing devices. In addition to that the DMA port of 4-channel (two port) controller is in different port number than the previous controllers. There are some other minor differences as well.

This patch add support for Intel Titan Ridge and the new ICM firmware message format.

Signed-off-by Radion Mirchevsky <[email protected]>

Signed-off-by Mika Westerberg <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
 drivers/thunderbolt/dma_port.c |  28 ++-
 drivers/thunderbolt/icm.c      | 382 +++++++++++++++++++++++++++++++++++++++++
 drivers/thunderbolt/nhi.c      |   2 +
 drivers/thunderbolt/nhi.h      |   5 +
 drivers/thunderbolt/switch.c   |   3 +
 drivers/thunderbolt/tb_msgs.h  | 141 +++++++++++++++
 6 files changed, 546 insertions(+), 15 deletions(-)

drivers/thunderbolt/dma_port.c +13 ~~-15~~

index af6dde347bee..f2701194f810 100644
@@ -170,24 +170,22 @@ static int dma_port_write(struct tb_ctl *ctl, const void *buffer, u64 route,
 
 static int dma_find_port(struct tb_switch *sw)
 {
-	int port, ret;
-	u32 type;
+	static const int ports[] = { 3, 5, 7 };
+	int i;
 
 	/*
-	 * The DMA (NHI) port is either 3 or 5 depending on the
-	 * controller. Try both starting from 5 which is more common.
+	 * The DMA (NHI) port is either 3, 5 or 7 depending on the
+	 * controller. Try all of them.
 	 */
-	port = 5;
-	ret = dma_port_read(sw->tb->ctl, &type, tb_route(sw), port, 2, 1,
-			    DMA_PORT_TIMEOUT);
-	if (!ret && (type & 0xffffff) == TB_TYPE_NHI)
-		return port;
-
-	port = 3;
-	ret = dma_port_read(sw->tb->ctl, &type, tb_route(sw), port, 2, 1,
-			    DMA_PORT_TIMEOUT);
-	if (!ret && (type & 0xffffff) == TB_TYPE_NHI)
-		return port;
+	for (i = 0; i < ARRAY_SIZE(ports); i++) {
+		u32 type;
+		int ret;
+
+		ret = dma_port_read(sw->tb->ctl, &type, tb_route(sw), ports[i],
+				    2, 1, DMA_PORT_TIMEOUT);
+		if (!ret && (type & 0xffffff) == TB_TYPE_NHI)
+			return ports[i];
+	}
 
 	return -ENODEV;
 }

drivers/thunderbolt/icm.c +382 -0

406 lines not on this page · raw message

drivers/thunderbolt/nhi.c +2 -0

index 9e58d09f6029..f5a33e88e676 100644
@@ -1111,6 +1111,8 @@ static struct pci_device_id nhi_ids[] = {
 	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_NHI) },
 	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_NHI) },
 	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_USBONLY_NHI) },
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_NHI) },
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_NHI) },
 
 	{ 0,}
 };

drivers/thunderbolt/nhi.h +5 -0

index 4476ab4cfd0c..1696a4560948 100644
@@ -45,5 +45,10 @@ enum nhi_fw_mode nhi_mailbox_mode(struct tb_nhi *nhi);
 #define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_USBONLY_NHI	0x15dc
 #define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_USBONLY_NHI	0x15dd
 #define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_USBONLY_NHI	0x15de
+#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_BRIDGE	0x15e7
+#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_NHI		0x15e8
+#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_BRIDGE	0x15ea
+#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_NHI		0x15eb
+#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_BRIDGE	0x15ef
 
 #endif

drivers/thunderbolt/switch.c +3 -0

index e9e30aaab2a3..25758671ddf4 100644
@@ -1051,6 +1051,9 @@ static int tb_switch_get_generation(struct tb_switch *sw)
 	case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_BRIDGE:
 	case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_BRIDGE:
 	case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_BRIDGE:
+	case PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_BRIDGE:
+	case PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_BRIDGE:
+	case PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_BRIDGE:
 		return 3;
 
 	default:

drivers/thunderbolt/tb_msgs.h +141 -0

153 lines not on this page · raw message

Signature · 1 line

2.16.1

Mika Westerberg

author

2018-03-09 10:06 UTC #19 lore raw

In reply to Mika Westerberg

Quoted 15 lines · Mika Westerberg

Mika Westerberg (13):
thunderbolt: Resume control channel after hibernation image is created
thunderbolt: Serialize PCIe tunnel creation with PCI rescan
thunderbolt: Handle connecting device in place of host properly
thunderbolt: Do not overwrite error code when domain adding fails
thunderbolt: Wait a bit longer for root switch config space
thunderbolt: Wait a bit longer for ICM to authenticate the active NVM
thunderbolt: Handle rejected Thunderbolt devices
thunderbolt: Factor common ICM add and update operations out
thunderbolt: Add tb_switch_get()
thunderbolt: Add constant for approval timeout
thunderbolt: Move driver ready handling to struct icm
thunderbolt: Add support for preboot ACL
thunderbolt: Introduce USB only (SL4) security level

Quoted 8 lines

Radion Mirchevsky (4):
thunderbolt: Correct function name in kernel-doc comment
thunderbolt: Add tb_switch_find_by_route()
thunderbolt: Add tb_xdomain_find_by_route()
thunderbolt: Add support for Intel Titan Ridge
Yehezkel Bernat (1):
thunderbolt: Add 'boot' attribute for devices

Whole series applied to thunderbolt.git/next.