[PATCH v2] RISC-V: Always compile mm/init.c with cmodel=medany

[PATCH v2] RISC-V: Always compile mm/init.c with cmodel=medany

[Anup Patel]

The Linux RISC-V 32bit kernel is broken after we moved setup_vm() from
kernel/setup.c to mm/init.c because Linux RISC-V 32bit kernel by default
uses cmodel=medlow which results in a non-position-independent setup_vm().

This patch fixes Linux RISC-V 32bit kernel booting by:
1. Forcing cmodel=medany for mm/init.c
2. Moving remaing MM-related stuff va_pa_offset, pfn_base and
   empty_zero_page from kernel/setup.c to mm/init.c

Fixes: 6f1e9e946f0b ("RISC-V: Move setup_vm() to mm/init.c")
Suggested-by: Christoph Hellwig <hch@lst.de>
Suggested-by: Mike Rapoport <rppt@linux.ibm.com>
Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
v2: Removed CFLAGS_setup.o from kernel/Makefile and replaced SoBs
---
 arch/riscv/kernel/Makefile | 2 --
 arch/riscv/kernel/setup.c  | 8 --------
 arch/riscv/mm/Makefile     | 2 ++
 arch/riscv/mm/init.c       | 9 +++++++++
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index f13f7f276639..8b9780b6bd7f 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -29,8 +29,6 @@ obj-y	+= vdso.o
 obj-y	+= cacheinfo.o
 obj-y	+= vdso/
 
-CFLAGS_setup.o := -mcmodel=medany
-
 obj-$(CONFIG_FPU)		+= fpu.o
 obj-$(CONFIG_SMP)		+= smpboot.o
 obj-$(CONFIG_SMP)		+= smp.o
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index ecb654f6a79e..540a331d1376 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -48,14 +48,6 @@ struct screen_info screen_info = {
 };
 #endif
 
-unsigned long va_pa_offset;
-EXPORT_SYMBOL(va_pa_offset);
-unsigned long pfn_base;
-EXPORT_SYMBOL(pfn_base);
-
-unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)] __page_aligned_bss;
-EXPORT_SYMBOL(empty_zero_page);
-
 /* The lucky hart to first increment this variable will boot the other cores */
 atomic_t hart_lottery;
 unsigned long boot_cpu_hartid;
diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
index eb22ab49b3e0..7307609d405b 100644
--- a/arch/riscv/mm/Makefile
+++ b/arch/riscv/mm/Makefile
@@ -3,3 +3,5 @@ obj-y += fault.o
 obj-y += extable.o
 obj-y += ioremap.o
 obj-y += cacheflush.o
+
+CFLAGS_init.o := -mcmodel=medany
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index b379a75ac6a6..7a7c454378cb 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -25,6 +25,10 @@
 #include <asm/pgtable.h>
 #include <asm/io.h>
 
+unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)]
+							__page_aligned_bss;
+EXPORT_SYMBOL(empty_zero_page);
+
 static void __init zone_sizes_init(void)
 {
 	unsigned long max_zone_pfns[MAX_NR_ZONES] = { 0, };
@@ -143,6 +147,11 @@ void __init setup_bootmem(void)
 	}
 }
 
+unsigned long va_pa_offset;
+EXPORT_SYMBOL(va_pa_offset);
+unsigned long pfn_base;
+EXPORT_SYMBOL(pfn_base);
+
 pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
 pgd_t trampoline_pg_dir[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
 
-- 
2.17.1

Re: [PATCH v2] RISC-V: Always compile mm/init.c with cmodel=medany

[Alan Kao]

Hi Anup,

Sorry for being late to the party.  I think one more thing should
move together with setup_vm():

On Mon, Mar 25, 2019 at 03:37:38AM +0000, Anup Patel wrote:
> The Linux RISC-V 32bit kernel is broken after we moved setup_vm() from
> kernel/setup.c to mm/init.c because Linux RISC-V 32bit kernel by default
> uses cmodel=medlow which results in a non-position-independent setup_vm().
> 
> This patch fixes Linux RISC-V 32bit kernel booting by:
> 1. Forcing cmodel=medany for mm/init.c
> 2. Moving remaing MM-related stuff va_pa_offset, pfn_base and
>    empty_zero_page from kernel/setup.c to mm/init.c
> 
> Fixes: 6f1e9e946f0b ("RISC-V: Move setup_vm() to mm/init.c")
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Suggested-by: Mike Rapoport <rppt@linux.ibm.com>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
> v2: Removed CFLAGS_setup.o from kernel/Makefile and replaced SoBs
> ---
>  arch/riscv/kernel/Makefile | 2 --
>  arch/riscv/kernel/setup.c  | 8 --------
>  arch/riscv/mm/Makefile     | 2 ++
>  arch/riscv/mm/init.c       | 9 +++++++++
>  4 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index f13f7f276639..8b9780b6bd7f 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile

earlier in this file, there are four lines about ftrace,

  5 ifdef CONFIG_FTRACE
  6 CFLAGS_REMOVE_ftrace.o = -pg
  7 CFLAGS_REMOVE_setup.o = -pg
  8 endif

removing "-pg" flag from setup.o was necessary for ftrace to work, since
setup_vm() cannot process the trampoline of ftrace properly.

As setup_vm() is being moved to mm/init.c, you may either mark it with a
"notrace" attribute with its declaration, or adding corresponding CFLAGS_REMOVE*
to mm/Makefile.

> @@ -29,8 +29,6 @@ obj-y	+= vdso.o
>  obj-y	+= cacheinfo.o
>  obj-y	+= vdso/
>  
> -CFLAGS_setup.o := -mcmodel=medany
> -
>  obj-$(CONFIG_FPU)		+= fpu.o
>  obj-$(CONFIG_SMP)		+= smpboot.o
>  obj-$(CONFIG_SMP)		+= smp.o
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index ecb654f6a79e..540a331d1376 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -48,14 +48,6 @@ struct screen_info screen_info = {
>  };
>  #endif
>  
> -unsigned long va_pa_offset;
> -EXPORT_SYMBOL(va_pa_offset);
> -unsigned long pfn_base;
> -EXPORT_SYMBOL(pfn_base);
> -
> -unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)] __page_aligned_bss;
> -EXPORT_SYMBOL(empty_zero_page);
> -
>  /* The lucky hart to first increment this variable will boot the other cores */
>  atomic_t hart_lottery;
>  unsigned long boot_cpu_hartid;
> diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
> index eb22ab49b3e0..7307609d405b 100644
> --- a/arch/riscv/mm/Makefile
> +++ b/arch/riscv/mm/Makefile
> @@ -3,3 +3,5 @@ obj-y += fault.o
>  obj-y += extable.o
>  obj-y += ioremap.o
>  obj-y += cacheflush.o
> +
> +CFLAGS_init.o := -mcmodel=medany
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index b379a75ac6a6..7a7c454378cb 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -25,6 +25,10 @@
>  #include <asm/pgtable.h>
>  #include <asm/io.h>
>  
> +unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)]
> +							__page_aligned_bss;
> +EXPORT_SYMBOL(empty_zero_page);
> +
>  static void __init zone_sizes_init(void)
>  {
>  	unsigned long max_zone_pfns[MAX_NR_ZONES] = { 0, };
> @@ -143,6 +147,11 @@ void __init setup_bootmem(void)
>  	}
>  }
>  
> +unsigned long va_pa_offset;
> +EXPORT_SYMBOL(va_pa_offset);
> +unsigned long pfn_base;
> +EXPORT_SYMBOL(pfn_base);
> +
>  pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
>  pgd_t trampoline_pg_dir[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
>  
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Alan

Re: [PATCH v2] RISC-V: Always compile mm/init.c with cmodel=medany

[Mike Rapoport]

On Mon, Mar 25, 2019 at 03:37:38AM +0000, Anup Patel wrote:
> The Linux RISC-V 32bit kernel is broken after we moved setup_vm() from
> kernel/setup.c to mm/init.c because Linux RISC-V 32bit kernel by default
> uses cmodel=medlow which results in a non-position-independent setup_vm().
> 
> This patch fixes Linux RISC-V 32bit kernel booting by:
> 1. Forcing cmodel=medany for mm/init.c
> 2. Moving remaing MM-related stuff va_pa_offset, pfn_base and
>    empty_zero_page from kernel/setup.c to mm/init.c
> 
> Fixes: 6f1e9e946f0b ("RISC-V: Move setup_vm() to mm/init.c")
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Suggested-by: Mike Rapoport <rppt@linux.ibm.com>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>

Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>

> ---
> v2: Removed CFLAGS_setup.o from kernel/Makefile and replaced SoBs
> ---
>  arch/riscv/kernel/Makefile | 2 --
>  arch/riscv/kernel/setup.c  | 8 --------
>  arch/riscv/mm/Makefile     | 2 ++
>  arch/riscv/mm/init.c       | 9 +++++++++
>  4 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index f13f7f276639..8b9780b6bd7f 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -29,8 +29,6 @@ obj-y	+= vdso.o
>  obj-y	+= cacheinfo.o
>  obj-y	+= vdso/
>  
> -CFLAGS_setup.o := -mcmodel=medany
> -
>  obj-$(CONFIG_FPU)		+= fpu.o
>  obj-$(CONFIG_SMP)		+= smpboot.o
>  obj-$(CONFIG_SMP)		+= smp.o
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index ecb654f6a79e..540a331d1376 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -48,14 +48,6 @@ struct screen_info screen_info = {
>  };
>  #endif
>  
> -unsigned long va_pa_offset;
> -EXPORT_SYMBOL(va_pa_offset);
> -unsigned long pfn_base;
> -EXPORT_SYMBOL(pfn_base);
> -
> -unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)] __page_aligned_bss;
> -EXPORT_SYMBOL(empty_zero_page);
> -
>  /* The lucky hart to first increment this variable will boot the other cores */
>  atomic_t hart_lottery;
>  unsigned long boot_cpu_hartid;
> diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
> index eb22ab49b3e0..7307609d405b 100644
> --- a/arch/riscv/mm/Makefile
> +++ b/arch/riscv/mm/Makefile
> @@ -3,3 +3,5 @@ obj-y += fault.o
>  obj-y += extable.o
>  obj-y += ioremap.o
>  obj-y += cacheflush.o
> +
> +CFLAGS_init.o := -mcmodel=medany
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index b379a75ac6a6..7a7c454378cb 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -25,6 +25,10 @@
>  #include <asm/pgtable.h>
>  #include <asm/io.h>
>  
> +unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)]
> +							__page_aligned_bss;
> +EXPORT_SYMBOL(empty_zero_page);
> +
>  static void __init zone_sizes_init(void)
>  {
>  	unsigned long max_zone_pfns[MAX_NR_ZONES] = { 0, };
> @@ -143,6 +147,11 @@ void __init setup_bootmem(void)
>  	}
>  }
>  
> +unsigned long va_pa_offset;
> +EXPORT_SYMBOL(va_pa_offset);
> +unsigned long pfn_base;
> +EXPORT_SYMBOL(pfn_base);
> +
>  pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
>  pgd_t trampoline_pg_dir[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
>  
> -- 
> 2.17.1
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH v2] RISC-V: Always compile mm/init.c with cmodel=medany

