linux-mips
[Top] [All Lists]

RE: [Xen-devel] [PATCH v4] modify the IO_TLB_SEGSIZE and IO_TLB_DEFAULT_

To: "Xiaoming Wang" <xiaoming.wang@intel.com>
Subject: RE: [Xen-devel] [PATCH v4] modify the IO_TLB_SEGSIZE and IO_TLB_DEFAULT_SIZE configurable as flexible requirement about SW-IOMMU.
From: "Jan Beulich" <JBeulich@suse.com>
Date: Wed, 18 Feb 2015 09:34:50 +0000
Cc: "chris@chris-wilson.co.uk" <chris@chris-wilson.co.uk>, "david.vrabel@citrix.com" <david.vrabel@citrix.com>, "lauraa@codeaurora.org" <lauraa@codeaurora.org>, "heiko.carstens@de.ibm.com" <heiko.carstens@de.ibm.com>, "linux@horizon.com" <linux@horizon.com>, "Chuansheng Liu" <chuansheng.liu@intel.com>, "Dongxing Zhang" <dongxing.zhang@intel.com>, "takahiro.akashi@linaro.org" <takahiro.akashi@linaro.org>, "akpm@linux-foundation.org" <akpm@linux-foundation.org>, "linux-mips@linux-mips.org" <linux-mips@linux-mips.org>, "ralf@linux-mips.org" <ralf@linux-mips.org>, "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>, "boris.ostrovsky@oracle.com" <boris.ostrovsky@oracle.com>, "konrad.wilk@oracle.com" <konrad.wilk@oracle.com>, "d.kasatkin@samsung.com" <d.kasatkin@samsung.com>, "pebolle@tiscali.nl" <pebolle@tiscali.nl>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Envelope-id: groupwise.54E45CBA.ECD:78:76fe:N
In-reply-to: <FA47D36D6EC9FE4CB463299737C09B9901D00FBD@shsmsx102.ccr.corp.intel.com>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-id: linux-mips <linux-mips.eddie.linux-mips.org>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-software: Ecartis version 1.0.0
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
Original-recipient: rfc822;groupwise-linux-mips@linux-mips.org:9:1
References: <1424155903-4262-1-git-send-email-xiaoming.wang@intel.com> <54E321400200007800060865@mail.emea.novell.com> <FA47D36D6EC9FE4CB463299737C09B9901D00FBD@shsmsx102.ccr.corp.intel.com>
Sender: linux-mips-bounce@linux-mips.org
>>> On 18.02.15 at 10:09, <xiaoming.wang@intel.com> wrote:
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: Tuesday, February 17, 2015 6:09 PM
>> >>> On 17.02.15 at 07:51, <xiaoming.wang@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


<Prev in Thread] Current Thread [Next in Thread>