>>> On 10.11.17 at 15:02, <yu.c.zh...@linux.intel.com> wrote: > 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?
Oh, yes, definitely. I should have noticed this myself. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
