| Message ID | 389f5988-4700-da3e-e628-1513e87eb56a@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fully replace mfn_to_gmfn() | expand |
Hi Jan, On 28/06/2021 12:52, Jan Beulich wrote: > Convert the two remaining uses as well as Arm's stub to the properly > named and type-safe mfn_to_gfn(), dropping x86's definition (where we > already have mfn_to_gfn()). > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/common/domctl.c > +++ b/xen/common/domctl.c > @@ -111,7 +111,8 @@ void getdomaininfo(struct domain *d, str > info->outstanding_pages = d->outstanding_pages; > info->shr_pages = atomic_read(&d->shr_pages); > info->paged_pages = atomic_read(&d->paged_pages); > - info->shared_info_frame = mfn_to_gmfn(d, virt_to_mfn(d->shared_info)); > + info->shared_info_frame = > + gfn_x(mfn_to_gfn(d, _mfn(virt_to_mfn(d->shared_info)))); > BUG_ON(SHARED_M2P(info->shared_info_frame)); > > info->cpupool = cpupool_get_id(d); > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -714,13 +714,13 @@ static long memory_exchange(XEN_GUEST_HA > */ > while ( (page = page_list_remove_head(&in_chunk_list)) ) > { > - unsigned long gfn; > + gfn_t gfn; > > mfn = page_to_mfn(page); > - gfn = mfn_to_gmfn(d, mfn_x(mfn)); > + gfn = mfn_to_gfn(d, mfn); > /* Pages were unshared above */ > - BUG_ON(SHARED_M2P(gfn)); > - if ( guest_physmap_remove_page(d, _gfn(gfn), mfn, 0) ) > + BUG_ON(SHARED_M2P(gfn_x(gfn))); > + if ( guest_physmap_remove_page(d, gfn, mfn, 0) ) > domain_crash(d); > free_domheap_page(page); > } > --- a/xen/include/asm-arm/mm.h > +++ b/xen/include/asm-arm/mm.h > @@ -328,8 +328,7 @@ struct page_info *get_page_from_gva(stru > > /* Xen always owns P2M on ARM */ > #define set_gpfn_from_mfn(mfn, pfn) do { (void) (mfn), (void)(pfn); } while (0) > -#define mfn_to_gmfn(_d, mfn) (mfn) > - > +#define mfn_to_gfn(d, mfn) _gfn(mfn_x(mfn)) I still have a series pending to drop mfn_to_g{,m}fn() on Arm. But it has been stuck for quite a while on an agreement on for the toolstack interface (see [1]). Anyway, this function is not worse than before. So: Acked-by: Julien Grall <julien@xen.org> Cheers, [1] https://lore.kernel.org/xen-devel/32d4f762-a61d-bfdd-c4a8-38e5edef1aa8@xen.org/
On 28/06/2021 12:52, Jan Beulich wrote: > Convert the two remaining uses as well as Arm's stub to the properly > named and type-safe mfn_to_gfn(), dropping x86's definition (where we > already have mfn_to_gfn()). > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>, but ... > --- a/xen/include/asm-arm/mm.h > +++ b/xen/include/asm-arm/mm.h > @@ -328,8 +328,7 @@ struct page_info *get_page_from_gva(stru > > /* Xen always owns P2M on ARM */ > #define set_gpfn_from_mfn(mfn, pfn) do { (void) (mfn), (void)(pfn); } while (0) > -#define mfn_to_gmfn(_d, mfn) (mfn) > - > +#define mfn_to_gfn(d, mfn) _gfn(mfn_x(mfn)) ... surely this wants to be ((void)(d), _gfn(mfn_x(mfn))), even if it's just a latent bug right now? ~Andrew
On 28.06.2021 17:42, Andrew Cooper wrote: > On 28/06/2021 12:52, Jan Beulich wrote: >> Convert the two remaining uses as well as Arm's stub to the properly >> named and type-safe mfn_to_gfn(), dropping x86's definition (where we >> already have mfn_to_gfn()). >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>, but ... Thanks. >> --- a/xen/include/asm-arm/mm.h >> +++ b/xen/include/asm-arm/mm.h >> @@ -328,8 +328,7 @@ struct page_info *get_page_from_gva(stru >> >> /* Xen always owns P2M on ARM */ >> #define set_gpfn_from_mfn(mfn, pfn) do { (void) (mfn), (void)(pfn); } while (0) >> -#define mfn_to_gmfn(_d, mfn) (mfn) >> - >> +#define mfn_to_gfn(d, mfn) _gfn(mfn_x(mfn)) > > ... surely this wants to be ((void)(d), _gfn(mfn_x(mfn))), even if it's > just a latent bug right now? Well, Julien said he plans to get rid of this anyway. I'll do here whatever the Arm maintainers say is wanted. Julien, Stefano? Jan
Hi Jan, On 28/06/2021 17:15, Jan Beulich wrote: > On 28.06.2021 17:42, Andrew Cooper wrote: >> On 28/06/2021 12:52, Jan Beulich wrote: >>> Convert the two remaining uses as well as Arm's stub to the properly >>> named and type-safe mfn_to_gfn(), dropping x86's definition (where we >>> already have mfn_to_gfn()). >>> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>, but ... > > Thanks. > >>> --- a/xen/include/asm-arm/mm.h >>> +++ b/xen/include/asm-arm/mm.h >>> @@ -328,8 +328,7 @@ struct page_info *get_page_from_gva(stru >>> >>> /* Xen always owns P2M on ARM */ >>> #define set_gpfn_from_mfn(mfn, pfn) do { (void) (mfn), (void)(pfn); } while (0) >>> -#define mfn_to_gmfn(_d, mfn) (mfn) >>> - >>> +#define mfn_to_gfn(d, mfn) _gfn(mfn_x(mfn)) >> >> ... surely this wants to be ((void)(d), _gfn(mfn_x(mfn))), even if it's >> just a latent bug right now? > > Well, Julien said he plans to get rid of this anyway. I'll do here > whatever the Arm maintainers say is wanted. Julien, Stefano? I am fine with the change suggested by Andrew. Cheers,
--- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -111,7 +111,8 @@ void getdomaininfo(struct domain *d, str info->outstanding_pages = d->outstanding_pages; info->shr_pages = atomic_read(&d->shr_pages); info->paged_pages = atomic_read(&d->paged_pages); - info->shared_info_frame = mfn_to_gmfn(d, virt_to_mfn(d->shared_info)); + info->shared_info_frame = + gfn_x(mfn_to_gfn(d, _mfn(virt_to_mfn(d->shared_info)))); BUG_ON(SHARED_M2P(info->shared_info_frame)); info->cpupool = cpupool_get_id(d); --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -714,13 +714,13 @@ static long memory_exchange(XEN_GUEST_HA */ while ( (page = page_list_remove_head(&in_chunk_list)) ) { - unsigned long gfn; + gfn_t gfn; mfn = page_to_mfn(page); - gfn = mfn_to_gmfn(d, mfn_x(mfn)); + gfn = mfn_to_gfn(d, mfn); /* Pages were unshared above */ - BUG_ON(SHARED_M2P(gfn)); - if ( guest_physmap_remove_page(d, _gfn(gfn), mfn, 0) ) + BUG_ON(SHARED_M2P(gfn_x(gfn))); + if ( guest_physmap_remove_page(d, gfn, mfn, 0) ) domain_crash(d); free_domheap_page(page); } --- a/xen/include/asm-arm/mm.h +++ b/xen/include/asm-arm/mm.h @@ -328,8 +328,7 @@ struct page_info *get_page_from_gva(stru /* Xen always owns P2M on ARM */ #define set_gpfn_from_mfn(mfn, pfn) do { (void) (mfn), (void)(pfn); } while (0) -#define mfn_to_gmfn(_d, mfn) (mfn) - +#define mfn_to_gfn(d, mfn) _gfn(mfn_x(mfn)) /* Arch-specific portion of memory_op hypercall. */ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg); --- a/xen/include/asm-x86/mm.h +++ b/xen/include/asm-x86/mm.h @@ -527,11 +527,6 @@ extern struct rangeset *mmio_ro_ranges; #define get_gpfn_from_mfn(mfn) (machine_to_phys_mapping[(mfn)]) -#define mfn_to_gmfn(_d, mfn) \ - ( (paging_mode_translate(_d)) \ - ? get_gpfn_from_mfn(mfn) \ - : (mfn) ) - #define compat_pfn_to_cr3(pfn) (((unsigned)(pfn) << 12) | ((unsigned)(pfn) >> 20)) #define compat_cr3_to_pfn(cr3) (((unsigned)(cr3) >> 12) | ((unsigned)(cr3) << 20))
Convert the two remaining uses as well as Arm's stub to the properly named and type-safe mfn_to_gfn(), dropping x86's definition (where we already have mfn_to_gfn()). Signed-off-by: Jan Beulich <jbeulich@suse.com>