On 11/10/2017 5:57 PM, Jan Beulich wrote:
On 10.11.17 at 08:18, <yu.c.zh...@linux.intel.com> wrote:
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5097,6 +5097,17 @@ int modify_xen_mappings(unsigned long s, unsigned long
e, unsigned int nf)
*/
if ( (nf & _PAGE_PRESENT) || ((v != e) && (l1_table_offset(v) !=
0)) )
continue;
+ if ( locking )
+ spin_lock(&map_pgdir_lock);
+
+ /* L2E may be cleared on another CPU. */
+ if ( !(l2e_get_flags(*pl2e) & _PAGE_PRESENT) )
I think you also need a PSE check here, or else the l2e_to_l1e() below
may be illegal.
Hmm, interesting point, and thanks! :-)
I did not check the PSE, because modify_xen_mappings() will not do the
re-consolidation, and
concurrent invokes of this routine will not change this flag. But now I
believe this presumption
shall not be made, because the paging structures may be modified by
other routines, like
map_pages_to_xen() on other CPUs.
So yes, I think a _PAGE_PSE check is necessary here. And I suggest we
also check the _PAGE_PRESENT
flag as well, for the re-consolidation part in my first patch for
map_pages_to_xen(). Do you agree?
@@ -5105,11 +5116,16 @@ int modify_xen_mappings(unsigned long s, unsigned long
e, unsigned int nf)
{
/* Empty: zap the L2E and free the L1 page. */
l2e_write_atomic(pl2e, l2e_empty());
+ if ( locking )
+ spin_unlock(&map_pgdir_lock);
flush_area(NULL, FLUSH_TLB_GLOBAL); /* flush before free */
free_xen_pagetable(pl1e);
}
+ else if ( locking )
+ spin_unlock(&map_pgdir_lock);
}
+check_l3:
Labels indented by at least one space please.
Got it . Thanks.
Yu
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
