Re: [PATCH] RISC-V: Place DTB at 3GB boundary instead of 4GB

From: Palmer Dabbelt <palmer@dabbelt.com>
To: Atish Patra <Atish.Patra@wdc.com>
Cc: qemu-riscv@nongnu.org, sagark@eecs.berkeley.edu,
	Bastian Koppelmann <kbastian@mail.uni-paderborn.de>,
	Anup Patel <Anup.Patel@wdc.com>,
	qemu-devel@nongnu.org,
	Alistair Francis <Alistair.Francis@wdc.com>,
	bmeng.cn@gmail.com
Subject: Re: [PATCH] RISC-V: Place DTB at 3GB boundary instead of 4GB
Date: Thu, 17 Dec 2020 14:39:33 -0800 (PST)	[thread overview]
Message-ID: <mhng-46db8e2c-05a2-4333-99de-af0d7e13e71d@palmerdabbelt-glaptop> (raw)
In-Reply-To: <5b73f3cfc2c278acd4b0afc36c5d7fa764bcc9ae.camel@wdc.com>

On Thu, 17 Dec 2020 14:35:10 PST (-0800), Atish Patra wrote:
> On Thu, 2020-12-17 at 14:31 -0800, Palmer Dabbelt wrote:
>> On Thu, 17 Dec 2020 13:48:26 PST (-0800), Atish Patra wrote:
>> > Currently, we place the DTB at 2MB from 4GB or end of DRAM which
>> > ever is
>> > lesser. However, Linux kernel can address only 1GB of memory for
>> > RV32.
>> > Thus, it can not map anything beyond 3GB (assuming 2GB is the
>> > starting address).
>> > As a result, it can not process DT and panic if opensbi dynamic
>> > firmware
>> > is used.
>> > 
>> > Fix this by placing the DTB at 2MB from 3GB or end of DRAM
>> > whichever is lower.
>> > 
>> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
>> > ---
>> >  hw/riscv/boot.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> > 
>> > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
>> > index d62f3dc7581e..9e77b22e4d56 100644
>> > --- a/hw/riscv/boot.c
>> > +++ b/hw/riscv/boot.c
>> > @@ -203,9 +203,9 @@ uint32_t riscv_load_fdt(hwaddr dram_base,
>> > uint64_t mem_size, void *fdt)
>> >       * We should put fdt as far as possible to avoid kernel/initrd
>> > overwriting
>> >       * its content. But it should be addressable by 32 bit system
>> > as well.
>> >       * Thus, put it at an aligned address that less than fdt size
>> > from end of
>> > -     * dram or 4GB whichever is lesser.
>> > +     * dram or 3GB whichever is lesser.
>> >       */
>> > -    temp = MIN(dram_end, 4096 * MiB);
>> > +    temp = MIN(dram_end, 3072 * MiB);
>> >      fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
>> > 
>> >      fdt_pack(fdt);
>> 
>> Presumably this was the cause of that 32-bit boot issue?
> 
> 
> This fixes the fw_dynamic case if you use more than 1GB of memory.
> The other issue was with the kernel. I sent the kernel fix yesterday.
> 
> https://patchwork.kernel.org/project/linux-riscv/patch/20201217074855.1948743-1-atish.patra@wdc.com/
> 
> Can you include the kernel fix in your next PR ?

Not the one I'm writing up right now, as I want to get the new stuff up earlier
in the merge window this time around.  I'm planning on sending fixes during the
merge window, though, as we have a few.  We have tomorrow off, so I should be
able to get through all the email and send it either over the weekend or early
next week.