LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
* Re: [patch 01/15] mm/memory.c: avoid access flag update TLB flush for retried page fault
       [not found]               ` <CAHk-=wha6f0gF1SJg96R77h0oTuc_oO7-37wD=mYGy6TyJOwbQ@mail.gmail.com>
@ 2020-07-28 10:53                 ` Nicholas Piggin
  2020-07-28 19:02                   ` Linus Torvalds
  0 siblings, 1 reply; 4+ messages in thread
From: Nicholas Piggin @ 2020-07-28 10:53 UTC (permalink / raw)
  To: linux-arch, Linus Torvalds, Yang Shi
  Cc: Hillf Danton, mm-commits, Catalin Marinas, Hugh Dickins,
	Josef Bacik, Will Deacon, Linux-MM, Matthew Wilcox,
	Johannes Weiner, Yu Xu, Andrew Morton, linuxppc-dev,
	Kirill A . Shutemov

Excerpts from Linus Torvalds's message of July 28, 2020 4:37 am:
> [ Adding linux-arch, just to make other architectures aware of this issue too.
> 
>   We have a "flush_tlb_fix_spurious_fault()" thing to take care of the
> "TLB may contain stale entries, we can't take the same fault over and
> over again" situation.
> 
>   On x86, it's a no-op, because x86 doesn't do that. x86 will re-walk
> the page tables - or possibly just always invalidate the faulting TLB
> entry - before taking a fault, so there can be no long-term stale
> TLB's.

[snip]

>   It looks like powerpc people at least thought about this, and only
> do it if there is a coprocessor. Which sounds a bit confused, but I
> don't know the rules.

I'm not sure about ppc32 and 64e, I'm almost certain they should do a 
local flush if anyting, and someone with a good understanding of the 
ISAs and CPUs might be able to nop it entirely. I agree global can't 
ever really make sense (except as a default because we have no generic 
local flush).

powerpc/64s reloads translations after taking a fault, so it's fine with 
a nop here.

The quirk is a problem with coprocessor where it's supposed to 
invalidate the translation after a fault but it doesn't, so we can get a 
read-only TLB stuck after something else does a RO->RW upgrade on the 
TLB. Something like that IIRC.  Coprocessors have their own MMU which 
lives in the nest not the core, so you need a global TLB flush to
invalidate that thing.

Thanks,
Nick

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [patch 01/15] mm/memory.c: avoid access flag update TLB flush for retried page fault
  2020-07-28 10:53                 ` [patch 01/15] mm/memory.c: avoid access flag update TLB flush for retried page fault Nicholas Piggin
@ 2020-07-28 19:02                   ` Linus Torvalds
  2020-07-28 22:53                     ` Nicholas Piggin
  0 siblings, 1 reply; 4+ messages in thread
From: Linus Torvalds @ 2020-07-28 19:02 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: linux-arch, Hillf Danton, Yang Shi, Yu Xu, Catalin Marinas,
	Hugh Dickins, Josef Bacik, Will Deacon, Linux-MM, Matthew Wilcox,
	Johannes Weiner, mm-commits, Andrew Morton, linuxppc-dev,
	Kirill A . Shutemov

On Tue, Jul 28, 2020 at 3:53 AM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> The quirk is a problem with coprocessor where it's supposed to
> invalidate the translation after a fault but it doesn't, so we can get a
> read-only TLB stuck after something else does a RO->RW upgrade on the
> TLB. Something like that IIRC.  Coprocessors have their own MMU which
> lives in the nest not the core, so you need a global TLB flush to
> invalidate that thing.

So I assumed, but it does seem confused.

Why? Because if there are stale translations on the co-processor,
there's no guarantee that one of the CPU's will have them and take a
fault.

So I'm not seeing why a core CPU doing spurious TLB invalidation would
follow from "stale TLB in the Nest".

If anything, I think "we have a coprocessor that needs to never have
stale TLB entries" would impact the _regular_ TLB invalidates (by
update_mmu_cache()) and perhaps make those more aggressive, exactly
because the coprocessor may not handle the fault as gracefully.

I dunno. I don't know the coprocessor side well enough to judge, I'm
just looking at it from a conceptual standpoint.

          Linus

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [patch 01/15] mm/memory.c: avoid access flag update TLB flush for retried page fault
  2020-07-28 19:02                   ` Linus Torvalds
@ 2020-07-28 22:53                     ` Nicholas Piggin
  2020-07-29 13:58                       ` Michael Ellerman
  0 siblings, 1 reply; 4+ messages in thread
From: Nicholas Piggin @ 2020-07-28 22:53 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-arch, Hillf Danton, mm-commits, Catalin Marinas,
	Hugh Dickins, Josef Bacik, Will Deacon, Linux-MM, Matthew Wilcox,
	Johannes Weiner, Yu Xu, Andrew Morton, linuxppc-dev, Yang Shi,
	Kirill A . Shutemov

Excerpts from Linus Torvalds's message of July 29, 2020 5:02 am:
> On Tue, Jul 28, 2020 at 3:53 AM Nicholas Piggin <npiggin@gmail.com> wrote:
>>
>> The quirk is a problem with coprocessor where it's supposed to
>> invalidate the translation after a fault but it doesn't, so we can get a
>> read-only TLB stuck after something else does a RO->RW upgrade on the
>> TLB. Something like that IIRC.  Coprocessors have their own MMU which
>> lives in the nest not the core, so you need a global TLB flush to
>> invalidate that thing.
> 
> So I assumed, but it does seem confused.
> 
> Why? Because if there are stale translations on the co-processor,
> there's no guarantee that one of the CPU's will have them and take a
> fault.
> 
> So I'm not seeing why a core CPU doing spurious TLB invalidation would
> follow from "stale TLB in the Nest".

If the nest MMU access faults, it sends an interrupt to the CPU and
the driver tries to handle the page fault for it (I think that's how
it works).

> If anything, I think "we have a coprocessor that needs to never have
> stale TLB entries" would impact the _regular_ TLB invalidates (by
> update_mmu_cache()) and perhaps make those more aggressive, exactly
> because the coprocessor may not handle the fault as gracefully.

It could be done that way... Hmm although we do have something similar 
also in radix__ptep_set_access_flags for the relaxing permissions case
so maybe this is required for not-present faults as well? I'm not 
actually sure now.

But it's a bit weird and awkward because it's working around quirks in
the hardware which aren't regular, not because we're _completely_ 
confused (I hope).

Thanks,
Nick

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [patch 01/15] mm/memory.c: avoid access flag update TLB flush for retried page fault
  2020-07-28 22:53                     ` Nicholas Piggin
@ 2020-07-29 13:58                       ` Michael Ellerman
  0 siblings, 0 replies; 4+ messages in thread
From: Michael Ellerman @ 2020-07-29 13:58 UTC (permalink / raw)
  To: Nicholas Piggin, Linus Torvalds
  Cc: linux-arch, Hillf Danton, mm-commits, Catalin Marinas,
	Hugh Dickins, Josef Bacik, Will Deacon, Linux-MM, Matthew Wilcox,
	Johannes Weiner, Yu Xu, Andrew Morton, linuxppc-dev, Yang Shi,
	Kirill A . Shutemov

Nicholas Piggin <npiggin@gmail.com> writes:
> Excerpts from Linus Torvalds's message of July 29, 2020 5:02 am:
>> On Tue, Jul 28, 2020 at 3:53 AM Nicholas Piggin <npiggin@gmail.com> wrote:
>>>
>>> The quirk is a problem with coprocessor where it's supposed to
>>> invalidate the translation after a fault but it doesn't, so we can get a
>>> read-only TLB stuck after something else does a RO->RW upgrade on the
>>> TLB. Something like that IIRC.  Coprocessors have their own MMU which
>>> lives in the nest not the core, so you need a global TLB flush to
>>> invalidate that thing.
>> 
>> So I assumed, but it does seem confused.
>> 
>> Why? Because if there are stale translations on the co-processor,
>> there's no guarantee that one of the CPU's will have them and take a
>> fault.
>> 
>> So I'm not seeing why a core CPU doing spurious TLB invalidation would
>> follow from "stale TLB in the Nest".
>
> If the nest MMU access faults, it sends an interrupt to the CPU and
> the driver tries to handle the page fault for it (I think that's how
> it works).

Yeah AFAIK. I think they all end up calling copro_handle_mm_fault().

Except for NX/vas, where the model is a fault just causes the request to
be dropped and sent back to userspace to fix things up.

cheers

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200723211432.b31831a0df3bc2cbdae31b40@linux-foundation.org>
     [not found] ` <20200724041508.QlTbrHnfh%akpm@linux-foundation.org>
     [not found]   ` <CAHk-=wguPA=pDskR-eMMjwR5LDEaMXrqbmDbrKr0u=wV1LE4rg@mail.gmail.com>
     [not found]     ` <CAHk-=wh4kmU5FdT=Yy7N9wA=se=ALbrquCrOkjCMhiQnOBLvDA@mail.gmail.com>
     [not found]       ` <0323de82-cfbd-8506-fa9c-a702703dd654@linux.alibaba.com>
     [not found]         ` <20200727110512.GB25400@gaia>
     [not found]           ` <39560818-463f-da3a-fc9e-3a4a0a082f61@linux.alibaba.com>
     [not found]             ` <eb1f5cb4-7c3d-df42-f4aa-804e12df45e2@linux.alibaba.com>
     [not found]               ` <CAHk-=wha6f0gF1SJg96R77h0oTuc_oO7-37wD=mYGy6TyJOwbQ@mail.gmail.com>
2020-07-28 10:53                 ` [patch 01/15] mm/memory.c: avoid access flag update TLB flush for retried page fault Nicholas Piggin
2020-07-28 19:02                   ` Linus Torvalds
2020-07-28 22:53                     ` Nicholas Piggin
2020-07-29 13:58                       ` Michael Ellerman

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git