| Message ID | 1402394278-9850-3-git-send-email-ian.campbell@citrix.com |
|---|---|
| State | New |
| Headers | show |
Hi Ian, On 06/10/2014 10:57 AM, Ian Campbell wrote: > In particular do not do it for CACHEFLUSH. > > INSERT, RELINQUISH and REMOVE should all put the page (if the current pte is > valid). ALLOCATE doesn't need to since it asserts the current PTE must be > invalid. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > --- > xen/arch/arm/p2m.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index 9960e17..830a9f9 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -299,6 +299,23 @@ enum p2m_operation { > CACHEFLUSH, > }; > > +static void p2m_put_page(const lpae_t pte) > +{ > + /* TODO: Handle other p2m types > + * > + * It's safe to do the put_page here because page_alloc will > + * flush the TLBs if the page is reallocated before the end of > + * this loop. > + */ > + if ( p2m_is_foreign(pte.p2m.type) ) > + { > + unsigned long mfn = pte.p2m.base; > + > + ASSERT(mfn_valid(mfn)); > + put_page(mfn_to_page(mfn)); > + } > +} > + You forgot to drop the put_page in apply_p2m_changes. So, now we have 2 put_page rather than one. Regards,
On Tue, 2014-06-10 at 12:28 +0100, Julien Grall wrote: > You forgot to drop the put_page in apply_p2m_changes. So, now we have 2 > put_page rather than one. Rebasing snafu meant it got removed in the final patch instead of here. I'll fix. Ian.
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 9960e17..830a9f9 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -299,6 +299,23 @@ enum p2m_operation { CACHEFLUSH, }; +static void p2m_put_page(const lpae_t pte) +{ + /* TODO: Handle other p2m types + * + * It's safe to do the put_page here because page_alloc will + * flush the TLBs if the page is reallocated before the end of + * this loop. + */ + if ( p2m_is_foreign(pte.p2m.type) ) + { + unsigned long mfn = pte.p2m.base; + + ASSERT(mfn_valid(mfn)); + put_page(mfn_to_page(mfn)); + } +} + static int apply_p2m_changes(struct domain *d, enum p2m_operation op, paddr_t start_gpaddr, @@ -436,6 +453,8 @@ static int apply_p2m_changes(struct domain *d, break; case INSERT: { + if ( pte.p2m.valid ) + p2m_put_page(pte); pte = mfn_to_p2m_entry(maddr >> PAGE_SHIFT, mattr, t); p2m_write_pte(&third[third_table_offset(addr)], pte, flush_pt); @@ -451,6 +470,8 @@ static int apply_p2m_changes(struct domain *d, break; } + p2m_put_page(pte); + count += 0x10; memset(&pte, 0x00, sizeof(pte));
In particular do not do it for CACHEFLUSH. INSERT, RELINQUISH and REMOVE should all put the page (if the current pte is valid). ALLOCATE doesn't need to since it asserts the current PTE must be invalid. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- xen/arch/arm/p2m.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)