[Christoph Hellwig]

On Mon, Mar 25, 2019 at 01:25:50PM +0800, Alan Kao wrote:
> Hi Anup,
> 
> Sorry for being late to the party.  I think one more thing should
> move together with setup_vm():

Ah, I wonded about that yesterday but wasn't sure.  Maybe notrace
is a little cleaner?  Either way we should probably document both
the mcmodel and notrace assumptions in source comments for the next
person touching this code.

Re: [PATCH v2] RISC-V: Always compile mm/init.c with cmodel=medany

[Anup Patel]

On Mon, Mar 25, 2019 at 10:56 AM Alan Kao <alankao@andestech.com> wrote:
>
> Hi Anup,
>
> Sorry for being late to the party.  I think one more thing should
> move together with setup_vm():
>
> On Mon, Mar 25, 2019 at 03:37:38AM +0000, Anup Patel wrote:
> > The Linux RISC-V 32bit kernel is broken after we moved setup_vm() from
> > kernel/setup.c to mm/init.c because Linux RISC-V 32bit kernel by default
> > uses cmodel=medlow which results in a non-position-independent setup_vm().
> >
> > This patch fixes Linux RISC-V 32bit kernel booting by:
> > 1. Forcing cmodel=medany for mm/init.c
> > 2. Moving remaing MM-related stuff va_pa_offset, pfn_base and
> >    empty_zero_page from kernel/setup.c to mm/init.c
> >
> > Fixes: 6f1e9e946f0b ("RISC-V: Move setup_vm() to mm/init.c")
> > Suggested-by: Christoph Hellwig <hch@lst.de>
> > Suggested-by: Mike Rapoport <rppt@linux.ibm.com>
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > ---
> > v2: Removed CFLAGS_setup.o from kernel/Makefile and replaced SoBs
> > ---
> >  arch/riscv/kernel/Makefile | 2 --
> >  arch/riscv/kernel/setup.c  | 8 --------
> >  arch/riscv/mm/Makefile     | 2 ++
> >  arch/riscv/mm/init.c       | 9 +++++++++
> >  4 files changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> > index f13f7f276639..8b9780b6bd7f 100644
> > --- a/arch/riscv/kernel/Makefile
> > +++ b/arch/riscv/kernel/Makefile
>
> earlier in this file, there are four lines about ftrace,
>
>   5 ifdef CONFIG_FTRACE
>   6 CFLAGS_REMOVE_ftrace.o = -pg
>   7 CFLAGS_REMOVE_setup.o = -pg
>   8 endif
>
> removing "-pg" flag from setup.o was necessary for ftrace to work, since
> setup_vm() cannot process the trampoline of ftrace properly.
>
> As setup_vm() is being moved to mm/init.c, you may either mark it with a
> "notrace" attribute with its declaration, or adding corresponding CFLAGS_REMOVE*
> to mm/Makefile.

Let's handle it in same way as it was handled for kernel/setup.o

Most of the stuff is already moved to mm/init.c so no need for
"CFLAGS_REMOVE_setup.o = -pg"

I will send-out v3 with above changes and also update patch description
accordingly.

Regards,
Anup

Re: [PATCH v2] RISC-V: Always compile mm/init.c with cmodel=medany

[Anup Patel]

On Mon, Mar 25, 2019 at 12:18 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Mar 25, 2019 at 01:25:50PM +0800, Alan Kao wrote:
> > Hi Anup,
> >
> > Sorry for being late to the party.  I think one more thing should
> > move together with setup_vm():
>
> Ah, I wonded about that yesterday but wasn't sure.  Maybe notrace
> is a little cleaner?  Either way we should probably document both
> the mcmodel and notrace assumptions in source comments for the next
> person touching this code.

The setup_vm() should be allowed to call other functions within mm/init.c
so let's go with file-level notrace (just like how it was done) for
kernel/setup.c

I certainly add comments for setup_vm() based on all our findings so far.

Regards,
Anup

Re: [PATCH v2] RISC-V: Always compile mm/init.c with cmodel=medany

[Palmer Dabbelt]

On Mon, 25 Mar 2019 00:01:45 PDT (-0700), anup@brainfault.org wrote:
> On Mon, Mar 25, 2019 at 12:18 PM Christoph Hellwig <hch@infradead.org> wrote:
>>
>> On Mon, Mar 25, 2019 at 01:25:50PM +0800, Alan Kao wrote:
>> > Hi Anup,
>> >
>> > Sorry for being late to the party.  I think one more thing should
>> > move together with setup_vm():
>>
>> Ah, I wonded about that yesterday but wasn't sure.  Maybe notrace
>> is a little cleaner?  Either way we should probably document both
>> the mcmodel and notrace assumptions in source comments for the next
>> person touching this code.
>
> The setup_vm() should be allowed to call other functions within mm/init.c
> so let's go with file-level notrace (just like how it was done) for
> kernel/setup.c
>
> I certainly add comments for setup_vm() based on all our findings so far.

Sorry for being slow here, but this is the right approach: setup_vm is called
before relocate, which means the page tables won't be set up correctly for
absolute addressing.  We instead build setup_vm with medany, which causes all
addressing to be PC-relative.  This is all a bit of a hack, but it's the only
way we have to do this right now.

You should be able to add a preprocessor #error to check the code model with 
something like this

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index b379a75ac6a6..d6fde6af8d75 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -172,6 +172,9 @@ void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot)
        }
 }

+#ifndef __riscv_cmodel_medany
+#error "setup_vm() is called from head.S before relocate and must not make any absolute references."
+#endif
 asmlinkage void __init setup_vm(void)
 {
        extern char _start;

Marking this notrace is the right thing to do, as it can't call into any 
functions that aren't medany (there's probably other issues as well, since this 
is so early).

Sorry I missed this the first time around, I wasn't paying enough attention.

Can someone add instructions for 32-bit boots to the QEMU wiki?  It sounds like 
it's time to add that to the testing list...

Thanks for digging in to this!

>
> Regards,
> Anup

Re: [PATCH v2] RISC-V: Always compile mm/init.c with cmodel=medany

[Anup Patel]

On Tue, Mar 26, 2019 at 7:52 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Mon, 25 Mar 2019 00:01:45 PDT (-0700), anup@brainfault.org wrote:
> > On Mon, Mar 25, 2019 at 12:18 PM Christoph Hellwig <hch@infradead.org> wrote:
> >>
> >> On Mon, Mar 25, 2019 at 01:25:50PM +0800, Alan Kao wrote:
> >> > Hi Anup,
> >> >
> >> > Sorry for being late to the party.  I think one more thing should
> >> > move together with setup_vm():
> >>
> >> Ah, I wonded about that yesterday but wasn't sure.  Maybe notrace
> >> is a little cleaner?  Either way we should probably document both
> >> the mcmodel and notrace assumptions in source comments for the next
> >> person touching this code.
> >
> > The setup_vm() should be allowed to call other functions within mm/init.c
> > so let's go with file-level notrace (just like how it was done) for
> > kernel/setup.c
> >
> > I certainly add comments for setup_vm() based on all our findings so far.
>
> Sorry for being slow here, but this is the right approach: setup_vm is called
> before relocate, which means the page tables won't be set up correctly for
> absolute addressing.  We instead build setup_vm with medany, which causes all
> addressing to be PC-relative.  This is all a bit of a hack, but it's the only
> way we have to do this right now.
>
> You should be able to add a preprocessor #error to check the code model with
> something like this
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index b379a75ac6a6..d6fde6af8d75 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -172,6 +172,9 @@ void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot)
>         }
>  }
>
> +#ifndef __riscv_cmodel_medany
> +#error "setup_vm() is called from head.S before relocate and must not make any absolute references."
> +#endif
>  asmlinkage void __init setup_vm(void)
>  {
>         extern char _start;
>
> Marking this notrace is the right thing to do, as it can't call into any
> functions that aren't medany (there's probably other issues as well, since this
> is so early).

Thanks for the suggestion, I will add "#error" in v4 of this patch.

>
> Sorry I missed this the first time around, I wasn't paying enough attention.
>
> Can someone add instructions for 32-bit boots to the QEMU wiki?  It sounds like
> it's time to add that to the testing list...

To start with, I have send a patch for adding rv32_defconfig which is right now
"defconfig" plus "CONFIG_ARCH_RV32I=y"

Regards,
Anup

Re: [PATCH v2] RISC-V: Always compile mm/init.c with cmodel=medany

[Alistair Francis]

On Mon, 2019-03-25 at 19:22 -0700, Palmer Dabbelt wrote:
> On Mon, 25 Mar 2019 00:01:45 PDT (-0700), anup@brainfault.org wrote:
> > On Mon, Mar 25, 2019 at 12:18 PM Christoph Hellwig <
> > hch@infradead.org> wrote:
> > > On Mon, Mar 25, 2019 at 01:25:50PM +0800, Alan Kao wrote:
> > > > Hi Anup,
> > > > 
> > > > Sorry for being late to the party.  I think one more thing
> > > > should
> > > > move together with setup_vm():
> > > 
> > > Ah, I wonded about that yesterday but wasn't sure.  Maybe notrace
> > > is a little cleaner?  Either way we should probably document both
> > > the mcmodel and notrace assumptions in source comments for the
> > > next
> > > person touching this code.
> > 
> > The setup_vm() should be allowed to call other functions within
> > mm/init.c
> > so let's go with file-level notrace (just like how it was done) for
> > kernel/setup.c
> > 
> > I certainly add comments for setup_vm() based on all our findings
> > so far.
> 
> Sorry for being slow here, but this is the right approach: setup_vm
> is called
> before relocate, which means the page tables won't be set up
> correctly for
> absolute addressing.  We instead build setup_vm with medany, which
> causes all
> addressing to be PC-relative.  This is all a bit of a hack, but it's
> the only
> way we have to do this right now.
> 
> You should be able to add a preprocessor #error to check the code
> model with 
> something like this
> 
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index b379a75ac6a6..d6fde6af8d75 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -172,6 +172,9 @@ void __set_fixmap(enum fixed_addresses idx,
> phys_addr_t phys, pgprot_t prot)
>         }
>  }
> 
> +#ifndef __riscv_cmodel_medany
> +#error "setup_vm() is called from head.S before relocate and must
> not make any absolute references."
> +#endif
>  asmlinkage void __init setup_vm(void)
>  {
>         extern char _start;
> 
> Marking this notrace is the right thing to do, as it can't call into
> any 
> functions that aren't medany (there's probably other issues as well,
> since this 
> is so early).
> 
> Sorry I missed this the first time around, I wasn't paying enough
> attention.
> 
> Can someone add instructions for 32-bit boots to the QEMU wiki?  It
> sounds like 
> it's time to add that to the testing list...

Done!

https://wiki.qemu.org/Documentation/Platforms/RISCV

Alistair

> 
> Thanks for digging in to this!
> 
> > Regards,
> > Anup
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv