Re: [Xen-devel] TLB flushing

[Juergen Gross <jgross@xxxxxxxx>]

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?

> 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).


Juergen

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