2017-12-11 14:15:09

by Aleksey Makarov

[permalink] [raw]
Subject: [PATCH net-next v5 0/2] net: thunderx: add support for PTP clock

This series adds support for IEEE 1588 Precision Time Protocol
to Cavium ethernet driver.

The first patch adds support for the Precision Time Protocol Clocks and
Timestamping coprocessor (PTP) found on Cavium processors.
It registers a new PTP clock in the PTP core and provides functions
to use the counter in BGX, TNS, GTI, and NIC blocks.

The second patch introduces support for the PTP protocol to the
Cavium ThunderX ethernet driver.

v5:
- fix the file headers (add SPDX tags, remove advertisment) (Philippe Ombredanne)
- use "imply" instead of "select" (Richard Cochran)
- add some code in cavium_ptp_get() for the case when the PTP driver has not been
registered with the PTP core

v4: https://lkml.kernel.org/r/[email protected]
- use IS_ENABLED. This fixes compilation of the ptp as a module (David Miller)
- select PTP_1588_CLOCK, not depend on it. This fixes a build warning.
- change u64 to __be64. This fixes the sparse warning
"warning: cast to restricted __be64"
- make nicvf_config_hwtstamp() static. This fixes the sparse warning
"warning: symbol 'nicvf_config_hwtstamp' was not declared. Should it be static?"

v3: https://lkml.kernel.org/r/[email protected]
- rebase to net-next

v2: https://lkml.kernel.org/r/[email protected]
- use readq()/writeq() in place of cavium_ptp_reg_read()/cavium_ptp_reg_write(),
don't use readq_relaxed()/writeq_relaxed() (David Daney)

v1: https://lkml.kernel.org/r/[email protected]

Radoslaw Biernacki (1):
net: add support for Cavium PTP coprocessor

Sunil Goutham (1):
net: thunderx: add timestamping support

drivers/net/ethernet/cavium/Kconfig | 13 +
drivers/net/ethernet/cavium/Makefile | 1 +
drivers/net/ethernet/cavium/common/Makefile | 1 +
drivers/net/ethernet/cavium/common/cavium_ptp.c | 329 +++++++++++++++++++++
drivers/net/ethernet/cavium/common/cavium_ptp.h | 70 +++++
drivers/net/ethernet/cavium/thunder/nic.h | 15 +
drivers/net/ethernet/cavium/thunder/nic_main.c | 58 +++-
drivers/net/ethernet/cavium/thunder/nic_reg.h | 1 +
.../net/ethernet/cavium/thunder/nicvf_ethtool.c | 29 +-
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 173 ++++++++++-
drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 26 ++
drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 29 ++
drivers/net/ethernet/cavium/thunder/thunder_bgx.h | 4 +
13 files changed, 743 insertions(+), 6 deletions(-)
create mode 100644 drivers/net/ethernet/cavium/common/Makefile
create mode 100644 drivers/net/ethernet/cavium/common/cavium_ptp.c
create mode 100644 drivers/net/ethernet/cavium/common/cavium_ptp.h

--
2.15.1


2017-12-11 14:15:59

by Aleksey Makarov

[permalink] [raw]
Subject: [PATCH net-next v5 2/2] net: thunderx: add timestamping support

From: Sunil Goutham <[email protected]>

This adds timestamping support for both receive and transmit
paths. On the receive side no filters are supported i.e either
all pkts will get a timestamp appended infront of the packet or none.
On the transmit side HW doesn't support timestamp insertion but
only generates a separate CQE with transmitted packet's timestamp.
Also HW supports only one packet at a time for timestamping on the
transmit side.

Signed-off-by: Sunil Goutham <[email protected]>
Signed-off-by: Aleksey Makarov <[email protected]>
---
drivers/net/ethernet/cavium/Kconfig | 1 +
drivers/net/ethernet/cavium/thunder/nic.h | 15 ++
drivers/net/ethernet/cavium/thunder/nic_main.c | 58 ++++++-
drivers/net/ethernet/cavium/thunder/nic_reg.h | 1 +
.../net/ethernet/cavium/thunder/nicvf_ethtool.c | 29 +++-
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 173 ++++++++++++++++++++-
drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 26 ++++
drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 29 ++++
drivers/net/ethernet/cavium/thunder/thunder_bgx.h | 4 +
9 files changed, 330 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/cavium/Kconfig b/drivers/net/ethernet/cavium/Kconfig
index 96586c0b4490..043e3c11c42b 100644
--- a/drivers/net/ethernet/cavium/Kconfig
+++ b/drivers/net/ethernet/cavium/Kconfig
@@ -27,6 +27,7 @@ config THUNDER_NIC_PF

config THUNDER_NIC_VF
tristate "Thunder Virtual function driver"
+ imply CAVIUM_PTP
depends on 64BIT
---help---
This driver supports Thunder's NIC virtual function
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 4a02e618e318..204b234beb9d 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -263,6 +263,8 @@ struct nicvf_drv_stats {
struct u64_stats_sync syncp;
};

+struct cavium_ptp;
+
struct nicvf {
struct nicvf *pnicvf;
struct net_device *netdev;
@@ -312,6 +314,12 @@ struct nicvf {
struct tasklet_struct qs_err_task;
struct work_struct reset_task;

+ /* PTP timestamp */
+ struct cavium_ptp *ptp_clock;
+ bool hw_rx_tstamp;
+ struct sk_buff *ptp_skb;
+ atomic_t tx_ptp_skbs;
+
/* Interrupt coalescing settings */
u32 cq_coalesce_usecs;
u32 msg_enable;
@@ -371,6 +379,7 @@ struct nicvf {
#define NIC_MBOX_MSG_LOOPBACK 0x16 /* Set interface in loopback */
#define NIC_MBOX_MSG_RESET_STAT_COUNTER 0x17 /* Reset statistics counters */
#define NIC_MBOX_MSG_PFC 0x18 /* Pause frame control */
+#define NIC_MBOX_MSG_PTP_CFG 0x19 /* HW packet timestamp */
#define NIC_MBOX_MSG_CFG_DONE 0xF0 /* VF configuration done */
#define NIC_MBOX_MSG_SHUTDOWN 0xF1 /* VF is being shutdown */

@@ -521,6 +530,11 @@ struct pfc {
u8 fc_tx;
};

+struct set_ptp {
+ u8 msg;
+ bool enable;
+};
+
/* 128 bit shared memory between PF and each VF */
union nic_mbx {
struct { u8 msg; } msg;
@@ -540,6 +554,7 @@ union nic_mbx {
struct set_loopback lbk;
struct reset_stat_cfg reset_stat;
struct pfc pfc;
+ struct set_ptp ptp;
};

#define NIC_NODE_ID_MASK 0x03
diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 8f1dd55b3e08..4c1c5414a162 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -426,13 +426,22 @@ static void nic_init_hw(struct nicpf *nic)
/* Enable backpressure */
nic_reg_write(nic, NIC_PF_BP_CFG, (1ULL << 6) | 0x03);

- /* TNS and TNS bypass modes are present only on 88xx */
+ /* TNS and TNS bypass modes are present only on 88xx
+ * Also offset of this CSR has changed in 81xx and 83xx.
+ */
if (nic->pdev->subsystem_device == PCI_SUBSYS_DEVID_88XX_NIC_PF) {
/* Disable TNS mode on both interfaces */
nic_reg_write(nic, NIC_PF_INTF_0_1_SEND_CFG,
- (NIC_TNS_BYPASS_MODE << 7) | BGX0_BLOCK);
+ (NIC_TNS_BYPASS_MODE << 7) |
+ BGX0_BLOCK | (1ULL << 16));
nic_reg_write(nic, NIC_PF_INTF_0_1_SEND_CFG | (1 << 8),
- (NIC_TNS_BYPASS_MODE << 7) | BGX1_BLOCK);
+ (NIC_TNS_BYPASS_MODE << 7) |
+ BGX1_BLOCK | (1ULL << 16));
+ } else {
+ /* Configure timestamp generation timeout to 10us */
+ for (i = 0; i < nic->hw->bgx_cnt; i++)
+ nic_reg_write(nic, NIC_PF_INTFX_SEND_CFG | (i << 3),
+ (1ULL << 16));
}

nic_reg_write(nic, NIC_PF_INTF_0_1_BP_CFG,
@@ -880,6 +889,46 @@ static void nic_pause_frame(struct nicpf *nic, int vf, struct pfc *cfg)
}
}

+/* Enable or disable HW timestamping by BGX for pkts received on a LMAC */
+static void nic_config_timestamp(struct nicpf *nic, int vf, struct set_ptp *ptp)
+{
+ struct pkind_cfg *pkind;
+ u8 lmac, bgx_idx;
+ u64 pkind_val, pkind_idx;
+
+ if (vf >= nic->num_vf_en)
+ return;
+
+ bgx_idx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+ lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+
+ pkind_idx = lmac + bgx_idx * MAX_LMAC_PER_BGX;
+ pkind_val = nic_reg_read(nic, NIC_PF_PKIND_0_15_CFG | (pkind_idx << 3));
+ pkind = (struct pkind_cfg *)&pkind_val;
+
+ if (ptp->enable && !pkind->hdr_sl) {
+ /* Skiplen to exclude 8byte timestamp while parsing pkt
+ * If not configured, will result in L2 errors.
+ */
+ pkind->hdr_sl = 4;
+ /* Adjust max packet length allowed */
+ pkind->maxlen += (pkind->hdr_sl * 2);
+ bgx_config_timestamping(nic->node, bgx_idx, lmac, true);
+ nic_reg_write(nic,
+ NIC_PF_RX_ETYPE_0_7 | (1 << 3),
+ (ETYPE_ALG_ENDPARSE << 16) | ETH_P_1588);
+ } else if (!ptp->enable && pkind->hdr_sl) {
+ pkind->maxlen -= (pkind->hdr_sl * 2);
+ pkind->hdr_sl = 0;
+ bgx_config_timestamping(nic->node, bgx_idx, lmac, false);
+ nic_reg_write(nic,
+ NIC_PF_RX_ETYPE_0_7 | (1 << 3),
+ (1ULL << 16) | ETH_P_8021Q); /* reset value */
+ }
+
+ nic_reg_write(nic, NIC_PF_PKIND_0_15_CFG | (pkind_idx << 3), pkind_val);
+}
+
/* Interrupt handler to handle mailbox messages from VFs */
static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
{
@@ -1022,6 +1071,9 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
case NIC_MBOX_MSG_PFC:
nic_pause_frame(nic, vf, &mbx.pfc);
goto unlock;
+ case NIC_MBOX_MSG_PTP_CFG:
+ nic_config_timestamp(nic, vf, &mbx.ptp);
+ break;
default:
dev_err(&nic->pdev->dev,
"Invalid msg from VF%d, msg 0x%x\n", vf, mbx.msg.msg);
diff --git a/drivers/net/ethernet/cavium/thunder/nic_reg.h b/drivers/net/ethernet/cavium/thunder/nic_reg.h
index 80d46337cf29..a16c48a1ebb2 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_reg.h
+++ b/drivers/net/ethernet/cavium/thunder/nic_reg.h
@@ -99,6 +99,7 @@
#define NIC_PF_ECC3_DBE_INT_W1S (0x2708)
#define NIC_PF_ECC3_DBE_ENA_W1C (0x2710)
#define NIC_PF_ECC3_DBE_ENA_W1S (0x2718)
+#define NIC_PF_INTFX_SEND_CFG (0x4000)
#define NIC_PF_MCAM_0_191_ENA (0x100000)
#define NIC_PF_MCAM_0_191_M_0_5_DATA (0x110000)
#define NIC_PF_MCAM_CTRL (0x120000)
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c b/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
index b9ece9cbf98b..ed9f10bdf41e 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
@@ -9,12 +9,14 @@
/* ETHTOOL Support for VNIC_VF Device*/

#include <linux/pci.h>
+#include <linux/net_tstamp.h>

#include "nic_reg.h"
#include "nic.h"
#include "nicvf_queues.h"
#include "q_struct.h"
#include "thunder_bgx.h"
+#include "../common/cavium_ptp.h"

#define DRV_NAME "thunder-nicvf"
#define DRV_VERSION "1.0"
@@ -824,6 +826,31 @@ static int nicvf_set_pauseparam(struct net_device *dev,
return 0;
}

+static int nicvf_get_ts_info(struct net_device *netdev,
+ struct ethtool_ts_info *info)
+{
+ struct nicvf *nic = netdev_priv(netdev);
+
+ if (!nic->ptp_clock)
+ return ethtool_op_get_ts_info(netdev, info);
+
+ info->so_timestamping = SOF_TIMESTAMPING_TX_SOFTWARE |
+ SOF_TIMESTAMPING_RX_SOFTWARE |
+ SOF_TIMESTAMPING_SOFTWARE |
+ SOF_TIMESTAMPING_TX_HARDWARE |
+ SOF_TIMESTAMPING_RX_HARDWARE |
+ SOF_TIMESTAMPING_RAW_HARDWARE;
+
+ info->phc_index = cavium_ptp_clock_index(nic->ptp_clock);
+
+ info->tx_types = (1 << HWTSTAMP_TX_OFF) | (1 << HWTSTAMP_TX_ON);
+
+ info->rx_filters = (1 << HWTSTAMP_FILTER_NONE) |
+ (1 << HWTSTAMP_FILTER_ALL);
+
+ return 0;
+}
+
static const struct ethtool_ops nicvf_ethtool_ops = {
.get_link = nicvf_get_link,
.get_drvinfo = nicvf_get_drvinfo,
@@ -847,7 +874,7 @@ static const struct ethtool_ops nicvf_ethtool_ops = {
.set_channels = nicvf_set_channels,
.get_pauseparam = nicvf_get_pauseparam,
.set_pauseparam = nicvf_set_pauseparam,
- .get_ts_info = ethtool_op_get_ts_info,
+ .get_ts_info = nicvf_get_ts_info,
.get_link_ksettings = nicvf_get_link_ksettings,
};

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 52b3a6044f85..8e3338f31e54 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -20,11 +20,13 @@
#include <linux/bpf.h>
#include <linux/bpf_trace.h>
#include <linux/filter.h>
+#include <linux/net_tstamp.h>

#include "nic_reg.h"
#include "nic.h"
#include "nicvf_queues.h"
#include "thunder_bgx.h"
+#include "../common/cavium_ptp.h"

#define DRV_NAME "thunder-nicvf"
#define DRV_VERSION "1.0"
@@ -601,6 +603,44 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog,
return false;
}

+static void nicvf_snd_ptp_handler(struct net_device *netdev,
+ struct cqe_send_t *cqe_tx)
+{
+ struct nicvf *nic = netdev_priv(netdev);
+ struct skb_shared_hwtstamps ts;
+ u64 ns;
+
+ nic = nic->pnicvf;
+
+ /* Sync for 'ptp_skb' */
+ smp_rmb();
+
+ /* New timestamp request can be queued now */
+ atomic_set(&nic->tx_ptp_skbs, 0);
+
+ /* Check for timestamp requested skb */
+ if (!nic->ptp_skb)
+ return;
+
+ /* Check if timestamping is timedout, which is set to 10us */
+ if (cqe_tx->send_status == CQ_TX_ERROP_TSTMP_TIMEOUT ||
+ cqe_tx->send_status == CQ_TX_ERROP_TSTMP_CONFLICT)
+ goto no_tstamp;
+
+ /* Get the timestamp */
+ memset(&ts, 0, sizeof(ts));
+ ns = cavium_ptp_tstamp2time(nic->ptp_clock, cqe_tx->ptp_timestamp);
+ ts.hwtstamp = ns_to_ktime(ns);
+ skb_tstamp_tx(nic->ptp_skb, &ts);
+
+no_tstamp:
+ /* Free the original skb */
+ dev_kfree_skb_any(nic->ptp_skb);
+ nic->ptp_skb = NULL;
+ /* Sync 'ptp_skb' */
+ smp_wmb();
+}
+
static void nicvf_snd_pkt_handler(struct net_device *netdev,
struct cqe_send_t *cqe_tx,
int budget, int *subdesc_cnt,
@@ -657,7 +697,12 @@ static void nicvf_snd_pkt_handler(struct net_device *netdev,
prefetch(skb);
(*tx_pkts)++;
*tx_bytes += skb->len;
- napi_consume_skb(skb, budget);
+ /* If timestamp is requested for this skb, don't free it */
+ if (skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS &&
+ !nic->pnicvf->ptp_skb)
+ nic->pnicvf->ptp_skb = skb;
+ else
+ napi_consume_skb(skb, budget);
sq->skbuff[cqe_tx->sqe_ptr] = (u64)NULL;
} else {
/* In case of SW TSO on 88xx, only last segment will have
@@ -696,6 +741,21 @@ static inline void nicvf_set_rxhash(struct net_device *netdev,
skb_set_hash(skb, hash, hash_type);
}

+static inline void nicvf_set_rxtstamp(struct nicvf *nic, struct sk_buff *skb)
+{
+ u64 ns;
+
+ if (!nic->ptp_clock || !nic->hw_rx_tstamp)
+ return;
+
+ /* The first 8 bytes is the timestamp */
+ ns = cavium_ptp_tstamp2time(nic->ptp_clock,
+ be64_to_cpu(*(__be64 *)skb->data));
+ skb_hwtstamps(skb)->hwtstamp = ns_to_ktime(ns);
+
+ __skb_pull(skb, 8);
+}
+
static void nicvf_rcv_pkt_handler(struct net_device *netdev,
struct napi_struct *napi,
struct cqe_rx_t *cqe_rx, struct snd_queue *sq)
@@ -746,6 +806,7 @@ static void nicvf_rcv_pkt_handler(struct net_device *netdev,
return;
}

+ nicvf_set_rxtstamp(nic, skb);
nicvf_set_rxhash(netdev, cqe_rx, skb);

skb_record_rx_queue(skb, rq_idx);
@@ -820,10 +881,12 @@ static int nicvf_cq_intr_handler(struct net_device *netdev, u8 cq_idx,
&tx_pkts, &tx_bytes);
tx_done++;
break;
+ case CQE_TYPE_SEND_PTP:
+ nicvf_snd_ptp_handler(netdev, (void *)cq_desc);
+ break;
case CQE_TYPE_INVALID:
case CQE_TYPE_RX_SPLIT:
case CQE_TYPE_RX_TCP:
- case CQE_TYPE_SEND_PTP:
/* Ignore for now */
break;
}
@@ -1319,12 +1382,28 @@ int nicvf_stop(struct net_device *netdev)

nicvf_free_cq_poll(nic);

+ /* Free any pending SKB saved to receive timestamp */
+ if (nic->ptp_skb) {
+ dev_kfree_skb_any(nic->ptp_skb);
+ nic->ptp_skb = NULL;
+ }
+
/* Clear multiqset info */
nic->pnicvf = nic;

return 0;
}

+static int nicvf_config_hw_rx_tstamp(struct nicvf *nic, bool enable)
+{
+ union nic_mbx mbx = {};
+
+ mbx.ptp.msg = NIC_MBOX_MSG_PTP_CFG;
+ mbx.ptp.enable = enable;
+
+ return nicvf_send_msg_to_pf(nic, &mbx);
+}
+
static int nicvf_update_hw_max_frs(struct nicvf *nic, int mtu)
{
union nic_mbx mbx = {};
@@ -1394,6 +1473,12 @@ int nicvf_open(struct net_device *netdev)
if (nic->sqs_mode)
nicvf_get_primary_vf_struct(nic);

+ /* Configure PTP timestamp */
+ if (nic->ptp_clock)
+ nicvf_config_hw_rx_tstamp(nic, nic->hw_rx_tstamp);
+ atomic_set(&nic->tx_ptp_skbs, 0);
+ nic->ptp_skb = NULL;
+
/* Configure receive side scaling and MTU */
if (!nic->sqs_mode) {
nicvf_rss_init(nic);
@@ -1820,6 +1905,77 @@ static void nicvf_xdp_flush(struct net_device *dev)
return;
}

+static int nicvf_config_hwtstamp(struct net_device *netdev, struct ifreq *ifr)
+{
+ struct hwtstamp_config config;
+ struct nicvf *nic = netdev_priv(netdev);
+
+ if (!nic->ptp_clock)
+ return -ENODEV;
+
+ if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
+ return -EFAULT;
+
+ /* reserved for future extensions */
+ if (config.flags)
+ return -EINVAL;
+
+ switch (config.tx_type) {
+ case HWTSTAMP_TX_OFF:
+ case HWTSTAMP_TX_ON:
+ break;
+ default:
+ return -ERANGE;
+ }
+
+ switch (config.rx_filter) {
+ case HWTSTAMP_FILTER_NONE:
+ nic->hw_rx_tstamp = false;
+ break;
+ case HWTSTAMP_FILTER_ALL:
+ case HWTSTAMP_FILTER_SOME:
+ case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
+ case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
+ case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
+ case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
+ case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
+ case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
+ case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
+ case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
+ case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
+ case HWTSTAMP_FILTER_PTP_V2_EVENT:
+ case HWTSTAMP_FILTER_PTP_V2_SYNC:
+ case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
+ nic->hw_rx_tstamp = true;
+ config.rx_filter = HWTSTAMP_FILTER_ALL;
+ break;
+ default:
+ return -ERANGE;
+ }
+
+ if (netif_running(netdev)) {
+ if (nic->hw_rx_tstamp)
+ nicvf_config_hw_rx_tstamp(nic, true);
+ else
+ nicvf_config_hw_rx_tstamp(nic, false);
+ }
+
+ if (copy_to_user(ifr->ifr_data, &config, sizeof(config)))
+ return -EFAULT;
+
+ return 0;
+}
+
+static int nicvf_ioctl(struct net_device *netdev, struct ifreq *req, int cmd)
+{
+ switch (cmd) {
+ case SIOCSHWTSTAMP:
+ return nicvf_config_hwtstamp(netdev, req);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
static const struct net_device_ops nicvf_netdev_ops = {
.ndo_open = nicvf_open,
.ndo_stop = nicvf_stop,
@@ -1833,6 +1989,7 @@ static const struct net_device_ops nicvf_netdev_ops = {
.ndo_bpf = nicvf_xdp,
.ndo_xdp_xmit = nicvf_xdp_xmit,
.ndo_xdp_flush = nicvf_xdp_flush,
+ .ndo_do_ioctl = nicvf_ioctl,
};

static int nicvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
@@ -1842,6 +1999,16 @@ static int nicvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
struct nicvf *nic;
int err, qcount;
u16 sdevid;
+ struct cavium_ptp *ptp_clock;
+
+ ptp_clock = cavium_ptp_get();
+ if (IS_ERR(ptp_clock)) {
+ if (PTR_ERR(ptp_clock) == -ENODEV)
+ /* In virtualized environment we proceed without ptp */
+ ptp_clock = NULL;
+ else
+ return PTR_ERR(ptp_clock);
+ }

err = pci_enable_device(pdev);
if (err) {
@@ -1896,6 +2063,7 @@ static int nicvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
*/
if (!nic->t88)
nic->max_queues *= 2;
+ nic->ptp_clock = ptp_clock;

/* MAP VF's configuration registers */
nic->reg_base = pcim_iomap(pdev, PCI_CFG_REG_BAR_NUM, 0);
@@ -2009,6 +2177,7 @@ static void nicvf_remove(struct pci_dev *pdev)
pci_set_drvdata(pdev, NULL);
if (nic->drv_stats)
free_percpu(nic->drv_stats);
+ cavium_ptp_put(nic->ptp_clock);
free_netdev(netdev);
pci_release_regions(pdev);
pci_disable_device(pdev);
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
index f38ea349aa00..853fbeb84bd2 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
@@ -978,6 +978,9 @@ void nicvf_qset_config(struct nicvf *nic, bool enable)
qs_cfg->be = 1;
#endif
qs_cfg->vnic = qs->vnic_id;
+ /* Enable Tx timestamping capability */
+ if (nic->ptp_clock)
+ qs_cfg->send_tstmp_ena = 1;
}
nicvf_send_msg_to_pf(nic, &mbx);
}
@@ -1385,6 +1388,29 @@ nicvf_sq_add_hdr_subdesc(struct nicvf *nic, struct snd_queue *sq, int qentry,
hdr->inner_l3_offset = skb_network_offset(skb) - 2;
this_cpu_inc(nic->pnicvf->drv_stats->tx_tso);
}
+
+ /* Check if timestamp is requested */
+ if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
+ skb_tx_timestamp(skb);
+ return;
+ }
+
+ /* Tx timestamping not supported along with TSO, so ignore request */
+ if (skb_shinfo(skb)->gso_size)
+ return;
+
+ /* HW supports only a single outstanding packet to timestamp */
+ if (!atomic_add_unless(&nic->pnicvf->tx_ptp_skbs, 1, 1))
+ return;
+
+ /* Mark the SKB for later reference */
+ skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
+
+ /* Finally enable timestamp generation
+ * Since 'post_cqe' is also set, two CQEs will be posted
+ * for this packet i.e CQE_TYPE_SEND and CQE_TYPE_SEND_PTP.
+ */
+ hdr->tstmp = 1;
}

/* SQ GATHER subdescriptor
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
index 5e5c4d7796b8..0f23999c5bcf 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
@@ -245,6 +245,35 @@ void bgx_lmac_rx_tx_enable(int node, int bgx_idx, int lmacid, bool enable)
}
EXPORT_SYMBOL(bgx_lmac_rx_tx_enable);

+/* Enables or disables timestamp insertion by BGX for Rx packets */
+void bgx_config_timestamping(int node, int bgx_idx, int lmacid, bool enable)
+{
+ struct bgx *bgx = get_bgx(node, bgx_idx);
+ struct lmac *lmac;
+ u64 csr_offset, cfg;
+
+ if (!bgx)
+ return;
+
+ lmac = &bgx->lmac[lmacid];
+
+ if (lmac->lmac_type == BGX_MODE_SGMII ||
+ lmac->lmac_type == BGX_MODE_QSGMII ||
+ lmac->lmac_type == BGX_MODE_RGMII)
+ csr_offset = BGX_GMP_GMI_RXX_FRM_CTL;
+ else
+ csr_offset = BGX_SMUX_RX_FRM_CTL;
+
+ cfg = bgx_reg_read(bgx, lmacid, csr_offset);
+
+ if (enable)
+ cfg |= BGX_PKT_RX_PTP_EN;
+ else
+ cfg &= ~BGX_PKT_RX_PTP_EN;
+ bgx_reg_write(bgx, lmacid, csr_offset, cfg);
+}
+EXPORT_SYMBOL(bgx_config_timestamping);
+
void bgx_lmac_get_pfc(int node, int bgx_idx, int lmacid, void *pause)
{
struct pfc *pfc = (struct pfc *)pause;
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.h b/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
index 23acdc5ab896..5a7567d31138 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
@@ -122,6 +122,8 @@
#define SPU_DBG_CTL_AN_NONCE_MCT_DIS BIT_ULL(29)

#define BGX_SMUX_RX_INT 0x20000
+#define BGX_SMUX_RX_FRM_CTL 0x20020
+#define BGX_PKT_RX_PTP_EN BIT_ULL(12)
#define BGX_SMUX_RX_JABBER 0x20030
#define BGX_SMUX_RX_CTL 0x20048
#define SMU_RX_CTL_STATUS (3ull << 0)
@@ -172,6 +174,7 @@
#define GMI_PORT_CFG_SPEED_MSB BIT_ULL(8)
#define GMI_PORT_CFG_RX_IDLE BIT_ULL(12)
#define GMI_PORT_CFG_TX_IDLE BIT_ULL(13)
+#define BGX_GMP_GMI_RXX_FRM_CTL 0x38028
#define BGX_GMP_GMI_RXX_JABBER 0x38038
#define BGX_GMP_GMI_TXX_THRESH 0x38210
#define BGX_GMP_GMI_TXX_APPEND 0x38218
@@ -223,6 +226,7 @@ void bgx_set_lmac_mac(int node, int bgx_idx, int lmacid, const u8 *mac);
void bgx_get_lmac_link_state(int node, int bgx_idx, int lmacid, void *status);
void bgx_lmac_internal_loopback(int node, int bgx_idx,
int lmac_idx, bool enable);
+void bgx_config_timestamping(int node, int bgx_idx, int lmacid, bool enable);
void bgx_lmac_get_pfc(int node, int bgx_idx, int lmacid, void *pause);
void bgx_lmac_set_pfc(int node, int bgx_idx, int lmacid, void *pause);

--
2.15.1

2017-12-11 14:15:21

by Aleksey Makarov

[permalink] [raw]
Subject: [PATCH net-next v5 1/2] net: add support for Cavium PTP coprocessor

From: Radoslaw Biernacki <[email protected]>

This patch adds support for the Precision Time Protocol
Clocks and Timestamping hardware found on Cavium ThunderX
processors.

Signed-off-by: Radoslaw Biernacki <[email protected]>
Signed-off-by: Aleksey Makarov <[email protected]>
---
drivers/net/ethernet/cavium/Kconfig | 12 +
drivers/net/ethernet/cavium/Makefile | 1 +
drivers/net/ethernet/cavium/common/Makefile | 1 +
drivers/net/ethernet/cavium/common/cavium_ptp.c | 329 ++++++++++++++++++++++++
drivers/net/ethernet/cavium/common/cavium_ptp.h | 70 +++++
5 files changed, 413 insertions(+)
create mode 100644 drivers/net/ethernet/cavium/common/Makefile
create mode 100644 drivers/net/ethernet/cavium/common/cavium_ptp.c
create mode 100644 drivers/net/ethernet/cavium/common/cavium_ptp.h

diff --git a/drivers/net/ethernet/cavium/Kconfig b/drivers/net/ethernet/cavium/Kconfig
index 63be75eb34d2..96586c0b4490 100644
--- a/drivers/net/ethernet/cavium/Kconfig
+++ b/drivers/net/ethernet/cavium/Kconfig
@@ -50,6 +50,18 @@ config THUNDER_NIC_RGX
This driver supports configuring XCV block of RGX interface
present on CN81XX chip.

+config CAVIUM_PTP
+ tristate "Cavium PTP coprocessor as PTP clock"
+ depends on 64BIT
+ imply PTP_1588_CLOCK
+ default y
+ ---help---
+ This driver adds support for the Precision Time Protocol Clocks and
+ Timestamping coprocessor (PTP) found on Cavium processors.
+ PTP provides timestamping mechanism that is suitable for use in IEEE 1588
+ Precision Time Protocol or other purposes. Timestamps can be used in
+ BGX, TNS, GTI, and NIC blocks.
+
config LIQUIDIO
tristate "Cavium LiquidIO support"
depends on 64BIT
diff --git a/drivers/net/ethernet/cavium/Makefile b/drivers/net/ethernet/cavium/Makefile
index 872da9f7c31a..946bba84e81d 100644
--- a/drivers/net/ethernet/cavium/Makefile
+++ b/drivers/net/ethernet/cavium/Makefile
@@ -1,6 +1,7 @@
#
# Makefile for the Cavium ethernet device drivers.
#
+obj-$(CONFIG_NET_VENDOR_CAVIUM) += common/
obj-$(CONFIG_NET_VENDOR_CAVIUM) += thunder/
obj-$(CONFIG_NET_VENDOR_CAVIUM) += liquidio/
obj-$(CONFIG_NET_VENDOR_CAVIUM) += octeon/
diff --git a/drivers/net/ethernet/cavium/common/Makefile b/drivers/net/ethernet/cavium/common/Makefile
new file mode 100644
index 000000000000..dd8561b8060b
--- /dev/null
+++ b/drivers/net/ethernet/cavium/common/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_CAVIUM_PTP) += cavium_ptp.o
diff --git a/drivers/net/ethernet/cavium/common/cavium_ptp.c b/drivers/net/ethernet/cavium/common/cavium_ptp.c
new file mode 100644
index 000000000000..8b48863bfdec
--- /dev/null
+++ b/drivers/net/ethernet/cavium/common/cavium_ptp.c
@@ -0,0 +1,329 @@
+// SPDX-License-Identifier: GPL-2.0
+/* cavium_ptp.c - PTP 1588 clock on Cavium hardware
+ * Copyright (c) 2003-2015, 2017 Cavium, Inc.
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/timecounter.h>
+#include <linux/pci.h>
+
+#include "cavium_ptp.h"
+
+#define DRV_NAME "Cavium PTP Driver"
+
+#define PCI_DEVICE_ID_CAVIUM_PTP 0xA00C
+#define PCI_DEVICE_ID_CAVIUM_RST 0xA00E
+
+#define PCI_PTP_BAR_NO 0
+#define PCI_RST_BAR_NO 0
+
+#define PTP_CLOCK_CFG 0xF00ULL
+#define PTP_CLOCK_CFG_PTP_EN BIT(0)
+#define PTP_CLOCK_LO 0xF08ULL
+#define PTP_CLOCK_HI 0xF10ULL
+#define PTP_CLOCK_COMP 0xF18ULL
+
+#define RST_BOOT 0x1600ULL
+#define CLOCK_BASE_RATE 50000000ULL
+
+static u64 ptp_cavium_clock_get(void)
+{
+ struct pci_dev *pdev;
+ void __iomem *base;
+ u64 ret = CLOCK_BASE_RATE * 16;
+
+ pdev = pci_get_device(PCI_VENDOR_ID_CAVIUM,
+ PCI_DEVICE_ID_CAVIUM_RST, NULL);
+ if (!pdev)
+ goto error;
+
+ base = pci_ioremap_bar(pdev, PCI_RST_BAR_NO);
+ if (!base)
+ goto error_put_pdev;
+
+ ret = CLOCK_BASE_RATE * ((readq(base + RST_BOOT) >> 33) & 0x3f);
+
+ iounmap(base);
+
+error_put_pdev:
+ pci_dev_put(pdev);
+
+error:
+ return ret;
+}
+
+struct cavium_ptp *cavium_ptp_get(void)
+{
+ struct cavium_ptp *ptp;
+ struct pci_dev *pdev;
+
+ pdev = pci_get_device(PCI_VENDOR_ID_CAVIUM,
+ PCI_DEVICE_ID_CAVIUM_PTP, NULL);
+ if (!pdev)
+ return ERR_PTR(-ENODEV);
+
+ ptp = pci_get_drvdata(pdev);
+ if (!ptp) {
+ pci_dev_put(pdev);
+ ptp = ERR_PTR(-EPROBE_DEFER);
+ } else if (!ptp->ptp_clock) {
+ pci_dev_put(pdev);
+ ptp = ERR_PTR(-ENODEV);
+ }
+
+ return ptp;
+}
+EXPORT_SYMBOL(cavium_ptp_get);
+
+void cavium_ptp_put(struct cavium_ptp *ptp)
+{
+ pci_dev_put(ptp->pdev);
+}
+EXPORT_SYMBOL(cavium_ptp_put);
+
+/**
+ * cavium_ptp_adjfreq() - Adjust ptp frequency
+ * @ptp: PTP clock info
+ * @ppb: how much to adjust by, in parts-per-billion
+ */
+static int cavium_ptp_adjfreq(struct ptp_clock_info *ptp_info, s32 ppb)
+{
+ struct cavium_ptp *clock =
+ container_of(ptp_info, struct cavium_ptp, ptp_info);
+ unsigned long flags;
+ u64 comp;
+ u64 adj;
+ bool neg_adj = false;
+
+ if (ppb < 0) {
+ neg_adj = true;
+ ppb = -ppb;
+ }
+
+ /* The hardware adds the clock compensation value to the PTP clock
+ * on every coprocessor clock cycle. Typical convention is that it
+ * represent number of nanosecond betwen each cycle. In this
+ * convention compensation value is in 64 bit fixed-point
+ * representation where upper 32 bits are number of nanoseconds
+ * and lower is fractions of nanosecond.
+ * The ppb represent the ratio in "parts per bilion" by which the
+ * compensation value should be corrected.
+ * To calculate new compenstation value we use 64bit fixed point
+ * arithmetic on following formula comp = tbase + tbase * ppb / 1G
+ * where tbase is the basic compensation value calculated initialy
+ * in cavium_ptp_init() -> tbase = 1/Hz. Then we use endian
+ * independent structure definition to write data to PTP register.
+ */
+ comp = ((u64)1000000000ull << 32) / clock->clock_rate;
+ adj = comp * ppb;
+ adj = div_u64(adj, 1000000000ull);
+ comp = neg_adj ? comp - adj : comp + adj;
+
+ spin_lock_irqsave(&clock->spin_lock, flags);
+ writeq(comp, clock->reg_base + PTP_CLOCK_COMP);
+ spin_unlock_irqrestore(&clock->spin_lock, flags);
+
+ return 0;
+}
+
+/**
+ * cavium_ptp_adjtime() - Adjust ptp time
+ * @ptp: PTP clock info
+ * @delta: how much to adjust by, in nanosecs
+ */
+static int cavium_ptp_adjtime(struct ptp_clock_info *ptp_info, s64 delta)
+{
+ struct cavium_ptp *clock =
+ container_of(ptp_info, struct cavium_ptp, ptp_info);
+ unsigned long flags;
+
+ spin_lock_irqsave(&clock->spin_lock, flags);
+ timecounter_adjtime(&clock->time_counter, delta);
+ spin_unlock_irqrestore(&clock->spin_lock, flags);
+
+ /* Sync, for network driver to get latest value */
+ smp_mb();
+
+ return 0;
+}
+
+/**
+ * cavium_ptp_gettime() - Get hardware clock time with adjustment
+ * @ptp: PTP clock info
+ * @ts: timespec
+ */
+static int cavium_ptp_gettime(struct ptp_clock_info *ptp_info,
+ struct timespec64 *ts)
+{
+ struct cavium_ptp *clock =
+ container_of(ptp_info, struct cavium_ptp, ptp_info);
+ unsigned long flags;
+ u64 nsec;
+
+ spin_lock_irqsave(&clock->spin_lock, flags);
+ nsec = timecounter_read(&clock->time_counter);
+ spin_unlock_irqrestore(&clock->spin_lock, flags);
+
+ *ts = ns_to_timespec64(nsec);
+
+ return 0;
+}
+
+/**
+ * cavium_ptp_settime() - Set hardware clock time. Reset adjustment
+ * @ptp: PTP clock info
+ * @ts: timespec
+ */
+static int cavium_ptp_settime(struct ptp_clock_info *ptp_info,
+ const struct timespec64 *ts)
+{
+ struct cavium_ptp *clock =
+ container_of(ptp_info, struct cavium_ptp, ptp_info);
+ unsigned long flags;
+ u64 nsec;
+
+ nsec = timespec64_to_ns(ts);
+
+ spin_lock_irqsave(&clock->spin_lock, flags);
+ timecounter_init(&clock->time_counter, &clock->cycle_counter, nsec);
+ spin_unlock_irqrestore(&clock->spin_lock, flags);
+
+ return 0;
+}
+
+/**
+ * cavium_ptp_enable() - Check if PTP is enabled
+ * @ptp: PTP clock info
+ * @rq: request
+ * @on: is it on
+ */
+static int cavium_ptp_enable(struct ptp_clock_info *ptp_info,
+ struct ptp_clock_request *rq, int on)
+{
+ return -EOPNOTSUPP;
+}
+
+static u64 cavium_ptp_cc_read(const struct cyclecounter *cc)
+{
+ struct cavium_ptp *clock =
+ container_of(cc, struct cavium_ptp, cycle_counter);
+
+ return readq(clock->reg_base + PTP_CLOCK_HI);
+}
+
+static int cavium_ptp_probe(struct pci_dev *pdev,
+ const struct pci_device_id *ent)
+{
+ struct device *dev = &pdev->dev;
+ struct cavium_ptp *clock;
+ struct cyclecounter *cc;
+ u64 clock_cfg;
+ u64 clock_comp;
+ int err;
+
+ clock = devm_kzalloc(dev, sizeof(*clock), GFP_KERNEL);
+ if (!clock)
+ return -ENOMEM;
+
+ clock->pdev = pdev;
+
+ err = pcim_enable_device(pdev);
+ if (err)
+ return err;
+
+ err = pcim_iomap_regions(pdev, 1 << PCI_PTP_BAR_NO, pci_name(pdev));
+ if (err)
+ return err;
+
+ clock->reg_base = pcim_iomap_table(pdev)[PCI_PTP_BAR_NO];
+
+ spin_lock_init(&clock->spin_lock);
+
+ cc = &clock->cycle_counter;
+ cc->read = cavium_ptp_cc_read;
+ cc->mask = CYCLECOUNTER_MASK(64);
+ cc->mult = 1;
+ cc->shift = 0;
+
+ timecounter_init(&clock->time_counter, &clock->cycle_counter,
+ ktime_to_ns(ktime_get_real()));
+
+ clock->clock_rate = ptp_cavium_clock_get();
+
+ clock->ptp_info = (struct ptp_clock_info) {
+ .owner = THIS_MODULE,
+ .name = "ThunderX PTP",
+ .max_adj = 1000000000ull,
+ .n_ext_ts = 0,
+ .n_pins = 0,
+ .pps = 0,
+ .adjfreq = cavium_ptp_adjfreq,
+ .adjtime = cavium_ptp_adjtime,
+ .gettime64 = cavium_ptp_gettime,
+ .settime64 = cavium_ptp_settime,
+ .enable = cavium_ptp_enable,
+ };
+
+ clock_cfg = readq(clock->reg_base + PTP_CLOCK_CFG);
+ clock_cfg |= PTP_CLOCK_CFG_PTP_EN;
+ writeq(clock_cfg, clock->reg_base + PTP_CLOCK_CFG);
+
+ clock_comp = ((u64)1000000000ull << 32) / clock->clock_rate;
+ writeq(clock_comp, clock->reg_base + PTP_CLOCK_COMP);
+
+ clock->ptp_clock = ptp_clock_register(&clock->ptp_info, dev);
+ if (IS_ERR(clock->ptp_clock)) {
+ clock_cfg = readq(clock->reg_base + PTP_CLOCK_CFG);
+ clock_cfg &= ~PTP_CLOCK_CFG_PTP_EN;
+ writeq(clock_cfg, clock->reg_base + PTP_CLOCK_CFG);
+ return PTR_ERR(clock->ptp_clock);
+ }
+
+ pci_set_drvdata(pdev, clock);
+ return 0;
+}
+
+static void cavium_ptp_remove(struct pci_dev *pdev)
+{
+ struct cavium_ptp *clock = pci_get_drvdata(pdev);
+ u64 clock_cfg;
+
+ pci_set_drvdata(pdev, NULL);
+
+ ptp_clock_unregister(clock->ptp_clock);
+
+ clock_cfg = readq(clock->reg_base + PTP_CLOCK_CFG);
+ clock_cfg &= ~PTP_CLOCK_CFG_PTP_EN;
+ writeq(clock_cfg, clock->reg_base + PTP_CLOCK_CFG);
+}
+
+static const struct pci_device_id cavium_ptp_id_table[] = {
+ { PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, PCI_DEVICE_ID_CAVIUM_PTP) },
+ { 0, }
+};
+
+static struct pci_driver cavium_ptp_driver = {
+ .name = DRV_NAME,
+ .id_table = cavium_ptp_id_table,
+ .probe = cavium_ptp_probe,
+ .remove = cavium_ptp_remove,
+};
+
+static int __init cavium_ptp_init_module(void)
+{
+ return pci_register_driver(&cavium_ptp_driver);
+}
+
+static void __exit cavium_ptp_cleanup_module(void)
+{
+ pci_unregister_driver(&cavium_ptp_driver);
+}
+
+module_init(cavium_ptp_init_module);
+module_exit(cavium_ptp_cleanup_module);
+
+MODULE_DESCRIPTION(DRV_NAME);
+MODULE_AUTHOR("Cavium Networks <[email protected]>");
+MODULE_LICENSE("GPL v2");
+MODULE_DEVICE_TABLE(pci, cavium_ptp_id_table);
diff --git a/drivers/net/ethernet/cavium/common/cavium_ptp.h b/drivers/net/ethernet/cavium/common/cavium_ptp.h
new file mode 100644
index 000000000000..be2bafc7beeb
--- /dev/null
+++ b/drivers/net/ethernet/cavium/common/cavium_ptp.h
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+/* cavium_ptp.h - PTP 1588 clock on Cavium hardware
+ * Copyright (c) 2003-2015, 2017 Cavium, Inc.
+ */
+
+#ifndef CAVIUM_PTP_H
+#define CAVIUM_PTP_H
+
+#include <linux/ptp_clock_kernel.h>
+#include <linux/timecounter.h>
+
+struct cavium_ptp {
+ struct pci_dev *pdev;
+
+ /* Serialize access to cycle_counter, time_counter and hw_registers */
+ spinlock_t spin_lock;
+ struct cyclecounter cycle_counter;
+ struct timecounter time_counter;
+ void __iomem *reg_base;
+
+ u32 clock_rate;
+
+ struct ptp_clock_info ptp_info;
+ struct ptp_clock *ptp_clock;
+};
+
+#if IS_ENABLED(CONFIG_CAVIUM_PTP)
+
+struct cavium_ptp *cavium_ptp_get(void);
+void cavium_ptp_put(struct cavium_ptp *ptp);
+
+static inline u64 cavium_ptp_tstamp2time(struct cavium_ptp *ptp, u64 tstamp)
+{
+ unsigned long flags;
+ u64 ret;
+
+ spin_lock_irqsave(&ptp->spin_lock, flags);
+ ret = timecounter_cyc2time(&ptp->time_counter, tstamp);
+ spin_unlock_irqrestore(&ptp->spin_lock, flags);
+
+ return ret;
+}
+
+static inline int cavium_ptp_clock_index(struct cavium_ptp *clock)
+{
+ return ptp_clock_index(clock->ptp_clock);
+}
+
+#else
+
+static inline struct cavium_ptp *cavium_ptp_get(void)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline void cavium_ptp_put(struct cavium_ptp *ptp) {}
+
+static inline u64 cavium_ptp_tstamp2time(struct cavium_ptp *ptp, u64 tstamp)
+{
+ return 0;
+}
+
+static inline int cavium_ptp_clock_index(struct cavium_ptp *clock)
+{
+ return -1;
+}
+
+#endif
+
+#endif
--
2.15.1

2017-12-11 14:34:02

by Philippe Ombredanne

[permalink] [raw]
Subject: Re: [PATCH net-next v5 0/2] net: thunderx: add support for PTP clock

Aleksey,

On Mon, Dec 11, 2017 at 3:14 PM, Aleksey Makarov
<[email protected]> wrote:
> This series adds support for IEEE 1588 Precision Time Protocol
> to Cavium ethernet driver.
>
> The first patch adds support for the Precision Time Protocol Clocks and
> Timestamping coprocessor (PTP) found on Cavium processors.
> It registers a new PTP clock in the PTP core and provides functions
> to use the counter in BGX, TNS, GTI, and NIC blocks.
>
> The second patch introduces support for the PTP protocol to the
> Cavium ThunderX ethernet driver.
>
> v5:
> - fix the file headers (add SPDX tags, remove advertisment) (Philippe Ombredanne)

Thank you.

Acked-by: Philippe Ombredanne <[email protected]>

--
Cordially
Philippe Ombredanne

2017-12-11 23:00:00

by Richard Cochran

[permalink] [raw]
Subject: Re: [PATCH net-next v5 1/2] net: add support for Cavium PTP coprocessor


Sorry I didn't finish reviewing before...

On Mon, Dec 11, 2017 at 05:14:30PM +0300, Aleksey Makarov wrote:
> +/**
> + * cavium_ptp_adjfreq() - Adjust ptp frequency
> + * @ptp: PTP clock info
> + * @ppb: how much to adjust by, in parts-per-billion
> + */
> +static int cavium_ptp_adjfreq(struct ptp_clock_info *ptp_info, s32 ppb)

adjfreq() is deprecated. See ptp_clock_kernel.h. Please re-work this
to implement the adjfine() method instead.

> +/**
> + * cavium_ptp_enable() - Check if PTP is enabled

Nit - comment is not correct. This method is for the auxiliary PHC
functions.

> + * @ptp: PTP clock info
> + * @rq: request
> + * @on: is it on
> + */
> +static int cavium_ptp_enable(struct ptp_clock_info *ptp_info,
> + struct ptp_clock_request *rq, int on)
> +{
> + return -EOPNOTSUPP;
> +}

...

> +static int cavium_ptp_probe(struct pci_dev *pdev,
> + const struct pci_device_id *ent)
> +{
> + struct device *dev = &pdev->dev;
> + struct cavium_ptp *clock;
> + struct cyclecounter *cc;
> + u64 clock_cfg;
> + u64 clock_comp;
> + int err;
> +
> + clock = devm_kzalloc(dev, sizeof(*clock), GFP_KERNEL);
> + if (!clock)
> + return -ENOMEM;
> +
> + clock->pdev = pdev;
> +
> + err = pcim_enable_device(pdev);
> + if (err)
> + return err;
> +
> + err = pcim_iomap_regions(pdev, 1 << PCI_PTP_BAR_NO, pci_name(pdev));
> + if (err)
> + return err;
> +
> + clock->reg_base = pcim_iomap_table(pdev)[PCI_PTP_BAR_NO];
> +
> + spin_lock_init(&clock->spin_lock);
> +
> + cc = &clock->cycle_counter;
> + cc->read = cavium_ptp_cc_read;
> + cc->mask = CYCLECOUNTER_MASK(64);
> + cc->mult = 1;
> + cc->shift = 0;
> +
> + timecounter_init(&clock->time_counter, &clock->cycle_counter,
> + ktime_to_ns(ktime_get_real()));
> +
> + clock->clock_rate = ptp_cavium_clock_get();
> +
> + clock->ptp_info = (struct ptp_clock_info) {
> + .owner = THIS_MODULE,
> + .name = "ThunderX PTP",
> + .max_adj = 1000000000ull,
> + .n_ext_ts = 0,
> + .n_pins = 0,
> + .pps = 0,
> + .adjfreq = cavium_ptp_adjfreq,
> + .adjtime = cavium_ptp_adjtime,
> + .gettime64 = cavium_ptp_gettime,
> + .settime64 = cavium_ptp_settime,
> + .enable = cavium_ptp_enable,
> + };
> +
> + clock_cfg = readq(clock->reg_base + PTP_CLOCK_CFG);
> + clock_cfg |= PTP_CLOCK_CFG_PTP_EN;
> + writeq(clock_cfg, clock->reg_base + PTP_CLOCK_CFG);
> +
> + clock_comp = ((u64)1000000000ull << 32) / clock->clock_rate;
> + writeq(clock_comp, clock->reg_base + PTP_CLOCK_COMP);
> +
> + clock->ptp_clock = ptp_clock_register(&clock->ptp_info, dev);
> + if (IS_ERR(clock->ptp_clock)) {

You need to handle the case when ptp_clock_register() returns NULL.

from ptp_clock_kernel.h:

/**
* ptp_clock_register() - register a PTP hardware clock driver
*
* @info: Structure describing the new clock.
* @parent: Pointer to the parent device of the new clock.
*
* Returns a valid pointer on success or PTR_ERR on failure. If PHC
* support is missing at the configuration level, this function
* returns NULL, and drivers are expected to gracefully handle that
* case separately.
*/

> + clock_cfg = readq(clock->reg_base + PTP_CLOCK_CFG);
> + clock_cfg &= ~PTP_CLOCK_CFG_PTP_EN;
> + writeq(clock_cfg, clock->reg_base + PTP_CLOCK_CFG);
> + return PTR_ERR(clock->ptp_clock);
> + }
> +
> + pci_set_drvdata(pdev, clock);
> + return 0;
> +}

Thanks,
Richard

2017-12-11 23:32:27

by Richard Cochran

[permalink] [raw]
Subject: Re: [PATCH net-next v5 2/2] net: thunderx: add timestamping support

On Mon, Dec 11, 2017 at 05:14:31PM +0300, Aleksey Makarov wrote:
> diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
> index 4a02e618e318..204b234beb9d 100644
> --- a/drivers/net/ethernet/cavium/thunder/nic.h
> +++ b/drivers/net/ethernet/cavium/thunder/nic.h
> @@ -263,6 +263,8 @@ struct nicvf_drv_stats {
> struct u64_stats_sync syncp;
> };
>
> +struct cavium_ptp;
> +
> struct nicvf {
> struct nicvf *pnicvf;
> struct net_device *netdev;
> @@ -312,6 +314,12 @@ struct nicvf {
> struct tasklet_struct qs_err_task;
> struct work_struct reset_task;
>
> + /* PTP timestamp */
> + struct cavium_ptp *ptp_clock;
> + bool hw_rx_tstamp;
> + struct sk_buff *ptp_skb;
> + atomic_t tx_ptp_skbs;

It is disturbing that the above two fields are set in different
places. Shouldn't they be unified into one logical lock?

Here you clear them together:

> +static void nicvf_snd_ptp_handler(struct net_device *netdev,
> + struct cqe_send_t *cqe_tx)
> +{
> + struct nicvf *nic = netdev_priv(netdev);
> + struct skb_shared_hwtstamps ts;
> + u64 ns;
> +
> + nic = nic->pnicvf;
> +
> + /* Sync for 'ptp_skb' */
> + smp_rmb();
> +
> + /* New timestamp request can be queued now */
> + atomic_set(&nic->tx_ptp_skbs, 0);
> +
> + /* Check for timestamp requested skb */
> + if (!nic->ptp_skb)
> + return;
> +
> + /* Check if timestamping is timedout, which is set to 10us */
> + if (cqe_tx->send_status == CQ_TX_ERROP_TSTMP_TIMEOUT ||
> + cqe_tx->send_status == CQ_TX_ERROP_TSTMP_CONFLICT)
> + goto no_tstamp;
> +
> + /* Get the timestamp */
> + memset(&ts, 0, sizeof(ts));
> + ns = cavium_ptp_tstamp2time(nic->ptp_clock, cqe_tx->ptp_timestamp);
> + ts.hwtstamp = ns_to_ktime(ns);
> + skb_tstamp_tx(nic->ptp_skb, &ts);
> +
> +no_tstamp:
> + /* Free the original skb */
> + dev_kfree_skb_any(nic->ptp_skb);
> + nic->ptp_skb = NULL;
> + /* Sync 'ptp_skb' */
> + smp_wmb();
> +}
> +

but here you set the one:

> @@ -657,7 +697,12 @@ static void nicvf_snd_pkt_handler(struct net_device *netdev,
> prefetch(skb);
> (*tx_pkts)++;
> *tx_bytes += skb->len;
> - napi_consume_skb(skb, budget);
> + /* If timestamp is requested for this skb, don't free it */
> + if (skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS &&
> + !nic->pnicvf->ptp_skb)
> + nic->pnicvf->ptp_skb = skb;
> + else
> + napi_consume_skb(skb, budget);
> sq->skbuff[cqe_tx->sqe_ptr] = (u64)NULL;
> } else {
> /* In case of SW TSO on 88xx, only last segment will have

here you clear one:

> @@ -1319,12 +1382,28 @@ int nicvf_stop(struct net_device *netdev)
>
> nicvf_free_cq_poll(nic);
>
> + /* Free any pending SKB saved to receive timestamp */
> + if (nic->ptp_skb) {
> + dev_kfree_skb_any(nic->ptp_skb);
> + nic->ptp_skb = NULL;
> + }
> +
> /* Clear multiqset info */
> nic->pnicvf = nic;
>
> return 0;
> }

here you clear both:

> @@ -1394,6 +1473,12 @@ int nicvf_open(struct net_device *netdev)
> if (nic->sqs_mode)
> nicvf_get_primary_vf_struct(nic);
>
> + /* Configure PTP timestamp */
> + if (nic->ptp_clock)
> + nicvf_config_hw_rx_tstamp(nic, nic->hw_rx_tstamp);
> + atomic_set(&nic->tx_ptp_skbs, 0);
> + nic->ptp_skb = NULL;
> +
> /* Configure receive side scaling and MTU */
> if (!nic->sqs_mode) {
> nicvf_rss_init(nic);

here you set the other:

> @@ -1385,6 +1388,29 @@ nicvf_sq_add_hdr_subdesc(struct nicvf *nic, struct snd_queue *sq, int qentry,
> hdr->inner_l3_offset = skb_network_offset(skb) - 2;
> this_cpu_inc(nic->pnicvf->drv_stats->tx_tso);
> }
> +
> + /* Check if timestamp is requested */
> + if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
> + skb_tx_timestamp(skb);
> + return;
> + }
> +
> + /* Tx timestamping not supported along with TSO, so ignore request */
> + if (skb_shinfo(skb)->gso_size)
> + return;
> +
> + /* HW supports only a single outstanding packet to timestamp */
> + if (!atomic_add_unless(&nic->pnicvf->tx_ptp_skbs, 1, 1))
> + return;
> +
> + /* Mark the SKB for later reference */
> + skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> +
> + /* Finally enable timestamp generation
> + * Since 'post_cqe' is also set, two CQEs will be posted
> + * for this packet i.e CQE_TYPE_SEND and CQE_TYPE_SEND_PTP.
> + */
> + hdr->tstmp = 1;
> }

and so it is completely non-obvious whether this is race free or not.

Thanks,
Richard

2017-12-11 23:36:59

by Richard Cochran

[permalink] [raw]
Subject: Re: [PATCH net-next v5 2/2] net: thunderx: add timestamping support

On Mon, Dec 11, 2017 at 05:14:31PM +0300, Aleksey Makarov wrote:
> @@ -880,6 +889,46 @@ static void nic_pause_frame(struct nicpf *nic, int vf, struct pfc *cfg)
> }
> }
>
> +/* Enable or disable HW timestamping by BGX for pkts received on a LMAC */
> +static void nic_config_timestamp(struct nicpf *nic, int vf, struct set_ptp *ptp)
> +{
> + struct pkind_cfg *pkind;
> + u8 lmac, bgx_idx;
> + u64 pkind_val, pkind_idx;
> +
> + if (vf >= nic->num_vf_en)
> + return;
> +
> + bgx_idx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
> + lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
> +
> + pkind_idx = lmac + bgx_idx * MAX_LMAC_PER_BGX;
> + pkind_val = nic_reg_read(nic, NIC_PF_PKIND_0_15_CFG | (pkind_idx << 3));
> + pkind = (struct pkind_cfg *)&pkind_val;
> +
> + if (ptp->enable && !pkind->hdr_sl) {
> + /* Skiplen to exclude 8byte timestamp while parsing pkt
> + * If not configured, will result in L2 errors.
> + */
> + pkind->hdr_sl = 4;
> + /* Adjust max packet length allowed */
> + pkind->maxlen += (pkind->hdr_sl * 2);
> + bgx_config_timestamping(nic->node, bgx_idx, lmac, true);
> + nic_reg_write(nic,
> + NIC_PF_RX_ETYPE_0_7 | (1 << 3),
> + (ETYPE_ALG_ENDPARSE << 16) | ETH_P_1588);

don't need three lines for this function call.

> + } else if (!ptp->enable && pkind->hdr_sl) {
> + pkind->maxlen -= (pkind->hdr_sl * 2);
> + pkind->hdr_sl = 0;
> + bgx_config_timestamping(nic->node, bgx_idx, lmac, false);
> + nic_reg_write(nic,
> + NIC_PF_RX_ETYPE_0_7 | (1 << 3),
> + (1ULL << 16) | ETH_P_8021Q); /* reset value */

here neither. Also avoid comment on the LHS. If 1<<16 means "reset"
then just define a macro.

> + }
> +
> + nic_reg_write(nic, NIC_PF_PKIND_0_15_CFG | (pkind_idx << 3), pkind_val);
> +}
> +

Thanks,
Richard

2017-12-12 09:41:58

by Aleksey Makarov

[permalink] [raw]
Subject: Re: [PATCH net-next v5 1/2] net: add support for Cavium PTP coprocessor

Hi Richard,

On 12/12/2017 01:59 AM, Richard Cochran wrote:
>
> Sorry I didn't finish reviewing before...
>
> On Mon, Dec 11, 2017 at 05:14:30PM +0300, Aleksey Makarov wrote:

[ ... ]

>> +static int cavium_ptp_probe(struct pci_dev *pdev,
>> + const struct pci_device_id *ent)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct cavium_ptp *clock;
>> + struct cyclecounter *cc;
>> + u64 clock_cfg;
>> + u64 clock_comp;
>> + int err;
>> +
>> + clock = devm_kzalloc(dev, sizeof(*clock), GFP_KERNEL);
>> + if (!clock)
>> + return -ENOMEM;
>> +
>> + clock->pdev = pdev;
>> +
>> + err = pcim_enable_device(pdev);
>> + if (err)
>> + return err;
>> +
>> + err = pcim_iomap_regions(pdev, 1 << PCI_PTP_BAR_NO, pci_name(pdev));
>> + if (err)
>> + return err;
>> +
>> + clock->reg_base = pcim_iomap_table(pdev)[PCI_PTP_BAR_NO];
>> +
>> + spin_lock_init(&clock->spin_lock);
>> +
>> + cc = &clock->cycle_counter;
>> + cc->read = cavium_ptp_cc_read;
>> + cc->mask = CYCLECOUNTER_MASK(64);
>> + cc->mult = 1;
>> + cc->shift = 0;
>> +
>> + timecounter_init(&clock->time_counter, &clock->cycle_counter,
>> + ktime_to_ns(ktime_get_real()));
>> +
>> + clock->clock_rate = ptp_cavium_clock_get();
>> +
>> + clock->ptp_info = (struct ptp_clock_info) {
>> + .owner = THIS_MODULE,
>> + .name = "ThunderX PTP",
>> + .max_adj = 1000000000ull,
>> + .n_ext_ts = 0,
>> + .n_pins = 0,
>> + .pps = 0,
>> + .adjfreq = cavium_ptp_adjfreq,
>> + .adjtime = cavium_ptp_adjtime,
>> + .gettime64 = cavium_ptp_gettime,
>> + .settime64 = cavium_ptp_settime,
>> + .enable = cavium_ptp_enable,
>> + };
>> +
>> + clock_cfg = readq(clock->reg_base + PTP_CLOCK_CFG);
>> + clock_cfg |= PTP_CLOCK_CFG_PTP_EN;
>> + writeq(clock_cfg, clock->reg_base + PTP_CLOCK_CFG);
>> +
>> + clock_comp = ((u64)1000000000ull << 32) / clock->clock_rate;
>> + writeq(clock_comp, clock->reg_base + PTP_CLOCK_COMP);
>> +
>> + clock->ptp_clock = ptp_clock_register(&clock->ptp_info, dev);
>> + if (IS_ERR(clock->ptp_clock)) {
>
> You need to handle the case when ptp_clock_register() returns NULL.
>
> from ptp_clock_kernel.h:
>
> /**
> * ptp_clock_register() - register a PTP hardware clock driver
> *
> * @info: Structure describing the new clock.
> * @parent: Pointer to the parent device of the new clock.
> *
> * Returns a valid pointer on success or PTR_ERR on failure. If PHC
> * support is missing at the configuration level, this function
> * returns NULL, and drivers are expected to gracefully handle that
> * case separately.
> */

If ptp_clock_register() returns NULL, the device is still paired with the driver,
but the driver is not registered in the PTP core. When ethernet driver needs
the reference to this cavium PTP driver, it calls cavium_ptp_get() that checks
if ptp->ptp_clock is NULL and, if so, returns -ENODEV.

I need this behavior because I need to differentiate between two cases:

- the state when the driver is not initialized for the device because of PTP core
has not registered it. In this case function cavium_ptp_get() returns -ENODEV
and ethernet driver proceeds without PTP device.

- the state when the driver is not initialized because kernel has not tired
to initialize it yet. In this case function cavium_ptp_get() returns -EPROBE_DEFER
that is used in ethernet driver to defer initialization.

If you know how to do the same in more smoothly please share it. Or else I would
prefer to insert a comment about it and leave it as is.

Richard, thank you for review. I am going to address your comments in my next series.

Thank you
Aleksey Makarov

>> + clock_cfg = readq(clock->reg_base + PTP_CLOCK_CFG);
>> + clock_cfg &= ~PTP_CLOCK_CFG_PTP_EN;
>> + writeq(clock_cfg, clock->reg_base + PTP_CLOCK_CFG);
>> + return PTR_ERR(clock->ptp_clock);
>> + }
>> +
>> + pci_set_drvdata(pdev, clock);
>> + return 0;
>> +}
>
> Thanks,
> Richard
>

2017-12-12 17:12:41

by Richard Cochran

[permalink] [raw]
Subject: Re: [PATCH net-next v5 1/2] net: add support for Cavium PTP coprocessor

On Tue, Dec 12, 2017 at 12:41:35PM +0300, Aleksey Makarov wrote:
> If ptp_clock_register() returns NULL, the device is still paired with the driver,
> but the driver is not registered in the PTP core. When ethernet driver needs
> the reference to this cavium PTP driver, it calls cavium_ptp_get() that checks
> if ptp->ptp_clock is NULL and, if so, returns -ENODEV.

The pointer clock->ptp_clock can be NULL.

Yet you de-reference it here:

> +static void cavium_ptp_remove(struct pci_dev *pdev)
> +{
> + struct cavium_ptp *clock = pci_get_drvdata(pdev);
> + u64 clock_cfg;
> +
> + pci_set_drvdata(pdev, NULL);
> +
> + ptp_clock_unregister(clock->ptp_clock);
> +
> + clock_cfg = readq(clock->reg_base + PTP_CLOCK_CFG);
> + clock_cfg &= ~PTP_CLOCK_CFG_PTP_EN;
> + writeq(clock_cfg, clock->reg_base + PTP_CLOCK_CFG);
> +}

and here:

> +static inline int cavium_ptp_clock_index(struct cavium_ptp *clock)
> +{
> + return ptp_clock_index(clock->ptp_clock);
> +}

That needs to be fixed.

Thanks,
Richard

2017-12-12 19:47:13

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH net-next v5 2/2] net: thunderx: add timestamping support

On Mon, 2017-12-11 at 15:36 -0800, Richard Cochran wrote:
> On Mon, Dec 11, 2017 at 05:14:31PM +0300, Aleksey Makarov wrote:
> > @@ -880,6 +889,46 @@ static void nic_pause_frame(struct nicpf *nic, int vf, struct pfc *cfg)
> > }
> > }
> >
> > +/* Enable or disable HW timestamping by BGX for pkts received on a LMAC */
> > +static void nic_config_timestamp(struct nicpf *nic, int vf, struct set_ptp *ptp)
> > +{
> > + struct pkind_cfg *pkind;
> > + u8 lmac, bgx_idx;
> > + u64 pkind_val, pkind_idx;
> > +
> > + if (vf >= nic->num_vf_en)
> > + return;
> > +
> > + bgx_idx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
> > + lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
> > +
> > + pkind_idx = lmac + bgx_idx * MAX_LMAC_PER_BGX;
> > + pkind_val = nic_reg_read(nic, NIC_PF_PKIND_0_15_CFG | (pkind_idx << 3));
> > + pkind = (struct pkind_cfg *)&pkind_val;
> > +
> > + if (ptp->enable && !pkind->hdr_sl) {
> > + /* Skiplen to exclude 8byte timestamp while parsing pkt
> > + * If not configured, will result in L2 errors.
> > + */
> > + pkind->hdr_sl = 4;
> > + /* Adjust max packet length allowed */
> > + pkind->maxlen += (pkind->hdr_sl * 2);

Are all compilers smart enough to set this to 8?
I rather doubt a compiler is even allowed to.

2018-01-08 17:12:29

by Aleksey Makarov

[permalink] [raw]
Subject: Re: [PATCH net-next v5 2/2] net: thunderx: add timestamping support



On 12.12.2017 05:32, Richard Cochran wrote:
> On Mon, Dec 11, 2017 at 05:14:31PM +0300, Aleksey Makarov wrote:
>> diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
>> index 4a02e618e318..204b234beb9d 100644
>> --- a/drivers/net/ethernet/cavium/thunder/nic.h
>> +++ b/drivers/net/ethernet/cavium/thunder/nic.h
>> @@ -263,6 +263,8 @@ struct nicvf_drv_stats {
>> struct u64_stats_sync syncp;
>> };
>>
>> +struct cavium_ptp;
>> +
>> struct nicvf {
>> struct nicvf *pnicvf;
>> struct net_device *netdev;
>> @@ -312,6 +314,12 @@ struct nicvf {
>> struct tasklet_struct qs_err_task;
>> struct work_struct reset_task;
>>
>> + /* PTP timestamp */
>> + struct cavium_ptp *ptp_clock;
>> + bool hw_rx_tstamp;
>> + struct sk_buff *ptp_skb;
>> + atomic_t tx_ptp_skbs;
>
> It is disturbing that the above two fields are set in different
> places. Shouldn't they be unified into one logical lock?

No, they should not as they are not quite related.

`tx_ptp_skbs` is set when the hardware is sending a packet that requires
timestamping. Cavium hardware can not process more than one
such packet at once so this is set each time the driver submits
a packet that requires timestamping to the send queue and clears
each time it receives the entry on the completion queue saying
that such packet was sent.

So `tx_ptp_skbs` prevents driver from submitting more than one
packet that requires timestamping to the hardware for transmitting.

When that packet is sent, hardware inserts two entries to
the completion queue. First is the regular CQE_TYPE_SEND entry
that signals that the packet was sent. The second is CQE_TYPE_SEND_PTP
that contains the actual timestamp for that packet.

`ptp_skb` is initialized in the handler for the CQE_TYPE_SEND
entry and is used and zeroed in the handler for the CQE_TYPE_SEND_PTP
entry.

So `ptp_skb` is used to hold the pointer to the packet between
the calls to CQE_TYPE_SEND and CQE_TYPE_SEND_PTP handlers.

I am going to add those comments to the sources, fix other issues and
send v6 in short time.

Thank you
Aleksey Makarov

> Here you clear them together:
>
>> +static void nicvf_snd_ptp_handler(struct net_device *netdev,
>> + struct cqe_send_t *cqe_tx)
>> +{
>> + struct nicvf *nic = netdev_priv(netdev);
>> + struct skb_shared_hwtstamps ts;
>> + u64 ns;
>> +
>> + nic = nic->pnicvf;
>> +
>> + /* Sync for 'ptp_skb' */
>> + smp_rmb();
>> +
>> + /* New timestamp request can be queued now */
>> + atomic_set(&nic->tx_ptp_skbs, 0);
>> +
>> + /* Check for timestamp requested skb */
>> + if (!nic->ptp_skb)
>> + return;
>> +
>> + /* Check if timestamping is timedout, which is set to 10us */
>> + if (cqe_tx->send_status == CQ_TX_ERROP_TSTMP_TIMEOUT ||
>> + cqe_tx->send_status == CQ_TX_ERROP_TSTMP_CONFLICT)
>> + goto no_tstamp;
>> +
>> + /* Get the timestamp */
>> + memset(&ts, 0, sizeof(ts));
>> + ns = cavium_ptp_tstamp2time(nic->ptp_clock, cqe_tx->ptp_timestamp);
>> + ts.hwtstamp = ns_to_ktime(ns);
>> + skb_tstamp_tx(nic->ptp_skb, &ts);
>> +
>> +no_tstamp:
>> + /* Free the original skb */
>> + dev_kfree_skb_any(nic->ptp_skb);
>> + nic->ptp_skb = NULL;
>> + /* Sync 'ptp_skb' */
>> + smp_wmb();
>> +}
>> +
>
> but here you set the one:
>
>> @@ -657,7 +697,12 @@ static void nicvf_snd_pkt_handler(struct net_device *netdev,
>> prefetch(skb);
>> (*tx_pkts)++;
>> *tx_bytes += skb->len;
>> - napi_consume_skb(skb, budget);
>> + /* If timestamp is requested for this skb, don't free it */
>> + if (skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS &&
>> + !nic->pnicvf->ptp_skb)
>> + nic->pnicvf->ptp_skb = skb;
>> + else
>> + napi_consume_skb(skb, budget);
>> sq->skbuff[cqe_tx->sqe_ptr] = (u64)NULL;
>> } else {
>> /* In case of SW TSO on 88xx, only last segment will have
>
> here you clear one:
>
>> @@ -1319,12 +1382,28 @@ int nicvf_stop(struct net_device *netdev)
>>
>> nicvf_free_cq_poll(nic);
>>
>> + /* Free any pending SKB saved to receive timestamp */
>> + if (nic->ptp_skb) {
>> + dev_kfree_skb_any(nic->ptp_skb);
>> + nic->ptp_skb = NULL;
>> + }
>> +
>> /* Clear multiqset info */
>> nic->pnicvf = nic;
>>
>> return 0;
>> }
>
> here you clear both:
>
>> @@ -1394,6 +1473,12 @@ int nicvf_open(struct net_device *netdev)
>> if (nic->sqs_mode)
>> nicvf_get_primary_vf_struct(nic);
>>
>> + /* Configure PTP timestamp */
>> + if (nic->ptp_clock)
>> + nicvf_config_hw_rx_tstamp(nic, nic->hw_rx_tstamp);
>> + atomic_set(&nic->tx_ptp_skbs, 0);
>> + nic->ptp_skb = NULL;
>> +
>> /* Configure receive side scaling and MTU */
>> if (!nic->sqs_mode) {
>> nicvf_rss_init(nic);
>
> here you set the other:
>
>> @@ -1385,6 +1388,29 @@ nicvf_sq_add_hdr_subdesc(struct nicvf *nic, struct snd_queue *sq, int qentry,
>> hdr->inner_l3_offset = skb_network_offset(skb) - 2;
>> this_cpu_inc(nic->pnicvf->drv_stats->tx_tso);
>> }
>> +
>> + /* Check if timestamp is requested */
>> + if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
>> + skb_tx_timestamp(skb);
>> + return;
>> + }
>> +
>> + /* Tx timestamping not supported along with TSO, so ignore request */
>> + if (skb_shinfo(skb)->gso_size)
>> + return;
>> +
>> + /* HW supports only a single outstanding packet to timestamp */
>> + if (!atomic_add_unless(&nic->pnicvf->tx_ptp_skbs, 1, 1))
>> + return;
>> +
>> + /* Mark the SKB for later reference */
>> + skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
>> +
>> + /* Finally enable timestamp generation
>> + * Since 'post_cqe' is also set, two CQEs will be posted
>> + * for this packet i.e CQE_TYPE_SEND and CQE_TYPE_SEND_PTP.
>> + */
>> + hdr->tstmp = 1;
>> }
>
> and so it is completely non-obvious whether this is race free or not.
>
> Thanks,
> Richard
>