Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <tencent_D656D683705F41324657ED3393C3384C7408@qq.com>
In-Reply-To: <tencent_D656D683705F41324657ED3393C3384C7408@qq.com>
From:   Alexandre Ghiti <alexghiti@rivosinc.com>
Date:   Mon, 12 Jun 2023 09:15:38 +0200
Message-ID: <CAHVXubiYHQCYkymde2y_okNb2XcE-xVBMj8iZ7kM5d08bhqxiQ@mail.gmail.com>
Subject: Re: [PATCH] riscv: move memblock_allow_resize() after lm is ready
To:     Woody Zhang <woodylab@foxmail.com>
Cc:     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
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Woody,

On Sat, Jun 10, 2023 at 1:49=E2=80=AFAM 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