[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 4/4] xen: arm: support up to (almost) 1TB of guest RAM
Hi Ian, On 04/08/2014 03:19 PM, Ian Campbell wrote: > 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. What happen if you try to boot a non-LPAE guest with more than ~3GB? Does it boot or crash? > > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx> > --- > tools/libxc/xc_dom_arm.c | 95 > +++++++++++++++++++++++++++++------------ > tools/libxl/libxl_arm.c | 27 +++++++++--- > xen/include/public/arch-arm.h | 13 +++++- > 3 files changed, 100 insertions(+), 35 deletions(-) > > diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c > index 36b1487..65464c7 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> > @@ -245,28 +246,73 @@ 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) 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 ) It's very confusing to see pfn = rc = allocsz = 0 and each are not related. Any reason to not initialized rc/allocsz earlier? > + { > + allocsz = nr_pfns - pfn; In fact you are using allocsz here... so you don't need to initialize it. > + 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 ramend = rambase + ( dom->total_pages << XC_PAGE_SHIFT ); > + const uint64_t ramsize = (uint64_t)dom->total_pages << XC_PAGE_SHIFT; > + > + 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 = ram1size ? > + (ram1end - GUEST_RAM0_BASE) >> XC_PAGE_SHIFT : > + (ram0end - GUEST_RAM0_BASE) >> XC_PAGE_SHIFT; > + > const uint64_t kernend = ROUNDUP(dom->kernel_seg.vend, 21/*2MB*/); > const uint64_t dtb_size = dom->devicetree_blob ? > ROUNDUP(dom->devicetree_size, XC_PAGE_SHIFT) : 0; > 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); > > - if ( ramend - 1 > GUEST_RAM_END ) > + assert(dom->rambase_pfn << XC_PAGE_SHIFT == GUEST_RAM0_BASE); Can you add parenthesis here for readability? [..] > diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c > index 4f0f0e2..6170454 100644 > --- a/tools/libxl/libxl_arm.c > +++ b/tools/libxl/libxl_arm.c > @@ -255,9 +255,9 @@ static int make_psci_node(libxl__gc *gc, void *fdt) > return 0; > } > > -static int make_memory_node(libxl__gc *gc, void *fdt, > - unsigned long long base, > - unsigned long long size)> > +static int make_one_memory_node(libxl__gc *gc, void *fdt, > + unsigned long long base, > + unsigned long long size) I would use uint64_t rather unsigned long long. It's more clear the base/size are 64 bits. > { > int res; > const char *name = GCSPRINTF("memory@%08llx", base); > @@ -269,7 +269,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); > @@ -278,6 +278,24 @@ static int make_memory_node(libxl__gc *gc, void *fdt, > return 0; > } > > +static int make_memory_node(libxl__gc *gc, void *fdt, > + unsigned long long size) Same here. > +{ > + 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; > + } Any reason to not gather the both bank in one memory node? Regards, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |