From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v6 02/11] arch, arm: add consistency check to REMOVE p2m changes Date: Tue, 22 Apr 2014 20:35:30 +0100 Message-ID: <5356C482.9010401@linaro.org> References: <1398087904-16594-1-git-send-email-avanzini.arianna@gmail.com> <1398087904-16594-3-git-send-email-avanzini.arianna@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1398087904-16594-3-git-send-email-avanzini.arianna@gmail.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Arianna Avanzini , xen-devel@lists.xen.org Cc: julien.grall@citrix.com, paolo.valente@unimore.it, keir@xen.org, stefano.stabellini@eu.citrix.com, tim@xen.org, dario.faggioli@citrix.com, Ian.Jackson@eu.citrix.com, Ian.Campbell@eu.citrix.com, etrudeau@broadcom.com, JBeulich@suse.com, viktor.kleinik@globallogic.com List-Id: xen-devel@lists.xenproject.org Hi Arianna, On 21/04/14 14:44, Arianna Avanzini wrote: > --- > xen/arch/arm/p2m.c | 22 +++++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index 403fd89..7f03aa7 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -319,6 +319,7 @@ static int apply_p2m_changes(struct domain *d, > if ( !populate ) > { > addr = (addr + FIRST_SIZE) & FIRST_MASK; > + maddr = (maddr + FIRST_SIZE) & FIRST_MASK; We differ a bit from x86 on this point. We will silently ignore that the GFN is not mapped when op == REMOVE. I'm wondering if we should return an error in this case. I don't find any place where it's valid to remove non-present mapping. [..] > @@ -406,12 +408,26 @@ static int apply_p2m_changes(struct domain *d, > { > pte = mfn_to_p2m_entry(maddr >> PAGE_SHIFT, mattr, t); > write_pte(&third[third_table_offset(addr)], pte); > - maddr += PAGE_SIZE; > } > break; > - case RELINQUISH: > case REMOVE: > { > + unsigned long mfn = pte.p2m.base; > + > + /* > + * Ensure that the guest address given as argument to > + * this function is actually mapped to the specified > + * machine address. maddr here is the machine address > + * given to the function, while mfn is the machine > + * frame number actually mapped to the guest address: > + * check if the two correspond. > + */ > + if ( !pte.p2m.valid || maddr != pfn_to_paddr(mfn) ) > + return -EINVAL; > + } > + /* fall through */ > + case RELINQUISH: > + { > if ( !pte.p2m.valid ) > { > count++; > @@ -425,7 +441,6 @@ static int apply_p2m_changes(struct domain *d, > count++; > } > break; > - Spurious change here. Regards -- Julien Grall