Netdev List
 help / color / mirror / Atom feed
* [RFC bpf-next 1/4] bpf: error handling when map_lookup_elem isn't supported
From: Prashant Bhole @ 2018-09-19  7:51 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: Prashant Bhole, Jakub Kicinski, Quentin Monnet, David S . Miller,
	netdev
In-Reply-To: <20180919075143.9308-1-bhole_prashant_q7@lab.ntt.co.jp>

The error value returned by map_lookup_elem doesn't differentiate
whether lookup was failed because of invalid key or lookup is not
supported.

Lets add handling for -EOPNOTSUPP return value of map_lookup_elem()
method of map, with expectation from map's implementation that it
should return -EOPNOTSUPP if lookup is not supported.

The errno for bpf syscall for BPF_MAP_LOOKUP_ELEM command will be set
to EOPNOTSUPP if map lookup is not supported.

Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
---
 kernel/bpf/syscall.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index b3c2d09bcf7a..ecb06352b5a0 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -716,10 +716,15 @@ static int map_lookup_elem(union bpf_attr *attr)
 	} else {
 		rcu_read_lock();
 		ptr = map->ops->map_lookup_elem(map, key);
-		if (ptr)
+		if (IS_ERR(ptr)) {
+			err = PTR_ERR(ptr);
+		} else if (!ptr) {
+			err = -ENOENT;
+		} else {
+			err = 0;
 			memcpy(value, ptr, value_size);
+		}
 		rcu_read_unlock();
-		err = ptr ? 0 : -ENOENT;
 	}
 
 	if (err)
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH iproute2-next] iplink: add ipvtap support
From: Phil Sutter @ 2018-09-19  7:58 UTC (permalink / raw)
  To: Hangbin Liu
  Cc: netdev, Stephen Hemminger, David Ahern, Sainath Grandhi,
	Davide Caratti
In-Reply-To: <1537326209-30837-1-git-send-email-liuhangbin@gmail.com>

On Wed, Sep 19, 2018 at 11:03:29AM +0800, Hangbin Liu wrote:
> IPVLAN and IPVTAP are using the same functions and parameters. So we can
> just add a new link_util with id ipvtap. Others are the same.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

Acked-by: Phil Sutter <phil@nwl.cc>

^ permalink raw reply

* Re: [PATCH 18/21] arm64: dts: ls104xa: set mask to drop TBU ID from StreamID
From: Robin Murphy @ 2018-09-19 13:41 UTC (permalink / raw)
  To: laurentiu.tudor, devicetree, netdev, linux-kernel,
	linux-arm-kernel
  Cc: madalin.bucur, roy.pledge, leoyang.li, shawnguo, davem
In-Reply-To: <20180919123613.15092-19-laurentiu.tudor@nxp.com>

On 19/09/18 13:36, laurentiu.tudor@nxp.com wrote:
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> 
> The StreamID entering the SMMU is actually a concatenation of the
> SMMU TBU ID and the ICID configured in software.
> Since the TBU ID is internal to the SoC and since we want that the
> actual the ICID configured in software to enter the SMMU witout any
> additional set bits, mask out the TBU ID bits and leave only the
> relevant ICID bits to enter SMMU.
> 
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> ---
>   arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 1 +
>   arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 1 +
>   2 files changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> index 8b3eba167508..90296b9fb171 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> @@ -226,6 +226,7 @@
>   			compatible = "arm,mmu-500";
>   			reg = <0 0x9000000 0 0x400000>;
>   			dma-coherent;
> +			stream-match-mask = <0x7f00>;

The TBU ID only forms the top 5 bits, so also ignoring bits 9:8 raises 
an eyebrow - if the LS104x SMMU really is configured for 8-bit SID input 
then it's harmless, but if it's actually a 9 or 10-bit configuration 
then you probably want to avoid masking them (or at least document why) 
- IIRC there *was* stuff wired there on LS2085 at least.

Robin.

>   			#global-interrupts = <2>;
>   			#iommu-cells = <1>;
>   			interrupts = <0 142 4>, /* global secure fault */
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> index 06863d3e4a7d..15094dd8400e 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> @@ -232,6 +232,7 @@
>   			compatible = "arm,mmu-500";
>   			reg = <0 0x9000000 0 0x400000>;
>   			dma-coherent;
> +			stream-match-mask = <0x7f00>;
>   			#global-interrupts = <2>;
>   			#iommu-cells = <1>;
>   			interrupts = <0 142 4>, /* global secure fault */
> 

^ permalink raw reply

* [PATCH v2 net-next] ravb: remove tx buffer addr 4byte alilgnment restriction for R-Car Gen3
From: Simon Horman @ 2018-09-19  8:06 UTC (permalink / raw)
  To: David Miller, Sergei Shtylyov
  Cc: Magnus Damm, netdev, linux-renesas-soc, Kazuya Mizuguchi,
	Simon Horman

From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>

This patch sets from two descriptor to one descriptor because R-Car Gen3
does not have the 4 bytes alignment restriction of the transmission buffer.

Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
v2 [Simon Horman]
* As per review by Sergi Shtylyov
  - Use reverse xmas tree for variable declarations
  - Use > rather than >= for conditions
  - Dropped unnecessary parentheses
  - Don't allocate memory for tx_align when it will not be used
  - But, kept NUM_TX_DESC_GEN[23] as I see some value in
    the self-documentation provided by these #defines

v1 [Kazuya Mizuguchi]
---
 drivers/net/ethernet/renesas/ravb.h      |   6 +-
 drivers/net/ethernet/renesas/ravb_main.c | 143 +++++++++++++++++++------------
 2 files changed, 92 insertions(+), 57 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 1470fc12282b..b2b18036380e 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -954,7 +954,10 @@ enum RAVB_QUEUE {
 #define RX_QUEUE_OFFSET	4
 #define NUM_RX_QUEUE	2
 #define NUM_TX_QUEUE	2
-#define NUM_TX_DESC	2	/* TX descriptors per packet */
+
+/* TX descriptors per packet */
+#define NUM_TX_DESC_GEN2	2
+#define NUM_TX_DESC_GEN3	1
 
 struct ravb_tstamp_skb {
 	struct list_head list;
@@ -1033,6 +1036,7 @@ struct ravb_private {
 	unsigned no_avb_link:1;
 	unsigned avb_link_active_low:1;
 	unsigned wol_enabled:1;
+	int num_tx_desc;	/* TX descriptors per packet */
 };
 
 static inline u32 ravb_read(struct net_device *ndev, enum ravb_reg reg)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index fb2a1125780d..f7c92d48d0dd 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -182,6 +182,7 @@ static int ravb_tx_free(struct net_device *ndev, int q, bool free_txed_only)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
 	struct net_device_stats *stats = &priv->stats[q];
+	int num_tx_desc = priv->num_tx_desc;
 	struct ravb_tx_desc *desc;
 	int free_num = 0;
 	int entry;
@@ -191,7 +192,7 @@ static int ravb_tx_free(struct net_device *ndev, int q, bool free_txed_only)
 		bool txed;
 
 		entry = priv->dirty_tx[q] % (priv->num_tx_ring[q] *
-					     NUM_TX_DESC);
+					     num_tx_desc);
 		desc = &priv->tx_ring[q][entry];
 		txed = desc->die_dt == DT_FEMPTY;
 		if (free_txed_only && !txed)
@@ -200,12 +201,12 @@ static int ravb_tx_free(struct net_device *ndev, int q, bool free_txed_only)
 		dma_rmb();
 		size = le16_to_cpu(desc->ds_tagl) & TX_DS;
 		/* Free the original skb. */
-		if (priv->tx_skb[q][entry / NUM_TX_DESC]) {
+		if (priv->tx_skb[q][entry / num_tx_desc]) {
 			dma_unmap_single(ndev->dev.parent, le32_to_cpu(desc->dptr),
 					 size, DMA_TO_DEVICE);
 			/* Last packet descriptor? */
-			if (entry % NUM_TX_DESC == NUM_TX_DESC - 1) {
-				entry /= NUM_TX_DESC;
+			if (entry % num_tx_desc == num_tx_desc - 1) {
+				entry /= num_tx_desc;
 				dev_kfree_skb_any(priv->tx_skb[q][entry]);
 				priv->tx_skb[q][entry] = NULL;
 				if (txed)
@@ -224,6 +225,7 @@ static int ravb_tx_free(struct net_device *ndev, int q, bool free_txed_only)
 static void ravb_ring_free(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	int num_tx_desc = priv->num_tx_desc;
 	int ring_size;
 	int i;
 
@@ -249,7 +251,7 @@ static void ravb_ring_free(struct net_device *ndev, int q)
 		ravb_tx_free(ndev, q, false);
 
 		ring_size = sizeof(struct ravb_tx_desc) *
-			    (priv->num_tx_ring[q] * NUM_TX_DESC + 1);
+			    (priv->num_tx_ring[q] * num_tx_desc + 1);
 		dma_free_coherent(ndev->dev.parent, ring_size, priv->tx_ring[q],
 				  priv->tx_desc_dma[q]);
 		priv->tx_ring[q] = NULL;
@@ -278,12 +280,13 @@ static void ravb_ring_free(struct net_device *ndev, int q)
 static void ravb_ring_format(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	int num_tx_desc = priv->num_tx_desc;
 	struct ravb_ex_rx_desc *rx_desc;
 	struct ravb_tx_desc *tx_desc;
 	struct ravb_desc *desc;
 	int rx_ring_size = sizeof(*rx_desc) * priv->num_rx_ring[q];
 	int tx_ring_size = sizeof(*tx_desc) * priv->num_tx_ring[q] *
-			   NUM_TX_DESC;
+			   num_tx_desc;
 	dma_addr_t dma_addr;
 	int i;
 
@@ -318,8 +321,10 @@ static void ravb_ring_format(struct net_device *ndev, int q)
 	for (i = 0, tx_desc = priv->tx_ring[q]; i < priv->num_tx_ring[q];
 	     i++, tx_desc++) {
 		tx_desc->die_dt = DT_EEMPTY;
-		tx_desc++;
-		tx_desc->die_dt = DT_EEMPTY;
+		if (num_tx_desc > 1) {
+			tx_desc++;
+			tx_desc->die_dt = DT_EEMPTY;
+		}
 	}
 	tx_desc->dptr = cpu_to_le32((u32)priv->tx_desc_dma[q]);
 	tx_desc->die_dt = DT_LINKFIX; /* type */
@@ -339,6 +344,7 @@ static void ravb_ring_format(struct net_device *ndev, int q)
 static int ravb_ring_init(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	int num_tx_desc = priv->num_tx_desc;
 	struct sk_buff *skb;
 	int ring_size;
 	int i;
@@ -362,11 +368,13 @@ static int ravb_ring_init(struct net_device *ndev, int q)
 		priv->rx_skb[q][i] = skb;
 	}
 
-	/* Allocate rings for the aligned buffers */
-	priv->tx_align[q] = kmalloc(DPTR_ALIGN * priv->num_tx_ring[q] +
-				    DPTR_ALIGN - 1, GFP_KERNEL);
-	if (!priv->tx_align[q])
-		goto error;
+	if (num_tx_desc > 1) {
+		/* Allocate rings for the aligned buffers */
+		priv->tx_align[q] = kmalloc(DPTR_ALIGN * priv->num_tx_ring[q] +
+					    DPTR_ALIGN - 1, GFP_KERNEL);
+		if (!priv->tx_align[q])
+			goto error;
+	}
 
 	/* Allocate all RX descriptors. */
 	ring_size = sizeof(struct ravb_ex_rx_desc) * (priv->num_rx_ring[q] + 1);
@@ -380,7 +388,7 @@ static int ravb_ring_init(struct net_device *ndev, int q)
 
 	/* Allocate all TX descriptors. */
 	ring_size = sizeof(struct ravb_tx_desc) *
-		    (priv->num_tx_ring[q] * NUM_TX_DESC + 1);
+		    (priv->num_tx_ring[q] * num_tx_desc + 1);
 	priv->tx_ring[q] = dma_alloc_coherent(ndev->dev.parent, ring_size,
 					      &priv->tx_desc_dma[q],
 					      GFP_KERNEL);
@@ -1485,6 +1493,7 @@ static void ravb_tx_timeout_work(struct work_struct *work)
 static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	int num_tx_desc = priv->num_tx_desc;
 	u16 q = skb_get_queue_mapping(skb);
 	struct ravb_tstamp_skb *ts_skb;
 	struct ravb_tx_desc *desc;
@@ -1496,7 +1505,7 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 
 	spin_lock_irqsave(&priv->lock, flags);
 	if (priv->cur_tx[q] - priv->dirty_tx[q] > (priv->num_tx_ring[q] - 1) *
-	    NUM_TX_DESC) {
+	    num_tx_desc) {
 		netif_err(priv, tx_queued, ndev,
 			  "still transmitting with the full ring!\n");
 		netif_stop_subqueue(ndev, q);
@@ -1507,41 +1516,55 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	if (skb_put_padto(skb, ETH_ZLEN))
 		goto exit;
 
-	entry = priv->cur_tx[q] % (priv->num_tx_ring[q] * NUM_TX_DESC);
-	priv->tx_skb[q][entry / NUM_TX_DESC] = skb;
-
-	buffer = PTR_ALIGN(priv->tx_align[q], DPTR_ALIGN) +
-		 entry / NUM_TX_DESC * DPTR_ALIGN;
-	len = PTR_ALIGN(skb->data, DPTR_ALIGN) - skb->data;
-	/* Zero length DMA descriptors are problematic as they seem to
-	 * terminate DMA transfers. Avoid them by simply using a length of
-	 * DPTR_ALIGN (4) when skb data is aligned to DPTR_ALIGN.
-	 *
-	 * As skb is guaranteed to have at least ETH_ZLEN (60) bytes of
-	 * data by the call to skb_put_padto() above this is safe with
-	 * respect to both the length of the first DMA descriptor (len)
-	 * overflowing the available data and the length of the second DMA
-	 * descriptor (skb->len - len) being negative.
-	 */
-	if (len == 0)
-		len = DPTR_ALIGN;
+	entry = priv->cur_tx[q] % (priv->num_tx_ring[q] * num_tx_desc);
+	priv->tx_skb[q][entry / num_tx_desc] = skb;
+
+	if (num_tx_desc > 1) {
+		buffer = PTR_ALIGN(priv->tx_align[q], DPTR_ALIGN) +
+			 entry / num_tx_desc * DPTR_ALIGN;
+		len = PTR_ALIGN(skb->data, DPTR_ALIGN) - skb->data;
+
+		/* Zero length DMA descriptors are problematic as they seem
+		 * to terminate DMA transfers. Avoid them by simply using a
+		 * length of DPTR_ALIGN (4) when skb data is aligned to
+		 * DPTR_ALIGN.
+		 *
+		 * As skb is guaranteed to have at least ETH_ZLEN (60)
+		 * bytes of data by the call to skb_put_padto() above this
+		 * is safe with respect to both the length of the first DMA
+		 * descriptor (len) overflowing the available data and the
+		 * length of the second DMA descriptor (skb->len - len)
+		 * being negative.
+		 */
+		if (len == 0)
+			len = DPTR_ALIGN;
 
-	memcpy(buffer, skb->data, len);
-	dma_addr = dma_map_single(ndev->dev.parent, buffer, len, DMA_TO_DEVICE);
-	if (dma_mapping_error(ndev->dev.parent, dma_addr))
-		goto drop;
+		memcpy(buffer, skb->data, len);
+		dma_addr = dma_map_single(ndev->dev.parent, buffer, len,
+					  DMA_TO_DEVICE);
+		if (dma_mapping_error(ndev->dev.parent, dma_addr))
+			goto drop;
 
-	desc = &priv->tx_ring[q][entry];
-	desc->ds_tagl = cpu_to_le16(len);
-	desc->dptr = cpu_to_le32(dma_addr);
+		desc = &priv->tx_ring[q][entry];
+		desc->ds_tagl = cpu_to_le16(len);
+		desc->dptr = cpu_to_le32(dma_addr);
 
-	buffer = skb->data + len;
-	len = skb->len - len;
-	dma_addr = dma_map_single(ndev->dev.parent, buffer, len, DMA_TO_DEVICE);
-	if (dma_mapping_error(ndev->dev.parent, dma_addr))
-		goto unmap;
+		buffer = skb->data + len;
+		len = skb->len - len;
+		dma_addr = dma_map_single(ndev->dev.parent, buffer, len,
+					  DMA_TO_DEVICE);
+		if (dma_mapping_error(ndev->dev.parent, dma_addr))
+			goto unmap;
 
-	desc++;
+		desc++;
+	} else {
+		desc = &priv->tx_ring[q][entry];
+		len = skb->len;
+		dma_addr = dma_map_single(ndev->dev.parent, skb->data, skb->len,
+					  DMA_TO_DEVICE);
+		if (dma_mapping_error(ndev->dev.parent, dma_addr))
+			goto drop;
+	}
 	desc->ds_tagl = cpu_to_le16(len);
 	desc->dptr = cpu_to_le32(dma_addr);
 
@@ -1549,9 +1572,11 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	if (q == RAVB_NC) {
 		ts_skb = kmalloc(sizeof(*ts_skb), GFP_ATOMIC);
 		if (!ts_skb) {
-			desc--;
-			dma_unmap_single(ndev->dev.parent, dma_addr, len,
-					 DMA_TO_DEVICE);
+			if (num_tx_desc > 1) {
+				desc--;
+				dma_unmap_single(ndev->dev.parent, dma_addr,
+						 len, DMA_TO_DEVICE);
+			}
 			goto unmap;
 		}
 		ts_skb->skb = skb;
@@ -1568,15 +1593,18 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	skb_tx_timestamp(skb);
 	/* Descriptor type must be set after all the above writes */
 	dma_wmb();
-	desc->die_dt = DT_FEND;
-	desc--;
-	desc->die_dt = DT_FSTART;
-
+	if (num_tx_desc > 1) {
+		desc->die_dt = DT_FEND;
+		desc--;
+		desc->die_dt = DT_FSTART;
+	} else {
+		desc->die_dt = DT_FSINGLE;
+	}
 	ravb_modify(ndev, TCCR, TCCR_TSRQ0 << q, TCCR_TSRQ0 << q);
 
-	priv->cur_tx[q] += NUM_TX_DESC;
+	priv->cur_tx[q] += num_tx_desc;
 	if (priv->cur_tx[q] - priv->dirty_tx[q] >
-	    (priv->num_tx_ring[q] - 1) * NUM_TX_DESC &&
+	    (priv->num_tx_ring[q] - 1) * num_tx_desc &&
 	    !ravb_tx_free(ndev, q, true))
 		netif_stop_subqueue(ndev, q);
 
@@ -1590,7 +1618,7 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 			 le16_to_cpu(desc->ds_tagl), DMA_TO_DEVICE);
 drop:
 	dev_kfree_skb_any(skb);
-	priv->tx_skb[q][entry / NUM_TX_DESC] = NULL;
+	priv->tx_skb[q][entry / num_tx_desc] = NULL;
 	goto exit;
 }
 
@@ -2076,6 +2104,9 @@ static int ravb_probe(struct platform_device *pdev)
 	ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
 	ndev->min_mtu = ETH_MIN_MTU;
 
+	priv->num_tx_desc = chip_id == RCAR_GEN2 ?
+		NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3;
+
 	/* Set function */
 	ndev->netdev_ops = &ravb_netdev_ops;
 	ndev->ethtool_ops = &ravb_ethtool_ops;
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH 16/21] arm64: dts: ls1046a: add smmu node
From: Laurentiu Tudor @ 2018-09-19 13:51 UTC (permalink / raw)
  To: Robin Murphy, devicetree@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: Roy Pledge, Leo Li, shawnguo@kernel.org, davem@davemloft.net,
	Madalin-cristian Bucur
In-Reply-To: <a4d32163-71b1-35df-3a4c-eb27b605fc46@arm.com>

Hi Robin,

On 19.09.2018 16:30, Robin Murphy wrote:
> On 19/09/18 13:36, laurentiu.tudor@nxp.com wrote:
>> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>
>> This allows for the SMMU device to be probed by the SMMU kernel driver.
>>
>> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>> ---
>>   .../arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 42 +++++++++++++++++++
>>   1 file changed, 42 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi 
>> b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
>> index ef83786b8b90..06863d3e4a7d 100644
>> --- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
>> @@ -228,6 +228,48 @@
>>               bus-width = <4>;
>>           };
>> +        mmu: iommu@9000000 {
>> +            compatible = "arm,mmu-500";
>> +            reg = <0 0x9000000 0 0x400000>;
>> +            dma-coherent;
>> +            #global-interrupts = <2>;
>> +            #iommu-cells = <1>;
>> +            interrupts = <0 142 4>, /* global secure fault */
> 
> Either that's not really the secure global interrupt, or those context 
> interrupts are wrong.

Now that you pointing out, I realize that the comments don't make much 
sense. Actually, 142 is the non-secure interrupt (all ints are ORed on 
this IRQ) while 143 is the secure version. I'll update the comments in 
the next re-spin.

---
Thanks & Best Regards, Laurentiu


> 
>> +                     <0 143 4>, /* combined secure interrupt */
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>,
>> +                     <0 142 4>;
>> +        };
>> +
>>           scfg: scfg@1570000 {
>>               compatible = "fsl,ls1046a-scfg", "syscon";
>>               reg = <0x0 0x1570000 0x0 0x10000>;
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [Patch net-next] ipv4: initialize ra_mutex in inet_init_net()
From: Kirill Tkhai @ 2018-09-19  8:25 UTC (permalink / raw)
  To: Cong Wang; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAM_iQpUqVx91g6aWTERXGnNo7BoD20Ac1wGsqMK-_Ejk-2c+DQ@mail.gmail.com>

On 18.09.2018 23:17, Cong Wang wrote:
> On Mon, Sep 17, 2018 at 12:25 AM Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
>> In inet_init() the order of registration is:
>>
>>         ip_mr_init();
>>         init_inet_pernet_ops();
>>
>> This means, ipmr_net_ops pernet operations are before af_inet_ops
>> in pernet_list. So, there is a theoretical probability, sometimes
>> in the future, we will have a problem during a fail of net initialization.
>>
>> Say,
>>
>> setup_net():
>>         ipmr_net_ops->init() returns 0
>>         xxx->init()          returns error
>> and then we do:
>>         ipmr_net_ops->exit(),
>>
>> which could touch ra_mutex (theoretically).
> 
> How could ra_mutex be touched in this scenario?
> 
> ra_mutex is only used in ip_ra_control() which is called
> only by {get,set}sockopt(). I don't see anything related
> to netns exit() path here.

Currently, it is not touched. But it's an ordinary practice,
someone closes sockets in pernet ->exit methods. For example,
we close percpu icmp sockets in icmp_sk_exit(), which are
also of RAW type, and there is also called ip_ra_control()
for them. Yes, they differ by their protocol; icmp sockets
are of IPPROTO_ICMP protocol, while ip_ra_control() acts
on IPPROTO_RAW sockets, but it's not good anyway. This does
not look reliable for the future. In case of someone changes
something here, we may do not notice this for the long time,
while some users will meet bugs on their places.

Problems on error paths is not easy to detect on testing,
while user may meet them. We had issue of same type with
uninitialized xfrm_policy_lock. It was introduced in 2013,
while the problem was found only in 2017:

	introduced by 283bc9f35bbb
	fixed      by c282222a45cb

(Last week I met it on RH7 kernel, which still has no a fix.
 But this talk is not about distribution kernels, just about
 the way).

I just want to say if someone makes some creativity on top
of this code, it will be to more friendly from us to him/her
to not force this person to think about such not obvious details,
but just to implement nice architecture right now.

Thanks,
Kirill

^ permalink raw reply

* Re: [PATCH 18/21] arm64: dts: ls104xa: set mask to drop TBU ID from StreamID
From: Laurentiu Tudor @ 2018-09-19 14:06 UTC (permalink / raw)
  To: Robin Murphy, devicetree@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: Madalin-cristian Bucur, Roy Pledge, Leo Li, shawnguo@kernel.org,
	davem@davemloft.net
In-Reply-To: <fe2af486-3d7d-a38f-ef62-6754f809298f@arm.com>

Hi Robin,

On 19.09.2018 16:41, Robin Murphy wrote:
> On 19/09/18 13:36, laurentiu.tudor@nxp.com wrote:
>> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>
>> The StreamID entering the SMMU is actually a concatenation of the
>> SMMU TBU ID and the ICID configured in software.
>> Since the TBU ID is internal to the SoC and since we want that the
>> actual the ICID configured in software to enter the SMMU witout any
>> additional set bits, mask out the TBU ID bits and leave only the
>> relevant ICID bits to enter SMMU.
>>
>> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>> ---
>>   arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 1 +
>>   arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 1 +
>>   2 files changed, 2 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi 
>> b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
>> index 8b3eba167508..90296b9fb171 100644
>> --- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
>> @@ -226,6 +226,7 @@
>>               compatible = "arm,mmu-500";
>>               reg = <0 0x9000000 0 0x400000>;
>>               dma-coherent;
>> +            stream-match-mask = <0x7f00>;
> 
> The TBU ID only forms the top 5 bits, so also ignoring bits 9:8 raises 
> an eyebrow - if the LS104x SMMU really is configured for 8-bit SID input 
> then it's harmless, 

On these lower-end platforms the SID input is configured and documented 
as 8-bit.

> but if it's actually a 9 or 10-bit configuration 
> then you probably want to avoid masking them (or at least document why) 
> - IIRC there *was* stuff wired there on LS2085 at least.

Yes, on LS2s there are 2 extra-bits in there carrying some signaling. 
However, on LS1s they are not present.

---
Thanks & Best Regards, Laurentiu

> 
>>               #global-interrupts = <2>;
>>               #iommu-cells = <1>;
>>               interrupts = <0 142 4>, /* global secure fault */
>> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi 
>> b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
>> index 06863d3e4a7d..15094dd8400e 100644
>> --- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
>> @@ -232,6 +232,7 @@
>>               compatible = "arm,mmu-500";
>>               reg = <0 0x9000000 0 0x400000>;
>>               dma-coherent;
>> +            stream-match-mask = <0x7f00>;
>>               #global-interrupts = <2>;
>>               #iommu-cells = <1>;
>>               interrupts = <0 142 4>, /* global secure fault */
>>

^ permalink raw reply

* Re: [PATCH 00/21] SMMU enablement for NXP LS1043A and LS1046A
From: Laurentiu Tudor @ 2018-09-19 14:18 UTC (permalink / raw)
  To: Robin Murphy, devicetree@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: Roy Pledge, Leo Li, shawnguo@kernel.org, davem@davemloft.net,
	Madalin-cristian Bucur
In-Reply-To: <7d7646dc-9d0b-013d-75d7-a6cb4453f41f@arm.com>

Hi Robin,

On 19.09.2018 16:25, Robin Murphy wrote:
> Hi Laurentiu,
> 
> On 19/09/18 13:35, laurentiu.tudor@nxp.com wrote:
>> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>
>> This patch series adds SMMU support for NXP LS1043A and LS1046A chips
>> and consists mostly in important driver fixes and the required device
>> tree updates. It touches several subsystems and consists of three main
>> parts:
>>   - changes in soc/drivers/fsl/qbman drivers adding iommu mapping of
>>     reserved memory areas, fixes and defered probe support
>>   - changes in drivers/net/ethernet/freescale/dpaa_eth drivers
>>     consisting in misc dma mapping related fixes and probe ordering
>>   - addition of the actual arm smmu device tree node together with
>>     various adjustments to the device trees
>>
>> Performance impact
>>
>>      Running iperf benchmarks in a back-to-back setup (both sides
>>      having smmu enabled) on a 10GBps port show an important
>>      networking performance degradation of around %40 (9.48Gbps
>>      linerate vs 5.45Gbps). If you need performance but without
>>      SMMU support you can use "iommu.passthrough=1" to disable
>>      SMMU.
>>
>> USB issue and workaround
>>
>>      There's a problem with the usb controllers in these chips
>>      generating smaller, 40-bit wide dma addresses instead of the 48-bit
>>      supported at the smmu input. So you end up in a situation where the
>>      smmu is mapped with 48-bit address translations, but the device
>>      generates transactions with clipped 40-bit addresses, thus smmu
>>      context faults are triggered. I encountered a similar situation for
>>      mmc that I  managed to fix in software [1] however for USB I did not
>>      find a proper place in the code to add a similar fix. The only
>>      workaround I found was to add this kernel parameter which limits the
>>      usb dma to 32-bit size: "xhci-hcd.quirks=0x800000".
>>      This workaround if far from ideal, so any suggestions for a code
>>      based workaround in this area would be greatly appreciated.
> 
> If you have a nominally-64-bit device with a 
> narrower-than-the-main-interconnect link in front of it, that should 
> already be fixed in 4.19-rc by bus_dma_mask picking up DT dma-ranges, 
> provided the interconnect hierarchy can be described appropriately (or 
> at least massaged sufficiently to satisfy the binding), e.g.:
> 
> / {
>      ...
> 
>      soc {
>          ranges;
>          dma-ranges = <0 0 10000 0>;
> 
>          dev_48bit { ... };
> 
>          periph_bus {
>              ranges;
>              dma-ranges = <0 0 100 0>;
> 
>              dev_40bit { ... };
>          };
>      };
> };
> 
> and if that fails to work as expected (except for PCI hosts where 
> handling dma-ranges properly still needs sorting out), please do let us 
> know ;)
> 

Just to confirm, Is this [1] the change I was supposed to test?
Because if so, I'm still seeing context faults [2] with what looks like 
clipped to 40-bits addresses. :-(
IIRC, the usb subsystem explicitly set 64-bit dma masks which in turn 
will be limited to the SMMU input size of 48-bit. Won't that overwrite 
the default dma mask derived from dma-ranges?

---
Best Regards, Laurentiu

[1] -----------------------------------------------------------------

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi 
b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index 3bdea0470f69..a214c3df37fd 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -612,6 +612,7 @@
                         compatible = "snps,dwc3";
                         reg = <0x0 0x2f00000 0x0 0x10000>;
                         interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
+                       dma-ranges = <0x0 0x0 0x0 0x0 0x100 0x00000000>;
                         dr_mode = "host";
                         snps,quirk-frame-length-adjustment = <0x20>;
                         snps,dis_rxdet_inp3_quirk;
@@ -621,6 +622,7 @@
                         compatible = "snps,dwc3";
                         reg = <0x0 0x3000000 0x0 0x10000>;
                         interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
+                       dma-ranges = <0x0 0x0 0x0 0x0 0x100 0x00000000>;
                         dr_mode = "host";
                         snps,quirk-frame-length-adjustment = <0x20>;
                         snps,dis_rxdet_inp3_quirk;
@@ -630,6 +632,7 @@
                         compatible = "snps,dwc3";
                         reg = <0x0 0x3100000 0x0 0x10000>;
                         interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>;
+                       dma-ranges = <0x0 0x0 0x0 0x0 0x100 0x00000000>;
                         dr_mode = "host";
                         snps,quirk-frame-length-adjustment = <0x20>;
                         snps,dis_rxdet_inp3_quirk;

[2] -----------------------------------------------------------------
[    2.090577] xhci-hcd xhci-hcd.0.auto: xHCI Host Controller
[    2.096064] xhci-hcd xhci-hcd.0.auto: new USB bus registered, 
assigned bus number 2
[    2.103720] xhci-hcd xhci-hcd.0.auto: Host supports USB 3.0  SuperSpeed
[    2.110346] arm-smmu 9000000.iommu: Unhandled context fault: 
fsr=0x402, iova=0xffffffb000, fsynr=0x1b0000, cb=3
[    2.120449] usb usb2: We don't know the algorithms for LPM for this 
host, disabling LPM.
[    2.128717] hub 2-0:1.0: USB hub found
[    2.132473] hub 2-0:1.0: 1 port detected
[    2.136527] xhci-hcd xhci-hcd.1.auto: xHCI Host Controller
[    2.142014] xhci-hcd xhci-hcd.1.auto: new USB bus registered, 
assigned bus number 3
[    2.149747] xhci-hcd xhci-hcd.1.auto: hcc params 0x0220f66d hci 
version 0x100 quirks 0x0000000002010010
[    2.159149] xhci-hcd xhci-hcd.1.auto: irq 50, io mem 0x03000000
[    2.165284] hub 3-0:1.0: USB hub found
[    2.169039] hub 3-0:1.0: 1 port detected
[    2.173051] xhci-hcd xhci-hcd.1.auto: xHCI Host Controller
[    2.178536] xhci-hcd xhci-hcd.1.auto: new USB bus registered, 
assigned bus number 4
[    2.186193] xhci-hcd xhci-hcd.1.auto: Host supports USB 3.0  SuperSpeed
[    2.192809] arm-smmu 9000000.iommu: Unhandled context fault: 
fsr=0x402, iova=0xffffffb000, fsynr=0x1f0000, cb=4
[    2.192822] usb usb4: We don't know the algorithms for LPM for this 
host, disabling LPM.
[    2.211141] hub 4-0:1.0: USB hub found
[    2.214896] hub 4-0:1.0: 1 port detected
[    2.218935] xhci-hcd xhci-hcd.2.auto: xHCI Host Controller
[    2.224425] xhci-hcd xhci-hcd.2.auto: new USB bus registered, 
assigned bus number 5
[    2.232153] xhci-hcd xhci-hcd.2.auto: hcc params 0x0220f66d hci 
version 0x100 quirks 0x0000000002010010
[    2.241562] xhci-hcd xhci-hcd.2.auto: irq 51, io mem 0x03100000
[    2.247694] hub 5-0:1.0: USB hub found
[    2.251449] hub 5-0:1.0: 1 port detected
[    2.255458] xhci-hcd xhci-hcd.2.auto: xHCI Host Controller
[    2.260945] xhci-hcd xhci-hcd.2.auto: new USB bus registered, 
assigned bus number 6
[    2.268601] xhci-hcd xhci-hcd.2.auto: Host supports USB 3.0  SuperSpeed
[    2.275218] arm-smmu 9000000.iommu: Unhandled context fault: 
fsr=0x402, iova=0xffffffb000, fsynr=0x110000, cb=5
[    2.275230] usb usb6: We don't know the algorithms for LPM for this 
host, disabling LPM.


>> The patch set is based on net-next so, if generally agreed, I'd suggest
>> to get the patches through the netdev tree after getting all the Acks.
>>
>> [1] 
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fpatch%2F10506627%2F&amp;data=02%7C01%7Claurentiu.tudor%40nxp.com%7C63c4e1dfc126488eb4ba08d61e336607%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636729603447603039&amp;sdata=XhjOX9aLgoe%2BSTBgZztv6zCz0vMebSXW%2Fnb2QcD5shY%3D&amp;reserved=0 
>>
>>
>> Laurentiu Tudor (21):
>>    soc/fsl/qman: fixup liodns only on ppc targets
>>    soc/fsl/bman: map FBPR area in the iommu
>>    soc/fsl/qman: map FQD and PFDR areas in the iommu
>>    soc/fsl/qman-portal: map CENA area in the iommu
>>    soc/fsl/qbman: add APIs to retrieve the probing status
>>    soc/fsl/qman_portals: defer probe after qman's probe
>>    soc/fsl/bman_portals: defer probe after bman's probe
>>    soc/fsl/qbman_portals: add APIs to retrieve the probing status
>>    fsl/fman: backup and restore ICID registers
>>    fsl/fman: add API to get the device behind a fman port
>>    dpaa_eth: defer probing after qbman
>>    dpaa_eth: base dma mappings on the fman rx port
>>    dpaa_eth: fix iova handling for contiguous frames
>>    dpaa_eth: fix iova handling for sg frames
>>    dpaa_eth: fix SG frame cleanup
>>    arm64: dts: ls1046a: add smmu node
>>    arm64: dts: ls1043a: add smmu node
>>    arm64: dts: ls104xa: set mask to drop TBU ID from StreamID
>>    arm64: dts: ls104x: add missing dma ranges property
>>    arm64: dts: ls104x: add iommu-map to pci controllers
>>    arm64: dts: ls104x: make dma-coherent global to the SoC
>>
>>   .../arm64/boot/dts/freescale/fsl-ls1043a.dtsi |  52 ++++++-
>>   .../arm64/boot/dts/freescale/fsl-ls1046a.dtsi |  48 +++++++
>>   .../net/ethernet/freescale/dpaa/dpaa_eth.c    | 136 ++++++++++++------
>>   drivers/net/ethernet/freescale/fman/fman.c    |  35 ++++-
>>   drivers/net/ethernet/freescale/fman/fman.h    |   4 +
>>   .../net/ethernet/freescale/fman/fman_port.c   |  14 ++
>>   .../net/ethernet/freescale/fman/fman_port.h   |   2 +
>>   drivers/soc/fsl/qbman/bman_ccsr.c             |  23 +++
>>   drivers/soc/fsl/qbman/bman_portal.c           |  20 ++-
>>   drivers/soc/fsl/qbman/qman_ccsr.c             |  30 ++++
>>   drivers/soc/fsl/qbman/qman_portal.c           |  35 +++++
>>   include/soc/fsl/bman.h                        |  16 +++
>>   include/soc/fsl/qman.h                        |  17 +++
>>   13 files changed, 379 insertions(+), 53 deletions(-)
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH] net: ibm: remove a redundant local variable 'k'
From: zhong jiang @ 2018-09-19 14:23 UTC (permalink / raw)
  To: davem; +Cc: dougmill, netdev, linux-kernel

The local variable 'k' is never used after being assigned.
hence it should be redundant adn can be removed.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/net/ethernet/ibm/ehea/ehea_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index ba580bf..7f6a401 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -778,12 +778,11 @@ static void check_sqs(struct ehea_port *port)
 {
 	struct ehea_swqe *swqe;
 	int swqe_index;
-	int i, k;
+	int i;
 
 	for (i = 0; i < port->num_def_qps; i++) {
 		struct ehea_port_res *pr = &port->port_res[i];
 		int ret;
-		k = 0;
 		swqe = ehea_get_swqe(pr->qp, &swqe_index);
 		memset(swqe, 0, SWQE_HEADER_SIZE);
 		atomic_dec(&pr->swqe_avail);
-- 
1.7.12.4

^ permalink raw reply related

* Re: [PATCH net-next 1/2] net: linkwatch: add check for netdevice being present to linkwatch_do_dev
From: Geert Uytterhoeven @ 2018-09-19  8:48 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Florian Fainelli, Andrew Lunn, David S. Miller, netdev,
	Geert Uytterhoeven
In-Reply-To: <11beeaa9-57d5-e641-9486-f2ba202d0998@gmail.com>

On Tue, Sep 18, 2018 at 9:56 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> When bringing down the netdevice (incl. detaching it) and calling
> netif_carrier_off directly or indirectly the latter triggers an
> asynchronous linkwatch event.
> This linkwatch event eventually may fail to access chip registers in
> the ndo_get_stats/ndo_get_stats64 callback because the device isn't
> accessible any longer, see call trace in [0].
>
> To prevent this scenario don't check for IFF_UP only, but also make
> sure that the netdevice is present.
>
> [0] https://lists.openwall.net/netdev/2018/03/15/62
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Survived 100 suspend/resume cycles on sh73a0/kzm9g and r8a73a4/ape6evm.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: phy: call state machine synchronously in phy_stop
From: Geert Uytterhoeven @ 2018-09-19  8:49 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Florian Fainelli, Andrew Lunn, David S. Miller, netdev,
	Geert Uytterhoeven
In-Reply-To: <a58e1448-0a10-b7a1-ccca-ca99f146c8ea@gmail.com>

On Tue, Sep 18, 2018 at 9:56 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> phy_stop() may be called e.g. when suspending, therefore all needed
> actions should be performed synchronously. Therefore add a synchronous
> call to the state machine.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Survived 100 suspend/resume cycles on sh73a0/kzm9g and r8a73a4/ape6evm.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH] net: ibm: remove redundant local variables 'act_nr_of_entries' and 'act_pages'
From: zhong jiang @ 2018-09-19 14:30 UTC (permalink / raw)
  To: davem; +Cc: dougmill, netdev, linux-kernel

That local variable are never used after being assigned.
hence it should be redundant and can be removed.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/net/ethernet/ibm/ehea/ehea_qmr.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ehea/ehea_qmr.c b/drivers/net/ethernet/ibm/ehea/ehea_qmr.c
index a0820f7..5e4e371 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_qmr.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_qmr.c
@@ -125,7 +125,7 @@ struct ehea_cq *ehea_create_cq(struct ehea_adapter *adapter,
 	struct ehea_cq *cq;
 	struct h_epa epa;
 	u64 *cq_handle_ref, hret, rpage;
-	u32 act_nr_of_entries, act_pages, counter;
+	u32 counter;
 	int ret;
 	void *vpage;
 
@@ -140,8 +140,6 @@ struct ehea_cq *ehea_create_cq(struct ehea_adapter *adapter,
 	cq->adapter = adapter;
 
 	cq_handle_ref = &cq->fw_handle;
-	act_nr_of_entries = 0;
-	act_pages = 0;
 
 	hret = ehea_h_alloc_resource_cq(adapter->handle, &cq->attr,
 					&cq->fw_handle, &cq->epas);
-- 
1.7.12.4

^ permalink raw reply related

* Re: [PATCH 00/21] SMMU enablement for NXP LS1043A and LS1046A
From: Robin Murphy @ 2018-09-19 14:37 UTC (permalink / raw)
  To: Laurentiu Tudor, devicetree@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: Madalin-cristian Bucur, Roy Pledge, Leo Li, shawnguo@kernel.org,
	davem@davemloft.net
In-Reply-To: <fd8ab351-46aa-f73e-593b-502a57de1975@nxp.com>

On 19/09/18 15:18, Laurentiu Tudor wrote:
> Hi Robin,
> 
> On 19.09.2018 16:25, Robin Murphy wrote:
>> Hi Laurentiu,
>>
>> On 19/09/18 13:35, laurentiu.tudor@nxp.com wrote:
>>> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>>
>>> This patch series adds SMMU support for NXP LS1043A and LS1046A chips
>>> and consists mostly in important driver fixes and the required device
>>> tree updates. It touches several subsystems and consists of three main
>>> parts:
>>>    - changes in soc/drivers/fsl/qbman drivers adding iommu mapping of
>>>      reserved memory areas, fixes and defered probe support
>>>    - changes in drivers/net/ethernet/freescale/dpaa_eth drivers
>>>      consisting in misc dma mapping related fixes and probe ordering
>>>    - addition of the actual arm smmu device tree node together with
>>>      various adjustments to the device trees
>>>
>>> Performance impact
>>>
>>>       Running iperf benchmarks in a back-to-back setup (both sides
>>>       having smmu enabled) on a 10GBps port show an important
>>>       networking performance degradation of around %40 (9.48Gbps
>>>       linerate vs 5.45Gbps). If you need performance but without
>>>       SMMU support you can use "iommu.passthrough=1" to disable
>>>       SMMU.
>>>
>>> USB issue and workaround
>>>
>>>       There's a problem with the usb controllers in these chips
>>>       generating smaller, 40-bit wide dma addresses instead of the 48-bit
>>>       supported at the smmu input. So you end up in a situation where the
>>>       smmu is mapped with 48-bit address translations, but the device
>>>       generates transactions with clipped 40-bit addresses, thus smmu
>>>       context faults are triggered. I encountered a similar situation for
>>>       mmc that I  managed to fix in software [1] however for USB I did not
>>>       find a proper place in the code to add a similar fix. The only
>>>       workaround I found was to add this kernel parameter which limits the
>>>       usb dma to 32-bit size: "xhci-hcd.quirks=0x800000".
>>>       This workaround if far from ideal, so any suggestions for a code
>>>       based workaround in this area would be greatly appreciated.
>>
>> If you have a nominally-64-bit device with a
>> narrower-than-the-main-interconnect link in front of it, that should
>> already be fixed in 4.19-rc by bus_dma_mask picking up DT dma-ranges,
>> provided the interconnect hierarchy can be described appropriately (or
>> at least massaged sufficiently to satisfy the binding), e.g.:
>>
>> / {
>>       ...
>>
>>       soc {
>>           ranges;
>>           dma-ranges = <0 0 10000 0>;
>>
>>           dev_48bit { ... };
>>
>>           periph_bus {
>>               ranges;
>>               dma-ranges = <0 0 100 0>;
>>
>>               dev_40bit { ... };
>>           };
>>       };
>> };
>>
>> and if that fails to work as expected (except for PCI hosts where
>> handling dma-ranges properly still needs sorting out), please do let us
>> know ;)
>>
> 
> Just to confirm, Is this [1] the change I was supposed to test?

Not quite - dma-ranges is only valid for nodes representing a bus, so 
putting it directly in the USB device nodes doesn't work (FWIW that's 
why PCI is broken, because the parser doesn't expect the 
bus-as-leaf-node case). That's teh point of that intermediate simple-bus 
node represented by "periph_bus" in my example (sorry, I should have put 
compatibles in to make it clearer) - often that's actually true to life 
(i.e. "soc" is something like a CCI and "periph_bus" is something like 
an AXI NIC gluing a bunch of lower-bandwidth DMA masters to one of the 
CCI ports) but at worst it's just a necessary evil to make the binding 
happy (if it literally only represents the point-to-point link between 
the device master port and interconnect slave port).

> Because if so, I'm still seeing context faults [2] with what looks like
> clipped to 40-bits addresses. :-(
> IIRC, the usb subsystem explicitly set 64-bit dma masks which in turn
> will be limited to the SMMU input size of 48-bit. Won't that overwrite
> the default dma mask derived from dma-ranges?

Indeed it will, but those default masks were effectively only ever a 
best-effort thing anyway - it's an ease-of-implementation detail that 
bus_dma_mask is not currently reflected in the device masks, although we 
may eventually change that; the crucial part is that the DMA ops 
implementations know about it and should now enforce it properly 
regardless of whether drivers set something wider.

Robin.

> 
> ---
> Best Regards, Laurentiu
> 
> [1] -----------------------------------------------------------------
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> index 3bdea0470f69..a214c3df37fd 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> @@ -612,6 +612,7 @@
>                           compatible = "snps,dwc3";
>                           reg = <0x0 0x2f00000 0x0 0x10000>;
>                           interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
> +                       dma-ranges = <0x0 0x0 0x0 0x0 0x100 0x00000000>;
>                           dr_mode = "host";
>                           snps,quirk-frame-length-adjustment = <0x20>;
>                           snps,dis_rxdet_inp3_quirk;
> @@ -621,6 +622,7 @@
>                           compatible = "snps,dwc3";
>                           reg = <0x0 0x3000000 0x0 0x10000>;
>                           interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
> +                       dma-ranges = <0x0 0x0 0x0 0x0 0x100 0x00000000>;
>                           dr_mode = "host";
>                           snps,quirk-frame-length-adjustment = <0x20>;
>                           snps,dis_rxdet_inp3_quirk;
> @@ -630,6 +632,7 @@
>                           compatible = "snps,dwc3";
>                           reg = <0x0 0x3100000 0x0 0x10000>;
>                           interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>;
> +                       dma-ranges = <0x0 0x0 0x0 0x0 0x100 0x00000000>;
>                           dr_mode = "host";
>                           snps,quirk-frame-length-adjustment = <0x20>;
>                           snps,dis_rxdet_inp3_quirk;
> 
> [2] -----------------------------------------------------------------
> [    2.090577] xhci-hcd xhci-hcd.0.auto: xHCI Host Controller
> [    2.096064] xhci-hcd xhci-hcd.0.auto: new USB bus registered,
> assigned bus number 2
> [    2.103720] xhci-hcd xhci-hcd.0.auto: Host supports USB 3.0  SuperSpeed
> [    2.110346] arm-smmu 9000000.iommu: Unhandled context fault:
> fsr=0x402, iova=0xffffffb000, fsynr=0x1b0000, cb=3
> [    2.120449] usb usb2: We don't know the algorithms for LPM for this
> host, disabling LPM.
> [    2.128717] hub 2-0:1.0: USB hub found
> [    2.132473] hub 2-0:1.0: 1 port detected
> [    2.136527] xhci-hcd xhci-hcd.1.auto: xHCI Host Controller
> [    2.142014] xhci-hcd xhci-hcd.1.auto: new USB bus registered,
> assigned bus number 3
> [    2.149747] xhci-hcd xhci-hcd.1.auto: hcc params 0x0220f66d hci
> version 0x100 quirks 0x0000000002010010
> [    2.159149] xhci-hcd xhci-hcd.1.auto: irq 50, io mem 0x03000000
> [    2.165284] hub 3-0:1.0: USB hub found
> [    2.169039] hub 3-0:1.0: 1 port detected
> [    2.173051] xhci-hcd xhci-hcd.1.auto: xHCI Host Controller
> [    2.178536] xhci-hcd xhci-hcd.1.auto: new USB bus registered,
> assigned bus number 4
> [    2.186193] xhci-hcd xhci-hcd.1.auto: Host supports USB 3.0  SuperSpeed
> [    2.192809] arm-smmu 9000000.iommu: Unhandled context fault:
> fsr=0x402, iova=0xffffffb000, fsynr=0x1f0000, cb=4
> [    2.192822] usb usb4: We don't know the algorithms for LPM for this
> host, disabling LPM.
> [    2.211141] hub 4-0:1.0: USB hub found
> [    2.214896] hub 4-0:1.0: 1 port detected
> [    2.218935] xhci-hcd xhci-hcd.2.auto: xHCI Host Controller
> [    2.224425] xhci-hcd xhci-hcd.2.auto: new USB bus registered,
> assigned bus number 5
> [    2.232153] xhci-hcd xhci-hcd.2.auto: hcc params 0x0220f66d hci
> version 0x100 quirks 0x0000000002010010
> [    2.241562] xhci-hcd xhci-hcd.2.auto: irq 51, io mem 0x03100000
> [    2.247694] hub 5-0:1.0: USB hub found
> [    2.251449] hub 5-0:1.0: 1 port detected
> [    2.255458] xhci-hcd xhci-hcd.2.auto: xHCI Host Controller
> [    2.260945] xhci-hcd xhci-hcd.2.auto: new USB bus registered,
> assigned bus number 6
> [    2.268601] xhci-hcd xhci-hcd.2.auto: Host supports USB 3.0  SuperSpeed
> [    2.275218] arm-smmu 9000000.iommu: Unhandled context fault:
> fsr=0x402, iova=0xffffffb000, fsynr=0x110000, cb=5
> [    2.275230] usb usb6: We don't know the algorithms for LPM for this
> host, disabling LPM.
> 
> 
>>> The patch set is based on net-next so, if generally agreed, I'd suggest
>>> to get the patches through the netdev tree after getting all the Acks.
>>>
>>> [1]
>>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fpatch%2F10506627%2F&amp;data=02%7C01%7Claurentiu.tudor%40nxp.com%7C63c4e1dfc126488eb4ba08d61e336607%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636729603447603039&amp;sdata=XhjOX9aLgoe%2BSTBgZztv6zCz0vMebSXW%2Fnb2QcD5shY%3D&amp;reserved=0
>>>
>>>
>>> Laurentiu Tudor (21):
>>>     soc/fsl/qman: fixup liodns only on ppc targets
>>>     soc/fsl/bman: map FBPR area in the iommu
>>>     soc/fsl/qman: map FQD and PFDR areas in the iommu
>>>     soc/fsl/qman-portal: map CENA area in the iommu
>>>     soc/fsl/qbman: add APIs to retrieve the probing status
>>>     soc/fsl/qman_portals: defer probe after qman's probe
>>>     soc/fsl/bman_portals: defer probe after bman's probe
>>>     soc/fsl/qbman_portals: add APIs to retrieve the probing status
>>>     fsl/fman: backup and restore ICID registers
>>>     fsl/fman: add API to get the device behind a fman port
>>>     dpaa_eth: defer probing after qbman
>>>     dpaa_eth: base dma mappings on the fman rx port
>>>     dpaa_eth: fix iova handling for contiguous frames
>>>     dpaa_eth: fix iova handling for sg frames
>>>     dpaa_eth: fix SG frame cleanup
>>>     arm64: dts: ls1046a: add smmu node
>>>     arm64: dts: ls1043a: add smmu node
>>>     arm64: dts: ls104xa: set mask to drop TBU ID from StreamID
>>>     arm64: dts: ls104x: add missing dma ranges property
>>>     arm64: dts: ls104x: add iommu-map to pci controllers
>>>     arm64: dts: ls104x: make dma-coherent global to the SoC
>>>
>>>    .../arm64/boot/dts/freescale/fsl-ls1043a.dtsi |  52 ++++++-
>>>    .../arm64/boot/dts/freescale/fsl-ls1046a.dtsi |  48 +++++++
>>>    .../net/ethernet/freescale/dpaa/dpaa_eth.c    | 136 ++++++++++++------
>>>    drivers/net/ethernet/freescale/fman/fman.c    |  35 ++++-
>>>    drivers/net/ethernet/freescale/fman/fman.h    |   4 +
>>>    .../net/ethernet/freescale/fman/fman_port.c   |  14 ++
>>>    .../net/ethernet/freescale/fman/fman_port.h   |   2 +
>>>    drivers/soc/fsl/qbman/bman_ccsr.c             |  23 +++
>>>    drivers/soc/fsl/qbman/bman_portal.c           |  20 ++-
>>>    drivers/soc/fsl/qbman/qman_ccsr.c             |  30 ++++
>>>    drivers/soc/fsl/qbman/qman_portal.c           |  35 +++++
>>>    include/soc/fsl/bman.h                        |  16 +++
>>>    include/soc/fsl/qman.h                        |  17 +++
>>>    13 files changed, 379 insertions(+), 53 deletions(-)
>> >

^ permalink raw reply

* [PATCH] ieee802154: remove a redundant local variable 'i'
From: zhong jiang @ 2018-09-19 14:41 UTC (permalink / raw)
  To: davem; +Cc: stefan, alex.aring, netdev, linux-kernel

The local variable 'i' is never used after being assigned.
hence it should be redundant adn can be removed.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 net/ieee802154/nl802154.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 99f6c25..5b90151 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -445,7 +445,6 @@ static int nl802154_send_wpan_phy(struct cfg802154_registered_device *rdev,
 {
 	struct nlattr *nl_cmds;
 	void *hdr;
-	int i;
 
 	hdr = nl802154hdr_put(msg, portid, seq, flags, cmd);
 	if (!hdr)
@@ -508,7 +507,6 @@ static int nl802154_send_wpan_phy(struct cfg802154_registered_device *rdev,
 	if (!nl_cmds)
 		goto nla_put_failure;
 
-	i = 0;
 #define CMD(op, n)							\
 	do {								\
 		if (rdev->ops->op) {					\
-- 
1.7.12.4

^ permalink raw reply related

* Re: [RFC 4/5] netlink: prepare validate extack setting for recursion
From: Jiri Benc @ 2018-09-19  9:10 UTC (permalink / raw)
  To: Johannes Berg; +Cc: netdev, Johannes Berg
In-Reply-To: <20180918131212.20266-4-johannes@sipsolutions.net>

On Tue, 18 Sep 2018 15:12:11 +0200, Johannes Berg wrote:
>  static int validate_nla(const struct nlattr *nla, int maxtype,
>  			const struct nla_policy *policy,
> -			const char **error_msg)
> +			struct netlink_ext_ack *extack, bool *extack_set)

Can't the extack_set be included in the struct netlink_ext_ack? One less
parameter to pass around and the NL_SET_* macros could check this on
their own.

This way, it can be also easily turned to a more complex mechanism,
such as the one Marcelo proposed.

Thanks,

 Jiri

^ permalink raw reply

* Bridge connectivity interruptions while devices join or leave the bridge
From: Johannes Wienke @ 2018-09-19  9:10 UTC (permalink / raw)
  To: netdev

I am sorry for probably misusing this list, but I couldn't find any
other mailing list suitable for asking in-detail Linux networking
questions. As I am not subscribed, please CC me in a potential reply.

I am currently tracking down a connectivity issues of docker containers
on a custom bridge network, which I could reduce to the Linux network
stack without docker being involved.

The situation that I am observing is the following: I have a bridge
device, which is connected to the outer world using forwarding and
masquerading (so the bridge does not contain the outgoing network
interface of the host). This bridge is used to perform network
operations by a long-running process, which is restricted to this bridge
using network namespaces and veth devices (exactly what docker does
internally). What I see is that every time a (virtual) network device is
added to or removed from the bridge, the communication of the
long-running process is interrupted.

I have created two scripts that can be used to replicate the situation.
They are available at:
https://gist.github.com/languitar/9ac8dc5c8db7cf4a89e1546f6e32ca7b

setup.bash sets up the bridge, veth devices, network namespace and the
iptables rules to replicate the network setup and simulates the
long-running process by periodically performing (volatile) UDP DNS
requests in a while loop.

When launching this script, all DNS requests should succeed and you
should see success messages at a regular pace.

To simulate devices joining and leaving the bridge, you can start
interruptor.bash.

As soon as this script is running, you can observe that DNS requests
will be delayed frequently and some of them even fail. In a parallel
pcap you would see that sometimes the UDP packages from the DNS lookup
are not routed to the outside world, but instead end up at the bridge
device without ever leaving the host system.

Can someone explain what is happening here and why adding and removing
devices to a bridge results in the connectivity issues? How to avoid
this behavior? I'd be glad for any hint on that.

Kind regards
Johannes
-- 
Johannes Wienke, Researcher at CoR-Lab / CITEC, Bielefeld University
Address: Inspiration 1, D-33619 Bielefeld, Germany (Room 1.307)
Phone: +49 521 106-67277

^ permalink raw reply

* Re: [RFC 4/5] netlink: prepare validate extack setting for recursion
From: Johannes Berg @ 2018-09-19  9:15 UTC (permalink / raw)
  To: Jiri Benc; +Cc: netdev
In-Reply-To: <20180919111048.092376b2@redhat.com>

On Wed, 2018-09-19 at 11:10 +0200, Jiri Benc wrote:
> On Tue, 18 Sep 2018 15:12:11 +0200, Johannes Berg wrote:
> >  static int validate_nla(const struct nlattr *nla, int maxtype,
> >  			const struct nla_policy *policy,
> > -			const char **error_msg)
> > +			struct netlink_ext_ack *extack, bool *extack_set)
> 
> Can't the extack_set be included in the struct netlink_ext_ack? One less
> parameter to pass around and the NL_SET_* macros could check this on
> their own.

No, I don't think it can or should.

For one, having the NL_SET_* macros check it on their own will already
not work - as we discussed over in the NLA_REJECT thread, we do need to
be able to override the data, e.g. if somebody does

NL_SET_ERR_MSG(extack, "warning: deprecated command");
err = nla_parse(..., extack);

and nla_parse() sets a new message. Thus, hiding all the logic in there
already will not work, and is also IMHO rather unexpected. Normally,
*later* messages should win, not *earlier* ones - and that doesn't
require any tracking, just setting them unconditionally.

It's just a side effect of how we do the recursive validation here that
we want *earlier* messages to win (but only within this code piece - if
a previous message was set, we want it to be overwritten by nla_parse!).

It might be possible to do this differently, in theory, but all the ways
I've tried to come up with so far made the code vastly more complex.

johannes

^ permalink raw reply

* Re: [RFC] net;sched: Try to find idle cpu for RPS to handle packets
From: Eric Dumazet @ 2018-09-19 14:55 UTC (permalink / raw)
  To: Kirill Tkhai
  Cc: Peter Zijlstra, David Miller, Daniel Borkmann, tom, netdev, LKML
In-Reply-To: <153736009982.24033.13696245431713246950.stgit@localhost.localdomain>

On Wed, Sep 19, 2018 at 5:29 AM Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
>
> Many workloads have polling mode of work. The application
> checks for incomming packets from time to time, but it also
> has a work to do, when there is no packets. This RFC
> tries to develop an idea to queue RPS packets on idle
> CPU in the the L3 domain of the consumer, so backlog
> processing of the packets and the application can execute
> in parallel.
>
> We require this in case of network cards does not
> have enough RX queues to cover all online CPUs (this seems
> to be the most cards), and  get_rps_cpu() actually chooses
> remote cpu, and SMP interrupt is sent. Here we may try
> our best, and to find idle CPU nearly the consumer's CPU.
> Note, that in case of consumer works in poll mode and it
> does not waits for incomming packets, its CPU will be not
> idle, while CPU of a sleeping consumer may be idle. So,
> not polling consumers will still be able to have skb
> handled on its CPU.
>
> In case of network card has many queues, the device
> interrupts will come on consumer's CPU, and this patch
> won't try to find idle cpu for them.
>
> I've tried simple netperf test for this:
> netserver -p 1234
> netperf -L 127.0.0.1 -p 1234 -l 100
>
> Before:
>  87380  16384  16384    100.00   60323.56
>  87380  16384  16384    100.00   60388.46
>  87380  16384  16384    100.00   60217.68
>  87380  16384  16384    100.00   57995.41
>  87380  16384  16384    100.00   60659.00
>
> After:
>  87380  16384  16384    100.00   64569.09
>  87380  16384  16384    100.00   64569.25
>  87380  16384  16384    100.00   64691.63
>  87380  16384  16384    100.00   64930.14
>  87380  16384  16384    100.00   62670.15
>
> The difference between best runs is +7%,
> the worst runs differ +8%.
>
> What do you think about following somehow in this way?

Hi Kirill

In my experience, scheduler has a poor view of softirq processing
happening on various cpus.
A cpu spending 90% of its cycles processing IRQ might be considered 'idle'

So please run a real workload (it is _very_ uncommon anyone set up RPS
on lo interface !)

Like 400 or more concurrent netperf -t TCP_RR on a 10Gbit NIC.

Thanks.

PS: Idea of playing with L3 domains is interesting, I have personally
tried various strategies in the past but none of them
demonstrated a clear win.

^ permalink raw reply

* Re: [PATCH v2 2/4] dt-bindings: net: qcom: Add binding for shared mdio bus
From: Wang, Dongsheng @ 2018-09-19  9:19 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, timur@kernel.org, davem@davemloft.net,
	Zheng, Joey, netdev@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <20180918123545.GA29092@lunn.ch>

On 2018/9/18 20:35, Andrew Lunn wrote:
>>> If you want to describe the MDIO controller, then you embed a mdio
>>> subnode into your Ethernet MAC node:
>>>
>>>  emac0: ethernet@feb20000 {
>>> 	mdio {
>>> 		#address-cells = <1>;
>>> 		#size-cells = <0>;
>>>
>>> 		phy0: ethernet-phy@0 {
>>> 			reg = <0>;
>>> 		};
>>> 	};
>>> };
>>>
>>> And then each Ethernet MAC controller refers to their appropriate PHY
>>> device tree node using a phy-handle property to point to either their
>>> own MDIO controller, or another MAC's MDIO controller.
>> Sorry, I do not understand how phy-handle point to MDIO controller,
>> because phy-handle is defined to point to a phy.
> The MAC driver does not care what MDIO controller a PHY is on. All you
> need to do to register the PHY is:

Yes, these are all things that must be done, and emac driver will
connect phy when mac up.
If we had a separate MDIO controller, the MAC would not care about MDIO
bus. But MDIO is integrated within the EMAC, and emac driver maintains
the mdio.

Each EMAC do their mdio register/unregister. But in the shared scenario,
the EMACs that use the shared bus do not need to create an MDIO and
cannot release the Shared bus.

In device tree environment as you and Florian said. Just use phy-handle
get the phy_node.
The EMAC would not care about the phy come from which MDIO bus because
the phy device gets from the device_node match(phy-handle). And if the
phy_dev cannot get through phy_node means, the mdio bus is not ready.

But ACPI environment my understand is this:
First method. EMAC driver gets the shared MDIO bus, and maintain it.
The second way, EMAC match the phy_dev from the name.
These patch series try to use the FIRST way. Now I prefer to use the
second way to do the shared function.

I will rework this patchset and maybe patches will be a delay for a few
days.

Cheers,
Dongsheng
> 	phy_node = of_parse_phandle(np, "phy-handle", 0);
> 	phy_interface = of_get_phy_mode(np);
> 	phydev = of_phy_connect(dev, phy_node,
>                                 &handle_link_change, 0,
>                                 phy_interface);
>
> 	Andrew
>

^ permalink raw reply

* Re: [RFC 4/5] netlink: prepare validate extack setting for recursion
From: Johannes Berg @ 2018-09-19  9:25 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner; +Cc: netdev
In-Reply-To: <20180919033733.GK4590@localhost.localdomain>

On Wed, 2018-09-19 at 00:37 -0300, Marcelo Ricardo Leitner wrote:

> Did you consider indicating the message level, and only overwrite the
> message that is already in there if the new message level is higher
> than the current one?

Hmm, no, I guess I didn't - I'm not even sure I understand what you're
saying.

This code in itself generates no "warning" messages; that was just a
construct we discussed in the NLA_REJECT thread, e.g. if you say (like I
just also wrote in my reply to Jiri):

	NL_SET_ERR_MSG(extack, "warning: deprecated command");
	err = nla_parse(..., extack);
	if (err)
		return err;
	/* do something */
	return 0;

Here you could consider the message there a warning that's transported
out even if we return 0, but if we return with a failure from
nla_parse() (or nla_validate instead if you wish), then that failure
message "wins".

> This way the first to set an Error message will have it, and Warning
> messages would be overwritten by such if needed. But it also would
> cause the first warning to be held, and not the last one, as it does
> today. We want the first error, but the last warning otherwise.
> 
> It would not be possible to overwrite if new_msglvl >= cur_msglvl
> because then it would trigger the initial issue again, so some extra
> logic would be needed to solve this.

That sounds way more complex than what I'm doing now?

Note, like I said above, this isn't *generic* in any way. This code here
will only ever set error messages that should "win".

I suppose we could - technically - make that generic, in that we could
have both

  NLA_SET_WARN_MSG(extack, "...");
  NLA_SET_ERR_MSG(extack, "...");

and keep track of warning vs. error; however, just like my first version
of the NLA_REJECT patch, that would break existing code.

I also don't think that we actually *need* this complexity in general.
It should almost always be possible (and realistically, pretty easy) to
structure your code in a way that warning messages only go out if no
error message overwrites them. The only reason we were ever even
discussing this was that in NLA_REJECT I missed the fact that somebody
could've set a message before and thus would keep it rather than
overwrite it, which was a change in behaviour.

Now, with this patch, all I'm doing is changing the internal behaviour
of nla_parse/nla_validate - externally, it still overwrites any existing
message if an error occurs, but internally it keeps the inner-most
error.

Why is this? Consider this:

static const struct nla_policy inner_policy[] = {
	[INNER_FLAG] = { .type = NLA_REJECT,
                         .validation_data = "must not set this flag" }
};

static const struct nla_policy outer_policy[] = {
	[OUTER_NESTING] = { .type = NLA_NESTED, .len = INNER_MAX,
                            .validation_data = inner_policy,
};

Now if you invoke nla_parse/nla_validate with a message like this

[ OUTER_NESTING => [ INNER_FLAG, ... ], ... ]

you'd get "must not set this flag" with the error offset pointing to
that; if I didn't do this construction here with inner messages winning,
you'd get "Attribute failed policy validation" with the error offset
pointing to the "OUTER_NESTING" attribute, that's pretty useless.

>From an external API POV though, nla_validate/nla_parse will continue to
unconditionally overwrite any existing "warning" messages with errors,
if such occurred. They just won't overwrite their own messages when
returning from a nested policy validation.

johannes

^ permalink raw reply

* Re: [RFC 4/5] netlink: prepare validate extack setting for recursion
From: Jiri Benc @ 2018-09-19  9:28 UTC (permalink / raw)
  To: Johannes Berg; +Cc: netdev
In-Reply-To: <1537348525.10305.15.camel@sipsolutions.net>

On Wed, 19 Sep 2018 11:15:25 +0200, Johannes Berg wrote:
> For one, having the NL_SET_* macros check it on their own will already
> not work - as we discussed over in the NLA_REJECT thread, we do need to
> be able to override the data, e.g. if somebody does
> 
> NL_SET_ERR_MSG(extack, "warning: deprecated command");
> err = nla_parse(..., extack);
> 
> and nla_parse() sets a new message. Thus, hiding all the logic in there
> already will not work, and is also IMHO rather unexpected. Normally,
> *later* messages should win, not *earlier* ones - and that doesn't
> require any tracking, just setting them unconditionally.
> 
> It's just a side effect of how we do the recursive validation here that
> we want *earlier* messages to win (but only within this code piece - if
> a previous message was set, we want it to be overwritten by nla_parse!).

Fair enough.

> It might be possible to do this differently, in theory, but all the ways
> I've tried to come up with so far made the code vastly more complex.

Wouldn't still make sense to store the flag in the struct
netlink_ext_ack, though? The way the parameters are passed around in
this patch looks ugly. And adding more users of the flag later (there
may be other cases when we want the earlier messages to be preserved)
would mean adding parameters all around, while the flag in the struct
would be readily available.

I don't have a strong opinion on this, just the patch seems to be
inelegant.

 Jiri

^ permalink raw reply

* [PATCH net-next 03/12] Documentation/bindings: net: marvell-pp2: update the IRQs description
From: Antoine Tenart @ 2018-09-19  9:27 UTC (permalink / raw)
  To: davem
  Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh,
	stefanc, ymarkman, mw
In-Reply-To: <20180919092711.22296-1-antoine.tenart@bootlin.com>

This patch updates the interrupts part of the Marvell PPv2 driver
bindings documentation, to keep it in sync with the driver.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 .../devicetree/bindings/net/marvell-pp2.txt   | 45 +++++++++++++------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/marvell-pp2.txt b/Documentation/devicetree/bindings/net/marvell-pp2.txt
index fc019df0d863..b78397669320 100644
--- a/Documentation/devicetree/bindings/net/marvell-pp2.txt
+++ b/Documentation/devicetree/bindings/net/marvell-pp2.txt
@@ -31,7 +31,7 @@ required.
 
 Required properties (port):
 
-- interrupts: interrupt for the port
+- interrupts: interrupt(s) for the port
 - port-id: ID of the port from the MAC point of view
 - gop-port-id: only for marvell,armada-7k-pp2, ID of the port from the
   GOP (Group Of Ports) point of view. This ID is used to index the
@@ -43,10 +43,12 @@ Optional properties (port):
 - marvell,loopback: port is loopback mode
 - phy: a phandle to a phy node defining the PHY address (as the reg
   property, a single integer).
-- interrupt-names: if more than a single interrupt for rx is given, must
-                   be the name associated to the interrupts listed. Valid
-                   names are: "tx-cpu0", "tx-cpu1", "tx-cpu2", "tx-cpu3",
-		   "rx-shared", "link".
+- interrupt-names: if more than a single interrupt for is given, must be the
+                   name associated to the interrupts listed. Valid names are:
+                   "hifX", with X in [0..8], and "link". The names "tx-cpu0",
+                   "tx-cpu1", "tx-cpu2", "tx-cpu3" and "rx-shared" are supported
+                   for backward compatibility but shouldn't be used for new
+                   additions.
 - marvell,system-controller: a phandle to the system controller.
 
 Example for marvell,armada-375-pp2:
@@ -89,9 +91,14 @@ cpm_ethernet: ethernet@0 {
 			     <ICU_GRP_NSR 43 IRQ_TYPE_LEVEL_HIGH>,
 			     <ICU_GRP_NSR 47 IRQ_TYPE_LEVEL_HIGH>,
 			     <ICU_GRP_NSR 51 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 55 IRQ_TYPE_LEVEL_HIGH>;
-		interrupt-names = "tx-cpu0", "tx-cpu1", "tx-cpu2",
-				  "tx-cpu3", "rx-shared";
+			     <ICU_GRP_NSR 55 IRQ_TYPE_LEVEL_HIGH>,
+			     <ICU_GRP_NSR 59 IRQ_TYPE_LEVEL_HIGH>,
+			     <ICU_GRP_NSR 63 IRQ_TYPE_LEVEL_HIGH>,
+			     <ICU_GRP_NSR 67 IRQ_TYPE_LEVEL_HIGH>,
+			     <ICU_GRP_NSR 71 IRQ_TYPE_LEVEL_HIGH>,
+			     <ICU_GRP_NSR 129 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "hif0", "hif1", "hif2", "hif3", "hif4",
+				  "hif5", "hif6", "hif7", "hif8", "link";
 		port-id = <0>;
 		gop-port-id = <0>;
 	};
@@ -101,9 +108,14 @@ cpm_ethernet: ethernet@0 {
 			     <ICU_GRP_NSR 44 IRQ_TYPE_LEVEL_HIGH>,
 			     <ICU_GRP_NSR 48 IRQ_TYPE_LEVEL_HIGH>,
 			     <ICU_GRP_NSR 52 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 56 IRQ_TYPE_LEVEL_HIGH>;
-		interrupt-names = "tx-cpu0", "tx-cpu1", "tx-cpu2",
-				  "tx-cpu3", "rx-shared";
+			     <ICU_GRP_NSR 56 IRQ_TYPE_LEVEL_HIGH>,
+			     <ICU_GRP_NSR 60 IRQ_TYPE_LEVEL_HIGH>,
+			     <ICU_GRP_NSR 64 IRQ_TYPE_LEVEL_HIGH>,
+			     <ICU_GRP_NSR 68 IRQ_TYPE_LEVEL_HIGH>,
+			     <ICU_GRP_NSR 72 IRQ_TYPE_LEVEL_HIGH>,
+			     <ICU_GRP_NSR 128 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "hif0", "hif1", "hif2", "hif3", "hif4",
+				  "hif5", "hif6", "hif7", "hif8", "link";
 		port-id = <1>;
 		gop-port-id = <2>;
 	};
@@ -113,9 +125,14 @@ cpm_ethernet: ethernet@0 {
 			     <ICU_GRP_NSR 45 IRQ_TYPE_LEVEL_HIGH>,
 			     <ICU_GRP_NSR 49 IRQ_TYPE_LEVEL_HIGH>,
 			     <ICU_GRP_NSR 53 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 57 IRQ_TYPE_LEVEL_HIGH>;
-		interrupt-names = "tx-cpu0", "tx-cpu1", "tx-cpu2",
-				  "tx-cpu3", "rx-shared";
+			     <ICU_GRP_NSR 57 IRQ_TYPE_LEVEL_HIGH>,
+			     <ICU_GRP_NSR 61 IRQ_TYPE_LEVEL_HIGH>,
+			     <ICU_GRP_NSR 65 IRQ_TYPE_LEVEL_HIGH>,
+			     <ICU_GRP_NSR 69 IRQ_TYPE_LEVEL_HIGH>,
+			     <ICU_GRP_NSR 73 IRQ_TYPE_LEVEL_HIGH>,
+			     <ICU_GRP_NSR 127 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "hif0", "hif1", "hif2", "hif3", "hif4",
+				  "hif5", "hif6", "hif7", "hif8", "link";
 		port-id = <2>;
 		gop-port-id = <3>;
 	};
-- 
2.17.1

^ permalink raw reply related

* [PATCH net-next 00/12] net: mvpp2: improve the interrupt usage
From: Antoine Tenart @ 2018-09-19  9:26 UTC (permalink / raw)
  To: davem
  Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh,
	stefanc, ymarkman, mw

Hi all,

This series aims to improve the interrupts descriptions and usage in the
Marvell PPv2 driver.

- Before the series interrupts were named after their s/w usage,
  which in fact can be configured. The series rename all those
  interrupts and add a description of the ones left over.

- In PPv2 the interrupts are mapped to vectors. Those vectors were
  directly mapped to a given CPU, and per-cpu accesses were done. While
  this worked on our cases, the registers accesses mapped to the vectors
  are not actually linked to a given CPU. They instead are linked to
  what is called a "s/w thread". The series modify this so that the s/w
  threads are used instead of the CPU numbers, by adding an indirection.
  This means we now can have systems with more CPUs than s/w threads.

This is based on today's net-next, and was tested on various boards
using both versions of the PPv2 engine.

Two more patches will be coming, to update the device trees describing a
PPv2 engine. The patches are ready, but will go through a different
tree. I'll send them once this series will be accepted. This is not an
issue as the PPv2 driver keeps the dt bindings backward compatibility.

Thanks!
Antoine

Antoine Tenart (12):
  net: mvpp2: increase the number of s/w threads to 9
  net: mvpp2: rename the IRQs to match the hardware
  Documentation/bindings: net: marvell-pp2: update the IRQs description
  net: mvpp2: do not update the queue mode while probing
  net: mvpp2: fix the number of queues per cpu for PPv2.2
  net: mvpp2: cpu should always be unsigned
  net: mvpp2: make the per-cpu helpers static
  net: mvpp2: make mvpp2_read_relaxed static
  net: mvpp2: do not use the CPU number to access the per-thread
    registers
  net: mvpp2: map the CPUs to threads
  net: mvpp2: handle cases where more CPUs are available than s/w
    threads
  net: mvpp2: rename mvpp2_percpu function to mvpp2_thread

 .../devicetree/bindings/net/marvell-pp2.txt   |  45 +-
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |  27 +-
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 431 +++++++++++-------
 3 files changed, 315 insertions(+), 188 deletions(-)

-- 
2.17.1

^ permalink raw reply

* [PATCH net-next 07/12] net: mvpp2: make the per-cpu helpers static
From: Antoine Tenart @ 2018-09-19  9:27 UTC (permalink / raw)
  To: davem
  Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh,
	stefanc, ymarkman, mw
In-Reply-To: <20180919092711.22296-1-antoine.tenart@bootlin.com>

The Marvell PPv2 driver has per-cpu functions. As they only are used in
the main file, make them static and remove their prototype from the
header.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h      | 7 -------
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 6 +++---
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 57fea9193a49..5247f2d1f573 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -1099,13 +1099,6 @@ u32 mvpp2_read(struct mvpp2 *priv, u32 offset);
 
 u32 mvpp2_read_relaxed(struct mvpp2 *priv, u32 offset);
 
-void mvpp2_percpu_write(struct mvpp2 *priv, unsigned int cpu, u32 offset,
-			u32 data);
-u32 mvpp2_percpu_read(struct mvpp2 *priv, unsigned int cpu, u32 offset);
-
-void mvpp2_percpu_write_relaxed(struct mvpp2 *priv, unsigned int cpu,
-				u32 offset, u32 data);
-
 void mvpp2_dbgfs_init(struct mvpp2 *priv, const char *name);
 
 void mvpp2_dbgfs_cleanup(struct mvpp2 *priv);
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 8d4e5d0c0ad8..f5edde2994e0 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -123,19 +123,19 @@ u32 mvpp2_read_relaxed(struct mvpp2 *priv, u32 offset)
  *   MVPP2_TXQ_PREF_BUF_REG    (related to MVPP2_TXQ_NUM_REG)
  *   MVPP2_TXQ_PREF_BUF_REG    (related to MVPP2_TXQ_NUM_REG)
  */
-void mvpp2_percpu_write(struct mvpp2 *priv, unsigned int cpu,
+static void mvpp2_percpu_write(struct mvpp2 *priv, unsigned int cpu,
 			       u32 offset, u32 data)
 {
 	writel(data, priv->swth_base[cpu] + offset);
 }
 
-u32 mvpp2_percpu_read(struct mvpp2 *priv, unsigned int cpu,
+static u32 mvpp2_percpu_read(struct mvpp2 *priv, unsigned int cpu,
 			     u32 offset)
 {
 	return readl(priv->swth_base[cpu] + offset);
 }
 
-void mvpp2_percpu_write_relaxed(struct mvpp2 *priv, unsigned int cpu,
+static void mvpp2_percpu_write_relaxed(struct mvpp2 *priv, unsigned int cpu,
 				       u32 offset, u32 data)
 {
 	writel_relaxed(data, priv->swth_base[cpu] + offset);
-- 
2.17.1

^ permalink raw reply related

* [PATCH net-next 1/3] net: phy: phylink: ensure the carrier is off when starting phylink
From: Antoine Tenart @ 2018-09-19  9:39 UTC (permalink / raw)
  To: davem, linux, andrew, f.fainelli
  Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni,
	maxime.chevallier, gregory.clement, miquel.raynal, nadavh,
	stefanc, ymarkman, mw
In-Reply-To: <20180919093933.24411-1-antoine.tenart@bootlin.com>

Phylink made an assumption about the carrier state being down when
calling phylink_start(). If this assumption isn't satisfied, the
internal phylink state could misbehave and a net device could end up not
being functional.

This patch fixes this by explicitly calling netif_carrier_off() in
phylink_start().

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/phy/phylink.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 3ba5cf2a8a5f..1d01e0c625a5 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -901,6 +901,9 @@ void phylink_start(struct phylink *pl)
 		    phylink_an_mode_str(pl->link_an_mode),
 		    phy_modes(pl->link_config.interface));
 
+	/* Always set the carrier off */
+	netif_carrier_off(pl->netdev);
+
 	/* Apply the link configuration to the MAC when starting. This allows
 	 * a fixed-link to start with the correct parameters, and also
 	 * ensures that we set the appropriate advertisement for Serdes links.
-- 
2.17.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox