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