RE: [PATCH] modify the IO_TLB_SEGSIZE to io_tlb_segsize configurable as flexible requirement about SW-IOMMU.

From: Wang, Xiaoming
Date: Thu Feb 05 2015 - 19:10:30 EST



> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@xxxxxxxxxx]
> Sent: Friday, February 6, 2015 3:33 AM
> To: Wang, Xiaoming
> Cc: ralf@xxxxxxxxxxxxxx; boris.ostrovsky@xxxxxxxxxx;
> david.vrabel@xxxxxxxxxx; linux-mips@xxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; xen-devel@xxxxxxxxxxxxxxxxxxxx; akpm@linux-
> foundation.org; linux@xxxxxxxxxxx; lauraa@xxxxxxxxxxxxxx;
> heiko.carstens@xxxxxxxxxx; d.kasatkin@xxxxxxxxxxx;
> takahiro.akashi@xxxxxxxxxx; chris@xxxxxxxxxxxxxxxxxx; pebolle@xxxxxxxxxx; Liu,
> Chuansheng; Zhang, Dongxing
> Subject: Re: [PATCH] modify the IO_TLB_SEGSIZE to io_tlb_segsize
> configurable as flexible requirement about SW-IOMMU.
>
> On Fri, Feb 06, 2015 at 07:01:14AM +0800, xiaomin1 wrote:
> > The maximum of SW-IOMMU is limited to 2^11*128 = 256K.
> > While in different platform and different requirements this seems improper.
> > So modify the IO_TLB_SEGSIZE to io_tlb_segsize as configurable is make
> sense.
>
> More details please. What is the issue you are hitting?
>
Example:
If 1M bytes are requied. There has an error like.
[ 31.474769] dwc3_otg 0000:00:16.0: dwc3_intel_byt_notify_charger_type(): dwc3_intel_byt_notify_charger_type: invalid SDP current!
[ 31.554077] android_work: sent uevent USB_STATE=CONNECTED
[ 31.564244] android_usb gadget: high-speed config #1: android
[ 31.571468] android_work: sent uevent USB_STATE=CONFIGURED
[ 31.942738] DMA: Out of SW-IOMMU space for 1048576 bytes at device gadget
[ 31.950345] Kernel panic - not syncing: DMA: Random memory could be DMA written
[ 31.950345]
[ 31.960170] CPU: 1 PID: 172 Comm: droidboot Tainted: G W 3.10.20-x86_64_byt-g1077f87 #2
[ 31.970086] Hardware name: Intel Corp. VALLEYVIEW C0 PLATFORM/BYT-T FFD8, BIOS BLADE_21.X64.0004.R14.1412311144 FFD8_X64_R_2014_12_31_1151 12/31/2014
[ 31.985053] 0000000000100000 ffff880136c2fc98 ffffffff82967d45 ffff880136c2fd10
[ 31.993327] ffffffff82961761 0000000000000008 ffff880136c2fd20 ffff880136c2fcc0
[ 32.001590] ffffffff829618fb 0000000000000002 ffffffff820aeff9 0000000000008d8c
[ 32.009871] Call Trace:
[ 32.012610] [<ffffffff82967d45>] dump_stack+0x19/0x1b
[ 32.018353] [<ffffffff82961761>] panic+0xc8/0x1d6
[ 32.023707] [<ffffffff829618fb>] ? printk+0x55/0x57
[ 32.029258] [<ffffffff820aeff9>] ? console_unlock+0x1f9/0x460
[ 32.035772] [<ffffffff82347cbe>] swiotlb_map_page+0x12e/0x140
[ 32.042283] [<ffffffff82599d4d>] usb_gadget_map_request+0x16d/0x220
[ 32.049387] [<ffffffff8255ce89>] dwc3_gadget_ep_queue+0x229/0x460
[ 32.056297] [<ffffffff825b4624>] ffs_epfile_io.isra.96+0x3e4/0x520
[ 32.063296] [<ffffffff820e438d>] ? get_parent_ip+0xd/0x50
[ 32.069427] [<ffffffff82975a61>] ? sub_preempt_count+0x71/0x100
[ 32.076142] [<ffffffff825b47b8>] ffs_epfile_read+0x28/0x30
[ 32.082370] [<ffffffff821b6b8c>] vfs_read+0x9c/0x170
[ 32.088014] [<ffffffff821b765d>] SyS_read+0x4d/0xa0
[ 32.093562] [<ffffffff8297b179>] ia32_do_call+0x13/0x13
> >
> > Signed-off-by: Chuansheng Liu <chuansheng.liu@xxxxxxxxx>
> > Signed-off-by: Zhang Dongxing <dongxing.zhang@xxxxxxxxx>
> > Signed-off-by: xiaomin1 <xiaoming.wang@xxxxxxxxx>
> > ---
> > arch/mips/cavium-octeon/dma-octeon.c | 2 +-
> > arch/mips/netlogic/common/nlm-dma.c | 2 +-
> > drivers/xen/swiotlb-xen.c | 6 +++---
> > include/linux/swiotlb.h | 8 +------
> > lib/swiotlb.c | 39 ++++++++++++++++++++++++----------
> > 5 files changed, 34 insertions(+), 23 deletions(-)
> >
> > diff --git a/arch/mips/cavium-octeon/dma-octeon.c
> > b/arch/mips/cavium-octeon/dma-octeon.c
> > index 3778655..a521af6 100644
> > --- a/arch/mips/cavium-octeon/dma-octeon.c
> > +++ b/arch/mips/cavium-octeon/dma-octeon.c
> > @@ -312,7 +312,7 @@ void __init plat_swiotlb_setup(void)
> > swiotlbsize = 64 * (1<<20);
> > #endif
> > swiotlb_nslabs = swiotlbsize >> IO_TLB_SHIFT;
> > - swiotlb_nslabs = ALIGN(swiotlb_nslabs, IO_TLB_SEGSIZE);
> > + swiotlb_nslabs = ALIGN(swiotlb_nslabs, io_tlb_segsize);
> > swiotlbsize = swiotlb_nslabs << IO_TLB_SHIFT;
> >
> > octeon_swiotlb = alloc_bootmem_low_pages(swiotlbsize);
> > diff --git a/arch/mips/netlogic/common/nlm-dma.c
> > b/arch/mips/netlogic/common/nlm-dma.c
> > index f3d4ae8..eeffa8f 100644
> > --- a/arch/mips/netlogic/common/nlm-dma.c
> > +++ b/arch/mips/netlogic/common/nlm-dma.c
> > @@ -99,7 +99,7 @@ void __init plat_swiotlb_setup(void)
> >
> > swiotlbsize = 1 << 20; /* 1 MB for now */
> > swiotlb_nslabs = swiotlbsize >> IO_TLB_SHIFT;
> > - swiotlb_nslabs = ALIGN(swiotlb_nslabs, IO_TLB_SEGSIZE);
> > + swiotlb_nslabs = ALIGN(swiotlb_nslabs, io_tlb_segsize);
> > swiotlbsize = swiotlb_nslabs << IO_TLB_SHIFT;
> >
> > nlm_swiotlb = alloc_bootmem_low_pages(swiotlbsize);
> > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> > index 810ad41..3b3e9fe 100644
> > --- a/drivers/xen/swiotlb-xen.c
> > +++ b/drivers/xen/swiotlb-xen.c
> > @@ -164,11 +164,11 @@ xen_swiotlb_fixup(void *buf, size_t size,
> unsigned long nslabs)
> > dma_addr_t dma_handle;
> > phys_addr_t p = virt_to_phys(buf);
> >
> > - dma_bits = get_order(IO_TLB_SEGSIZE << IO_TLB_SHIFT) +
> PAGE_SHIFT;
> > + dma_bits = get_order(io_tlb_segsize << IO_TLB_SHIFT) + PAGE_SHIFT;
> >
> > i = 0;
> > do {
> > - int slabs = min(nslabs - i, (unsigned long)IO_TLB_SEGSIZE);
> > + int slabs = min(nslabs - i, (unsigned long)io_tlb_segsize);
> >
> > do {
> > rc = xen_create_contiguous_region( @@ -187,7
> +187,7 @@ static
> > unsigned long xen_set_nslabs(unsigned long nr_tbl) {
> > if (!nr_tbl) {
> > xen_io_tlb_nslabs = (64 * 1024 * 1024 >> IO_TLB_SHIFT);
> > - xen_io_tlb_nslabs = ALIGN(xen_io_tlb_nslabs,
> IO_TLB_SEGSIZE);
> > + xen_io_tlb_nslabs = ALIGN(xen_io_tlb_nslabs, io_tlb_segsize);
> > } else
> > xen_io_tlb_nslabs = nr_tbl;
> >
> > diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index
> > e7a018e..13506db 100644
> > --- a/include/linux/swiotlb.h
> > +++ b/include/linux/swiotlb.h
> > @@ -8,13 +8,7 @@ struct dma_attrs;
> > struct scatterlist;
> >
> > extern int swiotlb_force;
> > -
> > -/*
> > - * Maximum allowable number of contiguous slabs to map,
> > - * must be a power of 2. What is the appropriate value ?
> > - * The complexity of {map,unmap}_single is linearly dependent on this
> value.
> > - */
> > -#define IO_TLB_SEGSIZE 128
> > +extern int io_tlb_segsize;
> >
> > /*
> > * log of the size of each IO TLB slab. The number of slabs is
> > command line diff --git a/lib/swiotlb.c b/lib/swiotlb.c index
> > 4abda07..50c415a 100644
> > --- a/lib/swiotlb.c
> > +++ b/lib/swiotlb.c
> > @@ -56,6 +56,15 @@
> > int swiotlb_force;
> >
> > /*
> > + * Maximum allowable number of contiguous slabs to map,
> > + * must be a power of 2. What is the appropriate value ?
> > + * define io_tlb_segsize as a parameter
> > + * which can be changed dynamically in config file for special usage.
> > + * The complexity of {map,unmap}_single is linearly dependent on this
> value.
> > + */
> > +int io_tlb_segsize = 128;
> > +
> > +/*
> > * Used to do a quick range check in swiotlb_tbl_unmap_single and
> > * swiotlb_tbl_sync_single_*, to see if the memory was in fact allocated by
> this
> > * API.
> > @@ -97,12 +106,20 @@ static DEFINE_SPINLOCK(io_tlb_lock); static int
> > late_alloc;
> >
> > static int __init
> > +setup_io_tlb_segsize(char *str)
> > +{
> > + get_option(&str, &io_tlb_segsize);
> > + return 0;
> > +}
> > +__setup("io_tlb_segsize=", setup_io_tlb_segsize);
> > +
> > +static int __init
> > setup_io_tlb_npages(char *str)
> > {
> > if (isdigit(*str)) {
> > io_tlb_nslabs = simple_strtoul(str, &str, 0);
> > - /* avoid tail segment of size < IO_TLB_SEGSIZE */
> > - io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
> > + /* avoid tail segment of size < io_tlb_segsize */
> > + io_tlb_nslabs = ALIGN(io_tlb_nslabs, io_tlb_segsize);
> > }
> > if (*str == ',')
> > ++str;
> > @@ -183,7 +200,7 @@ int __init swiotlb_init_with_tbl(char *tlb,
> > unsigned long nslabs, int verbose)
> >
> > /*
> > * Allocate and initialize the free list array. This array is used
> > - * to find contiguous free memory regions of size up to
> IO_TLB_SEGSIZE
> > + * to find contiguous free memory regions of size up to
> > +io_tlb_segsize
> > * between io_tlb_start and io_tlb_end.
> > */
> > io_tlb_list = memblock_virt_alloc(
> > @@ -193,7 +210,7 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned
> long nslabs, int verbose)
> > PAGE_ALIGN(io_tlb_nslabs *
> sizeof(phys_addr_t)),
> > PAGE_SIZE);
> > for (i = 0; i < io_tlb_nslabs; i++) {
> > - io_tlb_list[i] = IO_TLB_SEGSIZE - OFFSET(i, IO_TLB_SEGSIZE);
> > + io_tlb_list[i] = io_tlb_segsize - OFFSET(i, io_tlb_segsize);
> > io_tlb_orig_addr[i] = INVALID_PHYS_ADDR;
> > }
> > io_tlb_index = 0;
> > @@ -217,7 +234,7 @@ swiotlb_init(int verbose)
> >
> > if (!io_tlb_nslabs) {
> > io_tlb_nslabs = (default_size >> IO_TLB_SHIFT);
> > - io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
> > + io_tlb_nslabs = ALIGN(io_tlb_nslabs, io_tlb_segsize);
> > }
> >
> > bytes = io_tlb_nslabs << IO_TLB_SHIFT; @@ -249,7 +266,7 @@
> > swiotlb_late_init_with_default_size(size_t default_size)
> >
> > if (!io_tlb_nslabs) {
> > io_tlb_nslabs = (default_size >> IO_TLB_SHIFT);
> > - io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
> > + io_tlb_nslabs = ALIGN(io_tlb_nslabs, io_tlb_segsize);
> > }
> >
> > /*
> > @@ -308,7 +325,7 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned
> > long nslabs)
> >
> > /*
> > * Allocate and initialize the free list array. This array is used
> > - * to find contiguous free memory regions of size up to
> IO_TLB_SEGSIZE
> > + * to find contiguous free memory regions of size up to
> > +io_tlb_segsize
> > * between io_tlb_start and io_tlb_end.
> > */
> > io_tlb_list = (unsigned int *)__get_free_pages(GFP_KERNEL, @@ -
> 324,7
> > +341,7 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
> > goto cleanup4;
> >
> > for (i = 0; i < io_tlb_nslabs; i++) {
> > - io_tlb_list[i] = IO_TLB_SEGSIZE - OFFSET(i, IO_TLB_SEGSIZE);
> > + io_tlb_list[i] = io_tlb_segsize - OFFSET(i, io_tlb_segsize);
> > io_tlb_orig_addr[i] = INVALID_PHYS_ADDR;
> > }
> > io_tlb_index = 0;
> > @@ -493,7 +510,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device
> > *hwdev,
> >
> > for (i = index; i < (int) (index + nslots); i++)
> > io_tlb_list[i] = 0;
> > - for (i = index - 1; (OFFSET(i, IO_TLB_SEGSIZE) !=
> IO_TLB_SEGSIZE - 1) && io_tlb_list[i]; i--)
> > + for (i = index - 1; (OFFSET(i, io_tlb_segsize) !=
> io_tlb_segsize -
> > +1) && io_tlb_list[i]; i--)
> > io_tlb_list[i] = ++count;
> > tlb_addr = io_tlb_start + (index << IO_TLB_SHIFT);
> >
> > @@ -571,7 +588,7 @@ void swiotlb_tbl_unmap_single(struct device
> *hwdev, phys_addr_t tlb_addr,
> > */
> > spin_lock_irqsave(&io_tlb_lock, flags);
> > {
> > - count = ((index + nslots) < ALIGN(index + 1, IO_TLB_SEGSIZE) ?
> > + count = ((index + nslots) < ALIGN(index + 1, io_tlb_segsize) ?
> > io_tlb_list[index + nslots] : 0);
> > /*
> > * Step 1: return the slots to the free list, merging the @@ -
> 585,7
> > +602,7 @@ void swiotlb_tbl_unmap_single(struct device *hwdev,
> phys_addr_t tlb_addr,
> > * Step 2: merge the returned slots with the preceding slots,
> > * if available (non zero)
> > */
> > - for (i = index - 1; (OFFSET(i, IO_TLB_SEGSIZE) !=
> IO_TLB_SEGSIZE -1) && io_tlb_list[i]; i--)
> > + for (i = index - 1; (OFFSET(i, io_tlb_segsize) != io_tlb_segsize
> > +-1) && io_tlb_list[i]; i--)
> > io_tlb_list[i] = ++count;
> > }
> > spin_unlock_irqrestore(&io_tlb_lock, flags);
> > --
> > 1.7.9.5
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/