Skip to contentSkip to conversation

VNIC: Adding support for Cavium ThunderX network controller

PATCH In review Robert Richter 3 participants 3 messages Nov 10, 2014 – Nov 12, 2014

Subsystems (5)
Networking Network drivers PCI arm64 +1 more subsystems
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 VNIC: Adding support for Cavium ThunderX network controller

From: Sunil Goutham <[email protected]>

This patch adds support for the Cavium ThunderX network controller. The driver is on the pci bus and thus requires the Thunder PCIe host controller driver to be enabled.

v2:
* non-generic module parameters removed
* ethtool support added (nicvf_set_rxnfc())

---
 MAINTAINERS                                        |    7 +
 arch/arm64/Kconfig                                 |    1 +
 drivers/net/ethernet/Kconfig                       |    1 +
 drivers/net/ethernet/Makefile                      |    1 +
 drivers/net/ethernet/cavium/Kconfig                |   40 +
 drivers/net/ethernet/cavium/Makefile               |    5 +
 drivers/net/ethernet/cavium/thunder/Makefile       |   13 +
 drivers/net/ethernet/cavium/thunder/nic.h          |  434 +++++++
 drivers/net/ethernet/cavium/thunder/nic_main.c     |  807 ++++++++++++
 drivers/net/ethernet/cavium/thunder/nic_reg.h      |  214 ++++
 .../net/ethernet/cavium/thunder/nicvf_ethtool.c    |  559 +++++++++
 drivers/net/ethernet/cavium/thunder/nicvf_main.c   | 1323 ++++++++++++++++++++
 drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 1302 +++++++++++++++++++
 drivers/net/ethernet/cavium/thunder/nicvf_queues.h |  355 ++++++
 drivers/net/ethernet/cavium/thunder/q_struct.h     |  702 +++++++++++
 drivers/net/ethernet/cavium/thunder/thunder_bgx.c  |  386 ++++++
 drivers/net/ethernet/cavium/thunder/thunder_bgx.h  |   78 ++
 include/linux/pci_ids.h                            |    2 +
 18 files changed, 6230 insertions(+)
 create mode 100644 drivers/net/ethernet/cavium/Kconfig
 create mode 100644 drivers/net/ethernet/cavium/Makefile
 create mode 100644 drivers/net/ethernet/cavium/thunder/Makefile
 create mode 100644 drivers/net/ethernet/cavium/thunder/nic.h
 create mode 100644 drivers/net/ethernet/cavium/thunder/nic_main.c
 create mode 100644 drivers/net/ethernet/cavium/thunder/nic_reg.h
 create mode 100644 drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
 create mode 100644 drivers/net/ethernet/cavium/thunder/nicvf_main.c
 create mode 100644 drivers/net/ethernet/cavium/thunder/nicvf_queues.c
 create mode 100644 drivers/net/ethernet/cavium/thunder/nicvf_queues.h
 create mode 100644 drivers/net/ethernet/cavium/thunder/q_struct.h
 create mode 100644 drivers/net/ethernet/cavium/thunder/thunder_bgx.c
 create mode 100644 drivers/net/ethernet/cavium/thunder/thunder_bgx.h
MAINTAINERS +7 -0
index a20df9bf8ab0..9a36569d75df 100644
@@ -882,6 +882,13 @@ M:	Krzysztof Halasa <[email protected]>
 S:	Maintained
 F:	arch/arm/mach-cns3xxx/
 
+ARM/CAVIUM THUNDER ARCHITECTURE
+M:	Sunil Goutham <[email protected]>
+M:	Robert Richter <[email protected]>
+L:	[email protected] (moderated for non-subscribers)
+S:	Supported
+F:	drivers/net/ethernet/cavium/
+
 ARM/CIRRUS LOGIC CLPS711X ARM ARCHITECTURE
 M:	Alexander Shiyan <[email protected]>
 L:	[email protected] (moderated for non-subscribers)
arch/arm64/Kconfig +1 -0
index ac9afde76dea..bc14b1a78ae2 100644
@@ -145,6 +145,7 @@ config ARCH_THUNDER
 	bool "Cavium Inc. Thunder SoC Family"
 	help
 	  This enables support for Cavium's Thunder Family of SoCs.
+	select NET_VENDOR_CAVIUM
 
 config ARCH_VEXPRESS
 	bool "ARMv8 software model (Versatile Express)"
drivers/net/ethernet/Kconfig +1 -0
index 1ed1fbba5d58..7c872ff82aed 100644
@@ -34,6 +34,7 @@ source "drivers/net/ethernet/adi/Kconfig"
 source "drivers/net/ethernet/broadcom/Kconfig"
 source "drivers/net/ethernet/brocade/Kconfig"
 source "drivers/net/ethernet/calxeda/Kconfig"
+source "drivers/net/ethernet/cavium/Kconfig"
 source "drivers/net/ethernet/chelsio/Kconfig"
 source "drivers/net/ethernet/cirrus/Kconfig"
 source "drivers/net/ethernet/cisco/Kconfig"
drivers/net/ethernet/Makefile +1 -0
index 6e0b629e9859..7a9f7d7b55bf 100644
@@ -20,6 +20,7 @@ obj-$(CONFIG_NET_BFIN) += adi/
 obj-$(CONFIG_NET_VENDOR_BROADCOM) += broadcom/
 obj-$(CONFIG_NET_VENDOR_BROCADE) += brocade/
 obj-$(CONFIG_NET_CALXEDA_XGMAC) += calxeda/
+obj-$(CONFIG_NET_VENDOR_CAVIUM) += cavium/
 obj-$(CONFIG_NET_VENDOR_CHELSIO) += chelsio/
 obj-$(CONFIG_NET_VENDOR_CIRRUS) += cirrus/
 obj-$(CONFIG_NET_VENDOR_CISCO) += cisco/
drivers/net/ethernet/cavium/Kconfig new file +40 -0
new file mode 100644
index 000000000000..6365fb4242be
@@ -0,0 +1,40 @@
+#
+# Cavium ethernet device configuration
+#
+
+config NET_VENDOR_CAVIUM
+	tristate "Cavium ethernet drivers"
+	depends on PCI
+	---help---
+	  Enable support for the Cavium ThunderX Network Interface
+	  Controller (NIC). The NIC provides the controller and DMA
+	  engines to move network traffic to/from the memory. The NIC
+	  works closely with TNS, BGX and SerDes to implement the
+	  functions replacing and virtualizing those of a typical
+	  standalone PCIe NIC chip.
+
+	  If you have a Cavium Thunder board, say Y.
+
+if NET_VENDOR_CAVIUM
+
+config THUNDER_NIC_PF
+	tristate "Thunder Physical function driver"
+	default NET_VENDOR_CAVIUM
+	select THUNDER_NIC_BGX
+	---help---
+	  This driver supports Thunder's NIC physical function.
+
+config THUNDER_NIC_VF
+	tristate "Thunder Virtual function driver"
+	default NET_VENDOR_CAVIUM
+	---help---
+	  This driver supports Thunder's NIC virtual function
+
+config	THUNDER_NIC_BGX
+	tristate "Thunder MAC interface driver (BGX)"
+	default NET_VENDOR_CAVIUM
+	---help---
+	  This driver supports programming and controlling of MAC
+	  interface from NIC physical function driver.
+
+endif # NET_VENDOR_CAVIUM
drivers/net/ethernet/cavium/Makefile new file +5 -0
new file mode 100644
index 000000000000..7aac4780d050
@@ -0,0 +1,5 @@
+#
+# Makefile for the Cavium ethernet device drivers.
+#
+
+obj-$(CONFIG_NET_VENDOR_CAVIUM) += thunder/
drivers/net/ethernet/cavium/thunder/Makefile new file +13 -0
new file mode 100644
index 000000000000..8ee6e043f8ff
@@ -0,0 +1,13 @@
+#
+# Makefile for Cavium's Thunder ethernet device
+#
+
+
+# Don't change the order, NICPF driver is dependent on BGX driver init
+obj-$(CONFIG_THUNDER_NIC_BGX) += thunder_bgx.o
+obj-$(CONFIG_THUNDER_NIC_PF) += nicpf.o
+obj-$(CONFIG_THUNDER_NIC_VF) += nicvf.o
+
+nicpf-y := nic_main.o
+nicvf-y := nicvf_main.o nicvf_queues.o
+nicvf-y += nicvf_ethtool.o
drivers/net/ethernet/cavium/thunder/nic.h new file +434 -0

434 lines not on this page · raw message

drivers/net/ethernet/cavium/thunder/nic_main.c new file +807 -0

807 lines not on this page · raw message

drivers/net/ethernet/cavium/thunder/nic_reg.h new file +214 -0

214 lines not on this page · raw message

drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c new file +559 -0

559 lines not on this page · raw message

drivers/net/ethernet/cavium/thunder/nicvf_main.c new file +1323 -0

894 lines not on this page · raw message

drivers/net/ethernet/cavium/thunder/nicvf_queues.c new file +1302 -0

1,302 lines not on this page · raw message

drivers/net/ethernet/cavium/thunder/nicvf_queues.h new file +355 -0

355 lines not on this page · raw message

drivers/net/ethernet/cavium/thunder/q_struct.h new file +702 -0

702 lines not on this page · raw message

drivers/net/ethernet/cavium/thunder/thunder_bgx.c new file +386 -0

386 lines not on this page · raw message

drivers/net/ethernet/cavium/thunder/thunder_bgx.h new file +78 -0

78 lines not on this page · raw message

include/linux/pci_ids.h +2 -0
index 1fa99a301817..80bd3336691e 100644

Lines of this file are not shown. View the raw message.

The rest of this diff is not shown. View the raw message.

Signature · 1 line
2.1.3

Part of the diff is not shown here. View the raw message.

maintainer
#1 lore raw

From: Robert Richter <[email protected]>
Date: Sun, 9 Nov 2014 21:14:05 -0800

Quoted 1 line

+config NET_VENDOR_CAVIUM

...

Quoted 1 line

+config THUNDER_NIC_PF

...

Quoted 1 line

+config THUNDER_NIC_VF

...

Quoted 1 line

+config THUNDER_NIC_BGX

These config options seem excessive, if not confusing. What would a distribution be expected to enable? Everything?

drivers/net/ethernet/cavium/thunder/Makefile line 9

Quoted 4 lines
drivers/net/ethernet/cavium/thunder/Makefile new file +4 -0
+# Don't change the order, NICPF driver is dependent on BGX driver init
+obj-$(CONFIG_THUNDER_NIC_BGX) += thunder_bgx.o
+obj-$(CONFIG_THUNDER_NIC_PF) += nicpf.o
+obj-$(CONFIG_THUNDER_NIC_VF) += nicvf.o

Nothing ensures ordering if these things are built all modular.

Such ordering dependencies need to be resolved in another way.

#2 lore raw
In reply to Robert Richter

On 313, 11 09, 2014 at 09:14:05PM -0800, Robert Richter wrote:

I apologize for possibly repeated mail, my mailsystem was misconfigured :(

Some comments from quick look are below.

Quoted 13 lines
drivers/net/ethernet/cavium/thunder/nic_main.c new file +13 -0
+static int nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
+{
+	struct device *dev = &pdev->dev;
+	struct net_device *netdev;
+	struct nicpf *nic;
+	int    err;
+
+	netdev = alloc_etherdev(sizeof(struct nicpf));
+	if (!netdev)
+		return -ENOMEM;
+
+	pci_set_drvdata(pdev, netdev);
+

drivers/net/ethernet/cavium/thunder/nic_main.c line 698

Quoted 10 lines
drivers/net/ethernet/cavium/thunder/nic_main.c new file +10 -0
+	SET_NETDEV_DEV(netdev, &pdev->dev);
+
+	nic = netdev_priv(netdev);
+	nic->netdev = netdev;
+	nic->pdev = pdev;
+
+	err = pci_enable_device(pdev);
+	if (err) {
+		dev_err(dev, "Failed to enable PCI device\n");
+		goto exit;

Looks like you leaked netdev here.

Quoted 40 lines
drivers/net/ethernet/cavium/thunder/nic_main.c new file +40 -0
+	}
+
+	err = pci_request_regions(pdev, DRV_NAME);
+	if (err) {
+		dev_err(dev, "PCI request regions failed 0x%x\n", err);
+		goto err_disable_device;
+	}
+
+	err = pci_set_dma_mask(pdev, DMA_BIT_MASK(48));
+	if (err) {
+		dev_err(dev, "Unable to get usable DMA configuration\n");
+		goto err_release_regions;
+	}
+
+	err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(48));
+	if (err) {
+		dev_err(dev, "unable to get 48-bit DMA for consistent allocations\n");
+		goto err_release_regions;
+	}
+
+	/* MAP PF's configuration registers */
+	nic->reg_base = (uint64_t)pci_ioremap_bar(pdev, PCI_CFG_REG_BAR_NUM);
+	if (!nic->reg_base) {
+		dev_err(dev, "Cannot map config register space, aborting\n");
+		err = -ENOMEM;
+		goto err_release_regions;
+	}
+	nic->node = NIC_NODE_ID(pci_resource_start(pdev, PCI_CFG_REG_BAR_NUM));
+
+	/* By default set NIC in TNS bypass mode */
+	nic->flags &= ~NIC_TNS_ENABLED;
+	nic_set_lmac_vf_mapping(nic);
+
+	/* Initialize hardware */
+	nic_init_hw(nic);
+
+	/* Set RSS TBL size for each VF */
+	nic->rss_ind_tbl_size = max((NIC_RSSI_COUNT / nic->num_vf_en),
+				    NIC_MAX_RSS_IDR_TBL_SIZE);
+	nic->rss_ind_tbl_size = rounddown_pow_of_two(nic->rss_ind_tbl_size);

drivers/net/ethernet/cavium/thunder/nic_main.c line 748

Quoted 10 lines
drivers/net/ethernet/cavium/thunder/nic_main.c new file +10 -0
+
+	/* Register interrupts */
+	if (nic_register_interrupts(nic))
+		goto err_unmap_resources;
+
+	/* Configure SRIOV */
+	if (!nic_sriov_init(pdev, nic))
+		goto err_unmap_resources;
+
+	goto exit;

Why not simply return 0; ?

drivers/net/ethernet/cavium/thunder/nic_main.c line 751

Quoted 3 lines
drivers/net/ethernet/cavium/thunder/nic_main.c new file +3 -0
+
+err_unmap_resources:
+	if (nic->reg_base)

This check looks unnecessary.

Quoted 12 lines
drivers/net/ethernet/cavium/thunder/thunder_bgx.c new file +10 -0
+		iounmap((void *)nic->reg_base);
+err_release_regions:
+	pci_release_regions(pdev);
+err_disable_device:
+	pci_disable_device(pdev);
+exit:
+	return err;
+}
 
 
+static int bgx_register_interrupts(struct bgx *bgx, uint8_t lmac)
+{

drivers/net/ethernet/cavium/thunder/thunder_bgx.c line 172

Quoted 10 lines
drivers/net/ethernet/cavium/thunder/thunder_bgx.c new file +10 -0
+	int irq, ret = 0;
+
+	/* Register only link interrupts now */
+	irq = SPUX_INT + (lmac * BGX_LMAC_VEC_OFFSET);
+	sprintf(bgx->irq_name[irq], "LMAC%d", lmac);
+	ret = request_irq(bgx->msix_entries[irq].vector,
+			  bgx_lmac_intr_handler, 0, bgx->irq_name[irq], bgx);
+	if (ret)
+		goto fail;
+	else

This else just hurts readability.

drivers/net/ethernet/cavium/thunder/thunder_bgx.c line 179

Quoted 7 lines
drivers/net/ethernet/cavium/thunder/thunder_bgx.c new file +7 -0
+		bgx->irq_allocated[irq] = 1;
+
+	/* Enable link interrupt */
+	bgx_enable_link_intr(bgx, lmac);
+	return 0;
+
+fail:

The code below copypasted from bgx_unregister_interrupts() Looks like good candidate for helper function.

Quoted 22 lines
drivers/net/ethernet/cavium/thunder/thunder_bgx.c new file +22 -0
+	dev_err(&bgx->pdev->dev, "Request irq failed\n");
+	for (irq = 0; irq < bgx->num_vec; irq++) {
+		if (bgx->irq_allocated[irq])
+			free_irq(bgx->msix_entries[irq].vector, bgx);
+		bgx->irq_allocated[irq] = 0;
+	}
+	return 1;
+}
+
+static void bgx_unregister_interrupts(struct bgx *bgx)
+{
+	int irq;
+	/* Free registered interrupts */
+	for (irq = 0; irq < bgx->num_vec; irq++) {
+		if (bgx->irq_allocated[irq])
+			free_irq(bgx->msix_entries[irq].vector, bgx);
+		bgx->irq_allocated[irq] = 0;
+	}
+	/* Disable MSI-X */
+	bgx_disable_msix(bgx);
+}
+

drivers/net/ethernet/cavium/thunder/thunder_bgx.c line 260

Quoted 10 lines
drivers/net/ethernet/cavium/thunder/thunder_bgx.c new file +9 -0
 
+void bgx_lmac_enable(struct bgx *bgx, int8_t lmac)
+{
+	uint64_t dmac_bcast = (1ULL << 48) - 1;
+
+	bgx_reg_write(bgx, lmac, BGX_CMRX_CFG,
+		      (1 << 15) | (1 << 14) | (1 << 13));
+
+	/* Register interrupts */
+	bgx_register_interrupts(bgx, lmac);

bgx_register_interrupts() can fail (due to request_irq), but it's return value is not checked.

Quoted 4 lines
drivers/net/ethernet/cavium/thunder/thunder_bgx.c new file +4 -0
+
+	/* Add broadcast MAC into all LMAC's DMAC filters */
+	for (lmac = 0; lmac < bgx->lmac_count; lmac++)
+		bgx_add_dmac_addr(dmac_bcast, 0, bgx->bgx_id, lmac);

drivers/net/ethernet/cavium/thunder/thunder_bgx.c line 295

Quoted 10 lines
drivers/net/ethernet/cavium/thunder/thunder_bgx.c new file +9 -0
+}
 
+static int bgx_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
+{
+	struct device *dev = &pdev->dev;
+	struct bgx *bgx;
+	int    err;
+	uint8_t lmac = 0;
+
+	bgx = kzalloc(sizeof(*bgx), GFP_KERNEL);

kzalloc() return value not checked

drivers/net/ethernet/cavium/thunder/thunder_bgx.c line 303

Quoted 8 lines
drivers/net/ethernet/cavium/thunder/thunder_bgx.c new file +8 -0
+	bgx->pdev = pdev;
+
+	pci_set_drvdata(pdev, bgx);
+
+	err = pci_enable_device(pdev);
+	if (err) {
+		dev_err(dev, "Failed to enable PCI device\n");
+		goto exit;

bgx is leaked here.

Quoted 15 lines
drivers/net/ethernet/cavium/thunder/thunder_bgx.c new file +15 -0
+	}
+
+	err = pci_request_regions(pdev, DRV_NAME);
+	if (err) {
+		dev_err(dev, "PCI request regions failed 0x%x\n", err);
+		goto err_disable_device;
+	}
+
+	/* MAP configuration registers */
+	bgx->reg_base = (uint64_t)pci_ioremap_bar(pdev, PCI_CFG_REG_BAR_NUM);
+	if (!bgx->reg_base) {
+		dev_err(dev, "BGX: Cannot map CSR memory space, aborting\n");
+		err = -ENOMEM;
+		goto err_release_regions;
+	}

drivers/net/ethernet/cavium/thunder/thunder_bgx.c line 328

Quoted 10 lines
drivers/net/ethernet/cavium/thunder/thunder_bgx.c new file +10 -0
+	bgx->bgx_id = (pci_resource_start(pdev, PCI_CFG_REG_BAR_NUM) >> 24) & 1;
+	bgx->bgx_id += NODE_ID(pci_resource_start(pdev, PCI_CFG_REG_BAR_NUM))
+							* MAX_BGX_PER_CN88XX;
+	bgx_vnic[bgx->bgx_id] = bgx;
+
+	/* Initialize BGX hardware */
+	bgx_init_hw(bgx);
+	/* Enable MSI-X */
+	if (!bgx_enable_msix(bgx))
+		return 1;

Massive resource leak here.

drivers/net/ethernet/cavium/thunder/thunder_bgx.c line 331

Quoted 3 lines
drivers/net/ethernet/cavium/thunder/thunder_bgx.c new file +3 -0
+	/* Enable all LMACs */
+	for (lmac = 0; lmac < bgx->lmac_count; lmac++)
+		bgx_lmac_enable(bgx, lmac);

See comment about bgx_lmac_enable() above.

Review

Quoted 2 lines
(quoted diff) +2 -0
+	goto exit;
+

Lines below are unreachable.

Quoted 9 lines
drivers/net/ethernet/cavium/thunder/thunder_bgx.c new file +9 -0
+	if (bgx->reg_base)
+		iounmap((void *)bgx->reg_base);
+err_release_regions:
+	pci_release_regions(pdev);
+err_disable_device:
+	pci_disable_device(pdev);
+exit:
+	return err;
+}