[Intel-wired-lan] [next-queue PATCH v3 0/2] igc: Add initial TSN qdiscs offloading

[Intel-wired-lan] [next-queue PATCH v3 0/2] igc: Add initial TSN qdiscs offloading

[Vinicius Costa Gomes]

Hi,

Changes from v2:
  - Fixed review comments (Andre Guedes);
  - Just a note, about the comment about ETF + taprio, when both are
    in offload mode, since commit bfabd41da341 ("taprio: Fix dropping
    packets when using taprio + ETF offloading") it should work fine.

Changes from v1:
  - Fixed review comments (Andre Guedes);

Original cover letter:

This adds support for offloading taprio (which handles what was
formerly known as Qbv) and ETF (using the LaunchTime feature) to i225.

To try and keep things organized, a new file is added: igc_tsn.c. The
idea is that the more TSN specific code will live here, in the future
this will include frame-preemption support, and CBS (formerly Qav)
support.

The current support has a few limitations:
  - because the BASET registers can only be written once per reset,
    when removing taprio or ETF, we force the NIC to reset;
  - In part because of the above limitation, we don't support changing
    schedules;
  - The way the registers for the schedule are organized, each queue
    can only be opened and closed once per cycle, so some schedules
    provided by the user are going to be rejected by the driver;

Future improvements:
  - When configuring a Qbv cycle, when we detect that a queue would
    stay open for two intervals, we could want to merge them;
  - More testing with taprio and ETF together, when ETF is installed
    under taprio (when using them separately, it works fine);


Vinicius Costa Gomes (2):
  igc: Add support for taprio offloading
  igc: Add support for ETF offloading

 drivers/net/ethernet/intel/igc/Makefile      |   2 +-
 drivers/net/ethernet/intel/igc/igc.h         |   7 +
 drivers/net/ethernet/intel/igc/igc_defines.h |  13 ++
 drivers/net/ethernet/intel/igc/igc_main.c    | 183 ++++++++++++++++++-
 drivers/net/ethernet/intel/igc/igc_regs.h    |  12 ++
 drivers/net/ethernet/intel/igc/igc_tsn.c     | 157 ++++++++++++++++
 drivers/net/ethernet/intel/igc/igc_tsn.h     |   9 +
 7 files changed, 379 insertions(+), 4 deletions(-)
 create mode 100644 drivers/net/ethernet/intel/igc/igc_tsn.c
 create mode 100644 drivers/net/ethernet/intel/igc/igc_tsn.h

-- 
2.25.0

[Intel-wired-lan] [next-queue PATCH v3 1/2] igc: Add support for taprio offloading

[Vinicius Costa Gomes]

Adds support for translating taprio schedules into i225 cycles. This
will allow schedules to run in the hardware, making the schedules
enforcement more precise and saving CPU time.

Right now, the only simple schedules are allowed, complex schedules are
rejected. "simple" in this context are schedules that each HW queue is
opened and closed only once in each cycle.

Changing schedules is still not supported as well.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igc/Makefile      |   2 +-
 drivers/net/ethernet/intel/igc/igc.h         |   7 +
 drivers/net/ethernet/intel/igc/igc_defines.h |  12 ++
 drivers/net/ethernet/intel/igc/igc_main.c    | 113 +++++++++++++++
 drivers/net/ethernet/intel/igc/igc_regs.h    |  12 ++
 drivers/net/ethernet/intel/igc/igc_tsn.c     | 140 +++++++++++++++++++
 drivers/net/ethernet/intel/igc/igc_tsn.h     |   9 ++
 7 files changed, 294 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/intel/igc/igc_tsn.c
 create mode 100644 drivers/net/ethernet/intel/igc/igc_tsn.h

diff --git a/drivers/net/ethernet/intel/igc/Makefile b/drivers/net/ethernet/intel/igc/Makefile
index e3c164c12e10..3652f211f351 100644
--- a/drivers/net/ethernet/intel/igc/Makefile
+++ b/drivers/net/ethernet/intel/igc/Makefile
@@ -8,4 +8,4 @@
 obj-$(CONFIG_IGC) += igc.o
 
 igc-objs := igc_main.o igc_mac.o igc_i225.o igc_base.o igc_nvm.o igc_phy.o \
-igc_ethtool.o igc_ptp.o igc_dump.o
+igc_ethtool.o igc_ptp.o igc_dump.o igc_tsn.o
diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index 0014828eec46..4c40dc8f276c 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -70,6 +70,7 @@ extern char igc_driver_version[];
 #define IGC_FLAG_HAS_MSIX		BIT(13)
 #define IGC_FLAG_VLAN_PROMISC		BIT(15)
 #define IGC_FLAG_RX_LEGACY		BIT(16)
+#define IGC_FLAG_TSN_QBV_ENABLED	BIT(17)
 
 #define IGC_FLAG_RSS_FIELD_IPV4_UDP	BIT(6)
 #define IGC_FLAG_RSS_FIELD_IPV6_UDP	BIT(7)
@@ -287,6 +288,9 @@ struct igc_ring {
 	u8 reg_idx;                     /* physical index of the ring */
 	bool launchtime_enable;		/* true if LaunchTime is enabled */
 
+	u32 start_time;
+	u32 end_time;
+
 	/* everything past this point are written often */
 	u16 next_to_clean;
 	u16 next_to_use;
@@ -421,6 +425,9 @@ struct igc_adapter {
 	u32 max_frame_size;
 	u32 min_frame_size;
 
+	ktime_t base_time;
+	ktime_t cycle_time;
+
 	/* OS defined structs */
 	struct pci_dev *pdev;
 	/* lock for statistics */
diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h
index 044c0f8d4c16..3077d0a69b04 100644
--- a/drivers/net/ethernet/intel/igc/igc_defines.h
+++ b/drivers/net/ethernet/intel/igc/igc_defines.h
@@ -377,6 +377,11 @@
 #define I225_TXPBSIZE_DEFAULT	0x04000014 /* TXPBSIZE default */
 #define IGC_RXPBS_CFG_TS_EN	0x80000000 /* Timestamp in Rx buffer */
 
+#define IGC_TXPBSIZE_TSN	0x04145145 /* 5k bytes buffer for each queue */
+
+#define IGC_DTXMXPKTSZ_TSN	0x19 /* 1600 bytes of max TX DMA packet size */
+#define IGC_DTXMXPKTSZ_DEFAULT	0x98 /* 9728-byte Jumbo frames */
+
 /* Time Sync Interrupt Causes */
 #define IGC_TSICR_SYS_WRAP	BIT(0) /* SYSTIM Wrap around. */
 #define IGC_TSICR_TXTS		BIT(1) /* Transmit Timestamp. */
@@ -431,6 +436,13 @@
 #define IGC_TSYNCTXCTL_START_SYNC		0x80000000  /* initiate sync */
 #define IGC_TSYNCTXCTL_TXSYNSIG			0x00000020  /* Sample TX tstamp in PHY sop */
 
+/* Transmit Scheduling */
+#define IGC_TQAVCTRL_TRANSMIT_MODE_TSN	0x00000001
+#define IGC_TQAVCTRL_ENHANCED_QAV	0x00000008
+
+#define IGC_TXQCTL_STRICT_CYCLE		0x00000002
+#define IGC_TXQCTL_STRICT_END		0x00000004
+
 /* Receive Checksum Control */
 #define IGC_RXCSUM_CRCOFL	0x00000800   /* CRC32 offload enable */
 #define IGC_RXCSUM_PCSD		0x00002000   /* packet checksum disabled */
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 3407af11dff8..5fb52768de18 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -9,11 +9,13 @@
 #include <linux/udp.h>
 #include <linux/ip.h>
 #include <linux/pm_runtime.h>
+#include <net/pkt_sched.h>
 
 #include <net/ipv6.h>
 
 #include "igc.h"
 #include "igc_hw.h"
+#include "igc_tsn.h"
 
 #define DRV_VERSION	"0.0.1-k"
 #define DRV_SUMMARY	"Intel(R) 2.5G Ethernet Linux Driver"
@@ -106,6 +108,9 @@ void igc_reset(struct igc_adapter *adapter)
 	/* Re-enable PTP, where applicable. */
 	igc_ptp_reset(adapter);
 
+	/* Re-enable TSN offloading, where applicable. */
+	igc_tsn_offload_apply(adapter);
+
 	igc_get_phy_info(hw);
 }
 
@@ -4492,6 +4497,113 @@ static int igc_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 	}
 }
 
+static bool validate_schedule(const struct tc_taprio_qopt_offload *qopt)
+{
+	int queue_uses[IGC_MAX_TX_QUEUES] = { };
+	size_t n;
+
+	if (qopt->cycle_time_extension)
+		return false;
+
+	for (n = 0; n < qopt->num_entries; n++) {
+		const struct tc_taprio_sched_entry *e;
+		int i;
+
+		e = &qopt->entries[n];
+
+		/* i225 only supports "global" frame preemption
+		 * settings.
+		 */
+		if (e->command != TC_TAPRIO_CMD_SET_GATES)
+			return false;
+
+		for (i = 0; i < IGC_MAX_TX_QUEUES; i++) {
+			if (e->gate_mask & BIT(i))
+				queue_uses[i]++;
+
+			if (queue_uses[i] > 1)
+				return false;
+		}
+	}
+
+	return true;
+}
+
+static int igc_save_qbv_schedule(struct igc_adapter *adapter,
+				 struct tc_taprio_qopt_offload *qopt)
+{
+	u32 start_time = 0, end_time = 0;
+	size_t n;
+
+	if (!qopt->enable) {
+		adapter->base_time = 0;
+		return 0;
+	}
+
+	if (adapter->base_time)
+		return -EALREADY;
+
+	if (!validate_schedule(qopt))
+		return -EINVAL;
+
+	adapter->cycle_time = qopt->cycle_time;
+	adapter->base_time = qopt->base_time;
+
+	/* FIXME: be a little smarter about cases when the gate for a
+	 * queue stays open for more than one entry.
+	 */
+	for (n = 0; n < qopt->num_entries; n++) {
+		struct tc_taprio_sched_entry *e = &qopt->entries[n];
+		int i;
+
+		end_time += e->interval;
+
+		for (i = 0; i < IGC_MAX_TX_QUEUES; i++) {
+			struct igc_ring *ring = adapter->tx_ring[i];
+
+			if (!(e->gate_mask & BIT(i)))
+				continue;
+
+			ring->start_time = start_time;
+			ring->end_time = end_time;
+		}
+
+		start_time += e->interval;
+	}
+
+	return 0;
+}
+
+static int igc_tsn_enable_qbv_scheduling(struct igc_adapter *adapter,
+					 struct tc_taprio_qopt_offload *qopt)
+{
+	struct igc_hw *hw = &adapter->hw;
+	int err;
+
+	if (hw->mac.type != igc_i225)
+		return -EOPNOTSUPP;
+
+	err = igc_save_qbv_schedule(adapter, qopt);
+	if (err)
+		return err;
+
+	return igc_tsn_offload_apply(adapter);
+}
+
+static int igc_setup_tc(struct net_device *dev, enum tc_setup_type type,
+			void *type_data)
+{
+	struct igc_adapter *adapter = netdev_priv(dev);
+
+	switch (type) {
+	case TC_SETUP_QDISC_TAPRIO:
+		return igc_tsn_enable_qbv_scheduling(adapter, type_data);
+
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static const struct net_device_ops igc_netdev_ops = {
 	.ndo_open		= igc_open,
 	.ndo_stop		= igc_close,
@@ -4504,6 +4616,7 @@ static const struct net_device_ops igc_netdev_ops = {
 	.ndo_set_features	= igc_set_features,
 	.ndo_features_check	= igc_features_check,
 	.ndo_do_ioctl		= igc_ioctl,
+	.ndo_setup_tc           = igc_setup_tc,
 };
 
 /* PCIe configuration access */
diff --git a/drivers/net/ethernet/intel/igc/igc_regs.h b/drivers/net/ethernet/intel/igc/igc_regs.h
index d4af53a80f11..96dee3c1a5f7 100644
--- a/drivers/net/ethernet/intel/igc/igc_regs.h
+++ b/drivers/net/ethernet/intel/igc/igc_regs.h
@@ -231,6 +231,18 @@
 
 #define IGC_RXPBS	0x02404  /* Rx Packet Buffer Size - RW */
 
+/* Transmit Scheduling Registers */
+#define IGC_TQAVCTRL                   0x3570
+#define IGC_TXQCTL(_n)                 (0x3344 + 0x4 * (_n))
+#define IGC_BASET_L                    0x3314
+#define IGC_BASET_H                    0x3318
+#define IGC_QBVCYCLET                  0x331C
+#define IGC_QBVCYCLET_S                0x3320
+
+#define IGC_STQT(_n)                   (0x3324 + 0x4 * (_n))
+#define IGC_ENDQT(_n)                  (0x3334 + 0x4 * (_n))
+#define IGC_DTXMXPKTSZ                 0x355C
+
 /* System Time Registers */
 #define IGC_SYSTIML	0x0B600  /* System time register Low - RO */
 #define IGC_SYSTIMH	0x0B604  /* System time register High - RO */
diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
new file mode 100644
index 000000000000..257fe970afe8
--- /dev/null
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c)  2019 Intel Corporation */
+
+#include "igc.h"
+#include "igc_tsn.h"
+
+/* Returns the TSN specific registers to their default values after
+ * TSN offloading is disabled.
+ */
+static int igc_tsn_disable_offload(struct igc_adapter *adapter)
+{
+	struct igc_hw *hw = &adapter->hw;
+	u32 tqavctrl;
+	int i;
+
+	if (!(adapter->flags & IGC_FLAG_TSN_QBV_ENABLED))
+		return 0;
+
+	adapter->cycle_time = 0;
+
+	wr32(IGC_TXPBS, I225_TXPBSIZE_DEFAULT);
+	wr32(IGC_DTXMXPKTSZ, IGC_DTXMXPKTSZ_DEFAULT);
+
+	tqavctrl = rd32(IGC_TQAVCTRL);
+	tqavctrl &= ~(IGC_TQAVCTRL_TRANSMIT_MODE_TSN |
+		      IGC_TQAVCTRL_ENHANCED_QAV);
+	wr32(IGC_TQAVCTRL, tqavctrl);
+
+	for (i = 0; i < adapter->num_tx_queues; i++) {
+		struct igc_ring *ring = adapter->tx_ring[i];
+
+		ring->start_time = 0;
+		ring->end_time = 0;
+		ring->launchtime_enable = false;
+
+		wr32(IGC_TXQCTL(i), 0);
+		wr32(IGC_STQT(i), 0);
+		wr32(IGC_ENDQT(i), NSEC_PER_SEC);
+	}
+
+	wr32(IGC_QBVCYCLET_S, NSEC_PER_SEC);
+	wr32(IGC_QBVCYCLET, NSEC_PER_SEC);
+
+	adapter->flags &= ~IGC_FLAG_TSN_QBV_ENABLED;
+
+	return 0;
+}
+
+static int igc_tsn_enable_offload(struct igc_adapter *adapter)
+{
+	struct igc_hw *hw = &adapter->hw;
+	u32 tqavctrl, baset_l, baset_h;
+	u32 sec, nsec, cycle;
+	ktime_t base_time, systim;
+	int i;
+
+	if (adapter->flags & IGC_FLAG_TSN_QBV_ENABLED)
+		return 0;
+
+	cycle = adapter->cycle_time;
+	base_time = adapter->base_time;
+
+	wr32(IGC_TSAUXC, 0);
+	wr32(IGC_DTXMXPKTSZ, IGC_DTXMXPKTSZ_TSN);
+	wr32(IGC_TXPBS, IGC_TXPBSIZE_TSN);
+
+	tqavctrl = rd32(IGC_TQAVCTRL);
+	tqavctrl |= IGC_TQAVCTRL_TRANSMIT_MODE_TSN | IGC_TQAVCTRL_ENHANCED_QAV;
+	wr32(IGC_TQAVCTRL, tqavctrl);
+
+	wr32(IGC_QBVCYCLET_S, cycle);
+	wr32(IGC_QBVCYCLET, cycle);
+
+	for (i = 0; i < adapter->num_tx_queues; i++) {
+		struct igc_ring *ring = adapter->tx_ring[i];
+		u32 txqctl = 0;
+
+		wr32(IGC_STQT(i), ring->start_time);
+		wr32(IGC_ENDQT(i), ring->end_time);
+
+		if (adapter->base_time) {
+			/* If we have a base_time we are in "taprio"
+			 * mode and we need to be strict about the
+			 * cycles: only transmit a packet if it can be
+			 * completed during that cycle.
+			 */
+			txqctl |= IGC_TXQCTL_STRICT_CYCLE |
+				IGC_TXQCTL_STRICT_END;
+		}
+
+		wr32(IGC_TXQCTL(i), txqctl);
+	}
+
+	nsec = rd32(IGC_SYSTIML);
+	sec = rd32(IGC_SYSTIMH);
+
+	systim = ktime_set(sec, nsec);
+
+	if (ktime_compare(systim, base_time) > 0) {
+		s64 n;
+
+		n = div64_s64(ktime_sub_ns(systim, base_time), cycle);
+		base_time = ktime_add_ns(base_time, (n + 1) * cycle);
+	}
+
+	baset_h = div_s64_rem(base_time, NSEC_PER_SEC, &baset_l);
+
+	wr32(IGC_BASET_H, baset_h);
+	wr32(IGC_BASET_L, baset_l);
+
+	adapter->flags |= IGC_FLAG_TSN_QBV_ENABLED;
+
+	return 0;
+}
+
+int igc_tsn_offload_apply(struct igc_adapter *adapter)
+{
+	bool is_any_enabled = adapter->base_time;
+
+	if (!(adapter->flags & IGC_FLAG_TSN_QBV_ENABLED) && !is_any_enabled)
+		return 0;
+
+	if (!is_any_enabled) {
+		int err = igc_tsn_disable_offload(adapter);
+
+		if (err < 0)
+			return err;
+
+		/* The BASET registers aren't cleared when writing
+		 * into them, force a reset if the interface is
+		 * running.
+		 */
+		if (netif_running(adapter->netdev))
+			schedule_work(&adapter->reset_task);
+
+		return 0;
+	}
+
+	return igc_tsn_enable_offload(adapter);
+}
diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.h b/drivers/net/ethernet/intel/igc/igc_tsn.h
new file mode 100644
index 000000000000..f76bc86ddccd
--- /dev/null
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c)  2020 Intel Corporation */
+
+#ifndef _IGC_TSN_H_
+#define _IGC_TSN_H_
+
+int igc_tsn_offload_apply(struct igc_adapter *adapter);
+
+#endif /* _IGC_BASE_H */
-- 
2.25.0

[Intel-wired-lan] [next-queue PATCH v3 2/2] igc: Add support for ETF offloading

[Vinicius Costa Gomes]

This adds support for ETF offloading for the i225 controller.

For i225, the LaunchTime feature is almost a subset of the Qbv
feature. The main change from the i210 is that the launchtime of each
packet is specified as an offset applied to the BASET register. BASET
is automatically incremented each cycle.

For i225, the approach chosen is to re-use most of the setup used for
taprio offloading. With a few changes:

 - The more or less obvious one is that when ETF is enabled, we should
 set add the expected launchtime to the (advanced) transmit
 descriptor;

 - The less obvious, is that when taprio offloading is not enabled, we
 add a dummy schedule (all queues are open all the time, with a cycle
 time of 1 second).

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_defines.h |  1 +
 drivers/net/ethernet/intel/igc/igc_main.c    | 70 +++++++++++++++++++-
 drivers/net/ethernet/intel/igc/igc_tsn.c     | 19 +++++-
 3 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h
index 3077d0a69b04..0746fa42ff3f 100644
--- a/drivers/net/ethernet/intel/igc/igc_defines.h
+++ b/drivers/net/ethernet/intel/igc/igc_defines.h
@@ -440,6 +440,7 @@
 #define IGC_TQAVCTRL_TRANSMIT_MODE_TSN	0x00000001
 #define IGC_TQAVCTRL_ENHANCED_QAV	0x00000008
 
+#define IGC_TXQCTL_QUEUE_MODE_LAUNCHT	0x00000001
 #define IGC_TXQCTL_STRICT_CYCLE		0x00000002
 #define IGC_TXQCTL_STRICT_END		0x00000004
 
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 5fb52768de18..55ab6077455d 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -869,6 +869,23 @@ static int igc_write_mc_addr_list(struct net_device *netdev)
 	return netdev_mc_count(netdev);
 }
 
+static __le32 igc_tx_launchtime(struct igc_adapter *adapter, ktime_t txtime)
+{
+	ktime_t cycle_time = adapter->cycle_time;
+	ktime_t base_time = adapter->base_time;
+	u32 launchtime;
+
+	/* FIXME: when using ETF together with taprio, we may have a
+	 * case where 'delta' is larger than the cycle_time, this may
+	 * cause problems if we don't read the current value of
+	 * IGC_BASET, as the value writen into the launchtime
+	 * descriptor field may be misinterpreted.
+	 */
+	div_s64_rem(ktime_sub_ns(txtime, base_time), cycle_time, &launchtime);
+
+	return cpu_to_le32(launchtime);
+}
+
 static void igc_tx_ctxtdesc(struct igc_ring *tx_ring,
 			    struct igc_tx_buffer *first,
 			    u32 vlan_macip_lens, u32 type_tucmd,
@@ -876,7 +893,6 @@ static void igc_tx_ctxtdesc(struct igc_ring *tx_ring,
 {
 	struct igc_adv_tx_context_desc *context_desc;
 	u16 i = tx_ring->next_to_use;
-	struct timespec64 ts;
 
 	context_desc = IGC_TX_CTXTDESC(tx_ring, i);
 
@@ -898,9 +914,12 @@ static void igc_tx_ctxtdesc(struct igc_ring *tx_ring,
 	 * should have been handled by the upper layers.
 	 */
 	if (tx_ring->launchtime_enable) {
-		ts = ktime_to_timespec64(first->skb->tstamp);
+		struct igc_adapter *adapter = netdev_priv(tx_ring->netdev);
+		ktime_t txtime = first->skb->tstamp;
+
 		first->skb->tstamp = ktime_set(0, 0);
-		context_desc->launch_time = cpu_to_le32(ts.tv_nsec / 32);
+		context_desc->launch_time = igc_tx_launchtime(adapter,
+							      txtime);
 	} else {
 		context_desc->launch_time = 0;
 	}
@@ -4497,6 +4516,32 @@ static int igc_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 	}
 }
 
+static int igc_save_launchtime_params(struct igc_adapter *adapter, int queue,
+				      bool enable)
+{
+	struct igc_ring *ring;
+	int i;
+
+	if (queue < 0 || queue >= adapter->num_tx_queues)
+		return -EINVAL;
+
+	ring = adapter->tx_ring[queue];
+	ring->launchtime_enable = enable;
+
+	if (adapter->base_time)
+		return 0;
+
+	adapter->cycle_time = NSEC_PER_SEC;
+
+	for (i = 0; i < adapter->num_tx_queues; i++) {
+		ring = adapter->tx_ring[i];
+		ring->start_time = 0;
+		ring->end_time = NSEC_PER_SEC;
+	}
+
+	return 0;
+}
+
 static bool validate_schedule(const struct tc_taprio_qopt_offload *qopt)
 {
 	int queue_uses[IGC_MAX_TX_QUEUES] = { };
@@ -4529,6 +4574,22 @@ static bool validate_schedule(const struct tc_taprio_qopt_offload *qopt)
 	return true;
 }
 
+static int igc_tsn_enable_launchtime(struct igc_adapter *adapter,
+				     struct tc_etf_qopt_offload *qopt)
+{
+	struct igc_hw *hw = &adapter->hw;
+	int err;
+
+	if (hw->mac.type != igc_i225)
+		return -EOPNOTSUPP;
+
+	err = igc_save_launchtime_params(adapter, qopt->queue, qopt->enable);
+	if (err)
+		return err;
+
+	return igc_tsn_offload_apply(adapter);
+}
+
 static int igc_save_qbv_schedule(struct igc_adapter *adapter,
 				 struct tc_taprio_qopt_offload *qopt)
 {
@@ -4599,6 +4660,9 @@ static int igc_setup_tc(struct net_device *dev, enum tc_setup_type type,
 	case TC_SETUP_QDISC_TAPRIO:
 		return igc_tsn_enable_qbv_scheduling(adapter, type_data);
 
+	case TC_SETUP_QDISC_ETF:
+		return igc_tsn_enable_launchtime(adapter, type_data);
+
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
index 257fe970afe8..174103c4bea6 100644
--- a/drivers/net/ethernet/intel/igc/igc_tsn.c
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
@@ -4,6 +4,20 @@
 #include "igc.h"
 #include "igc_tsn.h"
 
+static bool is_any_launchtime(struct igc_adapter *adapter)
+{
+	int i;
+
+	for (i = 0; i < adapter->num_tx_queues; i++) {
+		struct igc_ring *ring = adapter->tx_ring[i];
+
+		if (ring->launchtime_enable)
+			return true;
+	}
+
+	return false;
+}
+
 /* Returns the TSN specific registers to their default values after
  * TSN offloading is disabled.
  */
@@ -88,6 +102,9 @@ static int igc_tsn_enable_offload(struct igc_adapter *adapter)
 				IGC_TXQCTL_STRICT_END;
 		}
 
+		if (ring->launchtime_enable)
+			txqctl |= IGC_TXQCTL_QUEUE_MODE_LAUNCHT;
+
 		wr32(IGC_TXQCTL(i), txqctl);
 	}
 
@@ -115,7 +132,7 @@ static int igc_tsn_enable_offload(struct igc_adapter *adapter)
 
 int igc_tsn_offload_apply(struct igc_adapter *adapter)
 {
-	bool is_any_enabled = adapter->base_time;
+	bool is_any_enabled = adapter->base_time || is_any_launchtime(adapter);
 
 	if (!(adapter->flags & IGC_FLAG_TSN_QBV_ENABLED) && !is_any_enabled)
 		return 0;
-- 
2.25.0

[Intel-wired-lan] [next-queue PATCH v3 1/2] igc: Add support for taprio offloading

[Andre Guedes]

Quoting Vinicius Costa Gomes (2020-02-14 15:52:02)
> Adds support for translating taprio schedules into i225 cycles. This
> will allow schedules to run in the hardware, making the schedules
> enforcement more precise and saving CPU time.
> 
> Right now, the only simple schedules are allowed, complex schedules are
> rejected. "simple" in this context are schedules that each HW queue is
> opened and closed only once in each cycle.
> 
> Changing schedules is still not supported as well.
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>

Reviewed-by: Andre Guedes <andre.guedes@intel.com>

[Intel-wired-lan] [next-queue PATCH v3 2/2] igc: Add support for ETF offloading

[Andre Guedes]

Quoting Vinicius Costa Gomes (2020-02-14 15:52:03)
> This adds support for ETF offloading for the i225 controller.
> 
> For i225, the LaunchTime feature is almost a subset of the Qbv
> feature. The main change from the i210 is that the launchtime of each
> packet is specified as an offset applied to the BASET register. BASET
> is automatically incremented each cycle.
> 
> For i225, the approach chosen is to re-use most of the setup used for
> taprio offloading. With a few changes:
> 
>  - The more or less obvious one is that when ETF is enabled, we should
>  set add the expected launchtime to the (advanced) transmit
>  descriptor;
> 
>  - The less obvious, is that when taprio offloading is not enabled, we
>  add a dummy schedule (all queues are open all the time, with a cycle
>  time of 1 second).
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>

Reviewed-by: Andre Guedes <andre.guedes@intel.com>

[Intel-wired-lan] [next-queue PATCH v3 2/2] igc: Add support for ETF offloading

[Brown, Aaron F]

> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Vinicius Costa Gomes
> Sent: Friday, February 14, 2020 3:52 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next-queue PATCH v3 2/2] igc: Add support for
> ETF offloading
> 
> This adds support for ETF offloading for the i225 controller.
> 
> For i225, the LaunchTime feature is almost a subset of the Qbv
> feature. The main change from the i210 is that the launchtime of each
> packet is specified as an offset applied to the BASET register. BASET
> is automatically incremented each cycle.
> 
> For i225, the approach chosen is to re-use most of the setup used for
> taprio offloading. With a few changes:
> 
>  - The more or less obvious one is that when ETF is enabled, we should
>  set add the expected launchtime to the (advanced) transmit
>  descriptor;
> 
>  - The less obvious, is that when taprio offloading is not enabled, we
>  add a dummy schedule (all queues are open all the time, with a cycle
>  time of 1 second).
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> ---
>  drivers/net/ethernet/intel/igc/igc_defines.h |  1 +
>  drivers/net/ethernet/intel/igc/igc_main.c    | 70 +++++++++++++++++++-
>  drivers/net/ethernet/intel/igc/igc_tsn.c     | 19 +++++-
>  3 files changed, 86 insertions(+), 4 deletions(-)
> 
I'm using the TSN Scheduled TX Tools from https://gist.github.com/jeez/bd3afeff081ba64a695008dd8215866f, and the process (from both the README.etf and README.taprio) seems to work fine with an i210 (igb adapter) but when I try to use the same process with an i225 (igc based adapter) I get a series of Tx Unit Hangs when I start sending traffic.  Packets are still getting sent, but lot of ones just hang.
------------------------------------------------------------
[  936.406229] igc 0000:01:00.0: Detected Tx Unit Hang
                 Tx Queue             <0>
                 TDH                  <de>
                 TDT                  <e4>
                 next_to_use          <e4>
                 next_to_clean        <de>
               buffer_info[next_to_clean]
                 time_stamp           <100099d84>
                 next_to_watch        <0000000052519a89>
                 jiffies              <10009a393>
                 desc.status          <4a8200>
[  941.932530] igc 0000:01:00.0: Detected Tx Unit Hang
                 Tx Queue             <0>
                 TDH                  <1e>
                 TDT                  <22>
                 next_to_use          <22>
                 next_to_clean        <1e>
               buffer_info[next_to_clean]
                 time_stamp           <10009b0e0>
                 next_to_watch        <00000000ff485dca>
                 jiffies              <10009bb52>
                 desc.status          <4a8200>
[  945.581031] igc 0000:01:00.0: Detected Tx Unit Hang
                 Tx Queue             <0>
                 TDH                  <4a>
                 TDT                  <52>
                 next_to_use          <52>
                 next_to_clean        <4a>
               buffer_info[next_to_clean]
                 time_stamp           <10009c388>
                 next_to_watch        <00000000073a6ad3>
                 jiffies              <10009caff>
                 desc.status          <4a8200>

...
And so on until I stop the talker.  Other (regular) traffic still gets through without any apparent problem.  But only a portion of the etf scheduled ones, the rest left TX Hanging.

> diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h
> b/drivers/net/ethernet/intel/igc/igc_defines.h
> index 3077d0a69b04..0746fa42ff3f 100644
> --- a/drivers/net/ethernet/intel/igc/igc_defines.h
> +++ b/drivers/net/ethernet/intel/igc/igc_defines.h
> @@ -440,6 +440,7 @@
>  #define IGC_TQAVCTRL_TRANSMIT_MODE_TSN	0x00000001
>  #define IGC_TQAVCTRL_ENHANCED_QAV	0x00000008
> 
> +#define IGC_TXQCTL_QUEUE_MODE_LAUNCHT	0x00000001
>  #define IGC_TXQCTL_STRICT_CYCLE		0x00000002
>  #define IGC_TXQCTL_STRICT_END		0x00000004
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> b/drivers/net/ethernet/intel/igc/igc_main.c
> index 5fb52768de18..55ab6077455d 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -869,6 +869,23 @@ static int igc_write_mc_addr_list(struct net_device
> *netdev)
>  	return netdev_mc_count(netdev);
>  }
> 
> +static __le32 igc_tx_launchtime(struct igc_adapter *adapter, ktime_t
> txtime)
> +{
> +	ktime_t cycle_time = adapter->cycle_time;
> +	ktime_t base_time = adapter->base_time;
> +	u32 launchtime;
> +
> +	/* FIXME: when using ETF together with taprio, we may have a
> +	 * case where 'delta' is larger than the cycle_time, this may
> +	 * cause problems if we don't read the current value of
> +	 * IGC_BASET, as the value writen into the launchtime
> +	 * descriptor field may be misinterpreted.
> +	 */
> +	div_s64_rem(ktime_sub_ns(txtime, base_time), cycle_time,
> &launchtime);
> +
> +	return cpu_to_le32(launchtime);
> +}
> +
>  static void igc_tx_ctxtdesc(struct igc_ring *tx_ring,
>  			    struct igc_tx_buffer *first,
>  			    u32 vlan_macip_lens, u32 type_tucmd,
> @@ -876,7 +893,6 @@ static void igc_tx_ctxtdesc(struct igc_ring *tx_ring,
>  {
>  	struct igc_adv_tx_context_desc *context_desc;
>  	u16 i = tx_ring->next_to_use;
> -	struct timespec64 ts;
> 
>  	context_desc = IGC_TX_CTXTDESC(tx_ring, i);
> 
> @@ -898,9 +914,12 @@ static void igc_tx_ctxtdesc(struct igc_ring *tx_ring,
>  	 * should have been handled by the upper layers.
>  	 */
>  	if (tx_ring->launchtime_enable) {
> -		ts = ktime_to_timespec64(first->skb->tstamp);
> +		struct igc_adapter *adapter = netdev_priv(tx_ring->netdev);
> +		ktime_t txtime = first->skb->tstamp;
> +
>  		first->skb->tstamp = ktime_set(0, 0);
> -		context_desc->launch_time = cpu_to_le32(ts.tv_nsec / 32);
> +		context_desc->launch_time = igc_tx_launchtime(adapter,
> +							      txtime);
>  	} else {
>  		context_desc->launch_time = 0;
>  	}
> @@ -4497,6 +4516,32 @@ static int igc_ioctl(struct net_device *netdev,
> struct ifreq *ifr, int cmd)
>  	}
>  }
> 
> +static int igc_save_launchtime_params(struct igc_adapter *adapter, int
> queue,
> +				      bool enable)
> +{
> +	struct igc_ring *ring;
> +	int i;
> +
> +	if (queue < 0 || queue >= adapter->num_tx_queues)
> +		return -EINVAL;
> +
> +	ring = adapter->tx_ring[queue];
> +	ring->launchtime_enable = enable;
> +
> +	if (adapter->base_time)
> +		return 0;
> +
> +	adapter->cycle_time = NSEC_PER_SEC;
> +
> +	for (i = 0; i < adapter->num_tx_queues; i++) {
> +		ring = adapter->tx_ring[i];
> +		ring->start_time = 0;
> +		ring->end_time = NSEC_PER_SEC;
> +	}
> +
> +	return 0;
> +}
> +
>  static bool validate_schedule(const struct tc_taprio_qopt_offload *qopt)
>  {
>  	int queue_uses[IGC_MAX_TX_QUEUES] = { };
> @@ -4529,6 +4574,22 @@ static bool validate_schedule(const struct
> tc_taprio_qopt_offload *qopt)
>  	return true;
>  }
> 
> +static int igc_tsn_enable_launchtime(struct igc_adapter *adapter,
> +				     struct tc_etf_qopt_offload *qopt)
> +{
> +	struct igc_hw *hw = &adapter->hw;
> +	int err;
> +
> +	if (hw->mac.type != igc_i225)
> +		return -EOPNOTSUPP;
> +
> +	err = igc_save_launchtime_params(adapter, qopt->queue, qopt-
> >enable);
> +	if (err)
> +		return err;
> +
> +	return igc_tsn_offload_apply(adapter);
> +}
> +
>  static int igc_save_qbv_schedule(struct igc_adapter *adapter,
>  				 struct tc_taprio_qopt_offload *qopt)
>  {
> @@ -4599,6 +4660,9 @@ static int igc_setup_tc(struct net_device *dev,
> enum tc_setup_type type,
>  	case TC_SETUP_QDISC_TAPRIO:
>  		return igc_tsn_enable_qbv_scheduling(adapter, type_data);
> 
> +	case TC_SETUP_QDISC_ETF:
> +		return igc_tsn_enable_launchtime(adapter, type_data);
> +
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c
> b/drivers/net/ethernet/intel/igc/igc_tsn.c
> index 257fe970afe8..174103c4bea6 100644
> --- a/drivers/net/ethernet/intel/igc/igc_tsn.c
> +++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
> @@ -4,6 +4,20 @@
>  #include "igc.h"
>  #include "igc_tsn.h"
> 
> +static bool is_any_launchtime(struct igc_adapter *adapter)
> +{
> +	int i;
> +
> +	for (i = 0; i < adapter->num_tx_queues; i++) {
> +		struct igc_ring *ring = adapter->tx_ring[i];
> +
> +		if (ring->launchtime_enable)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>  /* Returns the TSN specific registers to their default values after
>   * TSN offloading is disabled.
>   */
> @@ -88,6 +102,9 @@ static int igc_tsn_enable_offload(struct igc_adapter
> *adapter)
>  				IGC_TXQCTL_STRICT_END;
>  		}
> 
> +		if (ring->launchtime_enable)
> +			txqctl |= IGC_TXQCTL_QUEUE_MODE_LAUNCHT;
> +
>  		wr32(IGC_TXQCTL(i), txqctl);
>  	}
> 
> @@ -115,7 +132,7 @@ static int igc_tsn_enable_offload(struct igc_adapter
> *adapter)
> 
>  int igc_tsn_offload_apply(struct igc_adapter *adapter)
>  {
> -	bool is_any_enabled = adapter->base_time;
> +	bool is_any_enabled = adapter->base_time ||
> is_any_launchtime(adapter);
> 
>  	if (!(adapter->flags & IGC_FLAG_TSN_QBV_ENABLED) &&
> !is_any_enabled)
>  		return 0;
> --
> 2.25.0
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [next-queue PATCH v3 1/2] igc: Add support for taprio offloading

[Brown, Aaron F]

> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Vinicius Costa Gomes
> Sent: Friday, February 14, 2020 3:52 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next-queue PATCH v3 1/2] igc: Add support for
> taprio offloading
> 
> Adds support for translating taprio schedules into i225 cycles. This
> will allow schedules to run in the hardware, making the schedules
> enforcement more precise and saving CPU time.
> 
> Right now, the only simple schedules are allowed, complex schedules are
> rejected. "simple" in this context are schedules that each HW queue is
> opened and closed only once in each cycle.
> 
> Changing schedules is still not supported as well.
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> ---
>  drivers/net/ethernet/intel/igc/Makefile      |   2 +-
>  drivers/net/ethernet/intel/igc/igc.h         |   7 +
>  drivers/net/ethernet/intel/igc/igc_defines.h |  12 ++
>  drivers/net/ethernet/intel/igc/igc_main.c    | 113 +++++++++++++++
>  drivers/net/ethernet/intel/igc/igc_regs.h    |  12 ++
>  drivers/net/ethernet/intel/igc/igc_tsn.c     | 140 +++++++++++++++++++
>  drivers/net/ethernet/intel/igc/igc_tsn.h     |   9 ++
>  7 files changed, 294 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/intel/igc/igc_tsn.c
>  create mode 100644 drivers/net/ethernet/intel/igc/igc_tsn.h

I'm using the TSN Scheduled TX Tools from https://gist.github.com/jeez/bd3afeff081ba64a695008dd8215866f, and the process (from both the README.etf and README.taprio) seems to work fine with an i210 (igb adapter) but when I try to use the same process with an i225 (igc based adapter) for the taprio session the system panics, freezing the system and leaving me a panic RIP frozen on my screen, but no hint of that panic got saved to disk (crash dump or log.)  I tried to get a capture via a serial console, but unfortunately it freezes to fast to get the trace out and instead of a full trace I am left with a single line from the panic:
[  718.732753] BUG: kernel NULL pointer dereference, address: 0000000000000008

I am able to get a jpg of the frozen screen which I can make available, though the start of the trace has scrolled off the top of the screen.  
> 
> diff --git a/drivers/net/ethernet/intel/igc/Makefile
> b/drivers/net/ethernet/intel/igc/Makefile
> index e3c164c12e10..3652f211f351 100644
> --- a/drivers/net/ethernet/intel/igc/Makefile
> +++ b/drivers/net/ethernet/intel/igc/Makefile
> @@ -8,4 +8,4 @@
>  obj-$(CONFIG_IGC) += igc.o
> 
>  igc-objs := igc_main.o igc_mac.o igc_i225.o igc_base.o igc_nvm.o igc_phy.o \
> -igc_ethtool.o igc_ptp.o igc_dump.o
> +igc_ethtool.o igc_ptp.o igc_dump.o igc_tsn.o
> diff --git a/drivers/net/ethernet/intel/igc/igc.h
> b/drivers/net/ethernet/intel/igc/igc.h
> index 0014828eec46..4c40dc8f276c 100644
> --- a/drivers/net/ethernet/intel/igc/igc.h
> +++ b/drivers/net/ethernet/intel/igc/igc.h
> @@ -70,6 +70,7 @@ extern char igc_driver_version[];
>  #define IGC_FLAG_HAS_MSIX		BIT(13)
>  #define IGC_FLAG_VLAN_PROMISC		BIT(15)
>  #define IGC_FLAG_RX_LEGACY		BIT(16)
> +#define IGC_FLAG_TSN_QBV_ENABLED	BIT(17)
> 
>  #define IGC_FLAG_RSS_FIELD_IPV4_UDP	BIT(6)
>  #define IGC_FLAG_RSS_FIELD_IPV6_UDP	BIT(7)
> @@ -287,6 +288,9 @@ struct igc_ring {
>  	u8 reg_idx;                     /* physical index of the ring */
>  	bool launchtime_enable;		/* true if LaunchTime is
> enabled */
> 
> +	u32 start_time;
> +	u32 end_time;
> +
>  	/* everything past this point are written often */
>  	u16 next_to_clean;
>  	u16 next_to_use;
> @@ -421,6 +425,9 @@ struct igc_adapter {
>  	u32 max_frame_size;
>  	u32 min_frame_size;
> 
> +	ktime_t base_time;
> +	ktime_t cycle_time;
> +
>  	/* OS defined structs */
>  	struct pci_dev *pdev;
>  	/* lock for statistics */
> diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h
> b/drivers/net/ethernet/intel/igc/igc_defines.h
> index 044c0f8d4c16..3077d0a69b04 100644
> --- a/drivers/net/ethernet/intel/igc/igc_defines.h
> +++ b/drivers/net/ethernet/intel/igc/igc_defines.h
> @@ -377,6 +377,11 @@
>  #define I225_TXPBSIZE_DEFAULT	0x04000014 /* TXPBSIZE default */
>  #define IGC_RXPBS_CFG_TS_EN	0x80000000 /* Timestamp in Rx
> buffer */
> 
> +#define IGC_TXPBSIZE_TSN	0x04145145 /* 5k bytes buffer for each
> queue */
> +
> +#define IGC_DTXMXPKTSZ_TSN	0x19 /* 1600 bytes of max TX DMA
> packet size */
> +#define IGC_DTXMXPKTSZ_DEFAULT	0x98 /* 9728-byte Jumbo frames */
> +
>  /* Time Sync Interrupt Causes */
>  #define IGC_TSICR_SYS_WRAP	BIT(0) /* SYSTIM Wrap around. */
>  #define IGC_TSICR_TXTS		BIT(1) /* Transmit Timestamp. */
> @@ -431,6 +436,13 @@
>  #define IGC_TSYNCTXCTL_START_SYNC		0x80000000  /*
> initiate sync */
>  #define IGC_TSYNCTXCTL_TXSYNSIG			0x00000020  /*
> Sample TX tstamp in PHY sop */
> 
> +/* Transmit Scheduling */
> +#define IGC_TQAVCTRL_TRANSMIT_MODE_TSN	0x00000001
> +#define IGC_TQAVCTRL_ENHANCED_QAV	0x00000008
> +
> +#define IGC_TXQCTL_STRICT_CYCLE		0x00000002
> +#define IGC_TXQCTL_STRICT_END		0x00000004
> +
>  /* Receive Checksum Control */
>  #define IGC_RXCSUM_CRCOFL	0x00000800   /* CRC32 offload enable
> */
>  #define IGC_RXCSUM_PCSD		0x00002000   /* packet checksum
> disabled */
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> b/drivers/net/ethernet/intel/igc/igc_main.c
> index 3407af11dff8..5fb52768de18 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -9,11 +9,13 @@
>  #include <linux/udp.h>
>  #include <linux/ip.h>
>  #include <linux/pm_runtime.h>
> +#include <net/pkt_sched.h>
> 
>  #include <net/ipv6.h>
> 
>  #include "igc.h"
>  #include "igc_hw.h"
> +#include "igc_tsn.h"
> 
>  #define DRV_VERSION	"0.0.1-k"
>  #define DRV_SUMMARY	"Intel(R) 2.5G Ethernet Linux Driver"
> @@ -106,6 +108,9 @@ void igc_reset(struct igc_adapter *adapter)
>  	/* Re-enable PTP, where applicable. */
>  	igc_ptp_reset(adapter);
> 
> +	/* Re-enable TSN offloading, where applicable. */
> +	igc_tsn_offload_apply(adapter);
> +
>  	igc_get_phy_info(hw);
>  }
> 
> @@ -4492,6 +4497,113 @@ static int igc_ioctl(struct net_device *netdev,
> struct ifreq *ifr, int cmd)
>  	}
>  }
> 
> +static bool validate_schedule(const struct tc_taprio_qopt_offload *qopt)
> +{
> +	int queue_uses[IGC_MAX_TX_QUEUES] = { };
> +	size_t n;
> +
> +	if (qopt->cycle_time_extension)
> +		return false;
> +
> +	for (n = 0; n < qopt->num_entries; n++) {
> +		const struct tc_taprio_sched_entry *e;
> +		int i;
> +
> +		e = &qopt->entries[n];
> +
> +		/* i225 only supports "global" frame preemption
> +		 * settings.
> +		 */
> +		if (e->command != TC_TAPRIO_CMD_SET_GATES)
> +			return false;
> +
> +		for (i = 0; i < IGC_MAX_TX_QUEUES; i++) {
> +			if (e->gate_mask & BIT(i))
> +				queue_uses[i]++;
> +
> +			if (queue_uses[i] > 1)
> +				return false;
> +		}
> +	}
> +
> +	return true;
> +}
> +
> +static int igc_save_qbv_schedule(struct igc_adapter *adapter,
> +				 struct tc_taprio_qopt_offload *qopt)
> +{
> +	u32 start_time = 0, end_time = 0;
> +	size_t n;
> +
> +	if (!qopt->enable) {
> +		adapter->base_time = 0;
> +		return 0;
> +	}
> +
> +	if (adapter->base_time)
> +		return -EALREADY;
> +
> +	if (!validate_schedule(qopt))
> +		return -EINVAL;
> +
> +	adapter->cycle_time = qopt->cycle_time;
> +	adapter->base_time = qopt->base_time;
> +
> +	/* FIXME: be a little smarter about cases when the gate for a
> +	 * queue stays open for more than one entry.
> +	 */
> +	for (n = 0; n < qopt->num_entries; n++) {
> +		struct tc_taprio_sched_entry *e = &qopt->entries[n];
> +		int i;
> +
> +		end_time += e->interval;
> +
> +		for (i = 0; i < IGC_MAX_TX_QUEUES; i++) {
> +			struct igc_ring *ring = adapter->tx_ring[i];
> +
> +			if (!(e->gate_mask & BIT(i)))
> +				continue;
> +
> +			ring->start_time = start_time;
> +			ring->end_time = end_time;
> +		}
> +
> +		start_time += e->interval;
> +	}
> +
> +	return 0;
> +}
> +
> +static int igc_tsn_enable_qbv_scheduling(struct igc_adapter *adapter,
> +					 struct tc_taprio_qopt_offload *qopt)
> +{
> +	struct igc_hw *hw = &adapter->hw;
> +	int err;
> +
> +	if (hw->mac.type != igc_i225)
> +		return -EOPNOTSUPP;
> +
> +	err = igc_save_qbv_schedule(adapter, qopt);
> +	if (err)
> +		return err;
> +
> +	return igc_tsn_offload_apply(adapter);
> +}
> +
> +static int igc_setup_tc(struct net_device *dev, enum tc_setup_type type,
> +			void *type_data)
> +{
> +	struct igc_adapter *adapter = netdev_priv(dev);
> +
> +	switch (type) {
> +	case TC_SETUP_QDISC_TAPRIO:
> +		return igc_tsn_enable_qbv_scheduling(adapter, type_data);
> +
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
>  static const struct net_device_ops igc_netdev_ops = {
>  	.ndo_open		= igc_open,
>  	.ndo_stop		= igc_close,
> @@ -4504,6 +4616,7 @@ static const struct net_device_ops igc_netdev_ops
> = {
>  	.ndo_set_features	= igc_set_features,
>  	.ndo_features_check	= igc_features_check,
>  	.ndo_do_ioctl		= igc_ioctl,
> +	.ndo_setup_tc           = igc_setup_tc,
>  };
> 
>  /* PCIe configuration access */
> diff --git a/drivers/net/ethernet/intel/igc/igc_regs.h
> b/drivers/net/ethernet/intel/igc/igc_regs.h
> index d4af53a80f11..96dee3c1a5f7 100644
> --- a/drivers/net/ethernet/intel/igc/igc_regs.h
> +++ b/drivers/net/ethernet/intel/igc/igc_regs.h
> @@ -231,6 +231,18 @@
> 
>  #define IGC_RXPBS	0x02404  /* Rx Packet Buffer Size - RW */
> 
> +/* Transmit Scheduling Registers */
> +#define IGC_TQAVCTRL                   0x3570
> +#define IGC_TXQCTL(_n)                 (0x3344 + 0x4 * (_n))
> +#define IGC_BASET_L                    0x3314
> +#define IGC_BASET_H                    0x3318
> +#define IGC_QBVCYCLET                  0x331C
> +#define IGC_QBVCYCLET_S                0x3320
> +
> +#define IGC_STQT(_n)                   (0x3324 + 0x4 * (_n))
> +#define IGC_ENDQT(_n)                  (0x3334 + 0x4 * (_n))
> +#define IGC_DTXMXPKTSZ                 0x355C
> +
>  /* System Time Registers */
>  #define IGC_SYSTIML	0x0B600  /* System time register Low - RO */
>  #define IGC_SYSTIMH	0x0B604  /* System time register High - RO */
> diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c
> b/drivers/net/ethernet/intel/igc/igc_tsn.c
> new file mode 100644
> index 000000000000..257fe970afe8
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
> @@ -0,0 +1,140 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c)  2019 Intel Corporation */
> +
> +#include "igc.h"
> +#include "igc_tsn.h"
> +
> +/* Returns the TSN specific registers to their default values after
> + * TSN offloading is disabled.
> + */
> +static int igc_tsn_disable_offload(struct igc_adapter *adapter)
> +{
> +	struct igc_hw *hw = &adapter->hw;
> +	u32 tqavctrl;
> +	int i;
> +
> +	if (!(adapter->flags & IGC_FLAG_TSN_QBV_ENABLED))
> +		return 0;
> +
> +	adapter->cycle_time = 0;
> +
> +	wr32(IGC_TXPBS, I225_TXPBSIZE_DEFAULT);
> +	wr32(IGC_DTXMXPKTSZ, IGC_DTXMXPKTSZ_DEFAULT);
> +
> +	tqavctrl = rd32(IGC_TQAVCTRL);
> +	tqavctrl &= ~(IGC_TQAVCTRL_TRANSMIT_MODE_TSN |
> +		      IGC_TQAVCTRL_ENHANCED_QAV);
> +	wr32(IGC_TQAVCTRL, tqavctrl);
> +
> +	for (i = 0; i < adapter->num_tx_queues; i++) {
> +		struct igc_ring *ring = adapter->tx_ring[i];
> +
> +		ring->start_time = 0;
> +		ring->end_time = 0;
> +		ring->launchtime_enable = false;
> +
> +		wr32(IGC_TXQCTL(i), 0);
> +		wr32(IGC_STQT(i), 0);
> +		wr32(IGC_ENDQT(i), NSEC_PER_SEC);
> +	}
> +
> +	wr32(IGC_QBVCYCLET_S, NSEC_PER_SEC);
> +	wr32(IGC_QBVCYCLET, NSEC_PER_SEC);
> +
> +	adapter->flags &= ~IGC_FLAG_TSN_QBV_ENABLED;
> +
> +	return 0;
> +}
> +
> +static int igc_tsn_enable_offload(struct igc_adapter *adapter)
> +{
> +	struct igc_hw *hw = &adapter->hw;
> +	u32 tqavctrl, baset_l, baset_h;
> +	u32 sec, nsec, cycle;
> +	ktime_t base_time, systim;
> +	int i;
> +
> +	if (adapter->flags & IGC_FLAG_TSN_QBV_ENABLED)
> +		return 0;
> +
> +	cycle = adapter->cycle_time;
> +	base_time = adapter->base_time;
> +
> +	wr32(IGC_TSAUXC, 0);
> +	wr32(IGC_DTXMXPKTSZ, IGC_DTXMXPKTSZ_TSN);
> +	wr32(IGC_TXPBS, IGC_TXPBSIZE_TSN);
> +
> +	tqavctrl = rd32(IGC_TQAVCTRL);
> +	tqavctrl |= IGC_TQAVCTRL_TRANSMIT_MODE_TSN |
> IGC_TQAVCTRL_ENHANCED_QAV;
> +	wr32(IGC_TQAVCTRL, tqavctrl);
> +
> +	wr32(IGC_QBVCYCLET_S, cycle);
> +	wr32(IGC_QBVCYCLET, cycle);
> +
> +	for (i = 0; i < adapter->num_tx_queues; i++) {
> +		struct igc_ring *ring = adapter->tx_ring[i];
> +		u32 txqctl = 0;
> +
> +		wr32(IGC_STQT(i), ring->start_time);
> +		wr32(IGC_ENDQT(i), ring->end_time);
> +
> +		if (adapter->base_time) {
> +			/* If we have a base_time we are in "taprio"
> +			 * mode and we need to be strict about the
> +			 * cycles: only transmit a packet if it can be
> +			 * completed during that cycle.
> +			 */
> +			txqctl |= IGC_TXQCTL_STRICT_CYCLE |
> +				IGC_TXQCTL_STRICT_END;
> +		}
> +
> +		wr32(IGC_TXQCTL(i), txqctl);
> +	}
> +
> +	nsec = rd32(IGC_SYSTIML);
> +	sec = rd32(IGC_SYSTIMH);
> +
> +	systim = ktime_set(sec, nsec);
> +
> +	if (ktime_compare(systim, base_time) > 0) {
> +		s64 n;
> +
> +		n = div64_s64(ktime_sub_ns(systim, base_time), cycle);
> +		base_time = ktime_add_ns(base_time, (n + 1) * cycle);
> +	}
> +
> +	baset_h = div_s64_rem(base_time, NSEC_PER_SEC, &baset_l);
> +
> +	wr32(IGC_BASET_H, baset_h);
> +	wr32(IGC_BASET_L, baset_l);
> +
> +	adapter->flags |= IGC_FLAG_TSN_QBV_ENABLED;
> +
> +	return 0;
> +}
> +
> +int igc_tsn_offload_apply(struct igc_adapter *adapter)
> +{
> +	bool is_any_enabled = adapter->base_time;
> +
> +	if (!(adapter->flags & IGC_FLAG_TSN_QBV_ENABLED) &&
> !is_any_enabled)
> +		return 0;
> +
> +	if (!is_any_enabled) {
> +		int err = igc_tsn_disable_offload(adapter);
> +
> +		if (err < 0)
> +			return err;
> +
> +		/* The BASET registers aren't cleared when writing
> +		 * into them, force a reset if the interface is
> +		 * running.
> +		 */
> +		if (netif_running(adapter->netdev))
> +			schedule_work(&adapter->reset_task);
> +
> +		return 0;
> +	}
> +
> +	return igc_tsn_enable_offload(adapter);
> +}
> diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.h
> b/drivers/net/ethernet/intel/igc/igc_tsn.h
> new file mode 100644
> index 000000000000..f76bc86ddccd
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/igc/igc_tsn.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c)  2020 Intel Corporation */
> +
> +#ifndef _IGC_TSN_H_
> +#define _IGC_TSN_H_
> +
> +int igc_tsn_offload_apply(struct igc_adapter *adapter);
> +
> +#endif /* _IGC_BASE_H */
> --
> 2.25.0
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [next-queue PATCH v3 2/2] igc: Add support for ETF offloading

[Vinicius Costa Gomes]

Hi Aaron,

"Brown, Aaron F" <aaron.f.brown@intel.com> writes:

>> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
>> Vinicius Costa Gomes
>> Sent: Friday, February 14, 2020 3:52 PM
>> To: intel-wired-lan at lists.osuosl.org
>> Subject: [Intel-wired-lan] [next-queue PATCH v3 2/2] igc: Add support for
>> ETF offloading
>> 
>> This adds support for ETF offloading for the i225 controller.
>> 
>> For i225, the LaunchTime feature is almost a subset of the Qbv
>> feature. The main change from the i210 is that the launchtime of each
>> packet is specified as an offset applied to the BASET register. BASET
>> is automatically incremented each cycle.
>> 
>> For i225, the approach chosen is to re-use most of the setup used for
>> taprio offloading. With a few changes:
>> 
>>  - The more or less obvious one is that when ETF is enabled, we should
>>  set add the expected launchtime to the (advanced) transmit
>>  descriptor;
>> 
>>  - The less obvious, is that when taprio offloading is not enabled, we
>>  add a dummy schedule (all queues are open all the time, with a cycle
>>  time of 1 second).
>> 
>> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
>> ---
>>  drivers/net/ethernet/intel/igc/igc_defines.h |  1 +
>>  drivers/net/ethernet/intel/igc/igc_main.c    | 70 +++++++++++++++++++-
>>  drivers/net/ethernet/intel/igc/igc_tsn.c     | 19 +++++-
>>  3 files changed, 86 insertions(+), 4 deletions(-)
>> 
> I'm using the TSN Scheduled TX Tools from https://gist.github.com/jeez/bd3afeff081ba64a695008dd8215866f, and the process (from both the README.etf and README.taprio) seems to work fine with an i210 (igb adapter) but when I try to use the same process with an i225 (igc based adapter) I get a series of Tx Unit Hangs when I start sending traffic.  Packets are still getting sent, but lot of ones just hang.
> ------------------------------------------------------------
> [  936.406229] igc 0000:01:00.0: Detected Tx Unit Hang
>                  Tx Queue             <0>
>                  TDH                  <de>
>                  TDT                  <e4>
>                  next_to_use          <e4>
>                  next_to_clean        <de>
>                buffer_info[next_to_clean]
>                  time_stamp           <100099d84>
>                  next_to_watch        <0000000052519a89>
>                  jiffies              <10009a393>
>                  desc.status          <4a8200>
> [  941.932530] igc 0000:01:00.0: Detected Tx Unit Hang
>                  Tx Queue             <0>
>                  TDH                  <1e>
>                  TDT                  <22>
>                  next_to_use          <22>
>                  next_to_clean        <1e>
>                buffer_info[next_to_clean]
>                  time_stamp           <10009b0e0>
>                  next_to_watch        <00000000ff485dca>
>                  jiffies              <10009bb52>
>                  desc.status          <4a8200>
> [  945.581031] igc 0000:01:00.0: Detected Tx Unit Hang
>                  Tx Queue             <0>
>                  TDH                  <4a>
>                  TDT                  <52>
>                  next_to_use          <52>
>                  next_to_clean        <4a>
>                buffer_info[next_to_clean]
>                  time_stamp           <10009c388>
>                  next_to_watch        <00000000073a6ad3>
>                  jiffies              <10009caff>
>                  desc.status          <4a8200>
>
> ...
> And so on until I stop the talker.  Other (regular) traffic still gets through without any apparent problem.  But only a portion of the etf scheduled ones, the rest left TX Hanging.
>

Thanks for the test.

I only got to reproduce this when the system clock and the NIC PHC are
out of sync, e.g. I only see this when I start udp_tai just after I
start ptp4l/phc2sys.

Just to confirm that we talking about the same problem, if possible,
wait a few minutes before starting udp_tai after starting ptp4l/phc2sys,
and see if the problem persists.

I am trying to think what I can do from the driver side.

Cheers,
-- 
Vinicius

[Intel-wired-lan] [next-queue PATCH v3 2/2] igc: Add support for ETF offloading

[Brown, Aaron F]

> From: Gomes, Vinicius <vinicius.gomes@intel.com>
> Sent: Thursday, February 27, 2020 11:03 AM
> To: Brown, Aaron F <aaron.f.brown@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: RE: [Intel-wired-lan] [next-queue PATCH v3 2/2] igc: Add support for
> ETF offloading
> 
> Hi Aaron,
> 
> "Brown, Aaron F" <aaron.f.brown@intel.com> writes:
> 
> >> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of
> >> Vinicius Costa Gomes
> >> Sent: Friday, February 14, 2020 3:52 PM
> >> To: intel-wired-lan at lists.osuosl.org
> >> Subject: [Intel-wired-lan] [next-queue PATCH v3 2/2] igc: Add support for
> >> ETF offloading
> >>
> >> This adds support for ETF offloading for the i225 controller.
> >>
> >> For i225, the LaunchTime feature is almost a subset of the Qbv
> >> feature. The main change from the i210 is that the launchtime of each
> >> packet is specified as an offset applied to the BASET register. BASET
> >> is automatically incremented each cycle.
> >>
> >> For i225, the approach chosen is to re-use most of the setup used for
> >> taprio offloading. With a few changes:
> >>
> >>  - The more or less obvious one is that when ETF is enabled, we should
> >>  set add the expected launchtime to the (advanced) transmit
> >>  descriptor;
> >>
> >>  - The less obvious, is that when taprio offloading is not enabled, we
> >>  add a dummy schedule (all queues are open all the time, with a cycle
> >>  time of 1 second).
> >>
> >> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> >> ---
> >>  drivers/net/ethernet/intel/igc/igc_defines.h |  1 +
> >>  drivers/net/ethernet/intel/igc/igc_main.c    | 70
> +++++++++++++++++++-
> >>  drivers/net/ethernet/intel/igc/igc_tsn.c     | 19 +++++-
> >>  3 files changed, 86 insertions(+), 4 deletions(-)
> >>
> > I'm using the TSN Scheduled TX Tools from
> https://gist.github.com/jeez/bd3afeff081ba64a695008dd8215866f, and the
> process (from both the README.etf and README.taprio) seems to work fine
> with an i210 (igb adapter) but when I try to use the same process with an i225
> (igc based adapter) I get a series of Tx Unit Hangs when I start sending traffic.
> Packets are still getting sent, but lot of ones just hang.
> > ------------------------------------------------------------
> > [  936.406229] igc 0000:01:00.0: Detected Tx Unit Hang
> >                  Tx Queue             <0>
> >                  TDH                  <de>
> >                  TDT                  <e4>
> >                  next_to_use          <e4>
> >                  next_to_clean        <de>
> >                buffer_info[next_to_clean]
> >                  time_stamp           <100099d84>
> >                  next_to_watch        <0000000052519a89>
> >                  jiffies              <10009a393>
> >                  desc.status          <4a8200>
> > [  941.932530] igc 0000:01:00.0: Detected Tx Unit Hang
> >                  Tx Queue             <0>
> >                  TDH                  <1e>
> >                  TDT                  <22>
> >                  next_to_use          <22>
> >                  next_to_clean        <1e>
> >                buffer_info[next_to_clean]
> >                  time_stamp           <10009b0e0>
> >                  next_to_watch        <00000000ff485dca>
> >                  jiffies              <10009bb52>
> >                  desc.status          <4a8200>
> > [  945.581031] igc 0000:01:00.0: Detected Tx Unit Hang
> >                  Tx Queue             <0>
> >                  TDH                  <4a>
> >                  TDT                  <52>
> >                  next_to_use          <52>
> >                  next_to_clean        <4a>
> >                buffer_info[next_to_clean]
> >                  time_stamp           <10009c388>
> >                  next_to_watch        <00000000073a6ad3>
> >                  jiffies              <10009caff>
> >                  desc.status          <4a8200>
> >
> > ...
> > And so on until I stop the talker.  Other (regular) traffic still gets through
> without any apparent problem.  But only a portion of the etf scheduled ones,
> the rest left TX Hanging.
> >
> 
> Thanks for the test.
> 
> I only got to reproduce this when the system clock and the NIC PHC are
> out of sync, e.g. I only see this when I start udp_tai just after I
> start ptp4l/phc2sys.

That could be the case on my system.  I am getting a fair amount of thrash from ptp4l, switching between the local ptp hw clock and the remote one that it supposed to be master.  I'll see what happens when I use more of a known quantity, probably i210, for the link partner ptp master.

> 
> Just to confirm that we talking about the same problem, if possible,
> wait a few minutes before starting udp_tai after starting ptp4l/phc2sys,
> and see if the problem persists.

I do still see the Tx Unit Hangs if I give it a few minutes between starting the clock synch and launching the talker (udp_tai.)  But I don't think my ptp4l session is settling down so it sounds plausible that we are looking the same thing.

> 
> I am trying to think what I can do from the driver side.
> 
> Cheers,
> --
> Vinicius

[Intel-wired-lan] [next-queue PATCH v3 1/2] igc: Add support for taprio offloading

[Brown, Aaron F]

> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Vinicius Costa Gomes
> Sent: Friday, February 14, 2020 3:52 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next-queue PATCH v3 1/2] igc: Add support for taprio
> offloading
> 
> Adds support for translating taprio schedules into i225 cycles. This
> will allow schedules to run in the hardware, making the schedules
> enforcement more precise and saving CPU time.
> 
> Right now, the only simple schedules are allowed, complex schedules are
> rejected. "simple" in this context are schedules that each HW queue is
> opened and closed only once in each cycle.
> 
> Changing schedules is still not supported as well.
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> ---
>  drivers/net/ethernet/intel/igc/Makefile      |   2 +-
>  drivers/net/ethernet/intel/igc/igc.h         |   7 +
>  drivers/net/ethernet/intel/igc/igc_defines.h |  12 ++
>  drivers/net/ethernet/intel/igc/igc_main.c    | 113 +++++++++++++++
>  drivers/net/ethernet/intel/igc/igc_regs.h    |  12 ++
>  drivers/net/ethernet/intel/igc/igc_tsn.c     | 140 +++++++++++++++++++
>  drivers/net/ethernet/intel/igc/igc_tsn.h     |   9 ++
>  7 files changed, 294 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/intel/igc/igc_tsn.c
>  create mode 100644 drivers/net/ethernet/intel/igc/igc_tsn.h
> 

Tested-by: Aaron Brown <aaron.f.brown@intel.com>

[Intel-wired-lan] [next-queue PATCH v3 2/2] igc: Add support for ETF offloading

[Brown, Aaron F]

> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Vinicius Costa Gomes
> Sent: Friday, February 14, 2020 3:52 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next-queue PATCH v3 2/2] igc: Add support for
> ETF offloading
> 
> This adds support for ETF offloading for the i225 controller.
> 
> For i225, the LaunchTime feature is almost a subset of the Qbv
> feature. The main change from the i210 is that the launchtime of each
> packet is specified as an offset applied to the BASET register. BASET
> is automatically incremented each cycle.
> 
> For i225, the approach chosen is to re-use most of the setup used for
> taprio offloading. With a few changes:
> 
>  - The more or less obvious one is that when ETF is enabled, we should
>  set add the expected launchtime to the (advanced) transmit
>  descriptor;
> 
>  - The less obvious, is that when taprio offloading is not enabled, we
>  add a dummy schedule (all queues are open all the time, with a cycle
>  time of 1 second).
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> ---
>  drivers/net/ethernet/intel/igc/igc_defines.h |  1 +
>  drivers/net/ethernet/intel/igc/igc_main.c    | 70 +++++++++++++++++++-
>  drivers/net/ethernet/intel/igc/igc_tsn.c     | 19 +++++-
>  3 files changed, 86 insertions(+), 4 deletions(-)
> 

Tested-by: Aaron Brown <aaron.f.brown@intel.com>