Re: [Xen-devel] TLB flushing

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.03.18 at 10:29, <jgross@xxxxxxxx> wrote:
> On 20/03/18 10:19, Jan Beulich wrote:
>>>>> On 20.03.18 at 09:50, <jgross@xxxxxxxx> wrote:
>>> While hunting a strange bug in my PCID patch series hinting at some
>>> TLB invalidation problem I discovered a piece of code looking rather
>>> fishy to me.
>>>
>>> Is it correct for new_tlbflush_clock_period() to use FLUSH_TLB instead
>>> of FLUSH_TLB_GLOBAL?
>>>
>>> While not being a problem in current code as both will flush all TLB
>>> entries my series will change that by using invpcid to flush only the
>>> non-global entries if FLUSH_TLB_GLOBAL wasn't set.
>>>
>>> I can send a patch if anyone can confirm that using FLUSH_TLB only is
>>> wrong.
>> 
>> I think this shouldn't be a separate patch, but an integral part of the
>> one introducing the distinction between "all" and non-global flushes.
>> This is because
>> - right now it doesn't make a difference (we do "all" flushes anyway),
>> - back in the 32-bit days it didn't matter because guest mappings
>>   would never have been allowed to be global, and transient Xen
>>   mappings also would never have had the G bit set.
>> IOW with what used to be named USER_MAPPINGS_ARE_GLOBAL
>> this would need to become FLUSH_TLB_GLOBAL at the point the
>> kind of flush gets altered, while without it could remain at FLUSH_TLB.
> 
> Really? Aren't global hypervisor mappings affected by this, too?

Yes and no. The timestamp here is needed to know whether to
flush when a page gets recycled. As long as G is never set on
guest controlled mappings for pages which may be recycled,
there's no issue. In particular, the G bits in the 1:1 mappings are
of no interest here (and in fact anything Xen maintains which no
guest can control), as those mappings never go away (or if they
did, e.g. when a page needs to be offlined for causing #MC, an
explicit global flush for that page would be required).

>> Perhaps it is worthwhile to retain this distinction just for
>> documentation purposes (in case a future change wants to turn off
>> that USER_MAPPINGS_ARE_GLOBAL behavior for whatever reason).
> 
> I think as long as FLUSH_TLB_GLOBAL is being used in the code
> new_tlbflush_clock_period() should do so, too. In case there is no need
> to do TLB flushes including global pages, FLUSH_TLB_GLOBAL can be
> modified to do only non-global flushing (with a comment explaining why
> this is secure).

No - an explicit global flush request (as per above explanation)
must never be converted down to a non-global one. Such a
"conversion" would only be valid when CR4.PGE is clear at all
times (but then it's not really a conversion anymore).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel