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

[PATCH v4 0/9] 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.

This has seen a bit of a rework since v3 (which I felt was significant
enough to invalidate the acks on some of the later patches). THe two
main changes were switching to use arrays instead of ram{0,1}size etc,
which allows loops to be used and removing the duplication of the
knowledge of memory layout in both libxc and libxl by exposing libxc's
answer to libxl.

Ian.

[PATCH v4 1/9] 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>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
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

[PATCH v4 2/9] 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>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
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

[PATCH v4 3/9] 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>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
--
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

[PATCH v4 4/9] 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>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 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

[PATCH v4 5/9] 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>
---
v4: Moved before "prepare for multiple banks of guest RAM" to allow for
    refactoring of that patch.
    Dropped unnecessary !nr_pfns early return from populate_guest_memory.

v2: New patch
---
 tools/libxc/xc_dom_arm.c |   60 +++++++++++++++++++++++++++++++---------------
 1 file changed, 41 insertions(+), 19 deletions(-)

diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index 74917c3..8c6a317 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -248,16 +248,48 @@ 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;
+
+    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 */
     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 xen_pfn_t p2m_size = dom->total_pages;
+
     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;
@@ -289,27 +321,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_RAM_BASE >> XC_PAGE_SHIFT,
+                                    ramsize >> XC_PAGE_SHIFT)))
+        return rc;
 
     /*
      * We try to place dtb+initrd at 128MB or if we have less RAM
-- 
1.7.10.4

[PATCH v4 6/9] tools: arm: prepare domain builder for multiple banks of guest RAM

[Ian Campbell]

Prepare for adding more banks of guest RAM by renaming a bunch of defines
as RAM0 and replacing variables with arrays and introducing loops.

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 rambank_size[0] (size of first bank)
are the same, but use the appropriate one for each context.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: Substantial rework, hence dropped existing acks
    Switched to using arrays instead of ram{0,1}* variables.
v3: Mention why GUEST_RAM_BASE is still defined.
v2: New patch
---
 tools/libxc/xc_dom_arm.c      |   71 ++++++++++++++++++++++++++++++-----------
 xen/include/public/arch-arm.h |   10 ++++--
 2 files changed, 61 insertions(+), 20 deletions(-)

diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index 8c6a317..6c255ac 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>
@@ -279,17 +280,20 @@ static int populate_guest_memory(struct xc_dom_image *dom,
 
 int arch_setup_meminit(struct xc_dom_image *dom)
 {
-    int rc;
+    int i, rc;
     xen_pfn_t pfn;
     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;
+    uint64_t ramsize = (uint64_t)dom->total_pages << XC_PAGE_SHIFT;
 
-    const xen_pfn_t p2m_size = dom->total_pages;
+    const uint64_t bankbase[GUEST_RAM_BANKS] = {
+        GUEST_RAM0_BASE
+    };
+    const uint64_t bankmax[GUEST_RAM_BANKS] = {
+        GUEST_RAM0_SIZE
+    };
 
+    /* Convenient */
     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;
@@ -298,20 +302,32 @@ 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);
+
+    xen_pfn_t p2m_size;
+    xen_pfn_t rambank_size[GUEST_RAM_BANKS];
+    uint64_t bank0end;
 
-    if ( modsize + kernsize > ramsize )
+    assert(dom->rambase_pfn << XC_PAGE_SHIFT == GUEST_RAM0_BASE);
+
+    if ( modsize + kernsize > bankmax[0] )
     {
         DOMPRINTF("%s: Not enough memory for the kernel+dtb+initrd",
                   __FUNCTION__);
         return -1;
     }
 
-    if ( ramsize > GUEST_RAM_SIZE )
+    if ( ramsize == 0 )
+    {
+        DOMPRINTF("%s: ram size is 0", __FUNCTION__);
+        return -1;
+    }
+
+    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;
     }
 
@@ -321,6 +337,20 @@ int arch_setup_meminit(struct xc_dom_image *dom)
 
     dom->shadow_enabled = 1;
 
+    for (i = 0; ramsize && i < GUEST_RAM_BANKS; i++)
+    {
+        uint64_t banksize = ramsize > bankmax[i] ? bankmax[i] : ramsize;
+
+        ramsize -= banksize;
+
+        p2m_size = ( bankbase[i] + banksize - bankbase[0] ) >> XC_PAGE_SHIFT;
+
+        rambank_size[i] = banksize >> XC_PAGE_SHIFT;
+    }
+
+    assert(rambank_size[0] != 0);
+    assert(ramsize == 0); /* Too much RAM is rejected above */
+
     dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * p2m_size);
     if ( dom->p2m_host == NULL )
         return -EINVAL;
@@ -328,10 +358,13 @@ int arch_setup_meminit(struct xc_dom_image *dom)
         dom->p2m_host[pfn] = INVALID_MFN;
 
     /* setup initial p2m and allocate guest memory */
-    if ((rc = populate_guest_memory(dom,
-                                    GUEST_RAM_BASE >> XC_PAGE_SHIFT,
-                                    ramsize >> XC_PAGE_SHIFT)))
-        return rc;
+    for (i = 0; rambank_size[i] && i < GUEST_RAM_BANKS; i++)
+    {
+        if ((rc = populate_guest_memory(dom,
+                                        bankbase[i] >> XC_PAGE_SHIFT,
+                                        rambank_size[i])))
+            return rc;
+    }
 
     /*
      * We try to place dtb+initrd at 128MB or if we have less RAM
@@ -341,11 +374,13 @@ 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 )
+    bank0end = bankbase[0] + ((uint64_t)rambank_size[0] << XC_PAGE_SHIFT);
+
+    if ( bank0end >= ram128mb + modsize && kernend < ram128mb )
         modbase = ram128mb;
-    else if ( ramend - modsize > kernend )
-        modbase = ramend - modsize;
-    else if (kernbase - rambase > modsize )
+    else if ( bank0end - modsize > kernend )
+        modbase = bank0end - 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..96c8786 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -375,8 +375,14 @@ 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_RAM_BANKS   1
+
+#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 v4 6/9] tools: arm: prepare domain builder for multiple banks of guest RAM

[Julien Grall]

Hi Ian,

On 05/13/2014 12:29 PM, Ian Campbell wrote:
> +    /* Convenient */
>      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;
> @@ -298,20 +302,32 @@ 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);

I would use bankbase[0] here for more clarity.

[..]

> @@ -321,6 +337,20 @@ int arch_setup_meminit(struct xc_dom_image *dom)
>  
>      dom->shadow_enabled = 1;
>  
> +    for (i = 0; ramsize && i < GUEST_RAM_BANKS; i++)

for ( ... )

[..]

>      /* setup initial p2m and allocate guest memory */
> -    if ((rc = populate_guest_memory(dom,
> -                                    GUEST_RAM_BASE >> XC_PAGE_SHIFT,
> -                                    ramsize >> XC_PAGE_SHIFT)))
> -        return rc;
> +    for (i = 0; rambank_size[i] && i < GUEST_RAM_BANKS; i++)

for ( ... )

Regards,

-- 
Julien Grall

[PATCH v4 7/9] tools: arm: prepare guest FDT building for multiple RAM banks

[Ian Campbell]

This required exposing the sizes of the banks determined by the domain builder
up to libxl via xc_dom_image.

Since the domain build needs to know the size of the DTB we create placeholder
nodes for each possible bank and when we finialise the DTB we fill in the ones
which are actually populated and NOP out the rest.

Note that the number of guest RAM banks is still 1 after this change.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: New patch
---
 tools/libxc/xc_dom.h     |   10 +++++-
 tools/libxc/xc_dom_arm.c |   12 +++-----
 tools/libxl/libxl_arm.c  |   77 ++++++++++++++++++++++++++++++++++++----------
 3 files changed, 74 insertions(+), 25 deletions(-)

diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
index c9af0ce..6ae6a9f 100644
--- a/tools/libxc/xc_dom.h
+++ b/tools/libxc/xc_dom.h
@@ -114,13 +114,21 @@ struct xc_dom_image {
 
     /* physical memory
      *
-     * A PV guest has a single contiguous block of physical RAM,
+     * An x86 PV guest has a single contiguous block of physical RAM,
      * consisting of total_pages starting at rambase_pfn.
+     *
+     * An ARM guest has GUEST_RAM_BANKS regions of RAM, with
+     * rambank_size[i] pages in each. The lowest RAM address
+     * (corresponding to the base of the p2m arrays above) is stored
+     * in rambase_pfn.
      */
     xen_pfn_t rambase_pfn;
     xen_pfn_t total_pages;
     struct xc_dom_phys *phys_pages;
     int realmodearea_log;
+#if defined (__arm__) || defined(__aarch64__)
+    xen_pfn_t rambank_size[GUEST_RAM_BANKS];
+#endif
 
     /* malloc memory pool */
     struct xc_dom_mem *memblocks;
diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index 6c255ac..c83965d 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -305,7 +305,6 @@ int arch_setup_meminit(struct xc_dom_image *dom)
     const uint64_t ram128mb = GUEST_RAM0_BASE + (128<<20);
 
     xen_pfn_t p2m_size;
-    xen_pfn_t rambank_size[GUEST_RAM_BANKS];
     uint64_t bank0end;
 
     assert(dom->rambase_pfn << XC_PAGE_SHIFT == GUEST_RAM0_BASE);
@@ -345,10 +344,10 @@ int arch_setup_meminit(struct xc_dom_image *dom)
 
         p2m_size = ( bankbase[i] + banksize - bankbase[0] ) >> XC_PAGE_SHIFT;
 
-        rambank_size[i] = banksize >> XC_PAGE_SHIFT;
+        dom->rambank_size[i] = banksize >> XC_PAGE_SHIFT;
     }
 
-    assert(rambank_size[0] != 0);
+    assert(dom->rambank_size[0] != 0);
     assert(ramsize == 0); /* Too much RAM is rejected above */
 
     dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * p2m_size);
@@ -358,11 +357,10 @@ int arch_setup_meminit(struct xc_dom_image *dom)
         dom->p2m_host[pfn] = INVALID_MFN;
 
     /* setup initial p2m and allocate guest memory */
-    for (i = 0; rambank_size[i] && i < GUEST_RAM_BANKS; i++)
-    {
+    for (i = 0; dom->rambank_size[i] && i < GUEST_RAM_BANKS; i++) {
         if ((rc = populate_guest_memory(dom,
                                         bankbase[i] >> XC_PAGE_SHIFT,
-                                        rambank_size[i])))
+                                        dom->rambank_size[i])))
             return rc;
     }
 
@@ -374,7 +372,7 @@ int arch_setup_meminit(struct xc_dom_image *dom)
      * If changing this then consider
      * xen/arch/arm/kernel.c:place_modules as well.
      */
-    bank0end = bankbase[0] + ((uint64_t)rambank_size[0] << XC_PAGE_SHIFT);
+    bank0end = bankbase[0] + ((uint64_t)dom->rambank_size[0] << XC_PAGE_SHIFT);
 
     if ( bank0end >= ram128mb + modsize && kernend < ram128mb )
         modbase = ram128mb;
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 215ef9e..90fca67 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -255,24 +255,33 @@ 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_memory_nodes(libxl__gc *gc, void *fdt,
+                             const struct xc_dom_image *dom)
 {
-    int res;
-    const char *name = GCSPRINTF("memory@%"PRIx64, base);
+    int res, i;
+    const char *name;
+    const uint64_t bankbase[GUEST_RAM_BANKS] = {
+        GUEST_RAM0_BASE
+    };
 
-    res = fdt_begin_node(fdt, name);
-    if (res) return res;
+    for (i = 0; i < GUEST_RAM_BANKS; i++) {
+        name = GCSPRINTF("memory@%"PRIx64, bankbase[i]);
 
-    res = fdt_property_string(fdt, "device_type", "memory");
-    if (res) return res;
+        LOG(DEBUG, "Creating placeholder node /%s", name);
 
-    res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
-                            1, base, size);
-    if (res) return res;
+        res = fdt_begin_node(fdt, name);
+        if (res) return res;
 
-    res = fdt_end_node(fdt);
-    if (res) return res;
+        res = fdt_property_string(fdt, "device_type", "memory");
+        if (res) return res;
+
+        res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
+                                1, 0, 0);
+        if (res) return res;
+
+        res = fdt_end_node(fdt);
+        if (res) return res;
+    }
 
     return 0;
 }
@@ -489,9 +498,7 @@ next_resize:
         FDT( make_cpus_node(gc, fdt, info->max_vcpus, ainfo) );
         FDT( make_psci_node(gc, fdt) );
 
-        FDT( make_memory_node(gc, fdt,
-                              dom->rambase_pfn << XC_PAGE_SHIFT,
-                              info->target_memkb * 1024) );
+        FDT( make_memory_nodes(gc, fdt, dom) );
         FDT( make_intc_node(gc, fdt,
                             GUEST_GICD_BASE, GUEST_GICD_SIZE,
                             GUEST_GICC_BASE, GUEST_GICD_SIZE) );
@@ -521,11 +528,40 @@ out:
     return rc;
 }
 
+static void finalise_one_memory_node(libxl__gc *gc, void *fdt,
+                                     uint64_t base, uint64_t size)
+{
+    int node, res;
+    const char *name = GCSPRINTF("/memory@%"PRIx64, base);
+
+    node = fdt_path_offset(fdt, name);
+    assert(node > 0);
+
+    if (size == 0) {
+        LOG(DEBUG, "Nopping out placeholder node %s", name);
+        fdt_nop_node(fdt, node);
+    } else {
+        uint32_t regs[ROOT_ADDRESS_CELLS+ROOT_SIZE_CELLS];
+        be32 *cells = &regs[0];
+
+        LOG(DEBUG, "Populating placeholder node %s", name);
+
+        set_range(&cells, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS, base, size);
+
+        res = fdt_setprop_inplace(fdt, node, "reg", regs, sizeof(regs));
+        assert(!res);
+    }
+}
+
 int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
                                                libxl_domain_build_info *info,
                                                struct xc_dom_image *dom)
 {
     void *fdt = dom->devicetree_blob;
+    int i;
+    const uint64_t bankbase[GUEST_RAM_BANKS] = {
+        GUEST_RAM0_BASE
+    };
 
     const struct xc_dom_seg *ramdisk = dom->ramdisk_blob ?
         &dom->ramdisk_seg : NULL;
@@ -552,9 +588,16 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
         assert(!res);
 
         val = cpu_to_fdt64(ramdisk->vend);
-        res = fdt_setprop_inplace(fdt, chosen,PROP_INITRD_END,
+        res = fdt_setprop_inplace(fdt, chosen, PROP_INITRD_END,
                                   &val, sizeof(val));
         assert(!res);
+
+    }
+
+    for (i = 0; i < GUEST_RAM_BANKS; i++) {
+        const uint64_t size = (uint64_t)dom->rambank_size[i] << XC_PAGE_SHIFT;
+
+        finalise_one_memory_node(gc, fdt, bankbase[i], size);
     }
 
     debug_dump_fdt(gc, fdt);
-- 
1.7.10.4

Re: [PATCH v4 7/9] tools: arm: prepare guest FDT building for multiple RAM banks

[Julien Grall]

Hi Ian,

On 05/13/2014 12:29 PM, Ian Campbell wrote:
> This required exposing the sizes of the banks determined by the domain builder
> up to libxl via xc_dom_image.
> 
> Since the domain build needs to know the size of the DTB we create placeholder
> nodes for each possible bank and when we finialise the DTB we fill in the ones

finalise

> which are actually populated and NOP out the rest.

It's annoying that we can't dissociate bank setup (i.e defining the size
of each banks) and placing modules (kernel, DTB,...).

It would avoid to have 2 step to create memory node.

Anyway, I'm fine with this solution for now.

[..]

>  int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
>                                                 libxl_domain_build_info *info,
>                                                 struct xc_dom_image *dom)
>  {
>      void *fdt = dom->devicetree_blob;
> +    int i;
> +    const uint64_t bankbase[GUEST_RAM_BANKS] = {
> +        GUEST_RAM0_BASE
> +    };

You've duplicate this variable in multiple place (twice in libxl and
once in libxc).

I think it's better if we can avoid duplicating this variable, maybe by
storing in xc_dom_image?

>  
>      const struct xc_dom_seg *ramdisk = dom->ramdisk_blob ?
>          &dom->ramdisk_seg : NULL;
> @@ -552,9 +588,16 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
>          assert(!res);
>  
>          val = cpu_to_fdt64(ramdisk->vend);
> -        res = fdt_setprop_inplace(fdt, chosen,PROP_INITRD_END,
> +        res = fdt_setprop_inplace(fdt, chosen, PROP_INITRD_END,
>                                    &val, sizeof(val));

Spurious change?

Regards,

-- 
Julien Grall

Re: [PATCH v4 7/9] tools: arm: prepare guest FDT building for multiple RAM banks

[Ian Campbell]

On Tue, 2014-05-13 at 13:35 +0100, Julien Grall wrote:
> Hi Ian,
> 
> On 05/13/2014 12:29 PM, Ian Campbell wrote:
> > This required exposing the sizes of the banks determined by the domain builder
> > up to libxl via xc_dom_image.
> > 
> > Since the domain build needs to know the size of the DTB we create placeholder
> > nodes for each possible bank and when we finialise the DTB we fill in the ones
> 
> finalise
> 
> > which are actually populated and NOP out the rest.
> 
> It's annoying that we can't dissociate bank setup (i.e defining the size
> of each banks) and placing modules (kernel, DTB,...).
> 
> It would avoid to have 2 step to create memory node.

We have to have 2 steps for the initrd case (that's unavoidable), given
that having something similar for the memory is not the end of the
world.

> Anyway, I'm fine with this solution for now.
> 
> [..]
> 
> >  int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
> >                                                 libxl_domain_build_info *info,
> >                                                 struct xc_dom_image *dom)
> >  {
> >      void *fdt = dom->devicetree_blob;
> > +    int i;
> > +    const uint64_t bankbase[GUEST_RAM_BANKS] = {
> > +        GUEST_RAM0_BASE
> > +    };
> 
> You've duplicate this variable in multiple place (twice in libxl and
> once in libxc).
> 
> I think it's better if we can avoid duplicating this variable, maybe by
> storing in xc_dom_image?

I'll take a look. Since these are currently constants I might just
#define GUEST_RAM_BASES { GUEST_RAM0_BASE , GUEST_RAM1_BASE }
So that these become const uunt64_t bankbase[] = GUEST_RAM_BASES;

> 
> >  
> >      const struct xc_dom_seg *ramdisk = dom->ramdisk_blob ?
> >          &dom->ramdisk_seg : NULL;
> > @@ -552,9 +588,16 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
> >          assert(!res);
> >  
> >          val = cpu_to_fdt64(ramdisk->vend);
> > -        res = fdt_setprop_inplace(fdt, chosen,PROP_INITRD_END,
> > +        res = fdt_setprop_inplace(fdt, chosen, PROP_INITRD_END,
> >                                    &val, sizeof(val));
> 
> Spurious change?

It was deliberate, I just spotted the coding style error while I was in
the region. Naughty to slip it in here I know. I'll mention it in the
commit log.

Ian.

[PATCH v4 8/9] 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>
---
v4: Significantly reworked (simplified) due to changes in earlier patches.
    Removed existing Acks.
v3: remove inadvertent whitespace change
---
 tools/libxc/xc_dom_arm.c      |    4 ++--
 tools/libxl/libxl_arm.c       |    4 ++--
 xen/include/public/arch-arm.h |    9 ++++++---
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index c83965d..549caa1 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -287,10 +287,10 @@ int arch_setup_meminit(struct xc_dom_image *dom)
     uint64_t ramsize = (uint64_t)dom->total_pages << XC_PAGE_SHIFT;
 
     const uint64_t bankbase[GUEST_RAM_BANKS] = {
-        GUEST_RAM0_BASE
+        GUEST_RAM0_BASE, GUEST_RAM1_BASE
     };
     const uint64_t bankmax[GUEST_RAM_BANKS] = {
-        GUEST_RAM0_SIZE
+        GUEST_RAM0_SIZE, GUEST_RAM1_SIZE
     };
 
     /* Convenient */
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 90fca67..058bed3 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -261,7 +261,7 @@ static int make_memory_nodes(libxl__gc *gc, void *fdt,
     int res, i;
     const char *name;
     const uint64_t bankbase[GUEST_RAM_BANKS] = {
-        GUEST_RAM0_BASE
+        GUEST_RAM0_BASE, GUEST_RAM1_BASE
     };
 
     for (i = 0; i < GUEST_RAM_BANKS; i++) {
@@ -560,7 +560,7 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
     void *fdt = dom->devicetree_blob;
     int i;
     const uint64_t bankbase[GUEST_RAM_BANKS] = {
-        GUEST_RAM0_BASE
+        GUEST_RAM0_BASE, GUEST_RAM1_BASE
     };
 
     const struct xc_dom_seg *ramdisk = dom->ramdisk_blob ?
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 96c8786..935a1eb 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -375,14 +375,17 @@ typedef uint64_t xen_callback_t;
 #define GUEST_MAGIC_BASE  0x39000000ULL
 #define GUEST_MAGIC_SIZE  0x01000000ULL
 
-#define GUEST_RAM_BANKS   1
+#define GUEST_RAM_BANKS   2
 
-#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

[PATCH v4 9/9] 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>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
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 935a1eb..cb74897 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