Re: [PATCH 3/3] RISC-V: Allow booting kernel from any 4KB aligned address

2019-04-10 Thread Nick Kossifidis

Στις 2019-03-13 00:08, Anup Patel έγραψε:

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  address (which is not same as
RAM start address) then RISCV kernel will map PAGE_OFFSET virtual 
address
lineraly to  physical address and memory between RAM start and 


will be reserved/unusable.

For example, RISCV64 kernel booted from 0x8020 will waste 2MB of 
RAM

and RISCV32 kernel booted from 0x8040 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.

With this patch in-place, the booting constraint for RISCV32 and 
RISCV64

kernel is much more relaxed and we can now boot kernel very close to
RAM start thereby minimizng memory wastage.

Signed-off-by: Anup Patel 
---
 arch/riscv/include/asm/fixmap.h |   5 +
 arch/riscv/include/asm/pgtable-64.h |   5 +
 arch/riscv/include/asm/pgtable.h|   6 +-
 arch/riscv/kernel/head.S|   1 +
 arch/riscv/kernel/setup.c   |   4 +-
 arch/riscv/mm/init.c| 357 +++-
 6 files changed, 317 insertions(+), 61 deletions(-)

diff --git a/arch/riscv/include/asm/fixmap.h 
b/arch/riscv/include/asm/fixmap.h

index 57afe604b495..5cf53dd882e5 100644
--- a/arch/riscv/include/asm/fixmap.h
+++ b/arch/riscv/include/asm/fixmap.h
@@ -21,6 +21,11 @@
  */
 enum fixed_addresses {
FIX_HOLE,
+#define FIX_FDT_SIZE   SZ_1M
+   FIX_FDT_END,
+   FIX_FDT = FIX_FDT_END + FIX_FDT_SIZE / PAGE_SIZE - 1,
+   FIX_PTE,
+   FIX_PMD,
FIX_EARLYCON_MEM_BASE,
__end_of_fixed_addresses
 };
diff --git a/arch/riscv/include/asm/pgtable-64.h
b/arch/riscv/include/asm/pgtable-64.h
index 7aa0ea9bd8bb..56ecc3dc939d 100644
--- a/arch/riscv/include/asm/pgtable-64.h
+++ b/arch/riscv/include/asm/pgtable-64.h
@@ -78,6 +78,11 @@ static inline pmd_t pfn_pmd(unsigned long pfn, 
pgprot_t prot)

return __pmd((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
 }

+static inline unsigned long _pmd_pfn(pmd_t pmd)
+{
+   return pmd_val(pmd) >> _PAGE_PFN_SHIFT;
+}
+
 #define pmd_ERROR(e) \
pr_err("%s:%d: bad pmd %016lx.\n", __FILE__, __LINE__, pmd_val(e))

diff --git a/arch/riscv/include/asm/pgtable.h 
b/arch/riscv/include/asm/pgtable.h

index 1141364d990e..05fa2115e736 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -121,12 +121,16 @@ static inline void pmd_clear(pmd_t *pmdp)
set_pmd(pmdp, __pmd(0));
 }

