Carry forward IMA measurement log on kexec on ARM64

From: Rob Herring <[email protected]>

Both arm64 and powerpc do essentially the same FDT /chosen setup for kexec. The differences are either omissions that arm64 should have or additional properties that will be ignored. The setup code can be combined and shared by both powerpc and arm64.

The differences relative to the arm64 version:
- If /chosen doesn't exist, it will be created (should never happen).
- Any old dtb and initrd reserved memory will be released.
- The new initrd and elfcorehdr are marked reserved.
- "linux,booted-from-kexec" is set.

The differences relative to the powerpc version:
- "kaslr-seed" and "rng-seed" may be set.
- "linux,elfcorehdr" is set.
- Any existing "linux,usable-memory-range" is removed.

Combine the code for setting up the /chosen node in the FDT and updating the memory reservation for kexec, for powerpc and arm64, in of_kexec_alloc_and_setup_fdt() and move it to "drivers/of/kexec.c".

Signed-off-by Rob Herring <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

---
 drivers/of/Makefile |   6 ++
 drivers/of/kexec.c  | 258 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h  |  13 +++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/of/kexec.c

drivers/of/Makefile +6 -0

index 6e1e5212f058..c13b982084a3 100644
@@ -14,4 +14,10 @@ obj-$(CONFIG_OF_RESOLVE)  += resolver.o
 obj-$(CONFIG_OF_OVERLAY) += overlay.o
 obj-$(CONFIG_OF_NUMA) += of_numa.o
 
+ifdef CONFIG_KEXEC_FILE
+ifdef CONFIG_OF_FLATTREE
+obj-y	+= kexec.o
+endif
+endif
+
 obj-$(CONFIG_OF_UNITTEST) += unittest-data/

drivers/of/kexec.c new file +258 -0

new file mode 100644
index 000000000000..469e09613cdd
@@ -0,0 +1,258 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2020 Arm Limited
+ *
+ * Based on arch/arm64/kernel/machine_kexec_file.c:
+ *  Copyright (C) 2018 Linaro Limited
+ *
+ * And arch/powerpc/kexec/file_load.c:
+ *  Copyright (C) 2016  IBM Corporation
+ */
+
+#include <linux/kernel.h>
+#include <linux/kexec.h>
+#include <linux/libfdt.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/random.h>
+#include <linux/types.h>
+
+/* relevant device tree properties */
+#define FDT_PROP_KEXEC_ELFHDR	"linux,elfcorehdr"
+#define FDT_PROP_MEM_RANGE	"linux,usable-memory-range"
+#define FDT_PROP_INITRD_START	"linux,initrd-start"
+#define FDT_PROP_INITRD_END	"linux,initrd-end"
+#define FDT_PROP_BOOTARGS	"bootargs"
+#define FDT_PROP_KASLR_SEED	"kaslr-seed"
+#define FDT_PROP_RNG_SEED	"rng-seed"
+#define RNG_SEED_SIZE		128
+
+/**
+ * fdt_find_and_del_mem_rsv - delete memory reservation with given address and size
+ *
+ * @fdt:	Flattened device tree for the current kernel.
+ * @start:	Starting address of the reserved memory.
+ * @size:	Size of the reserved memory.
+ *
+ * Return: 0 on success, or negative errno on error.
+ */
+static int fdt_find_and_del_mem_rsv(void *fdt, unsigned long start, unsigned long size)
+{
+	int i, ret, num_rsvs = fdt_num_mem_rsv(fdt);
+
+	for (i = 0; i < num_rsvs; i++) {
+		u64 rsv_start, rsv_size;
+
+		ret = fdt_get_mem_rsv(fdt, i, &rsv_start, &rsv_size);
+		if (ret) {
+			pr_err("Malformed device tree.\n");
+			return -EINVAL;
+		}
+
+		if (rsv_start == start && rsv_size == size) {
+			ret = fdt_del_mem_rsv(fdt, i);
+			if (ret) {
+				pr_err("Error deleting device tree reservation.\n");
+				return -EINVAL;
+			}
+
+			return 0;
+		}
+	}
+
+	return -ENOENT;
+}
+
+/*
+ * of_kexec_alloc_and_setup_fdt - Alloc and setup a new Flattened Device Tree
+ *
+ * @image:		kexec image being loaded.
+ * @initrd_load_addr:	Address where the next initrd will be loaded.
+ * @initrd_len:		Size of the next initrd, or 0 if there will be none.
+ * @cmdline:		Command line for the next kernel, or NULL if there will
+ *			be none.
+ *
+ * Return: fdt on success, or NULL errno on error.
+ */
+void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
+				   unsigned long initrd_load_addr,
+				   unsigned long initrd_len,
+				   const char *cmdline)
+{
+	void *fdt;
+	int ret, chosen_node;
+	const void *prop;
+	unsigned long fdt_size;
+
+	fdt_size = fdt_totalsize(initial_boot_params) +
+		   (cmdline ? strlen(cmdline) : 0) +
+		   FDT_EXTRA_SPACE;
+
+	fdt = kvmalloc(fdt_size, GFP_KERNEL);
+	if (!fdt)
+		return NULL;
+
+	ret = fdt_open_into(initial_boot_params, fdt, fdt_size);
+	if (ret < 0) {
+		pr_err("Error %d setting up the new device tree.\n", ret);
+		goto out;
+	}
+
+	/* Remove memory reservation for the current device tree. */
+	ret = fdt_find_and_del_mem_rsv(fdt, __pa(initial_boot_params),
+				       fdt_totalsize(initial_boot_params));
+	if (ret == -EINVAL) {
+		pr_err("Error removing memory reservation.\n");
+		goto out;
+	}
+
+	chosen_node = fdt_path_offset(fdt, "/chosen");
+	if (chosen_node == -FDT_ERR_NOTFOUND)
+		chosen_node = fdt_add_subnode(fdt, fdt_path_offset(fdt, "/"),
+					      "chosen");
+	if (chosen_node < 0) {
+		ret = chosen_node;
+		goto out;
+	}
+
+	ret = fdt_delprop(fdt, chosen_node, FDT_PROP_KEXEC_ELFHDR);
+	if (ret && ret != -FDT_ERR_NOTFOUND)
+		goto out;
+	ret = fdt_delprop(fdt, chosen_node, FDT_PROP_MEM_RANGE);
+	if (ret && ret != -FDT_ERR_NOTFOUND)
+		goto out;
+
+	/* Did we boot using an initrd? */
+	prop = fdt_getprop(fdt, chosen_node, "linux,initrd-start", NULL);
+	if (prop) {
+		u64 tmp_start, tmp_end, tmp_size;
+
+		tmp_start = fdt64_to_cpu(*((const fdt64_t *) prop));
+
+		prop = fdt_getprop(fdt, chosen_node, "linux,initrd-end", NULL);
+		if (!prop) {
+			ret = -EINVAL;
+			goto out;
+		}
+
+		tmp_end = fdt64_to_cpu(*((const fdt64_t *) prop));
+
+		/*
+		 * kexec reserves exact initrd size, while firmware may
+		 * reserve a multiple of PAGE_SIZE, so check for both.
+		 */
+		tmp_size = tmp_end - tmp_start;
+		ret = fdt_find_and_del_mem_rsv(fdt, tmp_start, tmp_size);
+		if (ret == -ENOENT)
+			ret = fdt_find_and_del_mem_rsv(fdt, tmp_start,
+						       round_up(tmp_size, PAGE_SIZE));
+		if (ret == -EINVAL)
+			goto out;
+	}
+
+	/* add initrd-* */
+	if (initrd_load_addr) {
+		ret = fdt_setprop_u64(fdt, chosen_node, FDT_PROP_INITRD_START,
+				      initrd_load_addr);
+		if (ret)
+			goto out;
+
+		ret = fdt_setprop_u64(fdt, chosen_node, FDT_PROP_INITRD_END,
+				      initrd_load_addr + initrd_len);
+		if (ret)
+			goto out;
+
+		ret = fdt_add_mem_rsv(fdt, initrd_load_addr, initrd_len);
+		if (ret)
+			goto out;
+
+	} else {
+		ret = fdt_delprop(fdt, chosen_node, FDT_PROP_INITRD_START);
+		if (ret && (ret != -FDT_ERR_NOTFOUND))
+			goto out;
+
+		ret = fdt_delprop(fdt, chosen_node, FDT_PROP_INITRD_END);
+		if (ret && (ret != -FDT_ERR_NOTFOUND))
+			goto out;
+	}
+
+	if (image->type == KEXEC_TYPE_CRASH) {
+		/* add linux,elfcorehdr */
+		ret = fdt_appendprop_addrrange(fdt, 0, chosen_node,
+				FDT_PROP_KEXEC_ELFHDR,
+				image->arch.elf_headers_mem,
+				image->arch.elf_headers_sz);
+		if (ret)
+			goto out;
+
+		/*
+		 * Avoid elfcorehdr from being stomped on in kdump kernel by
+		 * setting up memory reserve map.
+		 */
+		ret = fdt_add_mem_rsv(fdt, image->arch.elf_headers_mem,
+				      image->arch.elf_headers_sz);
+		if (ret)
+			goto out;
+
+		/* add linux,usable-memory-range */
+		ret = fdt_appendprop_addrrange(fdt, 0, chosen_node,
+				FDT_PROP_MEM_RANGE,
+				crashk_res.start,
+				crashk_res.end - crashk_res.start + 1);
+		if (ret)
+			goto out;
+	}
+
+	/* add bootargs */
+	if (cmdline) {
+		ret = fdt_setprop_string(fdt, chosen_node, FDT_PROP_BOOTARGS, cmdline);
+		if (ret)
+			goto out;
+	} else {
+		ret = fdt_delprop(fdt, chosen_node, FDT_PROP_BOOTARGS);
+		if (ret && (ret != -FDT_ERR_NOTFOUND))
+			goto out;
+	}
+
+	/* add kaslr-seed */
+	ret = fdt_delprop(fdt, chosen_node, FDT_PROP_KASLR_SEED);
+	if (ret == -FDT_ERR_NOTFOUND)
+		ret = 0;
+	else if (ret)
+		goto out;
+
+	if (rng_is_initialized()) {
+		u64 seed = get_random_u64();
+
+		ret = fdt_setprop_u64(fdt, chosen_node, FDT_PROP_KASLR_SEED, seed);
+		if (ret)
+			goto out;
+	} else {
+		pr_notice("RNG is not initialised: omitting \"%s\" property\n",
+				FDT_PROP_KASLR_SEED);
+	}
+
+	/* add rng-seed */
+	if (rng_is_initialized()) {
+		void *rng_seed;
+
+		ret = fdt_setprop_placeholder(fdt, chosen_node, FDT_PROP_RNG_SEED,
+				RNG_SEED_SIZE, &rng_seed);
+		if (ret)
+			goto out;
+		get_random_bytes(rng_seed, RNG_SEED_SIZE);
+	} else {
+		pr_notice("RNG is not initialised: omitting \"%s\" property\n",
+				FDT_PROP_RNG_SEED);
+	}
+
+	ret = fdt_setprop(fdt, chosen_node, "linux,booted-from-kexec", NULL, 0);
+
+out:
+	if (ret) {
+		kvfree(fdt);
+		fdt = NULL;
+	}
+
+	return fdt;
+}

