[PATCH v8 0/4] riscv: Use PUD/P4D/PGD pages for the linear mapping

[PATCH v8 0/4] riscv: Use PUD/P4D/PGD pages for the linear mapping

[Alexandre Ghiti]

This patchset intends to improve tlb utilization by using hugepages for
the linear mapping.

As reported by Anup in v6, when STRICT_KERNEL_RWX is enabled, we must
take care of isolating the kernel text and rodata so that they are not
mapped with a PUD mapping which would then assign wrong permissions to
the whole region: it is achieved by introducing a new memblock API.

Another patch makes use of this new API in arm64 which used some sort of
hack to solve this issue: it was built/boot tested successfully.

base-commit-tag: v6.3-rc1

v8:
- Fix rv32, as reported by Anup
- Do not modify memblock_isolate_range and fixes comment, as suggested by Mike
- Use the new memblock API for crash kernel too in arm64, as suggested by Andrew
- Fix arm64 double mapping (which to me did not work in v7), but ends up not
  being pretty at all, will wait for comments from arm64 reviewers, but
  this patch can easily be dropped if they do not want it.

v7:
- Fix Anup bug report by introducing memblock_isolate_memory which
  allows us to split the memblock mappings and then avoid to map the
  the PUD which contains the kernel as read only
- Add a patch to arm64 to use this newly introduced API

v6:
- quiet LLVM warning by casting phys_ram_base into an unsigned long

v5:
- Fix nommu builds by getting rid of riscv_pfn_base in patch 1, thanks
  Conor
- Add RB from Andrew

v4:
- Rebase on top of v6.2-rc3, as noted by Conor
- Add Acked-by Rob

v3:
- Change the comment about initrd_start VA conversion so that it fits
  ARM64 and RISCV64 (and others in the future if needed), as suggested
  by Rob

v2:
- Add a comment on why RISCV64 does not need to set initrd_start/end that
  early in the boot process, as asked by Rob

Alexandre Ghiti (4):
  riscv: Get rid of riscv_pfn_base variable
  mm: Introduce memblock_isolate_memory
  arm64: Make use of memblock_isolate_memory for the linear mapping
  riscv: Use PUD/P4D/PGD pages for the linear mapping

 arch/arm64/mm/mmu.c           | 25 +++++++++++------
 arch/riscv/include/asm/page.h | 19 +++++++++++--
 arch/riscv/mm/init.c          | 53 ++++++++++++++++++++++++++++-------
 arch/riscv/mm/physaddr.c      | 16 +++++++++++
 drivers/of/fdt.c              | 11 ++++----
 include/linux/memblock.h      |  1 +
 mm/memblock.c                 | 20 +++++++++++++
 7 files changed, 119 insertions(+), 26 deletions(-)

-- 
2.37.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v8 1/4] riscv: Get rid of riscv_pfn_base variable

[Alexandre Ghiti]

Use directly phys_ram_base instead, riscv_pfn_base is just the pfn of
the address contained in phys_ram_base.

Even if there is no functional change intended in this patch, actually
setting phys_ram_base that early changes the behaviour of
kernel_mapping_pa_to_va during the early boot: phys_ram_base used to be
zero before this patch and now it is set to the physical start address of
the kernel. But it does not break the conversion of a kernel physical
address into a virtual address since kernel_mapping_pa_to_va should only
be used on kernel physical addresses, i.e. addresses greater than the
physical start address of the kernel.

Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Tested-by: Anup Patel <anup@brainfault.org>
---
 arch/riscv/include/asm/page.h | 3 +--
 arch/riscv/mm/init.c          | 6 +-----
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
index 7fed7c431928..8dc686f549b6 100644
--- a/arch/riscv/include/asm/page.h
+++ b/arch/riscv/include/asm/page.h
@@ -91,8 +91,7 @@ typedef struct page *pgtable_t;
 #endif
 
 #ifdef CONFIG_MMU
-extern unsigned long riscv_pfn_base;
-#define ARCH_PFN_OFFSET		(riscv_pfn_base)
+#define ARCH_PFN_OFFSET		(PFN_DOWN((unsigned long)phys_ram_base))
 #else
 #define ARCH_PFN_OFFSET		(PAGE_OFFSET >> PAGE_SHIFT)
 #endif /* CONFIG_MMU */
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 87f6a5d475a6..cc558d94559a 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -271,9 +271,6 @@ static void __init setup_bootmem(void)
 #ifdef CONFIG_MMU
 struct pt_alloc_ops pt_ops __initdata;
 
-unsigned long riscv_pfn_base __ro_after_init;
-EXPORT_SYMBOL(riscv_pfn_base);
-
 pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
 pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
 static pte_t fixmap_pte[PTRS_PER_PTE] __page_aligned_bss;
@@ -285,7 +282,6 @@ static pmd_t __maybe_unused early_dtb_pmd[PTRS_PER_PMD] __initdata __aligned(PAG
 
 #ifdef CONFIG_XIP_KERNEL
 #define pt_ops			(*(struct pt_alloc_ops *)XIP_FIXUP(&pt_ops))
-#define riscv_pfn_base         (*(unsigned long  *)XIP_FIXUP(&riscv_pfn_base))
 #define trampoline_pg_dir      ((pgd_t *)XIP_FIXUP(trampoline_pg_dir))
 #define fixmap_pte             ((pte_t *)XIP_FIXUP(fixmap_pte))
 #define early_pg_dir           ((pgd_t *)XIP_FIXUP(early_pg_dir))
@@ -985,7 +981,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
 	kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
 	kernel_map.va_kernel_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr;
 
-	riscv_pfn_base = PFN_DOWN(kernel_map.phys_addr);
+	phys_ram_base = kernel_map.phys_addr;
 
 	/*
 	 * The default maximal physical memory size is KERN_VIRT_SIZE for 32-bit
-- 
2.37.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v8 2/4] mm: Introduce memblock_isolate_memory

[Alexandre Ghiti]

This function allows to split a region in memblock.memory and will be
useful when setting up the linear mapping with STRICT_KERNEL_RWX: it
allows to isolate the kernel text/rodata and then avoid to map those
regions with a PUD/P4D/PGD.

Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Tested-by: Anup Patel <anup@brainfault.org>
---
 include/linux/memblock.h |  1 +
 mm/memblock.c            | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 50ad19662a32..2f7ef97c0da7 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -125,6 +125,7 @@ int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
 int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
 int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
 int memblock_clear_nomap(phys_addr_t base, phys_addr_t size);
+int memblock_isolate_memory(phys_addr_t base, phys_addr_t size);
 
 void memblock_free_all(void);
 void memblock_free(void *ptr, size_t size);
diff --git a/mm/memblock.c b/mm/memblock.c
index 25fd0626a9e7..e8c651a37012 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -805,6 +805,26 @@ static int __init_memblock memblock_isolate_range(struct memblock_type *type,
 	return 0;
 }
 
+/**
+ * memblock_isolate_memory - isolate given range in memblock.memory
+ * @base: base of range to isolate
+ * @size: size of range to isolate
+ *
+ * Isolates the given range in memblock.memory so that it does not share any
+ * region with other ranges.
+ *
+ * Return:
+ * 0 on success, -errno on failure.
+ */
+
+int __init_memblock memblock_isolate_memory(phys_addr_t base, phys_addr_t size)
+{
+	int start_rgn, end_rgn;
+
+	return memblock_isolate_range(&memblock.memory, base, size,
+				      &start_rgn, &end_rgn);
+}
+
 static int __init_memblock memblock_remove_range(struct memblock_type *type,
 					  phys_addr_t base, phys_addr_t size)
 {
-- 
2.37.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v8 3/4] arm64: Make use of memblock_isolate_memory for the linear mapping

[Alexandre Ghiti]

In order to isolate the kernel text mapping and the crash kernel
region, we used some sort of hack to isolate thoses ranges which consisted
in marking them as not mappable with memblock_mark_nomap.

Simply use the newly introduced memblock_isolate_memory function which does
exactly the same but does not uselessly mark the region as not mappable.

Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
 arch/arm64/mm/mmu.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 6f9d8898a025..387c2a065a09 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -548,19 +548,18 @@ static void __init map_mem(pgd_t *pgdp)
 
 	/*
 	 * Take care not to create a writable alias for the
-	 * read-only text and rodata sections of the kernel image.
-	 * So temporarily mark them as NOMAP to skip mappings in
-	 * the following for-loop
+	 * read-only text and rodata sections of the kernel image so isolate
+	 * those regions and map them after the for loop.
 	 */
-	memblock_mark_nomap(kernel_start, kernel_end - kernel_start);
+	memblock_isolate_memory(kernel_start, kernel_end - kernel_start);
 
 #ifdef CONFIG_KEXEC_CORE
 	if (crash_mem_map) {
 		if (defer_reserve_crashkernel())
 			flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
 		else if (crashk_res.end)
-			memblock_mark_nomap(crashk_res.start,
-			    resource_size(&crashk_res));
+			memblock_isolate_memory(crashk_res.start,
+						resource_size(&crashk_res));
 	}
 #endif
 
@@ -568,6 +567,17 @@ static void __init map_mem(pgd_t *pgdp)
 	for_each_mem_range(i, &start, &end) {
 		if (start >= end)
 			break;
+
+		if (start == kernel_start)
+			continue;
+
+#ifdef CONFIG_KEXEC_CORE
+		if (start == crashk_res.start &&
+		    crash_mem_map && !defer_reserve_crashkernel() &&
+		    crashk_res.end)
+			continue;
+#endif
+
 		/*
 		 * The linear map must allow allocation tags reading/writing
 		 * if MTE is present. Otherwise, it has the same attributes as
@@ -589,7 +599,6 @@ static void __init map_mem(pgd_t *pgdp)
 	 */
 	__map_memblock(pgdp, kernel_start, kernel_end,
 		       PAGE_KERNEL, NO_CONT_MAPPINGS);
-	memblock_clear_nomap(kernel_start, kernel_end - kernel_start);
 
 	/*
 	 * Use page-level mappings here so that we can shrink the region
@@ -603,8 +612,6 @@ static void __init map_mem(pgd_t *pgdp)
 				       crashk_res.end + 1,
 				       PAGE_KERNEL,
 				       NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS);
-			memblock_clear_nomap(crashk_res.start,
-					     resource_size(&crashk_res));
 		}
 	}
 #endif
-- 
2.37.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v8 4/4] riscv: Use PUD/P4D/PGD pages for the linear mapping

[Alexandre Ghiti]

During the early page table creation, we used to set the mapping for
PAGE_OFFSET to the kernel load address: but the kernel load address is
always offseted by PMD_SIZE which makes it impossible to use PUD/P4D/PGD
pages as this physical address is not aligned on PUD/P4D/PGD size (whereas
PAGE_OFFSET is).

But actually we don't have to establish this mapping (ie set va_pa_offset)
that early in the boot process because:

- first, setup_vm installs a temporary kernel mapping and among other
  things, discovers the system memory,
- then, setup_vm_final creates the final kernel mapping and takes
  advantage of the discovered system memory to create the linear
  mapping.

During the first phase, we don't know the start of the system memory and
then until the second phase is finished, we can't use the linear mapping at
all and phys_to_virt/virt_to_phys translations must not be used because it
would result in a different translation from the 'real' one once the final
mapping is installed.

So here we simply delay the initialization of va_pa_offset to after the
system memory discovery. But to make sure noone uses the linear mapping
before, we add some guard in the DEBUG_VIRTUAL config.

Finally we can use PUD/P4D/PGD hugepages when possible, which will result
in a better TLB utilization.

Note that:
- this does not apply to rv32 as the kernel mapping lies in the linear
  mapping.
- we rely on the firmware to protect itself using PMP.

Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Acked-by: Rob Herring <robh@kernel.org> # DT bits
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Tested-by: Anup Patel <anup@brainfault.org>
---
 arch/riscv/include/asm/page.h | 16 ++++++++++++
 arch/riscv/mm/init.c          | 49 ++++++++++++++++++++++++++++++-----
 arch/riscv/mm/physaddr.c      | 16 ++++++++++++
 drivers/of/fdt.c              | 11 ++++----
 4 files changed, 81 insertions(+), 11 deletions(-)

diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
index 8dc686f549b6..ea1a0e237211 100644
--- a/arch/riscv/include/asm/page.h
+++ b/arch/riscv/include/asm/page.h
@@ -90,6 +90,14 @@ typedef struct page *pgtable_t;
 #define PTE_FMT "%08lx"
 #endif
 
+#ifdef CONFIG_64BIT
+/*
+ * We override this value as its generic definition uses __pa too early in
+ * the boot process (before kernel_map.va_pa_offset is set).
+ */
+#define MIN_MEMBLOCK_ADDR      0
+#endif
+
 #ifdef CONFIG_MMU
 #define ARCH_PFN_OFFSET		(PFN_DOWN((unsigned long)phys_ram_base))
 #else
@@ -121,7 +129,11 @@ extern phys_addr_t phys_ram_base;
 #define is_linear_mapping(x)	\
 	((x) >= PAGE_OFFSET && (!IS_ENABLED(CONFIG_64BIT) || (x) < PAGE_OFFSET + KERN_VIRT_SIZE))
 
+#ifndef CONFIG_DEBUG_VIRTUAL
 #define linear_mapping_pa_to_va(x)	((void *)((unsigned long)(x) + kernel_map.va_pa_offset))
+#else
+void *linear_mapping_pa_to_va(unsigned long x);
+#endif
 #define kernel_mapping_pa_to_va(y)	({					\
 	unsigned long _y = (unsigned long)(y);					\
 	(IS_ENABLED(CONFIG_XIP_KERNEL) && _y < phys_ram_base) ?			\
@@ -130,7 +142,11 @@ extern phys_addr_t phys_ram_base;
 	})
 #define __pa_to_va_nodebug(x)		linear_mapping_pa_to_va(x)
 
+#ifndef CONFIG_DEBUG_VIRTUAL
 #define linear_mapping_va_to_pa(x)	((unsigned long)(x) - kernel_map.va_pa_offset)
+#else
+phys_addr_t linear_mapping_va_to_pa(unsigned long x);
+#endif
 #define kernel_mapping_va_to_pa(y) ({						\
 	unsigned long _y = (unsigned long)(y);					\
 	(IS_ENABLED(CONFIG_XIP_KERNEL) && _y < kernel_map.virt_addr + XIP_OFFSET) ? \
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index cc558d94559a..7af7cd201a9c 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -213,6 +213,14 @@ static void __init setup_bootmem(void)
 	phys_ram_end = memblock_end_of_DRAM();
 	if (!IS_ENABLED(CONFIG_XIP_KERNEL))
 		phys_ram_base = memblock_start_of_DRAM();
+
+	/*
+	 * In 64-bit, any use of __va/__pa before this point is wrong as we
+	 * did not know the start of DRAM before.
+	 */
+	if (IS_ENABLED(CONFIG_64BIT))
+		kernel_map.va_pa_offset = PAGE_OFFSET - phys_ram_base;
+
 	/*
 	 * memblock allocator is not aware of the fact that last 4K bytes of
 	 * the addressable memory can not be mapped because of IS_ERR_VALUE
@@ -667,9 +675,16 @@ void __init create_pgd_mapping(pgd_t *pgdp,
 
 static uintptr_t __init best_map_size(phys_addr_t base, phys_addr_t size)
 {
-	/* Upgrade to PMD_SIZE mappings whenever possible */
-	base &= PMD_SIZE - 1;
-	if (!base && size >= PMD_SIZE)
+	if (!(base & (PGDIR_SIZE - 1)) && size >= PGDIR_SIZE)
+		return PGDIR_SIZE;
+
+	if (!(base & (P4D_SIZE - 1)) && size >= P4D_SIZE)
+		return P4D_SIZE;
+
+	if (!(base & (PUD_SIZE - 1)) && size >= PUD_SIZE)
+		return PUD_SIZE;
+
+	if (!(base & (PMD_SIZE - 1)) && size >= PMD_SIZE)
 		return PMD_SIZE;
 
 	return PAGE_SIZE;
@@ -978,11 +993,22 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
 	set_satp_mode();
 #endif
 
-	kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
+	/*
+	 * In 64-bit, we defer the setup of va_pa_offset to setup_bootmem,
+	 * where we have the system memory layout: this allows us to align
+	 * the physical and virtual mappings and then make use of PUD/P4D/PGD
+	 * for the linear mapping. This is only possible because the kernel
+	 * mapping lies outside the linear mapping.
+	 * In 32-bit however, as the kernel resides in the linear mapping,
+	 * setup_vm_final can not change the mapping established here,
+	 * otherwise the same kernel addresses would get mapped to different
+	 * physical addresses (if the start of dram is different from the
+	 * kernel physical address start).
+	 */
+	kernel_map.va_pa_offset = IS_ENABLED(CONFIG_64BIT) ?
+				0UL : PAGE_OFFSET - kernel_map.phys_addr;
 	kernel_map.va_kernel_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr;
 
-	phys_ram_base = kernel_map.phys_addr;
-
 	/*
 	 * The default maximal physical memory size is KERN_VIRT_SIZE for 32-bit
 	 * kernel, whereas for 64-bit kernel, the end of the virtual address
@@ -1097,6 +1123,17 @@ static void __init setup_vm_final(void)
 			   __pa_symbol(fixmap_pgd_next),
 			   PGDIR_SIZE, PAGE_TABLE);
 
+#ifdef CONFIG_STRICT_KERNEL_RWX
+	/*
+	 * Isolate the kernel text and rodata linear so they don't
+	 * get mapped with a PUD in the linear mapping.
+	 */
+	memblock_isolate_memory(__pa_symbol(_start),
+				__init_data_begin - _start);
+	memblock_isolate_memory(__pa_symbol(__start_rodata),
+				__start_rodata - _data);
+#endif
+
 	/* Map all memory banks in the linear mapping */
 	for_each_mem_range(i, &start, &end) {
 		if (start >= end)
diff --git a/arch/riscv/mm/physaddr.c b/arch/riscv/mm/physaddr.c
index 9b18bda74154..18706f457da7 100644
--- a/arch/riscv/mm/physaddr.c
+++ b/arch/riscv/mm/physaddr.c
@@ -33,3 +33,19 @@ phys_addr_t __phys_addr_symbol(unsigned long x)
 	return __va_to_pa_nodebug(x);
 }
 EXPORT_SYMBOL(__phys_addr_symbol);
+
+phys_addr_t linear_mapping_va_to_pa(unsigned long x)
+{
+	BUG_ON(!kernel_map.va_pa_offset);
+
+	return ((unsigned long)(x) - kernel_map.va_pa_offset);
+}
+EXPORT_SYMBOL(linear_mapping_va_to_pa);
+
+void *linear_mapping_pa_to_va(unsigned long x)
+{
+	BUG_ON(!kernel_map.va_pa_offset);
+
+	return ((void *)((unsigned long)(x) + kernel_map.va_pa_offset));
+}
+EXPORT_SYMBOL(linear_mapping_pa_to_va);
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index d1a68b6d03b3..d14735a81301 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -887,12 +887,13 @@ const void * __init of_flat_dt_match_machine(const void *default_match,
 static void __early_init_dt_declare_initrd(unsigned long start,
 					   unsigned long end)
 {
-	/* ARM64 would cause a BUG to occur here when CONFIG_DEBUG_VM is
-	 * enabled since __va() is called too early. ARM64 does make use
-	 * of phys_initrd_start/phys_initrd_size so we can skip this
-	 * conversion.
+	/*
+	 * __va() is not yet available this early on some platforms. In that
+	 * case, the platform uses phys_initrd_start/phys_initrd_size instead
+	 * and does the VA conversion itself.
 	 */
-	if (!IS_ENABLED(CONFIG_ARM64)) {
+	if (!IS_ENABLED(CONFIG_ARM64) &&
+	    !(IS_ENABLED(CONFIG_RISCV) && IS_ENABLED(CONFIG_64BIT))) {
 		initrd_start = (unsigned long)__va(start);
 		initrd_end = (unsigned long)__va(end);
 		initrd_below_start_ok = 1;
-- 
2.37.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v8 2/4] mm: Introduce memblock_isolate_memory

[Mike Rapoport]

Hi Alexandre,

On Thu, Mar 16, 2023 at 02:17:09PM +0100, Alexandre Ghiti wrote:
> This function allows to split a region in memblock.memory and will be
> useful when setting up the linear mapping with STRICT_KERNEL_RWX: it
> allows to isolate the kernel text/rodata and then avoid to map those
> regions with a PUD/P4D/PGD.
 
Sorry I've missed it last time. The changelog is fine in the context of
this series, but if you look at it as a part of memblock changelog it
doesn't provide enough background on why memblock_isolate_memory() is
useful.

Can you please add more context so it would be self explanatory?

> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> Reviewed-by: Anup Patel <anup@brainfault.org>
> Tested-by: Anup Patel <anup@brainfault.org>
> ---
>  include/linux/memblock.h |  1 +
>  mm/memblock.c            | 20 ++++++++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 50ad19662a32..2f7ef97c0da7 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -125,6 +125,7 @@ int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
>  int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
>  int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
>  int memblock_clear_nomap(phys_addr_t base, phys_addr_t size);
> +int memblock_isolate_memory(phys_addr_t base, phys_addr_t size);
>  
>  void memblock_free_all(void);
>  void memblock_free(void *ptr, size_t size);
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 25fd0626a9e7..e8c651a37012 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -805,6 +805,26 @@ static int __init_memblock memblock_isolate_range(struct memblock_type *type,
>  	return 0;
>  }
>  
> +/**
> + * memblock_isolate_memory - isolate given range in memblock.memory
> + * @base: base of range to isolate
> + * @size: size of range to isolate
> + *
> + * Isolates the given range in memblock.memory so that it does not share any
> + * region with other ranges.
> + *
> + * Return:
> + * 0 on success, -errno on failure.
> + */
> +
> +int __init_memblock memblock_isolate_memory(phys_addr_t base, phys_addr_t size)
> +{
> +	int start_rgn, end_rgn;
> +
> +	return memblock_isolate_range(&memblock.memory, base, size,
> +				      &start_rgn, &end_rgn);
> +}
> +
>  static int __init_memblock memblock_remove_range(struct memblock_type *type,
>  					  phys_addr_t base, phys_addr_t size)
>  {
> -- 
> 2.37.2
> 

-- 
Sincerely yours,
Mike.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v8 2/4] mm: Introduce memblock_isolate_memory

[Alexandre Ghiti]

Hi Mike,

On 3/16/23 21:12, Mike Rapoport wrote:
> Hi Alexandre,
>
> On Thu, Mar 16, 2023 at 02:17:09PM +0100, Alexandre Ghiti wrote:
>> This function allows to split a region in memblock.memory and will be
>> useful when setting up the linear mapping with STRICT_KERNEL_RWX: it
>> allows to isolate the kernel text/rodata and then avoid to map those
>> regions with a PUD/P4D/PGD.
>   
> Sorry I've missed it last time. The changelog is fine in the context of


No worries :)


> this series, but if you look at it as a part of memblock changelog it
> doesn't provide enough background on why memblock_isolate_memory() is
> useful.
>
> Can you please add more context so it would be self explanatory?


What about: "memblock.memory contains the list of memory regions and a 
memory region can cover memory that will be mapped with different 
permissions. So to ease the mapping process, allow to isolate those 
regions by introducing a new function called memblock_isolate_memory. 
This will be used in arch specific code to isolate the kernel 
text/rodata regions when STRICT_KERNEL_RWX is enabled so that we avoid 
mapping them with PUD/P4D/PGD mappings."

Thanks,

Alex


>
>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>> Reviewed-by: Anup Patel <anup@brainfault.org>
>> Tested-by: Anup Patel <anup@brainfault.org>
>> ---
>>   include/linux/memblock.h |  1 +
>>   mm/memblock.c            | 20 ++++++++++++++++++++
>>   2 files changed, 21 insertions(+)
>>
>> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
>> index 50ad19662a32..2f7ef97c0da7 100644
>> --- a/include/linux/memblock.h
>> +++ b/include/linux/memblock.h
>> @@ -125,6 +125,7 @@ int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
>>   int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
>>   int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
>>   int memblock_clear_nomap(phys_addr_t base, phys_addr_t size);
>> +int memblock_isolate_memory(phys_addr_t base, phys_addr_t size);
>>   
>>   void memblock_free_all(void);
>>   void memblock_free(void *ptr, size_t size);
>> diff --git a/mm/memblock.c b/mm/memblock.c
>> index 25fd0626a9e7..e8c651a37012 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -805,6 +805,26 @@ static int __init_memblock memblock_isolate_range(struct memblock_type *type,
>>   	return 0;
>>   }
>>   
>> +/**
>> + * memblock_isolate_memory - isolate given range in memblock.memory
>> + * @base: base of range to isolate
>> + * @size: size of range to isolate
>> + *
>> + * Isolates the given range in memblock.memory so that it does not share any
>> + * region with other ranges.
>> + *
>> + * Return:
>> + * 0 on success, -errno on failure.
>> + */
>> +
>> +int __init_memblock memblock_isolate_memory(phys_addr_t base, phys_addr_t size)
>> +{
>> +	int start_rgn, end_rgn;
>> +
>> +	return memblock_isolate_range(&memblock.memory, base, size,
>> +				      &start_rgn, &end_rgn);
>> +}
>> +
>>   static int __init_memblock memblock_remove_range(struct memblock_type *type,
>>   					  phys_addr_t base, phys_addr_t size)
>>   {
>> -- 
>> 2.37.2
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v8 2/4] mm: Introduce memblock_isolate_memory

[Mike Rapoport]

On Mon, Mar 20, 2023 at 11:54:14AM +0100, Alexandre Ghiti wrote:
> Hi Mike,
> 
> On 3/16/23 21:12, Mike Rapoport wrote:
> > Hi Alexandre,
> > 
> > On Thu, Mar 16, 2023 at 02:17:09PM +0100, Alexandre Ghiti wrote:
> > > This function allows to split a region in memblock.memory and will be
> > > useful when setting up the linear mapping with STRICT_KERNEL_RWX: it
> > > allows to isolate the kernel text/rodata and then avoid to map those
> > > regions with a PUD/P4D/PGD.
> > Sorry I've missed it last time. The changelog is fine in the context of
> 
> 
> No worries :)
> 
> 
> > this series, but if you look at it as a part of memblock changelog it
> > doesn't provide enough background on why memblock_isolate_memory() is
> > useful.
> > 
> > Can you please add more context so it would be self explanatory?
> 
> 
> What about: "memblock.memory contains the list of memory regions and a
> memory region can cover memory that will be mapped with different
> permissions. So to ease the mapping process, allow to isolate those regions
> by introducing a new function called memblock_isolate_memory. This will be
> used in arch specific code to isolate the kernel text/rodata regions when
> STRICT_KERNEL_RWX is enabled so that we avoid mapping them with PUD/P4D/PGD
> mappings."

With this change

... STRICT_KERNEL_RWX is enabled so that they can be mapped with base pages.

Acked-by: Mike Rapoport (IBM) <rppt@kernel.org>

> Thanks,
> 
> Alex
> 
> 
> > 
> > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > > Reviewed-by: Anup Patel <anup@brainfault.org>
> > > Tested-by: Anup Patel <anup@brainfault.org>
> > > ---
> > >   include/linux/memblock.h |  1 +
> > >   mm/memblock.c            | 20 ++++++++++++++++++++
> > >   2 files changed, 21 insertions(+)
> > > 
> > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > > index 50ad19662a32..2f7ef97c0da7 100644
> > > --- a/include/linux/memblock.h
> > > +++ b/include/linux/memblock.h
> > > @@ -125,6 +125,7 @@ int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
> > >   int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
> > >   int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
> > >   int memblock_clear_nomap(phys_addr_t base, phys_addr_t size);
> > > +int memblock_isolate_memory(phys_addr_t base, phys_addr_t size);
> > >   void memblock_free_all(void);
> > >   void memblock_free(void *ptr, size_t size);
> > > diff --git a/mm/memblock.c b/mm/memblock.c
> > > index 25fd0626a9e7..e8c651a37012 100644
> > > --- a/mm/memblock.c
> > > +++ b/mm/memblock.c
> > > @@ -805,6 +805,26 @@ static int __init_memblock memblock_isolate_range(struct memblock_type *type,
> > >   	return 0;
> > >   }
> > > +/**
> > > + * memblock_isolate_memory - isolate given range in memblock.memory
> > > + * @base: base of range to isolate
> > > + * @size: size of range to isolate
> > > + *
> > > + * Isolates the given range in memblock.memory so that it does not share any
> > > + * region with other ranges.
> > > + *
> > > + * Return:
> > > + * 0 on success, -errno on failure.
> > > + */
> > > +
> > > +int __init_memblock memblock_isolate_memory(phys_addr_t base, phys_addr_t size)
> > > +{
> > > +	int start_rgn, end_rgn;
> > > +
> > > +	return memblock_isolate_range(&memblock.memory, base, size,
> > > +				      &start_rgn, &end_rgn);
> > > +}
> > > +
> > >   static int __init_memblock memblock_remove_range(struct memblock_type *type,
> > >   					  phys_addr_t base, phys_addr_t size)
> > >   {
> > > -- 
> > > 2.37.2
> > > 
> 

-- 
Sincerely yours,
Mike.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v8 2/4] mm: Introduce memblock_isolate_memory

[Alexandre Ghiti]

On 3/20/23 18:44, Mike Rapoport wrote:
> On Mon, Mar 20, 2023 at 11:54:14AM +0100, Alexandre Ghiti wrote:
>> Hi Mike,
>>
>> On 3/16/23 21:12, Mike Rapoport wrote:
>>> Hi Alexandre,
>>>
>>> On Thu, Mar 16, 2023 at 02:17:09PM +0100, Alexandre Ghiti wrote:
>>>> This function allows to split a region in memblock.memory and will be
>>>> useful when setting up the linear mapping with STRICT_KERNEL_RWX: it
>>>> allows to isolate the kernel text/rodata and then avoid to map those
>>>> regions with a PUD/P4D/PGD.
>>> Sorry I've missed it last time. The changelog is fine in the context of
>>
>> No worries :)
>>
>>
>>> this series, but if you look at it as a part of memblock changelog it
>>> doesn't provide enough background on why memblock_isolate_memory() is
>>> useful.
>>>
>>> Can you please add more context so it would be self explanatory?
>>
>> What about: "memblock.memory contains the list of memory regions and a
>> memory region can cover memory that will be mapped with different
>> permissions. So to ease the mapping process, allow to isolate those regions
>> by introducing a new function called memblock_isolate_memory. This will be
>> used in arch specific code to isolate the kernel text/rodata regions when
>> STRICT_KERNEL_RWX is enabled so that we avoid mapping them with PUD/P4D/PGD
>> mappings."
> With this change
>
> ... STRICT_KERNEL_RWX is enabled so that they can be mapped with base pages.


Actually they will get mapped with PMD mappings :) I'll just append: "or 
PMD mapping" to your sentence above if that's ok with you.


>
> Acked-by: Mike Rapoport (IBM) <rppt@kernel.org>


Thanks for your review,

Alex


>
>> Thanks,
>>
>> Alex
>>
>>
>>>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>>>> Reviewed-by: Anup Patel <anup@brainfault.org>
>>>> Tested-by: Anup Patel <anup@brainfault.org>
>>>> ---
>>>>    include/linux/memblock.h |  1 +
>>>>    mm/memblock.c            | 20 ++++++++++++++++++++
>>>>    2 files changed, 21 insertions(+)
>>>>
>>>> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
>>>> index 50ad19662a32..2f7ef97c0da7 100644
>>>> --- a/include/linux/memblock.h
>>>> +++ b/include/linux/memblock.h
>>>> @@ -125,6 +125,7 @@ int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
>>>>    int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
>>>>    int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
>>>>    int memblock_clear_nomap(phys_addr_t base, phys_addr_t size);
>>>> +int memblock_isolate_memory(phys_addr_t base, phys_addr_t size);
>>>>    void memblock_free_all(void);
>>>>    void memblock_free(void *ptr, size_t size);
>>>> diff --git a/mm/memblock.c b/mm/memblock.c
>>>> index 25fd0626a9e7..e8c651a37012 100644
>>>> --- a/mm/memblock.c
>>>> +++ b/mm/memblock.c
>>>> @@ -805,6 +805,26 @@ static int __init_memblock memblock_isolate_range(struct memblock_type *type,
>>>>    	return 0;
>>>>    }
>>>> +/**
>>>> + * memblock_isolate_memory - isolate given range in memblock.memory
>>>> + * @base: base of range to isolate
>>>> + * @size: size of range to isolate
>>>> + *
>>>> + * Isolates the given range in memblock.memory so that it does not share any
>>>> + * region with other ranges.
>>>> + *
>>>> + * Return:
>>>> + * 0 on success, -errno on failure.
>>>> + */
>>>> +
>>>> +int __init_memblock memblock_isolate_memory(phys_addr_t base, phys_addr_t size)
>>>> +{
>>>> +	int start_rgn, end_rgn;
>>>> +
>>>> +	return memblock_isolate_range(&memblock.memory, base, size,
>>>> +				      &start_rgn, &end_rgn);
>>>> +}
>>>> +
>>>>    static int __init_memblock memblock_remove_range(struct memblock_type *type,
>>>>    					  phys_addr_t base, phys_addr_t size)
>>>>    {
>>>> -- 
>>>> 2.37.2
>>>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v8 2/4] mm: Introduce memblock_isolate_memory

[Mike Rapoport]

On Thu, Mar 23, 2023 at 12:52:36PM +0100, Alexandre Ghiti wrote:
> 
> On 3/20/23 18:44, Mike Rapoport wrote:
> > On Mon, Mar 20, 2023 at 11:54:14AM +0100, Alexandre Ghiti wrote:
> > > Hi Mike,
> > > 
> > > On 3/16/23 21:12, Mike Rapoport wrote:
> > > > Hi Alexandre,
> > > > 
> > > > On Thu, Mar 16, 2023 at 02:17:09PM +0100, Alexandre Ghiti wrote:
> > > > > This function allows to split a region in memblock.memory and will be
> > > > > useful when setting up the linear mapping with STRICT_KERNEL_RWX: it
> > > > > allows to isolate the kernel text/rodata and then avoid to map those
> > > > > regions with a PUD/P4D/PGD.
> > > > Sorry I've missed it last time. The changelog is fine in the context of
> > > 
> > > No worries :)
> > > 
> > > 
> > > > this series, but if you look at it as a part of memblock changelog it
> > > > doesn't provide enough background on why memblock_isolate_memory() is
> > > > useful.
> > > > 
> > > > Can you please add more context so it would be self explanatory?
> > > 
> > > What about: "memblock.memory contains the list of memory regions and a
> > > memory region can cover memory that will be mapped with different
> > > permissions. So to ease the mapping process, allow to isolate those regions
> > > by introducing a new function called memblock_isolate_memory. This will be
> > > used in arch specific code to isolate the kernel text/rodata regions when
> > > STRICT_KERNEL_RWX is enabled so that we avoid mapping them with PUD/P4D/PGD
> > > mappings."
> > With this change
> > 
> > ... STRICT_KERNEL_RWX is enabled so that they can be mapped with base pages.
> 
> 
> Actually they will get mapped with PMD mappings :) I'll just append: "or PMD
> mapping" to your sentence above if that's ok with you.

Didn't read the rest of the patches :)

.. STRICT_KERNEL_RWX is enabled so that they can be mapped with differently
than the rest of the memory.

Does it cover the usescases?

> > 
> > Acked-by: Mike Rapoport (IBM) <rppt@kernel.org>
> 
> 
> Thanks for your review,
> 
> Alex
> 
> 
> > 
> > > Thanks,
> > > 
> > > Alex
> > > 
> > > 
> > > > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > > > > Reviewed-by: Anup Patel <anup@brainfault.org>
> > > > > Tested-by: Anup Patel <anup@brainfault.org>
> > > > > ---
> > > > >    include/linux/memblock.h |  1 +
> > > > >    mm/memblock.c            | 20 ++++++++++++++++++++
> > > > >    2 files changed, 21 insertions(+)
> > > > > 
> > > > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > > > > index 50ad19662a32..2f7ef97c0da7 100644
> > > > > --- a/include/linux/memblock.h
> > > > > +++ b/include/linux/memblock.h
> > > > > @@ -125,6 +125,7 @@ int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
> > > > >    int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
> > > > >    int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
> > > > >    int memblock_clear_nomap(phys_addr_t base, phys_addr_t size);
> > > > > +int memblock_isolate_memory(phys_addr_t base, phys_addr_t size);
> > > > >    void memblock_free_all(void);
> > > > >    void memblock_free(void *ptr, size_t size);
> > > > > diff --git a/mm/memblock.c b/mm/memblock.c
> > > > > index 25fd0626a9e7..e8c651a37012 100644
> > > > > --- a/mm/memblock.c
> > > > > +++ b/mm/memblock.c
> > > > > @@ -805,6 +805,26 @@ static int __init_memblock memblock_isolate_range(struct memblock_type *type,
> > > > >    	return 0;
> > > > >    }
> > > > > +/**
> > > > > + * memblock_isolate_memory - isolate given range in memblock.memory
> > > > > + * @base: base of range to isolate
> > > > > + * @size: size of range to isolate
> > > > > + *
> > > > > + * Isolates the given range in memblock.memory so that it does not share any
> > > > > + * region with other ranges.
> > > > > + *
> > > > > + * Return:
> > > > > + * 0 on success, -errno on failure.
> > > > > + */
> > > > > +
> > > > > +int __init_memblock memblock_isolate_memory(phys_addr_t base, phys_addr_t size)
> > > > > +{
> > > > > +	int start_rgn, end_rgn;
> > > > > +
> > > > > +	return memblock_isolate_range(&memblock.memory, base, size,
> > > > > +				      &start_rgn, &end_rgn);
> > > > > +}
> > > > > +
> > > > >    static int __init_memblock memblock_remove_range(struct memblock_type *type,
> > > > >    					  phys_addr_t base, phys_addr_t size)
> > > > >    {
> > > > > -- 
> > > > > 2.37.2
> > > > > 

-- 
Sincerely yours,
Mike.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v8 0/4] riscv: Use PUD/P4D/PGD pages for the linear mapping

[Anup Patel]

Hi Alex,

On Thu, Mar 16, 2023 at 6:48 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>
> This patchset intends to improve tlb utilization by using hugepages for
> the linear mapping.
>
> As reported by Anup in v6, when STRICT_KERNEL_RWX is enabled, we must
> take care of isolating the kernel text and rodata so that they are not
> mapped with a PUD mapping which would then assign wrong permissions to
> the whole region: it is achieved by introducing a new memblock API.
>
> Another patch makes use of this new API in arm64 which used some sort of
> hack to solve this issue: it was built/boot tested successfully.
>
> base-commit-tag: v6.3-rc1
>
> v8:
> - Fix rv32, as reported by Anup
> - Do not modify memblock_isolate_range and fixes comment, as suggested by Mike
> - Use the new memblock API for crash kernel too in arm64, as suggested by Andrew
> - Fix arm64 double mapping (which to me did not work in v7), but ends up not
>   being pretty at all, will wait for comments from arm64 reviewers, but
>   this patch can easily be dropped if they do not want it.
>
> v7:
> - Fix Anup bug report by introducing memblock_isolate_memory which
>   allows us to split the memblock mappings and then avoid to map the
>   the PUD which contains the kernel as read only
> - Add a patch to arm64 to use this newly introduced API
>
> v6:
> - quiet LLVM warning by casting phys_ram_base into an unsigned long
>
> v5:
> - Fix nommu builds by getting rid of riscv_pfn_base in patch 1, thanks
>   Conor
> - Add RB from Andrew
>
> v4:
> - Rebase on top of v6.2-rc3, as noted by Conor
> - Add Acked-by Rob
>
> v3:
> - Change the comment about initrd_start VA conversion so that it fits
>   ARM64 and RISCV64 (and others in the future if needed), as suggested
>   by Rob
>
> v2:
> - Add a comment on why RISCV64 does not need to set initrd_start/end that
>   early in the boot process, as asked by Rob
>
> Alexandre Ghiti (4):
>   riscv: Get rid of riscv_pfn_base variable
>   mm: Introduce memblock_isolate_memory
>   arm64: Make use of memblock_isolate_memory for the linear mapping
>   riscv: Use PUD/P4D/PGD pages for the linear mapping

Kernel boot fine on RV64 but there is a failure which is still not
addressed. You can see this failure as following message in
kernel boot log:
    0.000000] Failed to add a System RAM resource at 80200000

Regards,
Anup

>
>  arch/arm64/mm/mmu.c           | 25 +++++++++++------
>  arch/riscv/include/asm/page.h | 19 +++++++++++--
>  arch/riscv/mm/init.c          | 53 ++++++++++++++++++++++++++++-------
>  arch/riscv/mm/physaddr.c      | 16 +++++++++++
>  drivers/of/fdt.c              | 11 ++++----
>  include/linux/memblock.h      |  1 +
>  mm/memblock.c                 | 20 +++++++++++++
>  7 files changed, 119 insertions(+), 26 deletions(-)
>
> --
> 2.37.2
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v8 2/4] mm: Introduce memblock_isolate_memory

[Alexandre Ghiti]

On 3/23/23 13:16, Mike Rapoport wrote:
> On Thu, Mar 23, 2023 at 12:52:36PM +0100, Alexandre Ghiti wrote:
>> On 3/20/23 18:44, Mike Rapoport wrote:
>>> On Mon, Mar 20, 2023 at 11:54:14AM +0100, Alexandre Ghiti wrote:
>>>> Hi Mike,
>>>>
>>>> On 3/16/23 21:12, Mike Rapoport wrote:
>>>>> Hi Alexandre,
>>>>>
>>>>> On Thu, Mar 16, 2023 at 02:17:09PM +0100, Alexandre Ghiti wrote:
>>>>>> This function allows to split a region in memblock.memory and will be
>>>>>> useful when setting up the linear mapping with STRICT_KERNEL_RWX: it
>>>>>> allows to isolate the kernel text/rodata and then avoid to map those
>>>>>> regions with a PUD/P4D/PGD.
>>>>> Sorry I've missed it last time. The changelog is fine in the context of
>>>> No worries :)
>>>>
>>>>
>>>>> this series, but if you look at it as a part of memblock changelog it
>>>>> doesn't provide enough background on why memblock_isolate_memory() is
>>>>> useful.
>>>>>
>>>>> Can you please add more context so it would be self explanatory?
>>>> What about: "memblock.memory contains the list of memory regions and a
>>>> memory region can cover memory that will be mapped with different
>>>> permissions. So to ease the mapping process, allow to isolate those regions
>>>> by introducing a new function called memblock_isolate_memory. This will be
>>>> used in arch specific code to isolate the kernel text/rodata regions when
>>>> STRICT_KERNEL_RWX is enabled so that we avoid mapping them with PUD/P4D/PGD
>>>> mappings."
>>> With this change
>>>
>>> ... STRICT_KERNEL_RWX is enabled so that they can be mapped with base pages.
>>
>> Actually they will get mapped with PMD mappings :) I'll just append: "or PMD
>> mapping" to your sentence above if that's ok with you.
> Didn't read the rest of the patches :)
>
> .. STRICT_KERNEL_RWX is enabled so that they can be mapped with differently
> than the rest of the memory.
>
> Does it cover the usescases?


Yes, fine with me!

Thanks again


>
>>> Acked-by: Mike Rapoport (IBM) <rppt@kernel.org>
>>
>> Thanks for your review,
>>
>> Alex
>>
>>
>>>> Thanks,
>>>>
>>>> Alex
>>>>
>>>>
>>>>>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>>>>>> Reviewed-by: Anup Patel <anup@brainfault.org>
>>>>>> Tested-by: Anup Patel <anup@brainfault.org>
>>>>>> ---
>>>>>>     include/linux/memblock.h |  1 +
>>>>>>     mm/memblock.c            | 20 ++++++++++++++++++++
>>>>>>     2 files changed, 21 insertions(+)
>>>>>>
>>>>>> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
>>>>>> index 50ad19662a32..2f7ef97c0da7 100644
>>>>>> --- a/include/linux/memblock.h
>>>>>> +++ b/include/linux/memblock.h
>>>>>> @@ -125,6 +125,7 @@ int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
>>>>>>     int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
>>>>>>     int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
>>>>>>     int memblock_clear_nomap(phys_addr_t base, phys_addr_t size);
>>>>>> +int memblock_isolate_memory(phys_addr_t base, phys_addr_t size);
>>>>>>     void memblock_free_all(void);
>>>>>>     void memblock_free(void *ptr, size_t size);
>>>>>> diff --git a/mm/memblock.c b/mm/memblock.c
>>>>>> index 25fd0626a9e7..e8c651a37012 100644
>>>>>> --- a/mm/memblock.c
>>>>>> +++ b/mm/memblock.c
>>>>>> @@ -805,6 +805,26 @@ static int __init_memblock memblock_isolate_range(struct memblock_type *type,
>>>>>>     	return 0;
>>>>>>     }
>>>>>> +/**
>>>>>> + * memblock_isolate_memory - isolate given range in memblock.memory
>>>>>> + * @base: base of range to isolate
>>>>>> + * @size: size of range to isolate
>>>>>> + *
>>>>>> + * Isolates the given range in memblock.memory so that it does not share any
>>>>>> + * region with other ranges.
>>>>>> + *
>>>>>> + * Return:
>>>>>> + * 0 on success, -errno on failure.
>>>>>> + */
>>>>>> +
>>>>>> +int __init_memblock memblock_isolate_memory(phys_addr_t base, phys_addr_t size)
>>>>>> +{
>>>>>> +	int start_rgn, end_rgn;
>>>>>> +
>>>>>> +	return memblock_isolate_range(&memblock.memory, base, size,
>>>>>> +				      &start_rgn, &end_rgn);
>>>>>> +}
>>>>>> +
>>>>>>     static int __init_memblock memblock_remove_range(struct memblock_type *type,
>>>>>>     					  phys_addr_t base, phys_addr_t size)
>>>>>>     {
>>>>>> -- 
>>>>>> 2.37.2
>>>>>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v8 0/4] riscv: Use PUD/P4D/PGD pages for the linear mapping

[Alexandre Ghiti]

Hi Anup,

On Thu, Mar 23, 2023 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> Hi Alex,
>
> On Thu, Mar 16, 2023 at 6:48 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> >
> > This patchset intends to improve tlb utilization by using hugepages for
> > the linear mapping.
> >
> > As reported by Anup in v6, when STRICT_KERNEL_RWX is enabled, we must
> > take care of isolating the kernel text and rodata so that they are not
> > mapped with a PUD mapping which would then assign wrong permissions to
> > the whole region: it is achieved by introducing a new memblock API.
> >
> > Another patch makes use of this new API in arm64 which used some sort of
> > hack to solve this issue: it was built/boot tested successfully.
> >
> > base-commit-tag: v6.3-rc1
> >
> > v8:
> > - Fix rv32, as reported by Anup
> > - Do not modify memblock_isolate_range and fixes comment, as suggested by Mike
> > - Use the new memblock API for crash kernel too in arm64, as suggested by Andrew
> > - Fix arm64 double mapping (which to me did not work in v7), but ends up not
> >   being pretty at all, will wait for comments from arm64 reviewers, but
> >   this patch can easily be dropped if they do not want it.
> >
> > v7:
> > - Fix Anup bug report by introducing memblock_isolate_memory which
> >   allows us to split the memblock mappings and then avoid to map the
> >   the PUD which contains the kernel as read only
> > - Add a patch to arm64 to use this newly introduced API
> >
> > v6:
> > - quiet LLVM warning by casting phys_ram_base into an unsigned long
> >
> > v5:
> > - Fix nommu builds by getting rid of riscv_pfn_base in patch 1, thanks
> >   Conor
> > - Add RB from Andrew
> >
> > v4:
> > - Rebase on top of v6.2-rc3, as noted by Conor
> > - Add Acked-by Rob
> >
> > v3:
> > - Change the comment about initrd_start VA conversion so that it fits
> >   ARM64 and RISCV64 (and others in the future if needed), as suggested
> >   by Rob
> >
> > v2:
> > - Add a comment on why RISCV64 does not need to set initrd_start/end that
> >   early in the boot process, as asked by Rob
> >
> > Alexandre Ghiti (4):
> >   riscv: Get rid of riscv_pfn_base variable
> >   mm: Introduce memblock_isolate_memory
> >   arm64: Make use of memblock_isolate_memory for the linear mapping
> >   riscv: Use PUD/P4D/PGD pages for the linear mapping
>
> Kernel boot fine on RV64 but there is a failure which is still not
> addressed. You can see this failure as following message in
> kernel boot log:
>     0.000000] Failed to add a System RAM resource at 80200000

Hmmm I don't get that in any of my test configs, would you mind
sharing yours and your qemu command line?

Thanks

>
> Regards,
> Anup
>
> >
> >  arch/arm64/mm/mmu.c           | 25 +++++++++++------
> >  arch/riscv/include/asm/page.h | 19 +++++++++++--
> >  arch/riscv/mm/init.c          | 53 ++++++++++++++++++++++++++++-------
> >  arch/riscv/mm/physaddr.c      | 16 +++++++++++
> >  drivers/of/fdt.c              | 11 ++++----
> >  include/linux/memblock.h      |  1 +
> >  mm/memblock.c                 | 20 +++++++++++++
> >  7 files changed, 119 insertions(+), 26 deletions(-)
> >
> > --
> > 2.37.2
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v8 0/4] riscv: Use PUD/P4D/PGD pages for the linear mapping

[Anup Patel]

On Thu, Mar 23, 2023 at 6:24 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>
> Hi Anup,
>
> On Thu, Mar 23, 2023 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > Hi Alex,
> >
> > On Thu, Mar 16, 2023 at 6:48 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > >
> > > This patchset intends to improve tlb utilization by using hugepages for
> > > the linear mapping.
> > >
> > > As reported by Anup in v6, when STRICT_KERNEL_RWX is enabled, we must
> > > take care of isolating the kernel text and rodata so that they are not
> > > mapped with a PUD mapping which would then assign wrong permissions to
> > > the whole region: it is achieved by introducing a new memblock API.
> > >
> > > Another patch makes use of this new API in arm64 which used some sort of
> > > hack to solve this issue: it was built/boot tested successfully.
> > >
> > > base-commit-tag: v6.3-rc1
> > >
> > > v8:
> > > - Fix rv32, as reported by Anup
> > > - Do not modify memblock_isolate_range and fixes comment, as suggested by Mike
> > > - Use the new memblock API for crash kernel too in arm64, as suggested by Andrew
> > > - Fix arm64 double mapping (which to me did not work in v7), but ends up not
> > >   being pretty at all, will wait for comments from arm64 reviewers, but
> > >   this patch can easily be dropped if they do not want it.
> > >
> > > v7:
> > > - Fix Anup bug report by introducing memblock_isolate_memory which
> > >   allows us to split the memblock mappings and then avoid to map the
> > >   the PUD which contains the kernel as read only
> > > - Add a patch to arm64 to use this newly introduced API
> > >
> > > v6:
> > > - quiet LLVM warning by casting phys_ram_base into an unsigned long
> > >
> > > v5:
> > > - Fix nommu builds by getting rid of riscv_pfn_base in patch 1, thanks
> > >   Conor
> > > - Add RB from Andrew
> > >
> > > v4:
> > > - Rebase on top of v6.2-rc3, as noted by Conor
> > > - Add Acked-by Rob
> > >
> > > v3:
> > > - Change the comment about initrd_start VA conversion so that it fits
> > >   ARM64 and RISCV64 (and others in the future if needed), as suggested
> > >   by Rob
> > >
> > > v2:
> > > - Add a comment on why RISCV64 does not need to set initrd_start/end that
> > >   early in the boot process, as asked by Rob
> > >
> > > Alexandre Ghiti (4):
> > >   riscv: Get rid of riscv_pfn_base variable
> > >   mm: Introduce memblock_isolate_memory
> > >   arm64: Make use of memblock_isolate_memory for the linear mapping
> > >   riscv: Use PUD/P4D/PGD pages for the linear mapping
> >
> > Kernel boot fine on RV64 but there is a failure which is still not
> > addressed. You can see this failure as following message in
> > kernel boot log:
> >     0.000000] Failed to add a System RAM resource at 80200000
>
> Hmmm I don't get that in any of my test configs, would you mind
> sharing yours and your qemu command line?

Try alexghiti_test branch at
https://github.com/avpatel/linux.git

I am building the kernel using defconfig and my rootfs is
based on busybox.

My QEMU command is:
qemu-system-riscv64 -M virt -m 512M -nographic -bios
opensbi/build/platform/generic/firmware/fw_dynamic.bin -kernel
./build-riscv64/arch/riscv/boot/Image -append "root=/dev/ram rw
console=ttyS0 earlycon" -initrd ./rootfs_riscv64.img -smp 4

Regards,
Anup

>
> Thanks
>
> >
> > Regards,
> > Anup
> >
> > >
> > >  arch/arm64/mm/mmu.c           | 25 +++++++++++------
> > >  arch/riscv/include/asm/page.h | 19 +++++++++++--
> > >  arch/riscv/mm/init.c          | 53 ++++++++++++++++++++++++++++-------
> > >  arch/riscv/mm/physaddr.c      | 16 +++++++++++
> > >  drivers/of/fdt.c              | 11 ++++----
> > >  include/linux/memblock.h      |  1 +
> > >  mm/memblock.c                 | 20 +++++++++++++
> > >  7 files changed, 119 insertions(+), 26 deletions(-)
> > >
> > > --
> > > 2.37.2
> > >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v8 0/4] riscv: Use PUD/P4D/PGD pages for the linear mapping

[Alexandre Ghiti]

On 3/23/23 15:55, Anup Patel wrote:
> On Thu, Mar 23, 2023 at 6:24 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>> Hi Anup,
>>
>> On Thu, Mar 23, 2023 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote:
>>> Hi Alex,
>>>
>>> On Thu, Mar 16, 2023 at 6:48 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>>>> This patchset intends to improve tlb utilization by using hugepages for
>>>> the linear mapping.
>>>>
>>>> As reported by Anup in v6, when STRICT_KERNEL_RWX is enabled, we must
>>>> take care of isolating the kernel text and rodata so that they are not
>>>> mapped with a PUD mapping which would then assign wrong permissions to
>>>> the whole region: it is achieved by introducing a new memblock API.
>>>>
>>>> Another patch makes use of this new API in arm64 which used some sort of
>>>> hack to solve this issue: it was built/boot tested successfully.
>>>>
>>>> base-commit-tag: v6.3-rc1
>>>>
>>>> v8:
>>>> - Fix rv32, as reported by Anup
>>>> - Do not modify memblock_isolate_range and fixes comment, as suggested by Mike
>>>> - Use the new memblock API for crash kernel too in arm64, as suggested by Andrew
>>>> - Fix arm64 double mapping (which to me did not work in v7), but ends up not
>>>>    being pretty at all, will wait for comments from arm64 reviewers, but
>>>>    this patch can easily be dropped if they do not want it.
>>>>
>>>> v7:
>>>> - Fix Anup bug report by introducing memblock_isolate_memory which
>>>>    allows us to split the memblock mappings and then avoid to map the
>>>>    the PUD which contains the kernel as read only
>>>> - Add a patch to arm64 to use this newly introduced API
>>>>
>>>> v6:
>>>> - quiet LLVM warning by casting phys_ram_base into an unsigned long
>>>>
>>>> v5:
>>>> - Fix nommu builds by getting rid of riscv_pfn_base in patch 1, thanks
>>>>    Conor
>>>> - Add RB from Andrew
>>>>
>>>> v4:
>>>> - Rebase on top of v6.2-rc3, as noted by Conor
>>>> - Add Acked-by Rob
>>>>
>>>> v3:
>>>> - Change the comment about initrd_start VA conversion so that it fits
>>>>    ARM64 and RISCV64 (and others in the future if needed), as suggested
>>>>    by Rob
>>>>
>>>> v2:
>>>> - Add a comment on why RISCV64 does not need to set initrd_start/end that
>>>>    early in the boot process, as asked by Rob
>>>>
>>>> Alexandre Ghiti (4):
>>>>    riscv: Get rid of riscv_pfn_base variable
>>>>    mm: Introduce memblock_isolate_memory
>>>>    arm64: Make use of memblock_isolate_memory for the linear mapping
>>>>    riscv: Use PUD/P4D/PGD pages for the linear mapping
>>> Kernel boot fine on RV64 but there is a failure which is still not
>>> addressed. You can see this failure as following message in
>>> kernel boot log:
>>>      0.000000] Failed to add a System RAM resource at 80200000
>> Hmmm I don't get that in any of my test configs, would you mind
>> sharing yours and your qemu command line?
> Try alexghiti_test branch at
> https://github.com/avpatel/linux.git
>
> I am building the kernel using defconfig and my rootfs is
> based on busybox.
>
> My QEMU command is:
> qemu-system-riscv64 -M virt -m 512M -nographic -bios
> opensbi/build/platform/generic/firmware/fw_dynamic.bin -kernel
> ./build-riscv64/arch/riscv/boot/Image -append "root=/dev/ram rw
> console=ttyS0 earlycon" -initrd ./rootfs_riscv64.img -smp 4


So splitting memblock.memory is the culprit, it "confuses" the resources 
addition and I can only find hacky ways to fix that...

So given that the arm64 patch with the new API is not pretty and that 
the simplest solution is to re-merge the memblock regions afterwards 
(which is done by memblock_clear_nomap), I'll drop the new API and the 
arm64 patch to use the nomap API like arm64: I'll take advantage of that 
to clean setup_vm_final which I have wanted to do for a long time.

@Mike Thanks for you reviews!

@Anup Thanks for all your bug reports on this patchset, I have to 
improve my test flow (it is in the work :)).


> Regards,
> Anup
>
>> Thanks
>>
>>> Regards,
>>> Anup
>>>
>>>>   arch/arm64/mm/mmu.c           | 25 +++++++++++------
>>>>   arch/riscv/include/asm/page.h | 19 +++++++++++--
>>>>   arch/riscv/mm/init.c          | 53 ++++++++++++++++++++++++++++-------
>>>>   arch/riscv/mm/physaddr.c      | 16 +++++++++++
>>>>   drivers/of/fdt.c              | 11 ++++----
>>>>   include/linux/memblock.h      |  1 +
>>>>   mm/memblock.c                 | 20 +++++++++++++
>>>>   7 files changed, 119 insertions(+), 26 deletions(-)
>>>>
>>>> --
>>>> 2.37.2
>>>>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v8 3/4] arm64: Make use of memblock_isolate_memory for the linear mapping

[Will Deacon]

On Thu, Mar 16, 2023 at 02:17:10PM +0100, Alexandre Ghiti wrote:
> In order to isolate the kernel text mapping and the crash kernel
> region, we used some sort of hack to isolate thoses ranges which consisted
> in marking them as not mappable with memblock_mark_nomap.
> 
> Simply use the newly introduced memblock_isolate_memory function which does
> exactly the same but does not uselessly mark the region as not mappable.

But that's not what this patch does -- it's also adding special-case code
for kexec and, honestly, I'm struggling to see why this is improving
anything.

Can we leave the code like it is, or is there something else going on?

Will

> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
>  arch/arm64/mm/mmu.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 6f9d8898a025..387c2a065a09 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -548,19 +548,18 @@ static void __init map_mem(pgd_t *pgdp)
>  
>  	/*
>  	 * Take care not to create a writable alias for the
> -	 * read-only text and rodata sections of the kernel image.
> -	 * So temporarily mark them as NOMAP to skip mappings in
> -	 * the following for-loop
> +	 * read-only text and rodata sections of the kernel image so isolate
> +	 * those regions and map them after the for loop.
>  	 */
> -	memblock_mark_nomap(kernel_start, kernel_end - kernel_start);
> +	memblock_isolate_memory(kernel_start, kernel_end - kernel_start);
>  
>  #ifdef CONFIG_KEXEC_CORE
>  	if (crash_mem_map) {
>  		if (defer_reserve_crashkernel())
>  			flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
>  		else if (crashk_res.end)
> -			memblock_mark_nomap(crashk_res.start,
> -			    resource_size(&crashk_res));
> +			memblock_isolate_memory(crashk_res.start,
> +						resource_size(&crashk_res));
>  	}
>  #endif
>  
> @@ -568,6 +567,17 @@ static void __init map_mem(pgd_t *pgdp)
>  	for_each_mem_range(i, &start, &end) {
>  		if (start >= end)
>  			break;
> +
> +		if (start == kernel_start)
> +			continue;
> +
> +#ifdef CONFIG_KEXEC_CORE
> +		if (start == crashk_res.start &&
> +		    crash_mem_map && !defer_reserve_crashkernel() &&
> +		    crashk_res.end)
> +			continue;
> +#endif
> +
>  		/*
>  		 * The linear map must allow allocation tags reading/writing
>  		 * if MTE is present. Otherwise, it has the same attributes as
> @@ -589,7 +599,6 @@ static void __init map_mem(pgd_t *pgdp)
>  	 */
>  	__map_memblock(pgdp, kernel_start, kernel_end,
>  		       PAGE_KERNEL, NO_CONT_MAPPINGS);
> -	memblock_clear_nomap(kernel_start, kernel_end - kernel_start);
>  
>  	/*
>  	 * Use page-level mappings here so that we can shrink the region
> @@ -603,8 +612,6 @@ static void __init map_mem(pgd_t *pgdp)
>  				       crashk_res.end + 1,
>  				       PAGE_KERNEL,
>  				       NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS);
> -			memblock_clear_nomap(crashk_res.start,
> -					     resource_size(&crashk_res));
>  		}
>  	}
>  #endif
> -- 
> 2.37.2
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v8 3/4] arm64: Make use of memblock_isolate_memory for the linear mapping

[Alexandre Ghiti]

Hi Will,

On 3/24/23 16:21, Will Deacon wrote:
> On Thu, Mar 16, 2023 at 02:17:10PM +0100, Alexandre Ghiti wrote:
>> In order to isolate the kernel text mapping and the crash kernel
>> region, we used some sort of hack to isolate thoses ranges which consisted
>> in marking them as not mappable with memblock_mark_nomap.
>>
>> Simply use the newly introduced memblock_isolate_memory function which does
>> exactly the same but does not uselessly mark the region as not mappable.
> But that's not what this patch does -- it's also adding special-case code
> for kexec and, honestly, I'm struggling to see why this is improving
> anything.
>
> Can we leave the code like it is, or is there something else going on?


Yes, the next version won't touch arm64 at all since actually, I'll 
remove this new api.

Thanks for your time,

Alex


>
> Will
>
>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>> ---
>>   arch/arm64/mm/mmu.c | 25 ++++++++++++++++---------
>>   1 file changed, 16 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index 6f9d8898a025..387c2a065a09 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -548,19 +548,18 @@ static void __init map_mem(pgd_t *pgdp)
>>   
>>   	/*
>>   	 * Take care not to create a writable alias for the
>> -	 * read-only text and rodata sections of the kernel image.
>> -	 * So temporarily mark them as NOMAP to skip mappings in
>> -	 * the following for-loop
>> +	 * read-only text and rodata sections of the kernel image so isolate
>> +	 * those regions and map them after the for loop.
>>   	 */
>> -	memblock_mark_nomap(kernel_start, kernel_end - kernel_start);
>> +	memblock_isolate_memory(kernel_start, kernel_end - kernel_start);
>>   
>>   #ifdef CONFIG_KEXEC_CORE
>>   	if (crash_mem_map) {
>>   		if (defer_reserve_crashkernel())
>>   			flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
>>   		else if (crashk_res.end)
>> -			memblock_mark_nomap(crashk_res.start,
>> -			    resource_size(&crashk_res));
>> +			memblock_isolate_memory(crashk_res.start,
>> +						resource_size(&crashk_res));
>>   	}
>>   #endif
>>   
>> @@ -568,6 +567,17 @@ static void __init map_mem(pgd_t *pgdp)
>>   	for_each_mem_range(i, &start, &end) {
>>   		if (start >= end)
>>   			break;
>> +
>> +		if (start == kernel_start)
>> +			continue;
>> +
>> +#ifdef CONFIG_KEXEC_CORE
>> +		if (start == crashk_res.start &&
>> +		    crash_mem_map && !defer_reserve_crashkernel() &&
>> +		    crashk_res.end)
>> +			continue;
>> +#endif
>> +
>>   		/*
>>   		 * The linear map must allow allocation tags reading/writing
>>   		 * if MTE is present. Otherwise, it has the same attributes as
>> @@ -589,7 +599,6 @@ static void __init map_mem(pgd_t *pgdp)
>>   	 */
>>   	__map_memblock(pgdp, kernel_start, kernel_end,
>>   		       PAGE_KERNEL, NO_CONT_MAPPINGS);
>> -	memblock_clear_nomap(kernel_start, kernel_end - kernel_start);
>>   
>>   	/*
>>   	 * Use page-level mappings here so that we can shrink the region
>> @@ -603,8 +612,6 @@ static void __init map_mem(pgd_t *pgdp)
>>   				       crashk_res.end + 1,
>>   				       PAGE_KERNEL,
>>   				       NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS);
>> -			memblock_clear_nomap(crashk_res.start,
>> -					     resource_size(&crashk_res));
>>   		}
>>   	}
>>   #endif
>> -- 
>> 2.37.2
>>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Fwd: [PATCH v8 0/4] riscv: Use PUD/P4D/PGD pages for the linear mapping

[Nylon Chen]

> On 3/23/23 15:55, Anup Patel wrote:
> > On Thu, Mar 23, 2023 at 6:24 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> >> Hi Anup,
> >>
> >> On Thu, Mar 23, 2023 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >>> Hi Alex,
> >>>
> >>> On Thu, Mar 16, 2023 at 6:48 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> >>>> This patchset intends to improve tlb utilization by using hugepages for
> >>>> the linear mapping.
> >>>>
> >>>> As reported by Anup in v6, when STRICT_KERNEL_RWX is enabled, we must
> >>>> take care of isolating the kernel text and rodata so that they are not
> >>>> mapped with a PUD mapping which would then assign wrong permissions to
> >>>> the whole region: it is achieved by introducing a new memblock API.
> >>>>
> >>>> Another patch makes use of this new API in arm64 which used some sort of
> >>>> hack to solve this issue: it was built/boot tested successfully.
> >>>>
> >>>> base-commit-tag: v6.3-rc1
> >>>>
> >>>> v8:
> >>>> - Fix rv32, as reported by Anup
> >>>> - Do not modify memblock_isolate_range and fixes comment, as suggested by Mike
> >>>> - Use the new memblock API for crash kernel too in arm64, as suggested by Andrew
> >>>> - Fix arm64 double mapping (which to me did not work in v7), but ends up not
> >>>>    being pretty at all, will wait for comments from arm64 reviewers, but
> >>>>    this patch can easily be dropped if they do not want it.
> >>>>
> >>>> v7:
> >>>> - Fix Anup bug report by introducing memblock_isolate_memory which
> >>>>    allows us to split the memblock mappings and then avoid to map the
> >>>>    the PUD which contains the kernel as read only
> >>>> - Add a patch to arm64 to use this newly introduced API
> >>>>
> >>>> v6:
> >>>> - quiet LLVM warning by casting phys_ram_base into an unsigned long
> >>>>
> >>>> v5:
> >>>> - Fix nommu builds by getting rid of riscv_pfn_base in patch 1, thanks
> >>>>    Conor
> >>>> - Add RB from Andrew
> >>>>
> >>>> v4:
> >>>> - Rebase on top of v6.2-rc3, as noted by Conor
> >>>> - Add Acked-by Rob
> >>>>
> >>>> v3:
> >>>> - Change the comment about initrd_start VA conversion so that it fits
> >>>>    ARM64 and RISCV64 (and others in the future if needed), as suggested
> >>>>    by Rob
> >>>>
> >>>> v2:
> >>>> - Add a comment on why RISCV64 does not need to set initrd_start/end that
> >>>>    early in the boot process, as asked by Rob
> >>>>
> >>>> Alexandre Ghiti (4):
> >>>>    riscv: Get rid of riscv_pfn_base variable
> >>>>    mm: Introduce memblock_isolate_memory
> >>>>    arm64: Make use of memblock_isolate_memory for the linear mapping
> >>>>    riscv: Use PUD/P4D/PGD pages for the linear mapping
> >>> Kernel boot fine on RV64 but there is a failure which is still not
> >>> addressed. You can see this failure as following message in
> >>> kernel boot log:
> >>>      0.000000] Failed to add a System RAM resource at 80200000
> >> Hmmm I don't get that in any of my test configs, would you mind
> >> sharing yours and your qemu command line?
> > Try alexghiti_test branch at
> > https://github.com/avpatel/linux.git
> >
> > I am building the kernel using defconfig and my rootfs is
> > based on busybox.
> >
> > My QEMU command is:
> > qemu-system-riscv64 -M virt -m 512M -nographic -bios
> > opensbi/build/platform/generic/firmware/fw_dynamic.bin -kernel
> > ./build-riscv64/arch/riscv/boot/Image -append "root=/dev/ram rw
> > console=ttyS0 earlycon" -initrd ./rootfs_riscv64.img -smp 4
> 
> 
> So splitting memblock.memory is the culprit, it "confuses" the resources
> addition and I can only find hacky ways to fix that...
Hi Alexandre,

We encountered the same error as Anup. After adding your patch
(3335068f87217ea59d08f462187dc856652eea15), we will not encounter the
error again.

What I have observed so far is

- before your patch
When merging consecutive memblocks, if the memblock types are different,
they will be merged into reserved
- after your patch
When consecutive memblocks are merged, if the memblock types are
different, they will be merged into memory.

Such a result will cause the memory location of OpenSBI to be changed
from reserved to memory. Will this have any side effects?
> 
> So given that the arm64 patch with the new API is not pretty and that
> the simplest solution is to re-merge the memblock regions afterwards
> (which is done by memblock_clear_nomap), I'll drop the new API and the
> arm64 patch to use the nomap API like arm64: I'll take advantage of that
> to clean setup_vm_final which I have wanted to do for a long time.
> 
> @Mike Thanks for you reviews!
> 
> @Anup Thanks for all your bug reports on this patchset, I have to
> improve my test flow (it is in the work :)).
> 
> 
> > Regards,
> > Anup
> >
> >> Thanks
> >>
> >>> Regards,
> >>> Anup
> >>>
> >>>>   arch/arm64/mm/mmu.c           | 25 +++++++++++------
> >>>>   arch/riscv/include/asm/page.h | 19 +++++++++++--
> >>>>   arch/riscv/mm/init.c          | 53 ++++++++++++++++++++++++++++-------
> >>>>   arch/riscv/mm/physaddr.c      | 16 +++++++++++
> >>>>   drivers/of/fdt.c              | 11 ++++----
> >>>>   include/linux/memblock.h      |  1 +
> >>>>   mm/memblock.c                 | 20 +++++++++++++
> >>>>   7 files changed, 119 insertions(+), 26 deletions(-)
> >>>>
> >>>> --
> >>>> 2.37.2
> >>>>
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Fwd: [PATCH v8 0/4] riscv: Use PUD/P4D/PGD pages for the linear mapping

[Alexandre Ghiti]

Hi Nylon,

On Thu, Jan 18, 2024 at 9:23 AM Nylon Chen <nylon.chen@sifive.com> wrote:
>
> > On 3/23/23 15:55, Anup Patel wrote:
> > > On Thu, Mar 23, 2023 at 6:24 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > >> Hi Anup,
> > >>
> > >> On Thu, Mar 23, 2023 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote:
> > >>> Hi Alex,
> > >>>
> > >>> On Thu, Mar 16, 2023 at 6:48 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > >>>> This patchset intends to improve tlb utilization by using hugepages for
> > >>>> the linear mapping.
> > >>>>
> > >>>> As reported by Anup in v6, when STRICT_KERNEL_RWX is enabled, we must
> > >>>> take care of isolating the kernel text and rodata so that they are not
> > >>>> mapped with a PUD mapping which would then assign wrong permissions to
> > >>>> the whole region: it is achieved by introducing a new memblock API.
> > >>>>
> > >>>> Another patch makes use of this new API in arm64 which used some sort of
> > >>>> hack to solve this issue: it was built/boot tested successfully.
> > >>>>
> > >>>> base-commit-tag: v6.3-rc1
> > >>>>
> > >>>> v8:
> > >>>> - Fix rv32, as reported by Anup
> > >>>> - Do not modify memblock_isolate_range and fixes comment, as suggested by Mike
> > >>>> - Use the new memblock API for crash kernel too in arm64, as suggested by Andrew
> > >>>> - Fix arm64 double mapping (which to me did not work in v7), but ends up not
> > >>>>    being pretty at all, will wait for comments from arm64 reviewers, but
> > >>>>    this patch can easily be dropped if they do not want it.
> > >>>>
> > >>>> v7:
> > >>>> - Fix Anup bug report by introducing memblock_isolate_memory which
> > >>>>    allows us to split the memblock mappings and then avoid to map the
> > >>>>    the PUD which contains the kernel as read only
> > >>>> - Add a patch to arm64 to use this newly introduced API
> > >>>>
> > >>>> v6:
> > >>>> - quiet LLVM warning by casting phys_ram_base into an unsigned long
> > >>>>
> > >>>> v5:
> > >>>> - Fix nommu builds by getting rid of riscv_pfn_base in patch 1, thanks
> > >>>>    Conor
> > >>>> - Add RB from Andrew
> > >>>>
> > >>>> v4:
> > >>>> - Rebase on top of v6.2-rc3, as noted by Conor
> > >>>> - Add Acked-by Rob
> > >>>>
> > >>>> v3:
> > >>>> - Change the comment about initrd_start VA conversion so that it fits
> > >>>>    ARM64 and RISCV64 (and others in the future if needed), as suggested
> > >>>>    by Rob
> > >>>>
> > >>>> v2:
> > >>>> - Add a comment on why RISCV64 does not need to set initrd_start/end that
> > >>>>    early in the boot process, as asked by Rob
> > >>>>
> > >>>> Alexandre Ghiti (4):
> > >>>>    riscv: Get rid of riscv_pfn_base variable
> > >>>>    mm: Introduce memblock_isolate_memory
> > >>>>    arm64: Make use of memblock_isolate_memory for the linear mapping
> > >>>>    riscv: Use PUD/P4D/PGD pages for the linear mapping
> > >>> Kernel boot fine on RV64 but there is a failure which is still not
> > >>> addressed. You can see this failure as following message in
> > >>> kernel boot log:
> > >>>      0.000000] Failed to add a System RAM resource at 80200000
> > >> Hmmm I don't get that in any of my test configs, would you mind
> > >> sharing yours and your qemu command line?
> > > Try alexghiti_test branch at
> > > https://github.com/avpatel/linux.git
> > >
> > > I am building the kernel using defconfig and my rootfs is
> > > based on busybox.
> > >
> > > My QEMU command is:
> > > qemu-system-riscv64 -M virt -m 512M -nographic -bios
> > > opensbi/build/platform/generic/firmware/fw_dynamic.bin -kernel
> > > ./build-riscv64/arch/riscv/boot/Image -append "root=/dev/ram rw
> > > console=ttyS0 earlycon" -initrd ./rootfs_riscv64.img -smp 4
> >
> >
> > So splitting memblock.memory is the culprit, it "confuses" the resources
> > addition and I can only find hacky ways to fix that...
> Hi Alexandre,
>
> We encountered the same error as Anup. After adding your patch
> (3335068f87217ea59d08f462187dc856652eea15), we will not encounter the
> error again.
>
> What I have observed so far is
>
> - before your patch
> When merging consecutive memblocks, if the memblock types are different,
> they will be merged into reserved
> - after your patch
> When consecutive memblocks are merged, if the memblock types are
> different, they will be merged into memory.
>
> Such a result will cause the memory location of OpenSBI to be changed
> from reserved to memory. Will this have any side effects?

I guess it will end up in the memory pool and pages from openSBI
region will be allocated, so we should see very quickly bad stuff
happening (either PMP violation or M-mode ecall never
returning/trapping/etc).

But I don't observe the same thing, I always see the openSBI region
being reserved:

reserved[0x0] [0x0000000080000000-0x000000008007ffff],
0x0000000000080000 bytes flags: 0x0

Can you elaborate a bit more about "When consecutive memblocks are
merged, if the memblock types are different, they will be merged into
memory"? Where/when does this merge happen? Can you give me a config
file and a kernel revision so that I can take a look?

Thanks,

Alex

> >
> > So given that the arm64 patch with the new API is not pretty and that
> > the simplest solution is to re-merge the memblock regions afterwards
> > (which is done by memblock_clear_nomap), I'll drop the new API and the
> > arm64 patch to use the nomap API like arm64: I'll take advantage of that
> > to clean setup_vm_final which I have wanted to do for a long time.
> >
> > @Mike Thanks for you reviews!
> >
> > @Anup Thanks for all your bug reports on this patchset, I have to
> > improve my test flow (it is in the work :)).
> >
> >
> > > Regards,
> > > Anup
> > >
> > >> Thanks
> > >>
> > >>> Regards,
> > >>> Anup
> > >>>
> > >>>>   arch/arm64/mm/mmu.c           | 25 +++++++++++------
> > >>>>   arch/riscv/include/asm/page.h | 19 +++++++++++--
> > >>>>   arch/riscv/mm/init.c          | 53 ++++++++++++++++++++++++++++-------
> > >>>>   arch/riscv/mm/physaddr.c      | 16 +++++++++++
> > >>>>   drivers/of/fdt.c              | 11 ++++----
> > >>>>   include/linux/memblock.h      |  1 +
> > >>>>   mm/memblock.c                 | 20 +++++++++++++
> > >>>>   7 files changed, 119 insertions(+), 26 deletions(-)
> > >>>>
> > >>>> --
> > >>>> 2.37.2
> > >>>>
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Fwd: [PATCH v8 0/4] riscv: Use PUD/P4D/PGD pages for the linear mapping

[Nylon Chen]

Alexandre Ghiti <alexghiti@rivosinc.com> 於 2024年1月18日 週四 下午9:01寫道：
>
> Hi Nylon,
Hi Alexandre, thanks for your feedback,
>
> On Thu, Jan 18, 2024 at 9:23 AM Nylon Chen <nylon.chen@sifive.com> wrote:
> >
> > > On 3/23/23 15:55, Anup Patel wrote:
> > > > On Thu, Mar 23, 2023 at 6:24 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > > >> Hi Anup,
> > > >>
> > > >> On Thu, Mar 23, 2023 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote:
> > > >>> Hi Alex,
> > > >>>
> > > >>> On Thu, Mar 16, 2023 at 6:48 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > > >>>> This patchset intends to improve tlb utilization by using hugepages for
> > > >>>> the linear mapping.
> > > >>>>
> > > >>>> As reported by Anup in v6, when STRICT_KERNEL_RWX is enabled, we must
> > > >>>> take care of isolating the kernel text and rodata so that they are not
> > > >>>> mapped with a PUD mapping which would then assign wrong permissions to
> > > >>>> the whole region: it is achieved by introducing a new memblock API.
> > > >>>>
> > > >>>> Another patch makes use of this new API in arm64 which used some sort of
> > > >>>> hack to solve this issue: it was built/boot tested successfully.
> > > >>>>
> > > >>>> base-commit-tag: v6.3-rc1
> > > >>>>
> > > >>>> v8:
> > > >>>> - Fix rv32, as reported by Anup
> > > >>>> - Do not modify memblock_isolate_range and fixes comment, as suggested by Mike
> > > >>>> - Use the new memblock API for crash kernel too in arm64, as suggested by Andrew
> > > >>>> - Fix arm64 double mapping (which to me did not work in v7), but ends up not
> > > >>>>    being pretty at all, will wait for comments from arm64 reviewers, but
> > > >>>>    this patch can easily be dropped if they do not want it.
> > > >>>>
> > > >>>> v7:
> > > >>>> - Fix Anup bug report by introducing memblock_isolate_memory which
> > > >>>>    allows us to split the memblock mappings and then avoid to map the
> > > >>>>    the PUD which contains the kernel as read only
> > > >>>> - Add a patch to arm64 to use this newly introduced API
> > > >>>>
> > > >>>> v6:
> > > >>>> - quiet LLVM warning by casting phys_ram_base into an unsigned long
> > > >>>>
> > > >>>> v5:
> > > >>>> - Fix nommu builds by getting rid of riscv_pfn_base in patch 1, thanks
> > > >>>>    Conor
> > > >>>> - Add RB from Andrew
> > > >>>>
> > > >>>> v4:
> > > >>>> - Rebase on top of v6.2-rc3, as noted by Conor
> > > >>>> - Add Acked-by Rob
> > > >>>>
> > > >>>> v3:
> > > >>>> - Change the comment about initrd_start VA conversion so that it fits
> > > >>>>    ARM64 and RISCV64 (and others in the future if needed), as suggested
> > > >>>>    by Rob
> > > >>>>
> > > >>>> v2:
> > > >>>> - Add a comment on why RISCV64 does not need to set initrd_start/end that
> > > >>>>    early in the boot process, as asked by Rob
> > > >>>>
> > > >>>> Alexandre Ghiti (4):
> > > >>>>    riscv: Get rid of riscv_pfn_base variable
> > > >>>>    mm: Introduce memblock_isolate_memory
> > > >>>>    arm64: Make use of memblock_isolate_memory for the linear mapping
> > > >>>>    riscv: Use PUD/P4D/PGD pages for the linear mapping
> > > >>> Kernel boot fine on RV64 but there is a failure which is still not
> > > >>> addressed. You can see this failure as following message in
> > > >>> kernel boot log:
> > > >>>      0.000000] Failed to add a System RAM resource at 80200000
> > > >> Hmmm I don't get that in any of my test configs, would you mind
> > > >> sharing yours and your qemu command line?
> > > > Try alexghiti_test branch at
> > > > https://github.com/avpatel/linux.git
> > > >
> > > > I am building the kernel using defconfig and my rootfs is
> > > > based on busybox.
> > > >
> > > > My QEMU command is:
> > > > qemu-system-riscv64 -M virt -m 512M -nographic -bios
> > > > opensbi/build/platform/generic/firmware/fw_dynamic.bin -kernel
> > > > ./build-riscv64/arch/riscv/boot/Image -append "root=/dev/ram rw
> > > > console=ttyS0 earlycon" -initrd ./rootfs_riscv64.img -smp 4
> > >
> > >
> > > So splitting memblock.memory is the culprit, it "confuses" the resources
> > > addition and I can only find hacky ways to fix that...
> > Hi Alexandre,
> >
> > We encountered the same error as Anup. After adding your patch
> > (3335068f87217ea59d08f462187dc856652eea15), we will not encounter the
> > error again.
> >
> > What I have observed so far is
> >
> > - before your patch
> > When merging consecutive memblocks, if the memblock types are different,
> > they will be merged into reserved
> > - after your patch
> > When consecutive memblocks are merged, if the memblock types are
> > different, they will be merged into memory.
> >
> > Such a result will cause the memory location of OpenSBI to be changed
> > from reserved to memory. Will this have any side effects?
>
> I guess it will end up in the memory pool and pages from openSBI
> region will be allocated, so we should see very quickly bad stuff
> happening (either PMP violation or M-mode ecall never
> returning/trapping/etc).
>
> But I don't observe the same thing, I always see the openSBI region
> being reserved:
>
> reserved[0x0] [0x0000000080000000-0x000000008007ffff],
> 0x0000000000080000 bytes flags: 0x0
>
> Can you elaborate a bit more about "When consecutive memblocks are
> merged, if the memblock types are different, they will be merged into
> memory"? Where/when does this merge happen? Can you give me a config
> file and a kernel revision so that I can take a look?
Ok, If you want to reproduce the same results you just need to modify OpenSBI

[ lib/sbi/sbi_domain.c ]
+#define TEST_SIZE 0x200000

-                                 (scratch->fw_size - scratch->fw_rw_offset),
+                                 (TEST_SIZE - scratch->fw_rw_offset),

In addition, you can insert checks in the kernel merged function
[ mm/memblock.c ]
static void __init_memblock memblock_merge_regions(struct memblock_type *type)
        while (i < type->cnt - 1) {
         ...
                /* move forward from next + 1, index of which is i + 2 */
                memmove(next, next + 1, (type->cnt - (i + 2)) * sizeof(*next));
                type->cnt--;
        }
+       pr_info("Merged memblock_type: cnt = %lu, max = %lu,
total_size = 0x%llx\n",type->cnt, type->max, type->total_size);
+       for (i = 0; i < type->cnt; i++) {
+               const char *region_type =
memblock_is_memory(type->regions[i].base) ? "memory" : "reserve";
+               pr_info("Region %d: base = 0x%llx, size = 0x%llx, type
= %s\n", i, type->regions[i].base, type->regions[i].size,
region_type);
+       }
 }
This is kernel boot log
- before your patch
...
[    0.000000] OF: fdt: Reserving memory: base = 0x80000000, size = 0x200000
[    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size = 0x1628501
[    0.000000] Region 0: base = 0x80000000, size = 0x1600000, type = reserve
...

- after your patch
...
[    0.000000] OF: fdt: Reserving memory: base = 0x80000000, size = 0x200000
[    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size = 0x180c42e
[    0.000000] Region 0: base = 0x80000000, size = 0x1800000, type = memory
...
[    0.000000] Failed to add a system RAM resource at 80200000
...
>
> Thanks,
>
> Alex
>
> > >
> > > So given that the arm64 patch with the new API is not pretty and that
> > > the simplest solution is to re-merge the memblock regions afterwards
> > > (which is done by memblock_clear_nomap), I'll drop the new API and the
> > > arm64 patch to use the nomap API like arm64: I'll take advantage of that
> > > to clean setup_vm_final which I have wanted to do for a long time.
> > >
> > > @Mike Thanks for you reviews!
> > >
> > > @Anup Thanks for all your bug reports on this patchset, I have to
> > > improve my test flow (it is in the work :)).
> > >
> > >
> > > > Regards,
> > > > Anup
> > > >
> > > >> Thanks
> > > >>
> > > >>> Regards,
> > > >>> Anup
> > > >>>
> > > >>>>   arch/arm64/mm/mmu.c           | 25 +++++++++++------
> > > >>>>   arch/riscv/include/asm/page.h | 19 +++++++++++--
> > > >>>>   arch/riscv/mm/init.c          | 53 ++++++++++++++++++++++++++++-------
> > > >>>>   arch/riscv/mm/physaddr.c      | 16 +++++++++++
> > > >>>>   drivers/of/fdt.c              | 11 ++++----
> > > >>>>   include/linux/memblock.h      |  1 +
> > > >>>>   mm/memblock.c                 | 20 +++++++++++++
> > > >>>>   7 files changed, 119 insertions(+), 26 deletions(-)
> > > >>>>
> > > >>>> --
> > > >>>> 2.37.2
> > > >>>>
> > > > _______________________________________________
> > > > linux-riscv mailing list
> > > > linux-riscv@lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > >
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Fwd: [PATCH v8 0/4] riscv: Use PUD/P4D/PGD pages for the linear mapping

[Alexandre Ghiti]

Hi Nylon,

On Fri, Jan 19, 2024 at 10:27 AM Nylon Chen <nylon.chen@sifive.com> wrote:
>
> Alexandre Ghiti <alexghiti@rivosinc.com> 於 2024年1月18日 週四 下午9:01寫道：
> >
> > Hi Nylon,
> Hi Alexandre, thanks for your feedback,
> >
> > On Thu, Jan 18, 2024 at 9:23 AM Nylon Chen <nylon.chen@sifive.com> wrote:
> > >
> > > > On 3/23/23 15:55, Anup Patel wrote:
> > > > > On Thu, Mar 23, 2023 at 6:24 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > > > >> Hi Anup,
> > > > >>
> > > > >> On Thu, Mar 23, 2023 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote:
> > > > >>> Hi Alex,
> > > > >>>
> > > > >>> On Thu, Mar 16, 2023 at 6:48 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > > > >>>> This patchset intends to improve tlb utilization by using hugepages for
> > > > >>>> the linear mapping.
> > > > >>>>
> > > > >>>> As reported by Anup in v6, when STRICT_KERNEL_RWX is enabled, we must
> > > > >>>> take care of isolating the kernel text and rodata so that they are not
> > > > >>>> mapped with a PUD mapping which would then assign wrong permissions to
> > > > >>>> the whole region: it is achieved by introducing a new memblock API.
> > > > >>>>
> > > > >>>> Another patch makes use of this new API in arm64 which used some sort of
> > > > >>>> hack to solve this issue: it was built/boot tested successfully.
> > > > >>>>
> > > > >>>> base-commit-tag: v6.3-rc1
> > > > >>>>
> > > > >>>> v8:
> > > > >>>> - Fix rv32, as reported by Anup
> > > > >>>> - Do not modify memblock_isolate_range and fixes comment, as suggested by Mike
> > > > >>>> - Use the new memblock API for crash kernel too in arm64, as suggested by Andrew
> > > > >>>> - Fix arm64 double mapping (which to me did not work in v7), but ends up not
> > > > >>>>    being pretty at all, will wait for comments from arm64 reviewers, but
> > > > >>>>    this patch can easily be dropped if they do not want it.
> > > > >>>>
> > > > >>>> v7:
> > > > >>>> - Fix Anup bug report by introducing memblock_isolate_memory which
> > > > >>>>    allows us to split the memblock mappings and then avoid to map the
> > > > >>>>    the PUD which contains the kernel as read only
> > > > >>>> - Add a patch to arm64 to use this newly introduced API
> > > > >>>>
> > > > >>>> v6:
> > > > >>>> - quiet LLVM warning by casting phys_ram_base into an unsigned long
> > > > >>>>
> > > > >>>> v5:
> > > > >>>> - Fix nommu builds by getting rid of riscv_pfn_base in patch 1, thanks
> > > > >>>>    Conor
> > > > >>>> - Add RB from Andrew
> > > > >>>>
> > > > >>>> v4:
> > > > >>>> - Rebase on top of v6.2-rc3, as noted by Conor
> > > > >>>> - Add Acked-by Rob
> > > > >>>>
> > > > >>>> v3:
> > > > >>>> - Change the comment about initrd_start VA conversion so that it fits
> > > > >>>>    ARM64 and RISCV64 (and others in the future if needed), as suggested
> > > > >>>>    by Rob
> > > > >>>>
> > > > >>>> v2:
> > > > >>>> - Add a comment on why RISCV64 does not need to set initrd_start/end that
> > > > >>>>    early in the boot process, as asked by Rob
> > > > >>>>
> > > > >>>> Alexandre Ghiti (4):
> > > > >>>>    riscv: Get rid of riscv_pfn_base variable
> > > > >>>>    mm: Introduce memblock_isolate_memory
> > > > >>>>    arm64: Make use of memblock_isolate_memory for the linear mapping
> > > > >>>>    riscv: Use PUD/P4D/PGD pages for the linear mapping
> > > > >>> Kernel boot fine on RV64 but there is a failure which is still not
> > > > >>> addressed. You can see this failure as following message in
> > > > >>> kernel boot log:
> > > > >>>      0.000000] Failed to add a System RAM resource at 80200000
> > > > >> Hmmm I don't get that in any of my test configs, would you mind
> > > > >> sharing yours and your qemu command line?
> > > > > Try alexghiti_test branch at
> > > > > https://github.com/avpatel/linux.git
> > > > >
> > > > > I am building the kernel using defconfig and my rootfs is
> > > > > based on busybox.
> > > > >
> > > > > My QEMU command is:
> > > > > qemu-system-riscv64 -M virt -m 512M -nographic -bios
> > > > > opensbi/build/platform/generic/firmware/fw_dynamic.bin -kernel
> > > > > ./build-riscv64/arch/riscv/boot/Image -append "root=/dev/ram rw
> > > > > console=ttyS0 earlycon" -initrd ./rootfs_riscv64.img -smp 4
> > > >
> > > >
> > > > So splitting memblock.memory is the culprit, it "confuses" the resources
> > > > addition and I can only find hacky ways to fix that...
> > > Hi Alexandre,
> > >
> > > We encountered the same error as Anup. After adding your patch
> > > (3335068f87217ea59d08f462187dc856652eea15), we will not encounter the
> > > error again.
> > >
> > > What I have observed so far is
> > >
> > > - before your patch
> > > When merging consecutive memblocks, if the memblock types are different,
> > > they will be merged into reserved
> > > - after your patch
> > > When consecutive memblocks are merged, if the memblock types are
> > > different, they will be merged into memory.
> > >
> > > Such a result will cause the memory location of OpenSBI to be changed
> > > from reserved to memory. Will this have any side effects?
> >
> > I guess it will end up in the memory pool and pages from openSBI
> > region will be allocated, so we should see very quickly bad stuff
> > happening (either PMP violation or M-mode ecall never
> > returning/trapping/etc).
> >
> > But I don't observe the same thing, I always see the openSBI region
> > being reserved:
> >
> > reserved[0x0] [0x0000000080000000-0x000000008007ffff],
> > 0x0000000000080000 bytes flags: 0x0
> >
> > Can you elaborate a bit more about "When consecutive memblocks are
> > merged, if the memblock types are different, they will be merged into
> > memory"? Where/when does this merge happen? Can you give me a config
> > file and a kernel revision so that I can take a look?
> Ok, If you want to reproduce the same results you just need to modify OpenSBI
>
> [ lib/sbi/sbi_domain.c ]
> +#define TEST_SIZE 0x200000
>
> -                                 (scratch->fw_size - scratch->fw_rw_offset),
> +                                 (TEST_SIZE - scratch->fw_rw_offset),
>
> In addition, you can insert checks in the kernel merged function
> [ mm/memblock.c ]
> static void __init_memblock memblock_merge_regions(struct memblock_type *type)
>         while (i < type->cnt - 1) {
>          ...
>                 /* move forward from next + 1, index of which is i + 2 */
>                 memmove(next, next + 1, (type->cnt - (i + 2)) * sizeof(*next));
>                 type->cnt--;
>         }
> +       pr_info("Merged memblock_type: cnt = %lu, max = %lu,
> total_size = 0x%llx\n",type->cnt, type->max, type->total_size);
> +       for (i = 0; i < type->cnt; i++) {
> +               const char *region_type =
> memblock_is_memory(type->regions[i].base) ? "memory" : "reserve";
> +               pr_info("Region %d: base = 0x%llx, size = 0x%llx, type
> = %s\n", i, type->regions[i].base, type->regions[i].size,
> region_type);
> +       }
>  }
> This is kernel boot log
> - before your patch
> ...
> [    0.000000] OF: fdt: Reserving memory: base = 0x80000000, size = 0x200000
> [    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size = 0x1628501
> [    0.000000] Region 0: base = 0x80000000, size = 0x1600000, type = reserve
> ...
>
> - after your patch
> ...
> [    0.000000] OF: fdt: Reserving memory: base = 0x80000000, size = 0x200000
> [    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size = 0x180c42e
> [    0.000000] Region 0: base = 0x80000000, size = 0x1800000, type = memory

So the openSBI region is marked as memory, and not reserved because
this region is now described as nomap, and memblock_mark_nomap() does
not move this region into the reserved memblock list, but keep it in
the memory list with the nomap flag
(https://elixir.bootlin.com/linux/latest/source/drivers/of/fdt.c#L479).
But as stated in the description of memblock_mark_nomap()
(https://elixir.bootlin.com/linux/latest/source/mm/memblock.c#L969),
the pages associated with the region will be marked as PageReserved
and the region will not be covered in the linear mapping.

So to me, this is normal and we are safe. Let me know if I made a mistake.

And sorry for the long delay, that slipped my mind!

Thanks,

Alex

> ...
> [    0.000000] Failed to add a system RAM resource at 80200000
> ...
> >
> > Thanks,
> >
> > Alex
> >
> > > >
> > > > So given that the arm64 patch with the new API is not pretty and that
> > > > the simplest solution is to re-merge the memblock regions afterwards
> > > > (which is done by memblock_clear_nomap), I'll drop the new API and the
> > > > arm64 patch to use the nomap API like arm64: I'll take advantage of that
> > > > to clean setup_vm_final which I have wanted to do for a long time.
> > > >
> > > > @Mike Thanks for you reviews!
> > > >
> > > > @Anup Thanks for all your bug reports on this patchset, I have to
> > > > improve my test flow (it is in the work :)).
> > > >
> > > >
> > > > > Regards,
> > > > > Anup
> > > > >
> > > > >> Thanks
> > > > >>
> > > > >>> Regards,
> > > > >>> Anup
> > > > >>>
> > > > >>>>   arch/arm64/mm/mmu.c           | 25 +++++++++++------
> > > > >>>>   arch/riscv/include/asm/page.h | 19 +++++++++++--
> > > > >>>>   arch/riscv/mm/init.c          | 53 ++++++++++++++++++++++++++++-------
> > > > >>>>   arch/riscv/mm/physaddr.c      | 16 +++++++++++
> > > > >>>>   drivers/of/fdt.c              | 11 ++++----
> > > > >>>>   include/linux/memblock.h      |  1 +
> > > > >>>>   mm/memblock.c                 | 20 +++++++++++++
> > > > >>>>   7 files changed, 119 insertions(+), 26 deletions(-)
> > > > >>>>
> > > > >>>> --
> > > > >>>> 2.37.2
> > > > >>>>
> > > > > _______________________________________________
> > > > > linux-riscv mailing list
> > > > > linux-riscv@lists.infradead.org
> > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > > >
> > > > _______________________________________________
> > > > linux-riscv mailing list
> > > > linux-riscv@lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Fwd: [PATCH v8 0/4] riscv: Use PUD/P4D/PGD pages for the linear mapping

[Nylon Chen]

Alexandre Ghiti <alexghiti@rivosinc.com> 於 2024年2月5日 週一 下午5:32寫道：
>
> Hi Nylon,
>
> On Fri, Jan 19, 2024 at 10:27 AM Nylon Chen <nylon.chen@sifive.com> wrote:
> >
> > Alexandre Ghiti <alexghiti@rivosinc.com> 於 2024年1月18日 週四 下午9:01寫道：
> > >
> > > Hi Nylon,
> > Hi Alexandre, thanks for your feedback,
> > >
> > > On Thu, Jan 18, 2024 at 9:23 AM Nylon Chen <nylon.chen@sifive.com> wrote:
> > > >
> > > > > On 3/23/23 15:55, Anup Patel wrote:
> > > > > > On Thu, Mar 23, 2023 at 6:24 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > > > > >> Hi Anup,
> > > > > >>
> > > > > >> On Thu, Mar 23, 2023 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote:
> > > > > >>> Hi Alex,
> > > > > >>>
> > > > > >>> On Thu, Mar 16, 2023 at 6:48 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > > > > >>>> This patchset intends to improve tlb utilization by using hugepages for
> > > > > >>>> the linear mapping.
> > > > > >>>>
> > > > > >>>> As reported by Anup in v6, when STRICT_KERNEL_RWX is enabled, we must
> > > > > >>>> take care of isolating the kernel text and rodata so that they are not
> > > > > >>>> mapped with a PUD mapping which would then assign wrong permissions to
> > > > > >>>> the whole region: it is achieved by introducing a new memblock API.
> > > > > >>>>
> > > > > >>>> Another patch makes use of this new API in arm64 which used some sort of
> > > > > >>>> hack to solve this issue: it was built/boot tested successfully.
> > > > > >>>>
> > > > > >>>> base-commit-tag: v6.3-rc1
> > > > > >>>>
> > > > > >>>> v8:
> > > > > >>>> - Fix rv32, as reported by Anup
> > > > > >>>> - Do not modify memblock_isolate_range and fixes comment, as suggested by Mike
> > > > > >>>> - Use the new memblock API for crash kernel too in arm64, as suggested by Andrew
> > > > > >>>> - Fix arm64 double mapping (which to me did not work in v7), but ends up not
> > > > > >>>>    being pretty at all, will wait for comments from arm64 reviewers, but
> > > > > >>>>    this patch can easily be dropped if they do not want it.
> > > > > >>>>
> > > > > >>>> v7:
> > > > > >>>> - Fix Anup bug report by introducing memblock_isolate_memory which
> > > > > >>>>    allows us to split the memblock mappings and then avoid to map the
> > > > > >>>>    the PUD which contains the kernel as read only
> > > > > >>>> - Add a patch to arm64 to use this newly introduced API
> > > > > >>>>
> > > > > >>>> v6:
> > > > > >>>> - quiet LLVM warning by casting phys_ram_base into an unsigned long
> > > > > >>>>
> > > > > >>>> v5:
> > > > > >>>> - Fix nommu builds by getting rid of riscv_pfn_base in patch 1, thanks
> > > > > >>>>    Conor
> > > > > >>>> - Add RB from Andrew
> > > > > >>>>
> > > > > >>>> v4:
> > > > > >>>> - Rebase on top of v6.2-rc3, as noted by Conor
> > > > > >>>> - Add Acked-by Rob
> > > > > >>>>
> > > > > >>>> v3:
> > > > > >>>> - Change the comment about initrd_start VA conversion so that it fits
> > > > > >>>>    ARM64 and RISCV64 (and others in the future if needed), as suggested
> > > > > >>>>    by Rob
> > > > > >>>>
> > > > > >>>> v2:
> > > > > >>>> - Add a comment on why RISCV64 does not need to set initrd_start/end that
> > > > > >>>>    early in the boot process, as asked by Rob
> > > > > >>>>
> > > > > >>>> Alexandre Ghiti (4):
> > > > > >>>>    riscv: Get rid of riscv_pfn_base variable
> > > > > >>>>    mm: Introduce memblock_isolate_memory
> > > > > >>>>    arm64: Make use of memblock_isolate_memory for the linear mapping
> > > > > >>>>    riscv: Use PUD/P4D/PGD pages for the linear mapping
> > > > > >>> Kernel boot fine on RV64 but there is a failure which is still not
> > > > > >>> addressed. You can see this failure as following message in
> > > > > >>> kernel boot log:
> > > > > >>>      0.000000] Failed to add a System RAM resource at 80200000
> > > > > >> Hmmm I don't get that in any of my test configs, would you mind
> > > > > >> sharing yours and your qemu command line?
> > > > > > Try alexghiti_test branch at
> > > > > > https://github.com/avpatel/linux.git
> > > > > >
> > > > > > I am building the kernel using defconfig and my rootfs is
> > > > > > based on busybox.
> > > > > >
> > > > > > My QEMU command is:
> > > > > > qemu-system-riscv64 -M virt -m 512M -nographic -bios
> > > > > > opensbi/build/platform/generic/firmware/fw_dynamic.bin -kernel
> > > > > > ./build-riscv64/arch/riscv/boot/Image -append "root=/dev/ram rw
> > > > > > console=ttyS0 earlycon" -initrd ./rootfs_riscv64.img -smp 4
> > > > >
> > > > >
> > > > > So splitting memblock.memory is the culprit, it "confuses" the resources
> > > > > addition and I can only find hacky ways to fix that...
> > > > Hi Alexandre,
> > > >
> > > > We encountered the same error as Anup. After adding your patch
> > > > (3335068f87217ea59d08f462187dc856652eea15), we will not encounter the
> > > > error again.
> > > >
> > > > What I have observed so far is
> > > >
> > > > - before your patch
> > > > When merging consecutive memblocks, if the memblock types are different,
> > > > they will be merged into reserved
> > > > - after your patch
> > > > When consecutive memblocks are merged, if the memblock types are
> > > > different, they will be merged into memory.
> > > >
> > > > Such a result will cause the memory location of OpenSBI to be changed
> > > > from reserved to memory. Will this have any side effects?
> > >
> > > I guess it will end up in the memory pool and pages from openSBI
> > > region will be allocated, so we should see very quickly bad stuff
> > > happening (either PMP violation or M-mode ecall never
> > > returning/trapping/etc).
> > >
> > > But I don't observe the same thing, I always see the openSBI region
> > > being reserved:
> > >
> > > reserved[0x0] [0x0000000080000000-0x000000008007ffff],
> > > 0x0000000000080000 bytes flags: 0x0
> > >
> > > Can you elaborate a bit more about "When consecutive memblocks are
> > > merged, if the memblock types are different, they will be merged into
> > > memory"? Where/when does this merge happen? Can you give me a config
> > > file and a kernel revision so that I can take a look?
> > Ok, If you want to reproduce the same results you just need to modify OpenSBI
> >
> > [ lib/sbi/sbi_domain.c ]
> > +#define TEST_SIZE 0x200000
> >
> > -                                 (scratch->fw_size - scratch->fw_rw_offset),
> > +                                 (TEST_SIZE - scratch->fw_rw_offset),
> >
> > In addition, you can insert checks in the kernel merged function
> > [ mm/memblock.c ]
> > static void __init_memblock memblock_merge_regions(struct memblock_type *type)
> >         while (i < type->cnt - 1) {
> >          ...
> >                 /* move forward from next + 1, index of which is i + 2 */
> >                 memmove(next, next + 1, (type->cnt - (i + 2)) * sizeof(*next));
> >                 type->cnt--;
> >         }
> > +       pr_info("Merged memblock_type: cnt = %lu, max = %lu,
> > total_size = 0x%llx\n",type->cnt, type->max, type->total_size);
> > +       for (i = 0; i < type->cnt; i++) {
> > +               const char *region_type =
> > memblock_is_memory(type->regions[i].base) ? "memory" : "reserve";
> > +               pr_info("Region %d: base = 0x%llx, size = 0x%llx, type
> > = %s\n", i, type->regions[i].base, type->regions[i].size,
> > region_type);
> > +       }
> >  }
> > This is kernel boot log
> > - before your patch
> > ...
> > [    0.000000] OF: fdt: Reserving memory: base = 0x80000000, size = 0x200000
> > [    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size = 0x1628501
> > [    0.000000] Region 0: base = 0x80000000, size = 0x1600000, type = reserve
> > ...
> >
> > - after your patch
> > ...
> > [    0.000000] OF: fdt: Reserving memory: base = 0x80000000, size = 0x200000
> > [    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size = 0x180c42e
> > [    0.000000] Region 0: base = 0x80000000, size = 0x1800000, type = memory
>
Hi Alex, thanks for your feedback.
> So the openSBI region is marked as memory, and not reserved because
> this region is now described as nomap, and memblock_mark_nomap() does
> not move this region into the reserved memblock list, but keep it in
> the memory list with the nomap flag
> (https://elixir.bootlin.com/linux/latest/source/drivers/of/fdt.c#L479).
> But as stated in the description of memblock_mark_nomap()
> (https://elixir.bootlin.com/linux/latest/source/mm/memblock.c#L969),
> the pages associated with the region will be marked as PageReserved
> and the region will not be covered in the linear mapping.
I traced it via GDB, and indeed, it enters
early_init_dt_reserve_memory() and calls memblock_reserve to reserve
this block of memory.

[before your patch]
[    0.000000] OF: fdt: check nomap Reserving memory: base =
0x80000000, size = 0x200000
[    0.000000] ---  Reserved memory: Base address: 80000000, Size:
200000---
[    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size =
0x1e28501
[    0.000000] Region 0: base = 0x80000000, size = 0x1e00000, type =
reserve
[    0.000000] Region 1: base = 0xbfe00000, size = 0x6002, type =
memory
....
[    0.000000] OF: fdt: Reserved memory: reserved region for node
'mmode_resv0@80000000': base 0x0000000080000000, size 2 MiB
[    0.000000] OF: reserved mem:
0x0000000080000000..0x00000000801fffff (2048 KiB) map non-reusable
mmode_resv0@80000000

[after your patch]
[    0.000000] OF: fdt: check nomap Reserving memory: base =
0x80000000, size = 0x200000
[    0.000000] --- Reserved memory: Base address: 80000000, Size: 200000---
[    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size = 0x1e25501
[    0.000000] Region 0: base = 0x80000000, size = 0x1e00000, type = memory
[    0.000000] Region 1: base = 0xbfe00000, size = 0x6002, type = memory
...
[    0.000000] OF: fdt: Reserved memory: reserved region for node
'mmode_resv0@80000000': base 0x0000000080000000, size 2 MiB
[    0.000000] OF: reserved mem:
0x0000000080000000..0x00000000801fffff (2048 KiB) map non-reusable
mmode_resv0@80000000

At the moment, it can be confirmed that there is no need to worry
about this block of memory being used.

But I still have a question I'd like to ask, which is why this
location is flagged as 'reserve' instead of 'memory' in the memblock

Thanks,
Nylon
>
> So to me, this is normal and we are safe. Let me know if I made a mistake.
>
> And sorry for the long delay, that slipped my mind!
>
> Thanks,
>
> Alex
>
> > ...
> > [    0.000000] Failed to add a system RAM resource at 80200000
> > ...
> > >
> > > Thanks,
> > >
> > > Alex
> > >
> > > > >
> > > > > So given that the arm64 patch with the new API is not pretty and that
> > > > > the simplest solution is to re-merge the memblock regions afterwards
> > > > > (which is done by memblock_clear_nomap), I'll drop the new API and the
> > > > > arm64 patch to use the nomap API like arm64: I'll take advantage of that
> > > > > to clean setup_vm_final which I have wanted to do for a long time.
> > > > >
> > > > > @Mike Thanks for you reviews!
> > > > >
> > > > > @Anup Thanks for all your bug reports on this patchset, I have to
> > > > > improve my test flow (it is in the work :)).
> > > > >
> > > > >
> > > > > > Regards,
> > > > > > Anup
> > > > > >
> > > > > >> Thanks
> > > > > >>
> > > > > >>> Regards,
> > > > > >>> Anup
> > > > > >>>
> > > > > >>>>   arch/arm64/mm/mmu.c           | 25 +++++++++++------
> > > > > >>>>   arch/riscv/include/asm/page.h | 19 +++++++++++--
> > > > > >>>>   arch/riscv/mm/init.c          | 53 ++++++++++++++++++++++++++++-------
> > > > > >>>>   arch/riscv/mm/physaddr.c      | 16 +++++++++++
> > > > > >>>>   drivers/of/fdt.c              | 11 ++++----
> > > > > >>>>   include/linux/memblock.h      |  1 +
> > > > > >>>>   mm/memblock.c                 | 20 +++++++++++++
> > > > > >>>>   7 files changed, 119 insertions(+), 26 deletions(-)
> > > > > >>>>
> > > > > >>>> --
> > > > > >>>> 2.37.2
> > > > > >>>>
> > > > > > _______________________________________________
> > > > > > linux-riscv mailing list
> > > > > > linux-riscv@lists.infradead.org
> > > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > > > >
> > > > > _______________________________________________
> > > > > linux-riscv mailing list
> > > > > linux-riscv@lists.infradead.org
> > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Fwd: [PATCH v8 0/4] riscv: Use PUD/P4D/PGD pages for the linear mapping

[Nylon Chen]

Nylon Chen <nylon.chen@sifive.com> 於 2024年3月12日 週二 下午2:40寫道：
>
> Alexandre Ghiti <alexghiti@rivosinc.com> 於 2024年2月5日 週一 下午5:32寫道：
> >
> > Hi Nylon,
> >
> > On Fri, Jan 19, 2024 at 10:27 AM Nylon Chen <nylon.chen@sifive.com> wrote:
> > >
> > > Alexandre Ghiti <alexghiti@rivosinc.com> 於 2024年1月18日 週四 下午9:01寫道：
> > > >
> > > > Hi Nylon,
> > > Hi Alexandre, thanks for your feedback,
> > > >
> > > > On Thu, Jan 18, 2024 at 9:23 AM Nylon Chen <nylon.chen@sifive.com> wrote:
> > > > >
> > > > > > On 3/23/23 15:55, Anup Patel wrote:
> > > > > > > On Thu, Mar 23, 2023 at 6:24 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > > > > > >> Hi Anup,
> > > > > > >>
> > > > > > >> On Thu, Mar 23, 2023 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote:
> > > > > > >>> Hi Alex,
> > > > > > >>>
> > > > > > >>> On Thu, Mar 16, 2023 at 6:48 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > > > > > >>>> This patchset intends to improve tlb utilization by using hugepages for
> > > > > > >>>> the linear mapping.
> > > > > > >>>>
> > > > > > >>>> As reported by Anup in v6, when STRICT_KERNEL_RWX is enabled, we must
> > > > > > >>>> take care of isolating the kernel text and rodata so that they are not
> > > > > > >>>> mapped with a PUD mapping which would then assign wrong permissions to
> > > > > > >>>> the whole region: it is achieved by introducing a new memblock API.
> > > > > > >>>>
> > > > > > >>>> Another patch makes use of this new API in arm64 which used some sort of
> > > > > > >>>> hack to solve this issue: it was built/boot tested successfully.
> > > > > > >>>>
> > > > > > >>>> base-commit-tag: v6.3-rc1
> > > > > > >>>>
> > > > > > >>>> v8:
> > > > > > >>>> - Fix rv32, as reported by Anup
> > > > > > >>>> - Do not modify memblock_isolate_range and fixes comment, as suggested by Mike
> > > > > > >>>> - Use the new memblock API for crash kernel too in arm64, as suggested by Andrew
> > > > > > >>>> - Fix arm64 double mapping (which to me did not work in v7), but ends up not
> > > > > > >>>>    being pretty at all, will wait for comments from arm64 reviewers, but
> > > > > > >>>>    this patch can easily be dropped if they do not want it.
> > > > > > >>>>
> > > > > > >>>> v7:
> > > > > > >>>> - Fix Anup bug report by introducing memblock_isolate_memory which
> > > > > > >>>>    allows us to split the memblock mappings and then avoid to map the
> > > > > > >>>>    the PUD which contains the kernel as read only
> > > > > > >>>> - Add a patch to arm64 to use this newly introduced API
> > > > > > >>>>
> > > > > > >>>> v6:
> > > > > > >>>> - quiet LLVM warning by casting phys_ram_base into an unsigned long
> > > > > > >>>>
> > > > > > >>>> v5:
> > > > > > >>>> - Fix nommu builds by getting rid of riscv_pfn_base in patch 1, thanks
> > > > > > >>>>    Conor
> > > > > > >>>> - Add RB from Andrew
> > > > > > >>>>
> > > > > > >>>> v4:
> > > > > > >>>> - Rebase on top of v6.2-rc3, as noted by Conor
> > > > > > >>>> - Add Acked-by Rob
> > > > > > >>>>
> > > > > > >>>> v3:
> > > > > > >>>> - Change the comment about initrd_start VA conversion so that it fits
> > > > > > >>>>    ARM64 and RISCV64 (and others in the future if needed), as suggested
> > > > > > >>>>    by Rob
> > > > > > >>>>
> > > > > > >>>> v2:
> > > > > > >>>> - Add a comment on why RISCV64 does not need to set initrd_start/end that
> > > > > > >>>>    early in the boot process, as asked by Rob
> > > > > > >>>>
> > > > > > >>>> Alexandre Ghiti (4):
> > > > > > >>>>    riscv: Get rid of riscv_pfn_base variable
> > > > > > >>>>    mm: Introduce memblock_isolate_memory
> > > > > > >>>>    arm64: Make use of memblock_isolate_memory for the linear mapping
> > > > > > >>>>    riscv: Use PUD/P4D/PGD pages for the linear mapping
> > > > > > >>> Kernel boot fine on RV64 but there is a failure which is still not
> > > > > > >>> addressed. You can see this failure as following message in
> > > > > > >>> kernel boot log:
> > > > > > >>>      0.000000] Failed to add a System RAM resource at 80200000
> > > > > > >> Hmmm I don't get that in any of my test configs, would you mind
> > > > > > >> sharing yours and your qemu command line?
> > > > > > > Try alexghiti_test branch at
> > > > > > > https://github.com/avpatel/linux.git
> > > > > > >
> > > > > > > I am building the kernel using defconfig and my rootfs is
> > > > > > > based on busybox.
> > > > > > >
> > > > > > > My QEMU command is:
> > > > > > > qemu-system-riscv64 -M virt -m 512M -nographic -bios
> > > > > > > opensbi/build/platform/generic/firmware/fw_dynamic.bin -kernel
> > > > > > > ./build-riscv64/arch/riscv/boot/Image -append "root=/dev/ram rw
> > > > > > > console=ttyS0 earlycon" -initrd ./rootfs_riscv64.img -smp 4
> > > > > >
> > > > > >
> > > > > > So splitting memblock.memory is the culprit, it "confuses" the resources
> > > > > > addition and I can only find hacky ways to fix that...
> > > > > Hi Alexandre,
> > > > >
> > > > > We encountered the same error as Anup. After adding your patch
> > > > > (3335068f87217ea59d08f462187dc856652eea15), we will not encounter the
> > > > > error again.
> > > > >
> > > > > What I have observed so far is
> > > > >
> > > > > - before your patch
> > > > > When merging consecutive memblocks, if the memblock types are different,
> > > > > they will be merged into reserved
> > > > > - after your patch
> > > > > When consecutive memblocks are merged, if the memblock types are
> > > > > different, they will be merged into memory.
> > > > >
> > > > > Such a result will cause the memory location of OpenSBI to be changed
> > > > > from reserved to memory. Will this have any side effects?
> > > >
> > > > I guess it will end up in the memory pool and pages from openSBI
> > > > region will be allocated, so we should see very quickly bad stuff
> > > > happening (either PMP violation or M-mode ecall never
> > > > returning/trapping/etc).
> > > >
> > > > But I don't observe the same thing, I always see the openSBI region
> > > > being reserved:
> > > >
> > > > reserved[0x0] [0x0000000080000000-0x000000008007ffff],
> > > > 0x0000000000080000 bytes flags: 0x0
> > > >
> > > > Can you elaborate a bit more about "When consecutive memblocks are
> > > > merged, if the memblock types are different, they will be merged into
> > > > memory"? Where/when does this merge happen? Can you give me a config
> > > > file and a kernel revision so that I can take a look?
> > > Ok, If you want to reproduce the same results you just need to modify OpenSBI
> > >
> > > [ lib/sbi/sbi_domain.c ]
> > > +#define TEST_SIZE 0x200000
> > >
> > > -                                 (scratch->fw_size - scratch->fw_rw_offset),
> > > +                                 (TEST_SIZE - scratch->fw_rw_offset),
> > >
> > > In addition, you can insert checks in the kernel merged function
> > > [ mm/memblock.c ]
> > > static void __init_memblock memblock_merge_regions(struct memblock_type *type)
> > >         while (i < type->cnt - 1) {
> > >          ...
> > >                 /* move forward from next + 1, index of which is i + 2 */
> > >                 memmove(next, next + 1, (type->cnt - (i + 2)) * sizeof(*next));
> > >                 type->cnt--;
> > >         }
> > > +       pr_info("Merged memblock_type: cnt = %lu, max = %lu,
> > > total_size = 0x%llx\n",type->cnt, type->max, type->total_size);
> > > +       for (i = 0; i < type->cnt; i++) {
> > > +               const char *region_type =
> > > memblock_is_memory(type->regions[i].base) ? "memory" : "reserve";
> > > +               pr_info("Region %d: base = 0x%llx, size = 0x%llx, type
> > > = %s\n", i, type->regions[i].base, type->regions[i].size,
> > > region_type);
> > > +       }
> > >  }
> > > This is kernel boot log
> > > - before your patch
> > > ...
> > > [    0.000000] OF: fdt: Reserving memory: base = 0x80000000, size = 0x200000
> > > [    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size = 0x1628501
> > > [    0.000000] Region 0: base = 0x80000000, size = 0x1600000, type = reserve
> > > ...
> > >
> > > - after your patch
> > > ...
> > > [    0.000000] OF: fdt: Reserving memory: base = 0x80000000, size = 0x200000
> > > [    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size = 0x180c42e
> > > [    0.000000] Region 0: base = 0x80000000, size = 0x1800000, type = memory
> >
> Hi Alex, thanks for your feedback.
> > So the openSBI region is marked as memory, and not reserved because
> > this region is now described as nomap, and memblock_mark_nomap() does
> > not move this region into the reserved memblock list, but keep it in
> > the memory list with the nomap flag
> > (https://elixir.bootlin.com/linux/latest/source/drivers/of/fdt.c#L479).
> > But as stated in the description of memblock_mark_nomap()
> > (https://elixir.bootlin.com/linux/latest/source/mm/memblock.c#L969),
> > the pages associated with the region will be marked as PageReserved
> > and the region will not be covered in the linear mapping.
> I traced it via GDB, and indeed, it enters
> early_init_dt_reserve_memory() and calls memblock_reserve to reserve
> this block of memory.
>
> [before your patch]
> [    0.000000] OF: fdt: check nomap Reserving memory: base =
> 0x80000000, size = 0x200000
> [    0.000000] ---  Reserved memory: Base address: 80000000, Size:
> 200000---
> [    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size =
> 0x1e28501
> [    0.000000] Region 0: base = 0x80000000, size = 0x1e00000, type =
> reserve
> [    0.000000] Region 1: base = 0xbfe00000, size = 0x6002, type =
> memory
> ....
> [    0.000000] OF: fdt: Reserved memory: reserved region for node
> 'mmode_resv0@80000000': base 0x0000000080000000, size 2 MiB
> [    0.000000] OF: reserved mem:
> 0x0000000080000000..0x00000000801fffff (2048 KiB) map non-reusable
> mmode_resv0@80000000
>
> [after your patch]
> [    0.000000] OF: fdt: check nomap Reserving memory: base =
> 0x80000000, size = 0x200000
> [    0.000000] --- Reserved memory: Base address: 80000000, Size: 200000---
> [    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size = 0x1e25501
> [    0.000000] Region 0: base = 0x80000000, size = 0x1e00000, type = memory
> [    0.000000] Region 1: base = 0xbfe00000, size = 0x6002, type = memory
> ...
> [    0.000000] OF: fdt: Reserved memory: reserved region for node
> 'mmode_resv0@80000000': base 0x0000000080000000, size 2 MiB
> [    0.000000] OF: reserved mem:
> 0x0000000080000000..0x00000000801fffff (2048 KiB) map non-reusable
> mmode_resv0@80000000
>
> At the moment, it can be confirmed that there is no need to worry
> about this block of memory being used.
>
> But I still have a question I'd like to ask, which is why this
> location is flagged as 'reserve' instead of 'memory' in the memblock
Sorry, I asked the wrong question.

Why is this location marked as "memory" instead of "reserve" in the memblock?
>
> Thanks,
> Nylon
> >
> > So to me, this is normal and we are safe. Let me know if I made a mistake.
> >
> > And sorry for the long delay, that slipped my mind!
> >
> > Thanks,
> >
> > Alex
> >
> > > ...
> > > [    0.000000] Failed to add a system RAM resource at 80200000
> > > ...
> > > >
> > > > Thanks,
> > > >
> > > > Alex
> > > >
> > > > > >
> > > > > > So given that the arm64 patch with the new API is not pretty and that
> > > > > > the simplest solution is to re-merge the memblock regions afterwards
> > > > > > (which is done by memblock_clear_nomap), I'll drop the new API and the
> > > > > > arm64 patch to use the nomap API like arm64: I'll take advantage of that
> > > > > > to clean setup_vm_final which I have wanted to do for a long time.
> > > > > >
> > > > > > @Mike Thanks for you reviews!
> > > > > >
> > > > > > @Anup Thanks for all your bug reports on this patchset, I have to
> > > > > > improve my test flow (it is in the work :)).
> > > > > >
> > > > > >
> > > > > > > Regards,
> > > > > > > Anup
> > > > > > >
> > > > > > >> Thanks
> > > > > > >>
> > > > > > >>> Regards,
> > > > > > >>> Anup
> > > > > > >>>
> > > > > > >>>>   arch/arm64/mm/mmu.c           | 25 +++++++++++------
> > > > > > >>>>   arch/riscv/include/asm/page.h | 19 +++++++++++--
> > > > > > >>>>   arch/riscv/mm/init.c          | 53 ++++++++++++++++++++++++++++-------
> > > > > > >>>>   arch/riscv/mm/physaddr.c      | 16 +++++++++++
> > > > > > >>>>   drivers/of/fdt.c              | 11 ++++----
> > > > > > >>>>   include/linux/memblock.h      |  1 +
> > > > > > >>>>   mm/memblock.c                 | 20 +++++++++++++
> > > > > > >>>>   7 files changed, 119 insertions(+), 26 deletions(-)
> > > > > > >>>>
> > > > > > >>>> --
> > > > > > >>>> 2.37.2
> > > > > > >>>>
> > > > > > > _______________________________________________
> > > > > > > linux-riscv mailing list
> > > > > > > linux-riscv@lists.infradead.org
> > > > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > > > > >
> > > > > > _______________________________________________
> > > > > > linux-riscv mailing list
> > > > > > linux-riscv@lists.infradead.org
> > > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Fwd: [PATCH v8 0/4] riscv: Use PUD/P4D/PGD pages for the linear mapping

[Alexandre Ghiti]

Hi Nylon,

On Tue, Mar 12, 2024 at 7:48 AM Nylon Chen <nylon.chen@sifive.com> wrote:
>
> Nylon Chen <nylon.chen@sifive.com> 於 2024年3月12日 週二 下午2:40寫道：
> >
> > Alexandre Ghiti <alexghiti@rivosinc.com> 於 2024年2月5日 週一 下午5:32寫道：
> > >
> > > Hi Nylon,
> > >
> > > On Fri, Jan 19, 2024 at 10:27 AM Nylon Chen <nylon.chen@sifive.com> wrote:
> > > >
> > > > Alexandre Ghiti <alexghiti@rivosinc.com> 於 2024年1月18日 週四 下午9:01寫道：
> > > > >
> > > > > Hi Nylon,
> > > > Hi Alexandre, thanks for your feedback,
> > > > >
> > > > > On Thu, Jan 18, 2024 at 9:23 AM Nylon Chen <nylon.chen@sifive.com> wrote:
> > > > > >
> > > > > > > On 3/23/23 15:55, Anup Patel wrote:
> > > > > > > > On Thu, Mar 23, 2023 at 6:24 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > > > > > > >> Hi Anup,
> > > > > > > >>
> > > > > > > >> On Thu, Mar 23, 2023 at 1:18 PM Anup Patel <apatel@ventanamicro.com> wrote:
> > > > > > > >>> Hi Alex,
> > > > > > > >>>
> > > > > > > >>> On Thu, Mar 16, 2023 at 6:48 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > > > > > > >>>> This patchset intends to improve tlb utilization by using hugepages for
> > > > > > > >>>> the linear mapping.
> > > > > > > >>>>
> > > > > > > >>>> As reported by Anup in v6, when STRICT_KERNEL_RWX is enabled, we must
> > > > > > > >>>> take care of isolating the kernel text and rodata so that they are not
> > > > > > > >>>> mapped with a PUD mapping which would then assign wrong permissions to
> > > > > > > >>>> the whole region: it is achieved by introducing a new memblock API.
> > > > > > > >>>>
> > > > > > > >>>> Another patch makes use of this new API in arm64 which used some sort of
> > > > > > > >>>> hack to solve this issue: it was built/boot tested successfully.
> > > > > > > >>>>
> > > > > > > >>>> base-commit-tag: v6.3-rc1
> > > > > > > >>>>
> > > > > > > >>>> v8:
> > > > > > > >>>> - Fix rv32, as reported by Anup
> > > > > > > >>>> - Do not modify memblock_isolate_range and fixes comment, as suggested by Mike
> > > > > > > >>>> - Use the new memblock API for crash kernel too in arm64, as suggested by Andrew
> > > > > > > >>>> - Fix arm64 double mapping (which to me did not work in v7), but ends up not
> > > > > > > >>>>    being pretty at all, will wait for comments from arm64 reviewers, but
> > > > > > > >>>>    this patch can easily be dropped if they do not want it.
> > > > > > > >>>>
> > > > > > > >>>> v7:
> > > > > > > >>>> - Fix Anup bug report by introducing memblock_isolate_memory which
> > > > > > > >>>>    allows us to split the memblock mappings and then avoid to map the
> > > > > > > >>>>    the PUD which contains the kernel as read only
> > > > > > > >>>> - Add a patch to arm64 to use this newly introduced API
> > > > > > > >>>>
> > > > > > > >>>> v6:
> > > > > > > >>>> - quiet LLVM warning by casting phys_ram_base into an unsigned long
> > > > > > > >>>>
> > > > > > > >>>> v5:
> > > > > > > >>>> - Fix nommu builds by getting rid of riscv_pfn_base in patch 1, thanks
> > > > > > > >>>>    Conor
> > > > > > > >>>> - Add RB from Andrew
> > > > > > > >>>>
> > > > > > > >>>> v4:
> > > > > > > >>>> - Rebase on top of v6.2-rc3, as noted by Conor
> > > > > > > >>>> - Add Acked-by Rob
> > > > > > > >>>>
> > > > > > > >>>> v3:
> > > > > > > >>>> - Change the comment about initrd_start VA conversion so that it fits
> > > > > > > >>>>    ARM64 and RISCV64 (and others in the future if needed), as suggested
> > > > > > > >>>>    by Rob
> > > > > > > >>>>
> > > > > > > >>>> v2:
> > > > > > > >>>> - Add a comment on why RISCV64 does not need to set initrd_start/end that
> > > > > > > >>>>    early in the boot process, as asked by Rob
> > > > > > > >>>>
> > > > > > > >>>> Alexandre Ghiti (4):
> > > > > > > >>>>    riscv: Get rid of riscv_pfn_base variable
> > > > > > > >>>>    mm: Introduce memblock_isolate_memory
> > > > > > > >>>>    arm64: Make use of memblock_isolate_memory for the linear mapping
> > > > > > > >>>>    riscv: Use PUD/P4D/PGD pages for the linear mapping
> > > > > > > >>> Kernel boot fine on RV64 but there is a failure which is still not
> > > > > > > >>> addressed. You can see this failure as following message in
> > > > > > > >>> kernel boot log:
> > > > > > > >>>      0.000000] Failed to add a System RAM resource at 80200000
> > > > > > > >> Hmmm I don't get that in any of my test configs, would you mind
> > > > > > > >> sharing yours and your qemu command line?
> > > > > > > > Try alexghiti_test branch at
> > > > > > > > https://github.com/avpatel/linux.git
> > > > > > > >
> > > > > > > > I am building the kernel using defconfig and my rootfs is
> > > > > > > > based on busybox.
> > > > > > > >
> > > > > > > > My QEMU command is:
> > > > > > > > qemu-system-riscv64 -M virt -m 512M -nographic -bios
> > > > > > > > opensbi/build/platform/generic/firmware/fw_dynamic.bin -kernel
> > > > > > > > ./build-riscv64/arch/riscv/boot/Image -append "root=/dev/ram rw
> > > > > > > > console=ttyS0 earlycon" -initrd ./rootfs_riscv64.img -smp 4
> > > > > > >
> > > > > > >
> > > > > > > So splitting memblock.memory is the culprit, it "confuses" the resources
> > > > > > > addition and I can only find hacky ways to fix that...
> > > > > > Hi Alexandre,
> > > > > >
> > > > > > We encountered the same error as Anup. After adding your patch
> > > > > > (3335068f87217ea59d08f462187dc856652eea15), we will not encounter the
> > > > > > error again.
> > > > > >
> > > > > > What I have observed so far is
> > > > > >
> > > > > > - before your patch
> > > > > > When merging consecutive memblocks, if the memblock types are different,
> > > > > > they will be merged into reserved
> > > > > > - after your patch
> > > > > > When consecutive memblocks are merged, if the memblock types are
> > > > > > different, they will be merged into memory.
> > > > > >
> > > > > > Such a result will cause the memory location of OpenSBI to be changed
> > > > > > from reserved to memory. Will this have any side effects?
> > > > >
> > > > > I guess it will end up in the memory pool and pages from openSBI
> > > > > region will be allocated, so we should see very quickly bad stuff
> > > > > happening (either PMP violation or M-mode ecall never
> > > > > returning/trapping/etc).
> > > > >
> > > > > But I don't observe the same thing, I always see the openSBI region
> > > > > being reserved:
> > > > >
> > > > > reserved[0x0] [0x0000000080000000-0x000000008007ffff],
> > > > > 0x0000000000080000 bytes flags: 0x0
> > > > >
> > > > > Can you elaborate a bit more about "When consecutive memblocks are
> > > > > merged, if the memblock types are different, they will be merged into
> > > > > memory"? Where/when does this merge happen? Can you give me a config
> > > > > file and a kernel revision so that I can take a look?
> > > > Ok, If you want to reproduce the same results you just need to modify OpenSBI
> > > >
> > > > [ lib/sbi/sbi_domain.c ]
> > > > +#define TEST_SIZE 0x200000
> > > >
> > > > -                                 (scratch->fw_size - scratch->fw_rw_offset),
> > > > +                                 (TEST_SIZE - scratch->fw_rw_offset),
> > > >
> > > > In addition, you can insert checks in the kernel merged function
> > > > [ mm/memblock.c ]
> > > > static void __init_memblock memblock_merge_regions(struct memblock_type *type)
> > > >         while (i < type->cnt - 1) {
> > > >          ...
> > > >                 /* move forward from next + 1, index of which is i + 2 */
> > > >                 memmove(next, next + 1, (type->cnt - (i + 2)) * sizeof(*next));
> > > >                 type->cnt--;
> > > >         }
> > > > +       pr_info("Merged memblock_type: cnt = %lu, max = %lu,
> > > > total_size = 0x%llx\n",type->cnt, type->max, type->total_size);
> > > > +       for (i = 0; i < type->cnt; i++) {
> > > > +               const char *region_type =
> > > > memblock_is_memory(type->regions[i].base) ? "memory" : "reserve";
> > > > +               pr_info("Region %d: base = 0x%llx, size = 0x%llx, type
> > > > = %s\n", i, type->regions[i].base, type->regions[i].size,
> > > > region_type);
> > > > +       }
> > > >  }
> > > > This is kernel boot log
> > > > - before your patch
> > > > ...
> > > > [    0.000000] OF: fdt: Reserving memory: base = 0x80000000, size = 0x200000
> > > > [    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size = 0x1628501
> > > > [    0.000000] Region 0: base = 0x80000000, size = 0x1600000, type = reserve
> > > > ...
> > > >
> > > > - after your patch
> > > > ...
> > > > [    0.000000] OF: fdt: Reserving memory: base = 0x80000000, size = 0x200000
> > > > [    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size = 0x180c42e
> > > > [    0.000000] Region 0: base = 0x80000000, size = 0x1800000, type = memory
> > >
> > Hi Alex, thanks for your feedback.
> > > So the openSBI region is marked as memory, and not reserved because
> > > this region is now described as nomap, and memblock_mark_nomap() does
> > > not move this region into the reserved memblock list, but keep it in
> > > the memory list with the nomap flag
> > > (https://elixir.bootlin.com/linux/latest/source/drivers/of/fdt.c#L479).
> > > But as stated in the description of memblock_mark_nomap()
> > > (https://elixir.bootlin.com/linux/latest/source/mm/memblock.c#L969),
> > > the pages associated with the region will be marked as PageReserved
> > > and the region will not be covered in the linear mapping.
> > I traced it via GDB, and indeed, it enters
> > early_init_dt_reserve_memory() and calls memblock_reserve to reserve
> > this block of memory.
> >
> > [before your patch]
> > [    0.000000] OF: fdt: check nomap Reserving memory: base =
> > 0x80000000, size = 0x200000
> > [    0.000000] ---  Reserved memory: Base address: 80000000, Size:
> > 200000---
> > [    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size =
> > 0x1e28501
> > [    0.000000] Region 0: base = 0x80000000, size = 0x1e00000, type =
> > reserve
> > [    0.000000] Region 1: base = 0xbfe00000, size = 0x6002, type =
> > memory
> > ....
> > [    0.000000] OF: fdt: Reserved memory: reserved region for node
> > 'mmode_resv0@80000000': base 0x0000000080000000, size 2 MiB
> > [    0.000000] OF: reserved mem:
> > 0x0000000080000000..0x00000000801fffff (2048 KiB) map non-reusable
> > mmode_resv0@80000000
> >
> > [after your patch]
> > [    0.000000] OF: fdt: check nomap Reserving memory: base =
> > 0x80000000, size = 0x200000
> > [    0.000000] --- Reserved memory: Base address: 80000000, Size: 200000---
> > [    0.000000] Merged memblock_type: cnt = 4, max = 128, total_size = 0x1e25501
> > [    0.000000] Region 0: base = 0x80000000, size = 0x1e00000, type = memory
> > [    0.000000] Region 1: base = 0xbfe00000, size = 0x6002, type = memory
> > ...
> > [    0.000000] OF: fdt: Reserved memory: reserved region for node
> > 'mmode_resv0@80000000': base 0x0000000080000000, size 2 MiB
> > [    0.000000] OF: reserved mem:
> > 0x0000000080000000..0x00000000801fffff (2048 KiB) map non-reusable
> > mmode_resv0@80000000
> >
> > At the moment, it can be confirmed that there is no need to worry
> > about this block of memory being used.
> >
> > But I still have a question I'd like to ask, which is why this
> > location is flagged as 'reserve' instead of 'memory' in the memblock
> Sorry, I asked the wrong question.
>
> Why is this location marked as "memory" instead of "reserve" in the memblock?

No idea, let's see if @Mike Rapoport can answer this :)

> >
> > Thanks,
> > Nylon
> > >
> > > So to me, this is normal and we are safe. Let me know if I made a mistake.
> > >
> > > And sorry for the long delay, that slipped my mind!
> > >
> > > Thanks,
> > >
> > > Alex
> > >
> > > > ...
> > > > [    0.000000] Failed to add a system RAM resource at 80200000
> > > > ...
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Alex
> > > > >
> > > > > > >
> > > > > > > So given that the arm64 patch with the new API is not pretty and that
> > > > > > > the simplest solution is to re-merge the memblock regions afterwards
> > > > > > > (which is done by memblock_clear_nomap), I'll drop the new API and the
> > > > > > > arm64 patch to use the nomap API like arm64: I'll take advantage of that
> > > > > > > to clean setup_vm_final which I have wanted to do for a long time.
> > > > > > >
> > > > > > > @Mike Thanks for you reviews!
> > > > > > >
> > > > > > > @Anup Thanks for all your bug reports on this patchset, I have to
> > > > > > > improve my test flow (it is in the work :)).
> > > > > > >
> > > > > > >
> > > > > > > > Regards,
> > > > > > > > Anup
> > > > > > > >
> > > > > > > >> Thanks
> > > > > > > >>
> > > > > > > >>> Regards,
> > > > > > > >>> Anup
> > > > > > > >>>
> > > > > > > >>>>   arch/arm64/mm/mmu.c           | 25 +++++++++++------
> > > > > > > >>>>   arch/riscv/include/asm/page.h | 19 +++++++++++--
> > > > > > > >>>>   arch/riscv/mm/init.c          | 53 ++++++++++++++++++++++++++++-------
> > > > > > > >>>>   arch/riscv/mm/physaddr.c      | 16 +++++++++++
> > > > > > > >>>>   drivers/of/fdt.c              | 11 ++++----
> > > > > > > >>>>   include/linux/memblock.h      |  1 +
> > > > > > > >>>>   mm/memblock.c                 | 20 +++++++++++++
> > > > > > > >>>>   7 files changed, 119 insertions(+), 26 deletions(-)
> > > > > > > >>>>
> > > > > > > >>>> --
> > > > > > > >>>> 2.37.2
> > > > > > > >>>>
> > > > > > > > _______________________________________________
> > > > > > > > linux-riscv mailing list
> > > > > > > > linux-riscv@lists.infradead.org
> > > > > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > > > > > >
> > > > > > > _______________________________________________
> > > > > > > linux-riscv mailing list
> > > > > > > linux-riscv@lists.infradead.org
> > > > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel