From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v10 01/12] arch/arm: add consistency check to REMOVE p2m changes Date: Tue, 29 Jul 2014 13:02:26 +0100 Message-ID: <53D78D52.3020906@citrix.com> References: <1406585529-32193-1-git-send-email-avanzini.arianna@gmail.com> <1406585529-32193-2-git-send-email-avanzini.arianna@gmail.com> <53D78BBF.7080108@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53D78BBF.7080108@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall , 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 On 29/07/14 12:55, Julien Grall wrote: > Hi Arianna, > > On 07/28/2014 11:11 PM, Arianna Avanzini wrote: >> @@ -609,26 +610,42 @@ static int apply_one_level(struct domain *d, >> if ( p2m_table(orig_pte) ) >> return P2M_ONE_DESCEND; >> >> - if ( op == REMOVE && >> - !is_mapping_aligned(*addr, end_gpaddr, >> - 0, /* maddr doesn't matter for remove */ >> - level_size) ) >> + if ( op == REMOVE ) >> { >> - /* >> - * Removing a mapping from the middle of a superpage. Shatter >> - * and descend. >> - */ >> - *flush = true; >> - rc = p2m_create_table(d, entry, >> - level_shift - PAGE_SHIFT, flush_cache); >> - if ( rc < 0 ) >> - return rc; >> - >> - p2m->stats.shattered[level]++; >> - p2m->stats.mappings[level]--; >> - p2m->stats.mappings[level+1] += LPAE_ENTRIES; >> - >> - return P2M_ONE_DESCEND; >> + if ( is_mapping_aligned(*addr, end_gpaddr, *maddr, level_size) ) >> + { > You don't cover every case with this solution. This code is only called > when (level < 3). So for single 4k (level 3), Xen will not print a > message when the MFN differ. > > I still think that the best place to check it, is after the if ( level < 3 ) > > The code would look like: > > if ( level < 3 ) > { > .... > } > > if ( op == REMOVE && pfn_to_paddr(orig_pte.p2m.base) != *maddr ) > printk(XENLOG_ERR "p2m_remove"....); > >> + unsigned long mfn = orig_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 ( *maddr != pfn_to_paddr(mfn) ) >> + printk("p2m_remove dom%d: mapping at %"PRIpaddr" is of maddr %"PRIpaddr" not %"PRIpaddr" as expected\n", > You have to prefix it at least by XENLOG_WARNING or XENLOG_ERR > > Regards, > XENLOG_G_WARNING or XENLOG_G_ERR Guest printk()s are rate limited differently to core hypervisor printk()s, and have a separate controllable loglevel. ~Andrew