Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 29 Jul 2016 17:15:42 +0000 (UTC)
From:      Alexander Motin <mav@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r303494 - in head: share/man/man4 sys/dev/ntb sys/dev/ntb/if_ntb
Message-ID:  <201607291715.u6THFg5l082797@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mav
Date: Fri Jul 29 17:15:41 2016
New Revision: 303494
URL: https://svnweb.freebsd.org/changeset/base/303494

Log:
  Once more refactor KPI between ntb_transport(4) and if_ntb(4)..
  
  New design allows to attach multiple consumers to ntb_transport(4) instance.
  Previous design obtained from Linux theoretically allowed that, but was not
  practically usable (Linux also has only one consumer driver now).

Modified:
  head/share/man/man4/if_ntb.4
  head/share/man/man4/ntb_transport.4
  head/sys/dev/ntb/if_ntb/if_ntb.c
  head/sys/dev/ntb/ntb_transport.c
  head/sys/dev/ntb/ntb_transport.h

Modified: head/share/man/man4/if_ntb.4
==============================================================================
--- head/share/man/man4/if_ntb.4	Fri Jul 29 17:12:31 2016	(r303493)
+++ head/share/man/man4/if_ntb.4	Fri Jul 29 17:15:41 2016	(r303494)
@@ -25,7 +25,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd July 10, 2016
+.Dd July 29, 2016
 .Dt IF_NTB 4
 .Os
 .Sh NAME
@@ -49,7 +49,7 @@ The following tunables are settable from
 .Bl -ohang
 .It Va hw.if_ntb.num_queues
 Number of transport queues to use per interface.
-Default is 1.
+Default is unlimited.
 .El
 .Sh DESCRIPTION
 The
@@ -84,3 +84,6 @@ Later improvements were done by
 .An Conrad E. Meyer Aq Mt cem@FreeBSD.org
 and
 .An Alexander Motin Aq Mt mav@FreeBSD.org .
+.Sh BUGS
+Linux supports only one queue per interface, so manual configuration
+may be required for compatibility.

Modified: head/share/man/man4/ntb_transport.4
==============================================================================
--- head/share/man/man4/ntb_transport.4	Fri Jul 29 17:12:31 2016	(r303493)
+++ head/share/man/man4/ntb_transport.4	Fri Jul 29 17:15:41 2016	(r303494)
@@ -25,7 +25,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd July 10, 2016
+.Dd July 29, 2016
 .Dt NTB_TRANSPORT 4
 .Os
 .Sh NAME
@@ -44,10 +44,15 @@ The following tunables are settable from
 .It Va hw.ntb_transport.debug_level
 Driver debug level.
 The default value is 0, higher means more verbose.
-.It Va hw.ntb_transport.max_num_clients
-Number of bidirectional queues to setup.
-The default value is 0, that means one queue per available memory window.
-Maximal number is limited by number of doorbells.
+.It Va hint.ntb_transport. Ns Ar X Ns Va .config
+Configures queues allocation for consumer devices, separated by commas.
+Each device can be configured as: "<name>[:<queues>]", where:
+.Va name
+is a name of the driver which should attach the device (empty means any),
+.Va queues
+is a number of queues to allocate (empty means automatic),
+The default configuration is empty string, which means single device
+with one queue per memory window allowing any driver attachment.
 .El
 .Sh DESCRIPTION
 The

Modified: head/sys/dev/ntb/if_ntb/if_ntb.c
==============================================================================
--- head/sys/dev/ntb/if_ntb/if_ntb.c	Fri Jul 29 17:12:31 2016	(r303493)
+++ head/sys/dev/ntb/if_ntb/if_ntb.c	Fri Jul 29 17:15:41 2016	(r303494)
@@ -76,7 +76,7 @@ __FBSDID("$FreeBSD$");
 
 static SYSCTL_NODE(_hw, OID_AUTO, if_ntb, CTLFLAG_RW, 0, "if_ntb");
 
-static unsigned g_if_ntb_num_queues = 1;
+static unsigned g_if_ntb_num_queues = UINT_MAX;
 SYSCTL_UINT(_hw_if_ntb, OID_AUTO, num_queues, CTLFLAG_RWTUN,
     &g_if_ntb_num_queues, 0, "Number of queues per interface");
 
@@ -144,7 +144,8 @@ ntb_net_attach(device_t dev)
 	if_initname(ifp, device_get_name(dev), device_get_unit(dev));
 	if_setdev(ifp, dev);
 
