RE: [Xen-devel] [PATCH v4] modify the IO_TLB_SEGSIZE and IO_TLB_DEFAULT_SIZE configurable as flexible requirement about SW-IOMMU.
Dear Jan
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Wednesday, February 18, 2015 5:35 PM
> To: Wang, Xiaoming
> Cc: ch...@chris-wilson.co.uk; david.vra...@citrix.com;
> lau...@codeaurora.org; heiko.carst...@de.ibm.com; li...@horizon.com;
> Liu, Chuansheng; Zhang, Dongxing; takahiro.aka...@linaro.org;
> a...@linux-foundation.org; linux-m...@linux-mips.org; ralf@linux-
> mips.org; xen-de...@lists.xenproject.org; boris.ostrov...@oracle.com;
> konrad.w...@oracle.com; d.kasat...@samsung.com; pebo...@tiscali.nl;
> linux-kernel@vger.kernel.org
> Subject: RE: [Xen-devel] [PATCH v4] modify the IO_TLB_SEGSIZE and
> IO_TLB_DEFAULT_SIZE configurable as flexible requirement about SW-
> IOMMU.
>
> >>> On 18.02.15 at 10:09, wrote:
> >> From: Jan Beulich [mailto:jbeul...@suse.com]
> >> Sent: Tuesday, February 17, 2015 6:09 PM
> >> >>> On 17.02.15 at 07:51, wrote:
> >> > --- a/Documentation/kernel-parameters.txt
> >> > +++ b/Documentation/kernel-parameters.txt
> >> > @@ -3438,10 +3438,12 @@ bytes respectively. Such letter suffixes
> >> > can also be entirely omitted.
> >> > it if 0 is given (See
> >> Documentation/cgroups/memory.txt)
> >> >
> >> > swiotlb=[ARM,IA-64,PPC,MIPS,X86]
> >> > -Format: { | force }
> >> > +Format: { | force | | }
> >> > -- Number of I/O TLB slabs
> >> > force -- force using of bounce buffers even if
> >> > they
> >> > wouldn't be automatically used by the
> >> > kernel
> >> > + -- Maximum allowable number of contiguous
> >> slabs to map
> >> > + -- The size of SW-MMU mapped.
> >>
> >> This makes no sense - the new numbers added aren't position
> >> independent (nor were the previous and "force").
> >>
> > Use "," can separate them one by one.
> > We do it at lib/swiotlb.c
>
> Right, but the documentation above doesn't say so.
>
OK, I will add some comments on next patch version.
> >> Also you are (supposedly) removing all uses of IO_TLB_DEFAULT_SIZE,
> >> yet you don't seem to remove the definition itself.
> >>
> > I have change all uses of IO_TLB_DEFAULT_SIZE to io_tlb_default_size
> > in lib/swiotlb.c
>
> Then are there any left elsewhere? If not, again - why don't you remove the
> definition of IO_TLB_DEFAULT_SIZE?
>
There hasn't any IO_TLB_DEFAULT_SIZE left.
I check the code IO_TLB_DEFAULT_SIZE only used in lib/swiotlb.c.
And I have removed the definition of IO_TLB_DEFAULT_SIZE, in my patch
@@ -120,15 +146,13 @@ unsigned long swiotlb_nr_tbl(void) }
EXPORT_SYMBOL_GPL(swiotlb_nr_tbl);
-/* default to 64MB */
-#define IO_TLB_DEFAULT_SIZE (64UL<<20)
> >> Finally - are arbitrary numbers really okay for the newly added
> >> command line options? I.e. shouldn't you add some checking of their
> validity?
> >>
> > I have validity these code is OK.
> > Example:
> > BOARD_KERNEL_CMDLINE += swiotlb=, ,512,268435456 Io_tlb_segsize has
> > been changed from 128 to 512 Io_tlb_default_size has been changed from
> > 64M to 268435456 (256M)
>
> I specifically said "arbitrary numbers", which in particular includes zero and
> non-power-of-2 values. If there are any restrictions on which numbers can
> validly be passed here (and it very much looks like there are), such
> restrictions should be enforced imo.
>
OK, we will validate for these variables' value in next patch version.
> Jan
Xiaoming
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [Xen-devel] [PATCH v4] modify the IO_TLB_SEGSIZE and IO_TLB_DEFAULT_SIZE configurable as flexible requirement about SW-IOMMU.
>>> On 18.02.15 at 10:09, wrote:
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: Tuesday, February 17, 2015 6:09 PM
>> >>> On 17.02.15 at 07:51, wrote:
>> > --- a/Documentation/kernel-parameters.txt
>> > +++ b/Documentation/kernel-parameters.txt
>> > @@ -3438,10 +3438,12 @@ bytes respectively. Such letter suffixes can
>> > also be entirely omitted.
>> >it if 0 is given (See
>> Documentation/cgroups/memory.txt)
>> >
>> >swiotlb=[ARM,IA-64,PPC,MIPS,X86]
>> > - Format: { | force }
>> > + Format: { | force | | }
>> > -- Number of I/O TLB slabs
>> >force -- force using of bounce buffers even if they
>> > wouldn't be automatically used by the kernel
>> > + -- Maximum allowable number of contiguous
>> slabs to map
>> > + -- The size of SW-MMU mapped.
>>
>> This makes no sense - the new numbers added aren't position independent
>> (nor were the previous and "force").
>>
> Use "," can separate them one by one.
> We do it at lib/swiotlb.c
Right, but the documentation above doesn't say so.
>> Also you are (supposedly) removing all uses of IO_TLB_DEFAULT_SIZE, yet
>> you don't seem to remove the definition itself.
>>
> I have change all uses of IO_TLB_DEFAULT_SIZE to io_tlb_default_size in
> lib/swiotlb.c
Then are there any left elsewhere? If not, again - why don't you
remove the definition of IO_TLB_DEFAULT_SIZE?
>> Finally - are arbitrary numbers really okay for the newly added command line
>> options? I.e. shouldn't you add some checking of their validity?
>>
> I have validity these code is OK.
> Example:
> BOARD_KERNEL_CMDLINE += swiotlb=, ,512,268435456
> Io_tlb_segsize has been changed from 128 to 512
> Io_tlb_default_size has been changed from 64M to 268435456 (256M)
I specifically said "arbitrary numbers", which in particular includes
zero and non-power-of-2 values. If there are any restrictions on
which numbers can validly be passed here (and it very much looks
like there are), such restrictions should be enforced imo.
Jan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [Xen-devel] [PATCH v4] modify the IO_TLB_SEGSIZE and IO_TLB_DEFAULT_SIZE configurable as flexible requirement about SW-IOMMU.
Dear Jan
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Tuesday, February 17, 2015 6:09 PM
> To: Wang, Xiaoming
> Cc: ch...@chris-wilson.co.uk; david.vra...@citrix.com;
> lau...@codeaurora.org; heiko.carst...@de.ibm.com; li...@horizon.com;
> Liu, Chuansheng; Zhang, Dongxing; takahiro.aka...@linaro.org;
> a...@linux-foundation.org; linux-m...@linux-mips.org; ralf@linux-
> mips.org; xen-de...@lists.xenproject.org; boris.ostrov...@oracle.com;
> konrad.w...@oracle.com; d.kasat...@samsung.com; pebo...@tiscali.nl;
> linux-kernel@vger.kernel.org
> Subject: Re: [Xen-devel] [PATCH v4] modify the IO_TLB_SEGSIZE and
> IO_TLB_DEFAULT_SIZE configurable as flexible requirement about SW-
> IOMMU.
>
> >>> On 17.02.15 at 07:51, wrote:
> > --- a/Documentation/kernel-parameters.txt
> > +++ b/Documentation/kernel-parameters.txt
> > @@ -3438,10 +3438,12 @@ bytes respectively. Such letter suffixes can
> > also be entirely omitted.
> > it if 0 is given (See
> Documentation/cgroups/memory.txt)
> >
> > swiotlb=[ARM,IA-64,PPC,MIPS,X86]
> > - Format: { | force }
> > + Format: { | force | | }
> > -- Number of I/O TLB slabs
> > force -- force using of bounce buffers even if they
> > wouldn't be automatically used by the kernel
> > +-- Maximum allowable number of contiguous
> slabs to map
> > +-- The size of SW-MMU mapped.
>
> This makes no sense - the new numbers added aren't position independent
> (nor were the previous and "force").
>
Use "," can separate them one by one.
We do it at lib/swiotlb.c
> Also you are (supposedly) removing all uses of IO_TLB_DEFAULT_SIZE, yet
> you don't seem to remove the definition itself.
>
I have change all uses of IO_TLB_DEFAULT_SIZE to io_tlb_default_size in
lib/swiotlb.c
> Finally - are arbitrary numbers really okay for the newly added command line
> options? I.e. shouldn't you add some checking of their validity?
>
I have validity these code is OK.
Example:
BOARD_KERNEL_CMDLINE += swiotlb=, ,512,268435456
Io_tlb_segsize has been changed from 128 to 512
Io_tlb_default_size has been changed from 64M to 268435456 (256M)
> Jan
Xiaoming.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [Xen-devel] [PATCH v4] modify the IO_TLB_SEGSIZE and IO_TLB_DEFAULT_SIZE configurable as flexible requirement about SW-IOMMU.
On 18.02.15 at 10:09, xiaoming.w...@intel.com wrote:
From: Jan Beulich [mailto:jbeul...@suse.com]
Sent: Tuesday, February 17, 2015 6:09 PM
On 17.02.15 at 07:51, xiaoming.w...@intel.com wrote:
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3438,10 +3438,12 @@ bytes respectively. Such letter suffixes can
also be entirely omitted.
it if 0 is given (See
Documentation/cgroups/memory.txt)
swiotlb=[ARM,IA-64,PPC,MIPS,X86]
- Format: { int | force }
+ Format: { int | force | int | int}
int -- Number of I/O TLB slabs
force -- force using of bounce buffers even if they
wouldn't be automatically used by the kernel
+ int -- Maximum allowable number of contiguous
slabs to map
+ int -- The size of SW-MMU mapped.
This makes no sense - the new numbers added aren't position independent
(nor were the previous int and force).
Use , can separate them one by one.
We do it at lib/swiotlb.c
Right, but the documentation above doesn't say so.
Also you are (supposedly) removing all uses of IO_TLB_DEFAULT_SIZE, yet
you don't seem to remove the definition itself.
I have change all uses of IO_TLB_DEFAULT_SIZE to io_tlb_default_size in
lib/swiotlb.c
Then are there any left elsewhere? If not, again - why don't you
remove the definition of IO_TLB_DEFAULT_SIZE?
Finally - are arbitrary numbers really okay for the newly added command line
options? I.e. shouldn't you add some checking of their validity?
I have validity these code is OK.
Example:
BOARD_KERNEL_CMDLINE += swiotlb=, ,512,268435456
Io_tlb_segsize has been changed from 128 to 512
Io_tlb_default_size has been changed from 64M to 268435456 (256M)
I specifically said arbitrary numbers, which in particular includes
zero and non-power-of-2 values. If there are any restrictions on
which numbers can validly be passed here (and it very much looks
like there are), such restrictions should be enforced imo.
Jan
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [Xen-devel] [PATCH v4] modify the IO_TLB_SEGSIZE and IO_TLB_DEFAULT_SIZE configurable as flexible requirement about SW-IOMMU.
Dear Jan
-Original Message-
From: Jan Beulich [mailto:jbeul...@suse.com]
Sent: Wednesday, February 18, 2015 5:35 PM
To: Wang, Xiaoming
Cc: ch...@chris-wilson.co.uk; david.vra...@citrix.com;
lau...@codeaurora.org; heiko.carst...@de.ibm.com; li...@horizon.com;
Liu, Chuansheng; Zhang, Dongxing; takahiro.aka...@linaro.org;
a...@linux-foundation.org; linux-m...@linux-mips.org; ralf@linux-
mips.org; xen-de...@lists.xenproject.org; boris.ostrov...@oracle.com;
konrad.w...@oracle.com; d.kasat...@samsung.com; pebo...@tiscali.nl;
linux-kernel@vger.kernel.org
Subject: RE: [Xen-devel] [PATCH v4] modify the IO_TLB_SEGSIZE and
IO_TLB_DEFAULT_SIZE configurable as flexible requirement about SW-
IOMMU.
On 18.02.15 at 10:09, xiaoming.w...@intel.com wrote:
From: Jan Beulich [mailto:jbeul...@suse.com]
Sent: Tuesday, February 17, 2015 6:09 PM
On 17.02.15 at 07:51, xiaoming.w...@intel.com wrote:
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3438,10 +3438,12 @@ bytes respectively. Such letter suffixes
can also be entirely omitted.
it if 0 is given (See
Documentation/cgroups/memory.txt)
swiotlb=[ARM,IA-64,PPC,MIPS,X86]
-Format: { int | force }
+Format: { int | force | int | int}
int -- Number of I/O TLB slabs
force -- force using of bounce buffers even if
they
wouldn't be automatically used by the
kernel
+int -- Maximum allowable number of contiguous
slabs to map
+int -- The size of SW-MMU mapped.
This makes no sense - the new numbers added aren't position
independent (nor were the previous int and force).
Use , can separate them one by one.
We do it at lib/swiotlb.c
Right, but the documentation above doesn't say so.
OK, I will add some comments on next patch version.
Also you are (supposedly) removing all uses of IO_TLB_DEFAULT_SIZE,
yet you don't seem to remove the definition itself.
I have change all uses of IO_TLB_DEFAULT_SIZE to io_tlb_default_size
in lib/swiotlb.c
Then are there any left elsewhere? If not, again - why don't you remove the
definition of IO_TLB_DEFAULT_SIZE?
There hasn't any IO_TLB_DEFAULT_SIZE left.
I check the code IO_TLB_DEFAULT_SIZE only used in lib/swiotlb.c.
And I have removed the definition of IO_TLB_DEFAULT_SIZE, in my patch
@@ -120,15 +146,13 @@ unsigned long swiotlb_nr_tbl(void) }
EXPORT_SYMBOL_GPL(swiotlb_nr_tbl);
-/* default to 64MB */
-#define IO_TLB_DEFAULT_SIZE (64UL20)
Finally - are arbitrary numbers really okay for the newly added
command line options? I.e. shouldn't you add some checking of their
validity?
I have validity these code is OK.
Example:
BOARD_KERNEL_CMDLINE += swiotlb=, ,512,268435456 Io_tlb_segsize has
been changed from 128 to 512 Io_tlb_default_size has been changed from
64M to 268435456 (256M)
I specifically said arbitrary numbers, which in particular includes zero and
non-power-of-2 values. If there are any restrictions on which numbers can
validly be passed here (and it very much looks like there are), such
restrictions should be enforced imo.
OK, we will validate for these variables' value in next patch version.
Jan
Xiaoming
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [Xen-devel] [PATCH v4] modify the IO_TLB_SEGSIZE and IO_TLB_DEFAULT_SIZE configurable as flexible requirement about SW-IOMMU.
Dear Jan
-Original Message-
From: Jan Beulich [mailto:jbeul...@suse.com]
Sent: Tuesday, February 17, 2015 6:09 PM
To: Wang, Xiaoming
Cc: ch...@chris-wilson.co.uk; david.vra...@citrix.com;
lau...@codeaurora.org; heiko.carst...@de.ibm.com; li...@horizon.com;
Liu, Chuansheng; Zhang, Dongxing; takahiro.aka...@linaro.org;
a...@linux-foundation.org; linux-m...@linux-mips.org; ralf@linux-
mips.org; xen-de...@lists.xenproject.org; boris.ostrov...@oracle.com;
konrad.w...@oracle.com; d.kasat...@samsung.com; pebo...@tiscali.nl;
linux-kernel@vger.kernel.org
Subject: Re: [Xen-devel] [PATCH v4] modify the IO_TLB_SEGSIZE and
IO_TLB_DEFAULT_SIZE configurable as flexible requirement about SW-
IOMMU.
On 17.02.15 at 07:51, xiaoming.w...@intel.com wrote:
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3438,10 +3438,12 @@ bytes respectively. Such letter suffixes can
also be entirely omitted.
it if 0 is given (See
Documentation/cgroups/memory.txt)
swiotlb=[ARM,IA-64,PPC,MIPS,X86]
- Format: { int | force }
+ Format: { int | force | int | int}
int -- Number of I/O TLB slabs
force -- force using of bounce buffers even if they
wouldn't be automatically used by the kernel
+ int -- Maximum allowable number of contiguous
slabs to map
+ int -- The size of SW-MMU mapped.
This makes no sense - the new numbers added aren't position independent
(nor were the previous int and force).
Use , can separate them one by one.
We do it at lib/swiotlb.c
Also you are (supposedly) removing all uses of IO_TLB_DEFAULT_SIZE, yet
you don't seem to remove the definition itself.
I have change all uses of IO_TLB_DEFAULT_SIZE to io_tlb_default_size in
lib/swiotlb.c
Finally - are arbitrary numbers really okay for the newly added command line
options? I.e. shouldn't you add some checking of their validity?
I have validity these code is OK.
Example:
BOARD_KERNEL_CMDLINE += swiotlb=, ,512,268435456
Io_tlb_segsize has been changed from 128 to 512
Io_tlb_default_size has been changed from 64M to 268435456 (256M)
Jan
Xiaoming.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [PATCH v4] modify the IO_TLB_SEGSIZE and IO_TLB_DEFAULT_SIZE configurable as flexible requirement about SW-IOMMU.
>>> On 17.02.15 at 07:51, wrote:
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -3438,10 +3438,12 @@ bytes respectively. Such letter suffixes can also be
> entirely omitted.
> it if 0 is given (See Documentation/cgroups/memory.txt)
>
> swiotlb=[ARM,IA-64,PPC,MIPS,X86]
> - Format: { | force }
> + Format: { | force | | }
>-- Number of I/O TLB slabs
> force -- force using of bounce buffers even if they
>wouldn't be automatically used by the kernel
> + -- Maximum allowable number of contiguous slabs
> to map
> + -- The size of SW-MMU mapped.
This makes no sense - the new numbers added aren't position
independent (nor were the previous and "force").
Also you are (supposedly) removing all uses of
IO_TLB_DEFAULT_SIZE, yet you don't seem to remove the
definition itself.
Finally - are arbitrary numbers really okay for the newly added
command line options? I.e. shouldn't you add some checking of
their validity?
Jan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [PATCH v4] modify the IO_TLB_SEGSIZE and IO_TLB_DEFAULT_SIZE configurable as flexible requirement about SW-IOMMU.
On 17.02.15 at 07:51, xiaoming.w...@intel.com wrote:
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3438,10 +3438,12 @@ bytes respectively. Such letter suffixes can also be
entirely omitted.
it if 0 is given (See Documentation/cgroups/memory.txt)
swiotlb=[ARM,IA-64,PPC,MIPS,X86]
- Format: { int | force }
+ Format: { int | force | int | int}
int -- Number of I/O TLB slabs
force -- force using of bounce buffers even if they
wouldn't be automatically used by the kernel
+ int -- Maximum allowable number of contiguous slabs
to map
+ int -- The size of SW-MMU mapped.
This makes no sense - the new numbers added aren't position
independent (nor were the previous int and force).
Also you are (supposedly) removing all uses of
IO_TLB_DEFAULT_SIZE, yet you don't seem to remove the
definition itself.
Finally - are arbitrary numbers really okay for the newly added
command line options? I.e. shouldn't you add some checking of
their validity?
Jan
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
