From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arianna Avanzini Subject: [PATCH v10 01/12] arch/arm: add consistency check to REMOVE p2m changes Date: Tue, 29 Jul 2014 00:11:58 +0200 Message-ID: <1406585529-32193-2-git-send-email-avanzini.arianna@gmail.com> References: <1406585529-32193-1-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: <1406585529-32193-1-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: xen-devel@lists.xen.org Cc: Ian.Campbell@eu.citrix.com, paolo.valente@unimore.it, keir@xen.org, stefano.stabellini@eu.citrix.com, Ian.Jackson@eu.citrix.com, dario.faggioli@citrix.com, tim@xen.org, julien.grall@citrix.com, etrudeau@broadcom.com, andrew.cooper3@citrix.com, JBeulich@suse.com, avanzini.arianna@gmail.com, viktor.kleinik@globallogic.com List-Id: xen-devel@lists.xenproject.org Currently, the REMOVE case of the switch in apply_p2m_changes() does not perform any consistency check on the mapping to be removed. More in detail, the code does not check if the guest address to be unmapped is actually mapped to the machine address given as a parameter. This commit adds the above-described consistency check to the REMOVE path of apply_p2m_changes() and lets a warning be emitted when trying to remove a non-existent mapping. This is instrumental to one of the following commits, which implements the possibility to trigger the removal of p2m ranges via the memory_mapping DOMCTL for ARM. Signed-off-by: Arianna Avanzini Acked-by: Stefano Stabellini Cc: Dario Faggioli Cc: Paolo Valente Cc: Julien Grall Cc: Ian Campbell Cc: Jan Beulich Cc: Keir Fraser Cc: Tim Deegan Cc: Ian Jackson Cc: Andrew Cooper Cc: Eric Trudeau Cc: Viktor Kleinik --- v10: - Emit a warning and still unmap the mapping when failing to remove a mapping. - Correctly place the check for an unexpected mapping in the REMOVE case of apply_one_level(). - Drop the check for non-present entries which is redundant. - Print the domain id when emitting a warning message. v9: - Don't return with an error when failing to remove a mapping, but simply keep unmapping. - Don't force assignment to the flush variable, as it is already set before the switch. - Change warning message to be more appropriate and clear; use the correct format for paddr_t and gdprintk(), which is more restricted than regular printk()s. - Adapt to rework of p2m-related functions for ARM. v8: - Re-add erroneously-removed increments to the maddr variable. - When failing to remove a mapping, add previously-mapped PT entry, unlock the p2m_lock and flush TLBs if necessary. - Emit an error message when failing to remove a mapping. - Remove tentative phrases from commit description. v7: - Silently ignore the fact that, when removing a mapping, the specified gfn is not mapped at all. - Remove spurious spacing change. v6: - Don't update "count" on REMOVE as it is only used inside the RELINQUISH case of the switch in apply_p2m_changes(). - Return with an error if removal of a page fails instead of just skipping the page. v5: - Do not use a temporary variable to hold the machine address: use the "maddr" function parameter itself. - Increment the machine address also when first and second level mappings are not valid. - Get the actual machine frame number mapped to the guest frame number given as parameter to the function directly in the REMOVE case of the switch construct, as it might not be valid in other cases and its value might be uncorrectly used in the future. - Remove useless and/or harmful ASSERT; check however if the mapping is valid and skip the page if it is not. v4: - Remove useless and slow lookup and use already-available data from pte instead. - Correctly increment the local variable used to keep the machine address whose mapping is currently being removed. - Return with an error upon finding a mismatch between the actual machine address mapped to the guest address and the machine address passed as parameter, instead of just skipping the page. --- xen/arch/arm/p2m.c | 56 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 143199b..8feb5c5 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -601,6 +601,7 @@ static int apply_one_level(struct domain *d, { /* Progress up to next boundary */ *addr = (*addr + level_size) & level_mask; + *maddr = (*maddr + level_size) & level_mask; return P2M_ONE_PROGRESS_NOP; } @@ -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) ) + { + 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", + d->domain_id, *addr, pfn_to_paddr(mfn), *maddr); + } + else + { + /* + * 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; + } } } @@ -638,6 +655,7 @@ static int apply_one_level(struct domain *d, p2m_write_pte(entry, pte, flush_cache); *addr += level_size; + *maddr += level_size; p2m->stats.mappings[level]--; -- 2.0.3