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

Groups > linux.kernel > #1491910 > unrolled thread

[PATCH v7 0/8] thunderbolt: Introducing Thunderbolt(TM) Networking

Started byAmir Levy <amir.jer.levy@intel.com>
First post2016-09-27 15:50 +0200
Last post2016-09-27 16:20 +0200
Articles 9 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v7 0/8] thunderbolt: Introducing Thunderbolt(TM) Networking Amir Levy <amir.jer.levy@intel.com> - 2016-09-27 15:50 +0200
    [PATCH v7 3/8] thunderbolt: Kconfig for Thunderbolt Networking Amir Levy <amir.jer.levy@intel.com> - 2016-09-27 15:50 +0200
      Re: [PATCH v7 3/8] thunderbolt: Kconfig for Thunderbolt Networking Greg KH <gregkh@linuxfoundation.org> - 2016-09-27 16:10 +0200
    [PATCH v7 1/8] thunderbolt: Macro rename Amir Levy <amir.jer.levy@intel.com> - 2016-09-27 15:50 +0200
    [PATCH v7 7/8] thunderbolt: Networking doc Amir Levy <amir.jer.levy@intel.com> - 2016-09-27 15:50 +0200
    [PATCH v7 2/8] thunderbolt: Updating the register definitions Amir Levy <amir.jer.levy@intel.com> - 2016-09-27 16:00 +0200
    Re: [PATCH v7 5/8] thunderbolt: Networking state machine Greg KH <gregkh@linuxfoundation.org> - 2016-09-27 16:20 +0200
    Re: [PATCH v7 4/8] thunderbolt: Communication with the ICM (firmware) Greg KH <gregkh@linuxfoundation.org> - 2016-09-27 16:20 +0200
    Re: [PATCH v7 5/8] thunderbolt: Networking state machine Greg KH <gregkh@linuxfoundation.org> - 2016-09-27 16:20 +0200

#1491910 — [PATCH v7 0/8] thunderbolt: Introducing Thunderbolt(TM) Networking

FromAmir Levy <amir.jer.levy@intel.com>
Date2016-09-27 15:50 +0200
Subject[PATCH v7 0/8] thunderbolt: Introducing Thunderbolt(TM) Networking
Message-ID<sm0VH-1H1-3@gated-at.bofh.it>
This driver enables Thunderbolt Networking on non-Apple platforms
running Linux.

Thunderbolt Networking provides peer-to-peer connections to transfer
files between computers, perform PC migrations, and/or set up small
workgroups with shared storage.

This is a virtual connection that emulates an Ethernet adapter that
enables Ethernet networking with the benefit of Thunderbolt superfast
medium capability.

Thunderbolt Networking enables two hosts and several devices that
have a Thunderbolt controller to be connected together in a linear
(Daisy chain) series from a single port.

Thunderbolt Networking for Linux is compatible with Thunderbolt
Networking on systems running macOS or Windows and also supports
Thunderbolt generation 2 and 3 controllers.

Note that all pre-existing Thunderbolt generation 3 features, such as
USB, Display and other Thunderbolt device connectivity will continue
to function exactly as they did prior to enabling Thunderbolt Networking.

Code and Software Specifications:
This kernel code creates a virtual ethernet device for computer to
computer communication over a Thunderbolt cable.
The new driver is a separate driver to the existing Thunderbolt driver.
It is designed to work on systems running Linux that
interface with Intel Connection Manager (ICM) firmware based
Thunderbolt controllers that support Thunderbolt Networking.
The kernel code operates in coordination with the Thunderbolt user-
space daemon to implement full Thunderbolt networking functionality.

Hardware Specifications:
Thunderbolt Hardware specs have not yet been published but are used
where necessary for register definitions. 

Changes since v6:
 - Removed (TM) from Kconfig
 - Removed redundant DRV_NAME macro
 - Removed copyright notice from Makefile
 - Moved networking.txt file to Documentation/Thunderbolt/
 - Edited patch descriptions
 - Edited documentation in networking.txt

These patches were pushed to GitHub where they can be reviewed more
comfortably with green/red highlighting:
	https://github.com/01org/thunderbolt-software-kernel-tree

Daemon code:
	https://github.com/01org/thunderbolt-software-daemon

For reference, here's a link to version 6:
[v6]:	https://lkml.org/lkml/2016/8/1/233

Amir Levy (8):
  thunderbolt: Macro rename
  thunderbolt: Updating the register definitions
  thunderbolt: Kconfig for Thunderbolt Networking
  thunderbolt: Communication with the ICM (firmware)
  thunderbolt: Networking state machine
  thunderbolt: Networking transmit and receive
  thunderbolt: Networking doc
  thunderbolt: Adding maintainer entry

 Documentation/00-INDEX                   |    2 +
 Documentation/thunderbolt/networking.txt |  132 ++
 MAINTAINERS                              |    8 +-
 drivers/thunderbolt/Kconfig              |   27 +-
 drivers/thunderbolt/Makefile             |    3 +-
 drivers/thunderbolt/icm/Makefile         |    2 +
 drivers/thunderbolt/icm/icm_nhi.c        | 1622 +++++++++++++++++++++
 drivers/thunderbolt/icm/icm_nhi.h        |   92 ++
 drivers/thunderbolt/icm/net.c            | 2264 ++++++++++++++++++++++++++++++
 drivers/thunderbolt/icm/net.h            |  297 ++++
 drivers/thunderbolt/nhi_regs.h           |  115 +-
 11 files changed, 4555 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/thunderbolt/networking.txt
 create mode 100644 drivers/thunderbolt/icm/Makefile
 create mode 100644 drivers/thunderbolt/icm/icm_nhi.c
 create mode 100644 drivers/thunderbolt/icm/icm_nhi.h
 create mode 100644 drivers/thunderbolt/icm/net.c
 create mode 100644 drivers/thunderbolt/icm/net.h

-- 
2.7.4

[toc] | [next] | [standalone]

#1491911 — [PATCH v7 3/8] thunderbolt: Kconfig for Thunderbolt Networking

FromAmir Levy <amir.jer.levy@intel.com>
Date2016-09-27 15:50 +0200
Subject[PATCH v7 3/8] thunderbolt: Kconfig for Thunderbolt Networking
Message-ID<sm0VI-1H1-27@gated-at.bofh.it>
In reply to#1491910
Update to the Kconfig Thunderbolt description to add
Thunderbolt networking as an option.
The menu item "Thunderbolt support" now offers:
"Apple Hardware Support" (existing)
        and/or
"Thunderbolt Networking" (new)

You can choose the driver for your platform or build both drivers -
each driver will detect if it can run on the specific platform.
If the Thunderbolt Networking option is chosen, Thunderbolt Networking
will be enabled between Linux non-Apple systems, macOS and
Windows based systems.
Thunderbolt Networking will not affect any other Thunderbolt feature that
was previous available to Linux users on either Apple or
non-Apple platforms.

Signed-off-by: Amir Levy <amir.jer.levy@intel.com>
---
 drivers/thunderbolt/Kconfig  | 27 +++++++++++++++++++++++----
 drivers/thunderbolt/Makefile |  2 +-
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
index c121acc..376e5bb 100644
--- a/drivers/thunderbolt/Kconfig
+++ b/drivers/thunderbolt/Kconfig
@@ -1,13 +1,32 @@
-menuconfig THUNDERBOLT
-	tristate "Thunderbolt support for Apple devices"
+config THUNDERBOLT
+	tristate "Thunderbolt support"
 	depends on PCI
 	select CRC32
 	help
-	  Cactus Ridge Thunderbolt Controller driver
+	  Thunderbolt Controller driver
+
+if THUNDERBOLT
+
+config THUNDERBOLT_APPLE
+	tristate "Apple hardware support"
+	help
 	  This driver is required if you want to hotplug Thunderbolt devices on
 	  Apple hardware.
 
 	  Device chaining is currently not supported.
 
-	  To compile this driver a module, choose M here. The module will be
+	  To compile this driver as a module, choose M here. The module will be
 	  called thunderbolt.
+
+config THUNDERBOLT_ICM
+	tristate "Thunderbolt Networking"
+	help
+	  This driver is required if you want Thunderbolt Networking on
+	  non-Apple hardware.
+	  It creates a virtual Ethernet device that enables computer to
+	  computer communication over a Thunderbolt cable.
+
+	  To compile this driver as a module, choose M here. The module will be
+	  called thunderbolt_icm.
+
+endif
diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
index 5d1053c..7a85bd1 100644
--- a/drivers/thunderbolt/Makefile
+++ b/drivers/thunderbolt/Makefile
@@ -1,3 +1,3 @@
-obj-${CONFIG_THUNDERBOLT} := thunderbolt.o
+obj-${CONFIG_THUNDERBOLT_APPLE} := thunderbolt.o
 thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o tunnel_pci.o eeprom.o
 
-- 
2.7.4

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

#1491926 — Re: [PATCH v7 3/8] thunderbolt: Kconfig for Thunderbolt Networking

FromGreg KH <gregkh@linuxfoundation.org>
Date2016-09-27 16:10 +0200
SubjectRe: [PATCH v7 3/8] thunderbolt: Kconfig for Thunderbolt Networking
Message-ID<sm1f3-22M-13@gated-at.bofh.it>
In reply to#1491911
On Tue, Sep 27, 2016 at 04:43:36PM +0300, Amir Levy wrote:
> Update to the Kconfig Thunderbolt description to add
> Thunderbolt networking as an option.
> The menu item "Thunderbolt support" now offers:
> "Apple Hardware Support" (existing)
>         and/or
> "Thunderbolt Networking" (new)
> 
> You can choose the driver for your platform or build both drivers -
> each driver will detect if it can run on the specific platform.
> If the Thunderbolt Networking option is chosen, Thunderbolt Networking
> will be enabled between Linux non-Apple systems, macOS and
> Windows based systems.
> Thunderbolt Networking will not affect any other Thunderbolt feature that
> was previous available to Linux users on either Apple or
> non-Apple platforms.
> 
> Signed-off-by: Amir Levy <amir.jer.levy@intel.com>
> ---
>  drivers/thunderbolt/Kconfig  | 27 +++++++++++++++++++++++----
>  drivers/thunderbolt/Makefile |  2 +-
>  2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
> index c121acc..376e5bb 100644
> --- a/drivers/thunderbolt/Kconfig
> +++ b/drivers/thunderbolt/Kconfig
> @@ -1,13 +1,32 @@
> -menuconfig THUNDERBOLT
> -	tristate "Thunderbolt support for Apple devices"
> +config THUNDERBOLT
> +	tristate "Thunderbolt support"
>  	depends on PCI
>  	select CRC32
>  	help
> -	  Cactus Ridge Thunderbolt Controller driver
> +	  Thunderbolt Controller driver
> +
> +if THUNDERBOLT
> +
> +config THUNDERBOLT_APPLE
> +	tristate "Apple hardware support"
> +	help
>  	  This driver is required if you want to hotplug Thunderbolt devices on
>  	  Apple hardware.
>  
>  	  Device chaining is currently not supported.
>  
> -	  To compile this driver a module, choose M here. The module will be
> +	  To compile this driver as a module, choose M here. The module will be
>  	  called thunderbolt.
> +
> +config THUNDERBOLT_ICM

You are adding a config option here that is not used in the tree yet.
Please wait until you need the option to actually add it :)

You might just want to do the APPLE stuff here, that way you can drop in
the networking stuff in a simple patch at the end of this series.

thanks,

greg k-h

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

#1491912 — [PATCH v7 1/8] thunderbolt: Macro rename

FromAmir Levy <amir.jer.levy@intel.com>
Date2016-09-27 15:50 +0200
Subject[PATCH v7 1/8] thunderbolt: Macro rename
Message-ID<sm0VI-1H1-17@gated-at.bofh.it>
In reply to#1491910
This first patch updates the NHI Thunderbolt controller registers file to
reflect that it is not only for Cactus Ridge.
No functional change intended.

Signed-off-by: Amir Levy <amir.jer.levy@intel.com>
Signed-off-by: Andreas Noever <andreas.noever@gmail.com>
---
 drivers/thunderbolt/nhi_regs.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/thunderbolt/nhi_regs.h b/drivers/thunderbolt/nhi_regs.h
index 86b996c..75cf069 100644
--- a/drivers/thunderbolt/nhi_regs.h
+++ b/drivers/thunderbolt/nhi_regs.h
@@ -1,11 +1,11 @@
 /*
- * Thunderbolt Cactus Ridge driver - NHI registers
+ * Thunderbolt driver - NHI registers
  *
  * Copyright (c) 2014 Andreas Noever <andreas.noever@gmail.com>
  */
 
-#ifndef DSL3510_REGS_H_
-#define DSL3510_REGS_H_
+#ifndef NHI_REGS_H_
+#define NHI_REGS_H_
 
 #include <linux/types.h>
 
-- 
2.7.4

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

#1491917 — [PATCH v7 7/8] thunderbolt: Networking doc

FromAmir Levy <amir.jer.levy@intel.com>
Date2016-09-27 15:50 +0200
Subject[PATCH v7 7/8] thunderbolt: Networking doc
Message-ID<sm0VI-1H1-37@gated-at.bofh.it>
In reply to#1491910
Adding Thunderbolt(TM) networking documentation.

Signed-off-by: Amir Levy <amir.jer.levy@intel.com>
---
 Documentation/00-INDEX                   |   2 +
 Documentation/thunderbolt/networking.txt | 132 +++++++++++++++++++++++++++++++
 2 files changed, 134 insertions(+)
 create mode 100644 Documentation/thunderbolt/networking.txt

diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX
index cb9a6c6..a448ba1 100644
--- a/Documentation/00-INDEX
+++ b/Documentation/00-INDEX
@@ -439,6 +439,8 @@ this_cpu_ops.txt
 	- List rationale behind and the way to use this_cpu operations.
 thermal/
 	- directory with information on managing thermal issues (CPU/temp)
+thunderbolt/
+	- directory with info regarding Thunderbolt.
 trace/
 	- directory with info on tracing technologies within linux
 unaligned-memory-access.txt
diff --git a/Documentation/thunderbolt/networking.txt b/Documentation/thunderbolt/networking.txt
new file mode 100644
index 0000000..88d1c12
--- /dev/null
+++ b/Documentation/thunderbolt/networking.txt
@@ -0,0 +1,132 @@
+Intel Thunderbolt(TM) Networking driver
+=======================================
+
+Copyright(c) 2013 - 2016 Intel Corporation.
+
+Contact Information:
+Intel Thunderbolt mailing list <thunderbolt-software@lists.01.org>
+Edited by Amir Levy <amir.jer.levy@intel.com>
+
+Overview
+========
+
+* The Thunderbolt Networking driver enables peer to peer networking on non-Apple
+  platforms running Linux.
+
+* The driver creates a virtual Ethernet device that enables computer to computer
+  communication over the Thunderbolt cable.
+
+* Using Thunderbolt Networking you can perform high speed file transfers between
+  computers, perform PC migrations and/or set up small workgroups with shared
+  storage without compromising any other Thunderbolt functionality.
+
+* The driver is located in drivers/thunderbolt/icm.
+
+* This driver will function only on non-Apple platforms with firmware based
+  Thunderbolt controllers that support Thunderbolt Networking.
+
+  +----------------+            +----------------+
+  |Host 1          |            |Host 2          |
+  |                |            |                |
+  |   +-------+    |            |    +-------+   |
+  |   |Network|    |            |    |Network|   |
+  |   |Stack  |    |            |    |Stack  |   |
+  |   +-------+    |            |    +-------+   |
+  |       ^        |            |        ^       |
+  |       |        |            |        |       |
+  |       v        |            |        v       |
+  | +-----------+  |            |  +-----------+ |
+  | |Thunderbolt|  |            |  |Thunderbolt| |
+  | |Networking |  |            |  |Networking | |
+  | |Driver     |  |            |  |Driver     | |
+  | +-----------+  |            |  +-----------+ |
+  |       ^        |            |        ^       |
+  |       |        |            |        |       |
+  |       v        |            |        v       |
+  | +-----------+  |            |  +-----------+ |
+  | |Thunderbolt|  |            |  |Thunderbolt| |
+  | |Controller |<-+------------+->|Controller | |
+  | |with ICM   |  |            |  |with ICM   | |
+  | |enabled    |  |            |  |enabled    | |
+  | +-----------+  |            |  +-----------+ |
+  +----------------+            +----------------+
+
+Files
+=====
+
+The following files are located in the drivers/thunderbolt/icm directory:
+
+- icm_nhi.c/h:	These files allow communication with the firmware (Intel
+  Connection Manager) based controller. They also create an interface for
+  netlink communication with a user space daemon.
+
+- net.c/net.h:	These files implement the 'eth' interface for the
+  Thunderbolt(TM) Networking.
+
+Interface to User Space
+=======================
+
+The interface to the user space module is implemented through a Generic Netlink.
+This is the communications protocol between the Thunderbolt driver and the user
+space application.
+
+Note that this interface mediates user space communication with ICM.
+(Existing Linux tools can be used to configure the network interface.)
+
+The Thunderbolt Daemon utilizes this interface to communicate with the driver.
+To be accessed by the user space module, both kernel and user space modules
+have to register with the same GENL_NAME.
+For the purpose of the Thunderbolt Network driver, "thunderbolt" is used.
+The registration is done at driver initialization time for all instances
+of the Thunderbolt controllers. The communication is carried through pre-defined
+Thunderbolt messages. Each specific message has a callback function that is
+called when the related message is received.
+
+Message Definitions:
+* NHI_CMD_UNSPEC: Not used.
+* NHI_CMD_SUBSCRIBE: Subscription request from daemon to driver to open the
+  communication channel.
+* NHI_CMD_UNSUBSCRIBE: Request from daemon to driver to unsubscribe and
+  to close communication channel.
+* NHI_CMD_QUERY_INFORMATION: Request information from the driver such as
+  driver version, FW version offset, number of ports in the controller
+  and DMA port.
+* NHI_CMD_MSG_TO_ICM: Message from user space module to FW.
+* NHI_CMD_MSG_FROM_ICM: Response from FW to user space module.
+* NHI_CMD_MAILBOX: Message that uses mailbox mechanism such as FW policy
+  changes or disconnect path.
+* NHI_CMD_APPROVE_TBT_NETWORKING: Request from user space module to FW to
+  establish path.
+* NHI_CMD_ICM_IN_SAFE_MODE: Indication that the FW has entered safe mode.
+
+Communication with Intel Connection Manager(ICM) Firmware
+=========================================================
+
+There are several circular buffers in Thunderbolt each using Direct Memory
+Access (DMA).
+
+Communication with ICM utilizes circular buffer ring #0. (The other rings are
+used for peer to peer communication, packet transmission and receiving).
+
+The driver allocates a shared memory that is physically mapped onto the DMA
+physical space at ring #0.
+For the software to communicate with the firmware, the driver sends a command
+in ring #0. The command contains a pre-defined field (PDF) value notifying the
+firmware that the driver is ready. To proceed, the driver must receive the
+appropriate PDF value in response from the firmware.
+
+Once the exchange is completed, messages can be sent to the firmware through
+the driver. Similarly, the firmware can now send notifications about hardware
+and firmware events.
+
+Information
+===========
+
+Mailing list:
+	thunderbolt-software@lists.01.org
+	Register at: https://lists.01.org/mailman/listinfo/thunderbolt-software
+	Archives at: https://lists.01.org/pipermail/thunderbolt-software/
+
+For additional information about Thunderbolt technology visit:
+	https://01.org/thunderbolt-sw
+	https://thunderbolttechnology.net/
-- 
2.7.4

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

#1491924 — [PATCH v7 2/8] thunderbolt: Updating the register definitions

FromAmir Levy <amir.jer.levy@intel.com>
Date2016-09-27 16:00 +0200
Subject[PATCH v7 2/8] thunderbolt: Updating the register definitions
Message-ID<sm15o-1KC-29@gated-at.bofh.it>
In reply to#1491910
Adding more Thunderbolt(TM) register definitions
and some helper macros.

Signed-off-by: Amir Levy <amir.jer.levy@intel.com>
---
 drivers/thunderbolt/nhi_regs.h | 109 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 109 insertions(+)

diff --git a/drivers/thunderbolt/nhi_regs.h b/drivers/thunderbolt/nhi_regs.h
index 75cf069..b8e961f 100644
--- a/drivers/thunderbolt/nhi_regs.h
+++ b/drivers/thunderbolt/nhi_regs.h
@@ -9,6 +9,11 @@
 
 #include <linux/types.h>
 
+#define NHI_MMIO_BAR 0
+
+#define TBT_RING_MIN_NUM_BUFFERS	2
+#define TBT_RING_MAX_FRAME_SIZE		(4 * 1024)
+
 enum ring_flags {
 	RING_FLAG_ISOCH_ENABLE = 1 << 27, /* TX only? */
 	RING_FLAG_E2E_FLOW_CONTROL = 1 << 28,
@@ -39,6 +44,33 @@ struct ring_desc {
 	u32 time; /* write zero */
 } __packed;
 
+/**
+ * struct tbt_buf_desc - TX/RX ring buffer descriptor.
+ * This is same as struct ring_desc, but without the use of bitfields and
+ * with explicit endianity.
+ */
+struct tbt_buf_desc {
+	__le64 phys;
+	__le32 attributes;
+	__le32 time;
+};
+
+#define DESC_ATTR_LEN_SHIFT		0
+#define DESC_ATTR_LEN_MASK		GENMASK(11, DESC_ATTR_LEN_SHIFT)
+#define DESC_ATTR_EOF_SHIFT		12
+#define DESC_ATTR_EOF_MASK		GENMASK(15, DESC_ATTR_EOF_SHIFT)
+#define DESC_ATTR_SOF_SHIFT		16
+#define DESC_ATTR_SOF_MASK		GENMASK(19, DESC_ATTR_SOF_SHIFT)
+#define DESC_ATTR_TX_ISOCH_DMA_EN	BIT(20)	/* TX */
+#define DESC_ATTR_RX_CRC_ERR		BIT(20)	/* RX after use */
+#define DESC_ATTR_DESC_DONE		BIT(21)
+#define DESC_ATTR_REQ_STS		BIT(22)	/* TX and RX before use */
+#define DESC_ATTR_RX_BUF_OVRN_ERR	BIT(22)	/* RX after use */
+#define DESC_ATTR_INT_EN		BIT(23)
+#define DESC_ATTR_OFFSET_SHIFT		24
+#define DESC_ATTR_OFFSET_MASK		GENMASK(31, DESC_ATTR_OFFSET_SHIFT)
+
+
 /* NHI registers in bar 0 */
 
 /*
@@ -60,6 +92,30 @@ struct ring_desc {
  */
 #define REG_RX_RING_BASE	0x08000
 
+#define REG_RING_STEP			16
+#define REG_RING_PHYS_LO_OFFSET		0
+#define REG_RING_PHYS_HI_OFFSET		4
+#define REG_RING_CONS_PROD_OFFSET	8	/* cons - RO, prod - RW */
+#define REG_RING_CONS_SHIFT		0
+#define REG_RING_CONS_MASK		GENMASK(15, REG_RING_CONS_SHIFT)
+#define REG_RING_PROD_SHIFT		16
+#define REG_RING_PROD_MASK		GENMASK(31, REG_RING_PROD_SHIFT)
+#define REG_RING_SIZE_OFFSET		12
+#define REG_RING_SIZE_SHIFT		0
+#define REG_RING_SIZE_MASK		GENMASK(15, REG_RING_SIZE_SHIFT)
+#define REG_RING_BUF_SIZE_SHIFT		16
+#define REG_RING_BUF_SIZE_MASK		GENMASK(27, REG_RING_BUF_SIZE_SHIFT)
+
+#define TBT_RING_CONS_PROD_REG(iobase, ringbase, ringnumber) \
+			      ((iobase) + (ringbase) + \
+			      ((ringnumber) * REG_RING_STEP) + \
+			      REG_RING_CONS_PROD_OFFSET)
+
+#define TBT_REG_RING_PROD_EXTRACT(val) (((val) & REG_RING_PROD_MASK) >> \
+				       REG_RING_PROD_SHIFT)
+
+#define TBT_REG_RING_CONS_EXTRACT(val) (((val) & REG_RING_CONS_MASK) >> \
+				       REG_RING_CONS_SHIFT)
 /*
  * 32 bytes per entry, one entry for every hop (REG_HOP_COUNT)
  * 00: enum_ring_flags
@@ -77,6 +133,19 @@ struct ring_desc {
  * ..: unknown
  */
 #define REG_RX_OPTIONS_BASE	0x29800
+#define REG_RX_OPTS_TX_E2E_HOP_ID_SHIFT	12
+#define REG_RX_OPTS_TX_E2E_HOP_ID_MASK	\
+				GENMASK(22, REG_RX_OPTS_TX_E2E_HOP_ID_SHIFT)
+#define REG_RX_OPTS_MASK_OFFSET		4
+#define REG_RX_OPTS_MASK_EOF_SHIFT	0
+#define REG_RX_OPTS_MASK_EOF_MASK	GENMASK(15, REG_RX_OPTS_MASK_EOF_SHIFT)
+#define REG_RX_OPTS_MASK_SOF_SHIFT	16
+#define REG_RX_OPTS_MASK_SOF_MASK	GENMASK(31, REG_RX_OPTS_MASK_SOF_SHIFT)
+
+#define REG_OPTS_STEP			32
+#define REG_OPTS_E2E_EN			BIT(28)
+#define REG_OPTS_RAW			BIT(30)
+#define REG_OPTS_VALID			BIT(31)
 
 /*
  * three bitfields: tx, rx, rx overflow
@@ -86,6 +155,7 @@ struct ring_desc {
  */
 #define REG_RING_NOTIFY_BASE	0x37800
 #define RING_NOTIFY_REG_COUNT(nhi) ((31 + 3 * nhi->hop_count) / 32)
+#define REG_RING_NOTIFY_STEP	4
 
 /*
  * two bitfields: rx, tx
@@ -94,8 +164,47 @@ struct ring_desc {
  */
 #define REG_RING_INTERRUPT_BASE	0x38200
 #define RING_INTERRUPT_REG_COUNT(nhi) ((31 + 2 * nhi->hop_count) / 32)
+#define REG_RING_INT_TX_PROCESSED(ring_num)		BIT(ring_num)
+#define REG_RING_INT_RX_PROCESSED(ring_num, num_paths)	BIT((ring_num) + \
+							    (num_paths))
+#define RING_INT_DISABLE(base, val) iowrite32( \
+			ioread32((base) + REG_RING_INTERRUPT_BASE) & ~(val), \
+			(base) + REG_RING_INTERRUPT_BASE)
+#define RING_INT_ENABLE(base, val) iowrite32( \
+			ioread32((base) + REG_RING_INTERRUPT_BASE) | (val), \
+			(base) + REG_RING_INTERRUPT_BASE)
+#define RING_INT_DISABLE_TX(base, ring_num) \
+	RING_INT_DISABLE(base, REG_RING_INT_TX_PROCESSED(ring_num))
+#define RING_INT_DISABLE_RX(base, ring_num, num_paths) \
+	RING_INT_DISABLE(base, REG_RING_INT_RX_PROCESSED(ring_num, num_paths))
+#define RING_INT_ENABLE_TX(base, ring_num) \
+	RING_INT_ENABLE(base, REG_RING_INT_TX_PROCESSED(ring_num))
+#define RING_INT_ENABLE_RX(base, ring_num, num_paths) \
+	RING_INT_ENABLE(base, REG_RING_INT_RX_PROCESSED(ring_num, num_paths))
+#define RING_INT_DISABLE_TX_RX(base, ring_num, num_paths) \
+	RING_INT_DISABLE(base, REG_RING_INT_TX_PROCESSED(ring_num) | \
+			       REG_RING_INT_RX_PROCESSED(ring_num, num_paths))
+
+#define REG_RING_INTERRUPT_STEP	4
+
+#define REG_INT_THROTTLING_RATE	0x38c00
+#define REG_INT_THROTTLING_RATE_STEP	4
+#define NUM_INT_VECTORS			16
+
+#define REG_INT_VEC_ALLOC_BASE	0x38c40
+#define REG_INT_VEC_ALLOC_STEP		4
+#define REG_INT_VEC_ALLOC_FIELD_BITS	4
+#define REG_INT_VEC_ALLOC_FIELD_MASK	(BIT(REG_INT_VEC_ALLOC_FIELD_BITS) - 1)
+#define REG_INT_VEC_ALLOC_PER_REG	((BITS_PER_BYTE * sizeof(u32)) / \
+					 REG_INT_VEC_ALLOC_FIELD_BITS)
 
 /* The last 11 bits contain the number of hops supported by the NHI port. */
 #define REG_HOP_COUNT		0x39640
+#define REG_HOP_COUNT_TOTAL_PATHS_MASK	GENMASK(10, 0)
+
+#define REG_HOST_INTERFACE_RST	0x39858
+
+#define REG_DMA_MISC		0x39864
+#define REG_DMA_MISC_INT_AUTO_CLEAR	BIT(2)
 
 #endif
-- 
2.7.4

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

#1491930 — Re: [PATCH v7 5/8] thunderbolt: Networking state machine

FromGreg KH <gregkh@linuxfoundation.org>
Date2016-09-27 16:20 +0200
SubjectRe: [PATCH v7 5/8] thunderbolt: Networking state machine
Message-ID<sm1oJ-260-5@gated-at.bofh.it>
In reply to#1491910
On Tue, Sep 27, 2016 at 04:43:38PM +0300, Amir Levy wrote:
> This patch builds the peer to peer communication path.
> Communication is established by a negotiation process whereby messages are
> sent back and forth between the peers until a connection is established.
> This includes the Thunderbolt Network driver communication with the second
> peer via Intel Connection Manager(ICM) firmware.
>   +--------------------+            +--------------------+
>   |Host 1              |            |Host 2              |
>   |                    |            |                    |
>   |     +-----------+  |            |     +-----------+  |
>   |     |Thunderbolt|  |            |     |Thunderbolt|  |
>   |     |Networking |  |            |     |Networking |  |
>   |     |Driver     |  |            |     |Driver     |  |
>   |     +-----------+  |            |     +-----------+  |
>   |              ^     |            |              ^     |
>   |              |     |            |              |     |
>   | +------------+---+ |            | +------------+---+ |
>   | |Thunderbolt |   | |            | |Thunderbolt |   | |
>   | |Controller  v   | |            | |Controller  v   | |
>   | |         +---+  | |            | |         +---+  | |
>   | |         |ICM|<-+-+------------+-+-------->|ICM|  | |
>   | |         +---+  | |            | |         +---+  | |
>   | +----------------+ |            | +----------------+ |
>   +--------------------+            +--------------------+
> Note that this patch only establishes the link between the two hosts and
> not Network Packet handling - this is dealt with in the next patch.
> 
> Signed-off-by: Amir Levy <amir.jer.levy@intel.com>
> ---
>  drivers/thunderbolt/icm/Makefile  |   2 +-
>  drivers/thunderbolt/icm/icm_nhi.c | 303 ++++++++++++++-
>  drivers/thunderbolt/icm/net.c     | 793 ++++++++++++++++++++++++++++++++++++++
>  drivers/thunderbolt/icm/net.h     |  70 ++++
>  4 files changed, 1157 insertions(+), 11 deletions(-)
>  create mode 100644 drivers/thunderbolt/icm/net.c
> 
> diff --git a/drivers/thunderbolt/icm/Makefile b/drivers/thunderbolt/icm/Makefile
> index f0d0fbb..94a2797 100644
> --- a/drivers/thunderbolt/icm/Makefile
> +++ b/drivers/thunderbolt/icm/Makefile
> @@ -1,2 +1,2 @@
>  obj-${CONFIG_THUNDERBOLT_ICM} += thunderbolt-icm.o
> -thunderbolt-icm-objs := icm_nhi.o
> +thunderbolt-icm-objs := icm_nhi.o net.o
> diff --git a/drivers/thunderbolt/icm/icm_nhi.c b/drivers/thunderbolt/icm/icm_nhi.c
> index 984aa7c..578eb14 100644
> --- a/drivers/thunderbolt/icm/icm_nhi.c
> +++ b/drivers/thunderbolt/icm/icm_nhi.c
> @@ -74,6 +74,12 @@ static const struct nla_policy nhi_genl_policy[NHI_ATTR_MAX + 1] = {
>  					.len = TBT_ICM_RING_MAX_FRAME_SIZE },
>  	[NHI_ATTR_MSG_FROM_ICM]		= { .type = NLA_BINARY,
>  					.len = TBT_ICM_RING_MAX_FRAME_SIZE },
> +	[NHI_ATTR_LOCAL_ROUTE_STRING]	= {.len = sizeof(struct route_string)},
> +	[NHI_ATTR_LOCAL_UUID]		= { .len = sizeof(uuid_be) },
> +	[NHI_ATTR_REMOTE_UUID]		= { .len = sizeof(uuid_be) },
> +	[NHI_ATTR_LOCAL_DEPTH]		= { .type = NLA_U8, },
> +	[NHI_ATTR_ENABLE_FULL_E2E]	= { .type = NLA_FLAG, },
> +	[NHI_ATTR_MATCH_FRAME_ID]	= { .type = NLA_FLAG, },
>  };
>  
>  /* NHI genetlink family */
> @@ -522,6 +528,29 @@ int nhi_mailbox(struct tbt_nhi_ctxt *nhi_ctxt, u32 cmd, u32 data, bool deinit)
>  	return 0;
>  }
>  
> +static inline bool nhi_is_path_disconnected(u32 cmd, u8 num_ports)
> +{
> +	return (cmd >= DISCONNECT_PORT_A_INTER_DOMAIN_PATH &&
> +		cmd < (DISCONNECT_PORT_A_INTER_DOMAIN_PATH + num_ports));
> +}
> +
> +static int nhi_mailbox_disconn_path(struct tbt_nhi_ctxt *nhi_ctxt, u32 cmd)
> +	__releases(&controllers_list_mutex)
> +{
> +	struct port_net_dev *port;
> +	u32 port_num = cmd - DISCONNECT_PORT_A_INTER_DOMAIN_PATH;
> +
> +	port = &(nhi_ctxt->net_devices[port_num]);
> +	mutex_lock(&port->state_mutex);
> +
> +	mutex_unlock(&controllers_list_mutex);
> +	port->medium_sts = MEDIUM_READY_FOR_APPROVAL;
> +	if (port->net_dev)
> +		negotiation_events(port->net_dev, MEDIUM_DISCONNECTED);
> +	mutex_unlock(&port->state_mutex);
> +	return  0;
> +}
> +
>  static int nhi_mailbox_generic(struct tbt_nhi_ctxt *nhi_ctxt, u32 mb_cmd)
>  	__releases(&controllers_list_mutex)
>  {
> @@ -571,13 +600,94 @@ static int nhi_genl_mailbox(__always_unused struct sk_buff *u_skb,
>  		return -ERESTART;
>  
>  	nhi_ctxt = nhi_search_ctxt(*(u32 *)info->userhdr);
> -	if (nhi_ctxt && !nhi_ctxt->d0_exit)
> -		return nhi_mailbox_generic(nhi_ctxt, mb_cmd);
> +	if (nhi_ctxt && !nhi_ctxt->d0_exit) {
> +
> +		/* rwsem is released later by the below functions */
> +		if (nhi_is_path_disconnected(cmd, nhi_ctxt->num_ports))
> +			return nhi_mailbox_disconn_path(nhi_ctxt, cmd);
> +		else
> +			return nhi_mailbox_generic(nhi_ctxt, mb_cmd);
> +
> +	}
>  
>  	mutex_unlock(&controllers_list_mutex);
>  	return -ENODEV;
>  }
>  
> +static int nhi_genl_approve_networking(__always_unused struct sk_buff *u_skb,
> +				       struct genl_info *info)
> +{
> +	struct tbt_nhi_ctxt *nhi_ctxt;
> +	struct route_string *route_str;
> +	int res = -ENODEV;
> +	u8 port_num;
> +
> +	if (!info || !info->userhdr || !info->attrs ||
> +	    !info->attrs[NHI_ATTR_LOCAL_ROUTE_STRING] ||
> +	    !info->attrs[NHI_ATTR_LOCAL_UUID] ||
> +	    !info->attrs[NHI_ATTR_REMOTE_UUID] ||
> +	    !info->attrs[NHI_ATTR_LOCAL_DEPTH])
> +		return -EINVAL;
> +
> +	/*
> +	 * route_str is an unique topological address
> +	 * used for approving remote controller
> +	 */
> +	route_str = nla_data(info->attrs[NHI_ATTR_LOCAL_ROUTE_STRING]);
> +	/* extracts the port we're connected to */
> +	port_num = PORT_NUM_FROM_LINK(L0_PORT_NUM(route_str->lo));
> +
> +	if (mutex_lock_interruptible(&controllers_list_mutex))
> +		return -ERESTART;
> +
> +	nhi_ctxt = nhi_search_ctxt(*(u32 *)info->userhdr);
> +	if (nhi_ctxt && !nhi_ctxt->d0_exit) {
> +		struct port_net_dev *port;
> +
> +		if (port_num >= nhi_ctxt->num_ports) {
> +			res = -EINVAL;
> +			goto free_ctl_list;
> +		}
> +
> +		port = &(nhi_ctxt->net_devices[port_num]);
> +
> +		mutex_lock(&port->state_mutex);
> +		mutex_unlock(&controllers_list_mutex);
> +
> +		if (port->medium_sts != MEDIUM_READY_FOR_APPROVAL) {
> +			dev_info(&nhi_ctxt->pdev->dev,
> +				"%s: controller id %#x in state %u <> MEDIUM_READY_FOR_APPROVAL\n",
> +				__func__, nhi_ctxt->id, port->medium_sts);

Why is this needed?  Don't spam the kernel log for a normal device
operation please, only report serious errors when something goes wrong.

And if this is an error, why is it dev_info()?

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

#1491936 — Re: [PATCH v7 4/8] thunderbolt: Communication with the ICM (firmware)

FromGreg KH <gregkh@linuxfoundation.org>
Date2016-09-27 16:20 +0200
SubjectRe: [PATCH v7 4/8] thunderbolt: Communication with the ICM (firmware)
Message-ID<sm1oK-260-15@gated-at.bofh.it>
In reply to#1491910
On Tue, Sep 27, 2016 at 04:43:37PM +0300, Amir Levy wrote:
> This patch provides the communication protocol between the
> Intel Connection Manager(ICM) firmware that is operational in the
> Thunderbolt controller in non-Apple hardware.
> The ICM firmware-based controller is used for establishing and maintaining
> the Thunderbolt Networking connection - we need to be able to communicate
> with it.
> 
> Signed-off-by: Amir Levy <amir.jer.levy@intel.com>
> ---
>  drivers/thunderbolt/Makefile      |    1 +
>  drivers/thunderbolt/icm/Makefile  |    2 +
>  drivers/thunderbolt/icm/icm_nhi.c | 1324 +++++++++++++++++++++++++++++++++++++
>  drivers/thunderbolt/icm/icm_nhi.h |   92 +++
>  drivers/thunderbolt/icm/net.h     |  227 +++++++
>  5 files changed, 1646 insertions(+)
>  create mode 100644 drivers/thunderbolt/icm/Makefile
>  create mode 100644 drivers/thunderbolt/icm/icm_nhi.c
>  create mode 100644 drivers/thunderbolt/icm/icm_nhi.h
>  create mode 100644 drivers/thunderbolt/icm/net.h
> 
> diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
> index 7a85bd1..b6aa6a3 100644
> --- a/drivers/thunderbolt/Makefile
> +++ b/drivers/thunderbolt/Makefile
> @@ -1,3 +1,4 @@
>  obj-${CONFIG_THUNDERBOLT_APPLE} := thunderbolt.o
>  thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o tunnel_pci.o eeprom.o
>  
> +obj-${CONFIG_THUNDERBOLT_ICM} += icm/
> diff --git a/drivers/thunderbolt/icm/Makefile b/drivers/thunderbolt/icm/Makefile
> new file mode 100644
> index 0000000..f0d0fbb
> --- /dev/null
> +++ b/drivers/thunderbolt/icm/Makefile
> @@ -0,0 +1,2 @@
> +obj-${CONFIG_THUNDERBOLT_ICM} += thunderbolt-icm.o
> +thunderbolt-icm-objs := icm_nhi.o
> diff --git a/drivers/thunderbolt/icm/icm_nhi.c b/drivers/thunderbolt/icm/icm_nhi.c
> new file mode 100644
> index 0000000..984aa7c
> --- /dev/null
> +++ b/drivers/thunderbolt/icm/icm_nhi.c
> @@ -0,0 +1,1324 @@
> +/*******************************************************************************
> + *
> + * Intel Thunderbolt(TM) driver
> + * Copyright(c) 2014 - 2016 Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program.  If not, see <http://www.gnu.org/licenses/>.

Why is this sentence needed?

> + *
> + * The full GNU General Public License is included in this distribution in
> + * the file called "COPYING".

Why is this sentence needed?

> + *
> + * Contact Information:
> + * Intel Thunderbolt Mailing List <thunderbolt-software@lists.01.org>

Shouldn't this just be in the MAINTAINERS file?

> + * Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497

Unless you are going to track the address of Intel for the next 40+
years, don't put it in a source comment.  Please just drop it.

> + *
> + ******************************************************************************/
> +
> +#include <linux/printk.h>
> +#include <linux/crc32.h>
> +#include <linux/delay.h>
> +#include <linux/dmi.h>
> +#include "icm_nhi.h"
> +#include "net.h"
> +
> +#define NHI_GENL_VERSION 1
> +#define NHI_GENL_NAME "thunderbolt"
> +
> +#define DEVICE_DATA(num_ports, dma_port, nvm_ver_offset, nvm_auth_on_boot,\
> +		    support_full_e2e) \
> +	((num_ports) | ((dma_port) << 4) | ((nvm_ver_offset) << 10) | \
> +	 ((nvm_auth_on_boot) << 22) | ((support_full_e2e) << 23))
> +#define DEVICE_DATA_NUM_PORTS(device_data) ((device_data) & 0xf)
> +#define DEVICE_DATA_DMA_PORT(device_data) (((device_data) >> 4) & 0x3f)
> +#define DEVICE_DATA_NVM_VER_OFFSET(device_data) (((device_data) >> 10) & 0xfff)
> +#define DEVICE_DATA_NVM_AUTH_ON_BOOT(device_data) (((device_data) >> 22) & 0x1)
> +#define DEVICE_DATA_SUPPORT_FULL_E2E(device_data) (((device_data) >> 23) & 0x1)
> +
> +#define USEC_TO_256_NSECS(usec) DIV_ROUND_UP((usec) * NSEC_PER_USEC, 256)
> +
> +/* NHI genetlink commands */
> +enum {
> +	NHI_CMD_UNSPEC,
> +	NHI_CMD_SUBSCRIBE,
> +	NHI_CMD_UNSUBSCRIBE,
> +	NHI_CMD_QUERY_INFORMATION,
> +	NHI_CMD_MSG_TO_ICM,
> +	NHI_CMD_MSG_FROM_ICM,
> +	NHI_CMD_MAILBOX,
> +	NHI_CMD_APPROVE_TBT_NETWORKING,
> +	NHI_CMD_ICM_IN_SAFE_MODE,
> +	__NHI_CMD_MAX,
> +};
> +#define NHI_CMD_MAX (__NHI_CMD_MAX - 1)
> +
> +/* NHI genetlink policy */
> +static const struct nla_policy nhi_genl_policy[NHI_ATTR_MAX + 1] = {
> +	[NHI_ATTR_DRV_VERSION]		= { .type = NLA_NUL_STRING, },
> +	[NHI_ATTR_NVM_VER_OFFSET]	= { .type = NLA_U16, },
> +	[NHI_ATTR_NUM_PORTS]		= { .type = NLA_U8, },
> +	[NHI_ATTR_DMA_PORT]		= { .type = NLA_U8, },
> +	[NHI_ATTR_SUPPORT_FULL_E2E]	= { .type = NLA_FLAG, },
> +	[NHI_ATTR_MAILBOX_CMD]		= { .type = NLA_U32, },
> +	[NHI_ATTR_PDF]			= { .type = NLA_U32, },
> +	[NHI_ATTR_MSG_TO_ICM]		= { .type = NLA_BINARY,
> +					.len = TBT_ICM_RING_MAX_FRAME_SIZE },
> +	[NHI_ATTR_MSG_FROM_ICM]		= { .type = NLA_BINARY,
> +					.len = TBT_ICM_RING_MAX_FRAME_SIZE },
> +};
> +
> +/* NHI genetlink family */
> +static struct genl_family nhi_genl_family = {
> +	.id		= GENL_ID_GENERATE,
> +	.hdrsize	= FIELD_SIZEOF(struct tbt_nhi_ctxt, id),
> +	.name		= NHI_GENL_NAME,
> +	.version	= NHI_GENL_VERSION,
> +	.maxattr	= NHI_ATTR_MAX,
> +};
> +
> +static LIST_HEAD(controllers_list);
> +static DEFINE_MUTEX(controllers_list_mutex);
> +static atomic_t subscribers = ATOMIC_INIT(0);
> +static u32 portid;

This is really odd, why have a global single portid?

> +
> +static bool nhi_nvm_authenticated(struct tbt_nhi_ctxt *nhi_ctxt)
> +{
> +	enum icm_operation_mode op_mode;
> +	u32 *msg_head, port_id, reg;
> +	struct sk_buff *skb;
> +	int i;
> +
> +	if (!nhi_ctxt->nvm_auth_on_boot)
> +		return true;
> +
> +	/*
> +	 * The check for NVM authentication can take time for iCM,
> +	 * especially in low power configuration.
> +	 */
> +	for (i = 0; i < 5; i++) {
> +		u32 status = ioread32(nhi_ctxt->iobase + REG_FW_STS);
> +
> +		if (status & REG_FW_STS_NVM_AUTH_DONE)
> +			break;
> +
> +		msleep(30);
> +	}
> +	/*
> +	 * The check for authentication is done after checking if iCM
> +	 * is present so it shouldn't reach the max tries (=5).
> +	 * Anyway, the check for full functionality below covers the error case.
> +	 */
> +	reg = ioread32(nhi_ctxt->iobase + REG_OUTMAIL_CMD);
> +	op_mode = (reg & REG_OUTMAIL_CMD_OP_MODE_MASK) >>
> +		  REG_OUTMAIL_CMD_OP_MODE_SHIFT;
> +	if (op_mode == FULL_FUNCTIONALITY)
> +		return true;
> +
> +	dev_warn(&nhi_ctxt->pdev->dev, "controller id %#x is in operation mode %#x status %#lx\n",
> +		 nhi_ctxt->id, op_mode,
> +		 (reg & REG_OUTMAIL_CMD_STS_MASK)>>REG_OUTMAIL_CMD_STS_SHIFT);
> +
> +	skb = genlmsg_new(NLMSG_ALIGN(nhi_genl_family.hdrsize), GFP_KERNEL);
> +	if (!skb) {
> +		dev_err(&nhi_ctxt->pdev->dev, "genlmsg_new failed: not enough memory to send controller operational mode\n");
> +		return false;
> +	}
> +
> +	/* keeping port_id into a local variable for next use */
> +	port_id = portid;
> +	msg_head = genlmsg_put(skb, port_id, 0, &nhi_genl_family, 0,
> +			       NHI_CMD_ICM_IN_SAFE_MODE);
> +	if (!msg_head) {
> +		nlmsg_free(skb);
> +		dev_err(&nhi_ctxt->pdev->dev, "genlmsg_put failed: not enough memory to send controller operational mode\n");
> +		return false;
> +	}
> +
> +	*msg_head = nhi_ctxt->id;
> +
> +	genlmsg_end(skb, msg_head);
> +
> +	genlmsg_unicast(&init_net, skb, port_id);
> +
> +	return false;
> +}
> +
> +int nhi_send_message(struct tbt_nhi_ctxt *nhi_ctxt, enum pdf_value pdf,
> +		     u32 msg_len, const void *msg, bool ignore_icm_resp)
> +{
> +	u32 prod_cons, prod, cons, attr;
> +	struct tbt_icm_ring_shared_memory *shared_mem;
> +	void __iomem *reg = TBT_RING_CONS_PROD_REG(nhi_ctxt->iobase,
> +						   REG_TX_RING_BASE,
> +						   TBT_ICM_RING_NUM);
> +
> +	dev_dbg(&nhi_ctxt->pdev->dev,
> +		"send msg: controller id %#x pdf %u cmd %hhu msg len %u\n",
> +		nhi_ctxt->id, pdf, ((u8 *)msg)[3], msg_len);

Don't put trace debug calls in your code, either use a real tracepoint,
or just use ftrace.  Delete these, they aren't needed once you want to
submit the code for merging as it should not be needed.

You do this in a lot of other places as well, please fix all of them.

> +
> +	if (nhi_ctxt->d0_exit) {
> +		dev_notice(&nhi_ctxt->pdev->dev,
> +			   "controller id %#x is exiting D0\n",
> +			   nhi_ctxt->id);

What can a user do about this?


> +		return -ENODEV;
> +	}
> +
> +	prod_cons = ioread32(reg);
> +	prod = TBT_REG_RING_PROD_EXTRACT(prod_cons);
> +	cons = TBT_REG_RING_CONS_EXTRACT(prod_cons);
> +	if (prod >= TBT_ICM_RING_NUM_TX_BUFS) {
> +		dev_warn(&nhi_ctxt->pdev->dev,
> +			 "controller id %#x producer %u out of range\n",
> +			 nhi_ctxt->id, prod);

What can a user do about this?

> +		return -ENODEV;
> +	}
> +	if (TBT_TX_RING_FULL(prod, cons, TBT_ICM_RING_NUM_TX_BUFS)) {
> +		dev_err(&nhi_ctxt->pdev->dev,
> +			"controller id %#x TX ring full\n",
> +			nhi_ctxt->id);

What can a user do about this?

> +		return -ENOSPC;
> +	}
> +
> +	attr = (msg_len << DESC_ATTR_LEN_SHIFT) & DESC_ATTR_LEN_MASK;
> +	attr |= (pdf << DESC_ATTR_EOF_SHIFT) & DESC_ATTR_EOF_MASK;
> +
> +	shared_mem = nhi_ctxt->icm_ring_shared_mem;
> +	shared_mem->tx_buf_desc[prod].attributes = cpu_to_le32(attr);
> +
> +	memcpy(shared_mem->tx_buf[prod], msg, msg_len);

No zero-copy, sad :(

> +
> +	prod_cons &= ~REG_RING_PROD_MASK;
> +	prod_cons |= (((prod + 1) % TBT_ICM_RING_NUM_TX_BUFS) <<
> +		      REG_RING_PROD_SHIFT) & REG_RING_PROD_MASK;
> +
> +	if (likely(!nhi_ctxt->wait_for_icm_resp))
> +		nhi_ctxt->wait_for_icm_resp = true;

Can you measure the speed difference with likely() here?  If so, great,
if not, drop it as the cpu can guess this better than you or I can.

> +	else
> +		dev_dbg(&nhi_ctxt->pdev->dev,
> +			"controller id %#x wait_for_icm_resp should have been cleared\n",
> +			nhi_ctxt->id);

Huh?  So this isn't a real issue?

> +
> +	nhi_ctxt->ignore_icm_resp = ignore_icm_resp;
> +
> +	iowrite32(prod_cons, reg);
> +
> +	return 0;
> +}

thanks,

greg k-h

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

#1491937 — Re: [PATCH v7 5/8] thunderbolt: Networking state machine

FromGreg KH <gregkh@linuxfoundation.org>
Date2016-09-27 16:20 +0200
SubjectRe: [PATCH v7 5/8] thunderbolt: Networking state machine
Message-ID<sm1oK-260-19@gated-at.bofh.it>
In reply to#1491910
On Tue, Sep 27, 2016 at 04:43:38PM +0300, Amir Levy wrote:
> This patch builds the peer to peer communication path.
> Communication is established by a negotiation process whereby messages are
> sent back and forth between the peers until a connection is established.
> This includes the Thunderbolt Network driver communication with the second
> peer via Intel Connection Manager(ICM) firmware.
>   +--------------------+            +--------------------+
>   |Host 1              |            |Host 2              |
>   |                    |            |                    |
>   |     +-----------+  |            |     +-----------+  |
>   |     |Thunderbolt|  |            |     |Thunderbolt|  |
>   |     |Networking |  |            |     |Networking |  |
>   |     |Driver     |  |            |     |Driver     |  |
>   |     +-----------+  |            |     +-----------+  |
>   |              ^     |            |              ^     |
>   |              |     |            |              |     |
>   | +------------+---+ |            | +------------+---+ |
>   | |Thunderbolt |   | |            | |Thunderbolt |   | |
>   | |Controller  v   | |            | |Controller  v   | |
>   | |         +---+  | |            | |         +---+  | |
>   | |         |ICM|<-+-+------------+-+-------->|ICM|  | |
>   | |         +---+  | |            | |         +---+  | |
>   | +----------------+ |            | +----------------+ |
>   +--------------------+            +--------------------+
> Note that this patch only establishes the link between the two hosts and
> not Network Packet handling - this is dealt with in the next patch.
> 
> Signed-off-by: Amir Levy <amir.jer.levy@intel.com>
> ---
>  drivers/thunderbolt/icm/Makefile  |   2 +-
>  drivers/thunderbolt/icm/icm_nhi.c | 303 ++++++++++++++-
>  drivers/thunderbolt/icm/net.c     | 793 ++++++++++++++++++++++++++++++++++++++
>  drivers/thunderbolt/icm/net.h     |  70 ++++
>  4 files changed, 1157 insertions(+), 11 deletions(-)
>  create mode 100644 drivers/thunderbolt/icm/net.c
> 
> diff --git a/drivers/thunderbolt/icm/Makefile b/drivers/thunderbolt/icm/Makefile
> index f0d0fbb..94a2797 100644
> --- a/drivers/thunderbolt/icm/Makefile
> +++ b/drivers/thunderbolt/icm/Makefile
> @@ -1,2 +1,2 @@
>  obj-${CONFIG_THUNDERBOLT_ICM} += thunderbolt-icm.o
> -thunderbolt-icm-objs := icm_nhi.o
> +thunderbolt-icm-objs := icm_nhi.o net.o
> diff --git a/drivers/thunderbolt/icm/icm_nhi.c b/drivers/thunderbolt/icm/icm_nhi.c
> index 984aa7c..578eb14 100644
> --- a/drivers/thunderbolt/icm/icm_nhi.c
> +++ b/drivers/thunderbolt/icm/icm_nhi.c
> @@ -74,6 +74,12 @@ static const struct nla_policy nhi_genl_policy[NHI_ATTR_MAX + 1] = {
>  					.len = TBT_ICM_RING_MAX_FRAME_SIZE },
>  	[NHI_ATTR_MSG_FROM_ICM]		= { .type = NLA_BINARY,
>  					.len = TBT_ICM_RING_MAX_FRAME_SIZE },
> +	[NHI_ATTR_LOCAL_ROUTE_STRING]	= {.len = sizeof(struct route_string)},
> +	[NHI_ATTR_LOCAL_UUID]		= { .len = sizeof(uuid_be) },
> +	[NHI_ATTR_REMOTE_UUID]		= { .len = sizeof(uuid_be) },

Be consistent in your choice of whitespace please.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web