Re: [PATCH OLK-5.10] arm64: enabled e820_pmem and add memmap parameter

Hi Ling,

On 2021/2/27 14:52, Zhuling wrote:

From: zhuling zhuling8@huawei.com

hulk inclusion category: feature bugzilla: 48159 CVE: NA

Enabled e820_pmem in arm64:

e820 is specific to x86 to report memory map to OS, which is not used for ARM64, could you please update the name?

Use memmap=nn[KMG]!ss[KMG] reserve memory for persistent storage when the kernel restart or update. the data in PMEM will not be lost and can be loaded faster.this is a general features.

if you want use this features, you need do as follows: 1.reserve memory: add memmap to reserve memory in grub.cfg memmap=nn[KMG]!ss[KMG] exp:memmap=100K!0x1a0000000. 2.insmod nd_e820.ko: modprobe nd_e820.

How do you compile the x86's ko on arm64?

3.check pmem device in /dev exp: /dev/pmem0.

Signed-off-by: zhuling zhuling8@huawei.com

arch/arm64/Kconfig | 24 ++++++++++ arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/pmem.c | 35 ++++++++++++++ arch/arm64/kernel/setup.c | 6 +++ arch/arm64/mm/init.c | 98 ++++++++++++++++++++++++++++++++++++++ drivers/nvdimm/Makefile | 1 + include/linux/mm.h | 4 ++ 7 files changed, 169 insertions(+) create mode 100644 arch/arm64/kernel/pmem.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index b9c56543c..f1e05d9d2 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1141,6 +1141,30 @@ config XEN_DOM0 def_bool y depends on XEN

+config ARM64_PMEM_LEGACY_DEVICE

• bool

+config ARM64_PMEM_RESERVE

• bool "reserve memory for persistent storage"
• default y
• help

•  Use memmap=nn[KMG]!ss[KMG](memmap=100K!0x1a0000000) reserve memory for
  

•  persistent storage
  

•  Say y here to enable this feature
  

+config ARM64_PMEM_LEGACY

• tristate "create persistent storage"
• depends on ARM64_PMEM_RESERVE
• depends on BLK_DEV
• select ARM64_PMEM_LEGACY_DEVICE
• select LIBNVDIMM
• help

•  Use reserved memory for persistent storage when the kernel restart
  

•  or update. the data in PMEM will not be lost and can be loaded faster.
  

•  Say y if unsure.
  

Why not add the config in alphabetical order? you see, you just add the code between config XEN...

• config XEN bool "Xen guest support on ARM64" depends on ARM64 && OF

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index 2621d5c2b..c363639b8 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -63,6 +63,7 @@ obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o obj-$(CONFIG_SHADOW_CALL_STACK) += scs.o obj-$(CONFIG_ARM64_MTE) += mte.o +obj-$(CONFIG_ARM64_PMEM_LEGACY_DEVICE) += pmem.o

obj-y += vdso/ probes/ obj-$(CONFIG_COMPAT_VDSO) += vdso32/ diff --git a/arch/arm64/kernel/pmem.c b/arch/arm64/kernel/pmem.c new file mode 100644 index 000000000..16eaf706f --- /dev/null +++ b/arch/arm64/kernel/pmem.c @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: GPL-2.0 +/*

• • Copyright(c) 2021 Huawei Technologies Co., Ltd
• • Derived from x86 and arm64 implement PMEM.
• */

+#include <linux/platform_device.h> +#include <linux/init.h> +#include <linux/ioport.h> +#include <linux/module.h>

+static int found(struct resource *res, void *data) +{

• return 1;

+}

+static int __init register_e820_pmem(void) +{

• struct platform_device *pdev;
• int rc;
• rc = walk_iomem_res_desc(IORES_DESC_PERSISTENT_MEMORY_LEGACY,

• 		 IORESOURCE_MEM, 0, -1, NULL, found);
  

• if (rc <= 0)

• return 0;
  

• /*

• * See drivers/nvdimm/e820.c for the implementation, this is
  

• * simply here to trigger the module to load on demand.
  

• */
  

• pdev = platform_device_alloc("e820_pmem", -1);

e820 is only for x86, don't use this name.

• return platform_device_add(pdev);

+} +device_initcall(register_e820_pmem); diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index 6aff30de8..7f506036d 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -255,6 +255,12 @@ static void __init request_standard_resources(void) request_resource(res, &crashk_res); #endif }

+#ifdef CONFIG_ARM64_PMEM_RESERVE

• if (pmem_res.end && pmem_res.start)

• request_resource(&iomem_resource, &pmem_res);
  

+#endif

• }

  static int __init reserve_memblock_reserved_regions(void)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index 794f992cb..e4dc19145 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -63,6 +63,18 @@ EXPORT_SYMBOL(memstart_addr); phys_addr_t arm64_dma_phys_limit __ro_after_init; phys_addr_t arm64_dma32_phys_limit __ro_after_init;

+static unsigned long long pmem_size, pmem_start;

+#ifdef CONFIG_ARM64_PMEM_RESERVE +struct resource pmem_res = {

• .name = "Persistent Memory (legacy)",
• .start = 0,
• .end = 0,
• .flags = IORESOURCE_MEM,
• .desc = IORES_DESC_PERSISTENT_MEMORY_LEGACY

+}; +#endif

• #ifndef CONFIG_KEXEC_CORE static void __init reserve_crashkernel(void) {

@@ -236,6 +248,88 @@ static void __init fdt_enforce_memory_region(void) memblock_add(usable_rgns[1].base, usable_rgns[1].size); }

+static int __init is_mem_valid(unsigned long long mem_size, unsigned long long mem_start) +{

• if (!memblock_is_region_memory(mem_start, mem_size)) {

• pr_warn("cannot reserve mem: region is not memory!\n");
  

• return -EINVAL;
  

• }
• if (memblock_is_region_reserved(mem_start, mem_size)) {

• pr_warn("cannot reserve mem: region overlaps reserved memory!\n");
  

• return -EINVAL;
  

• }
• if (!IS_ALIGNED(mem_start, SZ_2M)) {

• pr_warn("cannot reserve mem: base address is not 2MB aligned!\n");
  

• return -EINVAL;
  

• }
• return 0;

+}

+static int __init parse_memmap_one(char *p) +{

• char *oldp;
• phys_addr_t start_at, mem_size;
• int ret;
• if (!p)

• return -EINVAL;
  

• oldp = p;
• mem_size = memparse(p, &p);
• if (p == oldp)

• return -EINVAL;
  

• if (!mem_size)

• return -EINVAL;
  

• mem_size = PAGE_ALIGN(mem_size);
• if (*p == '!') {

• start_at = memparse(p+1, &p);
  

• if (is_mem_valid(mem_size, start_at) != 0)
  

• 	return -EINVAL;
  

• pr_info("pmem reserved: 0x%016llx - 0x%016llx (%lld MB)\n",
  

• 		start_at, start_at + mem_size, mem_size >> 20);
  

• pmem_start = start_at;
  

• pmem_size = mem_size;
  

• } else

• pr_info("Unrecognized memmap option, please check the parameter.\n");
  

• return *p == '\0' ? 0 : -EINVAL;

+}

+static int __init parse_memmap_opt(char *str) +{

• while (str) {

• char *k = strchr(str, ',');
  

• if (k)
  

• 	*k++ = 0;
  

• parse_memmap_one(str);
  

• str = k;
  

• }
• return 0;

+} +early_param("memmap", parse_memmap_opt);

+#ifdef CONFIG_ARM64_PMEM_RESERVE +static void __init reserve_pmem(void) +{

• memblock_remove(pmem_start, pmem_size);
• pr_info("pmem reserved: 0x%016llx - 0x%016llx (%lld MB)\n",

• 	pmem_start, pmem_start + pmem_size, pmem_size >> 20);
  

• pmem_res.start = pmem_start;
• pmem_res.end = pmem_start + pmem_size - 1;

+} +#endif

• void __init arm64_memblock_init(void) { const s64 linear_region_size = BIT(vabits_actual - 1);

@@ -359,6 +453,10 @@ void __init arm64_memblock_init(void)

reserve_elfcorehdr();

+#ifdef CONFIG_ARM64_PMEM_RESERVE

• reserve_pmem();

+#endif

• high_memory = __va(memblock_end_of_DRAM() - 1) + 1;

  dma_contiguous_reserve(arm64_dma32_phys_limit);

diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile index 29203f3d3..b97760e9f 100644 --- a/drivers/nvdimm/Makefile +++ b/drivers/nvdimm/Makefile @@ -4,6 +4,7 @@ obj-$(CONFIG_BLK_DEV_PMEM) += nd_pmem.o obj-$(CONFIG_ND_BTT) += nd_btt.o obj-$(CONFIG_ND_BLK) += nd_blk.o obj-$(CONFIG_X86_PMEM_LEGACY) += nd_e820.o +obj-$(CONFIG_ARM64_PMEM_LEGACY) += nd_e820.o

OK, so you just add ARM64 support, this is not the right way to do the work, you can make nd_e820 arch independent.

But as I said, e820 is only for x86, did you add such support in ARM64 firmware?

obj-$(CONFIG_OF_PMEM) += of_pmem.o obj-$(CONFIG_VIRTIO_PMEM) += virtio_pmem.o nd_virtio.o

diff --git a/include/linux/mm.h b/include/linux/mm.h index cd5c31372..a5e50495e 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -45,6 +45,10 @@ extern int sysctl_page_lock_unfairness;

void init_mm_internals(void);

+#ifdef CONFIG_ARM64_PMEM_RESERVE +extern struct resource pmem_res; +#endif

Don't add arch specific code in the common head file, especially in linux/mm.h

Thanks Hanjun

• #ifndef CONFIG_NEED_MULTIPLE_NODES /* Don't use mapnrs, do it properly */ extern unsigned long max_mapnr;