-
 static inline pgd_t pfn_pgd(unsigned long pfn, pgprot_t prot)
 {
return __pgd((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
 }

+static inline unsigned long _pgd_pfn(pgd_t pgd)
+{
+   return pgd_val(pgd) >> _PAGE_PFN_SHIFT;
+}
+
 #define pgd_index(addr) (((addr) >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1))

 /* Locate an entry in the page global directory */
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 7966262b4f9d..12a3ec5eb8ab 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -63,6 +63,7 @@ clear_bss_done:
/* Initialize page tables and relocate to virtual addresses */
la sp, init_thread_union + THREAD_SIZE
la a0, _start
+   mv a1, s1
call setup_vm
call relocate

diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index ecb654f6a79e..acdd0f74982b 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -30,6 +30,7 @@
 #include 
 #include 

+#include 
 #include 
 #include 
 #include 
@@ -62,7 +63,8 @@ unsigned long boot_cpu_hartid;

 void __init parse_dtb(unsigned int hartid, void *dtb)
 {
-   if (early_init_dt_scan(__va(dtb)))
+   dtb = (void *)fix_to_virt(FIX_FDT) + ((uintptr_t)dtb & ~PAGE_MASK);
+   if (early_init_dt_scan(dtb))
return;

pr_err("No DTB passed to the kernel\n");
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index f35299f2f3d5..ee55a4b90dec 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -1,14 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
+ * Copyright (C) 2019 Western Digital Corporation or its affiliates.
  * Copyright (C) 2012 Regents of the University of California
- *
- *   This program is free software; you can redistribute it and/or
- *   modify it under the terms of the GNU General Public 

Re: [PATCH 3/3] RISC-V: Allow booting kernel from any 4KB aligned address

2019-03-18 Thread Mike Rapoport
On Mon, Mar 18, 2019 at 06:46:18PM +0530, Anup Patel wrote:
> On Mon, Mar 18, 2019 at 12:48 PM Mike Rapoport  wrote:
> >
> > On Sat, Mar 16, 2019 at 04:55:30AM +0530, Anup Patel wrote:
> > > On Fri, Mar 15, 2019 at 9:52 PM Anup Patel  wrote:
> > > >
> > > > On Fri, Mar 15, 2019 at 9:28 PM Mike Rapoport  
> > > > wrote:
> > > > >
> > > > > I still don't get why it is that important to relax alignment of the 
> > > > > kernel
> > > > > load address. Provided you can use the memory below the kernel, it 
> > > > > really
> > > > > should not matter.
> > > >
> > > > Irrespective to constraint on kernel load address, we certainly need
> > > > to allow memory below kernel to be usable but that's a separate change.
> > > >
> > > > Currently, the memory below kernel is ignored by
> > > > early_init_dt_add_memory_arch() in
> > > > drivers/of/fdt.c
> > > >
> > >
> > > I explored the possibility of re-claiming memory below kernel but then
> > > we have an issue in this case.
> > >
> > > For RISC-V kernel, PAGE_OFFSET is mapped to kernel load address
> > > (i.e. load_pa in this code). The va_pa_offset is based on load_pa so 
> > > linear
> > > conversion of VA-to-PA and PA-to-VA won't be possible on the memory
> > > below kernel. I guess this is why early_init_dt_add_memory_arch() is
> > > marking memory below kernel as reserved. Is there better way to do it??
> > >
> > > We started exploring ways to re-claim memory below kernel because
> > > we are trying to get Linux working on Kendryte K210 board
> > > (https://kendryte.com/). This board has dual-core 64bit RISC-V but it
> > > only has 8MB RAM.
> >
> > Huh, 8MB of RAM is tough...
> >
> > It is possible to use the memory below the kernel, e.g x86-64 does that.
> > But it is definitely a separate change and with such RAM diet using 4K
> > pages seems unavoidable.
> >
> > I still have concern about using 4K pages whenever the load address is not
> > 2M (4M) aligned. People tend to not pay enough attention to such details
> > and they would load the kernel at an arbitrary address and get the
> > performance hit.
> >
> > I think the default should remain as is and the ability to map the kernel
> > with 4K pages (and use 4K aligned load address) should be a Kconfig option.
> 
> I agree people will tend to not pay attention on the load address alignment
> but this is also possible with current approach. Currently, if user boots 
> kernel
> form any non-2M aligned address then we don't see any prints at all which
> let's users think it to be kernel bug. In fact, I have done same mistake 
> couple
> of times.
> 
> Another approach (apart from kconfig option) would be to throw big-fat
> warning when users boot kernel form 4K aligned load address this way
> atleast kernel boots instead of no prints. Your thoughts??

That should be panic() rather than warning. If the trampoline_pg_dir will
map everything, it can be emitted during the initialization of
swapper_pg_dir.
 
> >
> > Another thing I'd like to suggest is to completely split swapper_pg_dir
> > initialization from setup_vm() and keep this function solely for
> > initialization of the trampoline_pg_dir. The trampoline_pg_dir can use
> > large pages and the memory below the kernel start can be mapped there
> > simply by mapping the entire large page containing the kernel start.
> > Then, the swapper_pg_dir setup can run with virtual memory enabled and can
> > have much more flexibility.
> 
> Sure, this is a good suggestion. I will add this as separate patch in this
> series.

> Regards,
> Anup
> 

-- 
Sincerely yours,
Mike.



Re: [PATCH 3/3] RISC-V: Allow booting kernel from any 4KB aligned address

2019-03-18 Thread Paul Walmsley
On Mon, 18 Mar 2019, Mike Rapoport wrote:

> On Sat, Mar 16, 2019 at 04:55:30AM +0530, Anup Patel wrote:
> 
> > We started exploring ways to re-claim memory below kernel because
> > we are trying to get Linux working on Kendryte K210 board
> > (https://kendryte.com/). This board has dual-core 64bit RISC-V but it
> > only has 8MB RAM.
> 
> Huh, 8MB of RAM is tough...
> 
> It is possible to use the memory below the kernel, e.g x86-64 does that.
> But it is definitely a separate change and with such RAM diet using 4K
> pages seems unavoidable.
> 
> I still have concern about using 4K pages whenever the load address is not
> 2M (4M) aligned. People tend to not pay enough attention to such details
> and they would load the kernel at an arbitrary address and get the
> performance hit.
> 
> I think the default should remain as is and the ability to map the kernel
> with 4K pages (and use 4K aligned load address) should be a Kconfig option.

Agreed.  That Kconfig parameter should also be default-off.
  
Only a small number of people will try to run RISC-V Linux on a Kendryte 
board.  That niche use-case shouldn't impact the much larger group of 
people who will run Linux on more reasonably-sized systems.  No one should 
need to ask people to report their kernel load address whenever someone
reports a performance regression.


- Paul


Re: [PATCH 3/3] RISC-V: Allow booting kernel from any 4KB aligned address

2019-03-18 Thread Anup Patel
On Mon, Mar 18, 2019 at 12:48 PM Mike Rapoport  wrote:
>
> On Sat, Mar 16, 2019 at 04:55:30AM +0530, Anup Patel wrote:
> > On Fri, Mar 15, 2019 at 9:52 PM Anup Patel  wrote:
> > >
> > > On Fri, Mar 15, 2019 at 9:28 PM Mike Rapoport  wrote:
> > > >
> > > > I still don't get why it is that important to relax alignment of the 
> > > > kernel
> > > > load address. Provided you can use the memory below the kernel, it 
> > > > really
> > > > should not matter.
> > >
> > > Irrespective to constraint on kernel load address, we certainly need
> > > to allow memory below kernel to be usable but that's a separate change.
> > >
> > > Currently, the memory below kernel is ignored by
> > > early_init_dt_add_memory_arch() in
> > > drivers/of/fdt.c
> > >
> >
> > I explored the possibility of re-claiming memory below kernel but then
> > we have an issue in this case.
> >
> > For RISC-V kernel, PAGE_OFFSET is mapped to kernel load address
> > (i.e. load_pa in this code). The va_pa_offset is based on load_pa so linear
> > conversion of VA-to-PA and PA-to-VA won't be possible on the memory
> > below kernel. I guess this is why early_init_dt_add_memory_arch() is
> > marking memory below kernel as reserved. Is there better way to do it??
> >
> > We started exploring ways to re-claim memory below kernel because
> > we are trying to get Linux working on Kendryte K210 board
> > (https://kendryte.com/). This board has dual-core 64bit RISC-V but it
> > only has 8MB RAM.
>
> Huh, 8MB of RAM is tough...
>
> It is possible to use the memory below the kernel, e.g x86-64 does that.
> But it is definitely a separate change and with such RAM diet using 4K
> pages seems unavoidable.
>
> I still have concern about using 4K pages whenever the load address is not
> 2M (4M) aligned. People tend to not pay enough attention to such details
> and they would load the kernel at an arbitrary address and get the
> performance hit.
>
> I think the default should remain as is and the ability to map the kernel
> with 4K pages (and use 4K aligned load address) should be a Kconfig option.

I agree people will tend to not pay attention on the load address alignment
but this is also possible with current approach. Currently, if user boots kernel
form any non-2M aligned address then we don't see any prints at all which
let's users think it to be kernel bug. In fact, I have done same mistake couple
of times.

Another approach (apart from kconfig option) would be to throw big-fat
warning when users boot kernel form 4K aligned load address this way
atleast kernel boots instead of no prints. Your thoughts??

>
> Another thing I'd like to suggest is to completely split swapper_pg_dir
> initialization from setup_vm() and keep this function solely for
> initialization of the trampoline_pg_dir. The trampoline_pg_dir can use
> large pages and the memory below the kernel start can be mapped there
> simply by mapping the entire large page containing the kernel start.
> Then, the swapper_pg_dir setup can run with virtual memory enabled and can
> have much more flexibility.

Sure, this is a good suggestion. I will add this as separate patch in this
series.

Regards,
Anup


Re: [PATCH 3/3] RISC-V: Allow booting kernel from any 4KB aligned address

2019-03-18 Thread Mike Rapoport
On Sat, Mar 16, 2019 at 04:55:30AM +0530, Anup Patel wrote:
> On Fri, Mar 15, 2019 at 9:52 PM Anup Patel  wrote:
> >
> > On Fri, Mar 15, 2019 at 9:28 PM Mike Rapoport  wrote:
> > >
> > > I still don't get why it is that important to relax alignment of the 
> > > kernel
> > > load address. Provided you can use the memory below the kernel, it really
> > > should not matter.
> >
> > Irrespective to constraint on kernel load address, we certainly need
> > to allow memory below kernel to be usable but that's a separate change.
> >
> > Currently, the memory below kernel is ignored by
> > early_init_dt_add_memory_arch() in
> > drivers/of/fdt.c
> >
> 
> I explored the possibility of re-claiming memory below kernel but then
> we have an issue in this case.
> 
> For RISC-V kernel, PAGE_OFFSET is mapped to kernel load address
> (i.e. load_pa in this code). The va_pa_offset is based on load_pa so linear
> conversion of VA-to-PA and PA-to-VA won't be possible on the memory
> below kernel. I guess this is why early_init_dt_add_memory_arch() is
> marking memory below kernel as reserved. Is there better way to do it??
> 
> We started exploring ways to re-claim memory below kernel because
> we are trying to get Linux working on Kendryte K210 board
> (https://kendryte.com/). This board has dual-core 64bit RISC-V but it
> only has 8MB RAM.

Huh, 8MB of RAM is tough...

It is possible to use the memory below the kernel, e.g x86-64 does that.
But it is definitely a separate change and with such RAM diet using 4K
pages seems unavoidable.

I still have concern about using 4K pages whenever the load address is not
2M (4M) aligned. People tend to not pay enough attention to such details
and they would load the kernel at an arbitrary address and get the
performance hit.

I think the default should remain as is and the ability to map the kernel
with 4K pages (and use 4K aligned load address) should be a Kconfig option.

Another thing I'd like to suggest is to completely split swapper_pg_dir
initialization from setup_vm() and keep this function solely for
initialization of the trampoline_pg_dir. The trampoline_pg_dir can use
large pages and the memory below the kernel start can be mapped there
simply by mapping the entire large page containing the kernel start.
Then, the swapper_pg_dir setup can run with virtual memory enabled and can
have much more flexibility.

> 
> Regards,
> Anup
> 

-- 
Sincerely yours,
Mike.



Re: [PATCH 3/3] RISC-V: Allow booting kernel from any 4KB aligned address

2019-03-15 Thread Anup Patel
On Fri, Mar 15, 2019 at 9:52 PM Anup Patel  wrote:
>
> On Fri, Mar 15, 2019 at 9:28 PM Mike Rapoport  wrote:
> >
> > On Thu, Mar 14, 2019 at 11:28:32PM +0530, Anup Patel wrote:
> > > On Thu, Mar 14, 2019 at 12:23 PM Mike Rapoport  wrote:
> > > >
> > > > On Thu, Mar 14, 2019 at 02:36:01AM +0530, Anup Patel wrote:
> > > > > On Thu, Mar 14, 2019 at 12:01 AM Mike Rapoport  
> > > > > wrote:
> > > > > >
> > > > > > On Tue, Mar 12, 2019 at 10:08:22PM +, 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  address (which is not 
> > > > > > > same as
> > > > > > > RAM start address) then RISCV kernel will map PAGE_OFFSET virtual 
> > > > > > > address
> > > > > > > lineraly to  physical address and memory between RAM start 
> > > > > > > and 
> > > > > > > will be reserved/unusable.
> > > > > > >
> > > > > > > For example, RISCV64 kernel booted from 0x8020 will waste 2MB 
> > > > > > > of RAM
> > > > > > > and RISCV32 kernel booted from 0x8040 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
> > > >
> > > > Can't they both start from 4MB aligned address provided the memory below
> > > > the kernel can be freed?
> > >
> > > Yes, they can both start from 4MB aligned address.
> > >
> > > >
> > > > > 2. Save memory by allowing users to place kernel just after the 
> > > > > runtime
> > > > > firmware at starting of RAM.
> > > >
> > > > If the firmware should be alive after kernel boot, it's memory is the 
> > > > only
> > > > part that should be reserved below the kernel. Otherwise, the entire 
> > > > region
> > > >  -  can be free.
> > > >
> > > > Using 4K pages for the swapper_pg_dir is quite a change and I'm not
> > > > convinced its really justified.
> > >
> > > I understand your concern about TLB performance and more page
> > > tables.
> > >
> > > Not just 2MB/4MB mappings, we should be able to create even 1GB
> > > mappings as well for good TLB performance.
> > >
> > > I suggest we should use best possible mapping size (4KB, 2MB, or
> > > 1GB) based on alignment of kernel load address. This way users can
> > > boot from any 4KB aligned address and setup_vm() will try to use
> > > biggest possible mapping size.
> > >
> > > For example, If the kernel load address is aligned to 2MB then we 2MB
> > > mappings bigger mappings and use fewer page tables. Same thing
> > > possible for 1GB mappings as well.
> >
> > I still don't get 

Re: [PATCH 3/3] RISC-V: Allow booting kernel from any 4KB aligned address

2019-03-15 Thread Anup Patel
On Fri, Mar 15, 2019 at 9:28 PM Mike Rapoport  wrote:
>
> On Thu, Mar 14, 2019 at 11:28:32PM +0530, Anup Patel wrote:
> > On Thu, Mar 14, 2019 at 12:23 PM Mike Rapoport  wrote:
> > >
> > > On Thu, Mar 14, 2019 at 02:36:01AM +0530, Anup Patel wrote:
> > > > On Thu, Mar 14, 2019 at 12:01 AM Mike Rapoport  
> > > > wrote:
> > > > >
> > > > > On Tue, Mar 12, 2019 at 10:08:22PM +, 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  address (which is not 
> > > > > > same as
> > > > > > RAM start address) then RISCV kernel will map PAGE_OFFSET virtual 
> > > > > > address
> > > > > > lineraly to  physical address and memory between RAM start and 
> > > > > > 
> > > > > > will be reserved/unusable.
> > > > > >
> > > > > > For example, RISCV64 kernel booted from 0x8020 will waste 2MB 
> > > > > > of RAM
> > > > > > and RISCV32 kernel booted from 0x8040 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
> > >
> > > Can't they both start from 4MB aligned address provided the memory below
> > > the kernel can be freed?
> >
> > Yes, they can both start from 4MB aligned address.
> >
> > >
> > > > 2. Save memory by allowing users to place kernel just after the runtime
> > > > firmware at starting of RAM.
> > >
> > > If the firmware should be alive after kernel boot, it's memory is the only
> > > part that should be reserved below the kernel. Otherwise, the entire 
> > > region
> > >  -  can be free.
> > >
> > > Using 4K pages for the swapper_pg_dir is quite a change and I'm not
> > > convinced its really justified.
> >
> > I understand your concern about TLB performance and more page
> > tables.
> >
> > Not just 2MB/4MB mappings, we should be able to create even 1GB
> > mappings as well for good TLB performance.
> >
> > I suggest we should use best possible mapping size (4KB, 2MB, or
> > 1GB) based on alignment of kernel load address. This way users can
> > boot from any 4KB aligned address and setup_vm() will try to use
> > biggest possible mapping size.
> >
> > For example, If the kernel load address is aligned to 2MB then we 2MB
> > mappings bigger mappings and use fewer page tables. Same thing
> > possible for 1GB mappings as well.
>
> I still don't get why it is that important to relax alignment of the kernel
> load address. Provided you can use the memory below the kernel, it really
> should not matter.

Irrespective to constraint on kernel load address, we certainly need
to allow memory below kernel to be usable but that's a separate change.

Currently, the memory below kernel is ignored by

Re: [PATCH 3/3] RISC-V: Allow booting kernel from any 4KB aligned address

2019-03-15 Thread Anup Patel
On Fri, Mar 15, 2019 at 9:28 PM Mike Rapoport  wrote:
>
> On Thu, Mar 14, 2019 at 11:28:32PM +0530, Anup Patel wrote:
> > On Thu, Mar 14, 2019 at 12:23 PM Mike Rapoport  wrote:
> > >
> > > On Thu, Mar 14, 2019 at 02:36:01AM +0530, Anup Patel wrote:
> > > > On Thu, Mar 14, 2019 at 12:01 AM Mike Rapoport  
> > > > wrote:
> > > > >
> > > > > On Tue, Mar 12, 2019 at 10:08:22PM +, 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  address (which is not 
> > > > > > same as
> > > > > > RAM start address) then RISCV kernel will map PAGE_OFFSET virtual 
> > > > > > address
> > > > > > lineraly to  physical address and memory between RAM start and 
> > > > > > 
> > > > > > will be reserved/unusable.
> > > > > >
> > > > > > For example, RISCV64 kernel booted from 0x8020 will waste 2MB 
> > > > > > of RAM
> > > > > > and RISCV32 kernel booted from 0x8040 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
> > >
> > > Can't they both start from 4MB aligned address provided the memory below
> > > the kernel can be freed?
> >
> > Yes, they can both start from 4MB aligned address.
> >
> > >
> > > > 2. Save memory by allowing users to place kernel just after the runtime
> > > > firmware at starting of RAM.
> > >
> > > If the firmware should be alive after kernel boot, it's memory is the only
> > > part that should be reserved below the kernel. Otherwise, the entire 
> > > region
> > >  -  can be free.
> > >
> > > Using 4K pages for the swapper_pg_dir is quite a change and I'm not
> > > convinced its really justified.
> >
> > I understand your concern about TLB performance and more page
> > tables.
> >
> > Not just 2MB/4MB mappings, we should be able to create even 1GB
> > mappings as well for good TLB performance.
> >
> > I suggest we should use best possible mapping size (4KB, 2MB, or
> > 1GB) based on alignment of kernel load address. This way users can
> > boot from any 4KB aligned address and setup_vm() will try to use
> > biggest possible mapping size.
> >
> > For example, If the kernel load address is aligned to 2MB then we 2MB
> > mappings bigger mappings and use fewer page tables. Same thing
> > possible for 1GB mappings as well.
>
> I still don't get why it is that important to relax alignment of the kernel
> load address. Provided you can use the memory below the kernel, it really
> should not matter.

The original idea was just to relax the alignment constraint on the kernel
load address.

What I am suggesting now is to improve this patch so that we can
dynamically select mapping size based on kernel 

Re: [PATCH 3/3] RISC-V: Allow booting kernel from any 4KB aligned address

2019-03-15 Thread Mike Rapoport
On Thu, Mar 14, 2019 at 11:28:32PM +0530, Anup Patel wrote:
> On Thu, Mar 14, 2019 at 12:23 PM Mike Rapoport  wrote:
> >
> > On Thu, Mar 14, 2019 at 02:36:01AM +0530, Anup Patel wrote:
> > > On Thu, Mar 14, 2019 at 12:01 AM Mike Rapoport  wrote:
> > > >
> > > > On Tue, Mar 12, 2019 at 10:08:22PM +, 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  address (which is not same 
> > > > > as
> > > > > RAM start address) then RISCV kernel will map PAGE_OFFSET virtual 
> > > > > address
> > > > > lineraly to  physical address and memory between RAM start and 
> > > > > 
> > > > > will be reserved/unusable.
> > > > >
> > > > > For example, RISCV64 kernel booted from 0x8020 will waste 2MB of 
> > > > > RAM
> > > > > and RISCV32 kernel booted from 0x8040 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
> >
> > Can't they both start from 4MB aligned address provided the memory below
> > the kernel can be freed?
> 
> Yes, they can both start from 4MB aligned address.
> 
> >
> > > 2. Save memory by allowing users to place kernel just after the runtime
> > > firmware at starting of RAM.
> >
> > If the firmware should be alive after kernel boot, it's memory is the only
> > part that should be reserved below the kernel. Otherwise, the entire region
> >  -  can be free.
> >
> > Using 4K pages for the swapper_pg_dir is quite a change and I'm not
> > convinced its really justified.
> 
> I understand your concern about TLB performance and more page
> tables.
> 
> Not just 2MB/4MB mappings, we should be able to create even 1GB
> mappings as well for good TLB performance.
> 
> I suggest we should use best possible mapping size (4KB, 2MB, or
> 1GB) based on alignment of kernel load address. This way users can
> boot from any 4KB aligned address and setup_vm() will try to use
> biggest possible mapping size.
> 
> For example, If the kernel load address is aligned to 2MB then we 2MB
> mappings bigger mappings and use fewer page tables. Same thing
> possible for 1GB mappings as well.

I still don't get why it is that important to relax alignment of the kernel
load address. Provided you can use the memory below the kernel, it really
should not matter.

> Regards,
> Anup
> 

-- 
Sincerely yours,
Mike.



Re: [PATCH 3/3] RISC-V: Allow booting kernel from any 4KB aligned address

2019-03-14 Thread Anup Patel
On Thu, Mar 14, 2019 at 12:23 PM Mike Rapoport  wrote:
>
> On Thu, Mar 14, 2019 at 02:36:01AM +0530, Anup Patel wrote:
> > On Thu, Mar 14, 2019 at 12:01 AM Mike Rapoport  wrote:
> > >
> > > On Tue, Mar 12, 2019 at 10:08:22PM +, 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  address (which is not same as
> > > > RAM start address) then RISCV kernel will map PAGE_OFFSET virtual 
> > > > address
> > > > lineraly to  physical address and memory between RAM start and 
> > > > 
> > > > will be reserved/unusable.
> > > >
> > > > For example, RISCV64 kernel booted from 0x8020 will waste 2MB of RAM
> > > > and RISCV32 kernel booted from 0x8040 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
>
> Can't they both start from 4MB aligned address provided the memory below
> the kernel can be freed?

Yes, they can both start from 4MB aligned address.

>
> > 2. Save memory by allowing users to place kernel just after the runtime
> > firmware at starting of RAM.
>
> If the firmware should be alive after kernel boot, it's memory is the only
> part that should be reserved below the kernel. Otherwise, the entire region
>  -  can be free.
>
> Using 4K pages for the swapper_pg_dir is quite a change and I'm not
> convinced its really justified.

I understand your concern about TLB performance and more page
tables.

Not just 2MB/4MB mappings, we should be able to create even 1GB
mappings as well for good TLB performance.

I suggest we should use best possible mapping size (4KB, 2MB, or
1GB) based on alignment of kernel load address. This way users can
boot from any 4KB aligned address and setup_vm() will try to use
biggest possible mapping size.

For example, If the kernel load address is aligned to 2MB then we 2MB
mappings bigger mappings and use fewer page tables. Same thing
possible for 1GB mappings as well.

Regards,
Anup


Re: [PATCH 3/3] RISC-V: Allow booting kernel from any 4KB aligned address

2019-03-14 Thread Mike Rapoport
On Thu, Mar 14, 2019 at 02:36:01AM +0530, Anup Patel wrote:
> On Thu, Mar 14, 2019 at 12:01 AM Mike Rapoport  wrote:
> >
> > On Tue, Mar 12, 2019 at 10:08:22PM +, 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  address (which is not same as
> > > RAM start address) then RISCV kernel will map PAGE_OFFSET virtual address
> > > lineraly to  physical address and memory between RAM start and 
> > > will be reserved/unusable.
> > >
> > > For example, RISCV64 kernel booted from 0x8020 will waste 2MB of RAM
> > > and RISCV32 kernel booted from 0x8040 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

Can't they both start from 4MB aligned address provided the memory below
the kernel can be freed?

> 2. Save memory by allowing users to place kernel just after the runtime
> firmware at starting of RAM.

If the firmware should be alive after kernel boot, it's memory is the only
part that should be reserved below the kernel. Otherwise, the entire region
 -  can be free.

Using 4K pages for the swapper_pg_dir is quite a change and I'm not
convinced its really justified.
 
> >
> > 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
> 

-- 
Sincerely yours,
Mike.



Re: [PATCH 3/3] RISC-V: Allow booting kernel from any 4KB aligned address

2019-03-13 Thread Anup Patel
On Thu, Mar 14, 2019 at 12:01 AM Mike Rapoport  wrote:
>
> On Tue, Mar 12, 2019 at 10:08:22PM +, 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  address (which is not same as
> > RAM start address) then RISCV kernel will map PAGE_OFFSET virtual address
> > lineraly to  physical address and memory between RAM start and 
> > will be reserved/unusable.
> >
> > For example, RISCV64 kernel booted from 0x8020 will waste 2MB of RAM
> > and RISCV32 kernel booted from 0x8040 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


Re: [PATCH 3/3] RISC-V: Allow booting kernel from any 4KB aligned address

2019-03-13 Thread Mike Rapoport
On Tue, Mar 12, 2019 at 10:08:22PM +, 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  address (which is not same as
> RAM start address) then RISCV kernel will map PAGE_OFFSET virtual address
> lineraly to  physical address and memory between RAM start and 
> will be reserved/unusable.
> 
> For example, RISCV64 kernel booted from 0x8020 will waste 2MB of RAM
> and RISCV32 kernel booted from 0x8040 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.

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:

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);
}
}
 
> With this patch in-place, the booting constraint for RISCV32 and RISCV64
> kernel is much more relaxed and we can now boot kernel very close to
> RAM start thereby minimizng memory wastage.
> 
> Signed-off-by: Anup Patel 
> ---
>  arch/riscv/include/asm/fixmap.h |   5 +
>  arch/riscv/include/asm/pgtable-64.h |   5 +
>  arch/riscv/include/asm/pgtable.h|   6 +-
>  arch/riscv/kernel/head.S|   1 +
>  arch/riscv/kernel/setup.c   |   4 +-
>  arch/riscv/mm/init.c| 357 +++-
>  6 files changed, 317 insertions(+), 61 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/fixmap.h b/arch/riscv/include/asm/fixmap.h
> index 57afe604b495..5cf53dd882e5 100644
> --- a/arch/riscv/include/asm/fixmap.h
> +++ b/arch/riscv/include/asm/fixmap.h
> @@ -21,6 +21,11 @@
>   */
>  enum fixed_addresses {
>   FIX_HOLE,
> +#define FIX_FDT_SIZE SZ_1M
> + FIX_FDT_END,
> + FIX_FDT = FIX_FDT_END + FIX_FDT_SIZE / PAGE_SIZE - 1,
> + FIX_PTE,
> + FIX_PMD,
>   FIX_EARLYCON_MEM_BASE,
>   __end_of_fixed_addresses
>  };
> diff --git a/arch/riscv/include/asm/pgtable-64.h 
> b/arch/riscv/include/asm/pgtable-64.h
> index 7aa0ea9bd8bb..56ecc3dc939d 100644
> --- a/arch/riscv/include/asm/pgtable-64.h
> +++ b/arch/riscv/include/asm/pgtable-64.h
> @@ -78,6 +78,11 @@ static inline pmd_t pfn_pmd(unsigned long pfn, pgprot_t 
> prot)
>   return __pmd((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
>  }
>  
> +static inline unsigned long _pmd_pfn(pmd_t pmd)
> +{
> + return pmd_val(pmd) >> _PAGE_PFN_SHIFT;
> +}
> +
>  #define pmd_ERROR(e) \
>   pr_err("%s:%d: bad pmd %016lx.\n", __FILE__, __LINE__, pmd_val(e))
>  
> diff --git a/arch/riscv/include/asm/pgtable.h 
> b/arch/riscv/include/asm/pgtable.h
> index 1141364d990e..05fa2115e736 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -121,12 +121,16 @@ static inline void pmd_clear(pmd_t *pmdp)
>   set_pmd(pmdp, __pmd(0));
>  }
>  
> -
>  static inline pgd_t pfn_pgd(unsigned long pfn, pgprot_t prot)
>  {
>   return __pgd((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
>  }
>  
> +static inline unsigned long _pgd_pfn(pgd_t pgd)
> +{
> + return pgd_val(pgd) >> _PAGE_PFN_SHIFT;
> +}
> +
>  #define pgd_index(addr) (((addr) >> PGDIR_SHIFT) & (PTRS_PER_PGD - 

[PATCH 3/3] RISC-V: Allow booting kernel from any 4KB aligned address

2019-03-12 Thread Anup Patel
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  address (which is not same as
RAM start address) then RISCV kernel will map PAGE_OFFSET virtual address
lineraly to  physical address and memory between RAM start and 
will be reserved/unusable.

For example, RISCV64 kernel booted from 0x8020 will waste 2MB of RAM
and RISCV32 kernel booted from 0x8040 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.

With this patch in-place, the booting constraint for RISCV32 and RISCV64
kernel is much more relaxed and we can now boot kernel very close to
RAM start thereby minimizng memory wastage.

Signed-off-by: Anup Patel 
---
 arch/riscv/include/asm/fixmap.h |   5 +
 arch/riscv/include/asm/pgtable-64.h |   5 +
 arch/riscv/include/asm/pgtable.h|   6 +-
 arch/riscv/kernel/head.S|   1 +
 arch/riscv/kernel/setup.c   |   4 +-
 arch/riscv/mm/init.c| 357 +++-
 6 files changed, 317 insertions(+), 61 deletions(-)

diff --git a/arch/riscv/include/asm/fixmap.h b/arch/riscv/include/asm/fixmap.h
index 57afe604b495..5cf53dd882e5 100644
--- a/arch/riscv/include/asm/fixmap.h
+++ b/arch/riscv/include/asm/fixmap.h
@@ -21,6 +21,11 @@
  */
 enum fixed_addresses {
FIX_HOLE,
+#define FIX_FDT_SIZE   SZ_1M
+   FIX_FDT_END,
+   FIX_FDT = FIX_FDT_END + FIX_FDT_SIZE / PAGE_SIZE - 1,
+   FIX_PTE,
+   FIX_PMD,
FIX_EARLYCON_MEM_BASE,
__end_of_fixed_addresses
 };
diff --git a/arch/riscv/include/asm/pgtable-64.h 
b/arch/riscv/include/asm/pgtable-64.h
index 7aa0ea9bd8bb..56ecc3dc939d 100644
--- a/arch/riscv/include/asm/pgtable-64.h
+++ b/arch/riscv/include/asm/pgtable-64.h
@@ -78,6 +78,11 @@ static inline pmd_t pfn_pmd(unsigned long pfn, pgprot_t prot)
return __pmd((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
 }
 
+static inline unsigned long _pmd_pfn(pmd_t pmd)
+{
+   return pmd_val(pmd) >> _PAGE_PFN_SHIFT;
+}
+
 #define pmd_ERROR(e) \
pr_err("%s:%d: bad pmd %016lx.\n", __FILE__, __LINE__, pmd_val(e))
 
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 1141364d990e..05fa2115e736 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -121,12 +121,16 @@ static inline void pmd_clear(pmd_t *pmdp)
set_pmd(pmdp, __pmd(0));
 }
 
-
 static inline pgd_t pfn_pgd(unsigned long pfn, pgprot_t prot)
 {
return __pgd((pfn << _PAGE_PFN_SHIFT) | pgprot_val(prot));
 }
 
+static inline unsigned long _pgd_pfn(pgd_t pgd)
+{
+   return pgd_val(pgd) >> _PAGE_PFN_SHIFT;
+}
+
 #define pgd_index(addr) (((addr) >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1))
 
 /* Locate an entry in the page global directory */
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 7966262b4f9d..12a3ec5eb8ab 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -63,6 +63,7 @@ clear_bss_done:
/* Initialize page tables and relocate to virtual addresses */
la sp, init_thread_union + THREAD_SIZE
la a0, _start
+   mv a1, s1
call setup_vm
call relocate
 
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index ecb654f6a79e..acdd0f74982b 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -62,7 +63,8 @@ unsigned long boot_cpu_hartid;
 
 void __init parse_dtb(unsigned int hartid, void *dtb)
 {
-   if (early_init_dt_scan(__va(dtb)))
+   dtb = (void *)fix_to_virt(FIX_FDT) + ((uintptr_t)dtb & ~PAGE_MASK);
+   if (early_init_dt_scan(dtb))
return;
 
pr_err("No DTB passed to the kernel\n");
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index f35299f2f3d5..ee55a4b90dec 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -1,14 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
+ * Copyright (C) 2019 Western Digital Corporation or its affiliates.
  * Copyright (C) 2012 Regents of the University of California
- *
- *   This program is free software; you can redistribute it and/or
- *   modify it under the terms of the GNU General Public License
- *   as published by the Free Software