| Message ID | 1386560047-17500-9-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, 2013-12-09 at 03:34 +0000, Julien Grall wrote: > Signed-off-by: Julien Grall <julien.grall@linaro.org> > > --- > Changes in v2: > - Introduce the patch > --- > xen/arch/arm/mm.c | 16 ++++++++++++++++ > xen/include/asm-arm/p2m.h | 6 +----- > 2 files changed, 17 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 960c872..ba51f6e 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -968,6 +968,22 @@ void share_xen_page_with_privileged_guests( > share_xen_page_with_guest(page, dom_xen, readonly); > } > > +int xenmem_rem_foreign_from_p2m(struct domain *d, unsigned long gpfn) > +{ > + unsigned long mfn = gmfn_to_mfn(d, gpfn); > + if ( !mfn_valid(mfn) ) > + { > + gdprintk(XENLOG_WARNING, "Invalid mfn for gpfn:%lx domid:%d\n", > + gpfn, d->domain_id); Is this guest triggerable? Or does it indicate a mistake when we made the mapping? > + return -EINVAL; > + } > + > + guest_physmap_remove_page(d, gpfn, mfn, 0); > + put_page(mfn_to_page(mfn)); Some of this could become a common helper with create_p2m_entries and the relinquish stuff in the previous patch. > + > + return 0; > +} > + > static int xenmem_add_to_physmap_one( > struct domain *d, > uint16_t space, > diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h > index b0d3aea..cdd80d8 100644 > --- a/xen/include/asm-arm/p2m.h > +++ b/xen/include/asm-arm/p2m.h > @@ -152,11 +152,7 @@ static inline int get_page_and_type(struct page_info *page, > return rc; > } > > -static inline int xenmem_rem_foreign_from_p2m(struct domain *d, > - unsigned long gpfn) > -{ > - return -ENOSYS; > -} > +int xenmem_rem_foreign_from_p2m(struct domain *d, unsigned long gpfn); > > #endif /* _XEN_P2M_H */ >
On 12/09/2013 04:31 PM, Ian Campbell wrote: > On Mon, 2013-12-09 at 03:34 +0000, Julien Grall wrote: >> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> >> --- >> Changes in v2: >> - Introduce the patch >> --- >> xen/arch/arm/mm.c | 16 ++++++++++++++++ >> xen/include/asm-arm/p2m.h | 6 +----- >> 2 files changed, 17 insertions(+), 5 deletions(-) >> >> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c >> index 960c872..ba51f6e 100644 >> --- a/xen/arch/arm/mm.c >> +++ b/xen/arch/arm/mm.c >> @@ -968,6 +968,22 @@ void share_xen_page_with_privileged_guests( >> share_xen_page_with_guest(page, dom_xen, readonly); >> } >> >> +int xenmem_rem_foreign_from_p2m(struct domain *d, unsigned long gpfn) >> +{ >> + unsigned long mfn = gmfn_to_mfn(d, gpfn); >> + if ( !mfn_valid(mfn) ) >> + { >> + gdprintk(XENLOG_WARNING, "Invalid mfn for gpfn:%lx domid:%d\n", >> + gpfn, d->domain_id); > > Is this guest triggerable? Or does it indicate a mistake when we made > the mapping? It indicates a mistake when we made the mapping. Perhaps an ASSERT is better here. > >> + return -EINVAL; >> + } >> + >> + guest_physmap_remove_page(d, gpfn, mfn, 0); >> + put_page(mfn_to_page(mfn)); > > Some of this could become a common helper with create_p2m_entries and > the relinquish stuff in the previous patch. The best solution is to move put_page in guest_physmap_remove_page. But as we didn't plan to handle refcount for each mapping in Xen 4.4, perhaps we can delay it?
On Mon, 2013-12-09 at 17:08 +0000, Julien Grall wrote: > On 12/09/2013 04:31 PM, Ian Campbell wrote: > > On Mon, 2013-12-09 at 03:34 +0000, Julien Grall wrote: > >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > >> > >> --- > >> Changes in v2: > >> - Introduce the patch > >> --- > >> xen/arch/arm/mm.c | 16 ++++++++++++++++ > >> xen/include/asm-arm/p2m.h | 6 +----- > >> 2 files changed, 17 insertions(+), 5 deletions(-) > >> > >> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > >> index 960c872..ba51f6e 100644 > >> --- a/xen/arch/arm/mm.c > >> +++ b/xen/arch/arm/mm.c > >> @@ -968,6 +968,22 @@ void share_xen_page_with_privileged_guests( > >> share_xen_page_with_guest(page, dom_xen, readonly); > >> } > >> > >> +int xenmem_rem_foreign_from_p2m(struct domain *d, unsigned long gpfn) > >> +{ > >> + unsigned long mfn = gmfn_to_mfn(d, gpfn); > >> + if ( !mfn_valid(mfn) ) > >> + { > >> + gdprintk(XENLOG_WARNING, "Invalid mfn for gpfn:%lx domid:%d\n", > >> + gpfn, d->domain_id); > > > > Is this guest triggerable? Or does it indicate a mistake when we made > > the mapping? > > It indicates a mistake when we made the mapping. Perhaps an ASSERT is > better here. In that case either works for me. > > > >> + return -EINVAL; > >> + } > >> + > >> + guest_physmap_remove_page(d, gpfn, mfn, 0); > >> + put_page(mfn_to_page(mfn)); > > > > Some of this could become a common helper with create_p2m_entries and > > the relinquish stuff in the previous patch. > > The best solution is to move put_page in guest_physmap_remove_page. But > as we didn't plan to handle refcount for each mapping in Xen 4.4, > perhaps we can delay it? You can move it and still have it be conditional I think, since g_p_r_p can see the type in the pte, can't it? Ian.
On 12/09/2013 05:18 PM, Ian Campbell wrote: > On Mon, 2013-12-09 at 17:08 +0000, Julien Grall wrote: >> On 12/09/2013 04:31 PM, Ian Campbell wrote: >>> On Mon, 2013-12-09 at 03:34 +0000, Julien Grall wrote: [..] > >>> >>>> + return -EINVAL; >>>> + } >>>> + >>>> + guest_physmap_remove_page(d, gpfn, mfn, 0); >>>> + put_page(mfn_to_page(mfn)); >>> >>> Some of this could become a common helper with create_p2m_entries and >>> the relinquish stuff in the previous patch. >> >> The best solution is to move put_page in guest_physmap_remove_page. But >> as we didn't plan to handle refcount for each mapping in Xen 4.4, >> perhaps we can delay it? > > You can move it and still have it be conditional I think, since g_p_r_p > can see the type in the pte, can't it? I plan to move the put_page in create_p2m_entries and check pte type.
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 960c872..ba51f6e 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -968,6 +968,22 @@ void share_xen_page_with_privileged_guests( share_xen_page_with_guest(page, dom_xen, readonly); } +int xenmem_rem_foreign_from_p2m(struct domain *d, unsigned long gpfn) +{ + unsigned long mfn = gmfn_to_mfn(d, gpfn); + if ( !mfn_valid(mfn) ) + { + gdprintk(XENLOG_WARNING, "Invalid mfn for gpfn:%lx domid:%d\n", + gpfn, d->domain_id); + return -EINVAL; + } + + guest_physmap_remove_page(d, gpfn, mfn, 0); + put_page(mfn_to_page(mfn)); + + return 0; +} + static int xenmem_add_to_physmap_one( struct domain *d, uint16_t space, diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index b0d3aea..cdd80d8 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -152,11 +152,7 @@ static inline int get_page_and_type(struct page_info *page, return rc; } -static inline int xenmem_rem_foreign_from_p2m(struct domain *d, - unsigned long gpfn) -{ - return -ENOSYS; -} +int xenmem_rem_foreign_from_p2m(struct domain *d, unsigned long gpfn); #endif /* _XEN_P2M_H */
Signed-off-by: Julien Grall <julien.grall@linaro.org> --- Changes in v2: - Introduce the patch --- xen/arch/arm/mm.c | 16 ++++++++++++++++ xen/include/asm-arm/p2m.h | 6 +----- 2 files changed, 17 insertions(+), 5 deletions(-)