[PATCH v3 0/8] xen: arm: support up to (almost) 1TB of guest RAM

[PATCH v3 0/8] xen: arm: support up to (almost) 1TB of guest RAM

[Ian Campbell]

This series rejigs the guest physical address space to allow for up to
1019GB of RAM, with up to 3GB below the 4GB boundary.

The second patch here (an error check) should be backported to 4.4 but
the rest are not suitable IMHO since they change the guest layout.
Although we reserve the right to do so I think we should avoid such
changes in stable branches.

Not much change since v2, a whitespace cleanup, collected the acks and
made the size of the magic PFN space explicit. Since I was reposting I
folded http://article.gmane.org/gmane.comp.emulators.xen.devel/197208
into the correct patch.

I think this is all now acked and ready to be committed, but I'll hold
off for now until we start getting some osstest passes again.

Ian.

[PATCH v3 1/8] tools: libxl: use uint64_t not unsigned long long for addresses

[Ian Campbell]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
v2: New patch
---
 tools/libxl/libxl_arm.c |   19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 4f0f0e2..215ef9e 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -256,11 +256,10 @@ static int make_psci_node(libxl__gc *gc, void *fdt)
 }
 
 static int make_memory_node(libxl__gc *gc, void *fdt,
-                            unsigned long long base,
-                            unsigned long long size)
+                            uint64_t base, uint64_t size)
 {
     int res;
-    const char *name = GCSPRINTF("memory@%08llx", base);
+    const char *name = GCSPRINTF("memory@%"PRIx64, base);
 
     res = fdt_begin_node(fdt, name);
     if (res) return res;
@@ -269,7 +268,7 @@ static int make_memory_node(libxl__gc *gc, void *fdt,
     if (res) return res;
 
     res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
-                            1, (uint64_t)base, (uint64_t)size);
+                            1, base, size);
     if (res) return res;
 
     res = fdt_end_node(fdt);
@@ -279,13 +278,11 @@ static int make_memory_node(libxl__gc *gc, void *fdt,
 }
 
 static int make_intc_node(libxl__gc *gc, void *fdt,
-                          unsigned long long gicd_base,
-                          unsigned long long gicd_size,
-                          unsigned long long gicc_base,
-                          unsigned long long gicc_size)
+                          uint64_t gicd_base, uint64_t gicd_size,
+                          uint64_t gicc_base, uint64_t gicc_size)
 {
     int res;
-    const char *name = GCSPRINTF("interrupt-controller@%08llx", gicd_base);
+    const char *name = GCSPRINTF("interrupt-controller@%"PRIx64, gicd_base);
 
     res = fdt_begin_node(fdt, name);
     if (res) return res;
@@ -307,8 +304,8 @@ static int make_intc_node(libxl__gc *gc, void *fdt,
 
     res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
                             2,
-                            (uint64_t)gicd_base, (uint64_t)gicd_size,
-                            (uint64_t)gicc_base, (uint64_t)gicc_size);
+                            gicd_base, gicd_size,
+                            gicc_base, gicc_size);
     if (res) return res;
 
     res = fdt_property_cell(fdt, "linux,phandle", PHANDLE_GIC);
-- 
1.7.10.4

Re: [PATCH v3 1/8] tools: libxl: use uint64_t not unsigned long long for addresses

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 1/8] tools: libxl: use uint64_t not unsigned long long for addresses"):
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Acked-by: Julien Grall <julien.grall@linaro.org>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH v3 2/8] tools: arm: report an error if the guest RAM is too large

[Ian Campbell]

Due to the layout of the guest physical address space we cannot support more
than 768M of RAM before overrunning the area set aside for the grant table. Due
to the presence of the magic pages at the end of the RAM region guests are
actually limited to 767M.

Catch this case during domain build and fail gracefully instead of obscurely
later on.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
This is the only patch in this series which I consider to be suitable for
backporting to Xen 4.4

v2:
 - Use GUEST_RAM_SIZE instead of GUEST_RAM_END
 - Refactor the ramlimit into one place.
---
 tools/libxc/xc_dom_arm.c      |    9 +++++++++
 xen/include/public/arch-arm.h |    3 ++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index 60ac51a..46dfc36 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -272,6 +272,15 @@ int arch_setup_meminit(struct xc_dom_image *dom)
         return -1;
     }
 
+    if ( ramsize > GUEST_RAM_SIZE - NR_MAGIC_PAGES*XC_PAGE_SIZE )
+    {
+        DOMPRINTF("%s: ram size is too large for guest address space: "
+                  "%"PRIx64" > %"PRIx64,
+                  __FUNCTION__, ramsize,
+                  GUEST_RAM_SIZE - NR_MAGIC_PAGES*XC_PAGE_SIZE);
+        return -1;
+    }
+
     rc = set_mode(dom->xch, dom->guest_domid, dom->guest_type);
     if ( rc )
         return rc;
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 7496556..dd53c94 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -369,7 +369,8 @@ typedef uint64_t xen_callback_t;
 #define GUEST_GICC_BASE   0x2c002000ULL
 #define GUEST_GICC_SIZE   0x100ULL
 
-#define GUEST_RAM_BASE    0x80000000ULL
+#define GUEST_RAM_BASE    0x80000000ULL /* 768M @ 2GB */
+#define GUEST_RAM_SIZE    0x30000000ULL
 
 #define GUEST_GNTTAB_BASE 0xb0000000ULL
 #define GUEST_GNTTAB_SIZE 0x00020000ULL
-- 
1.7.10.4

Re: [PATCH v3 2/8] tools: arm: report an error if the guest RAM is too large

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 2/8] tools: arm: report an error if the guest RAM is too large"):
> Due to the layout of the guest physical address space we cannot support more
> than 768M of RAM before overrunning the area set aside for the grant table. Due
> to the presence of the magic pages at the end of the RAM region guests are
> actually limited to 767M.
> 
> Catch this case during domain build and fail gracefully instead of obscurely
> later on.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Acked-by: Julien Grall <julien.grall@linaro.org>

Not my area, but:

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Ian.

[PATCH v3 3/8] tools: arm: move magic pfns out of guest RAM region

[Ian Campbell]

Because toolstacks (at least libxl) only allow RAM to be specified in 1M
increments these two pages were effectively costing 1M of guest RAM space.

Since these pages don't actually need to live in RAM just move them out.

With this a guest can now use the full 768M of the address space reserved
for RAM. (ok, not that impressive, but it simplifies things later)

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
--
v3: make the size of the region explicit.
v2: remove spurious w/s change

tools: arm: make the size of the magic page region explicit

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxc/xc_dom_arm.c      |   14 ++++++++------
 xen/include/public/arch-arm.h |    3 +++
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index 46dfc36..74917c3 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -58,12 +58,15 @@ static int setup_pgtables_arm(struct xc_dom_image *dom)
 static int alloc_magic_pages(struct xc_dom_image *dom)
 {
     int rc, i;
+    const xen_pfn_t base = GUEST_MAGIC_BASE >> XC_PAGE_SHIFT;
     xen_pfn_t p2m[NR_MAGIC_PAGES];
 
+    XC_BUILD_BUG_ON(NR_MAGIC_PAGES > GUEST_MAGIC_SIZE >> XC_PAGE_SHIFT);
+
     DOMPRINTF_CALLED(dom->xch);
 
     for (i = 0; i < NR_MAGIC_PAGES; i++)
-        p2m[i] = dom->rambase_pfn + dom->total_pages + i;
+        p2m[i] = base + i;
 
     rc = xc_domain_populate_physmap_exact(
             dom->xch, dom->guest_domid, NR_MAGIC_PAGES,
@@ -71,8 +74,8 @@ static int alloc_magic_pages(struct xc_dom_image *dom)
     if ( rc < 0 )
         return rc;
 
-    dom->console_pfn = dom->rambase_pfn + dom->total_pages + CONSOLE_PFN_OFFSET;
-    dom->xenstore_pfn = dom->rambase_pfn + dom->total_pages + XENSTORE_PFN_OFFSET;
+    dom->console_pfn = base + CONSOLE_PFN_OFFSET;
+    dom->xenstore_pfn = base + XENSTORE_PFN_OFFSET;
 
     xc_clear_domain_page(dom->xch, dom->guest_domid, dom->console_pfn);
     xc_clear_domain_page(dom->xch, dom->guest_domid, dom->xenstore_pfn);
@@ -272,12 +275,11 @@ int arch_setup_meminit(struct xc_dom_image *dom)
         return -1;
     }
 
-    if ( ramsize > GUEST_RAM_SIZE - NR_MAGIC_PAGES*XC_PAGE_SIZE )
+    if ( ramsize > GUEST_RAM_SIZE )
     {
         DOMPRINTF("%s: ram size is too large for guest address space: "
                   "%"PRIx64" > %"PRIx64,
-                  __FUNCTION__, ramsize,
-                  GUEST_RAM_SIZE - NR_MAGIC_PAGES*XC_PAGE_SIZE);
+                  __FUNCTION__, ramsize, GUEST_RAM_SIZE);
         return -1;
     }
 
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index dd53c94..6630f36 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -375,6 +375,9 @@ typedef uint64_t xen_callback_t;
 #define GUEST_GNTTAB_BASE 0xb0000000ULL
 #define GUEST_GNTTAB_SIZE 0x00020000ULL
 
+#define GUEST_MAGIC_BASE  0xc0000000ULL
+#define GUEST_MAGIC_SIZE  0x01000000ULL
+
 /* Interrupts */
 #define GUEST_TIMER_VIRT_PPI    27
 #define GUEST_TIMER_PHYS_S_PPI  29
-- 
1.7.10.4

Re: [PATCH v3 3/8] tools: arm: move magic pfns out of guest RAM region

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 3/8] tools: arm: move magic pfns out of guest RAM region"):
> Because toolstacks (at least libxl) only allow RAM to be specified in 1M
> increments these two pages were effectively costing 1M of guest RAM space.
...
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Acked-by: Julien Grall <julien.grall@linaro.org>

I haven't checked the semantics of this from an arm architecture pov.
I hope Julien has :-).  But with a tools hat on:

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Ian.

[PATCH v3 4/8] tools: arm: rearrange guest physical address space to increase max RAM

[Ian Campbell]

By switching things around we can manage to expose up to 3GB of RAM to guests.

I deliberately didn't place the RAM at address 0 to avoid coming to rely on
this, so the various peripherals, MMIO and magic pages etc all live in the
lower 1GB leaving the upper 3GB available for RAM.

It would likely have been possible to reduce the space used by the peripherals
etc and allow for 3.5 or 3.75GB but I decided to keep things simple and will
handle >3GB memory in a subsequent patch.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
 xen/include/public/arch-arm.h |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 6630f36..d5090fb 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -364,20 +364,20 @@ typedef uint64_t xen_callback_t;
  */
 
 /* Physical Address Space */
-#define GUEST_GICD_BASE   0x2c001000ULL
-#define GUEST_GICD_SIZE   0x1000ULL
-#define GUEST_GICC_BASE   0x2c002000ULL
-#define GUEST_GICC_SIZE   0x100ULL
+#define GUEST_GICD_BASE   0x03001000ULL
+#define GUEST_GICD_SIZE   0x00001000ULL
+#define GUEST_GICC_BASE   0x03002000ULL
+#define GUEST_GICC_SIZE   0x00000100ULL
 
-#define GUEST_RAM_BASE    0x80000000ULL /* 768M @ 2GB */
-#define GUEST_RAM_SIZE    0x30000000ULL
-
-#define GUEST_GNTTAB_BASE 0xb0000000ULL
+#define GUEST_GNTTAB_BASE 0x38000000ULL
 #define GUEST_GNTTAB_SIZE 0x00020000ULL
 
-#define GUEST_MAGIC_BASE  0xc0000000ULL
+#define GUEST_MAGIC_BASE  0x39000000ULL
 #define GUEST_MAGIC_SIZE  0x01000000ULL
 
+#define GUEST_RAM_BASE    0x40000000ULL /* 3GB of RAM @ 1GB */
+#define GUEST_RAM_SIZE    0xc0000000ULL
+
 /* Interrupts */
 #define GUEST_TIMER_VIRT_PPI    27
 #define GUEST_TIMER_PHYS_S_PPI  29
-- 
1.7.10.4

Re: [PATCH v3 4/8] tools: arm: rearrange guest physical address space to increase max RAM

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 4/8] tools: arm: rearrange guest physical address space to increase max RAM"):
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Acked-by: Julien Grall <julien.grall@linaro.org>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

(on the same basis as the previous patch)

Ian.

[PATCH v3 5/8] tools: arm: prepare for multiple banks of guest RAM

[Ian Campbell]

Prepare for adding more banks of guest RAM by renaming a bunch of variables
and defines as RAM0 etc.

Also in preparation switch to using GUEST_RAM0_BASE explicitly instead of
implicitly via dom->rambase_pfn (while asserting that they must be the same).
This makes the multiple bank case cleaner (although it looks a bit odd for
now).

GUEST_RAM_BASE is defined as the address of the lowest RAM bank, it is used in
tools/libxl/libxl_dom.c to call xc_dom_rambase_init().

Lastly for now ramsize (total size) and ram0size (size of first bank) are the
same, but use the appropriate one for each context.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
v3: Mention why GUEST_RAM_BASE is still defined.
v2: New patch
---
 tools/libxc/xc_dom_arm.c      |   25 +++++++++++++++----------
 xen/include/public/arch-arm.h |    8 ++++++--
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index 74917c3..b218e0f 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -18,6 +18,7 @@
  * Copyright (c) 2011, Citrix Systems
  */
 #include <inttypes.h>
+#include <assert.h>
 
 #include <xen/xen.h>
 #include <xen/io/protocols.h>
@@ -255,9 +256,11 @@ int arch_setup_meminit(struct xc_dom_image *dom)
     uint64_t modbase;
 
     /* Convenient */
-    const uint64_t rambase = dom->rambase_pfn << XC_PAGE_SHIFT;
     const uint64_t ramsize = dom->total_pages << XC_PAGE_SHIFT;
-    const uint64_t ramend = rambase + ramsize;
+
+    const uint64_t ram0size = ramsize;
+    const uint64_t ram0end = GUEST_RAM0_BASE + ram0size;
+
     const uint64_t kernbase = dom->kernel_seg.vstart;
     const uint64_t kernend = ROUNDUP(dom->kernel_seg.vend, 21/*2MB*/);
     const uint64_t kernsize = kernend - kernbase;
@@ -266,20 +269,22 @@ int arch_setup_meminit(struct xc_dom_image *dom)
     const uint64_t ramdisk_size = dom->ramdisk_blob ?
         ROUNDUP(dom->ramdisk_size, XC_PAGE_SHIFT) : 0;
     const uint64_t modsize = dtb_size + ramdisk_size;
-    const uint64_t ram128mb = rambase + (128<<20);
+    const uint64_t ram128mb = GUEST_RAM0_BASE + (128<<20);
+
+    assert(dom->rambase_pfn << XC_PAGE_SHIFT == GUEST_RAM0_BASE);
 
-    if ( modsize + kernsize > ramsize )
+    if ( modsize + kernsize > ram0size )
     {
         DOMPRINTF("%s: Not enough memory for the kernel+dtb+initrd",
                   __FUNCTION__);
         return -1;
     }
 
-    if ( ramsize > GUEST_RAM_SIZE )
+    if ( ramsize > GUEST_RAM_MAX )
     {
         DOMPRINTF("%s: ram size is too large for guest address space: "
                   "%"PRIx64" > %"PRIx64,
-                  __FUNCTION__, ramsize, GUEST_RAM_SIZE);
+                  __FUNCTION__, ramsize, GUEST_RAM_MAX);
         return -1;
     }
 
@@ -319,11 +324,11 @@ int arch_setup_meminit(struct xc_dom_image *dom)
      * If changing this then consider
      * xen/arch/arm/kernel.c:place_modules as well.
      */
-    if ( ramend >= ram128mb + modsize && kernend < ram128mb )
+    if ( ram0end >= ram128mb + modsize && kernend < ram128mb )
         modbase = ram128mb;
-    else if ( ramend - modsize > kernend )
-        modbase = ramend - modsize;
-    else if (kernbase - rambase > modsize )
+    else if ( ram0end - modsize > kernend )
+        modbase = ram0end - modsize;
+    else if (kernbase - GUEST_RAM0_BASE > modsize )
         modbase = kernbase - modsize;
     else
         return -1;
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index d5090fb..41944fe 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -375,8 +375,12 @@ typedef uint64_t xen_callback_t;
 #define GUEST_MAGIC_BASE  0x39000000ULL
 #define GUEST_MAGIC_SIZE  0x01000000ULL
 
-#define GUEST_RAM_BASE    0x40000000ULL /* 3GB of RAM @ 1GB */
-#define GUEST_RAM_SIZE    0xc0000000ULL
+#define GUEST_RAM0_BASE   0x40000000ULL /* 3GB of RAM @ 1GB */
+#define GUEST_RAM0_SIZE   0xc0000000ULL
+
+#define GUEST_RAM_BASE    GUEST_RAM0_BASE /* Lowest RAM address */
+/* Largest amount of actual RAM, not including holes */
+#define GUEST_RAM_MAX     (GUEST_RAM0_SIZE)
 
 /* Interrupts */
 #define GUEST_TIMER_VIRT_PPI    27
-- 
1.7.10.4

Re: [PATCH v3 5/8] tools: arm: prepare for multiple banks of guest RAM

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 5/8] tools: arm: prepare for multiple banks of guest RAM"):
> Prepare for adding more banks of guest RAM by renaming a bunch of variables
> and defines as RAM0 etc.
> 
> Also in preparation switch to using GUEST_RAM0_BASE explicitly instead of
> implicitly via dom->rambase_pfn (while asserting that they must be the same).
> This makes the multiple bank case cleaner (although it looks a bit odd for
> now).

AIUI this is supposed to have no function change.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Re: [PATCH v3 5/8] tools: arm: prepare for multiple banks of guest RAM

[Ian Campbell]

On Fri, 2014-05-02 at 15:37 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH v3 5/8] tools: arm: prepare for multiple banks of guest RAM"):
> > Prepare for adding more banks of guest RAM by renaming a bunch of variables
> > and defines as RAM0 etc.
> > 
> > Also in preparation switch to using GUEST_RAM0_BASE explicitly instead of
> > implicitly via dom->rambase_pfn (while asserting that they must be the same).
> > This makes the multiple bank case cleaner (although it looks a bit odd for
> > now).
> 
> AIUI this is supposed to have no function change.

Correct.

> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Thanks.

[PATCH v3 6/8] tools: arm: refactor code to setup guest p2m and fill it with RAM

[Ian Campbell]

This will help when we have more guest RAM banks.

Mostly code motion of the p2m_host initialisation and allocation loop into the
new function populate_guest_memory, but in addition in the caller we now
initialise the p2m all the INVALID_MFN to handle any holes, although in this
patch we still fill in the entire allocated region.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
v2: New patch
---
 tools/libxc/xc_dom_arm.c |   62 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 43 insertions(+), 19 deletions(-)

diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index b218e0f..cdb56e3 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -249,10 +249,42 @@ static int set_mode(xc_interface *xch, domid_t domid, char *guest_type)
     return rc;
 }
 
+static int populate_guest_memory(struct xc_dom_image *dom,
+                                 xen_pfn_t base_pfn, xen_pfn_t nr_pfns)
+{
+    int rc;
+    xen_pfn_t allocsz, pfn;
+
+    if (!nr_pfns)
+        return 0;
+
+    DOMPRINTF("%s: populating RAM @ %016"PRIx64"-%016"PRIx64" (%"PRId64"MB)",
+              __FUNCTION__,
+              (uint64_t)base_pfn << XC_PAGE_SHIFT,
+              (uint64_t)(base_pfn + nr_pfns) << XC_PAGE_SHIFT,
+              (uint64_t)nr_pfns >> (20-XC_PAGE_SHIFT));
+
+    for ( pfn = 0; pfn < nr_pfns; pfn++ )
+        dom->p2m_host[pfn] = base_pfn + pfn;
+
+    for ( pfn = rc = allocsz = 0; (pfn < nr_pfns) && !rc; pfn += allocsz )
+    {
+        allocsz = nr_pfns - pfn;
+        if ( allocsz > 1024*1024 )
+            allocsz = 1024*1024;
+
+        rc = xc_domain_populate_physmap_exact(
+            dom->xch, dom->guest_domid, allocsz,
+            0, 0, &dom->p2m_host[pfn]);
+    }
+
+    return rc;
+}
+
 int arch_setup_meminit(struct xc_dom_image *dom)
 {
     int rc;
-    xen_pfn_t pfn, allocsz, i;
+    xen_pfn_t pfn;
     uint64_t modbase;
 
     /* Convenient */
@@ -261,6 +293,8 @@ int arch_setup_meminit(struct xc_dom_image *dom)
     const uint64_t ram0size = ramsize;
     const uint64_t ram0end = GUEST_RAM0_BASE + ram0size;
 
+    const xen_pfn_t p2m_size = (ram0end - GUEST_RAM0_BASE) >> XC_PAGE_SHIFT;
+
     const uint64_t kernbase = dom->kernel_seg.vstart;
     const uint64_t kernend = ROUNDUP(dom->kernel_seg.vend, 21/*2MB*/);
     const uint64_t kernsize = kernend - kernbase;
@@ -294,27 +328,17 @@ int arch_setup_meminit(struct xc_dom_image *dom)
 
     dom->shadow_enabled = 1;
 
-    dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->total_pages);
+    dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * p2m_size);
     if ( dom->p2m_host == NULL )
         return -EINVAL;
+    for ( pfn = 0; pfn < p2m_size; pfn++ )
+        dom->p2m_host[pfn] = INVALID_MFN;
 
-    /* setup initial p2m */
-    for ( pfn = 0; pfn < dom->total_pages; pfn++ )
-        dom->p2m_host[pfn] = pfn + dom->rambase_pfn;
-
-    /* allocate guest memory */
-    for ( i = rc = allocsz = 0;
-          (i < dom->total_pages) && !rc;
-          i += allocsz )
-    {
-        allocsz = dom->total_pages - i;
-        if ( allocsz > 1024*1024 )
-            allocsz = 1024*1024;
-
-        rc = xc_domain_populate_physmap_exact(
-            dom->xch, dom->guest_domid, allocsz,
-            0, 0, &dom->p2m_host[i]);
-    }
+    /* setup initial p2m and allocate guest memory */
+    if ((rc = populate_guest_memory(dom,
+                                    GUEST_RAM0_BASE >> XC_PAGE_SHIFT,
+                                    ram0size >> XC_PAGE_SHIFT)))
+        return rc;
 
     /*
      * We try to place dtb+initrd at 128MB or if we have less RAM
-- 
1.7.10.4

Re: [PATCH v3 6/8] tools: arm: refactor code to setup guest p2m and fill it with RAM

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 6/8] tools: arm: refactor code to setup guest p2m and fill it with RAM"):
...
> +static int populate_guest_memory(struct xc_dom_image *dom,
> +                                 xen_pfn_t base_pfn, xen_pfn_t nr_pfns)
> +{
> +    int rc;
> +    xen_pfn_t allocsz, pfn;
> +
> +    if (!nr_pfns)
> +        return 0;

Why bother with this early return ?

Ian.

Re: [PATCH v3 6/8] tools: arm: refactor code to setup guest p2m and fill it with RAM

[Ian Campbell]

On Fri, 2014-05-02 at 15:42 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH v3 6/8] tools: arm: refactor code to setup guest p2m and fill it with RAM"):
> ...
> > +static int populate_guest_memory(struct xc_dom_image *dom,
> > +                                 xen_pfn_t base_pfn, xen_pfn_t nr_pfns)
> > +{
> > +    int rc;
> > +    xen_pfn_t allocsz, pfn;
> > +
> > +    if (!nr_pfns)
> > +        return 0;
> 
> Why bother with this early return ?

Mainly it just avoids the DOMPRINTF, which once we add a second bank
would be redundant a lot of the time.

Ian.

Re: [PATCH v3 6/8] tools: arm: refactor code to setup guest p2m and fill it with RAM

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH v3 6/8] tools: arm: refactor code to setup guest p2m and fill it with RAM"):
> On Fri, 2014-05-02 at 15:42 +0100, Ian Jackson wrote:
> > Ian Campbell writes ("[PATCH v3 6/8] tools: arm: refactor code to setup guest p2m and fill it with RAM"):
> > Why bother with this early return ?
> 
> Mainly it just avoids the DOMPRINTF, which once we add a second bank
> would be redundant a lot of the time.

Err, OK.

Ian.

[PATCH v3 7/8] tools: arm: support up to (almost) 1TB of guest RAM

[Ian Campbell]

This creates a second bank of RAM starting at 8GB and potentially extending to
the 1TB boundary, which is the limit imposed by our current use of a 3 level
p2m with 2 pages at level 0 (2^40 bits).

I've deliberately left a gap between the two banks just to exercise those code paths.

The second bank is 1016GB in size which plus the 3GB below 4GB is 1019GB
maximum guest RAM. At the point where the fact that this is slightly less than
a full TB starts to become an issue for people then we can switch to a 4 level
p2m, which would be needed to support guests larger than 1TB anyhow.

Tested on 32-bit with 1, 4 and 6GB guests. Anything more than ~3GB requires an
LPAE enabled kernel, or a 64-bit guest.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
v3: remove inadvertent whitespace change
---
 tools/libxc/xc_dom_arm.c      |   16 +++++++++++++---
 tools/libxl/libxl_arm.c       |   22 +++++++++++++++++++---
 xen/include/public/arch-arm.h |    7 +++++--
 3 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index cdb56e3..18c7adb 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -288,12 +288,18 @@ int arch_setup_meminit(struct xc_dom_image *dom)
     uint64_t modbase;
 
     /* Convenient */
-    const uint64_t ramsize = dom->total_pages << XC_PAGE_SHIFT;
+    const uint64_t ramsize = (uint64_t)dom->total_pages << XC_PAGE_SHIFT;
 
-    const uint64_t ram0size = ramsize;
+    const uint64_t ram0size =
+        ramsize > GUEST_RAM0_SIZE ? GUEST_RAM0_SIZE : ramsize;
     const uint64_t ram0end = GUEST_RAM0_BASE + ram0size;
+    const uint64_t ram1size =
+        ramsize > ram0size ? ramsize - ram0size : 0;
+    const uint64_t ram1end = GUEST_RAM1_BASE + ram1size;
 
-    const xen_pfn_t p2m_size = (ram0end - GUEST_RAM0_BASE) >> XC_PAGE_SHIFT;
+    const xen_pfn_t p2m_size = ram1size ?
+        (ram1end - GUEST_RAM0_BASE) >> XC_PAGE_SHIFT :
+        (ram0end - GUEST_RAM0_BASE) >> XC_PAGE_SHIFT;
 
     const uint64_t kernbase = dom->kernel_seg.vstart;
     const uint64_t kernend = ROUNDUP(dom->kernel_seg.vend, 21/*2MB*/);
@@ -339,6 +345,10 @@ int arch_setup_meminit(struct xc_dom_image *dom)
                                     GUEST_RAM0_BASE >> XC_PAGE_SHIFT,
                                     ram0size >> XC_PAGE_SHIFT)))
         return rc;
+    if ((rc = populate_guest_memory(dom,
+                                    GUEST_RAM1_BASE >> XC_PAGE_SHIFT,
+                                    ram1size >> XC_PAGE_SHIFT)))
+        return rc;
 
     /*
      * We try to place dtb+initrd at 128MB or if we have less RAM
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 215ef9e..1af6b4a 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -255,8 +255,8 @@ static int make_psci_node(libxl__gc *gc, void *fdt)
     return 0;
 }
 
-static int make_memory_node(libxl__gc *gc, void *fdt,
-                            uint64_t base, uint64_t size)
+static int make_one_memory_node(libxl__gc *gc, void *fdt,
+                                uint64_t base, uint64_t size)
 {
     int res;
     const char *name = GCSPRINTF("memory@%"PRIx64, base);
@@ -277,6 +277,23 @@ static int make_memory_node(libxl__gc *gc, void *fdt,
     return 0;
 }
 
+static int make_memory_node(libxl__gc *gc, void *fdt, uint64_t size)
+{
+    int res;
+    /* This had better match libxc's arch_setup_meminit... */
+    const uint64_t size0 = size > GUEST_RAM0_SIZE ? GUEST_RAM0_SIZE : size;
+    const uint64_t size1 = size > GUEST_RAM0_SIZE ? size - size0 : 0;
+
+    res = make_one_memory_node(gc, fdt, GUEST_RAM0_BASE, size0);
+    if (res) return res;
+    if (size1) {
+        res = make_one_memory_node(gc, fdt, GUEST_RAM1_BASE, size1);
+        if (res) return res;
+    }
+
+    return 0;
+}
+
 static int make_intc_node(libxl__gc *gc, void *fdt,
                           uint64_t gicd_base, uint64_t gicd_size,
                           uint64_t gicc_base, uint64_t gicc_size)
