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: <20230704121837.248976-1-alexghiti@rivosinc.com> <mhng-c5a71036-061e-4704-bcdf-2b23cab36908@palmer-ri-x1c9a>
In-Reply-To: <mhng-c5a71036-061e-4704-bcdf-2b23cab36908@palmer-ri-x1c9a>
From:   Alexandre Ghiti <alexghiti@rivosinc.com>
Date:   Tue, 11 Jul 2023 12:51:56 +0200
Message-ID: <CAHVXubiU7XUSDmJ1YuUX85kLtMO=Y00fYYHuepc2=d3oG8Aq4g@mail.gmail.com>
Subject: Re: [PATCH] riscv: Start of DRAM should at least be aligned on PMD
 size for the direct mapping
To:     Palmer Dabbelt <palmer@dabbelt.com>
Cc:     Paul Walmsley <paul.walmsley@sifive.com>, aou@eecs.berkeley.edu,
        suagrfillet@gmail.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

(sorry for the delay!)

On Thu, Jul 6, 2023 at 7:05=E2=80=AFPM Palmer Dabbelt <palmer@dabbelt.com> =
wrote:
>
> On Tue, 04 Jul 2023 05:18:37 PDT (-0700), alexghiti@rivosinc.com wrote:
> > So that we do not end up mapping the whole linear mapping using 4K
> > pages, which is slow at boot time, and also very likely at runtime.
> >
> > So make sure we align the start of DRAM on a PMD boundary.
> >
> > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > ---
> >  arch/riscv/mm/init.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 4fa420faa780..4a43ec275c6d 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -214,8 +214,13 @@ static void __init setup_bootmem(void)
> >       memblock_reserve(vmlinux_start, vmlinux_end - vmlinux_start);
> >
> >       phys_ram_end =3D memblock_end_of_DRAM();
> > +
> > +     /*
> > +      * Make sure we align the start of the memory on a PMD boundary s=
o that
> > +      * at worst, we map the linear mapping with PMD mappings.
> > +      */
> >       if (!IS_ENABLED(CONFIG_XIP_KERNEL))
> > -             phys_ram_base =3D memblock_start_of_DRAM();
> > +             phys_ram_base =3D memblock_start_of_DRAM() & PMD_MASK;
>
> This rounds down, which IIUC will result in mappings outside what
> memblock detected as the start af DRAM.  I'd expect that to cause bad
> behavior somewhere.

Actually we are not mapping this new region as it is not present in
the memblock regions, we are just re-aligning the virtual and physical
address: phys_ram_base is only used for the virtual to physical
translations.

>
> Shouldn't we be rounding up?

Doing so would remove memory from the memory map, but I'm not sure
this is correct, we could remove memory that contains "something" that
needs to be accessed using the linear mapping (ACPI tables? DT?).

More testing is welcome as I can be wrong of course.

Thanks,

Alex


>
> >
> >       /*
> >        * In 64-bit, any use of __va/__pa before this point is wrong as =
we