Re: [V8 PATCH 2/8] pvh dom0: construct_dom0 changes

[George Dunlap <george.dunlap@eu.citrix.com>]

On 03/22/2014 01:39 AM, Mukesh Rathor wrote:
> This patch changes construct_dom0() to boot in PVH mode.

[Apologies in advance for not following the previous review cycles.]

This needs a better description.  Minimally:

* Call guest_physmap_add_range rather than simple physmap setting for 
pvh guests.
* Map all non-RAM regions for dom0 as 1:1 up to 4GiB, so dom0 has IO 
memory mapped.
* If pvh was specified, make sure that dom0 supports pvh mode
* Allocate p2m pages, copying calculation from toolstack
* Make space for the shared info in the p2m (?)
* [A description of what "fixup_page_tables_for_hap" does]

> +    /*
> +     * pvh: we temporarily disable paging mode so that we can build cr3 needed
> +     * to run on dom0's page tables.
> +     */
> +    save_pvh_pg_mode = d->arch.paging.mode;
> +    d->arch.paging.mode = 0;
> +

What on earth is this about?  Setting d->arch.paging.mode to zero isn't 
actually disabling paging; it's just making people *thing* paging is 
disabled.  AFAICT paging should be enabled at this point anyway, should it?

  
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index b8c5422..878697e 100644
--- 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.

  -George