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

Re: [Xen-devel] [PATCH] libxc: Defer initialization of start_page for HVM guests

On 01/05/2016 09:06 AM, Ian Campbell wrote:
On Tue, 2015-12-22 at 09:48 -0500, Boris Ostrovsky wrote:
On 12/22/2015 09:36 AM, Roger Pau Monnà wrote:
El 22/12/15 a les 15.12, Boris Ostrovsky ha escrit:
On 12/22/2015 04:42 AM, Roger Pau Monnà wrote:
Also, why is everything done inside of alloc_magic_pages_hvm moved
start_info_hvm? AFAICT we should only need to move the code that
the hvm_start_info struct, but the rest of the code already present
alloc_magic_pages_hvm could be left as-is.
That's what I was referring to in the commit message (and in the
below it): most, if not all, of the stuff that
currently does really has nothing to do with magic pages allocation.
E.g. hvm_params settting, special pages initialization, etc. So I
figured I could move all of this to a later point. But, as I said in
comment, I am not convinced that start_info() is the right place
Some of it has to do with magic pages IMHO (although I have to admit
this is probably a question of taste), for example it makes sense from
my PoV to allocate the hvm_info_page, the special pages (xenstore,
console...), the ioreq server pages and possibly the EPT identity map.
Moving the start_info stuff into it's own function also makes sense.
Since there are many ways to separate this I'll wait for maintainers to
express their preference.
So AIUI the issue here is that alloc_magic_pages_hvm has become something
of a dumping ground for stuff that needs to (or can) happen at this
particular point in time, some of which is certainly not "magic pages" and
others for which it is debatable.

One particular piece of this is the start_info which is not a magic page
either and which as it happens needs to be done later. (Much later it

As part of fixing that you essentially moved the "dumping ground" from
alloc_magic_pages_hvm to start_info_hvm (which is a new callback on an
existing hook), since, why not.

Looking at xc_dom_boot_image (the caller of the start_info hook) I see that
it also calls, not long after the start_info hook, other hooks such as
->bootlate ("/* misc x86 stuff */").

So if all the random stuff which this patch moves from ->alloc_magic to
->start_info can really be safely moved like that (which I didn't check)
I'd expect it could equally well move to ->bootlate (the only thing in the
middle is setup_hypercall_page() and xc_dom_log_memory_footprint(), and
that stuff does seem much more in keeping with "misc x86 stuff" than
->start_info or ->alloc_magic.

Maybe the xc_dom_log_memory_footprint call would need to move after the
->bootlate, that doesn't seem like the end of the world.

Of course if there is functionality which seems worthy of its own new
callback or which would be best placed in some other existing callback I'm
sure that would be fine too.

I didn't notice bootlate --- it looks to me like a place where we'd do things we couldn't do earlier, which is exactly what I was trying to achieve.

So I'll use it (and see if I can keep some of the things Roger thought are magic pages-related in alloc_magic_pages_hvm()).


Xen-devel mailing list



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