RE: [PATCH 1/2] staging: fsl-dpaa2/eth: Fix address translations
> -Original Message-
> From: Laurentiu Tudor
> Sent: Wednesday, May 24, 2017 3:34 PM
> To: Ruxandra Ioana Radulescu ;
> gre...@linuxfoundation.org
> Cc: de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org;
> ag...@suse.de; a...@arndb.de; linux-arm-ker...@lists.infradead.org;
> iommu@lists.linux-foundation.org; Bogdan Purcareata
> ; stuyo...@gmail.com; Nipun Gupta
>
> Subject: Re: [PATCH 1/2] staging: fsl-dpaa2/eth: Fix address translations
>
> Hi Ioana,
>
> Debatable nit inline.
>
> On 05/24/2017 03:13 PM, Ioana Radulescu wrote:
> > Use the correct mechanisms for translating a DMA-mapped IOVA
> > address into a virtual one. Without this fix, once SMMU is
> > enabled on Layerscape platforms, the Ethernet driver throws
> > IOMMU translation faults.
> >
> > Signed-off-by: Nipun Gupta
> > Signed-off-by: Ioana Radulescu
> > ---
> > drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 25
> +++--
> > drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h | 1 +
> > 2 files changed, 20 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> > index 6f9eed66c64d..3fee0d6f17e0 100644
> > --- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> > +++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> > @@ -37,6 +37,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include "../../fsl-mc/include/mc.h"
> > #include "../../fsl-mc/include/mc-sys.h"
> > @@ -54,6 +55,16 @@ MODULE_DESCRIPTION("Freescale DPAA2 Ethernet
> Driver");
> >
> > const char dpaa2_eth_drv_version[] = "0.1";
> >
> > +static void *dpaa2_iova_to_virt(struct iommu_domain *domain,
>
> if you pass a "struct dpaa2_eth_priv *priv" instead of "iommu_domain"
> you can move the priv->iommu_domain reference in the function and
> slightly simplify the call sites.
Fair point, but I'd prefer keeping this function independent of the
Ethernet driver's private data structure. This way, if other (future)
DPAA2 drivers will need a similar function, we can just move it
to a common area instead of duplicating the code.
Thanks,
Ioana
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/2] staging: fsl-dpaa2/eth: Map Tx buffers as bidirectional
WRIOP hardware may need to write to the hardware annotation
area of Tx buffers (e.g. frame status bits) and also to
the data area (e.g. L4 checksum in frame header).
Map these buffers as DMA_BIDIRECTIONAL, otherwise the
write transaction through SMMU will not be allowed.
Signed-off-by: Nipun Gupta
Signed-off-by: Ioana Radulescu
---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
index 3fee0d6f17e0..49c435bad706 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
@@ -355,7 +355,7 @@ static int build_sg_fd(struct dpaa2_eth_priv *priv,
sg_init_table(scl, nr_frags + 1);
num_sg = skb_to_sgvec(skb, scl, 0, skb->len);
- num_dma_bufs = dma_map_sg(dev, scl, num_sg, DMA_TO_DEVICE);
+ num_dma_bufs = dma_map_sg(dev, scl, num_sg, DMA_BIDIRECTIONAL);
if (unlikely(!num_dma_bufs)) {
err = -ENOMEM;
goto dma_map_sg_failed;
@@ -406,7 +406,7 @@ static int build_sg_fd(struct dpaa2_eth_priv *priv,
swa->num_dma_bufs = num_dma_bufs;
/* Separately map the SGT buffer */
- addr = dma_map_single(dev, sgt_buf, sgt_buf_size, DMA_TO_DEVICE);
+ addr = dma_map_single(dev, sgt_buf, sgt_buf_size, DMA_BIDIRECTIONAL);
if (unlikely(dma_mapping_error(dev, addr))) {
err = -ENOMEM;
goto dma_map_single_failed;
@@ -423,7 +423,7 @@ static int build_sg_fd(struct dpaa2_eth_priv *priv,
dma_map_single_failed:
kfree(sgt_buf);
sgt_buf_alloc_failed:
- dma_unmap_sg(dev, scl, num_sg, DMA_TO_DEVICE);
+ dma_unmap_sg(dev, scl, num_sg, DMA_BIDIRECTIONAL);
dma_map_sg_failed:
kfree(scl);
return err;
@@ -461,7 +461,7 @@ static int build_single_fd(struct dpaa2_eth_priv *priv,
addr = dma_map_single(dev, buffer_start,
skb_tail_pointer(skb) - buffer_start,
- DMA_TO_DEVICE);
+ DMA_BIDIRECTIONAL);
if (unlikely(dma_mapping_error(dev, addr)))
return -ENOMEM;
@@ -510,7 +510,7 @@ static void free_tx_fd(const struct dpaa2_eth_priv *priv,
*/
dma_unmap_single(dev, fd_addr,
skb_tail_pointer(skb) - buffer_start,
-DMA_TO_DEVICE);
+DMA_BIDIRECTIONAL);
} else if (fd_format == dpaa2_fd_sg) {
swa = (struct dpaa2_eth_swa *)skbh;
skb = swa->skb;
@@ -519,13 +519,13 @@ static void free_tx_fd(const struct dpaa2_eth_priv *priv,
num_dma_bufs = swa->num_dma_bufs;
/* Unmap the scatterlist */
- dma_unmap_sg(dev, scl, num_sg, DMA_TO_DEVICE);
+ dma_unmap_sg(dev, scl, num_sg, DMA_BIDIRECTIONAL);
kfree(scl);
/* Unmap the SGT buffer */
unmap_size = priv->tx_data_offset +
sizeof(struct dpaa2_sg_entry) * (1 + num_dma_bufs);
- dma_unmap_single(dev, fd_addr, unmap_size, DMA_TO_DEVICE);
+ dma_unmap_single(dev, fd_addr, unmap_size, DMA_BIDIRECTIONAL);
} else {
/* Unsupported format, mark it as errored and give up */
if (status)
--
2.11.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/2] staging: fsl-dpaa2/eth: Fix address translations
Use the correct mechanisms for translating a DMA-mapped IOVA
address into a virtual one. Without this fix, once SMMU is
enabled on Layerscape platforms, the Ethernet driver throws
IOMMU translation faults.
Signed-off-by: Nipun Gupta
Signed-off-by: Ioana Radulescu
---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 25 +++--
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h | 1 +
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
index 6f9eed66c64d..3fee0d6f17e0 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include "../../fsl-mc/include/mc.h"
#include "../../fsl-mc/include/mc-sys.h"
@@ -54,6 +55,16 @@ MODULE_DESCRIPTION("Freescale DPAA2 Ethernet Driver");
const char dpaa2_eth_drv_version[] = "0.1";
+static void *dpaa2_iova_to_virt(struct iommu_domain *domain,
+ dma_addr_t iova_addr)
+{
+ phys_addr_t phys_addr;
+
+ phys_addr = domain ? iommu_iova_to_phys(domain, iova_addr) : iova_addr;
+
+ return phys_to_virt(phys_addr);
+}
+
static void validate_rx_csum(struct dpaa2_eth_priv *priv,
u32 fd_status,
struct sk_buff *skb)
@@ -98,12 +109,11 @@ static void free_rx_fd(struct dpaa2_eth_priv *priv,
sgt = vaddr + dpaa2_fd_get_offset(fd);
for (i = 0; i < DPAA2_ETH_MAX_SG_ENTRIES; i++) {
addr = dpaa2_sg_get_addr(&sgt[i]);
+ sg_vaddr = dpaa2_iova_to_virt(priv->iommu_domain, addr);
dma_unmap_single(dev, addr, DPAA2_ETH_RX_BUF_SIZE,
DMA_FROM_DEVICE);
- sg_vaddr = phys_to_virt(addr);
skb_free_frag(sg_vaddr);
-
if (dpaa2_sg_is_final(&sgt[i]))
break;
}
@@ -159,10 +169,10 @@ static struct sk_buff *build_frag_skb(struct
dpaa2_eth_priv *priv,
/* Get the address and length from the S/G entry */
sg_addr = dpaa2_sg_get_addr(sge);
+ sg_vaddr = dpaa2_iova_to_virt(priv->iommu_domain, sg_addr);
dma_unmap_single(dev, sg_addr, DPAA2_ETH_RX_BUF_SIZE,
DMA_FROM_DEVICE);
- sg_vaddr = phys_to_virt(sg_addr);
sg_length = dpaa2_sg_get_len(sge);
if (i == 0) {
@@ -222,8 +232,8 @@ static void dpaa2_eth_rx(struct dpaa2_eth_priv *priv,
/* Tracing point */
trace_dpaa2_rx_fd(priv->net_dev, fd);
+ vaddr = dpaa2_iova_to_virt(priv->iommu_domain, addr);
dma_unmap_single(dev, addr, DPAA2_ETH_RX_BUF_SIZE, DMA_FROM_DEVICE);
- vaddr = phys_to_virt(addr);
prefetch(vaddr + priv->buf_layout.private_data_size);
prefetch(vaddr + dpaa2_fd_get_offset(fd));
@@ -490,7 +500,7 @@ static void free_tx_fd(const struct dpaa2_eth_priv *priv,
struct dpaa2_fas *fas;
fd_addr = dpaa2_fd_get_addr(fd);
- skbh = phys_to_virt(fd_addr);
+ skbh = dpaa2_iova_to_virt(priv->iommu_domain, fd_addr);
if (fd_format == dpaa2_fd_single) {
skb = *skbh;
@@ -802,10 +812,11 @@ static void drain_bufs(struct dpaa2_eth_priv *priv, int
count)
}
for (i = 0; i < ret; i++) {
/* Same logic as on regular Rx path */
+ vaddr = dpaa2_iova_to_virt(priv->iommu_domain,
+ buf_array[i]);
dma_unmap_single(dev, buf_array[i],
DPAA2_ETH_RX_BUF_SIZE,
DMA_FROM_DEVICE);
- vaddr = phys_to_virt(buf_array[i]);
skb_free_frag(vaddr);
}
} while (ret);
@@ -2358,6 +2369,8 @@ static int dpaa2_eth_probe(struct fsl_mc_device *dpni_dev)
priv = netdev_priv(net_dev);
priv->net_dev = net_dev;
+ priv->iommu_domain = iommu_get_domain_for_dev(dev);
+
/* Obtain a MC portal */
err = fsl_mc_portal_allocate(dpni_dev, FSL_MC_IO_ATOMIC_CONTEXT_PORTAL,
&priv->mc_io);
diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
index c67cced55b72..55b47623008c 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
@@ -301,6 +301,7 @@ struct dpaa2_eth_priv {
struct fsl_mc_device *dpbp_dev;
struct dpbp_attr dpbp_attrs;
+ struct iommu_domain *iommu_domain;
u16 tx_qdid;
struct fsl_mc_io *mc_io;
--
2.11.0
_
