Re: [PATCH 00/24] Thunderbolt security levels and NVM firmware upgrade

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, 2017-05-18 at 17:38 +0300, Mika Westerberg wrote:
> Hi all,
> 
> This patch series adds support for Thunderbolt security levels, which
> were
> first introduced in Intel Falcon Ridge Thunderbolt controller, to
> prevent
> DMA attacks when PCIe is tunneled over Thunderbolt fabric. This is
> needed
> if there is no IOMMU available for various reasons.
> 
> Most PCs out there having Falcon Ridge or newer have security level
> set to
> "user" which means that user authorization is needed before PCIe
> tunnel is
> creaded (the PCIe device appears). This effectively means that without
> driver support the user needs to configure security level from BIOS to
> "none" to get Thunderbolt devices connected. With these patches the
> user
> can authorize devices using sysfs attributes like:
> 
>   # echo 1 > /sys/bus/thunderbolt/devices/0-1/authorized
> 
> 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
> 
> This is documented with more details in patch [23/24].
> 
> This series is based on Amir's networking patches [1] but instead of
> splitting the functionality between kernel driver and userspace
> daemon, we
> take advantage of Linux driver core by converting the existing driver
> to
> expose a Linux bus (domain) and devices (switches). Notifications to
> the
> userspace about plugged/unplugged devices is handled by standard
> uevents
> when a device is added to/removed from the Thunderbolt bus.
> 
> Since thunderbolt device identification and authorization can be done
> directly through sysfs attributes there is no need for userspace
> daemon.
> However, there still should be an application that promps user for
> unknown
> devices and allows selecting between "single connect" and "connect
> always"
> keeping this information in a database or similar persistent storage.
> This
> patch series only provides mechanism for userspace applications to
> achieve
> that.
> 
> 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).
> 
> We are also in works of porting Amir's networking driver to work on
> top of
> the new Thunderbolt bus pretty much the same way firewire networking
> is
> currently done. In addition this makes is possible to introduce other
> protocols like a char device that allows userspace directly to
> communicate
> accross Thunderbolt domains.
> 
> 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.
> 
> These patches use uuid_be from uuid.h but I've learned that there is a
> work
> to remove the type completely in favor of new uuid_t [2]. I'm not sure
> what
> to do regarding that because those patches are not yet in the
> mainline.

Looks like we may use uuid_be for now, though having a patch to switch
to uuid_t eventually.

I have commented few patches (some minor comments), other than that,
FWIW:
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> 
> [1] https://lkml.org/lkml/2016/11/9/341
> [2] http://git.infradead.org/users/hch/vfs.git/shortlog/refs/heads/uui
> d-types
> 
> Mika Westerberg (24):
>   thunderbolt: Use const buffer pointer in write operations
>   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: 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 Thunderbolt 3 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 |  108 +++
>  Documentation/admin-guide/index.rst             |    1 +
>  Documentation/admin-guide/thunderbolt.rst       |  197 ++++
>  MAINTAINERS                                     |    3 +
>  drivers/thunderbolt/Kconfig                     |   13 +-
>  drivers/thunderbolt/Makefile                    |    2 +-
>  drivers/thunderbolt/cap.c                       |  169 ++--
>  drivers/thunderbolt/ctl.c                       |  655 +++++++++----
>  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                    |   84 +-
>  drivers/thunderbolt/icm.c                       | 1098
> ++++++++++++++++++++++
>  drivers/thunderbolt/nhi.c                       |  302 +++++-
>  drivers/thunderbolt/nhi.h                       |   91 +-
>  drivers/thunderbolt/nhi_regs.h                  |   27 +
>  drivers/thunderbolt/switch.c                    | 1109
> +++++++++++++++++++++--
>  drivers/thunderbolt/tb.c                        |  237 ++---
>  drivers/thunderbolt/tb.h                        |  242 ++++-
>  drivers/thunderbolt/tb_msgs.h                   |  260 ++++++
>  drivers/thunderbolt/tb_regs.h                   |   31 +-
>  drivers/thunderbolt/tunnel_pci.c                |   17 +-
>  23 files changed, 5213 insertions(+), 551 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
> 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy