diff mbox

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

Message ID 1398690132-5651-7-git-send-email-ian.campbell@citrix.com
State New
Headers show

Commit Message

Ian Campbell April 28, 2014, 1:02 p.m. UTC 
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(-)

Comments

Ian Jackson May 2, 2014, 2:50 p.m. UTC | #1 
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.
Ian Campbell May 2, 2014, 3:35 p.m. UTC | #2 
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.
diff mbox

Patch

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