[RFC v2 PATCH 0/2] arm64: support FEAT_BBM level 2 and large block mapping when rodata=full

[RFC v2 PATCH 0/2] arm64: support FEAT_BBM level 2 and large block mapping when rodata=full

[Yang Shi]

When rodata=full kernel linear mapping is mapped by PTE due to arm's
break-before-make rule.

This resulted in a couple of problems:
  - performance degradation
  - more TLB pressure
  - memory waste for kernel page table

There are some workarounds to mitigate the problems, for example, using
rodata=on, but this compromises the security measurement.

With FEAT_BBM level 2 support, splitting large block page table to
smaller ones doesn't need to make the page table entry invalid anymore.
This allows kernel split large block mapping on the fly.

Add kernel page table split support and use large block mapping by
default when FEAT_BBM level 2 is supported for rodata=full.  When
changing permissions for kernel linear mapping, the page table will be
split to PTE level.

The machine without FEAT_BBM level 2 will fallback to have kernel linear
mapping PTE-mapped when rodata=full.

With this we saw significant performance boost with some benchmarks with
keeping rodata=full security protection in the mean time.

The test was done on AmpereOne machine (192 cores, 1P) with 256GB memory and
4K page size + 48 bit VA.

Function test (4K/16K/64K page size)
  - Kernel boot.  Kernel needs change kernel linear mapping permission at
    boot stage, if the patch didn't work, kernel typically didn't boot.
  - Module stress from stress-ng. Kernel module load change permission for
    module sections.
  - A test kernel module which allocates 80% of total memory via vmalloc(),
    then change the vmalloc area permission to RO, then change it back
    before vfree(). Then launch a VM which consumes almost all physical
    memory.
  - VM with the patchset applied in guest kernel too.
  - Kernel build in VM with patched guest kernel.

Memory consumption
Before:
MemTotal:       258988984 kB
MemFree:        254821700 kB

After:
MemTotal:       259505132 kB
MemFree:        255410264 kB

Around 500MB more memory are free to use.  The larger the machine, the
more memory saved.

Performance benchmarking
* Memcached
We saw performance degradation when running Memcached benchmark with
rodata=full vs rodata=on.  Our profiling pointed to kernel TLB pressure.
With this patchset we saw ops/sec is increased by around 3.5%, P99
latency is reduced by around 9.6%.
The gain mainly came from reduced kernel TLB misses.  The kernel TLB
MPKI is reduced by 28.5%.

The benchmark data is now on par with rodata=on too.

* Disk encryption (dm-crypt) benchmark
Ran fio benchmark with the below command on a 128G ramdisk (ext4) with disk
encryption (by dm-crypt).
fio --directory=/data --random_generator=lfsr --norandommap --randrepeat 1 \
    --status-interval=999 --rw=write --bs=4k --loops=1 --ioengine=sync \
    --iodepth=1 --numjobs=1 --fsync_on_close=1 --group_reporting --thread \
    --name=iops-test-job --eta-newline=1 --size 100G

The IOPS is increased by 90% - 150% (the variance is high, but the worst
number of good case is around 90% more than the best number of bad case).
The bandwidth is increased and the avg clat is reduced proportionally.

* Sequential file read
Read 100G file sequentially on XFS (xfs_io read with page cache populated).
The bandwidth is increased by 150%.

RFC v2:
  * Used allowlist to advertise BBM lv2 on the CPUs which can handle TLB
    conflict gracefully per Will Deacon
  * Rebased onto v6.13-rc5

RFC v1: https://lore.kernel.org/lkml/20241118181711.962576-1-yang@os.amperecomputing.com/

Yang Shi (2):
      arm64: cpufeature: detect FEAT_BBM level 2
      arm64: mm: support large block mapping when rodata=full

 arch/arm64/include/asm/cpufeature.h |  19 ++++++++++++
 arch/arm64/include/asm/pgtable.h    |   7 ++++-
 arch/arm64/kernel/cpufeature.c      |  11 +++++++
 arch/arm64/mm/mmu.c                 |  32 ++++++++++++++++++--
 arch/arm64/mm/pageattr.c            | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 arch/arm64/tools/cpucaps            |   1 +
 6 files changed, 234 insertions(+), 9 deletions(-)

[v2 PATCH 1/2] arm64: cpufeature: detect FEAT_BBM level 2

[Yang Shi]

FEAT_BBM level 2 allow changing block size of a translation with relaxed
TLB flushing.  But it may incur TLB conflict abort.  We can handle the
abort in kernel, however it is hard to guarantee the recuesive TLB
conflct will never happen in the handling itself.

Some implementations can handle TLB conflict gracefully without fault
handler in kernel so FEAT_BBM level 2 can be enabled on those
implementations safely.

Look up MIDR to filter out those CPUs.  AmpereOne is one of them.

Suggested-by: Will Deacon <will@kernel.org>
Signed-off-by: Yang Shi <yang@os.amperecomputing.com>
---
 arch/arm64/include/asm/cpufeature.h | 19 +++++++++++++++++++
 arch/arm64/kernel/cpufeature.c      | 11 +++++++++++
 arch/arm64/tools/cpucaps            |  1 +
 3 files changed, 31 insertions(+)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 8b4e5a3cd24c..33ca9db42741 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -866,6 +866,25 @@ static __always_inline bool system_supports_mpam_hcr(void)
 	return alternative_has_cap_unlikely(ARM64_MPAM_HCR);
 }
 
+static inline bool system_supports_bbmlv2(void)
+{
+	return cpus_have_final_boot_cap(ARM64_HAS_BBMLV2);
+}
+
+static inline bool bbmlv2_available(void)
+{
+	static const struct midr_range support_bbmlv2[] = {
+		MIDR_ALL_VERSIONS(MIDR_AMPERE1),
+		MIDR_ALL_VERSIONS(MIDR_AMPERE1A),
+		{}
+	};
+
+	if (is_midr_in_range_list(read_cpuid_id(), support_bbmlv2))
+		return true;
+
+	return false;
+}
+
 int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt);
 bool try_emulate_mrs(struct pt_regs *regs, u32 isn);
 
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 6ce71f444ed8..a60d5fa04828 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1889,6 +1889,11 @@ static bool has_lpa2(const struct arm64_cpu_capabilities *entry, int scope)
 }
 #endif
 
+static bool has_bbmlv2(const struct arm64_cpu_capabilities *entry, int scope)
+{
+	return bbmlv2_available();
+}
+
 #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
 #define KPTI_NG_TEMP_VA		(-(1UL << PMD_SHIFT))
 
@@ -2990,6 +2995,12 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		ARM64_CPUID_FIELDS(ID_AA64PFR1_EL1, GCS, IMP)
 	},
 #endif
+	{
+		.desc = "BBM Level 2",
+		.capability = ARM64_HAS_BBMLV2,
+		.type = ARM64_CPUCAP_BOOT_CPU_FEATURE,
+		.matches = has_bbmlv2,
+	},
 	{},
 };
 
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index eb17f59e543c..287bdede53f5 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -14,6 +14,7 @@ HAS_ADDRESS_AUTH_ARCH_QARMA5
 HAS_ADDRESS_AUTH_IMP_DEF
 HAS_AMU_EXTN
 HAS_ARMv8_4_TTL
+HAS_BBMLV2
 HAS_CACHE_DIC
 HAS_CACHE_IDC
 HAS_CNP
-- 
2.45.2

[v2 PATCH 2/2] arm64: mm: support large block mapping when rodata=full

[Yang Shi]

When rodata=full is specified, kernel linear mapping has to be mapped at
PTE level since large page table can't be split due to break-before-make
rule on ARM64.

This resulted in a couple of problems:
  - performance degradation
  - more TLB pressure
  - memory waste for kernel page table

With FEAT_BBM level 2 support, splitting large block page table to
smaller ones doesn't need to make the page table entry invalid anymore.
This allows kernel split large block mapping on the fly.

Add kernel page table split support and use large block mapping by
default when FEAT_BBM level 2 is supported for rodata=full.  When
changing permissions for kernel linear mapping, the page table will be
split to PTE level.

The machine without FEAT_BBM level 2 will fallback to have kernel linear
mapping PTE-mapped when rodata=full.

With this we saw significant performance boost with some benchmarks and
much less memory consumption on my AmpereOne machine (192 cores, 1P) with
256GB memory.

* Memory use after boot
Before:
MemTotal:       258988984 kB
MemFree:        254821700 kB

After:
MemTotal:       259505132 kB
MemFree:        255410264 kB

Around 500MB more memory are free to use.  The larger the machine, the
more memory saved.

* Memcached
We saw performance degradation when running Memcached benchmark with
rodata=full vs rodata=on.  Our profiling pointed to kernel TLB pressure.
With this patchset we saw ops/sec is increased by around 3.5%, P99
latency is reduced by around 9.6%.
The gain mainly came from reduced kernel TLB misses.  The kernel TLB
MPKI is reduced by 28.5%.

The benchmark data is now on par with rodata=on too.

* Disk encryption (dm-crypt) benchmark
Ran fio benchmark with the below command on a 128G ramdisk (ext4) with disk
encryption (by dm-crypt).
fio --directory=/data --random_generator=lfsr --norandommap --randrepeat 1 \
    --status-interval=999 --rw=write --bs=4k --loops=1 --ioengine=sync \
    --iodepth=1 --numjobs=1 --fsync_on_close=1 --group_reporting --thread \
    --name=iops-test-job --eta-newline=1 --size 100G

The IOPS is increased by 90% - 150% (the variance is high, but the worst
number of good case is around 90% more than the best number of bad case).
The bandwidth is increased and the avg clat is reduced proportionally.

* Sequential file read
Read 100G file sequentially on XFS (xfs_io read with page cache populated).
The bandwidth is increased by 150%.

Keep using PTE mapping when pagealloc debug is enabled.  It is not worth
the complexity.

Kfence can be converted to use page block mapping later.

Signed-off-by: Yang Shi <yang@os.amperecomputing.com>
---
 arch/arm64/include/asm/pgtable.h |   7 +-
 arch/arm64/mm/mmu.c              |  32 +++++-
 arch/arm64/mm/pageattr.c         | 173 +++++++++++++++++++++++++++++--
 3 files changed, 203 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 6986345b537a..2927f59ae9e2 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -749,7 +749,7 @@ static inline bool in_swapper_pgdir(void *addr)
 	        ((unsigned long)swapper_pg_dir & PAGE_MASK);
 }
 
-static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
+static inline void __set_pmd_nosync(pmd_t *pmdp, pmd_t pmd)
 {
 #ifdef __PAGETABLE_PMD_FOLDED
 	if (in_swapper_pgdir(pmdp)) {
@@ -759,6 +759,11 @@ static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
 #endif /* __PAGETABLE_PMD_FOLDED */
 
 	WRITE_ONCE(*pmdp, pmd);
+}
+
+static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
+{
+	__set_pmd_nosync(pmdp, pmd);
 
 	if (pmd_valid(pmd)) {
 		dsb(ishst);
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index e2739b69e11b..65b5b1ba48a7 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -620,6 +620,19 @@ static inline void arm64_kfence_map_pool(phys_addr_t kfence_pool, pgd_t *pgdp) {
 
 #endif /* CONFIG_KFENCE */
 
+static inline bool force_pte_mapping(void)
+{
+	/*
+	 * Can't use cpufeature API to determine whether BBM level 2
+	 * is supported or not since cpufeature have not been
+	 * finalized yet.
+	 */
+	return (rodata_full && !bbmlv2_available()) ||
+		debug_pagealloc_enabled() ||
+		arm64_kfence_can_set_direct_map() ||
+		is_realm_world();
+}
+
 static void __init map_mem(pgd_t *pgdp)
 {
 	static const u64 direct_map_end = _PAGE_END(VA_BITS_MIN);
@@ -645,9 +658,21 @@ static void __init map_mem(pgd_t *pgdp)
 
 	early_kfence_pool = arm64_kfence_alloc_pool();
 
-	if (can_set_direct_map())
+	if (force_pte_mapping())
 		flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
 
+	/*
+	 * With FEAT_BBM level 2 we can split large block mapping without
+	 * making it invalid.  So kernel linear mapping can be mapped with
+	 * large block instead of PTE level.
+	 *
+	 * Need to break cont for CONT_MAPPINGS when changing permission,
+	 * and need to inspect the adjacent page table entries to make
+	 * them cont again later.  It sounds not worth the complexity.
+	 */
+	if (rodata_full)
+		flags |= NO_CONT_MAPPINGS;
+
 	/*
 	 * Take care not to create a writable alias for the
 	 * read-only text and rodata sections of the kernel image.
@@ -1342,9 +1367,12 @@ int arch_add_memory(int nid, u64 start, u64 size,
 
 	VM_BUG_ON(!mhp_range_allowed(start, size, true));
 
-	if (can_set_direct_map())
+	if (force_pte_mapping())
 		flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
 
+	if (rodata_full)
+		flags |= NO_CONT_MAPPINGS;
+
 	__create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
 			     size, params->pgprot, __pgd_pgtable_alloc,
 			     flags);
diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
index 39fd1f7ff02a..ba2360ecb030 100644
--- a/arch/arm64/mm/pageattr.c
+++ b/arch/arm64/mm/pageattr.c
@@ -49,6 +49,145 @@ static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
 	return 0;
 }
 
+static int __split_linear_mapping_pmd(pud_t *pudp,
+				      unsigned long vaddr, unsigned long end)
+{
+	pmd_t *pmdp;
+	unsigned long next;
+
+	pmdp = pmd_offset(pudp, vaddr);
+
+	do {
+		next = pmd_addr_end(vaddr, end);
+
+		if (pmd_leaf(pmdp_get(pmdp))) {
+			struct page *pte_page;
+			unsigned long pfn = pmd_pfn(pmdp_get(pmdp));
+			pgprot_t prot = pmd_pgprot(pmdp_get(pmdp));
+			pte_t *ptep_new;
+			int i;
+
+			pte_page = alloc_page(GFP_KERNEL);
+			if (!pte_page)
+				return -ENOMEM;
+
+			prot = __pgprot(pgprot_val(prot) | PTE_TYPE_PAGE);
+			ptep_new = (pte_t *)page_address(pte_page);
+			for (i = 0; i < PTRS_PER_PTE; ++i, ++ptep_new)
+				__set_pte_nosync(ptep_new,
+						 pfn_pte(pfn + i, prot));
+
+			dsb(ishst);
+			isb();
+
+			set_pmd(pmdp, pfn_pmd(page_to_pfn(pte_page),
+				__pgprot(PMD_TYPE_TABLE)));
+		}
+	} while (pmdp++, vaddr = next, vaddr != end);
+
+	return 0;
+}
+
+static int __split_linear_mapping_pud(p4d_t *p4dp,
+				      unsigned long vaddr, unsigned long end)
+{
+	pud_t *pudp;
+	unsigned long next;
+	int ret;
+
+	pudp = pud_offset(p4dp, vaddr);
+
+	do {
+		next = pud_addr_end(vaddr, end);
+
+		if (pud_leaf(pudp_get(pudp))) {
+			struct page *pmd_page;
+			unsigned long pfn = pud_pfn(pudp_get(pudp));
+			pgprot_t prot = pud_pgprot(pudp_get(pudp));
+			pmd_t *pmdp_new;
+			int i;
+			unsigned int step;
+
+			pmd_page = alloc_page(GFP_KERNEL);
+			if (!pmd_page)
+				return -ENOMEM;
+
+			pmdp_new = (pmd_t *)page_address(pmd_page);
+			for (i = 0; i < PTRS_PER_PMD; ++i, ++pmdp_new) {
+				step = (i * PMD_SIZE) >> PAGE_SHIFT;
+				__set_pmd_nosync(pmdp_new,
+						 pfn_pmd(pfn + step, prot));
+			}
+
+			dsb(ishst);
+			isb();
+
+			set_pud(pudp, pfn_pud(page_to_pfn(pmd_page),
+				__pgprot(PUD_TYPE_TABLE)));
+		}
+
+		ret = __split_linear_mapping_pmd(pudp, vaddr, next);
+		if (ret)
+			return ret;
+	} while (pudp++, vaddr = next, vaddr != end);
+
+	return 0;
+}
+
+static int __split_linear_mapping_p4d(pgd_t *pgdp,
+				      unsigned long vaddr, unsigned long end)
+{
+	p4d_t *p4dp;
+	unsigned long next;
+	int ret;
+
+	p4dp = p4d_offset(pgdp, vaddr);
+
+	do {
+		next = p4d_addr_end(vaddr, end);
+
+		ret = __split_linear_mapping_pud(p4dp, vaddr, next);
+		if (ret)
+			return ret;
+	} while (p4dp++, vaddr = next, vaddr != end);
+
+	return 0;
+}
+
+static int __split_linear_mapping_pgd(pgd_t *pgdp,
+				      unsigned long vaddr,
+				      unsigned long end)
+{
+	unsigned long next;
+	int ret = 0;
+
+	mmap_write_lock(&init_mm);
+
+	do {
+		next = pgd_addr_end(vaddr, end);
+		ret = __split_linear_mapping_p4d(pgdp, vaddr, next);
+		if (ret)
+			break;
+	} while (pgdp++, vaddr = next, vaddr != end);
+
+	mmap_write_unlock(&init_mm);
+
+	return ret;
+}
+
+static int split_linear_mapping(unsigned long start, unsigned long end)
+{
+	int ret;
+
+	if (!system_supports_bbmlv2())
+		return 0;
+
+	ret = __split_linear_mapping_pgd(pgd_offset_k(start), start, end);
+	flush_tlb_kernel_range(start, end);
+
+	return ret;
+}
+
 /*
  * This function assumes that the range is mapped with PAGE_SIZE pages.
  */
@@ -80,8 +219,9 @@ static int change_memory_common(unsigned long addr, int numpages,
 	unsigned long start = addr;
 	unsigned long size = PAGE_SIZE * numpages;
 	unsigned long end = start + size;
+	unsigned long l_start;
 	struct vm_struct *area;
-	int i;
+	int i, ret;
 
 	if (!PAGE_ALIGNED(addr)) {
 		start &= PAGE_MASK;
@@ -118,7 +258,12 @@ static int change_memory_common(unsigned long addr, int numpages,
 	if (rodata_full && (pgprot_val(set_mask) == PTE_RDONLY ||
 			    pgprot_val(clear_mask) == PTE_RDONLY)) {
 		for (i = 0; i < area->nr_pages; i++) {
-			__change_memory_common((u64)page_address(area->pages[i]),
+			l_start = (u64)page_address(area->pages[i]);
+			ret = split_linear_mapping(l_start, l_start + PAGE_SIZE);
+			if (WARN_ON_ONCE(ret))
+				return ret;
+
+			__change_memory_common(l_start,
 					       PAGE_SIZE, set_mask, clear_mask);
 		}
 	}
@@ -174,6 +319,9 @@ int set_memory_valid(unsigned long addr, int numpages, int enable)
 
 int set_direct_map_invalid_noflush(struct page *page)
 {
+	unsigned long l_start;
+	int ret;
+
 	struct page_change_data data = {
 		.set_mask = __pgprot(0),
 		.clear_mask = __pgprot(PTE_VALID),
@@ -182,13 +330,21 @@ int set_direct_map_invalid_noflush(struct page *page)
 	if (!can_set_direct_map())
 		return 0;
 
+	l_start = (unsigned long)page_address(page);
+	ret = split_linear_mapping(l_start, l_start + PAGE_SIZE);
+	if (WARN_ON_ONCE(ret))
+		return ret;
+
 	return apply_to_page_range(&init_mm,
-				   (unsigned long)page_address(page),
-				   PAGE_SIZE, change_page_range, &data);
+				   l_start, PAGE_SIZE, change_page_range,
+				   &data);
 }
 
 int set_direct_map_default_noflush(struct page *page)
 {
+	unsigned long l_start;
+	int ret;
+
 	struct page_change_data data = {
 		.set_mask = __pgprot(PTE_VALID | PTE_WRITE),
 		.clear_mask = __pgprot(PTE_RDONLY),
@@ -197,9 +353,14 @@ int set_direct_map_default_noflush(struct page *page)
 	if (!can_set_direct_map())
 		return 0;
 
+	l_start = (unsigned long)page_address(page);
+	ret = split_linear_mapping(l_start, l_start + PAGE_SIZE);
+	if (WARN_ON_ONCE(ret))
+		return ret;
+
 	return apply_to_page_range(&init_mm,
-				   (unsigned long)page_address(page),
-				   PAGE_SIZE, change_page_range, &data);
+				   l_start, PAGE_SIZE, change_page_range,
+				   &data);
 }
 
 static int __set_memory_enc_dec(unsigned long addr,
-- 
2.45.2

Re: [RFC v2 PATCH 0/2] arm64: support FEAT_BBM level 2 and large block mapping when rodata=full

[Yang Shi]

Ping...

Per the prior discussion, the BBM level 2 feature is advertised via a 
allow list in this version. Any more comment on the implementation and 
patch 2? I expected there should be more comments on patch 2.

Thanks,
Yang



On 1/2/25 5:17 PM, Yang Shi wrote:
> When rodata=full kernel linear mapping is mapped by PTE due to arm's
> break-before-make rule.
>
> This resulted in a couple of problems:
>    - performance degradation
>    - more TLB pressure
>    - memory waste for kernel page table
>
> There are some workarounds to mitigate the problems, for example, using
> rodata=on, but this compromises the security measurement.
>
> With FEAT_BBM level 2 support, splitting large block page table to
> smaller ones doesn't need to make the page table entry invalid anymore.
> This allows kernel split large block mapping on the fly.
>
> Add kernel page table split support and use large block mapping by
> default when FEAT_BBM level 2 is supported for rodata=full.  When
> changing permissions for kernel linear mapping, the page table will be
> split to PTE level.
>
> The machine without FEAT_BBM level 2 will fallback to have kernel linear
> mapping PTE-mapped when rodata=full.
>
> With this we saw significant performance boost with some benchmarks with
> keeping rodata=full security protection in the mean time.
>
> The test was done on AmpereOne machine (192 cores, 1P) with 256GB memory and
> 4K page size + 48 bit VA.
>
> Function test (4K/16K/64K page size)
>    - Kernel boot.  Kernel needs change kernel linear mapping permission at
>      boot stage, if the patch didn't work, kernel typically didn't boot.
>    - Module stress from stress-ng. Kernel module load change permission for
>      module sections.
>    - A test kernel module which allocates 80% of total memory via vmalloc(),
>      then change the vmalloc area permission to RO, then change it back
>      before vfree(). Then launch a VM which consumes almost all physical
>      memory.
>    - VM with the patchset applied in guest kernel too.
>    - Kernel build in VM with patched guest kernel.
>
> Memory consumption
> Before:
> MemTotal:       258988984 kB
> MemFree:        254821700 kB
>
> After:
> MemTotal:       259505132 kB
> MemFree:        255410264 kB
>
> Around 500MB more memory are free to use.  The larger the machine, the
> more memory saved.
>
> Performance benchmarking
> * Memcached
> We saw performance degradation when running Memcached benchmark with
> rodata=full vs rodata=on.  Our profiling pointed to kernel TLB pressure.
> With this patchset we saw ops/sec is increased by around 3.5%, P99
> latency is reduced by around 9.6%.
> The gain mainly came from reduced kernel TLB misses.  The kernel TLB
> MPKI is reduced by 28.5%.
>
> The benchmark data is now on par with rodata=on too.
>
> * Disk encryption (dm-crypt) benchmark
> Ran fio benchmark with the below command on a 128G ramdisk (ext4) with disk
> encryption (by dm-crypt).
> fio --directory=/data --random_generator=lfsr --norandommap --randrepeat 1 \
>      --status-interval=999 --rw=write --bs=4k --loops=1 --ioengine=sync \
>      --iodepth=1 --numjobs=1 --fsync_on_close=1 --group_reporting --thread \
>      --name=iops-test-job --eta-newline=1 --size 100G
>
> The IOPS is increased by 90% - 150% (the variance is high, but the worst
> number of good case is around 90% more than the best number of bad case).
> The bandwidth is increased and the avg clat is reduced proportionally.
>
> * Sequential file read
> Read 100G file sequentially on XFS (xfs_io read with page cache populated).
> The bandwidth is increased by 150%.
>
> RFC v2:
>    * Used allowlist to advertise BBM lv2 on the CPUs which can handle TLB
>      conflict gracefully per Will Deacon
>    * Rebased onto v6.13-rc5
>
> RFC v1: https://lore.kernel.org/lkml/20241118181711.962576-1-yang@os.amperecomputing.com/
>
> Yang Shi (2):
>        arm64: cpufeature: detect FEAT_BBM level 2
>        arm64: mm: support large block mapping when rodata=full
>
>   arch/arm64/include/asm/cpufeature.h |  19 ++++++++++++
>   arch/arm64/include/asm/pgtable.h    |   7 ++++-
>   arch/arm64/kernel/cpufeature.c      |  11 +++++++
>   arch/arm64/mm/mmu.c                 |  32 ++++++++++++++++++--
>   arch/arm64/mm/pageattr.c            | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>   arch/arm64/tools/cpucaps            |   1 +
>   6 files changed, 234 insertions(+), 9 deletions(-)

Re: [RFC v2 PATCH 0/2] arm64: support FEAT_BBM level 2 and large block mapping when rodata=full

[Ryan Roberts]

Sorry managed to send this to the list only. Resending with original recipients
added back in...


On 11/02/2025 11:34, Ryan Roberts wrote:
> Hi Yang,
> 
> Thanks for putting this together; I'm hoping to piggyback on this and use BBML2
> to reduce the cost of contpte_convert().
> 
> review incoming...
> 
> 
> On 03/01/2025 01:17, Yang Shi wrote:
>>
>> When rodata=full kernel linear mapping is mapped by PTE due to arm's
>> break-before-make rule.
>>
>> This resulted in a couple of problems:
>>   - performance degradation
>>   - more TLB pressure
>>   - memory waste for kernel page table
>>
>> There are some workarounds to mitigate the problems, for example, using
>> rodata=on, but this compromises the security measurement.
>>
>> With FEAT_BBM level 2 support, splitting large block page table to
>> smaller ones doesn't need to make the page table entry invalid anymore.
>> This allows kernel split large block mapping on the fly.
>>
>> Add kernel page table split support and use large block mapping by
>> default when FEAT_BBM level 2 is supported for rodata=full.  When
>> changing permissions for kernel linear mapping, the page table will be
>> split to PTE level.
>>
>> The machine without FEAT_BBM level 2 will fallback to have kernel linear
>> mapping PTE-mapped when rodata=full.
>>
>> With this we saw significant performance boost with some benchmarks with
>> keeping rodata=full security protection in the mean time.
>>
>> The test was done on AmpereOne machine (192 cores, 1P) with 256GB memory and
>> 4K page size + 48 bit VA.
>>
>> Function test (4K/16K/64K page size)
>>   - Kernel boot.  Kernel needs change kernel linear mapping permission at
>>     boot stage, if the patch didn't work, kernel typically didn't boot.
>>   - Module stress from stress-ng. Kernel module load change permission for
>>     module sections.
>>   - A test kernel module which allocates 80% of total memory via vmalloc(),
>>     then change the vmalloc area permission to RO, then change it back
>>     before vfree(). Then launch a VM which consumes almost all physical
>>     memory.
> 
> I don't really understand how vmalloc is relevant here? vmalloc can already map
> huge pages you use vmalloc_huge() and changing the permissions of vmalloc
> mapping will only affect the ptes pertaining to that mapping; I don't see why
> that would cause permissions to be changed on the linear map or for huge pages
> in the linear map to be split?
> 
>>   - VM with the patchset applied in guest kernel too.
>>   - Kernel build in VM with patched guest kernel.
>>
>> Memory consumption
>> Before:
>> MemTotal:       258988984 kB
>> MemFree:        254821700 kB
>>
>> After:
>> MemTotal:       259505132 kB
>> MemFree:        255410264 kB
>>
>> Around 500MB more memory are free to use.  The larger the machine, the
>> more memory saved.
>>
>> Performance benchmarking
>> * Memcached
>> We saw performance degradation when running Memcached benchmark with
>> rodata=full vs rodata=on.  Our profiling pointed to kernel TLB pressure.
>> With this patchset we saw ops/sec is increased by around 3.5%, P99
>> latency is reduced by around 9.6%.
>> The gain mainly came from reduced kernel TLB misses.  The kernel TLB
>> MPKI is reduced by 28.5%.
>>
>> The benchmark data is now on par with rodata=on too.
>>
>> * Disk encryption (dm-crypt) benchmark
>> Ran fio benchmark with the below command on a 128G ramdisk (ext4) with disk
>> encryption (by dm-crypt).
>> fio --directory=/data --random_generator=lfsr --norandommap --randrepeat 1 \
>>     --status-interval=999 --rw=write --bs=4k --loops=1 --ioengine=sync \
>>     --iodepth=1 --numjobs=1 --fsync_on_close=1 --group_reporting --thread \
>>     --name=iops-test-job --eta-newline=1 --size 100G
>>
>> The IOPS is increased by 90% - 150% (the variance is high, but the worst
>> number of good case is around 90% more than the best number of bad case).
>> The bandwidth is increased and the avg clat is reduced proportionally.
>>
>> * Sequential file read
>> Read 100G file sequentially on XFS (xfs_io read with page cache populated).
>> The bandwidth is increased by 150%.
> 
> The performance gains definitely look worthwhile!
> 
> Thanks,
> Ryan
> 
>>
>> RFC v2:
>>   * Used allowlist to advertise BBM lv2 on the CPUs which can handle TLB
>>     conflict gracefully per Will Deacon
>>   * Rebased onto v6.13-rc5
>>
>> RFC v1: https://lore.kernel.org/lkml/20241118181711.962576-1-yang@os.amperecomputing.com/
>>
>> Yang Shi (2):
>>       arm64: cpufeature: detect FEAT_BBM level 2
>>       arm64: mm: support large block mapping when rodata=full
>>
>>  arch/arm64/include/asm/cpufeature.h |  19 ++++++++++++
>>  arch/arm64/include/asm/pgtable.h    |   7 ++++-
>>  arch/arm64/kernel/cpufeature.c      |  11 +++++++
>>  arch/arm64/mm/mmu.c                 |  32 ++++++++++++++++++--
>>  arch/arm64/mm/pageattr.c            | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>>  arch/arm64/tools/cpucaps            |   1 +
>>  6 files changed, 234 insertions(+), 9 deletions(-)
>>
>>
> 

Re: [v2 PATCH 1/2] arm64: cpufeature: detect FEAT_BBM level 2

[Ryan Roberts]

On 03/01/2025 01:17, Yang Shi wrote:
> FEAT_BBM level 2 allow changing block size of a translation with relaxed
> TLB flushing.  But it may incur TLB conflict abort.  We can handle the
> abort in kernel, however it is hard to guarantee the recuesive TLB

nit: recuesive -> recursive ?

> conflct will never happen in the handling itself.
> 
> Some implementations can handle TLB conflict gracefully without fault
> handler in kernel so FEAT_BBM level 2 can be enabled on those
> implementations safely.
> 
> Look up MIDR to filter out those CPUs.  AmpereOne is one of them.
> 
> Suggested-by: Will Deacon <will@kernel.org>
> Signed-off-by: Yang Shi <yang@os.amperecomputing.com>
> ---
>  arch/arm64/include/asm/cpufeature.h | 19 +++++++++++++++++++
>  arch/arm64/kernel/cpufeature.c      | 11 +++++++++++
>  arch/arm64/tools/cpucaps            |  1 +
>  3 files changed, 31 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index 8b4e5a3cd24c..33ca9db42741 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -866,6 +866,25 @@ static __always_inline bool system_supports_mpam_hcr(void)
>  	return alternative_has_cap_unlikely(ARM64_MPAM_HCR);
>  }
>  
> +static inline bool system_supports_bbmlv2(void)

nit: Arm language internally is starting to refer to FEAT_BBML1 / FEAT_BBML2 and
I believe this will soon make it's way to the Arm ARM. So probably better to
refer to bbml2 rather than bbmlv2 throughout.

> +{
> +	return cpus_have_final_boot_cap(ARM64_HAS_BBMLV2);
> +}
> +
> +static inline bool bbmlv2_available(void)

This function has no need to be in the header. system_supports_bbmlv2() is what
users should use. Suggest moving to has_bbmlv2() in cpufeature.c.

> +{
> +	static const struct midr_range support_bbmlv2[] = {
> +		MIDR_ALL_VERSIONS(MIDR_AMPERE1),
> +		MIDR_ALL_VERSIONS(MIDR_AMPERE1A),
> +		{}
> +	};
> +
> +	if (is_midr_in_range_list(read_cpuid_id(), support_bbmlv2))
> +		return true;
> +
> +	return false;
> +}
> +
>  int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt);
>  bool try_emulate_mrs(struct pt_regs *regs, u32 isn);
>  
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 6ce71f444ed8..a60d5fa04828 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1889,6 +1889,11 @@ static bool has_lpa2(const struct arm64_cpu_capabilities *entry, int scope)
>  }
>  #endif
>  
> +static bool has_bbmlv2(const struct arm64_cpu_capabilities *entry, int scope)
> +{
> +	return bbmlv2_available();
> +}
> +
>  #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
>  #define KPTI_NG_TEMP_VA		(-(1UL << PMD_SHIFT))
>  
> @@ -2990,6 +2995,12 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>  		ARM64_CPUID_FIELDS(ID_AA64PFR1_EL1, GCS, IMP)
>  	},
>  #endif
> +	{
> +		.desc = "BBM Level 2",
> +		.capability = ARM64_HAS_BBMLV2,
> +		.type = ARM64_CPUCAP_BOOT_CPU_FEATURE,

I'm wondering if this will potentially lead to problems for assymetric
collections of CPUs (e.g. big.LITTLE)? I can imagine that little CPUs might not
support BBML2. In this case if you boot on a big CPU that does have BBML2, you
will require the feature and refuse to online the secondary little CPUs.

Perhaps this really needs to be a system feature, where it is only enabled if
all CPUs in the system support it? I'm guessing that will make painting the
linear map harder; I guess you will need to initially set it up with PTE
mappings, then repaint as block mappings if BBML2 is determined to be supported
if that's not already what you are doing.

Thanks,
Ryan

> +		.matches = has_bbmlv2,
> +	},
>  	{},
>  };
>  
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index eb17f59e543c..287bdede53f5 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -14,6 +14,7 @@ HAS_ADDRESS_AUTH_ARCH_QARMA5
>  HAS_ADDRESS_AUTH_IMP_DEF
>  HAS_AMU_EXTN
>  HAS_ARMv8_4_TTL
> +HAS_BBMLV2
>  HAS_CACHE_DIC
>  HAS_CACHE_IDC
>  HAS_CNP

Re: [v2 PATCH 2/2] arm64: mm: support large block mapping when rodata=full

[Ryan Roberts]

On 03/01/2025 01:17, Yang Shi wrote:
> When rodata=full is specified, kernel linear mapping has to be mapped at
> PTE level since large page table can't be split due to break-before-make
> rule on ARM64.
> 
> This resulted in a couple of problems:
>   - performance degradation
>   - more TLB pressure
>   - memory waste for kernel page table
> 
> With FEAT_BBM level 2 support, splitting large block page table to
> smaller ones doesn't need to make the page table entry invalid anymore.
> This allows kernel split large block mapping on the fly.
> 
> Add kernel page table split support and use large block mapping by
> default when FEAT_BBM level 2 is supported for rodata=full.  When
> changing permissions for kernel linear mapping, the page table will be
> split to PTE level.
> 
> The machine without FEAT_BBM level 2 will fallback to have kernel linear
> mapping PTE-mapped when rodata=full.
> 
> With this we saw significant performance boost with some benchmarks and
> much less memory consumption on my AmpereOne machine (192 cores, 1P) with
> 256GB memory.
> 
> * Memory use after boot
> Before:
> MemTotal:       258988984 kB
> MemFree:        254821700 kB
> 
> After:
> MemTotal:       259505132 kB
> MemFree:        255410264 kB
> 
> Around 500MB more memory are free to use.  The larger the machine, the
> more memory saved.
> 
> * Memcached
> We saw performance degradation when running Memcached benchmark with
> rodata=full vs rodata=on.  Our profiling pointed to kernel TLB pressure.
> With this patchset we saw ops/sec is increased by around 3.5%, P99
> latency is reduced by around 9.6%.
> The gain mainly came from reduced kernel TLB misses.  The kernel TLB
> MPKI is reduced by 28.5%.
> 
> The benchmark data is now on par with rodata=on too.
> 
> * Disk encryption (dm-crypt) benchmark
> Ran fio benchmark with the below command on a 128G ramdisk (ext4) with disk
> encryption (by dm-crypt).
> fio --directory=/data --random_generator=lfsr --norandommap --randrepeat 1 \
>     --status-interval=999 --rw=write --bs=4k --loops=1 --ioengine=sync \
>     --iodepth=1 --numjobs=1 --fsync_on_close=1 --group_reporting --thread \
>     --name=iops-test-job --eta-newline=1 --size 100G
> 
> The IOPS is increased by 90% - 150% (the variance is high, but the worst
> number of good case is around 90% more than the best number of bad case).
> The bandwidth is increased and the avg clat is reduced proportionally.
> 
> * Sequential file read
> Read 100G file sequentially on XFS (xfs_io read with page cache populated).
> The bandwidth is increased by 150%.
> 
> Keep using PTE mapping when pagealloc debug is enabled.  It is not worth
> the complexity.
> 
> Kfence can be converted to use page block mapping later.
> 
> Signed-off-by: Yang Shi <yang@os.amperecomputing.com>
> ---
>  arch/arm64/include/asm/pgtable.h |   7 +-
>  arch/arm64/mm/mmu.c              |  32 +++++-
>  arch/arm64/mm/pageattr.c         | 173 +++++++++++++++++++++++++++++--
>  3 files changed, 203 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 6986345b537a..2927f59ae9e2 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -749,7 +749,7 @@ static inline bool in_swapper_pgdir(void *addr)
>  	        ((unsigned long)swapper_pg_dir & PAGE_MASK);
>  }
>  
> -static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
> +static inline void __set_pmd_nosync(pmd_t *pmdp, pmd_t pmd)
>  {
>  #ifdef __PAGETABLE_PMD_FOLDED
>  	if (in_swapper_pgdir(pmdp)) {
> @@ -759,6 +759,11 @@ static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
>  #endif /* __PAGETABLE_PMD_FOLDED */
>  
>  	WRITE_ONCE(*pmdp, pmd);
> +}
> +
> +static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
> +{
> +	__set_pmd_nosync(pmdp, pmd);

I have a series at [1] which enables batching/deferring these barriers. I reckon
it would be udeful to ultimately apply that same technique for painting the
linear map if/when that series goes upstream. The benefit is that we wouldn't
need to expose the _nosync helpers. But that's something to cleanup later. This
looks fine for now.

>  
>  	if (pmd_valid(pmd)) {
>  		dsb(ishst);
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index e2739b69e11b..65b5b1ba48a7 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -620,6 +620,19 @@ static inline void arm64_kfence_map_pool(phys_addr_t kfence_pool, pgd_t *pgdp) {
>  
>  #endif /* CONFIG_KFENCE */
>  
> +static inline bool force_pte_mapping(void)
> +{
> +	/*
> +	 * Can't use cpufeature API to determine whether BBM level 2
> +	 * is supported or not since cpufeature have not been
> +	 * finalized yet.
> +	 */

I think assuming the system has BMML2 just because the boot CPU has it is going
to be problematic. I think you are going to continue to need to initially paint
the linear map with ptes then later, once the system cap is finalized, repaint
with blocks.

> +	return (rodata_full && !bbmlv2_available()) ||
> +		debug_pagealloc_enabled() ||
> +		arm64_kfence_can_set_direct_map() ||
> +		is_realm_world();
> +}

Previously the decision was determined by:

bool can_set_direct_map(void)
{
	/*
	 * rodata_full, DEBUG_PAGEALLOC and a Realm guest all require linear
	 * map to be mapped at page granularity, so that it is possible to
	 * protect/unprotect single pages.
	 *
	 * KFENCE pool requires page-granular mapping if initialized late.
	 *
	 * Realms need to make pages shared/protected at page granularity.
	 */
	return rodata_full || debug_pagealloc_enabled() ||
		arm64_kfence_can_set_direct_map() || is_realm_world();
}

But now that you can safely split block mappings to page mappings when needed
doesn't that cover some of these requirements? So perhaps you can do block
mappings even when some (all?) of these things are active?


> +
>  static void __init map_mem(pgd_t *pgdp)
>  {
>  	static const u64 direct_map_end = _PAGE_END(VA_BITS_MIN);
> @@ -645,9 +658,21 @@ static void __init map_mem(pgd_t *pgdp)
>  
>  	early_kfence_pool = arm64_kfence_alloc_pool();
>  
> -	if (can_set_direct_map())
> +	if (force_pte_mapping())
>  		flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
>  
> +	/*
> +	 * With FEAT_BBM level 2 we can split large block mapping without
> +	 * making it invalid.  So kernel linear mapping can be mapped with
> +	 * large block instead of PTE level.
> +	 *
> +	 * Need to break cont for CONT_MAPPINGS when changing permission,
> +	 * and need to inspect the adjacent page table entries to make
> +	 * them cont again later.  It sounds not worth the complexity.

I'm not sure I follow the argument? As far as I can tell, you only ever split
mappings. You never collapse. So I don't think it's too complex to check for
CONT bits and if set, clear the CONT bit from the block? That's a safe operation
with BBML2. It means you need to iterate over the ptes which you don't currently
do, but you only need to check a 1 PTE in every CONT_PTES.

> +	 */
> +	if (rodata_full)
> +		flags |= NO_CONT_MAPPINGS;
> +
>  	/*
>  	 * Take care not to create a writable alias for the
>  	 * read-only text and rodata sections of the kernel image.
> @@ -1342,9 +1367,12 @@ int arch_add_memory(int nid, u64 start, u64 size,
>  
>  	VM_BUG_ON(!mhp_range_allowed(start, size, true));
>  
> -	if (can_set_direct_map())
> +	if (force_pte_mapping())
>  		flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
>  
> +	if (rodata_full)
> +		flags |= NO_CONT_MAPPINGS;
> +
>  	__create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
>  			     size, params->pgprot, __pgd_pgtable_alloc,
>  			     flags);
> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> index 39fd1f7ff02a..ba2360ecb030 100644
> --- a/arch/arm64/mm/pageattr.c
> +++ b/arch/arm64/mm/pageattr.c
> @@ -49,6 +49,145 @@ static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
>  	return 0;
>  }
>  
> +static int __split_linear_mapping_pmd(pud_t *pudp,
> +				      unsigned long vaddr, unsigned long end)

nit: are the double really underscores needed?

> +{
> +	pmd_t *pmdp;
> +	unsigned long next;
> +
> +	pmdp = pmd_offset(pudp, vaddr);
> +
> +	do {
> +		next = pmd_addr_end(vaddr, end);
> +
> +		if (pmd_leaf(pmdp_get(pmdp))) {
> +			struct page *pte_page;
> +			unsigned long pfn = pmd_pfn(pmdp_get(pmdp));
> +			pgprot_t prot = pmd_pgprot(pmdp_get(pmdp));
> +			pte_t *ptep_new;
> +			int i;
> +
> +			pte_page = alloc_page(GFP_KERNEL);
> +			if (!pte_page)
> +				return -ENOMEM;
> +
> +			prot = __pgprot(pgprot_val(prot) | PTE_TYPE_PAGE);
> +			ptep_new = (pte_t *)page_address(pte_page);
> +			for (i = 0; i < PTRS_PER_PTE; ++i, ++ptep_new)
> +				__set_pte_nosync(ptep_new,
> +						 pfn_pte(pfn + i, prot));
> +
> +			dsb(ishst);
> +			isb();
> +
> +			set_pmd(pmdp, pfn_pmd(page_to_pfn(pte_page),
> +				__pgprot(PMD_TYPE_TABLE)));

All the same comments that I made below for __split_linear_mapping_pud() apply here.

> +		}
> +	} while (pmdp++, vaddr = next, vaddr != end);
> +
> +	return 0;
> +}
> +
> +static int __split_linear_mapping_pud(p4d_t *p4dp,
> +				      unsigned long vaddr, unsigned long end)
> +{
> +	pud_t *pudp;
> +	unsigned long next;
> +	int ret;
> +
> +	pudp = pud_offset(p4dp, vaddr);
> +
> +	do {
> +		next = pud_addr_end(vaddr, end);
> +
> +		if (pud_leaf(pudp_get(pudp))) {

Just do the pudp_get() once and cache the result. It's implemented by
READ_ONCE() so your 3 invocations are actually doing 3 separate loads.

> +			struct page *pmd_page;
> +			unsigned long pfn = pud_pfn(pudp_get(pudp));
> +			pgprot_t prot = pud_pgprot(pudp_get(pudp));
> +			pmd_t *pmdp_new;
> +			int i;
> +			unsigned int step;
> +
> +			pmd_page = alloc_page(GFP_KERNEL);
> +			if (!pmd_page)
> +				return -ENOMEM;

Is this really how we allocate pgtable pages for kernel space? For user space we
have quite a lot of machinery for this.

> +
> +			pmdp_new = (pmd_t *)page_address(pmd_page);
> +			for (i = 0; i < PTRS_PER_PMD; ++i, ++pmdp_new) {
> +				step = (i * PMD_SIZE) >> PAGE_SHIFT;

Suggest hoisting this out of the loop, then just increment pfn by step every
iteration.

> +				__set_pmd_nosync(pmdp_new,
> +						 pfn_pmd(pfn + step, prot));
> +			}
> +
> +			dsb(ishst);
> +			isb();

I'm not convinced by the need for this isb(). The dsb ensures that the table is
visible to the table walker; that's needed. The isb() present in set_pmd() and
friends to remove any "cached" speculative invalid translation marker from the
pipeline, if I understand things correctly. I don't think we need to solve that
problem here because you weren't working on a live table. The isb() in set_pud()
is sufficient, I think.

> +
> +			set_pud(pudp, pfn_pud(page_to_pfn(pmd_page),
> +				__pgprot(PUD_TYPE_TABLE)));
> +		}
> +
> +		ret = __split_linear_mapping_pmd(pudp, vaddr, next);
> +		if (ret)
> +			return ret;
> +	} while (pudp++, vaddr = next, vaddr != end);
> +
> +	return 0;
> +}
> +
> +static int __split_linear_mapping_p4d(pgd_t *pgdp,
> +				      unsigned long vaddr, unsigned long end)
> +{
> +	p4d_t *p4dp;
> +	unsigned long next;
> +	int ret;
> +
> +	p4dp = p4d_offset(pgdp, vaddr);
> +
> +	do {
> +		next = p4d_addr_end(vaddr, end);
> +
> +		ret = __split_linear_mapping_pud(p4dp, vaddr, next);
> +		if (ret)
> +			return ret;

nit: personally I would make this break. Then have single "return ret", like you
have for __split_linear_mapping_pgd(). Then all levels follow the same template.

> +	} while (p4dp++, vaddr = next, vaddr != end);
> +
> +	return 0;
> +}
> +
> +static int __split_linear_mapping_pgd(pgd_t *pgdp,
> +				      unsigned long vaddr,
> +				      unsigned long end)
> +{> +	unsigned long next;
> +	int ret = 0;
> +
> +	mmap_write_lock(&init_mm);

nit: I think the locking would be more natural in split_linear_mapping().

> +
> +	do {
> +		next = pgd_addr_end(vaddr, end);
> +		ret = __split_linear_mapping_p4d(pgdp, vaddr, next);
> +		if (ret)
> +			break;
> +	} while (pgdp++, vaddr = next, vaddr != end);
> +
> +	mmap_write_unlock(&init_mm);
> +
> +	return ret;
> +}
> +
> +static int split_linear_mapping(unsigned long start, unsigned long end)

As far as I can tell, you only ever split the linear mapping. You never collapse
it. In that case, won't it just degrade to pte-mappings over time? I suspect you
should aim to detect collapse opportunities.

> +{
> +	int ret;
> +
> +	if (!system_supports_bbmlv2())
> +		return 0;
> +
> +	ret = __split_linear_mapping_pgd(pgd_offset_k(start), start, end);

nit: personally I would put pgd_offset_k() into __split_linear_mapping_pgd() so
that it takes the exact same form as the other levels. Or mege
__split_linear_mapping_pgd() into split_linear_mapping().

> +	flush_tlb_kernel_range(start, end);
> +
> +	return ret;
> +}
> +
>  /*
>   * This function assumes that the range is mapped with PAGE_SIZE pages.
>   */
> @@ -80,8 +219,9 @@ static int change_memory_common(unsigned long addr, int numpages,
>  	unsigned long start = addr;
>  	unsigned long size = PAGE_SIZE * numpages;
>  	unsigned long end = start + size;
> +	unsigned long l_start;
>  	struct vm_struct *area;
> -	int i;
> +	int i, ret;
>  
>  	if (!PAGE_ALIGNED(addr)) {
>  		start &= PAGE_MASK;
> @@ -118,7 +258,12 @@ static int change_memory_common(unsigned long addr, int numpages,
>  	if (rodata_full && (pgprot_val(set_mask) == PTE_RDONLY ||
>  			    pgprot_val(clear_mask) == PTE_RDONLY)) {
>  		for (i = 0; i < area->nr_pages; i++) {
> -			__change_memory_common((u64)page_address(area->pages[i]),
> +			l_start = (u64)page_address(area->pages[i]);
> +			ret = split_linear_mapping(l_start, l_start + PAGE_SIZE);
> +			if (WARN_ON_ONCE(ret))
> +				return ret;
> +
> +			__change_memory_common(l_start,
>  					       PAGE_SIZE, set_mask, clear_mask);
>  		}
>  	}
> @@ -174,6 +319,9 @@ int set_memory_valid(unsigned long addr, int numpages, int enable)

set_memory_valid() actually calls __change_memory_common() directly (so it
bypassed the split logic in change_memory_common()). Is it possible that it gets
called for a non-split region? I guess you'll need to handle that case too?

Thanks,
Ryan

>  
>  int set_direct_map_invalid_noflush(struct page *page)
>  {
> +	unsigned long l_start;
> +	int ret;
> +
>  	struct page_change_data data = {
>  		.set_mask = __pgprot(0),
>  		.clear_mask = __pgprot(PTE_VALID),
> @@ -182,13 +330,21 @@ int set_direct_map_invalid_noflush(struct page *page)
>  	if (!can_set_direct_map())
>  		return 0;
>  
> +	l_start = (unsigned long)page_address(page);
> +	ret = split_linear_mapping(l_start, l_start + PAGE_SIZE);
> +	if (WARN_ON_ONCE(ret))
> +		return ret;
> +
>  	return apply_to_page_range(&init_mm,
> -				   (unsigned long)page_address(page),
> -				   PAGE_SIZE, change_page_range, &data);
> +				   l_start, PAGE_SIZE, change_page_range,
> +				   &data);
>  }
>  
>  int set_direct_map_default_noflush(struct page *page)
>  {
> +	unsigned long l_start;
> +	int ret;
> +
>  	struct page_change_data data = {
>  		.set_mask = __pgprot(PTE_VALID | PTE_WRITE),
>  		.clear_mask = __pgprot(PTE_RDONLY),
> @@ -197,9 +353,14 @@ int set_direct_map_default_noflush(struct page *page)
>  	if (!can_set_direct_map())
>  		return 0;
>  
> +	l_start = (unsigned long)page_address(page);
> +	ret = split_linear_mapping(l_start, l_start + PAGE_SIZE);
> +	if (WARN_ON_ONCE(ret))
> +		return ret;
> +
>  	return apply_to_page_range(&init_mm,
> -				   (unsigned long)page_address(page),
> -				   PAGE_SIZE, change_page_range, &data);
> +				   l_start, PAGE_SIZE, change_page_range,
> +				   &data);
>  }
>  
>  static int __set_memory_enc_dec(unsigned long addr,

Re: [v2 PATCH 1/2] arm64: cpufeature: detect FEAT_BBM level 2

[Yang Shi]

On 2/11/25 3:55 AM, Ryan Roberts wrote:

Hi Ryan,

Thanks for taking time to review the patches.

> On 03/01/2025 01:17, Yang Shi wrote:
>> FEAT_BBM level 2 allow changing block size of a translation with relaxed
>> TLB flushing.  But it may incur TLB conflict abort.  We can handle the
>> abort in kernel, however it is hard to guarantee the recuesive TLB
> nit: recuesive -> recursive ?

Yes, it is a typo. Will fix in the next version.

>> conflct will never happen in the handling itself.
>>
>> Some implementations can handle TLB conflict gracefully without fault
>> handler in kernel so FEAT_BBM level 2 can be enabled on those
>> implementations safely.
>>
>> Look up MIDR to filter out those CPUs.  AmpereOne is one of them.
>>
>> Suggested-by: Will Deacon<will@kernel.org>
>> Signed-off-by: Yang Shi<yang@os.amperecomputing.com>
>> ---
>>   arch/arm64/include/asm/cpufeature.h | 19 +++++++++++++++++++
>>   arch/arm64/kernel/cpufeature.c      | 11 +++++++++++
>>   arch/arm64/tools/cpucaps            |  1 +
>>   3 files changed, 31 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
>> index 8b4e5a3cd24c..33ca9db42741 100644
>> --- a/arch/arm64/include/asm/cpufeature.h
>> +++ b/arch/arm64/include/asm/cpufeature.h
>> @@ -866,6 +866,25 @@ static __always_inline bool system_supports_mpam_hcr(void)
>>   	return alternative_has_cap_unlikely(ARM64_MPAM_HCR);
>>   }
>>   
>> +static inline bool system_supports_bbmlv2(void)
> nit: Arm language internally is starting to refer to FEAT_BBML1 / FEAT_BBML2 and
> I believe this will soon make it's way to the Arm ARM. So probably better to
> refer to bbml2 rather than bbmlv2 throughout.

Sure.

>> +{
>> +	return cpus_have_final_boot_cap(ARM64_HAS_BBMLV2);
>> +}
>> +
>> +static inline bool bbmlv2_available(void)
> This function has no need to be in the header. system_supports_bbmlv2() is what
> users should use. Suggest moving to has_bbmlv2() in cpufeature.c.

bbmlv2_available() will be called by map_mem() in patch 2, but map_mem() 
is called before CPU feature is finalized. I saw you suggest collapse 
the page table in the below comment, if it works we don't need this 
function anymore. But I have more questions about that.

>> +{
>> +	static const struct midr_range support_bbmlv2[] = {
>> +		MIDR_ALL_VERSIONS(MIDR_AMPERE1),
>> +		MIDR_ALL_VERSIONS(MIDR_AMPERE1A),
>> +		{}
>> +	};
>> +
>> +	if (is_midr_in_range_list(read_cpuid_id(), support_bbmlv2))
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>>   int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt);
>>   bool try_emulate_mrs(struct pt_regs *regs, u32 isn);
>>   
>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> index 6ce71f444ed8..a60d5fa04828 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -1889,6 +1889,11 @@ static bool has_lpa2(const struct arm64_cpu_capabilities *entry, int scope)
>>   }
>>   #endif
>>   
>> +static bool has_bbmlv2(const struct arm64_cpu_capabilities *entry, int scope)
>> +{
>> +	return bbmlv2_available();
>> +}
>> +
>>   #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
>>   #define KPTI_NG_TEMP_VA		(-(1UL << PMD_SHIFT))
>>   
>> @@ -2990,6 +2995,12 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>>   		ARM64_CPUID_FIELDS(ID_AA64PFR1_EL1, GCS, IMP)
>>   	},
>>   #endif
>> +	{
>> +		.desc = "BBM Level 2",
>> +		.capability = ARM64_HAS_BBMLV2,
>> +		.type = ARM64_CPUCAP_BOOT_CPU_FEATURE,
> I'm wondering if this will potentially lead to problems for assymetric
> collections of CPUs (e.g. big.LITTLE)? I can imagine that little CPUs might not
> support BBML2. In this case if you boot on a big CPU that does have BBML2, you
> will require the feature and refuse to online the secondary little CPUs.

Yes. This is the behavior of this patch.

> Perhaps this really needs to be a system feature, where it is only enabled if
> all CPUs in the system support it? I'm guessing that will make painting the
> linear map harder; I guess you will need to initially set it up with PTE
> mappings, then repaint as block mappings if BBML2 is determined to be supported
> if that's not already what you are doing.

Actually I thought about this before I posted the RFC patches to 
upstream. There are a couple of options, but I can't tell which one is 
the preferred and whether it is really that important to handle 
asymmetric systems gracefully or not, so I did it in the simplest way: 
just fail online the conflict cores. I also noticed some features behave 
similarly, for example, MPAM. And this RFC patch is mainly aimed to get 
some feedback from the community about whether it is worth it and the 
direction is right or not. So I tried to make it as simple as possible 
(for example, I didn't add CONT_PTE support in patch 2 either).

If I understand correctly, system feature needs to read the "sanitized" 
register value per the comment in cpufeature.c, but we read MIDR here. 
So it actually just uses the current CPU's (likely boot CPU) MIDR if it 
is s system feature, right? If we really want to handle such asymmetric 
systems gracefully, we need:
     - read all cores' MIDR then determine whether BBML2 should be 
advertised or not
     - update a flag or bitmap to tell us whether it is asymmetric or not
     - take actions based on the flag or bitmap (i.e. collapse page 
table or do nothing)

But system feature is not checked on the secondary cores. The 
check_local_cpu_capabilities() called by secondary_start_kernel() just 
checks SCOPE_LOCAL_CPU features if I read the code correctly. So local 
cpu feature may be better? The local cpu feature maintains a cpumask, it 
can tell us whether BBML2 is asymmetric or not.

In addition I'm also thinking about whether collapse is the best way or 
not. We should be able to have large block mapping in the first place if 
the boot CPU has BBML2, then split the page table if it is asymmetric. 
I'm supposed we need to stop machine anyway even though we do collapse. 
The split need to be called on the boot CPU. We already have split 
logic, we can reuse it anyway (maybe need some minor tweak to fit). It 
sounds simpler than collapse. And the asymmetric systems may be not that 
many in real world? I know there are a lot of big.LITTLE SoCs in the 
wild, but those big cores may typically not support BBML2. If so we can 
save boot up time for the most cases.

The other concern is about cpu hotplug. For example, if all the booting 
cores have BBML2, but the hot plugged cores don't, shall we split the 
page table when the cores are hot added, and collapse the page table 
when the cores are hot removed?

I'm not sure whether the extra logic to support asymmetric systems is 
worth it or not. Maybe we should start from the symmetric systems, then 
add more graceful handle to asymmetric systems later if it turns out to 
be a real problem? And unfortunately I don't have the appropriate 
hardware to test the code. Maybe you or someone else from ARM has the 
right hardware?

Thanks,
Yang

> Thanks,
> Ryan
>
>> +		.matches = has_bbmlv2,
>> +	},
>>   	{},
>>   };
>>   
>> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>> index eb17f59e543c..287bdede53f5 100644
>> --- a/arch/arm64/tools/cpucaps
>> +++ b/arch/arm64/tools/cpucaps
>> @@ -14,6 +14,7 @@ HAS_ADDRESS_AUTH_ARCH_QARMA5
>>   HAS_ADDRESS_AUTH_IMP_DEF
>>   HAS_AMU_EXTN
>>   HAS_ARMv8_4_TTL
>> +HAS_BBMLV2
>>   HAS_CACHE_DIC
>>   HAS_CACHE_IDC
>>   HAS_CNP

Re: [v2 PATCH 2/2] arm64: mm: support large block mapping when rodata=full

[Yang Shi]

On 2/11/25 4:58 AM, Ryan Roberts wrote:
> On 03/01/2025 01:17, Yang Shi wrote:
>> When rodata=full is specified, kernel linear mapping has to be mapped at
>> PTE level since large page table can't be split due to break-before-make
>> rule on ARM64.
>>
>> This resulted in a couple of problems:
>>    - performance degradation
>>    - more TLB pressure
>>    - memory waste for kernel page table
>>
>> With FEAT_BBM level 2 support, splitting large block page table to
>> smaller ones doesn't need to make the page table entry invalid anymore.
>> This allows kernel split large block mapping on the fly.
>>
>> Add kernel page table split support and use large block mapping by
>> default when FEAT_BBM level 2 is supported for rodata=full.  When
>> changing permissions for kernel linear mapping, the page table will be
>> split to PTE level.
>>
>> The machine without FEAT_BBM level 2 will fallback to have kernel linear
>> mapping PTE-mapped when rodata=full.
>>
>> With this we saw significant performance boost with some benchmarks and
>> much less memory consumption on my AmpereOne machine (192 cores, 1P) with
>> 256GB memory.
>>
>> * Memory use after boot
>> Before:
>> MemTotal:       258988984 kB
>> MemFree:        254821700 kB
>>
>> After:
>> MemTotal:       259505132 kB
>> MemFree:        255410264 kB
>>
>> Around 500MB more memory are free to use.  The larger the machine, the
>> more memory saved.
>>
>> * Memcached
>> We saw performance degradation when running Memcached benchmark with
>> rodata=full vs rodata=on.  Our profiling pointed to kernel TLB pressure.
>> With this patchset we saw ops/sec is increased by around 3.5%, P99
>> latency is reduced by around 9.6%.
>> The gain mainly came from reduced kernel TLB misses.  The kernel TLB
>> MPKI is reduced by 28.5%.
>>
>> The benchmark data is now on par with rodata=on too.
>>
>> * Disk encryption (dm-crypt) benchmark
>> Ran fio benchmark with the below command on a 128G ramdisk (ext4) with disk
>> encryption (by dm-crypt).
>> fio --directory=/data --random_generator=lfsr --norandommap --randrepeat 1 \
>>      --status-interval=999 --rw=write --bs=4k --loops=1 --ioengine=sync \
>>      --iodepth=1 --numjobs=1 --fsync_on_close=1 --group_reporting --thread \
>>      --name=iops-test-job --eta-newline=1 --size 100G
>>
>> The IOPS is increased by 90% - 150% (the variance is high, but the worst
>> number of good case is around 90% more than the best number of bad case).
>> The bandwidth is increased and the avg clat is reduced proportionally.
>>
>> * Sequential file read
>> Read 100G file sequentially on XFS (xfs_io read with page cache populated).
>> The bandwidth is increased by 150%.
>>
>> Keep using PTE mapping when pagealloc debug is enabled.  It is not worth
>> the complexity.
>>
>> Kfence can be converted to use page block mapping later.
>>
>> Signed-off-by: Yang Shi<yang@os.amperecomputing.com>
>> ---
>>   arch/arm64/include/asm/pgtable.h |   7 +-
>>   arch/arm64/mm/mmu.c              |  32 +++++-
>>   arch/arm64/mm/pageattr.c         | 173 +++++++++++++++++++++++++++++--
>>   3 files changed, 203 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index 6986345b537a..2927f59ae9e2 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -749,7 +749,7 @@ static inline bool in_swapper_pgdir(void *addr)
>>   	        ((unsigned long)swapper_pg_dir & PAGE_MASK);
>>   }
>>   
>> -static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
>> +static inline void __set_pmd_nosync(pmd_t *pmdp, pmd_t pmd)
>>   {
>>   #ifdef __PAGETABLE_PMD_FOLDED
>>   	if (in_swapper_pgdir(pmdp)) {
>> @@ -759,6 +759,11 @@ static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
>>   #endif /* __PAGETABLE_PMD_FOLDED */
>>   
>>   	WRITE_ONCE(*pmdp, pmd);
>> +}
>> +
>> +static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
>> +{
>> +	__set_pmd_nosync(pmdp, pmd);
> I have a series at [1] which enables batching/deferring these barriers. I reckon
> it would be udeful to ultimately apply that same technique for painting the
> linear map if/when that series goes upstream. The benefit is that we wouldn't
> need to expose the _nosync helpers. But that's something to cleanup later. This
> looks fine for now.

Thanks.

>>   
>>   	if (pmd_valid(pmd)) {
>>   		dsb(ishst);
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index e2739b69e11b..65b5b1ba48a7 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -620,6 +620,19 @@ static inline void arm64_kfence_map_pool(phys_addr_t kfence_pool, pgd_t *pgdp) {
>>   
>>   #endif /* CONFIG_KFENCE */
>>   
>> +static inline bool force_pte_mapping(void)
>> +{
>> +	/*
>> +	 * Can't use cpufeature API to determine whether BBM level 2
>> +	 * is supported or not since cpufeature have not been
>> +	 * finalized yet.
>> +	 */
> I think assuming the system has BMML2 just because the boot CPU has it is going
> to be problematic. I think you are going to continue to need to initially paint
> the linear map with ptes then later, once the system cap is finalized, repaint
> with blocks.

Please see the response for patch 1. We may be able to split it instead 
of collapse it.

>> +	return (rodata_full && !bbmlv2_available()) ||
>> +		debug_pagealloc_enabled() ||
>> +		arm64_kfence_can_set_direct_map() ||
>> +		is_realm_world();
>> +}
> Previously the decision was determined by:
>
> bool can_set_direct_map(void)
> {
> 	/*
> 	 * rodata_full, DEBUG_PAGEALLOC and a Realm guest all require linear
> 	 * map to be mapped at page granularity, so that it is possible to
> 	 * protect/unprotect single pages.
> 	 *
> 	 * KFENCE pool requires page-granular mapping if initialized late.
> 	 *
> 	 * Realms need to make pages shared/protected at page granularity.
> 	 */
> 	return rodata_full || debug_pagealloc_enabled() ||
> 		arm64_kfence_can_set_direct_map() || is_realm_world();
> }
>
> But now that you can safely split block mappings to page mappings when needed
> doesn't that cover some of these requirements? So perhaps you can do block
> mappings even when some (all?) of these things are active?

I did look into some of them. I don't know too much about realm, so I 
can't tell.

But pagealloc will change kernel direct map to invalid when freeing page 
so it is called quite often. The pagealloc debug is typically not 
enabled for production.

Kfence will change it to invalid when allocating kfence object 
(typically when allocating slab), but kfence typically allocates and 
maps its pool very early on ARM64 and the pool is separate from direct 
map and mapped at PTE level. So IMHO I don't see to much value support 
large block mapping for them for now.

>> +
>>   static void __init map_mem(pgd_t *pgdp)
>>   {
>>   	static const u64 direct_map_end = _PAGE_END(VA_BITS_MIN);
>> @@ -645,9 +658,21 @@ static void __init map_mem(pgd_t *pgdp)
>>   
>>   	early_kfence_pool = arm64_kfence_alloc_pool();
>>   
>> -	if (can_set_direct_map())
>> +	if (force_pte_mapping())
>>   		flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
>>   
>> +	/*
>> +	 * With FEAT_BBM level 2 we can split large block mapping without
>> +	 * making it invalid.  So kernel linear mapping can be mapped with
>> +	 * large block instead of PTE level.
>> +	 *
>> +	 * Need to break cont for CONT_MAPPINGS when changing permission,
>> +	 * and need to inspect the adjacent page table entries to make
>> +	 * them cont again later.  It sounds not worth the complexity.
> I'm not sure I follow the argument? As far as I can tell, you only ever split
> mappings. You never collapse. So I don't think it's too complex to check for
> CONT bits and if set, clear the CONT bit from the block? That's a safe operation
> with BBML2. It means you need to iterate over the ptes which you don't currently
> do, but you only need to check a 1 PTE in every CONT_PTES.

Sorry for the confusion. Yes, I don't collapse. It seems I overthought 
this case, I thought I need to clear CONT bit for all PTEs in the block 
(for example, 16 PTEs). But it seems I just need to clear one PTE.

>> +	 */
>> +	if (rodata_full)
>> +		flags |= NO_CONT_MAPPINGS;
>> +
>>   	/*
>>   	 * Take care not to create a writable alias for the
>>   	 * read-only text and rodata sections of the kernel image.
>> @@ -1342,9 +1367,12 @@ int arch_add_memory(int nid, u64 start, u64 size,
>>   
>>   	VM_BUG_ON(!mhp_range_allowed(start, size, true));
>>   
>> -	if (can_set_direct_map())
>> +	if (force_pte_mapping())
>>   		flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
>>   
>> +	if (rodata_full)
>> +		flags |= NO_CONT_MAPPINGS;
>> +
>>   	__create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
>>   			     size, params->pgprot, __pgd_pgtable_alloc,
>>   			     flags);
>> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
>> index 39fd1f7ff02a..ba2360ecb030 100644
>> --- a/arch/arm64/mm/pageattr.c
>> +++ b/arch/arm64/mm/pageattr.c
>> @@ -49,6 +49,145 @@ static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
>>   	return 0;
>>   }
>>   
>> +static int __split_linear_mapping_pmd(pud_t *pudp,
>> +				      unsigned long vaddr, unsigned long end)
> nit: are the double really underscores needed?

No, not really.

>> +{
>> +	pmd_t *pmdp;
>> +	unsigned long next;
>> +
>> +	pmdp = pmd_offset(pudp, vaddr);
>> +
>> +	do {
>> +		next = pmd_addr_end(vaddr, end);
>> +
>> +		if (pmd_leaf(pmdp_get(pmdp))) {
>> +			struct page *pte_page;
>> +			unsigned long pfn = pmd_pfn(pmdp_get(pmdp));
>> +			pgprot_t prot = pmd_pgprot(pmdp_get(pmdp));
>> +			pte_t *ptep_new;
>> +			int i;
>> +
>> +			pte_page = alloc_page(GFP_KERNEL);
>> +			if (!pte_page)
>> +				return -ENOMEM;
>> +
>> +			prot = __pgprot(pgprot_val(prot) | PTE_TYPE_PAGE);
>> +			ptep_new = (pte_t *)page_address(pte_page);
>> +			for (i = 0; i < PTRS_PER_PTE; ++i, ++ptep_new)
>> +				__set_pte_nosync(ptep_new,
>> +						 pfn_pte(pfn + i, prot));
>> +
>> +			dsb(ishst);
>> +			isb();
>> +
>> +			set_pmd(pmdp, pfn_pmd(page_to_pfn(pte_page),
>> +				__pgprot(PMD_TYPE_TABLE)));
> All the same comments that I made below for __split_linear_mapping_pud() apply here.

Sure

>> +		}
>> +	} while (pmdp++, vaddr = next, vaddr != end);
>> +
>> +	return 0;
>> +}
>> +
>> +static int __split_linear_mapping_pud(p4d_t *p4dp,
>> +				      unsigned long vaddr, unsigned long end)
>> +{
>> +	pud_t *pudp;
>> +	unsigned long next;
>> +	int ret;
>> +
>> +	pudp = pud_offset(p4dp, vaddr);
>> +
>> +	do {
>> +		next = pud_addr_end(vaddr, end);
>> +
>> +		if (pud_leaf(pudp_get(pudp))) {
> Just do the pudp_get() once and cache the result. It's implemented by
> READ_ONCE() so your 3 invocations are actually doing 3 separate loads.

Sure

>> +			struct page *pmd_page;
>> +			unsigned long pfn = pud_pfn(pudp_get(pudp));
>> +			pgprot_t prot = pud_pgprot(pudp_get(pudp));
>> +			pmd_t *pmdp_new;
>> +			int i;
>> +			unsigned int step;
>> +
>> +			pmd_page = alloc_page(GFP_KERNEL);
>> +			if (!pmd_page)
>> +				return -ENOMEM;
> Is this really how we allocate pgtable pages for kernel space? For user space we
> have quite a lot of machinery for this.

The userspace page table allocation does something extra, for example,
     - initialize ptlock
     - set folio type
     - update NR_PGTABLE counter

We don't need them for kernel direct map page table, so just took the 
most straightforward way. However, __get_free_page(GFP_PGTABLE_KERNEL) 
may be better than alloc_page(GFP_KERNEL). It is used by memory hotplug.

>> +
>> +			pmdp_new = (pmd_t *)page_address(pmd_page);
>> +			for (i = 0; i < PTRS_PER_PMD; ++i, ++pmdp_new) {
>> +				step = (i * PMD_SIZE) >> PAGE_SHIFT;
> Suggest hoisting this out of the loop, then just increment pfn by step every
> iteration.

Yeah, sure

>> +				__set_pmd_nosync(pmdp_new,
>> +						 pfn_pmd(pfn + step, prot));
>> +			}
>> +
>> +			dsb(ishst);
>> +			isb();
> I'm not convinced by the need for this isb(). The dsb ensures that the table is
> visible to the table walker; that's needed. The isb() present in set_pmd() and
> friends to remove any "cached" speculative invalid translation marker from the
> pipeline, if I understand things correctly. I don't think we need to solve that
> problem here because you weren't working on a live table. The isb() in set_pud()
> is sufficient, I think.

I see, we just need one isb() when the table becomes reachable. Will 
remove it and the isb() in __split_linear_mapping_pmd().

>> +
>> +			set_pud(pudp, pfn_pud(page_to_pfn(pmd_page),
>> +				__pgprot(PUD_TYPE_TABLE)));
>> +		}
>> +
>> +		ret = __split_linear_mapping_pmd(pudp, vaddr, next);
>> +		if (ret)
>> +			return ret;
>> +	} while (pudp++, vaddr = next, vaddr != end);
>> +
>> +	return 0;
>> +}
>> +
>> +static int __split_linear_mapping_p4d(pgd_t *pgdp,
>> +				      unsigned long vaddr, unsigned long end)
>> +{
>> +	p4d_t *p4dp;
>> +	unsigned long next;
>> +	int ret;
>> +
>> +	p4dp = p4d_offset(pgdp, vaddr);
>> +
>> +	do {
>> +		next = p4d_addr_end(vaddr, end);
>> +
>> +		ret = __split_linear_mapping_pud(p4dp, vaddr, next);
>> +		if (ret)
>> +			return ret;
> nit: personally I would make this break. Then have single "return ret", like you
> have for __split_linear_mapping_pgd(). Then all levels follow the same template.

Sure

>> +	} while (p4dp++, vaddr = next, vaddr != end);
>> +
>> +	return 0;
>> +}
>> +
>> +static int __split_linear_mapping_pgd(pgd_t *pgdp,
>> +				      unsigned long vaddr,
>> +				      unsigned long end)
>> +{> +	unsigned long next;
>> +	int ret = 0;
>> +
>> +	mmap_write_lock(&init_mm);
> nit: I think the locking would be more natural in split_linear_mapping().

OK

>> +
>> +	do {
>> +		next = pgd_addr_end(vaddr, end);
>> +		ret = __split_linear_mapping_p4d(pgdp, vaddr, next);
>> +		if (ret)
>> +			break;
>> +	} while (pgdp++, vaddr = next, vaddr != end);
>> +
>> +	mmap_write_unlock(&init_mm);
>> +
>> +	return ret;
>> +}
>> +
>> +static int split_linear_mapping(unsigned long start, unsigned long end)
> As far as I can tell, you only ever split the linear mapping. You never collapse
> it. In that case, won't it just degrade to pte-mappings over time? I suspect you
> should aim to detect collapse opportunities.

In theory there may be kernel linear mapping fragmentation problem, for 
example, huge number of modules are loaded, but I've never heard of 
anyone reported this as a real issue. And AFAICT, no architecture 
supports this for now.

>> +{
>> +	int ret;
>> +
>> +	if (!system_supports_bbmlv2())
>> +		return 0;
>> +
>> +	ret = __split_linear_mapping_pgd(pgd_offset_k(start), start, end);
> nit: personally I would put pgd_offset_k() into __split_linear_mapping_pgd() so
> that it takes the exact same form as the other levels. Or mege
> __split_linear_mapping_pgd() into split_linear_mapping().

OK, I will clean it up

>> +	flush_tlb_kernel_range(start, end);
>> +
>> +	return ret;
>> +}
>> +
>>   /*
>>    * This function assumes that the range is mapped with PAGE_SIZE pages.
>>    */
>> @@ -80,8 +219,9 @@ static int change_memory_common(unsigned long addr, int numpages,
>>   	unsigned long start = addr;
>>   	unsigned long size = PAGE_SIZE * numpages;
>>   	unsigned long end = start + size;
>> +	unsigned long l_start;
>>   	struct vm_struct *area;
>> -	int i;
>> +	int i, ret;
>>   
>>   	if (!PAGE_ALIGNED(addr)) {
>>   		start &= PAGE_MASK;
>> @@ -118,7 +258,12 @@ static int change_memory_common(unsigned long addr, int numpages,
>>   	if (rodata_full && (pgprot_val(set_mask) == PTE_RDONLY ||
>>   			    pgprot_val(clear_mask) == PTE_RDONLY)) {
>>   		for (i = 0; i < area->nr_pages; i++) {
>> -			__change_memory_common((u64)page_address(area->pages[i]),
>> +			l_start = (u64)page_address(area->pages[i]);
>> +			ret = split_linear_mapping(l_start, l_start + PAGE_SIZE);
>> +			if (WARN_ON_ONCE(ret))
>> +				return ret;
>> +
>> +			__change_memory_common(l_start,
>>   					       PAGE_SIZE, set_mask, clear_mask);
>>   		}
>>   	}
>> @@ -174,6 +319,9 @@ int set_memory_valid(unsigned long addr, int numpages, int enable)
> set_memory_valid() actually calls __change_memory_common() directly (so it
> bypassed the split logic in change_memory_common()). Is it possible that it gets
> called for a non-split region? I guess you'll need to handle that case too?

The set_memopry_valid() is called in the below places:
     - kfence: it requires map at PTE level regardless of 
kfence_early_init or not
     - pagealloc debug: it requires map at PTE level too
     - execmem ROX cache: it seems like only x86 supports this right now

I don't think we are going to support large block mapping for direct map 
for pagealloc debug and kfence as I said above. We need make 
set_memory_valid() split page table when execmem ROX supported is added 
for arm64. But we don't need do it right now.

Thanks,
Yang

> Thanks,
> Ryan
>
>>   
>>   int set_direct_map_invalid_noflush(struct page *page)
>>   {
>> +	unsigned long l_start;
>> +	int ret;
>> +
>>   	struct page_change_data data = {
>>   		.set_mask = __pgprot(0),
>>   		.clear_mask = __pgprot(PTE_VALID),
>> @@ -182,13 +330,21 @@ int set_direct_map_invalid_noflush(struct page *page)
>>   	if (!can_set_direct_map())
>>   		return 0;
>>   
>> +	l_start = (unsigned long)page_address(page);
>> +	ret = split_linear_mapping(l_start, l_start + PAGE_SIZE);
>> +	if (WARN_ON_ONCE(ret))
>> +		return ret;
>> +
>>   	return apply_to_page_range(&init_mm,
>> -				   (unsigned long)page_address(page),
>> -				   PAGE_SIZE, change_page_range, &data);
>> +				   l_start, PAGE_SIZE, change_page_range,
>> +				   &data);
>>   }
>>   
>>   int set_direct_map_default_noflush(struct page *page)
>>   {
>> +	unsigned long l_start;
>> +	int ret;
>> +
>>   	struct page_change_data data = {
>>   		.set_mask = __pgprot(PTE_VALID | PTE_WRITE),
>>   		.clear_mask = __pgprot(PTE_RDONLY),
>> @@ -197,9 +353,14 @@ int set_direct_map_default_noflush(struct page *page)
>>   	if (!can_set_direct_map())
>>   		return 0;
>>   
>> +	l_start = (unsigned long)page_address(page);
>> +	ret = split_linear_mapping(l_start, l_start + PAGE_SIZE);
>> +	if (WARN_ON_ONCE(ret))
>> +		return ret;
>> +
>>   	return apply_to_page_range(&init_mm,
>> -				   (unsigned long)page_address(page),
>> -				   PAGE_SIZE, change_page_range, &data);
>> +				   l_start, PAGE_SIZE, change_page_range,
>> +				   &data);
>>   }
>>   
>>   static int __set_memory_enc_dec(unsigned long addr,

Re: [RFC v2 PATCH 0/2] arm64: support FEAT_BBM level 2 and large block mapping when rodata=full

[Yang Shi]

On 2/11/25 3:36 AM, Ryan Roberts wrote:
> Sorry managed to send this to the list only. Resending with original recipients
> added back in...
>
>
> On 11/02/2025 11:34, Ryan Roberts wrote:
>> Hi Yang,
>>
>> Thanks for putting this together; I'm hoping to piggyback on this and use BBML2
>> to reduce the cost of contpte_convert().

Thanks for sharing another usecase.

>>
>> review incoming...
>>
>>
>> On 03/01/2025 01:17, Yang Shi wrote:
>>> When rodata=full kernel linear mapping is mapped by PTE due to arm's
>>> break-before-make rule.
>>>
>>> This resulted in a couple of problems:
>>>    - performance degradation
>>>    - more TLB pressure
>>>    - memory waste for kernel page table
>>>
>>> There are some workarounds to mitigate the problems, for example, using
>>> rodata=on, but this compromises the security measurement.
>>>
>>> With FEAT_BBM level 2 support, splitting large block page table to
>>> smaller ones doesn't need to make the page table entry invalid anymore.
>>> This allows kernel split large block mapping on the fly.
>>>
>>> Add kernel page table split support and use large block mapping by
>>> default when FEAT_BBM level 2 is supported for rodata=full.  When
>>> changing permissions for kernel linear mapping, the page table will be
>>> split to PTE level.
>>>
>>> The machine without FEAT_BBM level 2 will fallback to have kernel linear
>>> mapping PTE-mapped when rodata=full.
>>>
>>> With this we saw significant performance boost with some benchmarks with
>>> keeping rodata=full security protection in the mean time.
>>>
>>> The test was done on AmpereOne machine (192 cores, 1P) with 256GB memory and
>>> 4K page size + 48 bit VA.
>>>
>>> Function test (4K/16K/64K page size)
>>>    - Kernel boot.  Kernel needs change kernel linear mapping permission at
>>>      boot stage, if the patch didn't work, kernel typically didn't boot.
>>>    - Module stress from stress-ng. Kernel module load change permission for
>>>      module sections.
>>>    - A test kernel module which allocates 80% of total memory via vmalloc(),
>>>      then change the vmalloc area permission to RO, then change it back
>>>      before vfree(). Then launch a VM which consumes almost all physical
>>>      memory.
>> I don't really understand how vmalloc is relevant here? vmalloc can already map
>> huge pages you use vmalloc_huge() and changing the permissions of vmalloc
>> mapping will only affect the ptes pertaining to that mapping; I don't see why
>> that would cause permissions to be changed on the linear map or for huge pages
>> in the linear map to be split?

I just uses vmalloc() API to emulate what modules loading does. Allocate 
memory via vmalloc() then change permission to, for example, read-only, 
by calling set_memory_ro(). So I can stress the page split by doing it 
on the most of memory, for example, 80% of memory. It is more efficient 
than loading real modules.

It is implemented by a patch against test_vmalloc. I don't include the 
patch in this series, if you think it is useful, I can include it in v3 
anyway.

>>
>>>    - VM with the patchset applied in guest kernel too.
>>>    - Kernel build in VM with patched guest kernel.
>>>
>>> Memory consumption
>>> Before:
>>> MemTotal:       258988984 kB
>>> MemFree:        254821700 kB
>>>
>>> After:
>>> MemTotal:       259505132 kB
>>> MemFree:        255410264 kB
>>>
>>> Around 500MB more memory are free to use.  The larger the machine, the
>>> more memory saved.
>>>
>>> Performance benchmarking
>>> * Memcached
>>> We saw performance degradation when running Memcached benchmark with
>>> rodata=full vs rodata=on.  Our profiling pointed to kernel TLB pressure.
>>> With this patchset we saw ops/sec is increased by around 3.5%, P99
>>> latency is reduced by around 9.6%.
>>> The gain mainly came from reduced kernel TLB misses.  The kernel TLB
>>> MPKI is reduced by 28.5%.
>>>
>>> The benchmark data is now on par with rodata=on too.
>>>
>>> * Disk encryption (dm-crypt) benchmark
>>> Ran fio benchmark with the below command on a 128G ramdisk (ext4) with disk
>>> encryption (by dm-crypt).
>>> fio --directory=/data --random_generator=lfsr --norandommap --randrepeat 1 \
>>>      --status-interval=999 --rw=write --bs=4k --loops=1 --ioengine=sync \
>>>      --iodepth=1 --numjobs=1 --fsync_on_close=1 --group_reporting --thread \
>>>      --name=iops-test-job --eta-newline=1 --size 100G
>>>
>>> The IOPS is increased by 90% - 150% (the variance is high, but the worst
>>> number of good case is around 90% more than the best number of bad case).
>>> The bandwidth is increased and the avg clat is reduced proportionally.
>>>
>>> * Sequential file read
>>> Read 100G file sequentially on XFS (xfs_io read with page cache populated).
>>> The bandwidth is increased by 150%.
>> The performance gains definitely look worthwhile!

Yeah, thanks for taking your time review the patches. I think the 
feedback is positive enough so far to get rid off the "RFC" tag.

Yang


>>
>> Thanks,
>> Ryan
>>
>>> RFC v2:
>>>    * Used allowlist to advertise BBM lv2 on the CPUs which can handle TLB
>>>      conflict gracefully per Will Deacon
>>>    * Rebased onto v6.13-rc5
>>>
>>> RFC v1: https://lore.kernel.org/lkml/20241118181711.962576-1-yang@os.amperecomputing.com/
>>>
>>> Yang Shi (2):
>>>        arm64: cpufeature: detect FEAT_BBM level 2
>>>        arm64: mm: support large block mapping when rodata=full
>>>
>>>   arch/arm64/include/asm/cpufeature.h |  19 ++++++++++++
>>>   arch/arm64/include/asm/pgtable.h    |   7 ++++-
>>>   arch/arm64/kernel/cpufeature.c      |  11 +++++++
>>>   arch/arm64/mm/mmu.c                 |  32 ++++++++++++++++++--
>>>   arch/arm64/mm/pageattr.c            | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>>>   arch/arm64/tools/cpucaps            |   1 +
>>>   6 files changed, 234 insertions(+), 9 deletions(-)
>>>
>>>

Re: [v2 PATCH 1/2] arm64: cpufeature: detect FEAT_BBM level 2

[Ryan Roberts]

Hi Yang,

On 13/02/2025 21:14, Yang Shi wrote:
> 
> 
> 
> On 2/11/25 3:55 AM, Ryan Roberts wrote:
> 
> Hi Ryan,
> 
> Thanks for taking time to review the patches.
> 
>> On 03/01/2025 01:17, Yang Shi wrote:
>>> FEAT_BBM level 2 allow changing block size of a translation with relaxed
>>> TLB flushing.  But it may incur TLB conflict abort.  We can handle the
>>> abort in kernel, however it is hard to guarantee the recuesive TLB
>> nit: recuesive -> recursive ?
> 
> Yes, it is a typo. Will fix in the next version.
> 
>>> conflct will never happen in the handling itself.
>>>
>>> Some implementations can handle TLB conflict gracefully without fault
>>> handler in kernel so FEAT_BBM level 2 can be enabled on those
>>> implementations safely.
>>>
>>> Look up MIDR to filter out those CPUs.  AmpereOne is one of them.
>>>
>>> Suggested-by: Will Deacon<will@kernel.org>
>>> Signed-off-by: Yang Shi<yang@os.amperecomputing.com>
>>> ---
>>>   arch/arm64/include/asm/cpufeature.h | 19 +++++++++++++++++++
>>>   arch/arm64/kernel/cpufeature.c      | 11 +++++++++++
>>>   arch/arm64/tools/cpucaps            |  1 +
>>>   3 files changed, 31 insertions(+)
>>>
>>> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/
>>> cpufeature.h
>>> index 8b4e5a3cd24c..33ca9db42741 100644
>>> --- a/arch/arm64/include/asm/cpufeature.h
>>> +++ b/arch/arm64/include/asm/cpufeature.h
>>> @@ -866,6 +866,25 @@ static __always_inline bool system_supports_mpam_hcr(void)
>>>       return alternative_has_cap_unlikely(ARM64_MPAM_HCR);
>>>   }
>>>   +static inline bool system_supports_bbmlv2(void)
>> nit: Arm language internally is starting to refer to FEAT_BBML1 / FEAT_BBML2 and
>> I believe this will soon make it's way to the Arm ARM. So probably better to
>> refer to bbml2 rather than bbmlv2 throughout.
> 
> Sure.
> 
>>> +{
>>> +    return cpus_have_final_boot_cap(ARM64_HAS_BBMLV2);
>>> +}
>>> +
>>> +static inline bool bbmlv2_available(void)
>> This function has no need to be in the header. system_supports_bbmlv2() is what
>> users should use. Suggest moving to has_bbmlv2() in cpufeature.c.
> 
> bbmlv2_available() will be called by map_mem() in patch 2, but map_mem() is
> called before CPU feature is finalized. I saw you suggest collapse the page
> table in the below comment, if it works we don't need this function anymore. But
> I have more questions about that.
> 
>>> +{
>>> +    static const struct midr_range support_bbmlv2[] = {
>>> +        MIDR_ALL_VERSIONS(MIDR_AMPERE1),
>>> +        MIDR_ALL_VERSIONS(MIDR_AMPERE1A),
>>> +        {}
>>> +    };
>>> +
>>> +    if (is_midr_in_range_list(read_cpuid_id(), support_bbmlv2))
>>> +        return true;
>>> +
>>> +    return false;
>>> +}
>>> +
>>>   int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt);
>>>   bool try_emulate_mrs(struct pt_regs *regs, u32 isn);
>>>   diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>>> index 6ce71f444ed8..a60d5fa04828 100644
>>> --- a/arch/arm64/kernel/cpufeature.c
>>> +++ b/arch/arm64/kernel/cpufeature.c
>>> @@ -1889,6 +1889,11 @@ static bool has_lpa2(const struct
>>> arm64_cpu_capabilities *entry, int scope)
>>>   }
>>>   #endif
>>>   +static bool has_bbmlv2(const struct arm64_cpu_capabilities *entry, int scope)
>>> +{
>>> +    return bbmlv2_available();
>>> +}
>>> +
>>>   #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
>>>   #define KPTI_NG_TEMP_VA        (-(1UL << PMD_SHIFT))
>>>   @@ -2990,6 +2995,12 @@ static const struct arm64_cpu_capabilities
>>> arm64_features[] = {
>>>           ARM64_CPUID_FIELDS(ID_AA64PFR1_EL1, GCS, IMP)
>>>       },
>>>   #endif
>>> +    {
>>> +        .desc = "BBM Level 2",
>>> +        .capability = ARM64_HAS_BBMLV2,
>>> +        .type = ARM64_CPUCAP_BOOT_CPU_FEATURE,
>> I'm wondering if this will potentially lead to problems for assymetric
>> collections of CPUs (e.g. big.LITTLE)? I can imagine that little CPUs might not
>> support BBML2. In this case if you boot on a big CPU that does have BBML2, you
>> will require the feature and refuse to online the secondary little CPUs.
> 
> Yes. This is the behavior of this patch.
> >> Perhaps this really needs to be a system feature, where it is only enabled if
>> all CPUs in the system support it? I'm guessing that will make painting the
>> linear map harder; I guess you will need to initially set it up with PTE
>> mappings, then repaint as block mappings if BBML2 is determined to be supported
>> if that's not already what you are doing.
> 
> Actually I thought about this before I posted the RFC patches to upstream. There
> are a couple of options, but I can't tell which one is the preferred and whether
> it is really that important to handle asymmetric systems gracefully or not, so I

It is certainly important to handle asymmetric systems gracefully in general;
Almost all mobile handsets use big.LITTLE. And I think it's entirely possible
(likely even) that we will see systems where the big cores have BBML2 and the
little ones don't. There are cases where we don't currently handle asymetric
systems gracefully (e.g. LPA2), but we try to make those the exception rather
than the rule. In LPA2's case, the VA size is a compile-time option and I
suspect it's very unlikely that a 52-bit VA capable kernel would ever be
deployed on an asymmetric system (i.e. mobile handset). Given BBML2 is runtime
controlled only, I think we need to make it play nice with asymmetric systems.

> did it in the simplest way: just fail online the conflict cores. I also noticed
> some features behave similarly, for example, MPAM. And this RFC patch is mainly
> aimed to get some feedback from the community about whether it is worth it and
> the direction is right or not. So I tried to make it as simple as possible (for
> example, I didn't add CONT_PTE support in patch 2 either).

Based on your perf numbers, I defintely think this is something we should aim to
get upstream.

I think there are 3 options for the approach:

 - refuse to online secondary CPUs that don't support BBML2 if the boot CPU does
   support BBML2.
    - I don't personally think this is viable for the reasons above.

 - initially paint the linear map with ptes, then update it to block mappings if
   BBML2 is supported system-wide.
    - I'm guessing the main cost is the pte-mapping, and we already have that
      today, so it's not going to slow the boot down vs today. Re-painting with
      (mostly) PUD-sized block mappings will be significantly faster.
    - I anticipate we can just call the same function to do the mapping over the
      top of what's already there but with the BLOCK flag set.
    - It's easy to test on all systems

 - Guess at the system supporting BBML2 based only on the boot CPU, then if any
   secondaries don't support it, stop the machine and repaint linear map as
   PTEs (we do something similar today for nG)
    - It means that symmetric systems will boot marginally faster because they
      never map the linear map with ptes
    - Asymmetric systems will boot slower because they have to stop the machine
      to do the pte re-mapping
    - It's a code path that won't get tested very often

My view is that option 1 is a non-starter. Personally I lean towards option 2,
but could be persuaded that option 3 is better.

> 
> If I understand correctly, system feature needs to read the "sanitized" register
> value per the comment in cpufeature.c, but we read MIDR here. So it actually
> just uses the current CPU's (likely boot CPU) MIDR if it is s system feature,
> right? If we really want to handle such asymmetric systems gracefully, we need:
>     - read all cores' MIDR then determine whether BBML2 should be advertised or not
>     - update a flag or bitmap to tell us whether it is asymmetric or not
>     - take actions based on the flag or bitmap (i.e. collapse page table or do
> nothing)

It's my understanding that we want is ARM64_CPUCAP_SYSTEM_FEATURE. See it's
comment in cpufeature.h:

/*
 * CPU feature detected at boot time based on system-wide value of a
 * feature. It is safe for a late CPU to have this feature even though
 * the system hasn't enabled it, although the feature will not be used
 * by Linux in this case. If the system has enabled this feature already,
 * then every late CPU must have it.
 */

> 
> But system feature is not checked on the secondary cores. The
> check_local_cpu_capabilities() called by secondary_start_kernel() just checks
> SCOPE_LOCAL_CPU features if I read the code correctly. So local cpu feature may
> be better? The local cpu feature maintains a cpumask, it can tell us whether
> BBML2 is asymmetric or not.

I've not read through the details of the implementation but the docs point to
ARM64_CPUCAP_SYSTEM_FEATURE and there are many other cpu features defined as
ARM64_CPUCAP_SYSTEM_FEATURE which need these semantics. e.g. ARM64_HAS_TLB_RANGE.

> 
> In addition I'm also thinking about whether collapse is the best way or not. We
> should be able to have large block mapping in the first place if the boot CPU
> has BBML2, then split the page table if it is asymmetric. I'm supposed we need
> to stop machine anyway even though we do collapse. 

Yes I think you could do it this way as long as you stop the machine first.
There is already a pattern to follow for this with nG; see
kpti_install_ng_mappings().

I don't think you need to stop the machine if collapsing, as long as BBML2 is
supported.

> The split need to be called
> on the boot CPU. We already have split logic, we can reuse it anyway (maybe need
> some minor tweak to fit). It sounds simpler than collapse. 

I'm wondering if we can enhance the existing table walker in mmu.c
(__create_pgd_mapping()) to handle collapse and split gracefully. Then we could
reuse that logic for all collapse, split and permission change operations so
that we always have the largest possible blocks mapped.

It looks like it used to be able to do (some of) this prior to commit Commit
e98216b52176 ("arm64: mm: BUG on unsupported manipulations of live kernel
mappings"). That approach may turn out to be cleaner and more general than the
current logic you have to split a range to ptes?

> And the asymmetric
> systems may be not that many in real world? I know there are a lot of big.LITTLE
> SoCs in the wild, but those big cores may typically not support BBML2. If so we
> can save boot up time for the most cases.

Yes, you are beginning to persuade me that stop_machine() for asymmetric systems
may be the better way to go.

If the boot CPU does not support BBML2, paint with page mappings as before; no
change.

If the boot CPU does support BBML2, speculatively paint with block mappings. If
all CPUs later turn out to support BBML2 then no further action is needed, and
this gives us a reduction in boot time because we never had to paint per-page
PTEs. It's also a runtime advantage due to the TLB pressure reduciton.

If any secondary CPU later turns out not to support BBML2, stop the machine and
repaint with page mappings. Assuming the major cost is writing all the PTEs,
then this is lightly about the same speed as it is today. And as you say, this
path is lightly to be rare-ish.

OK I'm convinced.

> 
> The other concern is about cpu hotplug. For example, if all the booting cores
> have BBML2, but the hot plugged cores don't, shall we split the page table when
> the cores are hot added, and collapse the page table when the cores are hot
> removed?

I don't think we need to worry about this case too much. There are lots of
things today that require that the kernel can see at least 1 of each possible
CPU during boot (applying errata is a good example). If a new CPU model gets
hotplugged that it hasn't seen during boot then I think it's completely
reasonable to refuse to online it if it doesn't meet the configured feature
requirements. If you define BBML2 as a system feature, you'll get this behaviour
for free. In reality, hotplugged CPUs are going to be symmetric so I don't think
we will fall down this path in the real world.

> 
> I'm not sure whether the extra logic to support asymmetric systems is worth it
> or not. Maybe we should start from the symmetric systems, then add more graceful
> handle to asymmetric systems later if it turns out to be a real problem? And
> unfortunately I don't have the appropriate hardware to test the code. Maybe you
> or someone else from ARM has the right hardware?

My view is that we need to make BMML2 a system feature. Speculatively paint the
linear map with block mappings if the primary CPU supports BBML2. Stop machine
and repaint as page mappings if the system as a whole doesn't support BBML2 at
system feature finalization. In the unlikely event that the system does support
BBML2 and a CPU is hotplugged that doesn't support it, refuse to online it (that
bit is for free as a result of defining it as a system feature).

You can hack the code to always stop machine and remap as page mappings for
testing purposes.

I'm happy to collaborate on development of these patches and/or do testing as
required; I think the performance numbers you posted look compelling.

> 
> Thanks,
> Yang
> 
>> Thanks,
>> Ryan
>>
>>> +        .matches = has_bbmlv2,
>>> +    },
>>>       {},
>>>   };
>>>   diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>>> index eb17f59e543c..287bdede53f5 100644
>>> --- a/arch/arm64/tools/cpucaps
>>> +++ b/arch/arm64/tools/cpucaps
>>> @@ -14,6 +14,7 @@ HAS_ADDRESS_AUTH_ARCH_QARMA5
>>>   HAS_ADDRESS_AUTH_IMP_DEF
>>>   HAS_AMU_EXTN
>>>   HAS_ARMv8_4_TTL
>>> +HAS_BBMLV2
>>>   HAS_CACHE_DIC
>>>   HAS_CACHE_IDC
>>>   HAS_CNP
> 

Re: [RFC v2 PATCH 0/2] arm64: support FEAT_BBM level 2 and large block mapping when rodata=full

[Ryan Roberts]

On 13/02/2025 21:27, Yang Shi wrote:

>>> I don't really understand how vmalloc is relevant here? vmalloc can already map
>>> huge pages you use vmalloc_huge() and changing the permissions of vmalloc
>>> mapping will only affect the ptes pertaining to that mapping; I don't see why
>>> that would cause permissions to be changed on the linear map or for huge pages
>>> in the linear map to be split?
> 
> I just uses vmalloc() API to emulate what modules loading does. Allocate memory
> via vmalloc() then change permission to, for example, read-only, by calling
> set_memory_ro(). So I can stress the page split by doing it on the most of
> memory, for example, 80% of memory. It is more efficient than loading real modules.

Ahh I see. I hadn't previously appreciated that we change the linear map
permissions at the same time as changing the vmap permissions. Makes sense.

> 
> It is implemented by a patch against test_vmalloc. I don't include the patch in
> this series, if you think it is useful, I can include it in v3 anyway.

I don't think it's necessary to include in this series. Although perhaps useful
as follow up patch to beef up the vmalloc perf tests.

Thanks,
Ryan

Re: [v2 PATCH 2/2] arm64: mm: support large block mapping when rodata=full

[Ryan Roberts]

On 13/02/2025 21:18, Yang Shi wrote:
> 
> 
> 
> On 2/11/25 4:58 AM, Ryan Roberts wrote:
>> On 03/01/2025 01:17, Yang Shi wrote:
>>> When rodata=full is specified, kernel linear mapping has to be mapped at
>>> PTE level since large page table can't be split due to break-before-make
>>> rule on ARM64.
>>>
>>> This resulted in a couple of problems:
>>>    - performance degradation
>>>    - more TLB pressure
>>>    - memory waste for kernel page table
>>>
>>> With FEAT_BBM level 2 support, splitting large block page table to
>>> smaller ones doesn't need to make the page table entry invalid anymore.
>>> This allows kernel split large block mapping on the fly.
>>>
>>> Add kernel page table split support and use large block mapping by
>>> default when FEAT_BBM level 2 is supported for rodata=full.  When
>>> changing permissions for kernel linear mapping, the page table will be
>>> split to PTE level.
>>>
>>> The machine without FEAT_BBM level 2 will fallback to have kernel linear
>>> mapping PTE-mapped when rodata=full.
>>>
>>> With this we saw significant performance boost with some benchmarks and
>>> much less memory consumption on my AmpereOne machine (192 cores, 1P) with
>>> 256GB memory.
>>>
>>> * Memory use after boot
>>> Before:
>>> MemTotal:       258988984 kB
>>> MemFree:        254821700 kB
>>>
>>> After:
>>> MemTotal:       259505132 kB
>>> MemFree:        255410264 kB
>>>
>>> Around 500MB more memory are free to use.  The larger the machine, the
>>> more memory saved.
>>>
>>> * Memcached
>>> We saw performance degradation when running Memcached benchmark with
>>> rodata=full vs rodata=on.  Our profiling pointed to kernel TLB pressure.
>>> With this patchset we saw ops/sec is increased by around 3.5%, P99
>>> latency is reduced by around 9.6%.
>>> The gain mainly came from reduced kernel TLB misses.  The kernel TLB
>>> MPKI is reduced by 28.5%.
>>>
>>> The benchmark data is now on par with rodata=on too.
>>>
>>> * Disk encryption (dm-crypt) benchmark
>>> Ran fio benchmark with the below command on a 128G ramdisk (ext4) with disk
>>> encryption (by dm-crypt).
>>> fio --directory=/data --random_generator=lfsr --norandommap --randrepeat 1 \
>>>      --status-interval=999 --rw=write --bs=4k --loops=1 --ioengine=sync \
>>>      --iodepth=1 --numjobs=1 --fsync_on_close=1 --group_reporting --thread \
>>>      --name=iops-test-job --eta-newline=1 --size 100G
>>>
>>> The IOPS is increased by 90% - 150% (the variance is high, but the worst
>>> number of good case is around 90% more than the best number of bad case).
>>> The bandwidth is increased and the avg clat is reduced proportionally.
>>>
>>> * Sequential file read
>>> Read 100G file sequentially on XFS (xfs_io read with page cache populated).
>>> The bandwidth is increased by 150%.
>>>
>>> Keep using PTE mapping when pagealloc debug is enabled.  It is not worth
>>> the complexity.
>>>
>>> Kfence can be converted to use page block mapping later.
>>>
>>> Signed-off-by: Yang Shi<yang@os.amperecomputing.com>
>>> ---
>>>   arch/arm64/include/asm/pgtable.h |   7 +-
>>>   arch/arm64/mm/mmu.c              |  32 +++++-
>>>   arch/arm64/mm/pageattr.c         | 173 +++++++++++++++++++++++++++++--
>>>   3 files changed, 203 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>>> index 6986345b537a..2927f59ae9e2 100644
>>> --- a/arch/arm64/include/asm/pgtable.h
>>> +++ b/arch/arm64/include/asm/pgtable.h
>>> @@ -749,7 +749,7 @@ static inline bool in_swapper_pgdir(void *addr)
>>>               ((unsigned long)swapper_pg_dir & PAGE_MASK);
>>>   }
>>>   -static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
>>> +static inline void __set_pmd_nosync(pmd_t *pmdp, pmd_t pmd)
>>>   {
>>>   #ifdef __PAGETABLE_PMD_FOLDED
>>>       if (in_swapper_pgdir(pmdp)) {
>>> @@ -759,6 +759,11 @@ static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
>>>   #endif /* __PAGETABLE_PMD_FOLDED */
>>>         WRITE_ONCE(*pmdp, pmd);
>>> +}
>>> +
>>> +static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
>>> +{
>>> +    __set_pmd_nosync(pmdp, pmd);
>> I have a series at [1] which enables batching/deferring these barriers. I reckon
>> it would be udeful to ultimately apply that same technique for painting the
>> linear map if/when that series goes upstream. The benefit is that we wouldn't
>> need to expose the _nosync helpers. But that's something to cleanup later. This
>> looks fine for now.
> 
> Thanks.
> 
>>>         if (pmd_valid(pmd)) {
>>>           dsb(ishst);
>>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>>> index e2739b69e11b..65b5b1ba48a7 100644
>>> --- a/arch/arm64/mm/mmu.c
>>> +++ b/arch/arm64/mm/mmu.c
>>> @@ -620,6 +620,19 @@ static inline void arm64_kfence_map_pool(phys_addr_t
>>> kfence_pool, pgd_t *pgdp) {
>>>     #endif /* CONFIG_KFENCE */
>>>   +static inline bool force_pte_mapping(void)
>>> +{
>>> +    /*
>>> +     * Can't use cpufeature API to determine whether BBM level 2
>>> +     * is supported or not since cpufeature have not been
>>> +     * finalized yet.
>>> +     */
>> I think assuming the system has BMML2 just because the boot CPU has it is going
>> to be problematic. I think you are going to continue to need to initially paint
>> the linear map with ptes then later, once the system cap is finalized, repaint
>> with blocks.
> 
> Please see the response for patch 1. We may be able to split it instead of
> collapse it.
> 
>>> +    return (rodata_full && !bbmlv2_available()) ||
>>> +        debug_pagealloc_enabled() ||
>>> +        arm64_kfence_can_set_direct_map() ||
>>> +        is_realm_world();
>>> +}
>> Previously the decision was determined by:
>>
>> bool can_set_direct_map(void)
>> {
>>     /*
>>      * rodata_full, DEBUG_PAGEALLOC and a Realm guest all require linear
>>      * map to be mapped at page granularity, so that it is possible to
>>      * protect/unprotect single pages.
>>      *
>>      * KFENCE pool requires page-granular mapping if initialized late.
>>      *
>>      * Realms need to make pages shared/protected at page granularity.
>>      */
>>     return rodata_full || debug_pagealloc_enabled() ||
>>         arm64_kfence_can_set_direct_map() || is_realm_world();
>> }
>>
>> But now that you can safely split block mappings to page mappings when needed
>> doesn't that cover some of these requirements? So perhaps you can do block
>> mappings even when some (all?) of these things are active?
> 
> I did look into some of them. I don't know too much about realm, so I can't tell.
> 
> But pagealloc will change kernel direct map to invalid when freeing page so it
> is called quite often. The pagealloc debug is typically not enabled for production.
> 
> Kfence will change it to invalid when allocating kfence object (typically when
> allocating slab), but kfence typically allocates and maps its pool very early on
> ARM64 and the pool is separate from direct map and mapped at PTE level. So IMHO
> I don't see to much value support large block mapping for them for now.

OK perhaps leave as is for now, and we can take a view on whether we want to
extend later as incremental patches.

> 
>>> +
>>>   static void __init map_mem(pgd_t *pgdp)
>>>   {
>>>       static const u64 direct_map_end = _PAGE_END(VA_BITS_MIN);
>>> @@ -645,9 +658,21 @@ static void __init map_mem(pgd_t *pgdp)
>>>         early_kfence_pool = arm64_kfence_alloc_pool();
>>>   -    if (can_set_direct_map())
>>> +    if (force_pte_mapping())
>>>           flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
>>>   +    /*
>>> +     * With FEAT_BBM level 2 we can split large block mapping without
>>> +     * making it invalid.  So kernel linear mapping can be mapped with
>>> +     * large block instead of PTE level.
>>> +     *
>>> +     * Need to break cont for CONT_MAPPINGS when changing permission,
>>> +     * and need to inspect the adjacent page table entries to make
>>> +     * them cont again later.  It sounds not worth the complexity.
>> I'm not sure I follow the argument? As far as I can tell, you only ever split
>> mappings. You never collapse. So I don't think it's too complex to check for
>> CONT bits and if set, clear the CONT bit from the block? That's a safe operation
>> with BBML2. It means you need to iterate over the ptes which you don't currently
>> do, but you only need to check a 1 PTE in every CONT_PTES.
> 
> Sorry for the confusion. Yes, I don't collapse. It seems I overthought this
> case, I thought I need to clear CONT bit for all PTEs in the block (for example,
> 16 PTEs). But it seems I just need to clear one PTE.

No you were right; you need to clear the CONT bit from all 16 PTEs in the block.
But since all PTEs in the block will either have CONT all set or all clear, you
only need to check 1 in every 16 PTEs. If that check shows that CONT is set, you
can clear the bit from the block that contains the PTE you checked. It will slow
down the split process though, because now you have to descend to the PTE table,
check 32 of the PTEs (512 / 16) and for any CONT blocks, clear the CONT bit.

Personally I think it's worth checking how much slower that is though, given you
already have a test case. In general, I've found that there is value in working
hard to keep the TLB usage as low as possible.

> 
>>> +     */
>>> +    if (rodata_full)
>>> +        flags |= NO_CONT_MAPPINGS;
>>> +
>>>       /*
>>>        * Take care not to create a writable alias for the
>>>        * read-only text and rodata sections of the kernel image.
>>> @@ -1342,9 +1367,12 @@ int arch_add_memory(int nid, u64 start, u64 size,
>>>         VM_BUG_ON(!mhp_range_allowed(start, size, true));
>>>   -    if (can_set_direct_map())
>>> +    if (force_pte_mapping())
>>>           flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
>>>   +    if (rodata_full)
>>> +        flags |= NO_CONT_MAPPINGS;
>>> +
>>>       __create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
>>>                    size, params->pgprot, __pgd_pgtable_alloc,
>>>                    flags);
>>> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
>>> index 39fd1f7ff02a..ba2360ecb030 100644
>>> --- a/arch/arm64/mm/pageattr.c
>>> +++ b/arch/arm64/mm/pageattr.c
>>> @@ -49,6 +49,145 @@ static int change_page_range(pte_t *ptep, unsigned long
>>> addr, void *data)
>>>       return 0;
>>>   }
>>>   +static int __split_linear_mapping_pmd(pud_t *pudp,
>>> +                      unsigned long vaddr, unsigned long end)
>> nit: are the double really underscores needed?
> 
> No, not really.
> 
>>> +{
>>> +    pmd_t *pmdp;
>>> +    unsigned long next;
>>> +
>>> +    pmdp = pmd_offset(pudp, vaddr);
>>> +
>>> +    do {
>>> +        next = pmd_addr_end(vaddr, end);
>>> +
>>> +        if (pmd_leaf(pmdp_get(pmdp))) {
>>> +            struct page *pte_page;
>>> +            unsigned long pfn = pmd_pfn(pmdp_get(pmdp));
>>> +            pgprot_t prot = pmd_pgprot(pmdp_get(pmdp));
>>> +            pte_t *ptep_new;
>>> +            int i;
>>> +
>>> +            pte_page = alloc_page(GFP_KERNEL);
>>> +            if (!pte_page)
>>> +                return -ENOMEM;
>>> +
>>> +            prot = __pgprot(pgprot_val(prot) | PTE_TYPE_PAGE);
>>> +            ptep_new = (pte_t *)page_address(pte_page);
>>> +            for (i = 0; i < PTRS_PER_PTE; ++i, ++ptep_new)
>>> +                __set_pte_nosync(ptep_new,
>>> +                         pfn_pte(pfn + i, prot));
>>> +
>>> +            dsb(ishst);
>>> +            isb();
>>> +
>>> +            set_pmd(pmdp, pfn_pmd(page_to_pfn(pte_page),
>>> +                __pgprot(PMD_TYPE_TABLE)));
>> All the same comments that I made below for __split_linear_mapping_pud() apply
>> here.
> 
> Sure
> 
>>> +        }
>>> +    } while (pmdp++, vaddr = next, vaddr != end);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int __split_linear_mapping_pud(p4d_t *p4dp,
>>> +                      unsigned long vaddr, unsigned long end)
>>> +{
>>> +    pud_t *pudp;
>>> +    unsigned long next;
>>> +    int ret;
>>> +
>>> +    pudp = pud_offset(p4dp, vaddr);
>>> +
>>> +    do {
>>> +        next = pud_addr_end(vaddr, end);
>>> +
>>> +        if (pud_leaf(pudp_get(pudp))) {
>> Just do the pudp_get() once and cache the result. It's implemented by
>> READ_ONCE() so your 3 invocations are actually doing 3 separate loads.
> 
> Sure
> 
>>> +            struct page *pmd_page;
>>> +            unsigned long pfn = pud_pfn(pudp_get(pudp));
>>> +            pgprot_t prot = pud_pgprot(pudp_get(pudp));
>>> +            pmd_t *pmdp_new;
>>> +            int i;
>>> +            unsigned int step;
>>> +
>>> +            pmd_page = alloc_page(GFP_KERNEL);
>>> +            if (!pmd_page)
>>> +                return -ENOMEM;
>> Is this really how we allocate pgtable pages for kernel space? For user space we
>> have quite a lot of machinery for this.
> 
> The userspace page table allocation does something extra, for example,
>     - initialize ptlock
>     - set folio type
>     - update NR_PGTABLE counter
> 
> We don't need them for kernel direct map page table, so just took the most
> straightforward way. However, __get_free_page(GFP_PGTABLE_KERNEL) may be better
> than alloc_page(GFP_KERNEL). It is used by memory hotplug.

__create_pgd_mapping() takes a callback that it uses to allocate pgtable pages.
The initial linear map creation passes in early_pgtable_alloc(), which allocates
from memblock. I'm not sure if it's possible that we would ever need to split so
early that we need to use memblock too?

For late allocations, we have __pgd_pgtable_alloc() and pgd_pgtable_alloc(). The
latter constructs the ptdesc, although it's not completely clear to me when each
should be used. Both of those functions BUG_ON() memory allocation failure which
seems bad, especially given __pgd_pgtable_alloc() is used for mem hotplug, which
really should be returning an error. Ouch.

I think ideally we would rework __create_pgd_mapping() to be able to return an
error (so we can fix the mem hotplug issue), handle collapse, split and
permissions changes then use it universally. Or failing that,
__get_free_page(GFP_PGTABLE_KERNEL) is the second best option.

> 
>>> +
>>> +            pmdp_new = (pmd_t *)page_address(pmd_page);
>>> +            for (i = 0; i < PTRS_PER_PMD; ++i, ++pmdp_new) {
>>> +                step = (i * PMD_SIZE) >> PAGE_SHIFT;
>> Suggest hoisting this out of the loop, then just increment pfn by step every
>> iteration.
> 
> Yeah, sure
> 
>>> +                __set_pmd_nosync(pmdp_new,
>>> +                         pfn_pmd(pfn + step, prot));
>>> +            }
>>> +
>>> +            dsb(ishst);
>>> +            isb();
>> I'm not convinced by the need for this isb(). The dsb ensures that the table is
>> visible to the table walker; that's needed. The isb() present in set_pmd() and
>> friends to remove any "cached" speculative invalid translation marker from the
>> pipeline, if I understand things correctly. I don't think we need to solve that
>> problem here because you weren't working on a live table. The isb() in set_pud()
>> is sufficient, I think.
> 
> I see, we just need one isb() when the table becomes reachable. Will remove it
> and the isb() in __split_linear_mapping_pmd().
> >>> +
>>> +            set_pud(pudp, pfn_pud(page_to_pfn(pmd_page),
>>> +                __pgprot(PUD_TYPE_TABLE)));
>>> +        }
>>> +
>>> +        ret = __split_linear_mapping_pmd(pudp, vaddr, next);
>>> +        if (ret)
>>> +            return ret;
>>> +    } while (pudp++, vaddr = next, vaddr != end);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int __split_linear_mapping_p4d(pgd_t *pgdp,
>>> +                      unsigned long vaddr, unsigned long end)
>>> +{
>>> +    p4d_t *p4dp;
>>> +    unsigned long next;
>>> +    int ret;
>>> +
>>> +    p4dp = p4d_offset(pgdp, vaddr);
>>> +
>>> +    do {
>>> +        next = p4d_addr_end(vaddr, end);
>>> +
>>> +        ret = __split_linear_mapping_pud(p4dp, vaddr, next);
>>> +        if (ret)
>>> +            return ret;
>> nit: personally I would make this break. Then have single "return ret", like you
>> have for __split_linear_mapping_pgd(). Then all levels follow the same template.
> 
> Sure
> 
>>> +    } while (p4dp++, vaddr = next, vaddr != end);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int __split_linear_mapping_pgd(pgd_t *pgdp,
>>> +                      unsigned long vaddr,
>>> +                      unsigned long end)
>>> +{> +    unsigned long next;
>>> +    int ret = 0;
>>> +
>>> +    mmap_write_lock(&init_mm);
>> nit: I think the locking would be more natural in split_linear_mapping().
> 
> OK
> 
>>> +
>>> +    do {
>>> +        next = pgd_addr_end(vaddr, end);
>>> +        ret = __split_linear_mapping_p4d(pgdp, vaddr, next);
>>> +        if (ret)
>>> +            break;
>>> +    } while (pgdp++, vaddr = next, vaddr != end);
>>> +
>>> +    mmap_write_unlock(&init_mm);
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static int split_linear_mapping(unsigned long start, unsigned long end)
>> As far as I can tell, you only ever split the linear mapping. You never collapse
>> it. In that case, won't it just degrade to pte-mappings over time? I suspect you
>> should aim to detect collapse opportunities.
> 
> In theory there may be kernel linear mapping fragmentation problem, for example,
> huge number of modules are loaded, but I've never heard of anyone reported this
> as a real issue. And AFAICT, no architecture supports this for now.

I'm not really talking about fragmentation. I'm talking about allocating
different portions of the linear map over time and having to split the mapping
down to ptes due to changing permissions. Over time this will tend towards the
entire linear map being pte mapped if you don't proactively collapse too?

I accept this is unlikely to happen due to module loading since you tend to load
the modules at boot and they stay loaded. But what about other features that
cause the linear map permissions to be changed? e.g. secret mem, bpf programs
being loaded, guest_memfd (although I don't think that's upstream yet).

> 
>>> +{
>>> +    int ret;
>>> +
>>> +    if (!system_supports_bbmlv2())
>>> +        return 0;
>>> +
>>> +    ret = __split_linear_mapping_pgd(pgd_offset_k(start), start, end);
>> nit: personally I would put pgd_offset_k() into __split_linear_mapping_pgd() so
>> that it takes the exact same form as the other levels. Or mege
>> __split_linear_mapping_pgd() into split_linear_mapping().
> 
> OK, I will clean it up
> 
>>> +    flush_tlb_kernel_range(start, end);
>>> +
>>> +    return ret;
>>> +}
>>> +
>>>   /*
>>>    * This function assumes that the range is mapped with PAGE_SIZE pages.
>>>    */
>>> @@ -80,8 +219,9 @@ static int change_memory_common(unsigned long addr, int
>>> numpages,
>>>       unsigned long start = addr;
>>>       unsigned long size = PAGE_SIZE * numpages;
>>>       unsigned long end = start + size;
>>> +    unsigned long l_start;
>>>       struct vm_struct *area;
>>> -    int i;
>>> +    int i, ret;
>>>         if (!PAGE_ALIGNED(addr)) {
>>>           start &= PAGE_MASK;
>>> @@ -118,7 +258,12 @@ static int change_memory_common(unsigned long addr, int
>>> numpages,
>>>       if (rodata_full && (pgprot_val(set_mask) == PTE_RDONLY ||
>>>                   pgprot_val(clear_mask) == PTE_RDONLY)) {
>>>           for (i = 0; i < area->nr_pages; i++) {
>>> -            __change_memory_common((u64)page_address(area->pages[i]),
>>> +            l_start = (u64)page_address(area->pages[i]);
>>> +            ret = split_linear_mapping(l_start, l_start + PAGE_SIZE);
>>> +            if (WARN_ON_ONCE(ret))
>>> +                return ret;
>>> +
>>> +            __change_memory_common(l_start,
>>>                              PAGE_SIZE, set_mask, clear_mask);
>>>           }
>>>       }
>>> @@ -174,6 +319,9 @@ int set_memory_valid(unsigned long addr, int numpages,
>>> int enable)
>> set_memory_valid() actually calls __change_memory_common() directly (so it
>> bypassed the split logic in change_memory_common()). Is it possible that it gets
>> called for a non-split region? I guess you'll need to handle that case too?
> 
> The set_memopry_valid() is called in the below places:
>     - kfence: it requires map at PTE level regardless of kfence_early_init or not
>     - pagealloc debug: it requires map at PTE level too
>     - execmem ROX cache: it seems like only x86 supports this right now
> 
> I don't think we are going to support large block mapping for direct map for
> pagealloc debug and kfence as I said above. We need make set_memory_valid()
> split page table when execmem ROX supported is added for arm64. But we don't
> need do it right now.

Fair enough.

Thanks,
Ryan

> 
> Thanks,
> Yang
> 
>> Thanks,
>> Ryan
>>
>>>     int set_direct_map_invalid_noflush(struct page *page)
>>>   {
>>> +    unsigned long l_start;
>>> +    int ret;
>>> +
>>>       struct page_change_data data = {
>>>           .set_mask = __pgprot(0),
>>>           .clear_mask = __pgprot(PTE_VALID),
>>> @@ -182,13 +330,21 @@ int set_direct_map_invalid_noflush(struct page *page)
>>>       if (!can_set_direct_map())
>>>           return 0;
>>>   +    l_start = (unsigned long)page_address(page);
>>> +    ret = split_linear_mapping(l_start, l_start + PAGE_SIZE);
>>> +    if (WARN_ON_ONCE(ret))
>>> +        return ret;
>>> +
>>>       return apply_to_page_range(&init_mm,
>>> -                   (unsigned long)page_address(page),
>>> -                   PAGE_SIZE, change_page_range, &data);
>>> +                   l_start, PAGE_SIZE, change_page_range,
>>> +                   &data);
>>>   }
>>>     int set_direct_map_default_noflush(struct page *page)
>>>   {
>>> +    unsigned long l_start;
>>> +    int ret;
>>> +
>>>       struct page_change_data data = {
>>>           .set_mask = __pgprot(PTE_VALID | PTE_WRITE),
>>>           .clear_mask = __pgprot(PTE_RDONLY),
>>> @@ -197,9 +353,14 @@ int set_direct_map_default_noflush(struct page *page)
>>>       if (!can_set_direct_map())
>>>           return 0;
>>>   +    l_start = (unsigned long)page_address(page);
>>> +    ret = split_linear_mapping(l_start, l_start + PAGE_SIZE);
>>> +    if (WARN_ON_ONCE(ret))
>>> +        return ret;
>>> +
>>>       return apply_to_page_range(&init_mm,
>>> -                   (unsigned long)page_address(page),
>>> -                   PAGE_SIZE, change_page_range, &data);
>>> +                   l_start, PAGE_SIZE, change_page_range,
>>> +                   &data);
>>>   }
>>>     static int __set_memory_enc_dec(unsigned long addr,
> 

Re: [v2 PATCH 1/2] arm64: cpufeature: detect FEAT_BBM level 2

[Yang Shi]

On 2/18/25 3:17 AM, Ryan Roberts wrote:
> Hi Yang,
>
> On 13/02/2025 21:14, Yang Shi wrote:
>> On 2/11/25 3:55 AM, Ryan Roberts wrote:
>>
>> Hi Ryan,
>>
>> Thanks for taking time to review the patches.
>>
>>> On 03/01/2025 01:17, Yang Shi wrote:
>>>> FEAT_BBM level 2 allow changing block size of a translation with relaxed
>>>> TLB flushing.  But it may incur TLB conflict abort.  We can handle the
>>>> abort in kernel, however it is hard to guarantee the recuesive TLB
>>> nit: recuesive -> recursive ?
>> Yes, it is a typo. Will fix in the next version.
>>
>>>> conflct will never happen in the handling itself.
>>>>
>>>> Some implementations can handle TLB conflict gracefully without fault
>>>> handler in kernel so FEAT_BBM level 2 can be enabled on those
>>>> implementations safely.
>>>>
>>>> Look up MIDR to filter out those CPUs.  AmpereOne is one of them.
>>>>
>>>> Suggested-by: Will Deacon<will@kernel.org>
>>>> Signed-off-by: Yang Shi<yang@os.amperecomputing.com>
>>>> ---
>>>>    arch/arm64/include/asm/cpufeature.h | 19 +++++++++++++++++++
>>>>    arch/arm64/kernel/cpufeature.c      | 11 +++++++++++
>>>>    arch/arm64/tools/cpucaps            |  1 +
>>>>    3 files changed, 31 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/
>>>> cpufeature.h
>>>> index 8b4e5a3cd24c..33ca9db42741 100644
>>>> --- a/arch/arm64/include/asm/cpufeature.h
>>>> +++ b/arch/arm64/include/asm/cpufeature.h
>>>> @@ -866,6 +866,25 @@ static __always_inline bool system_supports_mpam_hcr(void)
>>>>        return alternative_has_cap_unlikely(ARM64_MPAM_HCR);
>>>>    }
>>>>    +static inline bool system_supports_bbmlv2(void)
>>> nit: Arm language internally is starting to refer to FEAT_BBML1 / FEAT_BBML2 and
>>> I believe this will soon make it's way to the Arm ARM. So probably better to
>>> refer to bbml2 rather than bbmlv2 throughout.
>> Sure.
>>
>>>> +{
>>>> +    return cpus_have_final_boot_cap(ARM64_HAS_BBMLV2);
>>>> +}
>>>> +
>>>> +static inline bool bbmlv2_available(void)
>>> This function has no need to be in the header. system_supports_bbmlv2() is what
>>> users should use. Suggest moving to has_bbmlv2() in cpufeature.c.
>> bbmlv2_available() will be called by map_mem() in patch 2, but map_mem() is
>> called before CPU feature is finalized. I saw you suggest collapse the page
>> table in the below comment, if it works we don't need this function anymore. But
>> I have more questions about that.
>>
>>>> +{
>>>> +    static const struct midr_range support_bbmlv2[] = {
>>>> +        MIDR_ALL_VERSIONS(MIDR_AMPERE1),
>>>> +        MIDR_ALL_VERSIONS(MIDR_AMPERE1A),
>>>> +        {}
>>>> +    };
>>>> +
>>>> +    if (is_midr_in_range_list(read_cpuid_id(), support_bbmlv2))
>>>> +        return true;
>>>> +
>>>> +    return false;
>>>> +}
>>>> +
>>>>    int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt);
>>>>    bool try_emulate_mrs(struct pt_regs *regs, u32 isn);
>>>>    diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>>>> index 6ce71f444ed8..a60d5fa04828 100644
>>>> --- a/arch/arm64/kernel/cpufeature.c
>>>> +++ b/arch/arm64/kernel/cpufeature.c
>>>> @@ -1889,6 +1889,11 @@ static bool has_lpa2(const struct
>>>> arm64_cpu_capabilities *entry, int scope)
>>>>    }
>>>>    #endif
>>>>    +static bool has_bbmlv2(const struct arm64_cpu_capabilities *entry, int scope)
>>>> +{
>>>> +    return bbmlv2_available();
>>>> +}
>>>> +
>>>>    #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
>>>>    #define KPTI_NG_TEMP_VA        (-(1UL << PMD_SHIFT))
>>>>    @@ -2990,6 +2995,12 @@ static const struct arm64_cpu_capabilities
>>>> arm64_features[] = {
>>>>            ARM64_CPUID_FIELDS(ID_AA64PFR1_EL1, GCS, IMP)
>>>>        },
>>>>    #endif
>>>> +    {
>>>> +        .desc = "BBM Level 2",
>>>> +        .capability = ARM64_HAS_BBMLV2,
>>>> +        .type = ARM64_CPUCAP_BOOT_CPU_FEATURE,
>>> I'm wondering if this will potentially lead to problems for assymetric
>>> collections of CPUs (e.g. big.LITTLE)? I can imagine that little CPUs might not
>>> support BBML2. In this case if you boot on a big CPU that does have BBML2, you
>>> will require the feature and refuse to online the secondary little CPUs.
>> Yes. This is the behavior of this patch.
>>>> Perhaps this really needs to be a system feature, where it is only enabled if
>>> all CPUs in the system support it? I'm guessing that will make painting the
>>> linear map harder; I guess you will need to initially set it up with PTE
>>> mappings, then repaint as block mappings if BBML2 is determined to be supported
>>> if that's not already what you are doing.
>> Actually I thought about this before I posted the RFC patches to upstream. There
>> are a couple of options, but I can't tell which one is the preferred and whether
>> it is really that important to handle asymmetric systems gracefully or not, so I
> It is certainly important to handle asymmetric systems gracefully in general;
> Almost all mobile handsets use big.LITTLE. And I think it's entirely possible
> (likely even) that we will see systems where the big cores have BBML2 and the
> little ones don't. There are cases where we don't currently handle asymetric
> systems gracefully (e.g. LPA2), but we try to make those the exception rather
> than the rule. In LPA2's case, the VA size is a compile-time option and I
> suspect it's very unlikely that a 52-bit VA capable kernel would ever be
> deployed on an asymmetric system (i.e. mobile handset). Given BBML2 is runtime
> controlled only, I think we need to make it play nice with asymmetric systems.

OK, makes sense to me.

>> did it in the simplest way: just fail online the conflict cores. I also noticed
>> some features behave similarly, for example, MPAM. And this RFC patch is mainly
>> aimed to get some feedback from the community about whether it is worth it and
>> the direction is right or not. So I tried to make it as simple as possible (for
>> example, I didn't add CONT_PTE support in patch 2 either).
> Based on your perf numbers, I defintely think this is something we should aim to
> get upstream.
>
> I think there are 3 options for the approach:
>
>   - refuse to online secondary CPUs that don't support BBML2 if the boot CPU does
>     support BBML2.
>      - I don't personally think this is viable for the reasons above.
>
>   - initially paint the linear map with ptes, then update it to block mappings if
>     BBML2 is supported system-wide.
>      - I'm guessing the main cost is the pte-mapping, and we already have that
>        today, so it's not going to slow the boot down vs today. Re-painting with
>        (mostly) PUD-sized block mappings will be significantly faster.
>      - I anticipate we can just call the same function to do the mapping over the
>        top of what's already there but with the BLOCK flag set.
>      - It's easy to test on all systems
>
>   - Guess at the system supporting BBML2 based only on the boot CPU, then if any
>     secondaries don't support it, stop the machine and repaint linear map as
>     PTEs (we do something similar today for nG)
>      - It means that symmetric systems will boot marginally faster because they
>        never map the linear map with ptes
>      - Asymmetric systems will boot slower because they have to stop the machine
>        to do the pte re-mapping
>      - It's a code path that won't get tested very often
>
> My view is that option 1 is a non-starter. Personally I lean towards option 2,
> but could be persuaded that option 3 is better.
>
>> If I understand correctly, system feature needs to read the "sanitized" register
>> value per the comment in cpufeature.c, but we read MIDR here. So it actually
>> just uses the current CPU's (likely boot CPU) MIDR if it is s system feature,
>> right? If we really want to handle such asymmetric systems gracefully, we need:
>>      - read all cores' MIDR then determine whether BBML2 should be advertised or not
>>      - update a flag or bitmap to tell us whether it is asymmetric or not
>>      - take actions based on the flag or bitmap (i.e. collapse page table or do
>> nothing)
> It's my understanding that we want is ARM64_CPUCAP_SYSTEM_FEATURE. See it's
> comment in cpufeature.h:
>
> /*
>   * CPU feature detected at boot time based on system-wide value of a
>   * feature. It is safe for a late CPU to have this feature even though
>   * the system hasn't enabled it, although the feature will not be used
>   * by Linux in this case. If the system has enabled this feature already,
>   * then every late CPU must have it.
>   */
>
>> But system feature is not checked on the secondary cores. The
>> check_local_cpu_capabilities() called by secondary_start_kernel() just checks
>> SCOPE_LOCAL_CPU features if I read the code correctly. So local cpu feature may
>> be better? The local cpu feature maintains a cpumask, it can tell us whether
>> BBML2 is asymmetric or not.
> I've not read through the details of the implementation but the docs point to
> ARM64_CPUCAP_SYSTEM_FEATURE and there are many other cpu features defined as
> ARM64_CPUCAP_SYSTEM_FEATURE which need these semantics. e.g. ARM64_HAS_TLB_RANGE.

I basically figured out how it works. The boot cpu initializes the 
boot_cpu_data, then the secondary cpus need to update it, the 
"sanitized" register value will be generated. For example, TLB range 
capability is determined by ISAR0_EL1. If all the cpus have this 
feature, the "sanitized" register value will show true otherwise it will 
show false.

BBML2 can be determined by MMFR2_EL1. If we can rely on it then system 
feature does work. But the problem is some implementations may have 
MMFR2_EL1 set, but they may not be able to handle TLB conflict. We can't 
rely on it solely so we check MIDR in .matches callback instead of 
MMFR2_EL1. But system feature .matches callback is just called once on 
boot CPU because it is supposed to read the sanitized register value if 
I read the code correctly.

So this may not work for BBML2. It seems like the cpufeature 
infrastructure needs to be enhanced to support this case? For example, 
off the top of my head, we can define a global midr list and a bool flag 
in struct cpuinfo_arm64. The boot cpu initializes it to true or false. 
Then the secondary cpu update the flag according to their MIDR. Then we 
just need to check this flag in .matches callback. You should have 
better idea.

>> In addition I'm also thinking about whether collapse is the best way or not. We
>> should be able to have large block mapping in the first place if the boot CPU
>> has BBML2, then split the page table if it is asymmetric. I'm supposed we need
>> to stop machine anyway even though we do collapse.
> Yes I think you could do it this way as long as you stop the machine first.
> There is already a pattern to follow for this with nG; see
> kpti_install_ng_mappings().

Thank you.

>
> I don't think you need to stop the machine if collapsing, as long as BBML2 is
> supported.

Yes, I agree. But when collapsing, are the secondary CPUs actually doing 
anything? Should be just in idle loop? If the secondary cpus are in idle 
loop, then stop_machine won't make too much difference.

>
>> The split need to be called
>> on the boot CPU. We already have split logic, we can reuse it anyway (maybe need
>> some minor tweak to fit). It sounds simpler than collapse.
> I'm wondering if we can enhance the existing table walker in mmu.c
> (__create_pgd_mapping()) to handle collapse and split gracefully. Then we could
> reuse that logic for all collapse, split and permission change operations so
> that we always have the largest possible blocks mapped.
>
> It looks like it used to be able to do (some of) this prior to commit Commit
> e98216b52176 ("arm64: mm: BUG on unsupported manipulations of live kernel
> mappings"). That approach may turn out to be cleaner and more general than the
> current logic you have to split a range to ptes?

We should be able to handle splitting in __create_pgd_mapping_locked(). 
__create_pgd_mapping() takes fixmap_lock, it sounds unnecessary for 
splitting. If we want it handle split as well, we should need pass in a 
flag to tell __create_pgd_mapping_locked() in split path. But I can't 
tell how much code actually can be reused right now.

Both creating and splitting walk the page table from root to leaf, but 
collapse is different, it goes from leaf to root. I don't see it can be 
easily handled by __create_pmd_mapping_locked() without introducing too 
much churn at the first glance.

>
>> And the asymmetric
>> systems may be not that many in real world? I know there are a lot of big.LITTLE
>> SoCs in the wild, but those big cores may typically not support BBML2. If so we
>> can save boot up time for the most cases.
> Yes, you are beginning to persuade me that stop_machine() for asymmetric systems
> may be the better way to go.
>
> If the boot CPU does not support BBML2, paint with page mappings as before; no
> change.
>
> If the boot CPU does support BBML2, speculatively paint with block mappings. If
> all CPUs later turn out to support BBML2 then no further action is needed, and
> this gives us a reduction in boot time because we never had to paint per-page
> PTEs. It's also a runtime advantage due to the TLB pressure reduciton.
>
> If any secondary CPU later turns out not to support BBML2, stop the machine and
> repaint with page mappings. Assuming the major cost is writing all the PTEs,
> then this is lightly about the same speed as it is today. And as you say, this
> path is lightly to be rare-ish.
>
> OK I'm convinced.

Thank you.

>
>> The other concern is about cpu hotplug. For example, if all the booting cores
>> have BBML2, but the hot plugged cores don't, shall we split the page table when
>> the cores are hot added, and collapse the page table when the cores are hot
>> removed?
> I don't think we need to worry about this case too much. There are lots of
> things today that require that the kernel can see at least 1 of each possible
> CPU during boot (applying errata is a good example). If a new CPU model gets
> hotplugged that it hasn't seen during boot then I think it's completely
> reasonable to refuse to online it if it doesn't meet the configured feature
> requirements. If you define BBML2 as a system feature, you'll get this behaviour
> for free. In reality, hotplugged CPUs are going to be symmetric so I don't think
> we will fall down this path in the real world.

OK

>
>> I'm not sure whether the extra logic to support asymmetric systems is worth it
>> or not. Maybe we should start from the symmetric systems, then add more graceful
>> handle to asymmetric systems later if it turns out to be a real problem? And
>> unfortunately I don't have the appropriate hardware to test the code. Maybe you
>> or someone else from ARM has the right hardware?
> My view is that we need to make BMML2 a system feature. Speculatively paint the
> linear map with block mappings if the primary CPU supports BBML2. Stop machine
> and repaint as page mappings if the system as a whole doesn't support BBML2 at
> system feature finalization. In the unlikely event that the system does support
> BBML2 and a CPU is hotplugged that doesn't support it, refuse to online it (that
> bit is for free as a result of defining it as a system feature).
>
> You can hack the code to always stop machine and remap as page mappings for
> testing purposes.

Yeah, sure.

>
> I'm happy to collaborate on development of these patches and/or do testing as
> required; I think the performance numbers you posted look compelling.

Thank you.

Yang

>
>> Thanks,
>> Yang
>>
>>> Thanks,
>>> Ryan
>>>
>>>> +        .matches = has_bbmlv2,
>>>> +    },
>>>>        {},
>>>>    };
>>>>    diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
>>>> index eb17f59e543c..287bdede53f5 100644
>>>> --- a/arch/arm64/tools/cpucaps
>>>> +++ b/arch/arm64/tools/cpucaps
>>>> @@ -14,6 +14,7 @@ HAS_ADDRESS_AUTH_ARCH_QARMA5
>>>>    HAS_ADDRESS_AUTH_IMP_DEF
>>>>    HAS_AMU_EXTN
>>>>    HAS_ARMv8_4_TTL
>>>> +HAS_BBMLV2
>>>>    HAS_CACHE_DIC
>>>>    HAS_CACHE_IDC
>>>>    HAS_CNP

Re: [v2 PATCH 2/2] arm64: mm: support large block mapping when rodata=full

[Yang Shi]

On 2/18/25 5:28 AM, Ryan Roberts wrote:
> On 13/02/2025 21:18, Yang Shi wrote:
>>
>>
>> On 2/11/25 4:58 AM, Ryan Roberts wrote:
>>> On 03/01/2025 01:17, Yang Shi wrote:
>>>> When rodata=full is specified, kernel linear mapping has to be mapped at
>>>> PTE level since large page table can't be split due to break-before-make
>>>> rule on ARM64.
>>>>
>>>> This resulted in a couple of problems:
>>>>     - performance degradation
>>>>     - more TLB pressure
>>>>     - memory waste for kernel page table
>>>>
>>>> With FEAT_BBM level 2 support, splitting large block page table to
>>>> smaller ones doesn't need to make the page table entry invalid anymore.
>>>> This allows kernel split large block mapping on the fly.
>>>>
>>>> Add kernel page table split support and use large block mapping by
>>>> default when FEAT_BBM level 2 is supported for rodata=full.  When
>>>> changing permissions for kernel linear mapping, the page table will be
>>>> split to PTE level.
>>>>
>>>> The machine without FEAT_BBM level 2 will fallback to have kernel linear
>>>> mapping PTE-mapped when rodata=full.
>>>>
>>>> With this we saw significant performance boost with some benchmarks and
>>>> much less memory consumption on my AmpereOne machine (192 cores, 1P) with
>>>> 256GB memory.
>>>>
>>>> * Memory use after boot
>>>> Before:
>>>> MemTotal:       258988984 kB
>>>> MemFree:        254821700 kB
>>>>
>>>> After:
>>>> MemTotal:       259505132 kB
>>>> MemFree:        255410264 kB
>>>>
>>>> Around 500MB more memory are free to use.  The larger the machine, the
>>>> more memory saved.
>>>>
>>>> * Memcached
>>>> We saw performance degradation when running Memcached benchmark with
>>>> rodata=full vs rodata=on.  Our profiling pointed to kernel TLB pressure.
>>>> With this patchset we saw ops/sec is increased by around 3.5%, P99
>>>> latency is reduced by around 9.6%.
>>>> The gain mainly came from reduced kernel TLB misses.  The kernel TLB
>>>> MPKI is reduced by 28.5%.
>>>>
>>>> The benchmark data is now on par with rodata=on too.
>>>>
>>>> * Disk encryption (dm-crypt) benchmark
>>>> Ran fio benchmark with the below command on a 128G ramdisk (ext4) with disk
>>>> encryption (by dm-crypt).
>>>> fio --directory=/data --random_generator=lfsr --norandommap --randrepeat 1 \
>>>>       --status-interval=999 --rw=write --bs=4k --loops=1 --ioengine=sync \
>>>>       --iodepth=1 --numjobs=1 --fsync_on_close=1 --group_reporting --thread \
>>>>       --name=iops-test-job --eta-newline=1 --size 100G
>>>>
>>>> The IOPS is increased by 90% - 150% (the variance is high, but the worst
>>>> number of good case is around 90% more than the best number of bad case).
>>>> The bandwidth is increased and the avg clat is reduced proportionally.
>>>>
>>>> * Sequential file read
>>>> Read 100G file sequentially on XFS (xfs_io read with page cache populated).
>>>> The bandwidth is increased by 150%.
>>>>
>>>> Keep using PTE mapping when pagealloc debug is enabled.  It is not worth
>>>> the complexity.
>>>>
>>>> Kfence can be converted to use page block mapping later.
>>>>
>>>> Signed-off-by: Yang Shi<yang@os.amperecomputing.com>
>>>> ---
>>>>    arch/arm64/include/asm/pgtable.h |   7 +-
>>>>    arch/arm64/mm/mmu.c              |  32 +++++-
>>>>    arch/arm64/mm/pageattr.c         | 173 +++++++++++++++++++++++++++++--
>>>>    3 files changed, 203 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>>>> index 6986345b537a..2927f59ae9e2 100644
>>>> --- a/arch/arm64/include/asm/pgtable.h
>>>> +++ b/arch/arm64/include/asm/pgtable.h
>>>> @@ -749,7 +749,7 @@ static inline bool in_swapper_pgdir(void *addr)
>>>>                ((unsigned long)swapper_pg_dir & PAGE_MASK);
>>>>    }
>>>>    -static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
>>>> +static inline void __set_pmd_nosync(pmd_t *pmdp, pmd_t pmd)
>>>>    {
>>>>    #ifdef __PAGETABLE_PMD_FOLDED
>>>>        if (in_swapper_pgdir(pmdp)) {
>>>> @@ -759,6 +759,11 @@ static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
>>>>    #endif /* __PAGETABLE_PMD_FOLDED */
>>>>          WRITE_ONCE(*pmdp, pmd);
>>>> +}
>>>> +
>>>> +static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
>>>> +{
>>>> +    __set_pmd_nosync(pmdp, pmd);
>>> I have a series at [1] which enables batching/deferring these barriers. I reckon
>>> it would be udeful to ultimately apply that same technique for painting the
>>> linear map if/when that series goes upstream. The benefit is that we wouldn't
>>> need to expose the _nosync helpers. But that's something to cleanup later. This
>>> looks fine for now.
>> Thanks.
>>
>>>>          if (pmd_valid(pmd)) {
>>>>            dsb(ishst);
>>>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>>>> index e2739b69e11b..65b5b1ba48a7 100644
>>>> --- a/arch/arm64/mm/mmu.c
>>>> +++ b/arch/arm64/mm/mmu.c
>>>> @@ -620,6 +620,19 @@ static inline void arm64_kfence_map_pool(phys_addr_t
>>>> kfence_pool, pgd_t *pgdp) {
>>>>      #endif /* CONFIG_KFENCE */
>>>>    +static inline bool force_pte_mapping(void)
>>>> +{
>>>> +    /*
>>>> +     * Can't use cpufeature API to determine whether BBM level 2
>>>> +     * is supported or not since cpufeature have not been
>>>> +     * finalized yet.
>>>> +     */
>>> I think assuming the system has BMML2 just because the boot CPU has it is going
>>> to be problematic. I think you are going to continue to need to initially paint
>>> the linear map with ptes then later, once the system cap is finalized, repaint
>>> with blocks.
>> Please see the response for patch 1. We may be able to split it instead of
>> collapse it.
>>
>>>> +    return (rodata_full && !bbmlv2_available()) ||
>>>> +        debug_pagealloc_enabled() ||
>>>> +        arm64_kfence_can_set_direct_map() ||
>>>> +        is_realm_world();
>>>> +}
>>> Previously the decision was determined by:
>>>
>>> bool can_set_direct_map(void)
>>> {
>>>      /*
>>>       * rodata_full, DEBUG_PAGEALLOC and a Realm guest all require linear
>>>       * map to be mapped at page granularity, so that it is possible to
>>>       * protect/unprotect single pages.
>>>       *
>>>       * KFENCE pool requires page-granular mapping if initialized late.
>>>       *
>>>       * Realms need to make pages shared/protected at page granularity.
>>>       */
>>>      return rodata_full || debug_pagealloc_enabled() ||
>>>          arm64_kfence_can_set_direct_map() || is_realm_world();
>>> }
>>>
>>> But now that you can safely split block mappings to page mappings when needed
>>> doesn't that cover some of these requirements? So perhaps you can do block
>>> mappings even when some (all?) of these things are active?
>> I did look into some of them. I don't know too much about realm, so I can't tell.
>>
>> But pagealloc will change kernel direct map to invalid when freeing page so it
>> is called quite often. The pagealloc debug is typically not enabled for production.
>>
>> Kfence will change it to invalid when allocating kfence object (typically when
>> allocating slab), but kfence typically allocates and maps its pool very early on
>> ARM64 and the pool is separate from direct map and mapped at PTE level. So IMHO
>> I don't see to much value support large block mapping for them for now.
> OK perhaps leave as is for now, and we can take a view on whether we want to
> extend later as incremental patches.

Yes, definitely.

>
>>>> +
>>>>    static void __init map_mem(pgd_t *pgdp)
>>>>    {
>>>>        static const u64 direct_map_end = _PAGE_END(VA_BITS_MIN);
>>>> @@ -645,9 +658,21 @@ static void __init map_mem(pgd_t *pgdp)
>>>>          early_kfence_pool = arm64_kfence_alloc_pool();
>>>>    -    if (can_set_direct_map())
>>>> +    if (force_pte_mapping())
>>>>            flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
>>>>    +    /*
>>>> +     * With FEAT_BBM level 2 we can split large block mapping without
>>>> +     * making it invalid.  So kernel linear mapping can be mapped with
>>>> +     * large block instead of PTE level.
>>>> +     *
>>>> +     * Need to break cont for CONT_MAPPINGS when changing permission,
>>>> +     * and need to inspect the adjacent page table entries to make
>>>> +     * them cont again later.  It sounds not worth the complexity.
>>> I'm not sure I follow the argument? As far as I can tell, you only ever split
>>> mappings. You never collapse. So I don't think it's too complex to check for
>>> CONT bits and if set, clear the CONT bit from the block? That's a safe operation
>>> with BBML2. It means you need to iterate over the ptes which you don't currently
>>> do, but you only need to check a 1 PTE in every CONT_PTES.
>> Sorry for the confusion. Yes, I don't collapse. It seems I overthought this
>> case, I thought I need to clear CONT bit for all PTEs in the block (for example,
>> 16 PTEs). But it seems I just need to clear one PTE.
> No you were right; you need to clear the CONT bit from all 16 PTEs in the block.
> But since all PTEs in the block will either have CONT all set or all clear, you
> only need to check 1 in every 16 PTEs. If that check shows that CONT is set, you
> can clear the bit from the block that contains the PTE you checked. It will slow
> down the split process though, because now you have to descend to the PTE table,
> check 32 of the PTEs (512 / 16) and for any CONT blocks, clear the CONT bit.

Not only PTEs, but also PMDs if CONT_PMD entries existed. We need clear 
CONT bit for all the PMDs then split the affected PMD.

>
> Personally I think it's worth checking how much slower that is though, given you
> already have a test case. In general, I've found that there is value in working
> hard to keep the TLB usage as low as possible.
>
>>>> +     */
>>>> +    if (rodata_full)
>>>> +        flags |= NO_CONT_MAPPINGS;
>>>> +
>>>>        /*
>>>>         * Take care not to create a writable alias for the
>>>>         * read-only text and rodata sections of the kernel image.
>>>> @@ -1342,9 +1367,12 @@ int arch_add_memory(int nid, u64 start, u64 size,
>>>>          VM_BUG_ON(!mhp_range_allowed(start, size, true));
>>>>    -    if (can_set_direct_map())
>>>> +    if (force_pte_mapping())
>>>>            flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
>>>>    +    if (rodata_full)
>>>> +        flags |= NO_CONT_MAPPINGS;
>>>> +
>>>>        __create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
>>>>                     size, params->pgprot, __pgd_pgtable_alloc,
>>>>                     flags);
>>>> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
>>>> index 39fd1f7ff02a..ba2360ecb030 100644
>>>> --- a/arch/arm64/mm/pageattr.c
>>>> +++ b/arch/arm64/mm/pageattr.c
>>>> @@ -49,6 +49,145 @@ static int change_page_range(pte_t *ptep, unsigned long
>>>> addr, void *data)
>>>>        return 0;
>>>>    }
>>>>    +static int __split_linear_mapping_pmd(pud_t *pudp,
>>>> +                      unsigned long vaddr, unsigned long end)
>>> nit: are the double really underscores needed?
>> No, not really.
>>
>>>> +{
>>>> +    pmd_t *pmdp;
>>>> +    unsigned long next;
>>>> +
>>>> +    pmdp = pmd_offset(pudp, vaddr);
>>>> +
>>>> +    do {
>>>> +        next = pmd_addr_end(vaddr, end);
>>>> +
>>>> +        if (pmd_leaf(pmdp_get(pmdp))) {
>>>> +            struct page *pte_page;
>>>> +            unsigned long pfn = pmd_pfn(pmdp_get(pmdp));
>>>> +            pgprot_t prot = pmd_pgprot(pmdp_get(pmdp));
>>>> +            pte_t *ptep_new;
>>>> +            int i;
>>>> +
>>>> +            pte_page = alloc_page(GFP_KERNEL);
>>>> +            if (!pte_page)
>>>> +                return -ENOMEM;
>>>> +
>>>> +            prot = __pgprot(pgprot_val(prot) | PTE_TYPE_PAGE);
>>>> +            ptep_new = (pte_t *)page_address(pte_page);
>>>> +            for (i = 0; i < PTRS_PER_PTE; ++i, ++ptep_new)
>>>> +                __set_pte_nosync(ptep_new,
>>>> +                         pfn_pte(pfn + i, prot));
>>>> +
>>>> +            dsb(ishst);
>>>> +            isb();
>>>> +
>>>> +            set_pmd(pmdp, pfn_pmd(page_to_pfn(pte_page),
>>>> +                __pgprot(PMD_TYPE_TABLE)));
>>> All the same comments that I made below for __split_linear_mapping_pud() apply
>>> here.
>> Sure
>>
>>>> +        }
>>>> +    } while (pmdp++, vaddr = next, vaddr != end);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int __split_linear_mapping_pud(p4d_t *p4dp,
>>>> +                      unsigned long vaddr, unsigned long end)
>>>> +{
>>>> +    pud_t *pudp;
>>>> +    unsigned long next;
>>>> +    int ret;
>>>> +
>>>> +    pudp = pud_offset(p4dp, vaddr);
>>>> +
>>>> +    do {
>>>> +        next = pud_addr_end(vaddr, end);
>>>> +
>>>> +        if (pud_leaf(pudp_get(pudp))) {
>>> Just do the pudp_get() once and cache the result. It's implemented by
>>> READ_ONCE() so your 3 invocations are actually doing 3 separate loads.
>> Sure
>>
>>>> +            struct page *pmd_page;
>>>> +            unsigned long pfn = pud_pfn(pudp_get(pudp));
>>>> +            pgprot_t prot = pud_pgprot(pudp_get(pudp));
>>>> +            pmd_t *pmdp_new;
>>>> +            int i;
>>>> +            unsigned int step;
>>>> +
>>>> +            pmd_page = alloc_page(GFP_KERNEL);
>>>> +            if (!pmd_page)
>>>> +                return -ENOMEM;
>>> Is this really how we allocate pgtable pages for kernel space? For user space we
>>> have quite a lot of machinery for this.
>> The userspace page table allocation does something extra, for example,
>>      - initialize ptlock
>>      - set folio type
>>      - update NR_PGTABLE counter
>>
>> We don't need them for kernel direct map page table, so just took the most
>> straightforward way. However, __get_free_page(GFP_PGTABLE_KERNEL) may be better
>> than alloc_page(GFP_KERNEL). It is used by memory hotplug.
> __create_pgd_mapping() takes a callback that it uses to allocate pgtable pages.
> The initial linear map creation passes in early_pgtable_alloc(), which allocates
> from memblock. I'm not sure if it's possible that we would ever need to split so
> early that we need to use memblock too?

No, AFAICT the earliest call to set_memory_rox() is allocating BPF prog 
in sock_init(). Buddy allocator has been ready at that point.

>
> For late allocations, we have __pgd_pgtable_alloc() and pgd_pgtable_alloc(). The
> latter constructs the ptdesc, although it's not completely clear to me when each
> should be used. Both of those functions BUG_ON() memory allocation failure which
> seems bad, especially given __pgd_pgtable_alloc() is used for mem hotplug, which
> really should be returning an error. Ouch.

I think __pgd_pgtable_alloc() should be good enough for splitting direct 
map.

>
> I think ideally we would rework __create_pgd_mapping() to be able to return an
> error (so we can fix the mem hotplug issue), handle collapse, split and
> permissions changes then use it universally. Or failing that,
> __get_free_page(GFP_PGTABLE_KERNEL) is the second best option.

I can try to make __create_pgd_mapping() handle splitting.

>
>>>> +
>>>> +            pmdp_new = (pmd_t *)page_address(pmd_page);
>>>> +            for (i = 0; i < PTRS_PER_PMD; ++i, ++pmdp_new) {
>>>> +                step = (i * PMD_SIZE) >> PAGE_SHIFT;
>>> Suggest hoisting this out of the loop, then just increment pfn by step every
>>> iteration.
>> Yeah, sure
>>
>>>> +                __set_pmd_nosync(pmdp_new,
>>>> +                         pfn_pmd(pfn + step, prot));
>>>> +            }
>>>> +
>>>> +            dsb(ishst);
>>>> +            isb();
>>> I'm not convinced by the need for this isb(). The dsb ensures that the table is
>>> visible to the table walker; that's needed. The isb() present in set_pmd() and
>>> friends to remove any "cached" speculative invalid translation marker from the
>>> pipeline, if I understand things correctly. I don't think we need to solve that
>>> problem here because you weren't working on a live table. The isb() in set_pud()
>>> is sufficient, I think.
>> I see, we just need one isb() when the table becomes reachable. Will remove it
>> and the isb() in __split_linear_mapping_pmd().
>>>>> +
>>>> +            set_pud(pudp, pfn_pud(page_to_pfn(pmd_page),
>>>> +                __pgprot(PUD_TYPE_TABLE)));
>>>> +        }
>>>> +
>>>> +        ret = __split_linear_mapping_pmd(pudp, vaddr, next);
>>>> +        if (ret)
>>>> +            return ret;
>>>> +    } while (pudp++, vaddr = next, vaddr != end);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int __split_linear_mapping_p4d(pgd_t *pgdp,
>>>> +                      unsigned long vaddr, unsigned long end)
>>>> +{
>>>> +    p4d_t *p4dp;
>>>> +    unsigned long next;
>>>> +    int ret;
>>>> +
>>>> +    p4dp = p4d_offset(pgdp, vaddr);
>>>> +
>>>> +    do {
>>>> +        next = p4d_addr_end(vaddr, end);
>>>> +
>>>> +        ret = __split_linear_mapping_pud(p4dp, vaddr, next);
>>>> +        if (ret)
>>>> +            return ret;
>>> nit: personally I would make this break. Then have single "return ret", like you
>>> have for __split_linear_mapping_pgd(). Then all levels follow the same template.
>> Sure
>>
>>>> +    } while (p4dp++, vaddr = next, vaddr != end);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int __split_linear_mapping_pgd(pgd_t *pgdp,
>>>> +                      unsigned long vaddr,
>>>> +                      unsigned long end)
>>>> +{> +    unsigned long next;
>>>> +    int ret = 0;
>>>> +
>>>> +    mmap_write_lock(&init_mm);
>>> nit: I think the locking would be more natural in split_linear_mapping().
>> OK
>>
>>>> +
>>>> +    do {
>>>> +        next = pgd_addr_end(vaddr, end);
>>>> +        ret = __split_linear_mapping_p4d(pgdp, vaddr, next);
>>>> +        if (ret)
>>>> +            break;
>>>> +    } while (pgdp++, vaddr = next, vaddr != end);
>>>> +
>>>> +    mmap_write_unlock(&init_mm);
>>>> +
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +static int split_linear_mapping(unsigned long start, unsigned long end)
>>> As far as I can tell, you only ever split the linear mapping. You never collapse
>>> it. In that case, won't it just degrade to pte-mappings over time? I suspect you
>>> should aim to detect collapse opportunities.
>> In theory there may be kernel linear mapping fragmentation problem, for example,
>> huge number of modules are loaded, but I've never heard of anyone reported this
>> as a real issue. And AFAICT, no architecture supports this for now.
> I'm not really talking about fragmentation. I'm talking about allocating
> different portions of the linear map over time and having to split the mapping
> down to ptes due to changing permissions. Over time this will tend towards the
> entire linear map being pte mapped if you don't proactively collapse too?

I actually meant the same thing.

>
> I accept this is unlikely to happen due to module loading since you tend to load
> the modules at boot and they stay loaded. But what about other features that
> cause the linear map permissions to be changed? e.g. secret mem, bpf programs
> being loaded, guest_memfd (although I don't think that's upstream yet).

Yes, but those features cause the same on other architectures too, for 
example, x86 and riscv, and we can't tell how bad it is in real life. It 
may turn out to be a problem at some point. But TBH I don't worry it too 
much right now. So I'd prefer keep this series as simple as possible. We 
can add collapse incrementally in the future.

Thanks,
Yang


>
>>>> +{
>>>> +    int ret;
>>>> +
>>>> +    if (!system_supports_bbmlv2())
>>>> +        return 0;
>>>> +
>>>> +    ret = __split_linear_mapping_pgd(pgd_offset_k(start), start, end);
>>> nit: personally I would put pgd_offset_k() into __split_linear_mapping_pgd() so
>>> that it takes the exact same form as the other levels. Or mege
>>> __split_linear_mapping_pgd() into split_linear_mapping().
>> OK, I will clean it up
>>
>>>> +    flush_tlb_kernel_range(start, end);
>>>> +
>>>> +    return ret;
>>>> +}
>>>> +
>>>>    /*
>>>>     * This function assumes that the range is mapped with PAGE_SIZE pages.
>>>>     */
>>>> @@ -80,8 +219,9 @@ static int change_memory_common(unsigned long addr, int
>>>> numpages,
>>>>        unsigned long start = addr;
>>>>        unsigned long size = PAGE_SIZE * numpages;
>>>>        unsigned long end = start + size;
>>>> +    unsigned long l_start;
>>>>        struct vm_struct *area;
>>>> -    int i;
>>>> +    int i, ret;
>>>>          if (!PAGE_ALIGNED(addr)) {
>>>>            start &= PAGE_MASK;
>>>> @@ -118,7 +258,12 @@ static int change_memory_common(unsigned long addr, int
>>>> numpages,
>>>>        if (rodata_full && (pgprot_val(set_mask) == PTE_RDONLY ||
>>>>                    pgprot_val(clear_mask) == PTE_RDONLY)) {
>>>>            for (i = 0; i < area->nr_pages; i++) {
>>>> -            __change_memory_common((u64)page_address(area->pages[i]),
>>>> +            l_start = (u64)page_address(area->pages[i]);
>>>> +            ret = split_linear_mapping(l_start, l_start + PAGE_SIZE);
>>>> +            if (WARN_ON_ONCE(ret))
>>>> +                return ret;
>>>> +
>>>> +            __change_memory_common(l_start,
>>>>                               PAGE_SIZE, set_mask, clear_mask);
>>>>            }
>>>>        }
>>>> @@ -174,6 +319,9 @@ int set_memory_valid(unsigned long addr, int numpages,
>>>> int enable)
>>> set_memory_valid() actually calls __change_memory_common() directly (so it
>>> bypassed the split logic in change_memory_common()). Is it possible that it gets
>>> called for a non-split region? I guess you'll need to handle that case too?
>> The set_memopry_valid() is called in the below places:
>>      - kfence: it requires map at PTE level regardless of kfence_early_init or not
>>      - pagealloc debug: it requires map at PTE level too
>>      - execmem ROX cache: it seems like only x86 supports this right now
>>
>> I don't think we are going to support large block mapping for direct map for
>> pagealloc debug and kfence as I said above. We need make set_memory_valid()
>> split page table when execmem ROX supported is added for arm64. But we don't
>> need do it right now.
> Fair enough.
>
> Thanks,
> Ryan
>
>> Thanks,
>> Yang
>>
>>> Thanks,
>>> Ryan
>>>
>>>>      int set_direct_map_invalid_noflush(struct page *page)
>>>>    {
>>>> +    unsigned long l_start;
>>>> +    int ret;
>>>> +
>>>>        struct page_change_data data = {
>>>>            .set_mask = __pgprot(0),
>>>>            .clear_mask = __pgprot(PTE_VALID),
>>>> @@ -182,13 +330,21 @@ int set_direct_map_invalid_noflush(struct page *page)
>>>>        if (!can_set_direct_map())
>>>>            return 0;
>>>>    +    l_start = (unsigned long)page_address(page);
>>>> +    ret = split_linear_mapping(l_start, l_start + PAGE_SIZE);
>>>> +    if (WARN_ON_ONCE(ret))
>>>> +        return ret;
>>>> +
>>>>        return apply_to_page_range(&init_mm,
>>>> -                   (unsigned long)page_address(page),
>>>> -                   PAGE_SIZE, change_page_range, &data);
>>>> +                   l_start, PAGE_SIZE, change_page_range,
>>>> +                   &data);
>>>>    }
>>>>      int set_direct_map_default_noflush(struct page *page)
>>>>    {
>>>> +    unsigned long l_start;
>>>> +    int ret;
>>>> +
>>>>        struct page_change_data data = {
>>>>            .set_mask = __pgprot(PTE_VALID | PTE_WRITE),
>>>>            .clear_mask = __pgprot(PTE_RDONLY),
>>>> @@ -197,9 +353,14 @@ int set_direct_map_default_noflush(struct page *page)
>>>>        if (!can_set_direct_map())
>>>>            return 0;
>>>>    +    l_start = (unsigned long)page_address(page);
>>>> +    ret = split_linear_mapping(l_start, l_start + PAGE_SIZE);
>>>> +    if (WARN_ON_ONCE(ret))
>>>> +        return ret;
>>>> +
>>>>        return apply_to_page_range(&init_mm,
>>>> -                   (unsigned long)page_address(page),
>>>> -                   PAGE_SIZE, change_page_range, &data);
>>>> +                   l_start, PAGE_SIZE, change_page_range,
>>>> +                   &data);
>>>>    }
>>>>      static int __set_memory_enc_dec(unsigned long addr,