Possibility of conflicting memory types in lazier TLB mode?

Possibility of conflicting memory types in lazier TLB mode?

[Nicholas Piggin]

Hi Rik,

Commit 145f573b89a62 ("Make lazy TLB mode lazier").

A couple of questions here (and I don't know the x86 architecture too 
well let alone the ASID stuff, so bear with me). I'm assuming, and it 
appears to be in the x86 manual that you can't map the same physical 
page with conflicting memory types on different processors in general
(or in different ASIDs on the same processor?)

Firstly, the freed_tables check, that's to prevent CPUs in the lazy mode 
with this mm loaded in their ASID from bringing in new translations 
based on random "stuff" if they happen to speculatively load userspace 
addresses (but in lazy mode they would never explicitly load such 
addresses), right?

I'm guessing that's a problem but the changed pte case is not, is 
because the table walker is going to barf if it sees garbage, but a 
valid pte is okay.

Now the intel manual says conflicting attributes are bad because you'll 
lose cache coherency on stores. But the speculative accesses from the 
lazy thread will never push stores to cache coherency and result of the 
loads doesn't matter, so maybe that's how this special case avoids the
problem.

But what about if there are (real, not speculative) stores in the store 
queue still on the lazy thread from when it was switched, that have not 
yet become coherent? The page is freed by another CPU and reallocated
for something that maps it as nocache. Do you have a coherency problem 
there?

Ensuring the store queue is drained when switching to lazy seems like it 
would fix it, maybe context switch code does that already or you have 
some other trick or reason it's not a problem. Am I way off base here?

Thanks,
Nick

Re: Possibility of conflicting memory types in lazier TLB mode?

[Rik van Riel]

[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]

On Fri, 2020-05-15 at 16:50 +1000, Nicholas Piggin wrote:
> 
> But what about if there are (real, not speculative) stores in the
> store 
> queue still on the lazy thread from when it was switched, that have
> not 
> yet become coherent? The page is freed by another CPU and reallocated
> for something that maps it as nocache. Do you have a coherency
> problem 
> there?
> 
> Ensuring the store queue is drained when switching to lazy seems like
> it 
> would fix it, maybe context switch code does that already or you
> have 
> some other trick or reason it's not a problem. Am I way off base
> here?

On x86, all stores become visible in-order globally.

I suspect that
means any pending stores in the queue
would become visible to the rest of the system before
the store to the "current" cpu-local variable, as
well as other writes from the context switch code
become visible to the rest of the system.

Is that too naive a way of preventing the scenario you
describe?

What am I overlooking?

-- 
All Rights Reversed.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Possibility of conflicting memory types in lazier TLB mode?

[Nicholas Piggin]

Excerpts from Rik van Riel's message of May 16, 2020 5:24 am:
> On Fri, 2020-05-15 at 16:50 +1000, Nicholas Piggin wrote:
>> 
>> But what about if there are (real, not speculative) stores in the
>> store 
>> queue still on the lazy thread from when it was switched, that have
>> not 
>> yet become coherent? The page is freed by another CPU and reallocated
>> for something that maps it as nocache. Do you have a coherency
>> problem 
>> there?
>> 
>> Ensuring the store queue is drained when switching to lazy seems like
>> it 
>> would fix it, maybe context switch code does that already or you
>> have 
>> some other trick or reason it's not a problem. Am I way off base
>> here?
> 
> On x86, all stores become visible in-order globally.
> 
> I suspect that
> means any pending stores in the queue
> would become visible to the rest of the system before
> the store to the "current" cpu-local variable, as
> well as other writes from the context switch code
> become visible to the rest of the system.
> 
> Is that too naive a way of preventing the scenario you
> describe?
> 
> What am I overlooking?

I'm concerned if the physical address gets mapped with different 
cacheability attributes where that ordering is not enforced by cache 
coherency

 "The PAT allows any memory type to be specified in the page tables, and 
 therefore it is possible to have a single physical page mapped to two 
 or more different linear addresses, each with different memory types. 
 Intel does not support this practice because it may lead to undefined 
 operations that can result in a system failure. In particular, a WC 
 page must never be aliased to a cacheable page because WC writes may 
 not check the processor caches." -- Vol. 3A 11-35

Maybe I'm over thinking it, and this would never happen anyway because 
if anyone were to map a RAM page WC, they might always have to ensure 
all processor caches are flushed first anyway so perhaps this is just a 
non-issue?

Thanks,
Nick

Re: Possibility of conflicting memory types in lazier TLB mode?

[Andy Lutomirski]

[cc Andrew Cooper and Dave Hansen]

On Fri, May 15, 2020 at 7:35 PM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> Excerpts from Rik van Riel's message of May 16, 2020 5:24 am:
> > On Fri, 2020-05-15 at 16:50 +1000, Nicholas Piggin wrote:
> >>
> >> But what about if there are (real, not speculative) stores in the
> >> store
> >> queue still on the lazy thread from when it was switched, that have
> >> not
> >> yet become coherent? The page is freed by another CPU and reallocated
> >> for something that maps it as nocache. Do you have a coherency
> >> problem
> >> there?
> >>
> >> Ensuring the store queue is drained when switching to lazy seems like
> >> it
> >> would fix it, maybe context switch code does that already or you
> >> have
> >> some other trick or reason it's not a problem. Am I way off base
> >> here?
> >
> > On x86, all stores become visible in-order globally.
> >
> > I suspect that
> > means any pending stores in the queue
> > would become visible to the rest of the system before
> > the store to the "current" cpu-local variable, as
> > well as other writes from the context switch code
> > become visible to the rest of the system.
> >
> > Is that too naive a way of preventing the scenario you
> > describe?
> >
> > What am I overlooking?
>
> I'm concerned if the physical address gets mapped with different
> cacheability attributes where that ordering is not enforced by cache
> coherency
>
>  "The PAT allows any memory type to be specified in the page tables, and
>  therefore it is possible to have a single physical page mapped to two
>  or more different linear addresses, each with different memory types.
>  Intel does not support this practice because it may lead to undefined
>  operations that can result in a system failure. In particular, a WC
>  page must never be aliased to a cacheable page because WC writes may
>  not check the processor caches." -- Vol. 3A 11-35
>
> Maybe I'm over thinking it, and this would never happen anyway because
> if anyone were to map a RAM page WC, they might always have to ensure
> all processor caches are flushed first anyway so perhaps this is just a
> non-issue?
>

After talking to Andrew Cooper (hi!), I think that, on reasonably
modern Intel machines, WC memory is still *coherent* with the whole
system -- it's just not ordered the usual way.  So I'm not convinced
there's an actual problem here.  I don't know about AMD.

Re: Possibility of conflicting memory types in lazier TLB mode?

[Andrew Cooper]

On 27/05/2020 01:09, Andy Lutomirski wrote:
> [cc Andrew Cooper and Dave Hansen]
>
> On Fri, May 15, 2020 at 7:35 PM Nicholas Piggin <npiggin@gmail.com> wrote:
>> Excerpts from Rik van Riel's message of May 16, 2020 5:24 am:
>>> On Fri, 2020-05-15 at 16:50 +1000, Nicholas Piggin wrote:
>>>> But what about if there are (real, not speculative) stores in the
>>>> store
>>>> queue still on the lazy thread from when it was switched, that have
>>>> not
>>>> yet become coherent? The page is freed by another CPU and reallocated
>>>> for something that maps it as nocache. Do you have a coherency
>>>> problem
>>>> there?
>>>>
>>>> Ensuring the store queue is drained when switching to lazy seems like
>>>> it
>>>> would fix it, maybe context switch code does that already or you
>>>> have
>>>> some other trick or reason it's not a problem. Am I way off base
>>>> here?
>>> On x86, all stores become visible in-order globally.
>>>
>>> I suspect that
>>> means any pending stores in the queue
>>> would become visible to the rest of the system before
>>> the store to the "current" cpu-local variable, as
>>> well as other writes from the context switch code
>>> become visible to the rest of the system.
>>>
>>> Is that too naive a way of preventing the scenario you
>>> describe?
>>>
>>> What am I overlooking?
>> I'm concerned if the physical address gets mapped with different
>> cacheability attributes where that ordering is not enforced by cache
>> coherency
>>
>>  "The PAT allows any memory type to be specified in the page tables, and
>>  therefore it is possible to have a single physical page mapped to two
>>  or more different linear addresses, each with different memory types.
>>  Intel does not support this practice because it may lead to undefined
>>  operations that can result in a system failure. In particular, a WC
>>  page must never be aliased to a cacheable page because WC writes may
>>  not check the processor caches." -- Vol. 3A 11-35
>>
>> Maybe I'm over thinking it, and this would never happen anyway because
>> if anyone were to map a RAM page WC, they might always have to ensure
>> all processor caches are flushed first anyway so perhaps this is just a
>> non-issue?
>>
> After talking to Andrew Cooper (hi!), I think that, on reasonably
> modern Intel machines, WC memory is still *coherent* with the whole
> system -- it's just not ordered the usual way.

So actually, on further reading, Vol 3 11.3 states "coherency is not
enforced by the processor’s bus coherency protocol" and later in 11.3.1,
"The WC buffer is not snooped and thus does not provide data coherency".


So, it would seem like it is possible to engineer a situation where the
cache line is WB according to the caches, and has pending WC data in one
or more cores/threads.  The question is whether this manifests as a
problem in practice, or not.

When changing the memory type of a mapping, you typically need to do
break/flush/make to be SMP-safe.  The IPI, as well as the TLB flush are
actions which cause WC buffers to be flushed.

x86 will tolerate a make/flush sequence as well.  In this scenario, a
3rd core/thread could pick up the line via its WB property, use it, and
cause it to be written back, between the pagetable change and the IPI
hitting.

But does this matter?  WC is by definition weakly ordered writes for
more efficient bus usage.  The device at the far end can't tell whether
the incoming write was from a WC or a WB eviction, and any late WC
evictions are semantically indistinguishable from a general concurrency
hazards with multiple writers.

~Andrew