linux-kernel - Re: [PATCH 00/16] IOMMU driver for Kirin 960/970

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, 18 Aug 2020 15:02:54 -0700
From:   John Stultz <john.stultz@...aro.org>
To:     Robin Murphy <robin.murphy@....com>
Cc:     Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        driverdevel <devel@...verdev.osuosl.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, Joerg Roedel <jroedel@...e.de>,
        Manivannan Sadhasivam <mani@...nel.org>,
        Chenfeng <puck.chen@...ilicon.com>, linuxarm@...wei.com,
        Wei Xu <xuwei5@...ilicon.com>,
        lkml <linux-kernel@...r.kernel.org>,
        iommu@...ts.linux-foundation.org, Rob Herring <robh+dt@...nel.org>,
        mauro.chehab@...wei.com, Suzhuangluan <suzhuangluan@...ilicon.com>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 00/16] IOMMU driver for Kirin 960/970

On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy <robin.murphy@....com> wrote:
> On 2020-08-18 16:29, Mauro Carvalho Chehab wrote:
> > Em Tue, 18 Aug 2020 15:47:55 +0100
> > Basically, the DT binding has this, for IOMMU:
> >
> >
> >       smmu_lpae {
> >               compatible = "hisilicon,smmu-lpae";
> >       };
> >
> > ...
> >       dpe: dpe@...00000 {
> >               compatible = "hisilicon,kirin970-dpe";
> >               memory-region = <&drm_dma_reserved>;
> > ...
> >               iommu_info {
> >                       start-addr = <0x8000>;
> >                       size = <0xbfff8000>;
> >               };
> >       }
> >
> > This is used by kirin9xx_drm_dss.c in order to enable and use
> > the iommu:
> >
> >
> >       static int dss_enable_iommu(struct platform_device *pdev, struct dss_hw_ctx *ctx)
> >       {
> >               struct device *dev = NULL;
> >
> >               dev = &pdev->dev;
> >
> >               /* create iommu domain */
> >               ctx->mmu_domain = iommu_domain_alloc(dev->bus);
> >               if (!ctx->mmu_domain) {
> >                       pr_err("iommu_domain_alloc failed!\n");
> >                       return -EINVAL;
> >               }
> >
> >               iommu_attach_device(ctx->mmu_domain, dev);
> >
> >               return 0;
> >       }
> >
> > The only place where the IOMMU domain is used is on this part of the
> > code(error part simplified here) [1]:
> >
> >       void hisi_dss_smmu_on(struct dss_hw_ctx *ctx)
> >       {
> >               uint64_t fama_phy_pgd_base;
> >               uint32_t phy_pgd_base;
> > ...
> >               fama_phy_pgd_base = iommu_iova_to_phys(ctx->mmu_domain, 0);
> >               phy_pgd_base = (uint32_t)fama_phy_pgd_base;
> >               if (WARN_ON(!phy_pgd_base))
> >                       return;
> >
> >               set_reg(smmu_base + SMMU_CB_TTBR0, phy_pgd_base, 32, 0);
> >       }
> >
> > [1] https://github.com/mchehab/linux/commit/36da105e719b47bbe9d6cb7e5619b30c7f3eb1bd
> >
> > In other words, the driver needs to get the physical address of the frame
> > buffer (mapped via iommu) in order to set some DRM-specific register.
> >
> > Yeah, the above code is somewhat hackish. I would love to replace
> > this part by a more standard approach.
>
> OK, so from a quick look at that, my impression is that your display
> controller has its own MMU and you don't need to pretend to use the
> IOMMU API at all. Just have the DRM driver use io-pgtable directly to
> run its own set of ARM_32_LPAE_S1 pagetables - see Panfrost for an
> example (but try to ignore the wacky "Mali LPAE" format).

Yea. For the HiKey960, there was originally a similar patch series but
it was refactored out and the (still out of tree) DRM driver I'm
carrying doesn't seem to need it (though looking we still have the
iommu_info subnode in the dts that maybe needs to be cleaned up).

thanks
-john