IOMMU driver for Kirin 960/970
Subsystems (4)
cover IOMMU driver for Kirin 960/970
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
dt: add an spec for the Kirin36x0 SMMU
dt: hi3670-hikey970.dts: load the SMMU driver on Hikey970
staging: hikey9xx: add an item about the iommu driver
.../iommu/hisilicon,kirin36x0-smmu.yaml | 55 ++ .../boot/dts/hisilicon/hi3670-hikey970.dts | 3 + drivers/staging/hikey9xx/Kconfig | 9 + drivers/staging/hikey9xx/Makefile | 1 + drivers/staging/hikey9xx/TODO | 1 + drivers/staging/hikey9xx/hisi_smmu.h | 196 ++++++ drivers/staging/hikey9xx/hisi_smmu_lpae.c | 648 ++++++++++++++++++ 7 files changed, 913 insertions(+) create mode 100644 Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml create mode 100644 drivers/staging/hikey9xx/hisi_smmu.h create mode 100644 drivers/staging/hikey9xx/hisi_smmu_lpae.c
Signature · 1 line
2.26.2
PATCH 1/16 iommu: add support for HiSilicon Kirin 960/970 iommu
From: Chenfeng <[email protected]>
Add the IOMMU code used on Hikey 960/970 and required for its DRM/KMS driver.
[[email protected]: split out all the ion changes, and kept just the iommu bits] [[email protected]: dropped ION and test code]
--- drivers/staging/hikey9xx/hisi_smmu.h | 178 +++++ drivers/staging/hikey9xx/hisi_smmu_lpae.c | 849 ++++++++++++++++++++++ include/linux/hisi/hisi-iommu.h | 13 + 3 files changed, 1040 insertions(+) create mode 100644 drivers/staging/hikey9xx/hisi_smmu.h create mode 100644 drivers/staging/hikey9xx/hisi_smmu_lpae.c create mode 100644 include/linux/hisi/hisi-iommu.h
drivers/staging/hikey9xx/hisi_smmu.h new file +178 -0
@@ -0,0 +1,178 @@ +#ifndef HISI_SMMU_H +#define HISI_SMMU_H + +/*#define IOMMU_DEBUG*/ +#ifdef IOMMU_DEBUG +#define dbg(format, arg...) printk(KERN_ERR "[iommu]"format, ##arg); +#else +#define dbg(format, arg...) +#endif + +#define SMMU_PHY_PTRS_PER_PTE (256) +/*#define SMMU_PHY_PTRS_PER_PGD (4096)*/ +#define SMMU_PTRS_PER_PGD (4) +#define SMMU_PTRS_PER_PMD (512) +#define SMMU_PTRS_PER_PTE (512) +#define SMMU_PAGE_SHIFT (12) + +#define PAGE_TABLE_ADDR_MASK (UL(0xFFFFFFF) << SMMU_PAGE_SHIFT) + +#define SMMU_PAGE_SIZE BIT(SMMU_PAGE_SHIFT) +#define SMMU_PAGE_MASK (~(SMMU_PAGE_SIZE-1)) + +#define SMMU_PGDIR_SHIFT (30) +#define SMMU_PGDIR_SIZE BIT(SMMU_PGDIR_SHIFT) +#define SMMU_PGDIR_MASK (~(SMMU_PGDIR_SIZE-1)) + +#define SMMU_PMDIR_SHIFT (21) +#define SMMU_PMDIR_SIZE BIT(SMMU_PMDIR_SHIFT) +#define SMMU_PMDIR_MASK (~(SMMU_PMDIR_SIZE-1)) +#define SMMU_PGD_TYPE (BIT(0) | BIT(1)) +#define SMMU_PMD_TYPE (BIT(0) | BIT(1)) +#define SMMU_PTE_TYPE (BIT(0) | BIT(1)) + +#define SMMU_PGD_NS BIT(63) +#define SMMU_PMD_NS BIT(63) +#define SMMU_PTE_NS BIT(5) + +#define SMMU_PTE_PXN BIT(53)/* Privileged XN */ +#define SMMU_PTE_UXN BIT(54)/* User XN */ +#define SMMU_PTE_USER BIT(6)/* AP[1] */ +#define SMMU_PTE_RDONLY BIT(7)/* AP[2] */ +#define SMMU_PTE_SHARED (BIT(8) | BIT(9))/* SH[1:0], inner shareable */ +#define SMMU_PTE_AF BIT(10)/* Access Flag */ +#define SMMU_PTE_NG BIT(11)/* nG */ +#define SMMU_PTE_ATTRINDX (t) ((t) << 2) +/* +* Memory types available. +* USED BY A7 +*/ +#define HISI_MT_NORMAL 0 +#define HISI_MT_NORMAL_CACHE 4 +#define HISI_MT_NORMAL_NC 5 +#define HISI_MT_DEVICE_nGnRE 6 + + +#define SMMU_PAGE_DEFAULT (SMMU_PTE_TYPE | SMMU_PTE_AF | SMMU_PTE_SHARED) + +#define SMMU_PROT_DEVICE_nGnRE (SMMU_PAGE_DEFAULT | SMMU_PTE_PXN | \ + SMMU_PTE_UXN | SMMU_PTE_ATTRINDX(HISI_MT_DEVICE_nGnRE)) +#define SMMU_PROT_NORMAL_CACHE (SMMU_PAGE_DEFAULT | SMMU_PTE_PXN | \ + SMMU_PTE_UXN | SMMU_PTE_ATTRINDX(HISI_MT_NORMAL_CACHE)) +#define SMMU_PROT_NORMAL_NC (SMMU_PAGE_DEFAULT | SMMU_PTE_PXN | \ + SMMU_PTE_UXN | SMMU_PTE_ATTRINDX(HISI_MT_NORMAL_NC)) +#define SMMU_PROT_NORMAL (SMMU_PAGE_DEFAULT | SMMU_PTE_PXN | \ + SMMU_PTE_UXN | SMMU_PTE_ATTRINDX(HISI_MT_NORMAL)) + +#define SMMU_PAGE_READWRITE (SMMU_PAGE_DEFAULT | SMMU_PTE_USER | \ + SMMU_PTE_NG | SMMU_PTE_PXN | SMMU_PTE_UXN) +#define SMMU_PAGE_READONLY (SMMU_PAGE_DEFAULT | SMMU_PTE_USER | \ + SMMU_PTE_RDONLY | SMMU_PTE_NG | SMMU_PTE_PXN | SMMU_PTE_UXN) +#define SMMU_PAGE_READONLY_EXEC (SMMU_PAGE_DEFAULT | SMMU_PTE_USER | \ + SMMU_PTE_NG) + +#define smmu_pte_index (addr) (((addr) >> SMMU_PAGE_SHIFT) & (SMMU_PTRS_PER_PTE - 1)) +#define smmu_pmd_index (addr) (((addr) >> SMMU_PMDIR_SHIFT) & (SMMU_PTRS_PER_PMD - 1)) +#define smmu_pgd_index (addr) (((addr) >> SMMU_PGDIR_SHIFT) & (SMMU_PTRS_PER_PGD - 1)) +#define SMMU_PAGE_ALIGN (addr) ALIGN(addr, PAGE_SIZE) + +typedef u64 smmu_pgd_t ; +typedef u64 smmu_pmd_t ; +typedef u64 smmu_pte_t ; + +/*smmu device object*/ +struct hisi_smmu_device_lpae { +struct device *dev ; +struct list_head domain_list; +unsigned int ref_count; +spinlock_t lock; +unsigned long va_pgtable_addr; +phys_addr_t smmu_phy_pgtable_addr; +smmu_pgd_t *smmu_pgd; +}; + +struct hisi_map_tile_position_lpae { +struct scatterlist *sg ; +unsigned long offset; +}; + +extern struct hisi_smmu_device_lpae *hisi_smmu_dev; + +static inline unsigned int smmu_pgd_none_lpae (smmu_pgd_t pgd) { +return !(pgd ? pgd :0 ); +} + +static inline unsigned int smmu_pmd_none_lpae (smmu_pmd_t pmd) { +return !(pmd ? pmd :0 ); +} + +static inline unsigned int smmu_pte_none_lpae (smmu_pte_t pte) { +return !(pte ? pte :0 ); +} + +static inline unsigned int pte_is_valid_lpae (smmu_pte_t *ptep) { +return (unsigned int )((*(ptep)&SMMU_PTE_TYPE ) ?1 :0 ); +} + +/* Find an entry in the second-level page table.. */ +static inline void *smmu_pmd_page_vaddr_lpae (smmu_pmd_t *pgd) +{ +return phys_to_virt (*pgd &PAGE_TABLE_ADDR_MASK ); +} + +/* Find an entry in the third-level page table.. */ +static inline void *smmu_pte_page_vaddr_lpae (smmu_pmd_t *pmd) +{ +return phys_to_virt (*pmd &PAGE_TABLE_ADDR_MASK ); +} + + +/*fill the pgd entry, pgd value must be 64bit */ +static inline void smmu_set_pgd_lpae (smmu_pgd_t *pgdp,u64 pgd) +{ + *pgdp = pgd; +dsb (ishst); +isb (); +} + +/*fill the pmd entry, pgd value must be 64bit */ +static inline void smmu_set_pmd_lpae (smmu_pgd_t *pmdp,u64 pmd) +{ +dbg ("smmu_set_pmd_lpae: pmd = 0x%lx \n" , pmd); + *pmdp = pmd; +dsb (ishst); +isb (); +} + +static inline void smmu_pmd_populate_lpae (smmu_pmd_t *pmdp,pgtable_t ptep,pgdval_t prot) +{ +smmu_set_pmd_lpae (pmdp, (u64)(page_to_phys (ptep) | prot)); +} + +static inline void smmu_pgd_populate_lpae (smmu_pgd_t *pgdp,pgtable_t pmdp,pgdval_t prot) +{ +smmu_set_pgd_lpae (pgdp, (u64)(page_to_phys (pmdp) | prot)); +} + +static inline unsigned long smmu_pgd_addr_end_lpae (unsigned long addr,unsigned long end) +{ +unsigned long boundary = (addr +SMMU_PGDIR_SIZE ) &SMMU_PGDIR_MASK ; + +return (boundary -1 < end -1 ) ? boundary : end; +} + +static inline unsigned long smmu_pmd_addr_end_lpae (unsigned long addr,unsigned long end) +{ +unsigned long boundary = (addr +SMMU_PMDIR_SIZE ) &SMMU_PMDIR_MASK ; + +return (boundary -1 < end -1 ) ? boundary : end; +} + +int hisi_smmu_handle_mapping_lpae (struct iommu_domain *domain, +unsigned long iova,phys_addr_t paddr, +size_t size,int prot); + +unsigned int hisi_smmu_handle_unmapping_lpae (struct iommu_domain *domain, +unsigned long iova,size_t size); + +#endif
drivers/staging/hikey9xx/hisi_smmu_lpae.c new file +849 -0
849 lines not on this page · raw message
include/linux/hisi/hisi-iommu.h new file +13 -0
@@ -0,0 +1,13 @@ +#ifndef _HI36XX_SMMU_H +#define _HI36XX_SMMU_H + +#include <linux/types.h> +struct iommu_domain_data { +unsigned int iova_start; +unsigned int iova_size; +phys_addr_t phy_pgd_base; +unsigned long iova_align; +struct list_head list; +}; + +#endif
Signature · 1 line
2.26.2
PATCH 2/16 iommu: hisilicon: remove default iommu_map_sg handler
In reply to Mauro Carvalho ChehabThe code there is just a copy of the default iommu sg handler. Well, callback fops was removed, as all iommu drivers are doing the same.
--- drivers/staging/hikey9xx/hisi_smmu_lpae.c | 42 ----------------------- 1 file changed, 42 deletions(-)
drivers/staging/hikey9xx/hisi_smmu_lpae.c +0 -42
@@ -720,48 +720,7 @@ static size_t hisi_smmu_unmap_tile_lpae(struct iommu_domain *domain, unsigned long iova, size_t size) {return hisi_smmu_unmap_lpae (domain, iova, size); -} -size_t hisi_iommu_map_sg_lpae (struct iommu_domain *domain,unsigned long iova, -struct scatterlist *sg,unsigned int nents,int prot) -{ -struct scatterlist *s; -size_t mapped =0 ; -unsigned int i, min_pagesz; -int ret; - -if (domain->ops->pgsize_bitmap ==0UL ) -return 0 ; - - min_pagesz = (unsigned int )1 <<__ffs (domain->ops->pgsize_bitmap); - -for_each_sg (sg, s, nents, i) { -phys_addr_t phys =page_to_phys (sg_page (s)) + s->offset; - -/* -* We are mapping on IOMMU page boundaries, so offset within -* the page must be 0. However, the IOMMU may support pages -* smaller than PAGE_SIZE, so s->offset may still represent -* an offset of that boundary within the CPU page. -*/ -if (!IS_ALIGNED (s->offset, min_pagesz)) - goto out_err; - - ret =hisi_smmu_map_lpae (domain, iova + mapped, phys, - (size_t )s->length, prot); -if (ret) - goto out_err; - - mapped += s->length; - } - -return mapped; - -out_err: -/* undo mappings already done */ -hisi_smmu_unmap_lpae (domain, iova, mapped); - -return 0 ; }static struct iommu_ops hisi_smmu_ops = {
@@ -769,7 +728,6 @@ static struct iommu_ops hisi_smmu_ops = { .domain_free = hisi_smmu_domain_free_lpae, .map = hisi_smmu_map_lpae, .unmap = hisi_smmu_unmap_lpae, - .map_sg = hisi_iommu_map_sg_lpae, .attach_dev = hisi_attach_dev_lpae, .detach_dev = hisi_detach_dev_lpae, .iova_to_phys = hisi_smmu_iova_to_phys_lpae,
Signature · 1 line
2.26.2
PATCH 3/16 iommu: hisilicon: map and unmap ops gained new arguments
In reply to Mauro Carvalho ChehabAs both map() and unmap() ops gained new arguments upstream, update their headers accordingly.
--- drivers/staging/hikey9xx/hisi_smmu_lpae.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
drivers/staging/hikey9xx/hisi_smmu_lpae.c +6 -5
@@ -346,7 +346,8 @@ int hisi_smmu_handle_mapping_lpae(struct iommu_domain *domain,static int hisi_smmu_map_lpae (struct iommu_domain *domain,unsigned long iova,phys_addr_t paddr,size_t size, -int prot) +int prot, +gfp_t gfp) {unsigned long max_iova;struct iommu_domain_data *data;
@@ -437,7 +438,8 @@ unsigned int hisi_smmu_handle_unmapping_lpae(struct iommu_domain *domain, }static size_t hisi_smmu_unmap_lpae (struct iommu_domain *domain, -unsigned long iova,size_t size) +unsigned long iova,size_t size, +struct iommu_iotlb_gather *iotlb_gather) {unsigned long max_iova;unsigned int ret;
@@ -593,7 +595,7 @@ static size_t hisi_map_tile_row_lpae(struct iommu_domain *domain, unsigned long/*get the start physical address*/ phys_addr = (unsigned long )get_phys_addr_lpae (sg) + sg_offset; ret =hisi_smmu_map_lpae (domain, - iova + mapped_size, phys_addr, map_size, prot); + iova + mapped_size, phys_addr, map_size, prot,GFP_KERNEL );if (ret) {dbg ("[%s] hisi_smmu_map failed!\n" , __func__);break ;
@@ -719,8 +721,7 @@ static int hisi_smmu_map_tile_lpae(struct iommu_domain *domain,static size_t hisi_smmu_unmap_tile_lpae (struct iommu_domain *domain,unsigned long iova,size_t size) { -return hisi_smmu_unmap_lpae (domain, iova, size); - +return hisi_smmu_unmap_lpae (domain, iova, size,NULL ); }static struct iommu_ops hisi_smmu_ops = {
Signature · 1 line
2.26.2
PATCH 4/16 iommu: hisi_smmu_lpae: rebase it to work with upstream
In reply to Mauro Carvalho ChehabCurrently, this driver OOPSes. It turns that this driver needs to be updated in order to work with the current iommu kAPI.
--- drivers/staging/hikey9xx/hisi_smmu.h | 2 + drivers/staging/hikey9xx/hisi_smmu_lpae.c | 81 ++++++++++++++++++++--- 2 files changed, 74 insertions(+), 9 deletions(-)
drivers/staging/hikey9xx/hisi_smmu.h +2 -0
@@ -89,6 +89,8 @@ struct hisi_smmu_device_lpae {unsigned long va_pgtable_addr;phys_addr_t smmu_phy_pgtable_addr;smmu_pgd_t *smmu_pgd; + +struct iommu_device iommu; };struct hisi_map_tile_position_lpae {
drivers/staging/hikey9xx/hisi_smmu_lpae.c +72 -9
127 lines not on this page · raw message
Signature · 1 line
2.26.2
PATCH 5/16 iommu: hisi_smmu: remove linux/hisi/hisi-iommu.h
In reply to Mauro Carvalho ChehabJust move its contents into drivers/iommu/hisi_smmu.h.
--- drivers/staging/hikey9xx/hisi_smmu.h | 8 ++++++++ drivers/staging/hikey9xx/hisi_smmu_lpae.c | 1 - include/linux/hisi/hisi-iommu.h | 13 ------------- 3 files changed, 8 insertions(+), 14 deletions(-) delete mode 100644 include/linux/hisi/hisi-iommu.h
drivers/staging/hikey9xx/hisi_smmu.h +8 -0
@@ -93,6 +93,14 @@ struct hisi_smmu_device_lpae {struct iommu_device iommu; }; +struct iommu_domain_data { +unsigned int iova_start; +unsigned int iova_size; +phys_addr_t phy_pgd_base; +unsigned long iova_align; +struct list_head list; +}; +struct hisi_map_tile_position_lpae {struct scatterlist *sg ;unsigned long offset;
drivers/staging/hikey9xx/hisi_smmu_lpae.c +0 -1
@@ -24,7 +24,6 @@#include <linux/spinlock.h> #include <asm/pgalloc.h> #include <linux/debugfs.h> -#include <linux/hisi/hisi-iommu.h> #include <linux/uaccess.h> #include <linux/bitops.h> #include "hisi_smmu.h"
include/linux/hisi/hisi-iommu.h deleted +0 -13
@@ -1,13 +0,0 @@ -#ifndef _HI36XX_SMMU_H -#define _HI36XX_SMMU_H - -#include <linux/types.h> -struct iommu_domain_data { -unsigned int iova_start; -unsigned int iova_size; -phys_addr_t phy_pgd_base; -unsigned long iova_align; -struct list_head list; -}; - -#endif
Signature · 1 line
2.26.2
PATCH 6/16 iommu: hisilicon: cleanup its code style
In reply to Mauro Carvalho ChehabFix most of the things complained by checkpatch on strict mode:
- Replaced BUG_ON to WARN_ON;
- added SPDX headers;
- adjusted alignments;
- used --fix-inplace to solve other minor issues.
--- drivers/staging/hikey9xx/hisi_smmu.h | 40 ++-- drivers/staging/hikey9xx/hisi_smmu_lpae.c | 243 +++++++++++----------- 2 files changed, 143 insertions(+), 140 deletions(-)
drivers/staging/hikey9xx/hisi_smmu.h +22 -18
@@ -1,9 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef HISI_SMMU_H #define HISI_SMMU_H /*#define IOMMU_DEBUG*/ #ifdef IOMMU_DEBUG -#define dbg(format, arg...) printk(KERN_ERR "[iommu]"format, ##arg); +#define dbg(format, arg...) printk(KERN_ERR "[iommu]" format, ##arg)#else #define dbg(format, arg...)#endif
@@ -18,15 +20,15 @@#define PAGE_TABLE_ADDR_MASK (UL(0xFFFFFFF) << SMMU_PAGE_SHIFT)#define SMMU_PAGE_SIZE BIT(SMMU_PAGE_SHIFT) -#define SMMU_PAGE_MASK (~(SMMU_PAGE_SIZE-1)) +#define SMMU_PAGE_MASK (~(SMMU_PAGE_SIZE - 1))#define SMMU_PGDIR_SHIFT (30)#define SMMU_PGDIR_SIZE BIT(SMMU_PGDIR_SHIFT) -#define SMMU_PGDIR_MASK (~(SMMU_PGDIR_SIZE-1)) +#define SMMU_PGDIR_MASK (~(SMMU_PGDIR_SIZE - 1))#define SMMU_PMDIR_SHIFT (21)#define SMMU_PMDIR_SIZE BIT(SMMU_PMDIR_SHIFT) -#define SMMU_PMDIR_MASK (~(SMMU_PMDIR_SIZE-1)) +#define SMMU_PMDIR_MASK (~(SMMU_PMDIR_SIZE - 1))#define SMMU_PGD_TYPE (BIT(0) | BIT(1))#define SMMU_PMD_TYPE (BIT(0) | BIT(1))#define SMMU_PTE_TYPE (BIT(0) | BIT(1))
@@ -41,7 +43,7 @@#define SMMU_PTE_RDONLY BIT(7)/* AP[2] */ #define SMMU_PTE_SHARED (BIT(8) | BIT(9))/* SH[1:0], inner shareable */ #define SMMU_PTE_AF BIT(10)/* Access Flag */ -#define SMMU_PTE_NG BIT(11)/* nG */ +#define SMMU_PTE_NG BIT(11)/* nG */ #define SMMU_PTE_ATTRINDX (t) ((t) << 2)/* * Memory types available.
@@ -52,7 +54,6 @@#define HISI_MT_NORMAL_NC 5#define HISI_MT_DEVICE_nGnRE 6 -#define SMMU_PAGE_DEFAULT (SMMU_PTE_TYPE | SMMU_PTE_AF | SMMU_PTE_SHARED)#define SMMU_PROT_DEVICE_nGnRE (SMMU_PAGE_DEFAULT | SMMU_PTE_PXN | \
@@ -82,7 +83,7 @@ typedef u64 smmu_pte_t;/*smmu device object*/ struct hisi_smmu_device_lpae { -struct device *dev ; +struct device *dev;struct list_head domain_list;unsigned int ref_count;spinlock_t lock;
@@ -102,26 +103,30 @@ struct iommu_domain_data { };struct hisi_map_tile_position_lpae { -struct scatterlist *sg ; +struct scatterlist *sg;unsigned long offset; };extern struct hisi_smmu_device_lpae *hisi_smmu_dev; -static inline unsigned int smmu_pgd_none_lpae (smmu_pgd_t pgd) { +static inline unsigned int smmu_pgd_none_lpae (smmu_pgd_t pgd) +{return !(pgd ? pgd :0 ); } -static inline unsigned int smmu_pmd_none_lpae (smmu_pmd_t pmd) { +static inline unsigned int smmu_pmd_none_lpae (smmu_pmd_t pmd) +{return !(pmd ? pmd :0 ); } -static inline unsigned int smmu_pte_none_lpae (smmu_pte_t pte) { +static inline unsigned int smmu_pte_none_lpae (smmu_pte_t pte) +{return !(pte ? pte :0 ); } -static inline unsigned int pte_is_valid_lpae (smmu_pte_t *ptep) { -return (unsigned int )((*(ptep)&SMMU_PTE_TYPE ) ?1 :0 ); +static inline unsigned int pte_is_valid_lpae (smmu_pte_t *ptep) +{ +return (unsigned int )((*(ptep) &SMMU_PTE_TYPE ) ?1 :0 ); }/* Find an entry in the second-level page table.. */
@@ -136,7 +141,6 @@ static inline void *smmu_pte_page_vaddr_lpae(smmu_pmd_t *pmd)return phys_to_virt (*pmd &PAGE_TABLE_ADDR_MASK ); } -/*fill the pgd entry, pgd value must be 64bit */ static inline void smmu_set_pgd_lpae (smmu_pgd_t *pgdp,u64 pgd) {
@@ -148,7 +152,7 @@ static inline void smmu_set_pgd_lpae(smmu_pgd_t *pgdp, u64 pgd)/*fill the pmd entry, pgd value must be 64bit */ static inline void smmu_set_pmd_lpae (smmu_pgd_t *pmdp,u64 pmd) { -dbg ("smmu_set_pmd_lpae: pmd = 0x%lx \n" , pmd); +dbg ("smmu_set_pmd_lpae: pmd = 0x%lx\n" , pmd); *pmdp = pmd;dsb (ishst);isb ();
@@ -179,10 +183,10 @@ static inline unsigned long smmu_pmd_addr_end_lpae(unsigned long addr, unsigned }int hisi_smmu_handle_mapping_lpae (struct iommu_domain *domain, -unsigned long iova,phys_addr_t paddr, -size_t size,int prot); +unsigned long iova,phys_addr_t paddr, +size_t size,int prot);unsigned int hisi_smmu_handle_unmapping_lpae (struct iommu_domain *domain, -unsigned long iova,size_t size); +unsigned long iova,size_t size);#endif
drivers/staging/hikey9xx/hisi_smmu_lpae.c +121 -122
524 lines not on this page · raw message
Signature · 1 line
2.26.2
PATCH 7/16 iommu: hisi_smmu_lpae: get rid of IOMMU_SEC and IOMMU_DEVICE
In reply to Mauro Carvalho ChehabThose prot bits aren't needed for Hikey970's GPU code, and depends on some patch not on upstream.
So, get rid of them.
--- drivers/staging/hikey9xx/hisi_smmu_lpae.c | 41 +++++++++-------------- 1 file changed, 15 insertions(+), 26 deletions(-)
drivers/staging/hikey9xx/hisi_smmu_lpae.c +15 -26
@@ -205,9 +205,6 @@ static int hisi_smmu_alloc_init_pte_lpae(smmu_pmd_t *ppmd, } pte_ready: -if (prot &IOMMU_SEC ) - *ppmd &= (~SMMU_PMD_NS ); - start = (smmu_pte_t *)smmu_pte_page_vaddr_lpae (ppmd) +smmu_pte_index (addr); pte = start;
@@ -215,30 +212,24 @@ static int hisi_smmu_alloc_init_pte_lpae(smmu_pmd_t *ppmd, pteval |=SMMU_PROT_NORMAL ; pteval |=SMMU_PTE_NS ; }else { -if (prot &IOMMU_DEVICE ) { - pteval |= SMMU_PROT_DEVICE_nGnRE; - }else { -if (prot &IOMMU_CACHE ) - pteval |=SMMU_PROT_NORMAL_CACHE ; -else - pteval |=SMMU_PROT_NORMAL_NC ; +if (prot &IOMMU_CACHE ) + pteval |=SMMU_PROT_NORMAL_CACHE ; +else + pteval |=SMMU_PROT_NORMAL_NC ; -if ((prot &IOMMU_READ ) && (prot &IOMMU_WRITE )) - pteval |=SMMU_PAGE_READWRITE ; -else if ((prot &IOMMU_READ ) && !(prot &IOMMU_WRITE )) - pteval |=SMMU_PAGE_READONLY ; -else -WARN_ON ("you do not set read attribute!" ); +if ((prot &IOMMU_READ ) && (prot &IOMMU_WRITE )) + pteval |=SMMU_PAGE_READWRITE ; +else if ((prot &IOMMU_READ ) && !(prot &IOMMU_WRITE )) + pteval |=SMMU_PAGE_READONLY ; +else +WARN_ON ("you do not set read attribute!" ); -if (prot &IOMMU_EXEC ) { - pteval |=SMMU_PAGE_READONLY_EXEC ; - pteval &= ~(SMMU_PTE_PXN |SMMU_PTE_UXN ); - } +if (!(prot &IOMMU_NOEXEC )) { + pteval |=SMMU_PAGE_READONLY_EXEC ; + pteval &= ~(SMMU_PTE_PXN |SMMU_PTE_UXN ); } -if (prot &IOMMU_SEC ) - pteval &= (~SMMU_PTE_NS ); -else - pteval |=SMMU_PTE_NS ; + + pteval |=SMMU_PTE_NS ; } do {
@@ -287,8 +278,6 @@ static int hisi_smmu_alloc_init_pmd_lpae(smmu_pgd_t *ppgd, } pmd_ready: -if (prot &IOMMU_SEC ) - *ppgd &= (~SMMU_PGD_NS ); start = (smmu_pmd_t *)smmu_pmd_page_vaddr_lpae (ppgd) +smmu_pmd_index (addr); ppmd = start;
Signature · 1 line
2.26.2
PATCH 8/16 iommu: get rid of map/unmap tile functions
In reply to Mauro Carvalho ChehabThose are needed by the ION-specific downstream code.
Such code would require changes at the core iommu header file. As we won't be using the ION-specific binding, we can just get rid of those.
--- drivers/staging/hikey9xx/hisi_smmu_lpae.c | 201 ---------------------- 1 file changed, 201 deletions(-)
drivers/staging/hikey9xx/hisi_smmu_lpae.c +0 -201
@@ -518,205 +518,6 @@ static void hisi_detach_dev_lpae(struct iommu_domain *domain, } } -static dma_addr_t get_phys_addr_lpae (struct scatterlist *sg) -{ -dma_addr_t dma_addr =sg_dma_address (sg); - -if (!dma_addr) - dma_addr =sg_phys (sg); -return dma_addr; -} - -int iommu_map_tile (struct iommu_domain *domain,unsigned long iova, -struct scatterlist *sg,size_t size,int prot, -struct tile_format *format) -{ -if (unlikely (!(domain->ops->map_tile))) -return -ENODEV ; - -if (WARN_ON (iova & (~PAGE_MASK ))) -return -EINVAL ; - -return domain->ops->map_tile (domain, iova, sg, size, prot, format); -} - -int iommu_unmap_tile (struct iommu_domain *domain,unsigned long iova, -size_t size) -{ -if (unlikely (!(domain->ops->unmap_tile))) -return -ENODEV ; - -if (WARN_ON (iova & (~PAGE_MASK ))) -return -EINVAL ; - -return domain->ops->unmap_tile (domain, iova, size); -} - -/* -* iova: the start address for tile mapping -* size: the physical memory size -* sg: the node of scatter list where are the start node of physical memory -* sg_offset: the physical memory offset in the sg node ,where is the start -* position of physical memory -* prot: the pape property of virtual memory -* -* this function complete one row mapping. -*/ -static size_t -hisi_map_tile_row_lpae (struct iommu_domain *domain, -unsigned long iova,size_t size,struct scatterlist *sg, -size_t sg_offset, -struct hisi_map_tile_position_lpae *map_position, -unsigned int prot) -{ -unsigned long map_size;/*the memory size that will be mapped*/ -unsigned long phys_addr; -unsigned long mapped_size =0 ;/*memory size that has been mapped*/ -int ret; - -while (1 ) { -/* -*get the remain memory,if current sg node is not enough memory, -*we map the remain memory firstly. -*/ - map_size = size - mapped_size; -if (map_size > (sg->length - sg_offset)) - map_size = (sg->length - sg_offset); - -/* get the start physical address */ - phys_addr = (unsigned long )get_phys_addr_lpae (sg) + sg_offset; - ret =hisi_smmu_map_lpae (domain, - iova + mapped_size, phys_addr, - map_size, prot,GFP_KERNEL ); -if (ret) { -dbg ("[%s] hisi_smmu_map failed!\n" , __func__); -break ; - } -/* update mapped memory size */ - mapped_size += map_size; -/* -* if finished mapping, -* we update the memory offset of current node and -* save the memory position. otherwise we clean the sg_offset -* to zero and get next sg node. -*/ -if (mapped_size < size) { - sg_offset =0 ; - sg =sg_next (sg); -if (!sg) { -dbg ("[%s] phy memory not enough\n" , __func__); -break ; - } - }else { - sg_offset += map_size; -/* if physcial memory of this node is exhausted, -* we choose next node -*/ -if (sg_offset == sg->length) { - sg_offset =0 ; - sg =sg_next (sg); - } -break ; - } - } -/* save current position */ - map_position->sg = sg; - map_position->offset = sg_offset; - -return mapped_size; -} - -/* -* domain:the iommu domain for mapping -* iova:the start virtual address -* sg: the scatter list of physical memory -* size:the total size of all virtual memory -* port:the property of page table of virtual memory -* format:the parameter of tile mapping -* this function map physical memory in tile mode -*/ -static int hisi_smmu_map_tile_lpae (struct iommu_domain *domain, -unsigned long iova, -struct scatterlist *sg, -size_t size,int prot, -struct tile_format *format) -{ -unsigned int phys_length; -struct scatterlist *sg_node; -unsigned int row_number, row; -unsigned int size_virt, size_phys; -unsigned int sg_offset; -int ret = size; -unsigned int mapped_size, header_size; -struct hisi_map_tile_position_lpae map_position; - -/* calculate the whole length of phys mem */ -for (phys_length =0 , sg_node = sg; sg_node; sg_node =sg_next (sg_node)) - phys_length +=ALIGN (sg_node->length,PAGE_SIZE ); - - header_size = format->header_size; - -/* calculate the number of raws */ - row_number = ((phys_length - header_size) >>PAGE_SHIFT ) - / format->phys_page_line; -dbg ("phys_length: 0x%x, rows: 0x%x, header_size: 0x%x\n" , - phys_length, row_number, header_size); - -/* calculate the need physical memory and virtual memory for one row */ - size_phys = (format->phys_page_line *PAGE_SIZE ); - size_virt = (format->virt_page_line *PAGE_SIZE ); - - sg_offset =0 ; - sg_node = sg; - -/* set start position */ - map_position.sg = sg; - map_position.offset =0 ; - -/* map header */ -if (header_size) { - mapped_size =hisi_map_tile_row_lpae (domain, iova, - header_size, sg_node, - sg_offset, &map_position, - prot); -if (mapped_size != header_size) { -WARN (1 ,"map head fail\n" ); - ret = -EINVAL ; - goto error; - } - iova +=ALIGN (header_size, size_virt); - } -/* map row by row */ -for (row =0 ; row < row_number; row++) { -/* get physical memory position */ -if (map_position.sg) { - sg_node = map_position.sg; - sg_offset = map_position.offset; - }else { -dbg ("[%s]:physical memory is not enough\n" , __func__); -break ; - } -/* map one row*/ - mapped_size =hisi_map_tile_row_lpae (domain, - iova + (size_virt * row), - size_phys, sg_node, sg_offset, - &map_position, prot); -if (mapped_size != size_phys) { -WARN (1 ,"hisi_map_tile_row failed!\n" ); - ret = -EINVAL ; -break ; - } - }; -error: -return ret; -} - -static size_t hisi_smmu_unmap_tile_lpae (struct iommu_domain *domain, -unsigned long iova,size_t size) -{ -return hisi_smmu_unmap_lpae (domain, iova, size,NULL ); -} -static bool hisi_smmu_capable (enum iommu_cap cap) {return false;
@@ -764,8 +565,6 @@ static struct iommu_ops hisi_smmu_ops = { .remove_device = hisi_smmu_remove_device, .device_group = generic_device_group, .pgsize_bitmap = SMMU_PAGE_SIZE, - .map_tile = hisi_smmu_map_tile_lpae, - .unmap_tile = hisi_smmu_unmap_tile_lpae, };static int hisi_smmu_probe_lpae (struct platform_device *pdev)
Signature · 1 line
2.26.2
PATCH 9/16 iommu: hisi_smmu_lpae: use the right code to get domain-priv data
In reply to Mauro Carvalho ChehabThe downstream code needed to use a priv data within the domain struct. Change it to work like other iommu drivers:
use dev_iommu_priv_get() and dev_iommu_priv_set() instead.
--- drivers/staging/hikey9xx/hisi_smmu.h | 17 ++++++++-- drivers/staging/hikey9xx/hisi_smmu_lpae.c | 38 +++++++++++++---------- 2 files changed, 36 insertions(+), 19 deletions(-)
drivers/staging/hikey9xx/hisi_smmu.h +14 -3
@@ -94,7 +94,7 @@ struct hisi_smmu_device_lpae {struct iommu_device iommu; }; -struct iommu_domain_data { +struct hisi_smmu_domain_data {unsigned int iova_start;unsigned int iova_size;phys_addr_t phy_pgd_base;
@@ -102,13 +102,24 @@ struct iommu_domain_data {struct list_head list; }; +struct hisi_smmu_domain { +struct iommu_domain domain; +struct hisi_smmu_domain_data *iommu_info; +}; + +static struct hisi_smmu_domain_data *to_smmu (struct iommu_domain *dom) +{ +struct hisi_smmu_domain *hisi_dom; + + hisi_dom = container_of(dom, struct hisi_smmu_domain, domain); +return hisi_dom->iommu_info; +} +struct hisi_map_tile_position_lpae {struct scatterlist *sg;unsigned long offset; }; -extern struct hisi_smmu_device_lpae *hisi_smmu_dev; -static inline unsigned int smmu_pgd_none_lpae (smmu_pgd_t pgd) {return !(pgd ? pgd :0 );
drivers/staging/hikey9xx/hisi_smmu_lpae.c +22 -16
@@ -28,7 +28,7 @@#include <linux/bitops.h> #include "hisi_smmu.h" -struct hisi_smmu_device_lpae *hisi_smmu_dev; +static struct hisi_smmu_device_lpae *hisi_smmu_dev;/* transfer 64bit pte table pointer to struct page */ static pgtable_t smmu_pgd_to_pte_lpae (unsigned int ppte_table)
@@ -57,7 +57,7 @@ static pgtable_t smmu_pmd_to_pte_lpae(unsigned long ppte_table) }static int get_domain_data_lpae (struct device_node *np, -struct iommu_domain_data *data) +struct hisi_smmu_domain_data *data) {unsigned long long align;struct device_node *node =NULL ;
@@ -103,14 +103,16 @@ static int get_domain_data_lpae(struct device_node *np,static struct iommu_domain *hisi_smmu_domain_alloc_lpae (unsigned int iommu_domain_type) { -struct iommu_domain *domain; +struct hisi_smmu_domain *hisi_dom;if (iommu_domain_type !=IOMMU_DOMAIN_UNMANAGED )return NULL ; - domain =kzalloc (sizeof (*domain),GFP_KERNEL ); + hisi_dom =kzalloc (sizeof (*hisi_dom),GFP_KERNEL ); -return domain; +pr_debug ("%s: domain allocated\n" , __func__); + +return &hisi_dom->domain; }static void hisi_smmu_flush_pgtable_lpae (void *addr,size_t size)
@@ -336,13 +338,13 @@ static int hisi_smmu_map_lpae(struct iommu_domain *domain, gfp_t gfp) {unsigned long max_iova; -struct iommu_domain_data *data; +struct hisi_smmu_domain_data *data;if (!domain) {dbg ("domain is null\n" );return -ENODEV ; } - data = domain->priv; + data =to_smmu (domain); max_iova = data->iova_start + data->iova_size;if (iova < data->iova_start) { dbg("iova failed: iova = 0x%lx, start = 0x%8x\n",
@@ -429,13 +431,13 @@ static size_t hisi_smmu_unmap_lpae(struct iommu_domain *domain, {unsigned long max_iova;unsigned int ret; -struct iommu_domain_data *data; +struct hisi_smmu_domain_data *data;if (!domain) {dbg ("domain is null\n" );return -ENODEV ; } - data = domain->priv; + data =to_smmu (domain);/*calculate the max io virtual address */ max_iova = data->iova_start + data->iova_size;/*check the iova */
@@ -490,28 +492,32 @@ static int hisi_attach_dev_lpae(struct iommu_domain *domain, struct device *dev) {struct device_node *np = dev->of_node;int ret =0 ; -struct iommu_domain_data *iommu_info =NULL ; +struct hisi_smmu_domain_data *iommu_info =NULL ; +struct hisi_smmu_domain *hisi_dom; iommu_info =kzalloc (sizeof (*iommu_info),GFP_KERNEL );if (!iommu_info) { -dbg ("alloc iommu_domain_data fail\n" ); +dbg ("alloc hisi_smmu_domain_data fail\n" );return -EINVAL ; }list_add (&iommu_info->list, &hisi_smmu_dev->domain_list); - domain->priv = iommu_info; - ret =get_domain_data_lpae (np, domain->priv); + + hisi_dom = container_of(domain, struct hisi_smmu_domain, domain); + hisi_dom->iommu_info = iommu_info; +dev_iommu_priv_set (dev, iommu_info); + ret =get_domain_data_lpae (np, iommu_info);return ret; }static void hisi_detach_dev_lpae (struct iommu_domain *domain,struct device *dev) { -struct iommu_domain_data *data; +struct hisi_smmu_domain_data *data; - data = (struct iommu_domain_data *)domain->priv; + data =dev_iommu_priv_get (dev);if (data) {list_del (&data->list); - domain->priv =NULL ; +dev_iommu_priv_set (dev,NULL );kfree (data); }else {dbg ("%s:error! data entry has been delected\n" , __func__);
Signature · 1 line
2.26.2
PATCH 10/16 iommu: hisi_smmu_lpae: convert it to probe_device
In reply to Mauro Carvalho ChehabThe add_device()/remove_device() was removed on Kernel 5.8.
Convert the driver to use probe_device/release_device.
--- drivers/staging/hikey9xx/hisi_smmu_lpae.c | 31 +++++------------------ 1 file changed, 6 insertions(+), 25 deletions(-)
drivers/staging/hikey9xx/hisi_smmu_lpae.c +6 -25
@@ -529,33 +529,14 @@ static bool hisi_smmu_capable(enum iommu_cap cap)return false; } -static int hisi_smmu_add_device (struct device *dev) +static struct iommu_device *hisi_smmu_probe_device (struct device *dev) { -struct hisi_smmu_device_lpae *iommu = hisi_smmu_dev; -struct iommu_group *group; - -if (iommu) -iommu_device_link (&iommu->iommu, dev); -else -return -ENODEV ; - - group =iommu_group_get_for_dev (dev); -if (IS_ERR (group)) -return PTR_ERR (group); - -iommu_group_put (group); - -return 0 ; +return &hisi_smmu_dev->iommu; } -static void hisi_smmu_remove_device (struct device *dev) +static void hisi_smmu_release_device (struct device *dev) { -struct hisi_smmu_device_lpae *iommu = hisi_smmu_dev; - -if (iommu) -iommu_device_unlink (&iommu->iommu, dev); - -iommu_group_remove_device (dev); +/* As just one SMMU is possible, nothing to do here */ }static struct iommu_ops hisi_smmu_ops = {
@@ -567,8 +548,8 @@ static struct iommu_ops hisi_smmu_ops = { .map = hisi_smmu_map_lpae, .unmap = hisi_smmu_unmap_lpae, .iova_to_phys = hisi_smmu_iova_to_phys_lpae, - .add_device = hisi_smmu_add_device, - .remove_device = hisi_smmu_remove_device, + .probe_device = hisi_smmu_probe_device, + .release_device = hisi_smmu_release_device, .device_group = generic_device_group, .pgsize_bitmap = SMMU_PAGE_SIZE, };
Signature · 1 line
2.26.2
PATCH 11/16 iommu: add Hisilicon Kirin970 iommu at the building system
In reply to Mauro Carvalho ChehabNow that the iommu driver is ready, add it to the building system.
--- drivers/staging/hikey9xx/Kconfig | 9 +++++++++ drivers/staging/hikey9xx/Makefile | 1 + 2 files changed, 10 insertions(+)
drivers/staging/hikey9xx/Kconfig +9 -0
@@ -33,3 +33,12 @@ config REGULATOR_HI6421V600 This driver provides support for the voltage regulators on HiSilicon Hi6421v600 PMU / Codec IC. This is used on Kirin 3670 boards, like HiKey 970. + +# to be placed at drivers/iommu +config HISI_IOMMU_LPAE + bool "Hisilicon IOMMU LPAE Support" + select IOMMU_API + select IODOMAIN_API + help + This driver provides support for the IOMMU found on Kirin 970. + This is used on Kirin 3670 boards, like HiKey 970.
drivers/staging/hikey9xx/Makefile +1 -0
@@ -3,3 +3,4 @@ obj-$(CONFIG_SPMI_HISI3670) += hisi-spmi-controller.o obj-$(CONFIG_MFD_HI6421_SPMI) += hi6421-spmi-pmic.o obj-$(CONFIG_REGULATOR_HI6421V600) += hi6421v600-regulator.o +obj-$(CONFIG_HISI_IOMMU_LPAE) += hisi_smmu_lpae.o
Signature · 1 line
2.26.2
PATCH 12/16 iommu: hisi_smmu_lpae: cleanup printk macros
In reply to Mauro Carvalho Chehab- Whenever possible, use dev_foo() instead of pr_foo();
- Replace dbg() macro by either pr_debug() or dev_dbg();
- Fix some warnings due to different integer types.
- add debug at hisi_smmu_domain_alloc_lpae() to allow checking what domains were mapped.
--- drivers/staging/hikey9xx/hisi_smmu.h | 9 +-- drivers/staging/hikey9xx/hisi_smmu_lpae.c | 72 ++++++++++++----------- 2 files changed, 39 insertions(+), 42 deletions(-)
drivers/staging/hikey9xx/hisi_smmu.h +1 -8
@@ -3,13 +3,6 @@#ifndef HISI_SMMU_H #define HISI_SMMU_H -/*#define IOMMU_DEBUG*/ -#ifdef IOMMU_DEBUG -#define dbg(format, arg...) printk(KERN_ERR "[iommu]" format, ##arg) -#else -#define dbg(format, arg...) -#endif -#define SMMU_PHY_PTRS_PER_PTE (256)/*#define SMMU_PHY_PTRS_PER_PGD (4096)*/ #define SMMU_PTRS_PER_PGD (4)
@@ -163,7 +156,7 @@ static inline void smmu_set_pgd_lpae(smmu_pgd_t *pgdp, u64 pgd)/*fill the pmd entry, pgd value must be 64bit */ static inline void smmu_set_pmd_lpae (smmu_pgd_t *pmdp,u64 pmd) { -dbg ("smmu_set_pmd_lpae: pmd = 0x%lx\n" , pmd); +pr_debug ("smmu_set_pmd_lpae: pmd = 0x%llx\n" , pmd); *pmdp = pmd;dsb (ishst);isb ();
drivers/staging/hikey9xx/hisi_smmu_lpae.c +38 -34
239 lines not on this page · raw message
Signature · 1 line
2.26.2
PATCH 13/16 iommu: hisi_smmu_lpae: make OF compatible more standard
In reply to Mauro Carvalho ChehabUse "manufacturer,model" pattern for the expected compatible string for this board.
Most of compatible lines for Huawei's Hisilicon SoCs start with "hisilicon,". Use the same pattern here for this new driver.
--- drivers/staging/hikey9xx/hisi_smmu_lpae.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/hikey9xx/hisi_smmu_lpae.c +1 -1
@@ -616,7 +616,7 @@ static int hisi_smmu_remove_lpae(struct platform_device *pdev) }static const struct of_device_id hisi_smmu_of_match_lpae[] = { - { .compatible ="hisi,hisi-smmu-lpae" }, + { .compatible ="hisilicon,smmu-lpae" }, { }, };MODULE_DEVICE_TABLE (of, hisi_smmu_of_match_lpae);
Signature · 1 line
2.26.2
PATCH 14/16 dt: add an spec for the Kirin36x0 SMMU
In reply to Mauro Carvalho ChehabDescribe the properties expected by the IOMMU driver used on Hikey960 and Hikey970 boards.
--- .../iommu/hisilicon,kirin36x0-smmu.yaml | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml
Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml new file +55 -0
@@ -0,0 +1,55 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iommu/hisilicon,kirin36x0-smmu.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Hisilicon support for HI3660/HI3670 SMMU + +maintainers: + - Mauro Carvalho Chehab <[email protected]> + +description: |+ + Huawei's Hisilicon Kirin 3660/3670 contains a System MMU that enables + scattered physical memory chunks visible as a contiguous region to + DMA-capable peripheral devices like GPU and ISP. + + The IOMMU domains are described via iommu_info settings. + +properties: + compatible: + const: hisilicon,hisi-smmu-lpae + + iommu_info: + type: object + + properties: + start-addr: + maxItems: 1 + description: Memory start address (32 bits) + + size: + maxItems: 1 + description: size of the I/O MMU block (32 bits) + + iova-align: + minItems: 2 + maxItems: 2 + description: DMA address alignment of the mapped memory (64 bits) + +required: + - compatible + +additionalProperties: false + +examples: + - | + smmu_lpae { + compatible = "hisilicon,smmu-lpae"; + + iommu_info { + start-addr = <0x40000>; + size = <0xbffc0000>; + iova-align = <0x0 0x8000>; + }; + };
Signature · 6 lines
2.26.2 _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
PATCH 15/16 dt: hi3670-hikey970.dts: load the SMMU driver on Hikey970
In reply to Mauro Carvalho ChehabThose boards come with a system IOMMU, which are required by certain drivers (DRM/KMS).
Add its dependency to Hikey970 device tree bindings.
--- arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts | 3 +++ 1 file changed, 3 insertions(+)
arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts +3 -0
@@ -53,6 +53,9 @@ wlan_en: wlan-en-1-8v { startup-delay-us = <70000>; enable-active-high; }; + smmu_lpae { + compatible = "hisilicon,smmu-lpae"; + }; }; /*
Signature · 1 line
2.26.2
PATCH 16/16 staging: hikey9xx: add an item about the iommu driver
In reply to Mauro Carvalho ChehabThis driver doesn't use a too standard DT. Need to adjust it and the upcoming DRM driver who uses it, in order to do things on a more standard way.
--- drivers/staging/hikey9xx/TODO | 1 + 1 file changed, 1 insertion(+)
drivers/staging/hikey9xx/TODO +1 -0
@@ -2,4 +2,5 @@ ToDo list: - Port other drivers needed by Hikey 960/970; - Test drivers on Hikey 960; +- Adjust the iommu driver's code for it to use more standard DT settings; - Validate device tree bindings.
Signature · 1 line
2.26.2
Quoted 9 lines · Mauro Carvalho Chehab
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.9The remaining patches add SPDX headers and make it build and run with the upstream Kernel.
Please don't add iommu drivers to staging, and just work with the maintainers to properly clean it up.
I also don't think adding a totally out of date not compiling version is a good idea. Please do a proper rollup, and if required (probably not in this case), split it into useful chunks.
Hi Christoph,
Em Mon, 17 Aug 2020 09:21:06 +0100
Christoph Hellwig <[email protected]> escreveu:
Quoted 6 lines · Mauro Carvalho Chehab
Quoted 5 lines
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:
Quoted 7 lines
Quoted 4 lines
https://github.com/96boards-hikey/linux/tree/hikey970-v4.9The remaining patches add SPDX headers and make it build and run with the upstream Kernel.
Please don't add iommu drivers to staging, and just work with the maintainers to properly clean it up.
I need to start from the original patch in order to preserve its authorship.
My plan is to work with the iommu subsystem maintainers after have this (and another pending patch series for DRM) merged.
Quoted 3 lines
I also don't think adding a totally out of date not compiling version is a good idea. Please do a proper rollup, and if required (probably not in this case), split it into useful chunks.
This series make this driver working as expected.
I mean, while patch 01/16 is against Kernel 4.9, the other patches on this series ports it to upstream, cleans up the driver and address several issues on it.
This specific IOMMU seems to be an specific part of the SoC dedicated for the display engine and by the encoding/decoding images via the ISP. With this series, this driver builds and runs as expected, providing IOMMU support needed by the upcoming KMS/DRM driver.
The only issue on it (as far as I can tell) is that the DT bindings require some work, as, instead of using dma-ranges, the DRM driver binds into it with:
smmu_lpae {
compatible = "hisilicon,smmu-lpae";
};
dpe: dpe@e8600000 {
compatible = "hisilicon,kirin970-dpe";
...
iommu_info {
start-addr = <0x8000>;
size = <0xbfff8000>;
};
};In order to properly address it, the best would be to also have the DRM driver merged upstream, as it relies on it. So, a change in DT will also mean a change at the way the DRM uses it.
The DRM itself should go via staging, as it has some bugs that I'd like to fix before moving it to drivers/gpu/drm. Those are more tricky to solve, as they seem to require using different settings for some hardware registers, and the downstream driver also have the same issues. Fixing them will likely require some time.
Thanks,
Mauro
Quoted 2 lines · Mauro Carvalho Chehab
I need to start from the original patch in order to preserve its authorship.
Nom you don't. We have plenty of example where people submit code written by other, especially when it is significantly reworked,
Quoted 2 lines
My plan is to work with the iommu subsystem maintainers after have this (and another pending patch series for DRM) merged.
Please submit it to the iommu list directly. Seriously - you did not even cc the relevant list, and you don't even have a comment from the maintainers. Don't do this kind of crap.
Greg Kroah-Hartman
maintainerQuoted 61 lines · Mauro Carvalho Chehab
Hi Christoph,
Em Mon, 17 Aug 2020 09:21:06 +0100
Christoph Hellwig <[email protected]> escreveu:Quoted 6 lines · Mauro Carvalho Chehab
Quoted 5 lines
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:
Quoted 7 lines
Quoted 4 lines
https://github.com/96boards-hikey/linux/tree/hikey970-v4.9The remaining patches add SPDX headers and make it build and run with the upstream Kernel.
Please don't add iommu drivers to staging, and just work with the maintainers to properly clean it up.
I need to start from the original patch in order to preserve its authorship.
My plan is to work with the iommu subsystem maintainers after have this (and another pending patch series for DRM) merged.
Quoted 3 lines
I also don't think adding a totally out of date not compiling version is a good idea. Please do a proper rollup, and if required (probably not in this case), split it into useful chunks.
This series make this driver working as expected.
I mean, while patch 01/16 is against Kernel 4.9, the other patches on this series ports it to upstream, cleans up the driver and address several issues on it.
This specific IOMMU seems to be an specific part of the SoC dedicated for the display engine and by the encoding/decoding images via the ISP. With this series, this driver builds and runs as expected, providing IOMMU support needed by the upcoming KMS/DRM driver.
The only issue on it (as far as I can tell) is that the DT bindings require some work, as, instead of using dma-ranges, the DRM driver binds into it with:
smmu_lpae { compatible = "hisilicon,smmu-lpae"; }; dpe: dpe@e8600000 { compatible = "hisilicon,kirin970-dpe"; ... iommu_info { start-addr = <0x8000>; size = <0xbfff8000>; }; };In order to properly address it, the best would be to also have the DRM driver merged upstream, as it relies on it. So, a change in DT will also mean a change at the way the DRM uses it.
Quoted 5 lines
The DRM itself should go via staging, as it has some bugs that I'd like to fix before moving it to
drivers/gpu/drm. Those are more tricky to solve, as they seem to require using different settings for some hardware registers, and the downstream driver also have the same issues. Fixing them will likely require some time.
DRM drivers can't go through staging unless you get the DRM developers to agree with it, and last I heard, they were strongly against it.
It's _always_ faster to just do the work out-of-tree for a week or so and then merge it correctly to the proper part of the kernel tree. I'd recommend doing that here for the iommu driver, as well as the DRM driver.
There's no issues with authorship and the like, just properly attribute it when you submit it and you are fine.
Again, merging in staging always takes more work and energy, don't do it unless there is no other way.
thanks,
greg k-h
Em Mon, 17 Aug 2020 11:37:03 +0200
Greg Kroah-Hartman <[email protected]> escreveu:
Quoted 62 lines · Mauro Carvalho Chehab
Quoted 61 lines
Hi Christoph,
Em Mon, 17 Aug 2020 09:21:06 +0100
Christoph Hellwig <[email protected]> escreveu:Quoted 6 lines · Mauro Carvalho Chehab
Quoted 5 lines
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:
Quoted 7 lines
Quoted 4 lines
https://github.com/96boards-hikey/linux/tree/hikey970-v4.9The remaining patches add SPDX headers and make it build and run with the upstream Kernel.
Please don't add iommu drivers to staging, and just work with the maintainers to properly clean it up.
I need to start from the original patch in order to preserve its authorship.
My plan is to work with the iommu subsystem maintainers after have this (and another pending patch series for DRM) merged.
Quoted 3 lines
I also don't think adding a totally out of date not compiling version is a good idea. Please do a proper rollup, and if required (probably not in this case), split it into useful chunks.
This series make this driver working as expected.
I mean, while patch 01/16 is against Kernel 4.9, the other patches on this series ports it to upstream, cleans up the driver and address several issues on it.
This specific IOMMU seems to be an specific part of the SoC dedicated for the display engine and by the encoding/decoding images via the ISP. With this series, this driver builds and runs as expected, providing IOMMU support needed by the upcoming KMS/DRM driver.
The only issue on it (as far as I can tell) is that the DT bindings require some work, as, instead of using dma-ranges, the DRM driver binds into it with:
smmu_lpae { compatible = "hisilicon,smmu-lpae"; }; dpe: dpe@e8600000 { compatible = "hisilicon,kirin970-dpe"; ... iommu_info { start-addr = <0x8000>; size = <0xbfff8000>; }; };In order to properly address it, the best would be to also have the DRM driver merged upstream, as it relies on it. So, a change in DT will also mean a change at the way the DRM uses it.
Quoted 8 lines
Quoted 5 lines
The DRM itself should go via staging, as it has some bugs that I'd like to fix before moving it to
drivers/gpu/drm. Those are more tricky to solve, as they seem to require using different settings for some hardware registers, and the downstream driver also have the same issues. Fixing them will likely require some time.DRM drivers can't go through staging unless you get the DRM developers to agree with it, and last I heard, they were strongly against it.
Ok, I'll ping them. There's already a thread I opened at the DRM devel ML about this driver. IMHO, there's nothing on this specific driver that would prevent having it on staging for a while, until the two or three remaining bugs gets fixed.
On the other hand, the code already follows the current DRM policies, as far as I can tell. The bugs are related to some specific register settings that would need to be better tuned (and maybe some delay when waiting for EDID data at the already-existing adv7535 driver). Maybe it would be preferred to have it at drivers/gpu even with such bugs.
Quoted 4 lines
It's _always_ faster to just do the work out-of-tree for a week or so and then merge it correctly to the proper part of the kernel tree. I'd recommend doing that here for the iommu driver, as well as the DRM driver.
It is OK for me to working for a week or so, until the iommu people become happy with that.
The main reason of submitting via staging is that I need to preserve the patch that added this driver as-is, in order to preserve its SoB and not causing legal issues.
It it is OK for iommu to accept a submission like that, I can re-submit it, doing the changes at drivers/iommu.
If not, besides this series, the only alternative I can think of is to merge it first at the staging, with the incremental changes, and with a final patch moving the code out of staging.
Thanks,
Mauro
Greg Kroah-Hartman
maintainerQuoted 6 lines · Mauro Carvalho Chehab
The main reason of submitting via staging is that I need to preserve the patch that added this driver as-is, in order to preserve its SoB and not causing legal issues.
It it is OK for iommu to accept a submission like that, I can re-submit it, doing the changes at
drivers/iommu.
You can always do this just fine, as one single patch. You do know about the co-developed-by: line, right?
thanks,
greg k-h
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Quoted 2 lines · Greg Kroah-Hartman
You can always do this just fine, as one single patch. You do know about the co-developed-by: line, right?
Agreed. Please keep the main iommu driver in one patch and use co-developed-by. This makes it easier for me to review it and provide feedback. And please Cc me on the whole patch-set for v2.
Thanks,
Joerg
Robin Murphy
reviewerQuoted 13 lines · Mauro Carvalho Chehab
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.9The 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
Quoted 13 lines
Mauro Carvalho Chehab (15):
iommu: hisilicon: remove defaultiommu_map_sghandler
iommu: hisilicon: map and unmap ops gained new arguments
iommu:hisi_smmu_lpae: rebase it to work with upstream
iommu:hisi_smmu: removelinux/hisi/hisi-iommu.h
iommu: hisilicon: cleanup its code style
iommu:hisi_smmu_lpae: get rid ofIOMMU_SECandIOMMU_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 toprobe_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 ;)
Robin.
Quoted 16 lines
dt: add an spec for the Kirin36x0 SMMU
dt:hi3670-hikey970.dts: load the SMMU driver on Hikey970
staging: hikey9xx: add an item about the iommu driver.../iommu/hisilicon,kirin36x0-smmu.yaml | 55 ++ .../boot/dts/hisilicon/hi3670-hikey970.dts | 3 + drivers/staging/hikey9xx/Kconfig | 9 + drivers/staging/hikey9xx/Makefile | 1 + drivers/staging/hikey9xx/TODO | 1 + drivers/staging/hikey9xx/hisi_smmu.h | 196 ++++++ drivers/staging/hikey9xx/hisi_smmu_lpae.c | 648 ++++++++++++++++++ 7 files changed, 913 insertions(+)create mode 100644
Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yamlcreate mode 100644drivers/staging/hikey9xx/hisi_smmu.h
create mode 100644drivers/staging/hikey9xx/hisi_smmu_lpae.c
Hi Robin,
Em Tue, 18 Aug 2020 15:47:55 +0100
Robin Murphy <[email protected]> escreveu:
Quoted 37 lines · Mauro Carvalho Chehab
Quoted 13 lines
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.9The 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 iommuQuoted 13 lines
Mauro Carvalho Chehab (15):
iommu: hisilicon: remove defaultiommu_map_sghandler
iommu: hisilicon: map and unmap ops gained new arguments
iommu:hisi_smmu_lpae: rebase it to work with upstream
iommu:hisi_smmu: removelinux/hisi/hisi-iommu.h
iommu: hisilicon: cleanup its code style
iommu:hisi_smmu_lpae: get rid ofIOMMU_SECandIOMMU_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 toprobe_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 standardEchoing 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?
Quoted 5 lines
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
Robin Murphy
reviewerQuoted 117 lines · Mauro Carvalho Chehab
Hi Robin,
Em Tue, 18 Aug 2020 15:47:55 +0100
Robin Murphy <[email protected]> escreveu:Quoted 37 lines · Mauro Carvalho Chehab
Quoted 13 lines
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.9The 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 iommuQuoted 13 lines
Mauro Carvalho Chehab (15):
iommu: hisilicon: remove defaultiommu_map_sghandler
iommu: hisilicon: map and unmap ops gained new arguments
iommu:hisi_smmu_lpae: rebase it to work with upstream
iommu:hisi_smmu: removelinux/hisi/hisi-iommu.h
iommu: hisilicon: cleanup its code style
iommu:hisi_smmu_lpae: get rid ofIOMMU_SECandIOMMU_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 toprobe_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 standardEchoing 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?
Quoted 5 lines
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.cin 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); }
Quoted 7 lines
[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).
Robin.
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Quoted 63 lines · Robin Murphy
Quoted 62 lines · Mauro Carvalho Chehab
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@e8600000 { compatible = "hisilicon,kirin970-dpe"; memory-region = <&drm_dma_reserved>; ... iommu_info { start-addr = <0x8000>; size = <0xbfff8000>; }; }This is used by
kirin9xx_drm_dss.cin 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.
Quoted 8 lines
Quoted 2 lines
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_S1pagetables - 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
Robin Murphy
reviewerQuoted 73 lines · John Stultz
Quoted 63 lines · Robin Murphy
Quoted 62 lines · Mauro Carvalho Chehab
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@e8600000 { compatible = "hisilicon,kirin970-dpe"; memory-region = <&drm_dma_reserved>; ... iommu_info { start-addr = <0x8000>; size = <0xbfff8000>; }; }This is used by
kirin9xx_drm_dss.cin 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.
Quoted 8 lines
Quoted 2 lines
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_S1pagetables - see Panfrost for an example (but try to ignore the wacky "Mali LPAE" format).
Quoted 4 lines
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_infosubnode in the dts that maybe needs to be cleaned up).
Indeed, I'd assume it's possible to leave the MMU off and just use CMA buffers instead, but wiring it up properly without the downstream mis-design should be pretty clean, so maybe that could ultimately be shared with 960 too (assuming the hardware isn't wildly dissimilar).
I notice there's already a whole load of MMU configuration hard-coded into the DRM driver - does iommu_info even need to be in the DT, or could that also be decided directly by the driver? (Most other MMU-aware DRM drivers seem to hard-code their drm_mm dimensions.) I can't imagine the *virtual* address space limits need to vary on a per-board basis, and they could easily be tied to the compatible if they legitimately differ across SoCs and a simple lowest-common-denominator approach wouldn't suffice for whatever reason.
Robin.
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Em Tue, 18 Aug 2020 15:02:54 -0700
John Stultz <[email protected]> escreveu:
Quoted 73 lines · Robin Murphy
Quoted 63 lines · Mauro Carvalho Chehab
Quoted 62 lines
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@e8600000 { compatible = "hisilicon,kirin970-dpe"; memory-region = <&drm_dma_reserved>; ... iommu_info { start-addr = <0x8000>; size = <0xbfff8000>; }; }This is used by
kirin9xx_drm_dss.cin 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.
Quoted 8 lines
Quoted 2 lines
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_S1pagetables - see Panfrost for an example (but try to ignore the wacky "Mali LPAE" format).
Quoted 4 lines
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_infosubnode in the dts that maybe needs to be cleaned up).
Funny... while the Hikey 970 DRM driver has such IOMMU code, it doesn't actually use it!
The driver has a function called hisi_dss_smmu_config() with sets the registers on a different way in order to use IOMMU or not, at the hisi_fb_pan_display() function. It can also use a mode called "afbcd".
Well, this function sets both to false:
bool afbcd = false; bool mmu_enable = false;
I ended commenting out the code which depends at the iommu driver and everything is working as before.
So, I'll just forget about this iommu driver, as we can live without that.
For now, I'll keep the mmu code there commented out, as it could be useful on a future port for it to use io-pgtable.
-
Robin,
Can the Panfrost driver use io-pgtable while the KMS driver won't be using it? Or this would cause it to not work?
My end goal here is to be able to test the Panfrost driver ;-)
Thanks,
Mauro
Robin Murphy
reviewerQuoted 103 lines · Mauro Carvalho Chehab
Em Tue, 18 Aug 2020 15:02:54 -0700
John Stultz <[email protected]> escreveu:Quoted 73 lines · Robin Murphy
Quoted 63 lines · Mauro Carvalho Chehab
Quoted 62 lines
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@e8600000 { compatible = "hisilicon,kirin970-dpe"; memory-region = <&drm_dma_reserved>; ... iommu_info { start-addr = <0x8000>; size = <0xbfff8000>; }; }This is used by
kirin9xx_drm_dss.cin 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.
Quoted 8 lines
Quoted 2 lines
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_S1pagetables - see Panfrost for an example (but try to ignore the wacky "Mali LPAE" format).Quoted 4 lines
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_infosubnode in the dts that maybe needs to be cleaned up).Funny... while the Hikey 970 DRM driver has such IOMMU code, it doesn't actually use it!
The driver has a function called
hisi_dss_smmu_config()with sets the registers on a different way in order to use IOMMU or not, at thehisi_fb_pan_display()function. It can also use a mode called "afbcd".Well, this function sets both to false:
bool afbcd = false; bool mmu_enable = false;I ended commenting out the code which depends at the iommu driver and everything is working as before.
So, I'll just forget about this iommu driver, as we can live without that.
For now, I'll keep the mmu code there commented out, as it could be useful on a future port for it to use io-pgtable.
Quoted 8 lines
-
Robin,
Can the Panfrost driver use io-pgtable while the KMS driver won't be using it? Or this would cause it to not work?
My end goal here is to be able to test the Panfrost driver ;-)
Yup, the GPU has its own independent MMU, so Panfrost can import display buffers regardless of whether they're physically contiguous or not. Since Mesa master has recently landed AFBC support, there's probably more immediate benefit in getting that AFBC decoder working before the display MMU (although ultimately things are likely to work better under memory pressure if you don't have to rely on CMA, so it should still be worth coming back to at some point).
Robin.