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

From: Palmer Dabbelt <palmer@sifive.com>
To: anup@brainfault.org
Cc: aou@eecs.berkeley.edu, alankao@andestech.com,
	Anup Patel <Anup.Patel@wdc.com>,
	linux-kernel@vger.kernel.org, rppt@linux.ibm.com,
	Christoph Hellwig <hch@infradead.org>,
	Atish Patra <Atish.Patra@wdc.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH v2] RISC-V: Always compile mm/init.c with cmodel=medany
Date: Mon, 25 Mar 2019 19:22:30 -0700 (PDT)	[thread overview]
Message-ID: <mhng-8e00a3e3-5a69-4828-b7bb-2bf000199b48@palmer-si-x1c4> (raw)
In-Reply-To: <CAAhSdy2-CX729tVuuZzArswpFEC_OeRXhwXtCaiNe53AuzqdTw@mail.gmail.com>

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
