From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v9 01/14] arch/arm: add consistency check to REMOVE p2m changes Date: Thu, 03 Jul 2014 11:24:01 +0100 Message-ID: <53B52F41.50905@linaro.org> References: <1404326543-16875-1-git-send-email-avanzini.arianna@gmail.com> <1404326543-16875-2-git-send-email-avanzini.arianna@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1404326543-16875-2-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, andrew.cooper3@citrix.com, viktor.kleinik@globallogic.com List-Id: xen-devel@lists.xenproject.org Hi Arianna, On 07/02/2014 07:42 PM, Arianna Avanzini wrote: > xen/arch/arm/p2m.c | 32 ++++++++++++++++++++++++++++++-- > 1 file changed, 30 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index 9960e17..7cb4a27 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c [..] > @@ -439,12 +441,37 @@ static int apply_p2m_changes(struct domain *d, > pte = mfn_to_p2m_entry(maddr >> PAGE_SHIFT, mattr, t); > p2m_write_pte(&third[third_table_offset(addr)], > pte, flush_pt); > - maddr += PAGE_SIZE; > } > break; > - case RELINQUISH: > case REMOVE: > { > + unsigned long mfn = pte.p2m.base; > + > + /* > + * Ensure that the guest address addr currently being > + * handled (that is in the range given as argument to > + * this function) is actually mapped to the corresponding > + * machine address in the specified range. 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) ) > + { > + gdprintk(XENLOG_WARNING, > + "p2m_remove: mapping at %"PRIpaddr" is of maddr %"PRIpaddr" not %"PRIpaddr" as expected", > + addr, pfn_to_paddr(mfn), maddr); > + /* > + * Continue to process the range even if an error is > + * encountered, to prevent I/O-memory regions from > + * being partially accessible to a domain. > + */ > + continue; This is buggy, you never update addr and maddr. So if a mapping is not there, the code will end up in infinite loop. Regards, -- Julien Grall