From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3EB83C25B74 for ; Mon, 3 Jun 2024 02:26:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A206B6B0093; Sun, 2 Jun 2024 22:26:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9A8F66B0095; Sun, 2 Jun 2024 22:26:24 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 822406B0098; Sun, 2 Jun 2024 22:26:24 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 619EA6B0093 for ; Sun, 2 Jun 2024 22:26:24 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id C075C1605B4 for ; Mon, 3 Jun 2024 02:26:23 +0000 (UTC) X-FDA: 82187988246.26.B7FA7C6 Received: from mail-oi1-f172.google.com (mail-oi1-f172.google.com [209.85.167.172]) by imf15.hostedemail.com (Postfix) with ESMTP id 7D9A5A000B for ; Mon, 3 Jun 2024 02:26:21 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b="Q/ctB9G4"; spf=none (imf15.hostedemail.com: domain of cuiyunhui@bytedance.com has no SPF policy when checking 209.85.167.172) smtp.mailfrom=cuiyunhui@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1717381582; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=9FVO8FfbvRSq3lInIu0BX0h7ckGi2C3dp5Cu4roUh7Y=; b=yiprG//1PKR+PSwrwBi7hjoD1HJBPkjbC5W2Atzc9ib7x5dshp0/rN69Rum3TEi/ldj34c K4XABDioOdjvCrJ8PZGvEwNhTJlRhFnukp8bULG652ZDeWRzpV/SisEiqh3i2EJ5kx5Qkc FP2Bz0RXZqqXZUivf2hR5XOYAQqcHUk= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b="Q/ctB9G4"; spf=none (imf15.hostedemail.com: domain of cuiyunhui@bytedance.com has no SPF policy when checking 209.85.167.172) smtp.mailfrom=cuiyunhui@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1717381582; a=rsa-sha256; cv=none; b=u/9XtFJtrj18HUpdU40NRq/22aSZm+GY98IgXdL+pT9c2/9+kdd7PYxG4vTkwVCcPqc3ga o3D0jEtyd306UWGpr+Q+IH8W/8lrNSISfl0IlImmu9DCmJ/qntWExrlCIIyMPKpA4VkEsf P4Viu1cELdXm9PiRkxj4es7ku1ZZj+k= Received: by mail-oi1-f172.google.com with SMTP id 5614622812f47-3d1d65a471aso2296681b6e.1 for ; Sun, 02 Jun 2024 19:26:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance.com; s=google; t=1717381580; x=1717986380; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=9FVO8FfbvRSq3lInIu0BX0h7ckGi2C3dp5Cu4roUh7Y=; b=Q/ctB9G4xhfyHsL9OZmxZozT1/ZaPg7FE8CTdL5tuEtcdEiaD2xL+I2SzE++z7Mhgw eRM2+FzTxGytIKzGqr+BV1vkCUPlxN89QyIAsCNgQpZrvGkHdh/yBQGL2nbvkevdez/8 ygnuOGQUSDq3eaht53iGgjAFifp585J10HUzaKwhPB1IISI49RcPmtmbopikYOHtWlFr rszlyQeLYkhNdM0DZUGzgG4E8Gtt67bF8L34Y8qHyU9qA7JBZAFrLLBnykfmkmUhERyQ 9eKSCc6rJ961oSlMfousXo3XRUOt8quIoGQ7JKmaGt7F3if0FxFj2y43WQqR82olStK+ 14Vg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717381580; x=1717986380; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=9FVO8FfbvRSq3lInIu0BX0h7ckGi2C3dp5Cu4roUh7Y=; b=giVt27NwCTqOyVjS0xtlnpHIZDcTXAPuK3aompPB7bS4a0LTACPENusd+AiEh4i0ka VOeapNkBGXJq3NRlpvixUQ7+8GhnD3or2MBcOBOrnhAmcjPlvhCeUSYDDJYFyQCTHqhT +d0A7S5LurkTR+zvI5F0p7qaHAUJ/wEFQ3rDNAwDGYZNH5BRcZO7lWnxDBcKnFSHsMZU d18RTd98iIhBCktAUh8WI2yqudxUaGxIsar0QeFG/ivH7PsbO8md4AV6umr6KYel5GOm +8vyKQftBser/WCh1dbFiTS/kiAHFNokJLMURVrGiLoFlIm40wox5YknEBizy0Kofmmm tp4A== X-Forwarded-Encrypted: i=1; AJvYcCVwu9s5wAMqkRFynTtgNW3Exx1cstW6S29kHIY8iUFfGFF+E77PSbxGPt3TT/W3MX5Pq/9li/WF4e5lS5DCDfQt1jw= X-Gm-Message-State: AOJu0Yyl6PPnl6ZG1853GoSHiQZGeniBshBRyYbgfoxazqlBfRozR+Q2 LwNap8aQ+AVm322cPGaaTzxD9H8oCSpc2BaDASEK2zRWGI1wPv79IEO+00qvHZmna4v/D8YewHU wimDR8Qd+FV1U9qsxBym54FkkkRBGaCZNsfFMkw== X-Google-Smtp-Source: AGHT+IGCM0LMNQOovDp9kRtkZJlNH3sjfjIsZCkVxfHkVGktafrBKJCLQKJDpHSDg3IP3UkbyTARIWGa6hF9KT8otKE= X-Received: by 2002:a05:6870:55ce:b0:24f:df31:9bbf with SMTP id 586e51a60fabf-2508bcb8b9bmr9214174fac.39.1717381580128; Sun, 02 Jun 2024 19:26:20 -0700 (PDT) MIME-Version: 1.0 References: <20240131155929.169961-1-alexghiti@rivosinc.com> <20240131155929.169961-4-alexghiti@rivosinc.com> In-Reply-To: <20240131155929.169961-4-alexghiti@rivosinc.com> From: yunhui cui Date: Mon, 3 Jun 2024 10:26:09 +0800 Message-ID: Subject: Re: [External] [PATCH RFC/RFT v2 3/4] riscv: Stop emitting preventive sfence.vma for new vmalloc mappings To: Alexandre Ghiti Cc: Catalin Marinas , Will Deacon , Thomas Bogendoerfer , Michael Ellerman , Nicholas Piggin , Christophe Leroy , Paul Walmsley , Palmer Dabbelt , Albert Ou , Andrew Morton , Ved Shanbhogue , Matt Evans , Dylan Jhong , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 7D9A5A000B X-Stat-Signature: ca8d93d6d56yug7fyohcns5gwc33dxdd X-HE-Tag: 1717381581-718686 X-HE-Meta: U2FsdGVkX1/juIh9hSgyANaQi/xOPtCEKVs7mMlCy0H4i+6ySL4wyibRxtkZLzqKJlvA8rWm7SmEs9diXA96E5QLizHV0J3avVrUaM6W7AO1UQqJuWenvJQ61XdXudMVPvKTktd8Tjt8GgJYFVF6qXOm+k8gwyf82KyJz5DYlcnhT0iH/j49vv0t011lGsdTPET4p8P/iW0fGiYsfKNHRerDRCNyCzGBYMBpumkQxxVVWJu3c165cjiRCNItsG9sfPNrmOUjQ/FMCFeN/+OOjZeberl9Gl0AgHPASjTmR4Lr3DvB3VHzcT3uXdJ9viOXKOMb87+cK3Yd1Gxmd3cIofIPCjJyrwpUdbY08wmuKg5KkvItqAnxZ89jQrkD0CZ+S1hrxet+MYKiSGAqh4RpgM4uRUzRVDsGkYDf+oTtB7wH/02oASV246lUKux+P7cLUIRNBoPZ4SIjhTdsQ/OemX9yBSn6tqlO/tR2aW+LCvp1yeabAu5J5DvT2K9JZVgXH0D0rPtDf6qKZ7gVd2Sq4dG/ZKNoS+TFDTm6NN53Jd9BovqW7hm+yEDKqku9Xm0B9C2FR5XPi1CdLgmH08uIPxMqLV7qRmlK/E2+G5THJg8eQ/yUwa0llvyef+b8JiqbxJwhyzZ8JUgYA7SwisPGVWvuZd6+cRni0Jbr/rpZwKGBDjfOtrqf3Z2aRUYmtZs9dylhmHa0N3J1n6XQhtARH+VrHyhQ0hzbPINeyDiA4aOxhWVIMW+C2BT8uWFPIaDKvpfOhe/zi0yBECu19xMrO3AjwweTW+tXoOmHjjE9/JF2Cj+2Ek+MzfP1/daVtU52UbXc3dzpLJlnwWvoQ5FLGxqstjifELkQYJg8Bs/J3g8uz3oruTTJoroHIiYVNlYfCQts2ZUL3AfOnn9Ixv/OA22QznJJy0nkqtx/TS+w7imAO/rzjPnivNillKQ4E4mJNXpbqmb0soXgW4gzxwF i2sfWRoR FsX0Ia8lovGuBwXfusZIHQDiySnvTYpWdLHfgRoQdx7VabH4/rBuutLNJOZxUxr1Js9gRQSS83IZZfbqN9FYv18QXU/UQUDv3gql7xjHEECmyw/7spffj8QdI0tJ+E/JNk0UdrSBA039T/QJ8ucmMScSaHkJhQowQC5RD9L4BWkuuraVmaNl+wO+LC27/VXdr5tNGSoo2g84F30BeVWFI0jim3hH89oh1CJIjfD6AhdbnJAc8v4GiH4teITj1Sum4aYox18+w+PDemPfI+7CLrbf9/H103ZFD8NLzT1dq6C1zlUPNJr7ufWBHU1qF2lRQ8UEmHlztwpeB91UiT9sRGy3nr/G9hiANb8uyQ5ghDyqNTYAnh1SdO1Wop64mGCbBA4yKxn5Yh5MlluUPGceLWQNKQr66odNCR2GGkG2akvRZ+vzJ8TpNBFZhXRb4TCPtvOVBSMLeHarI9eda3i5c2tCVcw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Hi Alexandre, On Thu, Feb 1, 2024 at 12:03=E2=80=AFAM Alexandre Ghiti wrote: > > In 6.5, we removed the vmalloc fault path because that can't work (see > [1] [2]). Then in order to make sure that new page table entries were > seen by the page table walker, we had to preventively emit a sfence.vma > on all harts [3] but this solution is very costly since it relies on IPI. > > And even there, we could end up in a loop of vmalloc faults if a vmalloc > allocation is done in the IPI path (for example if it is traced, see > [4]), which could result in a kernel stack overflow. > > Those preventive sfence.vma needed to be emitted because: > > - if the uarch caches invalid entries, the new mapping may not be > observed by the page table walker and an invalidation may be needed. > - if the uarch does not cache invalid entries, a reordered access > could "miss" the new mapping and traps: in that case, we would actually > only need to retry the access, no sfence.vma is required. > > So this patch removes those preventive sfence.vma and actually handles > the possible (and unlikely) exceptions. And since the kernel stacks > mappings lie in the vmalloc area, this handling must be done very early > when the trap is taken, at the very beginning of handle_exception: this > also rules out the vmalloc allocations in the fault path. > > Link: https://lore.kernel.org/linux-riscv/20230531093817.665799-1-bjorn@k= ernel.org/ [1] > Link: https://lore.kernel.org/linux-riscv/20230801090927.2018653-1-dylan@= andestech.com [2] > Link: https://lore.kernel.org/linux-riscv/20230725132246.817726-1-alexghi= ti@rivosinc.com/ [3] > Link: https://lore.kernel.org/lkml/20200508144043.13893-1-joro@8bytes.org= / [4] > Signed-off-by: Alexandre Ghiti > --- > arch/riscv/include/asm/cacheflush.h | 18 +++++- > arch/riscv/include/asm/thread_info.h | 5 ++ > arch/riscv/kernel/asm-offsets.c | 5 ++ > arch/riscv/kernel/entry.S | 84 ++++++++++++++++++++++++++++ > arch/riscv/mm/init.c | 2 + > 5 files changed, 113 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/include/asm/cacheflush.h b/arch/riscv/include/asm= /cacheflush.h > index a129dac4521d..b0d631701757 100644 > --- a/arch/riscv/include/asm/cacheflush.h > +++ b/arch/riscv/include/asm/cacheflush.h > @@ -37,7 +37,23 @@ static inline void flush_dcache_page(struct page *page= ) > flush_icache_mm(vma->vm_mm, 0) > > #ifdef CONFIG_64BIT > -#define flush_cache_vmap(start, end) flush_tlb_kernel_range(st= art, end) > +extern u64 new_vmalloc[NR_CPUS / sizeof(u64) + 1]; > +extern char _end[]; > +#define flush_cache_vmap flush_cache_vmap > +static inline void flush_cache_vmap(unsigned long start, unsigned long e= nd) > +{ > + if (is_vmalloc_or_module_addr((void *)start)) { > + int i; > + > + /* > + * We don't care if concurrently a cpu resets this value = since > + * the only place this can happen is in handle_exception(= ) where > + * an sfence.vma is emitted. > + */ > + for (i =3D 0; i < ARRAY_SIZE(new_vmalloc); ++i) > + new_vmalloc[i] =3D -1ULL; > + } > +} > #define flush_cache_vmap_early(start, end) local_flush_tlb_kernel_ra= nge(start, end) > #endif > > diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/as= m/thread_info.h > index 5d473343634b..32631acdcdd4 100644 > --- a/arch/riscv/include/asm/thread_info.h > +++ b/arch/riscv/include/asm/thread_info.h > @@ -60,6 +60,11 @@ struct thread_info { > void *scs_base; > void *scs_sp; > #endif > + /* > + * Used in handle_exception() to save a0, a1 and a2 before knowin= g if we > + * can access the kernel stack. > + */ > + unsigned long a0, a1, a2; > }; > > #ifdef CONFIG_SHADOW_CALL_STACK > diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offs= ets.c > index a03129f40c46..939ddc0e3c6e 100644 > --- a/arch/riscv/kernel/asm-offsets.c > +++ b/arch/riscv/kernel/asm-offsets.c > @@ -35,6 +35,8 @@ void asm_offsets(void) > OFFSET(TASK_THREAD_S9, task_struct, thread.s[9]); > OFFSET(TASK_THREAD_S10, task_struct, thread.s[10]); > OFFSET(TASK_THREAD_S11, task_struct, thread.s[11]); > + > + OFFSET(TASK_TI_CPU, task_struct, thread_info.cpu); > OFFSET(TASK_TI_FLAGS, task_struct, thread_info.flags); > OFFSET(TASK_TI_PREEMPT_COUNT, task_struct, thread_info.preempt_co= unt); > OFFSET(TASK_TI_KERNEL_SP, task_struct, thread_info.kernel_sp); > @@ -42,6 +44,9 @@ void asm_offsets(void) > #ifdef CONFIG_SHADOW_CALL_STACK > OFFSET(TASK_TI_SCS_SP, task_struct, thread_info.scs_sp); > #endif > + OFFSET(TASK_TI_A0, task_struct, thread_info.a0); > + OFFSET(TASK_TI_A1, task_struct, thread_info.a1); > + OFFSET(TASK_TI_A2, task_struct, thread_info.a2); > > OFFSET(TASK_TI_CPU_NUM, task_struct, thread_info.cpu); > OFFSET(TASK_THREAD_F0, task_struct, thread.fstate.f[0]); > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S > index 9d1a305d5508..c1ffaeaba7aa 100644 > --- a/arch/riscv/kernel/entry.S > +++ b/arch/riscv/kernel/entry.S > @@ -19,6 +19,78 @@ > > .section .irqentry.text, "ax" > > +.macro new_vmalloc_check > + REG_S a0, TASK_TI_A0(tp) > + REG_S a1, TASK_TI_A1(tp) > + REG_S a2, TASK_TI_A2(tp) > + > + csrr a0, CSR_CAUSE > + /* Exclude IRQs */ > + blt a0, zero, _new_vmalloc_restore_context > + /* Only check new_vmalloc if we are in page/protection fault */ > + li a1, EXC_LOAD_PAGE_FAULT > + beq a0, a1, _new_vmalloc_kernel_address > + li a1, EXC_STORE_PAGE_FAULT > + beq a0, a1, _new_vmalloc_kernel_address > + li a1, EXC_INST_PAGE_FAULT > + bne a0, a1, _new_vmalloc_restore_context > + > +_new_vmalloc_kernel_address: > + /* Is it a kernel address? */ > + csrr a0, CSR_TVAL > + bge a0, zero, _new_vmalloc_restore_context > + > + /* Check if a new vmalloc mapping appeared that could explain the= trap */ > + > + /* > + * Computes: > + * a0 =3D &new_vmalloc[BIT_WORD(cpu)] > + * a1 =3D BIT_MASK(cpu) > + */ > + REG_L a2, TASK_TI_CPU(tp) > + /* > + * Compute the new_vmalloc element position: > + * (cpu / 64) * 8 =3D (cpu >> 6) << 3 > + */ > + srli a1, a2, 6 > + slli a1, a1, 3 > + la a0, new_vmalloc > + add a0, a0, a1 > + /* > + * Compute the bit position in the new_vmalloc element: > + * bit_pos =3D cpu % 64 =3D cpu - (cpu / 64) * 64 =3D cpu - (cpu = >> 6) << 6 > + * =3D cpu - ((cpu >> 6) << 3) << 3 > + */ > + slli a1, a1, 3 > + sub a1, a2, a1 > + /* Compute the "get mask": 1 << bit_pos */ > + li a2, 1 > + sll a1, a2, a1 > + > + /* Check the value of new_vmalloc for this cpu */ > + REG_L a2, 0(a0) > + and a2, a2, a1 > + beq a2, zero, _new_vmalloc_restore_context > + > + /* Atomically reset the current cpu bit in new_vmalloc */ > + amoxor.w a0, a1, (a0) > + > + /* Only emit a sfence.vma if the uarch caches invalid entries */ > + ALTERNATIVE("sfence.vma", "nop", 0, RISCV_ISA_EXT_SVVPTC, 1) > + > + REG_L a0, TASK_TI_A0(tp) > + REG_L a1, TASK_TI_A1(tp) > + REG_L a2, TASK_TI_A2(tp) > + csrw CSR_SCRATCH, x0 > + sret > + > +_new_vmalloc_restore_context: > + REG_L a0, TASK_TI_A0(tp) > + REG_L a1, TASK_TI_A1(tp) > + REG_L a2, TASK_TI_A2(tp) > +.endm > + > + > SYM_CODE_START(handle_exception) > /* > * If coming from userspace, preserve the user thread pointer and= load > @@ -30,6 +102,18 @@ SYM_CODE_START(handle_exception) > > .Lrestore_kernel_tpsp: > csrr tp, CSR_SCRATCH > + > + /* > + * The RISC-V kernel does not eagerly emit a sfence.vma after eac= h > + * new vmalloc mapping, which may result in exceptions: > + * - if the uarch caches invalid entries, the new mapping would n= ot be > + * observed by the page table walker and an invalidation is nee= ded. > + * - if the uarch does not cache invalid entries, a reordered acc= ess > + * could "miss" the new mapping and traps: in that case, we onl= y need > + * to retry the access, no sfence.vma is required. > + */ > + new_vmalloc_check > + > REG_S sp, TASK_TI_KERNEL_SP(tp) > > #ifdef CONFIG_VMAP_STACK > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c > index eafc4c2200f2..54c9fdeda11e 100644 > --- a/arch/riscv/mm/init.c > +++ b/arch/riscv/mm/init.c > @@ -36,6 +36,8 @@ > > #include "../kernel/head.h" > > +u64 new_vmalloc[NR_CPUS / sizeof(u64) + 1]; > + > struct kernel_mapping kernel_map __ro_after_init; > EXPORT_SYMBOL(kernel_map); > #ifdef CONFIG_XIP_KERNEL > -- > 2.39.2 > > Can we consider using new_vmalloc as a percpu variable, so that we don't need to add a0/1/2 in thread_info? Also, try not to do too much calculation logic in new_vmalloc_check, after all, handle_exception is a high-frequency path. In this case, can we consider writing new_vmalloc_check in C language to increase readability? Thanks, Yunhui