lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 22 Oct 2019 08:49:40 +0000
From:   Laurentiu Tudor <laurentiu.tudor@....com>
To:     Madalin-cristian Bucur <madalin.bucur@....com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     Roy Pledge <roy.pledge@....com>
Subject: Re: [PATCH net-next 5/6] dpaa_eth: change DMA device

Hello,

On 21.10.2019 15:28, Madalin-cristian Bucur wrote:
> The DPAA Ethernet driver is using the FMan MAC as the device for DMA
> mapping. This is not actually correct, as the real DMA device is the
> FMan port (the FMan Rx port for reception and the FMan Tx port for
> transmission). Changing the device used for DMA mapping to the Fman
> Rx and Tx port devices.
> 
> Signed-off-by: Madalin Bucur <madalin.bucur@....com>
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@....com>
> ---
>   drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 105 +++++++++++++------------
>   drivers/net/ethernet/freescale/dpaa/dpaa_eth.h |   8 +-
>   2 files changed, 62 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> index 8d5686d88d30..639cafaa59b8 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> @@ -1335,15 +1335,15 @@ static void dpaa_fd_release(const struct net_device *net_dev,
>   		vaddr = phys_to_virt(qm_fd_addr(fd));
>   		sgt = vaddr + qm_fd_get_offset(fd);
>   
> -		dma_unmap_single(dpaa_bp->dev, qm_fd_addr(fd), dpaa_bp->size,
> -				 DMA_FROM_DEVICE);
> +		dma_unmap_single(dpaa_bp->priv->rx_dma_dev, qm_fd_addr(fd),
> +				 dpaa_bp->size, DMA_FROM_DEVICE);
>   
>   		dpaa_release_sgt_members(sgt);
>   
> -		addr = dma_map_single(dpaa_bp->dev, vaddr, dpaa_bp->size,
> -				      DMA_FROM_DEVICE);
> -		if (dma_mapping_error(dpaa_bp->dev, addr)) {
> -			dev_err(dpaa_bp->dev, "DMA mapping failed");
> +		addr = dma_map_single(dpaa_bp->priv->rx_dma_dev, vaddr,
> +				      dpaa_bp->size, DMA_FROM_DEVICE);
> +		if (dma_mapping_error(dpaa_bp->priv->rx_dma_dev, addr)) {
> +			netdev_err(net_dev, "DMA mapping failed");
>   			return;
>   		}
>   		bm_buffer_set64(&bmb, addr);
> @@ -1488,7 +1488,7 @@ static int dpaa_enable_tx_csum(struct dpaa_priv *priv,
>   
>   static int dpaa_bp_add_8_bufs(const struct dpaa_bp *dpaa_bp)
>   {
> -	struct device *dev = dpaa_bp->dev;
> +	struct net_device *net_dev = dpaa_bp->priv->net_dev;
>   	struct bm_buffer bmb[8];
>   	dma_addr_t addr;
>   	void *new_buf;
> @@ -1497,16 +1497,18 @@ static int dpaa_bp_add_8_bufs(const struct dpaa_bp *dpaa_bp)
>   	for (i = 0; i < 8; i++) {
>   		new_buf = netdev_alloc_frag(dpaa_bp->raw_size);
>   		if (unlikely(!new_buf)) {
> -			dev_err(dev, "netdev_alloc_frag() failed, size %zu\n",
> -				dpaa_bp->raw_size);
> +			netdev_err(net_dev,
> +				   "netdev_alloc_frag() failed, size %zu\n",
> +				   dpaa_bp->raw_size);
>   			goto release_previous_buffs;
>   		}
>   		new_buf = PTR_ALIGN(new_buf, SMP_CACHE_BYTES);
>   
> -		addr = dma_map_single(dev, new_buf,
> +		addr = dma_map_single(dpaa_bp->priv->rx_dma_dev, new_buf,
>   				      dpaa_bp->size, DMA_FROM_DEVICE);
> -		if (unlikely(dma_mapping_error(dev, addr))) {
> -			dev_err(dpaa_bp->dev, "DMA map failed");
> +		if (unlikely(dma_mapping_error(dpaa_bp->priv->rx_dma_dev,
> +					       addr))) {
> +			netdev_err(net_dev, "DMA map failed");
>   			goto release_previous_buffs;
>   		}
>   
> @@ -1634,7 +1636,7 @@ static struct sk_buff *dpaa_cleanup_tx_fd(const struct dpaa_priv *priv,
>   
>   	if (unlikely(qm_fd_get_format(fd) == qm_fd_sg)) {
>   		nr_frags = skb_shinfo(skb)->nr_frags;
> -		dma_unmap_single(dev, addr,
> +		dma_unmap_single(priv->tx_dma_dev, addr,
>   				 qm_fd_get_offset(fd) + DPAA_SGT_SIZE,
>   				 dma_dir);
>   
> @@ -1644,21 +1646,21 @@ static struct sk_buff *dpaa_cleanup_tx_fd(const struct dpaa_priv *priv,
>   		sgt = phys_to_virt(addr + qm_fd_get_offset(fd));
>   
>   		/* sgt[0] is from lowmem, was dma_map_single()-ed */
> -		dma_unmap_single(dev, qm_sg_addr(&sgt[0]),
> +		dma_unmap_single(priv->tx_dma_dev, qm_sg_addr(&sgt[0]),
>   				 qm_sg_entry_get_len(&sgt[0]), dma_dir);
>   
>   		/* remaining pages were mapped with skb_frag_dma_map() */
>   		for (i = 1; i <= nr_frags; i++) {
>   			WARN_ON(qm_sg_entry_is_ext(&sgt[i]));
>   
> -			dma_unmap_page(dev, qm_sg_addr(&sgt[i]),
> +			dma_unmap_page(priv->tx_dma_dev, qm_sg_addr(&sgt[i]),
>   				       qm_sg_entry_get_len(&sgt[i]), dma_dir);
>   		}
>   
>   		/* Free the page frag that we allocated on Tx */
>   		skb_free_frag(phys_to_virt(addr));
>   	} else {
> -		dma_unmap_single(dev, addr,
> +		dma_unmap_single(priv->tx_dma_dev, addr,
>   				 skb_tail_pointer(skb) - (u8 *)skbh, dma_dir);
>   	}
>   
> @@ -1762,8 +1764,8 @@ static struct sk_buff *sg_fd_to_skb(const struct dpaa_priv *priv,
>   			goto free_buffers;
>   
>   		count_ptr = this_cpu_ptr(dpaa_bp->percpu_count);
> -		dma_unmap_single(dpaa_bp->dev, sg_addr, dpaa_bp->size,
> -				 DMA_FROM_DEVICE);
> +		dma_unmap_single(dpaa_bp->priv->rx_dma_dev, sg_addr,
> +				 dpaa_bp->size, DMA_FROM_DEVICE);
>   		if (!skb) {
>   			sz = dpaa_bp->size +
>   				SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> @@ -1853,7 +1855,6 @@ static int skb_to_contig_fd(struct dpaa_priv *priv,
>   			    int *offset)
>   {
>   	struct net_device *net_dev = priv->net_dev;
> -	struct device *dev = net_dev->dev.parent;
>   	enum dma_data_direction dma_dir;
>   	unsigned char *buffer_start;
>   	struct sk_buff **skbh;
> @@ -1889,9 +1890,9 @@ static int skb_to_contig_fd(struct dpaa_priv *priv,
>   	fd->cmd |= cpu_to_be32(FM_FD_CMD_FCO);
>   
>   	/* Map the entire buffer size that may be seen by FMan, but no more */
> -	addr = dma_map_single(dev, skbh,
> +	addr = dma_map_single(priv->tx_dma_dev, skbh,
>   			      skb_tail_pointer(skb) - buffer_start, dma_dir);
> -	if (unlikely(dma_mapping_error(dev, addr))) {
> +	if (unlikely(dma_mapping_error(priv->tx_dma_dev, addr))) {
>   		if (net_ratelimit())
>   			netif_err(priv, tx_err, net_dev, "dma_map_single() failed\n");
>   		return -EINVAL;
> @@ -1907,7 +1908,6 @@ static int skb_to_sg_fd(struct dpaa_priv *priv,
>   	const enum dma_data_direction dma_dir = DMA_TO_DEVICE;
>   	const int nr_frags = skb_shinfo(skb)->nr_frags;
>   	struct net_device *net_dev = priv->net_dev;
> -	struct device *dev = net_dev->dev.parent;
>   	struct qm_sg_entry *sgt;
>   	struct sk_buff **skbh;
>   	int i, j, err, sz;
> @@ -1946,10 +1946,10 @@ static int skb_to_sg_fd(struct dpaa_priv *priv,
>   	qm_sg_entry_set_len(&sgt[0], frag_len);
>   	sgt[0].bpid = FSL_DPAA_BPID_INV;
>   	sgt[0].offset = 0;
> -	addr = dma_map_single(dev, skb->data,
> +	addr = dma_map_single(priv->tx_dma_dev, skb->data,
>   			      skb_headlen(skb), dma_dir);
> -	if (unlikely(dma_mapping_error(dev, addr))) {
> -		dev_err(dev, "DMA mapping failed");
> +	if (unlikely(dma_mapping_error(priv->tx_dma_dev, addr))) {
> +		netdev_err(priv->net_dev, "DMA mapping failed");
>   		err = -EINVAL;
>   		goto sg0_map_failed;
>   	}
> @@ -1960,10 +1960,10 @@ static int skb_to_sg_fd(struct dpaa_priv *priv,
>   		frag = &skb_shinfo(skb)->frags[i];
>   		frag_len = skb_frag_size(frag);
>   		WARN_ON(!skb_frag_page(frag));
> -		addr = skb_frag_dma_map(dev, frag, 0,
> +		addr = skb_frag_dma_map(priv->tx_dma_dev, frag, 0,
>   					frag_len, dma_dir);
> -		if (unlikely(dma_mapping_error(dev, addr))) {
> -			dev_err(dev, "DMA mapping failed");
> +		if (unlikely(dma_mapping_error(priv->tx_dma_dev, addr))) {
> +			netdev_err(priv->net_dev, "DMA mapping failed");
>   			err = -EINVAL;
>   			goto sg_map_failed;
>   		}
> @@ -1986,10 +1986,10 @@ static int skb_to_sg_fd(struct dpaa_priv *priv,
>   	skbh = (struct sk_buff **)buffer_start;
>   	*skbh = skb;
>   
> -	addr = dma_map_single(dev, buffer_start,
> +	addr = dma_map_single(priv->tx_dma_dev, buffer_start,
>   			      priv->tx_headroom + DPAA_SGT_SIZE, dma_dir);
> -	if (unlikely(dma_mapping_error(dev, addr))) {
> -		dev_err(dev, "DMA mapping failed");
> +	if (unlikely(dma_mapping_error(priv->tx_dma_dev, addr))) {
> +		netdev_err(priv->net_dev, "DMA mapping failed");
>   		err = -EINVAL;
>   		goto sgt_map_failed;
>   	}
> @@ -2003,7 +2003,7 @@ static int skb_to_sg_fd(struct dpaa_priv *priv,
>   sgt_map_failed:
>   sg_map_failed:
>   	for (j = 0; j < i; j++)
> -		dma_unmap_page(dev, qm_sg_addr(&sgt[j]),
> +		dma_unmap_page(priv->tx_dma_dev, qm_sg_addr(&sgt[j]),
>   			       qm_sg_entry_get_len(&sgt[j]), dma_dir);
>   sg0_map_failed:
>   csum_failed:
> @@ -2304,7 +2304,8 @@ static enum qman_cb_dqrr_result rx_default_dqrr(struct qman_portal *portal,
>   		return qman_cb_dqrr_consume;
>   	}
>   
> -	dma_unmap_single(dpaa_bp->dev, addr, dpaa_bp->size, DMA_FROM_DEVICE);
> +	dma_unmap_single(dpaa_bp->priv->rx_dma_dev, addr, dpaa_bp->size,
> +			 DMA_FROM_DEVICE);
>   
>   	/* prefetch the first 64 bytes of the frame or the SGT start */
>   	vaddr = phys_to_virt(addr);
> @@ -2659,7 +2660,7 @@ static inline void dpaa_bp_free_pf(const struct dpaa_bp *bp,
>   {
>   	dma_addr_t addr = bm_buf_addr(bmb);
>   
> -	dma_unmap_single(bp->dev, addr, bp->size, DMA_FROM_DEVICE);
> +	dma_unmap_single(bp->priv->rx_dma_dev, addr, bp->size, DMA_FROM_DEVICE);
>   
>   	skb_free_frag(phys_to_virt(addr));
>   }
> @@ -2769,25 +2770,27 @@ static int dpaa_eth_probe(struct platform_device *pdev)
>   	int err = 0, i, channel;
>   	struct device *dev;
>   
> +	dev = &pdev->dev;
> +
>   	err = bman_is_probed();
>   	if (!err)
>   		return -EPROBE_DEFER;
>   	if (err < 0) {
> -		dev_err(&pdev->dev, "failing probe due to bman probe error\n");
> +		dev_err(dev, "failing probe due to bman probe error\n");

These changes seem unrelated.

>   		return -ENODEV;
>   	}
>   	err = qman_is_probed();
>   	if (!err)
>   		return -EPROBE_DEFER;
>   	if (err < 0) {
> -		dev_err(&pdev->dev, "failing probe due to qman probe error\n");
> +		dev_err(dev, "failing probe due to qman probe error\n");
>   		return -ENODEV;
>   	}
>   	err = bman_portals_probed();
>   	if (!err)
>   		return -EPROBE_DEFER;
>   	if (err < 0) {
> -		dev_err(&pdev->dev,
> +		dev_err(dev,
>   			"failing probe due to bman portals probe error\n");
>   		return -ENODEV;
>   	}
> @@ -2795,19 +2798,11 @@ static int dpaa_eth_probe(struct platform_device *pdev)
>   	if (!err)
>   		return -EPROBE_DEFER;
>   	if (err < 0) {
> -		dev_err(&pdev->dev,
> +		dev_err(dev,
>   			"failing probe due to qman portals probe error\n");
>   		return -ENODEV;
>   	}
>   
> -	/* device used for DMA mapping */
> -	dev = pdev->dev.parent;
> -	err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(40));
> -	if (err) {
> -		dev_err(dev, "dma_coerce_mask_and_coherent() failed\n");
> -		return err;
> -	}

Why are we dropping this explicit setting of the dma mask?

---
Best Regards, Laurentiu

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