| Message ID | 20221115025235.1378931-4-Penny.Zheng@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Follow-up static shared memory | expand |
Hi Penny, On 11/14/22 21:52, Penny Zheng wrote: > We split the codes of allocate_bank_memory into two parts, > allocate_domheap_memory and guest_physmap_memory. > > One is about allocating guest RAM from heap, which could be re-used later for > allocating static shared memory from heap when host address is not provided. > > The other is building up guest P2M mapping. > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> > --- > xen/arch/arm/domain_build.c | 93 +++++++++++++++++++++++++++---------- > 1 file changed, 68 insertions(+), 25 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index d2b9e60b5c..92763e96fc 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -390,34 +390,18 @@ static void __init allocate_memory_11(struct domain *d, > } > } > > -static bool __init allocate_bank_memory(struct domain *d, > - struct kernel_info *kinfo, > - gfn_t sgfn, > - paddr_t tot_size) > +static bool __init allocate_domheap_memory(struct domain *d, > + paddr_t tot_size, > + struct meminfo *mem) > { > - int res; > struct page_info *pg; > - struct membank *bank; > unsigned int max_order = ~0; > > - /* > - * allocate_bank_memory can be called with a tot_size of zero for > - * the second memory bank. It is not an error and we can safely > - * avoid creating a zero-size memory bank. > - */ > - if ( tot_size == 0 ) > - return true; > - > - bank = &kinfo->mem.bank[kinfo->mem.nr_banks]; > - bank->start = gfn_to_gaddr(sgfn); > - bank->size = tot_size; > - > while ( tot_size > 0 ) > { > unsigned int order = get_allocation_size(tot_size); > > order = min(max_order, order); > - > pg = alloc_domheap_pages(d, order, 0); > if ( !pg ) > { > @@ -437,15 +421,74 @@ static bool __init allocate_bank_memory(struct domain *d, > continue; > } > > - res = guest_physmap_add_page(d, sgfn, page_to_mfn(pg), order); > - if ( res ) > - { > - dprintk(XENLOG_ERR, "Failed map pages to DOMU: %d", res); > + if ( mem->nr_banks == NR_MEM_BANKS ) > return false; > - } > + > + mem->bank[mem->nr_banks].start = mfn_to_maddr(page_to_mfn(pg)); > + mem->bank[mem->nr_banks].size = 1UL << (PAGE_SHIFT + order); > + mem->nr_banks++; > + tot_size -= (1UL << (PAGE_SHIFT + order)); Why the change from 1ULL to 1UL? > + } > + > + return true; > +} > + > +static int __init guest_physmap_memory(struct domain *d, > + const struct meminfo *mem, gfn_t sgfn) > +{ > + unsigned int i; > + int res; > + > + for ( i = 0; i < mem->nr_banks; i++ ) > + { > + paddr_t size = mem->bank[i].size; > + unsigned int order = get_order_from_bytes(size); > + > + /* Size must be power of two */ > + BUG_ON(!size || (size & (size - 1))); > + res = guest_physmap_add_page(d, sgfn, maddr_to_mfn(mem->bank[i].start), > + order); > + if ( res ) > + return res; > > sgfn = gfn_add(sgfn, 1UL << order); > - tot_size -= (1ULL << (PAGE_SHIFT + order)); > + } > + > + return 0; > +} > + > +static bool __init allocate_bank_memory(struct domain *d, > + struct kernel_info *kinfo, > + gfn_t sgfn, > + paddr_t total_size) > +{ > + struct membank *bank; > + struct meminfo host = {0}; This function uses 6k of stack, largely due to the sizeof struct meminfo. arch/arm/domain_build.c: In function ‘allocate_bank_memory’: arch/arm/domain_build.c:461:20: warning: stack usage is 6224 bytes [-Wstack-usage=] 461 | static bool __init allocate_bank_memory(struct domain *d, | ^~~~~~~~~~~~~~~~~~~~ It may be fine for now, but I wanted to at least check if anyone else had an opinion on allocating "struct meminfo host" by some other means, such as xzalloc/free or by making it static: static struct meminfo __initdata host; memset(&host, 0, sizeof(struct meminfo)); Particularly if we ever plan on increasing NR_MEM_BANKS again in the future. > + > + /* > + * allocate_bank_memory can be called with a total_size of zero for > + * the second memory bank. It is not an error and we can safely > + * avoid creating a zero-size memory bank. > + */ > + if ( total_size == 0 ) > + return true; > + > + bank = &kinfo->mem.bank[kinfo->mem.nr_banks]; > + bank->start = gfn_to_gaddr(sgfn); > + bank->size = total_size; > + > + if ( !allocate_domheap_memory(d, total_size, &host) ) > + { > + printk(XENLOG_ERR "Failed to allocate (%"PRIpaddr"MB) pages to %pd\n", > + total_size >> 20, d); > + return false; > + } > + > + if ( guest_physmap_memory(d, &host, sgfn) ) > + { > + printk(XENLOG_ERR "Failed to map (%"PRIpaddr"MB) pages to %pd\n", > + total_size >> 20, d); > + return false; > } > > kinfo->mem.nr_banks++; > -- > 2.25.1 > >
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index d2b9e60b5c..92763e96fc 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -390,34 +390,18 @@ static void __init allocate_memory_11(struct domain *d, } } -static bool __init allocate_bank_memory(struct domain *d, - struct kernel_info *kinfo, - gfn_t sgfn, - paddr_t tot_size) +static bool __init allocate_domheap_memory(struct domain *d, + paddr_t tot_size, + struct meminfo *mem) { - int res; struct page_info *pg; - struct membank *bank; unsigned int max_order = ~0; - /* - * allocate_bank_memory can be called with a tot_size of zero for - * the second memory bank. It is not an error and we can safely - * avoid creating a zero-size memory bank. - */ - if ( tot_size == 0 ) - return true; - - bank = &kinfo->mem.bank[kinfo->mem.nr_banks]; - bank->start = gfn_to_gaddr(sgfn); - bank->size = tot_size; - while ( tot_size > 0 ) { unsigned int order = get_allocation_size(tot_size); order = min(max_order, order); - pg = alloc_domheap_pages(d, order, 0); if ( !pg ) { @@ -437,15 +421,74 @@ static bool __init allocate_bank_memory(struct domain *d, continue; } - res = guest_physmap_add_page(d, sgfn, page_to_mfn(pg), order); - if ( res ) - { - dprintk(XENLOG_ERR, "Failed map pages to DOMU: %d", res); + if ( mem->nr_banks == NR_MEM_BANKS ) return false; - } + + mem->bank[mem->nr_banks].start = mfn_to_maddr(page_to_mfn(pg)); + mem->bank[mem->nr_banks].size = 1UL << (PAGE_SHIFT + order); + mem->nr_banks++; + tot_size -= (1UL << (PAGE_SHIFT + order)); + } + + return true; +} + +static int __init guest_physmap_memory(struct domain *d, + const struct meminfo *mem, gfn_t sgfn) +{ + unsigned int i; + int res; + + for ( i = 0; i < mem->nr_banks; i++ ) + { + paddr_t size = mem->bank[i].size; + unsigned int order = get_order_from_bytes(size); + + /* Size must be power of two */ + BUG_ON(!size || (size & (size - 1))); + res = guest_physmap_add_page(d, sgfn, maddr_to_mfn(mem->bank[i].start), + order); + if ( res ) + return res; sgfn = gfn_add(sgfn, 1UL << order); - tot_size -= (1ULL << (PAGE_SHIFT + order)); + } + + return 0; +} + +static bool __init allocate_bank_memory(struct domain *d, + struct kernel_info *kinfo, + gfn_t sgfn, + paddr_t total_size) +{ + struct membank *bank; + struct meminfo host = {0}; + + /* + * allocate_bank_memory can be called with a total_size of zero for + * the second memory bank. It is not an error and we can safely + * avoid creating a zero-size memory bank. + */ + if ( total_size == 0 ) + return true; + + bank = &kinfo->mem.bank[kinfo->mem.nr_banks]; + bank->start = gfn_to_gaddr(sgfn); + bank->size = total_size; + + if ( !allocate_domheap_memory(d, total_size, &host) ) + { + printk(XENLOG_ERR "Failed to allocate (%"PRIpaddr"MB) pages to %pd\n", + total_size >> 20, d); + return false; + } + + if ( guest_physmap_memory(d, &host, sgfn) ) + { + printk(XENLOG_ERR "Failed to map (%"PRIpaddr"MB) pages to %pd\n", + total_size >> 20, d); + return false; } kinfo->mem.nr_banks++;
We split the codes of allocate_bank_memory into two parts, allocate_domheap_memory and guest_physmap_memory. One is about allocating guest RAM from heap, which could be re-used later for allocating static shared memory from heap when host address is not provided. The other is building up guest P2M mapping. Signed-off-by: Penny Zheng <penny.zheng@arm.com> --- xen/arch/arm/domain_build.c | 93 +++++++++++++++++++++++++++---------- 1 file changed, 68 insertions(+), 25 deletions(-)