Re: [PATCH v3 1/2] x86/pv: Drop FLUSH_TLB_GLOBAL in do_mmu_update() for XPTI

[Jan Beulich <jbeulich@xxxxxxxx>]

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@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan