32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Anup Patel]

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

Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Mike Rapoport]

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

Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Anup Patel]

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

Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Christoph Hellwig]

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

Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Christoph Hellwig]

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

Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Anup Patel]

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

Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Anup Patel]

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

Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Christoph Hellwig]

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

Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Anup Patel]

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

Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Christoph Hellwig]

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

Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Anup Patel]

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

RE: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Gary Guo]

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

RE: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Anup Patel]

> -----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

Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Anup Patel]

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

Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Mike Rapoport]

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

Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Christoph Hellwig]

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

Re: 32bit kernel is broken for Linux-5.1-rc1 due to GCC cmodel=medlow

[Anup Patel]

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