| Message ID | 20210510095026.3477496-5-tientzu@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Restricted DMA | expand |
> +#ifdef CONFIG_DMA_RESTRICTED_POOL > +#include <linux/io.h> > +#include <linux/of.h> > +#include <linux/of_fdt.h> > +#include <linux/of_reserved_mem.h> > +#include <linux/slab.h> > +#endif I don't think any of this belongs into swiotlb.c. Marking swiotlb_init_io_tlb_mem non-static and having all this code in a separate file is probably a better idea. > +#ifdef CONFIG_DMA_RESTRICTED_POOL > +static int rmem_swiotlb_device_init(struct reserved_mem *rmem, > + struct device *dev) > +{ > + struct io_tlb_mem *mem = rmem->priv; > + unsigned long nslabs = rmem->size >> IO_TLB_SHIFT; > + > + if (dev->dma_io_tlb_mem) > + return 0; > + > + /* Since multiple devices can share the same pool, the private data, > + * io_tlb_mem struct, will be initialized by the first device attached > + * to it. > + */ This is not the normal kernel comment style. > +#ifdef CONFIG_ARM > + if (!PageHighMem(pfn_to_page(PHYS_PFN(rmem->base)))) { > + kfree(mem); > + return -EINVAL; > + } > +#endif /* CONFIG_ARM */ And this is weird. Why would ARM have such a restriction? And if we have such rstrictions it absolutely belongs into an arch helper. > + swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, false); > + > + rmem->priv = mem; > + > +#ifdef CONFIG_DEBUG_FS > + if (!debugfs_dir) > + debugfs_dir = debugfs_create_dir("swiotlb", NULL); > + > + swiotlb_create_debugfs(mem, rmem->name, debugfs_dir); Doesn't the debugfs_create_dir belong into swiotlb_create_debugfs? Also please use IS_ENABLEd or a stub to avoid ifdefs like this.
On Mon, May 10, 2021 at 11:03 PM Christoph Hellwig <hch@lst.de> wrote: > > > +#ifdef CONFIG_DMA_RESTRICTED_POOL > > +#include <linux/io.h> > > +#include <linux/of.h> > > +#include <linux/of_fdt.h> > > +#include <linux/of_reserved_mem.h> > > +#include <linux/slab.h> > > +#endif > > I don't think any of this belongs into swiotlb.c. Marking > swiotlb_init_io_tlb_mem non-static and having all this code in a separate > file is probably a better idea. Will do in the next version. > > > +#ifdef CONFIG_DMA_RESTRICTED_POOL > > +static int rmem_swiotlb_device_init(struct reserved_mem *rmem, > > + struct device *dev) > > +{ > > + struct io_tlb_mem *mem = rmem->priv; > > + unsigned long nslabs = rmem->size >> IO_TLB_SHIFT; > > + > > + if (dev->dma_io_tlb_mem) > > + return 0; > > + > > + /* Since multiple devices can share the same pool, the private data, > > + * io_tlb_mem struct, will be initialized by the first device attached > > + * to it. > > + */ > > This is not the normal kernel comment style. Will fix this in the next version. > > > +#ifdef CONFIG_ARM > > + if (!PageHighMem(pfn_to_page(PHYS_PFN(rmem->base)))) { > > + kfree(mem); > > + return -EINVAL; > > + } > > +#endif /* CONFIG_ARM */ > > And this is weird. Why would ARM have such a restriction? And if we have > such rstrictions it absolutely belongs into an arch helper. Now I think the CONFIG_ARM can just be removed? The goal here is to make sure we're using linear map and can safely use phys_to_dma/dma_to_phys. > > > + swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, false); > > + > > + rmem->priv = mem; > > + > > +#ifdef CONFIG_DEBUG_FS > > + if (!debugfs_dir) > > + debugfs_dir = debugfs_create_dir("swiotlb", NULL); > > + > > + swiotlb_create_debugfs(mem, rmem->name, debugfs_dir); > > Doesn't the debugfs_create_dir belong into swiotlb_create_debugfs? Also > please use IS_ENABLEd or a stub to avoid ifdefs like this. Will move it into swiotlb_create_debugfs and use IS_ENABLED in the next version.
diff --git a/include/linux/device.h b/include/linux/device.h index 38a2071cf776..4987608ea4ff 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -416,6 +416,7 @@ struct dev_links_info { * @dma_pools: Dma pools (if dma'ble device). * @dma_mem: Internal for coherent mem override. * @cma_area: Contiguous memory area for dma allocations + * @dma_io_tlb_mem: Internal for swiotlb io_tlb_mem override. * @archdata: For arch-specific additions. * @of_node: Associated device tree node. * @fwnode: Associated device node supplied by platform firmware. @@ -521,6 +522,9 @@ struct device { #ifdef CONFIG_DMA_CMA struct cma *cma_area; /* contiguous memory area for dma allocations */ +#endif +#ifdef CONFIG_DMA_RESTRICTED_POOL + struct io_tlb_mem *dma_io_tlb_mem; #endif /* arch specific additions */ struct dev_archdata archdata; diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index 216854a5e513..03ad6e3b4056 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h @@ -72,7 +72,8 @@ extern enum swiotlb_force swiotlb_force; * range check to see if the memory was in fact allocated by this * API. * @nslabs: The number of IO TLB blocks (in groups of 64) between @start and - * @end. This is command line adjustable via setup_io_tlb_npages. + * @end. For default swiotlb, this is command line adjustable via + * setup_io_tlb_npages. * @used: The number of used IO TLB block. * @list: The free list describing the number of free entries available * from each index. diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index 858475bd6923..4ea027b75013 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -39,6 +39,13 @@ #ifdef CONFIG_DEBUG_FS #include <linux/debugfs.h> #endif +#ifdef CONFIG_DMA_RESTRICTED_POOL +#include <linux/io.h> +#include <linux/of.h> +#include <linux/of_fdt.h> +#include <linux/of_reserved_mem.h> +#include <linux/slab.h> +#endif #include <asm/io.h> #include <asm/dma.h> @@ -687,3 +694,75 @@ static int __init swiotlb_create_default_debugfs(void) late_initcall(swiotlb_create_default_debugfs); #endif + +#ifdef CONFIG_DMA_RESTRICTED_POOL +static int rmem_swiotlb_device_init(struct reserved_mem *rmem, + struct device *dev) +{ + struct io_tlb_mem *mem = rmem->priv; + unsigned long nslabs = rmem->size >> IO_TLB_SHIFT; + + if (dev->dma_io_tlb_mem) + return 0; + + /* Since multiple devices can share the same pool, the private data, + * io_tlb_mem struct, will be initialized by the first device attached + * to it. + */ + if (!mem) { + mem = kzalloc(struct_size(mem, slots, nslabs), GFP_KERNEL); + if (!mem) + return -ENOMEM; +#ifdef CONFIG_ARM + if (!PageHighMem(pfn_to_page(PHYS_PFN(rmem->base)))) { + kfree(mem); + return -EINVAL; + } +#endif /* CONFIG_ARM */ + swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, false); + + rmem->priv = mem; + +#ifdef CONFIG_DEBUG_FS + if (!debugfs_dir) + debugfs_dir = debugfs_create_dir("swiotlb", NULL); + + swiotlb_create_debugfs(mem, rmem->name, debugfs_dir); +#endif /* CONFIG_DEBUG_FS */ + } + + dev->dma_io_tlb_mem = mem; + + return 0; +} + +static void rmem_swiotlb_device_release(struct reserved_mem *rmem, + struct device *dev) +{ + if (dev) + dev->dma_io_tlb_mem = NULL; +} + +static const struct reserved_mem_ops rmem_swiotlb_ops = { + .device_init = rmem_swiotlb_device_init, + .device_release = rmem_swiotlb_device_release, +}; + +static int __init rmem_swiotlb_setup(struct reserved_mem *rmem) +{ + unsigned long node = rmem->fdt_node; + + if (of_get_flat_dt_prop(node, "reusable", NULL) || + of_get_flat_dt_prop(node, "linux,cma-default", NULL) || + of_get_flat_dt_prop(node, "linux,dma-default", NULL) || + of_get_flat_dt_prop(node, "no-map", NULL)) + return -EINVAL; + + rmem->ops = &rmem_swiotlb_ops; + pr_info("Reserved memory: created device swiotlb memory pool at %pa, size %ld MiB\n", + &rmem->base, (unsigned long)rmem->size / SZ_1M); + return 0; +} + +RESERVEDMEM_OF_DECLARE(dma, "restricted-dma-pool", rmem_swiotlb_setup); +#endif /* CONFIG_DMA_RESTRICTED_POOL */
Add the initialization function to create restricted DMA pools from matching reserved-memory nodes. Signed-off-by: Claire Chang <tientzu@chromium.org> --- include/linux/device.h | 4 +++ include/linux/swiotlb.h | 3 +- kernel/dma/swiotlb.c | 79 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 1 deletion(-)