@@ -490,7 +507,6 @@ next_resize:
         FDT( make_psci_node(gc, fdt) );
 
         FDT( make_memory_node(gc, fdt,
-                              dom->rambase_pfn << XC_PAGE_SHIFT,
                               info->target_memkb * 1024) );
         FDT( make_intc_node(gc, fdt,
                             GUEST_GICD_BASE, GUEST_GICD_SIZE,
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 41944fe..749b7b3 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -375,12 +375,15 @@ typedef uint64_t xen_callback_t;
 #define GUEST_MAGIC_BASE  0x39000000ULL
 #define GUEST_MAGIC_SIZE  0x01000000ULL
 
-#define GUEST_RAM0_BASE   0x40000000ULL /* 3GB of RAM @ 1GB */
+#define GUEST_RAM0_BASE   0x40000000ULL /* 3GB of low RAM @ 1GB */
 #define GUEST_RAM0_SIZE   0xc0000000ULL
 
+#define GUEST_RAM1_BASE   0x0200000000ULL /* 1016GB of RAM @ 8GB */
+#define GUEST_RAM1_SIZE   0xfe00000000ULL
+
 #define GUEST_RAM_BASE    GUEST_RAM0_BASE /* Lowest RAM address */
 /* Largest amount of actual RAM, not including holes */
-#define GUEST_RAM_MAX     (GUEST_RAM0_SIZE)
+#define GUEST_RAM_MAX     (GUEST_RAM0_SIZE + GUEST_RAM1_SIZE)
 
 /* Interrupts */
 #define GUEST_TIMER_VIRT_PPI    27
-- 
1.7.10.4

Re: [PATCH v3 7/8] tools: arm: support up to (almost) 1TB of guest RAM

[Ian Jackson]

Ian Campbell writes:
> This creates a second bank of RAM starting at 8GB and potentially extending \
to
> the 1TB boundary, which is the limit imposed by our current use of a 3 level
> p2m with 2 pages at level 0 (2^40 bits).
>
> I've deliberately left a gap between the two banks just to exercise those co\
de paths.
> 
> The second bank is 1016GB in size which plus the 3GB below 4GB is 1019GB
> maximum guest RAM. At the point where the fact that this is slightly less th\
an
> a full TB starts to become an issue for people then we can switch to a 4 lev\
el

Your commit messages have wrap damage when quoted.  Can you make your
editor shrink them a bit ?

> -    const uint64_t ram0size = ramsize;
> +    const uint64_t ram0size =
> +        ramsize > GUEST_RAM0_SIZE ? GUEST_RAM0_SIZE : ramsize;
>      const uint64_t ram0end = GUEST_RAM0_BASE + ram0size;
> +    const uint64_t ram1size =
> +        ramsize > ram0size ? ramsize - ram0size : 0;
> +    const uint64_t ram1end = GUEST_RAM1_BASE + ram1size;

Why ram0 and ram1 not ram[0] and ram[1] ?  If the latter

> +    if ((rc = populate_guest_memory(dom,
> +                                    GUEST_RAM1_BASE >> XC_PAGE_SHIFT,
> +                                    ram1size >> XC_PAGE_SHIFT)))
> +        return rc;

this could perhaps be a loop.  (You would need
  const uint64_t guest_ram_size[2] = { GUEST_RAM_SIZES };
or, if you must,
  const uint64_t guest_ram_size[2] = { GUEST_RAM0_SIZE, GUEST_RAM1_SIZE };
or something.)

> +static int make_memory_node(libxl__gc *gc, void *fdt, uint64_t size)
> +{
> +    int res;
> +    /* This had better match libxc's arch_setup_meminit... */
> +    const uint64_t size0 = size > GUEST_RAM0_SIZE ? GUEST_RAM0_SIZE : size;
> +    const uint64_t size1 = size > GUEST_RAM0_SIZE ? size - size0 : 0;

Glrgk.  Is it necessary to have this in two places ?

Ian.

Re: [PATCH v3 7/8] tools: arm: support up to (almost) 1TB of guest RAM

[Ian Campbell]

On Fri, 2014-05-02 at 15:50 +0100, Ian Jackson wrote:
> Ian Campbell writes:
> > This creates a second bank of RAM starting at 8GB and potentially extending \
> to
> > the 1TB boundary, which is the limit imposed by our current use of a 3 level
> > p2m with 2 pages at level 0 (2^40 bits).
> >
> > I've deliberately left a gap between the two banks just to exercise those co\
> de paths.
> > 
> > The second bank is 1016GB in size which plus the 3GB below 4GB is 1019GB
> > maximum guest RAM. At the point where the fact that this is slightly less th\
> an
> > a full TB starts to become an issue for people then we can switch to a 4 lev\
> el
> 
> Your commit messages have wrap damage when quoted.  Can you make your
> editor shrink them a bit ?

I shall try.

> > -    const uint64_t ram0size = ramsize;
> > +    const uint64_t ram0size =
> > +        ramsize > GUEST_RAM0_SIZE ? GUEST_RAM0_SIZE : ramsize;
> >      const uint64_t ram0end = GUEST_RAM0_BASE + ram0size;
> > +    const uint64_t ram1size =
> > +        ramsize > ram0size ? ramsize - ram0size : 0;
> > +    const uint64_t ram1end = GUEST_RAM1_BASE + ram1size;
> 
> Why ram0 and ram1 not ram[0] and ram[1] ?  If the latter
> 
> > +    if ((rc = populate_guest_memory(dom,
> > +                                    GUEST_RAM1_BASE >> XC_PAGE_SHIFT,
> > +                                    ram1size >> XC_PAGE_SHIFT)))
> > +        return rc;
> 
> this could perhaps be a loop.  (You would need
>   const uint64_t guest_ram_size[2] = { GUEST_RAM_SIZES };
> or, if you must,
>   const uint64_t guest_ram_size[2] = { GUEST_RAM0_SIZE, GUEST_RAM1_SIZE };
> or something.)

I'll take a look at this.

> > +static int make_memory_node(libxl__gc *gc, void *fdt, uint64_t size)
> > +{
> > +    int res;
> > +    /* This had better match libxc's arch_setup_meminit... */
> > +    const uint64_t size0 = size > GUEST_RAM0_SIZE ? GUEST_RAM0_SIZE : size;
> > +    const uint64_t size1 = size > GUEST_RAM0_SIZE ? size - size0 : 0;
> 
> Glrgk.  Is it necessary to have this in two places ?

Stupid layering reasons sadly :-( libxc has to do the building stuff,
but the dt stuff needs things which only libxl knows.

Ian.

[PATCH v3 8/8] tools: arm: increase size of region set aside for guest grant table

[Ian Campbell]

The current size is sufficient for the default maximum grant table size
(32-frames), but increase the reserved region to 16M/4096 pages to allow for
the use of the gnttab_max_nr_frames command line option.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
v2: New patch
---
 xen/include/public/arch-arm.h |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 749b7b3..9f923dc 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -369,8 +369,11 @@ typedef uint64_t xen_callback_t;
 #define GUEST_GICC_BASE   0x03002000ULL
 #define GUEST_GICC_SIZE   0x00000100ULL
 
+/* 16MB == 4096 pages reserved for guest to use as a region to map its
+ * grant table in.
+ */
 #define GUEST_GNTTAB_BASE 0x38000000ULL
-#define GUEST_GNTTAB_SIZE 0x00020000ULL
+#define GUEST_GNTTAB_SIZE 0x01000000ULL
 
 #define GUEST_MAGIC_BASE  0x39000000ULL
 #define GUEST_MAGIC_SIZE  0x01000000ULL
-- 
1.7.10.4

Re: [PATCH v3 8/8] tools: arm: increase size of region set aside for guest grant table

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 8/8] tools: arm: increase size of region set aside for guest grant table"):
> The current size is sufficient for the default maximum grant table size
> (32-frames), but increase the reserved region to 16M/4096 pages to allow for
> the use of the gnttab_max_nr_frames command line option.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Acked-by: Julien Grall <julien.grall@linaro.org>

As before, I'm not qualified to judge the merits, but from a tools hat
pov this is fine.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Ian.