Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1656243 > unrolled thread

[PATCH v3 00/27] Thunderbolt security levels and NVM firmware upgrade

Started byMika Westerberg <mika.westerberg@linux.intel.com>
First post2017-06-02 16:10 +0200
Last post2017-06-06 08:50 +0200
Articles 20 on this page of 52 — 7 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v3 00/27] Thunderbolt security levels and NVM firmware upgrade Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    [PATCH v3 05/27] thunderbolt: Add MSI-X support Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    [PATCH v3 07/27] thunderbolt: Allow passing NULL to tb_ctl_free() Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    [PATCH v3 18/27] thunderbolt: Rework control channel to be more reliable Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    [PATCH v3 27/27] MAINTAINERS: Add maintainers for Thunderbolt driver Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    [PATCH v3 25/27] thunderbolt: Add support for host and device NVM firmware upgrade Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    [PATCH v3 01/27] thunderbolt: Use const buffer pointer in write operations Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    [PATCH v3 22/27] thunderbolt: Add support for DMA configuration based mailbox Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    [PATCH v3 06/27] thunderbolt: Rework capability handling Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
      Re: [PATCH v3 06/27] thunderbolt: Rework capability handling Lukas Wunner <lukas@wunner.de> - 2017-06-04 16:00 +0200
        Re: [PATCH v3 06/27] thunderbolt: Rework capability handling Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-04 17:10 +0200
          Re: [PATCH v3 06/27] thunderbolt: Rework capability handling Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-04 17:10 +0200
            Re: [PATCH v3 06/27] thunderbolt: Rework capability handling Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-06-04 20:20 +0200
              Re: [PATCH v3 06/27] thunderbolt: Rework capability handling Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 08:50 +0200
    [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
      Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Lukas Wunner <lukas@wunner.de> - 2017-06-05 10:20 +0200
        Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 11:40 +0200
          Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Lukas Wunner <lukas@wunner.de> - 2017-06-05 14:10 +0200
            Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 15:00 +0200
              Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 15:40 +0200
                Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Lukas Wunner <lukas@wunner.de> - 2017-06-05 15:50 +0200
                  Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 16:00 +0200
                Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-06-05 16:30 +0200
                  Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 16:50 +0200
                  Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-05 17:30 +0200
                    Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 17:50 +0200
                      RE: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs <Mario.Limonciello@dell.com> - 2017-06-05 18:00 +0200
                        Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 19:00 +0200
    [PATCH v3 17/27] thunderbolt: Let the connection manager handle all notifications Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    [PATCH v3 04/27] thunderbolt: Do not warn about newer DROM versions Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    [PATCH v3 16/27] thunderbolt: Expose make_header() to other files Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    [PATCH v3 26/27] thunderbolt: Add documentation how Thunderbolt bus can be used Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
      Re: [PATCH v3 26/27] thunderbolt: Add documentation how Thunderbolt  bus can be used Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-03 16:20 +0200
    [PATCH v3 10/27] thunderbolt: Fail switch adding operation if reading DROM fails Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    [PATCH v3 03/27] thunderbolt: Do not try to read UID if DROM offset is read as 0 Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    [PATCH v3 20/27] thunderbolt: Add support for NHI mailbox Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:10 +0200
    [PATCH v3 08/27] thunderbolt: Introduce thunderbolt bus and connection manager Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:20 +0200
      Re: [PATCH v3 08/27] thunderbolt: Introduce thunderbolt bus and  connection manager Andreas Noever <andreas.noever@gmail.com> - 2017-06-05 17:20 +0200
    [PATCH v3 02/27] thunderbolt: No need to read UID of the root switch on resume Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:20 +0200
    [PATCH v3 14/27] thunderbolt: Move control channel messages to tb_msgs.h Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:20 +0200
    [PATCH v3 13/27] thunderbolt: Read vendor and device name from DROM Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:20 +0200
    [PATCH v3 11/27] thunderbolt: Do not fail if DROM data CRC32 is invalid Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:20 +0200
    [PATCH v3 09/27] thunderbolt: Convert switch to a device Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:20 +0200
      Re: [PATCH v3 09/27] thunderbolt: Convert switch to a device Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-03 16:20 +0200
    [PATCH v3 21/27] thunderbolt: Store Thunderbolt generation in the switch structure Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-02 16:20 +0200
    Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware  upgrade Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-06-02 20:10 +0200
    Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware  upgrade Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-03 16:20 +0200
      Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware  upgrade Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 09:20 +0200
        Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware upgrade Andreas Noever <andreas.noever@gmail.com> - 2017-06-05 19:10 +0200
          Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware  upgrade Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-05 21:50 +0200
          Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware  upgrade Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-06 08:10 +0200
            Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware  upgrade Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-06-06 08:50 +0200

Page 1 of 3  [1] 2 3  Next page →


#1656243 — [PATCH v3 00/27] Thunderbolt security levels and NVM firmware upgrade

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-02 16:10 +0200
Subject[PATCH v3 00/27] Thunderbolt security levels and NVM firmware upgrade
Message-ID<tNVr4-2dT-5@gated-at.bofh.it>
Hi,

This is a third version of the patch series adding support for Thunderbolt
security levels and NVM firmware upgrade. PCs running Intel Falcon Ridge or
newer need these in order to connect devices if the security level is set
to "user(SL1) or secure(SL2)" from BIOS.

The security levels were added to to prevent DMA attacks when PCIe is
tunneled over Thunderbolt fabric where IOMMU is not available or cannot be
enabled for different reasons.

This series converts the Thunderbolt driver to a Linux bus and makes each
Thunderbolt device (aka Thunderbolt switch or router or endpoint) a Linux
device which is connected to that bus addressed by their routing string
(this is the addressing Thunderbolt uses).

With these patches the user can authorize devices using sysfs attributes
like (following USB):

  # echo 1 > /sys/bus/thunderbolt/devices/0-1/authorized

If the BIOS security level is set to "secure" and the device supports
secure connect on the first connect the device is uploaded a random key:

  # key=$(openssl rand -hex 32)
  # echo $key > /sys/bus/thunderbolt/devices/0-1/key
  # echo 1 > /sys/bus/thunderbolt/devices/0-1/authorized

Next time the device is plugged in the user can verify (challenge) the
device using the same key:

  # echo $key > /sys/bus/thunderbolt/devices/0-1/key
  # echo 2 > /sys/bus/thunderbolt/devices/0-1/authorized

The device identification information is found below each device sysfs
node and includes following attributes:

  device	- Device ID
  device_name	- Name of the device
  vendor	- Vendor ID
  vendor_name	- Name of the vendor
  unique_id	- Unique ID string identifying this particular device

In addition these patches add support for upgrading NVM firmware running on
a host or device by running something like:

  # dd if=KYK_TBT_FW_0018.bin of=/sys/bus/thunderbolt/devices/0-0/nvm_non_active0/nvmem
  # echo 1 > /sys/bus/thunderbolt/devices/0-0/nvm_authenticate

More information how to use the Thunderbolt bus can be found in patch [26/27].

Where Internal Connection Manager (ICM) firmware is available and usable,
we use it in the driver. This also includes newer Apple Macbooks with
Alpine Ridge. For older Macbooks the driver works as before but in addition
the Thunderbolt bus is available there as well (including possibility to
upgrade NVM firmware of connected devices).

Note for Macs the Linux native PCIe hotplug support does not work well with
the Thunderbolt PCIe topologies where there is need to put all available
resources to the PCIe downstream port where the PCIe chain is extended.
This is something we need to fix. In the mean time is a way to work it
around by passing "pci=hpbussize=10,hpmemsize=2M" or so to the kernel
command line.

Regarding Christoph's UUID series here [1], they now include the existing
types for backwards compatibility so this series compiles and works fine
with them. We will convert the Thunderbolt driver to use these new types
once that series hits mainline.

Andreas and Lukas, do you have any comments or concerns regarding this
series?

We are currently porting Amir's network driver on top of the Thunderbolt
bus.

The previous version (v2) can be found in [2].

[1] http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/uuid-types
[2] https://lwn.net/Articles/723935/

Changes from the previos version (v2):

[PATCH v2 03/27] thunderbolt: Do not try to read UID if DROM offset is read as 0
  - Add Suggested-by Andreas

[PATCH v2 07/27] thunderbolt: Allow passing NULL to tb_ctl_free()
  - Add Suggested-by Andy

[PATCH v2 09/27] thunderbolt: Convert switch to a device
  - Rework comment regarding UUID to mention that it is not strictly
    following rules of UUID format.
  - Update Documentation/ABI/testing/sysfs-bus-thunderbolt unique_id
    attribute description accordingly.

[PATCH v2 12/27] thunderbolt: Refactor and fix parsing of port drom entries
  - Add Reviewed-by Andy

[PATCH v2 13/27] thunderbolt: Read vendor and device name from DROM
  - Use ksrtndup() instead and drop parse_name()

[PATCH v2 21/27] thunderbolt: Store Thunderbolt generation in the switch structure
  - Make the unknown switch defaulting to generation 1 more explicit
  - Add Reviewed-by Andy

[PATCH v2 24/27] thunderbolt: Add support for Internal Connection Manager (ICM)
  - Allow secure connect only when BIOS setting is in "secure
    connect/SL2" to follow what Windows does.

[PATCH v2 25/27] thunderbolt: Add support for host and device NVM firmware upgrade
  - Add check for sw->nvm in nvm_authenticate_store() to prevent NULL
    pointer dereference when NVMem devices are not yet added.

[PATCH v2 26/27] thunderbolt: Add documentation how Thunderbolt bus can be used
  - Update documentation to mention that secure connect is only
    available when the BIOS setting is in "secure connect/SL2".

Lukas Wunner (1):
  thunderbolt: Refactor and fix parsing of port drom entries

Mika Westerberg (26):
  thunderbolt: Use const buffer pointer in write operations
  thunderbolt: No need to read UID of the root switch on resume
  thunderbolt: Do not try to read UID if DROM offset is read as 0
  thunderbolt: Do not warn about newer DROM versions
  thunderbolt: Add MSI-X support
  thunderbolt: Rework capability handling
  thunderbolt: Allow passing NULL to tb_ctl_free()
  thunderbolt: Introduce thunderbolt bus and connection manager
  thunderbolt: Convert switch to a device
  thunderbolt: Fail switch adding operation if reading DROM fails
  thunderbolt: Do not fail if DROM data CRC32 is invalid
  thunderbolt: Read vendor and device name from DROM
  thunderbolt: Move control channel messages to tb_msgs.h
  thunderbolt: Expose get_route() to other files
  thunderbolt: Expose make_header() to other files
  thunderbolt: Let the connection manager handle all notifications
  thunderbolt: Rework control channel to be more reliable
  thunderbolt: Add new Thunderbolt PCI IDs
  thunderbolt: Add support for NHI mailbox
  thunderbolt: Store Thunderbolt generation in the switch structure
  thunderbolt: Add support for DMA configuration based mailbox
  thunderbolt: Do not touch the hardware if the NHI is gone on resume
  thunderbolt: Add support for Internal Connection Manager (ICM)
  thunderbolt: Add support for host and device NVM firmware upgrade
  thunderbolt: Add documentation how Thunderbolt bus can be used
  MAINTAINERS: Add maintainers for Thunderbolt driver

 Documentation/ABI/testing/sysfs-bus-thunderbolt |  110 +++
 Documentation/admin-guide/index.rst             |    1 +
 Documentation/admin-guide/thunderbolt.rst       |  199 ++++
 MAINTAINERS                                     |    3 +
 drivers/thunderbolt/Kconfig                     |   13 +-
 drivers/thunderbolt/Makefile                    |    2 +-
 drivers/thunderbolt/cap.c                       |  169 ++--
 drivers/thunderbolt/ctl.c                       |  665 +++++++++----
 drivers/thunderbolt/ctl.h                       |  105 +-
 drivers/thunderbolt/dma_port.c                  |  524 ++++++++++
 drivers/thunderbolt/dma_port.h                  |   34 +
 drivers/thunderbolt/domain.c                    |  455 +++++++++
 drivers/thunderbolt/eeprom.c                    |  119 ++-
 drivers/thunderbolt/icm.c                       | 1089 +++++++++++++++++++++
 drivers/thunderbolt/nhi.c                       |  306 +++++-
 drivers/thunderbolt/nhi.h                       |   93 +-
 drivers/thunderbolt/nhi_regs.h                  |   27 +
 drivers/thunderbolt/switch.c                    | 1178 +++++++++++++++++++++--
 drivers/thunderbolt/tb.c                        |  237 +++--
 drivers/thunderbolt/tb.h                        |  251 ++++-
 drivers/thunderbolt/tb_msgs.h                   |  260 +++++
 drivers/thunderbolt/tb_regs.h                   |   50 +-
 drivers/thunderbolt/tunnel_pci.c                |   17 +-
 23 files changed, 5323 insertions(+), 584 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-thunderbolt
 create mode 100644 Documentation/admin-guide/thunderbolt.rst
 create mode 100644 drivers/thunderbolt/dma_port.c
 create mode 100644 drivers/thunderbolt/dma_port.h
 create mode 100644 drivers/thunderbolt/domain.c
 create mode 100644 drivers/thunderbolt/icm.c
 create mode 100644 drivers/thunderbolt/tb_msgs.h

-- 
2.11.0

[toc] | [next] | [standalone]


#1656245 — [PATCH v3 05/27] thunderbolt: Add MSI-X support

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-02 16:10 +0200
Subject[PATCH v3 05/27] thunderbolt: Add MSI-X support
Message-ID<tNVr4-2dT-13@gated-at.bofh.it>
In reply to#1656243
Intel Thunderbolt controllers support up to 16 MSI-X vectors. Using
MSI-X is preferred over MSI or legacy interrupt and may bring additional
performance because there is no need to check the status registers which
interrupt was triggered.

While there we convert comments in structs tb_ring and tb_nhi to follow
kernel-doc format more closely.

This code is based on the work done by Amir Levy and Michael Jamet.

Signed-off-by: Michael Jamet <michael.jamet@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Yehezkel Bernat <yehezkel.bernat@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/thunderbolt/ctl.c      |   4 +-
 drivers/thunderbolt/nhi.c      | 165 +++++++++++++++++++++++++++++++++++------
 drivers/thunderbolt/nhi.h      |  56 +++++++++++---
 drivers/thunderbolt/nhi_regs.h |   9 +++
 4 files changed, 198 insertions(+), 36 deletions(-)

diff --git a/drivers/thunderbolt/ctl.c b/drivers/thunderbolt/ctl.c
index 1031d97407a8..889a32dd21e7 100644
--- a/drivers/thunderbolt/ctl.c
+++ b/drivers/thunderbolt/ctl.c
@@ -488,11 +488,11 @@ struct tb_ctl *tb_ctl_alloc(struct tb_nhi *nhi, hotplug_cb cb, void *cb_data)
 	if (!ctl->frame_pool)
 		goto err;
 
-	ctl->tx = ring_alloc_tx(nhi, 0, 10);
+	ctl->tx = ring_alloc_tx(nhi, 0, 10, RING_FLAG_NO_SUSPEND);
 	if (!ctl->tx)
 		goto err;
 
-	ctl->rx = ring_alloc_rx(nhi, 0, 10);
+	ctl->rx = ring_alloc_rx(nhi, 0, 10, RING_FLAG_NO_SUSPEND);
 	if (!ctl->rx)
 		goto err;
 
diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index a8c20413dbda..ed75c49748f5 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -21,6 +21,12 @@
 
 #define RING_TYPE(ring) ((ring)->is_tx ? "TX ring" : "RX ring")
 
+/*
+ * Minimal number of vectors when we use MSI-X. Two for control channel
+ * Rx/Tx and the rest four are for cross domain DMA paths.
+ */
+#define MSIX_MIN_VECS		6
+#define MSIX_MAX_VECS		16
 
 static int ring_interrupt_index(struct tb_ring *ring)
 {
@@ -42,6 +48,37 @@ static void ring_interrupt_active(struct tb_ring *ring, bool active)
 	int bit = ring_interrupt_index(ring) & 31;
 	int mask = 1 << bit;
 	u32 old, new;
+
+	if (ring->irq > 0) {
+		u32 step, shift, ivr, misc;
+		void __iomem *ivr_base;
+		int index;
+
+		if (ring->is_tx)
+			index = ring->hop;
+		else
+			index = ring->hop + ring->nhi->hop_count;
+
+		/*
+		 * Ask the hardware to clear interrupt status bits automatically
+		 * since we already know which interrupt was triggered.
+		 */
+		misc = ioread32(ring->nhi->iobase + REG_DMA_MISC);
+		if (!(misc & REG_DMA_MISC_INT_AUTO_CLEAR)) {
+			misc |= REG_DMA_MISC_INT_AUTO_CLEAR;
+			iowrite32(misc, ring->nhi->iobase + REG_DMA_MISC);
+		}
+
+		ivr_base = ring->nhi->iobase + REG_INT_VEC_ALLOC_BASE;
+		step = index / REG_INT_VEC_ALLOC_REGS * REG_INT_VEC_ALLOC_BITS;
+		shift = index % REG_INT_VEC_ALLOC_REGS * REG_INT_VEC_ALLOC_BITS;
+		ivr = ioread32(ivr_base + step);
+		ivr &= ~(REG_INT_VEC_ALLOC_MASK << shift);
+		if (active)
+			ivr |= ring->vector << shift;
+		iowrite32(ivr, ivr_base + step);
+	}
+
 	old = ioread32(ring->nhi->iobase + reg);
 	if (active)
 		new = old | mask;
@@ -239,8 +276,50 @@ int __ring_enqueue(struct tb_ring *ring, struct ring_frame *frame)
 	return ret;
 }
 
+static irqreturn_t ring_msix(int irq, void *data)
+{
+	struct tb_ring *ring = data;
+
+	schedule_work(&ring->work);
+	return IRQ_HANDLED;
+}
+
+static int ring_request_msix(struct tb_ring *ring, bool no_suspend)
+{
+	struct tb_nhi *nhi = ring->nhi;
+	unsigned long irqflags;
+	int ret;
+
+	if (!nhi->pdev->msix_enabled)
+		return 0;
+
+	ret = ida_simple_get(&nhi->msix_ida, 0, MSIX_MAX_VECS, GFP_KERNEL);
+	if (ret < 0)
+		return ret;
+
+	ring->vector = ret;
+
+	ring->irq = pci_irq_vector(ring->nhi->pdev, ring->vector);
+	if (ring->irq < 0)
+		return ring->irq;
+
+	irqflags = no_suspend ? IRQF_NO_SUSPEND : 0;
+	return request_irq(ring->irq, ring_msix, irqflags, "thunderbolt", ring);
+}
+
+static void ring_release_msix(struct tb_ring *ring)
+{
+	if (ring->irq <= 0)
+		return;
+
+	free_irq(ring->irq, ring);
+	ida_simple_remove(&ring->nhi->msix_ida, ring->vector);
+	ring->vector = 0;
+	ring->irq = 0;
+}
+
 static struct tb_ring *ring_alloc(struct tb_nhi *nhi, u32 hop, int size,
-				  bool transmit)
+				  bool transmit, unsigned int flags)
 {
 	struct tb_ring *ring = NULL;
 	dev_info(&nhi->pdev->dev, "allocating %s ring %d of size %d\n",
@@ -271,9 +350,14 @@ static struct tb_ring *ring_alloc(struct tb_nhi *nhi, u32 hop, int size,
 	ring->hop = hop;
 	ring->is_tx = transmit;
 	ring->size = size;
+	ring->flags = flags;
 	ring->head = 0;
 	ring->tail = 0;
 	ring->running = false;
+
+	if (ring_request_msix(ring, flags & RING_FLAG_NO_SUSPEND))
+		goto err;
+
 	ring->descriptors = dma_alloc_coherent(&ring->nhi->pdev->dev,
 			size * sizeof(*ring->descriptors),
 			&ring->descriptors_dma, GFP_KERNEL | __GFP_ZERO);
@@ -295,14 +379,16 @@ static struct tb_ring *ring_alloc(struct tb_nhi *nhi, u32 hop, int size,
 	return NULL;
 }
 
-struct tb_ring *ring_alloc_tx(struct tb_nhi *nhi, int hop, int size)
+struct tb_ring *ring_alloc_tx(struct tb_nhi *nhi, int hop, int size,
+			      unsigned int flags)
 {
-	return ring_alloc(nhi, hop, size, true);
+	return ring_alloc(nhi, hop, size, true, flags);
 }
 
-struct tb_ring *ring_alloc_rx(struct tb_nhi *nhi, int hop, int size)
+struct tb_ring *ring_alloc_rx(struct tb_nhi *nhi, int hop, int size,
+			      unsigned int flags)
 {
-	return ring_alloc(nhi, hop, size, false);
+	return ring_alloc(nhi, hop, size, false, flags);
 }
 
 /**
@@ -413,6 +499,8 @@ void ring_free(struct tb_ring *ring)
 			 RING_TYPE(ring), ring->hop);
 	}
 
+	ring_release_msix(ring);
+
 	dma_free_coherent(&ring->nhi->pdev->dev,
 			  ring->size * sizeof(*ring->descriptors),
 			  ring->descriptors, ring->descriptors_dma);
@@ -428,9 +516,9 @@ void ring_free(struct tb_ring *ring)
 
 	mutex_unlock(&ring->nhi->lock);
 	/**
-	 * ring->work can no longer be scheduled (it is scheduled only by
-	 * nhi_interrupt_work and ring_stop). Wait for it to finish before
-	 * freeing the ring.
+	 * ring->work can no longer be scheduled (it is scheduled only
+	 * by nhi_interrupt_work, ring_stop and ring_msix). Wait for it
+	 * to finish before freeing the ring.
 	 */
 	flush_work(&ring->work);
 	mutex_destroy(&ring->lock);
@@ -528,9 +616,52 @@ static void nhi_shutdown(struct tb_nhi *nhi)
 	 * We have to release the irq before calling flush_work. Otherwise an
 	 * already executing IRQ handler could call schedule_work again.
 	 */
-	devm_free_irq(&nhi->pdev->dev, nhi->pdev->irq, nhi);
-	flush_work(&nhi->interrupt_work);
+	if (!nhi->pdev->msix_enabled) {
+		devm_free_irq(&nhi->pdev->dev, nhi->pdev->irq, nhi);
+		flush_work(&nhi->interrupt_work);
+	}
 	mutex_destroy(&nhi->lock);
+	ida_destroy(&nhi->msix_ida);
+}
+
+static int nhi_init_msi(struct tb_nhi *nhi)
+{
+	struct pci_dev *pdev = nhi->pdev;
+	int res, irq, nvec;
+
+	/* In case someone left them on. */
+	nhi_disable_interrupts(nhi);
+
+	ida_init(&nhi->msix_ida);
+
+	/*
+	 * The NHI has 16 MSI-X vectors or a single MSI. We first try to
+	 * get all MSI-X vectors and if we succeed, each ring will have
+	 * one MSI-X. If for some reason that does not work out, we
+	 * fallback to a single MSI.
+	 */
+	nvec = pci_alloc_irq_vectors(pdev, MSIX_MIN_VECS, MSIX_MAX_VECS,
+				     PCI_IRQ_MSIX);
+	if (nvec < 0) {
+		nvec = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI);
+		if (nvec < 0)
+			return nvec;
+
+		INIT_WORK(&nhi->interrupt_work, nhi_interrupt_work);
+
+		irq = pci_irq_vector(nhi->pdev, 0);
+		if (irq < 0)
+			return irq;
+
+		res = devm_request_irq(&pdev->dev, irq, nhi_msi,
+				       IRQF_NO_SUSPEND, "thunderbolt", nhi);
+		if (res) {
+			dev_err(&pdev->dev, "request_irq failed, aborting\n");
+			return res;
+		}
+	}
+
+	return 0;
 }
 
 static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
@@ -545,12 +676,6 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		return res;
 	}
 
-	res = pci_enable_msi(pdev);
-	if (res) {
-		dev_err(&pdev->dev, "cannot enable MSI, aborting\n");
-		return res;
-	}
-
 	res = pcim_iomap_regions(pdev, 1 << 0, "thunderbolt");
 	if (res) {
 		dev_err(&pdev->dev, "cannot obtain PCI resources, aborting\n");
@@ -568,7 +693,6 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (nhi->hop_count != 12 && nhi->hop_count != 32)
 		dev_warn(&pdev->dev, "unexpected hop count: %d\n",
 			 nhi->hop_count);
-	INIT_WORK(&nhi->interrupt_work, nhi_interrupt_work);
 
 	nhi->tx_rings = devm_kcalloc(&pdev->dev, nhi->hop_count,
 				     sizeof(*nhi->tx_rings), GFP_KERNEL);
@@ -577,12 +701,9 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (!nhi->tx_rings || !nhi->rx_rings)
 		return -ENOMEM;
 
-	nhi_disable_interrupts(nhi); /* In case someone left them on. */
-	res = devm_request_irq(&pdev->dev, pdev->irq, nhi_msi,
-			       IRQF_NO_SUSPEND, /* must work during _noirq */
-			       "thunderbolt", nhi);
+	res = nhi_init_msi(nhi);
 	if (res) {
-		dev_err(&pdev->dev, "request_irq failed, aborting\n");
+		dev_err(&pdev->dev, "cannot enable MSI, aborting\n");
 		return res;
 	}
 
diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
index 317242939b31..630f44140530 100644
--- a/drivers/thunderbolt/nhi.h
+++ b/drivers/thunderbolt/nhi.h
@@ -7,45 +7,75 @@
 #ifndef DSL3510_H_
 #define DSL3510_H_
 
+#include <linux/idr.h>
 #include <linux/mutex.h>
 #include <linux/workqueue.h>
 
 /**
  * struct tb_nhi - thunderbolt native host interface
+ * @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
+ * @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
+ * @msix_ida: Used to allocate MSI-X vectors for rings
+ * @interrupt_work: Work scheduled to handle ring interrupt when no
+ *		    MSI-X is used.
+ * @hop_count: Number of rings (end point hops) supported by NHI.
  */
 struct tb_nhi {
-	struct mutex lock; /*
-			    * Must be held during ring creation/destruction.
-			    * Is acquired by interrupt_work when dispatching
-			    * interrupts to individual rings.
-			    **/
+	struct mutex lock;
 	struct pci_dev *pdev;
 	void __iomem *iobase;
 	struct tb_ring **tx_rings;
 	struct tb_ring **rx_rings;
+	struct ida msix_ida;
 	struct work_struct interrupt_work;
-	u32 hop_count; /* Number of rings (end point hops) supported by NHI. */
+	u32 hop_count;
 };
 
 /**
  * struct tb_ring - thunderbolt TX or RX ring associated with a NHI
+ * @lock: Lock serializing actions to this ring. Must be acquired after
+ *	  nhi->lock.
+ * @nhi: Pointer to the native host controller interface
+ * @size: Size of the ring
+ * @hop: Hop (DMA channel) associated with this ring
+ * @head: Head of the ring (write next descriptor here)
+ * @tail: Tail of the ring (complete next descriptor here)
+ * @descriptors: Allocated descriptors for this ring
+ * @queue: Queue holding frames to be transferred over this ring
+ * @in_flight: Queue holding frames that are currently in flight
+ * @work: Interrupt work structure
+ * @is_tx: Is the ring Tx or Rx
+ * @running: Is the ring running
+ * @irq: MSI-X irq number if the ring uses MSI-X. %0 otherwise.
+ * @vector: MSI-X vector number the ring uses (only set if @irq is > 0)
+ * @flags: Ring specific flags
  */
 struct tb_ring {
-	struct mutex lock; /* must be acquired after nhi->lock */
+	struct mutex lock;
 	struct tb_nhi *nhi;
 	int size;
 	int hop;
-	int head; /* write next descriptor here */
-	int tail; /* complete next descriptor here */
+	int head;
+	int tail;
 	struct ring_desc *descriptors;
 	dma_addr_t descriptors_dma;
 	struct list_head queue;
 	struct list_head in_flight;
 	struct work_struct work;
-	bool is_tx:1; /* rx otherwise */
+	bool is_tx:1;
 	bool running:1;
+	int irq;
+	u8 vector;
+	unsigned int flags;
 };
 
+/* Leave ring interrupt enabled on suspend */
+#define RING_FLAG_NO_SUSPEND	BIT(0)
+
 struct ring_frame;
 typedef void (*ring_cb)(struct tb_ring*, struct ring_frame*, bool canceled);
 
@@ -64,8 +94,10 @@ struct ring_frame {
 
 #define TB_FRAME_SIZE 0x100    /* minimum size for ring_rx */
 
-struct tb_ring *ring_alloc_tx(struct tb_nhi *nhi, int hop, int size);
-struct tb_ring *ring_alloc_rx(struct tb_nhi *nhi, int hop, int size);
+struct tb_ring *ring_alloc_tx(struct tb_nhi *nhi, int hop, int size,
+			      unsigned int flags);
+struct tb_ring *ring_alloc_rx(struct tb_nhi *nhi, int hop, int size,
+			      unsigned int flags);
 void ring_start(struct tb_ring *ring);
 void ring_stop(struct tb_ring *ring);
 void ring_free(struct tb_ring *ring);
diff --git a/drivers/thunderbolt/nhi_regs.h b/drivers/thunderbolt/nhi_regs.h
index 75cf0691e6c5..48b98d3c7e6a 100644
--- a/drivers/thunderbolt/nhi_regs.h
+++ b/drivers/thunderbolt/nhi_regs.h
@@ -95,7 +95,16 @@ struct ring_desc {
 #define REG_RING_INTERRUPT_BASE	0x38200
 #define RING_INTERRUPT_REG_COUNT(nhi) ((31 + 2 * nhi->hop_count) / 32)
 
+/* Interrupt Vector Allocation */
+#define REG_INT_VEC_ALLOC_BASE	0x38c40
+#define REG_INT_VEC_ALLOC_BITS	4
+#define REG_INT_VEC_ALLOC_MASK	GENMASK(3, 0)
+#define REG_INT_VEC_ALLOC_REGS	(32 / REG_INT_VEC_ALLOC_BITS)
+
 /* The last 11 bits contain the number of hops supported by the NHI port. */
 #define REG_HOP_COUNT		0x39640
 
+#define REG_DMA_MISC			0x39864
+#define REG_DMA_MISC_INT_AUTO_CLEAR     BIT(2)
+
 #endif
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]


#1656246 — [PATCH v3 07/27] thunderbolt: Allow passing NULL to tb_ctl_free()

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-02 16:10 +0200
Subject[PATCH v3 07/27] thunderbolt: Allow passing NULL to tb_ctl_free()
Message-ID<tNVr4-2dT-19@gated-at.bofh.it>
In reply to#1656243
Following the usual pattern used in many places, we allow passing NULL
pointer to tb_ctl_free(). Then the user can call the function regardless
if it has allocated control channel or not making the code bit simpler.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/thunderbolt/ctl.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/thunderbolt/ctl.c b/drivers/thunderbolt/ctl.c
index 889a32dd21e7..f8290a577b2b 100644
--- a/drivers/thunderbolt/ctl.c
+++ b/drivers/thunderbolt/ctl.c
@@ -520,6 +520,10 @@ struct tb_ctl *tb_ctl_alloc(struct tb_nhi *nhi, hotplug_cb cb, void *cb_data)
 void tb_ctl_free(struct tb_ctl *ctl)
 {
 	int i;
+
+	if (!ctl)
+		return;
+
 	if (ctl->rx)
 		ring_free(ctl->rx);
 	if (ctl->tx)
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]


#1656247 — [PATCH v3 18/27] thunderbolt: Rework control channel to be more reliable

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-02 16:10 +0200
Subject[PATCH v3 18/27] thunderbolt: Rework control channel to be more reliable
Message-ID<tNVr4-2dT-23@gated-at.bofh.it>
In reply to#1656243
If a request times out the response might arrive right after the request
is failed. This response is pushed to the kfifo and next request will
read it instead. Since it most likely will not pass our validation
checks in parse_header() the next request will fail as well, and
response to that request will be pushed to the kfifo, ad infinitum.

We end up in a situation where all requests fail and no devices can be
added anymore until the driver is unloaded and reloaded again.

To overcome this, rework the control channel so that we will have a
queue of outstanding requests. Each request will be handled in turn and
the response is validated against what is expected. Unexpected packets
(for example responses for requests that have been timed out) are
dropped. This model is copied from Greybus implementation with small
changes here and there to get it cope with Thunderbolt control packets.

In addition the configuration packets support sequence number which the
switch is supposed to copy from the request to response. We use this to
drop responses that are already timed out. Taking advantage of the
sequence number, we automatically retry configuration read/write 4 times
before giving up.

Also timeout is not a programming error so there is no need to trigger a
scary backtrace (WARN), instead we just log a warning.  After all
Thunderbolt devices are hot-pluggable by definition which means user can
unplug a device any time and that is totally acceptable.

With this change there is no need to take the global domain lock when
sending configuration packets anymore. This is useful when we add
support for cross-domain (XDomain) communication later on.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Yehezkel Bernat <yehezkel.bernat@intel.com>
Reviewed-by: Michael Jamet <michael.jamet@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/thunderbolt/ctl.c | 477 +++++++++++++++++++++++++++++++++++++++-------
 drivers/thunderbolt/ctl.h |  65 +++++++
 drivers/thunderbolt/tb.h  |   2 +-
 3 files changed, 473 insertions(+), 71 deletions(-)

diff --git a/drivers/thunderbolt/ctl.c b/drivers/thunderbolt/ctl.c
index 5417ed244edc..27c30ff79a84 100644
--- a/drivers/thunderbolt/ctl.c
+++ b/drivers/thunderbolt/ctl.c
@@ -5,22 +5,17 @@
  */
 
 #include <linux/crc32.h>
+#include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/pci.h>
 #include <linux/dmapool.h>
 #include <linux/workqueue.h>
-#include <linux/kfifo.h>
 
 #include "ctl.h"
 
 
-struct ctl_pkg {
-	struct tb_ctl *ctl;
-	void *buffer;
-	struct ring_frame frame;
-};
-
-#define TB_CTL_RX_PKG_COUNT 10
+#define TB_CTL_RX_PKG_COUNT	10
+#define TB_CTL_RETRIES		4
 
 /**
  * struct tb_cfg - thunderbolt control channel
@@ -32,8 +27,9 @@ struct tb_ctl {
 
 	struct dma_pool *frame_pool;
 	struct ctl_pkg *rx_packets[TB_CTL_RX_PKG_COUNT];
-	DECLARE_KFIFO(response_fifo, struct ctl_pkg*, 16);
-	struct completion response_ready;
+	struct mutex request_queue_lock;
+	struct list_head request_queue;
+	bool running;
 
 	event_cb callback;
 	void *callback_data;
@@ -55,10 +51,121 @@ struct tb_ctl {
 #define tb_ctl_dbg(ctl, format, arg...) \
 	dev_dbg(&(ctl)->nhi->pdev->dev, format, ## arg)
 
+static DECLARE_WAIT_QUEUE_HEAD(tb_cfg_request_cancel_queue);
+/* Serializes access to request kref_get/put */
+static DEFINE_MUTEX(tb_cfg_request_lock);
+
+/**
+ * tb_cfg_request_alloc() - Allocates a new config request
+ *
+ * This is refcounted object so when you are done with this, call
+ * tb_cfg_request_put() to it.
+ */
+struct tb_cfg_request *tb_cfg_request_alloc(void)
+{
+	struct tb_cfg_request *req;
+
+	req = kzalloc(sizeof(*req), GFP_KERNEL);
+	if (!req)
+		return NULL;
+
+	kref_init(&req->kref);
+
+	return req;
+}
+
+/**
+ * tb_cfg_request_get() - Increase refcount of a request
+ * @req: Request whose refcount is increased
+ */
+void tb_cfg_request_get(struct tb_cfg_request *req)
+{
+	mutex_lock(&tb_cfg_request_lock);
+	kref_get(&req->kref);
+	mutex_unlock(&tb_cfg_request_lock);
+}
+
+static void tb_cfg_request_destroy(struct kref *kref)
+{
+	struct tb_cfg_request *req = container_of(kref, typeof(*req), kref);
+
+	kfree(req);
+}
+
+/**
+ * tb_cfg_request_put() - Decrease refcount and possibly release the request
+ * @req: Request whose refcount is decreased
+ *
+ * Call this function when you are done with the request. When refcount
+ * goes to %0 the object is released.
+ */
+void tb_cfg_request_put(struct tb_cfg_request *req)
+{
+	mutex_lock(&tb_cfg_request_lock);
+	kref_put(&req->kref, tb_cfg_request_destroy);
+	mutex_unlock(&tb_cfg_request_lock);
+}
+
+static int tb_cfg_request_enqueue(struct tb_ctl *ctl,
+				  struct tb_cfg_request *req)
+{
+	WARN_ON(test_bit(TB_CFG_REQUEST_ACTIVE, &req->flags));
+	WARN_ON(req->ctl);
+
+	mutex_lock(&ctl->request_queue_lock);
+	if (!ctl->running) {
+		mutex_unlock(&ctl->request_queue_lock);
+		return -ENOTCONN;
+	}
+	req->ctl = ctl;
+	list_add_tail(&req->list, &ctl->request_queue);
+	set_bit(TB_CFG_REQUEST_ACTIVE, &req->flags);
+	mutex_unlock(&ctl->request_queue_lock);
+	return 0;
+}
+
+static void tb_cfg_request_dequeue(struct tb_cfg_request *req)
+{
+	struct tb_ctl *ctl = req->ctl;
+
+	mutex_lock(&ctl->request_queue_lock);
+	list_del(&req->list);
+	clear_bit(TB_CFG_REQUEST_ACTIVE, &req->flags);
+	if (test_bit(TB_CFG_REQUEST_CANCELED, &req->flags))
+		wake_up(&tb_cfg_request_cancel_queue);
+	mutex_unlock(&ctl->request_queue_lock);
+}
+
+static bool tb_cfg_request_is_active(struct tb_cfg_request *req)
+{
+	return test_bit(TB_CFG_REQUEST_ACTIVE, &req->flags);
+}
+
+static struct tb_cfg_request *
+tb_cfg_request_find(struct tb_ctl *ctl, struct ctl_pkg *pkg)
+{
+	struct tb_cfg_request *req;
+	bool found = false;
+
+	mutex_lock(&pkg->ctl->request_queue_lock);
+	list_for_each_entry(req, &pkg->ctl->request_queue, list) {
+		tb_cfg_request_get(req);
+		if (req->match(req, pkg)) {
+			found = true;
+			break;
+		}
+		tb_cfg_request_put(req);
+	}
+	mutex_unlock(&pkg->ctl->request_queue_lock);
+
+	return found ? req : NULL;
+}
+
 /* utility functions */
 
-static int check_header(struct ctl_pkg *pkg, u32 len, enum tb_cfg_pkg_type type,
-			u64 route)
+
+static int check_header(const struct ctl_pkg *pkg, u32 len,
+			enum tb_cfg_pkg_type type, u64 route)
 {
 	struct tb_cfg_header *header = pkg->buffer;
 
@@ -100,8 +207,6 @@ static int check_config_address(struct tb_cfg_address addr,
 	if (WARN(length != addr.length, "wrong space (expected %x, got %x\n)",
 			length, addr.length))
 		return -EIO;
-	if (WARN(addr.seq, "addr.seq is %#x\n", addr.seq))
-		return -EIO;
 	/*
 	 * We cannot check addr->port as it is set to the upstream port of the
 	 * sender.
@@ -109,7 +214,7 @@ static int check_config_address(struct tb_cfg_address addr,
 	return 0;
 }
 
-static struct tb_cfg_result decode_error(struct ctl_pkg *response)
+static struct tb_cfg_result decode_error(const struct ctl_pkg *response)
 {
 	struct cfg_error_pkg *pkg = response->buffer;
 	struct tb_cfg_result res = { 0 };
@@ -130,7 +235,7 @@ static struct tb_cfg_result decode_error(struct ctl_pkg *response)
 
 }
 
-static struct tb_cfg_result parse_header(struct ctl_pkg *pkg, u32 len,
+static struct tb_cfg_result parse_header(const struct ctl_pkg *pkg, u32 len,
 					 enum tb_cfg_pkg_type type, u64 route)
 {
 	struct tb_cfg_header *header = pkg->buffer;
@@ -198,7 +303,7 @@ static void be32_to_cpu_array(u32 *dst, __be32 *src, size_t len)
 		dst[i] = be32_to_cpu(src[i]);
 }
 
-static __be32 tb_crc(void *data, size_t len)
+static __be32 tb_crc(const void *data, size_t len)
 {
 	return cpu_to_be32(~__crc32c_le(~0, data, len));
 }
@@ -315,6 +420,7 @@ static void tb_ctl_rx_callback(struct tb_ring *ring, struct ring_frame *frame,
 			       bool canceled)
 {
 	struct ctl_pkg *pkg = container_of(frame, typeof(*pkg), frame);
+	struct tb_cfg_request *req;
 	__be32 crc32;
 
 	if (canceled)
@@ -361,48 +467,135 @@ static void tb_ctl_rx_callback(struct tb_ring *ring, struct ring_frame *frame,
 		goto rx;
 
 	default:
-		tb_ctl_dbg(pkg->ctl, "RX: unknown package %#x, dropping\n",
-			   frame->eof);
-		goto rx;
+		break;
 	}
 
-	if (!kfifo_put(&pkg->ctl->response_fifo, pkg)) {
-		tb_ctl_err(pkg->ctl, "RX: fifo is full\n");
-		goto rx;
+	/*
+	 * The received packet will be processed only if there is an
+	 * active request and that the packet is what is expected. This
+	 * prevents packets such as replies coming after timeout has
+	 * triggered from messing with the active requests.
+	 */
+	req = tb_cfg_request_find(pkg->ctl, pkg);
+	if (req) {
+		if (req->copy(req, pkg))
+			schedule_work(&req->work);
+		tb_cfg_request_put(req);
 	}
-	complete(&pkg->ctl->response_ready);
-	return;
+
 rx:
 	tb_ctl_rx_submit(pkg);
 }
 
+static void tb_cfg_request_work(struct work_struct *work)
+{
+	struct tb_cfg_request *req = container_of(work, typeof(*req), work);
+
+	if (!test_bit(TB_CFG_REQUEST_CANCELED, &req->flags))
+		req->callback(req->callback_data);
+
+	tb_cfg_request_dequeue(req);
+	tb_cfg_request_put(req);
+}
+
 /**
- * tb_ctl_rx() - receive a packet from the control channel
+ * tb_cfg_request() - Start control request not waiting for it to complete
+ * @ctl: Control channel to use
+ * @req: Request to start
+ * @callback: Callback called when the request is completed
+ * @callback_data: Data to be passed to @callback
+ *
+ * This queues @req on the given control channel without waiting for it
+ * to complete. When the request completes @callback is called.
  */
-static struct tb_cfg_result tb_ctl_rx(struct tb_ctl *ctl, void *buffer,
-				      size_t length, int timeout_msec,
-				      u64 route, enum tb_cfg_pkg_type type)
+int tb_cfg_request(struct tb_ctl *ctl, struct tb_cfg_request *req,
+		   void (*callback)(void *), void *callback_data)
 {
-	struct tb_cfg_result res;
-	struct ctl_pkg *pkg;
+	int ret;
 
-	if (!wait_for_completion_timeout(&ctl->response_ready,
-					 msecs_to_jiffies(timeout_msec))) {
-		tb_ctl_WARN(ctl, "RX: timeout\n");
-		return (struct tb_cfg_result) { .err = -ETIMEDOUT };
-	}
-	if (!kfifo_get(&ctl->response_fifo, &pkg)) {
-		tb_ctl_WARN(ctl, "empty kfifo\n");
-		return (struct tb_cfg_result) { .err = -EIO };
-	}
+	req->flags = 0;
+	req->callback = callback;
+	req->callback_data = callback_data;
+	INIT_WORK(&req->work, tb_cfg_request_work);
+	INIT_LIST_HEAD(&req->list);
 
-	res = parse_header(pkg, length, type, route);
-	if (!res.err)
-		memcpy(buffer, pkg->buffer, length);
-	tb_ctl_rx_submit(pkg);
-	return res;
+	tb_cfg_request_get(req);
+	ret = tb_cfg_request_enqueue(ctl, req);
+	if (ret)
+		goto err_put;
+
+	ret = tb_ctl_tx(ctl, req->request, req->request_size,
+			req->request_type);
+	if (ret)
+		goto err_dequeue;
+
+	if (!req->response)
+		schedule_work(&req->work);
+
+	return 0;
+
+err_dequeue:
+	tb_cfg_request_dequeue(req);
+err_put:
+	tb_cfg_request_put(req);
+
+	return ret;
+}
+
+/**
+ * tb_cfg_request_cancel() - Cancel a control request
+ * @req: Request to cancel
+ * @err: Error to assign to the request
+ *
+ * This function can be used to cancel ongoing request. It will wait
+ * until the request is not active anymore.
+ */
+void tb_cfg_request_cancel(struct tb_cfg_request *req, int err)
+{
+	set_bit(TB_CFG_REQUEST_CANCELED, &req->flags);
+	schedule_work(&req->work);
+	wait_event(tb_cfg_request_cancel_queue, !tb_cfg_request_is_active(req));
+	req->result.err = err;
 }
 
+static void tb_cfg_request_complete(void *data)
+{
+	complete(data);
+}
+
+/**
+ * tb_cfg_request_sync() - Start control request and wait until it completes
+ * @ctl: Control channel to use
+ * @req: Request to start
+ * @timeout_msec: Timeout how long to wait @req to complete
+ *
+ * Starts a control request and waits until it completes. If timeout
+ * triggers the request is canceled before function returns. Note the
+ * caller needs to make sure only one message for given switch is active
+ * at a time.
+ */
+struct tb_cfg_result tb_cfg_request_sync(struct tb_ctl *ctl,
+					 struct tb_cfg_request *req,
+					 int timeout_msec)
+{
+	unsigned long timeout = msecs_to_jiffies(timeout_msec);
+	struct tb_cfg_result res = { 0 };
+	DECLARE_COMPLETION_ONSTACK(done);
+	int ret;
+
+	ret = tb_cfg_request(ctl, req, tb_cfg_request_complete, &done);
+	if (ret) {
+		res.err = ret;
+		return res;
+	}
+
+	if (!wait_for_completion_timeout(&done, timeout))
+		tb_cfg_request_cancel(req, -ETIMEDOUT);
+
+	flush_work(&req->work);
+
+	return req->result;
+}
 
 /* public interface, alloc/start/stop/free */
 
@@ -423,8 +616,8 @@ struct tb_ctl *tb_ctl_alloc(struct tb_nhi *nhi, event_cb cb, void *cb_data)
 	ctl->callback = cb;
 	ctl->callback_data = cb_data;
 
-	init_completion(&ctl->response_ready);
-	INIT_KFIFO(ctl->response_fifo);
+	mutex_init(&ctl->request_queue_lock);
+	INIT_LIST_HEAD(&ctl->request_queue);
 	ctl->frame_pool = dma_pool_create("thunderbolt_ctl", &nhi->pdev->dev,
 					 TB_FRAME_SIZE, 4, 0);
 	if (!ctl->frame_pool)
@@ -492,6 +685,8 @@ void tb_ctl_start(struct tb_ctl *ctl)
 	ring_start(ctl->rx);
 	for (i = 0; i < TB_CTL_RX_PKG_COUNT; i++)
 		tb_ctl_rx_submit(ctl->rx_packets[i]);
+
+	ctl->running = true;
 }
 
 /**
@@ -504,12 +699,16 @@ void tb_ctl_start(struct tb_ctl *ctl)
  */
 void tb_ctl_stop(struct tb_ctl *ctl)
 {
+	mutex_lock(&ctl->request_queue_lock);
+	ctl->running = false;
+	mutex_unlock(&ctl->request_queue_lock);
+
 	ring_stop(ctl->rx);
 	ring_stop(ctl->tx);
 
-	if (!kfifo_is_empty(&ctl->response_fifo))
-		tb_ctl_WARN(ctl, "dangling response in response_fifo\n");
-	kfifo_reset(&ctl->response_fifo);
+	if (!list_empty(&ctl->request_queue))
+		tb_ctl_WARN(ctl, "dangling request in request_queue\n");
+	INIT_LIST_HEAD(&ctl->request_queue);
 	tb_ctl_info(ctl, "control channel stopped\n");
 }
 
@@ -532,6 +731,49 @@ int tb_cfg_error(struct tb_ctl *ctl, u64 route, u32 port,
 	return tb_ctl_tx(ctl, &pkg, sizeof(pkg), TB_CFG_PKG_ERROR);
 }
 
+static bool tb_cfg_match(const struct tb_cfg_request *req,
+			 const struct ctl_pkg *pkg)
+{
+	u64 route = tb_cfg_get_route(pkg->buffer) & ~BIT_ULL(63);
+
+	if (pkg->frame.eof == TB_CFG_PKG_ERROR)
+		return true;
+
+	if (pkg->frame.eof != req->response_type)
+		return false;
+	if (route != tb_cfg_get_route(req->request))
+		return false;
+	if (pkg->frame.size != req->response_size)
+		return false;
+
+	if (pkg->frame.eof == TB_CFG_PKG_READ ||
+	    pkg->frame.eof == TB_CFG_PKG_WRITE) {
+		const struct cfg_read_pkg *req_hdr = req->request;
+		const struct cfg_read_pkg *res_hdr = pkg->buffer;
+
+		if (req_hdr->addr.seq != res_hdr->addr.seq)
+			return false;
+	}
+
+	return true;
+}
+
+static bool tb_cfg_copy(struct tb_cfg_request *req, const struct ctl_pkg *pkg)
+{
+	struct tb_cfg_result res;
+
+	/* Now make sure it is in expected format */
+	res = parse_header(pkg, req->response_size, req->response_type,
+			   tb_cfg_get_route(req->request));
+	if (!res.err)
+		memcpy(req->response, pkg->buffer, req->response_size);
+
+	req->result = res;
+
+	/* Always complete when first response is received */
+	return true;
+}
+
 /**
  * tb_cfg_reset() - send a reset packet and wait for a response
  *
@@ -542,16 +784,31 @@ int tb_cfg_error(struct tb_ctl *ctl, u64 route, u32 port,
 struct tb_cfg_result tb_cfg_reset(struct tb_ctl *ctl, u64 route,
 				  int timeout_msec)
 {
-	int err;
 	struct cfg_reset_pkg request = { .header = tb_cfg_make_header(route) };
+	struct tb_cfg_result res = { 0 };
 	struct tb_cfg_header reply;
+	struct tb_cfg_request *req;
+
+	req = tb_cfg_request_alloc();
+	if (!req) {
+		res.err = -ENOMEM;
+		return res;
+	}
+
+	req->match = tb_cfg_match;
+	req->copy = tb_cfg_copy;
+	req->request = &request;
+	req->request_size = sizeof(request);
+	req->request_type = TB_CFG_PKG_RESET;
+	req->response = &reply;
+	req->response_size = sizeof(reply);
+	req->response_type = sizeof(TB_CFG_PKG_RESET);
+
+	res = tb_cfg_request_sync(ctl, req, timeout_msec);
 
-	err = tb_ctl_tx(ctl, &request, sizeof(request), TB_CFG_PKG_RESET);
-	if (err)
-		return (struct tb_cfg_result) { .err = err };
+	tb_cfg_request_put(req);
 
-	return tb_ctl_rx(ctl, &reply, sizeof(reply), timeout_msec, route,
-			 TB_CFG_PKG_RESET);
+	return res;
 }
 
 /**
@@ -574,13 +831,39 @@ struct tb_cfg_result tb_cfg_read_raw(struct tb_ctl *ctl, void *buffer,
 		},
 	};
 	struct cfg_write_pkg reply;
+	int retries = 0;
 
-	res.err = tb_ctl_tx(ctl, &request, sizeof(request), TB_CFG_PKG_READ);
-	if (res.err)
-		return res;
+	while (retries < TB_CTL_RETRIES) {
+		struct tb_cfg_request *req;
+
+		req = tb_cfg_request_alloc();
+		if (!req) {
+			res.err = -ENOMEM;
+			return res;
+		}
+
+		request.addr.seq = retries++;
+
+		req->match = tb_cfg_match;
+		req->copy = tb_cfg_copy;
+		req->request = &request;
+		req->request_size = sizeof(request);
+		req->request_type = TB_CFG_PKG_READ;
+		req->response = &reply;
+		req->response_size = 12 + 4 * length;
+		req->response_type = TB_CFG_PKG_READ;
+
+		res = tb_cfg_request_sync(ctl, req, timeout_msec);
+
+		tb_cfg_request_put(req);
+
+		if (res.err != -ETIMEDOUT)
+			break;
+
+		/* Wait a bit (arbitrary time) until we send a retry */
+		usleep_range(10, 100);
+	}
 
-	res = tb_ctl_rx(ctl, &reply, 12 + 4 * length, timeout_msec, route,
-			TB_CFG_PKG_READ);
 	if (res.err)
 		return res;
 
@@ -611,15 +894,41 @@ struct tb_cfg_result tb_cfg_write_raw(struct tb_ctl *ctl, const void *buffer,
 		},
 	};
 	struct cfg_read_pkg reply;
+	int retries = 0;
 
 	memcpy(&request.data, buffer, length * 4);
 
-	res.err = tb_ctl_tx(ctl, &request, 12 + 4 * length, TB_CFG_PKG_WRITE);
-	if (res.err)
-		return res;
+	while (retries < TB_CTL_RETRIES) {
+		struct tb_cfg_request *req;
+
+		req = tb_cfg_request_alloc();
+		if (!req) {
+			res.err = -ENOMEM;
+			return res;
+		}
+
+		request.addr.seq = retries++;
+
+		req->match = tb_cfg_match;
+		req->copy = tb_cfg_copy;
+		req->request = &request;
+		req->request_size = 12 + 4 * length;
+		req->request_type = TB_CFG_PKG_WRITE;
+		req->response = &reply;
+		req->response_size = sizeof(reply);
+		req->response_type = TB_CFG_PKG_WRITE;
+
+		res = tb_cfg_request_sync(ctl, req, timeout_msec);
+
+		tb_cfg_request_put(req);
+
+		if (res.err != -ETIMEDOUT)
+			break;
+
+		/* Wait a bit (arbitrary time) until we send a retry */
+		usleep_range(10, 100);
+	}
 
-	res = tb_ctl_rx(ctl, &reply, sizeof(reply), timeout_msec, route,
-			TB_CFG_PKG_WRITE);
 	if (res.err)
 		return res;
 
@@ -633,11 +942,25 @@ int tb_cfg_read(struct tb_ctl *ctl, void *buffer, u64 route, u32 port,
 {
 	struct tb_cfg_result res = tb_cfg_read_raw(ctl, buffer, route, port,
 			space, offset, length, TB_CFG_DEFAULT_TIMEOUT);
-	if (res.err == 1) {
+	switch (res.err) {
+	case 0:
+		/* Success */
+		break;
+
+	case 1:
+		/* Thunderbolt error, tb_error holds the actual number */
 		tb_cfg_print_error(ctl, &res);
 		return -EIO;
+
+	case -ETIMEDOUT:
+		tb_ctl_warn(ctl, "timeout reading config space %u from %#x\n",
+			    space, offset);
+		break;
+
+	default:
+		WARN(1, "tb_cfg_read: %d\n", res.err);
+		break;
 	}
-	WARN(res.err, "tb_cfg_read: %d\n", res.err);
 	return res.err;
 }
 
@@ -646,11 +969,25 @@ int tb_cfg_write(struct tb_ctl *ctl, const void *buffer, u64 route, u32 port,
 {
 	struct tb_cfg_result res = tb_cfg_write_raw(ctl, buffer, route, port,
 			space, offset, length, TB_CFG_DEFAULT_TIMEOUT);
-	if (res.err == 1) {
+	switch (res.err) {
+	case 0:
+		/* Success */
+		break;
+
+	case 1:
+		/* Thunderbolt error, tb_error holds the actual number */
 		tb_cfg_print_error(ctl, &res);
 		return -EIO;
+
+	case -ETIMEDOUT:
+		tb_ctl_warn(ctl, "timeout writing config space %u to %#x\n",
+			    space, offset);
+		break;
+
+	default:
+		WARN(1, "tb_cfg_write: %d\n", res.err);
+		break;
 	}
-	WARN(res.err, "tb_cfg_write: %d\n", res.err);
 	return res.err;
 }
 
diff --git a/drivers/thunderbolt/ctl.h b/drivers/thunderbolt/ctl.h
index 2b23e030a85b..36fd28b1c1c5 100644
--- a/drivers/thunderbolt/ctl.h
+++ b/drivers/thunderbolt/ctl.h
@@ -7,6 +7,8 @@
 #ifndef _TB_CFG
 #define _TB_CFG
 
+#include <linux/kref.h>
+
 #include "nhi.h"
 #include "tb_msgs.h"
 
@@ -39,6 +41,69 @@ struct tb_cfg_result {
 	enum tb_cfg_error tb_error; /* valid if err == 1 */
 };
 
+struct ctl_pkg {
+	struct tb_ctl *ctl;
+	void *buffer;
+	struct ring_frame frame;
+};
+
+/**
+ * struct tb_cfg_request - Control channel request
+ * @kref: Reference count
+ * @ctl: Pointer to the control channel structure. Only set when the
+ *	 request is queued.
+ * @request_size: Size of the request packet (in bytes)
+ * @request_type: Type of the request packet
+ * @response: Response is stored here
+ * @response_size: Maximum size of one response packet
+ * @response_type: Expected type of the response packet
+ * @npackets: Number of packets expected to be returned with this request
+ * @match: Function used to match the incoming packet
+ * @copy: Function used to copy the incoming packet to @response
+ * @callback: Callback called when the request is finished successfully
+ * @callback_data: Data to be passed to @callback
+ * @flags: Flags for the request
+ * @work: Work item used to complete the request
+ * @result: Result after the request has been completed
+ * @list: Requests are queued using this field
+ *
+ * An arbitrary request over Thunderbolt control channel. For standard
+ * control channel message, one should use tb_cfg_read/write() and
+ * friends if possible.
+ */
+struct tb_cfg_request {
+	struct kref kref;
+	struct tb_ctl *ctl;
+	const void *request;
+	size_t request_size;
+	enum tb_cfg_pkg_type request_type;
+	void *response;
+	size_t response_size;
+	enum tb_cfg_pkg_type response_type;
+	size_t npackets;
+	bool (*match)(const struct tb_cfg_request *req,
+		      const struct ctl_pkg *pkg);
+	bool (*copy)(struct tb_cfg_request *req, const struct ctl_pkg *pkg);
+	void (*callback)(void *callback_data);
+	void *callback_data;
+	unsigned long flags;
+	struct work_struct work;
+	struct tb_cfg_result result;
+	struct list_head list;
+};
+
+#define TB_CFG_REQUEST_ACTIVE		0
+#define TB_CFG_REQUEST_CANCELED		1
+
+struct tb_cfg_request *tb_cfg_request_alloc(void);
+void tb_cfg_request_get(struct tb_cfg_request *req);
+void tb_cfg_request_put(struct tb_cfg_request *req);
+int tb_cfg_request(struct tb_ctl *ctl, struct tb_cfg_request *req,
+		   void (*callback)(void *), void *callback_data);
+void tb_cfg_request_cancel(struct tb_cfg_request *req, int err);
+struct tb_cfg_result tb_cfg_request_sync(struct tb_ctl *ctl,
+			struct tb_cfg_request *req, int timeout_msec);
+
 static inline u64 tb_cfg_get_route(const struct tb_cfg_header *header)
 {
 	return (u64) header->route_hi << 32 | header->route_lo;
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 73db614fa913..0be989069941 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -132,7 +132,7 @@ struct tb_cm_ops {
 /**
  * struct tb - main thunderbolt bus structure
  * @dev: Domain device
- * @lock: Big lock. Must be held when accessing cfg or any struct
+ * @lock: Big lock. Must be held when accessing any struct
  *	  tb_switch / struct tb_port.
  * @nhi: Pointer to the NHI structure
  * @ctl: Control channel for this domain
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]


#1656248 — [PATCH v3 27/27] MAINTAINERS: Add maintainers for Thunderbolt driver

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-02 16:10 +0200
Subject[PATCH v3 27/27] MAINTAINERS: Add maintainers for Thunderbolt driver
Message-ID<tNVr4-2dT-25@gated-at.bofh.it>
In reply to#1656243
We will be helping Andreas to maintain the Thunderbolt driver.

Signed-off-by: Michael Jamet <michael.jamet@intel.com>
Signed-off-by: Yehezkel Bernat <yehezkel.bernat@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 MAINTAINERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 053c3bdd1fe5..f1504e283ae2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11328,6 +11328,9 @@ F:	Documentation/tee.txt
 
 THUNDERBOLT DRIVER
 M:	Andreas Noever <andreas.noever@gmail.com>
+M:	Michael Jamet <michael.jamet@intel.com>
+M:	Mika Westerberg <mika.westerberg@linux.intel.com>
+M:	Yehezkel Bernat <yehezkel.bernat@intel.com>
 S:	Maintained
 F:	drivers/thunderbolt/
 
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]


#1656249 — [PATCH v3 25/27] thunderbolt: Add support for host and device NVM firmware upgrade

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-02 16:10 +0200
Subject[PATCH v3 25/27] thunderbolt: Add support for host and device NVM firmware upgrade
Message-ID<tNVr4-2dT-27@gated-at.bofh.it>
In reply to#1656243
Starting from Intel Falcon Ridge the NVM firmware can be upgraded by
using DMA configuration based mailbox commands. If we detect that the
host or device (device support starts from Intel Alpine Ridge) has the
DMA configuration based mailbox we expose NVM information to the
userspace as two separate Linux NVMem devices: nvm_active and
nvm_non_active. The former is read-only portion of the active NVM which
firmware upgrade tools can be use to find out suitable NVM image if the
device identification strings are not enough.

The latter is write-only portion where the new NVM image is to be
written by the userspace. It is up to the userspace to find out right
NVM image (the kernel does very minimal validation). The ICM firmware
itself authenticates the new NVM firmware and fails the operation if it
is not what is expected.

We also expose two new sysfs files per each switch: nvm_version and
nvm_authenticate which can be used to read the active NVM version and
start the upgrade process.

We also introduce safe mode which is the mode a switch goes when it does
not have properly authenticated firmware. In this mode the switch only
accepts a couple of commands including flashing a new NVM firmware image
and triggering power cycle.

This code is based on the work done by Amir Levy and Michael Jamet.

Signed-off-by: Michael Jamet <michael.jamet@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Yehezkel Bernat <yehezkel.bernat@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 Documentation/ABI/testing/sysfs-bus-thunderbolt |  26 +
 drivers/thunderbolt/Kconfig                     |   1 +
 drivers/thunderbolt/domain.c                    |  18 +
 drivers/thunderbolt/icm.c                       |  33 +-
 drivers/thunderbolt/nhi.h                       |   1 +
 drivers/thunderbolt/switch.c                    | 603 +++++++++++++++++++++++-
 drivers/thunderbolt/tb.h                        |  40 +-
 7 files changed, 699 insertions(+), 23 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-thunderbolt b/Documentation/ABI/testing/sysfs-bus-thunderbolt
index 05b7f9a6431f..2a98149943ea 100644
--- a/Documentation/ABI/testing/sysfs-bus-thunderbolt
+++ b/Documentation/ABI/testing/sysfs-bus-thunderbolt
@@ -82,3 +82,29 @@ Description:	This attribute contains unique_id string of this device.
 		This is either read from hardware registers (UUID on
 		newer hardware) or based on UID from the device DROM.
 		Can be used to uniquely identify particular device.
+
+What:		/sys/bus/thunderbolt/devices/.../nvm_version
+Date:		Sep 2017
+KernelVersion:	4.13
+Contact:	thunderbolt-software@lists.01.org
+Description:	If the device has upgradeable firmware the version
+		number is available here. Format: %x.%x, major.minor.
+		If the device is in safe mode reading the file returns
+		-ENODATA instead as the NVM version is not available.
+
+What:		/sys/bus/thunderbolt/devices/.../nvm_authenticate
+Date:		Sep 2017
+KernelVersion:	4.13
+Contact:	thunderbolt-software@lists.01.org
+Description:	When new NVM image is written to the non-active NVM
+		area (through non_activeX NVMem device), the
+		authentication procedure is started by writing 1 to
+		this file. If everything goes well, the device is
+		restarted with the new NVM firmware. If the image
+		verification fails an error code is returned instead.
+
+		When read holds status of the last authentication
+		operation if an error occurred during the process. This
+		is directly the status value from the DMA configuration
+		based mailbox before the device is power cycled. Writing
+		0 here clears the status.
diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
index a9cc724985ad..f4869c38c7e4 100644
--- a/drivers/thunderbolt/Kconfig
+++ b/drivers/thunderbolt/Kconfig
@@ -6,6 +6,7 @@ menuconfig THUNDERBOLT
 	select CRC32
 	select CRYPTO
 	select CRYPTO_HASH
+	select NVMEM
 	help
 	  Thunderbolt Controller driver. This driver is required if you
 	  want to hotplug Thunderbolt devices on Apple hardware or on PCs
diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index bd491d483f7a..4544103b2cb2 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -425,6 +425,23 @@ int tb_domain_challenge_switch_key(struct tb *tb, struct tb_switch *sw)
 	return ret;
 }
 
+/**
+ * tb_domain_disconnect_pcie_paths() - Disconnect all PCIe paths
+ * @tb: Domain whose PCIe paths to disconnect
+ *
+ * This needs to be called in preparation for NVM upgrade of the host
+ * controller. Makes sure all PCIe paths are disconnected.
+ *
+ * Return %0 on success and negative errno in case of error.
+ */
+int tb_domain_disconnect_pcie_paths(struct tb *tb)
+{
+	if (!tb->cm_ops->disconnect_pcie_paths)
+		return -EPERM;
+
+	return tb->cm_ops->disconnect_pcie_paths(tb);
+}
+
 int tb_domain_init(void)
 {
 	return bus_register(&tb_bus_type);
@@ -434,4 +451,5 @@ void tb_domain_exit(void)
 {
 	bus_unregister(&tb_bus_type);
 	ida_destroy(&tb_domain_ida);
+	tb_switch_exit();
 }
diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
index 0ffa4ec249ac..8ee340290219 100644
--- a/drivers/thunderbolt/icm.c
+++ b/drivers/thunderbolt/icm.c
@@ -54,6 +54,7 @@
  *		   where ICM needs to be started manually
  * @vnd_cap: Vendor defined capability where PCIe2CIO mailbox resides
  *	     (only set when @upstream_port is not %NULL)
+ * @safe_mode: ICM is in safe mode
  * @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
@@ -65,6 +66,7 @@ struct icm {
 	struct delayed_work rescan_work;
 	struct pci_dev *upstream_port;
 	int vnd_cap;
+	bool safe_mode;
 	bool (*is_supported)(struct tb *tb);
 	int (*get_mode)(struct tb *tb);
 	int (*get_route)(struct tb *tb, u8 link, u8 depth, u64 *route);
@@ -852,6 +854,10 @@ static int icm_firmware_init(struct tb *tb)
 		ret = icm->get_mode(tb);
 
 		switch (ret) {
+		case NHI_FW_SAFE_MODE:
+			icm->safe_mode = true;
+			break;
+
 		case NHI_FW_CM_MODE:
 			/* Ask ICM to accept all Thunderbolt devices */
 			nhi_mailbox_cmd(nhi, NHI_MAILBOX_ALLOW_ALL_DEVS, 0);
@@ -879,12 +885,20 @@ static int icm_firmware_init(struct tb *tb)
 
 static int icm_driver_ready(struct tb *tb)
 {
+	struct icm *icm = tb_priv(tb);
 	int ret;
 
 	ret = icm_firmware_init(tb);
 	if (ret)
 		return ret;
 
+	if (icm->safe_mode) {
+		tb_info(tb, "Thunderbolt host controller is in safe mode.\n");
+		tb_info(tb, "You need to update NVM firmware of the controller before it can be used.\n");
+		tb_info(tb, "For latest updates check https://thunderbolttechnology.net/updates.\n");
+		return 0;
+	}
+
 	return __icm_driver_ready(tb, &tb->security_level);
 }
 
@@ -975,12 +989,23 @@ static void icm_complete(struct tb *tb)
 
 static int icm_start(struct tb *tb)
 {
+	struct icm *icm = tb_priv(tb);
 	int ret;
 
-	tb->root_switch = tb_switch_alloc(tb, &tb->dev, 0);
+	if (icm->safe_mode)
+		tb->root_switch = tb_switch_alloc_safe_mode(tb, &tb->dev, 0);
+	else
+		tb->root_switch = tb_switch_alloc(tb, &tb->dev, 0);
 	if (!tb->root_switch)
 		return -ENODEV;
 
+	/*
+	 * 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 = is_apple();
+
 	ret = tb_switch_add(tb->root_switch);
 	if (ret)
 		tb_switch_put(tb->root_switch);
@@ -998,6 +1023,11 @@ static void icm_stop(struct tb *tb)
 	nhi_mailbox_cmd(tb->nhi, NHI_MAILBOX_DRV_UNLOADS, 0);
 }
 
+static int icm_disconnect_pcie_paths(struct tb *tb)
+{
+	return nhi_mailbox_cmd(tb->nhi, NHI_MAILBOX_DISCONNECT_PCIE_PATHS, 0);
+}
+
 /* Falcon Ridge and Alpine Ridge */
 static const struct tb_cm_ops icm_fr_ops = {
 	.driver_ready = icm_driver_ready,
@@ -1009,6 +1039,7 @@ static const struct tb_cm_ops icm_fr_ops = {
 	.approve_switch = icm_fr_approve_switch,
 	.add_switch_key = icm_fr_add_switch_key,
 	.challenge_switch_key = icm_fr_challenge_switch_key,
+	.disconnect_pcie_paths = icm_disconnect_pcie_paths,
 };
 
 struct tb *icm_probe(struct tb_nhi *nhi)
diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
index 953864ae0ab3..5b5bb2c436be 100644
--- a/drivers/thunderbolt/nhi.h
+++ b/drivers/thunderbolt/nhi.h
@@ -155,6 +155,7 @@ enum nhi_fw_mode {
 
 enum nhi_mailbox_cmd {
 	NHI_MAILBOX_SAVE_DEVS = 0x05,
+	NHI_MAILBOX_DISCONNECT_PCIE_PATHS = 0x06,
 	NHI_MAILBOX_DRV_UNLOADS = 0x07,
 	NHI_MAILBOX_ALLOW_ALL_DEVS = 0x23,
 };
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 49e70cfece5d..8bdf61ed138e 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -5,13 +5,395 @@
  */
 
 #include <linux/delay.h>
+#include <linux/idr.h>
+#include <linux/nvmem-provider.h>
+#include <linux/sizes.h>
 #include <linux/slab.h>
+#include <linux/vmalloc.h>
 
 #include "tb.h"
 
 /* Switch authorization from userspace is serialized by this lock */
 static DEFINE_MUTEX(switch_lock);
 
+/* Switch NVM support */
+
+#define NVM_DEVID		0x05
+#define NVM_VERSION		0x08
+#define NVM_CSS			0x10
+#define NVM_FLASH_SIZE		0x45
+
+#define NVM_MIN_SIZE		SZ_32K
+#define NVM_MAX_SIZE		SZ_512K
+
+static DEFINE_IDA(nvm_ida);
+
+struct nvm_auth_status {
+	struct list_head list;
+	uuid_be uuid;
+	u32 status;
+};
+
+/*
+ * Hold NVM authentication failure status per switch This information
+ * needs to stay around even when the switch gets power cycled so we
+ * keep it separately.
+ */
+static LIST_HEAD(nvm_auth_status_cache);
+static DEFINE_MUTEX(nvm_auth_status_lock);
+
+static struct nvm_auth_status *__nvm_get_auth_status(const struct tb_switch *sw)
+{
+	struct nvm_auth_status *st;
+
+	list_for_each_entry(st, &nvm_auth_status_cache, list) {
+		if (!uuid_be_cmp(st->uuid, *sw->uuid))
+			return st;
+	}
+
+	return NULL;
+}
+
+static void nvm_get_auth_status(const struct tb_switch *sw, u32 *status)
+{
+	struct nvm_auth_status *st;
+
+	mutex_lock(&nvm_auth_status_lock);
+	st = __nvm_get_auth_status(sw);
+	mutex_unlock(&nvm_auth_status_lock);
+
+	*status = st ? st->status : 0;
+}
+
+static void nvm_set_auth_status(const struct tb_switch *sw, u32 status)
+{
+	struct nvm_auth_status *st;
+
+	if (WARN_ON(!sw->uuid))
+		return;
+
+	mutex_lock(&nvm_auth_status_lock);
+	st = __nvm_get_auth_status(sw);
+
+	if (!st) {
+		st = kzalloc(sizeof(*st), GFP_KERNEL);
+		if (!st)
+			goto unlock;
+
+		memcpy(&st->uuid, sw->uuid, sizeof(st->uuid));
+		INIT_LIST_HEAD(&st->list);
+		list_add_tail(&st->list, &nvm_auth_status_cache);
+	}
+
+	st->status = status;
+unlock:
+	mutex_unlock(&nvm_auth_status_lock);
+}
+
+static void nvm_clear_auth_status(const struct tb_switch *sw)
+{
+	struct nvm_auth_status *st;
+
+	mutex_lock(&nvm_auth_status_lock);
+	st = __nvm_get_auth_status(sw);
+	if (st) {
+		list_del(&st->list);
+		kfree(st);
+	}
+	mutex_unlock(&nvm_auth_status_lock);
+}
+
+static int nvm_validate_and_write(struct tb_switch *sw)
+{
+	unsigned int image_size, hdr_size;
+	const u8 *buf = sw->nvm->buf;
+	u16 ds_size;
+	int ret;
+
+	if (!buf)
+		return -EINVAL;
+
+	image_size = sw->nvm->buf_data_size;
+	if (image_size < NVM_MIN_SIZE || image_size > NVM_MAX_SIZE)
+		return -EINVAL;
+
+	/*
+	 * FARB pointer must point inside the image and must at least
+	 * contain parts of the digital section we will be reading here.
+	 */
+	hdr_size = (*(u32 *)buf) & 0xffffff;
+	if (hdr_size + NVM_DEVID + 2 >= image_size)
+		return -EINVAL;
+
+	/* Digital section start should be aligned to 4k page */
+	if (!IS_ALIGNED(hdr_size, SZ_4K))
+		return -EINVAL;
+
+	/*
+	 * Read digital section size and check that it also fits inside
+	 * the image.
+	 */
+	ds_size = *(u16 *)(buf + hdr_size);
+	if (ds_size >= image_size)
+		return -EINVAL;
+
+	if (!sw->safe_mode) {
+		u16 device_id;
+
+		/*
+		 * Make sure the device ID in the image matches the one
+		 * we read from the switch config space.
+		 */
+		device_id = *(u16 *)(buf + hdr_size + NVM_DEVID);
+		if (device_id != sw->config.device_id)
+			return -EINVAL;
+
+		if (sw->generation < 3) {
+			/* Write CSS headers first */
+			ret = dma_port_flash_write(sw->dma_port,
+				DMA_PORT_CSS_ADDRESS, buf + NVM_CSS,
+				DMA_PORT_CSS_MAX_SIZE);
+			if (ret)
+				return ret;
+		}
+
+		/* Skip headers in the image */
+		buf += hdr_size;
+		image_size -= hdr_size;
+	}
+
+	return dma_port_flash_write(sw->dma_port, 0, buf, image_size);
+}
+
+static int nvm_authenticate_host(struct tb_switch *sw)
+{
+	int ret;
+
+	/*
+	 * Root switch NVM upgrade requires that we disconnect the
+	 * existing PCIe paths first (in case it is not in safe mode
+	 * already).
+	 */
+	if (!sw->safe_mode) {
+		ret = tb_domain_disconnect_pcie_paths(sw->tb);
+		if (ret)
+			return ret;
+		/*
+		 * The host controller goes away pretty soon after this if
+		 * everything goes well so getting timeout is expected.
+		 */
+		ret = dma_port_flash_update_auth(sw->dma_port);
+		return ret == -ETIMEDOUT ? 0 : ret;
+	}
+
+	/*
+	 * From safe mode we can get out by just power cycling the
+	 * switch.
+	 */
+	dma_port_power_cycle(sw->dma_port);
+	return 0;
+}
+
+static int nvm_authenticate_device(struct tb_switch *sw)
+{
+	int ret, retries = 10;
+
+	ret = dma_port_flash_update_auth(sw->dma_port);
+	if (ret && ret != -ETIMEDOUT)
+		return ret;
+
+	/*
+	 * Poll here for the authentication status. It takes some time
+	 * for the device to respond (we get timeout for a while). Once
+	 * we get response the device needs to be power cycled in order
+	 * to the new NVM to be taken into use.
+	 */
+	do {
+		u32 status;
+
+		ret = dma_port_flash_update_auth_status(sw->dma_port, &status);
+		if (ret < 0 && ret != -ETIMEDOUT)
+			return ret;
+		if (ret > 0) {
+			if (status) {
+				tb_sw_warn(sw, "failed to authenticate NVM\n");
+				nvm_set_auth_status(sw, status);
+			}
+
+			tb_sw_info(sw, "power cycling the switch now\n");
+			dma_port_power_cycle(sw->dma_port);
+			return 0;
+		}
+
+		msleep(500);
+	} while (--retries);
+
+	return -ETIMEDOUT;
+}
+
+static int tb_switch_nvm_read(void *priv, unsigned int offset, void *val,
+			      size_t bytes)
+{
+	struct tb_switch *sw = priv;
+
+	return dma_port_flash_read(sw->dma_port, offset, val, bytes);
+}
+
+static int tb_switch_nvm_write(void *priv, unsigned int offset, void *val,
+			       size_t bytes)
+{
+	struct tb_switch *sw = priv;
+	int ret = 0;
+
+	if (mutex_lock_interruptible(&switch_lock))
+		return -ERESTARTSYS;
+
+	/*
+	 * Since writing the NVM image might require some special steps,
+	 * for example when CSS headers are written, we cache the image
+	 * locally here and handle the special cases when the user asks
+	 * us to authenticate the image.
+	 */
+	if (!sw->nvm->buf) {
+		sw->nvm->buf = vmalloc(NVM_MAX_SIZE);
+		if (!sw->nvm->buf) {
+			ret = -ENOMEM;
+			goto unlock;
+		}
+	}
+
+	sw->nvm->buf_data_size = offset + bytes;
+	memcpy(sw->nvm->buf + offset, val, bytes);
+
+unlock:
+	mutex_unlock(&switch_lock);
+
+	return ret;
+}
+
+static struct nvmem_device *register_nvmem(struct tb_switch *sw, int id,
+					   size_t size, bool active)
+{
+	struct nvmem_config config;
+
+	memset(&config, 0, sizeof(config));
+
+	if (active) {
+		config.name = "nvm_active";
+		config.reg_read = tb_switch_nvm_read;
+	} else {
+		config.name = "nvm_non_active";
+		config.reg_write = tb_switch_nvm_write;
+	}
+
+	config.id = id;
+	config.stride = 4;
+	config.word_size = 4;
+	config.size = size;
+	config.dev = &sw->dev;
+	config.owner = THIS_MODULE;
+	config.root_only = true;
+	config.priv = sw;
+
+	return nvmem_register(&config);
+}
+
+static int tb_switch_nvm_add(struct tb_switch *sw)
+{
+	struct nvmem_device *nvm_dev;
+	struct tb_switch_nvm *nvm;
+	u32 val;
+	int ret;
+
+	if (!sw->dma_port)
+		return 0;
+
+	nvm = kzalloc(sizeof(*nvm), GFP_KERNEL);
+	if (!nvm)
+		return -ENOMEM;
+
+	nvm->id = ida_simple_get(&nvm_ida, 0, 0, GFP_KERNEL);
+
+	/*
+	 * If the switch is in safe-mode the only accessible portion of
+	 * the NVM is the non-active one where userspace is expected to
+	 * write new functional NVM.
+	 */
+	if (!sw->safe_mode) {
+		u32 nvm_size, hdr_size;
+
+		ret = dma_port_flash_read(sw->dma_port, NVM_FLASH_SIZE, &val,
+					  sizeof(val));
+		if (ret)
+			goto err_ida;
+
+		hdr_size = sw->generation < 3 ? SZ_8K : SZ_16K;
+		nvm_size = (SZ_1M << (val & 7)) / 8;
+		nvm_size = (nvm_size - hdr_size) / 2;
+
+		ret = dma_port_flash_read(sw->dma_port, NVM_VERSION, &val,
+					  sizeof(val));
+		if (ret)
+			goto err_ida;
+
+		nvm->major = val >> 16;
+		nvm->minor = val >> 8;
+
+		nvm_dev = register_nvmem(sw, nvm->id, nvm_size, true);
+		if (IS_ERR(nvm_dev)) {
+			ret = PTR_ERR(nvm_dev);
+			goto err_ida;
+		}
+		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;
+	}
+	nvm->non_active = nvm_dev;
+
+	mutex_lock(&switch_lock);
+	sw->nvm = nvm;
+	mutex_unlock(&switch_lock);
+
+	return 0;
+
+err_nvm_active:
+	if (nvm->active)
+		nvmem_unregister(nvm->active);
+err_ida:
+	ida_simple_remove(&nvm_ida, nvm->id);
+	kfree(nvm);
+
+	return ret;
+}
+
+static void tb_switch_nvm_remove(struct tb_switch *sw)
+{
+	struct tb_switch_nvm *nvm;
+
+	mutex_lock(&switch_lock);
+	nvm = sw->nvm;
+	sw->nvm = NULL;
+	mutex_unlock(&switch_lock);
+
+	if (!nvm)
+		return;
+
+	/* Remove authentication status in case the switch is unplugged */
+	if (!nvm->authenticating)
+		nvm_clear_auth_status(sw);
+
+	nvmem_unregister(nvm->non_active);
+	if (nvm->active)
+		nvmem_unregister(nvm->active);
+	ida_simple_remove(&nvm_ida, nvm->id);
+	vfree(nvm->buf);
+	kfree(nvm);
+}
+
 /* port utility functions */
 
 static const char *tb_port_type(struct tb_regs_port_header *port)
@@ -448,6 +830,83 @@ static ssize_t key_store(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RW(key);
 
+static ssize_t nvm_authenticate_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct tb_switch *sw = tb_to_switch(dev);
+	u32 status;
+
+	nvm_get_auth_status(sw, &status);
+	return sprintf(buf, "%#x\n", status);
+}
+
+static ssize_t nvm_authenticate_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct tb_switch *sw = tb_to_switch(dev);
+	bool val;
+	int ret;
+
+	if (mutex_lock_interruptible(&switch_lock))
+		return -ERESTARTSYS;
+
+	/* If NVMem devices are not yet added */
+	if (!sw->nvm) {
+		ret = -EAGAIN;
+		goto exit_unlock;
+	}
+
+	ret = kstrtobool(buf, &val);
+	if (ret)
+		goto exit_unlock;
+
+	/* Always clear the authentication status */
+	nvm_clear_auth_status(sw);
+
+	if (val) {
+		ret = nvm_validate_and_write(sw);
+		if (ret)
+			goto exit_unlock;
+
+		sw->nvm->authenticating = true;
+
+		if (!tb_route(sw))
+			ret = nvm_authenticate_host(sw);
+		else
+			ret = nvm_authenticate_device(sw);
+	}
+
+exit_unlock:
+	mutex_unlock(&switch_lock);
+
+	if (ret)
+		return ret;
+	return count;
+}
+static DEVICE_ATTR_RW(nvm_authenticate);
+
+static ssize_t nvm_version_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct tb_switch *sw = tb_to_switch(dev);
+	int ret;
+
+	if (mutex_lock_interruptible(&switch_lock))
+		return -ERESTARTSYS;
+
+	if (sw->safe_mode)
+		ret = -ENODATA;
+	else if (!sw->nvm)
+		ret = -EAGAIN;
+	else
+		ret = sprintf(buf, "%x.%x\n", sw->nvm->major, sw->nvm->minor);
+
+	mutex_unlock(&switch_lock);
+
+	return ret;
+}
+static DEVICE_ATTR_RO(nvm_version);
+
 static ssize_t vendor_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)
 {
@@ -480,6 +939,8 @@ static struct attribute *switch_attrs[] = {
 	&dev_attr_device.attr,
 	&dev_attr_device_name.attr,
 	&dev_attr_key.attr,
+	&dev_attr_nvm_authenticate.attr,
+	&dev_attr_nvm_version.attr,
 	&dev_attr_vendor.attr,
 	&dev_attr_vendor_name.attr,
 	&dev_attr_unique_id.attr,
@@ -498,9 +959,14 @@ 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) {
+		if (sw->dma_port)
+			return attr->mode;
+		return 0;
 	}
 
-	return attr->mode;
+	return sw->safe_mode ? 0 : attr->mode;
 }
 
 static struct attribute_group switch_group = {
@@ -652,6 +1118,45 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, struct device *parent,
 }
 
 /**
+ * tb_switch_alloc_safe_mode() - allocate a switch that is in safe mode
+ * @tb: Pointer to the owning domain
+ * @parent: Parent device for this switch
+ * @route: Route string for this switch
+ *
+ * This creates a switch in safe mode. This means the switch pretty much
+ * lacks all capabilities except DMA configuration port before it is
+ * flashed with a valid NVM firmware.
+ *
+ * The returned switch must be released by calling tb_switch_put().
+ *
+ * Return: Pointer to the allocated switch or %NULL in case of failure
+ */
+struct tb_switch *
+tb_switch_alloc_safe_mode(struct tb *tb, struct device *parent, u64 route)
+{
+	struct tb_switch *sw;
+
+	sw = kzalloc(sizeof(*sw), GFP_KERNEL);
+	if (!sw)
+		return NULL;
+
+	sw->tb = tb;
+	sw->config.depth = tb_route_length(route);
+	sw->config.route_hi = upper_32_bits(route);
+	sw->config.route_lo = lower_32_bits(route);
+	sw->safe_mode = true;
+
+	device_initialize(&sw->dev);
+	sw->dev.parent = parent;
+	sw->dev.bus = &tb_bus_type;
+	sw->dev.type = &tb_switch_type;
+	sw->dev.groups = switch_groups;
+	dev_set_name(&sw->dev, "%u-%llx", tb->index, tb_route(sw));
+
+	return sw;
+}
+
+/**
  * tb_switch_configure() - Uploads configuration to the switch
  * @sw: Switch to configure
  *
@@ -717,8 +1222,11 @@ static void tb_switch_set_uuid(struct tb_switch *sw)
 	sw->uuid = kmemdup(uuid, sizeof(uuid), GFP_KERNEL);
 }
 
-static void tb_switch_add_dma_port(struct tb_switch *sw)
+static int tb_switch_add_dma_port(struct tb_switch *sw)
 {
+	u32 status;
+	int ret;
+
 	switch (sw->generation) {
 	case 3:
 		break;
@@ -726,14 +1234,49 @@ static void tb_switch_add_dma_port(struct tb_switch *sw)
 	case 2:
 		/* Only root switch can be upgraded */
 		if (tb_route(sw))
-			return;
+			return 0;
 		break;
 
 	default:
-		return;
+		/*
+		 * DMA port is the only thing available when the switch
+		 * is in safe mode.
+		 */
+		if (!sw->safe_mode)
+			return 0;
+		break;
 	}
 
+	if (sw->no_nvm_upgrade)
+		return 0;
+
 	sw->dma_port = dma_port_alloc(sw);
+	if (!sw->dma_port)
+		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
+	 * it functional again.
+	 */
+	ret = dma_port_flash_update_auth_status(sw->dma_port, &status);
+	if (ret <= 0)
+		return ret;
+
+	if (status) {
+		tb_sw_info(sw, "switch flash authentication failed\n");
+		tb_switch_set_uuid(sw);
+		nvm_set_auth_status(sw, status);
+	}
+
+	tb_sw_info(sw, "power cycling the switch now\n");
+	dma_port_power_cycle(sw->dma_port);
+
+	/*
+	 * We return error here which causes the switch adding failure.
+	 * It should appear back after power cycle is complete.
+	 */
+	return -ESHUTDOWN;
 }
 
 /**
@@ -759,29 +1302,41 @@ int tb_switch_add(struct tb_switch *sw)
 	 * to the userspace. NVM can be accessed through DMA
 	 * configuration based mailbox.
 	 */
-	tb_switch_add_dma_port(sw);
-
-	/* read drom */
-	ret = tb_drom_read(sw);
-	if (ret) {
-		tb_sw_warn(sw, "tb_eeprom_read_rom failed\n");
+	ret = tb_switch_add_dma_port(sw);
+	if (ret)
 		return ret;
-	}
-	tb_sw_info(sw, "uid: %#llx\n", sw->uid);
 
-	tb_switch_set_uuid(sw);
+	if (!sw->safe_mode) {
+		/* read drom */
+		ret = tb_drom_read(sw);
+		if (ret) {
+			tb_sw_warn(sw, "tb_eeprom_read_rom failed\n");
+			return ret;
+		}
+		tb_sw_info(sw, "uid: %#llx\n", sw->uid);
 
-	for (i = 0; i <= sw->config.max_port_number; i++) {
-		if (sw->ports[i].disabled) {
-			tb_port_info(&sw->ports[i], "disabled by eeprom\n");
-			continue;
+		tb_switch_set_uuid(sw);
+
+		for (i = 0; i <= sw->config.max_port_number; i++) {
+			if (sw->ports[i].disabled) {
+				tb_port_info(&sw->ports[i], "disabled by eeprom\n");
+				continue;
+			}
+			ret = tb_init_port(&sw->ports[i]);
+			if (ret)
+				return ret;
 		}
-		ret = tb_init_port(&sw->ports[i]);
-		if (ret)
-			return ret;
 	}
 
-	return device_add(&sw->dev);
+	ret = device_add(&sw->dev);
+	if (ret)
+		return ret;
+
+	ret = tb_switch_nvm_add(sw);
+	if (ret)
+		device_del(&sw->dev);
+
+	return ret;
 }
 
 /**
@@ -808,6 +1363,7 @@ void tb_switch_remove(struct tb_switch *sw)
 	if (!sw->is_unplugged)
 		tb_plug_events_active(sw, false);
 
+	tb_switch_nvm_remove(sw);
 	device_unregister(&sw->dev);
 }
 
@@ -976,3 +1532,8 @@ struct tb_switch *tb_switch_find_by_uuid(struct tb *tb, const uuid_be *uuid)
 
 	return NULL;
 }
+
+void tb_switch_exit(void)
+{
+	ida_destroy(&nvm_ida);
+}
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index e02f4e0b2b4d..cbe3e91a3167 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -7,6 +7,7 @@
 #ifndef TB_H_
 #define TB_H_
 
+#include <linux/nvmem-provider.h>
 #include <linux/pci.h>
 #include <linux/uuid.h>
 
@@ -15,6 +16,30 @@
 #include "dma_port.h"
 
 /**
+ * struct tb_switch_nvm - Structure holding switch NVM information
+ * @major: Major version number of the active NVM portion
+ * @minor: Minor version number of the active NVM portion
+ * @id: Identifier used with both NVM portions
+ * @active: Active portion NVMem device
+ * @non_active: Non-active portion NVMem device
+ * @buf: Buffer where the NVM image is stored before it is written to
+ *	 the actual NVM flash device
+ * @buf_data_size: Number of bytes actually consumed by the new NVM
+ *		   image
+ * @authenticating: The switch is authenticating the new NVM
+ */
+struct tb_switch_nvm {
+	u8 major;
+	u8 minor;
+	int id;
+	struct nvmem_device *active;
+	struct nvmem_device *non_active;
+	void *buf;
+	size_t buf_data_size;
+	bool authenticating;
+};
+
+/**
  * enum tb_security_level - Thunderbolt security level
  * @TB_SECURITY_NONE: No security, legacy mode
  * @TB_SECURITY_USER: User approval required at minimum
@@ -39,7 +64,8 @@ enum tb_security_level {
  * @ports: Ports in this switch
  * @dma_port: If the switch has port supporting DMA configuration based
  *	      mailbox this will hold the pointer to that (%NULL
- *	      otherwise).
+ *	      otherwise). If set it also means the switch has
+ *	      upgradeable NVM.
  * @tb: Pointer to the domain the switch belongs to
  * @uid: Unique ID of the switch
  * @uuid: UUID of the switch (or %NULL if not supported)
@@ -51,6 +77,9 @@ enum tb_security_level {
  * @cap_plug_events: Offset to the plug events capability (%0 if not found)
  * @is_unplugged: The switch is going away
  * @drom: DROM of the switch (%NULL if not found)
+ * @nvm: Pointer to the NVM if the switch has one (%NULL otherwise)
+ * @no_nvm_upgrade: Prevent NVM upgrade of this switch
+ * @safe_mode: The switch is in safe-mode
  * @authorized: Whether the switch is authorized by user or policy
  * @work: Work used to automatically authorize a switch
  * @security_level: Switch supported security level
@@ -81,6 +110,9 @@ struct tb_switch {
 	int cap_plug_events;
 	bool is_unplugged;
 	u8 *drom;
+	struct tb_switch_nvm *nvm;
+	bool no_nvm_upgrade;
+	bool safe_mode;
 	unsigned int authorized;
 	struct work_struct work;
 	enum tb_security_level security_level;
@@ -172,6 +204,7 @@ struct tb_path {
  * @approve_switch: Approve switch
  * @add_switch_key: Add key to switch
  * @challenge_switch_key: Challenge switch using key
+ * @disconnect_pcie_paths: Disconnects PCIe paths before NVM update
  */
 struct tb_cm_ops {
 	int (*driver_ready)(struct tb *tb);
@@ -187,6 +220,7 @@ struct tb_cm_ops {
 	int (*add_switch_key)(struct tb *tb, struct tb_switch *sw);
 	int (*challenge_switch_key)(struct tb *tb, struct tb_switch *sw,
 				    const u8 *challenge, u8 *response);
+	int (*disconnect_pcie_paths)(struct tb *tb);
 };
 
 /**
@@ -340,6 +374,7 @@ extern struct device_type tb_switch_type;
 
 int tb_domain_init(void);
 void tb_domain_exit(void);
+void tb_switch_exit(void);
 
 struct tb *tb_domain_alloc(struct tb_nhi *nhi, size_t privsize);
 int tb_domain_add(struct tb *tb);
@@ -351,6 +386,7 @@ void tb_domain_complete(struct tb *tb);
 int tb_domain_approve_switch(struct tb *tb, struct tb_switch *sw);
 int tb_domain_approve_switch_key(struct tb *tb, struct tb_switch *sw);
 int tb_domain_challenge_switch_key(struct tb *tb, struct tb_switch *sw);
+int tb_domain_disconnect_pcie_paths(struct tb *tb);
 
 static inline void tb_domain_put(struct tb *tb)
 {
@@ -359,6 +395,8 @@ static inline void tb_domain_put(struct tb *tb)
 
 struct tb_switch *tb_switch_alloc(struct tb *tb, struct device *parent,
 				  u64 route);
+struct tb_switch *tb_switch_alloc_safe_mode(struct tb *tb,
+			struct device *parent, u64 route);
 int tb_switch_configure(struct tb_switch *sw);
 int tb_switch_add(struct tb_switch *sw);
 void tb_switch_remove(struct tb_switch *sw);
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]


#1656250 — [PATCH v3 01/27] thunderbolt: Use const buffer pointer in write operations

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-02 16:10 +0200
Subject[PATCH v3 01/27] thunderbolt: Use const buffer pointer in write operations
Message-ID<tNVr5-2dT-35@gated-at.bofh.it>
In reply to#1656243
These functions should not (and do not) modify the argument in any way
so make it const.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Yehezkel Bernat <yehezkel.bernat@intel.com>
Reviewed-by: Michael Jamet <michael.jamet@intel.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/thunderbolt/ctl.c | 8 ++++----
 drivers/thunderbolt/ctl.h | 4 ++--
 drivers/thunderbolt/tb.h  | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/thunderbolt/ctl.c b/drivers/thunderbolt/ctl.c
index 1146ff4210a9..1031d97407a8 100644
--- a/drivers/thunderbolt/ctl.c
+++ b/drivers/thunderbolt/ctl.c
@@ -273,7 +273,7 @@ static void tb_cfg_print_error(struct tb_ctl *ctl,
 	}
 }
 
-static void cpu_to_be32_array(__be32 *dst, u32 *src, size_t len)
+static void cpu_to_be32_array(__be32 *dst, const u32 *src, size_t len)
 {
 	int i;
 	for (i = 0; i < len; i++)
@@ -333,7 +333,7 @@ static void tb_ctl_tx_callback(struct tb_ring *ring, struct ring_frame *frame,
  *
  * Return: Returns 0 on success or an error code on failure.
  */
-static int tb_ctl_tx(struct tb_ctl *ctl, void *data, size_t len,
+static int tb_ctl_tx(struct tb_ctl *ctl, const void *data, size_t len,
 		     enum tb_cfg_pkg_type type)
 {
 	int res;
@@ -650,7 +650,7 @@ struct tb_cfg_result tb_cfg_read_raw(struct tb_ctl *ctl, void *buffer,
  *
  * Offset and length are in dwords.
  */
-struct tb_cfg_result tb_cfg_write_raw(struct tb_ctl *ctl, void *buffer,
+struct tb_cfg_result tb_cfg_write_raw(struct tb_ctl *ctl, const void *buffer,
 		u64 route, u32 port, enum tb_cfg_space space,
 		u32 offset, u32 length, int timeout_msec)
 {
@@ -695,7 +695,7 @@ int tb_cfg_read(struct tb_ctl *ctl, void *buffer, u64 route, u32 port,
 	return res.err;
 }
 
-int tb_cfg_write(struct tb_ctl *ctl, void *buffer, u64 route, u32 port,
+int tb_cfg_write(struct tb_ctl *ctl, const void *buffer, u64 route, u32 port,
 		 enum tb_cfg_space space, u32 offset, u32 length)
 {
 	struct tb_cfg_result res = tb_cfg_write_raw(ctl, buffer, route, port,
diff --git a/drivers/thunderbolt/ctl.h b/drivers/thunderbolt/ctl.h
index ba87d6e731dd..83ae54947082 100644
--- a/drivers/thunderbolt/ctl.h
+++ b/drivers/thunderbolt/ctl.h
@@ -61,13 +61,13 @@ struct tb_cfg_result tb_cfg_read_raw(struct tb_ctl *ctl, void *buffer,
 				     u64 route, u32 port,
 				     enum tb_cfg_space space, u32 offset,
 				     u32 length, int timeout_msec);
-struct tb_cfg_result tb_cfg_write_raw(struct tb_ctl *ctl, void *buffer,
+struct tb_cfg_result tb_cfg_write_raw(struct tb_ctl *ctl, const void *buffer,
 				      u64 route, u32 port,
 				      enum tb_cfg_space space, u32 offset,
 				      u32 length, int timeout_msec);
 int tb_cfg_read(struct tb_ctl *ctl, void *buffer, u64 route, u32 port,
 		enum tb_cfg_space space, u32 offset, u32 length);
-int tb_cfg_write(struct tb_ctl *ctl, void *buffer, u64 route, u32 port,
+int tb_cfg_write(struct tb_ctl *ctl, const void *buffer, u64 route, u32 port,
 		 enum tb_cfg_space space, u32 offset, u32 length);
 int tb_cfg_get_upstream_port(struct tb_ctl *ctl, u64 route);
 
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 61d57ba64035..ba2b85750335 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -173,7 +173,7 @@ static inline int tb_port_read(struct tb_port *port, void *buffer,
 			   length);
 }
 
-static inline int tb_port_write(struct tb_port *port, void *buffer,
+static inline int tb_port_write(struct tb_port *port, const void *buffer,
 				enum tb_cfg_space space, u32 offset, u32 length)
 {
 	return tb_cfg_write(port->sw->tb->ctl,
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]


#1656251 — [PATCH v3 22/27] thunderbolt: Add support for DMA configuration based mailbox

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-02 16:10 +0200
Subject[PATCH v3 22/27] thunderbolt: Add support for DMA configuration based mailbox
Message-ID<tNVr4-2dT-15@gated-at.bofh.it>
In reply to#1656243
The DMA (NHI) port of a switch provides access to the NVM of the host
controller (and devices starting from Intel Alpine Ridge). The NVM
contains also more complete DROM for the root switch including vendor
and device identification strings.

This will look for the DMA port capability for each switch and if found
populates sw->dma_port. We then teach tb_drom_read() to read the DROM
information from NVM if available for the root switch.

The DMA port capability also supports upgrading the NVM for both host
controller and devices which will be added in subsequent patches.

This code is based on the work done by Amir Levy and Michael Jamet.

Signed-off-by: Michael Jamet <michael.jamet@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Yehezkel Bernat <yehezkel.bernat@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/thunderbolt/Makefile   |   2 +-
 drivers/thunderbolt/dma_port.c | 524 +++++++++++++++++++++++++++++++++++++++++
 drivers/thunderbolt/dma_port.h |  34 +++
 drivers/thunderbolt/eeprom.c   |  51 +++-
 drivers/thunderbolt/switch.c   |  30 +++
 drivers/thunderbolt/tb.h       |   5 +
 6 files changed, 644 insertions(+), 2 deletions(-)
 create mode 100644 drivers/thunderbolt/dma_port.c
 create mode 100644 drivers/thunderbolt/dma_port.h

diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
index e276a9a62261..9828e862dd35 100644
--- a/drivers/thunderbolt/Makefile
+++ b/drivers/thunderbolt/Makefile
@@ -1,3 +1,3 @@
 obj-${CONFIG_THUNDERBOLT} := thunderbolt.o
 thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o tunnel_pci.o eeprom.o
-thunderbolt-objs += domain.o
+thunderbolt-objs += domain.o dma_port.o
diff --git a/drivers/thunderbolt/dma_port.c b/drivers/thunderbolt/dma_port.c
new file mode 100644
index 000000000000..af6dde347bee
--- /dev/null
+++ b/drivers/thunderbolt/dma_port.c
@@ -0,0 +1,524 @@
+/*
+ * Thunderbolt DMA configuration based mailbox support
+ *
+ * Copyright (C) 2017, Intel Corporation
+ * Authors: Michael Jamet <michael.jamet@intel.com>
+ *          Mika Westerberg <mika.westerberg@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/delay.h>
+#include <linux/slab.h>
+
+#include "dma_port.h"
+#include "tb_regs.h"
+
+#define DMA_PORT_CAP			0x3e
+
+#define MAIL_DATA			1
+#define MAIL_DATA_DWORDS		16
+
+#define MAIL_IN				17
+#define MAIL_IN_CMD_SHIFT		28
+#define MAIL_IN_CMD_MASK		GENMASK(31, 28)
+#define MAIL_IN_CMD_FLASH_WRITE		0x0
+#define MAIL_IN_CMD_FLASH_UPDATE_AUTH	0x1
+#define MAIL_IN_CMD_FLASH_READ		0x2
+#define MAIL_IN_CMD_POWER_CYCLE		0x4
+#define MAIL_IN_DWORDS_SHIFT		24
+#define MAIL_IN_DWORDS_MASK		GENMASK(27, 24)
+#define MAIL_IN_ADDRESS_SHIFT		2
+#define MAIL_IN_ADDRESS_MASK		GENMASK(23, 2)
+#define MAIL_IN_CSS			BIT(1)
+#define MAIL_IN_OP_REQUEST		BIT(0)
+
+#define MAIL_OUT			18
+#define MAIL_OUT_STATUS_RESPONSE	BIT(29)
+#define MAIL_OUT_STATUS_CMD_SHIFT	4
+#define MAIL_OUT_STATUS_CMD_MASK	GENMASK(7, 4)
+#define MAIL_OUT_STATUS_MASK		GENMASK(3, 0)
+#define MAIL_OUT_STATUS_COMPLETED	0
+#define MAIL_OUT_STATUS_ERR_AUTH	1
+#define MAIL_OUT_STATUS_ERR_ACCESS	2
+
+#define DMA_PORT_TIMEOUT		5000 /* ms */
+#define DMA_PORT_RETRIES		3
+
+/**
+ * struct tb_dma_port - DMA control port
+ * @sw: Switch the DMA port belongs to
+ * @port: Switch port number where DMA capability is found
+ * @base: Start offset of the mailbox registers
+ * @buf: Temporary buffer to store a single block
+ */
+struct tb_dma_port {
+	struct tb_switch *sw;
+	u8 port;
+	u32 base;
+	u8 *buf;
+};
+
+/*
+ * When the switch is in safe mode it supports very little functionality
+ * so we don't validate that much here.
+ */
+static bool dma_port_match(const struct tb_cfg_request *req,
+			   const struct ctl_pkg *pkg)
+{
+	u64 route = tb_cfg_get_route(pkg->buffer) & ~BIT_ULL(63);
+
+	if (pkg->frame.eof == TB_CFG_PKG_ERROR)
+		return true;
+	if (pkg->frame.eof != req->response_type)
+		return false;
+	if (route != tb_cfg_get_route(req->request))
+		return false;
+	if (pkg->frame.size != req->response_size)
+		return false;
+
+	return true;
+}
+
+static bool dma_port_copy(struct tb_cfg_request *req, const struct ctl_pkg *pkg)
+{
+	memcpy(req->response, pkg->buffer, req->response_size);
+	return true;
+}
+
+static int dma_port_read(struct tb_ctl *ctl, void *buffer, u64 route,
+			 u32 port, u32 offset, u32 length, int timeout_msec)
+{
+	struct cfg_read_pkg request = {
+		.header = tb_cfg_make_header(route),
+		.addr = {
+			.seq = 1,
+			.port = port,
+			.space = TB_CFG_PORT,
+			.offset = offset,
+			.length = length,
+		},
+	};
+	struct tb_cfg_request *req;
+	struct cfg_write_pkg reply;
+	struct tb_cfg_result res;
+
+	req = tb_cfg_request_alloc();
+	if (!req)
+		return -ENOMEM;
+
+	req->match = dma_port_match;
+	req->copy = dma_port_copy;
+	req->request = &request;
+	req->request_size = sizeof(request);
+	req->request_type = TB_CFG_PKG_READ;
+	req->response = &reply;
+	req->response_size = 12 + 4 * length;
+	req->response_type = TB_CFG_PKG_READ;
+
+	res = tb_cfg_request_sync(ctl, req, timeout_msec);
+
+	tb_cfg_request_put(req);
+
+	if (res.err)
+		return res.err;
+
+	memcpy(buffer, &reply.data, 4 * length);
+	return 0;
+}
+
+static int dma_port_write(struct tb_ctl *ctl, const void *buffer, u64 route,
+			  u32 port, u32 offset, u32 length, int timeout_msec)
+{
+	struct cfg_write_pkg request = {
+		.header = tb_cfg_make_header(route),
+		.addr = {
+			.seq = 1,
+			.port = port,
+			.space = TB_CFG_PORT,
+			.offset = offset,
+			.length = length,
+		},
+	};
+	struct tb_cfg_request *req;
+	struct cfg_read_pkg reply;
+	struct tb_cfg_result res;
+
+	memcpy(&request.data, buffer, length * 4);
+
+	req = tb_cfg_request_alloc();
+	if (!req)
+		return -ENOMEM;
+
+	req->match = dma_port_match;
+	req->copy = dma_port_copy;
+	req->request = &request;
+	req->request_size = 12 + 4 * length;
+	req->request_type = TB_CFG_PKG_WRITE;
+	req->response = &reply;
+	req->response_size = sizeof(reply);
+	req->response_type = TB_CFG_PKG_WRITE;
+
+	res = tb_cfg_request_sync(ctl, req, timeout_msec);
+
+	tb_cfg_request_put(req);
+
+	return res.err;
+}
+
+static int dma_find_port(struct tb_switch *sw)
+{
+	int port, ret;
+	u32 type;
+
+	/*
+	 * The DMA (NHI) port is either 3 or 5 depending on the
+	 * controller. Try both starting from 5 which is more common.
+	 */
+	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;
+
+	return -ENODEV;
+}
+
+/**
+ * dma_port_alloc() - Finds DMA control port from a switch pointed by route
+ * @sw: Switch from where find the DMA port
+ *
+ * Function checks if the switch NHI port supports DMA configuration
+ * based mailbox capability and if it does, allocates and initializes
+ * DMA port structure. Returns %NULL if the capabity was not found.
+ *
+ * The DMA control port is functional also when the switch is in safe
+ * mode.
+ */
+struct tb_dma_port *dma_port_alloc(struct tb_switch *sw)
+{
+	struct tb_dma_port *dma;
+	int port;
+
+	port = dma_find_port(sw);
+	if (port < 0)
+		return NULL;
+
+	dma = kzalloc(sizeof(*dma), GFP_KERNEL);
+	if (!dma)
+		return NULL;
+
+	dma->buf = kmalloc_array(MAIL_DATA_DWORDS, sizeof(u32), GFP_KERNEL);
+	if (!dma->buf) {
+		kfree(dma);
+		return NULL;
+	}
+
+	dma->sw = sw;
+	dma->port = port;
+	dma->base = DMA_PORT_CAP;
+
+	return dma;
+}
+
+/**
+ * dma_port_free() - Release DMA control port structure
+ * @dma: DMA control port
+ */
+void dma_port_free(struct tb_dma_port *dma)
+{
+	if (dma) {
+		kfree(dma->buf);
+		kfree(dma);
+	}
+}
+
+static int dma_port_wait_for_completion(struct tb_dma_port *dma,
+					unsigned int timeout)
+{
+	unsigned long end = jiffies + msecs_to_jiffies(timeout);
+	struct tb_switch *sw = dma->sw;
+
+	do {
+		int ret;
+		u32 in;
+
+		ret = dma_port_read(sw->tb->ctl, &in, tb_route(sw), dma->port,
+				    dma->base + MAIL_IN, 1, 50);
+		if (ret) {
+			if (ret != -ETIMEDOUT)
+				return ret;
+		} else if (!(in & MAIL_IN_OP_REQUEST)) {
+			return 0;
+		}
+
+		usleep_range(50, 100);
+	} while (time_before(jiffies, end));
+
+	return -ETIMEDOUT;
+}
+
+static int status_to_errno(u32 status)
+{
+	switch (status & MAIL_OUT_STATUS_MASK) {
+	case MAIL_OUT_STATUS_COMPLETED:
+		return 0;
+	case MAIL_OUT_STATUS_ERR_AUTH:
+		return -EINVAL;
+	case MAIL_OUT_STATUS_ERR_ACCESS:
+		return -EACCES;
+	}
+
+	return -EIO;
+}
+
+static int dma_port_request(struct tb_dma_port *dma, u32 in,
+			    unsigned int timeout)
+{
+	struct tb_switch *sw = dma->sw;
+	u32 out;
+	int ret;
+
+	ret = dma_port_write(sw->tb->ctl, &in, tb_route(sw), dma->port,
+			     dma->base + MAIL_IN, 1, DMA_PORT_TIMEOUT);
+	if (ret)
+		return ret;
+
+	ret = dma_port_wait_for_completion(dma, timeout);
+	if (ret)
+		return ret;
+
+	ret = dma_port_read(sw->tb->ctl, &out, tb_route(sw), dma->port,
+			    dma->base + MAIL_OUT, 1, DMA_PORT_TIMEOUT);
+	if (ret)
+		return ret;
+
+	return status_to_errno(out);
+}
+
+static int dma_port_flash_read_block(struct tb_dma_port *dma, u32 address,
+				     void *buf, u32 size)
+{
+	struct tb_switch *sw = dma->sw;
+	u32 in, dwaddress, dwords;
+	int ret;
+
+	dwaddress = address / 4;
+	dwords = size / 4;
+
+	in = MAIL_IN_CMD_FLASH_READ << MAIL_IN_CMD_SHIFT;
+	if (dwords < MAIL_DATA_DWORDS)
+		in |= (dwords << MAIL_IN_DWORDS_SHIFT) & MAIL_IN_DWORDS_MASK;
+	in |= (dwaddress << MAIL_IN_ADDRESS_SHIFT) & MAIL_IN_ADDRESS_MASK;
+	in |= MAIL_IN_OP_REQUEST;
+
+	ret = dma_port_request(dma, in, DMA_PORT_TIMEOUT);
+	if (ret)
+		return ret;
+
+	return dma_port_read(sw->tb->ctl, buf, tb_route(sw), dma->port,
+			     dma->base + MAIL_DATA, dwords, DMA_PORT_TIMEOUT);
+}
+
+static int dma_port_flash_write_block(struct tb_dma_port *dma, u32 address,
+				      const void *buf, u32 size)
+{
+	struct tb_switch *sw = dma->sw;
+	u32 in, dwaddress, dwords;
+	int ret;
+
+	dwords = size / 4;
+
+	/* Write the block to MAIL_DATA registers */
+	ret = dma_port_write(sw->tb->ctl, buf, tb_route(sw), dma->port,
+			    dma->base + MAIL_DATA, dwords, DMA_PORT_TIMEOUT);
+
+	in = MAIL_IN_CMD_FLASH_WRITE << MAIL_IN_CMD_SHIFT;
+
+	/* CSS header write is always done to the same magic address */
+	if (address >= DMA_PORT_CSS_ADDRESS) {
+		dwaddress = DMA_PORT_CSS_ADDRESS;
+		in |= MAIL_IN_CSS;
+	} else {
+		dwaddress = address / 4;
+	}
+
+	in |= ((dwords - 1) << MAIL_IN_DWORDS_SHIFT) & MAIL_IN_DWORDS_MASK;
+	in |= (dwaddress << MAIL_IN_ADDRESS_SHIFT) & MAIL_IN_ADDRESS_MASK;
+	in |= MAIL_IN_OP_REQUEST;
+
+	return dma_port_request(dma, in, DMA_PORT_TIMEOUT);
+}
+
+/**
+ * dma_port_flash_read() - Read from active flash region
+ * @dma: DMA control port
+ * @address: Address relative to the start of active region
+ * @buf: Buffer where the data is read
+ * @size: Size of the buffer
+ */
+int dma_port_flash_read(struct tb_dma_port *dma, unsigned int address,
+			void *buf, size_t size)
+{
+	unsigned int retries = DMA_PORT_RETRIES;
+	unsigned int offset;
+
+	offset = address & 3;
+	address = address & ~3;
+
+	do {
+		u32 nbytes = min_t(u32, size, MAIL_DATA_DWORDS * 4);
+		int ret;
+
+		ret = dma_port_flash_read_block(dma, address, dma->buf,
+						ALIGN(nbytes, 4));
+		if (ret) {
+			if (ret == -ETIMEDOUT) {
+				if (retries--)
+					continue;
+				ret = -EIO;
+			}
+			return ret;
+		}
+
+		memcpy(buf, dma->buf + offset, nbytes);
+
+		size -= nbytes;
+		address += nbytes;
+		buf += nbytes;
+	} while (size > 0);
+
+	return 0;
+}
+
+/**
+ * dma_port_flash_write() - Write to non-active flash region
+ * @dma: DMA control port
+ * @address: Address relative to the start of non-active region
+ * @buf: Data to write
+ * @size: Size of the buffer
+ *
+ * Writes block of data to the non-active flash region of the switch. If
+ * the address is given as %DMA_PORT_CSS_ADDRESS the block is written
+ * using CSS command.
+ */
+int dma_port_flash_write(struct tb_dma_port *dma, unsigned int address,
+			 const void *buf, size_t size)
+{
+	unsigned int retries = DMA_PORT_RETRIES;
+	unsigned int offset;
+
+	if (address >= DMA_PORT_CSS_ADDRESS) {
+		offset = 0;
+		if (size > DMA_PORT_CSS_MAX_SIZE)
+			return -E2BIG;
+	} else {
+		offset = address & 3;
+		address = address & ~3;
+	}
+
+	do {
+		u32 nbytes = min_t(u32, size, MAIL_DATA_DWORDS * 4);
+		int ret;
+
+		memcpy(dma->buf + offset, buf, nbytes);
+
+		ret = dma_port_flash_write_block(dma, address, buf, nbytes);
+		if (ret) {
+			if (ret == -ETIMEDOUT) {
+				if (retries--)
+					continue;
+				ret = -EIO;
+			}
+			return ret;
+		}
+
+		size -= nbytes;
+		address += nbytes;
+		buf += nbytes;
+	} while (size > 0);
+
+	return 0;
+}
+
+/**
+ * dma_port_flash_update_auth() - Starts flash authenticate cycle
+ * @dma: DMA control port
+ *
+ * Starts the flash update authentication cycle. If the image in the
+ * non-active area was valid, the switch starts upgrade process where
+ * active and non-active area get swapped in the end. Caller should call
+ * dma_port_flash_update_auth_status() to get status of this command.
+ * This is because if the switch in question is root switch the
+ * thunderbolt host controller gets reset as well.
+ */
+int dma_port_flash_update_auth(struct tb_dma_port *dma)
+{
+	u32 in;
+
+	in = MAIL_IN_CMD_FLASH_UPDATE_AUTH << MAIL_IN_CMD_SHIFT;
+	in |= MAIL_IN_OP_REQUEST;
+
+	return dma_port_request(dma, in, 150);
+}
+
+/**
+ * dma_port_flash_update_auth_status() - Reads status of update auth command
+ * @dma: DMA control port
+ * @status: Status code of the operation
+ *
+ * The function checks if there is status available from the last update
+ * auth command. Returns %0 if there is no status and no further
+ * action is required. If there is status, %1 is returned instead and
+ * @status holds the failure code.
+ *
+ * Negative return means there was an error reading status from the
+ * switch.
+ */
+int dma_port_flash_update_auth_status(struct tb_dma_port *dma, u32 *status)
+{
+	struct tb_switch *sw = dma->sw;
+	u32 out, cmd;
+	int ret;
+
+	ret = dma_port_read(sw->tb->ctl, &out, tb_route(sw), dma->port,
+			    dma->base + MAIL_OUT, 1, DMA_PORT_TIMEOUT);
+	if (ret)
+		return ret;
+
+	/* Check if the status relates to flash update auth */
+	cmd = (out & MAIL_OUT_STATUS_CMD_MASK) >> MAIL_OUT_STATUS_CMD_SHIFT;
+	if (cmd == MAIL_IN_CMD_FLASH_UPDATE_AUTH) {
+		if (status)
+			*status = out & MAIL_OUT_STATUS_MASK;
+
+		/* Reset is needed in any case */
+		return 1;
+	}
+
+	return 0;
+}
+
+/**
+ * dma_port_power_cycle() - Power cycles the switch
+ * @dma: DMA control port
+ *
+ * Triggers power cycle to the switch.
+ */
+int dma_port_power_cycle(struct tb_dma_port *dma)
+{
+	u32 in;
+
+	in = MAIL_IN_CMD_POWER_CYCLE << MAIL_IN_CMD_SHIFT;
+	in |= MAIL_IN_OP_REQUEST;
+
+	return dma_port_request(dma, in, 150);
+}
diff --git a/drivers/thunderbolt/dma_port.h b/drivers/thunderbolt/dma_port.h
new file mode 100644
index 000000000000..c4a69e0fbff7
--- /dev/null
+++ b/drivers/thunderbolt/dma_port.h
@@ -0,0 +1,34 @@
+/*
+ * Thunderbolt DMA configuration based mailbox support
+ *
+ * Copyright (C) 2017, Intel Corporation
+ * Authors: Michael Jamet <michael.jamet@intel.com>
+ *          Mika Westerberg <mika.westerberg@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef DMA_PORT_H_
+#define DMA_PORT_H_
+
+#include "tb.h"
+
+struct tb_switch;
+struct tb_dma_port;
+
+#define DMA_PORT_CSS_ADDRESS		0x3fffff
+#define DMA_PORT_CSS_MAX_SIZE		SZ_128
+
+struct tb_dma_port *dma_port_alloc(struct tb_switch *sw);
+void dma_port_free(struct tb_dma_port *dma);
+int dma_port_flash_read(struct tb_dma_port *dma, unsigned int address,
+			void *buf, size_t size);
+int dma_port_flash_update_auth(struct tb_dma_port *dma);
+int dma_port_flash_update_auth_status(struct tb_dma_port *dma, u32 *status);
+int dma_port_flash_write(struct tb_dma_port *dma, unsigned int address,
+			 const void *buf, size_t size);
+int dma_port_power_cycle(struct tb_dma_port *dma);
+
+#endif
diff --git a/drivers/thunderbolt/eeprom.c b/drivers/thunderbolt/eeprom.c
index d40a5f07fc4c..996c6e29c8ad 100644
--- a/drivers/thunderbolt/eeprom.c
+++ b/drivers/thunderbolt/eeprom.c
@@ -429,6 +429,50 @@ static int tb_drom_copy_efi(struct tb_switch *sw, u16 *size)
 	return -EINVAL;
 }
 
+static int tb_drom_copy_nvm(struct tb_switch *sw, u16 *size)
+{
+	u32 drom_offset;
+	int ret;
+
+	if (!sw->dma_port)
+		return -ENODEV;
+
+	ret = tb_sw_read(sw, &drom_offset, TB_CFG_SWITCH,
+			 sw->cap_plug_events + 12, 1);
+	if (ret)
+		return ret;
+
+	if (!drom_offset)
+		return -ENODEV;
+
+	ret = dma_port_flash_read(sw->dma_port, drom_offset + 14, size,
+				  sizeof(*size));
+	if (ret)
+		return ret;
+
+	/* Size includes CRC8 + UID + CRC32 */
+	*size += 1 + 8 + 4;
+	sw->drom = kzalloc(*size, GFP_KERNEL);
+	if (!sw->drom)
+		return -ENOMEM;
+
+	ret = dma_port_flash_read(sw->dma_port, drom_offset, sw->drom, *size);
+	if (ret)
+		goto err_free;
+
+	/*
+	 * Read UID from the minimal DROM because the one in NVM is just
+	 * a placeholder.
+	 */
+	tb_drom_read_uid_only(sw, &sw->uid);
+	return 0;
+
+err_free:
+	kfree(sw->drom);
+	sw->drom = NULL;
+	return ret;
+}
+
 /**
  * tb_drom_read - copy drom to sw->drom and parse it
  */
@@ -450,6 +494,10 @@ int tb_drom_read(struct tb_switch *sw)
 		if (tb_drom_copy_efi(sw, &size) == 0)
 			goto parse;
 
+		/* Non-Apple hardware has the DROM as part of NVM */
+		if (tb_drom_copy_nvm(sw, &size) == 0)
+			goto parse;
+
 		/*
 		 * The root switch contains only a dummy drom (header only,
 		 * no entries). Hardcode the configuration here.
@@ -510,7 +558,8 @@ int tb_drom_read(struct tb_switch *sw)
 			header->uid_crc8, crc);
 		goto err;
 	}
-	sw->uid = header->uid;
+	if (!sw->uid)
+		sw->uid = header->uid;
 	sw->vendor = header->vendor_id;
 	sw->device = header->model_id;
 
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 56b99ad2099d..2c99fa5884a2 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -377,6 +377,8 @@ static void tb_switch_release(struct device *dev)
 {
 	struct tb_switch *sw = tb_to_switch(dev);
 
+	dma_port_free(sw->dma_port);
+
 	kfree(sw->uuid);
 	kfree(sw->device_name);
 	kfree(sw->vendor_name);
@@ -570,6 +572,25 @@ static void tb_switch_set_uuid(struct tb_switch *sw)
 	sw->uuid = kmemdup(uuid, sizeof(uuid), GFP_KERNEL);
 }
 
+static void tb_switch_add_dma_port(struct tb_switch *sw)
+{
+	switch (sw->generation) {
+	case 3:
+		break;
+
+	case 2:
+		/* Only root switch can be upgraded */
+		if (tb_route(sw))
+			return;
+		break;
+
+	default:
+		return;
+	}
+
+	sw->dma_port = dma_port_alloc(sw);
+}
+
 /**
  * tb_switch_add() - Add a switch to the domain
  * @sw: Switch to add
@@ -586,6 +607,15 @@ int tb_switch_add(struct tb_switch *sw)
 {
 	int i, ret;
 
+	/*
+	 * Initialize DMA control port now before we read DROM. Recent
+	 * host controllers have more complete DROM on NVM that includes
+	 * vendor and model identification strings which we then expose
+	 * to the userspace. NVM can be accessed through DMA
+	 * configuration based mailbox.
+	 */
+	tb_switch_add_dma_port(sw);
+
 	/* read drom */
 	ret = tb_drom_read(sw);
 	if (ret) {
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index b3cda7605619..1facf9e4b827 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -12,12 +12,16 @@
 
 #include "tb_regs.h"
 #include "ctl.h"
+#include "dma_port.h"
 
 /**
  * struct tb_switch - a thunderbolt switch
  * @dev: Device for the switch
  * @config: Switch configuration
  * @ports: Ports in this switch
+ * @dma_port: If the switch has port supporting DMA configuration based
+ *	      mailbox this will hold the pointer to that (%NULL
+ *	      otherwise).
  * @tb: Pointer to the domain the switch belongs to
  * @uid: Unique ID of the switch
  * @uuid: UUID of the switch (or %NULL if not supported)
@@ -34,6 +38,7 @@ struct tb_switch {
 	struct device dev;
 	struct tb_regs_switch_header config;
 	struct tb_port *ports;
+	struct tb_dma_port *dma_port;
 	struct tb *tb;
 	u64 uid;
 	uuid_be *uuid;
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]


#1656252 — [PATCH v3 06/27] thunderbolt: Rework capability handling

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-02 16:10 +0200
Subject[PATCH v3 06/27] thunderbolt: Rework capability handling
Message-ID<tNVr4-2dT-17@gated-at.bofh.it>
In reply to#1656243
Organization of the capabilities in switches and ports is not so random
after all. Rework the capability handling functionality so that it
follows how capabilities are organized and provide two new functions
(tb_switch_find_vsec_cap() and tb_port_find_cap()) which can be used to
extract capabilities for ports and switches. Then convert the current
users over these.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Yehezkel Bernat <yehezkel.bernat@intel.com>
Reviewed-by: Michael Jamet <michael.jamet@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/thunderbolt/cap.c        | 169 +++++++++++++++++++++------------------
 drivers/thunderbolt/switch.c     |   6 +-
 drivers/thunderbolt/tb.c         |   8 +-
 drivers/thunderbolt/tb.h         |   3 +-
 drivers/thunderbolt/tb_regs.h    |  50 +++++++++---
 drivers/thunderbolt/tunnel_pci.c |   8 +-
 6 files changed, 142 insertions(+), 102 deletions(-)

diff --git a/drivers/thunderbolt/cap.c b/drivers/thunderbolt/cap.c
index a7b47e7cddbd..7260971acc4f 100644
--- a/drivers/thunderbolt/cap.c
+++ b/drivers/thunderbolt/cap.c
@@ -9,6 +9,8 @@
 
 #include "tb.h"
 
+#define CAP_OFFSET_MAX		0xff
+#define VSEC_CAP_OFFSET_MAX	0xffff
 
 struct tb_cap_any {
 	union {
@@ -18,99 +20,110 @@ struct tb_cap_any {
 	};
 } __packed;
 
-static bool tb_cap_is_basic(struct tb_cap_any *cap)
-{
-	/* basic.cap is u8. This checks only the lower 8 bit of cap. */
-	return cap->basic.cap != 5;
-}
-
-static bool tb_cap_is_long(struct tb_cap_any *cap)
+/**
+ * tb_port_find_cap() - Find port capability
+ * @port: Port to find the capability for
+ * @cap: Capability to look
+ *
+ * Returns offset to start of capability or %-ENOENT if no such
+ * capability was found. Negative errno is returned if there was an
+ * error.
+ */
+int tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap)
 {
-	return !tb_cap_is_basic(cap)
-	       && cap->extended_short.next == 0
-	       && cap->extended_short.length == 0;
-}
+	u32 offset;
 
-static enum tb_cap tb_cap(struct tb_cap_any *cap)
-{
-	if (tb_cap_is_basic(cap))
-		return cap->basic.cap;
+	/*
+	 * DP out adapters claim to implement TMU capability but in
+	 * reality they do not so we hard code the adapter specific
+	 * capability offset here.
+	 */
+	if (port->config.type == TB_TYPE_DP_HDMI_OUT)
+		offset = 0x39;
 	else
-		/* extended_short/long have cap at the same offset. */
-		return cap->extended_short.cap;
+		offset = 0x1;
+
+	do {
+		struct tb_cap_any header;
+		int ret;
+
+		ret = tb_port_read(port, &header, TB_CFG_PORT, offset, 1);
+		if (ret)
+			return ret;
+
+		if (header.basic.cap == cap)
+			return offset;
+
+		offset = header.basic.next;
+	} while (offset);
+
+	return -ENOENT;
 }
 
-static u32 tb_cap_next(struct tb_cap_any *cap, u32 offset)
+static int tb_switch_find_cap(struct tb_switch *sw, enum tb_switch_cap cap)
 {
-	int next;
-	if (offset == 1) {
-		/*
-		 * The first pointer is part of the switch header and always
-		 * a simple pointer.
-		 */
-		next = cap->basic.next;
-	} else {
-		/*
-		 * Somehow Intel decided to use 3 different types of capability
-		 * headers. It is not like anyone could have predicted that
-		 * single byte offsets are not enough...
-		 */
-		if (tb_cap_is_basic(cap))
-			next = cap->basic.next;
-		else if (!tb_cap_is_long(cap))
-			next = cap->extended_short.next;
-		else
-			next = cap->extended_long.next;
+	int offset = sw->config.first_cap_offset;
+
+	while (offset > 0 && offset < CAP_OFFSET_MAX) {
+		struct tb_cap_any header;
+		int ret;
+
+		ret = tb_sw_read(sw, &header, TB_CFG_SWITCH, offset, 1);
+		if (ret)
+			return ret;
+
+		if (header.basic.cap == cap)
+			return offset;
+
+		offset = header.basic.next;
 	}
-	/*
-	 * "Hey, we could terminate some capability lists with a null offset
-	 *  and others with a pointer to the last element." - "Great idea!"
-	 */
-	if (next == offset)
-		return 0;
-	return next;
+
+	return -ENOENT;
 }
 
 /**
- * tb_find_cap() - find a capability
+ * tb_switch_find_vsec_cap() - Find switch vendor specific capability
+ * @sw: Switch to find the capability for
+ * @vsec: Vendor specific capability to look
  *
- * Return: Returns a positive offset if the capability was found and 0 if not.
- * Returns an error code on failure.
+ * Functions enumerates vendor specific (VSEC) capabilities of a switch
+ * and returns offset when capability matching @vsec is found. If no
+ * such capability is found returns %-ENOENT. In case of error returns
+ * negative errno.
  */
-int tb_find_cap(struct tb_port *port, enum tb_cfg_space space, enum tb_cap cap)
+int tb_switch_find_vsec_cap(struct tb_switch *sw, enum tb_switch_vsec_cap vsec)
 {
-	u32 offset = 1;
 	struct tb_cap_any header;
-	int res;
-	int retries = 10;
-	while (retries--) {
-		res = tb_port_read(port, &header, space, offset, 1);
-		if (res) {
-			/* Intel needs some help with linked lists. */
-			if (space == TB_CFG_PORT && offset == 0xa
-			    && port->config.type == TB_TYPE_DP_HDMI_OUT) {
-				offset = 0x39;
-				continue;
-			}
-			return res;
-		}
-		if (offset != 1) {
-			if (tb_cap(&header) == cap)
+	int offset;
+
+	offset = tb_switch_find_cap(sw, TB_SWITCH_CAP_VSEC);
+	if (offset < 0)
+		return offset;
+
+	while (offset > 0 && offset < VSEC_CAP_OFFSET_MAX) {
+		int ret;
+
+		ret = tb_sw_read(sw, &header, TB_CFG_SWITCH, offset, 2);
+		if (ret)
+			return ret;
+
+		/*
+		 * Extended vendor specific capabilities come in two
+		 * flavors: short and long. The latter is used when
+		 * offset is over 0xff.
+		 */
+		if (offset >= CAP_OFFSET_MAX) {
+			if (header.extended_long.vsec_id == vsec)
 				return offset;
-			if (tb_cap_is_long(&header)) {
-				/* tb_cap_extended_long is 2 dwords */
-				res = tb_port_read(port, &header, space,
-						   offset, 2);
-				if (res)
-					return res;
-			}
+			offset = header.extended_long.next;
+		} else {
+			if (header.extended_short.vsec_id == vsec)
+				return offset;
+			if (!header.extended_short.length)
+				return -ENOENT;
+			offset = header.extended_short.next;
 		}
-		offset = tb_cap_next(&header, offset);
-		if (!offset)
-			return 0;
 	}
-	tb_port_WARN(port,
-		     "run out of retries while looking for cap %#x in config space %d, last offset: %#x\n",
-		     cap, space, offset);
-	return -EIO;
+
+	return -ENOENT;
 }
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 81f5164a6364..c02079424d0e 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -192,7 +192,7 @@ static int tb_init_port(struct tb_port *port)
 
 	/* Port 0 is the switch itself and has no PHY. */
 	if (port->config.type == TB_TYPE_PORT && port->port != 0) {
-		cap = tb_find_cap(port, TB_CFG_PORT, TB_CAP_PHY);
+		cap = tb_port_find_cap(port, TB_PORT_CAP_PHY);
 
 		if (cap > 0)
 			port->cap_phy = cap;
@@ -394,9 +394,9 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, u64 route)
 		sw->ports[i].port = i;
 	}
 
-	cap = tb_find_cap(&sw->ports[0], TB_CFG_SWITCH, TB_CAP_PLUG_EVENTS);
+	cap = tb_switch_find_vsec_cap(sw, TB_VSEC_CAP_PLUG_EVENTS);
 	if (cap < 0) {
-		tb_sw_warn(sw, "cannot find TB_CAP_PLUG_EVENTS aborting\n");
+		tb_sw_warn(sw, "cannot find TB_VSEC_CAP_PLUG_EVENTS aborting\n");
 		goto err;
 	}
 	sw->cap_plug_events = cap;
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 24b6d30c3c86..6b44076e1380 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -121,8 +121,8 @@ static struct tb_port *tb_find_unused_down_port(struct tb_switch *sw)
 			continue;
 		if (sw->ports[i].config.type != TB_TYPE_PCIE_DOWN)
 			continue;
-		cap = tb_find_cap(&sw->ports[i], TB_CFG_PORT, TB_CAP_PCIE);
-		if (cap <= 0)
+		cap = tb_port_find_cap(&sw->ports[i], TB_PORT_CAP_ADAP);
+		if (cap < 0)
 			continue;
 		res = tb_port_read(&sw->ports[i], &data, TB_CFG_PORT, cap, 1);
 		if (res < 0)
@@ -165,8 +165,8 @@ static void tb_activate_pcie_devices(struct tb *tb)
 		}
 
 		/* check whether port is already activated */
-		cap = tb_find_cap(up_port, TB_CFG_PORT, TB_CAP_PCIE);
-		if (cap <= 0)
+		cap = tb_port_find_cap(up_port, TB_PORT_CAP_ADAP);
+		if (cap < 0)
 			continue;
 		if (tb_port_read(up_port, &data, TB_CFG_PORT, cap, 1))
 			continue;
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index ba2b85750335..9b60e71562dc 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -233,7 +233,8 @@ int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged);
 int tb_port_add_nfc_credits(struct tb_port *port, int credits);
 int tb_port_clear_counter(struct tb_port *port, int counter);
 
-int tb_find_cap(struct tb_port *port, enum tb_cfg_space space, enum tb_cap cap);
+int tb_switch_find_vsec_cap(struct tb_switch *sw, enum tb_switch_vsec_cap vsec);
+int tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap);
 
 struct tb_path *tb_path_alloc(struct tb *tb, int num_hops);
 void tb_path_free(struct tb_path *path);
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index 1e2a4a8046be..982c1ddcaa2a 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -23,15 +23,22 @@
  */
 #define TB_MAX_CONFIG_RW_LENGTH 60
 
-enum tb_cap {
-	TB_CAP_PHY		= 0x0001,
-	TB_CAP_TIME1		= 0x0003,
-	TB_CAP_PCIE		= 0x0004,
-	TB_CAP_I2C		= 0x0005,
-	TB_CAP_PLUG_EVENTS	= 0x0105, /* also EEPROM */
-	TB_CAP_TIME2		= 0x0305,
-	TB_CAP_IECS		= 0x0405,
-	TB_CAP_LINK_CONTROLLER	= 0x0605, /* also IECS */
+enum tb_switch_cap {
+	TB_SWITCH_CAP_VSEC		= 0x05,
+};
+
+enum tb_switch_vsec_cap {
+	TB_VSEC_CAP_PLUG_EVENTS		= 0x01, /* also EEPROM */
+	TB_VSEC_CAP_TIME2		= 0x03,
+	TB_VSEC_CAP_IECS		= 0x04,
+	TB_VSEC_CAP_LINK_CONTROLLER	= 0x06, /* also IECS */
+};
+
+enum tb_port_cap {
+	TB_PORT_CAP_PHY			= 0x01,
+	TB_PORT_CAP_TIME1		= 0x03,
+	TB_PORT_CAP_ADAP		= 0x04,
+	TB_PORT_CAP_VSEC		= 0x05,
 };
 
 enum tb_port_state {
@@ -49,15 +56,34 @@ struct tb_cap_basic {
 	u8 cap; /* if cap == 0x05 then we have a extended capability */
 } __packed;
 
+/**
+ * struct tb_cap_extended_short - Switch extended short capability
+ * @next: Pointer to the next capability. If @next and @length are zero
+ *	  then we have a long cap.
+ * @cap: Base capability ID (see &enum tb_switch_cap)
+ * @vsec_id: Vendor specific capability ID (see &enum switch_vsec_cap)
+ * @length: Length of this capability
+ */
 struct tb_cap_extended_short {
-	u8 next; /* if next and length are zero then we have a long cap */
-	enum tb_cap cap:16;
+	u8 next;
+	u8 cap;
+	u8 vsec_id;
 	u8 length;
 } __packed;
 
+/**
+ * struct tb_cap_extended_long - Switch extended long capability
+ * @zero1: This field should be zero
+ * @cap: Base capability ID (see &enum tb_switch_cap)
+ * @vsec_id: Vendor specific capability ID (see &enum switch_vsec_cap)
+ * @zero2: This field should be zero
+ * @next: Pointer to the next capability
+ * @length: Length of this capability
+ */
 struct tb_cap_extended_long {
 	u8 zero1;
-	enum tb_cap cap:16;
+	u8 cap;
+	u8 vsec_id;
 	u8 zero2;
 	u16 next;
 	u16 length;
diff --git a/drivers/thunderbolt/tunnel_pci.c b/drivers/thunderbolt/tunnel_pci.c
index baf1cd370446..f4ce9845e42a 100644
--- a/drivers/thunderbolt/tunnel_pci.c
+++ b/drivers/thunderbolt/tunnel_pci.c
@@ -147,10 +147,10 @@ bool tb_pci_is_invalid(struct tb_pci_tunnel *tunnel)
 static int tb_pci_port_active(struct tb_port *port, bool active)
 {
 	u32 word = active ? 0x80000000 : 0x0;
-	int cap = tb_find_cap(port, TB_CFG_PORT, TB_CAP_PCIE);
-	if (cap <= 0) {
-		tb_port_warn(port, "TB_CAP_PCIE not found: %d\n", cap);
-		return cap ? cap : -ENXIO;
+	int cap = tb_port_find_cap(port, TB_PORT_CAP_ADAP);
+	if (cap < 0) {
+		tb_port_warn(port, "TB_PORT_CAP_ADAP not found: %d\n", cap);
+		return cap;
 	}
 	return tb_port_write(port, &word, TB_CFG_PORT, cap, 1);
 }
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]


#1657077 — Re: [PATCH v3 06/27] thunderbolt: Rework capability handling

FromLukas Wunner <lukas@wunner.de>
Date2017-06-04 16:00 +0200
SubjectRe: [PATCH v3 06/27] thunderbolt: Rework capability handling
Message-ID<tOEeu-5Rg-13@gated-at.bofh.it>
In reply to#1656252
On Fri, Jun 02, 2017 at 05:05:03PM +0300, Mika Westerberg wrote:
> Organization of the capabilities in switches and ports is not so random
> after all. Rework the capability handling functionality so that it
> follows how capabilities are organized and provide two new functions
> (tb_switch_find_vsec_cap() and tb_port_find_cap()) which can be used to
                  ^^^^^^^^

I assume VSEC is the same acronym as in the PCIe spec, so this naming
scheme results in "vendor specific extended capability capability",
which is maybe a bit odd.

Thanks,

Lukas

[toc] | [prev] | [next] | [standalone]


#1657091 — Re: [PATCH v3 06/27] thunderbolt: Rework capability handling

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-04 17:10 +0200
SubjectRe: [PATCH v3 06/27] thunderbolt: Rework capability handling
Message-ID<tOFkd-6N9-11@gated-at.bofh.it>
In reply to#1657077
On Sun, Jun 04, 2017 at 03:52:29PM +0200, Lukas Wunner wrote:
> On Fri, Jun 02, 2017 at 05:05:03PM +0300, Mika Westerberg wrote:
> > Organization of the capabilities in switches and ports is not so random
> > after all. Rework the capability handling functionality so that it
> > follows how capabilities are organized and provide two new functions
> > (tb_switch_find_vsec_cap() and tb_port_find_cap()) which can be used to
>                   ^^^^^^^^
> 
> I assume VSEC is the same acronym as in the PCIe spec, so this naming
> scheme results in "vendor specific extended capability capability",
> which is maybe a bit odd.

AFAIK it comes from Vendor SpEcifiC but I'm not 100% sure ;-) The Alpine
Ridge datasheet calls it also VSEC capability which is why we chose the
naming accordingly.

[toc] | [prev] | [next] | [standalone]


#1657094 — Re: [PATCH v3 06/27] thunderbolt: Rework capability handling

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-04 17:10 +0200
SubjectRe: [PATCH v3 06/27] thunderbolt: Rework capability handling
Message-ID<tOFke-6N9-27@gated-at.bofh.it>
In reply to#1657091
On Sun, Jun 04, 2017 at 06:00:02PM +0300, Mika Westerberg wrote:
> On Sun, Jun 04, 2017 at 03:52:29PM +0200, Lukas Wunner wrote:
> > On Fri, Jun 02, 2017 at 05:05:03PM +0300, Mika Westerberg wrote:
> > > Organization of the capabilities in switches and ports is not so random
> > > after all. Rework the capability handling functionality so that it
> > > follows how capabilities are organized and provide two new functions
> > > (tb_switch_find_vsec_cap() and tb_port_find_cap()) which can be used to
> >                   ^^^^^^^^
> > 
> > I assume VSEC is the same acronym as in the PCIe spec, so this naming
> > scheme results in "vendor specific extended capability capability",
> > which is maybe a bit odd.
> 
> AFAIK it comes from Vendor SpEcifiC but I'm not 100% sure ;-) The Alpine
> Ridge datasheet calls it also VSEC capability which is why we chose the
> naming accordingly.

That said, it could also come from Vendor SpEcific Capability. I can
change it to tb_switch_find_vsec() if that works better.

[toc] | [prev] | [next] | [standalone]


#1657129 — Re: [PATCH v3 06/27] thunderbolt: Rework capability handling

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2017-06-04 20:20 +0200
SubjectRe: [PATCH v3 06/27] thunderbolt: Rework capability handling
Message-ID<tOIi5-iC-1@gated-at.bofh.it>
In reply to#1657094
On Sun, Jun 4, 2017 at 6:06 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Sun, Jun 04, 2017 at 06:00:02PM +0300, Mika Westerberg wrote:
>> On Sun, Jun 04, 2017 at 03:52:29PM +0200, Lukas Wunner wrote:
>> > On Fri, Jun 02, 2017 at 05:05:03PM +0300, Mika Westerberg wrote:
>> > > Organization of the capabilities in switches and ports is not so random
>> > > after all. Rework the capability handling functionality so that it
>> > > follows how capabilities are organized and provide two new functions
>> > > (tb_switch_find_vsec_cap() and tb_port_find_cap()) which can be used to
>> >                   ^^^^^^^^
>> >
>> > I assume VSEC is the same acronym as in the PCIe spec, so this naming
>> > scheme results in "vendor specific extended capability capability",
>> > which is maybe a bit odd.
>>
>> AFAIK it comes from Vendor SpEcifiC but I'm not 100% sure ;-) The Alpine
>> Ridge datasheet calls it also VSEC capability which is why we chose the
>> naming accordingly.
>
> That said, it could also come from Vendor SpEcific Capability. I can
> change it to tb_switch_find_vsec() if that works better.

I would folllow PCI existing namings, i.e.
_vse_cap()

_vse_capability() also possible, but too long to me.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]


#1657307 — Re: [PATCH v3 06/27] thunderbolt: Rework capability handling

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-05 08:50 +0200
SubjectRe: [PATCH v3 06/27] thunderbolt: Rework capability handling
Message-ID<tOTZT-7QM-1@gated-at.bofh.it>
In reply to#1657129
On Sun, Jun 04, 2017 at 09:15:03PM +0300, Andy Shevchenko wrote:
> On Sun, Jun 4, 2017 at 6:06 PM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > On Sun, Jun 04, 2017 at 06:00:02PM +0300, Mika Westerberg wrote:
> >> On Sun, Jun 04, 2017 at 03:52:29PM +0200, Lukas Wunner wrote:
> >> > On Fri, Jun 02, 2017 at 05:05:03PM +0300, Mika Westerberg wrote:
> >> > > Organization of the capabilities in switches and ports is not so random
> >> > > after all. Rework the capability handling functionality so that it
> >> > > follows how capabilities are organized and provide two new functions
> >> > > (tb_switch_find_vsec_cap() and tb_port_find_cap()) which can be used to
> >> >                   ^^^^^^^^
> >> >
> >> > I assume VSEC is the same acronym as in the PCIe spec, so this naming
> >> > scheme results in "vendor specific extended capability capability",
> >> > which is maybe a bit odd.
> >>
> >> AFAIK it comes from Vendor SpEcifiC but I'm not 100% sure ;-) The Alpine
> >> Ridge datasheet calls it also VSEC capability which is why we chose the
> >> naming accordingly.
> >
> > That said, it could also come from Vendor SpEcific Capability. I can
> > change it to tb_switch_find_vsec() if that works better.
> 
> I would folllow PCI existing namings, i.e.
> _vse_cap()

Indeed this sounds better.

I looked at the TBT specs again and they just use VSEC but do not open
the acronym any further. I guess the "Vendor Specific Extended
Capability" as Lukas pointed out is the correct one.

I'll change the patch to use tb_switch_find_vse_cap() and rename
TB_VSEC_CAP_* to TB_VSE_CAP_* if there are no objections.

[toc] | [prev] | [next] | [standalone]


#1656253 — [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-02 16:10 +0200
Subject[PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs
Message-ID<tNVr4-2dT-29@gated-at.bofh.it>
In reply to#1656243
Add Intel Win Ridge (Thunderbolt 2) and Alpine Ridge (Thunderbolt 3)
controller PCI IDs to the list of supported devices.

This code is based on the work done by Amir Levy and Michael Jamet.

Signed-off-by: Michael Jamet <michael.jamet@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Yehezkel Bernat <yehezkel.bernat@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/thunderbolt/nhi.c    | 11 +++++++++++
 drivers/thunderbolt/nhi.h    | 17 +++++++++++++++++
 drivers/thunderbolt/switch.c | 19 ++++++++++++++-----
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index c1113a3c4128..fa4c2745dba2 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -787,6 +787,17 @@ static struct pci_device_id nhi_ids[] = {
 		.device = PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_NHI,
 		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID,
 	},
+
+	/* Thunderbolt 3 */
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_2C_NHI) },
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_NHI) },
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_USBONLY_NHI) },
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_NHI) },
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_USBONLY_NHI) },
+	{ 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) },
+
 	{ 0,}
 };
 
diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
index 630f44140530..8bd9b4e5a0b1 100644
--- a/drivers/thunderbolt/nhi.h
+++ b/drivers/thunderbolt/nhi.h
@@ -143,4 +143,21 @@ static inline int ring_tx(struct tb_ring *ring, struct ring_frame *frame)
 	return __ring_enqueue(ring, frame);
 }
 
+/*
+ * 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
+ * hardware.
+ */
+#define PCI_DEVICE_ID_INTEL_WIN_RIDGE_2C_NHI            0x157d
+#define PCI_DEVICE_ID_INTEL_WIN_RIDGE_2C_BRIDGE         0x157e
+#define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_NHI		0x15bf
+#define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_BRIDGE	0x15c0
+#define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_NHI	0x15d2
+#define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_BRIDGE	0x15d3
+#define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_NHI	0x15d9
+#define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_BRIDGE	0x15da
+#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
+
 #endif
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 0a592ccb7923..8006577590ed 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -491,13 +491,22 @@ int tb_switch_configure(struct tb_switch *sw)
 		tb_sw_warn(sw, "unknown switch vendor id %#x\n",
 			   sw->config.vendor_id);
 
-	if (sw->config.device_id != PCI_DEVICE_ID_INTEL_LIGHT_RIDGE &&
-	    sw->config.device_id != PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C &&
-	    sw->config.device_id != PCI_DEVICE_ID_INTEL_PORT_RIDGE &&
-	    sw->config.device_id != PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_BRIDGE &&
-	    sw->config.device_id != PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_BRIDGE)
+	switch (sw->config.device_id) {
+	case PCI_DEVICE_ID_INTEL_LIGHT_RIDGE:
+	case PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C:
+	case PCI_DEVICE_ID_INTEL_PORT_RIDGE:
+	case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_BRIDGE:
+	case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_BRIDGE:
+	case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_2C_BRIDGE:
+	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:
+		break;
+
+	default:
 		tb_sw_warn(sw, "unsupported switch device id %#x\n",
 			   sw->config.device_id);
+	}
 
 	sw->config.enabled = 1;
 
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]


#1657349 — Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs

FromLukas Wunner <lukas@wunner.de>
Date2017-06-05 10:20 +0200
SubjectRe: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs
Message-ID<tOVp0-nU-17@gated-at.bofh.it>
In reply to#1656253
On Fri, Jun 02, 2017 at 05:05:16PM +0300, Mika Westerberg wrote:
> --- a/drivers/thunderbolt/nhi.h
> +++ b/drivers/thunderbolt/nhi.h
> @@ -143,4 +143,21 @@ static inline int ring_tx(struct tb_ring *ring, struct ring_frame *frame)
>  	return __ring_enqueue(ring, frame);
>  }
>  
> +/*
> + * 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
> + * hardware.
> + */

I wrote a patch a few months ago which replaces the PCI quirk with
device links:

https://github.com/l1k/linux/commit/8e2e7eaa1163

I was going to upstream this sometime this year, it probably would
have been better if I had done that already but I wasn't expecting
your series.

In any case, all Thunderbolt PCI device IDs can then be moved to a
header in drivers/thunderbolt/, except for a few TB1 devices which
are referenced by quirk_thunderbolt_hotplug_msi() and
quirk_apple_poweroff_thunderbolt().

As to using ICM on Apple hardware, I've heard from people with
Alpine Ridge MacBook Pros that the native (i.e. non-ICM) driver
at least probes fine.  I've yet to hear from folks who have
actually tested it with any attached TB3 devices, but my expectation
would be that it should work fine in native mode since the protocol
seems to be the same.

I think I saw somewhere that you reset the root switch when reconfiguring
the chip to run in ICM mode.  That's a problem because on Apple hardware,
there's a (native, non-ICM) EFI NHI driver which sets up tunnels to any
devices which are already attached on boot.  This can be used to boot
from Thunderbolt-attached harddisks.  I imagine that resetting the chip
to switch to ICM-mode will cause those already established tunnels to
go away, essentially breaking booting from TB-attached disks.  That's
a regression which we should try to avoid.

Ideally, the user should be able to choose whether to use ICM-mode or
native mode.  This could be implemented via a module parameter.
I imagine it might even be possible to use native mode on non-Macs
and this should be allowed as well.

Thanks,

Lukas

[toc] | [prev] | [next] | [standalone]


#1657461 — Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-05 11:40 +0200
SubjectRe: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs
Message-ID<tOWEp-193-7@gated-at.bofh.it>
In reply to#1657349
On Mon, Jun 05, 2017 at 10:14:37AM +0200, Lukas Wunner wrote:
> On Fri, Jun 02, 2017 at 05:05:16PM +0300, Mika Westerberg wrote:
> > --- a/drivers/thunderbolt/nhi.h
> > +++ b/drivers/thunderbolt/nhi.h
> > @@ -143,4 +143,21 @@ static inline int ring_tx(struct tb_ring *ring, struct ring_frame *frame)
> >  	return __ring_enqueue(ring, frame);
> >  }
> >  
> > +/*
> > + * 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
> > + * hardware.
> > + */
> 
> I wrote a patch a few months ago which replaces the PCI quirk with
> device links:
> 
> https://github.com/l1k/linux/commit/8e2e7eaa1163
> 
> I was going to upstream this sometime this year, it probably would
> have been better if I had done that already but I wasn't expecting
> your series.
> 
> In any case, all Thunderbolt PCI device IDs can then be moved to a
> header in drivers/thunderbolt/, except for a few TB1 devices which
> are referenced by quirk_thunderbolt_hotplug_msi() and
> quirk_apple_poweroff_thunderbolt().

OK, but that should be a separate patch, right? On top of your device
links patch.

> As to using ICM on Apple hardware, I've heard from people with
> Alpine Ridge MacBook Pros that the native (i.e. non-ICM) driver
> at least probes fine.  I've yet to hear from folks who have
> actually tested it with any attached TB3 devices, but my expectation
> would be that it should work fine in native mode since the protocol
> seems to be the same.

I have one Mac with Alpine Ridge (well 4 Thunderbolt ports, 2 Alpine
Ridges) and it indeed works in native mode but it can tunnel only one
device, no display port.

However, starting ICM on them allows you to connect up to 6 devices
including display port. It also allows cross-domain connections where we
can implement things like Amir's networking driver.

That's the reason we did it this way - to get Thunderbolt working the
same way in Linux than it works on Macs running OS X :)

> I think I saw somewhere that you reset the root switch when reconfiguring
> the chip to run in ICM mode.  That's a problem because on Apple hardware,
> there's a (native, non-ICM) EFI NHI driver which sets up tunnels to any
> devices which are already attached on boot.  This can be used to boot
> from Thunderbolt-attached harddisks.  I imagine that resetting the chip
> to switch to ICM-mode will cause those already established tunnels to
> go away, essentially breaking booting from TB-attached disks.  That's
> a regression which we should try to avoid.

Yes, we reset the links and ICM re-establishes them the but during that
time the PCIe tunnel is down. However, it should not affect booting as
that is done in the EFI firmware as far as I can tell. If you have
rootfs on a TBT harddisk, it should not be problem either if your initrd
includes drivers for Thunderbolt and the disks you are using.

Unless I'm missing something.

> Ideally, the user should be able to choose whether to use ICM-mode or
> native mode.  This could be implemented via a module parameter.

I have nothing against that in principle but at least we should default
to the most functioning connection manager then, and I'm not sure it is
worth adding right now until we get some feedback from people actually
running ICM on Macs.

> I imagine it might even be possible to use native mode on non-Macs
> and this should be allowed as well.

While I guess it is doable, it is a path that is not tested at all so
you will get more problems than you are trying to solve, IMHO.

[toc] | [prev] | [next] | [standalone]


#1657540 — Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs

FromLukas Wunner <lukas@wunner.de>
Date2017-06-05 14:10 +0200
SubjectRe: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs
Message-ID<tOYZz-2JP-3@gated-at.bofh.it>
In reply to#1657461
On Mon, Jun 05, 2017 at 12:32:49PM +0300, Mika Westerberg wrote:
> On Mon, Jun 05, 2017 at 10:14:37AM +0200, Lukas Wunner wrote:
> > On Fri, Jun 02, 2017 at 05:05:16PM +0300, Mika Westerberg wrote:
> > > --- a/drivers/thunderbolt/nhi.h
> > > +++ b/drivers/thunderbolt/nhi.h
> > > @@ -143,4 +143,21 @@ static inline int ring_tx(struct tb_ring *ring, struct ring_frame *frame)
> > >  	return __ring_enqueue(ring, frame);
> > >  }
> > >  
> > > +/*
> > > + * 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
> > > + * hardware.
> > > + */
> > 
> > I wrote a patch a few months ago which replaces the PCI quirk with
> > device links:
> > 
> > https://github.com/l1k/linux/commit/8e2e7eaa1163
> > 
> > I was going to upstream this sometime this year, it probably would
> > have been better if I had done that already but I wasn't expecting
> > your series.
> > 
> > In any case, all Thunderbolt PCI device IDs can then be moved to a
> > header in drivers/thunderbolt/, except for a few TB1 devices which
> > are referenced by quirk_thunderbolt_hotplug_msi() and
> > quirk_apple_poweroff_thunderbolt().
> 
> OK, but that should be a separate patch, right? On top of your device
> links patch.

Yes.  If you like the device links patch, feel free to include it
in your series or modify it as you see fit.

I forgot to mention, on Alpine Ridge there's an additional downstream
bridge with an XHCI controller below it.  A device link will also be
established from that downstream bridge to the NHI.  I'm not sure if
that is actually necessary, perhaps it's even undesirable.


> > As to using ICM on Apple hardware, I've heard from people with
> > Alpine Ridge MacBook Pros that the native (i.e. non-ICM) driver
> > at least probes fine.  I've yet to hear from folks who have
> > actually tested it with any attached TB3 devices, but my expectation
> > would be that it should work fine in native mode since the protocol
> > seems to be the same.
> 
> I have one Mac with Alpine Ridge (well 4 Thunderbolt ports, 2 Alpine
> Ridges) and it indeed works in native mode but it can tunnel only one
> device, no display port.

Yes.  Those are limitations of the native driver.  It doesn't support
chaining and DP tunnels yet.  If chained devices and DP devices are
present on boot, then the EFI NHI driver (on Macs) will establish the
tunnels and thunderbolt.ko will inherit them.  Once the devices are
unplugged and replugged, the tunnels are gone and cannot be re-established
until the machine is rebooted.


> However, starting ICM on them allows you to connect up to 6 devices
> including display port. It also allows cross-domain connections where we
> can implement things like Amir's networking driver.
> 
> That's the reason we did it this way - to get Thunderbolt working the
> same way in Linux than it works on Macs running OS X :)

Yes, it's useful to have ICM-based Thunderbolt on Macs and personally
I'm totally fine with making it the default for Macs which support it.
(I can't speak for Andreas, obviously.)

However it would be great to give people the *choice* between ICM versus
native mode, for at least two reasons:

(1) Native mode uses free software.  (I assume the ICM firmware remains
    closed source.)

(2) Native mode allows more versatility, e.g. how PCI tunnels are set up
    to chained devices: PCI fanout or PCI direct routing, see:
    https://developer.apple.com/library/content/documentation/HardwareDrivers/Conceptual/ThunderboltDevGuide/Basics/Basics.html

    Apple supports traffic prioritization to enable audio over Thunderbolt
    with higher accuracy / minimal skew, I assume their choice to use
    native mode was largely motivated by being able to support specialized
    applications like that which are difficult or perhaps impossible to
    implement in firmware:
    http://pdfpiw.uspto.gov/.piw?Docid=09015384


By the way, you wrote in one of the commit messages that ICM is
"a firmware running on the host controller".  Is it really running on
the *controller* (i.e. Thunderbolt chip)?  My understanding was that
ICM is part of the BIOS and executed on the host CPU in System Management
Mode.

Thanks,

Lukas

[toc] | [prev] | [next] | [standalone]


#1657577 — Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-05 15:00 +0200
SubjectRe: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs
Message-ID<tOZLY-30O-27@gated-at.bofh.it>
In reply to#1657540
On Mon, Jun 05, 2017 at 02:07:56PM +0200, Lukas Wunner wrote:
> On Mon, Jun 05, 2017 at 12:32:49PM +0300, Mika Westerberg wrote:
> > On Mon, Jun 05, 2017 at 10:14:37AM +0200, Lukas Wunner wrote:
> > > On Fri, Jun 02, 2017 at 05:05:16PM +0300, Mika Westerberg wrote:
> > > > --- a/drivers/thunderbolt/nhi.h
> > > > +++ b/drivers/thunderbolt/nhi.h
> > > > @@ -143,4 +143,21 @@ static inline int ring_tx(struct tb_ring *ring, struct ring_frame *frame)
> > > >  	return __ring_enqueue(ring, frame);
> > > >  }
> > > >  
> > > > +/*
> > > > + * 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
> > > > + * hardware.
> > > > + */
> > > 
> > > I wrote a patch a few months ago which replaces the PCI quirk with
> > > device links:
> > > 
> > > https://github.com/l1k/linux/commit/8e2e7eaa1163
> > > 
> > > I was going to upstream this sometime this year, it probably would
> > > have been better if I had done that already but I wasn't expecting
> > > your series.
> > > 
> > > In any case, all Thunderbolt PCI device IDs can then be moved to a
> > > header in drivers/thunderbolt/, except for a few TB1 devices which
> > > are referenced by quirk_thunderbolt_hotplug_msi() and
> > > quirk_apple_poweroff_thunderbolt().
> > 
> > OK, but that should be a separate patch, right? On top of your device
> > links patch.
> 
> Yes.  If you like the device links patch, feel free to include it
> in your series or modify it as you see fit.

It is kind of separate thing - here we add the the missing functionality
so that PC users can finally get their TBT devices connected.

We can do those things later when the basic functionality is in place
IMHO.

> I forgot to mention, on Alpine Ridge there's an additional downstream
> bridge with an XHCI controller below it.  A device link will also be
> established from that downstream bridge to the NHI.  I'm not sure if
> that is actually necessary, perhaps it's even undesirable.

OK

> > > As to using ICM on Apple hardware, I've heard from people with
> > > Alpine Ridge MacBook Pros that the native (i.e. non-ICM) driver
> > > at least probes fine.  I've yet to hear from folks who have
> > > actually tested it with any attached TB3 devices, but my expectation
> > > would be that it should work fine in native mode since the protocol
> > > seems to be the same.
> > 
> > I have one Mac with Alpine Ridge (well 4 Thunderbolt ports, 2 Alpine
> > Ridges) and it indeed works in native mode but it can tunnel only one
> > device, no display port.
> 
> Yes.  Those are limitations of the native driver.  It doesn't support
> chaining and DP tunnels yet.  If chained devices and DP devices are
> present on boot, then the EFI NHI driver (on Macs) will establish the
> tunnels and thunderbolt.ko will inherit them.  Once the devices are
> unplugged and replugged, the tunnels are gone and cannot be re-established
> until the machine is rebooted.
> 
> > However, starting ICM on them allows you to connect up to 6 devices
> > including display port. It also allows cross-domain connections where we
> > can implement things like Amir's networking driver.
> > 
> > That's the reason we did it this way - to get Thunderbolt working the
> > same way in Linux than it works on Macs running OS X :)
> 
> Yes, it's useful to have ICM-based Thunderbolt on Macs and personally
> I'm totally fine with making it the default for Macs which support it.
> (I can't speak for Andreas, obviously.)
> 
> However it would be great to give people the *choice* between ICM versus
> native mode, for at least two reasons:
> 
> (1) Native mode uses free software.  (I assume the ICM firmware remains
>     closed source.)
> 
> (2) Native mode allows more versatility, e.g. how PCI tunnels are set up
>     to chained devices: PCI fanout or PCI direct routing, see:
>     https://developer.apple.com/library/content/documentation/HardwareDrivers/Conceptual/ThunderboltDevGuide/Basics/Basics.html
> 
>     Apple supports traffic prioritization to enable audio over Thunderbolt
>     with higher accuracy / minimal skew, I assume their choice to use
>     native mode was largely motivated by being able to support specialized
>     applications like that which are difficult or perhaps impossible to
>     implement in firmware:
>     http://pdfpiw.uspto.gov/.piw?Docid=09015384

Right, but do we need to do it now before we have got any feedback from
users using Macs with Alpine Ridge and ICM? We can always add the module
parameter later if really needed.

> By the way, you wrote in one of the commit messages that ICM is
> "a firmware running on the host controller".  Is it really running on
> the *controller* (i.e. Thunderbolt chip)?  My understanding was that
> ICM is part of the BIOS and executed on the host CPU in System Management
> Mode.

Yes it is. The TBT3 devices (well even TBT2 devices) supporting secure
connect for example implement it in ICM firmware.

With these patches you can even upgrade that ICM firmware on devices and
that also works if you are using the native connection manager.

[toc] | [prev] | [next] | [standalone]


#1657596 — Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs

FromMika Westerberg <mika.westerberg@linux.intel.com>
Date2017-06-05 15:40 +0200
SubjectRe: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs
Message-ID<tP0oF-3td-5@gated-at.bofh.it>
In reply to#1657577
On Mon, Jun 05, 2017 at 03:55:07PM +0300, Mika Westerberg wrote:
> > Yes, it's useful to have ICM-based Thunderbolt on Macs and personally
> > I'm totally fine with making it the default for Macs which support it.
> > (I can't speak for Andreas, obviously.)
> > 
> > However it would be great to give people the *choice* between ICM versus
> > native mode, for at least two reasons:
> > 
> > (1) Native mode uses free software.  (I assume the ICM firmware remains
> >     closed source.)
> > 
> > (2) Native mode allows more versatility, e.g. how PCI tunnels are set up
> >     to chained devices: PCI fanout or PCI direct routing, see:
> >     https://developer.apple.com/library/content/documentation/HardwareDrivers/Conceptual/ThunderboltDevGuide/Basics/Basics.html
> > 
> >     Apple supports traffic prioritization to enable audio over Thunderbolt
> >     with higher accuracy / minimal skew, I assume their choice to use
> >     native mode was largely motivated by being able to support specialized
> >     applications like that which are difficult or perhaps impossible to
> >     implement in firmware:
> >     http://pdfpiw.uspto.gov/.piw?Docid=09015384
> 
> Right, but do we need to do it now before we have got any feedback from
> users using Macs with Alpine Ridge and ICM? We can always add the module
> parameter later if really needed.

It turns out that this is not too big change after all so I think it can
be included in the ICM patch if you want.

Basically we would have module parameter:

	cm=auto|native|icm

where auto is the default.

Does that work?

[toc] | [prev] | [next] | [standalone]


Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel


csiph-web