Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190312220752.128141-1-anup.patel@wdc.com> <20190312220752.128141-4-anup.patel@wdc.com>
 <20190313183121.GB28630@rapoport-lnx>
In-Reply-To: <20190313183121.GB28630@rapoport-lnx>
From:   Anup Patel <anup@brainfault.org>
Date:   Thu, 14 Mar 2019 02:36:01 +0530
Message-ID: <CAAhSdy1mrV2CakS0cfOY8F1B7uH_yXjO3W-CiMUcYonkRnVbeQ@mail.gmail.com>
Subject: Re: [PATCH 3/3] RISC-V: Allow booting kernel from any 4KB aligned address
To:     Mike Rapoport <rppt@linux.ibm.com>
Cc:     Anup Patel <Anup.Patel@wdc.com>,
        Palmer Dabbelt <palmer@sifive.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Atish Patra <Atish.Patra@wdc.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Christoph Hellwig <hch@infradead.org>,
        "linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Mar 14, 2019 at 12:01 AM Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> On Tue, Mar 12, 2019 at 10:08:22PM +0000, Anup Patel wrote:
> > Currently, we have to boot RISCV64 kernel from a 2MB aligned physical
> > address and RISCV32 kernel from a 4MB aligned physical address. This
> > constraint is because initial pagetable setup (i.e. setup_vm()) maps
> > entire RAM using hugepages (i.e. 2MB for 3-level pagetable and 4MB for
> > 2-level pagetable).
> >
> > Further, the above booting contraint also results in memory wastage
> > because if we boot kernel from some <xyz> address (which is not same as
> > RAM start address) then RISCV kernel will map PAGE_OFFSET virtual address
> > lineraly to <xyz> physical address and memory between RAM start and <xyz>
> > will be reserved/unusable.
> >
> > For example, RISCV64 kernel booted from 0x80200000 will waste 2MB of RAM
> > and RISCV32 kernel booted from 0x80400000 will waste 4MB of RAM.
> >
> > This patch re-writes the initial pagetable setup code to allow booting
> > RISV32 and RISCV64 kernel from any 4KB (i.e. PAGE_SIZE) aligned address.
> >
> > To achieve this:
> > 1. We map kernel, dtb and only some amount of RAM (few MBs) using 4KB
> >    mappings in setup_vm() (called from head.S)
> > 2. Once we reach paging_init() (called from setup_arch()) after
> >    memblock setup, we map all available memory banks using 4KB
> >    mappings and memblock APIs.
>
> I'm not really familiar with RISC-V, but my guess would be that you'd get
> worse TLB performance with 4KB mappings. Not mentioning the amount of
> memory required for the page table itself.

I agree we will see a hit in TLB performance due to 4KB mappings.

To address this we can create, 2MB (or 4MB on 32bit system) mappings
whenever load_pa is aligned to it otherwise we prefer 4KB mappings. In other
words, we create bigger mappings whenever possible and fallback to 4KB
mappings when not possible.

This way if kernel is booted from 2MB (or 4MB) aligned address then we will
see good TLB performance for kernel addresses. Also, users are still free to
boot Linux RISC-V kernel from any 4KB aligned address.

Of course, we will have to document this as part of Linux RISC-V booting
requirements under Documentation/ (which does not exist currently).

>
> If the only goal is to utilize the physical memory below the kernel, it
> simply should not be reserved at the first place, something like:

Well, our goal was two-fold:

1. We wanted to unify boot-time alignment requirements for 32bit and
64bit RISC-V systems
2. Save memory by allowing users to place kernel just after the runtime
firmware at starting of RAM.

>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index b379a75..6301ced 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -108,6 +108,7 @@ void __init setup_bootmem(void)
>         /* Find the memory region containing the kernel */
>         for_each_memblock(memory, reg) {
>                 phys_addr_t vmlinux_end = __pa(_end);
> +               phys_addr_t vmlinux_start = __pa(start);
>                 phys_addr_t end = reg->base + reg->size;
>
>                 if (reg->base <= vmlinux_end && vmlinux_end <= end) {
> @@ -115,7 +116,8 @@ void __init setup_bootmem(void)
>                          * Reserve from the start of the region to the end of
>                          * the kernel
>                          */
> -                       memblock_reserve(reg->base, vmlinux_end - reg->base);
> +                       memblock_reserve(vmlinux_start,
> +                                        vmlinux_end - vmlinux_start);
>                         mem_size = min(reg->size, (phys_addr_t)-PAGE_OFFSET);
>                 }
>         }

Thanks for above changes. I will include them in my next revision.

Regards,
Anup