Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   <Mario.Limonciello@dell.com>
To:     <mika.westerberg@linux.intel.com>, <linux-kernel@vger.kernel.org>
CC:     <michael.jamet@intel.com>, <YehezkelShB@gmail.com>,
        <andreas.noever@gmail.com>, <lukas@wunner.de>,
        <davem@davemloft.net>, <andriy.shevchenko@linux.intel.com>,
        <ckellner@redhat.com>, <netdev@vger.kernel.org>
Subject: RE: [PATCH v3 00/36] thunderbolt: Software connection manager
 improvements
Thread-Topic: [PATCH v3 00/36] thunderbolt: Software connection manager
 improvements
Thread-Index: AQHU5WLl07JI2cAT10qiEqN6uf0TUaYhJGhw
Date:   Thu, 28 Mar 2019 15:17:57 +0000
Message-ID: <1aaab9baea4f46b481b5601dfb060104@ausx13mpc120.AMER.DELL.COM>
References: <20190328123633.42882-1-mika.westerberg@linux.intel.com>
In-Reply-To: <20190328123633.42882-1-mika.westerberg@linux.intel.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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

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

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

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

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

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

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

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