[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [V8 PATCH 2/8] pvh dom0: construct_dom0 changes
On 04/05/2014 01:53 AM, Mukesh Rathor wrote:
On Thu, 27 Mar 2014 16:30:37 +0100
Tim Deegan <tim@xxxxxxx> wrote:
At 15:04 +0000 on 27 Mar (1395929085), Jan Beulich wrote:
On 27.03.14 at 11:55, <george.dunlap@xxxxxxxxxxxxx> wrote:
On 03/27/2014 10:14 AM, Jan Beulich wrote:
On 26.03.14 at 20:05, <george.dunlap@xxxxxxxxxxxxx> wrote:
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -589,6 +589,21 @@ int hap_domctl(struct domain *d,
xen_domctl_shadow_op_t
*sc,
}
}
+void __init hap_set_pvh_alloc_for_dom0(struct domain *d,
+ unsigned long num_pages)
+{
+ int rc;
+ unsigned long memkb = num_pages * (PAGE_SIZE / 1024);
+
+ /* Copied from: libxl_get_required_shadow_memory() */
+ memkb = 4 * (256 * d->max_vcpus + 2 * (memkb / 1024));
+ num_pages = ( (memkb + 1023) / 1024) << (20 - PAGE_SHIFT);
+ paging_lock(d);
+ rc = hap_set_allocation(d, num_pages, NULL);
+ paging_unlock(d);
The calculation for how many pages of shadow memory are needed
logically belongs in domain_build.c, not hap.c.
Iirc it was me requesting it to be moved here, on the basis that
the calculation should match the one for other domains, and
hence be done alongside that for other domains. domain_build.c
shouldn't carry HAP-specific knowledge imo.
I thought that might be the case, which is why I apologized in
advance for not reading the previous thread.
The calculation should indeed match that done for other domains;
however, as the comment clearly says, this calculation is done in
libxl, not in Xen at all. Everything done to build dom0 in Xen
which corresponds to things done by the toolstack to build a domU
logically belongs in domain_build.c.
Putting it in hap.c would be wrong in any case; what would you do
when we enable shadow mode for HAP dom0?
The function calls hap_set_allocation(), so it's already
HAP-specific.
It really ought to be calling a paging_set_allocation() wrapper;
there's nothing _inherently_ HAP-specific about PVH. Right now that
wrapper doesn't exist, because that path goes via paging_domctl() for
other domains.
Correct, PVH requires HAP at present, and I like when the code makes
that clear. In general, my approach is to make change in future when
demanded, thus, when shadow is added, it can be rearranged with small
effort.
Anyways, I'm flexible. Jan, final word: Leave it as is (my first pref),
move to domain_build.c, or put a wrapper(my last preference) ?
I'm fine with the code in domain_build.c calling hap_set_allocation()
directly for the time being, and implementing a paging_set_allocation()
wrapper later; but I think the code doing the calculation should be in
domain_build.c.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
