| Message ID | 1397044276-30185-2-git-send-email-ian.campbell@citrix.com |
|---|---|
| State | Accepted |
| Commit | 00dea7acf444891f789aad213c871095422fe9d0 |
| Headers | show |
On 04/09/2014 12:51 PM, Ian Campbell wrote: > static void place_modules(struct kernel_info *info, > - paddr_t kernel_start, > - paddr_t kernel_end) > + paddr_t kernbase, paddr_t kernend) > { > /* Align DTB and initrd size to 2Mb. Linux only requires 4 byte alignment */ > const paddr_t initrd_len = > ROUNDUP(early_info.modules.module[MOD_INITRD].size, MB(2)); > const paddr_t dtb_len = ROUNDUP(fdt_totalsize(info->fdt), MB(2)); > - const paddr_t total = initrd_len + dtb_len; > + const paddr_t modsize = initrd_len + dtb_len; > > /* Convenient */ > - const paddr_t mem_start = info->mem.bank[0].start; > - const paddr_t mem_size = info->mem.bank[0].size; > - const paddr_t mem_end = mem_start + mem_size; > - const paddr_t kernel_size = kernel_end - kernel_start; > + const paddr_t rambase = info->mem.bank[0].start; > + const paddr_t ramsize = info->mem.bank[0].size; > + const paddr_t ramend = rambase + ramsize; > + const paddr_t kernsize = kernend - kernbase; > + const paddr_t ram128mb = rambase + MB(128); Shall we use ram0base, ram0size, ram0end to show that we are using that the first bank? Regards,
On Wed, 2014-04-09 at 13:45 +0100, Julien Grall wrote: > On 04/09/2014 12:51 PM, Ian Campbell wrote: > > static void place_modules(struct kernel_info *info, > > - paddr_t kernel_start, > > - paddr_t kernel_end) > > + paddr_t kernbase, paddr_t kernend) > > { > > /* Align DTB and initrd size to 2Mb. Linux only requires 4 byte alignment */ > > const paddr_t initrd_len = > > ROUNDUP(early_info.modules.module[MOD_INITRD].size, MB(2)); > > const paddr_t dtb_len = ROUNDUP(fdt_totalsize(info->fdt), MB(2)); > > - const paddr_t total = initrd_len + dtb_len; > > + const paddr_t modsize = initrd_len + dtb_len; > > > > /* Convenient */ > > - const paddr_t mem_start = info->mem.bank[0].start; > > - const paddr_t mem_size = info->mem.bank[0].size; > > - const paddr_t mem_end = mem_start + mem_size; > > - const paddr_t kernel_size = kernel_end - kernel_start; > > + const paddr_t rambase = info->mem.bank[0].start; > > + const paddr_t ramsize = info->mem.bank[0].size; > > + const paddr_t ramend = rambase + ramsize; > > + const paddr_t kernsize = kernend - kernbase; > > + const paddr_t ram128mb = rambase + MB(128); > > Shall we use ram0base, ram0size, ram0end to show that we are using that > the first bank? I was planning to do that in my 1TB RAM series, but this is different code isn't it. Oops. I'll change if there is some other reason to repost, but I don't think it is worth reposting just for that. Ian.
On 04/09/2014 01:52 PM, Ian Campbell wrote: > On Wed, 2014-04-09 at 13:45 +0100, Julien Grall wrote: >> On 04/09/2014 12:51 PM, Ian Campbell wrote: >>> static void place_modules(struct kernel_info *info, >>> - paddr_t kernel_start, >>> - paddr_t kernel_end) >>> + paddr_t kernbase, paddr_t kernend) >>> { >>> /* Align DTB and initrd size to 2Mb. Linux only requires 4 byte alignment */ >>> const paddr_t initrd_len = >>> ROUNDUP(early_info.modules.module[MOD_INITRD].size, MB(2)); >>> const paddr_t dtb_len = ROUNDUP(fdt_totalsize(info->fdt), MB(2)); >>> - const paddr_t total = initrd_len + dtb_len; >>> + const paddr_t modsize = initrd_len + dtb_len; >>> >>> /* Convenient */ >>> - const paddr_t mem_start = info->mem.bank[0].start; >>> - const paddr_t mem_size = info->mem.bank[0].size; >>> - const paddr_t mem_end = mem_start + mem_size; >>> - const paddr_t kernel_size = kernel_end - kernel_start; >>> + const paddr_t rambase = info->mem.bank[0].start; >>> + const paddr_t ramsize = info->mem.bank[0].size; >>> + const paddr_t ramend = rambase + ramsize; >>> + const paddr_t kernsize = kernend - kernbase; >>> + const paddr_t ram128mb = rambase + MB(128); >> >> Shall we use ram0base, ram0size, ram0end to show that we are using that >> the first bank? > > I was planning to do that in my 1TB RAM series, but this is different > code isn't it. Oops. > > I'll change if there is some other reason to repost, but I don't think > it is worth reposting just for that. Ok. With or without this change: Acked-by: Julien Grall <julien.grall@linaro.org>
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c index ae86772..bc625a4 100644 --- a/xen/arch/arm/kernel.c +++ b/xen/arch/arm/kernel.c @@ -65,25 +65,25 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len) } static void place_modules(struct kernel_info *info, - paddr_t kernel_start, - paddr_t kernel_end) + paddr_t kernbase, paddr_t kernend) { /* Align DTB and initrd size to 2Mb. Linux only requires 4 byte alignment */ const paddr_t initrd_len = ROUNDUP(early_info.modules.module[MOD_INITRD].size, MB(2)); const paddr_t dtb_len = ROUNDUP(fdt_totalsize(info->fdt), MB(2)); - const paddr_t total = initrd_len + dtb_len; + const paddr_t modsize = initrd_len + dtb_len; /* Convenient */ - const paddr_t mem_start = info->mem.bank[0].start; - const paddr_t mem_size = info->mem.bank[0].size; - const paddr_t mem_end = mem_start + mem_size; - const paddr_t kernel_size = kernel_end - kernel_start; + const paddr_t rambase = info->mem.bank[0].start; + const paddr_t ramsize = info->mem.bank[0].size; + const paddr_t ramend = rambase + ramsize; + const paddr_t kernsize = kernend - kernbase; + const paddr_t ram128mb = rambase + MB(128); - paddr_t addr; + paddr_t modbase; - if ( total + kernel_size > mem_size ) - panic("Not enough memory in the first bank for the dtb+initrd"); + if ( modsize + kernsize > ramsize ) + panic("Not enough memory in the first bank for the kernel+dtb+initrd"); /* * DTB must be loaded such that it does not conflict with the @@ -100,19 +100,19 @@ static void place_modules(struct kernel_info *info, * just after the kernel, if there is room, otherwise just before. */ - if ( kernel_end < MIN(mem_start + MB(128), mem_end - total) ) - addr = MIN(mem_start + MB(128), mem_end - total); - else if ( mem_end - ROUNDUP(kernel_end, MB(2)) >= total ) - addr = ROUNDUP(kernel_end, MB(2)); - else if ( kernel_start - mem_start >= total ) - addr = kernel_start - total; + if ( kernend < MIN(ram128mb, ramend - modsize) ) + modbase = MIN(ram128mb, ramend - modsize); + else if ( ramend - ROUNDUP(kernend, MB(2)) >= modsize ) + modbase = ROUNDUP(kernend, MB(2)); + else if ( kernbase - rambase >= modsize ) + modbase = kernbase - modsize; else { panic("Unable to find suitable location for dtb+initrd"); return; } - info->dtb_paddr = addr; + info->dtb_paddr = modbase; info->initrd_paddr = info->dtb_paddr + dtb_len; }
The placement algorithm should be effectively the same and using different variable names makes my head hurt when I try to compare. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- xen/arch/arm/kernel.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-)