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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.10.14 at 16:57, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> On Mon, 6 Oct 2014, Jan Beulich wrote:
>> >>> 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. 
> 
> That makes sense.
> Would you prefer having the changes as part of this series or separately?

It needs to be a prereq to what the series does - including it in the
series would therefore seem reasonable, but not a strict requirement.

Jan


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