[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH V2 16/23] xen/mm: Handle properly reference in set_foreign_p2m_entry() on Arm
On 15.10.2020 18:44, Oleksandr Tyshchenko wrote: > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -1380,6 +1380,27 @@ int guest_physmap_remove_page(struct domain *d, gfn_t > gfn, mfn_t mfn, > return p2m_remove_mapping(d, gfn, (1 << page_order), mfn); > } > > +int set_foreign_p2m_entry(struct domain *d, const struct domain *fd, > + unsigned long gfn, mfn_t mfn) > +{ > + struct page_info *page = mfn_to_page(mfn); > + int rc; > + > + if ( !get_page(page, fd) ) > + return -EINVAL; > + > + /* > + * It is valid to always use p2m_map_foreign_rw here as if this gets > + * called that d != fd. A case when d == fd would be rejected by > + * rcu_lock_remote_domain_by_id() earlier. > + */ Are you describing things here on the assumption that no new callers may surface later on? To catch such, I'd recommend adding at least a respective ASSERT(). > + rc = guest_physmap_add_entry(d, _gfn(gfn), mfn, 0, p2m_map_foreign_rw); > + if ( rc ) > + put_page(page); > + > + return 0; I can't imagine it's correct to not signal the error to the caller(s). > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -1099,7 +1099,8 @@ static int acquire_resource( > * reference counted, it is unsafe to allow mapping of > * resource pages unless the caller is the hardware domain. > */ > - if ( paging_mode_translate(currd) && !is_hardware_domain(currd) ) > + if ( paging_mode_translate(currd) && !is_hardware_domain(currd) && > + !arch_refcounts_p2m() ) > return -EACCES; I guess the hook may want naming differently, as both prior parts of the condition should be needed only on the x86 side, and there (for PV) there's no p2m involved in the refcounting. Maybe arch_acquire_resource_check()? And then the comment wants moving into the x86 hook as well. You may want to consider leaving a more generic one here... Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |