On 18/08/2020 09:40, Leonardo Bras wrote:
> Both iommu_alloc_coherent() and iommu_free_coherent() assume that once
> size is aligned to PAGE_SIZE it will be aligned to IOMMU_PAGE_SIZE.
The only case when it is not aligned is when IOMMU_PAGE_SIZE > PAGE_SIZE
which is unlikely but not impossible, we could configure the kernel for
4K system pages and 64K IOMMU pages I suppose. Do we really want to do
this here, or simply put WARN_ON(tbl->it_page_shift > PAGE_SHIFT)?
Because if we want the former (==support), then we'll have to align the
size up to the bigger page size when allocating/zeroing system pages,
etc. Bigger pages are not the case here as I understand it.
>
> Update those functions to guarantee alignment with requested size
> using IOMMU_PAGE_ALIGN() before doing iommu_alloc() / iommu_free().
>
> Also, on iommu_range_alloc(), replace ALIGN(n, 1 << tbl->it_page_shift)
> with IOMMU_PAGE_ALIGN(n, tbl), which seems easier to read.
>
> Signed-off-by: Leonardo Bras <leobra...@gmail.com>
> ---
> arch/powerpc/kernel/iommu.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index 9704f3f76e63..d7086087830f 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -237,10 +237,9 @@ static unsigned long iommu_range_alloc(struct device
> *dev,
> }
>
> if (dev)
> - boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
> - 1 << tbl->it_page_shift);
> + boundary_size = IOMMU_PAGE_ALIGN(dma_get_seg_boundary(dev) + 1,
> tbl);
Run checkpatch.pl, should complain about a long line.
> else
> - boundary_size = ALIGN(1UL << 32, 1 << tbl->it_page_shift);
> + boundary_size = IOMMU_PAGE_ALIGN(1UL << 32, tbl);
> /* 4GB boundary for iseries_hv_alloc and iseries_hv_map */
>
> n = iommu_area_alloc(tbl->it_map, limit, start, npages, tbl->it_offset,
> @@ -858,6 +857,7 @@ void *iommu_alloc_coherent(struct device *dev, struct
> iommu_table *tbl,
> unsigned int order;
> unsigned int nio_pages, io_order;
> struct page *page;
> + size_t size_io = size;
>
> size = PAGE_ALIGN(size);
> order = get_order(size);
> @@ -884,8 +884,9 @@ void *iommu_alloc_coherent(struct device *dev, struct
> iommu_table *tbl,
> memset(ret, 0, size);
>
> /* Set up tces to cover the allocated range */
> - nio_pages = size >> tbl->it_page_shift;
> - io_order = get_iommu_order(size, tbl);
> + size_io = IOMMU_PAGE_ALIGN(size_io, tbl);
> + nio_pages = size_io >> tbl->it_page_shift;
> + io_order = get_iommu_order(size_io, tbl);
> mapping = iommu_alloc(dev, tbl, ret, nio_pages, DMA_BIDIRECTIONAL,
> mask >> tbl->it_page_shift, io_order, 0);
> if (mapping == DMA_MAPPING_ERROR) {
> @@ -900,11 +901,11 @@ void iommu_free_coherent(struct iommu_table *tbl,
> size_t size,
> void *vaddr, dma_addr_t dma_handle)
> {
> if (tbl) {
> - unsigned int nio_pages;
> + size_t size_io = IOMMU_PAGE_ALIGN(size, tbl);
> + unsigned int nio_pages = size_io >> tbl->it_page_shift;
>
> - size = PAGE_ALIGN(size);
> - nio_pages = size >> tbl->it_page_shift;
> iommu_free(tbl, dma_handle, nio_pages);
> +
Unrelated new line.
> size = PAGE_ALIGN(size);
> free_pages((unsigned long)vaddr, get_order(size));
> }
>
--
Alexey
