* 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow @ 2019-03-22 6:16 Anup Patel 2019-03-22 10:42 ` Mike Rapoport 0 siblings, 1 reply; 17+ messages in thread From: Anup Patel @ 2019-03-22 6:16 UTC (permalink / raw) To: Palmer Dabbelt Cc: Damien Le Moal, Anup Patel, Mike Rapoport, Christoph Hellwig, Atish Patra, linux-riscv Hi Palmer, The 32bit kernel booting is broken for Linux-5.1-rc1 due to GCC cmodel=medlow affecting setup_vm() movement from kernel/setup.c to mm/init.c. There is no issue with 64bit kernel booting. The "[PATCH v2 2/5] RISC-V: Make setup_vm() independent of GCC code model" fixes this issue. https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1959102.html If possible please include above patch as Linux-5.1-rc1 fix. Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-22 6:16 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow Anup Patel @ 2019-03-22 10:42 ` Mike Rapoport 2019-03-22 12:29 ` Anup Patel 2019-03-22 13:25 ` Christoph Hellwig 0 siblings, 2 replies; 17+ messages in thread From: Mike Rapoport @ 2019-03-22 10:42 UTC (permalink / raw) To: Anup Patel Cc: Damien Le Moal, Anup Patel, Palmer Dabbelt, Christoph Hellwig, Atish Patra, linux-riscv Hi, On Fri, Mar 22, 2019 at 11:46:24AM +0530, Anup Patel wrote: > Hi Palmer, > > The 32bit kernel booting is broken for Linux-5.1-rc1 due to GCC cmodel=medlow > affecting setup_vm() movement from kernel/setup.c to mm/init.c. > > There is no issue with 64bit kernel booting. > > The "[PATCH v2 2/5] RISC-V: Make setup_vm() independent of GCC code model" > fixes this issue. > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1959102.html > > If possible please include above patch as Linux-5.1-rc1 fix. I'm not convinced that it's the best solution. Multiple __load_pa() and __load_va() conversions make code unreadable. Is there any reason swapper_pg_dir cannot be setup after 'relocate'? It'll save a lot of churn for the current fix and for the addition of 4K mappings I've drafted a patch that separates trampoline_pd_dir and swapper_pg_dir setup, if it works, the __load_pa() and __load_va() conversions can be applied only to the trampoline initialization. I don't have riscv hardware, so it's compile tested only. From d1f6f68012c84e188711954f58d63c0a3ba005c5 Mon Sep 17 00:00:00 2001 From: Mike Rapoport <rppt@linux.ibm.com> Date: Fri, 22 Mar 2019 12:34:31 +0200 Subject: [PATCH] riscv: split trampoline_pg_dir and swapper_pg_dir initialization Signed-off-by: Mike Rapoport <rppt@linux.ibm.com> --- arch/riscv/kernel/head.S | 7 ++----- arch/riscv/mm/init.c | 33 +++++++++++++++++++++++++-------- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S index fe884cd..fe0fc70 100644 --- a/arch/riscv/kernel/head.S +++ b/arch/riscv/kernel/head.S @@ -62,8 +62,9 @@ clear_bss_done: /* Initialize page tables and relocate to virtual addresses */ la sp, init_thread_union + THREAD_SIZE - call setup_vm + call setup_trampoline call relocate + call setup_vm /* Restore C environment */ la tp, init_task @@ -117,10 +118,6 @@ relocate: .option norelax la gp, __global_pointer$ .option pop - - /* Switch to kernel page tables */ - csrw sptbr, a2 - ret .Lsecondary_start: diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c index b379a75..1742763 100644 --- a/arch/riscv/mm/init.c +++ b/arch/riscv/mm/init.c @@ -187,11 +187,6 @@ asmlinkage void __init setup_vm(void) BUG_ON((pa % (PAGE_SIZE * PTRS_PER_PTE)) != 0); #ifndef __PAGETABLE_PMD_FOLDED - trampoline_pg_dir[(PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD] = - pfn_pgd(PFN_DOWN((uintptr_t)trampoline_pmd), - __pgprot(_PAGE_TABLE)); - trampoline_pmd[0] = pfn_pmd(PFN_DOWN(pa), prot); - for (i = 0; i < (-PAGE_OFFSET)/PGDIR_SIZE; ++i) { size_t o = (PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD + i; @@ -209,9 +204,6 @@ asmlinkage void __init setup_vm(void) pfn_pmd(PFN_DOWN((uintptr_t)fixmap_pte), __pgprot(_PAGE_TABLE)); #else - trampoline_pg_dir[(PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD] = - pfn_pgd(PFN_DOWN(pa), prot); - for (i = 0; i < (-PAGE_OFFSET)/PGDIR_SIZE; ++i) { size_t o = (PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD + i; @@ -223,4 +215,29 @@ asmlinkage void __init setup_vm(void) pfn_pgd(PFN_DOWN((uintptr_t)fixmap_pte), __pgprot(_PAGE_TABLE)); #endif + csr_write(sptbr, virt_to_pfn(swapper_pg_dir) | SATP_MODE); +} + +asmlinkage void __init setup_trampoline(void) +{ + extern char _start; + uintptr_t pa = (uintptr_t) &_start; + pgprot_t prot = __pgprot(pgprot_val(PAGE_KERNEL) | _PAGE_EXEC); + + va_pa_offset = PAGE_OFFSET - pa; + pfn_base = PFN_DOWN(pa); + + /* Sanity check alignment and size */ + BUG_ON((PAGE_OFFSET % PGDIR_SIZE) != 0); + BUG_ON((pa % (PAGE_SIZE * PTRS_PER_PTE)) != 0); + +#ifndef __PAGETABLE_PMD_FOLDED + trampoline_pg_dir[pgd_index(PAGE_OFFSET)] = + pfn_pgd(PFN_DOWN((uintptr_t)trampoline_pmd), + __pgprot(_PAGE_TABLE)); + trampoline_pmd[0] = pfn_pmd(PFN_DOWN(pa), prot); + +#else + trampoline_pg_dir[pgd_index(PAGE_OFFSET)] = pfn_pgd(PFN_DOWN(pa), prot); +#endif } -- 2.7.4 > Regards, > Anup > -- Sincerely yours, Mike. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-22 10:42 ` Mike Rapoport @ 2019-03-22 12:29 ` Anup Patel 2019-03-22 13:26 ` Christoph Hellwig 2019-03-22 13:25 ` Christoph Hellwig 1 sibling, 1 reply; 17+ messages in thread From: Anup Patel @ 2019-03-22 12:29 UTC (permalink / raw) To: Mike Rapoport Cc: Damien Le Moal, Anup Patel, Palmer Dabbelt, Christoph Hellwig, Atish Patra, linux-riscv On Fri, Mar 22, 2019 at 4:12 PM Mike Rapoport <rppt@linux.ibm.com> wrote: > > Hi, > > On Fri, Mar 22, 2019 at 11:46:24AM +0530, Anup Patel wrote: > > Hi Palmer, > > > > The 32bit kernel booting is broken for Linux-5.1-rc1 due to GCC cmodel=medlow > > affecting setup_vm() movement from kernel/setup.c to mm/init.c. > > > > There is no issue with 64bit kernel booting. > > > > The "[PATCH v2 2/5] RISC-V: Make setup_vm() independent of GCC code model" > > fixes this issue. > > > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1959102.html > > > > If possible please include above patch as Linux-5.1-rc1 fix. > > I'm not convinced that it's the best solution. Multiple __load_pa() and > __load_va() conversions make code unreadable. We have many __load_pa() and __load_va() in setup_vm() here because this patch only get's things working for 32bit RISC-V kernel. If you look at PATCH3 of my series then you will see that use of __load_pa() and __load_va() is simplified a lot and is easily readable. This was possible by rewriting the initial page table setup code. > Is there any reason swapper_pg_dir cannot be setup after 'relocate'? It'll > save a lot of churn for the current fix and for the addition of 4K mappings > > I've drafted a patch that separates trampoline_pd_dir and swapper_pg_dir > setup, if it works, the __load_pa() and __load_va() conversions can be > applied only to the trampoline initialization. I don't have riscv hardware, > so it's compile tested only. I had similar views as yours but I found that trampoling_pg_dir is totally redundant and since it is part of __init section it will break runtime hart hotplug in future. All mapping in trampoling_pg_dir are already covered by swapper_pg_dir. The PATCH4 in my v2 series removes trampoline_pg_dir. We just need to map vmlinux_start to vmlinux_end and DTB in setup_vm() that's all. Complete memory is only mapped in setup_vm_final(). Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-22 12:29 ` Anup Patel @ 2019-03-22 13:26 ` Christoph Hellwig 2019-03-22 13:38 ` Anup Patel 0 siblings, 1 reply; 17+ messages in thread From: Christoph Hellwig @ 2019-03-22 13:26 UTC (permalink / raw) To: Anup Patel Cc: Damien Le Moal, Anup Patel, Palmer Dabbelt, Mike Rapoport, Christoph Hellwig, Atish Patra, linux-riscv On Fri, Mar 22, 2019 at 05:59:07PM +0530, Anup Patel wrote: > > I've drafted a patch that separates trampoline_pd_dir and swapper_pg_dir > > setup, if it works, the __load_pa() and __load_va() conversions can be > > applied only to the trampoline initialization. I don't have riscv hardware, > > so it's compile tested only. > > I had similar views as yours but I found that trampoling_pg_dir is totally > redundant and since it is part of __init section it will break runtime hart > hotplug in future. All mapping in trampoling_pg_dir are already covered > by swapper_pg_dir. The PATCH4 in my v2 series removes trampoline_pg_dir. Can you please move that to the front of the series? Killing trampoline_pg_dir sounds very useful indeed, while I'm still not really convinced of the 4k mappings to be honest. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-22 13:26 ` Christoph Hellwig @ 2019-03-22 13:38 ` Anup Patel 0 siblings, 0 replies; 17+ messages in thread From: Anup Patel @ 2019-03-22 13:38 UTC (permalink / raw) To: Christoph Hellwig Cc: Damien Le Moal, Palmer Dabbelt, Anup Patel, Mike Rapoport, Atish Patra, linux-riscv On Fri, Mar 22, 2019 at 6:56 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Fri, Mar 22, 2019 at 05:59:07PM +0530, Anup Patel wrote: > > > I've drafted a patch that separates trampoline_pd_dir and swapper_pg_dir > > > setup, if it works, the __load_pa() and __load_va() conversions can be > > > applied only to the trampoline initialization. I don't have riscv hardware, > > > so it's compile tested only. > > > > I had similar views as yours but I found that trampoling_pg_dir is totally > > redundant and since it is part of __init section it will break runtime hart > > hotplug in future. All mapping in trampoling_pg_dir are already covered > > by swapper_pg_dir. The PATCH4 in my v2 series removes trampoline_pg_dir. > > Can you please move that to the front of the series? Killing > trampoline_pg_dir sounds very useful indeed, while I'm still not really > convinced of the 4k mappings to be honest. 4k aligned booting is optional and only enabled via kconfig option. Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-22 10:42 ` Mike Rapoport 2019-03-22 12:29 ` Anup Patel @ 2019-03-22 13:25 ` Christoph Hellwig 2019-03-22 13:37 ` Anup Patel 1 sibling, 1 reply; 17+ messages in thread From: Christoph Hellwig @ 2019-03-22 13:25 UTC (permalink / raw) To: Mike Rapoport Cc: Damien Le Moal, Anup Patel, Anup Patel, Palmer Dabbelt, Christoph Hellwig, Atish Patra, linux-riscv On Fri, Mar 22, 2019 at 12:42:10PM +0200, Mike Rapoport wrote: > Hi, > > On Fri, Mar 22, 2019 at 11:46:24AM +0530, Anup Patel wrote: > > Hi Palmer, > > > > The 32bit kernel booting is broken for Linux-5.1-rc1 due to GCC cmodel=medlow > > affecting setup_vm() movement from kernel/setup.c to mm/init.c. > > > > There is no issue with 64bit kernel booting. > > > > The "[PATCH v2 2/5] RISC-V: Make setup_vm() independent of GCC code model" > > fixes this issue. > > > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1959102.html > > > > If possible please include above patch as Linux-5.1-rc1 fix. > > I'm not convinced that it's the best solution. Multiple __load_pa() and > __load_va() conversions make code unreadable. > Is there any reason swapper_pg_dir cannot be setup after 'relocate'? It'll > save a lot of churn for the current fix and for the addition of 4K mappings I think for 5.1-rc we should just revert the cleanup patch ASAP until we figure out what else we want to do. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-22 13:25 ` Christoph Hellwig @ 2019-03-22 13:37 ` Anup Patel 2019-03-22 13:40 ` Christoph Hellwig 0 siblings, 1 reply; 17+ messages in thread From: Anup Patel @ 2019-03-22 13:37 UTC (permalink / raw) To: Christoph Hellwig Cc: Damien Le Moal, Palmer Dabbelt, Anup Patel, Mike Rapoport, Atish Patra, linux-riscv On Fri, Mar 22, 2019 at 6:55 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Fri, Mar 22, 2019 at 12:42:10PM +0200, Mike Rapoport wrote: > > Hi, > > > > On Fri, Mar 22, 2019 at 11:46:24AM +0530, Anup Patel wrote: > > > Hi Palmer, > > > > > > The 32bit kernel booting is broken for Linux-5.1-rc1 due to GCC cmodel=medlow > > > affecting setup_vm() movement from kernel/setup.c to mm/init.c. > > > > > > There is no issue with 64bit kernel booting. > > > > > > The "[PATCH v2 2/5] RISC-V: Make setup_vm() independent of GCC code model" > > > fixes this issue. > > > > > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1959102.html > > > > > > If possible please include above patch as Linux-5.1-rc1 fix. > > > > I'm not convinced that it's the best solution. Multiple __load_pa() and > > __load_va() conversions make code unreadable. > > Is there any reason swapper_pg_dir cannot be setup after 'relocate'? It'll > > save a lot of churn for the current fix and for the addition of 4K mappings > > I think for 5.1-rc we should just revert the cleanup patch ASAP until > we figure out what else we want to do. I think reverting is not right way to go here because there is deeper issue in using GCC cmodel=medlow with setup_vm() which is called from assembly with MMU off. Even if we keep setup_vm() in kernel/setup.c then still in-future it can break for 32bit system as more code gets added to kernel/setup.c. The setup_vm() should not be sensitive to which source file it is placed in. The patch which breaks 32bit kernel just moves setup_vm() from kernel/setup.c to mm/init.c. Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-22 13:37 ` Anup Patel @ 2019-03-22 13:40 ` Christoph Hellwig 2019-03-22 13:45 ` Anup Patel 0 siblings, 1 reply; 17+ messages in thread From: Christoph Hellwig @ 2019-03-22 13:40 UTC (permalink / raw) To: Anup Patel Cc: Damien Le Moal, Anup Patel, Palmer Dabbelt, Mike Rapoport, Christoph Hellwig, Atish Patra, linux-riscv On Fri, Mar 22, 2019 at 07:07:29PM +0530, Anup Patel wrote: > I think reverting is not right way to go here because there is deeper issue > in using GCC cmodel=medlow with setup_vm() which is called from assembly > with MMU off. > > Even if we keep setup_vm() in kernel/setup.c then still in-future it can break > for 32bit system as more code gets added to kernel/setup.c. > > The setup_vm() should not be sensitive to which source file it is placed in. > The patch which breaks 32bit kernel just moves setup_vm() from > kernel/setup.c to mm/init.c. I am not saying the revert is the final solution. I'm just saying that 5.1-rc is not the place we should be doing the major surgery. Revert fox 5.1, fix for real for 5.2+. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-22 13:40 ` Christoph Hellwig @ 2019-03-22 13:45 ` Anup Patel 2019-03-23 17:21 ` Christoph Hellwig 0 siblings, 1 reply; 17+ messages in thread From: Anup Patel @ 2019-03-22 13:45 UTC (permalink / raw) To: Christoph Hellwig Cc: Damien Le Moal, Palmer Dabbelt, Anup Patel, Mike Rapoport, Atish Patra, linux-riscv On Fri, Mar 22, 2019 at 7:10 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Fri, Mar 22, 2019 at 07:07:29PM +0530, Anup Patel wrote: > > I think reverting is not right way to go here because there is deeper issue > > in using GCC cmodel=medlow with setup_vm() which is called from assembly > > with MMU off. > > > > Even if we keep setup_vm() in kernel/setup.c then still in-future it can break > > for 32bit system as more code gets added to kernel/setup.c. > > > > The setup_vm() should not be sensitive to which source file it is placed in. > > The patch which breaks 32bit kernel just moves setup_vm() from > > kernel/setup.c to mm/init.c. > > I am not saying the revert is the final solution. I'm just saying > that 5.1-rc is not the place we should be doing the major surgery. > Revert fox 5.1, fix for real for 5.2+. If we revert for time being then even fixmap support has to be reverted. The patch only updates access to kernel symbols via __load_pa() and __load_va(). There is no functional change as such. The patch is tested on QEMU/virt 64bit, QEMU/virt 32bit, and SiFive Unleashed. Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-22 13:45 ` Anup Patel @ 2019-03-23 17:21 ` Christoph Hellwig 2019-03-24 1:42 ` Anup Patel 0 siblings, 1 reply; 17+ messages in thread From: Christoph Hellwig @ 2019-03-23 17:21 UTC (permalink / raw) To: Anup Patel Cc: Damien Le Moal, Anup Patel, Palmer Dabbelt, Mike Rapoport, Christoph Hellwig, Atish Patra, linux-riscv On Fri, Mar 22, 2019 at 07:15:37PM +0530, Anup Patel wrote: > > > > I am not saying the revert is the final solution. I'm just saying > > that 5.1-rc is not the place we should be doing the major surgery. > > Revert fox 5.1, fix for real for 5.2+. > > If we revert for time being then even fixmap support has to be > reverted. > > The patch only updates access to kernel symbols via __load_pa() > and __load_va(). There is no functional change as such. The patch > is tested on QEMU/virt 64bit, QEMU/virt 32bit, and SiFive Unleashed. Just curios - if you just move setup_vm and dependencies into a new file that is compiled with -mcmodel=medany, just like setup.c, what issues would we still have that require explicitly passing the load address? _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-23 17:21 ` Christoph Hellwig @ 2019-03-24 1:42 ` Anup Patel 2019-03-24 1:56 ` Gary Guo 2019-03-24 8:37 ` Christoph Hellwig 0 siblings, 2 replies; 17+ messages in thread From: Anup Patel @ 2019-03-24 1:42 UTC (permalink / raw) To: Christoph Hellwig Cc: Damien Le Moal, Palmer Dabbelt, Anup Patel, Mike Rapoport, Atish Patra, linux-riscv On Sat, Mar 23, 2019 at 10:51 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Fri, Mar 22, 2019 at 07:15:37PM +0530, Anup Patel wrote: > > > > > > I am not saying the revert is the final solution. I'm just saying > > > that 5.1-rc is not the place we should be doing the major surgery. > > > Revert fox 5.1, fix for real for 5.2+. > > > > If we revert for time being then even fixmap support has to be > > reverted. > > > > The patch only updates access to kernel symbols via __load_pa() > > and __load_va(). There is no functional change as such. The patch > > is tested on QEMU/virt 64bit, QEMU/virt 32bit, and SiFive Unleashed. > > Just curios - if you just move setup_vm and dependencies into a new > file that is compiled with -mcmodel=medany, just like setup.c, > what issues would we still have that require explicitly passing the > load address? There are four cases here: 1. 64bit with cmodel=medany (default for 64bit system) 2. 64bit with cmodel=medlow 3. 32bit with cmodel=medany 4. 32bit with cmodel=medlow (default for 32bit system) I had tried using cmodel=medany as default for both 64bit and 32bit system and it works fine because there is no issue with cmodel=medany but it is a very crude fix and things break again if someone explicitly selects cmodel=medlow using kconfig. The patch for using __load_va()/__load_pa() accessor in setup_vm() solves the problem for all four cases above. If you still think the patch is too big as rcX fix then alternate approach is: 1. For Linux-5.1-rcX, we use cmodel=medany by default for both 32bit and 64bit RISC-V (I can send a patch for this immediately) 2. Consider __load_va()/__load_pa() patch for Linux-5.2 Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-24 1:42 ` Anup Patel @ 2019-03-24 1:56 ` Gary Guo 2019-03-24 3:04 ` Anup Patel 2019-03-24 3:10 ` Anup Patel 2019-03-24 8:37 ` Christoph Hellwig 1 sibling, 2 replies; 17+ messages in thread From: Gary Guo @ 2019-03-24 1:56 UTC (permalink / raw) To: Anup Patel Cc: Damien Le Moal, Palmer Dabbelt, Anup Patel, Mike Rapoport, Christoph Hellwig, Atish Patra, linux-riscv I think what he means is to force the file containing setup_vm to be compiled using medany, even if cmodel is configured to be medlow. Regards, Gary > -----Original Message----- > From: linux-riscv <linux-riscv-bounces@lists.infradead.org> On Behalf Of Anup > Patel > Sent: Sunday, March 24, 2019 01:42 > To: Christoph Hellwig <hch@infradead.org> > Cc: Damien Le Moal <Damien.LeMoal@wdc.com>; Palmer Dabbelt > <palmer@sifive.com>; Anup Patel <anup.patel@wdc.com>; Mike Rapoport > <rppt@linux.ibm.com>; Atish Patra <atish.patra@wdc.com>; linux- > riscv@lists.infradead.org > Subject: Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow > > On Sat, Mar 23, 2019 at 10:51 PM Christoph Hellwig <hch@infradead.org> > wrote: > > > > On Fri, Mar 22, 2019 at 07:15:37PM +0530, Anup Patel wrote: > > > > > > > > I am not saying the revert is the final solution. I'm just saying > > > > that 5.1-rc is not the place we should be doing the major surgery. > > > > Revert fox 5.1, fix for real for 5.2+. > > > > > > If we revert for time being then even fixmap support has to be > > > reverted. > > > > > > The patch only updates access to kernel symbols via __load_pa() and > > > __load_va(). There is no functional change as such. The patch is > > > tested on QEMU/virt 64bit, QEMU/virt 32bit, and SiFive Unleashed. > > > > Just curios - if you just move setup_vm and dependencies into a new > > file that is compiled with -mcmodel=medany, just like setup.c, what > > issues would we still have that require explicitly passing the load > > address? > > There are four cases here: > 1. 64bit with cmodel=medany (default for 64bit system) 2. 64bit with > cmodel=medlow 3. 32bit with cmodel=medany 4. 32bit with cmodel=medlow > (default for 32bit system) > > I had tried using cmodel=medany as default for both 64bit and 32bit system and > it works fine because there is no issue with cmodel=medany but it is a very crude > fix and things break again if someone explicitly selects cmodel=medlow using > kconfig. > > The patch for using __load_va()/__load_pa() accessor in setup_vm() solves the > problem for all four cases above. > > If you still think the patch is too big as rcX fix then alternate approach is: > 1. For Linux-5.1-rcX, we use cmodel=medany by default for both 32bit and 64bit > RISC-V (I can send a patch for this immediately) 2. Consider > __load_va()/__load_pa() patch for Linux-5.2 > > Regards, > Anup > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-24 1:56 ` Gary Guo @ 2019-03-24 3:04 ` Anup Patel 2019-03-24 3:10 ` Anup Patel 1 sibling, 0 replies; 17+ messages in thread From: Anup Patel @ 2019-03-24 3:04 UTC (permalink / raw) To: Gary Guo, Anup Patel Cc: Damien Le Moal, Palmer Dabbelt, Mike Rapoport, Christoph Hellwig, Atish Patra, linux-riscv > -----Original Message----- > From: Gary Guo <gary@garyguo.net> > Sent: Sunday, March 24, 2019 7:26 AM > To: Anup Patel <anup@brainfault.org> > Cc: Christoph Hellwig <hch@infradead.org>; Damien Le Moal > <Damien.LeMoal@wdc.com>; Palmer Dabbelt <palmer@sifive.com>; Anup > Patel <Anup.Patel@wdc.com>; Mike Rapoport <rppt@linux.ibm.com>; Atish > Patra <Atish.Patra@wdc.com>; linux-riscv@lists.infradead.org > Subject: RE: 32bit kernel is broken for Linux-5.1-rc1 due to GCC > cmodel=medlow > > I think what he means is to force the file containing setup_vm to be compiled > using medany, even if cmodel is configured to be medlow. That's not possible because code model is not a GCC function attribute. Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-24 1:56 ` Gary Guo 2019-03-24 3:04 ` Anup Patel @ 2019-03-24 3:10 ` Anup Patel 2019-03-24 6:05 ` Mike Rapoport 1 sibling, 1 reply; 17+ messages in thread From: Anup Patel @ 2019-03-24 3:10 UTC (permalink / raw) To: Gary Guo Cc: Damien Le Moal, Palmer Dabbelt, Anup Patel, Mike Rapoport, Christoph Hellwig, Atish Patra, linux-riscv On Sun, Mar 24, 2019 at 7:26 AM Gary Guo <gary@garyguo.net> wrote: > > I think what he means is to force the file containing setup_vm to be compiled using medany, even if cmodel is configured to be medlow. Ahh, sorry I misread previously. I have not explored having separate file for just setup_vm() but I am sure we cannot link two object files different code model and it will cause linker error. Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-24 3:10 ` Anup Patel @ 2019-03-24 6:05 ` Mike Rapoport 2019-03-24 9:41 ` Anup Patel 0 siblings, 1 reply; 17+ messages in thread From: Mike Rapoport @ 2019-03-24 6:05 UTC (permalink / raw) To: Anup Patel Cc: Damien Le Moal, Palmer Dabbelt, Anup Patel, Christoph Hellwig, Atish Patra, Gary Guo, linux-riscv On Sun, Mar 24, 2019 at 08:40:35AM +0530, Anup Patel wrote: > On Sun, Mar 24, 2019 at 7:26 AM Gary Guo <gary@garyguo.net> wrote: > > > > I think what he means is to force the file containing setup_vm to be compiled using medany, even if cmodel is configured to be medlow. > > Ahh, sorry I misread previously. > > I have not explored having separate file for just setup_vm() but I am sure > we cannot link two object files different code model and it will cause linker > error. Currently kernel/setup.c is always compiled with cmodel=medany: CFLAGS_setup.o := -mcmodel=medany I suppose that's what made it possible to have setup_vm() properly access global variables with relative addressing. Now when setup_vm() has been moved to mm/init.c, it's compiled with the cmodel chosen by the kernel configuration. AFAIU Christoph's suggestion was to move load_pa_offset and pfn_base to mm/init.c and add CFLAGS_init.o := -mcmodel=medany to mm/Makefile. > Regards, > Anup > -- Sincerely yours, Mike. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-24 6:05 ` Mike Rapoport @ 2019-03-24 9:41 ` Anup Patel 0 siblings, 0 replies; 17+ messages in thread From: Anup Patel @ 2019-03-24 9:41 UTC (permalink / raw) To: Mike Rapoport Cc: Damien Le Moal, Palmer Dabbelt, Anup Patel, Christoph Hellwig, Atish Patra, Gary Guo, linux-riscv On Sun, Mar 24, 2019 at 11:35 AM Mike Rapoport <rppt@linux.ibm.com> wrote: > > On Sun, Mar 24, 2019 at 08:40:35AM +0530, Anup Patel wrote: > > On Sun, Mar 24, 2019 at 7:26 AM Gary Guo <gary@garyguo.net> wrote: > > > > > > I think what he means is to force the file containing setup_vm to be compiled using medany, even if cmodel is configured to be medlow. > > > > Ahh, sorry I misread previously. > > > > I have not explored having separate file for just setup_vm() but I am sure > > we cannot link two object files different code model and it will cause linker > > error. > > Currently kernel/setup.c is always compiled with cmodel=medany: > > CFLAGS_setup.o := -mcmodel=medany > > I suppose that's what made it possible to have setup_vm() properly access > global variables with relative addressing. > > Now when setup_vm() has been moved to mm/init.c, it's compiled with the > cmodel chosen by the kernel configuration. > > AFAIU Christoph's suggestion was to move load_pa_offset and pfn_base to > mm/init.c and add > > CFLAGS_init.o := -mcmodel=medany > > to mm/Makefile. Cool, this looks good to me. I will try this and send RC fix soon. Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow 2019-03-24 1:42 ` Anup Patel 2019-03-24 1:56 ` Gary Guo @ 2019-03-24 8:37 ` Christoph Hellwig 1 sibling, 0 replies; 17+ messages in thread From: Christoph Hellwig @ 2019-03-24 8:37 UTC (permalink / raw) To: Anup Patel Cc: Damien Le Moal, Anup Patel, Palmer Dabbelt, Mike Rapoport, Christoph Hellwig, Atish Patra, linux-riscv On Sun, Mar 24, 2019 at 07:12:07AM +0530, Anup Patel wrote: > > Just curios - if you just move setup_vm and dependencies into a new > > file that is compiled with -mcmodel=medany, just like setup.c, > > what issues would we still have that require explicitly passing the > > load address? > > There are four cases here: > 1. 64bit with cmodel=medany (default for 64bit system) > 2. 64bit with cmodel=medlow > 3. 32bit with cmodel=medany > 4. 32bit with cmodel=medlow (default for 32bit system) > > I had tried using cmodel=medany as default for both 64bit and 32bit system > and it works fine because there is no issue with cmodel=medany but it is a very > crude fix and things break again if someone explicitly selects cmodel=medlow > using kconfig. Well. What I meant is to override the model just for the new file containing setup_vm, just like we explicitly override the model for setup.c in arch/riscv/kernel/Makefile: CFLAGS_setup.o := -mcmodel=medany _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2019-03-24 9:41 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-22 6:16 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow Anup Patel 2019-03-22 10:42 ` Mike Rapoport 2019-03-22 12:29 ` Anup Patel 2019-03-22 13:26 ` Christoph Hellwig 2019-03-22 13:38 ` Anup Patel 2019-03-22 13:25 ` Christoph Hellwig 2019-03-22 13:37 ` Anup Patel 2019-03-22 13:40 ` Christoph Hellwig 2019-03-22 13:45 ` Anup Patel 2019-03-23 17:21 ` Christoph Hellwig 2019-03-24 1:42 ` Anup Patel 2019-03-24 1:56 ` Gary Guo 2019-03-24 3:04 ` Anup Patel 2019-03-24 3:10 ` Anup Patel 2019-03-24 6:05 ` Mike Rapoport 2019-03-24 9:41 ` Anup Patel 2019-03-24 8:37 ` Christoph Hellwig
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.