Re: [PATCH] parisc: Remove locking from TLB handler and set page accessed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2018-09-20 6:42 PM, Mikulas Patocka wrote:

On Thu, 20 Sep 2018, John David Anglin wrote:

On 2018-09-20 3:30 PM, Mikulas Patocka wrote:
On Thu, 20 Sep 2018, John David Anglin wrote:

On 2018-09-20 2:39 PM, Mikulas Patocka wrote:
On Thu, 20 Sep 2018, John David Anglin wrote:

On 2018-09-20 12:11 PM, Mikulas Patocka wrote:
On Thu, 20 Sep 2018, Mikulas Patocka wrote:

But there's code like this that needs to be changed. It needs to
first
set
the PTE and then purge the tlb - otherwise the old pte value could
be
reloaded by a different CPU after purge_tlb_entries() because the
TLB
reload code no longer takes the lock.

static inline void ptep_set_wrprotect(struct mm_struct *mm,
unsigned
long
addr, pte_t *ptep)
{
            unsigned long flags;
            spin_lock_irqsave(&pa_tlb_lock, flags);
            purge_tlb_entries(mm, addr);
            set_pte(ptep, pte_wrprotect(*ptep));
            spin_unlock_irqrestore(&pa_tlb_lock, flags);
}



The accessed bit is in the same byte as the present bit, so what
will
happen if the TLB fault races with pte clearing?

Linux also encodes the swap location in non-present PTEs - could
that
race
with the update of the byte containing the accessed bit?

Perhaps the best solution would be to store the accessed and dirty
flags
in the two topmost bytes of the PTE and make sure that these bytes
are
not
used for anything else.

Mikulas
And I realized that even if you don't modify the PTE, dropping the
lock
from the TLB handler is wrong.

If you drop the lock, the CPU can read valid PTE, then wait
unlimited
amount of time (due to some microarchitectural event) and then
insert
the
PTE into TLB - long after the PTE was cleared and flushed by some
other
CPU.

If you drop the lock, you must stop relying on the instructions that
broadcast TLB shootdown and you must send IPIs to flush TLBs just
like
on
x86.
I don't believe the lock helps with the insert.  As far as I can tell
If you insert into TLB without holding the lock, you may be inserting
arbitrarily old value.
There is only a problem if this is done after purge.
Yes - and it may happen - I've posted an example:
https://www.spinics.net/lists/linux-parisc/msg09131.html

On PA 2.0, there are two reorder buffers - one for accesses and the
other for integer operations. Some instructions go in both.  The arch
does not say TLB inserts are accesses.  TLB purges are accesses.  So,
integer operations can leak from a spin lock except when there is a
dependency on an access.
This problem happens even if the CPU didn't reorder instructions at all.
The TLB handler first reads a PTE from memory and then inserts it into the
TLB. These two operations (load and insert) are not atomic. Between the
load and insert, the PTE value in memory may change arbitrarily.
Yes.  I believe that we probably can't serialize purge and insert
operations with a spin lock. That's probably why we still see some
random segmentation faults.
The kmap code is clearly violating the PA-RISC specification.

If a userspace page is simultaneously accessed from userspace process and
from kernel that called kmap on it, it violates the PA-RISC specification
because the virtual addresses are not congruent. It may result in data
corruption or even machine checks.

kmap should return addresses that are congruent with the current userspace
mapping of the page.
If you have thoughts on how to do that, let me know.  At the moment, we flush the userspace page before using the kernel page.  We flush the kernel page when we are done.  However, these flushes may leave TLB entries which potentially might corrupt the cache.  There are TLB purges after but there is a window where the TLB might do something bad.  I tried using tmpalias flushes but they caused lockups sometimes.  The tmpalias flushes suppress move in.

Dave

--
John David Anglin  dave.anglin@xxxxxxxx




[Index of Archives]     [Linux SoC]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux