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

[Alistair Francis <Alistair.Francis@wdc.com>]

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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv