Re: [Xen-devel] [PATCH v2 2/4] xen: introduce grant_map_exists

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.10.14 at 16:34, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> On Mon, 6 Oct 2014, Jan Beulich wrote:
>> >>> On 06.10.14 at 15:46, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
>> > On Mon, 6 Oct 2014, Jan Beulich wrote:
>> >> >>> On 06.10.14 at 15:08, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
>> >> > The other approach to do this, that I avoided because it is slower (2
>> >> > spinlocks instead of 1), is to use the existing mapcount function.
>> >> > mapcount is based on:
>> >> > 
>> >> > for ( handle = 0; handle < lgt->maptrack_limit; handle++ )
>> >> > 
>> >> > do you think it is better?
>> >> 
>> >> No, that's worse: This is the Dom0 side of things, and the loop
>> >> bound would still be controlled by that same command line option.
>> >> 
>> >> So between the two, the original solution would be better, and
>> >> maybe enforcing a lower limit on DomU-s might actually be an
>> >> option.
>> > 
>> > What about keeping track of the highest ref number actually used so far?
>> > I could use that as upper limit instead of the theoretical max. It would
>> > also make the execution faster.
>> 
>> But the loops are already using a value on the order of the high
>> watermark, not the theoretical one. I.e. you're not alway looping
>> for a long time, it is just possible for this to happen.
> 
> OK, I understand.
> 
> The default max value is 32 pages that lead to 16384 iterations in
> grant_map_exists. Should I reduce it?
> 
> Given the simple operations in the loop, the function shouldn't actually
> be so slow to be a security risk. It should take less that 0.1
> milliseconds to complete.

I don't think there's problem with the default (i.e. no reason to reduce
that value). But bumping the limit via command line option should still
result in a secure system. Therefore I think the limit should be split to
separate Dom0 and DomU ones (e.g. along the lines of the
"extra_guest_irqs=" option), and the DomU limit should then get
bounded more strictly. (And I wonder btw whether things work right
even without your patch if passing a sufficiently big value to
"gnttab_max_nr_frames=".)

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel