thunderbolt: Intel Ice Lake support

PATCH In review Mika Westerberg 3 participants 17 messages Aug 19, 2019 – Aug 26, 2019

Subsystems (2)

ACPI Thunderbolt driver

lore

Share & tools

lore

Mika Westerberg

author

2019-08-19 11:23 UTC #0 lore raw

v3 cover thunderbolt: Intel Ice Lake support

Hi all,

This is third iteration of the patch series adding support for the Intel Ice Lake integrated Thunderbolt controller. The biggest difference from the previous discrete controllers is that the Ice Lake Thunderbolt controller is now integrated as part of the SoC. The firmware messages pretty much follow Titan Ridge but there are some differences as well (such as the new RTD3 veto notification). Also Ice Lake does not implement security levels so DMA protection is handled by IOMMU.

The previous versions of the series can be found here:

v2: https://lwn.net/Articles/795979/
v1: https://lwn.net/Articles/793066/

Changes from v2:

* Moved ICL specific NHI ops into a separate file (nhi_ops.c)

* Postpone rescan thread during resume when if get device connected notification from the firmware. This fixes an issue that happened occasionally when resuming with full chain of devices. The driver started to remove last devices in the chain even if they are still connected.

* Move call to nhi->ops->init() before CM specific probe happens.

* Added ack from Rafael

Changes from v1:

* Check !x86_apple_machine for Titan Ridge also.

* Drop ioread32() when updating prod/cons because hardware ignores writes to the read-only parts of the register this allows us to save one read.

* Remove Light Ridge specific handling in eeprom.c as it is not necessary after patch 4/8.

* Moved RTD3 veto handling into separate functions to avoid code duplication.

* Reworked struct nhi_ops so that it provides hooks such as runtime_suspend and so on. The NHI implementation then can use those hooks to implement whatever additional logic is needed. This should allow us to move those bits into a separate files in future if needed.

I also looked whether we could replace quirk_apple_poweroff_thunderbolt() using struct nhi_ops as well but it turns out to be unsuitable. The reason is that the ACPI magic sequence needs to be done after PCI core has moved the device into D3. The NHI driver does not have easy means to do so.

* Clarified comment in icl_nhi_suspend()

* Added clarification comment to icl_nhi_force_power()

* Use "Thunderbolt" instead of "TBT" in new GUID entries

* Added reviewed tag from Yehezkel

Mika Westerberg (8):
thunderbolt: Correct path indices for PCIe tunnel
thunderbolt: Move NVM upgrade support flag to struct icm
thunderbolt: Use 32-bit writes when writing ring producer/consumer
thunderbolt: Do not fail adding switch if some port is not implemented
thunderbolt: Hide switch attributes that are not set
thunderbolt: Expose active parts of NVM even if upgrade is not supported
thunderbolt: Add support for Intel Ice Lake
ACPI / property: Add two new Thunderbolt property GUIDs to the list

 drivers/acpi/property.c        |   6 +
 drivers/thunderbolt/Makefile   |   2 +-
 drivers/thunderbolt/ctl.c      |  23 +++-
 drivers/thunderbolt/eeprom.c   |   4 -
 drivers/thunderbolt/icm.c      | 194 ++++++++++++++++++++++++++++++---
 drivers/thunderbolt/nhi.c      | 134 +++++++++++++++++++++--
 drivers/thunderbolt/nhi.h      |  22 ++++
 drivers/thunderbolt/nhi_ops.c  | 179 ++++++++++++++++++++++++++++++
 drivers/thunderbolt/nhi_regs.h |  37 +++++++
 drivers/thunderbolt/switch.c   |  52 +++++++--
 drivers/thunderbolt/tb_msgs.h  |  16 ++-
 drivers/thunderbolt/tunnel.c   |   4 +-
 include/linux/thunderbolt.h    |   2 +
 13 files changed, 622 insertions(+), 53 deletions(-)
 create mode 100644 drivers/thunderbolt/nhi_ops.c

Signature · 1 line

2.23.0.rc1

Mika Westerberg

author

2019-08-19 11:23 UTC #1 lore raw

PATCH v3 1/8 thunderbolt: Correct path indices for PCIe tunnel

PCIe tunnel path indices got mixed up when we added support for tunnels between switches that are not adjacent. This did not affect the functionality as it is just an index but fix it now nevertheless to make the code easier to understand.

Reported-by Rajmohan Mani <[email protected]>

Fixes 8c7acaaf020f ("thunderbolt: Extend tunnel creation to more than 2 adjacent switches")

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

Reviewed-by Yehezkel Bernat <[email protected]>

---
 drivers/thunderbolt/tunnel.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/thunderbolt/tunnel.c +2 -2

index 31d0234837e4..5a99234826e7 100644
@@ -211,7 +211,7 @@ struct tb_tunnel *tb_tunnel_alloc_pci(struct tb *tb, struct tb_port *up,
 		return NULL;
 	}
 	tb_pci_init_path(path);
-	tunnel->paths[TB_PCI_PATH_UP] = path;
+	tunnel->paths[TB_PCI_PATH_DOWN] = path;
 
 	path = tb_path_alloc(tb, up, TB_PCI_HOPID, down, TB_PCI_HOPID, 0,
 			     "PCIe Up");

@@ -220,7 +220,7 @@ struct tb_tunnel *tb_tunnel_alloc_pci(struct tb *tb, struct tb_port *up,
 		return NULL;
 	}
 	tb_pci_init_path(path);
-	tunnel->paths[TB_PCI_PATH_DOWN] = path;
+	tunnel->paths[TB_PCI_PATH_UP] = path;
 
 	return tunnel;
 }

Signature · 1 line

2.23.0.rc1

Mika Westerberg

author

2019-08-19 11:22 UTC #2 lore raw

PATCH v3 2/8 thunderbolt: Move NVM upgrade support flag to struct icm

In reply to Mika Westerberg

This is depends on the controller and on the platform/CPU we are running. Move it to struct icm so we can set it per controller.

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

Reviewed-by Yehezkel Bernat <[email protected]>

---
 drivers/thunderbolt/icm.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

drivers/thunderbolt/icm.c +12 -6

index fbdcef56a676..e9835ab35465 100644
@@ -55,6 +55,7 @@
  * @safe_mode: ICM is in safe mode
  * @max_boot_acl: Maximum number of preboot ACL entries (%0 if not supported)
  * @rpm: Does the controller support runtime PM (RTD3)
+ * @can_upgrade_nvm: Can the NVM firmware be upgrade on this controller
  * @is_supported: Checks if we can support ICM on this controller
  * @cio_reset: Trigger CIO reset
  * @get_mode: Read and return the ICM firmware mode (optional)

@@ -74,6 +75,7 @@ struct icm {
 	int vnd_cap;
 	bool safe_mode;
 	bool rpm;
+	bool can_upgrade_nvm;
 	bool (*is_supported)(struct tb *tb);
 	int (*cio_reset)(struct tb *tb);
 	int (*get_mode)(struct tb *tb);

@@ -1913,12 +1915,7 @@ static int icm_start(struct tb *tb)
 	if (IS_ERR(tb->root_switch))
 		return PTR_ERR(tb->root_switch);
 
-	/*
-	 * NVM upgrade has not been tested on Apple systems and they
-	 * don't provide images publicly either. To be on the safe side
-	 * prevent root switch NVM upgrade on Macs for now.
-	 */
-	tb->root_switch->no_nvm_upgrade = x86_apple_machine;
+	tb->root_switch->no_nvm_upgrade = !icm->can_upgrade_nvm;
 	tb->root_switch->rpm = icm->rpm;
 
 	ret = tb_switch_add(tb->root_switch);

@@ -2021,6 +2018,7 @@ struct tb *icm_probe(struct tb_nhi *nhi)
 	switch (nhi->pdev->device) {
 	case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_NHI:
 	case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_NHI:
+		icm->can_upgrade_nvm = true;
 		icm->is_supported = icm_fr_is_supported;
 		icm->get_route = icm_fr_get_route;
 		icm->save_devices = icm_fr_save_devices;

@@ -2038,6 +2036,13 @@ struct tb *icm_probe(struct tb_nhi *nhi)
 	case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_NHI:
 	case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_NHI:
 		icm->max_boot_acl = ICM_AR_PREBOOT_ACL_ENTRIES;
+		/*
+		 * NVM upgrade has not been tested on Apple systems and
+		 * they don't provide images publicly either. To be on
+		 * the safe side prevent root switch NVM upgrade on Macs
+		 * for now.
+		 */
+		icm->can_upgrade_nvm = !x86_apple_machine;
 		icm->is_supported = icm_ar_is_supported;
 		icm->cio_reset = icm_ar_cio_reset;
 		icm->get_mode = icm_ar_get_mode;

@@ -2054,6 +2059,7 @@ struct tb *icm_probe(struct tb_nhi *nhi)
 	case PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_NHI:
 	case PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_NHI:
 		icm->max_boot_acl = ICM_AR_PREBOOT_ACL_ENTRIES;
+		icm->can_upgrade_nvm = !x86_apple_machine;
 		icm->is_supported = icm_ar_is_supported;
 		icm->cio_reset = icm_tr_cio_reset;
 		icm->get_mode = icm_ar_get_mode;

Signature · 1 line

2.23.0.rc1

Mika Westerberg

author

2019-08-19 11:22 UTC #3 lore raw

PATCH v3 3/8 thunderbolt: Use 32-bit writes when writing ring producer/consumer

In reply to Mika Westerberg

The register access should be using 32-bit reads/writes according to the datasheet. With the previous generation hardware 16-bit writes have been working but starting with ICL this is not the case anymore so fix producer/consumer register update to use correct width register address.

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

Reviewed-by Yehezkel Bernat <[email protected]>

---
 drivers/thunderbolt/nhi.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

drivers/thunderbolt/nhi.c +18 -4

index 27fbe62c7ddd..9c782706e652 100644
@@ -143,9 +143,20 @@ static void __iomem *ring_options_base(struct tb_ring *ring)
 	return io;
 }
 
-static void ring_iowrite16desc(struct tb_ring *ring, u32 value, u32 offset)
+static void ring_iowrite_cons(struct tb_ring *ring, u16 cons)
 {
-	iowrite16(value, ring_desc_base(ring) + offset);
+	/*
+	 * The other 16-bits in the register is read-only and writes to it
+	 * are ignored by the hardware so we can save one ioread32() by
+	 * filling the read-only bits with zeroes.
+	 */
+	iowrite32(cons, ring_desc_base(ring) + 8);
+}
+
+static void ring_iowrite_prod(struct tb_ring *ring, u16 prod)
+{
+	/* See ring_iowrite_cons() above for explanation */
+	iowrite32(prod << 16, ring_desc_base(ring) + 8);
 }
 
 static void ring_iowrite32desc(struct tb_ring *ring, u32 value, u32 offset)

@@ -197,7 +208,10 @@ static void ring_write_descriptors(struct tb_ring *ring)
 			descriptor->sof = frame->sof;
 		}
 		ring->head = (ring->head + 1) % ring->size;
