Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1734105 > unrolled thread

[PATCH net 1/3] net: mvpp2: fix the dma_mask and coherent_dma_mask settings for PPv2.2

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH net 1/3] net: mvpp2: fix the dma_mask and coherent_dma_mask settings for PPv2.2 Antoine Tenart <antoine.tenart@free-electrons.com> - 2017-09-18 15:10 +0200
    Re: [PATCH net 1/3] net: mvpp2: fix the dma_mask and  coherent_dma_mask settings for PPv2.2 David Miller <davem@davemloft.net> - 2017-09-19 02:20 +0200
      Re: [PATCH net 1/3] net: mvpp2: fix the dma_mask and  coherent_dma_mask settings for PPv2.2 Antoine Tenart <antoine.tenart@free-electrons.com> - 2017-09-21 16:30 +0200
        Re: [PATCH net 1/3] net: mvpp2: fix the dma_mask and  coherent_dma_mask settings for PPv2.2 David Miller <davem@davemloft.net> - 2017-09-21 19:40 +0200
          Re: [PATCH net 1/3] net: mvpp2: fix the dma_mask and  coherent_dma_mask settings for PPv2.2 Antoine Tenart <antoine.tenart@free-electrons.com> - 2017-09-25 14:50 +0200

#1734105 — [PATCH net 1/3] net: mvpp2: fix the dma_mask and coherent_dma_mask settings for PPv2.2

The dev->dma_mask usually points to dev->coherent_dma_mask. This is an
issue as setting both of them will override the other. This is
problematic here as the PPv2 driver uses a 32-bit-mask for coherent
accesses (txq, rxq, bm) and a 40-bit mask for all other accesses due to
an hardware limitation.

This can lead to a memory remap for all dma_map_single() calls when
dealing with memory above 4GB.

Fixes: 2067e0a13cfe ("net: mvpp2: set dma mask and coherent dma mask on PPv2.2")
Reported-by: Stefan Chulski <stefanc@marvell.com>
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index dd0ee2691c86..7024d4dbb461 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -7969,9 +7969,25 @@ static int mvpp2_probe(struct platform_device *pdev)
 	priv->tclk = clk_get_rate(priv->pp_clk);
 
 	if (priv->hw_version == MVPP22) {
+		/* If dma_mask points to coherent_dma_mask, setting both will
+		 * override the value of the other. This is problematic as the
+		 * PPv2 driver uses a 32-bit-mask for coherent accesses (txq,
+		 * rxq, bm) and a 40-bit mask for all other accesses.
+		 */
+		if (pdev->dev.dma_mask == &pdev->dev.coherent_dma_mask) {
+			pdev->dev.dma_mask = devm_kzalloc(&pdev->dev,
+							  sizeof(*pdev->dev.dma_mask),
+							  GFP_KERNEL);
+			if (!pdev->dev.dma_mask) {
+				err = -ENOMEM;
+				goto err_mg_clk;
+			}
+		}
+
 		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(40));
 		if (err)
 			goto err_mg_clk;
+
 		/* Sadly, the BM pools all share the same register to
 		 * store the high 32 bits of their address. So they
 		 * must all have the same high 32 bits, which forces
-- 
2.13.5

[toc] | [next] | [standalone]

#1734554 — Re: [PATCH net 1/3] net: mvpp2: fix the dma_mask and coherent_dma_mask settings for PPv2.2

From: Antoine Tenart <antoine.tenart@free-electrons.com>
Date: Mon, 18 Sep 2017 15:04:06 +0200

> The dev->dma_mask usually points to dev->coherent_dma_mask. This is an
> issue as setting both of them will override the other. This is
> problematic here as the PPv2 driver uses a 32-bit-mask for coherent
> accesses (txq, rxq, bm) and a 40-bit mask for all other accesses due to
> an hardware limitation.
> 
> This can lead to a memory remap for all dma_map_single() calls when
> dealing with memory above 4GB.
> 
> Fixes: 2067e0a13cfe ("net: mvpp2: set dma mask and coherent dma mask on PPv2.2")
> Reported-by: Stefan Chulski <stefanc@marvell.com>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>

Yikes.

I surrmise that if the platform has made dev->dma_mask point to
&dev->coherent_dma_mask, it is because it does not allow the two
settings to be set separately.

By rearranging the pointer, you are bypassing that, and probably
breaking things or creating a situation that the DMA mapping
layer is not expecting.

I want to know more about the situations where dma_mask is set to
point to &coherent_dma_mask and how that is supposed to work.

At a minimum this commit log message needs to go into more detail.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1736710 — Re: [PATCH net 1/3] net: mvpp2: fix the dma_mask and coherent_dma_mask settings for PPv2.2

Hi David,

On Mon, Sep 18, 2017 at 05:18:58PM -0700, David Miller wrote:
> From: Antoine Tenart <antoine.tenart@free-electrons.com>
> Date: Mon, 18 Sep 2017 15:04:06 +0200
> 
> > The dev->dma_mask usually points to dev->coherent_dma_mask. This is an
> > issue as setting both of them will override the other. This is
> > problematic here as the PPv2 driver uses a 32-bit-mask for coherent
> > accesses (txq, rxq, bm) and a 40-bit mask for all other accesses due to
> > an hardware limitation.
> > 
> > This can lead to a memory remap for all dma_map_single() calls when
> > dealing with memory above 4GB.
> > 
> > Fixes: 2067e0a13cfe ("net: mvpp2: set dma mask and coherent dma mask on PPv2.2")
> > Reported-by: Stefan Chulski <stefanc@marvell.com>
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> 
> I surrmise that if the platform has made dev->dma_mask point to
> &dev->coherent_dma_mask, it is because it does not allow the two
> settings to be set separately.

That's also the default when the platform does not allocate dma_mask.
You have a point, that could be because it's not supported. But I don't
know what would be a good check then.

> By rearranging the pointer, you are bypassing that, and probably
> breaking things or creating a situation that the DMA mapping
> layer is not expecting.
> 
> I want to know more about the situations where dma_mask is set to
> point to &coherent_dma_mask and how that is supposed to work.

From what I see in other parts of the kernel, dma_mask points to
&coherent_dma_mask by default and having two different values for these
two masks isn't a use case for many drivers.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1736971 — Re: [PATCH net 1/3] net: mvpp2: fix the dma_mask and coherent_dma_mask settings for PPv2.2

From: Antoine Tenart <antoine.tenart@free-electrons.com>
Date: Thu, 21 Sep 2017 16:24:13 +0200

> That's also the default when the platform does not allocate dma_mask.

That's the problem that needs to be fixed then.

[toc] | [prev] | [next] | [standalone]

#1739013 — Re: [PATCH net 1/3] net: mvpp2: fix the dma_mask and coherent_dma_mask settings for PPv2.2

On Thu, Sep 21, 2017 at 10:07:18AM -0700, David Miller wrote:
> From: Antoine Tenart <antoine.tenart@free-electrons.com>
> Date: Thu, 21 Sep 2017 16:24:13 +0200
> 
> > That's also the default when the platform does not allocate dma_mask.
> 
> That's the problem that needs to be fixed then.

OK, I'll drop this patch until I find a proper solution.

Thanks,
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web