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 X-Spam-Level: X-Spam-Status: No, score=-15.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id ADDF4C433B4 for ; Fri, 23 Apr 2021 20:49:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8DB48613B4 for ; Fri, 23 Apr 2021 20:49:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243951AbhDWUt4 (ORCPT ); Fri, 23 Apr 2021 16:49:56 -0400 Received: from relay3-d.mail.gandi.net ([217.70.183.195]:46501 "EHLO relay3-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232193AbhDWUtw (ORCPT ); Fri, 23 Apr 2021 16:49:52 -0400 X-Originating-IP: 2.7.49.219 Received: from [192.168.1.12] (lfbn-lyo-1-457-219.w2-7.abo.wanadoo.fr [2.7.49.219]) (Authenticated sender: alex@ghiti.fr) by relay3-d.mail.gandi.net (Postfix) with ESMTPSA id CAC7460003; Fri, 23 Apr 2021 20:49:10 +0000 (UTC) Subject: Re: [PATCH] riscv: Fix 32b kernel caused by 64b kernel mapping moving outside linear mapping To: Palmer Dabbelt Cc: anup@brainfault.org, corbet@lwn.net, Paul Walmsley , aou@eecs.berkeley.edu, Arnd Bergmann , aryabinin@virtuozzo.com, glider@google.com, dvyukov@google.com, linux-doc@vger.kernel.org, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com, linux-arch@vger.kernel.org, linux-mm@kvack.org References: From: Alex Ghiti Message-ID: <97819559-0af0-0422-5b6c-30872f759daa@ghiti.fr> Date: Fri, 23 Apr 2021 16:49:10 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: fr Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Le 4/23/21 à 12:57 PM, Palmer Dabbelt a écrit : > On Fri, 23 Apr 2021 01:34:02 PDT (-0700), alex@ghiti.fr wrote: >> Le 4/20/21 à 12:18 AM, Anup Patel a écrit : >>> On Sat, Apr 17, 2021 at 10:52 PM Alexandre Ghiti wrote: >>>> >>>> Fix multiple leftovers when moving the kernel mapping outside the >>>> linear >>>> mapping for 64b kernel that left the 32b kernel unusable. >>>> >>>> Fixes: 4b67f48da707 ("riscv: Move kernel mapping outside of linear >>>> mapping") >>>> Signed-off-by: Alexandre Ghiti >>> >>> Quite a few #ifdef but I don't see any better way at the moment. >>> Maybe we can >>> clean this later. Otherwise looks good to me. > > Agreed.  I'd recently sent out a patch set that got NACK'd because we're > supposed to be relying on the compiler to optimize away references that > can be staticly determined to not be exercised, which is probably the > way forward to getting rid of a lot of of preprocessor stuff.  That all > seems very fragile and is a bigger problem than this, though, so it's > probably best to do it as its own thing. > >>> Reviewed-by: Anup Patel >> >> Thanks Anup! >> >> @Palmer: This is not on for-next yet and then rv32 is broken. This does >> not apply immediately on top of for-next though, so if you need a new >> version, I can do that. But this squashes nicely with the patch it fixes >> if you prefer. > > Thanks.  I just hadn't gotten to this one yet, but as you pointed out > it's probably best to just squash it.  It's in the version on for-next > now, it caused few conflicts but I think I got everything sorted out. > > Now that everything is in I'm going to stop rewriting this stuff, as it > touches pretty much the whole tree.  I don't have much of a patch back > log as of right now, and as the new stuff will be on top of it that will > make everyone's lives easier. > >> >> Let me know, I can do that very quickly. >> >> Alex >> >>> >>> Regards, >>> Anup >>> >>>> --- >>>>   arch/riscv/include/asm/page.h    |  9 +++++++++ >>>>   arch/riscv/include/asm/pgtable.h | 16 ++++++++++++---- >>>>   arch/riscv/mm/init.c             | 25 ++++++++++++++++++++++++- >>>>   3 files changed, 45 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/arch/riscv/include/asm/page.h >>>> b/arch/riscv/include/asm/page.h >>>> index 22cfb2be60dc..f64b61296c0c 100644 >>>> --- a/arch/riscv/include/asm/page.h >>>> +++ b/arch/riscv/include/asm/page.h >>>> @@ -90,15 +90,20 @@ typedef struct page *pgtable_t; >>>> >>>>   #ifdef CONFIG_MMU >>>>   extern unsigned long va_pa_offset; >>>> +#ifdef CONFIG_64BIT >>>>   extern unsigned long va_kernel_pa_offset; >>>> +#endif >>>>   extern unsigned long pfn_base; >>>>   #define ARCH_PFN_OFFSET                (pfn_base) >>>>   #else >>>>   #define va_pa_offset           0 >>>> +#ifdef CONFIG_64BIT >>>>   #define va_kernel_pa_offset    0 >>>> +#endif >>>>   #define ARCH_PFN_OFFSET                (PAGE_OFFSET >> PAGE_SHIFT) >>>>   #endif /* CONFIG_MMU */ >>>> >>>> +#ifdef CONFIG_64BIT This one is incorrect as kernel_virt_addr is used also in 32b kernel, which causes 32b failure when CONFIG_DEBUG_VIRTUAL is set, the following diff fixes it: diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h index e280ba60cb34..6a7761c86ec2 100644 --- a/arch/riscv/include/asm/page.h +++ b/arch/riscv/include/asm/page.h @@ -106,9 +106,9 @@ extern unsigned long pfn_base; #define ARCH_PFN_OFFSET (PAGE_OFFSET >> PAGE_SHIFT) #endif /* CONFIG_MMU */ -#ifdef CONFIG_64BIT extern unsigned long kernel_virt_addr; +#ifdef CONFIG_64BIT #define linear_mapping_pa_to_va(x) ((void *)((unsigned long)(x) + va_pa_offset)) #ifdef CONFIG_XIP_KERNEL #define kernel_mapping_pa_to_va(y) ({ \ >>>>   extern unsigned long kernel_virt_addr; >>>> >>>>   #define linear_mapping_pa_to_va(x)     ((void *)((unsigned >>>> long)(x) + va_pa_offset)) >>>> @@ -112,6 +117,10 @@ extern unsigned long kernel_virt_addr; >>>>          (_x < kernel_virt_addr) >>>> ?                                               \ >>>>                  linear_mapping_va_to_pa(_x) : >>>> kernel_mapping_va_to_pa(_x);      \ >>>>          }) >>>> +#else >>>> +#define __pa_to_va_nodebug(x)  ((void *)((unsigned long) (x) + >>>> va_pa_offset)) >>>> +#define __va_to_pa_nodebug(x)  ((unsigned long)(x) - va_pa_offset) >>>> +#endif >>>> >>>>   #ifdef CONFIG_DEBUG_VIRTUAL >>>>   extern phys_addr_t __virt_to_phys(unsigned long x); >>>> diff --git a/arch/riscv/include/asm/pgtable.h >>>> b/arch/riscv/include/asm/pgtable.h >>>> index 80e63a93e903..5afda75cc2c3 100644 >>>> --- a/arch/riscv/include/asm/pgtable.h >>>> +++ b/arch/riscv/include/asm/pgtable.h >>>> @@ -16,19 +16,27 @@ >>>>   #else >>>> >>>>   #define ADDRESS_SPACE_END      (UL(-1)) >>>> -/* >>>> - * Leave 2GB for kernel and BPF at the end of the address space >>>> - */ >>>> + >>>> +#ifdef CONFIG_64BIT >>>> +/* Leave 2GB for kernel and BPF at the end of the address space */ >>>>   #define KERNEL_LINK_ADDR       (ADDRESS_SPACE_END - SZ_2G + 1) >>>> +#else >>>> +#define KERNEL_LINK_ADDR       PAGE_OFFSET >>>> +#endif >>>> >>>>   #define VMALLOC_SIZE     (KERN_VIRT_SIZE >> 1) >>>>   #define VMALLOC_END      (PAGE_OFFSET - 1) >>>>   #define VMALLOC_START    (PAGE_OFFSET - VMALLOC_SIZE) >>>> >>>> -/* KASLR should leave at least 128MB for BPF after the kernel */ >>>>   #define BPF_JIT_REGION_SIZE    (SZ_128M) >>>> +#ifdef CONFIG_64BIT >>>> +/* KASLR should leave at least 128MB for BPF after the kernel */ >>>>   #define BPF_JIT_REGION_START   PFN_ALIGN((unsigned long)&_end) >>>>   #define BPF_JIT_REGION_END     (BPF_JIT_REGION_START + >>>> BPF_JIT_REGION_SIZE) >>>> +#else >>>> +#define BPF_JIT_REGION_START   (PAGE_OFFSET - BPF_JIT_REGION_SIZE) >>>> +#define BPF_JIT_REGION_END     (VMALLOC_END) >>>> +#endif >>>> >>>>   /* Modules always live before the kernel */ >>>>   #ifdef CONFIG_64BIT >>>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c >>>> index 093f3a96ecfc..dc9b988e0778 100644 >>>> --- a/arch/riscv/mm/init.c >>>> +++ b/arch/riscv/mm/init.c >>>> @@ -91,8 +91,10 @@ static void print_vm_layout(void) >>>>                    (unsigned long)VMALLOC_END); >>>>          print_mlm("lowmem", (unsigned long)PAGE_OFFSET, >>>>                    (unsigned long)high_memory); >>>> +#ifdef CONFIG_64BIT >>>>          print_mlm("kernel", (unsigned long)KERNEL_LINK_ADDR, >>>>                    (unsigned long)ADDRESS_SPACE_END); >>>> +#endif >>>>   } >>>>   #else >>>>   static void print_vm_layout(void) { } >>>> @@ -165,9 +167,11 @@ static struct pt_alloc_ops pt_ops; >>>>   /* Offset between linear mapping virtual address and kernel load >>>> address */ >>>>   unsigned long va_pa_offset; >>>>   EXPORT_SYMBOL(va_pa_offset); >>>> +#ifdef CONFIG_64BIT >>>>   /* Offset between kernel mapping virtual address and kernel load >>>> address */ >>>>   unsigned long va_kernel_pa_offset; >>>>   EXPORT_SYMBOL(va_kernel_pa_offset); >>>> +#endif >>>>   unsigned long pfn_base; >>>>   EXPORT_SYMBOL(pfn_base); >>>> >>>> @@ -410,7 +414,9 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) >>>>          load_sz = (uintptr_t)(&_end) - load_pa; >>>> >>>>          va_pa_offset = PAGE_OFFSET - load_pa; >>>> +#ifdef CONFIG_64BIT >>>>          va_kernel_pa_offset = kernel_virt_addr - load_pa; >>>> +#endif >>>> >>>>          pfn_base = PFN_DOWN(load_pa); >>>> >>>> @@ -469,12 +475,16 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) >>>>                             pa + PMD_SIZE, PMD_SIZE, PAGE_KERNEL); >>>>          dtb_early_va = (void *)DTB_EARLY_BASE_VA + (dtb_pa & >>>> (PMD_SIZE - 1)); >>>>   #else /* CONFIG_BUILTIN_DTB */ >>>> +#ifdef CONFIG_64BIT >>>>          /* >>>>           * __va can't be used since it would return a linear >>>> mapping address >>>>           * whereas dtb_early_va will be used before setup_vm_final >>>> installs >>>>           * the linear mapping. >>>>           */ >>>>          dtb_early_va = kernel_mapping_pa_to_va(dtb_pa); >>>> +#else >>>> +       dtb_early_va = __va(dtb_pa); >>>> +#endif /* CONFIG_64BIT */ >>>>   #endif /* CONFIG_BUILTIN_DTB */ >>>>   #else >>>>   #ifndef CONFIG_BUILTIN_DTB >>>> @@ -486,7 +496,11 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) >>>>                             pa + PGDIR_SIZE, PGDIR_SIZE, PAGE_KERNEL); >>>>          dtb_early_va = (void *)DTB_EARLY_BASE_VA + (dtb_pa & >>>> (PGDIR_SIZE - 1)); >>>>   #else /* CONFIG_BUILTIN_DTB */ >>>> +#ifdef CONFIG_64BIT >>>>          dtb_early_va = kernel_mapping_pa_to_va(dtb_pa); >>>> +#else >>>> +       dtb_early_va = __va(dtb_pa); >>>> +#endif /* CONFIG_64BIT */ >>>>   #endif /* CONFIG_BUILTIN_DTB */ >>>>   #endif >>>>          dtb_early_pa = dtb_pa; >>>> @@ -571,12 +585,21 @@ static void __init setup_vm_final(void) >>>>                  for (pa = start; pa < end; pa += map_size) { >>>>                          va = (uintptr_t)__va(pa); >>>>                          create_pgd_mapping(swapper_pg_dir, va, pa, >>>> -                                          map_size, PAGE_KERNEL); >>>> +                                          map_size, >>>> +#ifdef CONFIG_64BIT >>>> +                                          PAGE_KERNEL >>>> +#else >>>> +                                          PAGE_KERNEL_EXEC >>>> +#endif >>>> +                                       ); >>>> + >>>>                  } >>>>          } >>>> >>>> +#ifdef CONFIG_64BIT >>>>          /* Map the kernel */ >>>>          create_kernel_page_table(swapper_pg_dir, PMD_SIZE); >>>> +#endif >>>> >>>>          /* Clear fixmap PTE and PMD mappings */ >>>>          clear_fixmap(FIX_PTE); >>>> -- >>>> 2.20.1 >>>> I agree with you, too much #ifdef, it is hardly readable: I take a look at how I can make it simpler. Sorry for all those fixes, Alex > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv