[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 3/3] tools: introduce parameter max_wp_ram_ranges.





On 2/5/2016 1:12 AM, George Dunlap wrote:
On 04/02/16 14:08, Jan Beulich wrote:
On 04.02.16 at 14:33, <Ian.Jackson@xxxxxxxxxxxxx> wrote:
Jan Beulich writes ("Re: [Xen-devel] [PATCH v3 3/3] tools: introduce parameter
max_wp_ram_ranges."):
On 04.02.16 at 10:38, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote:
So another question is, if value of this limit really matters, will a
lower one be more acceptable(the current 256 being not enough)?

If you've carefully read George's replies, [...]

Thanks to George for the very clear explanation, and also to him for
an illuminating in-person discussion.

It is disturbing that as a result of me as a tools maintainer asking
questions about what seems to me to be a troublesome a user-visible
control setting in libxl, we are now apparently revisiting lower
layers of the hypervisor design, which have already been committed.

While I find George's line of argument convincing, neither I nor
George are maintainers of the relevant hypervisor code.  I am not
going to insist that anything in the hypervisor is done different and
am not trying to use my tools maintainer position to that end.

Clearly there has been a failure of our workflow to consider and
review everything properly together.  But given where we are now, I
think that this discussion about hypervisor internals is probably a
distraction.

While I recall George having made that alternative suggestion,
both Yu and Paul having reservations against it made me not
insist on that alternative. Instead I've been trying to limit some
of the bad effects that the variant originally proposed brought
with it. Clearly, with the more detailed reply George has now
given (involving areas where he is the maintainer for), I should
have been more demanding towards the exploration of that
alternative. That's clearly unfortunate, and I apologize for that,
but such things happen.

As to one of the patches already having for committed - I'm not
worried about that at all. We can always revert, that's why the
thing is called "unstable".

It looks like I should have been more careful to catch up on the current
state of things before I started arguing again -- please accept my
apologies.


In fact, I need to say thank you all for your patience and suggestions.
I'm thrilled to see XenGT is receiving so much attention. :)

I see that patch 2/3 addresses the gpfn/io question in the commit
message by saying, "Previously, a new hypercall or subop was suggested
to map write-protected pages into ioreq server. However, it turned out
handler of this new hypercall would be almost the same with the existing
pair - HVMOP_[un]map_io_range_to_ioreq_server, and there's already a
type parameter in this hypercall. So no new hypercall defined, only a
new type is introduced."

And I see that 2/3 internally separates the WP_RAM type into a separate
rangeset, whose size can be adjusted separately.

This addresses my complaint about the interface using gpfns rather than
MMIO ranges as an interface (somewhat anyway).  Sorry for not
acknowledging this at first.

The question of the internal implementation -- whether to use RB tree
rangesets, or radix trees (as apparently ARM memaccess does) or p2m
types -- is an internal implementation question.  I think p2m types is
long-term the best way to go, but it won't hurt to have the current
implementation checked in, as long as it doesn't have any impacts on the
stable interface.

At the moment, as far as I can tell, there's no way for libxl to even
run a version of qemu with XenGT enabled, so there's no real need for
libxl to be involved.


I agree.

The purpose of having the limit would putatively be to prevent a guest
being able to trigger an exhaustion of hypervisor memory by inducing the
device model to mark an arbitrary number of ranges as mmio_dm.

Two angles on this.

First, assuming that limiting the number of ranges is what we want:  I'm
not really a fan of using HVM_PARAMs for this, but as long as it's not
considered a public interface (i.e., it could go away or disappear and
everything would Just Work), then I wouldn't object.

Although I would ask: would it instead be suitable for now to just set
the default limit for WP_RAM to 8196 in the hypervisor, since we do
expect it to be tracking gpfn ranges rather than IO regions?  And if we

That is what we have suggesting in v9. But Jan proposed we leave this
option to the admin. And to some extent, I can understand his concern.

determine in the future that more ranges are necessary, to then do the
work of moving it to using p2m types (or exposing a knob to adjust it)?

But (and this the other angle): is simply marking a numerical limit
sufficient to avoid memory exhaustion? Is there a danger that after
creating several guests, such that Xen was now running very low on
memory, that a guest would (purposely or not) cause memory to be
exhausted sometime further after boot, causing a system-wide DoS (or
just general lack of stability)?


This worry sounds reasonable. So from this point of view, I guess value
of this limit does matter. Setting it to 256 will not cause any
trouble, but setting it to a huge one would make this limitation
useless. Previously, I had thought 8K is an acceptable one for XenGT to
run smoothly, and for Xen heap to not over consume its memory, but
seems this value is not small enough to be acceptable. :)

Yu


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.