netdev - RE: [PATCH 02/13] soc/fsl/bman: map FBPR area in the iommu

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]

Message-ID: <DB7PR04MB513214FB393EF3C514A26F21EC550@DB7PR04MB5132.eurprd04.prod.outlook.com>
Date:   Mon, 1 Apr 2019 11:15:21 +0000
From:   Laurentiu Tudor <laurentiu.tudor@....com>
To:     Leo Li <leoyang.li@....com>
CC:     Netdev <netdev@...r.kernel.org>,
        Madalin-cristian Bucur <madalin.bucur@....com>,
        Roy Pledge <roy.pledge@....com>,
        Camelia Alexandra Groza <camelia.groza@....com>,
        David Miller <davem@...emloft.net>,
        Linux IOMMU <iommu@...ts.linux-foundation.org>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        lkml <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 02/13] soc/fsl/bman: map FBPR area in the iommu

Hi Leo,

> -----Original Message-----
> From: Li Yang [mailto:leoyang.li@....com]
> Sent: Friday, March 29, 2019 11:16 PM
> 
> On Fri, Mar 29, 2019 at 9:03 AM <laurentiu.tudor@....com> wrote:
> >
> > From: Laurentiu Tudor <laurentiu.tudor@....com>
> >
> > Add a one-to-one iommu mapping for bman private data memory (FBPR).
> > This is required for BMAN to work without faults behind an iommu.
> >
> > Signed-off-by: Laurentiu Tudor <laurentiu.tudor@....com>
> > ---
> >  drivers/soc/fsl/qbman/bman_ccsr.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/soc/fsl/qbman/bman_ccsr.c
> b/drivers/soc/fsl/qbman/bman_ccsr.c
> > index 7c3cc968053c..b209c79511bb 100644
> > --- a/drivers/soc/fsl/qbman/bman_ccsr.c
> > +++ b/drivers/soc/fsl/qbman/bman_ccsr.c
> > @@ -29,6 +29,7 @@
> >   */
> >
> >  #include "bman_priv.h"
> > +#include <linux/iommu.h>
> >
> >  u16 bman_ip_rev;
> >  EXPORT_SYMBOL(bman_ip_rev);
> > @@ -178,6 +179,7 @@ static int fsl_bman_probe(struct platform_device
> *pdev)
> >         int ret, err_irq;
> >         struct device *dev = &pdev->dev;
> >         struct device_node *node = dev->of_node;
> > +       struct iommu_domain *domain;
> >         struct resource *res;
> >         u16 id, bm_pool_cnt;
> >         u8 major, minor;
> > @@ -225,6 +227,15 @@ static int fsl_bman_probe(struct platform_device
> *pdev)
> >
> >         dev_dbg(dev, "Allocated FBPR 0x%llx 0x%zx\n", fbpr_a, fbpr_sz);
> >
> > +       /* Create an 1-to-1 iommu mapping for FBPR area */
> > +       domain = iommu_get_domain_for_dev(dev);
> > +       if (domain) {
> > +               ret = iommu_map(domain, fbpr_a, fbpr_a, fbpr_sz,
> > +                               IOMMU_READ | IOMMU_WRITE | IOMMU_CACHE);
> > +               if (ret)
> > +                       dev_warn(dev, "failed to iommu_map() %d\n",
> ret);
> > +       }
> 
> Like Robin has pointed out, could you explain why the mapping in this
> patch and other similar patches cannot be dealt with the dma APIs
> automatically?  If the current bqman driver doesn't use the dma APIs
> correctly, we need to fix that instead of doing the mapping
> explicitly.

Please see my reply to Robin.
As a side comment, if we want to convert to dma api it will be interesting to see how we'll deal with the qman portal devices as they require a smmu mapping for an area of their register block (where stashing goes). Problem is they treated as distinct devices, the address where they should dma cannot be configured independently for each one of them. I think I can came up with a proposal but probably will not look very pretty.

---
Best Regards, Laurentiu

> > +
> >         bm_set_memory(fbpr_a, fbpr_sz);
> >
> >         err_irq = platform_get_irq(pdev, 0);
> > --
> > 2.17.1
> >