Hi Robin, Em Tue, 18 Aug 2020 15:47:55 +0100 Robin Murphy <robin.murphy@xxxxxxx> escreveu: > On 2020-08-17 08:49, Mauro Carvalho Chehab wrote: > > Add a driver for the Kirin 960/970 iommu. > > > > As on the past series, this starts from the original 4.9 driver from > > the 96boards tree: > > > > https://github.com/96boards-hikey/linux/tree/hikey970-v4.9 > > > > The remaining patches add SPDX headers and make it build and run with > > the upstream Kernel. > > > > Chenfeng (1): > > iommu: add support for HiSilicon Kirin 960/970 iommu > > > > Mauro Carvalho Chehab (15): > > iommu: hisilicon: remove default iommu_map_sg handler > > iommu: hisilicon: map and unmap ops gained new arguments > > iommu: hisi_smmu_lpae: rebase it to work with upstream > > iommu: hisi_smmu: remove linux/hisi/hisi-iommu.h > > iommu: hisilicon: cleanup its code style > > iommu: hisi_smmu_lpae: get rid of IOMMU_SEC and IOMMU_DEVICE > > iommu: get rid of map/unmap tile functions > > iommu: hisi_smmu_lpae: use the right code to get domain-priv data > > iommu: hisi_smmu_lpae: convert it to probe_device > > iommu: add Hisilicon Kirin970 iommu at the building system > > iommu: hisi_smmu_lpae: cleanup printk macros > > iommu: hisi_smmu_lpae: make OF compatible more standard > > Echoing the other comments about none of the driver patches being CC'd > to the IOMMU list... > > Still, I dug the series up on lore and frankly I'm not sure what to make > of it - AFAICS the "driver" is just yet another implementation of Arm > LPAE pagetable code, with no obvious indication of how those pagetables > ever get handed off to IOMMU hardware (and indeed no indication of IOMMU > hardware at all). Can you explain how it's supposed to work? > > And as a pre-emptive strike, we really don't need any more LPAE > implementations - that's what the io-pgtable library is all about (which > incidentally has been around since 4.0...). I think that should make the > issue of preserving authorship largely moot since there's no need to > preserve most of the code anyway ;) I didn't know about that, since I got a Hikey 970 board for the first time about one month ago, and that's the first time I looked into iommu code. My end goal with this is to make the DRM/KMS driver to work with upstream Kernels. The full patch series are at: https://github.com/mchehab/linux/commits/hikey970/to_upstream-2.0-v1.1 (I need to put a new version there, after some changes due to recent upstream discussions at the regulator's part of the code) Basically, the DT binding has this, for IOMMU: smmu_lpae { compatible = "hisilicon,smmu-lpae"; }; ... dpe: dpe@e8600000 { 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. Thanks, Mauro