-	sc->num_queues = g_if_ntb_num_queues;
+	sc->num_queues = min(g_if_ntb_num_queues,
+	    ntb_transport_queue_count(dev));
 	sc->queues = malloc(sc->num_queues * sizeof(struct ntb_net_queue),
 	    M_DEVBUF, M_WAITOK | M_ZERO);
 	sc->mtu = INT_MAX;
@@ -152,8 +153,7 @@ ntb_net_attach(device_t dev)
 		q = &sc->queues[i];
 		q->sc = sc;
 		q->ifp = ifp;
-		q->qp = ntb_transport_create_queue(q,
-		    device_get_parent(dev), &handlers);
+		q->qp = ntb_transport_create_queue(dev, i, &handlers, q);
 		if (q->qp == NULL)
 			break;
 		sc->mtu = imin(sc->mtu, ntb_transport_max_size(q->qp));
@@ -167,6 +167,7 @@ ntb_net_attach(device_t dev)
 		callout_init(&q->queue_full, 1);
 	}
 	sc->num_queues = i;
+	device_printf(dev, "%d queue(s)\n", sc->num_queues);
 
 	if_setinitfn(ifp, ntb_net_init);
 	if_setsoftc(ifp, sc);

Modified: head/sys/dev/ntb/ntb_transport.c
==============================================================================
--- head/sys/dev/ntb/ntb_transport.c	Fri Jul 29 17:12:31 2016	(r303493)
+++ head/sys/dev/ntb/ntb_transport.c	Fri Jul 29 17:15:41 2016	(r303494)
@@ -43,7 +43,6 @@ __FBSDID("$FreeBSD$");
 #include <sys/param.h>
 #include <sys/kernel.h>
 #include <sys/systm.h>
-#include <sys/bitset.h>
 #include <sys/bus.h>
 #include <sys/ktr.h>
 #include <sys/limits.h>
@@ -64,13 +63,6 @@ __FBSDID("$FreeBSD$");
 #include "ntb.h"
 #include "ntb_transport.h"
 
-#define QP_SETSIZE	64
-BITSET_DEFINE(_qpset, QP_SETSIZE);
-#define test_bit(pos, addr)	BIT_ISSET(QP_SETSIZE, (pos), (addr))
-#define set_bit(pos, addr)	BIT_SET(QP_SETSIZE, (pos), (addr))
-#define clear_bit(pos, addr)	BIT_CLR(QP_SETSIZE, (pos), (addr))
-#define ffs_bit(addr)		BIT_FFS(QP_SETSIZE, (addr))
-
 #define KTR_NTB KTR_SPARE3
 
 #define NTB_TRANSPORT_VERSION	4
@@ -94,12 +86,6 @@ SYSCTL_UQUAD(_hw_ntb_transport, OID_AUTO
     "If enabled (non-zero), limit the size of large memory windows. "
     "Both sides of the NTB MUST set the same value here.");
 
-static unsigned max_num_clients;
-SYSCTL_UINT(_hw_ntb_transport, OID_AUTO, max_num_clients, CTLFLAG_RDTUN,
-    &max_num_clients, 0, "Maximum number of NTB transport clients.  "
-    "0 (default) - use all available NTB memory windows; "
-    "positive integer N - Limit to N memory windows.");
-
 static unsigned enable_xeon_watchdog;
 SYSCTL_UINT(_hw_ntb_transport, OID_AUTO, enable_xeon_watchdog, CTLFLAG_RDTUN,
     &enable_xeon_watchdog, 0, "If non-zero, write a register every second to "
@@ -200,14 +186,21 @@ struct ntb_transport_mw {
 	bus_addr_t	dma_addr;
 };
 
+struct ntb_transport_child {
+	device_t	dev;
+	int		qpoff;
+	int		qpcnt;
+	struct ntb_transport_child *next;
+};
+
 struct ntb_transport_ctx {
 	device_t		 dev;
+	struct ntb_transport_child *child;
 	struct ntb_transport_mw	*mw_vec;
 	struct ntb_transport_qp	*qp_vec;
-	struct _qpset		qp_bitmap;
-	struct _qpset		qp_bitmap_free;
 	unsigned		mw_count;
 	unsigned		qp_count;
+	uint64_t		qp_bitmap;
 	volatile bool		link_is_up;
 	struct callout		link_work;
 	struct callout		link_watchdog;
@@ -242,7 +235,6 @@ enum {
 	NTBT_MW0_SZ_LOW,
 	NTBT_MW1_SZ_HIGH,
 	NTBT_MW1_SZ_LOW,
-	NTBT_MAX_SPAD,
 
 	/*
 	 * Some NTB-using hardware have a watchdog to work around NTB hangs; if
@@ -332,13 +324,44 @@ static int
 ntb_transport_attach(device_t dev)
 {
 	struct ntb_transport_ctx *nt = device_get_softc(dev);
+	struct ntb_transport_child **cpp = &nt->child;
+	struct ntb_transport_child *nc;
 	struct ntb_transport_mw *mw;
-	uint64_t qp_bitmap;
-	int rc;
-	unsigned i;
+	uint64_t db_bitmap;
+	int rc, i, db_count, spad_count, qp, qpu, qpo, qpt;
+	char cfg[128] = "";
+	char buf[32];
+	char *n, *np, *c, *name;
 
 	nt->dev = dev;
 	nt->mw_count = ntb_mw_count(dev);
+	spad_count = ntb_spad_count(dev);
+	db_bitmap = ntb_db_valid_mask(dev);
+	db_count = flsll(db_bitmap);
+	KASSERT(db_bitmap == (1 << db_count) - 1,
+	    ("Doorbells are not sequential (%jx).\n", db_bitmap));
+
+	device_printf(dev, "%d memory windows, %d scratchpads, "
+	    "%d doorbells\n", nt->mw_count, spad_count, db_count);
+
+	if (nt->mw_count == 0) {
+		device_printf(dev, "At least 1 memory window required.\n");
+		return (ENXIO);
+	}
+	if (spad_count < 6) {
+		device_printf(dev, "At least 6 scratchpads required.\n");
+		return (ENXIO);
+	}
+	if (spad_count < 4 + 2 * nt->mw_count) {
+		nt->mw_count = (spad_count - 4) / 2;
+		device_printf(dev, "Scratchpads enough only for %d "
+		    "memory windows.\n", nt->mw_count);
+	}
+	if (db_bitmap == 0) {
+		device_printf(dev, "At least one doorbell required.\n");
+		return (ENXIO);
+	}
+
 	nt->mw_vec = malloc(nt->mw_count * sizeof(*nt->mw_vec), M_NTB_T,
 	    M_WAITOK | M_ZERO);
 	for (i = 0; i < nt->mw_count; i++) {
@@ -360,25 +383,59 @@ ntb_transport_attach(device_t dev)
 			ntb_printf(0, "Unable to set mw%d caching\n", i);
 	}
 
-	qp_bitmap = ntb_db_valid_mask(dev);
-	nt->qp_count = flsll(qp_bitmap);
-	KASSERT(nt->qp_count != 0, ("bogus db bitmap"));
-	nt->qp_count -= 1;
-
-	if (max_num_clients != 0 && max_num_clients < nt->qp_count)
-		nt->qp_count = max_num_clients;
-	else if (nt->mw_count < nt->qp_count)
-		nt->qp_count = nt->mw_count;
-	KASSERT(nt->qp_count <= QP_SETSIZE, ("invalid qp_count"));
+	qpu = 0;
+	qpo = imin(db_count, nt->mw_count);
+	qpt = db_count;
+
+	snprintf(buf, sizeof(buf), "hint.%s.%d.config", device_get_name(dev),
+	    device_get_unit(dev));
+	TUNABLE_STR_FETCH(buf, cfg, sizeof(cfg));
+	n = cfg;
+	i = 0;
+	while ((c = strsep(&n, ",")) != NULL) {
+		np = c;
+		name = strsep(&np, ":");
+		if (name != NULL && name[0] == 0)
+			name = NULL;
+		qp = (np && np[0] != 0) ? strtol(np, NULL, 10) : qpo - qpu;
+		if (qp <= 0)
+			qp = 1;
+
+		if (qp > qpt - qpu) {
+			device_printf(dev, "Not enough resources for config\n");
+			break;
+		}
+
+		nc = malloc(sizeof(*nc), M_DEVBUF, M_WAITOK | M_ZERO);
+		nc->qpoff = qpu;
+		nc->qpcnt = qp;
+		nc->dev = device_add_child(dev, name, -1);
+		if (nc->dev == NULL) {
+			device_printf(dev, "Can not add child.\n");
+			break;
+		}
+		device_set_ivars(nc->dev, nc);
+		*cpp = nc;
+		cpp = &nc->next;
+
+		if (bootverbose) {
+			device_printf(dev, "%d \"%s\": queues %d",
+			    i, name, qpu);
+			if (qp > 1)
+				printf("-%d", qpu + qp - 1);
+			printf("\n");
+		}
+
+		qpu += qp;
+		i++;
+	}
+	nt->qp_count = qpu;
 
 	nt->qp_vec = malloc(nt->qp_count * sizeof(*nt->qp_vec), M_NTB_T,
 	    M_WAITOK | M_ZERO);
 
-	for (i = 0; i < nt->qp_count; i++) {
-		set_bit(i, &nt->qp_bitmap);
-		set_bit(i, &nt->qp_bitmap_free);
+	for (i = 0; i < nt->qp_count; i++)
 		ntb_transport_init_queue(nt, i);
-	}
 
 	callout_init(&nt->link_work, 0);
 	callout_init(&nt->link_watchdog, 0);
@@ -394,10 +451,7 @@ ntb_transport_attach(device_t dev)
 	if (enable_xeon_watchdog != 0)
 		callout_reset(&nt->link_watchdog, 0, xeon_link_watchdog_hb, nt);
 
-	/* Attach children to this transport */
-	device_add_child(dev, NULL, -1);
 	bus_generic_attach(dev);
-
 	return (0);
 
 err:
@@ -410,25 +464,25 @@ static int
 ntb_transport_detach(device_t dev)
 {
 	struct ntb_transport_ctx *nt = device_get_softc(dev);
-	struct _qpset qp_bitmap_alloc;
-	uint8_t i;
-
-	/* Detach & delete all children */
-	device_delete_children(dev);
+	struct ntb_transport_child **cpp = &nt->child;
+	struct ntb_transport_child *nc;
+	int error = 0, i;
+
+	while ((nc = *cpp) != NULL) {
+		*cpp = (*cpp)->next;
+		error = device_delete_child(dev, nc->dev);
+		if (error)
+			break;
+		free(nc, M_DEVBUF);
+	}
+	KASSERT(nt->qp_bitmap == 0,
+	    ("Some queues not freed on detach (%jx)", nt->qp_bitmap));
 
 	ntb_transport_link_cleanup(nt);
 	taskqueue_drain(taskqueue_swi, &nt->link_cleanup);
 	callout_drain(&nt->link_work);
 	callout_drain(&nt->link_watchdog);
 
-	BIT_COPY(QP_SETSIZE, &nt->qp_bitmap, &qp_bitmap_alloc);
-	BIT_NAND(QP_SETSIZE, &qp_bitmap_alloc, &nt->qp_bitmap_free);
-
-	/* Verify that all the QPs are freed */
-	for (i = 0; i < nt->qp_count; i++)
-		if (test_bit(i, &qp_bitmap_alloc))
-			ntb_transport_free_queue(&nt->qp_vec[i]);
-
 	ntb_link_disable(dev);
 	ntb_clear_ctx(dev);
 
@@ -440,6 +494,14 @@ ntb_transport_detach(device_t dev)
 	return (0);
 }
 
+int
+ntb_transport_queue_count(device_t dev)
+{
+	struct ntb_transport_child *nc = device_get_ivars(dev);
+
+	return (nc->qpcnt);
+}
+
 static void
 ntb_transport_init_queue(struct ntb_transport_ctx *nt, unsigned int qp_num)
 {
@@ -507,6 +569,7 @@ ntb_transport_init_queue(struct ntb_tran
 void
 ntb_transport_free_queue(struct ntb_transport_qp *qp)
 {
+	struct ntb_transport_ctx *nt = qp->transport;
 	struct ntb_queue_entry *entry;
 
 	if (qp == NULL)
@@ -532,7 +595,7 @@ ntb_transport_free_queue(struct ntb_tran
 	while ((entry = ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q)))
 		free(entry, M_NTB_T);
 
-	set_bit(qp->qp_num, &qp->transport->qp_bitmap_free);
+	nt->qp_bitmap &= ~(1 << qp->qp_num);
 }
 
 /**
@@ -550,24 +613,20 @@ ntb_transport_free_queue(struct ntb_tran
  * RETURNS: pointer to newly created ntb_queue, NULL on error.
  */
 struct ntb_transport_qp *
-ntb_transport_create_queue(void *data, device_t dev,
-    const struct ntb_queue_handlers *handlers)
+ntb_transport_create_queue(device_t dev, int q,
+    const struct ntb_queue_handlers *handlers, void *data)
 {
-	struct ntb_transport_ctx *nt = device_get_softc(dev);
+	struct ntb_transport_child *nc = device_get_ivars(dev);
+	struct ntb_transport_ctx *nt = device_get_softc(device_get_parent(dev));
 	struct ntb_queue_entry *entry;
 	struct ntb_transport_qp *qp;
-	unsigned int free_queue;
 	int i;
 
-	free_queue = ffs_bit(&nt->qp_bitmap_free);
-	if (free_queue == 0)
+	if (q < 0 || q >= nc->qpcnt)
 		return (NULL);
 
-	/* decrement free_queue to make it zero based */
-	free_queue--;
-
-	qp = &nt->qp_vec[free_queue];
-	clear_bit(qp->qp_num, &nt->qp_bitmap_free);
+	qp = &nt->qp_vec[nc->qpoff + q];
+	nt->qp_bitmap |= (1 << qp->qp_num);
 	qp->cb_data = data;
 	qp->rx_handler = handlers->rx_handler;
 	qp->tx_handler = handlers->tx_handler;
@@ -944,24 +1003,19 @@ ntb_transport_doorbell_callback(void *da
 {
 	struct ntb_transport_ctx *nt = data;
 	struct ntb_transport_qp *qp;
-	struct _qpset db_bits;
 	uint64_t vec_mask;
 	unsigned qp_num;
 
-	BIT_COPY(QP_SETSIZE, &nt->qp_bitmap, &db_bits);
-	BIT_NAND(QP_SETSIZE, &db_bits, &nt->qp_bitmap_free);
-
 	vec_mask = ntb_db_vector_mask(nt->dev, vector);
+	vec_mask &= nt->qp_bitmap;
 	if ((vec_mask & (vec_mask - 1)) != 0)
 		vec_mask &= ntb_db_read(nt->dev);
 	while (vec_mask != 0) {
 		qp_num = ffsll(vec_mask) - 1;
 
-		if (test_bit(qp_num, &db_bits)) {
-			qp = &nt->qp_vec[qp_num];
-			if (qp->link_is_up)
-				taskqueue_enqueue(qp->rxc_tq, &qp->rxc_db_work);
-		}
+		qp = &nt->qp_vec[qp_num];
+		if (qp->link_is_up)
+			taskqueue_enqueue(qp->rxc_tq, &qp->rxc_db_work);
 
 		vec_mask &= ~(1ull << qp_num);
 	}
@@ -1219,19 +1273,16 @@ static void
 ntb_transport_link_cleanup(struct ntb_transport_ctx *nt)
 {
 	struct ntb_transport_qp *qp;
-	struct _qpset qp_bitmap_alloc;
-	unsigned i;
-
-	BIT_COPY(QP_SETSIZE, &nt->qp_bitmap, &qp_bitmap_alloc);
-	BIT_NAND(QP_SETSIZE, &qp_bitmap_alloc, &nt->qp_bitmap_free);
+	int i;
 
 	/* Pass along the info to any clients */
-	for (i = 0; i < nt->qp_count; i++)
-		if (test_bit(i, &qp_bitmap_alloc)) {
+	for (i = 0; i < nt->qp_count; i++) {
+		if ((nt->qp_bitmap & (1 << i)) != 0) {
 			qp = &nt->qp_vec[i];
 			ntb_qp_link_cleanup(qp);
 			callout_drain(&qp->link_work);
 		}
+	}
 
 	if (!nt->link_is_up)
 		callout_drain(&nt->link_work);
@@ -1241,8 +1292,7 @@ ntb_transport_link_cleanup(struct ntb_tr
 	 * goes down, blast them now to give them a sane value the next
 	 * time they are accessed
 	 */
-	for (i = 0; i < NTBT_MAX_SPAD; i++)
-		ntb_spad_write(nt->dev, i, 0);
+	ntb_spad_clear(nt->dev);
 }
 
 static void

Modified: head/sys/dev/ntb/ntb_transport.h
==============================================================================
--- head/sys/dev/ntb/ntb_transport.h	Fri Jul 29 17:12:31 2016	(r303493)
+++ head/sys/dev/ntb/ntb_transport.h	Fri Jul 29 17:15:41 2016	(r303494)
@@ -43,12 +43,13 @@ struct ntb_queue_handlers {
 	void (*event_handler)(void *data, enum ntb_link_event status);
 };
 
-unsigned char ntb_transport_qp_num(struct ntb_transport_qp *qp);
-unsigned int ntb_transport_max_size(struct ntb_transport_qp *qp);
+int ntb_transport_queue_count(device_t dev);
 struct ntb_transport_qp *
-ntb_transport_create_queue(void *data, device_t dev,
-			   const struct ntb_queue_handlers *handlers);
+ntb_transport_create_queue(device_t dev, int q,
+    const struct ntb_queue_handlers *handlers, void *data);
 void ntb_transport_free_queue(struct ntb_transport_qp *qp);
+unsigned char ntb_transport_qp_num(struct ntb_transport_qp *qp);
+unsigned int ntb_transport_max_size(struct ntb_transport_qp *qp);
 int ntb_transport_rx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
 			     unsigned int len);
 int ntb_transport_tx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201607291715.u6THFg5l082797>