Linux-csky Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH RFC 11/14] arm64: Move the ASID allocator code in a separate file
       [not found]                   ` <20190701091711.GA21774@arrakis.emea.arm.com>
@ 2019-07-16  3:31                     ` Guo Ren
  2019-07-22 16:38                       ` Catalin Marinas
  0 siblings, 1 reply; 2+ messages in thread
From: Guo Ren @ 2019-07-16  3:31 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Julien Grall, Linux Kernel Mailing List, linux-arm-kernel,
	kvmarm, aou, gary, Atish Patra, hch, paul.walmsley, rppt,
	linux-riscv, Anup Patel, Palmer Dabbelt, suzuki.poulose,
	Marc Zyngier, julien.thierry, Will Deacon, christoffer.dall,
	james.morse, linux-csky

Hello Catalin,

Thanks for sharing about CnP assid experience. See my comment below.

On Mon, Jul 1, 2019 at 5:17 PM Catalin Marinas
> From the ASID reservation/allocation perspective, the mechanism is the
> same between multi-threaded with a shared TLB and multi-core. On arm64,
> a local_flush_tlb_all() on a thread invalidates the TLB for the other
> threads of the same core.
>
> The actual problem with multi-threaded CPUs is a lot more subtle.
> Digging some internal email from 1.5 years ago and pasting it below
> (where "current ASID algorithm" refers to the one prior to the fix and
> CnP - Common Not Private - means shared TLBs on a multi-threaded CPU):
>
>
> The current ASID roll-over algorithm allows for a small window where
> active_asids for a CPU (P1) is different from the actual ASID in TTBR0.
> This can lead to a roll-over on a different CPU (P2) allocating an ASID
> (for a different task) which is still hardware-active on P1.
>
> A TLBI on a CPU (or a peer CPU with CnP) does not guarantee that all the
> entries corresponding to a valid TTBRx are removed as they can still be
> speculatively loaded immediately after TLBI.
>
> While having two different page tables with the same ASID on different
> CPUs should be fine without CnP, it becomes problematic when CnP is
> enabled:
>
> P1                                      P2
> --                                      --
> TTBR0.BADDR = T1
> TTBR0.ASID = A1
> check_and_switch_context(T2,A2)
>   asid_maps[P1] = A2
>   goto fastpath
>                                         check_and_switch_context(T3,A0)
>                                           new_context
>                                             ASID roll-over allocates A1
>                                               since it is not active
>                                           TLBI ALL
> speculate TTBR0.ASID = A1 entry
>                                           TTBR0.BADDR = T3
>                                           TTBR0.ASID = A1
>   TTBR0.BADDR = T2
>   TTBR0.ASID = A2
>
> After this, the common TLB on P1 and P2 (CnP) contains entries
> corresponding to the old T1 and A1. Task T3 using the same ASID A1 can
> hit such entries. (T1,A1) will eventually be removed from the TLB on the
> next context switch on P1 since tlb_flush_pending was set but this is
> not guaranteed to happen.
>
>
> The fix on arm64 (as part of 5ffdfaedfa0a - "arm64: mm: Support Common
> Not Private translations") was to set the reserved TTBR0 in
> check_and_switch_context(), preventing speculative loads into the TLB
> being tagged with the wrong ASID. So this is specific to the ARM CPUs
> behaviour w.r.t. speculative TLB loads, it may not be the case (yet) for
> your architecture.

The most important thing is that TLBI ALL occurs between
"asid_maps[P1] = A2" and "TTBR0.BADDR = T2", then speculative
execution after TLBI which access to user space code/data will result
in a valid asid entry which re-filled into the TLB by PTW.

A similar problem should exist if C-SKY ISA supports SMT. Although the
C-SKY kernel prohibits the kernel from speculating on user space code
directly, ld/st can access user space memory in csky kernel mode.
Therefore, a similar problem occurs when it speculatively executes
copy_from / to_user codes in that window.

RISC-V ISA has a SUM setting bit that prevents the kernel from
speculating access to user space. So this problem has been bypassed
from the design.

I saw arm64 to prevent speculation by temporarily setting TTBR0.el1 to
a zero page table. Is that used to prevent speculative execution user
space code or just prevent ld/st in copy_use_* ?

-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

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

* Re: [PATCH RFC 11/14] arm64: Move the ASID allocator code in a separate file
  2019-07-16  3:31                     ` [PATCH RFC 11/14] arm64: Move the ASID allocator code in a separate file Guo Ren
@ 2019-07-22 16:38                       ` Catalin Marinas
  0 siblings, 0 replies; 2+ messages in thread
From: Catalin Marinas @ 2019-07-22 16:38 UTC (permalink / raw)
  To: Guo Ren
  Cc: Julien Grall, Linux Kernel Mailing List, linux-arm-kernel,
	kvmarm, aou, gary, Atish Patra, hch, paul.walmsley, rppt,
	linux-riscv, Anup Patel, Palmer Dabbelt, suzuki.poulose,
	Marc Zyngier, julien.thierry, Will Deacon, christoffer.dall,
	james.morse, linux-csky

On Tue, Jul 16, 2019 at 11:31:27AM +0800, Guo Ren wrote:
> I saw arm64 to prevent speculation by temporarily setting TTBR0.el1 to
> a zero page table. Is that used to prevent speculative execution user
> space code or just prevent ld/st in copy_use_* ?

Only to prevent explicit ld/st from user. On ARMv8.1+, we don't normally
use the TTBR0 trick but rather disable user space access using the PAN
(privileged access never) feature. However, I don't think PAN disables
speculative accesses, only explicit loads/stores. Also, with ARMv8.2
Linux uses the LDTR/STTR instructions in copy_*_user() which don't need
to disable PAN explicitly.

-- 
Catalin

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

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20190321163623.20219-1-julien.grall@arm.com>
     [not found] ` <20190321163623.20219-12-julien.grall@arm.com>
     [not found]   ` <0dfe120b-066a-2ac8-13bc-3f5a29e2caa3@arm.com>
     [not found]     ` <CAJF2gTTXHHgDboaexdHA284y6kNZVSjLis5-Q2rDnXCxr4RSmA@mail.gmail.com>
     [not found]       ` <c871a5ae-914f-a8bb-9474-1dcfec5d45bf@arm.com>
     [not found]         ` <CAJF2gTStSR7Jmu7=HaO5Wxz=Zn8A5-RD8ktori3oKEhM9vozAA@mail.gmail.com>
     [not found]           ` <20190621141606.GF18954@arrakis.emea.arm.com>
     [not found]             ` <CAJF2gTTVUToRkRtxTmtWDotMGXy5YQCpL1h_2neTBuN3e6oz1w@mail.gmail.com>
     [not found]               ` <20190624153820.GH29120@arrakis.emea.arm.com>
     [not found]                 ` <CAJF2gTRUzHUNV+nzECUp5n2L1akdy=Aovb6tSd+PNVnpasBrqw@mail.gmail.com>
     [not found]                   ` <20190701091711.GA21774@arrakis.emea.arm.com>
2019-07-16  3:31                     ` [PATCH RFC 11/14] arm64: Move the ASID allocator code in a separate file Guo Ren
2019-07-22 16:38                       ` Catalin Marinas

Linux-csky Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-csky/0 linux-csky/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 linux-csky linux-csky/ https://lore.kernel.org/linux-csky \
		linux-csky@vger.kernel.org
	public-inbox-index linux-csky

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-csky


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