[PATCH] arm64:swiotlb:Enable only when Input size through command line

Subsystems: arm64 port (aarch64 architecture), dma mapping helpers, documentation, library code, the rest

STALE3748d REVIEWED: 2 (0M)

2 review trailers.

8 messages, 7 authors, 2016-06-24 · open the first message on its own page

[PATCH] arm64:swiotlb:Enable only when Input size through command line

From: Manjeet Pawar <hidden>
Date: 2016-06-23 12:14:53

From: Rohit Thapliyal <redacted>

swiotlb default size of 64M is too big as
default value therefore it is made configurable
through command line through swiotlb_size parameter.
swiotlb allocation shall be done only when the
swiotlb size is given through command line.
Otherwise no swiotlb is allocated.

Signed-off-by: Rohit Thapliyal <redacted>
Signed-off-by: Manjeet Pawar <redacted>
Reviewed-by: Akhilesh Kumar <redacted>
Reviewed-by: Ajeet Kumar Yadav <redacted>
---
 Documentation/kernel-parameters.txt |    3 +++
 arch/arm64/mm/init.c                |    3 ++-
 include/linux/swiotlb.h             |    1 +
 lib/swiotlb.c                       |   33 +++++++++++++++++++++++++++++----
 4 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 82b42c9..12b680f 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt

@@ -3875,6 +3875,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			force -- force using of bounce buffers even if they
 			         wouldn't be automatically used by the kernel
 
+	swiotlb_sz= [KNL] enter swiotlb size.
+			Sets the swiotlb size for eg. swiotlb_sz=64M
+
 	switches=	[HW,M68k]
 
 	sysfs.deprecated=0|1 [KNL]

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index d45f862..89c6b39 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c

@@ -403,7 +403,8 @@ static void __init free_unused_memmap(void)
  */
 void __init mem_init(void)
 {
-	swiotlb_init(1);
+	if (swiotlb_enabled)
+		swiotlb_init(1);
 
 	set_max_mapnr(pfn_to_page(max_pfn) - mem_map);

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 017fced..c7eb146 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h

@@ -11,6 +11,7 @@ struct page;
 struct scatterlist;
 
 extern int swiotlb_force;
+extern int swiotlb_enabled;
 
 /*
  * Maximum allowable number of contiguous slabs to map,

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 76f29ec..e89296a 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c

@@ -54,6 +54,7 @@
 #define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT)
 
 int swiotlb_force;
+int swiotlb_enabled;
 
 /*
  * Used to do a quick range check in swiotlb_tbl_unmap_single and

@@ -96,6 +97,9 @@ static DEFINE_SPINLOCK(io_tlb_lock);
 
 static int late_alloc;
 
+unsigned long swiotlb_sz;
+unsigned int swiotlb_sz_shift;
+
 static int __init
 setup_io_tlb_npages(char *str)
 {

@@ -112,6 +116,24 @@ setup_io_tlb_npages(char *str)
 	return 0;
 }
 early_param("swiotlb", setup_io_tlb_npages);
+
+static int __init
+setup_io_tlb_size(char *str)
+{
+	int len = strlen(str);
+
+	if (str[len-1] == 'M')
+		swiotlb_sz_shift = 20;
+	else if (str[len-1] == 'K')
+		swiotlb_sz_shift = 10;
+	str[len-1] = '\0';
+	if (isdigit(*str))
+		swiotlb_sz = kstrtoul(str, &str, 0);
+
+	swiotlb_enabled = 1;
+	return 0;
+}
+early_param("swiotlb_sz", setup_io_tlb_size);
 /* make io_tlb_overflow tunable too? */
 
 unsigned long swiotlb_nr_tbl(void)

@@ -120,8 +142,9 @@ unsigned long swiotlb_nr_tbl(void)
 }
 EXPORT_SYMBOL_GPL(swiotlb_nr_tbl);
 
-/* default to 64MB */
-#define IO_TLB_DEFAULT_SIZE (64UL<<20)
+/* Pass from command line as swiotlb_sz=64M (for eg.)*/
+#define IO_TLB_DEFAULT_SIZE (swiotlb_sz<<swiotlb_sz_shift)
+
 unsigned long swiotlb_size_or_default(void)
 {
 	unsigned long size;

@@ -153,10 +176,12 @@ void swiotlb_print_info(void)
 	vstart = phys_to_virt(io_tlb_start);
 	vend = phys_to_virt(io_tlb_end);
 
-	printk(KERN_INFO "software IO TLB [mem %#010llx-%#010llx] (%luMB) mapped at [%p-%p]\n",
+	pr_info("software IO TLB [mem %#010llx-%#010llx] (%lu%cB) mapped@[%p-%p]\n",
 	       (unsigned long long)io_tlb_start,
 	       (unsigned long long)io_tlb_end,
-	       bytes >> 20, vstart, vend - 1);
+		bytes >> swiotlb_sz_shift,
+		swiotlb_sz_shift == 20 ? 'M' : 'K',
+		vstart, vend - 1);
 }
 
 int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)

-- 
1.7.9.5

Re: [PATCH] arm64:swiotlb:Enable only when Input size through command line

From: Robin Murphy <robin.murphy@arm.com>
Date: 2016-06-23 12:31:30

On 23/06/16 13:13, Manjeet Pawar wrote:

From: Rohit Thapliyal <redacted>

swiotlb default size of 64M is too big as
default value therefore it is made configurable
through command line through swiotlb_size parameter.
swiotlb allocation shall be done only when the
swiotlb size is given through command line.
Otherwise no swiotlb is allocated.

So all platforms with most memory physically above 4GB (which is quite a 
lot of them) are suddenly broken unless they go and muck about with 
their bootloader?

If anyone's got to muck about with their bootloader, why can't it be the 
memory-constrained platforms just passing "swiotlb=1" instead?

Robin.

quoted hunk

Signed-off-by: Rohit Thapliyal <redacted>
Signed-off-by: Manjeet Pawar <redacted>
Reviewed-by: Akhilesh Kumar <redacted>
Reviewed-by: Ajeet Kumar Yadav <redacted>
---
  Documentation/kernel-parameters.txt |    3 +++
  arch/arm64/mm/init.c                |    3 ++-
  include/linux/swiotlb.h             |    1 +
  lib/swiotlb.c                       |   33 +++++++++++++++++++++++++++++----
  4 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 82b42c9..12b680f 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt

@@ -3875,6 +3875,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
  			force -- force using of bounce buffers even if they
  			         wouldn't be automatically used by the kernel

+	swiotlb_sz= [KNL] enter swiotlb size.
+			Sets the swiotlb size for eg. swiotlb_sz=64M
+
  	switches=	[HW,M68k]

  	sysfs.deprecated=0|1 [KNL]

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index d45f862..89c6b39 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c

@@ -403,7 +403,8 @@ static void __init free_unused_memmap(void)
   */
  void __init mem_init(void)
  {
-	swiotlb_init(1);
+	if (swiotlb_enabled)
+		swiotlb_init(1);

  	set_max_mapnr(pfn_to_page(max_pfn) - mem_map);

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 017fced..c7eb146 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h

@@ -11,6 +11,7 @@ struct page;
  struct scatterlist;

  extern int swiotlb_force;
+extern int swiotlb_enabled;

  /*
   * Maximum allowable number of contiguous slabs to map,

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 76f29ec..e89296a 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c

@@ -54,6 +54,7 @@
  #define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT)

  int swiotlb_force;
+int swiotlb_enabled;

  /*
   * Used to do a quick range check in swiotlb_tbl_unmap_single and

@@ -96,6 +97,9 @@ static DEFINE_SPINLOCK(io_tlb_lock);

  static int late_alloc;

+unsigned long swiotlb_sz;
+unsigned int swiotlb_sz_shift;
+
  static int __init
  setup_io_tlb_npages(char *str)
  {

@@ -112,6 +116,24 @@ setup_io_tlb_npages(char *str)
  	return 0;
  }
  early_param("swiotlb", setup_io_tlb_npages);
+
+static int __init
+setup_io_tlb_size(char *str)
+{
+	int len = strlen(str);
+
+	if (str[len-1] == 'M')
+		swiotlb_sz_shift = 20;
+	else if (str[len-1] == 'K')
+		swiotlb_sz_shift = 10;
+	str[len-1] = '\0';
+	if (isdigit(*str))
+		swiotlb_sz = kstrtoul(str, &str, 0);
+
+	swiotlb_enabled = 1;
+	return 0;
+}
+early_param("swiotlb_sz", setup_io_tlb_size);
  /* make io_tlb_overflow tunable too? */

  unsigned long swiotlb_nr_tbl(void)

@@ -120,8 +142,9 @@ unsigned long swiotlb_nr_tbl(void)
  }
  EXPORT_SYMBOL_GPL(swiotlb_nr_tbl);

-/* default to 64MB */
-#define IO_TLB_DEFAULT_SIZE (64UL<<20)
+/* Pass from command line as swiotlb_sz=64M (for eg.)*/
+#define IO_TLB_DEFAULT_SIZE (swiotlb_sz<<swiotlb_sz_shift)
+
  unsigned long swiotlb_size_or_default(void)
  {
  	unsigned long size;

@@ -153,10 +176,12 @@ void swiotlb_print_info(void)
  	vstart = phys_to_virt(io_tlb_start);
  	vend = phys_to_virt(io_tlb_end);

-	printk(KERN_INFO "software IO TLB [mem %#010llx-%#010llx] (%luMB) mapped at [%p-%p]\n",
+	pr_info("software IO TLB [mem %#010llx-%#010llx] (%lu%cB) mapped at [%p-%p]\n",
  	       (unsigned long long)io_tlb_start,
  	       (unsigned long long)io_tlb_end,
-	       bytes >> 20, vstart, vend - 1);
+		bytes >> swiotlb_sz_shift,
+		swiotlb_sz_shift == 20 ? 'M' : 'K',
+		vstart, vend - 1);
  }

  int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)

Re: [PATCH] arm64:swiotlb:Enable only when Input size through command line

From: Catalin Marinas <catalin.marinas@arm.com>
Date: 2016-06-23 14:30:42

On Thu, Jun 23, 2016 at 05:43:40PM +0530, Manjeet Pawar wrote:

From: Rohit Thapliyal <redacted>

swiotlb default size of 64M is too big as
default value therefore it is made configurable
through command line through swiotlb_size parameter.
swiotlb allocation shall be done only when the
swiotlb size is given through command line.
Otherwise no swiotlb is allocated.

I already queued this patch:

http://lkml.kernel.org/g/1465372426-4077-1-git-send-email-jszhang at marvell.com

If you have any objections to it, please reply there.

-- 
Catalin

Re: [PATCH] arm64:swiotlb:Enable only when Input size through command line

From: kbuild test robot <hidden>
Date: 2016-06-23 14:40:21

Hi,

[auto build test WARNING on v4.7-rc4]
[cannot apply to arm64/for-next/core next-20160623]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Manjeet-Pawar/arm64-swiotlb-Enable-only-when-Input-size-through-command-line/20160623-205508
config: x86_64-randconfig-i0-201625 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   lib/swiotlb.c: In function 'setup_io_tlb_size':

quoted

lib/swiotlb.c:131:30: warning: passing argument 2 of 'kstrtoul' makes integer from pointer without a cast [-Wint-conversion]

      swiotlb_sz = kstrtoul(str, &str, 0);
                                 ^
   In file included from include/linux/list.h:8:0,
                    from include/linux/kobject.h:20,
                    from include/linux/device.h:17,
                    from include/linux/dma-mapping.h:6,
                    from lib/swiotlb.c:21:
   include/linux/kernel.h:298:32: note: expected 'unsigned int' but argument is of type 'char **'
    static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
                                   ^~~~~~~~

vim +/kstrtoul +131 lib/swiotlb.c

   115	
   116		return 0;
   117	}
   118	early_param("swiotlb", setup_io_tlb_npages);
   119	
   120	static int __init
   121	setup_io_tlb_size(char *str)
   122	{
   123		int len = strlen(str);
   124	
   125		if (str[len-1] == 'M')
   126			swiotlb_sz_shift = 20;
   127		else if (str[len-1] == 'K')
   128			swiotlb_sz_shift = 10;
   129		str[len-1] = '\0';
   130		if (isdigit(*str))
 > 131			swiotlb_sz = kstrtoul(str, &str, 0);
   132	
   133		swiotlb_enabled = 1;
   134		return 0;
   135	}
   136	early_param("swiotlb_sz", setup_io_tlb_size);
   137	/* make io_tlb_overflow tunable too? */
   138	
   139	unsigned long swiotlb_nr_tbl(void)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 28749 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160623/94ae0b4a/attachment-0001.obj>

Re: [PATCH] arm64:swiotlb:Enable only when Input size through command line

From: Konrad Rzeszutek Wilk <hidden>
Date: 2016-06-23 16:07:16

On June 23, 2016 10:30:34 AM EDT, Catalin Marinas [off-list ref] wrote:

On Thu, Jun 23, 2016 at 05:43:40PM +0530, Manjeet Pawar wrote:

quoted

From: Rohit Thapliyal <redacted>

swiotlb default size of 64M is too big as
default value therefore it is made configurable
through command line through swiotlb_size parameter.
swiotlb allocation shall be done only when the
swiotlb size is given through command line.
Otherwise no swiotlb is allocated.

I already queued this patch:

http://lkml.kernel.org/g/1465372426-4077-1-git-send-email-jszhang at marvell.com

If you have any objections to it, please reply there.


I do (sorry about duplicate email, the other got rejected by mailing lists).

Why not expand the swiotlb= parameter instead of introducing a new one?

Also, why not use the swiotlb by itself? That does the job as well?

Re: [PATCH] arm64:swiotlb:Enable only when Input size through command line

From: Jisheng Zhang <hidden>
Date: 2016-06-24 03:02:00

Dear Konrad,

On Thu, 23 Jun 2016 12:06:10 -0400 Konrad Rzeszutek Wilk wrote:

On June 23, 2016 10:30:34 AM EDT, Catalin Marinas [off-list ref] wrote:

quoted

On Thu, Jun 23, 2016 at 05:43:40PM +0530, Manjeet Pawar wrote:

quoted

From: Rohit Thapliyal <redacted>

swiotlb default size of 64M is too big as
default value therefore it is made configurable
through command line through swiotlb_size parameter.
swiotlb allocation shall be done only when the
swiotlb size is given through command line.
Otherwise no swiotlb is allocated.

I already queued this patch:

http://lkml.kernel.org/g/1465372426-4077-1-git-send-email-jszhang at marvell.com

If you have any objections to it, please reply there.


I do (sorry about duplicate email, the other got rejected by mailing lists).

Why not expand the swiotlb= parameter instead of introducing a new one?

Do you mean pass "swiotlb=" for those platforms(most probably, arm64 with less
than 4GB DDR) which don't need swiotlb? I'm afraid this is not convenient, and
users even don't notice swiotlb parameter. From another side, pass "swiotlb=0"
will make the swiotlb reserve 64MB instead, so how can we achieve zero reserved
memory for swiotlb through "swiotlb=" parameter?

PS: my patch didn't introduce new boot parameter.

I'm not sure I got your meaning, so could you please comment my patch
directly?

Thanks,
Jisheng

Also, why not use the swiotlb by itself? That does the job as well?


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64:swiotlb:Enable only when Input size through command line

From: Konrad Rzeszutek Wilk <hidden>
Date: 2016-06-24 10:46:26

On Fri, Jun 24, 2016 at 10:57:29AM +0800, Jisheng Zhang wrote:

Dear Konrad,

On Thu, 23 Jun 2016 12:06:10 -0400 Konrad Rzeszutek Wilk wrote:

quoted

On June 23, 2016 10:30:34 AM EDT, Catalin Marinas [off-list ref] wrote:

quoted

On Thu, Jun 23, 2016 at 05:43:40PM +0530, Manjeet Pawar wrote:

quoted

From: Rohit Thapliyal <redacted>

swiotlb default size of 64M is too big as
default value therefore it is made configurable
through command line through swiotlb_size parameter.
swiotlb allocation shall be done only when the
swiotlb size is given through command line.
Otherwise no swiotlb is allocated.

I already queued this patch:

http://lkml.kernel.org/g/1465372426-4077-1-git-send-email-jszhang at marvell.com

If you have any objections to it, please reply there.


I do (sorry about duplicate email, the other got rejected by mailing lists).

Why not expand the swiotlb= parameter instead of introducing a new one?

Do you mean pass "swiotlb=" for those platforms(most probably, arm64 with less
than 4GB DDR) which don't need swiotlb? I'm afraid this is not convenient, and

Why not just have a function that checks the amount of memory? x86 has
that - if it finds that the machine has less than 4GB it will not setup
SWIOTLB?

users even don't notice swiotlb parameter. From another side, pass "swiotlb=0"
will make the swiotlb reserve 64MB instead, so how can we achieve zero reserved
memory for swiotlb through "swiotlb=" parameter?

Obviously make the function understand that 0 is to turn it off.

PS: my patch didn't introduce new boot parameter.

swiotlb_sz ?

I'm not sure I got your meaning, so could you please comment my patch
directly?

Thanks,
Jisheng

quoted

Also, why not use the swiotlb by itself? That does the job as well?


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64:swiotlb:Enable only when Input size through command line

From: Robin Murphy <robin.murphy@arm.com>
Date: 2016-06-24 10:53:19

Hi Konrad,

On 24/06/16 11:46, Konrad Rzeszutek Wilk wrote:

On Fri, Jun 24, 2016 at 10:57:29AM +0800, Jisheng Zhang wrote:

quoted

Dear Konrad,

On Thu, 23 Jun 2016 12:06:10 -0400 Konrad Rzeszutek Wilk wrote:

quoted

On June 23, 2016 10:30:34 AM EDT, Catalin Marinas [off-list ref] wrote:

quoted

On Thu, Jun 23, 2016 at 05:43:40PM +0530, Manjeet Pawar wrote:

quoted

From: Rohit Thapliyal <redacted>

swiotlb default size of 64M is too big as
default value therefore it is made configurable
through command line through swiotlb_size parameter.
swiotlb allocation shall be done only when the
swiotlb size is given through command line.
Otherwise no swiotlb is allocated.

I already queued this patch:

http://lkml.kernel.org/g/1465372426-4077-1-git-send-email-jszhang at marvell.com

If you have any objections to it, please reply there.


I do (sorry about duplicate email, the other got rejected by mailing lists).

Why not expand the swiotlb= parameter instead of introducing a new one?

Do you mean pass "swiotlb=" for those platforms(most probably, arm64 with less
than 4GB DDR) which don't need swiotlb? I'm afraid this is not convenient, and

Why not just have a function that checks the amount of memory? x86 has
that - if it finds that the machine has less than 4GB it will not setup
SWIOTLB?

quoted

users even don't notice swiotlb parameter. From another side, pass "swiotlb=0"
will make the swiotlb reserve 64MB instead, so how can we achieve zero reserved
memory for swiotlb through "swiotlb=" parameter?

Obviously make the function understand that 0 is to turn it off.

quoted

PS: my patch didn't introduce new boot parameter.

swiotlb_sz ?

Note that Jisheng's patch is the one Catalin linked to, *not* this one,
and more or less does exactly what you describe.

Robin.

quoted

I'm not sure I got your meaning, so could you please comment my patch
directly?

Thanks,
Jisheng

quoted

Also, why not use the swiotlb by itself? That does the job as well?


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help