| Message ID | BN3PR0301MB12198CE5D736CDC6A221EDC2EA790@BN3PR0301MB1219.namprd03.prod.outlook.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Dec 05, 2014 at 10:55:11AM +0000, Varun Sethi wrote: > Hi Will, Hi Varun, Thanks for the review! > +static int arm_lpae_init_pte(struct arm_lpae_io_pgtable *data, > + unsigned long iova, phys_addr_t paddr, > + arm_lpae_iopte prot, int lvl, > + arm_lpae_iopte *ptep) > +{ > + arm_lpae_iopte pte = prot; > + > + /* We require an unmap first */ > + if (iopte_leaf(*ptep, lvl)) > + return -EEXIST; > [varun] Instead of returning an error, how about displaying a warning and > replacing the entry? I'd be ok with displaying a warning, but I don't think we should just continue. It indicates a misuse of the IOMMU API and probably a missing TLBI. > +static int __arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned long iova, > + phys_addr_t paddr, size_t size, arm_lpae_iopte prot, > + int lvl, arm_lpae_iopte *ptep) > +{ > + arm_lpae_iopte *cptep, pte; > + void *cookie = data->iop.cookie; > + size_t block_size = ARM_LPAE_BLOCK_SIZE(lvl, data); > + > + /* Find our entry at the current level */ > + ptep += ARM_LPAE_LVL_IDX(iova, lvl, data); > + > + /* If we can install a leaf entry at this level, then do so */ > + if (size == block_size && (size & data->iop.cfg.pgsize_bitmap)) > + return arm_lpae_init_pte(data, iova, paddr, prot, lvl, ptep); > + > + /* We can't allocate tables at the final level */ > + if (WARN_ON(lvl >= ARM_LPAE_MAX_LEVELS - 1)) > + return -EINVAL; > > [varun] A warning message would be helpful. Sure, I can add one. > +static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data, > + int prot) > +{ > + arm_lpae_iopte pte; > + > + if (data->iop.fmt == ARM_LPAE_S1) { > + pte = ARM_LPAE_PTE_AP_UNPRIV | ARM_LPAE_PTE_nG; > + > + if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ)) > + pte |= ARM_LPAE_PTE_AP_RDONLY; > + > + if (prot & IOMMU_CACHE) > + pte |= (ARM_LPAE_MAIR_ATTR_IDX_CACHE > + << ARM_LPAE_PTE_ATTRINDX_SHIFT); > + } else { > + pte = ARM_LPAE_PTE_HAP_FAULT; > + if (prot & IOMMU_READ) > + pte |= ARM_LPAE_PTE_HAP_READ; > + if (prot & IOMMU_WRITE) > + pte |= ARM_LPAE_PTE_HAP_WRITE; > + if (prot & IOMMU_CACHE) > + pte |= ARM_LPAE_PTE_MEMATTR_OIWB; > + else > + pte |= ARM_LPAE_PTE_MEMATTR_NC; > + } > + > + if (prot & IOMMU_NOEXEC) > + pte |= ARM_LPAE_PTE_XN; > + > + return pte; > +} > [[varun]] Do you plan to add a flag to indicate device memory? We had a > discussion about this on the patch submitted by me. May be you can include > that as a part of this patch. That needs to go in as a separate patch. I think you should continue to push that separately! > +static int arm_lpae_split_blk_unmap(struct arm_lpae_io_pgtable *data, > + unsigned long iova, size_t size, > + arm_lpae_iopte prot, int lvl, > + arm_lpae_iopte *ptep, size_t blk_size) { > + unsigned long blk_start, blk_end; > + phys_addr_t blk_paddr; > + arm_lpae_iopte table = 0; > + void *cookie = data->iop.cookie; > + struct iommu_gather_ops *tlb = data->iop.cfg.tlb; > + > + blk_start = iova & ~(blk_size - 1); > + blk_end = blk_start + blk_size; > + blk_paddr = iopte_to_pfn(*ptep, data) << data->pg_shift; > + > + for (; blk_start < blk_end; blk_start += size, blk_paddr += size) { > + arm_lpae_iopte *tablep; > + > + /* Unmap! */ > + if (blk_start == iova) > + continue; > + > + /* __arm_lpae_map expects a pointer to the start of the table */ > + tablep = &table - ARM_LPAE_LVL_IDX(blk_start, lvl, data); > > > [[varun]] Not clear what's happening here. May be I am missing something, > but where is the table allocated? It is allocated in __arm_lpae_map, because the pte will be 0. The subtraction above is to avoid us having to allocate a whole level, just for a single invalid pte. > > + if (__arm_lpae_map(data, blk_start, blk_paddr, size, prot, lvl, > + tablep) < 0) { > > > [[varun]] Again not clear how are we unmapping the range. Index at the > current level should point to a page table (with contiguous block > mappings). Unmap would applied to the mappings at the next level. Unmap > can happen anywhere in the contiguous range. It seems that you are just > creating a subset of the block mapping. We will be unmapping a single entry at the next level, so we basically create a table, then map everything at the next level apart from the part we need to unmap. > +static int __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, > + unsigned long iova, size_t size, int lvl, > + arm_lpae_iopte *ptep) > +{ > + arm_lpae_iopte pte; > + struct iommu_gather_ops *tlb = data->iop.cfg.tlb; > + void *cookie = data->iop.cookie; > + size_t blk_size = ARM_LPAE_BLOCK_SIZE(lvl, data); > + > + ptep += ARM_LPAE_LVL_IDX(iova, lvl, data); > + pte = *ptep; > + > + /* Something went horribly wrong and we ran out of page table */ > + if (WARN_ON(!pte || (lvl == ARM_LPAE_MAX_LEVELS))) > + return 0; > + > + /* If the size matches this level, we're in the right place */ > + if (size == blk_size) { > + *ptep = 0; > + tlb->flush_pgtable(ptep, sizeof(*ptep), cookie); > + > + if (!iopte_leaf(pte, lvl)) { > + /* Also flush any partial walks */ > + tlb->tlb_add_flush(iova, size, false, cookie); > + tlb->tlb_sync(data->iop.cookie); > + ptep = iopte_deref(pte, data); > + __arm_lpae_free_pgtable(data, lvl + 1, ptep); > + } else { > + tlb->tlb_add_flush(iova, size, true, cookie); > + } > + > + return size; > + } else if (iopte_leaf(pte, lvl)) { > + /* > + * Insert a table at the next level to map the old region, > + * minus the part we want to unmap > + */ > [[varun]] Minus could be somwhere in between the contiguous chunk? We > should first break the entire block mapping in to a next level page > mapping and then unmap a chunk. The amount to unmap will match exactly one entry at the next level -- that's enforced by the IOMMU API (and it will also be aligned as such). > +static struct arm_lpae_io_pgtable * > +arm_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg) { > + unsigned long va_bits; > + struct arm_lpae_io_pgtable *data; > + > + arm_lpae_restrict_pgsizes(cfg); > + > + if (!(cfg->pgsize_bitmap & (SZ_4K | SZ_16K | SZ_64K))) > + return NULL; > + > + if (cfg->ias > ARM_LPAE_MAX_ADDR_BITS) > + return NULL; > + > + if (cfg->oas > ARM_LPAE_MAX_ADDR_BITS) > + return NULL; > + > + data = kmalloc(sizeof(*data), GFP_KERNEL); > + if (!data) > + return NULL; > + > + data->pages_per_pgd = 1; > + data->pg_shift = __ffs(cfg->pgsize_bitmap); > + data->bits_per_level = data->pg_shift - ilog2(sizeof(arm_lpae_iopte)); > + > + va_bits = cfg->ias - data->pg_shift; > + data->levels = DIV_ROUND_UP(va_bits, data->bits_per_level); > > [[varun]] Not related to the patch, but this would be applicable to the > CPU tables as well i.e, we can't support 48bit VA with 64 KB page tables, > right? The AR64 memory maps shows possibility of using 6 bits for the > first level page table. Sure we can support 48-bit VAs with 64k pages. Why do you think we can't? > +static struct io_pgtable *arm_lpae_alloc_pgtable_s2(struct io_pgtable_cfg *cfg, > + void *cookie) > +{ > + u64 reg, sl; > + size_t pgd_size; > + struct arm_lpae_io_pgtable *data = arm_lpae_alloc_pgtable(cfg); > + > + if (!data) > + return NULL; > + > + /* > + * Concatenate PGDs at level 1 if possible in order to reduce > + * the depth of the stage-2 walk. > + */ > + if (data->levels == ARM_LPAE_MAX_LEVELS) { > + unsigned long pgd_bits, pgd_pages; > + unsigned long va_bits = cfg->ias - data->pg_shift; > + > + pgd_bits = data->bits_per_level * (data->levels - 1); > + pgd_pages = 1 << (va_bits - pgd_bits); > + if (pgd_pages <= ARM_LPAE_S2_MAX_CONCAT_PAGES) { > + data->pages_per_pgd = pgd_pages; > + data->levels--; > + } > + } > + > [[varun]] Can you point me to some documentation regarding stage 2 page > concatenation. Not sure why this is required? It's all in the ARM ARM. The idea is to reduce the depth of the stage-2 walk, since that can have an impact on performance when it gets too deep (remember that stage-1 table walks are themselves subjected to stage-2 translation). Will
Hi Will, Please find my response inline. Search for "varun". -----Original Message----- From: Will Deacon [mailto:will.deacon@arm.com] Sent: Saturday, December 06, 2014 12:18 AM To: Sethi Varun-B16395 Cc: linux-arm-kernel@lists.infradead.org; iommu@lists.linux-foundation.org; prem.mallappa@broadcom.com; Robin Murphy; lauraa@codeaurora.org; mitchelh@codeaurora.org; laurent.pinchart@ideasonboard.com; joro@8bytes.org; m.szyprowski@samsung.com Subject: Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator On Fri, Dec 05, 2014 at 10:55:11AM +0000, Varun Sethi wrote: > Hi Will, Hi Varun, Thanks for the review! > +static int arm_lpae_init_pte(struct arm_lpae_io_pgtable *data, > + unsigned long iova, phys_addr_t paddr, > + arm_lpae_iopte prot, int lvl, > + arm_lpae_iopte *ptep) { > + arm_lpae_iopte pte = prot; > + > + /* We require an unmap first */ > + if (iopte_leaf(*ptep, lvl)) > + return -EEXIST; > [varun] Instead of returning an error, how about displaying a warning > and replacing the entry? I'd be ok with displaying a warning, but I don't think we should just continue. It indicates a misuse of the IOMMU API and probably a missing TLBI. [[varun]] May not apply now, but what if we are dealing with a case where memory is not pinned? It may be possible to hookup (without an unmap) an iova to a different physical address. Offcourse, tlb invalidation would be required. Could this scenario be relevant in case of stall mode? > +static int __arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned long iova, > + phys_addr_t paddr, size_t size, arm_lpae_iopte prot, > + int lvl, arm_lpae_iopte *ptep) { > + arm_lpae_iopte *cptep, pte; > + void *cookie = data->iop.cookie; > + size_t block_size = ARM_LPAE_BLOCK_SIZE(lvl, data); > + > + /* Find our entry at the current level */ > + ptep += ARM_LPAE_LVL_IDX(iova, lvl, data); > + > + /* If we can install a leaf entry at this level, then do so */ > + if (size == block_size && (size & data->iop.cfg.pgsize_bitmap)) > + return arm_lpae_init_pte(data, iova, paddr, prot, lvl, > + ptep); > + > + /* We can't allocate tables at the final level */ > + if (WARN_ON(lvl >= ARM_LPAE_MAX_LEVELS - 1)) > + return -EINVAL; > > [varun] A warning message would be helpful. Sure, I can add one. > +static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data, > + int prot) { > + arm_lpae_iopte pte; > + > + if (data->iop.fmt == ARM_LPAE_S1) { > + pte = ARM_LPAE_PTE_AP_UNPRIV | ARM_LPAE_PTE_nG; > + > + if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ)) > + pte |= ARM_LPAE_PTE_AP_RDONLY; > + > + if (prot & IOMMU_CACHE) > + pte |= (ARM_LPAE_MAIR_ATTR_IDX_CACHE > + << ARM_LPAE_PTE_ATTRINDX_SHIFT); > + } else { > + pte = ARM_LPAE_PTE_HAP_FAULT; > + if (prot & IOMMU_READ) > + pte |= ARM_LPAE_PTE_HAP_READ; > + if (prot & IOMMU_WRITE) > + pte |= ARM_LPAE_PTE_HAP_WRITE; > + if (prot & IOMMU_CACHE) > + pte |= ARM_LPAE_PTE_MEMATTR_OIWB; > + else > + pte |= ARM_LPAE_PTE_MEMATTR_NC; > + } > + > + if (prot & IOMMU_NOEXEC) > + pte |= ARM_LPAE_PTE_XN; > + > + return pte; > +} > [[varun]] Do you plan to add a flag to indicate device memory? We had > a discussion about this on the patch submitted by me. May be you can > include that as a part of this patch. That needs to go in as a separate patch. I think you should continue to push that separately! > +static int arm_lpae_split_blk_unmap(struct arm_lpae_io_pgtable *data, > + unsigned long iova, size_t size, > + arm_lpae_iopte prot, int lvl, > + arm_lpae_iopte *ptep, size_t blk_size) { > + unsigned long blk_start, blk_end; > + phys_addr_t blk_paddr; > + arm_lpae_iopte table = 0; > + void *cookie = data->iop.cookie; > + struct iommu_gather_ops *tlb = data->iop.cfg.tlb; > + > + blk_start = iova & ~(blk_size - 1); > + blk_end = blk_start + blk_size; > + blk_paddr = iopte_to_pfn(*ptep, data) << data->pg_shift; > + > + for (; blk_start < blk_end; blk_start += size, blk_paddr += size) { > + arm_lpae_iopte *tablep; > + > + /* Unmap! */ > + if (blk_start == iova) > + continue; > + > + /* __arm_lpae_map expects a pointer to the start of the table */ > + tablep = &table - ARM_LPAE_LVL_IDX(blk_start, lvl, > + data); > > > [[varun]] Not clear what's happening here. May be I am missing > something, but where is the table allocated? It is allocated in __arm_lpae_map, because the pte will be 0. The subtraction above is to avoid us having to allocate a whole level, just for a single invalid pte. > > + if (__arm_lpae_map(data, blk_start, blk_paddr, size, prot, lvl, > + tablep) < 0) { > > > [[varun]] Again not clear how are we unmapping the range. Index at the > current level should point to a page table (with contiguous block > mappings). Unmap would applied to the mappings at the next level. > Unmap can happen anywhere in the contiguous range. It seems that you > are just creating a subset of the block mapping. We will be unmapping a single entry at the next level, so we basically create a table, then map everything at the next level apart from the part we need to unmap. [varun] ok, but you could potentially end up splitting mapping to the least possible page size e.g. 4K. You, don't seem to take in to account the possibility of using the block size at the next level. For example, take a case where we have a huge page mapping using 1G page size and we have an unmap request for 4K. We could still split maximum part of the mapping using 2M pages at the next level. The entry where we need to unmap the 4K region would potentially go to the next level. > +static int __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, > + unsigned long iova, size_t size, int lvl, > + arm_lpae_iopte *ptep) { > + arm_lpae_iopte pte; > + struct iommu_gather_ops *tlb = data->iop.cfg.tlb; > + void *cookie = data->iop.cookie; > + size_t blk_size = ARM_LPAE_BLOCK_SIZE(lvl, data); > + > + ptep += ARM_LPAE_LVL_IDX(iova, lvl, data); > + pte = *ptep; > + > + /* Something went horribly wrong and we ran out of page table */ > + if (WARN_ON(!pte || (lvl == ARM_LPAE_MAX_LEVELS))) > + return 0; > + > + /* If the size matches this level, we're in the right place */ > + if (size == blk_size) { > + *ptep = 0; > + tlb->flush_pgtable(ptep, sizeof(*ptep), cookie); > + > + if (!iopte_leaf(pte, lvl)) { > + /* Also flush any partial walks */ > + tlb->tlb_add_flush(iova, size, false, cookie); > + tlb->tlb_sync(data->iop.cookie); > + ptep = iopte_deref(pte, data); > + __arm_lpae_free_pgtable(data, lvl + 1, ptep); > + } else { > + tlb->tlb_add_flush(iova, size, true, cookie); > + } > + > + return size; > + } else if (iopte_leaf(pte, lvl)) { > + /* > + * Insert a table at the next level to map the old region, > + * minus the part we want to unmap > + */ > [[varun]] Minus could be somwhere in between the contiguous chunk? We > should first break the entire block mapping in to a next level page > mapping and then unmap a chunk. The amount to unmap will match exactly one entry at the next level -- that's enforced by the IOMMU API (and it will also be aligned as such). > +static struct arm_lpae_io_pgtable * > +arm_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg) { > + unsigned long va_bits; > + struct arm_lpae_io_pgtable *data; > + > + arm_lpae_restrict_pgsizes(cfg); > + > + if (!(cfg->pgsize_bitmap & (SZ_4K | SZ_16K | SZ_64K))) > + return NULL; > + > + if (cfg->ias > ARM_LPAE_MAX_ADDR_BITS) > + return NULL; > + > + if (cfg->oas > ARM_LPAE_MAX_ADDR_BITS) > + return NULL; > + > + data = kmalloc(sizeof(*data), GFP_KERNEL); > + if (!data) > + return NULL; > + > + data->pages_per_pgd = 1; > + data->pg_shift = __ffs(cfg->pgsize_bitmap); > + data->bits_per_level = data->pg_shift - > + ilog2(sizeof(arm_lpae_iopte)); > + > + va_bits = cfg->ias - data->pg_shift; > + data->levels = DIV_ROUND_UP(va_bits, data->bits_per_level); > > [[varun]] Not related to the patch, but this would be applicable to > the CPU tables as well i.e, we can't support 48bit VA with 64 KB page > tables, right? The AR64 memory maps shows possibility of using 6 bits > for the first level page table. Sure we can support 48-bit VAs with 64k pages. Why do you think we can't? [varun] My concern was with respect to the bits per level, which is uneven for the 64 K page sizes. Just wondering how would things work with 64K pages when we do a 3 level page lookup. > +static struct io_pgtable *arm_lpae_alloc_pgtable_s2(struct io_pgtable_cfg *cfg, > + void *cookie) { > + u64 reg, sl; > + size_t pgd_size; > + struct arm_lpae_io_pgtable *data = > +arm_lpae_alloc_pgtable(cfg); > + > + if (!data) > + return NULL; > + > + /* > + * Concatenate PGDs at level 1 if possible in order to reduce > + * the depth of the stage-2 walk. > + */ > + if (data->levels == ARM_LPAE_MAX_LEVELS) { > + unsigned long pgd_bits, pgd_pages; > + unsigned long va_bits = cfg->ias - data->pg_shift; > + > + pgd_bits = data->bits_per_level * (data->levels - 1); > + pgd_pages = 1 << (va_bits - pgd_bits); > + if (pgd_pages <= ARM_LPAE_S2_MAX_CONCAT_PAGES) { > + data->pages_per_pgd = pgd_pages; > + data->levels--; > + } > + } > + > [[varun]] Can you point me to some documentation regarding stage 2 > page concatenation. Not sure why this is required? It's all in the ARM ARM. The idea is to reduce the depth of the stage-2 walk, since that can have an impact on performance when it gets too deep (remember that stage-1 table walks are themselves subjected to stage-2 translation). [varun] Just curious, why not consider concatenation at stage 1? Thanks, Varun
On Sun, Dec 14, 2014 at 05:45:49PM +0000, Varun Sethi wrote: > Please find my response inline. Search for "varun". This is getting fiddly now that you've already replied once. Any chance you could sort your mail client out, please? > > +static int arm_lpae_init_pte(struct arm_lpae_io_pgtable *data, > > + unsigned long iova, phys_addr_t paddr, > > + arm_lpae_iopte prot, int lvl, > > + arm_lpae_iopte *ptep) { > > + arm_lpae_iopte pte = prot; > > + > > + /* We require an unmap first */ > > + if (iopte_leaf(*ptep, lvl)) > > + return -EEXIST; > > [varun] Instead of returning an error, how about displaying a warning > > and replacing the entry? > > I'd be ok with displaying a warning, but I don't think we should just > continue. It indicates a misuse of the IOMMU API and probably a missing > TLBI. > > [[varun]] May not apply now, but what if we are dealing with a case where > memory is not pinned? It may be possible to hookup (without an unmap) an > iova to a different physical address. Offcourse, tlb invalidation would be > required. Could this scenario be relevant in case of stall mode? If we wanted to support that, then we'd need some new functions for grabbing hold of the entry and manipulating it in a similar way to the CPU side (e.g. pte_mkdirty etc). > > + if (__arm_lpae_map(data, blk_start, blk_paddr, size, prot, lvl, > > + tablep) < 0) { > > > > > > [[varun]] Again not clear how are we unmapping the range. Index at the > > current level should point to a page table (with contiguous block > > mappings). Unmap would applied to the mappings at the next level. > > Unmap can happen anywhere in the contiguous range. It seems that you > > are just creating a subset of the block mapping. > > We will be unmapping a single entry at the next level, so we basically > create a table, then map everything at the next level apart from the part > we need to unmap. > > > [varun] ok, but you could potentially end up splitting mapping to the > least possible page size e.g. 4K. You, don't seem to take in to account > the possibility of using the block size at the next level. For example, > take a case where we have a huge page mapping using 1G page size and we > have an unmap request for 4K. We could still split maximum part of the > mapping using 2M pages at the next level. The entry where we need to unmap > the 4K region would potentially go to the next level. Aha, I see what you mean here, thanks. I'll take a look... > > +static int __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, > > + unsigned long iova, size_t size, int lvl, > > + arm_lpae_iopte *ptep) { > > + arm_lpae_iopte pte; > > + struct iommu_gather_ops *tlb = data->iop.cfg.tlb; > > + void *cookie = data->iop.cookie; > > + size_t blk_size = ARM_LPAE_BLOCK_SIZE(lvl, data); > > + > > + ptep += ARM_LPAE_LVL_IDX(iova, lvl, data); > > + pte = *ptep; > > + > > + /* Something went horribly wrong and we ran out of page table */ > > + if (WARN_ON(!pte || (lvl == ARM_LPAE_MAX_LEVELS))) > > + return 0; > > + > > + /* If the size matches this level, we're in the right place */ > > + if (size == blk_size) { > > + *ptep = 0; > > + tlb->flush_pgtable(ptep, sizeof(*ptep), cookie); > > + > > + if (!iopte_leaf(pte, lvl)) { > > + /* Also flush any partial walks */ > > + tlb->tlb_add_flush(iova, size, false, cookie); > > + tlb->tlb_sync(data->iop.cookie); > > + ptep = iopte_deref(pte, data); > > + __arm_lpae_free_pgtable(data, lvl + 1, ptep); > > + } else { > > + tlb->tlb_add_flush(iova, size, true, cookie); > > + } > > + > > + return size; > > + } else if (iopte_leaf(pte, lvl)) { > > + /* > > + * Insert a table at the next level to map the old region, > > + * minus the part we want to unmap > > + */ > > [[varun]] Minus could be somwhere in between the contiguous chunk? We > > should first break the entire block mapping in to a next level page > > mapping and then unmap a chunk. > > The amount to unmap will match exactly one entry at the next level -- > that's enforced by the IOMMU API (and it will also be aligned as such). > > > +static struct arm_lpae_io_pgtable * > > +arm_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg) { > > + unsigned long va_bits; > > + struct arm_lpae_io_pgtable *data; > > + > > + arm_lpae_restrict_pgsizes(cfg); > > + > > + if (!(cfg->pgsize_bitmap & (SZ_4K | SZ_16K | SZ_64K))) > > + return NULL; > > + > > + if (cfg->ias > ARM_LPAE_MAX_ADDR_BITS) > > + return NULL; > > + > > + if (cfg->oas > ARM_LPAE_MAX_ADDR_BITS) > > + return NULL; > > + > > + data = kmalloc(sizeof(*data), GFP_KERNEL); > > + if (!data) > > + return NULL; > > + > > + data->pages_per_pgd = 1; > > + data->pg_shift = __ffs(cfg->pgsize_bitmap); > > + data->bits_per_level = data->pg_shift - > > + ilog2(sizeof(arm_lpae_iopte)); > > + > > + va_bits = cfg->ias - data->pg_shift; > > + data->levels = DIV_ROUND_UP(va_bits, data->bits_per_level); > > > > [[varun]] Not related to the patch, but this would be applicable to > > the CPU tables as well i.e, we can't support 48bit VA with 64 KB page > > tables, right? The AR64 memory maps shows possibility of using 6 bits > > for the first level page table. > > Sure we can support 48-bit VAs with 64k pages. Why do you think we can't? > > [varun] My concern was with respect to the bits per level, which is > uneven for the 64 K page sizes. Just wondering how would things work with > 64K pages when we do a 3 level page lookup. Well, it's uneven (9) for the 4k case too. Do you actually see an issue here? 48-bit VA with 64k pages gives us: va_bits = (48 - 16) = 32 bits_per_level = (16 - 3) = 13 levels = ceil(32/13) = 3 so the starting level is 1, which resolves 32-(13*2) = 6 bits. Does that make sense? > > +static struct io_pgtable *arm_lpae_alloc_pgtable_s2(struct io_pgtable_cfg *cfg, > > + void *cookie) { > > + u64 reg, sl; > > + size_t pgd_size; > > + struct arm_lpae_io_pgtable *data = > > +arm_lpae_alloc_pgtable(cfg); > > + > > + if (!data) > > + return NULL; > > + > > + /* > > + * Concatenate PGDs at level 1 if possible in order to reduce > > + * the depth of the stage-2 walk. > > + */ > > + if (data->levels == ARM_LPAE_MAX_LEVELS) { > > + unsigned long pgd_bits, pgd_pages; > > + unsigned long va_bits = cfg->ias - data->pg_shift; > > + > > + pgd_bits = data->bits_per_level * (data->levels - 1); > > + pgd_pages = 1 << (va_bits - pgd_bits); > > + if (pgd_pages <= ARM_LPAE_S2_MAX_CONCAT_PAGES) { > > + data->pages_per_pgd = pgd_pages; > > + data->levels--; > > + } > > + } > > + > > [[varun]] Can you point me to some documentation regarding stage 2 > > page concatenation. Not sure why this is required? > > It's all in the ARM ARM. The idea is to reduce the depth of the stage-2 > walk, since that can have an impact on performance when it gets too deep > (remember that stage-1 table walks are themselves subjected to stage-2 > translation). > > [varun] Just curious, why not consider concatenation at stage 1? Well, the architecture doesn't support it. It's not as big a deal at stage 1 anyway, because there is no nesting in that case (unlike stage 2, which is applied to stage 1 table walks). Will
On Mon, Dec 15, 2014 at 01:30:20PM +0000, Will Deacon wrote: > On Sun, Dec 14, 2014 at 05:45:49PM +0000, Varun Sethi wrote: > > [varun] ok, but you could potentially end up splitting mapping to the > > least possible page size e.g. 4K. You, don't seem to take in to account > > the possibility of using the block size at the next level. For example, > > take a case where we have a huge page mapping using 1G page size and we > > have an unmap request for 4K. We could still split maximum part of the > > mapping using 2M pages at the next level. The entry where we need to unmap > > the 4K region would potentially go to the next level. > > Aha, I see what you mean here, thanks. I'll take a look... Scratch that, I think the code is fine as it is. For the case you highlight, we iterate over the 1GB region remapping it using 4k pages, but skipping the one we want to unmap, so I don't think there's a problem (__arm_lpae_map will create the relevant table entries). Will
Hi Will, -----Original Message----- From: Will Deacon [mailto:will.deacon@arm.com] Sent: Monday, December 15, 2014 9:13 PM To: Sethi Varun-B16395 Cc: linux-arm-kernel@lists.infradead.org; iommu@lists.linux-foundation.org; prem.mallappa@broadcom.com; Robin Murphy; lauraa@codeaurora.org; mitchelh@codeaurora.org; laurent.pinchart@ideasonboard.com; joro@8bytes.org; m.szyprowski@samsung.com Subject: Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator On Mon, Dec 15, 2014 at 01:30:20PM +0000, Will Deacon wrote: > On Sun, Dec 14, 2014 at 05:45:49PM +0000, Varun Sethi wrote: > > [varun] ok, but you could potentially end up splitting mapping to > > the least possible page size e.g. 4K. You, don't seem to take in to > > account the possibility of using the block size at the next level. > > For example, take a case where we have a huge page mapping using 1G > > page size and we have an unmap request for 4K. We could still split > > maximum part of the mapping using 2M pages at the next level. The > > entry where we need to unmap the 4K region would potentially go to the next level. > > Aha, I see what you mean here, thanks. I'll take a look... Scratch that, I think the code is fine as it is. For the case you highlight, we iterate over the 1GB region remapping it using 4k pages, but skipping the one we want to unmap, so I don't think there's a problem (__arm_lpae_map will create the relevant table entries). [[varun]] But you can split 1G in 2M mappings and then split up the unmapped region using 4K pages. In this case you split up the entire region using 4K pages. Thanks, Varun
-----Original Message----- From: Will Deacon [mailto:will.deacon@arm.com] Sent: Monday, December 15, 2014 7:00 PM To: Sethi Varun-B16395 Cc: linux-arm-kernel@lists.infradead.org; iommu@lists.linux-foundation.org; prem.mallappa@broadcom.com; Robin Murphy; lauraa@codeaurora.org; mitchelh@codeaurora.org; laurent.pinchart@ideasonboard.com; joro@8bytes.org; m.szyprowski@samsung.com Subject: Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator On Sun, Dec 14, 2014 at 05:45:49PM +0000, Varun Sethi wrote: > Please find my response inline. Search for "varun". This is getting fiddly now that you've already replied once. Any chance you could sort your mail client out, please? [[varun]] Yes I need to do that, this is painful. > > + if (!data) > > + return NULL; > > + > > + data->pages_per_pgd = 1; > > + data->pg_shift = __ffs(cfg->pgsize_bitmap); > > + data->bits_per_level = data->pg_shift - > > + ilog2(sizeof(arm_lpae_iopte)); > > + > > + va_bits = cfg->ias - data->pg_shift; > > + data->levels = DIV_ROUND_UP(va_bits, data->bits_per_level); > > > > [[varun]] Not related to the patch, but this would be applicable to > > the CPU tables as well i.e, we can't support 48bit VA with 64 KB > > page tables, right? The AR64 memory maps shows possibility of using > > 6 bits for the first level page table. > > Sure we can support 48-bit VAs with 64k pages. Why do you think we can't? > > [varun] My concern was with respect to the bits per level, which is > uneven for the 64 K page sizes. Just wondering how would things work > with 64K pages when we do a 3 level page lookup. Well, it's uneven (9) for the 4k case too. Do you actually see an issue here? 48-bit VA with 64k pages gives us: va_bits = (48 - 16) = 32 bits_per_level = (16 - 3) = 13 levels = ceil(32/13) = 3 so the starting level is 1, which resolves 32-(13*2) = 6 bits. Does that make sense? [[varun]]Yes, but what I meant was, is that in case of 4K pages you have 9 bits per level, but for 64K pages you have 6 bits for the first level and 13 each for second and third. So, bits per level would not work in case of 64 K pages? Thanks, Varun
On Mon, Dec 15, 2014 at 04:43:59PM +0000, Varun Sethi wrote: > > Sure we can support 48-bit VAs with 64k pages. Why do you think we can't? > > > > [varun] My concern was with respect to the bits per level, which is > > uneven for the 64 K page sizes. Just wondering how would things work > > with 64K pages when we do a 3 level page lookup. > > Well, it's uneven (9) for the 4k case too. Do you actually see an issue here? > > 48-bit VA with 64k pages gives us: > > va_bits = (48 - 16) = 32 > bits_per_level = (16 - 3) = 13 > levels = ceil(32/13) = 3 > > so the starting level is 1, which resolves 32-(13*2) = 6 bits. > > Does that make sense? > > [[varun]]Yes, but what I meant was, is that in case of 4K pages you have 9 > bits per level, but for 64K pages you have 6 bits for the first level and > 13 each for second and third. So, bits per level would not work in case of > 64 K pages? The current code takes this into account. Will
On Mon, Dec 15, 2014 at 04:35:12PM +0000, Varun Sethi wrote: > -----Original Message----- > From: Will Deacon [mailto:will.deacon@arm.com] > Sent: Monday, December 15, 2014 9:13 PM > To: Sethi Varun-B16395 > Cc: linux-arm-kernel@lists.infradead.org; iommu@lists.linux-foundation.org; prem.mallappa@broadcom.com; Robin Murphy; lauraa@codeaurora.org; mitchelh@codeaurora.org; laurent.pinchart@ideasonboard.com; joro@8bytes.org; m.szyprowski@samsung.com > Subject: Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator > > On Mon, Dec 15, 2014 at 01:30:20PM +0000, Will Deacon wrote: > > On Sun, Dec 14, 2014 at 05:45:49PM +0000, Varun Sethi wrote: > > > [varun] ok, but you could potentially end up splitting mapping to > > > the least possible page size e.g. 4K. You, don't seem to take in to > > > account the possibility of using the block size at the next level. > > > For example, take a case where we have a huge page mapping using 1G > > > page size and we have an unmap request for 4K. We could still split > > > maximum part of the mapping using 2M pages at the next level. The > > > entry where we need to unmap the 4K region would potentially go to the next level. > > > > Aha, I see what you mean here, thanks. I'll take a look... > > Scratch that, I think the code is fine as it is. For the case you > highlight, we iterate over the 1GB region remapping it using 4k pages, but > skipping the one we want to unmap, so I don't think there's a problem > (__arm_lpae_map will create the relevant table entries). > > > [[varun]] But you can split 1G in 2M mappings and then split up the > unmapped region using 4K pages. In this case you split up the entire > region using 4K pages. True, I miss an optimisation opportunity there, but I don't know that it's common enough to care (in the same way that we don't recreate a 1G mapping if you remapped that 4k page back like it was). You could add this by making arm_lpae_split_blk_unmap recursive, if you wanted to. Will
diff --git a/MAINTAINERS b/MAINTAINERS index 0ff630de8a6d..d3ca31b7c960 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1562,6 +1562,7 @@ M: Will Deacon <will.deacon@arm.com> L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) S: Maintained F: drivers/iommu/arm-smmu.c +F: drivers/iommu/io-pgtable-arm.c ARM64 PORT (AARCH64 ARCHITECTURE) M: Catalin Marinas <catalin.marinas@arm.com> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index 0f10554e7114..e1742a0146f8 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -19,6 +19,15 @@ menu "Generic IOMMU Pagetable Support" config IOMMU_IO_PGTABLE bool +config IOMMU_IO_PGTABLE_LPAE + bool "ARMv7/v8 Long Descriptor Format" + select IOMMU_IO_PGTABLE + help + Enable support for the ARM long descriptor pagetable format. + This allocator supports 4K/2M/1G, 16K/32M and 64K/512M page + sizes at both stage-1 and stage-2, as well as address spaces + up to 48-bits in size. + endmenu config OF_IOMMU diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile index aff244c78181..269cdd82b672 100644 --- a/drivers/iommu/Makefile +++ b/drivers/iommu/Makefile @@ -2,6 +2,7 @@ obj-$(CONFIG_IOMMU_API) += iommu.o obj-$(CONFIG_IOMMU_API) += iommu-traces.o obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o +obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o obj-$(CONFIG_OF_IOMMU) += of_iommu.o obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o msm_iommu_dev.o obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c new file mode 100644 index 000000000000..9dbaa2e48424 --- /dev/null +++ b/drivers/iommu/io-pgtable-arm.c @@ -0,0 +1,735 @@ +/* + * CPU-agnostic ARM page table allocator. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + * Copyright (C) 2014 ARM Limited + * + * Author: Will Deacon <will.deacon@arm.com> */ + +#define pr_fmt(fmt) "arm-lpae io-pgtable: " fmt + +#include <linux/iommu.h> +#include <linux/kernel.h> +#include <linux/sizes.h> +#include <linux/slab.h> +#include <linux/types.h> + +#include "io-pgtable.h" + +#define ARM_LPAE_MAX_ADDR_BITS 48 +#define ARM_LPAE_S2_MAX_CONCAT_PAGES 16 +#define ARM_LPAE_MAX_LEVELS 4 + +/* Struct accessors */ +#define io_pgtable_to_data(x) \ + container_of((x), struct arm_lpae_io_pgtable, iop) + +#define io_pgtable_ops_to_pgtable(x) \ + container_of((x), struct io_pgtable, ops) + +#define io_pgtable_ops_to_data(x) \ + io_pgtable_to_data(io_pgtable_ops_to_pgtable(x)) + +/* + * For consistency with the architecture, we always consider + * ARM_LPAE_MAX_LEVELS levels, with the walk starting at level n >=0 +*/ +#define ARM_LPAE_START_LVL(d) (ARM_LPAE_MAX_LEVELS - (d)->levels) + +/* + * Calculate the right shift amount to get to the portion describing +level l + * in a virtual address mapped by the pagetable in d. + */ +#define ARM_LPAE_LVL_SHIFT(l,d) \ + ((((d)->levels - ((l) - ARM_LPAE_START_LVL(d) + 1)) \ + * (d)->bits_per_level) + (d)->pg_shift) + +/* + * Calculate the index at level l used to map virtual address a using +the + * pagetable in d. + */ +#define ARM_LPAE_PGD_IDX(l,d) \ + ((l) == ARM_LPAE_START_LVL(d) ? ilog2((d)->pages_per_pgd) : 0) + +#define ARM_LPAE_LVL_IDX(a,l,d) \ + (((a) >> ARM_LPAE_LVL_SHIFT(l,d)) & \ + ((1 << ((d)->bits_per_level + ARM_LPAE_PGD_IDX(l,d))) - 1)) + +/* Calculate the block/page mapping size at level l for pagetable in d. */ +#define ARM_LPAE_BLOCK_SIZE(l,d) \ + (1 << (ilog2(sizeof(arm_lpae_iopte)) + \ + ((ARM_LPAE_MAX_LEVELS - (l)) * (d)->bits_per_level))) + +/* Page table bits */ +#define ARM_LPAE_PTE_TYPE_SHIFT 0 +#define ARM_LPAE_PTE_TYPE_MASK 0x3 + +#define ARM_LPAE_PTE_TYPE_BLOCK 1 +#define ARM_LPAE_PTE_TYPE_TABLE 3 +#define ARM_LPAE_PTE_TYPE_PAGE 3 + +#define ARM_LPAE_PTE_XN (((arm_lpae_iopte)3) << 53) +#define ARM_LPAE_PTE_AF (((arm_lpae_iopte)1) << 10) +#define ARM_LPAE_PTE_SH_NS (((arm_lpae_iopte)0) << 8) +#define ARM_LPAE_PTE_SH_OS (((arm_lpae_iopte)2) << 8) +#define ARM_LPAE_PTE_SH_IS (((arm_lpae_iopte)3) << 8) +#define ARM_LPAE_PTE_VALID (((arm_lpae_iopte)1) << 0) + +#define ARM_LPAE_PTE_ATTR_LO_MASK (((arm_lpae_iopte)0x3ff) << 2) +/* Ignore the contiguous bit for block splitting */ +#define ARM_LPAE_PTE_ATTR_HI_MASK (((arm_lpae_iopte)6) << 52) +#define ARM_LPAE_PTE_ATTR_MASK (ARM_LPAE_PTE_ATTR_LO_MASK | \ + ARM_LPAE_PTE_ATTR_HI_MASK) + +/* Stage-1 PTE */ +#define ARM_LPAE_PTE_AP_UNPRIV (((arm_lpae_iopte)1) << 6) +#define ARM_LPAE_PTE_AP_RDONLY (((arm_lpae_iopte)2) << 6) +#define ARM_LPAE_PTE_ATTRINDX_SHIFT 2 +#define ARM_LPAE_PTE_nG (((arm_lpae_iopte)1) << 11) + +/* Stage-2 PTE */ +#define ARM_LPAE_PTE_HAP_FAULT (((arm_lpae_iopte)0) << 6) +#define ARM_LPAE_PTE_HAP_READ (((arm_lpae_iopte)1) << 6) +#define ARM_LPAE_PTE_HAP_WRITE (((arm_lpae_iopte)2) << 6) +#define ARM_LPAE_PTE_MEMATTR_OIWB (((arm_lpae_iopte)0xf) << 2) +#define ARM_LPAE_PTE_MEMATTR_NC (((arm_lpae_iopte)0x5) << 2) +#define ARM_LPAE_PTE_MEMATTR_DEV (((arm_lpae_iopte)0x1) << 2) + +/* Register bits */ +#define ARM_LPAE_TCR_EAE (1 << 31) + +#define ARM_LPAE_TCR_TG0_4K (0 << 14) +#define ARM_LPAE_TCR_TG0_64K (1 << 14) +#define ARM_LPAE_TCR_TG0_16K (2 << 14) + +#define ARM_LPAE_TCR_SH0_SHIFT 12 +#define ARM_LPAE_TCR_SH0_MASK 0x3 +#define ARM_LPAE_TCR_SH_NS 0 +#define ARM_LPAE_TCR_SH_OS 2 +#define ARM_LPAE_TCR_SH_IS 3 + +#define ARM_LPAE_TCR_ORGN0_SHIFT 10 +#define ARM_LPAE_TCR_IRGN0_SHIFT 8 +#define ARM_LPAE_TCR_RGN_MASK 0x3 +#define ARM_LPAE_TCR_RGN_NC 0 +#define ARM_LPAE_TCR_RGN_WBWA 1 +#define ARM_LPAE_TCR_RGN_WT 2 +#define ARM_LPAE_TCR_RGN_WB 3 + +#define ARM_LPAE_TCR_SL0_SHIFT 6 +#define ARM_LPAE_TCR_SL0_MASK 0x3 + +#define ARM_LPAE_TCR_T0SZ_SHIFT 0 +#define ARM_LPAE_TCR_SZ_MASK 0xf + +#define ARM_LPAE_TCR_PS_SHIFT 16 +#define ARM_LPAE_TCR_PS_MASK 0x7 + +#define ARM_LPAE_TCR_IPS_SHIFT 32 +#define ARM_LPAE_TCR_IPS_MASK 0x7 + +#define ARM_LPAE_TCR_PS_32_BIT 0x0ULL +#define ARM_LPAE_TCR_PS_36_BIT 0x1ULL +#define ARM_LPAE_TCR_PS_40_BIT 0x2ULL +#define ARM_LPAE_TCR_PS_42_BIT 0x3ULL +#define ARM_LPAE_TCR_PS_44_BIT 0x4ULL +#define ARM_LPAE_TCR_PS_48_BIT 0x5ULL + +#define ARM_LPAE_MAIR_ATTR_SHIFT(n) ((n) << 3) +#define ARM_LPAE_MAIR_ATTR_MASK 0xff +#define ARM_LPAE_MAIR_ATTR_DEVICE 0x04 +#define ARM_LPAE_MAIR_ATTR_NC 0x44 +#define ARM_LPAE_MAIR_ATTR_WBRWA 0xff +#define ARM_LPAE_MAIR_ATTR_IDX_NC 0 +#define ARM_LPAE_MAIR_ATTR_IDX_CACHE 1 +#define ARM_LPAE_MAIR_ATTR_IDX_DEV 2 + +/* IOPTE accessors */ +#define iopte_deref(pte,d) \ + (__va((pte) & ((1ULL << ARM_LPAE_MAX_ADDR_BITS) - 1) \ + & ~((1ULL << (d)->pg_shift) - 1))) + +#define iopte_type(pte,l) \ + (((pte) >> ARM_LPAE_PTE_TYPE_SHIFT) & ARM_LPAE_PTE_TYPE_MASK) + +#define iopte_prot(pte) ((pte) & ARM_LPAE_PTE_ATTR_MASK) + +#define iopte_leaf(pte,l) \ + (l == (ARM_LPAE_MAX_LEVELS - 1) ? \ + (iopte_type(pte,l) == ARM_LPAE_PTE_TYPE_PAGE) : \ + (iopte_type(pte,l) == ARM_LPAE_PTE_TYPE_BLOCK)) + +#define iopte_to_pfn(pte,d) \ + (((pte) & ((1ULL << ARM_LPAE_MAX_ADDR_BITS) - 1)) >> (d)->pg_shift) + +#define pfn_to_iopte(pfn,d) \ + (((pfn) << (d)->pg_shift) & ((1ULL << ARM_LPAE_MAX_ADDR_BITS) - 1)) + +struct arm_lpae_io_pgtable { + struct io_pgtable iop; + + int levels; + int pages_per_pgd; + unsigned long pg_shift; + unsigned long bits_per_level; + + void *pgd; +}; + +typedef u64 arm_lpae_iopte; + +static int arm_lpae_init_pte(struct arm_lpae_io_pgtable *data, + unsigned long iova, phys_addr_t paddr, + arm_lpae_iopte prot, int lvl, + arm_lpae_iopte *ptep) +{ + arm_lpae_iopte pte = prot; + + /* We require an unmap first */ + if (iopte_leaf(*ptep, lvl)) + return -EEXIST; [varun] Instead of returning an error, how about displaying a warning and replacing the entry? + + if (lvl == ARM_LPAE_MAX_LEVELS - 1) + pte |= ARM_LPAE_PTE_TYPE_PAGE; + else + pte |= ARM_LPAE_PTE_TYPE_BLOCK; + + pte |= ARM_LPAE_PTE_AF | ARM_LPAE_PTE_SH_IS; + pte |= pfn_to_iopte(paddr >> data->pg_shift, data); + + *ptep = pte; + data->iop.cfg.tlb->flush_pgtable(ptep, sizeof(*ptep), data->iop.cookie); + return 0; +} + +static int __arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned long iova, + phys_addr_t paddr, size_t size, arm_lpae_iopte prot, + int lvl, arm_lpae_iopte *ptep) +{ + arm_lpae_iopte *cptep, pte; + void *cookie = data->iop.cookie; + size_t block_size = ARM_LPAE_BLOCK_SIZE(lvl, data); + + /* Find our entry at the current level */ + ptep += ARM_LPAE_LVL_IDX(iova, lvl, data); + + /* If we can install a leaf entry at this level, then do so */ + if (size == block_size && (size & data->iop.cfg.pgsize_bitmap)) + return arm_lpae_init_pte(data, iova, paddr, prot, lvl, ptep); + + /* We can't allocate tables at the final level */ + if (WARN_ON(lvl >= ARM_LPAE_MAX_LEVELS - 1)) + return -EINVAL; [varun] A warning message would be helpful. + + /* Grab a pointer to the next level */ + pte = *ptep; + if (!pte) { + cptep = alloc_pages_exact(1UL << data->pg_shift, + GFP_ATOMIC | __GFP_ZERO); + if (!cptep) + return -ENOMEM; + + data->iop.cfg.tlb->flush_pgtable(cptep, 1UL << data->pg_shift, + cookie); + pte = __pa(cptep) | ARM_LPAE_PTE_TYPE_TABLE; + *ptep = pte; + data->iop.cfg.tlb->flush_pgtable(ptep, sizeof(*ptep), cookie); + } else { + cptep = iopte_deref(pte, data); + } + + /* Rinse, repeat */ + return __arm_lpae_map(data, iova, paddr, size, prot, lvl + 1, cptep); +} + +static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data, + int prot) +{ + arm_lpae_iopte pte; + + if (data->iop.fmt == ARM_LPAE_S1) { + pte = ARM_LPAE_PTE_AP_UNPRIV | ARM_LPAE_PTE_nG; + + if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ)) + pte |= ARM_LPAE_PTE_AP_RDONLY; + + if (prot & IOMMU_CACHE) + pte |= (ARM_LPAE_MAIR_ATTR_IDX_CACHE + << ARM_LPAE_PTE_ATTRINDX_SHIFT); + } else { + pte = ARM_LPAE_PTE_HAP_FAULT; + if (prot & IOMMU_READ) + pte |= ARM_LPAE_PTE_HAP_READ; + if (prot & IOMMU_WRITE) + pte |= ARM_LPAE_PTE_HAP_WRITE; + if (prot & IOMMU_CACHE) + pte |= ARM_LPAE_PTE_MEMATTR_OIWB; + else + pte |= ARM_LPAE_PTE_MEMATTR_NC; + } + + if (prot & IOMMU_NOEXEC) + pte |= ARM_LPAE_PTE_XN; + + return pte; +} [[varun]] Do you plan to add a flag to indicate device memory? We had a discussion about this on the patch submitted by me. May be you can include that as a part of this patch. + +static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova, + phys_addr_t paddr, size_t size, int iommu_prot) { + struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops); + arm_lpae_iopte *ptep = data->pgd; + int lvl = ARM_LPAE_START_LVL(data); + arm_lpae_iopte prot; + + /* If no access, then nothing to do */ + if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE))) + return 0; + + prot = arm_lpae_prot_to_pte(data, iommu_prot); + return __arm_lpae_map(data, iova, paddr, size, prot, lvl, ptep); } + +static void __arm_lpae_free_pgtable(struct arm_lpae_io_pgtable *data, int lvl, + arm_lpae_iopte *ptep) +{ + arm_lpae_iopte *start, *end; + unsigned long table_size; + + /* Only leaf entries at the last level */ + if (lvl == ARM_LPAE_MAX_LEVELS - 1) + return; + + table_size = 1UL << data->pg_shift; + if (lvl == ARM_LPAE_START_LVL(data)) + table_size *= data->pages_per_pgd; + + start = ptep; + end = (void *)ptep + table_size; + + while (ptep != end) { + arm_lpae_iopte pte = *ptep++; + + if (!pte || iopte_leaf(pte, lvl)) + continue; + + __arm_lpae_free_pgtable(data, lvl + 1, iopte_deref(pte, data)); + } + + free_pages_exact(start, table_size); +} + +static void arm_lpae_free_pgtable(struct io_pgtable *iop) { + struct arm_lpae_io_pgtable *data = io_pgtable_to_data(iop); + + __arm_lpae_free_pgtable(data, ARM_LPAE_START_LVL(data), data->pgd); + kfree(data); +} + +static int arm_lpae_split_blk_unmap(struct arm_lpae_io_pgtable *data, + unsigned long iova, size_t size, + arm_lpae_iopte prot, int lvl, + arm_lpae_iopte *ptep, size_t blk_size) { + unsigned long blk_start, blk_end; + phys_addr_t blk_paddr; + arm_lpae_iopte table = 0; + void *cookie = data->iop.cookie; + struct iommu_gather_ops *tlb = data->iop.cfg.tlb; + + blk_start = iova & ~(blk_size - 1); + blk_end = blk_start + blk_size; + blk_paddr = iopte_to_pfn(*ptep, data) << data->pg_shift; + + for (; blk_start < blk_end; blk_start += size, blk_paddr += size) { + arm_lpae_iopte *tablep; + + /* Unmap! */ + if (blk_start == iova) + continue; + + /* __arm_lpae_map expects a pointer to the start of the table */ + tablep = &table - ARM_LPAE_LVL_IDX(blk_start, lvl, data); [[varun]] Not clear what's happening here. May be I am missing something, but where is the table allocated? + if (__arm_lpae_map(data, blk_start, blk_paddr, size, prot, lvl, + tablep) < 0) { [[varun]] Again not clear how are we unmapping the range. Index at the current level should point to a page table (with contiguous block mappings). Unmap would applied to the mappings at the next level. Unmap can happen anywhere in the contiguous range. It seems that you are just creating a subset of the block mapping. + if (table) { + /* Free the table we allocated */ + tablep = iopte_deref(table, data); + __arm_lpae_free_pgtable(data, lvl + 1, tablep); + } + return 0; /* Bytes unmapped */ + } + } + + *ptep = table; + tlb->flush_pgtable(ptep, sizeof(*ptep), cookie); + iova &= ~(blk_size - 1); + tlb->tlb_add_flush(iova, blk_size, true, cookie); + return size; +} + +static int __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, + unsigned long iova, size_t size, int lvl, + arm_lpae_iopte *ptep) +{ + arm_lpae_iopte pte; + struct iommu_gather_ops *tlb = data->iop.cfg.tlb; + void *cookie = data->iop.cookie; + size_t blk_size = ARM_LPAE_BLOCK_SIZE(lvl, data); + + ptep += ARM_LPAE_LVL_IDX(iova, lvl, data); + pte = *ptep; + + /* Something went horribly wrong and we ran out of page table */ + if (WARN_ON(!pte || (lvl == ARM_LPAE_MAX_LEVELS))) + return 0; + + /* If the size matches this level, we're in the right place */ + if (size == blk_size) { + *ptep = 0; + tlb->flush_pgtable(ptep, sizeof(*ptep), cookie); + + if (!iopte_leaf(pte, lvl)) { + /* Also flush any partial walks */ + tlb->tlb_add_flush(iova, size, false, cookie); + tlb->tlb_sync(data->iop.cookie); + ptep = iopte_deref(pte, data); + __arm_lpae_free_pgtable(data, lvl + 1, ptep); + } else { + tlb->tlb_add_flush(iova, size, true, cookie); + } + + return size; + } else if (iopte_leaf(pte, lvl)) { + /* + * Insert a table at the next level to map the old region, + * minus the part we want to unmap + */ [[varun]] Minus could be somwhere in between the contiguous chunk? We should first break the entire block mapping in to a next level page mapping and then unmap a chunk. + return arm_lpae_split_blk_unmap(data, iova, size, + iopte_prot(pte), lvl, ptep, + blk_size); + } + + /* Keep on walkin' */ + ptep = iopte_deref(pte, data); + return __arm_lpae_unmap(data, iova, size, lvl + 1, ptep); } + +static int arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova, + size_t size) +{ + size_t unmapped; + struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops); + struct io_pgtable *iop = &data->iop; + arm_lpae_iopte *ptep = data->pgd; + int lvl = ARM_LPAE_START_LVL(data); + + unmapped = __arm_lpae_unmap(data, iova, size, lvl, ptep); + if (unmapped) + iop->cfg.tlb->tlb_sync(iop->cookie); + + return unmapped; +} + +static phys_addr_t arm_lpae_iova_to_phys(struct io_pgtable_ops *ops, + unsigned long iova) +{ + struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops); + arm_lpae_iopte pte, *ptep = data->pgd; + int lvl = ARM_LPAE_START_LVL(data); + + do { + /* Valid IOPTE pointer? */ + if (!ptep) + return 0; + + /* Grab the IOPTE we're interested in */ + pte = *(ptep + ARM_LPAE_LVL_IDX(iova, lvl, data)); + + /* Valid entry? */ + if (!pte) + return 0; + + /* Leaf entry? */ + if (iopte_leaf(pte,lvl)) + goto found_translation; + + /* Take it to the next level */ + ptep = iopte_deref(pte, data); + } while (++lvl < ARM_LPAE_MAX_LEVELS); + + /* Ran out of page tables to walk */ + return 0; + +found_translation: + iova &= ((1 << data->pg_shift) - 1); + return ((phys_addr_t)iopte_to_pfn(pte,data) << data->pg_shift) | iova; +} + +static void arm_lpae_restrict_pgsizes(struct io_pgtable_cfg *cfg) { + unsigned long granule; + + /* + * We need to restrict the supported page sizes to match the + * translation regime for a particular granule. Aim to match + * the CPU page size if possible, otherwise prefer smaller sizes. + * While we're at it, restrict the block sizes to match the + * chosen granule. + */ + if (cfg->pgsize_bitmap & PAGE_SIZE) + granule = PAGE_SIZE; + else if (cfg->pgsize_bitmap & ~PAGE_MASK) + granule = 1UL << __fls(cfg->pgsize_bitmap & ~PAGE_MASK); + else if (cfg->pgsize_bitmap & PAGE_MASK) + granule = 1UL << __ffs(cfg->pgsize_bitmap & PAGE_MASK); + else + granule = 0; + + switch (granule) { + case SZ_4K: + cfg->pgsize_bitmap &= (SZ_4K | SZ_2M | SZ_1G); + break; + case SZ_16K: + cfg->pgsize_bitmap &= (SZ_16K | SZ_32M); + break; + case SZ_64K: + cfg->pgsize_bitmap &= (SZ_64K | SZ_512M); + break; + default: + cfg->pgsize_bitmap = 0; + } +} + +static struct arm_lpae_io_pgtable * +arm_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg) { + unsigned long va_bits; + struct arm_lpae_io_pgtable *data; + + arm_lpae_restrict_pgsizes(cfg); + + if (!(cfg->pgsize_bitmap & (SZ_4K | SZ_16K | SZ_64K))) + return NULL; + + if (cfg->ias > ARM_LPAE_MAX_ADDR_BITS) + return NULL; + + if (cfg->oas > ARM_LPAE_MAX_ADDR_BITS) + return NULL; + + data = kmalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return NULL; + + data->pages_per_pgd = 1; + data->pg_shift = __ffs(cfg->pgsize_bitmap); + data->bits_per_level = data->pg_shift - ilog2(sizeof(arm_lpae_iopte)); + + va_bits = cfg->ias - data->pg_shift; + data->levels = DIV_ROUND_UP(va_bits, data->bits_per_level); [[varun]] Not related to the patch, but this would be applicable to the CPU tables as well i.e, we can't support 48bit VA with 64 KB page tables, right? The AR64 memory maps shows possibility of using 6 bits for the first level page table. + + data->iop.ops = (struct io_pgtable_ops) { + .map = arm_lpae_map, + .unmap = arm_lpae_unmap, + .iova_to_phys = arm_lpae_iova_to_phys, + }; + + return data; +} + +static struct io_pgtable *arm_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg, + void *cookie) +{ + u64 reg; + struct arm_lpae_io_pgtable *data = arm_lpae_alloc_pgtable(cfg); + + if (!data) + return NULL; + + /* TCR */ + reg = ARM_LPAE_TCR_EAE | + (ARM_LPAE_TCR_SH_IS << ARM_LPAE_TCR_SH0_SHIFT) | + (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_IRGN0_SHIFT) | + (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_ORGN0_SHIFT); + + switch (1 << data->pg_shift) { + case SZ_4K: + reg |= ARM_LPAE_TCR_TG0_4K; + break; + case SZ_16K: + reg |= ARM_LPAE_TCR_TG0_16K; + break; + case SZ_64K: + reg |= ARM_LPAE_TCR_TG0_64K; + break; + } + + switch (cfg->oas) { + case 32: + reg |= (ARM_LPAE_TCR_PS_32_BIT << ARM_LPAE_TCR_IPS_SHIFT); + break; + case 36: + reg |= (ARM_LPAE_TCR_PS_36_BIT << ARM_LPAE_TCR_IPS_SHIFT); + break; + case 40: + reg |= (ARM_LPAE_TCR_PS_40_BIT << ARM_LPAE_TCR_IPS_SHIFT); + break; + case 42: + reg |= (ARM_LPAE_TCR_PS_42_BIT << ARM_LPAE_TCR_IPS_SHIFT); + break; + case 44: + reg |= (ARM_LPAE_TCR_PS_44_BIT << ARM_LPAE_TCR_IPS_SHIFT); + break; + case 48: + reg |= (ARM_LPAE_TCR_PS_48_BIT << ARM_LPAE_TCR_IPS_SHIFT); + break; + default: + goto out_free_data; + } + + reg |= (64ULL - cfg->ias) << ARM_LPAE_TCR_T0SZ_SHIFT; + cfg->arm_lpae_s1_cfg.tcr = reg; + + /* MAIRs */ + reg = (ARM_LPAE_MAIR_ATTR_NC + << ARM_LPAE_MAIR_ATTR_SHIFT(ARM_LPAE_MAIR_ATTR_IDX_NC)) | + (ARM_LPAE_MAIR_ATTR_WBRWA + << ARM_LPAE_MAIR_ATTR_SHIFT(ARM_LPAE_MAIR_ATTR_IDX_CACHE)) | + (ARM_LPAE_MAIR_ATTR_DEVICE + << ARM_LPAE_MAIR_ATTR_SHIFT(ARM_LPAE_MAIR_ATTR_IDX_DEV)); + + cfg->arm_lpae_s1_cfg.mair[0] = reg; + cfg->arm_lpae_s1_cfg.mair[1] = 0; + + /* Looking good; allocate a pgd */ + data->pgd = alloc_pages_exact(1UL << data->pg_shift, + GFP_KERNEL | __GFP_ZERO); + if (!data->pgd) + goto out_free_data; + + cfg->tlb->flush_pgtable(data->pgd, (1UL << data->pg_shift), cookie); + + /* TTBRs */ + cfg->arm_lpae_s1_cfg.ttbr[0] = virt_to_phys(data->pgd); + cfg->arm_lpae_s1_cfg.ttbr[1] = 0; + return &data->iop; + +out_free_data: + kfree(data); + return NULL; +} + +static struct io_pgtable *arm_lpae_alloc_pgtable_s2(struct io_pgtable_cfg *cfg, + void *cookie) +{ + u64 reg, sl; + size_t pgd_size; + struct arm_lpae_io_pgtable *data = arm_lpae_alloc_pgtable(cfg); + + if (!data) + return NULL; + + /* + * Concatenate PGDs at level 1 if possible in order to reduce + * the depth of the stage-2 walk. + */ + if (data->levels == ARM_LPAE_MAX_LEVELS) { + unsigned long pgd_bits, pgd_pages; + unsigned long va_bits = cfg->ias - data->pg_shift; + + pgd_bits = data->bits_per_level * (data->levels - 1); + pgd_pages = 1 << (va_bits - pgd_bits); + if (pgd_pages <= ARM_LPAE_S2_MAX_CONCAT_PAGES) { + data->pages_per_pgd = pgd_pages; + data->levels--; + } + } + [[varun]] Can you point me to some documentation regarding stage 2 page concatenation. Not sure why this is required? + /* VTCR */ + reg = ARM_LPAE_TCR_EAE | + (ARM_LPAE_TCR_SH_IS << ARM_LPAE_TCR_SH0_SHIFT) | + (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_IRGN0_SHIFT) | + (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_ORGN0_SHIFT); + + sl = ARM_LPAE_START_LVL(data); + + switch (1 << data->pg_shift) { + case SZ_4K: + reg |= ARM_LPAE_TCR_TG0_4K; + sl++; /* SL0 format is different for 4K granule size */ + break; + case SZ_16K: + reg |= ARM_LPAE_TCR_TG0_16K; + break; + case SZ_64K: + reg |= ARM_LPAE_TCR_TG0_64K; + break; + } + + switch (cfg->oas) { + case 32: + reg |= (ARM_LPAE_TCR_PS_32_BIT << ARM_LPAE_TCR_PS_SHIFT); + break; + case 36: + reg |= (ARM_LPAE_TCR_PS_36_BIT << ARM_LPAE_TCR_PS_SHIFT); + break; + case 40: + reg |= (ARM_LPAE_TCR_PS_40_BIT << ARM_LPAE_TCR_PS_SHIFT); + break; + case 42: + reg |= (ARM_LPAE_TCR_PS_42_BIT << ARM_LPAE_TCR_PS_SHIFT); + break; + case 44: + reg |= (ARM_LPAE_TCR_PS_44_BIT << ARM_LPAE_TCR_PS_SHIFT); + break; + case 48: + reg |= (ARM_LPAE_TCR_PS_48_BIT << ARM_LPAE_TCR_PS_SHIFT); + break; + default: + goto out_free_data; + } + + reg |= (64ULL - cfg->ias) << ARM_LPAE_TCR_T0SZ_SHIFT; + reg |= (~sl & ARM_LPAE_TCR_SL0_MASK) << ARM_LPAE_TCR_SL0_SHIFT; + cfg->arm_lpae_s2_cfg.vtcr = reg; + + /* Allocate pgd pages */ + pgd_size = data->pages_per_pgd * (1UL << data->pg_shift); + data->pgd = alloc_pages_exact(pgd_size, GFP_KERNEL | __GFP_ZERO); + if (!data->pgd) + goto out_free_data; + + cfg->tlb->flush_pgtable(data->pgd, pgd_size, cookie); + + /* VTTBR */ + cfg->arm_lpae_s2_cfg.vttbr = virt_to_phys(data->pgd); + return &data->iop; + +out_free_data: + kfree(data); + return NULL; +} + +struct io_pgtable_init_fns io_pgtable_arm_lpae_s1_init_fns = { + .alloc = arm_lpae_alloc_pgtable_s1, + .free = arm_lpae_free_pgtable, +}; + +struct io_pgtable_init_fns io_pgtable_arm_lpae_s2_init_fns = { + .alloc = arm_lpae_alloc_pgtable_s2, + .free = arm_lpae_free_pgtable, +}; diff --git a/drivers/iommu/io-pgtable.c b/drivers/iommu/io-pgtable.c index 82e39a0db94b..d0a2016efcb4 100644 --- a/drivers/iommu/io-pgtable.c +++ b/drivers/iommu/io-pgtable.c @@ -25,8 +25,15 @@ #include "io-pgtable.h" +extern struct io_pgtable_init_fns io_pgtable_arm_lpae_s1_init_fns; +extern struct io_pgtable_init_fns io_pgtable_arm_lpae_s2_init_fns; + static struct io_pgtable_init_fns *io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] = { +#ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE + [ARM_LPAE_S1] = &io_pgtable_arm_lpae_s1_init_fns, + [ARM_LPAE_S2] = &io_pgtable_arm_lpae_s2_init_fns, #endif }; struct io_pgtable_ops *alloc_io_pgtable_ops(enum io_pgtable_fmt fmt, diff --git a/drivers/iommu/io-pgtable.h b/drivers/iommu/io-pgtable.h index 5ae75d9cae50..c1cff3d045db 100644 --- a/drivers/iommu/io-pgtable.h +++ b/drivers/iommu/io-pgtable.h @@ -33,10 +33,22 @@ struct io_pgtable_cfg {