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 EAA30C4829A for ; Wed, 7 Feb 2024 07:28:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 55BD36B0078; Wed, 7 Feb 2024 02:28:08 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4E4F26B007D; Wed, 7 Feb 2024 02:28:08 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 385726B007E; Wed, 7 Feb 2024 02:28:08 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 23CFB6B0078 for ; Wed, 7 Feb 2024 02:28:08 -0500 (EST) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id E378C1C16E3 for ; Wed, 7 Feb 2024 07:28:07 +0000 (UTC) X-FDA: 81764179014.06.3320059 Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) by imf26.hostedemail.com (Postfix) with ESMTP id AEE03140003 for ; Wed, 7 Feb 2024 07:28:05 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=none; spf=pass (imf26.hostedemail.com: domain of alex@ghiti.fr designates 217.70.183.201 as permitted sender) smtp.mailfrom=alex@ghiti.fr; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707290886; a=rsa-sha256; cv=none; b=pmdjCPI3tHtT8tqpxogA6yA+hcMaknoEuNCIFszlHW3j8txkAm8VVJmaE1EP7UgRPVw0kb zjhNPhFpPeH6HNLwbtIuCmwCHoJMdplcJCwOeyB11ef7oe/Xko6W9II7sCooC7Wi0nJf/m ANK2CunfUv7IqiNQCU9/qT/SVvo49Kw= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=none; spf=pass (imf26.hostedemail.com: domain of alex@ghiti.fr designates 217.70.183.201 as permitted sender) smtp.mailfrom=alex@ghiti.fr; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707290886; 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; bh=BaBlNAm33H6Vz9uH6TgncznSQoduFk0xqTMYqtgodLg=; b=xg5SWTTLYvuak8ZLdA0M8sUsbShDBPy7Vp4SdbMLyoU3yKftbYyn6vagzff0waUecU7pi7 mNwPa9H7gz7NMlTgAJrRKXmFahuzi1lqYmA64HUncAcnoN5KhdPN9M0MkZX2l2Tssi+lxc o6sojwvMlsdIvoWGnQsR79GMmztObPY= Received: by mail.gandi.net (Postfix) with ESMTPSA id 61C0D1BF204; Wed, 7 Feb 2024 07:28:01 +0000 (UTC) Message-ID: <6d03704c-e0b3-40e8-a0b1-6ad357a775d1@ghiti.fr> Date: Wed, 7 Feb 2024 08:28:01 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/4] riscv: tlb: fix __p*d_free_tlb() Content-Language: en-US From: Alexandre Ghiti To: Jisheng Zhang , Paul Walmsley , Palmer Dabbelt , Albert Ou , Will Deacon , "Aneesh Kumar K . V" , Andrew Morton , Nick Piggin , Peter Zijlstra Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, linux-mm@kvack.org References: <20231219175046.2496-1-jszhang@kernel.org> <20231219175046.2496-2-jszhang@kernel.org> <08f55d3e-d68e-406a-9bc9-d62f3c86e949@ghiti.fr> In-Reply-To: <08f55d3e-d68e-406a-9bc9-d62f3c86e949@ghiti.fr> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-GND-Sasl: alex@ghiti.fr X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: AEE03140003 X-Stat-Signature: unbcd5xzhff9gwnb59zuixcebniz86ew X-HE-Tag: 1707290885-533198 X-HE-Meta: U2FsdGVkX1/4NJRFu00ZdJxPToC39K/rONcHH+jjqDKhVnWWH9kOFKzotgDm4npN79hugXuHWl0mpa/2QKqJjvwVDabfESkTFAcowfzFkbRv9bk+hP07y4cLhsRpkdhhMjarOiSiwmv8KqdTQAv9GCW2M8af4yl9Tdu85KYhTgTykh/jah2FYz7j/DHZDWrh40Lkb9VdU/JbEgzwaPoC3/eH4ytFzrgwM8xR2ltH/fuKTFeGdoC4BDGEK+hp1Ttn3nLjHdplwIqW4HLE+qZy+z1A+4e4gdVzNZvcurpxdritkRlV9HFeb/UaXKzoTKgNVnrbNUEKOJTRHdQzQszm1JrjW79TAsvCduKwgXcT+WlYaodt0ihEjxZ+cVn+JO7DTbtzcXRAWxkxVeqYcMaO5vaIWJdFZKL5eogWRkG/6hmfrcU2vzVDcHi6ReVH627V3NwKhypLwXC5jTT54qtnomGNVb4vjVz7+EU/LAdCyyjMgZJJuDc7llrCEJJDy489BZTmfzj1dXXFUOmeYqXYOse43S+GyyKlV/45MP/z5d88bvI/DB0Wy0HI2kwgp+g0c6fWoc09qhU5Jjs8yPQcsihO4lLkH9u2L4mVFfSKPMFjXAWTmdEgYAAQv3K9ofXTp64CVondAmW072UfrdtDNa1sn7I2HsXwpnqS541ptOD4yhQsbs6GvsQh1XmT9jxlnmzMAahHaN/JknpAStr383UWdlHAj9ZQiBR00RwBse5YLyzBxlfO8wdJAU0k/oX2DYujLj2j/48ddVKJOyEnVfSnPlkt5VHiXuEdEEyE5foMo/w6KGJVBVSj3K1G5CDbsuqzoEON+UCfLozEnBNd+b+ynn2OmFXp3vKMkhR5r2PGsi/HJkssHKdjXIRnAw7ZkMIIXYTVt0r1mho+fM4M2Y2ltaOHDubnJSpWMDSAGjm/LFU7EH5kmVkbbPpUVEW5JfyAvmzCSAKw9mDOQEL ZlLugLGD sMnFa/t7CtwG9TfJGpPVC8VOICJeyNW+FGOFJA6rXSrhWajrh/7+RUqHkwp3cRI2RkiIcg7X+u6UfYnXlxlbzdAENUfMsSuu5CEE9CgGbpUrZihCpDJPZP0cPIHeAqQrvVxjSPr49ghyrQ8fiv6szLxMLPTowyAME/qmZGnY7M8VbhVYiJ9AyPKtQ56PM1JW1evfsmiAS4zmkaHrhqqSaR2eW5WNmZr9jJCZmFdehijbb+TbzCGPsR6K5tWeC1EdVXlYPHXHOrXI/smKXKR0ga0jEZ8Sb0WT6lqXCbVpz64hTJyQNB1p77bLHs8fCopkQxALU/R6Ky/KBZqKumTC70OdQw98kgf7Ao/gX 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 Jisheng, On 31/12/2023 07:21, Alexandre Ghiti wrote: > Hi Jisheng, > > On 19/12/2023 18:50, Jisheng Zhang wrote: >> If non-leaf PTEs I.E pmd, pud or p4d is modified, a sfence.vma is >> a must for safe, imagine if an implementation caches the non-leaf >> translation in TLB, although I didn't meet this HW so far, but it's >> possible in theory. >> >> Signed-off-by: Jisheng Zhang >> --- >>   arch/riscv/include/asm/pgalloc.h | 20 +++++++++++++++++--- >>   1 file changed, 17 insertions(+), 3 deletions(-) >> >> diff --git a/arch/riscv/include/asm/pgalloc.h >> b/arch/riscv/include/asm/pgalloc.h >> index d169a4f41a2e..a12fb83fa1f5 100644 >> --- a/arch/riscv/include/asm/pgalloc.h >> +++ b/arch/riscv/include/asm/pgalloc.h >> @@ -95,7 +95,13 @@ static inline void pud_free(struct mm_struct *mm, >> pud_t *pud) >>           __pud_free(mm, pud); >>   } >>   -#define __pud_free_tlb(tlb, pud, addr) pud_free((tlb)->mm, pud) >> +#define __pud_free_tlb(tlb, pud, addr)                    \ >> +do {                                    \ >> +    if (pgtable_l4_enabled) {                    \ >> +        pagetable_pud_dtor(virt_to_ptdesc(pud));        \ >> +        tlb_remove_page_ptdesc((tlb), virt_to_ptdesc(pud)); \ > > > The specification indeed states that an sfence.vma must be emitted > after a page directory modification. Your change is not enough though > since eventually tlb_flush() is called and in this function we should > add: > > if (tlb->freed_tables) >     tlb_flush_mm(); I sent a patch for that here https://lore.kernel.org/linux-riscv/20240128120405.25876-1-alexghiti@rivosinc.com/ You can add: Reviewed-by: Alexandre Ghiti Thanks, Alex > > otherwise we are not guaranteed that a "global" sfence.vma is called. > > Would you be able to benchmark this change and see the performance > impact? > > Thanks, > > Alex > > >> +    }                                \ >> +} while (0) >>     #define p4d_alloc_one p4d_alloc_one >>   static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned >> long addr) >> @@ -124,7 +130,11 @@ static inline void p4d_free(struct mm_struct >> *mm, p4d_t *p4d) >>           __p4d_free(mm, p4d); >>   } >>   -#define __p4d_free_tlb(tlb, p4d, addr) p4d_free((tlb)->mm, p4d) >> +#define __p4d_free_tlb(tlb, p4d, addr)                    \ >> +do {                                    \ >> +    if (pgtable_l5_enabled)                        \ >> +        tlb_remove_page_ptdesc((tlb), virt_to_ptdesc(p4d)); \ >> +} while (0) >>   #endif /* __PAGETABLE_PMD_FOLDED */ >>     static inline void sync_kernel_mappings(pgd_t *pgd) >> @@ -149,7 +159,11 @@ static inline pgd_t *pgd_alloc(struct mm_struct >> *mm) >>     #ifndef __PAGETABLE_PMD_FOLDED >>   -#define __pmd_free_tlb(tlb, pmd, addr) pmd_free((tlb)->mm, pmd) >> +#define __pmd_free_tlb(tlb, pmd, addr)                \ >> +do {                                \ >> +    pagetable_pmd_dtor(virt_to_ptdesc(pmd));        \ >> +    tlb_remove_page_ptdesc((tlb), virt_to_ptdesc(pmd));    \ >> +} while (0) >>     #endif /* __PAGETABLE_PMD_FOLDED */ >