Genericise the IOVA allocator
Subsystems (2)
Robin Murphy
maintainercover Genericise the IOVA allocator
Hi all,
I've been implementing IOMMU DMA mapping for arm64, based on tidied-up parts of the existing arch/arm/mm/dma-mapping.c with a clear divide between the arch-specific parts and the general DMA-API to IOMMU-API layer so that that can be shared; similar to what Ritesh started before and was unable to complete[1], but working in the other direction.
The first part of that tidy-up involved ripping out the homebrewed IOVA allocator and plumbing in iova.c, necessitating the changes presented here. The rest is currently sat under arch/arm64 for the sake of getting it working quickly with minimal impact - ideally I'd move it out and port arch/arm before merging, but I don't know quite how impatient people are. Regardless of that decision, this bit stands alone, so here it is.
Feel free to ignore patches 1 and 2, since I see Sakari has recently posted a more thorough series for that[2], that frankly looks nicer ;) I've merely left them in as context here.
[1]:http://thread.gmane.org/gmane.linux.ports.arm.kernel/331299 [2]:http://article.gmane.org/gmane.linux.kernel.iommu/7436
Robin Murphy (4):
iommu: build iova.c for any IOMMU
iommu: consolidate IOVA allocator code
iommu: make IOVA domain low limit flexible
iommu: make IOVA domain page size explicit
drivers/iommu/Makefile | 4 ++-- drivers/iommu/intel-iommu.c | 45 ++++++++++------------------------------- drivers/iommu/iova.c | 49 +++++++++++++++++++++++++++++++++++++++++---- include/linux/iova.h | 41 +++++++++++++++++++++++++++++++++---- 4 files changed, 95 insertions(+), 44 deletions(-)
Signature · 1 line
1.9.1
Robin Murphy
maintainerRFC PATCH 1/4 iommu: build iova.c for any IOMMU
In preparation for sharing the IOVA allocator, build it for all IOMMU API users.
--- drivers/iommu/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iommu/Makefile +2 -2
@@ -1,4 +1,4 @@ -obj-$(CONFIG_IOMMU_API) += iommu.o +obj-$(CONFIG_IOMMU_API) += iommu.o iova.o obj-$(CONFIG_IOMMU_API) += iommu-traces.o obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o obj-$(CONFIG_OF_IOMMU) += of_iommu.o
@@ -7,7 +7,7 @@ obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o obj-$(CONFIG_ARM_SMMU) += arm-smmu.o obj-$(CONFIG_DMAR_TABLE) += dmar.o -obj-$(CONFIG_INTEL_IOMMU) += iova.o intel-iommu.o +obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o obj-$(CONFIG_IRQ_REMAP) += intel_irq_remapping.o irq_remapping.o obj-$(CONFIG_OMAP_IOMMU) += omap-iommu.o
Signature · 1 line
1.9.1
Robin Murphy
maintainerRFC PATCH 2/4 iommu: consolidate IOVA allocator code
In reply to Robin MurphyIn order to share the IOVA allocator with other architectures, break the unnecssary dependency on the Intel IOMMU driver and move the remaining IOVA internals to iova.c
--- drivers/iommu/intel-iommu.c | 33 ++------------------------------- drivers/iommu/iova.c | 35 +++++++++++++++++++++++++++++++++++ include/linux/iova.h | 3 +++ 3 files changed, 40 insertions(+), 31 deletions(-)
drivers/iommu/intel-iommu.c +2 -31
@@ -490,7 +490,6 @@ __setup("intel_iommu=", intel_iommu_setup);static struct kmem_cache *iommu_domain_cache;static struct kmem_cache *iommu_devinfo_cache; -static struct kmem_cache *iommu_iova_cache;static inline void *alloc_pgtable_page (int node) {
@@ -528,16 +527,6 @@ static inline void free_devinfo_mem(void *vaddr)kmem_cache_free (iommu_devinfo_cache, vaddr); } -struct iova *alloc_iova_mem (void ) -{ -return kmem_cache_alloc (iommu_iova_cache,GFP_ATOMIC ); -} - -void free_iova_mem (struct iova *iova) -{ -kmem_cache_free (iommu_iova_cache, iova); -} -static inline int domain_type_is_vm (struct dmar_domain *domain) {return domain->flags &DOMAIN_FLAG_VIRTUAL_MACHINE ;
@@ -3429,23 +3418,6 @@ static inline int iommu_devinfo_cache_init(void)return ret; } -static inline int iommu_iova_cache_init (void ) -{ -int ret =0 ; - - iommu_iova_cache =kmem_cache_create ("iommu_iova" , -sizeof (struct iova ), -0 , -SLAB_HWCACHE_ALIGN , -NULL ); -if (!iommu_iova_cache) { -printk (KERN_ERR "Couldn't create iova cache\n" ); - ret = -ENOMEM ; - } - -return ret; -} -static int __init iommu_init_mempool (void ) {int ret;
@@ -3463,7 +3435,7 @@ static int __init iommu_init_mempool(void)kmem_cache_destroy (iommu_domain_cache); domain_error: -kmem_cache_destroy (iommu_iova_cache); +iommu_iova_cache_destroy ();return -ENOMEM ; }
@@ -3472,8 +3444,7 @@ static void __init iommu_exit_mempool(void) {kmem_cache_destroy (iommu_devinfo_cache);kmem_cache_destroy (iommu_domain_cache); -kmem_cache_destroy (iommu_iova_cache); - +iommu_iova_cache_destroy (); }static void quirk_ioat_snb_local_iommu (struct pci_dev *pdev)
drivers/iommu/iova.c +35 -0
@@ -18,6 +18,41 @@*/ #include <linux/iova.h> +#include <linux/slab.h> + +static struct kmem_cache *iommu_iova_cache; + +int iommu_iova_cache_init (void ) +{ +int ret =0 ; + + iommu_iova_cache =kmem_cache_create ("iommu_iova" , +sizeof (struct iova ), +0 , +SLAB_HWCACHE_ALIGN , +NULL ); +if (!iommu_iova_cache) { +pr_err ("Couldn't create iova cache\n" ); + ret = -ENOMEM ; + } + +return ret; +} + +void iommu_iova_cache_destroy (void ) +{ +kmem_cache_destroy (iommu_iova_cache); +} + +struct iova *alloc_iova_mem (void ) +{ +return kmem_cache_alloc (iommu_iova_cache,GFP_ATOMIC ); +} + +void free_iova_mem (struct iova *iova) +{ +kmem_cache_free (iommu_iova_cache, iova); +}void init_iova_domain (struct iova_domain *iovad,unsigned long pfn_32bit)
include/linux/iova.h +3 -0
@@ -39,6 +39,9 @@ static inline unsigned long iova_size(struct iova *iova)return iova->pfn_hi - iova->pfn_lo +1 ; } +int iommu_iova_cache_init (void ); +void iommu_iova_cache_destroy (void ); +struct iova *alloc_iova_mem (void );void free_iova_mem (struct iova *iova);void free_iova (struct iova_domain *iovad,unsigned long pfn);
Signature · 1 line
1.9.1
Robin Murphy
maintainerRFC PATCH 3/4 iommu: make IOVA domain low limit flexible
In reply to Robin MurphyTo share the IOVA allocator with other architectures, it needs to accommodate more general aperture restrictions; move the lower limit from a compile-time constant to a runtime domain property to allow IOVA domains with different requirements to co-exist.
Also reword the slightly unclear description of alloc_iova since we're touching it anyway.
--- drivers/iommu/intel-iommu.c | 9 ++++++--- drivers/iommu/iova.c | 10 ++++++---- include/linux/iova.h | 7 +++---- 3 files changed, 15 insertions(+), 11 deletions(-)
drivers/iommu/intel-iommu.c +6 -3
@@ -71,6 +71,9 @@ __DOMAIN_MAX_PFN(gaw), (unsigned long)-1))#define DOMAIN_MAX_ADDR (gaw) (((uint64_t)__DOMAIN_MAX_PFN(gaw)) << VTD_PAGE_SHIFT) +/* IO virtual address start page frame number */ +#define IOVA_START_PFN (1) +#define IOVA_PFN (addr) ((addr) >> PAGE_SHIFT)#define DMA_32BIT_PFN IOVA_PFN(DMA_BIT_MASK(32))#define DMA_64BIT_PFN IOVA_PFN(DMA_BIT_MASK(64))
@@ -1629,7 +1632,7 @@ static int dmar_init_reserved_ranges(void)struct iova *iova;int i; -init_iova_domain (&reserved_iova_list,DMA_32BIT_PFN ); +init_iova_domain (&reserved_iova_list,IOVA_START_PFN ,DMA_32BIT_PFN );lockdep_set_class (&reserved_iova_list.iova_rbtree_lock, &reserved_rbtree_key);
@@ -1687,7 +1690,7 @@ static int domain_init(struct dmar_domain *domain, int guest_width)int adjust_width, agaw;unsigned long sagaw; -init_iova_domain (&domain->iovad,DMA_32BIT_PFN ); +init_iova_domain (&domain->iovad,IOVA_START_PFN ,DMA_32BIT_PFN );domain_reserve_special_ranges (domain);/* calculate AGAW */
@@ -4160,7 +4163,7 @@ static int md_domain_init(struct dmar_domain *domain, int guest_width) {int adjust_width; -init_iova_domain (&domain->iovad,DMA_32BIT_PFN ); +init_iova_domain (&domain->iovad,IOVA_START_PFN ,DMA_32BIT_PFN );domain_reserve_special_ranges (domain);/* calculate AGAW */
drivers/iommu/iova.c +6 -4
@@ -55,11 +55,13 @@ void free_iova_mem(struct iova *iova) }void -init_iova_domain (struct iova_domain *iovad,unsigned long pfn_32bit) +init_iova_domain (struct iova_domain *iovad,unsigned long start_pfn, +unsigned long pfn_32bit) {spin_lock_init (&iovad->iova_rbtree_lock); iovad->rbroot =RB_ROOT ; iovad->cached32_node =NULL ; + iovad->start_pfn = start_pfn; iovad->dma_32bit_pfn = pfn_32bit; }
@@ -162,7 +164,7 @@ move_left:if (!curr) {if (size_aligned) pad_size =iova_get_pad_size (size, limit_pfn); -if ((IOVA_START_PFN + size + pad_size) > limit_pfn) { +if ((iovad->start_pfn + size + pad_size) > limit_pfn) {spin_unlock_irqrestore (&iovad->iova_rbtree_lock, flags);return -ENOMEM ; }
@@ -237,8 +239,8 @@ iova_insert_rbtree(struct rb_root *root, struct iova *iova)* @size: - size of page frames to allocate * @limit_pfn: - max limit address * @size_aligned: - set if size_aligned address range is required -* This function allocates an iova in the range limit_pfn to IOVA_START_PFN -* looking from limit_pfn instead from IOVA_START_PFN. If the size_aligned +* This function allocates an iova in the range iovad->start_pfn to limit_pfn, +* searching top-down from limit_pfn to iovad->start_pfn. If the size_aligned * flag is set then the allocated address iova->pfn_lo will be naturally * aligned on roundup_power_of_two(size). */
include/linux/iova.h +3 -4
@@ -16,9 +16,6 @@#include <linux/rbtree.h> #include <linux/dma-mapping.h> -/* IO virtual address start page frame number */ -#define IOVA_START_PFN (1) -/* iova structure */ struct iova {struct rb_node node;
@@ -31,6 +28,7 @@ struct iova_domain {spinlock_t iova_rbtree_lock;/* Lock to protect update of rbtree */ struct rb_root rbroot;/* iova domain rbtree root */ struct rb_node *cached32_node;/* Save last alloced node */ +unsigned long start_pfn;/* Lower limit for this domain */ unsigned long dma_32bit_pfn; };
@@ -52,7 +50,8 @@ struct iova *alloc_iova(struct iova_domain *iovad, unsigned long size,struct iova *reserve_iova (struct iova_domain *iovad,unsigned long pfn_lo,unsigned long pfn_hi);void copy_reserved_iova (struct iova_domain *from,struct iova_domain *to); -void init_iova_domain (struct iova_domain *iovad,unsigned long pfn_32bit); +void init_iova_domain (struct iova_domain *iovad,unsigned long start_pfn, +unsigned long pfn_32bit);struct iova *find_iova (struct iova_domain *iovad,unsigned long pfn);void put_iova_domain (struct iova_domain *iovad); struct iova *split_and_remove_iova(struct iova_domain *iovad,
Signature · 1 line
1.9.1
Robin Murphy
maintainerRFC PATCH 4/4 iommu: make IOVA domain page size explicit
In reply to Robin MurphySystems may contain heterogeneous IOMMUs supporting differing minimum page sizes, which may also not be common with the CPU page size. Thus it is practical to have an explicit notion of IOVA granularity to simplify handling of mapping and allocation constraints.
As an initial step, move the IOVA page granularity from an implicit compile-time constant to a per-domain property so we can make use of it in IOVA domain context at runtime. To keep the abstraction tidy, extend the little API of inline iova_* helpers to parallel some of the equivalent PAGE_* macros.
--- drivers/iommu/intel-iommu.c | 9 ++++++--- drivers/iommu/iova.c | 8 ++++++-- include/linux/iova.h | 35 +++++++++++++++++++++++++++++++++-- 3 files changed, 45 insertions(+), 7 deletions(-)
drivers/iommu/intel-iommu.c +6 -3
@@ -1632,7 +1632,8 @@ static int dmar_init_reserved_ranges(void)struct iova *iova;int i; -init_iova_domain (&reserved_iova_list,IOVA_START_PFN ,DMA_32BIT_PFN ); +init_iova_domain (&reserved_iova_list,VTD_PAGE_SIZE ,IOVA_START_PFN , +DMA_32BIT_PFN );lockdep_set_class (&reserved_iova_list.iova_rbtree_lock, &reserved_rbtree_key);
@@ -1690,7 +1691,8 @@ static int domain_init(struct dmar_domain *domain, int guest_width)int adjust_width, agaw;unsigned long sagaw; -init_iova_domain (&domain->iovad,IOVA_START_PFN ,DMA_32BIT_PFN ); +init_iova_domain (&domain->iovad,VTD_PAGE_SIZE ,IOVA_START_PFN , +DMA_32BIT_PFN );domain_reserve_special_ranges (domain);/* calculate AGAW */
@@ -4163,7 +4165,8 @@ static int md_domain_init(struct dmar_domain *domain, int guest_width) {int adjust_width; -init_iova_domain (&domain->iovad,IOVA_START_PFN ,DMA_32BIT_PFN ); +init_iova_domain (&domain->iovad,VTD_PAGE_SIZE ,IOVA_START_PFN , +DMA_32BIT_PFN );domain_reserve_special_ranges (domain);/* calculate AGAW */
drivers/iommu/iova.c +6 -2
@@ -55,12 +55,16 @@ void free_iova_mem(struct iova *iova) }void -init_iova_domain (struct iova_domain *iovad,unsigned long start_pfn, -unsigned long pfn_32bit) +init_iova_domain (struct iova_domain *iovad,unsigned long granule, +unsigned long start_pfn,unsigned long pfn_32bit) { +/* IOMMUs must support at least the CPU page size */ +BUG_ON (granule >PAGE_SIZE ); +spin_lock_init (&iovad->iova_rbtree_lock); iovad->rbroot =RB_ROOT ; iovad->cached32_node =NULL ; + iovad->granule = granule; iovad->start_pfn = start_pfn; iovad->dma_32bit_pfn = pfn_32bit; }
include/linux/iova.h +33 -2
@@ -28,6 +28,7 @@ struct iova_domain {spinlock_t iova_rbtree_lock;/* Lock to protect update of rbtree */ struct rb_root rbroot;/* iova domain rbtree root */ struct rb_node *cached32_node;/* Save last alloced node */ +unsigned long granule;/* pfn granularity for this domain */ unsigned long start_pfn;/* Lower limit for this domain */ unsigned long dma_32bit_pfn; };
@@ -37,6 +38,36 @@ static inline unsigned long iova_size(struct iova *iova)return iova->pfn_hi - iova->pfn_lo +1 ; } +static inline unsigned long iova_shift (struct iova_domain *iovad) +{ +return __ffs (iovad->granule); +} + +static inline unsigned long iova_mask (struct iova_domain *iovad) +{ +return iovad->granule -1 ; +} + +static inline size_t iova_offset (struct iova_domain *iovad,dma_addr_t iova) +{ +return iova &iova_mask (iovad); +} + +static inline size_t iova_align (struct iova_domain *iovad,size_t size) +{ +return ALIGN (size, iovad->granule); +} + +static inline dma_addr_t iova_dma_addr (struct iova_domain *iovad,struct iova *iova) +{ +return (dma_addr_t )iova->pfn_lo <<iova_shift (iovad); +} + +static inline unsigned long iova_pfn (struct iova_domain *iovad,dma_addr_t iova) +{ +return iova >>iova_shift (iovad); +} +int iommu_iova_cache_init (void );void iommu_iova_cache_destroy (void );
@@ -50,8 +81,8 @@ struct iova *alloc_iova(struct iova_domain *iovad, unsigned long size,struct iova *reserve_iova (struct iova_domain *iovad,unsigned long pfn_lo,unsigned long pfn_hi);void copy_reserved_iova (struct iova_domain *from,struct iova_domain *to); -void init_iova_domain (struct iova_domain *iovad,unsigned long start_pfn, -unsigned long pfn_32bit); +void init_iova_domain (struct iova_domain *iovad,unsigned long granule, +unsigned long start_pfn,unsigned long pfn_32bit);struct iova *find_iova (struct iova_domain *iovad,unsigned long pfn);void put_iova_domain (struct iova_domain *iovad); struct iova *split_and_remove_iova(struct iova_domain *iovad,
Signature · 1 line
1.9.1
RFC PATCH 1/4 iommu: build iova.c for any IOMMU
In reply to Robin MurphyQuoted 13 lines · Robin Murphy
In preparation for sharing the IOVA allocator, build it for all IOMMU API users.
Signed-off-by Robin Murphy <[email protected]>--- drivers/iommu/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/iommu/Makefile +0
-0
drivers/iommu/Makefile line 1
Quoted 2 lines
drivers/iommu/Makefile +1
-1@@ -1,4 +1,4 @@ -obj-$(CONFIG_IOMMU_API) += iommu.o +obj-$(CONFIG_IOMMU_API) += iommu.o iova.o
I'm not a maintainer, so just code review comment.
I don't known whether or not you will apply iova to all iommu drivers. But if only x86 and arm/arm64, it's no good to force other iommu drivers to include iova.o, because they have not used it yet.
Maybe it's good to define a new config, like CONFIG_IOMMU_IOVA.
config INTEL_IOMMU
select IOMMU_IOVA
config ARM_SMMU
select IOMMU_IOVA
obj-$(CONFIG_IOMMU_IOVA) += iova.o
Quoted 13 lines
drivers/iommu/Makefile +1
-1obj-$(CONFIG_IOMMU_API) += iommu-traces.o obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o obj-$(CONFIG_OF_IOMMU) += of_iommu.o@@ -7,7 +7,7 @@ obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o obj-$(CONFIG_ARM_SMMU) += arm-smmu.o obj-$(CONFIG_DMAR_TABLE) += dmar.o -obj-$(CONFIG_INTEL_IOMMU) += iova.o intel-iommu.o +obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o obj-$(CONFIG_IRQ_REMAP) += intel_irq_remapping.o irq_remapping.o obj-$(CONFIG_OMAP_IOMMU) += omap-iommu.o
RFC PATCH 4/4 iommu: make IOVA domain page size explicit
In reply to Robin MurphyQuoted 57 lines · Robin Murphy
Systems may contain heterogeneous IOMMUs supporting differing minimum page sizes, which may also not be common with the CPU page size. Thus it is practical to have an explicit notion of IOVA granularity to simplify handling of mapping and allocation constraints.
As an initial step, move the IOVA page granularity from an implicit compile-time constant to a per-domain property so we can make use of it in IOVA domain context at runtime. To keep the abstraction tidy, extend the little API of inline iova_* helpers to parallel some of the equivalent PAGE_* macros.
Signed-off-by Robin Murphy <[email protected]>--- drivers/iommu/intel-iommu.c | 9 ++++++--- drivers/iommu/iova.c | 8 ++++++-- include/linux/iova.h | 35 +++++++++++++++++++++++++++++++++-- 3 files changed, 45 insertions(+), 7 deletions(-)drivers/iommu/intel-iommu.c +6
-3@@ -1632,7 +1632,8 @@ static int dmar_init_reserved_ranges(void)struct iova *iova;int i; -init_iova_domain (&reserved_iova_list,IOVA_START_PFN ,DMA_32BIT_PFN ); +init_iova_domain (&reserved_iova_list,VTD_PAGE_SIZE ,IOVA_START_PFN , +DMA_32BIT_PFN );lockdep_set_class (&reserved_iova_list.iova_rbtree_lock, &reserved_rbtree_key);@@ -1690,7 +1691,8 @@ static int domain_init(struct dmar_domain *domain, int guest_width)int adjust_width, agaw;unsigned long sagaw; -init_iova_domain (&domain->iovad,IOVA_START_PFN ,DMA_32BIT_PFN ); +init_iova_domain (&domain->iovad,VTD_PAGE_SIZE ,IOVA_START_PFN , +DMA_32BIT_PFN );domain_reserve_special_ranges (domain);/* calculate AGAW */ @@ -4163,7 +4165,8 @@ static int md_domain_init(struct dmar_domain *domain, int guest_width) {int adjust_width; -init_iova_domain (&domain->iovad,IOVA_START_PFN ,DMA_32BIT_PFN ); +init_iova_domain (&domain->iovad,VTD_PAGE_SIZE ,IOVA_START_PFN , +DMA_32BIT_PFN );domain_reserve_special_ranges (domain);/* calculate AGAW */ drivers/iommu/iova.c +0
-0
drivers/iommu/iova.c void free_iova_mem(struct iova *iova) · line 62
Quoted 10 lines
drivers/iommu/iova.c +4
-2@@ -55,12 +55,16 @@ void free_iova_mem(struct iova *iova) }void -init_iova_domain (struct iova_domain *iovad,unsigned long start_pfn, -unsigned long pfn_32bit) +init_iova_domain (struct iova_domain *iovad,unsigned long granule, +unsigned long start_pfn,unsigned long pfn_32bit) { +/* IOMMUs must support at least the CPU page size */ +BUG_ON (granule >PAGE_SIZE );
??? BUG_ON(granule < PAGE_SIZE);
drivers/iommu/iova.c void free_iova_mem(struct iova *iova) · line 67
Quoted 5 lines
drivers/iommu/iova.c +2
-0+spin_lock_init (&iovad->iova_rbtree_lock); iovad->rbroot =RB_ROOT ; iovad->cached32_node =NULL ; + iovad->granule = granule;
Do you need to check granule is 2^n ?
Quoted 16 lines
drivers/iommu/iova.c +0
-0iovad->start_pfn = start_pfn; iovad->dma_32bit_pfn = pfn_32bit; }include/linux/iova.h +1
-0@@ -28,6 +28,7 @@ struct iova_domain {spinlock_t iova_rbtree_lock;/* Lock to protect update of rbtree */ struct rb_root rbroot;/* iova domain rbtree root */ struct rb_node *cached32_node;/* Save last alloced node */ +unsigned long granule;/* pfn granularity for this domain */ unsigned long start_pfn;/* Lower limit for this domain */ unsigned long dma_32bit_pfn; };
include/linux/iova.h static inline unsigned long iova_size(struct iova *iova) · line 43
Quoted 6 lines
include/linux/iova.h +3
-0@@ -37,6 +38,36 @@ static inline unsigned long iova_size(struct iova *iova)return iova->pfn_hi - iova->pfn_lo +1 ; } +static inline unsigned long iova_shift (struct iova_domain *iovad) +{ +return __ffs (iovad->granule);
I think it's good to save the result to a struct member. If we caculate it ervertime, it's too slow.
Quoted 42 lines
include/linux/iova.h +29
-2+} + +static inline unsigned long iova_mask (struct iova_domain *iovad) +{ +return iovad->granule -1 ; +} + +static inline size_t iova_offset (struct iova_domain *iovad,dma_addr_t iova) +{ +return iova &iova_mask (iovad); +} + +static inline size_t iova_align (struct iova_domain *iovad,size_t size) +{ +return ALIGN (size, iovad->granule); +} + +static inline dma_addr_t iova_dma_addr (struct iova_domain *iovad,struct iova *iova) +{ +return (dma_addr_t )iova->pfn_lo <<iova_shift (iovad); +} + +static inline unsigned long iova_pfn (struct iova_domain *iovad,dma_addr_t iova) +{ +return iova >>iova_shift (iovad); +} +int iommu_iova_cache_init (void );void iommu_iova_cache_destroy (void );@@ -50,8 +81,8 @@ struct iova *alloc_iova(struct iova_domain *iovad, unsigned long size,struct iova *reserve_iova (struct iova_domain *iovad,unsigned long pfn_lo,unsigned long pfn_hi);void copy_reserved_iova (struct iova_domain *from,struct iova_domain *to); -void init_iova_domain (struct iova_domain *iovad,unsigned long start_pfn, -unsigned long pfn_32bit); +void init_iova_domain (struct iova_domain *iovad,unsigned long granule, +unsigned long start_pfn,unsigned long pfn_32bit);struct iova *find_iova (struct iova_domain *iovad,unsigned long pfn);void put_iova_domain (struct iova_domain *iovad); struct iova *split_and_remove_iova(struct iova_domain *iovad,
Robin Murphy
maintainerRFC PATCH 1/4 iommu: build iova.c for any IOMMU
In reply to leizhenHi,
Quoted 19 lines · leizhen
Quoted 13 lines · Robin Murphy
In preparation for sharing the IOVA allocator, build it for all IOMMU API users.
Signed-off-by Robin Murphy <[email protected]>--- drivers/iommu/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/iommu/Makefile +0
-0drivers/iommu/Makefile line 1
Quoted 2 lines
drivers/iommu/Makefile +1
-1@@ -1,4 +1,4 @@ -obj-$(CONFIG_IOMMU_API) += iommu.o +obj-$(CONFIG_IOMMU_API) += iommu.o iova.oI'm not a maintainer, so just code review comment.
Quoted 2 lines
I don't known whether or not you will apply iova to all iommu drivers. But if only x86 and arm/arm64, it's no good to force other iommu drivers to include iova.o, because they have not used it yet.
I agree, the "build it anyway and rely on the linker to throw it away" approach is very hacky. As I mentioned in the cover letter, the other series which appeared as I was cleaning this up for posting duplicates patch 1 and 2 here, but in a nicer way, so I'm fully expecting to simply rebase patches 3 and 4 on top of that series.
Thanks,
Robin.
Quoted 27 lines
Maybe it's good to define a new config, like
CONFIG_IOMMU_IOVA.config
INTEL_IOMMU
selectIOMMU_IOVAconfig
ARM_SMMU
selectIOMMU_IOVAobj-$(CONFIG_IOMMU_IOVA) += iova.o
Quoted 13 lines
drivers/iommu/Makefile +1
-1obj-$(CONFIG_IOMMU_API) += iommu-traces.o obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o obj-$(CONFIG_OF_IOMMU) += of_iommu.o@@ -7,7 +7,7 @@ obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o obj-$(CONFIG_ARM_SMMU) += arm-smmu.o obj-$(CONFIG_DMAR_TABLE) += dmar.o -obj-$(CONFIG_INTEL_IOMMU) += iova.o intel-iommu.o +obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o obj-$(CONFIG_IRQ_REMAP) += intel_irq_remapping.o irq_remapping.o obj-$(CONFIG_OMAP_IOMMU) += omap-iommu.o
Robin Murphy
maintainerRFC PATCH 4/4 iommu: make IOVA domain page size explicit
In reply to leizhenHi,
On 26/11/14 07:17, leizhen wrote:
[...]
Quoted 7 lines
Quoted 7 lines
drivers/iommu/iova.c +0
-0@@ -55,12 +55,16 @@ void free_iova_mem(struct iova *iova) }
Quoted 11 lines
drivers/iommu/iova.c void free_iova_mem(struct iova *iova) · line 62
Quoted 8 lines
drivers/iommu/iova.c +4
-2void -init_iova_domain (struct iova_domain *iovad,unsigned long start_pfn, -unsigned long pfn_32bit) +init_iova_domain (struct iova_domain *iovad,unsigned long granule, +unsigned long start_pfn,unsigned long pfn_32bit) { +/* IOMMUs must support at least the CPU page size */ +BUG_ON (granule >PAGE_SIZE );???
BUG_ON(granule < PAGE_SIZE);
Granule represents the smallest page size that the IOMMU can map - we'd never allocate less IOVA space than a single page, so we don't need a unit smaller than that, and a bigger unit would make it problematic to allocate IOVA space for individual pages.
If, for instance, the CPU has 64k pages, and the IOMMU supports 1k tiny pages, granule would be less than PAGE_SIZE, but the IOMMU could happily that 64k as 64 contiguous tiny pages, even if it's a waste of TLB space.
However, say the IOMMU only supports 16k pages and we try to map two noncontiguous 4k CPU pages as a contiguous 8k buffer in IOVA space - this can't work, hence why the _minimum_ IOMMU page size, and thus granule, may never be _bigger_ than a CPU page.
Quoted 8 lines
drivers/iommu/iova.c void free_iova_mem(struct iova *iova) · line 67
Quoted 5 lines
drivers/iommu/iova.c +2
-0+spin_lock_init (&iovad->iova_rbtree_lock); iovad->rbroot =RB_ROOT ; iovad->cached32_node =NULL ; + iovad->granule = granule;Do you need to check granule is 2^n ?
Yes, that would make a lot of sense.
Quoted 19 lines
Quoted 19 lines
drivers/iommu/iova.c +0
-0iovad->start_pfn = start_pfn; iovad->dma_32bit_pfn = pfn_32bit; }include/linux/iova.h +1
-0@@ -28,6 +28,7 @@ struct iova_domain {spinlock_t iova_rbtree_lock;/* Lock to protect update of rbtree */ struct rb_root rbroot;/* iova domain rbtree root */ struct rb_node *cached32_node;/* Save last alloced node */ +unsigned long granule;/* pfn granularity for this domain */ unsigned long start_pfn;/* Lower limit for this domain */ unsigned long dma_32bit_pfn; };@@ -37,6 +38,36 @@ static inline unsigned long iova_size(struct iova *iova)return iova->pfn_hi - iova->pfn_lo +1 ; }
Quoted 6 lines
include/linux/iova.h static inline unsigned long iova_size(struct iova *iova) · line 43
Quoted 3 lines
include/linux/iova.h +3
-0+static inline unsigned long iova_shift (struct iova_domain *iovad) +{ +return __ffs (iovad->granule);I think it's good to save the result to a struct member. If we caculate it ervertime, it's too slow.
I did give some consideration at the time to which of size, shift and mask to store - given the redundancy between them storing more than one seems excessively wasteful - and this made for the nicest-looking code IMO. These IOVA operations are going to be mostly used in map/unmap situations leading to an IOMMU TLB flush anyway, so I'm not sure how performance-critical they really are.
Are we likely to see this code used on architectures where __ffs takes more than a few cycles and has more impact than the general memory bloat of making structures bigger?
Thanks for the review,
Robin.
Quoted 45 lines
Quoted 42 lines
include/linux/iova.h +29
-2+} + +static inline unsigned long iova_mask (struct iova_domain *iovad) +{ +return iovad->granule -1 ; +} + +static inline size_t iova_offset (struct iova_domain *iovad,dma_addr_t iova) +{ +return iova &iova_mask (iovad); +} + +static inline size_t iova_align (struct iova_domain *iovad,size_t size) +{ +return ALIGN (size, iovad->granule); +} + +static inline dma_addr_t iova_dma_addr (struct iova_domain *iovad,struct iova *iova) +{ +return (dma_addr_t )iova->pfn_lo <<iova_shift (iovad); +} + +static inline unsigned long iova_pfn (struct iova_domain *iovad,dma_addr_t iova) +{ +return iova >>iova_shift (iovad); +} +int iommu_iova_cache_init (void );void iommu_iova_cache_destroy (void );@@ -50,8 +81,8 @@ struct iova *alloc_iova(struct iova_domain *iovad, unsigned long size,struct iova *reserve_iova (struct iova_domain *iovad,unsigned long pfn_lo,unsigned long pfn_hi);void copy_reserved_iova (struct iova_domain *from,struct iova_domain *to); -void init_iova_domain (struct iova_domain *iovad,unsigned long start_pfn, -unsigned long pfn_32bit); +void init_iova_domain (struct iova_domain *iovad,unsigned long granule, +unsigned long start_pfn,unsigned long pfn_32bit);struct iova *find_iova (struct iova_domain *iovad,unsigned long pfn);void put_iova_domain (struct iova_domain *iovad); struct iova *split_and_remove_iova(struct iova_domain *iovad,
RFC PATCH 4/4 iommu: make IOVA domain page size explicit
Quoted 23 lines · Robin Murphy
Hi,
On 26/11/14 07:17, leizhen wrote:
[...]Quoted 7 lines
Quoted 7 lines
drivers/iommu/iova.c +0
-0@@ -55,12 +55,16 @@ void free_iova_mem(struct iova *iova) }Quoted 11 lines
drivers/iommu/iova.c void free_iova_mem(struct iova *iova) · line 62
Quoted 8 lines
drivers/iommu/iova.c +4
-2void -init_iova_domain (struct iova_domain *iovad,unsigned long start_pfn, -unsigned long pfn_32bit) +init_iova_domain (struct iova_domain *iovad,unsigned long granule, +unsigned long start_pfn,unsigned long pfn_32bit) { +/* IOMMUs must support at least the CPU page size */ +BUG_ON (granule >PAGE_SIZE );???
BUG_ON(granule < PAGE_SIZE);
Quoted 9 lines
Granule represents the smallest page size that the IOMMU can map - we'd never allocate less IOVA space than a single page, so we don't need a unit smaller than that, and a bigger unit would make it problematic to allocate IOVA space for individual pages.
If, for instance, the CPU has 64k pages, and the IOMMU supports 1k tiny pages, granule would be less than
PAGE_SIZE, but the IOMMU could happily that 64k as 64 contiguous tiny pages, even if it's a waste of TLB space.However, say the IOMMU only supports 16k pages and we try to map two noncontiguous 4k CPU pages as a contiguous 8k buffer in IOVA space - this can't work, hence why the _minimum_ IOMMU page size, and thus granule, may never be _bigger_ than a CPU page.
Oh, I just confused with the comment: /* IOMMUs must support at least the CPU page size */ But the code means at most the CPU page size. Now, I got it.
Quoted 96 lines
Quoted 8 lines
drivers/iommu/iova.c void free_iova_mem(struct iova *iova) · line 67
Quoted 5 lines
drivers/iommu/iova.c +2
-0+spin_lock_init (&iovad->iova_rbtree_lock); iovad->rbroot =RB_ROOT ; iovad->cached32_node =NULL ; + iovad->granule = granule;Do you need to check granule is 2^n ?
Yes, that would make a lot of sense.
Quoted 19 lines
Quoted 19 lines
drivers/iommu/iova.c +0
-0iovad->start_pfn = start_pfn; iovad->dma_32bit_pfn = pfn_32bit; }include/linux/iova.h +1
-0@@ -28,6 +28,7 @@ struct iova_domain {spinlock_t iova_rbtree_lock;/* Lock to protect update of rbtree */ struct rb_root rbroot;/* iova domain rbtree root */ struct rb_node *cached32_node;/* Save last alloced node */ +unsigned long granule;/* pfn granularity for this domain */ unsigned long start_pfn;/* Lower limit for this domain */ unsigned long dma_32bit_pfn; };@@ -37,6 +38,36 @@ static inline unsigned long iova_size(struct iova *iova)return iova->pfn_hi - iova->pfn_lo +1 ; }Quoted 6 lines
include/linux/iova.h static inline unsigned long iova_size(struct iova *iova) · line 43
Quoted 3 lines
include/linux/iova.h +3
-0+static inline unsigned long iova_shift (struct iova_domain *iovad) +{ +return __ffs (iovad->granule);I think it's good to save the result to a struct member. If we caculate it ervertime, it's too slow.
I did give some consideration at the time to which of size, shift and mask to store - given the redundancy between them storing more than one seems excessively wasteful - and this made for the nicest-looking code IMO. These IOVA operations are going to be mostly used in map/unmap situations leading to an IOMMU TLB flush anyway, so I'm not sure how performance-critical they really are.
Are we likely to see this code used on architectures where
__ffstakes more than a few cycles and has more impact than the general memory bloat of making structures bigger?Thanks for the review,
Robin.Quoted 30 lines
Quoted 30 lines
include/linux/iova.h +27
-0+} + +static inline unsigned long iova_mask (struct iova_domain *iovad) +{ +return iovad->granule -1 ; +} + +static inline size_t iova_offset (struct iova_domain *iovad,dma_addr_t iova) +{ +return iova &iova_mask (iovad); +} + +static inline size_t iova_align (struct iova_domain *iovad,size_t size) +{ +return ALIGN (size, iovad->granule); +} + +static inline dma_addr_t iova_dma_addr (struct iova_domain *iovad,struct iova *iova) +{ +return (dma_addr_t )iova->pfn_lo <<iova_shift (iovad); +} + +static inline unsigned long iova_pfn (struct iova_domain *iovad,dma_addr_t iova) +{ +return iova >>iova_shift (iovad); +} +int iommu_iova_cache_init (void );void iommu_iova_cache_destroy (void );Quoted 15 lines
include/linux/iova.h struct iova *alloc_iova(struct iova_domain *iovad, unsigned long size, · line 88
Quoted 12 lines
include/linux/iova.h +2
-2@@ -50,8 +81,8 @@ struct iova *alloc_iova(struct iova_domain *iovad, unsigned long size,struct iova *reserve_iova (struct iova_domain *iovad,unsigned long pfn_lo,unsigned long pfn_hi);void copy_reserved_iova (struct iova_domain *from,struct iova_domain *to); -void init_iova_domain (struct iova_domain *iovad,unsigned long start_pfn, -unsigned long pfn_32bit); +void init_iova_domain (struct iova_domain *iovad,unsigned long granule, +unsigned long start_pfn,unsigned long pfn_32bit);struct iova *find_iova (struct iova_domain *iovad,unsigned long pfn);void put_iova_domain (struct iova_domain *iovad); struct iova *split_and_remove_iova(struct iova_domain *iovad,.
cover Genericise the IOVA allocator
In reply to Robin MurphyHi Robin,
Quoted 15 lines · Robin Murphy
Hi all,
I've been implementing IOMMU DMA mapping for arm64, based on tidied-up parts of the existing
arch/arm/mm/dma-mapping.cwith a clear divide between the arch-specific parts and the general DMA-API to IOMMU-API layer so that that can be shared; similar to what Ritesh started before and was unable to complete[1], but working in the other direction.The first part of that tidy-up involved ripping out the homebrewed IOVA allocator and plumbing in
iova.c, necessitating the changes presented here. The rest is currently sat underarch/arm64for the sake of getting it working quickly with minimal impact - ideally I'd move it out and portarch/armbefore merging, but I don't know quite how impatient people are. Regardless of that decision, this bit stands alone, so here it is.
Quoted 3 lines
Feel free to ignore patches 1 and 2, since I see Sakari has recently posted a more thorough series for that[2], that frankly looks nicer ;) I've merely left them in as context here.
Thank you. Could you review the set, please? I've gotten no comments on it so far.
You probably wouldn't need the 4th patch, and probably this isn't the right list to get that forward anyway.
Quoted 2 lines
[1]:http://thread.gmane.org/gmane.linux.ports.arm.kernel/331299 [2]:http://article.gmane.org/gmane.linux.kernel.iommu/7436
Signature · 4 lines
Kind regards, Sakari Ailus sakari.ailus at linux.intel.com
Joerg Roedel
maintainercover Genericise the IOVA allocator
In reply to Robin MurphyHi Robin,
Quoted 19 lines · Robin Murphy
Hi all,
I've been implementing IOMMU DMA mapping for arm64, based on tidied-up parts of the existing
arch/arm/mm/dma-mapping.cwith a clear divide between the arch-specific parts and the general DMA-API to IOMMU-API layer so that that can be shared; similar to what Ritesh started before and was unable to complete[1], but working in the other direction.The first part of that tidy-up involved ripping out the homebrewed IOVA allocator and plumbing in
iova.c, necessitating the changes presented here. The rest is currently sat underarch/arm64for the sake of getting it working quickly with minimal impact - ideally I'd move it out and portarch/armbefore merging, but I don't know quite how impatient people are. Regardless of that decision, this bit stands alone, so here it is.Feel free to ignore patches 1 and 2, since I see Sakari has recently posted a more thorough series for that[2], that frankly looks nicer ;) I've merely left them in as context here.
Quoted 8 lines
[1]:http://thread.gmane.org/gmane.linux.ports.arm.kernel/331299 [2]:http://article.gmane.org/gmane.linux.kernel.iommu/7436
Robin Murphy (4):
iommu: buildiova.cfor any IOMMU
iommu: consolidate IOVA allocator code
iommu: make IOVA domain low limit flexible
iommu: make IOVA domain page size explicit
Thanks for doing this, I like this patch-set.
I would also appreciate if someone from Intel could have a look at it, David?
Besides, can you please re-post this patch-set rebased to latest upstream with the better versions of patch 1 and 2, please?
I consider to apply these changes then.
Joerg
Robin Murphy
maintainercover Genericise the IOVA allocator
Hi Joerg,
On 12/01/15 15:52, Joerg Roedel wrote:
[...]
Quoted 11 lines
Thanks for doing this, I like this patch-set.
I would also appreciate if someone from Intel could have a look at it, David?
Besides, can you please re-post this patch-set rebased to latest upstream with the better versions of patch 1 and 2, please?
I consider to apply these changes then.
Thanks! Funnily enough, that's on my "things I didn't quite get round to yesterday" list. I have this series rebased onto -rc3 with the comments addressed, and I'm in the middle of a final cleanup and check of the arm64 dma-mapping stuff on top of it. Expect to see both later today.
Robin.