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-ker...@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 (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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
