2019-01-07 16:52:08

by Anup Patel

[permalink] [raw]
Subject: [PATCH 0/5] Fixmap support and MM cleanups

This patchset does:
1. Moves MM related code from kernel/setup.c to mm/init.c
2. Implements compile-time fixed mappings

Using fixed mappings available, we can now get earlyprints
even without SBI calls. For example, we can use kernel
parameter "earlycon=uart8250,mmio,0x10000000" to get early
prints on QEMU virt machine without using SBI calls.

The patchset is tested on QEMU virt machine. It is based on
Linux-5.0-rc1 and can be found at riscv_fixmap_v1 branch of:
https://github.com/avpatel/linux.git.

Anup Patel (5):
RISC-V: Move free_initrd_mem() to kernel/setup.c
RISC-V: Setup init_mm before parse_early_param()
RISC-V: Move setup_bootmem() to mm/init.c
RISC-V: Move setup_vm() to mm/init.c
RISC-V: Implement compile-time fixed mappings

arch/riscv/Kconfig | 3 +
arch/riscv/include/asm/fixmap.h | 52 +++++++++++++
arch/riscv/include/asm/pgtable.h | 1 +
arch/riscv/kernel/setup.c | 106 +++-----------------------
arch/riscv/mm/init.c | 123 ++++++++++++++++++++++++++++++-
5 files changed, 186 insertions(+), 99 deletions(-)
create mode 100644 arch/riscv/include/asm/fixmap.h

--
2.17.1



2019-01-07 16:52:30

by Anup Patel

[permalink] [raw]
Subject: [PATCH 1/5] RISC-V: Move free_initrd_mem() to kernel/setup.c

From: Anup Patel <[email protected]>

We move free_initrd_mem() to kernel/setup.c so that all initrd
related functions are in one place.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/kernel/setup.c | 4 ++++
arch/riscv/mm/init.c | 7 -------
2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index fc8006a042eb..64646d0b4d53 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -96,6 +96,10 @@ static void __init setup_initrd(void)
initrd_start = 0;
initrd_end = 0;
}
+
+void free_initrd_mem(unsigned long start, unsigned long end)
+{
+}
#endif /* CONFIG_BLK_DEV_INITRD */

pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 1d9bfaff60bc..f354486f9b78 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -14,7 +14,6 @@
#include <linux/init.h>
#include <linux/mm.h>
#include <linux/memblock.h>
-#include <linux/initrd.h>
#include <linux/swap.h>
#include <linux/sizes.h>

@@ -63,9 +62,3 @@ void free_initmem(void)
{
free_initmem_default(0);
}
-
-#ifdef CONFIG_BLK_DEV_INITRD
-void free_initrd_mem(unsigned long start, unsigned long end)
-{
-}
-#endif /* CONFIG_BLK_DEV_INITRD */
--
2.17.1


2019-01-07 16:53:25

by Anup Patel

[permalink] [raw]
Subject: [PATCH 4/5] RISC-V: Move setup_vm() to mm/init.c

From: Anup Patel <[email protected]>

The setup_vm() is responsible for setting up initial
page table hence should be placed in mm/init.c.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/kernel/setup.c | 49 --------------------------------------
arch/riscv/mm/init.c | 50 +++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 49a13b98e13f..e9c7f1a6db5a 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -102,55 +102,6 @@ void free_initrd_mem(unsigned long start, unsigned long end)
}
#endif /* CONFIG_BLK_DEV_INITRD */

-pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
-pgd_t trampoline_pg_dir[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
-
-#ifndef __PAGETABLE_PMD_FOLDED
-#define NUM_SWAPPER_PMDS ((uintptr_t)-PAGE_OFFSET >> PGDIR_SHIFT)
-pmd_t swapper_pmd[PTRS_PER_PMD*((-PAGE_OFFSET)/PGDIR_SIZE)] __page_aligned_bss;
-pmd_t trampoline_pmd[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
-#endif
-
-asmlinkage void __init setup_vm(void)
-{
- extern char _start;
- uintptr_t i;
- uintptr_t pa = (uintptr_t) &_start;
- pgprot_t prot = __pgprot(pgprot_val(PAGE_KERNEL) | _PAGE_EXEC);
-
- va_pa_offset = PAGE_OFFSET - pa;
- pfn_base = PFN_DOWN(pa);
-
- /* Sanity check alignment and size */
- BUG_ON((PAGE_OFFSET % PGDIR_SIZE) != 0);
- BUG_ON((pa % (PAGE_SIZE * PTRS_PER_PTE)) != 0);
-
-#ifndef __PAGETABLE_PMD_FOLDED
- trampoline_pg_dir[(PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD] =
- pfn_pgd(PFN_DOWN((uintptr_t)trampoline_pmd),
- __pgprot(_PAGE_TABLE));
- trampoline_pmd[0] = pfn_pmd(PFN_DOWN(pa), prot);
-
- for (i = 0; i < (-PAGE_OFFSET)/PGDIR_SIZE; ++i) {
- size_t o = (PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD + i;
- swapper_pg_dir[o] =
- pfn_pgd(PFN_DOWN((uintptr_t)swapper_pmd) + i,
- __pgprot(_PAGE_TABLE));
- }
- for (i = 0; i < ARRAY_SIZE(swapper_pmd); i++)
- swapper_pmd[i] = pfn_pmd(PFN_DOWN(pa + i * PMD_SIZE), prot);
-#else
- trampoline_pg_dir[(PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD] =
- pfn_pgd(PFN_DOWN(pa), prot);
-
- for (i = 0; i < (-PAGE_OFFSET)/PGDIR_SIZE; ++i) {
- size_t o = (PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD + i;
- swapper_pg_dir[o] =
- pfn_pgd(PFN_DOWN(pa + i * PGDIR_SIZE), prot);
- }
-#endif
-}
-
void __init parse_dtb(unsigned int hartid, void *dtb)
{
early_init_dt_scan(__va(dtb));
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index d0965979ee70..b487f76d6060 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -102,3 +102,53 @@ void __init setup_bootmem(void)
&memblock.memory, 0);
}
}
+
+pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
+pgd_t trampoline_pg_dir[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
+
+#ifndef __PAGETABLE_PMD_FOLDED
+#define NUM_SWAPPER_PMDS ((uintptr_t)-PAGE_OFFSET >> PGDIR_SHIFT)
+pmd_t swapper_pmd[PTRS_PER_PMD*((-PAGE_OFFSET)/PGDIR_SIZE)] __page_aligned_bss;
+pmd_t trampoline_pmd[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
+#endif
+
+asmlinkage void __init setup_vm(void)
+{
+ extern char _start;
+ size_t o;
+ uintptr_t i;
+ uintptr_t pa = (uintptr_t) &_start;
+ pgprot_t prot = __pgprot(pgprot_val(PAGE_KERNEL) | _PAGE_EXEC);
+
+ va_pa_offset = PAGE_OFFSET - pa;
+ pfn_base = PFN_DOWN(pa);
+
+ /* Sanity check alignment and size */
+ BUG_ON((PAGE_OFFSET % PGDIR_SIZE) != 0);
+ BUG_ON((pa % (PAGE_SIZE * PTRS_PER_PTE)) != 0);
+
+#ifndef __PAGETABLE_PMD_FOLDED
+ trampoline_pg_dir[(PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD] =
+ pfn_pgd(PFN_DOWN((uintptr_t)trampoline_pmd),
+ __pgprot(_PAGE_TABLE));
+ trampoline_pmd[0] = pfn_pmd(PFN_DOWN(pa), prot);
+
+ for (i = 0; i < (-PAGE_OFFSET)/PGDIR_SIZE; ++i) {
+ o = (PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD + i;
+ swapper_pg_dir[o] =
+ pfn_pgd(PFN_DOWN((uintptr_t)swapper_pmd) + i,
+ __pgprot(_PAGE_TABLE));
+ }
+ for (i = 0; i < ARRAY_SIZE(swapper_pmd); i++)
+ swapper_pmd[i] = pfn_pmd(PFN_DOWN(pa + i * PMD_SIZE), prot);
+#else
+ trampoline_pg_dir[(PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD] =
+ pfn_pgd(PFN_DOWN(pa), prot);
+
+ for (i = 0; i < (-PAGE_OFFSET)/PGDIR_SIZE; ++i) {
+ o = (PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD + i;
+ swapper_pg_dir[o] =
+ pfn_pgd(PFN_DOWN(pa + i * PGDIR_SIZE), prot);
+ }
+#endif
+}
--
2.17.1


2019-01-07 16:55:19

by Anup Patel

[permalink] [raw]
Subject: [PATCH 5/5] RISC-V: Implement compile-time fixed mappings

From: Anup Patel <[email protected]>

This patch implements compile-time virtual to physical
mappings. These compile-time fixed mappings can be used
by earlycon, ACPI, and early ioremap for creating fixed
mappings when FIX_EARLYCON_MEM=y.

To start with, we have enabled compile-time fixed
mappings for earlycon.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/Kconfig | 3 ++
arch/riscv/include/asm/fixmap.h | 52 +++++++++++++++++++++++++++++++++
arch/riscv/mm/init.c | 32 ++++++++++++++++++++
3 files changed, 87 insertions(+)
create mode 100644 arch/riscv/include/asm/fixmap.h

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index e0d7d61779a6..66094aba9a59 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -86,6 +86,9 @@ config GENERIC_CSUM
config GENERIC_HWEIGHT
def_bool y

+config FIX_EARLYCON_MEM
+ def_bool y
+
config PGTABLE_LEVELS
int
default 3 if 64BIT
diff --git a/arch/riscv/include/asm/fixmap.h b/arch/riscv/include/asm/fixmap.h
new file mode 100644
index 000000000000..88e99f506ab9
--- /dev/null
+++ b/arch/riscv/include/asm/fixmap.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 Western Digital Corporation or its affiliates.
+ *
+ * fixmap.h: compile-time virtual memory allocation
+ */
+
+#ifndef _ASM_RISCV_FIXMAP_H
+#define _ASM_RISCV_FIXMAP_H
+
+#ifndef __ASSEMBLY__
+
+#include <linux/kernel.h>
+#include <linux/sizes.h>
+#include <asm/page.h>
+#include <asm/pgtable.h>
+
+/*
+ * Here we define all the compile-time 'special' virtual
+ * addresses. The point is to have a constant address at
+ * compile time, but to set the physical address only
+ * in the boot process.
+ *
+ * These 'compile-time allocated' memory buffers are
+ * page-sized. Use set_fixmap(idx,phys) to associate
+ * physical memory with fixmap indices.
+ */
+enum fixed_addresses {
+ FIX_HOLE,
+ FIX_EARLYCON_MEM_BASE,
+ __end_of_fixed_addresses
+};
+
+#define FIXADDR_SIZE (__end_of_fixed_addresses * PAGE_SIZE)
+#define FIXADDR_TOP (PAGE_OFFSET)
+#define FIXADDR_START (FIXADDR_TOP - FIXADDR_SIZE)
+
+#define FIXMAP_PAGE_IO PAGE_KERNEL
+
+#define __early_set_fixmap __set_fixmap
+
+#define __late_set_fixmap __set_fixmap
+#define __late_clear_fixmap(idx) __set_fixmap((idx), 0, FIXMAP_PAGE_CLEAR)
+
+extern void __set_fixmap(enum fixed_addresses idx,
+ phys_addr_t phys, pgprot_t prot);
+
+#include <asm-generic/fixmap.h>
+
+#endif /* !__ASSEMBLY__ */
+
+#endif /* _ASM_RISCV_FIXMAP_H */
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index b487f76d6060..b10f1423c352 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -18,6 +18,7 @@
#include <linux/sizes.h>
#include <linux/of_fdt.h>

+#include <asm/fixmap.h>
#include <asm/tlbflush.h>
#include <asm/sections.h>
#include <asm/pgtable.h>
@@ -110,8 +111,28 @@ pgd_t trampoline_pg_dir[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
#define NUM_SWAPPER_PMDS ((uintptr_t)-PAGE_OFFSET >> PGDIR_SHIFT)
pmd_t swapper_pmd[PTRS_PER_PMD*((-PAGE_OFFSET)/PGDIR_SIZE)] __page_aligned_bss;
pmd_t trampoline_pmd[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
+pmd_t fixmap_pmd[PTRS_PER_PMD] __page_aligned_bss;
#endif

+pte_t fixmap_pte[PTRS_PER_PTE] __page_aligned_bss;
+
+void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot)
+{
+ unsigned long addr = __fix_to_virt(idx);
+ pte_t *ptep;
+
+ BUG_ON(idx <= FIX_HOLE || idx >= __end_of_fixed_addresses);
+
+ ptep = &fixmap_pte[pte_index(addr)];
+
+ if (pgprot_val(prot)) {
+ set_pte(ptep, pfn_pte(phys >> PAGE_SHIFT, prot));
+ } else {
+ pte_clear(&init_mm, addr, ptep);
+ local_flush_tlb_page(addr);
+ }
+}
+
asmlinkage void __init setup_vm(void)
{
extern char _start;
@@ -141,6 +162,13 @@ asmlinkage void __init setup_vm(void)
}
for (i = 0; i < ARRAY_SIZE(swapper_pmd); i++)
swapper_pmd[i] = pfn_pmd(PFN_DOWN(pa + i * PMD_SIZE), prot);
+
+ swapper_pg_dir[(FIXADDR_START >> PGDIR_SHIFT) % PTRS_PER_PGD] =
+ pfn_pgd(PFN_DOWN((uintptr_t)fixmap_pmd),
+ __pgprot(_PAGE_TABLE));
+ fixmap_pmd[(FIXADDR_START >> PMD_SHIFT) % PTRS_PER_PMD] =
+ pfn_pmd(PFN_DOWN((uintptr_t)fixmap_pte),
+ __pgprot(_PAGE_TABLE));
#else
trampoline_pg_dir[(PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD] =
pfn_pgd(PFN_DOWN(pa), prot);
@@ -150,5 +178,9 @@ asmlinkage void __init setup_vm(void)
swapper_pg_dir[o] =
pfn_pgd(PFN_DOWN(pa + i * PGDIR_SIZE), prot);
}
+
+ swapper_pg_dir[(FIXADDR_START >> PGDIR_SHIFT) % PTRS_PER_PGD] =
+ pfn_pgd(PFN_DOWN((uintptr_t)fixmap_pte),
+ __pgprot(_PAGE_TABLE));
#endif
}
--
2.17.1


2019-01-07 17:40:04

by Anup Patel

[permalink] [raw]
Subject: [PATCH 2/5] RISC-V: Setup init_mm before parse_early_param()

From: Anup Patel <[email protected]>

We should setup init_mm before doing parse_early_param()
in setup_arch() to be consistent with setup_arch() of
other architectures such as x86, ARM, and ARM64.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/kernel/setup.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 64646d0b4d53..f7132547b8b3 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -201,15 +201,15 @@ static void __init setup_bootmem(void)

void __init setup_arch(char **cmdline_p)
{
- *cmdline_p = boot_command_line;
-
- parse_early_param();
-
init_mm.start_code = (unsigned long) _stext;
init_mm.end_code = (unsigned long) _etext;
init_mm.end_data = (unsigned long) _edata;
init_mm.brk = (unsigned long) _end;

+ *cmdline_p = boot_command_line;
+
+ parse_early_param();
+
setup_bootmem();
paging_init();
unflatten_device_tree();
@@ -228,4 +228,3 @@ void __init setup_arch(char **cmdline_p)

riscv_fill_hwcap();
}
-
--
2.17.1


2019-01-07 19:54:47

by Anup Patel

[permalink] [raw]
Subject: [PATCH 3/5] RISC-V: Move setup_bootmem() to mm/init.c

From: Anup Patel <[email protected]>

The setup_bootmem() mainly populates memblocks and does
early memory reservations. The right location for this
function is mm/init.c.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/include/asm/pgtable.h | 1 +
arch/riscv/kernel/setup.c | 48 ++++----------------------------
arch/riscv/mm/init.c | 40 ++++++++++++++++++++++++++
3 files changed, 46 insertions(+), 43 deletions(-)

diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 16301966d65b..35893810cba3 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -404,6 +404,7 @@ static inline int ptep_clear_flush_young(struct vm_area_struct *vma,
#define kern_addr_valid(addr) (1) /* FIXME */
#endif

+extern void setup_bootmem(void);
extern void paging_init(void);

static inline void pgtable_cache_init(void)
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index f7132547b8b3..49a13b98e13f 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -156,49 +156,6 @@ void __init parse_dtb(unsigned int hartid, void *dtb)
early_init_dt_scan(__va(dtb));
}

-static void __init setup_bootmem(void)
-{
- struct memblock_region *reg;
- phys_addr_t mem_size = 0;
-
- /* Find the memory region containing the kernel */
- for_each_memblock(memory, reg) {
- phys_addr_t vmlinux_end = __pa(_end);
- phys_addr_t end = reg->base + reg->size;
-
- if (reg->base <= vmlinux_end && vmlinux_end <= end) {
- /*
- * Reserve from the start of the region to the end of
- * the kernel
- */
- memblock_reserve(reg->base, vmlinux_end - reg->base);
- mem_size = min(reg->size, (phys_addr_t)-PAGE_OFFSET);
- }
- }
- BUG_ON(mem_size == 0);
-
- set_max_mapnr(PFN_DOWN(mem_size));
- max_low_pfn = memblock_end_of_DRAM();
-
-#ifdef CONFIG_BLK_DEV_INITRD
- setup_initrd();
-#endif /* CONFIG_BLK_DEV_INITRD */
-
- early_init_fdt_reserve_self();
- early_init_fdt_scan_reserved_mem();
- memblock_allow_resize();
- memblock_dump_all();
-
- for_each_memblock(memory, reg) {
- unsigned long start_pfn = memblock_region_memory_base_pfn(reg);
- unsigned long end_pfn = memblock_region_memory_end_pfn(reg);
-
- memblock_set_node(PFN_PHYS(start_pfn),
- PFN_PHYS(end_pfn - start_pfn),
- &memblock.memory, 0);
- }
-}
-
void __init setup_arch(char **cmdline_p)
{
init_mm.start_code = (unsigned long) _stext;
@@ -211,6 +168,11 @@ void __init setup_arch(char **cmdline_p)
parse_early_param();

setup_bootmem();
+
+#ifdef CONFIG_BLK_DEV_INITRD
+ setup_initrd();
+#endif /* CONFIG_BLK_DEV_INITRD */
+
paging_init();
unflatten_device_tree();

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index f354486f9b78..d0965979ee70 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -16,6 +16,7 @@
#include <linux/memblock.h>
#include <linux/swap.h>
#include <linux/sizes.h>
+#include <linux/of_fdt.h>

#include <asm/tlbflush.h>
#include <asm/sections.h>
@@ -62,3 +63,42 @@ void free_initmem(void)
{
free_initmem_default(0);
}
+
+void __init setup_bootmem(void)
+{
+ struct memblock_region *reg;
+ phys_addr_t mem_size = 0;
+
+ /* Find the memory region containing the kernel */
+ for_each_memblock(memory, reg) {
+ phys_addr_t vmlinux_end = __pa(_end);
+ phys_addr_t end = reg->base + reg->size;
+
+ if (reg->base <= vmlinux_end && vmlinux_end <= end) {
+ /*
+ * Reserve from the start of the region to the end of
+ * the kernel
+ */
+ memblock_reserve(reg->base, vmlinux_end - reg->base);
+ mem_size = min(reg->size, (phys_addr_t)-PAGE_OFFSET);
+ }
+ }
+ BUG_ON(mem_size == 0);
+
+ set_max_mapnr(PFN_DOWN(mem_size));
+ max_low_pfn = memblock_end_of_DRAM();
+
+ early_init_fdt_reserve_self();
+ early_init_fdt_scan_reserved_mem();
+ memblock_allow_resize();
+ memblock_dump_all();
+
+ for_each_memblock(memory, reg) {
+ unsigned long start_pfn = memblock_region_memory_base_pfn(reg);
+ unsigned long end_pfn = memblock_region_memory_end_pfn(reg);
+
+ memblock_set_node(PFN_PHYS(start_pfn),
+ PFN_PHYS(end_pfn - start_pfn),
+ &memblock.memory, 0);
+ }
+}
--
2.17.1


2019-01-15 14:08:52

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 4/5] RISC-V: Move setup_vm() to mm/init.c

On Mon, Jan 07, 2019 at 09:40:46PM +0530, Anup Patel wrote:
> From: Anup Patel <[email protected]>
>
> The setup_vm() is responsible for setting up initial
> page table hence should be placed in mm/init.c.
>
> Signed-off-by: Anup Patel <[email protected]>

Looks fine,

Reviewed-by: Christoph Hellwig <[email protected]>

2019-01-15 14:17:35

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 5/5] RISC-V: Implement compile-time fixed mappings

On Mon, Jan 07, 2019 at 09:40:47PM +0530, Anup Patel wrote:
> From: Anup Patel <[email protected]>
>
> This patch implements compile-time virtual to physical
> mappings. These compile-time fixed mappings can be used
> by earlycon, ACPI, and early ioremap for creating fixed
> mappings when FIX_EARLYCON_MEM=y.
>
> To start with, we have enabled compile-time fixed
> mappings for earlycon.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/Kconfig | 3 ++
> arch/riscv/include/asm/fixmap.h | 52 +++++++++++++++++++++++++++++++++
> arch/riscv/mm/init.c | 32 ++++++++++++++++++++
> 3 files changed, 87 insertions(+)
> create mode 100644 arch/riscv/include/asm/fixmap.h
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index e0d7d61779a6..66094aba9a59 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -86,6 +86,9 @@ config GENERIC_CSUM
> config GENERIC_HWEIGHT
> def_bool y
>
> +config FIX_EARLYCON_MEM
> + def_bool y
> +

Can you please throw in a prep patch to move FIX_EARLYCON_MEM to
drivers/tty/serial/Kconfig and only select it from the architectures
that use it?

> + * fixmap.h: compile-time virtual memory allocation

No need to mention the header name.

> + */
> +
> +#ifndef _ASM_RISCV_FIXMAP_H
> +#define _ASM_RISCV_FIXMAP_H
> +
> +#ifndef __ASSEMBLY__

As far as I can tell we never include this header from assembly files,
so this ifdef should not be needed.

> +/*
> + * Here we define all the compile-time 'special' virtual
> + * addresses. The point is to have a constant address at
> + * compile time, but to set the physical address only
> + * in the boot process.
> + *
> + * These 'compile-time allocated' memory buffers are
> + * page-sized. Use set_fixmap(idx,phys) to associate
> + * physical memory with fixmap indices.
> + */

Please use up the available 80 chars per line for comments.

2019-01-15 15:48:29

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/5] RISC-V: Move setup_bootmem() to mm/init.c

On Mon, Jan 07, 2019 at 09:40:45PM +0530, Anup Patel wrote:
> From: Anup Patel <[email protected]>
>
> The setup_bootmem() mainly populates memblocks and does
> early memory reservations. The right location for this
> function is mm/init.c.
>
> Signed-off-by: Anup Patel <[email protected]>

Looks fine:

Reviewed-by: Christoph Hellwig <[email protected]>

Btw, please use up the available 72 chars per line in your changelogs
where possible.

2019-01-15 16:32:09

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 1/5] RISC-V: Move free_initrd_mem() to kernel/setup.c

On Mon, Jan 07, 2019 at 09:40:43PM +0530, Anup Patel wrote:
> From: Anup Patel <[email protected]>
>
> We move free_initrd_mem() to kernel/setup.c so that all initrd
> related functions are in one place.
>
> Signed-off-by: Anup Patel <[email protected]>

Looks fine:

Reviewed-by: Christoph Hellwig <[email protected]>

Talking about free_initrd_mem - don't we need to call free_reserved_area
to actually release the memory?

2019-01-15 16:52:20

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 2/5] RISC-V: Setup init_mm before parse_early_param()

On Mon, Jan 07, 2019 at 09:40:44PM +0530, Anup Patel wrote:
> From: Anup Patel <[email protected]>
>
> We should setup init_mm before doing parse_early_param()
> in setup_arch() to be consistent with setup_arch() of
> other architectures such as x86, ARM, and ARM64.
>
> Signed-off-by: Anup Patel <[email protected]>

Is there any good inherent reason why the order matters? Not that I
really care either way..

2019-01-19 10:36:31

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH 1/5] RISC-V: Move free_initrd_mem() to kernel/setup.c

On Tue, Jan 15, 2019 at 7:13 PM Christoph Hellwig <[email protected]> wrote:
>
> On Mon, Jan 07, 2019 at 09:40:43PM +0530, Anup Patel wrote:
> > From: Anup Patel <[email protected]>
> >
> > We move free_initrd_mem() to kernel/setup.c so that all initrd
> > related functions are in one place.
> >
> > Signed-off-by: Anup Patel <[email protected]>
>
> Looks fine:
>
> Reviewed-by: Christoph Hellwig <[email protected]>
>
> Talking about free_initrd_mem - don't we need to call free_reserved_area
> to actually release the memory?

Yes, we should. Thanks for pointing.

I will include separate patch for it.

Thanks,
Anup

2019-01-19 10:44:51

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH 2/5] RISC-V: Setup init_mm before parse_early_param()

On Tue, Jan 15, 2019 at 7:14 PM Christoph Hellwig <[email protected]> wrote:
>
> On Mon, Jan 07, 2019 at 09:40:44PM +0530, Anup Patel wrote:
> > From: Anup Patel <[email protected]>
> >
> > We should setup init_mm before doing parse_early_param()
> > in setup_arch() to be consistent with setup_arch() of
> > other architectures such as x86, ARM, and ARM64.
> >
> > Signed-off-by: Anup Patel <[email protected]>
>
> Is there any good inherent reason why the order matters? Not that I
> really care either way..

The parse_early_param() calls param callbacks in variety
of subsystems including MM. Doing init_mm setup before
parse_early_param() ensures that initial MM state is
available when MM param callbacks are called.

My intention was mainly to make boot-up flow similar
to x86, ARM and ARM64 so that it is easy to add
features already present in these arch ports.

Regards,
Anup

2019-01-19 10:45:19

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH 3/5] RISC-V: Move setup_bootmem() to mm/init.c

On Tue, Jan 15, 2019 at 7:14 PM Christoph Hellwig <[email protected]> wrote:
>
> On Mon, Jan 07, 2019 at 09:40:45PM +0530, Anup Patel wrote:
> > From: Anup Patel <[email protected]>
> >
> > The setup_bootmem() mainly populates memblocks and does
> > early memory reservations. The right location for this
> > function is mm/init.c.
> >
> > Signed-off-by: Anup Patel <[email protected]>
>
> Looks fine:
>
> Reviewed-by: Christoph Hellwig <[email protected]>
>
> Btw, please use up the available 72 chars per line in your changelogs
> where possible.

Sure will do.

Thanks,
Anup

2019-01-19 10:47:12

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH 5/5] RISC-V: Implement compile-time fixed mappings

On Tue, Jan 15, 2019 at 7:17 PM Christoph Hellwig <[email protected]> wrote:
>
> On Mon, Jan 07, 2019 at 09:40:47PM +0530, Anup Patel wrote:
> > From: Anup Patel <[email protected]>
> >
> > This patch implements compile-time virtual to physical
> > mappings. These compile-time fixed mappings can be used
> > by earlycon, ACPI, and early ioremap for creating fixed
> > mappings when FIX_EARLYCON_MEM=y.
> >
> > To start with, we have enabled compile-time fixed
> > mappings for earlycon.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > ---
> > arch/riscv/Kconfig | 3 ++
> > arch/riscv/include/asm/fixmap.h | 52 +++++++++++++++++++++++++++++++++
> > arch/riscv/mm/init.c | 32 ++++++++++++++++++++
> > 3 files changed, 87 insertions(+)
> > create mode 100644 arch/riscv/include/asm/fixmap.h
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index e0d7d61779a6..66094aba9a59 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -86,6 +86,9 @@ config GENERIC_CSUM
> > config GENERIC_HWEIGHT
> > def_bool y
> >
> > +config FIX_EARLYCON_MEM
> > + def_bool y
> > +
>
> Can you please throw in a prep patch to move FIX_EARLYCON_MEM to
> drivers/tty/serial/Kconfig and only select it from the architectures
> that use it?
>
> > + * fixmap.h: compile-time virtual memory allocation
>
> No need to mention the header name.
>
> > + */
> > +
> > +#ifndef _ASM_RISCV_FIXMAP_H
> > +#define _ASM_RISCV_FIXMAP_H
> > +
> > +#ifndef __ASSEMBLY__
>
> As far as I can tell we never include this header from assembly files,
> so this ifdef should not be needed.

Okay, will drop #ifndef

>
> > +/*
> > + * Here we define all the compile-time 'special' virtual
> > + * addresses. The point is to have a constant address at
> > + * compile time, but to set the physical address only
> > + * in the boot process.
> > + *
> > + * These 'compile-time allocated' memory buffers are
> > + * page-sized. Use set_fixmap(idx,phys) to associate
> > + * physical memory with fixmap indices.
> > + */
>
> Please use up the available 80 chars per line for comments.

Sure, will do.

Regards,
Anup

2019-01-19 11:46:43

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH 5/5] RISC-V: Implement compile-time fixed mappings

On Tue, Jan 15, 2019 at 7:17 PM Christoph Hellwig <[email protected]> wrote:
>
> On Mon, Jan 07, 2019 at 09:40:47PM +0530, Anup Patel wrote:
> > From: Anup Patel <[email protected]>
> >
> > This patch implements compile-time virtual to physical
> > mappings. These compile-time fixed mappings can be used
> > by earlycon, ACPI, and early ioremap for creating fixed
> > mappings when FIX_EARLYCON_MEM=y.
> >
> > To start with, we have enabled compile-time fixed
> > mappings for earlycon.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > ---
> > arch/riscv/Kconfig | 3 ++
> > arch/riscv/include/asm/fixmap.h | 52 +++++++++++++++++++++++++++++++++
> > arch/riscv/mm/init.c | 32 ++++++++++++++++++++
> > 3 files changed, 87 insertions(+)
> > create mode 100644 arch/riscv/include/asm/fixmap.h
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index e0d7d61779a6..66094aba9a59 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -86,6 +86,9 @@ config GENERIC_CSUM
> > config GENERIC_HWEIGHT
> > def_bool y
> >
> > +config FIX_EARLYCON_MEM
> > + def_bool y
> > +
>
> Can you please throw in a prep patch to move FIX_EARLYCON_MEM to
> drivers/tty/serial/Kconfig and only select it from the architectures
> that use it?

Implementing fixmap is totally optional for architectures so it
makes sense to have FIX_EARLYCON_MEM as architecture
specific kconfig option. There are many such architecture specific
options provided as "def_bool y" in arch/<xyz>/Kconfig. Also,
having such "def_bool y" kconfig options in arch/<xyz>/Kconfig
gives us idea about optional features implemented by <xyz>
arch support.

I don't agree with this suggestion. If you still feel this is right way
to go then please go ahead and send patch to move
FIX_EARLYCON_MEM option in drivers/tty/serial/Kconfig.

>
> > + * fixmap.h: compile-time virtual memory allocation
>
> No need to mention the header name.

Sure, will remove header name.

>
> > + */
> > +
> > +#ifndef _ASM_RISCV_FIXMAP_H
> > +#define _ASM_RISCV_FIXMAP_H
> > +
> > +#ifndef __ASSEMBLY__
>
> As far as I can tell we never include this header from assembly files,
> so this ifdef should not be needed.
>
> > +/*
> > + * Here we define all the compile-time 'special' virtual
> > + * addresses. The point is to have a constant address at
> > + * compile time, but to set the physical address only
> > + * in the boot process.
> > + *
> > + * These 'compile-time allocated' memory buffers are
> > + * page-sized. Use set_fixmap(idx,phys) to associate
> > + * physical memory with fixmap indices.
> > + */
>
> Please use up the available 80 chars per line for comments.

Regards,
Anup