09:27 <mrutland> arnd: that's because on arm64 the ASID is in TTBR1, and we have to ensure that the old TTBR0 tables aren't ever live at the same time as the *new* ASID we're switching to. So we point TTBR0 at the (empty) reserved TTBR0 tables, then switch the ASID in TTBR1, then switch to the new TTB0 tables

09:32 <mrutland> i.e. this is needed for any swtich, not only on rollover

09:40 <arnd> mrutland: got it, thanks! Is there any reason for having the ASID in TTBR1 other than CONFIG_QCOM_FALKOR_ERRATUM_1003?

09:41 <mrutland> arnd: it was moved there for KPTI; it's a bit painful to conditionally move it at runtime

09:42 <mrutland> The QCOM erratum is just something we have to handle, and wasn't the reason for moving the ASID into TTB1

09:44 <mrutland> See https://lore.kernel.org/linux-arm-kernel/1512563739-25239-1-git-send-email-will.deacon@arm.com/ for when that changed; we already had a workaround for the erratu before we moved the ASID

11:43 <arnd> mrutland: ok, I think I understand how this works for KPTI now, not sure about the non-KPTI case yet. So the isb() would prevent fetching a TLB with the wrong ASID, but doesn't prevent an IRQ between the two write_sysreg(ttbr?, ...), and an interrupt handler might speculatively fetch from userspace, where the access gets prevented by PAN but the invalid TLB load does not?

11:58 <mrutland> arnd: Unusually for the archtiecture, writes to TTBR0 and TTBR1 are ordered w.r.t. one another without an ISB; see commit b9293d457ff3de415fa07d7e35978bceb29c3827

11:58 <mrutland> ... so the trailing SB is just to ensure the final state has taken effect

11:58 <mrutland> * the trailing ISB, not SB

12:01 <mrutland> Interrupts are masked over calls to cpu_do_switch_mm() by virtue of callers having IRQs masked, and we rely on that to avoid a bunch of races

12:03 <mrutland> (I'm a bit worried about that last point; double-checking that's really true now, or we have other problems regardless of TTBR writes)

12:04 <mrutland> Ugh, we probably need CONFIG_ARCH_WANT_IRQS_OFF_ACTIVATE_MM, and I need to dig into that more

12:09 <mrutland> If we taken an IRQ mid-sequence which *doesn't* preempt and doesn't perform an explicit uaccess, there won't be a problem, the reserved TTBR value just prevents allocating new TLB entries for the TTBR0 range. Speculation can't do anything harmful in that state; if it could, that would be possible even without an IRQ.

12:16 <arnd> I'm still struggling to understand what could go wrong without an irq if the reserved ttbr0 wasn't set first: why isn't the isb sufficient to prevent allocating a TLB with the wrong ASID/ttb pair?

12:17 <mrutland> arnd: ISB where?

12:17 <mrutland> arnd: asynchronous translation table walks, as a result of prefetching or speculation, could occur *before* the trailing ISB

12:18 <mrutland> arnd: basically, the same problem as you were asking about for IRQs, but no IRQ needed

12:18 <mrutland> The CPU can jsut do that at any time, for any reason at all

12:22 <mrutland> From a quick test, core code calls switch_mm() with IRQs enabled, so we have a bigger bug there that needs to be fixed

12:28 <mrutland> I'm just going to grab lunch, then I'll take another look

12:38 <arnd> in the object code, I see that the three isntructions are always consecutive: "msr ttbr1_el1, x2; msr ttbr0_el1, x0; isb". My understanding was that any instruction from before the ttbr1_el1 store would be guaranteed to still use the old ttbr* value, even for prefetching. Any instruction after the isb would only prefetch use both the new ttbr0 and ttbr1 values because of the isb

12:41 <arnd> between the ttbr1_el1 access and the isb, I would expect no TLB fetch to happen because there are no load/store or even branch instructions that could be speculatively executed. What am I missing?

13:08 <mrutland> Your understanding is incorrect: the CPU is allowed to start using the new TTBR value immediately upon it being written, but is only guaranteed to be using the new value consistently after a subsequent ISB.

13:09 <mrutland> between the write and the ISB, it could speculate/prefetch using either the old or new values (and e.g. could go old->new->old), but has to use a consistent snapshot

13:10 <mrutland> TLB lookups and translation table walks can happen *at any time* and *for any reason* (e.g. the CPU might start prefetching a VA range or start prewalking the translation tables), and that's not limited by the specific instruction stream

13:11 <mrutland> The writes are ordered to avoid performing translation table walks or TLB allocation with an *inconsistent* snapshot of ASID + BADDR

13:12 <mrutland> *the TTBR writes are ordered the way they are

13:13 <mrutland> arnd: ^ does that make sense to you now?

13:41 <arnd> mrutland: yes, the example of immediately prefilling the TLB on TTBR update makes snese, and I guess if it traps to EL2 or EL3, before the isb, it's easy to construct more cases that actually fill the TLB the same way

13:43 <mrutland> Cool!