* [PATCH] riscv: move memblock_allow_resize() after lm is ready
@ 2023-06-09 23:43 Woody Zhang
2023-06-10 15:44 ` Conor Dooley
2023-06-12 7:15 ` Alexandre Ghiti
0 siblings, 2 replies; 7+ messages in thread
From: Woody Zhang @ 2023-06-09 23:43 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Conor Dooley
Cc: linux-riscv, linux-kernel, Woody Zhang
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: move memblock_allow_resize() after lm is ready
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-12 7:15 ` Alexandre Ghiti
1 sibling, 1 reply; 7+ messages in thread
From: Conor Dooley @ 2023-06-10 15:44 UTC (permalink / raw)
To: Woody Zhang
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Conor Dooley, linux-riscv, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 629 bytes --]
On Sat, Jun 10, 2023 at 07:43:20AM +0800, Woody Zhang wrote:
$subject: "riscv: move memblock_allow_resize() after lm is ready"
s/lm/linear map/ if you resubmit, please.
> 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>
What commit does this fix?
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: move memblock_allow_resize() after lm is ready
2023-06-10 15:44 ` Conor Dooley
@ 2023-06-10 23:31 ` Woody Zhang
2023-06-11 10:15 ` Conor Dooley
0 siblings, 1 reply; 7+ messages in thread
From: Woody Zhang @ 2023-06-10 23:31 UTC (permalink / raw)
To: Conor Dooley
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Conor Dooley, linux-riscv, linux-kernel
On Sat, Jun 10, 2023 at 04:44:19PM +0100, Conor Dooley wrote:
>On Sat, Jun 10, 2023 at 07:43:20AM +0800, Woody Zhang wrote:
>$subject: "riscv: move memblock_allow_resize() after lm is ready"
>
>s/lm/linear map/ if you resubmit, please.
Thanks, will send a v2.
>
>> 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>
>
>What commit does this fix?
Fixes: 671f9a3e2e24 ("RISC-V: Setup initial page tables in two stages")
Will add this tag in v2.
Woody
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: move memblock_allow_resize() after lm is ready
2023-06-10 23:31 ` Woody Zhang
@ 2023-06-11 10:15 ` Conor Dooley
0 siblings, 0 replies; 7+ messages in thread
From: Conor Dooley @ 2023-06-11 10:15 UTC (permalink / raw)
To: Woody Zhang
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Conor Dooley, linux-riscv, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1038 bytes --]
On Sun, Jun 11, 2023 at 07:31:03AM +0800, Woody Zhang wrote:
> On Sat, Jun 10, 2023 at 04:44:19PM +0100, Conor Dooley wrote:
> >On Sat, Jun 10, 2023 at 07:43:20AM +0800, Woody Zhang wrote:
> >$subject: "riscv: move memblock_allow_resize() after lm is ready"
> >
> >s/lm/linear map/ if you resubmit, please.
>
> Thanks, will send a v2.
>
> >
> >> 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>
> >
> >What commit does this fix?
>
> Fixes: 671f9a3e2e24 ("RISC-V: Setup initial page tables in two stages")
>
> Will add this tag in v2.
b4 should pick up fixes tags from replies, you shouldn't need to
resubmit for that alone.
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: move memblock_allow_resize() after lm is ready
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-12 7:15 ` Alexandre Ghiti
2023-06-14 9:51 ` Song Shuai
1 sibling, 1 reply; 7+ messages in thread
From: Alexandre Ghiti @ 2023-06-12 7:15 UTC (permalink / raw)
To: Woody Zhang
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Conor Dooley,
linux-riscv, linux-kernel
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").
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.
Anyway, that's a good catch, thanks!
Alex
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: move memblock_allow_resize() after lm is ready
2023-06-12 7:15 ` Alexandre Ghiti
@ 2023-06-14 9:51 ` Song Shuai
2023-06-14 12:41 ` Woody Zhang
0 siblings, 1 reply; 7+ messages in thread
From: Song Shuai @ 2023-06-14 9:51 UTC (permalink / raw)
To: Alexandre Ghiti, Woody Zhang
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Conor Dooley,
linux-riscv, linux-kernel
在 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.
[1]:
https://lore.kernel.org/linux-riscv/tencent_97F6C19BAF7C99B22BB1199FE16B266C2B07@qq.com/
> Anyway, that's a good catch, thanks!
>
> Alex
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Thanks
Song Shuai
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: move memblock_allow_resize() after lm is ready
2023-06-14 9:51 ` Song Shuai
@ 2023-06-14 12:41 ` Woody Zhang
0 siblings, 0 replies; 7+ messages in thread
From: Woody Zhang @ 2023-06-14 12:41 UTC (permalink / raw)
To: Song Shuai
Cc: Alexandre Ghiti, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Conor Dooley, linux-riscv, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-06-14 12:45 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 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).