* 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 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 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: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: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 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 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
* 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
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.