| Message ID | 20210518064215.2856977-5-tientzu@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Restricted DMA | expand |
I didn't move this to a separate file because I feel it might be confusing for swiotlb_alloc/free (and need more functions to be non-static). Maybe instead of moving to a separate file, we can try to come up with a better naming?
On 5/17/2021 11:42 PM, Claire Chang wrote: > 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 | 76 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 82 insertions(+), 1 deletion(-) > > 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 b849b01a446f..1d8eb4de0d01 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> > @@ -690,3 +697,72 @@ 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; > + > + if (PageHighMem(pfn_to_page(PHYS_PFN(rmem->base)))) { > + kfree(mem); > + return -EINVAL; This could probably deserve a warning here to indicate that the reserved area must be accessible within the linear mapping as I would expect a lot of people to trip over that. Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
On Thu, May 20, 2021 at 2:54 AM Florian Fainelli <f.fainelli@gmail.com> wrote: > > > > On 5/17/2021 11:42 PM, Claire Chang wrote: > > 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 | 76 +++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 82 insertions(+), 1 deletion(-) > > > > 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 b849b01a446f..1d8eb4de0d01 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> > > @@ -690,3 +697,72 @@ 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; > > + > > + if (PageHighMem(pfn_to_page(PHYS_PFN(rmem->base)))) { > > + kfree(mem); > > + return -EINVAL; > > This could probably deserve a warning here to indicate that the reserved > area must be accessible within the linear mapping as I would expect a > lot of people to trip over that. Ok. Will add it. > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> > -- > Florian
On Tue, May 18, 2021 at 02:48:35PM +0800, Claire Chang wrote: > I didn't move this to a separate file because I feel it might be > confusing for swiotlb_alloc/free (and need more functions to be > non-static). > Maybe instead of moving to a separate file, we can try to come up with > a better naming? I think you are referring to: rmem_swiotlb_setup ? Which is ARM specific and inside the generic code? <sigh> Christopher wants to unify it in all the code so there is one single source, but the "you seperate arch code out from generic" saying makes me want to move it out. I agree that if you move it out from generic to arch-specific we have to expose more of the swiotlb functions, which will undo's Christopher cleanup code. How about this - lets leave it as is now, and when there are more use-cases we can revisit it and then if need to move the code?
On Mon, May 24, 2021 at 11:49 PM Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > > On Tue, May 18, 2021 at 02:48:35PM +0800, Claire Chang wrote: > > I didn't move this to a separate file because I feel it might be > > confusing for swiotlb_alloc/free (and need more functions to be > > non-static). > > Maybe instead of moving to a separate file, we can try to come up with > > a better naming? > > I think you are referring to: > > rmem_swiotlb_setup > > ? Yes, and the following swiotlb_alloc/free. > > Which is ARM specific and inside the generic code? > > <sigh> > > Christopher wants to unify it in all the code so there is one single > source, but the "you seperate arch code out from generic" saying > makes me want to move it out. > > I agree that if you move it out from generic to arch-specific we have to > expose more of the swiotlb functions, which will undo's Christopher > cleanup code. > > How about this - lets leave it as is now, and when there are more > use-cases we can revisit it and then if need to move the code? > Ok! Sounds good!
On Mon, May 24, 2021 at 11:49:34AM -0400, Konrad Rzeszutek Wilk wrote: > rmem_swiotlb_setup > > ? > > Which is ARM specific and inside the generic code? I don't think it is arm specific at all. It is OF specific, but just about every platform but x86 uses OF. And I can think of an ACPI version of this as well.
I'd still much prefer to always have the pointer in struct device.
Especially as we're also looking into things like a global 64-bit bounce
buffer. Something like this untested patch ontop of your series:
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 628e33939aca..3cb95fa29f70 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -29,6 +29,7 @@
#include <linux/sched/mm.h>
#include <linux/sysfs.h>
#include <linux/dma-map-ops.h> /* for dma_default_coherent */
+#include <linux/swiotlb.h>
#include "base.h"
#include "power/power.h"
@@ -2814,6 +2815,9 @@ void device_initialize(struct device *dev)
defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
dev->dma_coherent = dma_default_coherent;
#endif
+#ifdef CONFIG_SWIOTLB
+ dev->dma_io_tlb_mem = &io_tlb_default_mem;
+#endif
}
EXPORT_SYMBOL_GPL(device_initialize);
diff --git a/include/linux/device.h b/include/linux/device.h
index 4987608ea4ff..6aca6fa0facc 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -416,7 +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.
+ * @dma_io_tlb_mem: Pointer to the swiotlb pool used. Not for driver use.
* @archdata: For arch-specific additions.
* @of_node: Associated device tree node.
* @fwnode: Associated device node supplied by platform firmware.
@@ -523,7 +523,7 @@ struct device {
struct cma *cma_area; /* contiguous memory area for dma
allocations */
#endif
-#ifdef CONFIG_DMA_RESTRICTED_POOL
+#ifdef CONFIG_SWIOTLB
struct io_tlb_mem *dma_io_tlb_mem;
#endif
/* arch specific additions */
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index e8cf49bd90c5..c153cd0c469c 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -95,6 +95,7 @@ struct io_tlb_mem {
spinlock_t lock;
struct dentry *debugfs;
bool late_alloc;
+ bool force_swiotlb;
struct io_tlb_slot {
phys_addr_t orig_addr;
size_t alloc_size;
@@ -103,30 +104,16 @@ struct io_tlb_mem {
};
extern struct io_tlb_mem *io_tlb_default_mem;
-static inline struct io_tlb_mem *get_io_tlb_mem(struct device *dev)
-{
-#ifdef CONFIG_DMA_RESTRICTED_POOL
- if (dev && dev->dma_io_tlb_mem)
- return dev->dma_io_tlb_mem;
-#endif /* CONFIG_DMA_RESTRICTED_POOL */
-
- return io_tlb_default_mem;
-}
-
static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
{
- struct io_tlb_mem *mem = get_io_tlb_mem(dev);
+ struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
return mem && paddr >= mem->start && paddr < mem->end;
}
static inline bool is_dev_swiotlb_force(struct device *dev)
{
-#ifdef CONFIG_DMA_RESTRICTED_POOL
- if (dev->dma_io_tlb_mem)
- return true;
-#endif /* CONFIG_DMA_RESTRICTED_POOL */
- return false;
+ return dev->dma_io_tlb_mem->force_swiotlb;
}
void __init swiotlb_exit(void);
@@ -134,10 +121,8 @@ unsigned int swiotlb_max_segment(void);
size_t swiotlb_max_mapping_size(struct device *dev);
bool is_swiotlb_active(struct device *dev);
void __init swiotlb_adjust_size(unsigned long size);
-#ifdef CONFIG_DMA_RESTRICTED_POOL
struct page *swiotlb_alloc(struct device *dev, size_t size);
bool swiotlb_free(struct device *dev, struct page *page, size_t size);
-#endif /* CONFIG_DMA_RESTRICTED_POOL */
#else
#define swiotlb_force SWIOTLB_NO_FORCE
static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index d3fa4669229b..69d62e18f493 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -347,7 +347,7 @@ void __init swiotlb_exit(void)
static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t size,
enum dma_data_direction dir)
{
- struct io_tlb_mem *mem = get_io_tlb_mem(dev);
+ struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
int index = (tlb_addr - mem->start) >> IO_TLB_SHIFT;
phys_addr_t orig_addr = mem->slots[index].orig_addr;
size_t alloc_size = mem->slots[index].alloc_size;
@@ -429,7 +429,7 @@ static unsigned int wrap_index(struct io_tlb_mem *mem, unsigned int index)
static int find_slots(struct device *dev, phys_addr_t orig_addr,
size_t alloc_size)
{
- struct io_tlb_mem *mem = get_io_tlb_mem(dev);
+ struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
unsigned long boundary_mask = dma_get_seg_boundary(dev);
dma_addr_t tbl_dma_addr =
phys_to_dma_unencrypted(dev, mem->start) & boundary_mask;
@@ -510,7 +510,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
size_t mapping_size, size_t alloc_size,
enum dma_data_direction dir, unsigned long attrs)
{
- struct io_tlb_mem *mem = get_io_tlb_mem(dev);
+ struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
unsigned int offset = swiotlb_align_offset(dev, orig_addr);
unsigned int i;
int index;
@@ -553,7 +553,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
static void release_slots(struct device *dev, phys_addr_t tlb_addr)
{
- struct io_tlb_mem *mem = get_io_tlb_mem(dev);
+ struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
unsigned long flags;
unsigned int offset = swiotlb_align_offset(dev, tlb_addr);
int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT;
@@ -670,7 +670,7 @@ size_t swiotlb_max_mapping_size(struct device *dev)
bool is_swiotlb_active(struct device *dev)
{
- return get_io_tlb_mem(dev) != NULL;
+ return dev->dma_io_tlb_mem;
}
EXPORT_SYMBOL_GPL(is_swiotlb_active);
@@ -741,7 +741,7 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
struct io_tlb_mem *mem = rmem->priv;
unsigned long nslabs = rmem->size >> IO_TLB_SHIFT;
- if (dev->dma_io_tlb_mem)
+ if (dev->dma_io_tlb_mem != io_tlb_default_mem)
return 0;
/*
@@ -760,6 +760,7 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
}
swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, false);
+ mem->force_swiotlb = true;
rmem->priv = mem;
@@ -768,15 +769,13 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
}
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;
+ dev->dma_io_tlb_mem = io_tlb_default_mem;
}
static const struct reserved_mem_ops rmem_swiotlb_ops = {
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 b849b01a446f..1d8eb4de0d01 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> @@ -690,3 +697,72 @@ 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; + + if (PageHighMem(pfn_to_page(PHYS_PFN(rmem->base)))) { + kfree(mem); + return -EINVAL; + } + + swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, false); + + rmem->priv = mem; + + if (IS_ENABLED(CONFIG_DEBUG_FS)) + swiotlb_create_debugfs(mem, rmem->name); + } + + 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 | 76 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-)