| Message ID | 20201027141037.27357-2-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/pv: Alternative XSA-286 fix | expand |
On 27.10.2020 15:10, Andrew Cooper wrote: > c/s 9d1d31ad9498 "x86: slightly reduce Meltdown band-aid overhead" removed the > use of Global TLB flushes on the Xen entry path, but added a FLUSH_TLB_GLOBAL > to the L4 path in do_mmu_update(). > > However, this was unnecessary. > > The L4 resync will pick up any new mappings created by the L4 change. Any > changes to existing mappings are the guests responsibility to flush, and if > one is needed, an MMUEXT_OP hypercall will follow. > > This is (not really) XSA-286 (but necessary to simplify the logic). > > Fixes: 9d1d31ad9498 ("x86: slightly reduce Meltdown band-aid overhead") While - with yet another round of consideration - I agree, I would have thought justifying the correctness of the change by d543fa409358 ("xen/x86: disable global pages for domains with XPTI active") would have been easier. Perhaps at least worth mentioning? I agree that this reasoning is necessary to warrant the Fixes: tag, and hence to indicate backporting of this change is going to be fine all the way through to (the backports of) this commit. The primary complication with the justification you use is with the commit you refer to saying "so that inside Xen we'll also pick up such changes" in its description. I assume your reasoning wrt this is that any hypercall started on another vCPU before the mmu-update here completes can't rely on Xen observing the new entries, which is what I believe I viewed differently at the time. Whereas any one started after the sync/flush will obviously observe the new entries (because of the very sync). My main problem with this reasoning has been that it "feels" as if there might be a way by which a guest could arrange a hypercall which ought to observe the new entries but, because of the sync happening only on the exit paths, doesn't. Quite likely such "arranging", if possible at all, would need to be called "out of spec". > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index b2f35b3e7d..38168189aa 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -4188,7 +4188,7 @@ long do_mmu_update( cpumask_andnot(mask, pt_owner->dirty_cpumask, cpumask_of(cpu)); if ( !cpumask_empty(mask) ) - flush_mask(mask, FLUSH_TLB_GLOBAL | FLUSH_ROOT_PGTBL); + flush_mask(mask, FLUSH_ROOT_PGTBL); } perfc_add(num_page_updates, i);
c/s 9d1d31ad9498 "x86: slightly reduce Meltdown band-aid overhead" removed the use of Global TLB flushes on the Xen entry path, but added a FLUSH_TLB_GLOBAL to the L4 path in do_mmu_update(). However, this was unnecessary. The L4 resync will pick up any new mappings created by the L4 change. Any changes to existing mappings are the guests responsibility to flush, and if one is needed, an MMUEXT_OP hypercall will follow. This is (not really) XSA-286 (but necessary to simplify the logic). Fixes: 9d1d31ad9498 ("x86: slightly reduce Meltdown band-aid overhead") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> v3: * New --- xen/arch/x86/mm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)