From: Woody Zhang <woodylab@foxmail.com>
To: Song Shuai <songshuaishuai@tinylab.org>
Cc: Alexandre Ghiti <alexghiti@rivosinc.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
Conor Dooley <conor.dooley@microchip.com>,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] riscv: move memblock_allow_resize() after lm is ready
Date: Wed, 14 Jun 2023 20:41:48 +0800 [thread overview]
Message-ID: <tencent_C1CE680F8B300559050D967C7DC8305CA90A@qq.com> (raw)
In-Reply-To: <6B512C74DCAB0BF0+0ad9a892-6933-baa3-0848-1e1efc685c9f@tinylab.org>
On Wed, Jun 14, 2023 at 05:51:23PM +0800, Song Shuai wrote:
>
>
>在 2023/6/12 15:15, Alexandre Ghiti 写道:
>> Hi Woody,
>>
>> On Sat, Jun 10, 2023 at 1:49 AM Woody Zhang <woodylab@foxmail.com> wrote:
>> >
>> > The initial memblock metadata is accessed from kernel image mapping. The
>> > regions arrays need to "reallocated" from memblock and accessed through
>> > linear mapping to cover more memblock regions. So the resizing should
>> > not be allowed until linear mapping is ready. Note that there are
>> > memblock allocations when building linear mapping.
>> >
>> > Signed-off-by: Woody Zhang <woodylab@foxmail.com>
>> > ---
>> > arch/riscv/mm/init.c | 4 +++-
>> > 1 file changed, 3 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> > index 9e9da69720ce..8a33ecbb4d0f 100644
>> > --- a/arch/riscv/mm/init.c
>> > +++ b/arch/riscv/mm/init.c
>> > @@ -258,7 +258,6 @@ static void __init setup_bootmem(void)
>> > dma_contiguous_reserve(dma32_phys_limit);
>> > if (IS_ENABLED(CONFIG_64BIT))
>> > hugetlb_cma_reserve(PUD_SHIFT - PAGE_SHIFT);
>> > - memblock_allow_resize();
>> > }
>> >
>> > #ifdef CONFIG_MMU
>> > @@ -1250,6 +1249,9 @@ static void __init setup_vm_final(void)
>> > csr_write(CSR_SATP, PFN_DOWN(__pa_symbol(swapper_pg_dir)) | satp_mode);
>> > local_flush_tlb_all();
>> >
>> > + /* Depend on that Linear Mapping is ready */
>> > + memblock_allow_resize();
>> > +
>> > pt_ops_set_late();
>> > }
>> > #else
>> > --
>> > 2.39.2
>> >
>>
>> The commit log does not describe the issue thoroughly enough to me,
>> maybe you could point to the arm64 commit that did the same? I mean
>> commit 24cc61d8cb5a ("arm64: memblock: don't permit memblock resizing
>> until linear mapping is up").
>@Alex
>
>I reproduced the problem as the arm64 commit describes.
>You can find the complete log via this link: https://termbin.com/bx0o
>
>I constructed the dtb with numerous discrete /memreserve/ regions
>(the numbers of these regions approximate INIT_MEMBLOCK_REGIONS) which full
>the reserved regions up.
>
>When memblock_allow_resize was set, the calling of memblock_reserve() would
>double/resize the reserved regions and do the __memcopy() from the old
>regions ( mapped by kernel)
>to the new ones (provided by __va()).
>But before the linear mapping was ready (like: during the creating of linear
>mapping),
>memblock_reserve() was called and the memcopy would trigger a Store/AMO page
>fault.
>
>>
>> Another point is that I would not put this call into setup_vm_final(),
>> I'd rather add it in paging_init() as it does not seem like a good fit
>> for setup_vm_final(). But that's a nit so up to you of course.
>>
>I agree.
>
>@Woody
>
>I noticed your V2 [1] didn't take this suggestion, maybe you can take it at
>V3.
>And it will be more sound if you supplement the commit-msg with the panic
>info from the log.
>
Thanks for your comments. I will update it with your panic log in next
version.
Woody
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
prev parent reply other threads:[~2023-06-14 12:42 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-09 23:43 [PATCH] riscv: move memblock_allow_resize() after lm is ready Woody Zhang
2023-06-10 15:44 ` Conor Dooley
2023-06-10 23:31 ` Woody Zhang
2023-06-11 10:15 ` Conor Dooley
2023-06-12 7:15 ` Alexandre Ghiti
2023-06-14 9:51 ` Song Shuai
2023-06-14 12:41 ` Woody Zhang [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=tencent_C1CE680F8B300559050D967C7DC8305CA90A@qq.com \
--to=woodylab@foxmail.com \
--cc=alexghiti@rivosinc.com \
--cc=aou@eecs.berkeley.edu \
--cc=conor.dooley@microchip.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=songshuaishuai@tinylab.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).