linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [V2 net-next] net: mvpp2: Add reserved port private flag configuration
@ 2021-03-11 16:43 stefanc
  2021-03-11 16:59 ` Andrew Lunn
  2021-03-11 20:33 ` Jakub Kicinski
  0 siblings, 2 replies; 10+ messages in thread
From: stefanc @ 2021-03-11 16:43 UTC (permalink / raw)
  To: netdev
  Cc: thomas.petazzoni, davem, nadavh, ymarkman, linux-kernel, stefanc,
	kuba, linux, mw, andrew, rmk+kernel, atenart, rabeeh

From: Stefan Chulski <stefanc@marvell.com>

According to Armada SoC architecture and design, all the PPv2 ports
which are populated on the same communication processor silicon die
(CP11x) share the same Classifier and Parser engines.

Armada is an embedded platform and therefore there is a need to reserve
some of the PPv2 ports for different use cases.

For example, a port can be reserved for a CM3 CPU running FreeRTOS for
management purposes or by user-space data plane application.

During port reservation all common configurations are preserved and
only RXQ, TXQ, and interrupt vectors are disabled.
Since TXQ's are disabled, the Kernel won't transmit any packet
from this port, and to due the closed RXQ interrupts, the Kernel won't
receive any packet.
The port MAC address and administrative UP/DOWN state can still
be changed.
The only permitted configuration in this mode is MTU change.
The driver's .ndo_change_mtu callback has logic that switches between
percpu_pools and shared pools buffer mode, since the buffer management
not done by Kernel this should be permitted.

v1 --> v2
- Rename existing mvpp2_ethtool_get_strings and helper _priv function
- Add more info to commit message

Signed-off-by: Stefan Chulski <stefanc@marvell.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h      |   4 +
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 248 ++++++++++++++++++--
 2 files changed, 228 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 8edba5e..e2f8eec 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -865,6 +865,7 @@
 /* Port flags */
 #define MVPP2_F_LOOPBACK		BIT(0)
 #define MVPP2_F_DT_COMPAT		BIT(1)
+#define MVPP22_F_IF_RESERVED		BIT(2)
 
 /* Marvell tag types */
 enum mvpp2_tag_type {
@@ -1251,6 +1252,9 @@ struct mvpp2_port {
 
 	/* Firmware TX flow control */
 	bool tx_fc;
+
+	/* private storage, allocated/used by Reserved/Normal mode toggling */
+	void *res_cfg;
 };
 
 /* The mvpp2_tx_desc and mvpp2_rx_desc structures describe the
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index d415447..7406724 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -55,6 +55,14 @@ enum mvpp2_bm_pool_log_num {
 	int buf_num;
 } mvpp2_pools[MVPP2_BM_POOLS_NUM];
 
+struct mvpp2_port_port_cfg {
+	unsigned int nqvecs;
+	unsigned int nrxqs;
+	unsigned int ntxqs;
+	int mtu;
+	bool rxhash_en;
+};
+
 /* The prototype is added here to be used in start_dev when using ACPI. This
  * will be removed once phylink is used for all modes (dt+ACPI).
  */
@@ -1431,6 +1439,9 @@ static void mvpp2_interrupts_unmask(void *arg)
 	if (cpu >= port->priv->nthreads)
 		return;
 
+	if (port->flags & MVPP22_F_IF_RESERVED)
+		return;
+
 	thread = mvpp2_cpu_to_thread(port->priv, cpu);
 
 	val = MVPP2_CAUSE_MISC_SUM_MASK |
@@ -1942,15 +1953,23 @@ static u32 mvpp2_read_index(struct mvpp2 *priv, u32 index, u32 reg)
 						 (ARRAY_SIZE(mvpp2_ethtool_rxq_regs) * (nrxqs)) + \
 						 ARRAY_SIZE(mvpp2_ethtool_xdp))
 
-static void mvpp2_ethtool_get_strings(struct net_device *netdev, u32 sset,
-				      u8 *data)
+static const char mvpp22_priv_flags_strings[][ETH_GSTRING_LEN] = {
+	"reserved",
+};
+
+#define MVPP22_F_IF_RESERVED_PRIV	BIT(0)
+
+static void mvpp2_ethtool_get_strings_priv(u8 *data)
+{
+	memcpy(data, mvpp22_priv_flags_strings,
+	       ARRAY_SIZE(mvpp22_priv_flags_strings) * ETH_GSTRING_LEN);
+}
+
+static void mvpp2_ethtool_get_strings_stats(struct net_device *netdev, u8 *data)
 {
 	struct mvpp2_port *port = netdev_priv(netdev);
 	int i, q;
 
-	if (sset != ETH_SS_STATS)
-		return;
-
 	for (i = 0; i < ARRAY_SIZE(mvpp2_ethtool_mib_regs); i++) {
 		strscpy(data, mvpp2_ethtool_mib_regs[i].string,
 			ETH_GSTRING_LEN);
@@ -1987,6 +2006,18 @@ static void mvpp2_ethtool_get_strings(struct net_device *netdev, u32 sset,
 	}
 }
 
+static void mvpp2_ethtool_get_strings(struct net_device *netdev, u32 sset,
+				      u8 *data)
+{
+	switch (sset) {
+	case ETH_SS_STATS:
+		mvpp2_ethtool_get_strings_stats(netdev, data);
+		break;
+	case ETH_SS_PRIV_FLAGS:
+		mvpp2_ethtool_get_strings_priv(data);
+	}
+}
+
 static void
 mvpp2_get_xdp_stats(struct mvpp2_port *port, struct mvpp2_pcpu_stats *xdp_stats)
 {
@@ -2130,8 +2161,13 @@ static int mvpp2_ethtool_get_sset_count(struct net_device *dev, int sset)
 {
 	struct mvpp2_port *port = netdev_priv(dev);
 
-	if (sset == ETH_SS_STATS)
+	switch (sset) {
+	case ETH_SS_STATS:
 		return MVPP2_N_ETHTOOL_STATS(port->ntxqs, port->nrxqs);
+	case ETH_SS_PRIV_FLAGS:
+		return (port->priv->hw_version == MVPP21) ?
+			0 : ARRAY_SIZE(mvpp22_priv_flags_strings);
+	}
 
 	return -EOPNOTSUPP;
 }
@@ -2207,6 +2243,9 @@ static inline void mvpp2_gmac_max_rx_size_set(struct mvpp2_port *port)
 {
 	u32 val;
 
+	if (port->flags & MVPP22_F_IF_RESERVED)
+		return;
+
 	val = readl(port->base + MVPP2_GMAC_CTRL_0_REG);
 	val &= ~MVPP2_GMAC_MAX_RX_SIZE_MASK;
 	val |= (((port->pkt_size - MVPP2_MH_SIZE) / 2) <<
@@ -2219,6 +2258,9 @@ static inline void mvpp2_xlg_max_rx_size_set(struct mvpp2_port *port)
 {
 	u32 val;
 
+	if (port->flags & MVPP22_F_IF_RESERVED)
+		return;
+
 	val =  readl(port->base + MVPP22_XLG_CTRL1_REG);
 	val &= ~MVPP22_XLG_CTRL1_FRAMESIZELIMIT_MASK;
 	val |= ((port->pkt_size - MVPP2_MH_SIZE) / 2) <<
@@ -2321,6 +2363,9 @@ static void mvpp2_egress_enable(struct mvpp2_port *port)
 	int queue;
 	int tx_port_num = mvpp2_egress_port(port);
 
+	if (port->flags & MVPP22_F_IF_RESERVED)
+		return;
+
 	/* Enable all initialized TXs. */
 	qmap = 0;
 	for (queue = 0; queue < port->ntxqs; queue++) {
@@ -2343,6 +2388,9 @@ static void mvpp2_egress_disable(struct mvpp2_port *port)
 	int delay;
 	int tx_port_num = mvpp2_egress_port(port);
 
+	if (port->flags & MVPP22_F_IF_RESERVED)
+		return;
+
 	/* Issue stop command for active channels only */
 	mvpp2_write(port->priv, MVPP2_TXP_SCHED_PORT_INDEX_REG, tx_port_num);
 	reg_data = (mvpp2_read(port->priv, MVPP2_TXP_SCHED_Q_CMD_REG)) &
@@ -3411,7 +3459,8 @@ static void mvpp2_isr_handle_link(struct mvpp2_port *port, bool link)
 		mvpp2_egress_enable(port);
 		mvpp2_ingress_enable(port);
 		netif_carrier_on(dev);
-		netif_tx_wake_all_queues(dev);
+		if (!(port->flags & MVPP22_F_IF_RESERVED))
+			netif_tx_wake_all_queues(dev);
 	} else {
 		netif_tx_stop_all_queues(dev);
 		netif_carrier_off(dev);
@@ -4556,7 +4605,8 @@ static void mvpp2_start_dev(struct mvpp2_port *port)
 		mvpp2_acpi_start(port);
 	}
 
-	netif_tx_start_all_queues(port->dev);
+	if (!(port->flags & MVPP22_F_IF_RESERVED))
+		netif_tx_start_all_queues(port->dev);
 
 	clear_bit(0, &port->state);
 }
@@ -4702,7 +4752,8 @@ static void mvpp2_irqs_deinit(struct mvpp2_port *port)
 static bool mvpp22_rss_is_supported(struct mvpp2_port *port)
 {
 	return (queue_mode == MVPP2_QDIST_MULTI_MODE) &&
-		!(port->flags & MVPP2_F_LOOPBACK);
+		!(port->flags & MVPP2_F_LOOPBACK) &&
+		!(port->flags & MVPP22_F_IF_RESERVED);
 }
 
 static int mvpp2_open(struct net_device *dev)
@@ -4719,20 +4770,23 @@ static int mvpp2_open(struct net_device *dev)
 		netdev_err(dev, "mvpp2_prs_mac_da_accept BC failed\n");
 		return err;
 	}
-	err = mvpp2_prs_mac_da_accept(port, dev->dev_addr, true);
-	if (err) {
-		netdev_err(dev, "mvpp2_prs_mac_da_accept own addr failed\n");
-		return err;
-	}
-	err = mvpp2_prs_tag_mode_set(port->priv, port->id, MVPP2_TAG_TYPE_MH);
-	if (err) {
-		netdev_err(dev, "mvpp2_prs_tag_mode_set failed\n");
-		return err;
-	}
-	err = mvpp2_prs_def_flow(port);
-	if (err) {
-		netdev_err(dev, "mvpp2_prs_def_flow failed\n");
-		return err;
+
+	if (!(port->flags & MVPP22_F_IF_RESERVED)) {
+		err = mvpp2_prs_mac_da_accept(port, dev->dev_addr, true);
+		if (err) {
+			netdev_err(dev, "mvpp2_prs_mac_da_accept own addr failed\n");
+			return err;
+		}
+		err = mvpp2_prs_tag_mode_set(port->priv, port->id, MVPP2_TAG_TYPE_MH);
+		if (err) {
+			netdev_err(dev, "mvpp2_prs_tag_mode_set failed\n");
+			return err;
+		}
+		err = mvpp2_prs_def_flow(port);
+		if (err) {
+			netdev_err(dev, "mvpp2_prs_def_flow failed\n");
+			return err;
+		}
 	}
 
 	/* Allocate the Rx/Tx queues */
@@ -4979,6 +5033,11 @@ static int mvpp2_change_mtu(struct net_device *dev, int mtu)
 	struct mvpp2 *priv = port->priv;
 	int err;
 
+	if (port->flags & MVPP22_F_IF_RESERVED) {
+		netdev_err(dev, "MTU can not be modified for port in reserved mode\n");
+		return -EPERM;
+	}
+
 	if (!IS_ALIGNED(MVPP2_RX_PKT_SIZE(mtu), 8)) {
 		netdev_info(dev, "illegal MTU value %d, round to %d\n", mtu,
 			    ALIGN(MVPP2_RX_PKT_SIZE(mtu), 8));
@@ -5385,12 +5444,16 @@ static int mvpp2_ethtool_get_coalesce(struct net_device *dev,
 static void mvpp2_ethtool_get_drvinfo(struct net_device *dev,
 				      struct ethtool_drvinfo *drvinfo)
 {
+	struct mvpp2_port *port = netdev_priv(dev);
+
 	strlcpy(drvinfo->driver, MVPP2_DRIVER_NAME,
 		sizeof(drvinfo->driver));
 	strlcpy(drvinfo->version, MVPP2_DRIVER_VERSION,
 		sizeof(drvinfo->version));
 	strlcpy(drvinfo->bus_info, dev_name(&dev->dev),
 		sizeof(drvinfo->bus_info));
+	drvinfo->n_priv_flags = (port->priv->hw_version == MVPP21) ?
+			0 : ARRAY_SIZE(mvpp22_priv_flags_strings);
 }
 
 static void mvpp2_ethtool_get_ringparam(struct net_device *dev,
@@ -5662,6 +5725,139 @@ static int mvpp2_ethtool_set_rxfh_context(struct net_device *dev,
 
 	return mvpp22_port_rss_ctx_indir_set(port, *rss_context, indir);
 }
+
+static u32 mvpp22_get_priv_flags(struct net_device *dev)
+{
+	struct mvpp2_port *port = netdev_priv(dev);
+	u32 priv_flags = 0;
+
+	if (port->flags & MVPP22_F_IF_RESERVED)
+		priv_flags |= MVPP22_F_IF_RESERVED_PRIV;
+	return priv_flags;
+}
+
+static int mvpp2_port_reserved_cfg(struct net_device *dev, bool ena)
+{
+	struct mvpp2_port *port = netdev_priv(dev);
+	struct mvpp2_port_port_cfg *cfg;
+
+	if (ena) {
+		/* Disable Queues and IntVec allocations for reserved ports,
+		 * but save original values.
+		 */
+		cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
+		if (!cfg)
+			return -ENOMEM;
+		port->res_cfg = (void *)cfg;
+		cfg->nqvecs = port->nqvecs;
+		cfg->nrxqs  = port->nrxqs;
+		cfg->ntxqs = port->ntxqs;
+		cfg->mtu = dev->mtu;
+		cfg->rxhash_en = !!(dev->hw_features & NETIF_F_RXHASH);
+
+		port->nqvecs = 0;
+		port->nrxqs  = 0;
+		port->ntxqs  = 0;
+		if (cfg->rxhash_en) {
+			dev->hw_features &= ~NETIF_F_RXHASH;
+			netdev_update_features(dev);
+		}
+	} else {
+		struct mvpp2_bm_pool *pool;
+		int i;
+
+		/* Back to normal mode */
+		cfg = port->res_cfg;
+		port->nqvecs = cfg->nqvecs;
+		port->nrxqs  = cfg->nrxqs;
+		port->ntxqs  = cfg->ntxqs;
+		if (cfg->rxhash_en) {
+			dev->hw_features |= NETIF_F_RXHASH;
+			netdev_update_features(dev);
+		}
+		kfree(cfg);
+		port->res_cfg = NULL;
+
+		/* Restore RxQ/pool association */
+		for (i = 0; i < port->nrxqs; i++) {
+			if (port->priv->percpu_pools) {
+				pool = &port->priv->bm_pools[i];
+				mvpp2_rxq_short_pool_set(port, i, pool->id);
+				pool = &port->priv->bm_pools[i + port->nrxqs];
+				mvpp2_rxq_long_pool_set(port, i, pool->id);
+			} else {
+				mvpp2_rxq_short_pool_set(port, i, port->pool_short->id);
+				mvpp2_rxq_long_pool_set(port, i, port->pool_long->id);
+			}
+		}
+	}
+	return 0;
+}
+
+static int mvpp2_port_reserved_set(struct net_device *dev, bool ena)
+{
+	struct mvpp2_port *port = netdev_priv(dev);
+	bool running = netif_running(dev);
+	struct mvpp2 *priv = port->priv;
+	int err;
+
+	/* This procedure is called by ethtool change or by Module-remove.
+	 * For "remove" do anything only if we are in reserved-mode
+	 * and toggling back to Normal-mode is required.
+	 */
+	if (!ena && !port->res_cfg)
+		return 0;
+
+	if (ena) {
+		port->flags |= MVPP22_F_IF_RESERVED;
+		if (priv->percpu_pools)
+			mvpp2_bm_switch_buffers(priv, false);
+	} else {
+		bool reserved = false;
+		int i;
+
+		port->flags &= ~MVPP22_F_IF_RESERVED;
+		for (i = 0; i < priv->port_count; i++)
+			if (priv->port_list[i] != port &&
+			    priv->port_list[i]->flags & MVPP22_F_IF_RESERVED) {
+				reserved = true;
+				break;
+			}
+
+		if (!reserved) {
+			dev_info(port->dev->dev.parent,
+				 "No ports in reserved mode, switching to per-cpu buffers");
+			mvpp2_bm_switch_buffers(priv, true);
+		}
+	}
+
+	if (running)
+		mvpp2_stop(dev);
+
+	err = mvpp2_port_reserved_cfg(dev, ena);
+	if (err)
+		netdev_err(dev, "reserved set=%d: error=%d\n", ena, err);
+
+	if (running)
+		mvpp2_open(dev);
+
+	return 0;
+}
+
+static int mvpp22_set_priv_flags(struct net_device *dev, u32 priv_flags)
+{
+	struct mvpp2_port *port = netdev_priv(dev);
+	bool f_old, f_new;
+	int err = 0;
+
+	f_old = port->flags & MVPP22_F_IF_RESERVED;
+	f_new = priv_flags & MVPP22_F_IF_RESERVED_PRIV;
+	if (f_old != f_new)
+		err = mvpp2_port_reserved_set(dev, f_new);
+
+	return err;
+}
+
 /* Device ops */
 
 static const struct net_device_ops mvpp2_netdev_ops = {
@@ -5705,6 +5901,8 @@ static int mvpp2_ethtool_set_rxfh_context(struct net_device *dev,
 	.set_rxfh		= mvpp2_ethtool_set_rxfh,
 	.get_rxfh_context	= mvpp2_ethtool_get_rxfh_context,
 	.set_rxfh_context	= mvpp2_ethtool_set_rxfh_context,
+	.get_priv_flags		= mvpp22_get_priv_flags,
+	.set_priv_flags		= mvpp22_set_priv_flags,
 };
 
 /* Used for PPv2.1, or PPv2.2 with the old Device Tree binding that
@@ -6602,7 +6800,8 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 
 	mvpp2_egress_enable(port);
 	mvpp2_ingress_enable(port);
-	netif_tx_wake_all_queues(port->dev);
+	if (!(port->flags & MVPP22_F_IF_RESERVED))
+		netif_tx_wake_all_queues(port->dev);
 }
 
 static void mvpp2_mac_link_down(struct phylink_config *config,
@@ -6944,6 +7143,7 @@ static void mvpp2_port_remove(struct mvpp2_port *port)
 {
 	int i;
 
+	mvpp2_port_reserved_set(port->dev, false);
 	unregister_netdev(port->dev);
 	if (port->phylink)
 		phylink_destroy(port->phylink);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [V2 net-next] net: mvpp2: Add reserved port private flag configuration
  2021-03-11 16:43 [V2 net-next] net: mvpp2: Add reserved port private flag configuration stefanc
@ 2021-03-11 16:59 ` Andrew Lunn
  2021-03-16 15:28   ` [EXT] " Stefan Chulski
  2021-03-11 20:33 ` Jakub Kicinski
  1 sibling, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2021-03-11 16:59 UTC (permalink / raw)
  To: stefanc
  Cc: netdev, thomas.petazzoni, davem, nadavh, ymarkman, linux-kernel,
	kuba, linux, mw, rmk+kernel, atenart, rabeeh

On Thu, Mar 11, 2021 at 06:43:27PM +0200, stefanc@marvell.com wrote:
> From: Stefan Chulski <stefanc@marvell.com>

Hi Stefan

Thanks for the strings change. Looks a lot better.

Now i took a look at the bigger picture.

> According to Armada SoC architecture and design, all the PPv2 ports
> which are populated on the same communication processor silicon die
> (CP11x) share the same Classifier and Parser engines.
> 
> Armada is an embedded platform and therefore there is a need to reserve
> some of the PPv2 ports for different use cases.
> 
> For example, a port can be reserved for a CM3 CPU running FreeRTOS
> for management purposes

So the CM3 CPU has its own driver for this hardware? It seems like we
should not even instantiate the Linux driver for this port. Does the
CM3 have its own DT blob? I think the better solution is that the
Armada DT for the board does not list the port, and the DT for the CM3
does. Linux never sees the port, since Linux should not be using it.

> or by user-space data plane application.

You mean XDP/AF_XDP? I don't see any other XDP capable drivers having
a flag like this. If this was required, i would expect it to be a
common properly, not driver private.

	  Andrew

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [V2 net-next] net: mvpp2: Add reserved port private flag configuration
  2021-03-11 16:43 [V2 net-next] net: mvpp2: Add reserved port private flag configuration stefanc
  2021-03-11 16:59 ` Andrew Lunn
@ 2021-03-11 20:33 ` Jakub Kicinski
  1 sibling, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2021-03-11 20:33 UTC (permalink / raw)
  To: stefanc
  Cc: netdev, thomas.petazzoni, davem, nadavh, ymarkman, linux-kernel,
	linux, mw, andrew, rmk+kernel, atenart, rabeeh

On Thu, 11 Mar 2021 18:43:27 +0200 stefanc@marvell.com wrote:
> According to Armada SoC architecture and design, all the PPv2 ports
> which are populated on the same communication processor silicon die
> (CP11x) share the same Classifier and Parser engines.
> 
> Armada is an embedded platform and therefore there is a need to reserve
> some of the PPv2 ports for different use cases.
> 
> For example, a port can be reserved for a CM3 CPU running FreeRTOS for
> management purposes or by user-space data plane application.
> 
> During port reservation all common configurations are preserved and
> only RXQ, TXQ, and interrupt vectors are disabled.
> Since TXQ's are disabled, the Kernel won't transmit any packet
> from this port, and to due the closed RXQ interrupts, the Kernel won't
> receive any packet.
> The port MAC address and administrative UP/DOWN state can still
> be changed.
> The only permitted configuration in this mode is MTU change.
> The driver's .ndo_change_mtu callback has logic that switches between
> percpu_pools and shared pools buffer mode, since the buffer management
> not done by Kernel this should be permitted.

Andrew asks good questions. This looks like a strange construct.

IMO Linux should either not see the port (like it doesn't see NC-SI),
or we need representors for physical and logical ports and explicit
forwarding rules.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [EXT] Re: [V2 net-next] net: mvpp2: Add reserved port private flag configuration
  2021-03-11 16:59 ` Andrew Lunn
@ 2021-03-16 15:28   ` Stefan Chulski
  2021-03-16 15:41     ` Russell King - ARM Linux admin
  2021-03-18 20:53     ` Andrew Lunn
  0 siblings, 2 replies; 10+ messages in thread
From: Stefan Chulski @ 2021-03-16 15:28 UTC (permalink / raw)
  To: Andrew Lunn, kuba
  Cc: netdev, thomas.petazzoni, davem, Nadav Haklai, Yan Markman,
	linux-kernel, linux, mw, rmk+kernel, atenart, rabeeh

> Hi Stefan
> 
> Thanks for the strings change. Looks a lot better.
> 
> Now i took a look at the bigger picture.
> 
> > According to Armada SoC architecture and design, all the PPv2 ports
> > which are populated on the same communication processor silicon die
> > (CP11x) share the same Classifier and Parser engines.
> >
> > Armada is an embedded platform and therefore there is a need to
> > reserve some of the PPv2 ports for different use cases.
> >
> > For example, a port can be reserved for a CM3 CPU running FreeRTOS for
> > management purposes

Hi Andrew and Jakub,

There are multiple reasons why we must let the kernel initialize and manage the reserved port:
1. According to pp2 hardware design, all classifier and parser configuration access are indirect. In order to prevent race conditions, it needs to be configured by single entity. 
2. CM3 code has very small footprint requirement, we cannot implement the complete Serdes and PHY infrastructure that kernel provides as part of CM3 application. Therefore I would like to continue relying on kernel configuration for that.
3. In some cases we need to dynamically switch the port "user" between CM3 and kernel. So I would like to preserve this functionality.

> So the CM3 CPU has its own driver for this hardware? It seems like we should
> not even instantiate the Linux driver for this port. Does the
> CM3 have its own DT blob? I think the better solution is that the Armada DT
> for the board does not list the port, and the DT for the CM3 does. Linux
> never sees the port, since Linux should not be using it.
> 
> > or by user-space data plane application.
> 
> You mean XDP/AF_XDP? I don't see any other XDP capable drivers having a
> flag like this. If this was required, i would expect it to be a common properly,
> not driver private.

No XDP doesn't require this. One of the use cases of the port reservation feature is the Marvell User Space SDK (MUSDK) which its latest code is publicly available here:
https://github.com/MarvellEmbeddedProcessors/musdk-marvell
You can find example use case for this application here:
http://wiki.macchiatobin.net/tiki-index.php?page=MUSDK+Introduction

Stefan.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [EXT] Re: [V2 net-next] net: mvpp2: Add reserved port private flag configuration
  2021-03-16 15:28   ` [EXT] " Stefan Chulski
@ 2021-03-16 15:41     ` Russell King - ARM Linux admin
  2021-03-16 16:51       ` Stefan Chulski
  2021-03-18 20:53     ` Andrew Lunn
  1 sibling, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux admin @ 2021-03-16 15:41 UTC (permalink / raw)
  To: Stefan Chulski
  Cc: Andrew Lunn, kuba, netdev, thomas.petazzoni, davem, Nadav Haklai,
	Yan Markman, linux-kernel, mw, atenart, rabeeh

On Tue, Mar 16, 2021 at 03:28:51PM +0000, Stefan Chulski wrote:
> No XDP doesn't require this. One of the use cases of the port reservation feature is the Marvell User Space SDK (MUSDK) which its latest code is publicly available here:
> https://github.com/MarvellEmbeddedProcessors/musdk-marvell
> You can find example use case for this application here:
> http://wiki.macchiatobin.net/tiki-index.php?page=MUSDK+Introduction

I really, really hope that someone has thought this through:

  Packet Processor I/O Interface (PPIO)

   The MUSDK PPIO driver provides low-level network interface API for
   User-Space network drivers/applications. The PPIO infrastrcuture maps
   Marvell's Packet Processor (PPv2) configuration space and I/O descriptors
   space directly to user-space memory. This allows user-space
   driver/application to directly process the packet processor I/O rings from
   user space, without any overhead of a copy operation.

I realy, really hope that you are not exposing the I/O descriptors to
userspace, allowing userspace to manipulate the physical addresses in
those descriptors, and that userspace is not dealing with physical
addresses.

If userspace has access to the I/O descriptors with physical addresses,
or userspace is dealing with physical addresses, then you can say
good bye to any kind of security on the platform. Essentially, in such
a scenario, the entire system memory becomes accessible to userspace,
which includes the kernel.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [EXT] Re: [V2 net-next] net: mvpp2: Add reserved port private flag configuration
  2021-03-16 15:41     ` Russell King - ARM Linux admin
@ 2021-03-16 16:51       ` Stefan Chulski
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Chulski @ 2021-03-16 16:51 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Andrew Lunn, kuba, netdev, thomas.petazzoni, davem, Nadav Haklai,
	Yan Markman, linux-kernel, mw, atenart, rabeeh

> I really, really hope that someone has thought this through:
> 
>   Packet Processor I/O Interface (PPIO)
> 
>    The MUSDK PPIO driver provides low-level network interface API for
>    User-Space network drivers/applications. The PPIO infrastrcuture maps
>    Marvell's Packet Processor (PPv2) configuration space and I/O descriptors
>    space directly to user-space memory. This allows user-space
>    driver/application to directly process the packet processor I/O rings from
>    user space, without any overhead of a copy operation.
> 
> I realy, really hope that you are not exposing the I/O descriptors to
> userspace, allowing userspace to manipulate the physical addresses in those
> descriptors, and that userspace is not dealing with physical addresses.
> 
> If userspace has access to the I/O descriptors with physical addresses, or
> userspace is dealing with physical addresses, then you can say good bye to
> any kind of security on the platform. Essentially, in such a scenario, the entire
> system memory becomes accessible to userspace, which includes the kernel.

Hi Russel,

This patch doesn't relate to MUSDK Packet Processor I/O Interface functionality.
MUSDK is just another possible use case I could think of for the port reservation feature.
I am not responsible for the MUSDK code, but as far as I know it is based on the generic UIO Kernel interface (uio_pdrv_genirq) so the user can decide whether he wants to enable it or not for his platform.
For the main CM3 management port use case, security is not an issue since the CM3 processor is secured by hardware in the device and its code is authenticated.

Stefan.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [EXT] Re: [V2 net-next] net: mvpp2: Add reserved port private flag configuration
  2021-03-16 15:28   ` [EXT] " Stefan Chulski
  2021-03-16 15:41     ` Russell King - ARM Linux admin
@ 2021-03-18 20:53     ` Andrew Lunn
  2021-03-22 15:59       ` Stefan Chulski
  1 sibling, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2021-03-18 20:53 UTC (permalink / raw)
  To: Stefan Chulski
  Cc: kuba, netdev, thomas.petazzoni, davem, Nadav Haklai, Yan Markman,
	linux-kernel, linux, mw, rmk+kernel, atenart, rabeeh

> 2. CM3 code has very small footprint requirement, we cannot
> implement the complete Serdes and PHY infrastructure that kernel
> provides as part of CM3 application. Therefore I would like to
> continue relying on kernel configuration for that.

How can that work? How does Linux know when CM3 has up'ed the
interface? How does CM3 know the status of the link? How does CM3 set
its flow control depending on what auto-neg determines, etc?

> 3. In some cases we need to dynamically switch the port "user"
> between CM3 and kernel. So I would like to preserve this
> functionality.

And how do you synchronize between Linux and CM3 so you know how is
using it and who cannot use it?

      Andrew

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [EXT] Re: [V2 net-next] net: mvpp2: Add reserved port private flag configuration
  2021-03-18 20:53     ` Andrew Lunn
@ 2021-03-22 15:59       ` Stefan Chulski
  2021-03-22 16:28         ` Andrew Lunn
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Chulski @ 2021-03-22 15:59 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: kuba, netdev, thomas.petazzoni, davem, Nadav Haklai, Yan Markman,
	linux-kernel, linux, mw, rmk+kernel, atenart, rabeeh


> > 2. CM3 code has very small footprint requirement, we cannot 
> > implement the complete Serdes and PHY infrastructure that kernel 
> > provides as part of CM3 application. Therefore I would like to 
> > continue relying on kernel configuration for that.
> 
> How can that work? How does Linux know when CM3 has up'ed the 
> interface?

CM3 won't use this interface till ethtool priv flag was set, it can be done by communication over CM3 SRAM memory.

> How does CM3 know the status of the link?

CM3 has access to MAC registers and can read port status bit.

> How does CM3 set its
> flow control depending on what auto-neg determines, etc?

Same as PPv2 Packet Processor RX and TX flow don't really care about auto-neg, flow control, etc.
CM3 can ignore it, all this stuff handled in MAC layer. CM3 just polling RX descriptor ring and using TX ring for transmit. 

> 
> > 3. In some cases we need to dynamically switch the port "user"
> > between CM3 and kernel. So I would like to preserve this 
> > functionality.
> 
> And how do you synchronize between Linux and CM3 so you know how is 
> using it and who cannot use it?
> 
>       Andrew

I can add CM3 SRAM update into ethtool priv flag callback, so CM3 won't use port till it was reserved to CM3.

Stefan.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [EXT] Re: [V2 net-next] net: mvpp2: Add reserved port private flag configuration
  2021-03-22 15:59       ` Stefan Chulski
@ 2021-03-22 16:28         ` Andrew Lunn
  2021-03-22 18:24           ` Stefan Chulski
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2021-03-22 16:28 UTC (permalink / raw)
  To: Stefan Chulski
  Cc: kuba, netdev, thomas.petazzoni, davem, Nadav Haklai, Yan Markman,
	linux-kernel, linux, mw, rmk+kernel, atenart, rabeeh

On Mon, Mar 22, 2021 at 03:59:43PM +0000, Stefan Chulski wrote:
> 
> > > 2. CM3 code has very small footprint requirement, we cannot 
> > > implement the complete Serdes and PHY infrastructure that kernel 
> > > provides as part of CM3 application. Therefore I would like to 
> > > continue relying on kernel configuration for that.
> > 
> > How can that work? How does Linux know when CM3 has up'ed the 
> > interface?
> 
> CM3 won't use this interface till ethtool priv flag was set, it can be done by communication over CM3 SRAM memory.
> 
> > How does CM3 know the status of the link?
> 
> CM3 has access to MAC registers and can read port status bit.
> 
> > How does CM3 set its
> > flow control depending on what auto-neg determines, etc?
> 
> Same as PPv2 Packet Processor RX and TX flow don't really care about auto-neg, flow control, etc.
> CM3 can ignore it, all this stuff handled in MAC layer. CM3 just polling RX descriptor ring and using TX ring for transmit. 
> 
> > 
> > > 3. In some cases we need to dynamically switch the port "user"
> > > between CM3 and kernel. So I would like to preserve this 
> > > functionality.
> > 
> > And how do you synchronize between Linux and CM3 so you know how is 
> > using it and who cannot use it?
> > 
> >       Andrew
> 
> I can add CM3 SRAM update into ethtool priv flag callback, so CM3 won't use port till it was reserved to CM3.

I really think you need to step back here and look at the bigger
picture. If linux should not use the interface, it should not
exist. If it does not exist, you cannot use ethtool on it.

What you might want to consider is adding remoteproc support between
Linux on the main processor and whatever you have on the CM3. You can
use RPMsg to send requests back and forth between Linux and the
CM3. It can request that the shared parts of the packet processor are
set up for it. Linux can tell it when the link comes up? It can
request how the PHY auto-neg should be configured.

	Andrew

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [EXT] Re: [V2 net-next] net: mvpp2: Add reserved port private flag configuration
  2021-03-22 16:28         ` Andrew Lunn
@ 2021-03-22 18:24           ` Stefan Chulski
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Chulski @ 2021-03-22 18:24 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: kuba, netdev, thomas.petazzoni, davem, Nadav Haklai, Yan Markman,
	linux-kernel, linux, mw, rmk+kernel, atenart, rabeeh

> > CM3 won't use this interface till ethtool priv flag was set, it can be done by
> communication over CM3 SRAM memory.
> >
> > > How does CM3 know the status of the link?
> >
> > CM3 has access to MAC registers and can read port status bit.
> >
> > > How does CM3 set its
> > > flow control depending on what auto-neg determines, etc?
> >
> > Same as PPv2 Packet Processor RX and TX flow don't really care about auto-
> neg, flow control, etc.
> > CM3 can ignore it, all this stuff handled in MAC layer. CM3 just polling RX
> descriptor ring and using TX ring for transmit.
> >
> > >
> > > > 3. In some cases we need to dynamically switch the port "user"
> > > > between CM3 and kernel. So I would like to preserve this
> > > > functionality.
> > >
> > > And how do you synchronize between Linux and CM3 so you know how
> is
> > > using it and who cannot use it?
> > >
> > >       Andrew
> >
> > I can add CM3 SRAM update into ethtool priv flag callback, so CM3 won't
> use port till it was reserved to CM3.
> 
> I really think you need to step back here and look at the bigger picture. If
> linux should not use the interface, it should not exist. If it does not exist, you
> cannot use ethtool on it.
> 
> What you might want to consider is adding remoteproc support between
> Linux on the main processor and whatever you have on the CM3. You can
> use RPMsg to send requests back and forth between Linux and the CM3. It
> can request that the shared parts of the packet processor are set up for it.
> Linux can tell it when the link comes up? It can request how the PHY auto-
> neg should be configured.
> 
> 	Andrew

I would check this option. 

Thanks,
Stefan.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-03-22 18:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-11 16:43 [V2 net-next] net: mvpp2: Add reserved port private flag configuration stefanc
2021-03-11 16:59 ` Andrew Lunn
2021-03-16 15:28   ` [EXT] " Stefan Chulski
2021-03-16 15:41     ` Russell King - ARM Linux admin
2021-03-16 16:51       ` Stefan Chulski
2021-03-18 20:53     ` Andrew Lunn
2021-03-22 15:59       ` Stefan Chulski
2021-03-22 16:28         ` Andrew Lunn
2021-03-22 18:24           ` Stefan Chulski
2021-03-11 20:33 ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).