| Message ID | 1398424967-9306-5-git-send-email-ian.campbell@citrix.com |
|---|---|
| State | New |
| Headers | show |
Hi Ian, On 25/04/14 12:22, Ian Campbell wrote: > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > index 4149d6f..c4f4990 100644 > --- a/xen/include/public/arch-arm.h > +++ b/xen/include/public/arch-arm.h > @@ -374,8 +374,12 @@ typedef uint64_t xen_callback_t; > > #define GUEST_MAGIC_BASE 0x39000000ULL > > -#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 */ Is it necessary to define GUEST_RAM_BASE? I don't see any usage of this define in this patch series. > +/* Largest amount of actual RAM, not including holes */ > +#define GUEST_RAM_MAX (GUEST_RAM0_SIZE) > > /* Interrupts */ > #define GUEST_TIMER_VIRT_PPI 27 Regards,
On Fri, 2014-04-25 at 13:19 +0100, Julien Grall wrote: > Hi Ian, > > On 25/04/14 12:22, Ian Campbell wrote: > > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > > index 4149d6f..c4f4990 100644 > > --- a/xen/include/public/arch-arm.h > > +++ b/xen/include/public/arch-arm.h > > @@ -374,8 +374,12 @@ typedef uint64_t xen_callback_t; > > > > #define GUEST_MAGIC_BASE 0x39000000ULL > > > > -#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 */ > > Is it necessary to define GUEST_RAM_BASE? I don't see any usage of this > define in this patch series. $ git grep GUEST_RAM_BASE tools/libxl/libxl_dom.c:#ifdef GUEST_RAM_BASE tools/libxl/libxl_dom.c: if ( (ret = xc_dom_rambase_init(dom, GUEST_RAM_BASE (this then ties back to the assert in arch_setup_meminit) Ian.
On 25/04/14 13:23, Ian Campbell wrote: > On Fri, 2014-04-25 at 13:19 +0100, Julien Grall wrote: >> Hi Ian, >> >> On 25/04/14 12:22, Ian Campbell wrote: >>> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h >>> index 4149d6f..c4f4990 100644 >>> --- a/xen/include/public/arch-arm.h >>> +++ b/xen/include/public/arch-arm.h >>> @@ -374,8 +374,12 @@ typedef uint64_t xen_callback_t; >>> >>> #define GUEST_MAGIC_BASE 0x39000000ULL >>> >>> -#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 */ >> >> Is it necessary to define GUEST_RAM_BASE? I don't see any usage of this >> define in this patch series. > > $ git grep GUEST_RAM_BASE > tools/libxl/libxl_dom.c:#ifdef GUEST_RAM_BASE > tools/libxl/libxl_dom.c: if ( (ret = xc_dom_rambase_init(dom, GUEST_RAM_BASE I forgot this one. Sorry. Acked-by: Julien Grall <julien.grall@linaro.org>
diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c index 5760bb1..8775ca4 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> @@ -253,9 +254,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; @@ -264,20 +267,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; } @@ -317,11 +322,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 4149d6f..c4f4990 100644 --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -374,8 +374,12 @@ typedef uint64_t xen_callback_t; #define GUEST_MAGIC_BASE 0x39000000ULL -#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
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). 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> --- v2: New patch --- tools/libxc/xc_dom_arm.c | 25 +++++++++++++++---------- xen/include/public/arch-arm.h | 8 ++++++-- 2 files changed, 21 insertions(+), 12 deletions(-)