-		ring_iowrite16desc(ring, ring->head, ring->is_tx ? 10 : 8);
+		if (ring->is_tx)
+			ring_iowrite_prod(ring, ring->head);
+		else
+			ring_iowrite_cons(ring, ring->head);
 	}
 }

@@ -662,7 +676,7 @@ void tb_ring_stop(struct tb_ring *ring)
 
 	ring_iowrite32options(ring, 0, 0);
 	ring_iowrite64desc(ring, 0, 0);
-	ring_iowrite16desc(ring, 0, ring->is_tx ? 10 : 8);
+	ring_iowrite32desc(ring, 0, 8);
 	ring_iowrite32desc(ring, 0, 12);
 	ring->head = 0;
 	ring->tail = 0;

Signature · 1 line

2.23.0.rc1

Mika Westerberg

author

2019-08-19 11:23 UTC #4 lore raw

PATCH v3 4/8 thunderbolt: Do not fail adding switch if some port is not implemented

In reply to Mika Westerberg

There are two ways to mark a port as unimplemented. Typical way is to return port type as TB_TYPE_INACTIVE when its config space is read. Alternatively if the port is not physically present (such as ports 10 and 11 in ICL) reading from port config space returns TB_CFG_ERROR_INVALID_CONFIG_SPACE instead. Currently the driver bails out from adding the switch if it receives any error during port inititialization which is wrong.

Handle this properly and just leave the port as TB_TYPE_INACTIVE before continuing to the next port.

This also allows us to get rid of special casing for Light Ridge port 5 in eeprom.c.

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

Reviewed-by Yehezkel Bernat <[email protected]>

---
 drivers/thunderbolt/ctl.c    | 23 +++++++++++++++++++----
 drivers/thunderbolt/eeprom.c |  4 ----
 drivers/thunderbolt/switch.c |  8 +++++++-
 3 files changed, 26 insertions(+), 9 deletions(-)

drivers/thunderbolt/ctl.c +19 -4

index 2427d73be731..2ec1af8f7968 100644
@@ -930,6 +930,23 @@ struct tb_cfg_result tb_cfg_write_raw(struct tb_ctl *ctl, const void *buffer,
 	return res;
 }
 
+static int tb_cfg_get_error(struct tb_ctl *ctl, enum tb_cfg_space space,
+			    const struct tb_cfg_result *res)
+{
+	/*
+	 * For unimplemented ports access to port config space may return
+	 * TB_CFG_ERROR_INVALID_CONFIG_SPACE (alternatively their type is
+	 * set to TB_TYPE_INACTIVE). In the former case return -ENODEV so
+	 * that the caller can mark the port as disabled.
+	 */
+	if (space == TB_CFG_PORT &&
+	    res->tb_error == TB_CFG_ERROR_INVALID_CONFIG_SPACE)
+		return -ENODEV;
+
+	tb_cfg_print_error(ctl, res);
+	return -EIO;
+}
+
 int tb_cfg_read(struct tb_ctl *ctl, void *buffer, u64 route, u32 port,
 		enum tb_cfg_space space, u32 offset, u32 length)
 {

@@ -942,8 +959,7 @@ int tb_cfg_read(struct tb_ctl *ctl, void *buffer, u64 route, u32 port,
 
 	case 1:
 		/* Thunderbolt error, tb_error holds the actual number */
-		tb_cfg_print_error(ctl, &res);
-		return -EIO;
+		return tb_cfg_get_error(ctl, space, &res);
 
 	case -ETIMEDOUT:
 		tb_ctl_warn(ctl, "timeout reading config space %u from %#x\n",

@@ -969,8 +985,7 @@ int tb_cfg_write(struct tb_ctl *ctl, const void *buffer, u64 route, u32 port,
 
 	case 1:
 		/* Thunderbolt error, tb_error holds the actual number */
-		tb_cfg_print_error(ctl, &res);
-		return -EIO;
+		return tb_cfg_get_error(ctl, space, &res);
 
 	case -ETIMEDOUT:
 		tb_ctl_warn(ctl, "timeout writing config space %u to %#x\n",

drivers/thunderbolt/eeprom.c +0 -4

index 81e8ac4c5805..3b2fb4aafd92 100644
@@ -525,10 +525,6 @@ int tb_drom_read(struct tb_switch *sw)
 		sw->ports[3].dual_link_port = &sw->ports[4];
 		sw->ports[4].dual_link_port = &sw->ports[3];
 
-		/* Port 5 is inaccessible on this gen 1 controller */
-		if (sw->config.device_id == PCI_DEVICE_ID_INTEL_LIGHT_RIDGE)
-			sw->ports[5].disabled = true;
-
 		return 0;
 	}

drivers/thunderbolt/switch.c +7 -1

index 5668a44e0653..64f845fe4690 100644
@@ -611,8 +611,14 @@ static int tb_init_port(struct tb_port *port)
 	int cap;
 
 	res = tb_port_read(port, &port->config, TB_CFG_PORT, 0, 8);
-	if (res)
+	if (res) {
+		if (res == -ENODEV) {
+			tb_dbg(port->sw->tb, " Port %d: not implemented\n",
+			       port->port);
+			return 0;
+		}
 		return res;
+	}
 
 	/* Port 0 is the switch itself and has no PHY. */
 	if (port->config.type == TB_TYPE_PORT && port->port != 0) {

Signature · 1 line

2.23.0.rc1

Mika Westerberg

author

2019-08-19 11:22 UTC #5 lore raw

PATCH v3 5/8 thunderbolt: Hide switch attributes that are not set

In reply to Mika Westerberg

Thunderbolt host routers may not always contain DROM that includes device identification information. This is mostly needed for Ice Lake systems but some Falcon Ridge controllers on PCs also do not have DROM.

In that case hide the identification attributes.

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

Reviewed-by Yehezkel Bernat <[email protected]>

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

drivers/thunderbolt/switch.c +13 -1

index 64f845fe4690..bd3eaaf34ea1 100644
@@ -1337,7 +1337,19 @@ static umode_t switch_attr_is_visible(struct kobject *kobj,
 	struct device *dev = container_of(kobj, struct device, kobj);
 	struct tb_switch *sw = tb_to_switch(dev);
 
-	if (attr == &dev_attr_key.attr) {
+	if (attr == &dev_attr_device.attr) {
+		if (!sw->device)
+			return 0;
+	} else if (attr == &dev_attr_device_name.attr) {
+		if (!sw->device_name)
+			return 0;
+	} else if (attr == &dev_attr_vendor.attr)  {
+		if (!sw->vendor)
+			return 0;
+	} else if (attr == &dev_attr_vendor_name.attr)  {
+		if (!sw->vendor_name)
+			return 0;
+	} else if (attr == &dev_attr_key.attr) {
 		if (tb_route(sw) &&
 		    sw->tb->security_level == TB_SECURITY_SECURE &&
 		    sw->security_level == TB_SECURITY_SECURE)

Signature · 1 line

2.23.0.rc1

Mika Westerberg

author

2019-08-19 11:22 UTC #6 lore raw

PATCH v3 6/8 thunderbolt: Expose active parts of NVM even if upgrade is not supported

In reply to Mika Westerberg

Ice Lake Thunderbolt controller NVM firmware is part of the BIOS image which means it is not writable through the DMA port anymore. However, we can still read it so we can keep nvm_version and active parts of NVM. This way users still can find out the active NVM version and other potentially useful information directly from Linux.

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

Reviewed-by Yehezkel Bernat <[email protected]>

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

drivers/thunderbolt/switch.c +19 -9

index bd3eaaf34ea1..531f11fecf75 100644
@@ -364,12 +364,14 @@ static int tb_switch_nvm_add(struct tb_switch *sw)
 		nvm->active = nvm_dev;
 	}
 
-	nvm_dev = register_nvmem(sw, nvm->id, NVM_MAX_SIZE, false);
-	if (IS_ERR(nvm_dev)) {
-		ret = PTR_ERR(nvm_dev);
-		goto err_nvm_active;
+	if (!sw->no_nvm_upgrade) {
+		nvm_dev = register_nvmem(sw, nvm->id, NVM_MAX_SIZE, false);
+		if (IS_ERR(nvm_dev)) {
+			ret = PTR_ERR(nvm_dev);
+			goto err_nvm_active;
+		}
+		nvm->non_active = nvm_dev;
 	}
-	nvm->non_active = nvm_dev;
 
 	sw->nvm = nvm;
 	return 0;

@@ -398,7 +400,8 @@ static void tb_switch_nvm_remove(struct tb_switch *sw)
 	if (!nvm->authenticating)
 		nvm_clear_auth_status(sw);
 
-	nvmem_unregister(nvm->non_active);
+	if (nvm->non_active)
+		nvmem_unregister(nvm->non_active);
 	if (nvm->active)
 		nvmem_unregister(nvm->active);
 	ida_simple_remove(&nvm_ida, nvm->id);

@@ -1355,8 +1358,11 @@ static umode_t switch_attr_is_visible(struct kobject *kobj,
 		    sw->security_level == TB_SECURITY_SECURE)
 			return attr->mode;
 		return 0;
-	} else if (attr == &dev_attr_nvm_authenticate.attr ||
-		   attr == &dev_attr_nvm_version.attr) {
+	} else if (attr == &dev_attr_nvm_authenticate.attr) {
+		if (sw->dma_port && !sw->no_nvm_upgrade)
+			return attr->mode;
+		return 0;
+	} else if (attr == &dev_attr_nvm_version.attr) {
 		if (sw->dma_port)
 			return attr->mode;
 		return 0;

@@ -1707,13 +1713,17 @@ static int tb_switch_add_dma_port(struct tb_switch *sw)
 		break;
 	}
 
-	if (sw->no_nvm_upgrade)
+	/* Root switch DMA port requires running firmware */
+	if (!tb_route(sw) && sw->config.enabled)
 		return 0;
 
 	sw->dma_port = dma_port_alloc(sw);
 	if (!sw->dma_port)
 		return 0;
 
+	if (sw->no_nvm_upgrade)
+		return 0;
+
 	/*
 	 * Check status of the previous flash authentication. If there
 	 * is one we need to power cycle the switch in any case to make

Signature · 1 line

2.23.0.rc1

Mika Westerberg

author

2019-08-19 11:22 UTC #7 lore raw

PATCH v3 7/8 thunderbolt: Add support for Intel Ice Lake

In reply to Mika Westerberg

The Thunderbolt controller is integrated into the Ice Lake CPU itself and requires special flows to power it on and off using force power bit in NHI VSEC registers. Runtime PM (RTD3) and Sx flows also differ from the discrete solutions. Now the firmware notifies the driver whether RTD3 entry or exit are possible. The driver is responsible of sending Go2Sx command through link controller mailbox when system enters Sx states (suspend-to-mem/disk). Rest of the ICM firwmare flows follow Titan Ridge.

Signed-off-by Raanan Avargil <[email protected]>

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

Reviewed-by Yehezkel Bernat <[email protected]>

---
 drivers/thunderbolt/Makefile   |   2 +-
 drivers/thunderbolt/icm.c      | 176 ++++++++++++++++++++++++++++++--
 drivers/thunderbolt/nhi.c      | 112 +++++++++++++++++++--
 drivers/thunderbolt/nhi.h      |  22 ++++
 drivers/thunderbolt/nhi_ops.c  | 179 +++++++++++++++++++++++++++++++++
 drivers/thunderbolt/nhi_regs.h |  37 +++++++
 drivers/thunderbolt/switch.c   |   2 +
 drivers/thunderbolt/tb_msgs.h  |  16 ++-
 include/linux/thunderbolt.h    |   2 +
 9 files changed, 526 insertions(+), 22 deletions(-)
 create mode 100644 drivers/thunderbolt/nhi_ops.c

drivers/thunderbolt/Makefile +1 -1

index 3f55cb3c81b2..001187c577bf 100644
@@ -1,4 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-${CONFIG_THUNDERBOLT} := thunderbolt.o
-thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o tunnel.o eeprom.o
+thunderbolt-objs := nhi.o nhi_ops.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

drivers/thunderbolt/icm.c +166 ~~-10~~

302 lines not on this page · raw message

drivers/thunderbolt/nhi.c +105 -7

197 lines not on this page · raw message

drivers/thunderbolt/nhi.h +22 -0

index 1b5d47ecd3ed..b7b973949f8e 100644
@@ -30,6 +30,26 @@ enum nhi_mailbox_cmd {
 int nhi_mailbox_cmd(struct tb_nhi *nhi, enum nhi_mailbox_cmd cmd, u32 data);
 enum nhi_fw_mode nhi_mailbox_mode(struct tb_nhi *nhi);
 
+/**
+ * struct tb_nhi_ops - NHI specific optional operations
+ * @init: NHI specific initialization
+ * @suspend_noirq: NHI specific suspend_noirq hook
+ * @resume_noirq: NHI specific resume_noirq hook
+ * @runtime_suspend: NHI specific runtime_suspend hook
+ * @runtime_resume: NHI specific runtime_resume hook
+ * @shutdown: NHI specific shutdown
+ */
+struct tb_nhi_ops {
+	int (*init)(struct tb_nhi *nhi);
+	int (*suspend_noirq)(struct tb_nhi *nhi, bool wakeup);
+	int (*resume_noirq)(struct tb_nhi *nhi);
+	int (*runtime_suspend)(struct tb_nhi *nhi);
+	int (*runtime_resume)(struct tb_nhi *nhi);
+	void (*shutdown)(struct tb_nhi *nhi);
+};
+
+extern const struct tb_nhi_ops icl_nhi_ops;
+
 /*
  * PCI IDs used in this driver from Win Ridge forward. There is no
  * need for the PCI quirk anymore as we will use ICM also on Apple

@@ -51,5 +71,7 @@ enum nhi_fw_mode nhi_mailbox_mode(struct tb_nhi *nhi);
 #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
+#define PCI_DEVICE_ID_INTEL_ICL_NHI1			0x8a0d
+#define PCI_DEVICE_ID_INTEL_ICL_NHI0			0x8a17
 
 #endif

drivers/thunderbolt/nhi_ops.c new file +179 -0

179 lines not on this page · raw message

drivers/thunderbolt/nhi_regs.h +37 -0

index a60bd98c1d04..0d4970dcef84 100644
@@ -124,4 +124,41 @@ struct ring_desc {
 #define REG_FW_STS_ICM_EN_INVERT	BIT(1)
 #define REG_FW_STS_ICM_EN		BIT(0)
 
+/* ICL NHI VSEC registers */
+
+/* FW ready */
+#define VS_CAP_9			0xc8
+#define VS_CAP_9_FW_READY		BIT(31)
+/* UUID */
+#define VS_CAP_10			0xcc
+#define VS_CAP_11			0xd0
+/* LTR */
+#define VS_CAP_15			0xe0
+#define VS_CAP_16			0xe4
+/* TBT2PCIe */
+#define VS_CAP_18			0xec
+#define VS_CAP_18_DONE			BIT(0)
+/* PCIe2TBT */
+#define VS_CAP_19			0xf0
+#define VS_CAP_19_VALID			BIT(0)
+#define VS_CAP_19_CMD_SHIFT		1
+#define VS_CAP_19_CMD_MASK		GENMASK(7, 1)
+/* Force power */
+#define VS_CAP_22			0xfc
+#define VS_CAP_22_FORCE_POWER		BIT(1)
+#define VS_CAP_22_DMA_DELAY_MASK	GENMASK(31, 24)
+#define VS_CAP_22_DMA_DELAY_SHIFT	24
+
+/**
+ * enum icl_lc_mailbox_cmd - ICL specific LC mailbox commands
+ * @ICL_LC_GO2SX: Ask LC to enter Sx without wake
+ * @ICL_LC_GO2SX_NO_WAKE: Ask LC to enter Sx with wake
+ * @ICL_LC_PREPARE_FOR_RESET: Prepare LC for reset
+ */
+enum icl_lc_mailbox_cmd {
+	ICL_LC_GO2SX = 0x02,
+	ICL_LC_GO2SX_NO_WAKE = 0x03,
+	ICL_LC_PREPARE_FOR_RESET = 0x21,
+};
+
 #endif

drivers/thunderbolt/switch.c +2 -0

index 531f11fecf75..410bf1bceeee 100644
@@ -1470,6 +1470,8 @@ static int tb_switch_get_generation(struct tb_switch *sw)
 	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:
+	case PCI_DEVICE_ID_INTEL_ICL_NHI0:
+	case PCI_DEVICE_ID_INTEL_ICL_NHI1:
 		return 3;
 
 	default:

drivers/thunderbolt/tb_msgs.h +12 -4

index afbe1d29bb03..4b641e4ee0c5 100644
@@ -104,10 +104,11 @@ enum icm_pkg_code {
 };
 
 enum icm_event_code {
-	ICM_EVENT_DEVICE_CONNECTED = 3,
-	ICM_EVENT_DEVICE_DISCONNECTED = 4,
-	ICM_EVENT_XDOMAIN_CONNECTED = 6,
-	ICM_EVENT_XDOMAIN_DISCONNECTED = 7,
+	ICM_EVENT_DEVICE_CONNECTED = 0x3,
+	ICM_EVENT_DEVICE_DISCONNECTED = 0x4,
+	ICM_EVENT_XDOMAIN_CONNECTED = 0x6,
+	ICM_EVENT_XDOMAIN_DISCONNECTED = 0x7,
+	ICM_EVENT_RTD3_VETO = 0xa,
 };
 
 struct icm_pkg_header {

@@ -463,6 +464,13 @@ struct icm_tr_pkg_disconnect_xdomain_response {
 	uuid_t remote_uuid;
 };
 
+/* Ice Lake messages */
+
+struct icm_icl_event_rtd3_veto {
+	struct icm_pkg_header hdr;
+	u32 veto_reason;
+};
+
 /* XDomain messages */
 
 struct tb_xdomain_header {

include/linux/thunderbolt.h +2 -0

index 2d7e012db03f..ece782ef5466 100644
@@ -429,6 +429,7 @@ static inline struct tb_xdomain *tb_service_parent(struct tb_service *svc)
  * @lock: Must be held during ring creation/destruction. Is acquired by
  *	  interrupt_work when dispatching interrupts to individual rings.
  * @pdev: Pointer to the PCI device
+ * @ops: NHI specific optional ops
  * @iobase: MMIO space of the NHI
  * @tx_rings: All Tx rings available on this host controller
  * @rx_rings: All Rx rings available on this host controller

@@ -442,6 +443,7 @@ static inline struct tb_xdomain *tb_service_parent(struct tb_service *svc)
 struct tb_nhi {
 	spinlock_t lock;
 	struct pci_dev *pdev;
+	const struct tb_nhi_ops *ops;
 	void __iomem *iobase;
 	struct tb_ring **tx_rings;
 	struct tb_ring **rx_rings;

Signature · 1 line

2.23.0.rc1

Mika Westerberg

author

2019-08-19 11:22 UTC #8 lore raw

PATCH v3 8/8 ACPI / property: Add two new Thunderbolt property GUIDs to the list

In reply to Mika Westerberg

Ice Lake Thunderbolt controller includes two new device property compatible properties that we need to be able to extract in the driver so add them to the growing array of GUIDs.

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

Acked-by Rafael J. Wysocki <[email protected]>

---
 drivers/acpi/property.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/acpi/property.c +6 -0

index ea3d700da3ca..e095334eaec9 100644
@@ -39,6 +39,12 @@ static const guid_t prp_guids[] = {
 	/* External facing port GUID: efcc06cc-73ac-4bc3-bff0-76143807c389 */
 	GUID_INIT(0xefcc06cc, 0x73ac, 0x4bc3,
 		  0xbf, 0xf0, 0x76, 0x14, 0x38, 0x07, 0xc3, 0x89),
+	/* Thunderbolt GUID for IMR_VALID: c44d002f-69f9-4e7d-a904-a7baabdf43f7 */
+	GUID_INIT(0xc44d002f, 0x69f9, 0x4e7d,
+		  0xa9, 0x04, 0xa7, 0xba, 0xab, 0xdf, 0x43, 0xf7),
+	/* Thunderbolt GUID for WAKE_SUPPORTED: 6c501103-c189-4296-ba72-9bf5a26ebe5d */
+	GUID_INIT(0x6c501103, 0xc189, 0x4296,
+		  0xba, 0x72, 0x9b, 0xf5, 0xa2, 0x6e, 0xbe, 0x5d),
 };
 
 /* ACPI _DSD data subnodes GUID: dbb8e3e6-5886-4ba6-8795-1319f52a966b */

Signature · 1 line

2.23.0.rc1

[email protected]

2019-08-19 16:29 UTC #9 lore raw

In reply to Mika Westerberg

Quoted 99 lines

-----Original Message-----
From: Mika Westerberg <[email protected]>
Sent: Monday, August 19, 2019 6:22 AM
To: [email protected]
Cc: Andreas Noever; Michael Jamet; Yehezkel Bernat; Rafael J. Wysocki; Len Brown; Lukas Wunner; Limonciello, Mario; Anthony Wong; Rajmohan Mani; Raanan Avargil; David Laight; Mika Westerberg; [email protected]
Subject: [PATCH v3 0/8] thunderbolt: Intel Ice Lake support
[EXTERNAL EMAIL]
Hi all,
This is third iteration of the patch series adding support for the Intel Ice Lake integrated Thunderbolt controller. The biggest difference from the previous discrete controllers is that the Ice Lake Thunderbolt controller is now integrated as part of the SoC. The firmware messages pretty much follow Titan Ridge but there are some differences as well (such as the new RTD3 veto notification). Also Ice Lake does not implement security levels so DMA protection is handled by IOMMU.
The previous versions of the series can be found here:
v2: https://lwn.net/Articles/795979/
v1: https://lwn.net/Articles/793066/
Changes from v2:
* Moved ICL specific NHI ops into a separate file (nhi_ops.c)
* Postpone rescan thread during resume when if get device connected notification from the firmware. This fixes an issue that happened occasionally when resuming with full chain of devices. The driver started to remove last devices in the chain even if they are still connected.
* Move call to nhi->ops->init() before CM specific probe happens.
* Added ack from Rafael
Changes from v1:
* Check !x86_apple_machine for Titan Ridge also.
* Drop ioread32() when updating prod/cons because hardware ignores writes to the read-only parts of the register this allows us to save one read.
* Remove Light Ridge specific handling in eeprom.c as it is not necessary after patch 4/8.
* Moved RTD3 veto handling into separate functions to avoid code duplication.
* Reworked struct nhi_ops so that it provides hooks such as runtime_suspend and so on. The NHI implementation then can use those hooks to implement whatever additional logic is needed. This should allow us to move those bits into a separate files in future if needed.
I also looked whether we could replace quirk_apple_poweroff_thunderbolt() using struct nhi_ops as well but it turns out to be unsuitable. The reason is that the ACPI magic sequence needs to be done after PCI core has moved the device into D3. The NHI driver does not have easy means to do so.
* Clarified comment in icl_nhi_suspend()
* Added clarification comment to icl_nhi_force_power()
* Use "Thunderbolt" instead of "TBT" in new GUID entries
* Added reviewed tag from Yehezkel
Mika Westerberg (8):
thunderbolt: Correct path indices for PCIe tunnel
thunderbolt: Move NVM upgrade support flag to struct icm
thunderbolt: Use 32-bit writes when writing ring producer/consumer
thunderbolt: Do not fail adding switch if some port is not implemented
thunderbolt: Hide switch attributes that are not set
thunderbolt: Expose active parts of NVM even if upgrade is not supported
thunderbolt: Add support for Intel Ice Lake
ACPI / property: Add two new Thunderbolt property GUIDs to the list
 drivers/acpi/property.c        |   6 +
 drivers/thunderbolt/Makefile   |   2 +-
 drivers/thunderbolt/ctl.c      |  23 +++-
 drivers/thunderbolt/eeprom.c   |   4 -
 drivers/thunderbolt/icm.c      | 194 ++++++++++++++++++++++++++++++---
 drivers/thunderbolt/nhi.c      | 134 +++++++++++++++++++++--
 drivers/thunderbolt/nhi.h      |  22 ++++
 drivers/thunderbolt/nhi_ops.c  | 179 ++++++++++++++++++++++++++++++
 drivers/thunderbolt/nhi_regs.h |  37 +++++++
 drivers/thunderbolt/switch.c   |  52 +++++++--
 drivers/thunderbolt/tb_msgs.h  |  16 ++-
 drivers/thunderbolt/tunnel.c   |   4 +-
 include/linux/thunderbolt.h    |   2 +
 13 files changed, 622 insertions(+), 53 deletions(-)
 create mode 100644 drivers/thunderbolt/nhi_ops.c

Quoted 2 lines

--
2.23.0.rc1

I've tested this on a pre-production ICL platform. This was on top of 5.3-rc5, both of your patches from -next and this series.

I've run into a problem when using
a WD19TB that after unplugging it will cause the following to spew in dmesg:

[ 2198.017003] ============================================
[ 2198.017005] WARNING: possible recursive locking detected
[ 2198.017008] 5.3.0-rc5+ #75 Not tainted
[ 2198.017009] --------------------------------------------
[ 2198.017012] irq/122-pciehp/121 is trying to acquire lock:
[ 2198.017015] 00000000801d4de8 (&ctrl->reset_lock){.+.+}, at: pciehp_check_presence+0x1b/0x80
[ 2198.017026] 
               but task is already holding lock:
[ 2198.017028] 000000000899e2eb (&ctrl->reset_lock){.+.+}, at: pciehp_ist+0xaf/0x1c0
[ 2198.017033] 
               other info that might help us debug this:
[ 2198.017035]  Possible unsafe locking scenario:

[ 2198.017037]        CPU0
[ 2198.017038]        ----
[ 2198.017041]   lock(&ctrl->reset_lock);
[ 2198.017044]   lock(&ctrl->reset_lock);
[ 2198.017046] 
                *** DEADLOCK ***

[ 2198.017048]  May be due to missing lock nesting notation

[ 2198.017051] 3 locks held by irq/122-pciehp/121:
[ 2198.017052]  #0: 000000000899e2eb (&ctrl->reset_lock){.+.+}, at: pciehp_ist+0xaf/0x1c0
[ 2198.017057]  #1: 0000000069e1fd47 (pci_rescan_remove_lock){+.+.}, at: pciehp_unconfigure_device+0x41/0x130
[ 2198.017064]  #2: 0000000046a5bfbf (&dev->mutex){....}, at: device_release_driver_internal+0x1c/0x1a0
[ 2198.017071] 
               stack backtrace:
[ 2198.017075] CPU: 3 PID: 121 Comm: irq/122-pciehp Not tainted 5.3.0-rc5+ #75
[ 2198.017077] Hardware name: Dell Inc. XPS 13 7300/, BIOS 0.1.16 08/06/2019
[ 2198.017079] Call Trace:
[ 2198.017089]  dump_stack+0x5e/0x8b
[ 2198.017096]  __lock_acquire+0x12fe/0x1520
[ 2198.017102]  lock_acquire+0x9d/0x1a0
[ 2198.017105]  ? pciehp_check_presence+0x1b/0x80
[ 2198.017113]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017118]  down_read+0x3b/0x160
[ 2198.017121]  ? pciehp_check_presence+0x1b/0x80
[ 2198.017123]  pciehp_check_presence+0x1b/0x80
[ 2198.017128]  ? quirk_disable_msi.part.29+0x31/0x31
[ 2198.017131]  pciehp_resume+0x27/0x30
[ 2198.017136]  device_for_each_child+0x47/0x90
[ 2198.017140]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017145]  pcie_port_device_runtime_resume+0x30/0x50
[ 2198.017150]  pci_pm_runtime_resume+0x6e/0xc0
[ 2198.017156]  __rpm_callback+0xb0/0x110
[ 2198.017161]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017165]  rpm_callback+0x1f/0x70
[ 2198.017169]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017173]  rpm_resume+0x4fe/0x690
[ 2198.017179]  ? irq_forced_thread_fn+0x70/0x70
[ 2198.017183]  __pm_runtime_resume+0x47/0x80
[ 2198.017187]  device_release_driver_internal+0x7c/0x1a0
[ 2198.017191]  pci_stop_bus_device+0x5d/0x80
[ 2198.017195]  pci_stop_bus_device+0x2b/0x80
[ 2198.017198]  pci_stop_and_remove_bus_device+0xe/0x20
[ 2198.017202]  pciehp_unconfigure_device+0x7c/0x130
[ 2198.017206]  pciehp_disable_slot+0x69/0x120
[ 2198.017211]  pciehp_handle_presence_or_link_change+0x7d/0x4b0
[ 2198.017215]  pciehp_ist+0x12a/0x1c0
[ 2198.017219]  irq_thread_fn+0x1b/0x60
[ 2198.017222]  irq_thread+0x15b/0x1c0
[ 2198.017226]  ? lockdep_hardirqs_on+0x115/0x180
[ 2198.017229]  ? wake_threads_waitq+0x30/0x30
[ 2198.017233]  ? irq_thread_dtor+0x90/0x90
[ 2198.017237]  kthread+0x101/0x140
[ 2198.017241]  ? kthread_cancel_delayed_work_sync+0x10/0x10
[ 2198.017245]  ret_from_fork+0x24/0x30
[ 2198.017897] xhci_hcd 0000:03:00.0: remove, state 1
[ 2198.017917] usb usb6: USB disconnect, device number 1
[ 2198.017925] usb 6-2: USB disconnect, device number 2
[ 2198.017932] usb 6-2.3: USB disconnect, device number 3
[ 2198.017939] usb 6-2.3.1: USB disconnect, device number 5
[ 2198.019565] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[ 2198.019671] sd 0:0:0:0: [sda] Synchronize Cache(10) failed: Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK
[ 2198.057200] xhci_hcd 0000:03:00.0: xHCI host controller not responding, assume dead
[ 2198.057227] r8152 6-2.4:1.0 enxd8d090034dcb: Stop submitting intr, status -108
[ 2198.057574] usb 6-2.4: USB disconnect, device number 4
[ 2198.074541] xhci_hcd 0000:03:00.0: USB bus 6 deregistered
[ 2198.074558] xhci_hcd 0000:03:00.0: remove, state 1
[ 2198.074564] usb usb5: USB disconnect, device number 1
[ 2198.074566] usb 5-2: USB disconnect, device number 2
[ 2198.074569] usb 5-2.3: USB disconnect, device number 3
[ 2198.074571] usb 5-2.3.4: USB disconnect, device number 5
[ 2198.077367] usb 5-2.3.5: USB disconnect, device number 6
[ 2198.078488] usb 5-2.5: USB disconnect, device number 4
[ 2198.080340] xhci_hcd 0000:03:00.0: Host halt failed, -19
[ 2198.080344] xhci_hcd 0000:03:00.0: Host not accessible, reset failed.
[ 2198.080770] xhci_hcd 0000:03:00.0: USB bus 5 deregistered
[ 2198.081153] pci 0000:03:00.0: Removing from iommu group 3
[ 2198.081180] pci_bus 0000:03: busn_res: [bus 03] is released
[ 2198.081287] pci 0000:02:02.0: Removing from iommu group 3
[ 2198.081396] pci_bus 0000:04: busn_res: [bus 04-2d] is released
[ 2198.081509] pci 0000:02:04.0: Removing from iommu group 3
[ 2198.082273] pci_bus 0000:02: busn_res: [bus 02-2d] is released
[ 2198.085759] pci 0000:01:00.0: Removing from iommu group 3
[ 2198.630098] thunderbolt 0-1: device disconnected

Additionally I've found after this I can't get the dock to work by hotplugging again until I reboot the system.

Mika Westerberg

author

2019-08-19 17:57 UTC #10 lore raw

Quoted 206 lines · On Mon, Aug 19, 2019 at 04:29:35PM +0000, [email protected] wrote:

Quoted 99 lines
-----Original Message-----
From: Mika Westerberg <[email protected]>
Sent: Monday, August 19, 2019 6:22 AM
To: [email protected]
Cc: Andreas Noever; Michael Jamet; Yehezkel Bernat; Rafael J. Wysocki; Len Brown; Lukas Wunner; Limonciello, Mario; Anthony Wong; Rajmohan Mani; Raanan Avargil; David Laight; Mika Westerberg; [email protected]
Subject: [PATCH v3 0/8] thunderbolt: Intel Ice Lake support
[EXTERNAL EMAIL]
Hi all,
This is third iteration of the patch series adding support for the Intel Ice Lake integrated Thunderbolt controller. The biggest difference from the previous discrete controllers is that the Ice Lake Thunderbolt controller is now integrated as part of the SoC. The firmware messages pretty much follow Titan Ridge but there are some differences as well (such as the new RTD3 veto notification). Also Ice Lake does not implement security levels so DMA protection is handled by IOMMU.
The previous versions of the series can be found here:
v2: https://lwn.net/Articles/795979/
v1: https://lwn.net/Articles/793066/
Changes from v2:
* Moved ICL specific NHI ops into a separate file (nhi_ops.c)
* Postpone rescan thread during resume when if get device connected notification from the firmware. This fixes an issue that happened occasionally when resuming with full chain of devices. The driver started to remove last devices in the chain even if they are still connected.
* Move call to nhi->ops->init() before CM specific probe happens.
* Added ack from Rafael
Changes from v1:
* Check !x86_apple_machine for Titan Ridge also.
* Drop ioread32() when updating prod/cons because hardware ignores writes to the read-only parts of the register this allows us to save one read.
* Remove Light Ridge specific handling in eeprom.c as it is not necessary after patch 4/8.
* Moved RTD3 veto handling into separate functions to avoid code duplication.
* Reworked struct nhi_ops so that it provides hooks such as runtime_suspend and so on. The NHI implementation then can use those hooks to implement whatever additional logic is needed. This should allow us to move those bits into a separate files in future if needed.
I also looked whether we could replace quirk_apple_poweroff_thunderbolt() using struct nhi_ops as well but it turns out to be unsuitable. The reason is that the ACPI magic sequence needs to be done after PCI core has moved the device into D3. The NHI driver does not have easy means to do so.
* Clarified comment in icl_nhi_suspend()
* Added clarification comment to icl_nhi_force_power()
* Use "Thunderbolt" instead of "TBT" in new GUID entries
* Added reviewed tag from Yehezkel
Mika Westerberg (8):
thunderbolt: Correct path indices for PCIe tunnel
thunderbolt: Move NVM upgrade support flag to struct icm
thunderbolt: Use 32-bit writes when writing ring producer/consumer
thunderbolt: Do not fail adding switch if some port is not implemented
thunderbolt: Hide switch attributes that are not set
thunderbolt: Expose active parts of NVM even if upgrade is not supported
thunderbolt: Add support for Intel Ice Lake
ACPI / property: Add two new Thunderbolt property GUIDs to the list
 drivers/acpi/property.c        |   6 +
 drivers/thunderbolt/Makefile   |   2 +-
 drivers/thunderbolt/ctl.c      |  23 +++-
 drivers/thunderbolt/eeprom.c   |   4 -
 drivers/thunderbolt/icm.c      | 194 ++++++++++++++++++++++++++++++---
 drivers/thunderbolt/nhi.c      | 134 +++++++++++++++++++++--
 drivers/thunderbolt/nhi.h      |  22 ++++
 drivers/thunderbolt/nhi_ops.c  | 179 ++++++++++++++++++++++++++++++
 drivers/thunderbolt/nhi_regs.h |  37 +++++++
 drivers/thunderbolt/switch.c   |  52 +++++++--
 drivers/thunderbolt/tb_msgs.h  |  16 ++-
 drivers/thunderbolt/tunnel.c   |   4 +-
 include/linux/thunderbolt.h    |   2 +
 13 files changed, 622 insertions(+), 53 deletions(-)
 create mode 100644 drivers/thunderbolt/nhi_ops.c
Quoted 2 lines
--
2.23.0.rc1
I've tested this on a pre-production ICL platform. This was on top of 5.3-rc5, both of your patches from -next and this series.
I've run into a problem when using
a WD19TB that after unplugging it will cause the following to spew in dmesg:
[ 2198.017003] ============================================
[ 2198.017005] WARNING: possible recursive locking detected
[ 2198.017008] 5.3.0-rc5+ #75 Not tainted
[ 2198.017009] --------------------------------------------
[ 2198.017012] irq/122-pciehp/121 is trying to acquire lock:
[ 2198.017015] 00000000801d4de8 (&ctrl->reset_lock){.+.+}, at: pciehp_check_presence+0x1b/0x80
[ 2198.017026] 
               but task is already holding lock:
[ 2198.017028] 000000000899e2eb (&ctrl->reset_lock){.+.+}, at: pciehp_ist+0xaf/0x1c0
[ 2198.017033] 
               other info that might help us debug this:
[ 2198.017035]  Possible unsafe locking scenario:
[ 2198.017037]        CPU0
[ 2198.017038]        ----
[ 2198.017041]   lock(&ctrl->reset_lock);
[ 2198.017044]   lock(&ctrl->reset_lock);
[ 2198.017046] 
                *** DEADLOCK ***
[ 2198.017048]  May be due to missing lock nesting notation
[ 2198.017051] 3 locks held by irq/122-pciehp/121:
[ 2198.017052]  #0: 000000000899e2eb (&ctrl->reset_lock){.+.+}, at: pciehp_ist+0xaf/0x1c0
[ 2198.017057]  #1: 0000000069e1fd47 (pci_rescan_remove_lock){+.+.}, at: pciehp_unconfigure_device+0x41/0x130
[ 2198.017064]  #2: 0000000046a5bfbf (&dev->mutex){....}, at: device_release_driver_internal+0x1c/0x1a0
[ 2198.017071] 
               stack backtrace:
[ 2198.017075] CPU: 3 PID: 121 Comm: irq/122-pciehp Not tainted 5.3.0-rc5+ #75
[ 2198.017077] Hardware name: Dell Inc. XPS 13 7300/, BIOS 0.1.16 08/06/2019
[ 2198.017079] Call Trace:
[ 2198.017089]  dump_stack+0x5e/0x8b
[ 2198.017096]  __lock_acquire+0x12fe/0x1520
[ 2198.017102]  lock_acquire+0x9d/0x1a0
[ 2198.017105]  ? pciehp_check_presence+0x1b/0x80
[ 2198.017113]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017118]  down_read+0x3b/0x160
[ 2198.017121]  ? pciehp_check_presence+0x1b/0x80
[ 2198.017123]  pciehp_check_presence+0x1b/0x80
[ 2198.017128]  ? quirk_disable_msi.part.29+0x31/0x31
[ 2198.017131]  pciehp_resume+0x27/0x30
[ 2198.017136]  device_for_each_child+0x47/0x90
[ 2198.017140]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017145]  pcie_port_device_runtime_resume+0x30/0x50
[ 2198.017150]  pci_pm_runtime_resume+0x6e/0xc0
[ 2198.017156]  __rpm_callback+0xb0/0x110
[ 2198.017161]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017165]  rpm_callback+0x1f/0x70
[ 2198.017169]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017173]  rpm_resume+0x4fe/0x690
[ 2198.017179]  ? irq_forced_thread_fn+0x70/0x70
[ 2198.017183]  __pm_runtime_resume+0x47/0x80
[ 2198.017187]  device_release_driver_internal+0x7c/0x1a0
[ 2198.017191]  pci_stop_bus_device+0x5d/0x80
[ 2198.017195]  pci_stop_bus_device+0x2b/0x80
[ 2198.017198]  pci_stop_and_remove_bus_device+0xe/0x20
[ 2198.017202]  pciehp_unconfigure_device+0x7c/0x130
[ 2198.017206]  pciehp_disable_slot+0x69/0x120
[ 2198.017211]  pciehp_handle_presence_or_link_change+0x7d/0x4b0
[ 2198.017215]  pciehp_ist+0x12a/0x1c0
[ 2198.017219]  irq_thread_fn+0x1b/0x60
[ 2198.017222]  irq_thread+0x15b/0x1c0
[ 2198.017226]  ? lockdep_hardirqs_on+0x115/0x180
[ 2198.017229]  ? wake_threads_waitq+0x30/0x30
[ 2198.017233]  ? irq_thread_dtor+0x90/0x90
[ 2198.017237]  kthread+0x101/0x140
[ 2198.017241]  ? kthread_cancel_delayed_work_sync+0x10/0x10
[ 2198.017245]  ret_from_fork+0x24/0x30
[ 2198.017897] xhci_hcd 0000:03:00.0: remove, state 1
[ 2198.017917] usb usb6: USB disconnect, device number 1
[ 2198.017925] usb 6-2: USB disconnect, device number 2
[ 2198.017932] usb 6-2.3: USB disconnect, device number 3
[ 2198.017939] usb 6-2.3.1: USB disconnect, device number 5
[ 2198.019565] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[ 2198.019671] sd 0:0:0:0: [sda] Synchronize Cache(10) failed: Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK
[ 2198.057200] xhci_hcd 0000:03:00.0: xHCI host controller not responding, assume dead
[ 2198.057227] r8152 6-2.4:1.0 enxd8d090034dcb: Stop submitting intr, status -108
[ 2198.057574] usb 6-2.4: USB disconnect, device number 4
[ 2198.074541] xhci_hcd 0000:03:00.0: USB bus 6 deregistered
[ 2198.074558] xhci_hcd 0000:03:00.0: remove, state 1
[ 2198.074564] usb usb5: USB disconnect, device number 1
[ 2198.074566] usb 5-2: USB disconnect, device number 2
[ 2198.074569] usb 5-2.3: USB disconnect, device number 3
[ 2198.074571] usb 5-2.3.4: USB disconnect, device number 5
[ 2198.077367] usb 5-2.3.5: USB disconnect, device number 6
[ 2198.078488] usb 5-2.5: USB disconnect, device number 4
[ 2198.080340] xhci_hcd 0000:03:00.0: Host halt failed, -19
[ 2198.080344] xhci_hcd 0000:03:00.0: Host not accessible, reset failed.
[ 2198.080770] xhci_hcd 0000:03:00.0: USB bus 5 deregistered
[ 2198.081153] pci 0000:03:00.0: Removing from iommu group 3
[ 2198.081180] pci_bus 0000:03: busn_res: [bus 03] is released
[ 2198.081287] pci 0000:02:02.0: Removing from iommu group 3
[ 2198.081396] pci_bus 0000:04: busn_res: [bus 04-2d] is released
[ 2198.081509] pci 0000:02:04.0: Removing from iommu group 3
[ 2198.082273] pci_bus 0000:02: busn_res: [bus 02-2d] is released
[ 2198.085759] pci 0000:01:00.0: Removing from iommu group 3
[ 2198.630098] thunderbolt 0-1: device disconnected

Quoted 2 lines

Additionally I've found after this I can't get the dock to work by hotplugging again until I reboot the system.

Can you try if the following patch helps?

https://lore.kernel.org/patchwork/patch/1113961/

[email protected]

2019-08-19 18:22 UTC #11 lore raw

Quoted 112 lines

Quoted 1 line

I've tested this on a pre-production ICL platform. This was on top of 5.3-rc5,

both of

Quoted 11 lines

your patches from -next and this series.
I've run into a problem when using
a WD19TB that after unplugging it will cause the following to spew in dmesg:
[ 2198.017003] ============================================
[ 2198.017005] WARNING: possible recursive locking detected
[ 2198.017008] 5.3.0-rc5+ #75 Not tainted
[ 2198.017009] --------------------------------------------
[ 2198.017012] irq/122-pciehp/121 is trying to acquire lock:
[ 2198.017015] 00000000801d4de8 (&ctrl->reset_lock){.+.+}, at:

pciehp_check_presence+0x1b/0x80

Quoted 3 lines

[ 2198.017026]
               but task is already holding lock:
[ 2198.017028] 000000000899e2eb (&ctrl->reset_lock){.+.+}, at:

pciehp_ist+0xaf/0x1c0

Quoted 11 lines

[ 2198.017033]
               other info that might help us debug this:
[ 2198.017035]  Possible unsafe locking scenario:

[ 2198.017037]        CPU0
[ 2198.017038]        ----
[ 2198.017041]   lock(&ctrl->reset_lock);
[ 2198.017044]   lock(&ctrl->reset_lock);
[ 2198.017046]
                *** DEADLOCK ***

Quoted 4 lines

[ 2198.017048]  May be due to missing lock nesting notation

[ 2198.017051] 3 locks held by irq/122-pciehp/121:
[ 2198.017052]  #0: 000000000899e2eb (&ctrl->reset_lock){.+.+}, at:

pciehp_ist+0xaf/0x1c0

Quoted 1 line

[ 2198.017057]  #1: 0000000069e1fd47 (pci_rescan_remove_lock){+.+.}, at:

pciehp_unconfigure_device+0x41/0x130

Quoted 1 line

[ 2198.017064]  #2: 0000000046a5bfbf (&dev->mutex){....}, at:

device_release_driver_internal+0x1c/0x1a0

Quoted 40 lines

[ 2198.017071]
               stack backtrace:
[ 2198.017075] CPU: 3 PID: 121 Comm: irq/122-pciehp Not tainted 5.3.0-rc5+ #75
[ 2198.017079] Call Trace:
[ 2198.017089]  dump_stack+0x5e/0x8b
[ 2198.017096]  __lock_acquire+0x12fe/0x1520
[ 2198.017102]  lock_acquire+0x9d/0x1a0
[ 2198.017105]  ? pciehp_check_presence+0x1b/0x80
[ 2198.017113]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017118]  down_read+0x3b/0x160
[ 2198.017121]  ? pciehp_check_presence+0x1b/0x80
[ 2198.017123]  pciehp_check_presence+0x1b/0x80
[ 2198.017128]  ? quirk_disable_msi.part.29+0x31/0x31
[ 2198.017131]  pciehp_resume+0x27/0x30
[ 2198.017136]  device_for_each_child+0x47/0x90
[ 2198.017140]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017145]  pcie_port_device_runtime_resume+0x30/0x50
[ 2198.017150]  pci_pm_runtime_resume+0x6e/0xc0
[ 2198.017156]  __rpm_callback+0xb0/0x110
[ 2198.017161]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017165]  rpm_callback+0x1f/0x70
[ 2198.017169]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017173]  rpm_resume+0x4fe/0x690
[ 2198.017179]  ? irq_forced_thread_fn+0x70/0x70
[ 2198.017183]  __pm_runtime_resume+0x47/0x80
[ 2198.017187]  device_release_driver_internal+0x7c/0x1a0
[ 2198.017191]  pci_stop_bus_device+0x5d/0x80
[ 2198.017195]  pci_stop_bus_device+0x2b/0x80
[ 2198.017198]  pci_stop_and_remove_bus_device+0xe/0x20
[ 2198.017202]  pciehp_unconfigure_device+0x7c/0x130
[ 2198.017206]  pciehp_disable_slot+0x69/0x120
[ 2198.017211]  pciehp_handle_presence_or_link_change+0x7d/0x4b0
[ 2198.017215]  pciehp_ist+0x12a/0x1c0
[ 2198.017219]  irq_thread_fn+0x1b/0x60
[ 2198.017222]  irq_thread+0x15b/0x1c0
[ 2198.017226]  ? lockdep_hardirqs_on+0x115/0x180
[ 2198.017229]  ? wake_threads_waitq+0x30/0x30
[ 2198.017233]  ? irq_thread_dtor+0x90/0x90
[ 2198.017237]  kthread+0x101/0x140
[ 2198.017241]  ? kthread_cancel_delayed_work_sync+0x10/0x10

Quoted 8 lines

[ 2198.017245]  ret_from_fork+0x24/0x30
[ 2198.017897] xhci_hcd 0000:03:00.0: remove, state 1
[ 2198.017917] usb usb6: USB disconnect, device number 1
[ 2198.017925] usb 6-2: USB disconnect, device number 2
[ 2198.017932] usb 6-2.3: USB disconnect, device number 3
[ 2198.017939] usb 6-2.3.1: USB disconnect, device number 5
[ 2198.019565] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[ 2198.019671] sd 0:0:0:0: [sda] Synchronize Cache(10) failed: Result:

hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK

Quoted 1 line

[ 2198.057200] xhci_hcd 0000:03:00.0: xHCI host controller not responding,

assume dead

Quoted 1 line

[ 2198.057227] r8152 6-2.4:1.0 enxd8d090034dcb: Stop submitting intr, status -

108

Quoted 21 lines

[ 2198.057574] usb 6-2.4: USB disconnect, device number 4
[ 2198.074541] xhci_hcd 0000:03:00.0: USB bus 6 deregistered
[ 2198.074558] xhci_hcd 0000:03:00.0: remove, state 1
[ 2198.074564] usb usb5: USB disconnect, device number 1
[ 2198.074566] usb 5-2: USB disconnect, device number 2
[ 2198.074569] usb 5-2.3: USB disconnect, device number 3
[ 2198.074571] usb 5-2.3.4: USB disconnect, device number 5
[ 2198.077367] usb 5-2.3.5: USB disconnect, device number 6
[ 2198.078488] usb 5-2.5: USB disconnect, device number 4
[ 2198.080340] xhci_hcd 0000:03:00.0: Host halt failed, -19
[ 2198.080344] xhci_hcd 0000:03:00.0: Host not accessible, reset failed.
[ 2198.080770] xhci_hcd 0000:03:00.0: USB bus 5 deregistered
[ 2198.081153] pci 0000:03:00.0: Removing from iommu group 3
[ 2198.081180] pci_bus 0000:03: busn_res: [bus 03] is released
[ 2198.081287] pci 0000:02:02.0: Removing from iommu group 3
[ 2198.081396] pci_bus 0000:04: busn_res: [bus 04-2d] is released
[ 2198.081509] pci 0000:02:04.0: Removing from iommu group 3
[ 2198.082273] pci_bus 0000:02: busn_res: [bus 02-2d] is released
[ 2198.085759] pci 0000:01:00.0: Removing from iommu group 3
[ 2198.630098] thunderbolt 0-1: device disconnected

Quoted 7 lines

Quoted 1 line
Additionally I've found after this I can't get the dock to work by hotplugging
again
Quoted 1 line
until I reboot the system.
Can you try if the following patch helps?
https://lore.kernel.org/patchwork/patch/1113961/

I just checked with that patch on top of my previous test kernel, same call trace situation.

[email protected]

Tested-by

2019-08-19 19:36 UTC #12 lore raw

Quoted 126 lines

-----Original Message-----
From: Limonciello, Mario
Sent: Monday, August 19, 2019 1:22 PM
To: 'Mika Westerberg'
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: RE: [PATCH v3 0/8] thunderbolt: Intel Ice Lake support

Quoted 31 lines

Quoted 1 line

I've tested this on a pre-production ICL platform. This was on top of 5.3-rc5,

both of

Quoted 11 lines

your patches from -next and this series.
I've run into a problem when using
a WD19TB that after unplugging it will cause the following to spew in dmesg:
[ 2198.017003] ============================================
[ 2198.017005] WARNING: possible recursive locking detected
[ 2198.017008] 5.3.0-rc5+ #75 Not tainted
[ 2198.017009] --------------------------------------------
[ 2198.017012] irq/122-pciehp/121 is trying to acquire lock:
[ 2198.017015] 00000000801d4de8 (&ctrl->reset_lock){.+.+}, at:

pciehp_check_presence+0x1b/0x80

Quoted 3 lines

[ 2198.017026]
               but task is already holding lock:
[ 2198.017028] 000000000899e2eb (&ctrl->reset_lock){.+.+}, at:

pciehp_ist+0xaf/0x1c0

Quoted 13 lines

[ 2198.017033]
               other info that might help us debug this:
[ 2198.017035]  Possible unsafe locking scenario:

[ 2198.017037]        CPU0
[ 2198.017038]        ----
[ 2198.017041]   lock(&ctrl->reset_lock);
[ 2198.017044]   lock(&ctrl->reset_lock);
[ 2198.017046]
                *** DEADLOCK ***

[ 2198.017048]  May be due to missing lock nesting notation

Quoted 10 lines

Quoted 2 lines

[ 2198.017051] 3 locks held by irq/122-pciehp/121:
[ 2198.017052]  #0: 000000000899e2eb (&ctrl->reset_lock){.+.+}, at:

pciehp_ist+0xaf/0x1c0

Quoted 1 line

[ 2198.017057]  #1: 0000000069e1fd47 (pci_rescan_remove_lock){+.+.}, at:

pciehp_unconfigure_device+0x41/0x130

Quoted 1 line

[ 2198.017064]  #2: 0000000046a5bfbf (&dev->mutex){....}, at:

device_release_driver_internal+0x1c/0x1a0

Quoted 3 lines

[ 2198.017071]
               stack backtrace:
[ 2198.017075] CPU: 3 PID: 121 Comm: irq/122-pciehp Not tainted 5.3.0-rc5+

#75

Quoted 73 lines

Quoted 37 lines

[ 2198.017079] Call Trace:
[ 2198.017089]  dump_stack+0x5e/0x8b
[ 2198.017096]  __lock_acquire+0x12fe/0x1520
[ 2198.017102]  lock_acquire+0x9d/0x1a0
[ 2198.017105]  ? pciehp_check_presence+0x1b/0x80
[ 2198.017113]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017118]  down_read+0x3b/0x160
[ 2198.017121]  ? pciehp_check_presence+0x1b/0x80
[ 2198.017123]  pciehp_check_presence+0x1b/0x80
[ 2198.017128]  ? quirk_disable_msi.part.29+0x31/0x31
[ 2198.017131]  pciehp_resume+0x27/0x30
[ 2198.017136]  device_for_each_child+0x47/0x90
[ 2198.017140]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017145]  pcie_port_device_runtime_resume+0x30/0x50
[ 2198.017150]  pci_pm_runtime_resume+0x6e/0xc0
[ 2198.017156]  __rpm_callback+0xb0/0x110
[ 2198.017161]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017165]  rpm_callback+0x1f/0x70
[ 2198.017169]  ? pci_restore_standard_config+0x40/0x40
[ 2198.017173]  rpm_resume+0x4fe/0x690
[ 2198.017179]  ? irq_forced_thread_fn+0x70/0x70
[ 2198.017183]  __pm_runtime_resume+0x47/0x80
[ 2198.017187]  device_release_driver_internal+0x7c/0x1a0
[ 2198.017191]  pci_stop_bus_device+0x5d/0x80
[ 2198.017195]  pci_stop_bus_device+0x2b/0x80
[ 2198.017198]  pci_stop_and_remove_bus_device+0xe/0x20
[ 2198.017202]  pciehp_unconfigure_device+0x7c/0x130
[ 2198.017206]  pciehp_disable_slot+0x69/0x120
[ 2198.017211]  pciehp_handle_presence_or_link_change+0x7d/0x4b0
[ 2198.017215]  pciehp_ist+0x12a/0x1c0
[ 2198.017219]  irq_thread_fn+0x1b/0x60
[ 2198.017222]  irq_thread+0x15b/0x1c0
[ 2198.017226]  ? lockdep_hardirqs_on+0x115/0x180
[ 2198.017229]  ? wake_threads_waitq+0x30/0x30
[ 2198.017233]  ? irq_thread_dtor+0x90/0x90
[ 2198.017237]  kthread+0x101/0x140
[ 2198.017241]  ? kthread_cancel_delayed_work_sync+0x10/0x10

Quoted 8 lines

[ 2198.017245]  ret_from_fork+0x24/0x30
[ 2198.017897] xhci_hcd 0000:03:00.0: remove, state 1
[ 2198.017917] usb usb6: USB disconnect, device number 1
[ 2198.017925] usb 6-2: USB disconnect, device number 2
[ 2198.017932] usb 6-2.3: USB disconnect, device number 3
[ 2198.017939] usb 6-2.3.1: USB disconnect, device number 5
[ 2198.019565] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[ 2198.019671] sd 0:0:0:0: [sda] Synchronize Cache(10) failed: Result:

hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK

Quoted 1 line

[ 2198.057200] xhci_hcd 0000:03:00.0: xHCI host controller not responding,

assume dead

Quoted 1 line

[ 2198.057227] r8152 6-2.4:1.0 enxd8d090034dcb: Stop submitting intr, status -

108

Quoted 21 lines

[ 2198.057574] usb 6-2.4: USB disconnect, device number 4
[ 2198.074541] xhci_hcd 0000:03:00.0: USB bus 6 deregistered
[ 2198.074558] xhci_hcd 0000:03:00.0: remove, state 1
[ 2198.074564] usb usb5: USB disconnect, device number 1
[ 2198.074566] usb 5-2: USB disconnect, device number 2
[ 2198.074569] usb 5-2.3: USB disconnect, device number 3
[ 2198.074571] usb 5-2.3.4: USB disconnect, device number 5
[ 2198.077367] usb 5-2.3.5: USB disconnect, device number 6
[ 2198.078488] usb 5-2.5: USB disconnect, device number 4
[ 2198.080340] xhci_hcd 0000:03:00.0: Host halt failed, -19
[ 2198.080344] xhci_hcd 0000:03:00.0: Host not accessible, reset failed.
[ 2198.080770] xhci_hcd 0000:03:00.0: USB bus 5 deregistered
[ 2198.081153] pci 0000:03:00.0: Removing from iommu group 3
[ 2198.081180] pci_bus 0000:03: busn_res: [bus 03] is released
[ 2198.081287] pci 0000:02:02.0: Removing from iommu group 3
[ 2198.081396] pci_bus 0000:04: busn_res: [bus 04-2d] is released
[ 2198.081509] pci 0000:02:04.0: Removing from iommu group 3
[ 2198.082273] pci_bus 0000:02: busn_res: [bus 02-2d] is released
[ 2198.085759] pci 0000:01:00.0: Removing from iommu group 3
[ 2198.630098] thunderbolt 0-1: device disconnected

Quoted 1 line

Additionally I've found after this I can't get the dock to work by hotplugging

again

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
until I reboot the system.
Can you try if the following patch helps?
https://lore.kernel.org/patchwork/patch/1113961/
I just checked with that patch on top of my previous test kernel, same call trace situation.

After checking, this is not introduced by this series, it happens on v5.3-rc5 as well. It's a problem in pciehp that will be debugged separately.

So the Thunderbolt portion of this works for me on ICL system.

Tested-by Mario Limonciello <[email protected]>

Mika Westerberg

author

2019-08-20 10:02 UTC #13 lore raw

Quoted 6 lines · On Mon, Aug 19, 2019 at 07:36:37PM +0000, [email protected] wrote:

After checking, this is not introduced by this series, it happens on v5.3-rc5 as well. It's a problem in pciehp that will be debugged separately.
So the Thunderbolt portion of this works for me on ICL system.
Tested-by Mario Limonciello <[email protected]>

Thanks for testing!

Regarding the issue, can you file a kernel bugzilla about it with the necessary dmesg and 'sudo lspci -vv' output so we can track it? I'll also try to reproduce it here.

Lukas Wunner

2019-08-20 11:34 UTC #14 lore raw

In reply to [email protected]

Quoted 12 lines · On Mon, Aug 19, 2019 at 04:29:35PM +0000, [email protected] wrote:

I've run into a problem when using
a WD19TB that after unplugging it will cause the following to spew in dmesg:

[ 2198.017003] ============================================
[ 2198.017005] WARNING: possible recursive locking detected
[ 2198.017008] 5.3.0-rc5+ #75 Not tainted
[ 2198.017009] --------------------------------------------
[ 2198.017012] irq/122-pciehp/121 is trying to acquire lock:
[ 2198.017015] 00000000801d4de8 (&ctrl->reset_lock){.+.+}, at: pciehp_check_presence+0x1b/0x80
[ 2198.017026] 
               but task is already holding lock:
[ 2198.017028] 000000000899e2eb (&ctrl->reset_lock){.+.+}, at: pciehp_ist+0xaf/0x1c0

This was first reported by Theodore in April and appears to be a false positive:

https://lore.kernel.org/linux-pci/[email protected]/

Thanks,

Lukas

[email protected]

2019-08-20 17:00 UTC #15 lore raw

Quoted 34 lines

-----Original Message-----
From: Lukas Wunner <[email protected]>
Sent: Tuesday, August 20, 2019 6:34 AM
To: Limonciello, Mario
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]
Subject: Re: [PATCH v3 0/8] thunderbolt: Intel Ice Lake support
[EXTERNAL EMAIL]
Quoted 9 lines · On Mon, Aug 19, 2019 at 04:29:35PM +0000, [email protected] wrote:
I've run into a problem when using
a WD19TB that after unplugging it will cause the following to spew in dmesg:
[ 2198.017003] ============================================
[ 2198.017005] WARNING: possible recursive locking detected
[ 2198.017008] 5.3.0-rc5+ #75 Not tainted
[ 2198.017009] --------------------------------------------
[ 2198.017012] irq/122-pciehp/121 is trying to acquire lock:
[ 2198.017015] 00000000801d4de8 (&ctrl->reset_lock){.+.+}, at:
pciehp_check_presence+0x1b/0x80
Quoted 3 lines
[ 2198.017026]
               but task is already holding lock:
[ 2198.017028] 000000000899e2eb (&ctrl->reset_lock){.+.+}, at:
pciehp_ist+0xaf/0x1c0
This was first reported by Theodore in April and appears to be a false positive:

Quoted 6 lines

https://lore.kernel.org/linux-
pci/[email protected]/
Thanks,
Lukas

Indeed it does actually seem harmless and only comes up once.

I've filed https://bugzilla.kernel.org/show_bug.cgi?id=204639 to track down further what's going on.

Mika Westerberg

author

2019-08-26 09:20 UTC #16 lore raw

In reply to Mika Westerberg

Quoted 9 lines · Mika Westerberg

Hi all,
This is third iteration of the patch series adding support for the Intel Ice Lake integrated Thunderbolt controller. The biggest difference from the previous discrete controllers is that the Ice Lake Thunderbolt controller is now integrated as part of the SoC. The firmware messages pretty much follow Titan Ridge but there are some differences as well (such as the new RTD3 veto notification). Also Ice Lake does not implement security levels so DMA protection is handled by IOMMU.

Series applied to thunderbolt.git/next.