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 <[email protected]>
---
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
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 <[email protected]>
What commit does this fix?
Cheers,
Conor.
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 <[email protected]>
>
>What commit does this fix?
Fixes: 671f9a3e2e24 ("RISC-V: Setup initial page tables in two stages")
Will add this tag in v2.
Woody
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 <[email protected]>
> >
> >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.
Hi Woody,
On Sat, Jun 10, 2023 at 1:49 AM Woody Zhang <[email protected]> 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 <[email protected]>
> ---
> 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
在 2023/6/12 15:15, Alexandre Ghiti 写道:
> Hi Woody,
>
> On Sat, Jun 10, 2023 at 1:49 AM Woody Zhang <[email protected]> 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 <[email protected]>
>> ---
>> 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/[email protected]/
> Anyway, that's a good catch, thanks!
>
> Alex
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Thanks
Song Shuai
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 <[email protected]> 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 <[email protected]>
>> > ---
>> > 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