| Message ID | 20220512091129.2802997-6-Penny.Zheng@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | static shared memory on dom0less system | expand |
On 12/05/2022 10:11, Penny Zheng wrote: > @@ -827,6 +858,37 @@ static int __init allocate_shared_memory(struct domain *d, > } > } > > + /* > + * Get the right amount of references per page, which is the number of > + * borrow domains. > + */ > + ret = acquire_nr_borrower_domain(d, pbase, psize, &nr_borrowers); > + if ( ret ) > + return ret; > + > + /* > + * Instead of let borrower domain get a page ref, we add as many > + * additional reference as the number of borrowers when the owner > + * is allocated, since there is a chance that owner is created > + * after borrower. > + */ > + page = mfn_to_page(smfn); Where do you check that the range [smfn, smfn + nr_pages] is actual RAM? If there are none, then you should use mfn_valid() for each mfn to ensure there will be a struct page for it. Cheers,
Hi Julien > -----Original Message----- > From: Julien Grall <julien@xen.org> > Sent: Thursday, May 12, 2022 6:54 PM > To: Penny Zheng <Penny.Zheng@arm.com>; xen-devel@lists.xenproject.org > Cc: Wei Chen <Wei.Chen@arm.com>; Stefano Stabellini > <sstabellini@kernel.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>; > Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> > Subject: Re: [PATCH v3 5/8] xen/arm: Add additional reference to owner > domain when the owner is allocated > > > > On 12/05/2022 10:11, Penny Zheng wrote: > > @@ -827,6 +858,37 @@ static int __init allocate_shared_memory(struct > domain *d, > > } > > } > > > > + /* > > + * Get the right amount of references per page, which is the number of > > + * borrow domains. > > + */ > > + ret = acquire_nr_borrower_domain(d, pbase, psize, &nr_borrowers); > > + if ( ret ) > > + return ret; > > + > > + /* > > + * Instead of let borrower domain get a page ref, we add as many > > + * additional reference as the number of borrowers when the owner > > + * is allocated, since there is a chance that owner is created > > + * after borrower. > > + */ > > + page = mfn_to_page(smfn); > Where do you check that the range [smfn, smfn + nr_pages] is actual RAM? > If there are none, then you should use mfn_valid() for each mfn to ensure > there will be a struct page for it. > Actually, before this, in acquire_shared_memory_bank, we will finally call into acquire_staticmem_pages to acquire [smfn, smfn + nr_pages], and in there, we will do the check. See xen/common/page_alloc.c:acquire_staticmem_pages:2676 > Cheers, > > -- > Julien Grall
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 9c53b9fa7b..293e79e4e7 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -791,6 +791,34 @@ static mfn_t __init acquire_shared_memory_bank(struct domain *d, } +static int __init acquire_nr_borrower_domain(struct domain *d, + paddr_t pbase, paddr_t psize, + unsigned long *nr_borrowers) +{ + unsigned long bank; + + /* Iterate reserved memory to find requested shm bank. */ + for ( bank = 0 ; bank < bootinfo.reserved_mem.nr_banks; bank++ ) + { + paddr_t bank_start = bootinfo.reserved_mem.bank[bank].start; + paddr_t bank_size = bootinfo.reserved_mem.bank[bank].size; + + if ( pbase == bank_start && psize == bank_size ) + break; + } + + if ( bank == bootinfo.reserved_mem.nr_banks ) + return -ENOENT; + + if ( d == dom_io ) + *nr_borrowers = bootinfo.reserved_mem.bank[bank].nr_shm_domain; + else + /* Exclude the owner domain itself. */ + *nr_borrowers = bootinfo.reserved_mem.bank[bank].nr_shm_domain - 1; + + return 0; +} + /* * Func allocate_shared_memory is supposed to be only called * from the owner. @@ -802,6 +830,8 @@ static int __init allocate_shared_memory(struct domain *d, { mfn_t smfn; int ret = 0; + unsigned long nr_pages, nr_borrowers, i; + struct page_info *page; dprintk(XENLOG_INFO, "Allocate static shared memory BANK %#"PRIpaddr"-%#"PRIpaddr".\n", @@ -816,6 +846,7 @@ static int __init allocate_shared_memory(struct domain *d, * DOMID_IO is the domain, like DOMID_XEN, that is not auto-translated. * It sees RAM 1:1 and we do not need to create P2M mapping for it */ + nr_pages = PFN_DOWN(psize); if ( d != dom_io ) { ret = guest_physmap_add_pages(d, gaddr_to_gfn(gbase), smfn, PFN_DOWN(psize)); @@ -827,6 +858,37 @@ static int __init allocate_shared_memory(struct domain *d, } } + /* + * Get the right amount of references per page, which is the number of + * borrow domains. + */ + ret = acquire_nr_borrower_domain(d, pbase, psize, &nr_borrowers); + if ( ret ) + return ret; + + /* + * Instead of let borrower domain get a page ref, we add as many + * additional reference as the number of borrowers when the owner + * is allocated, since there is a chance that owner is created + * after borrower. + */ + page = mfn_to_page(smfn); + for ( i = 0; i < nr_pages; i++ ) + { + if ( !get_page_nr(page + i, d, nr_borrowers) ) + { + printk(XENLOG_ERR + "Failed to add %lu references to page %"PRI_mfn".\n", + nr_borrowers, mfn_x(smfn) + i); + goto fail; + } + } + + return 0; + + fail: + while ( --i >= 0 ) + put_page_nr(page + i, nr_borrowers); return ret; }
Borrower domain will fail to get a page ref using the owner domain during allocation, when the owner is created after borrower. So here, we decide to get and add the right amount of reference, which is the number of borrowers, when the owner is allocated. Signed-off-by: Penny Zheng <penny.zheng@arm.com> --- v3 change: - printk rather than dprintk since it is a serious error --- v2 change: - new commit --- xen/arch/arm/domain_build.c | 62 +++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)