| Message ID | 1396966760-7752-1-git-send-email-ian.campbell@citrix.com |
|---|---|
| State | New |
| Headers | show |
Hi Ian, On 04/08/2014 03:19 PM, Ian Campbell wrote: > 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> > --- > This is the only patch in this series which I consider to be suitable for > backporting to Xen 4.4 Sounds good backport this patch. I'm wondering if patch #2 can also be backported. > --- > tools/libxc/xc_dom_arm.c | 8 ++++++++ > xen/include/public/arch-arm.h | 3 ++- > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c > index 3330f12..c085b4a 100644 > --- a/tools/libxc/xc_dom_arm.c > +++ b/tools/libxc/xc_dom_arm.c > @@ -262,6 +262,14 @@ int arch_setup_meminit(struct xc_dom_image *dom) > const uint64_t modsize = dtb_size + ramdisk_size; > const uint64_t ram128mb = rambase + (128<<20); > > + if ( ramend - 1 > GUEST_RAM_END - NR_MAGIC_PAGES*XC_PAGE_SIZE ) Can you add parenthesis for more readability? Regards,
Hi Ian, On 04/08/2014 03:19 PM, Ian Campbell wrote: > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > index 7496556..dc11040 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 at 2GB*/ > +#define GUEST_RAM_END 0xafffffffULL I didn't catch this on the first read, every other pairs of define use _BASE AND _SIZE. Can you be consistent here? Regards,
On Tue, 2014-04-08 at 15:55 +0100, Julien Grall wrote: > Hi Ian, > > On 04/08/2014 03:19 PM, Ian Campbell wrote: > > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > > index 7496556..dc11040 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 at 2GB*/ > > +#define GUEST_RAM_END 0xafffffffULL > > I didn't catch this on the first read, every other pairs of define use > _BASE AND _SIZE. Can you be consistent here? END is what the user of this particular #define needs. But in any case SIZE gets added later on. Ian.
On 04/08/2014 04:06 PM, Ian Campbell wrote: > On Tue, 2014-04-08 at 15:55 +0100, Julien Grall wrote: >> Hi Ian, >> >> On 04/08/2014 03:19 PM, Ian Campbell wrote: >>> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h >>> index 7496556..dc11040 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 at 2GB*/ >>> +#define GUEST_RAM_END 0xafffffffULL >> >> I didn't catch this on the first read, every other pairs of define use >> _BASE AND _SIZE. Can you be consistent here? > > END is what the user of this particular #define needs. You can deal with dom->total_pages. Anyway it was just to stay consistent in the name... > But in any case SIZE gets added later on. Oh right, I didn't see it.
diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c index 3330f12..c085b4a 100644 --- a/tools/libxc/xc_dom_arm.c +++ b/tools/libxc/xc_dom_arm.c @@ -262,6 +262,14 @@ int arch_setup_meminit(struct xc_dom_image *dom) const uint64_t modsize = dtb_size + ramdisk_size; const uint64_t ram128mb = rambase + (128<<20); + if ( ramend - 1 > GUEST_RAM_END - NR_MAGIC_PAGES*XC_PAGE_SIZE ) + { + DOMPRINTF("%s: ram size is too large for guest address space: " + "%"PRIx64" > %"PRIx64, + __FUNCTION__, ramend, GUEST_RAM_END); + 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..dc11040 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 at 2GB*/ +#define GUEST_RAM_END 0xafffffffULL #define GUEST_GNTTAB_BASE 0xb0000000ULL #define GUEST_GNTTAB_SIZE 0x00020000ULL
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> --- This is the only patch in this series which I consider to be suitable for backporting to Xen 4.4 --- tools/libxc/xc_dom_arm.c | 8 ++++++++ xen/include/public/arch-arm.h | 3 ++- 2 files changed, 10 insertions(+), 1 deletion(-)