Re: [PATCH 1/2] staging: fsl-dpaa2/eth: Fix address translations

2017-05-29 Thread Laurentiu Tudor


On 05/25/2017 03:31 PM, Ruxandra Ioana Radulescu wrote:
>> -Original Message-
>> From: Laurentiu Tudor
>> Sent: Wednesday, May 24, 2017 3:34 PM
>> To: Ruxandra Ioana Radulescu <ruxandra.radule...@nxp.com>;
>> 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
>> <bogdan.purcare...@nxp.com>; stuyo...@gmail.com; Nipun Gupta
>> <nipun.gu...@nxp.com>
>> 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 <nipun.gu...@nxp.com>
>>> Signed-off-by: Ioana Radulescu <ruxandra.radule...@nxp.com>
>>> ---
>>>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.

Understood. Fine by me then.

---
Best Regards, Laurentiu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH 1/2] staging: fsl-dpaa2/eth: Fix address translations

2017-05-25 Thread Ruxandra Ioana Radulescu
> -Original Message-
> From: Laurentiu Tudor
> Sent: Wednesday, May 24, 2017 3:34 PM
> To: Ruxandra Ioana Radulescu <ruxandra.radule...@nxp.com>;
> 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
> <bogdan.purcare...@nxp.com>; stuyo...@gmail.com; Nipun Gupta
> <nipun.gu...@nxp.com>
> 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 <nipun.gu...@nxp.com>
> > Signed-off-by: Ioana Radulescu <ruxandra.radule...@nxp.com>
> > ---
> >   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 1/2] staging: fsl-dpaa2/eth: Fix address translations

2017-05-24 Thread Ioana Radulescu
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([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([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,
 >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

___
iommu mailing list

Re: [PATCH 1/2] staging: fsl-dpaa2/eth: Fix address translations

2017-05-24 Thread Laurentiu Tudor
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.

---
Best Regards, Laurentiu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu