>>> On 01.03.18 at 20:21, <andrew.coop...@citrix.com> wrote: > On 08/02/18 09:20, Jan Beulich wrote: >>>>> On 07.02.18 at 20:35, <andrew.coop...@citrix.com> wrote: >>> On 07/02/18 16:12, Jan Beulich wrote: >>>> I'm not sure why I didn't do this right away: By avoiding the use of >>>> global PTEs in the cloned directmap, there's no need to fiddle with >>>> CR4.PGE on any of the entry paths. Only the exit paths need to flush >>>> global mappings. >>>> >>>> The reduced flushing, however, implies that we now need to have >>>> interrupts off on all entry paths until after the page table switch, so >>>> that flush IPIs can't arrive with the restricted page tables still >>>> active, but only a non-global flush happening with the CR3 loads. Along >>>> those lines the "sync" IPI after L4 entry updates now needs to become a >>>> real (and global) flush IPI, so that inside Xen we'll also pick up such >>>> changes. >>> Actually, on second consideration, why does reenabling interrupts need >>> to be deferred? >>> >>> The safety of the sync_guest path (which previously entered Xen, did >>> nothing, and exited again) relied on the entry part flushing global >>> mappings for safety, as the return-to-xen path doesn't necessarily >>> switch mappings. >>> >>> However, the first hunk upgrading the "do nothing" to a proper global >>> flush, covers that case. >>> >>> I don't see anything else which affects the safety of taking TLB flush >>> IPIs early in the entry-from-guest path. What am I missing? >> If a sync IPI arrives before we switch away from the restricted page >> tables, the processor may re-fetch a global entry from those tables >> through an L4 with the sync IPI is supposed to tell the processor to >> get rid of (or modify). The subsequent CR3 write won't invalidate such >> a TLB entry, and hence whatever we do internally may reference a >> stale mapping. > > In which case, can I propose that the commit message reads: > > The reduced flushing, however, requires that we now have > interrupts off on all entry paths until after the page table > switch, so that flush IPIs can't be serviced while on the > restricted pagetables, leaving a window where a potentially stale > guest global mapping can be brought into the TLB. Along those > lines the "sync" IPI after L4 entry updates now needs to become a > real (and global) flush IPI, so that inside Xen we'll also pick > up such changes. > > Or something similar?
I've used the above. > Also, you've got a bugfix needed in clone_mapping() as found by Juergen, > asserting that the flags are the same after clobbering PAGE_GLOBAL. > > With both of these suitably addressed, Reviewed-by: Andrew Cooper > <andrew.coop...@citrix.com> As you will likely have seen from the patch just sent, the fix is not to be made here. Please confirm that I can apply the R-b with just the description change. Of course this change should then go in only after that other bug fix has (in whatever shape). Jan _______________________________________________ Xen-devel mailing list Xenfirstname.lastname@example.org https://lists.xenproject.org/mailman/listinfo/xen-devel