include/linux/of.h +0 -0

index 4b27c9a27df3..f0eff5e84353 100644

Quoted 40 lines · Lakshmi Ramasubramanian

From: Rob Herring <[email protected]>

Both arm64 and powerpc do essentially the same FDT /chosen setup for kexec. The differences are either omissions that arm64 should have or additional properties that will be ignored. The setup code can be combined and shared by both powerpc and arm64.

The differences relative to the arm64 version:
- If /chosen doesn't exist, it will be created (should never happen).
- Any old dtb and initrd reserved memory will be released.
- The new initrd and elfcorehdr are marked reserved.
- "linux,booted-from-kexec" is set.

The differences relative to the powerpc version:
- "kaslr-seed" and "rng-seed" may be set.
- "linux,elfcorehdr" is set.
- Any existing "linux,usable-memory-range" is removed.

Combine the code for setting up the /chosen node in the FDT and updating the memory reservation for kexec, for powerpc and arm64, in of_kexec_alloc_and_setup_fdt() and move it to "drivers/of/kexec.c".

Signed-off-by Rob Herring <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

---
  drivers/of/Makefile |   6 ++
  drivers/of/kexec.c  | 258 ++++++++++++++++++++++++++++++++++++++++++++
  include/linux/of.h  |  13 +++
  3 files changed, 277 insertions(+)

create mode 100644 drivers/of/kexec.c

include/linux/of.h +0 -0

index 4b27c9a27df3..f0eff5e84353 100644
@@ -560,6 +560,19 @@ int of_map_id(struct device_node *np, u32 id,
 
  phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
 

include/linux/of.h int of_map_id(struct device_node *np, u32 id, · line 568

Quoted 6 lines

include/linux/of.h +6 -0

+/*
+ * Additional space needed for the buffer to build the new FDT
+ * so that we can add initrd, bootargs, kaslr-seed, rng-seed,
+ * userable-memory-range and elfcorehdr.
+ */
+#define FDT_EXTRA_SPACE 0x1000

No need for this to be public now. Move it to of/kexec.c.

From: Rob Herring <[email protected]>

Both arm64 and powerpc do essentially the same FDT /chosen setup for kexec. The differences are either omissions that arm64 should have or additional properties that will be ignored. The setup code can be combined and shared by both powerpc and arm64.

The differences relative to the arm64 version:
- If /chosen doesn't exist, it will be created (should never happen).
- Any old dtb and initrd reserved memory will be released.
- The new initrd and elfcorehdr are marked reserved.
- "linux,booted-from-kexec" is set.

The differences relative to the powerpc version:
- "kaslr-seed" and "rng-seed" may be set.
- "linux,elfcorehdr" is set.
- Any existing "linux,usable-memory-range" is removed.

Combine the code for setting up the /chosen node in the FDT and updating the memory reservation for kexec, for powerpc and arm64, in of_kexec_alloc_and_setup_fdt() and move it to "drivers/of/kexec.c".

Signed-off-by Rob Herring <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

---
 drivers/of/Makefile |   6 ++
 drivers/of/kexec.c  | 258 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h  |  13 +++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/of/kexec.c

Quoted 1 line

drivers/of/kexec.c:183:17: error: 'const struct kimage_arch' has no member named 'elf_headers_mem'; did you mean 'elf_headers_sz'?

183 image->arch.elf_headers_mem,

Hi Rob,

[PATCH] powerpc: Rename kexec elfcorehdr_addr to elf_headers_mem

This change causes build problem for x86_64 architecture (please see the mail from kernel test bot below) since arch/x86/include/asm/kexec.h uses "elf_load_addr" for the ELF header buffer address and not "elf_headers_mem".

struct kimage_arch {
    ...

    /* Core ELF header buffer */
    void *elf_headers;
    unsigned long elf_headers_sz;
    unsigned long elf_load_addr;
};

I am thinking of limiting of_kexec_alloc_and_setup_fdt() to ARM64 and PPC64 since they are the only ones using this function now.

#if defined(CONFIG_ARM64) && defined(CONFIG_PPC64)

void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
                   unsigned long initrd_load_addr, unsigned long initrd_len,
                   const char *cmdline)
{
    ...
}
#endif /* defined(CONFIG_ARM64) && defined(CONFIG_PPC64) */

Please let me know if you have any concerns.

thanks,
 -lakshmi

-------- Forwarded Message --------
Subject: Re: [PATCH v17 02/10] of: Add a common kexec FDT setup function
Date: Fri, 12 Feb 2021 00:50:20 +0800
From: kernel test robot <[email protected]>
To: Lakshmi Ramasubramanian <[email protected]>
CC: [email protected]

Hi Lakshmi,

I love your patch! Yet something to improve:

[auto build test ERROR on integrity/next-integrity]
[also build test ERROR on v5.11-rc7 next-20210211]
[cannot apply to powerpc/next robh/for-next arm64/for-next/core] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Lakshmi-Ramasubramanian/Carry-forward-IMA-measurement-log-on-kexec-on-ARM64/20210211-071924

base: https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity

config: x86_64-randconfig-m001-20210211 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        #
https://github.com/0day-ci/linux/commit/12ae86067d115b84092353109e8798693d102f0d

        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review
Lakshmi-Ramasubramanian/Carry-forward-IMA-measurement-log-on-kexec-on-ARM64/20210211-071924

        git checkout 12ae86067d115b84092353109e8798693d102f0d

         # save the attached .config to linux build tree
         make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

   drivers/of/kexec.c: In function 'of_kexec_alloc_and_setup_fdt':

Quoted 1 line

Quoted 1 line

drivers/of/kexec.c:183:17: error: 'const struct kimage_arch' has no member named 'elf_headers_mem'; did you mean 'elf_headers_sz'?

     183 |     image->arch.elf_headers_mem,
         |                 ^~~~~~~~~~~~~~~
         |                 elf_headers_sz drivers/of/kexec.c:192:42: error: 'const struct kimage_arch' has no
member named 'elf_headers_mem'; did you mean 'elf_headers_sz'?
     192 |   ret = fdt_add_mem_rsv(fdt, image->arch.elf_headers_mem, |                                          ^~~~~~~~~~~~~~~ |                                          elf_headers_sz

vim +183 drivers/of/kexec.c

    65
    66    /*
    67     * of_kexec_alloc_and_setup_fdt - Alloc and setup a new
Flattened Device Tree
    68     *
    69     * @image:        kexec image being loaded.
    70     * @initrd_load_addr:    Address where the next initrd will
be loaded.
    71     * @initrd_len:        Size of the next initrd, or 0 if there
will be none.
    72     * @cmdline:        Command line for the next kernel, or NULL
if there will
    73     *            be none.
    74     *
    75     * Return: fdt on success, or NULL errno on error. 76     */
    77    void *of_kexec_alloc_and_setup_fdt(const struct kimage *image, 78                       unsigned long initrd_load_addr, 79                       unsigned long initrd_len,
    80                       const char *cmdline)
    81    {
    82        void *fdt;
    83        int ret, chosen_node;
    84        const void *prop;
    85        unsigned long fdt_size;
    86
    87        fdt_size = fdt_totalsize(initial_boot_params) + 88               (cmdline ? strlen(cmdline) : 0) +
    89               FDT_EXTRA_SPACE;
    90
    91        fdt = kvmalloc(fdt_size, GFP_KERNEL);
    92        if (!fdt)
    93            return NULL;
    94
    95        ret = fdt_open_into(initial_boot_params, fdt, fdt_size); 96        if (ret < 0) {
    97            pr_err("Error %d setting up the new device tree.\n",
ret);
    98            goto out;
    99        }
   100
   101        /* Remove memory reservation for the current device tree. */ 102        ret = fdt_find_and_del_mem_rsv(fdt,
__pa(initial_boot_params),
   103                           fdt_totalsize(initial_boot_params)); 104        if (ret == -EINVAL) {
   105            pr_err("Error removing memory reservation.\n"); 106            goto out;
   107        }
   108
   109        chosen_node = fdt_path_offset(fdt, "/chosen"); 110        if (chosen_node == -FDT_ERR_NOTFOUND)
   111            chosen_node = fdt_add_subnode(fdt,
fdt_path_offset(fdt, "/"),
   112                              "chosen");
   113        if (chosen_node < 0) {
   114            ret = chosen_node;
   115            goto out;
   116        }
   117
   118        ret = fdt_delprop(fdt, chosen_node, FDT_PROP_KEXEC_ELFHDR); 119        if (ret && ret != -FDT_ERR_NOTFOUND)
   120            goto out;
   121        ret = fdt_delprop(fdt, chosen_node, FDT_PROP_MEM_RANGE); 122        if (ret && ret != -FDT_ERR_NOTFOUND)
   123            goto out;
   124
   125        /* Did we boot using an initrd? */
   126        prop = fdt_getprop(fdt, chosen_node,
"linux,initrd-start", NULL);
   127        if (prop) {
   128            u64 tmp_start, tmp_end, tmp_size;
   129
   130            tmp_start = fdt64_to_cpu(*((const fdt64_t *) prop)); 131
   132            prop = fdt_getprop(fdt, chosen_node,
"linux,initrd-end", NULL);
   133            if (!prop) {
   134                ret = -EINVAL;
   135                goto out;
   136            }
   137
   138            tmp_end = fdt64_to_cpu(*((const fdt64_t *) prop)); 139
   140            /*
   141             * kexec reserves exact initrd size, while firmware may 142             * reserve a multiple of PAGE_SIZE, so check for both. 143             */
   144            tmp_size = tmp_end - tmp_start;
   145            ret = fdt_find_and_del_mem_rsv(fdt, tmp_start,
tmp_size);
   146            if (ret == -ENOENT)
   147                ret = fdt_find_and_del_mem_rsv(fdt, tmp_start, 148                                   round_up(tmp_size, PAGE_SIZE)); 149            if (ret == -EINVAL)
   150                goto out;
   151        }
   152
   153        /* add initrd-* */
   154        if (initrd_load_addr) {
   155            ret = fdt_setprop_u64(fdt, chosen_node,
FDT_PROP_INITRD_START,
   156                          initrd_load_addr);
   157            if (ret)
   158                goto out;
   159
   160            ret = fdt_setprop_u64(fdt, chosen_node,
FDT_PROP_INITRD_END,
   161                          initrd_load_addr + initrd_len); 162            if (ret)
   163                goto out;
   164
   165            ret = fdt_add_mem_rsv(fdt, initrd_load_addr,
initrd_len);
   166            if (ret)
   167                goto out;
   168
   169        } else {
   170            ret = fdt_delprop(fdt, chosen_node,
FDT_PROP_INITRD_START);
   171            if (ret && (ret != -FDT_ERR_NOTFOUND))
   172                goto out;
   173
   174            ret = fdt_delprop(fdt, chosen_node,
FDT_PROP_INITRD_END);
   175            if (ret && (ret != -FDT_ERR_NOTFOUND))
   176                goto out;
   177        }
   178
   179        if (image->type == KEXEC_TYPE_CRASH) {
   180            /* add linux,elfcorehdr */
   181            ret = fdt_appendprop_addrrange(fdt, 0, chosen_node, 182                    FDT_PROP_KEXEC_ELFHDR,
 > 183                    image->arch.elf_headers_mem,

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

Quoted 13 lines · Lakshmi Ramasubramanian

Hi Rob,
[PATCH] powerpc: Rename kexec elfcorehdr_addr to elf_headers_mem This change causes build problem for x86_64 architecture (please see the mail from kernel test bot below) since arch/x86/include/asm/kexec.h uses "elf_load_addr" for the ELF header buffer address and not "elf_headers_mem".

struct kimage_arch {
     ...
     /* Core ELF header buffer */
     void *elf_headers;
     unsigned long elf_headers_sz;
     unsigned long elf_load_addr;
};

Quoted 3 lines

I am thinking of limiting of_kexec_alloc_and_setup_fdt() to ARM64 and PPC64 since they are the only ones using this function now. #if defined(CONFIG_ARM64) && defined(CONFIG_PPC64)

Sorry - I meant to say
#if defined(CONFIG_ARM64) || defined(CONFIG_PPC64)

Quoted 11 lines

void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
unsigned long initrd_load_addr, unsigned long initrd_len,
const char *cmdline)

{
     ...
}

#endif /* defined(CONFIG_ARM64) && defined(CONFIG_PPC64) */ Please let me know if you have any concerns. thanks,
-lakshmi

Quoted 17 lines · Lakshmi Ramasubramanian

Quoted 16 lines · Lakshmi Ramasubramanian

Hi Rob,
[PATCH] powerpc: Rename kexec elfcorehdr_addr to elf_headers_mem This change causes build problem for x86_64 architecture (please see the mail from kernel test bot below) since arch/x86/include/asm/kexec.h uses "elf_load_addr" for the ELF header buffer address and not "elf_headers_mem".

struct kimage_arch {
      ...
      /* Core ELF header buffer */
      void *elf_headers;
      unsigned long elf_headers_sz;
      unsigned long elf_load_addr;
};

I am thinking of limiting of_kexec_alloc_and_setup_fdt() to ARM64 and PPC64 since they are the only ones using this function now. #if defined(CONFIG_ARM64) && defined(CONFIG_PPC64)

Quoted 3 lines

Sorry - I meant to say
#if defined(CONFIG_ARM64) || defined(CONFIG_PPC64)

Does it build correctly if you rename elf_headers_mem to elf_load_addr? Or the other way around, renaming x86's elf_load_addr to elf_headers_mem. I don't really have a preference.

That would be better than adding an #if condition.

Quoted 11 lines

Quoted 11 lines

void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
unsigned long initrd_load_addr, unsigned long initrd_len,
const char *cmdline)

{
      ...
}

#endif /* defined(CONFIG_ARM64) && defined(CONFIG_PPC64) */ Please let me know if you have any concerns. thanks,
-lakshmi

Quoted 19 lines · Thiago Jung Bauermann

Quoted 17 lines · Lakshmi Ramasubramanian

Quoted 16 lines · Lakshmi Ramasubramanian

Hi Rob,
[PATCH] powerpc: Rename kexec elfcorehdr_addr to elf_headers_mem This change causes build problem for x86_64 architecture (please see the mail from kernel test bot below) since arch/x86/include/asm/kexec.h uses "elf_load_addr" for the ELF header buffer address and not "elf_headers_mem".

struct kimage_arch {
      ...
      /* Core ELF header buffer */
      void *elf_headers;
      unsigned long elf_headers_sz;
      unsigned long elf_load_addr;
};

I am thinking of limiting of_kexec_alloc_and_setup_fdt() to ARM64 and PPC64 since they are the only ones using this function now. #if defined(CONFIG_ARM64) && defined(CONFIG_PPC64)

Quoted 6 lines

Quoted 3 lines

Sorry - I meant to say
#if defined(CONFIG_ARM64) || defined(CONFIG_PPC64)

Does it build correctly if you rename elf_headers_mem to elf_load_addr? Or the other way around, renaming x86's elf_load_addr to elf_headers_mem. I don't really have a preference.

Yes - changing arm64 and ppc from "elf_headers_mem" to "elf_load_addr" builds fine.

But I am concerned about a few other architectures that also define "struct kimage_arch" such as "parisc", "arm" which do not have any ELF related fields. They would not build if the config defines CONFIG_KEXEC_FILE and CONFIG_OF_FLATTREE.

Do you think that could be an issue?

Quoted 30 lines · Lakshmi Ramasubramanian

Quoted 19 lines · Thiago Jung Bauermann

Quoted 17 lines · Lakshmi Ramasubramanian

Quoted 16 lines · Lakshmi Ramasubramanian

Hi Rob,
[PATCH] powerpc: Rename kexec elfcorehdr_addr to elf_headers_mem This change causes build problem for x86_64 architecture (please see the mail from kernel test bot below) since arch/x86/include/asm/kexec.h uses "elf_load_addr" for the ELF header buffer address and not "elf_headers_mem".

struct kimage_arch {
       ...
       /* Core ELF header buffer */
       void *elf_headers;
       unsigned long elf_headers_sz;
       unsigned long elf_load_addr;
};

I am thinking of limiting of_kexec_alloc_and_setup_fdt() to ARM64 and PPC64 since they are the only ones using this function now. #if defined(CONFIG_ARM64) && defined(CONFIG_PPC64)

Quoted 6 lines

Quoted 3 lines

Sorry - I meant to say
#if defined(CONFIG_ARM64) || defined(CONFIG_PPC64)

Does it build correctly if you rename elf_headers_mem to elf_load_addr? Or the other way around, renaming x86's elf_load_addr to elf_headers_mem. I don't really have a preference.

Yes - changing arm64 and ppc from "elf_headers_mem" to "elf_load_addr" builds fine.

Quoted 6 lines

But I am concerned about a few other architectures that also define "struct kimage_arch" such as "parisc", "arm" which do not have any ELF related fields. They would not build if the config defines CONFIG_KEXEC_FILE and CONFIG_OF_FLATTREE.

Do you think that could be an issue?

That's a good point. But in practice, arm doesn't support CONFIG_KEXEC_FILE. And while parisc does support CONFIG_KEXEC_FILE, as far as I could determine it doesn't support CONFIG_OF.

So IMHO we don't need to worry about them. We'll cross that bridge if we get there. If they ever implement KEXEC_FILE or OF_FLATTREE support, then (again, IMHO) the natural solution would be for them to name the ELF header member the same way the other arches do.

And since no other architecture defines struct kimage_arch, those are the only ones we need to consider.

Quoted 38 lines · Thiago Jung Bauermann

Quoted 30 lines · Lakshmi Ramasubramanian

Quoted 19 lines · Thiago Jung Bauermann

Quoted 17 lines · Lakshmi Ramasubramanian

Quoted 16 lines · Lakshmi Ramasubramanian

Hi Rob,
[PATCH] powerpc: Rename kexec elfcorehdr_addr to elf_headers_mem This change causes build problem for x86_64 architecture (please see the mail from kernel test bot below) since arch/x86/include/asm/kexec.h uses "elf_load_addr" for the ELF header buffer address and not "elf_headers_mem".

struct kimage_arch {
       ...
       /* Core ELF header buffer */
       void *elf_headers;
       unsigned long elf_headers_sz;
       unsigned long elf_load_addr;
};

I am thinking of limiting of_kexec_alloc_and_setup_fdt() to ARM64 and PPC64 since they are the only ones using this function now. #if defined(CONFIG_ARM64) && defined(CONFIG_PPC64)

Quoted 6 lines

Quoted 3 lines

Sorry - I meant to say
#if defined(CONFIG_ARM64) || defined(CONFIG_PPC64)

Does it build correctly if you rename elf_headers_mem to elf_load_addr? Or the other way around, renaming x86's elf_load_addr to elf_headers_mem. I don't really have a preference.

Yes - changing arm64 and ppc from "elf_headers_mem" to "elf_load_addr" builds fine.

Quoted 6 lines

But I am concerned about a few other architectures that also define "struct kimage_arch" such as "parisc", "arm" which do not have any ELF related fields. They would not build if the config defines CONFIG_KEXEC_FILE and CONFIG_OF_FLATTREE.

Do you think that could be an issue?

Quoted 10 lines

That's a good point. But in practice, arm doesn't support CONFIG_KEXEC_FILE. And while parisc does support CONFIG_KEXEC_FILE, as far as I could determine it doesn't support CONFIG_OF.
So IMHO we don't need to worry about them. We'll cross that bridge if we get there. If they ever implement KEXEC_FILE or OF_FLATTREE support, then (again, IMHO) the natural solution would be for them to name the ELF header member the same way the other arches do.
And since no other architecture defines struct kimage_arch, those are the only ones we need to consider.

Sounds good Thiago.

I'll rename arm64 and ppc kimage_arch ELF address field to match that defined for x86/x64.

Also, will add "fdt_size" param to of_kexec_alloc_and_setup_fdt(). For now, I'll use 2*fdt_totalsize(initial_boot_params) for ppc.

Will send the updated patches shortly.

From: Rob Herring <[email protected]>

Both arm64 and powerpc do essentially the same FDT /chosen setup for kexec. The differences are either omissions that arm64 should have or additional properties that will be ignored. The setup code can be combined and shared by both powerpc and arm64.

The differences relative to the arm64 version:
- If /chosen doesn't exist, it will be created (should never happen).
- Any old dtb and initrd reserved memory will be released.
- The new initrd and elfcorehdr are marked reserved.
- "linux,booted-from-kexec" is set.

The differences relative to the powerpc version:
- "kaslr-seed" and "rng-seed" may be set.
- "linux,elfcorehdr" is set.
- Any existing "linux,usable-memory-range" is removed.

Combine the code for setting up the /chosen node in the FDT and updating the memory reservation for kexec, for powerpc and arm64, in of_kexec_alloc_and_setup_fdt() and move it to "drivers/of/kexec.c".

Signed-off-by Rob Herring <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

---
 drivers/of/Makefile |   6 ++
 drivers/of/kexec.c  | 258 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h  |  13 +++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/of/kexec.c

drivers/of/Makefile +6 -0

index 6e1e5212f058..c13b982084a3 100644
@@ -14,4 +14,10 @@ obj-$(CONFIG_OF_RESOLVE)  += resolver.o
 obj-$(CONFIG_OF_OVERLAY) += overlay.o
 obj-$(CONFIG_OF_NUMA) += of_numa.o
 
+ifdef CONFIG_KEXEC_FILE
+ifdef CONFIG_OF_FLATTREE
+obj-y	+= kexec.o
+endif
+endif
+
 obj-$(CONFIG_OF_UNITTEST) += unittest-data/

drivers/of/kexec.c new file +79 -0

new file mode 100644
index 000000000000..469e09613cdd
@@ -0,0 +1,258 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2020 Arm Limited
+ *
+ * Based on arch/arm64/kernel/machine_kexec_file.c:
+ *  Copyright (C) 2018 Linaro Limited
+ *
+ * And arch/powerpc/kexec/file_load.c:
+ *  Copyright (C) 2016  IBM Corporation
+ */
+
+#include <linux/kernel.h>
+#include <linux/kexec.h>
+#include <linux/libfdt.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/random.h>
+#include <linux/types.h>
+
+/* relevant device tree properties */
+#define FDT_PROP_KEXEC_ELFHDR	"linux,elfcorehdr"
+#define FDT_PROP_MEM_RANGE	"linux,usable-memory-range"
+#define FDT_PROP_INITRD_START	"linux,initrd-start"
+#define FDT_PROP_INITRD_END	"linux,initrd-end"
+#define FDT_PROP_BOOTARGS	"bootargs"
+#define FDT_PROP_KASLR_SEED	"kaslr-seed"
+#define FDT_PROP_RNG_SEED	"rng-seed"
+#define RNG_SEED_SIZE		128
+
+/**
+ * fdt_find_and_del_mem_rsv - delete memory reservation with given address and size
+ *
+ * @fdt:	Flattened device tree for the current kernel.
+ * @start:	Starting address of the reserved memory.
+ * @size:	Size of the reserved memory.
+ *
+ * Return: 0 on success, or negative errno on error.
+ */
+static int fdt_find_and_del_mem_rsv(void *fdt, unsigned long start, unsigned long size)
+{
+	int i, ret, num_rsvs = fdt_num_mem_rsv(fdt);
+
+	for (i = 0; i < num_rsvs; i++) {
+		u64 rsv_start, rsv_size;
+
+		ret = fdt_get_mem_rsv(fdt, i, &rsv_start, &rsv_size);
+		if (ret) {
+			pr_err("Malformed device tree.\n");
+			return -EINVAL;
+		}
+
+		if (rsv_start == start && rsv_size == size) {
+			ret = fdt_del_mem_rsv(fdt, i);
+			if (ret) {
+				pr_err("Error deleting device tree reservation.\n");
+				return -EINVAL;
+			}
+
+			return 0;
+		}
+	}
+
+	return -ENOENT;
+}
+
+/*
+ * of_kexec_alloc_and_setup_fdt - Alloc and setup a new Flattened Device Tree
+ *
+ * @image:		kexec image being loaded.
+ * @initrd_load_addr:	Address where the next initrd will be loaded.
+ * @initrd_len:		Size of the next initrd, or 0 if there will be none.
+ * @cmdline:		Command line for the next kernel, or NULL if there will
+ *			be none.
+ *
+ * Return: fdt on success, or NULL errno on error.
+ */
+void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
+				   unsigned long initrd_load_addr,
+				   unsigned long initrd_len,

There's actually a complication that I just noticed and needs to be addressed. More below.

Quoted 15 lines

drivers/of/kexec.c new file +15 -0

+
+/*
+ * of_kexec_alloc_and_setup_fdt - Alloc and setup a new Flattened Device Tree
+ *
+ * @image:		kexec image being loaded.
+ * @initrd_load_addr:	Address where the next initrd will be loaded.
+ * @initrd_len:		Size of the next initrd, or 0 if there will be none.
+ * @cmdline:		Command line for the next kernel, or NULL if there will
+ *			be none.
+ *
+ * Return: fdt on success, or NULL errno on error.
+ */
+void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
+				   unsigned long initrd_load_addr,
+				   unsigned long initrd_len,

drivers/of/kexec.c line 89

Quoted 10 lines

drivers/of/kexec.c new file +10 -0

+				   const char *cmdline)
+{
+	void *fdt;
+	int ret, chosen_node;
+	const void *prop;
+	unsigned long fdt_size;
+
+	fdt_size = fdt_totalsize(initial_boot_params) +
+		   (cmdline ? strlen(cmdline) : 0) +
+		   FDT_EXTRA_SPACE;

Just adding 4 KB to initial_boot_params won't be enough for crash kernels on ppc64. The current powerpc code doubles the size of initial_boot_params (which is normally larger than 4 KB) and even that isn't enough. A patch was added to powerpc/next today which uses a more precise (but arch-specific) formula:

https://lore.kernel.org/linuxppc-dev/161243826811.119001.14083048209224609814.stgit@hbathini/

So I believe we need a hook here where architectures can provide their own specific calculation for the size of the fdt. Perhaps a weakly defined function providing a default implementation which an arch-specific file can override (a la arch_kexec_kernel_image_load())?

Then the powerpc specific hook would be the kexec_fdt_totalsize_ppc64() function from the patch I linked above.

Quoted 3 lines · Thiago Jung Bauermann

There's actually a complication that I just noticed and needs to be addressed. More below.

<...>

Quoted 29 lines

Quoted 15 lines

drivers/of/kexec.c new file +15 -0

+
+/*
+ * of_kexec_alloc_and_setup_fdt - Alloc and setup a new Flattened Device Tree
+ *
+ * @image:		kexec image being loaded.
+ * @initrd_load_addr:	Address where the next initrd will be loaded.
+ * @initrd_len:		Size of the next initrd, or 0 if there will be none.
+ * @cmdline:		Command line for the next kernel, or NULL if there will
+ *			be none.
+ *
+ * Return: fdt on success, or NULL errno on error.
+ */
+void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
+				   unsigned long initrd_load_addr,
+				   unsigned long initrd_len,

drivers/of/kexec.c line 89

Quoted 10 lines

drivers/of/kexec.c new file +10 -0

+				   const char *cmdline)
+{
+	void *fdt;
+	int ret, chosen_node;
+	const void *prop;
+	unsigned long fdt_size;
+
+	fdt_size = fdt_totalsize(initial_boot_params) +
+		   (cmdline ? strlen(cmdline) : 0) +
+		   FDT_EXTRA_SPACE;

Just adding 4 KB to initial_boot_params won't be enough for crash kernels on ppc64. The current powerpc code doubles the size of initial_boot_params (which is normally larger than 4 KB) and even that isn't enough. A patch was added to powerpc/next today which uses a more

Quoted 9 lines

precise (but arch-specific) formula:
https://lore.kernel.org/linuxppc-dev/161243826811.119001.14083048209224609814.stgit@hbathini/
So I believe we need a hook here where architectures can provide their own specific calculation for the size of the fdt. Perhaps a weakly defined function providing a default implementation which an arch-specific file can override (a la arch_kexec_kernel_image_load())? Then the powerpc specific hook would be the kexec_fdt_totalsize_ppc64() function from the patch I linked above.

Do you think it'd better to add "fdt_size" parameter to of_kexec_alloc_and_setup_fdt() so that the caller can provide the desired FDT buffer size?

Quoted 4 lines · Thiago Jung Bauermann

There's actually a complication that I just noticed and needs to be addressed. More below.

<...>

Quoted 39 lines

Quoted 15 lines

drivers/of/kexec.c new file +15 -0

+
+/*
+ * of_kexec_alloc_and_setup_fdt - Alloc and setup a new Flattened Device Tree
+ *
+ * @image:          kexec image being loaded.
+ * @initrd_load_addr:       Address where the next initrd will be loaded.
+ * @initrd_len:             Size of the next initrd, or 0 if there will be none.
+ * @cmdline:                Command line for the next kernel, or NULL if there will
+ *                  be none.
+ *
+ * Return: fdt on success, or NULL errno on error.
+ */
+void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
+                               unsigned long initrd_load_addr,
+                               unsigned long initrd_len,

drivers/of/kexec.c line 89

Quoted 10 lines

drivers/of/kexec.c new file +10 -0

+                               const char *cmdline)
+{
+    void *fdt;
+    int ret, chosen_node;
+    const void *prop;
+    unsigned long fdt_size;
+
+    fdt_size = fdt_totalsize(initial_boot_params) +
+               (cmdline ? strlen(cmdline) : 0) +
+               FDT_EXTRA_SPACE;

Just adding 4 KB to initial_boot_params won't be enough for crash kernels on ppc64. The current powerpc code doubles the size of initial_boot_params (which is normally larger than 4 KB) and even that isn't enough. A patch was added to powerpc/next today which uses a more precise (but arch-specific) formula:

https://lore.kernel.org/linuxppc-dev/161243826811.119001.14083048209224609814.stgit@hbathini/

So I believe we need a hook here where architectures can provide their own specific calculation for the size of the fdt. Perhaps a weakly defined function providing a default implementation which an arch-specific file can override (a la arch_kexec_kernel_image_load())?

Quoted 3 lines

Then the powerpc specific hook would be the kexec_fdt_totalsize_ppc64() function from the patch I linked above.

Do you think it'd better to add "fdt_size" parameter to of_kexec_alloc_and_setup_fdt() so that the caller can provide the desired FDT buffer size?

Quoted 52 lines · Lakshmi Ramasubramanian

Quoted 4 lines · Thiago Jung Bauermann

There's actually a complication that I just noticed and needs to be addressed. More below.

<...>

Quoted 42 lines

Quoted 15 lines

drivers/of/kexec.c new file +15 -0

+
+/*
+ * of_kexec_alloc_and_setup_fdt - Alloc and setup a new Flattened Device Tree
+ *
+ * @image:          kexec image being loaded.
+ * @initrd_load_addr:       Address where the next initrd will be loaded.
+ * @initrd_len:             Size of the next initrd, or 0 if there will be none.
+ * @cmdline:                Command line for the next kernel, or NULL if there will
+ *                  be none.
+ *
+ * Return: fdt on success, or NULL errno on error.
+ */
+void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
+                               unsigned long initrd_load_addr,
+                               unsigned long initrd_len,

drivers/of/kexec.c line 89

Quoted 10 lines

drivers/of/kexec.c new file +10 -0

+                               const char *cmdline)
+{
+    void *fdt;
+    int ret, chosen_node;
+    const void *prop;
+    unsigned long fdt_size;
+
+    fdt_size = fdt_totalsize(initial_boot_params) +
+               (cmdline ? strlen(cmdline) : 0) +
+               FDT_EXTRA_SPACE;

Just adding 4 KB to initial_boot_params won't be enough for crash kernels on ppc64. The current powerpc code doubles the size of initial_boot_params (which is normally larger than 4 KB) and even that isn't enough. A patch was added to powerpc/next today which uses a more precise (but arch-specific) formula:

https://lore.kernel.org/linuxppc-dev/161243826811.119001.14083048209224609814.stgit@hbathini/

So I believe we need a hook here where architectures can provide their own specific calculation for the size of the fdt. Perhaps a weakly defined function providing a default implementation which an arch-specific file can override (a la arch_kexec_kernel_image_load())?

Then the powerpc specific hook would be the kexec_fdt_totalsize_ppc64() function from the patch I linked above.

Quoted 3 lines

Do you think it'd better to add "fdt_size" parameter to of_kexec_alloc_and_setup_fdt() so that the caller can provide the desired FDT buffer size?

Yes, I guess so. But please define the param as extra size, not total size. The kernel command line size addition can be in the common code.

The above change is also going to conflict, so I think this may have to wait. Or I'll take the common and arm bits and powerpc can be converted next cycle (or after the merge window).

Quoted 54 lines · Rob Herring

Quoted 52 lines · Lakshmi Ramasubramanian

Quoted 4 lines · Thiago Jung Bauermann

There's actually a complication that I just noticed and needs to be addressed. More below.

<...>

Quoted 42 lines

Quoted 15 lines

drivers/of/kexec.c new file +15 -0

+
+/*
+ * of_kexec_alloc_and_setup_fdt - Alloc and setup a new Flattened Device Tree
+ *
+ * @image:          kexec image being loaded.
+ * @initrd_load_addr:       Address where the next initrd will be loaded.
+ * @initrd_len:             Size of the next initrd, or 0 if there will be none.
+ * @cmdline:                Command line for the next kernel, or NULL if there will
+ *                  be none.
+ *
+ * Return: fdt on success, or NULL errno on error.
+ */
+void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
+                               unsigned long initrd_load_addr,
+                               unsigned long initrd_len,

drivers/of/kexec.c line 89

Quoted 10 lines

drivers/of/kexec.c new file +10 -0

+                               const char *cmdline)
+{
+    void *fdt;
+    int ret, chosen_node;
+    const void *prop;
+    unsigned long fdt_size;
+
+    fdt_size = fdt_totalsize(initial_boot_params) +
+               (cmdline ? strlen(cmdline) : 0) +
+               FDT_EXTRA_SPACE;

Just adding 4 KB to initial_boot_params won't be enough for crash kernels on ppc64. The current powerpc code doubles the size of initial_boot_params (which is normally larger than 4 KB) and even that isn't enough. A patch was added to powerpc/next today which uses a more precise (but arch-specific) formula:

https://lore.kernel.org/linuxppc-dev/161243826811.119001.14083048209224609814.stgit@hbathini/

So I believe we need a hook here where architectures can provide their own specific calculation for the size of the fdt. Perhaps a weakly defined function providing a default implementation which an arch-specific file can override (a la arch_kexec_kernel_image_load())?

Then the powerpc specific hook would be the kexec_fdt_totalsize_ppc64() function from the patch I linked above.

Quoted 6 lines

Quoted 3 lines

Do you think it'd better to add "fdt_size" parameter to of_kexec_alloc_and_setup_fdt() so that the caller can provide the desired FDT buffer size?

Yes, I guess so. But please define the param as extra size, not total size. The kernel command line size addition can be in the common code.

Will do. Just to clarify -

The common code will do:

fdt_totalsize(initial_boot_params) + strlen(cmdline) + extra_fdt_size

The caller will pass "extra_fdt_size"
ARM64 => 4KB
PPC64 => fdt_totalsize(initial_boot_params) - which will be updated when the patch Thiago had referred to is merged.

Quoted 67 lines · Lakshmi Ramasubramanian

Quoted 54 lines · Rob Herring

Quoted 52 lines · Lakshmi Ramasubramanian

Quoted 4 lines · Thiago Jung Bauermann

There's actually a complication that I just noticed and needs to be addressed. More below.

<...>

Quoted 42 lines

Quoted 15 lines

drivers/of/kexec.c new file +15 -0

+
+/*
+ * of_kexec_alloc_and_setup_fdt - Alloc and setup a new Flattened Device Tree
+ *
+ * @image:          kexec image being loaded.
+ * @initrd_load_addr:       Address where the next initrd will be loaded.
+ * @initrd_len:             Size of the next initrd, or 0 if there will be none.
+ * @cmdline:                Command line for the next kernel, or NULL if there will
+ *                  be none.
+ *
+ * Return: fdt on success, or NULL errno on error.
+ */
+void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
+                               unsigned long initrd_load_addr,
+                               unsigned long initrd_len,

drivers/of/kexec.c line 89

Quoted 10 lines

drivers/of/kexec.c new file +10 -0

+                               const char *cmdline)
+{
+    void *fdt;
+    int ret, chosen_node;
+    const void *prop;
+    unsigned long fdt_size;
+
+    fdt_size = fdt_totalsize(initial_boot_params) +
+               (cmdline ? strlen(cmdline) : 0) +
+               FDT_EXTRA_SPACE;

Just adding 4 KB to initial_boot_params won't be enough for crash kernels on ppc64. The current powerpc code doubles the size of initial_boot_params (which is normally larger than 4 KB) and even that isn't enough. A patch was added to powerpc/next today which uses a more precise (but arch-specific) formula:

https://lore.kernel.org/linuxppc-dev/161243826811.119001.14083048209224609814.stgit@hbathini/

So I believe we need a hook here where architectures can provide their own specific calculation for the size of the fdt. Perhaps a weakly defined function providing a default implementation which an arch-specific file can override (a la arch_kexec_kernel_image_load())?

Then the powerpc specific hook would be the kexec_fdt_totalsize_ppc64() function from the patch I linked above.

Quoted 6 lines

Quoted 3 lines

Do you think it'd better to add "fdt_size" parameter to of_kexec_alloc_and_setup_fdt() so that the caller can provide the desired FDT buffer size?

Yes, I guess so. But please define the param as extra size, not total size. The kernel command line size addition can be in the common code.

Will do. Just to clarify -

The common code will do:

Quoted 6 lines

fdt_totalsize(initial_boot_params) + strlen(cmdline) + extra_fdt_size

The caller will pass "extra_fdt_size"
ARM64 => 4KB
PPC64 => fdt_totalsize(initial_boot_params) - which will be updated when the patch Thiago had referred to is merged.

Yes, I'd leave the 4KB in there by default and arm64 use 0.

Quoted 69 lines · Rob Herring

Quoted 67 lines · Lakshmi Ramasubramanian

Quoted 54 lines · Rob Herring

Quoted 52 lines · Lakshmi Ramasubramanian

Quoted 4 lines · Thiago Jung Bauermann

There's actually a complication that I just noticed and needs to be addressed. More below.

<...>

Quoted 42 lines

Quoted 15 lines

(quoted diff) +15 -0

+
+/*
+ * of_kexec_alloc_and_setup_fdt - Alloc and setup a new Flattened Device Tree
+ *
+ * @image:          kexec image being loaded.
+ * @initrd_load_addr:       Address where the next initrd will be loaded.
+ * @initrd_len:             Size of the next initrd, or 0 if there will be none.
+ * @cmdline:                Command line for the next kernel, or NULL if there will
+ *                  be none.
+ *
+ * Return: fdt on success, or NULL errno on error.
+ */
+void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
+                               unsigned long initrd_load_addr,
+                               unsigned long initrd_len,

Review

Quoted 10 lines

(quoted diff) +10 -0

+                               const char *cmdline)
+{
+    void *fdt;
+    int ret, chosen_node;
+    const void *prop;
+    unsigned long fdt_size;
+
+    fdt_size = fdt_totalsize(initial_boot_params) +
+               (cmdline ? strlen(cmdline) : 0) +
+               FDT_EXTRA_SPACE;

Just adding 4 KB to initial_boot_params won't be enough for crash kernels on ppc64. The current powerpc code doubles the size of initial_boot_params (which is normally larger than 4 KB) and even that isn't enough. A patch was added to powerpc/next today which uses a more precise (but arch-specific) formula:

https://lore.kernel.org/linuxppc-dev/161243826811.119001.14083048209224609814.stgit@hbathini/

So I believe we need a hook here where architectures can provide their own specific calculation for the size of the fdt. Perhaps a weakly defined function providing a default implementation which an arch-specific file can override (a la arch_kexec_kernel_image_load())?

Then the powerpc specific hook would be the kexec_fdt_totalsize_ppc64() function from the patch I linked above.

Quoted 6 lines

Quoted 3 lines

Do you think it'd better to add "fdt_size" parameter to of_kexec_alloc_and_setup_fdt() so that the caller can provide the desired FDT buffer size?

Yes, I guess so. But please define the param as extra size, not total size. The kernel command line size addition can be in the common code.

Will do. Just to clarify -

The common code will do:

Quoted 8 lines

Quoted 6 lines

fdt_totalsize(initial_boot_params) + strlen(cmdline) + extra_fdt_size

The caller will pass "extra_fdt_size"
ARM64 => 4KB
PPC64 => fdt_totalsize(initial_boot_params) - which will be updated when the patch Thiago had referred to is merged.

Yes, I'd leave the 4KB in there by default and arm64 use 0.

Sounds good.

common:
fdt_totalsize(initial_boot_params) + strlen(cmdline) + 0x1000 + extra

arm64 => 0 for extra
ppc => fdt_totalsize(initial_boot_params) for extra.

From: Rob Herring <[email protected]>

The code for setting up the /chosen node in the device tree and updating the memory reservation for the next kernel has been moved to of_kexec_alloc_and_setup_fdt() defined in "drivers/of/kexec.c".

Use the common of_kexec_alloc_and_setup_fdt() to setup the device tree and update the memory reservation for kexec for arm64.

Signed-off-by Rob Herring <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

---
 arch/arm64/kernel/machine_kexec_file.c | 180 ++-----------------------
 1 file changed, 8 insertions(+), 172 deletions(-)

From: Rob Herring <[email protected]>

The code for setting up the /chosen node in the device tree and updating the memory reservation for the next kernel has been moved to of_kexec_alloc_and_setup_fdt() defined in "drivers/of/kexec.c".

Use the common of_kexec_alloc_and_setup_fdt() to setup the device tree and update the memory reservation for kexec for arm64.

Signed-off-by Rob Herring <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

---
 arch/arm64/kernel/machine_kexec_file.c | 180 ++-----------------------
 1 file changed, 8 insertions(+), 172 deletions(-)

From: Rob Herring <[email protected]>

The code for setting up the /chosen node in the device tree and updating the memory reservation for the next kernel has been moved to of_kexec_alloc_and_setup_fdt() defined in "drivers/of/kexec.c".

Use the common of_kexec_alloc_and_setup_fdt() to setup the device tree and update the memory reservation for kexec for powerpc.

Signed-off-by Rob Herring <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

---
 arch/powerpc/include/asm/kexec.h  |   1 +
 arch/powerpc/kexec/elf_64.c       |  29 ++++---
 arch/powerpc/kexec/file_load.c    | 132 +-----------------------------
 arch/powerpc/kexec/file_load_64.c |   3 +
 4 files changed, 25 insertions(+), 140 deletions(-)

arch/powerpc/include/asm/kexec.h +1 -0

index dbf09d2f36d0..bdd0ddb9ac4d 100644
@@ -111,6 +111,7 @@ struct kimage_arch {
 	unsigned long elf_headers_mem;
 	unsigned long elf_headers_sz;
 	void *elf_headers;
+	void *fdt;
 
 #ifdef CONFIG_IMA_KEXEC
 	phys_addr_t ima_buffer_addr;

arch/powerpc/kexec/elf_64.c +1 -1

index d0e459bb2f05..bfabd06f99b1 100644
@@ -19,6 +19,7 @@
 #include <linux/kexec.h>
 #include <linux/libfdt.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/of_fdt.h>
 #include <linux/slab.h>
 #include <linux/types.h>

@@ -29,7 +30,6 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 			unsigned long cmdline_len)
 {
 	int ret;
-	unsigned int fdt_size;
 	unsigned long kernel_load_addr;
 	unsigned long initrd_load_addr = 0, fdt_load_addr;
 	void *fdt;

Quoted 51 lines · Lakshmi Ramasubramanian

From: Rob Herring <[email protected]>

The code for setting up the /chosen node in the device tree and updating the memory reservation for the next kernel has been moved to of_kexec_alloc_and_setup_fdt() defined in "drivers/of/kexec.c".

Use the common of_kexec_alloc_and_setup_fdt() to setup the device tree and update the memory reservation for kexec for powerpc.

Signed-off-by Rob Herring <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

---
  arch/powerpc/include/asm/kexec.h  |   1 +
  arch/powerpc/kexec/elf_64.c       |  29 ++++---
  arch/powerpc/kexec/file_load.c    | 132 +-----------------------------
  arch/powerpc/kexec/file_load_64.c |   3 +
  4 files changed, 25 insertions(+), 140 deletions(-)

arch/powerpc/include/asm/kexec.h +1 -0

index dbf09d2f36d0..bdd0ddb9ac4d 100644
@@ -111,6 +111,7 @@ struct kimage_arch {
  	unsigned long elf_headers_mem;
  	unsigned long elf_headers_sz;
  	void *elf_headers;
+	void *fdt;
 
  #ifdef CONFIG_IMA_KEXEC
  	phys_addr_t ima_buffer_addr;

arch/powerpc/kexec/elf_64.c +1 -1

index d0e459bb2f05..bfabd06f99b1 100644
@@ -19,6 +19,7 @@
  #include <linux/kexec.h>
  #include <linux/libfdt.h>
  #include <linux/module.h>
+#include <linux/of.h>
  #include <linux/of_fdt.h>
  #include <linux/slab.h>
  #include <linux/types.h>

@@ -29,7 +30,6 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
  			unsigned long cmdline_len)
  {
  	int ret;
-	unsigned int fdt_size;
  	unsigned long kernel_load_addr;
  	unsigned long initrd_load_addr = 0, fdt_load_addr;
  	void *fdt;

arch/powerpc/kexec/elf_64.c static void *elf64_load(struct kimage *image, char *kernel_buf, · line 108

Quoted 9 lines

arch/powerpc/kexec/elf_64.c +2 -2

@@ -102,19 +102,13 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
  		pr_debug("Loaded initrd at 0x%lx\n", initrd_load_addr);
  	}
 
-	fdt_size = fdt_totalsize(initial_boot_params) * 2;
-	fdt = kmalloc(fdt_size, GFP_KERNEL);
+	fdt = of_kexec_alloc_and_setup_fdt(image, initrd_load_addr,
+					   initrd_len, cmdline);
  	if (!fdt) {
  		pr_err("Not enough memory for the device tree.\n");

This error string can be a bit misleading now, since of_kexec_alloc_and_setup_fdt() can fail for reasons other than lack of memory. I suggest changing it to the error string from fdt_open_into() below:

		pr_err("Error setting up the new device tree.\n");

With this change:

Reviewed-by Thiago Jung Bauermann <[email protected]>

And also:

Tested-by Thiago Jung Bauermann <[email protected]>

The fields ima_buffer_addr and ima_buffer_size in "struct kimage_arch" for powerpc are used to carry forward the IMA measurement list across kexec system call. These fields are not architecture specific, but are currently limited to powerpc.

arch_ima_add_kexec_buffer() defined in "arch/powerpc/kexec/ima.c" sets ima_buffer_addr and ima_buffer_size for the kexec system call. This function does not have architecture specific code, but is currently limited to powerpc.

Move ima_buffer_addr and ima_buffer_size to "struct kimage". Rename arch_ima_add_kexec_buffer() to of_ima_add_kexec_buffer() and move it in drivers/of/kexec.c.

Co-developed-by Prakhar Srivastava <[email protected]>

Signed-off-by Prakhar Srivastava <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

Suggested-by Will Deacon <[email protected]>

---
 arch/powerpc/include/asm/ima.h     |  3 ---
 arch/powerpc/include/asm/kexec.h   |  5 -----
 arch/powerpc/kexec/ima.c           | 29 ++++++-----------------------
 drivers/of/kexec.c                 | 23 +++++++++++++++++++++++
 include/linux/kexec.h              |  3 +++
 include/linux/of.h                 |  5 +++++
 security/integrity/ima/ima_kexec.c |  3 ++-
 7 files changed, 39 insertions(+), 32 deletions(-)

arch/powerpc/include/asm/ima.h +0 -3

index ead488cf3981..51f64fd06c19 100644
@@ -14,9 +14,6 @@ static inline void remove_ima_buffer(void *fdt, int chosen_node) {}
 #endif
 
 #ifdef CONFIG_IMA_KEXEC
-int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
-			      size_t size);
-
 int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node);
 #else
 static inline int setup_ima_buffer(const struct kimage *image, void *fdt,

arch/powerpc/include/asm/kexec.h +0 -5

index bdd0ddb9ac4d..ecf88533d6b4 100644
@@ -112,11 +112,6 @@ struct kimage_arch {
 	unsigned long elf_headers_sz;
 	void *elf_headers;
 	void *fdt;
-
-#ifdef CONFIG_IMA_KEXEC
-	phys_addr_t ima_buffer_addr;
-	size_t ima_buffer_size;
-#endif
 };
 
 char *setup_kdump_cmdline(struct kimage *image, char *cmdline,

arch/powerpc/kexec/ima.c +6 -23

index 720e50e490b6..ed38125e2f87 100644
@@ -128,23 +128,6 @@ void remove_ima_buffer(void *fdt, int chosen_node)
 }
 
 #ifdef CONFIG_IMA_KEXEC
-/**
- * arch_ima_add_kexec_buffer - do arch-specific steps to add the IMA buffer
- *
- * Architectures should use this function to pass on the IMA buffer
- * information to the next kernel.
- *
- * Return: 0 on success, negative errno on error.
- */
-int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
-			      size_t size)
-{
-	image->arch.ima_buffer_addr = load_addr;
-	image->arch.ima_buffer_size = size;
-
-	return 0;
-}
-
 static int write_number(void *p, u64 value, int cells)
 {
 	if (cells == 1) {

@@ -180,7 +163,7 @@ int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node)
 	u8 value[16];
 
 	remove_ima_buffer(fdt, chosen_node);
-	if (!image->arch.ima_buffer_size)
+	if (!image->ima_buffer_size)
 		return 0;
 
 	ret = get_addr_size_cells(&addr_cells, &size_cells);

@@ -192,11 +175,11 @@ int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node)
 	if (entry_size > sizeof(value))
 		return -EINVAL;
 
-	ret = write_number(value, image->arch.ima_buffer_addr, addr_cells);
+	ret = write_number(value, image->ima_buffer_addr, addr_cells);
 	if (ret)
 		return ret;
 
-	ret = write_number(value + 4 * addr_cells, image->arch.ima_buffer_size,
+	ret = write_number(value + 4 * addr_cells, image->ima_buffer_size,
 			   size_cells);
 	if (ret)
 		return ret;

@@ -206,13 +189,13 @@ int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node)
 	if (ret < 0)
 		return -EINVAL;
 
-	ret = fdt_add_mem_rsv(fdt, image->arch.ima_buffer_addr,
-			      image->arch.ima_buffer_size);
+	ret = fdt_add_mem_rsv(fdt, image->ima_buffer_addr,
+			      image->ima_buffer_size);
 	if (ret)
 		return -EINVAL;
 
 	pr_debug("IMA buffer at 0x%llx, size = 0x%zx\n",
-		 image->arch.ima_buffer_addr, image->arch.ima_buffer_size);
+		 image->ima_buffer_addr, image->ima_buffer_size);
 
 	return 0;
 }

drivers/of/kexec.c +9 -0

index 469e09613cdd..9f33d215b9f2 100644
@@ -63,6 +63,29 @@ static int fdt_find_and_del_mem_rsv(void *fdt, unsigned long start, unsigned lon
 	return -ENOENT;
 }
 
+#ifdef CONFIG_IMA_KEXEC
+/**
+ * of_ima_add_kexec_buffer - Add IMA buffer for next kernel
+ *
+ * @image: kimage struct to set IMA buffer data
+ * @load_addr: Starting address where IMA buffer is loaded at
+ * @size: Number of bytes in the IMA buffer
+ *
+ * Use this function to pass on the IMA buffer information to

Quoted 50 lines · Lakshmi Ramasubramanian

The fields ima_buffer_addr and ima_buffer_size in "struct kimage_arch" for powerpc are used to carry forward the IMA measurement list across kexec system call. These fields are not architecture specific, but are currently limited to powerpc.

arch_ima_add_kexec_buffer() defined in "arch/powerpc/kexec/ima.c" sets ima_buffer_addr and ima_buffer_size for the kexec system call. This function does not have architecture specific code, but is currently limited to powerpc.

Move ima_buffer_addr and ima_buffer_size to "struct kimage". Rename arch_ima_add_kexec_buffer() to of_ima_add_kexec_buffer() and move it in drivers/of/kexec.c.

Co-developed-by Prakhar Srivastava <[email protected]>

Signed-off-by Prakhar Srivastava <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

Suggested-by Will Deacon <[email protected]>

---
  arch/powerpc/include/asm/ima.h     |  3 ---
  arch/powerpc/include/asm/kexec.h   |  5 -----
  arch/powerpc/kexec/ima.c           | 29 ++++++-----------------------
  drivers/of/kexec.c                 | 23 +++++++++++++++++++++++
  include/linux/kexec.h              |  3 +++
  include/linux/of.h                 |  5 +++++
  security/integrity/ima/ima_kexec.c |  3 ++-
  7 files changed, 39 insertions(+), 32 deletions(-)

drivers/of/kexec.c +14 -0

index 469e09613cdd..9f33d215b9f2 100644
@@ -63,6 +63,29 @@ static int fdt_find_and_del_mem_rsv(void *fdt, unsigned long start, unsigned lon
  	return -ENOENT;
  }
 
+#ifdef CONFIG_IMA_KEXEC
+/**
+ * of_ima_add_kexec_buffer - Add IMA buffer for next kernel
+ *
+ * @image: kimage struct to set IMA buffer data
+ * @load_addr: Starting address where IMA buffer is loaded at
+ * @size: Number of bytes in the IMA buffer
+ *
+ * Use this function to pass on the IMA buffer information to
+ * the next kernel across kexec system call.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int of_ima_add_kexec_buffer(struct kimage *image,

drivers/of/kexec.c static int fdt_find_and_del_mem_rsv(void *fdt, unsigned long start, unsigned lon · line 84

Quoted 5 lines

drivers/of/kexec.c +5 -0

+			    unsigned long load_addr, size_t size)
+{
+	image->ima_buffer_addr = load_addr;
+	image->ima_buffer_size = size;
+

There's nothing DT specific about this function, so this is the wrong place for it. I would just remove it and directly set the members.

CONFIG_HAVE_IMA_KEXEC is enabled to indicate that the IMA measurement log information is present in the device tree. This should be selected only if CONFIG_IMA is enabled.

Update CONFIG_KEXEC_FILE to select CONFIG_HAVE_IMA_KEXEC, if CONFIG_IMA is enabled, to indicate that the IMA measurement log information is present in the device tree for powerpc.

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

Suggested-by Thiago Jung Bauermann <[email protected]>

---
 arch/powerpc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

The functions defined in "arch/powerpc/kexec/ima.c" handle setting up and freeing the resources required to carry over the IMA measurement list from the current kernel to the next kernel across kexec system call. These functions do not have architecture specific code, but are currently limited to powerpc.

Move remove_ima_buffer() and setup_ima_buffer() calls into of_kexec_alloc_and_setup_fdt() defined in "drivers/of/kexec.c".

Move the remaining architecture independent functions from "arch/powerpc/kexec/ima.c" to "drivers/of/kexec.c".
Delete "arch/powerpc/kexec/ima.c" and "arch/powerpc/include/asm/ima.h". Remove references to the deleted files and functions in powerpc and in ima.

Co-developed-by Prakhar Srivastava <[email protected]>

Signed-off-by Prakhar Srivastava <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

---
 arch/powerpc/include/asm/ima.h    |  27 ----
 arch/powerpc/include/asm/kexec.h  |   3 -
 arch/powerpc/kexec/Makefile       |   7 -
 arch/powerpc/kexec/file_load.c    |  25 ----
 arch/powerpc/kexec/file_load_64.c |   4 -
 arch/powerpc/kexec/ima.c          | 202 -------------------------
 drivers/of/kexec.c                | 239 ++++++++++++++++++++++++++++++
 include/linux/of.h                |   2 +
 security/integrity/ima/ima.h      |   4 -
 9 files changed, 241 insertions(+), 272 deletions(-)
 delete mode 100644 arch/powerpc/include/asm/ima.h
 delete mode 100644 arch/powerpc/kexec/ima.c

delete_fdt_mem_rsv() defined in "arch/powerpc/kexec/file_load.c" has been renamed to fdt_find_and_del_mem_rsv(), and moved to "drivers/of/kexec.c".

Remove delete_fdt_mem_rsv() in "arch/powerpc/kexec/file_load.c".

Co-developed-by Prakhar Srivastava <[email protected]>

Signed-off-by Prakhar Srivastava <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

---
 arch/powerpc/include/asm/kexec.h |  1 -
 arch/powerpc/kexec/file_load.c   | 32 --------------------------------
 2 files changed, 33 deletions(-)

Update CONFIG_KEXEC_FILE to select CONFIG_HAVE_IMA_KEXEC, if CONFIG_IMA is enabled, to indicate that the IMA measurement log information is present in the device tree for ARM64.

Co-developed-by Prakhar Srivastava <[email protected]>

Signed-off-by Prakhar Srivastava <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

Suggested-by Thiago Jung Bauermann <[email protected]>

---
 arch/arm64/Kconfig | 1 +
 1 file changed, 1 insertion(+)

On kexec file load Integrity Measurement Architecture (IMA) subsystem may verify the IMA signature of the kernel and initramfs, and measure it. The command line parameters passed to the kernel in the kexec call may also be measured by IMA. A remote attestation service can verify a TPM quote based on the TPM event log, the IMA measurement list, and the TPM PCR data. This can be achieved only if the IMA measurement log is carried over from the current kernel to the next kernel across the kexec call.

powerpc already supports carrying forward the IMA measurement log on kexec. This patch set adds support for carrying forward the IMA measurement log on kexec on ARM64.

This patch set moves the platform independent code defined for powerpc such that it can be reused for other platforms as well. A chosen node "linux,ima-kexec-buffer" is added to the DTB for ARM64 to hold the address and the size of the memory reserved to carry the IMA measurement log.

This patch set has been tested for ARM64 platform using QEMU. I would like help from the community for testing this change on powerpc. Thanks.

This patch set is based on
commit 96acc833dec8 ("ima: Free IMA measurement buffer after kexec syscall") in https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
"next-integrity" branch.

Quoted 23 lines · Lakshmi Ramasubramanian

On kexec file load Integrity Measurement Architecture (IMA) subsystem may verify the IMA signature of the kernel and initramfs, and measure it. The command line parameters passed to the kernel in the kexec call may also be measured by IMA. A remote attestation service can verify a TPM quote based on the TPM event log, the IMA measurement list, and the TPM PCR data. This can be achieved only if the IMA measurement log is carried over from the current kernel to the next kernel across the kexec call.

powerpc already supports carrying forward the IMA measurement log on kexec. This patch set adds support for carrying forward the IMA measurement log on kexec on ARM64.

This patch set moves the platform independent code defined for powerpc such that it can be reused for other platforms as well. A chosen node "linux,ima-kexec-buffer" is added to the DTB for ARM64 to hold the address and the size of the memory reserved to carry the IMA measurement log.

This patch set has been tested for ARM64 platform using QEMU. I would like help from the community for testing this change on powerpc. Thanks.

Quoted 4 lines

This patch set is based on
commit 96acc833dec8 ("ima: Free IMA measurement buffer after kexec syscall") in https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
"next-integrity" branch.

Is that a hard dependency still? Given this is now almost entirely deleting arch code and adding drivers/of/ code, I was going to apply it.

Quoted 24 lines · Rob Herring

Quoted 23 lines · Lakshmi Ramasubramanian

On kexec file load Integrity Measurement Architecture (IMA) subsystem may verify the IMA signature of the kernel and initramfs, and measure it. The command line parameters passed to the kernel in the kexec call may also be measured by IMA. A remote attestation service can verify a TPM quote based on the TPM event log, the IMA measurement list, and the TPM PCR data. This can be achieved only if the IMA measurement log is carried over from the current kernel to the next kernel across the kexec call.

powerpc already supports carrying forward the IMA measurement log on kexec. This patch set adds support for carrying forward the IMA measurement log on kexec on ARM64.

This patch set moves the platform independent code defined for powerpc such that it can be reused for other platforms as well. A chosen node "linux,ima-kexec-buffer" is added to the DTB for ARM64 to hold the address and the size of the memory reserved to carry the IMA measurement log.

This patch set has been tested for ARM64 platform using QEMU. I would like help from the community for testing this change on powerpc. Thanks.

Quoted 8 lines

Quoted 4 lines

This patch set is based on
commit 96acc833dec8 ("ima: Free IMA measurement buffer after kexec syscall") in https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
"next-integrity" branch.

Is that a hard dependency still? Given this is now almost entirely deleting arch code and adding drivers/of/ code, I was going to apply it.

I tried applying the patches in Linus' mainline branch - PATCH #5 0005-powerpc-Move-ima-buffer-fields-to-struct-kimage.patch doesn't apply.

But if I apply the dependent patch set (link given below), all the patches in this patch set apply fine.

https://patchwork.kernel.org/project/linux-integrity/patch/[email protected]/

Quoted 42 lines · Lakshmi Ramasubramanian

Quoted 24 lines · Rob Herring

Quoted 23 lines · Lakshmi Ramasubramanian

On kexec file load Integrity Measurement Architecture (IMA) subsystem may verify the IMA signature of the kernel and initramfs, and measure it. The command line parameters passed to the kernel in the kexec call may also be measured by IMA. A remote attestation service can verify a TPM quote based on the TPM event log, the IMA measurement list, and the TPM PCR data. This can be achieved only if the IMA measurement log is carried over from the current kernel to the next kernel across the kexec call.

powerpc already supports carrying forward the IMA measurement log on kexec. This patch set adds support for carrying forward the IMA measurement log on kexec on ARM64.

This patch set moves the platform independent code defined for powerpc such that it can be reused for other platforms as well. A chosen node "linux,ima-kexec-buffer" is added to the DTB for ARM64 to hold the address and the size of the memory reserved to carry the IMA measurement log.

This patch set has been tested for ARM64 platform using QEMU. I would like help from the community for testing this change on powerpc. Thanks.

Quoted 8 lines

Quoted 4 lines

This patch set is based on
commit 96acc833dec8 ("ima: Free IMA measurement buffer after kexec syscall") in https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
"next-integrity" branch.

Is that a hard dependency still? Given this is now almost entirely deleting arch code and adding drivers/of/ code, I was going to apply it.

I tried applying the patches in Linus' mainline branch - PATCH #5 0005-powerpc-Move-ima-buffer-fields-to-struct-kimage.patch doesn't apply.

But if I apply the dependent patch set (link given below), all the patches in this patch set apply fine.

Quoted 1 line

https://patchwork.kernel.org/project/linux-integrity/patch/[email protected]/

Ideally, we don't apply the same patch in 2 branches. It looks like there's a conflict but no real dependence on the above patch (the ima_buffer part). The conflict seems trivial enough that Linus can resolve it in the merge window.

Or Mimi can take the whole thing if preferred?

Quoted 7 lines · Rob Herring

On Wed, Feb 10, 2021 at 11:33 AM Lakshmi Ramasubramanian

Ideally, we don't apply the same patch in 2 branches. It looks like there's a conflict but no real dependence on the above patch (the ima_buffer part). The conflict seems trivial enough that Linus can resolve it in the merge window.

Quoted 1 line

Or Mimi can take the whole thing if preferred?

How about I create a topic branch with just the two patches, allowing
both of us to merge it? There shouldn't be a problem with re-writing
next-integrity history.

Quoted 8 lines · Mimi Zohar

Quoted 7 lines · Rob Herring

On Wed, Feb 10, 2021 at 11:33 AM Lakshmi Ramasubramanian

Ideally, we don't apply the same patch in 2 branches. It looks like there's a conflict but no real dependence on the above patch (the ima_buffer part). The conflict seems trivial enough that Linus can resolve it in the merge window.

Quoted 5 lines

Quoted 1 line

Or Mimi can take the whole thing if preferred?

How about I create a topic branch with just the two patches, allowing
both of us to merge it? There shouldn't be a problem with re-writing
next-integrity history.

The 2 patches are now in the ima-kexec-fixes branch.