[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] RE: [PATCH v1 06/13] xen/arm: set up shared memory foreign mapping for borrower domain
Hi > -----Original Message----- > From: Julien Grall <julien@xxxxxxx> > Sent: Saturday, April 9, 2022 5:31 PM > To: Penny Zheng <Penny.Zheng@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx > Cc: nd <nd@xxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; Bertrand > Marquis <Bertrand.Marquis@xxxxxxx>; Volodymyr Babchuk > <Volodymyr_Babchuk@xxxxxxxx> > Subject: Re: [PATCH v1 06/13] xen/arm: set up shared memory foreign > mapping for borrower domain > > Hi, > > On 08/04/2022 23:59, Julien Grall wrote: > > On 11/03/2022 06:11, Penny Zheng wrote: > >> From: Penny Zheng <penny.zheng@xxxxxxx> > >> > >> This commits introduces a new helper guest_physmap_add_shm to set up > >> shared memory foreign mapping for borrower domain. > >> > >> Firstly it should get and take reference of statically shared pages > >> from owner dom_shared. Then it will setup P2M foreign memory map of > >> these statically shared pages for borrower domain. > >> > >> This commits only considers owner domain is the default dom_shared, > >> the other scenario will be covered in the following patches. > >> > >> Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx> > >> --- > >> xen/arch/arm/domain_build.c | 52 > >> +++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 52 insertions(+) > >> > >> diff --git a/xen/arch/arm/domain_build.c > >> b/xen/arch/arm/domain_build.c index 984e70e5fc..8cee5ffbd1 100644 > >> --- a/xen/arch/arm/domain_build.c > >> +++ b/xen/arch/arm/domain_build.c > >> @@ -798,6 +798,48 @@ static int __init allocate_shared_memory(struct > >> domain *d, > >> return ret; > >> } > >> +static int __init guest_physmap_add_shm(struct domain *od, struct > >> domain *bd, > >> + unsigned long o_gfn, > >> + unsigned long b_gfn, > >> + unsigned long nr_gfns) { > >> + struct page_info **pages = NULL; > >> + p2m_type_t p2mt, t; > >> + int ret = 0; > > > > You don't need to initialize ret. > > > >> + > >> + pages = xmalloc_array(struct page_info *, nr_gfns); > >> + if ( !pages ) > >> + return -ENOMEM; > >> + > >> + /* > >> + * Take reference of statically shared pages from owner domain. > >> + * Reference will be released when destroying shared memory region. > >> + */ > >> + ret = get_pages_from_gfn(od, o_gfn, nr_gfns, pages, &p2mt, > >> P2M_ALLOC); > >> + if ( ret ) > >> + { > >> + ret = -EINVAL; > >> + goto fail_pages; > >> + } > >> + > >> + if ( p2m_is_ram(p2mt) ) > >> + t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw : > >> p2m_map_foreign_ro; > >> + else > >> + { > >> + ret = -EINVAL; > >> + goto fail_pages; > > > > Where would we release the references? > > > >> + } > >> + > >> + /* Set up guest foreign map. */ > >> + ret = guest_physmap_add_pages(bd, _gfn(b_gfn), > >> page_to_mfn(pages[0]), > >> + nr_gfns, t); > > > > A few things: > > - The beginning of the code assumes that the MFN may be > > discontiguous in the physical memory. But here, you are assuming they are > contiguous. > > If you want the latter, then you should check the MFNs are contiguous. > > That said, I am not sure if this restriction is really necessary. > > > > - IIRC, guest_physmap_add_pages() doesn't revert the mappings. So > > you need to revert it in case of failure. > > > There is another issue here. guest_physmap_add_pages() may use superpage > mapping. The P2M code is currently assuming the foreing mapping will be > using L3 mapping (4KB). > > Do you need to use superpage mapping here? > Right now, there is no user case on my side needing superpage mapping. > Cheers, > > -- > Julien Grall
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |