Carry forward IMA measurement log on kexec on ARM64
Subsystems (8)
v16 cover Carry forward IMA measurement log on kexec on ARM64
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 b3f82afc1041 ("IMA: Measure kernel version in early boot") in https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
"next-integrity" branch.
Changelog:
v16
- Defined functions to allocate and free buffer for FDT for powerpc
and arm64.
- Moved ima_buffer_addr and ima_buffer_size fields from "struct kimage_arch" in powerpc to "struct kimage"
v15
- Included Rob's patches in the patch set, and rebased the changes to "next-integrity" branch.
- Allocate memory for DTB, on arm64, using kmalloc() instead of vmalloc() to keep it consistent with powerpc implementation.
- Call of_kexec_setup_new_fdt() from setup_new_fdt_ppc64() and remove setup_new_fdt() in the same patch to keep it bisect safe.
v14
- Select CONFIG_HAVE_IMA_KEXEC for CONFIG_KEXEC_FILE, for powerpc and arm64, if CONFIG_IMA is enabled.
- Use IS_ENABLED() macro instead of "#ifdef" in remove_ima_buffer(), ima_get_kexec_buffer(), and ima_free_kexec_buffer().
- Call of_kexec_setup_new_fdt() from setup_new_fdt_ppc64() and remove setup_new_fdt() in "arch/powerpc/kexec/file_load.c".
v13
- Moved the arch independent functions to drivers/of/kexec.c and then refactored the code.
- Moved arch_ima_add_kexec_buffer() to security/integrity/ima/ima_kexec.c
v12
- Use fdt_appendprop_addrrange() in setup_ima_buffer() to setup the IMA measurement list property in the device tree.
- Moved architecture independent functions from "arch/powerpc/kexec/ima.c" to "drivers/of/kexec."
- Deleted "arch/powerpc/kexec/ima.c" and "arch/powerpc/include/asm/ima.h".
v11
- Rebased the changes on the kexec code refactoring done by Rob Herring in his "dt/kexec" branch
- Removed "extern" keyword in function declarations
- Removed unnecessary header files included in C files
- Updated patch descriptions per Thiago's comments
v10
- Moved delete_fdt_mem_rsv(), remove_ima_buffer(), get_ima_kexec_buffer, and get_root_addr_size_cells() to drivers/of/kexec.c
- Moved arch_ima_add_kexec_buffer() to security/integrity/ima/ima_kexec.c
- Conditionally define IMA buffer fields in struct kimage_arch
v9
- Moved delete_fdt_mem_rsv() to drivers/of/kexec_fdt.c
- Defined a new function get_ima_kexec_buffer() in drivers/of/ima_kexec.c to replace do_get_kexec_buffer()
- Changed remove_ima_kexec_buffer() to the original function name remove_ima_buffer()
- Moved remove_ima_buffer() to drivers/of/ima_kexec.c
- Moved ima_get_kexec_buffer() and ima_free_kexec_buffer() to security/integrity/ima/ima_kexec.c
v8:
- Moved remove_ima_kexec_buffer(), do_get_kexec_buffer(), and delete_fdt_mem_rsv() to drivers/of/fdt.c
- Moved ima_dump_measurement_list() and ima_add_kexec_buffer() back to security/integrity/ima/ima_kexec.c
v7:
- Renamed remove_ima_buffer() to remove_ima_kexec_buffer() and moved this function definition to kernel.
- Moved delete_fdt_mem_rsv() definition to kernel
- Moved ima_dump_measurement_list() and ima_add_kexec_buffer() to a new file namely ima_kexec_fdt.c in IMA
v6:
- Remove any existing FDT_PROP_IMA_KEXEC_BUFFER property in the device tree and also its corresponding memory reservation in the currently running kernel.
- Moved the function remove_ima_buffer() defined for powerpc to IMA and renamed the function to ima_remove_kexec_buffer(). Also, moved delete_fdt_mem_rsv() from powerpc to IMA.
v5:
- Merged get_addr_size_cells() and do_get_kexec_buffer() into a single function when moving the arch independent code from powerpc to IMA
- Reverted the change to use FDT functions in powerpc code and added back the original code in get_addr_size_cells() and do_get_kexec_buffer() for powerpc.
- Added fdt_add_mem_rsv() for ARM64 to reserve the memory for the IMA log buffer during kexec.
- Fixed the warning reported by kernel test bot for ARM64 arch_ima_add_kexec_buffer() - moved this function to a new file namely arch/arm64/kernel/ima_kexec.c
v4:
- Submitting the patch series on behalf of the original author Prakhar Srivastava <[email protected]>
- Moved FDT_PROP_IMA_KEXEC_BUFFER ("linux,ima-kexec-buffer") to libfdt.h so that it can be shared by multiple platforms.
v3:
Breakup patches further into separate patches.
- Refactoring non architecture specific code out of powerpc
- Update powerpc related code to use fdt functions
- Update IMA buffer read related code to use of functions
- Add support to store the memory information of the IMA measurement logs to be carried forward.
- Update the property strings to align with documented nodes
https://github.com/devicetree-org/dt-schema/pull/46
v2:
Break patches into separate patches.
- Powerpc related Refactoring
- Updating the docuemntation for chosen node
- Updating arm64 to support IMA buffer pass
v1:
Refactoring carrying over IMA measuremnet logs over Kexec. This patch
moves the non-architecture specific code out of powerpc and adds to security/ima.(Suggested by Thiago)
Add Documentation regarding the ima-kexec-buffer node in the chosen
node documentation
v0:
Add a layer of abstraction to use the memory reserved by device tree
for ima buffer pass.
Add support for ima buffer pass using reserved memory for arm64 kexec.
Update the arch sepcific code path in kexec file load to store the ima buffer in the reserved memory. The same reserved memory is read on kexec or cold boot.
Lakshmi Ramasubramanian (8):
powerpc: Move ima buffer fields to struct kimage
powerpc: Move arch independent ima kexec functions to
drivers/of/kexec.c
kexec: Use fdt_appendprop_addrrange() to add ima buffer to FDT
powerpc: Delete unused function delete_fdt_mem_rsv()
of: Define functions to allocate and free FDT
arm64: Use OF alloc and free functions for FDT
powerpc: Use OF alloc and free for FDT
arm64: Enable passing IMA log to next kernel on kexec
Rob Herring (4):
powerpc: Rename kexec elfcorehdr_addr to elf_headers_mem
of: Add a common kexec FDT setup function
arm64: Use common of_kexec_setup_new_fdt()
powerpc: Use common of_kexec_setup_new_fdt()
arch/arm64/Kconfig | 1 + arch/arm64/kernel/machine_kexec_file.c | 158 +------- arch/powerpc/Kconfig | 2 +- arch/powerpc/include/asm/ima.h | 30 -- arch/powerpc/include/asm/kexec.h | 11 +- arch/powerpc/kexec/Makefile | 7 - arch/powerpc/kexec/elf_64.c | 26 +- arch/powerpc/kexec/file_load.c | 184 +--------- arch/powerpc/kexec/file_load_64.c | 11 +- arch/powerpc/kexec/ima.c | 219 ----------- drivers/of/Makefile | 1 + drivers/of/kexec.c | 488 +++++++++++++++++++++++++ include/linux/kexec.h | 5 + include/linux/of.h | 15 +- security/integrity/ima/ima.h | 4 - security/integrity/ima/ima_kexec.c | 3 +- 16 files changed, 552 insertions(+), 613 deletions(-) delete mode 100644 arch/powerpc/include/asm/ima.h delete mode 100644 arch/powerpc/kexec/ima.c create mode 100644 drivers/of/kexec.c
Signature · 1 line
2.30.0
PATCH v16 1/12 powerpc: Rename kexec elfcorehdr_addr to elf_headers_mem
From: Rob Herring <[email protected]>
The architecture specific field, elfcorehdr_addr in struct kimage_arch, that holds the address of the buffer in memory for ELF core header for powerpc has a different name than the one used for arm64. This makes it hard to have a common code for setting up the device tree for kexec system call.
Rename elfcorehdr_addr to elf_headers_mem to align with arm64 name so common code can use it.
--- arch/powerpc/include/asm/kexec.h | 2 +- arch/powerpc/kexec/file_load.c | 4 ++-- arch/powerpc/kexec/file_load_64.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-)
arch/powerpc/include/asm/kexec.h +1 -1
@@ -108,7 +108,7 @@ struct kimage_arch {unsigned long backup_start;void *backup_buf; -unsigned long elfcorehdr_addr; +unsigned long elf_headers_mem;unsigned long elf_headers_sz;void *elf_headers;
arch/powerpc/kexec/file_load.c +2 -2
@@ -45,7 +45,7 @@ char *setup_kdump_cmdline(struct kimage *image, char *cmdline,return NULL ; elfcorehdr_strlen =sprintf (cmdline_ptr,"elfcorehdr=0x%lx " , - image->arch.elfcorehdr_addr); + image->arch.elf_headers_mem);if (elfcorehdr_strlen + cmdline_len >COMMAND_LINE_SIZE ) {pr_err ("Appending elfcorehdr=<addr> exceeds cmdline size\n" );
@@ -263,7 +263,7 @@ int setup_new_fdt(const struct kimage *image, void *fdt,* Avoid elfcorehdr from being stomped on in kdump kernel by * setting up memory reserve map. */ - ret =fdt_add_mem_rsv (fdt, image->arch.elfcorehdr_addr, + ret =fdt_add_mem_rsv (fdt, image->arch.elf_headers_mem, image->arch.elf_headers_sz);if (ret) { pr_err("Error reserving elfcorehdr memory: %s\n",
arch/powerpc/kexec/file_load_64.c +2 -2
@@ -815,7 +815,7 @@ static int load_elfcorehdr_segment(struct kimage *image, struct kexec_buf *kbuf) goto out; } - image->arch.elfcorehdr_addr = kbuf->mem; + image->arch.elf_headers_mem = kbuf->mem; image->arch.elf_headers_sz = headers_sz; image->arch.elf_headers = headers; out:
@@ -851,7 +851,7 @@ int load_crashdump_segments_ppc64(struct kimage *image,return ret; }pr_debug ("Loaded elf core header at 0x%lx, bufsz=0x%lx memsz=0x%lx\n" , - image->arch.elfcorehdr_addr, kbuf->bufsz, kbuf->memsz); + image->arch.elf_headers_mem, kbuf->bufsz, kbuf->memsz);return 0 ; }
Signature · 1 line
2.30.0
PATCH v16 2/12 of: Add a common kexec FDT setup function
In reply to Lakshmi RamasubramanianFrom: 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_setup_new_fdt() and move it to "drivers/of/kexec.c".
--- drivers/of/Makefile | 1 + drivers/of/kexec.c | 236 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/of.h | 4 + 3 files changed, 241 insertions(+) create mode 100644 drivers/of/kexec.c
drivers/of/Makefile +1 -0
@@ -13,5 +13,6 @@ obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o obj-$(CONFIG_OF_RESOLVE) += resolver.o obj-$(CONFIG_OF_OVERLAY) += overlay.o obj-$(CONFIG_OF_NUMA) += of_numa.o +obj-$(CONFIG_KEXEC_FILE) += kexec.o obj-$(CONFIG_OF_UNITTEST) += unittest-data/
drivers/of/kexec.c new file +236 -0
236 lines not on this page · raw message
include/linux/of.h +4 -0
@@ -559,6 +559,10 @@ int of_map_id(struct device_node *np, u32 id, struct device_node **target, u32 *id_out);phys_addr_t of_dma_get_max_cpu_address (struct device_node *np); +struct kimage ; +int of_kexec_setup_new_fdt (const struct kimage *image,void *fdt, +unsigned long initrd_load_addr,unsigned long initrd_len, +const char *cmdline);#else /* CONFIG_OF */
Signature · 1 line
2.30.0
kernel test robot
build botRe: [PATCH v16 02/12] of: Add a common kexec FDT setup function
I love your patch! Yet something to improve:
Hi Lakshmi,
I love your patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on robh/for-next arm64/for-next/core v5.11-rc6 next-20210125] [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/20210205-010018 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next config: x86_64-randconfig-a006-20210205 (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/029dfec178c52eaec7d685d81c9b0a08da337d66 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/20210205-010018 git checkout 029dfec178c52eaec7d685d81c9b0a08da337d66 # 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
All errors (new ones prefixed by >>):
In file included from arch/x86/include/asm/user_64.h:6,
from arch/x86/include/asm/user.h:8,
from include/linux/user.h:1,
from include/linux/elfcore.h:5,
from include/linux/crash_core.h:6,
from include/linux/kexec.h:18,
from drivers/of/kexec.c:13:
drivers/of/kexec.c: In function 'of_kexec_setup_new_fdt':
>> drivers/of/kexec.c:86:43: error: 'initial_boot_params' undeclared (first use in this function)
86 | ret = fdt_find_and_del_mem_rsv(fdt, __pa(initial_boot_params),
| ^~~~~~~~~~~~~~~~~~~
arch/x86/include/asm/page.h:42:46: note: in definition of macro '__pa'
42 | #define __pa(x) __phys_addr((unsigned long)(x))
| ^
drivers/of/kexec.c:86:43: note: each undeclared identifier is reported only once for each function it appears in
86 | ret = fdt_find_and_del_mem_rsv(fdt, __pa(initial_boot_params),
| ^~~~~~~~~~~~~~~~~~~
arch/x86/include/asm/page.h:42:46: note: in definition of macro '__pa'
42 | #define __pa(x) __phys_addr((unsigned long)(x))
| ^
drivers/of/kexec.c:163:17: error: 'const struct kimage_arch' has no member named 'elf_headers_mem'; did you mean 'elf_headers_sz'?
163 | image->arch.elf_headers_mem,
| ^~~~~~~~~~~~~~~
| elf_headers_sz
drivers/of/kexec.c:172:42: error: 'const struct kimage_arch' has no member named 'elf_headers_mem'; did you mean 'elf_headers_sz'?
172 | ret = fdt_add_mem_rsv(fdt, image->arch.elf_headers_mem,
| ^~~~~~~~~~~~~~~
| elf_headers_szvim +/initial_boot_params +86 drivers/of/kexec.c
65
66 /*
67 * of_kexec_setup_new_fdt - modify /chosen and memory reservation for the next kernel
68 *
69 * @image: kexec image being loaded.
70 * @fdt: Flattened device tree for the next kernel.
71 * @initrd_load_addr: Address where the next initrd will be loaded.
72 * @initrd_len: Size of the next initrd, or 0 if there will be none.
73 * @cmdline: Command line for the next kernel, or NULL if there will
74 * be none.
75 *
76 * Return: 0 on success, or negative errno on error.
77 */
78 int of_kexec_setup_new_fdt(const struct kimage *image, void *fdt,
79 unsigned long initrd_load_addr, unsigned long initrd_len,
80 const char *cmdline)
81 {
82 int ret, chosen_node;
83 const void *prop;
84
85 /* Remove memory reservation for the current device tree. */
> 86 ret = fdt_find_and_del_mem_rsv(fdt, __pa(initial_boot_params),---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
PATCH v16 3/12 arm64: Use common of_kexec_setup_new_fdt()
In reply to Lakshmi RamasubramanianFrom: 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_setup_new_fdt() defined in "drivers/of/kexec.c".
Use the common of_kexec_setup_new_fdt() to setup the device tree and update the memory reservation for kexec for arm64.
--- arch/arm64/kernel/machine_kexec_file.c | 123 +------------------------ 1 file changed, 3 insertions(+), 120 deletions(-)
arch/arm64/kernel/machine_kexec_file.c +3 -120
@@ -15,23 +15,12 @@#include <linux/kexec.h> #include <linux/libfdt.h> #include <linux/memblock.h> +#include <linux/of.h> #include <linux/of_fdt.h> -#include <linux/random.h> #include <linux/slab.h> #include <linux/string.h> #include <linux/types.h> #include <linux/vmalloc.h> -#include <asm/byteorder.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 128const struct kexec_file_ops *const kexec_file_loaders[] = { &kexec_image_ops,
@@ -50,112 +39,6 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)return kexec_image_post_load_cleanup_default (image); } -static int setup_dtb (struct kimage *image, -unsigned long initrd_load_addr,unsigned long initrd_len, -char *cmdline,void *dtb) -{ -int off, ret; - - ret =fdt_path_offset (dtb,"/chosen" ); -if (ret <0 ) - goto out; - - off = ret; - - ret =fdt_delprop (dtb, off,FDT_PROP_KEXEC_ELFHDR ); -if (ret && ret != -FDT_ERR_NOTFOUND ) - goto out; - ret =fdt_delprop (dtb, off,FDT_PROP_MEM_RANGE ); -if (ret && ret != -FDT_ERR_NOTFOUND ) - goto out; - -if (image->type ==KEXEC_TYPE_CRASH ) { -/* add linux,elfcorehdr */ - ret =fdt_appendprop_addrrange (dtb,0 , off, -FDT_PROP_KEXEC_ELFHDR , - image->arch.elf_headers_mem, - image->arch.elf_headers_sz); -if (ret) -return (ret == -FDT_ERR_NOSPACE ? -ENOMEM : -EINVAL ); - -/* add linux,usable-memory-range */ - ret =fdt_appendprop_addrrange (dtb,0 , off, -FDT_PROP_MEM_RANGE , - crashk_res.start, - crashk_res.end - crashk_res.start +1 ); -if (ret) -return (ret == -FDT_ERR_NOSPACE ? -ENOMEM : -EINVAL ); - } - -/* add bootargs */ -if (cmdline) { - ret =fdt_setprop_string (dtb, off,FDT_PROP_BOOTARGS , cmdline); -if (ret) - goto out; - }else { - ret =fdt_delprop (dtb, off,FDT_PROP_BOOTARGS ); -if (ret && (ret != -FDT_ERR_NOTFOUND )) - goto out; - } - -/* add initrd-* */ -if (initrd_load_addr) { - ret =fdt_setprop_u64 (dtb, off,FDT_PROP_INITRD_START , - initrd_load_addr); -if (ret) - goto out; - - ret =fdt_setprop_u64 (dtb, off,FDT_PROP_INITRD_END , - initrd_load_addr + initrd_len); -if (ret) - goto out; - }else { - ret =fdt_delprop (dtb, off,FDT_PROP_INITRD_START ); -if (ret && (ret != -FDT_ERR_NOTFOUND )) - goto out; - - ret =fdt_delprop (dtb, off,FDT_PROP_INITRD_END ); -if (ret && (ret != -FDT_ERR_NOTFOUND )) - goto out; - } - -/* add kaslr-seed */ - ret =fdt_delprop (dtb, off,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 (dtb, off,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 (dtb, off,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 ); - } - -out: -if (ret) -return (ret == -FDT_ERR_NOSPACE ) ? -ENOMEM : -EINVAL ; - -return 0 ; -} -/* * More space needed so that we can add initrd, bootargs, kaslr-seed, * rng-seed, userable-memory-range and elfcorehdr.
@@ -185,8 +68,8 @@ static int create_dtb(struct kimage *image,if (ret)return -EINVAL ; - ret =setup_dtb (image, initrd_load_addr, initrd_len, - cmdline, buf); + ret =of_kexec_setup_new_fdt (image, buf, initrd_load_addr, + initrd_len, cmdline);if (ret) {vfree (buf);if (ret == -ENOMEM ) {
Signature · 1 line
2.30.0
PATCH v16 4/12 powerpc: Use common of_kexec_setup_new_fdt()
In reply to Lakshmi RamasubramanianFrom: 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_setup_new_fdt() defined in "drivers/of/kexec.c".
Use the common of_kexec_setup_new_fdt() to setup the device tree and update the memory reservation for kexec for powerpc.
--- arch/powerpc/kexec/file_load.c | 125 ++------------------------------- 1 file changed, 6 insertions(+), 119 deletions(-)
arch/powerpc/kexec/file_load.c +6 -119
@@ -16,6 +16,7 @@#include <linux/slab.h> #include <linux/kexec.h> +#include <linux/of.h> #include <linux/of_fdt.h> #include <linux/libfdt.h> #include <asm/setup.h>
@@ -156,132 +157,18 @@ int setup_new_fdt(const struct kimage *image, void *fdt, unsigned long initrd_load_addr, unsigned long initrd_len, const char *cmdline) { - int ret, chosen_node; -const void *prop; - -/* Remove memory reservation for the current device tree. */ - ret =delete_fdt_mem_rsv (fdt,__pa (initial_boot_params), -fdt_totalsize (initial_boot_params)); -if (ret ==0 ) -pr_debug ("Removed old device tree reservation.\n" ); -else if (ret != -ENOENT ) -return ret; - - 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 ) { -pr_err ("Error creating /chosen.\n" ); -return -EINVAL ; - } - }else if (chosen_node <0 ) { -pr_err ("Malformed device tree: error reading /chosen.\n" ); -return -EINVAL ; - } - -/* Did we boot using an initrd? */ - prop =fdt_getprop (fdt, chosen_node,"linux,initrd-start" ,NULL ); -if (prop) { -uint64_t 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) { -pr_err ("Malformed device tree.\n" ); -return -EINVAL ; - } - 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 =delete_fdt_mem_rsv (fdt, tmp_start, tmp_size); -if (ret == -ENOENT ) - ret =delete_fdt_mem_rsv (fdt, tmp_start, -round_up (tmp_size,PAGE_SIZE )); -if (ret ==0 ) -pr_debug ("Removed old initrd reservation.\n" ); -else if (ret != -ENOENT ) -return ret; - -/* If there's no new initrd, delete the old initrd's info. */ -if (initrd_len ==0 ) { - ret =fdt_delprop (fdt, chosen_node, -"linux,initrd-start" ); -if (ret) { -pr_err ("Error deleting linux,initrd-start.\n" ); -return -EINVAL ; - } - - ret =fdt_delprop (fdt, chosen_node,"linux,initrd-end" ); -if (ret) { -pr_err ("Error deleting linux,initrd-end.\n" ); -return -EINVAL ; - } - } - } - -if (initrd_len) { - ret =fdt_setprop_u64 (fdt, chosen_node, -"linux,initrd-start" , - initrd_load_addr); -if (ret <0 ) - goto err; - -/* initrd-end is the first address after the initrd image. */ - ret =fdt_setprop_u64 (fdt, chosen_node,"linux,initrd-end" , - initrd_load_addr + initrd_len); -if (ret <0 ) - goto err; - - ret =fdt_add_mem_rsv (fdt, initrd_load_addr, initrd_len); -if (ret) { -pr_err ("Error reserving initrd memory: %s\n" , -fdt_strerror (ret)); -return -EINVAL ; - } - } - -if (cmdline !=NULL ) { - ret =fdt_setprop_string (fdt, chosen_node,"bootargs" , cmdline); -if (ret <0 ) - goto err; - }else { - ret =fdt_delprop (fdt, chosen_node,"bootargs" ); -if (ret && ret != -FDT_ERR_NOTFOUND ) { -pr_err ("Error deleting bootargs.\n" ); -return -EINVAL ; - } - } + int ret; -if (image->type ==KEXEC_TYPE_CRASH ) { -/* -* 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) { -pr_err ("Error reserving elfcorehdr memory: %s\n" , -fdt_strerror (ret)); - goto err; - } - } + ret =of_kexec_setup_new_fdt (image, fdt, initrd_load_addr, initrd_len, cmdline); +if (ret) + goto err; - ret =setup_ima_buffer (image, fdt, chosen_node); + ret =setup_ima_buffer (image, fdt,fdt_path_offset (fdt,"/chosen" ));if (ret) {pr_err ("Error setting up the new device tree.\n" );return ret; } - ret =fdt_setprop (fdt, chosen_node,"linux,booted-from-kexec" ,NULL ,0 ); -if (ret) - goto err; -return 0 ; err:
Signature · 7 lines
2.30.0 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v16 5/12 powerpc: Move ima buffer fields to struct kimage
In reply to Lakshmi RamasubramanianThe 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.
--- 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 | 5 +++++ include/linux/of.h | 5 +++++ security/integrity/ima/ima_kexec.c | 3 ++- 7 files changed, 41 insertions(+), 32 deletions(-)
arch/powerpc/include/asm/ima.h +0 -3
@@ -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
@@ -111,11 +111,6 @@ struct kimage_arch {unsigned long elf_headers_mem;unsigned long elf_headers_sz;void *elf_headers; - -#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
@@ -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 +23 -0
@@ -63,6 +63,29 @@ static int fdt_find_and_del_mem_rsv(void *fdt, unsigned long start, unsigned lonreturn -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, +unsigned long load_addr,size_t size) +{ + image->ima_buffer_addr = load_addr; + image->ima_buffer_size = size; + +return 0 ; +} +#endif /* CONFIG_IMA_KEXEC */ +/* * of_kexec_setup_new_fdt - modify /chosen and memory reservation for the next kernel *
include/linux/kexec.h +5 -0
@@ -300,6 +300,11 @@ struct kimage {/* Information for loading purgatory */ struct purgatory_info purgatory_info;#endif + +#ifdef CONFIG_IMA_KEXEC +phys_addr_t ima_buffer_addr; +size_t ima_buffer_size; +#endif };/* kexec interface functions */
include/linux/of.h +5 -0
@@ -564,6 +564,11 @@ int of_kexec_setup_new_fdt(const struct kimage *image, void *fdt, unsigned long initrd_load_addr, unsigned long initrd_len, const char *cmdline); +#ifdef CONFIG_IMA_KEXEC +int of_ima_add_kexec_buffer (struct kimage *image, +unsigned long load_addr,size_t size); +#endif /* CONFIG_IMA_KEXEC */ +#else /* CONFIG_OF */ static inline void of_core_init (void )
security/integrity/ima/ima_kexec.c +2 -1
@@ -10,6 +10,7 @@#include <linux/seq_file.h> #include <linux/vmalloc.h> #include <linux/kexec.h> +#include <linux/of.h> #include "ima.h" #ifdef CONFIG_IMA_KEXEC
@@ -122,7 +123,7 @@ void ima_add_kexec_buffer(struct kimage *image)return ; } - ret =arch_ima_add_kexec_buffer (image, kbuf.mem, kexec_segment_size); + ret =of_ima_add_kexec_buffer (image, kbuf.mem, kexec_segment_size);if (ret) {pr_err ("Error passing over kexec measurement buffer.\n" );return ;
Signature · 7 lines
2.30.0 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v16 6/12 powerpc: Move arch independent ima kexec functions to drivers/of/kexec.c
In reply to Lakshmi RamasubramanianThe 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 setup_ima_buffer() call into of_kexec_setup_new_fdt() defined in "drivers/of/kexec.c". Call of_kexec_setup_new_fdt() from setup_new_fdt_ppc64() and remove setup_new_fdt() in "arch/powerpc/kexec/file_load.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 in powerpc and in ima.
--- arch/powerpc/Kconfig | 2 +- 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 | 35 ----- 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 - 10 files changed, 245 insertions(+), 280 deletions(-) delete mode 100644 arch/powerpc/include/asm/ima.h delete mode 100644 arch/powerpc/kexec/ima.c
arch/powerpc/Kconfig +1 -1
@@ -554,7 +554,7 @@ config KEXEC config KEXEC_FILE bool "kexec file based system call" select KEXEC_CORE - select HAVE_IMA_KEXEC + select HAVE_IMA_KEXEC if IMA select BUILD_BIN2C select KEXEC_ELF depends on PPC64
arch/powerpc/include/asm/ima.h deleted +0 -27
@@ -1,27 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -#ifndef _ASM_POWERPC_IMA_H -#define _ASM_POWERPC_IMA_H - -struct kimage ; - -int ima_get_kexec_buffer (void **addr,size_t *size); -int ima_free_kexec_buffer (void ); - -#ifdef CONFIG_IMA -void remove_ima_buffer (void *fdt,int chosen_node); -#else -static inline void remove_ima_buffer (void *fdt,int chosen_node) {} -#endif - -#ifdef CONFIG_IMA_KEXEC -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, -int chosen_node) -{ -remove_ima_buffer (fdt, chosen_node); -return 0 ; -} -#endif /* CONFIG_IMA_KEXEC */ - -#endif /* _ASM_POWERPC_IMA_H */
arch/powerpc/include/asm/kexec.h +0 -3
@@ -118,9 +118,6 @@ char *setup_kdump_cmdline(struct kimage *image, char *cmdline,int setup_purgatory (struct kimage *image,const void *slave_code,const void *fdt,unsigned long kernel_load_addr,unsigned long fdt_load_addr); -int setup_new_fdt (const struct kimage *image,void *fdt, -unsigned long initrd_load_addr,unsigned long initrd_len, -const char *cmdline);int delete_fdt_mem_rsv (void *fdt,unsigned long start,unsigned long size);#ifdef CONFIG_PPC64
arch/powerpc/kexec/Makefile +0 -7
@@ -9,13 +9,6 @@ obj-$(CONFIG_PPC32) += relocate_32.o obj-$(CONFIG_KEXEC_FILE) += file_load.o ranges.o file_load_$(BITS).o elf_$(BITS).o -ifdef CONFIG_HAVE_IMA_KEXEC -ifdef CONFIG_IMA -obj-y += ima.o -endif -endif - - # Disable GCOV, KCOV & sanitizers in odd or sensitive code GCOV_PROFILE_core_$(BITS).o := n KCOV_INSTRUMENT_core_$(BITS).o := n
arch/powerpc/kexec/file_load.c +0 -35
@@ -20,7 +20,6 @@#include <linux/of_fdt.h> #include <linux/libfdt.h> #include <asm/setup.h> -#include <asm/ima.h> #define SLAVE_CODE_SIZE 256/* First 0x100 bytes */
@@ -141,37 +140,3 @@ int delete_fdt_mem_rsv(void *fdt, unsigned long start, unsigned long size)return -ENOENT ; } - -/* -* setup_new_fdt - modify /chosen and memory reservation for the next kernel -* @image: kexec image being loaded. -* @fdt: Flattened device tree for the next kernel. -* @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: 0 on success, or negative errno on error. -*/ -int setup_new_fdt (const struct kimage *image,void *fdt, -unsigned long initrd_load_addr,unsigned long initrd_len, -const char *cmdline) -{ -int ret; - - ret =of_kexec_setup_new_fdt (image, fdt, initrd_load_addr, initrd_len, cmdline); -if (ret) - goto err; - - ret =setup_ima_buffer (image, fdt,fdt_path_offset (fdt,"/chosen" )); -if (ret) { -pr_err ("Error setting up the new device tree.\n" ); -return ret; - } - -return 0 ; - -err: -pr_err ("Error setting up the new device tree.\n" ); -return -EINVAL ; -}
arch/powerpc/kexec/file_load_64.c +3 -1
@@ -17,6 +17,7 @@#include <linux/kexec.h> #include <linux/of_fdt.h> #include <linux/libfdt.h> +#include <linux/of.h> #include <linux/of_device.h> #include <linux/memblock.h> #include <linux/slab.h>
@@ -944,7 +945,8 @@ int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,struct crash_mem *umem =NULL , *rmem =NULL ;int i, nr_ranges, ret; - ret =setup_new_fdt (image, fdt, initrd_load_addr, initrd_len, cmdline); + ret =of_kexec_setup_new_fdt (image, fdt, initrd_load_addr, initrd_len, + cmdline);if (ret) goto out;
arch/powerpc/kexec/ima.c deleted +0 -202
202 lines not on this page · raw message
drivers/of/kexec.c +239 -0
263 lines not on this page · raw message
include/linux/of.h +2 -0
@@ -569,6 +569,8 @@ int of_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr, size_t size);#endif /* CONFIG_IMA_KEXEC */ +int ima_get_kexec_buffer (void **addr,size_t *size); +int ima_free_kexec_buffer (void );#else /* CONFIG_OF */ static inline void of_core_init (void )
security/integrity/ima/ima.h +0 -4
@@ -24,10 +24,6 @@#include "../integrity.h" -#ifdef CONFIG_HAVE_IMA_KEXEC -#include <asm/ima.h> -#endif -enum ima_show_type {IMA_SHOW_BINARY ,IMA_SHOW_BINARY_NO_FIELD_LEN ,IMA_SHOW_BINARY_OLD_STRING_FMT ,IMA_SHOW_ASCII };enum tpm_pcrs {TPM_PCR0 =0 ,TPM_PCR8 =8 ,TPM_PCR10 =10 };
Signature · 7 lines
2.30.0 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v16 7/12 kexec: Use fdt_appendprop_addrrange() to add ima buffer to FDT
In reply to Lakshmi Ramasubramanianfdt_appendprop_addrrange() function adds a property, with the given name, to the device tree at the given node offset, and also sets the address and size of the property. This function should be used to add "linux,ima-kexec-buffer" property to the device tree and set the address and size of the IMA measurement buffer, instead of using custom function.
Use fdt_appendprop_addrrange() to add "linux,ima-kexec-buffer" property to the device tree. This property holds the address and size of the IMA measurement buffer that needs to be passed from the current kernel to the next kernel across kexec system call.
Remove custom code that is used in setup_ima_buffer() to add "linux,ima-kexec-buffer" property to the device tree.
--- drivers/of/kexec.c | 57 ++++------------------------------------------ 1 file changed, 5 insertions(+), 52 deletions(-)
drivers/of/kexec.c +5 -52
@@ -232,36 +232,6 @@ int of_ima_add_kexec_buffer(struct kimage *image,return 0 ; } -/** -* write_number - Convert number to big-endian format -* -* @p: Buffer to write the number to -* @value: Number to convert -* @cells: Number of cells -* -* Return: 0 on success, or negative errno on error. -*/ -static int write_number (void *p,u64 value,int cells) -{ -if (cells ==1 ) { -u32 tmp; - -if (value >U32_MAX ) -return -EINVAL ; - - tmp =cpu_to_be32 (value); -memcpy (p, &tmp,sizeof (tmp)); - }else if (cells ==2 ) { -u64 tmp; - - tmp =cpu_to_be64 (value); -memcpy (p, &tmp,sizeof (tmp)); - }else -return -EINVAL ; - -return 0 ; -} -/** * setup_ima_buffer - add IMA buffer information to the fdt * @image: kexec image being loaded.
@@ -273,32 +243,15 @@ static int write_number(void *p, u64 value, int cells)static int setup_ima_buffer (const struct kimage *image,void *fdt,int chosen_node) { -int ret, addr_cells, size_cells, entry_size; -u8 value[16 ]; +int ret;if (!image->ima_buffer_size)return 0 ; - ret =get_addr_size_cells (&addr_cells, &size_cells); -if (ret) -return ret; - - entry_size =4 * (addr_cells + size_cells); - -if (entry_size >sizeof (value)) -return -EINVAL ; - - ret =write_number (value, image->ima_buffer_addr, addr_cells); -if (ret) -return ret; - - ret =write_number (value +4 * addr_cells, image->ima_buffer_size, - size_cells); -if (ret) -return ret; - - ret =fdt_setprop (fdt, chosen_node,"linux,ima-kexec-buffer" , value, - entry_size); + ret =fdt_appendprop_addrrange (fdt,0 , chosen_node, +"linux,ima-kexec-buffer" , + image->ima_buffer_addr, + image->ima_buffer_size);if (ret <0 )return -EINVAL ;
Signature · 7 lines
2.30.0 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v16 8/12 powerpc: Delete unused function delete_fdt_mem_rsv()
In reply to Lakshmi Ramasubramaniandelete_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".
--- arch/powerpc/include/asm/kexec.h | 1 - arch/powerpc/kexec/file_load.c | 32 -------------------------------- 2 files changed, 33 deletions(-)
arch/powerpc/include/asm/kexec.h +0 -1
@@ -118,7 +118,6 @@ char *setup_kdump_cmdline(struct kimage *image, char *cmdline,int setup_purgatory (struct kimage *image,const void *slave_code,const void *fdt,unsigned long kernel_load_addr,unsigned long fdt_load_addr); -int delete_fdt_mem_rsv (void *fdt,unsigned long start,unsigned long size);#ifdef CONFIG_PPC64 struct kexec_buf ;
arch/powerpc/kexec/file_load.c +0 -32
@@ -108,35 +108,3 @@ int setup_purgatory(struct kimage *image, const void *slave_code,return 0 ; } - -/** -* delete_fdt_mem_rsv - delete memory reservation with given address and size -* -* Return: 0 on success, or negative errno on error. -*/ -int delete_fdt_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++) { -uint64_t 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 ; -}
Signature · 7 lines
2.30.0 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v16 9/12 of: Define functions to allocate and free FDT
In reply to Lakshmi RamasubramanianKernel components that use Flattened Device Tree (FDT) allocate kernel memory and call fdt_open_into() to reorganize the tree into a form suitable for the read-write operations. These operations can be combined into a single function to allocate and initialize the FDT so the different architecures do not have to duplicate the code.
Define of_alloc_and_init_fdt() and of_free_fdt() in drivers/of/kexec.c to allocate and initialize FDT, and to free the FDT buffer respectively.
--- drivers/of/kexec.c | 37 +++++++++++++++++++++++++++++++++++++ include/linux/of.h | 2 ++ 2 files changed, 39 insertions(+)
drivers/of/kexec.c +37 -0
@@ -11,6 +11,7 @@#include <linux/kernel.h> #include <linux/kexec.h> +#include <linux/mm.h> #include <linux/memblock.h> #include <linux/libfdt.h> #include <linux/of.h>
@@ -28,6 +29,42 @@#define FDT_PROP_RNG_SEED "rng-seed"#define RNG_SEED_SIZE 128 +/** +* of_alloc_and_init_fdt - Allocate and initialize a Flattened device tree +* +* @fdt_size: Flattened device tree size +* +* Return the allocated FDT buffer on success, or NULL on error. +*/ +void *of_alloc_and_init_fdt (unsigned int fdt_size) +{ +void *fdt; +int ret; + + 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 setting up the new device tree.\n" ); +kvfree (fdt); + fdt =NULL ; + } + +return fdt; +} + +/** +* of_free_fdt - Free the buffer for Flattened device tree +* +* @fdt: Flattened device tree buffer to free +*/ +void of_free_fdt (void *fdt) +{ +kvfree (fdt); +} +/** * fdt_find_and_del_mem_rsv - delete memory reservation with given address and size *
include/linux/of.h +2 -0
@@ -563,6 +563,8 @@ struct kimage;int of_kexec_setup_new_fdt (const struct kimage *image,void *fdt,unsigned long initrd_load_addr,unsigned long initrd_len,const char *cmdline); +void *of_alloc_and_init_fdt (unsigned int fdt_size); +void of_free_fdt (void *fdt);#ifdef CONFIG_IMA_KEXEC int of_ima_add_kexec_buffer(struct kimage *image,
Signature · 7 lines
2.30.0 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v16 10/12 arm64: Use OF alloc and free functions for FDT
In reply to Lakshmi Ramasubramanianof_alloc_and_init_fdt() and of_free_fdt() have been defined in drivers/of/kexec.c to allocate and free memory for FDT.
Use of_alloc_and_init_fdt() and of_free_fdt() to allocate and initialize the FDT, and to free the FDT respectively.
--- arch/arm64/kernel/machine_kexec_file.c | 37 +++++++------------------- 1 file changed, 10 insertions(+), 27 deletions(-)
arch/arm64/kernel/machine_kexec_file.c +10 -27
@@ -29,7 +29,7 @@ const struct kexec_file_ops * const kexec_file_loaders[] = {int arch_kimage_file_post_load_cleanup (struct kimage *image) { -vfree (image->arch.dtb); +of_free_fdt (image->arch.dtb); image->arch.dtb =NULL ;vfree (image->arch.elf_headers);
@@ -57,36 +57,19 @@ static int create_dtb(struct kimage *image, cmdline_len = cmdline ?strlen (cmdline) :0 ; buf_size =fdt_totalsize (initial_boot_params) + cmdline_len +DTB_EXTRA_SPACE ; - -for (;;) { - buf =vmalloc (buf_size); -if (!buf) -return -ENOMEM ; - -/* duplicate a device tree blob */ - ret =fdt_open_into (initial_boot_params, buf, buf_size); -if (ret) -return -EINVAL ; - - ret =of_kexec_setup_new_fdt (image, buf, initrd_load_addr, + buf =of_alloc_and_init_fdt (buf_size); +if (!buf) +return -ENOMEM ; + ret =of_kexec_setup_new_fdt (image, buf, initrd_load_addr, initrd_len, cmdline); -if (ret) { -vfree (buf); -if (ret == -ENOMEM ) { -/* unlikely, but just in case */ - buf_size +=DTB_EXTRA_SPACE ; -continue ; - }else { -return ret; - } - } - +if (!ret) {/* trim it */ fdt_pack (buf); *dtb = buf; + }else +of_free_fdt (buf); -return 0 ; - } +return ret; }static int prepare_elf_headers (void **addr,unsigned long *sz)
@@ -224,6 +207,6 @@ int load_other_segments(struct kimage *image, out_err: image->nr_segments = orig_segments; -vfree (dtb); +of_free_fdt (dtb);return ret; }
Signature · 1 line
2.30.0
Re: [PATCH v16 10/12] arm64: Use OF alloc and free functions for FDT
Quoted 11 lines · Lakshmi Ramasubramanian
of_alloc_and_init_fdt()andof_free_fdt()have been defined indrivers/of/kexec.cto allocate and free memory for FDT.Use
of_alloc_and_init_fdt()andof_free_fdt()to allocate and initialize the FDT, and to free the FDT respectively.Signed-off-by Lakshmi Ramasubramanian <[email protected]>Suggested-by Rob Herring <[email protected]>--- arch/arm64/kernel/machine_kexec_file.c | 37 +++++++------------------- 1 file changed, 10 insertions(+), 27 deletions(-)
Will
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v16 11/12 powerpc: Use OF alloc and free for FDT
In reply to Lakshmi Ramasubramanianof_alloc_and_init_fdt() and of_free_fdt() have been defined in drivers/of/kexec.c to allocate and free memory for FDT.
Use of_alloc_and_init_fdt() and of_free_fdt() to allocate and initialize the FDT, and to free the FDT respectively.
powerpc sets the FDT address in image_loader_data field in "struct kimage" and the memory is freed in kimage_file_post_load_cleanup(). This cleanup function uses kfree() to free the memory. But since of_alloc_and_init_fdt() uses kvmalloc() for allocation, the buffer needs to be freed using kvfree().
Define "fdt" field in "struct kimage_arch" for powerpc to store the address of FDT, and free the memory in powerpc specific arch_kimage_file_post_load_cleanup().
--- arch/powerpc/include/asm/kexec.h | 2 ++ arch/powerpc/kexec/elf_64.c | 26 ++++++++++++++++---------- arch/powerpc/kexec/file_load_64.c | 3 +++ 3 files changed, 21 insertions(+), 10 deletions(-)
arch/powerpc/include/asm/kexec.h +2 -0
@@ -111,6 +111,8 @@ struct kimage_arch {unsigned long elf_headers_mem;unsigned long elf_headers_sz;void *elf_headers; + +void *fdt; }; char *setup_kdump_cmdline(struct kimage *image, char *cmdline,
arch/powerpc/kexec/elf_64.c +16 -10
@@ -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>
@@ -32,7 +33,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,unsigned int fdt_size;unsigned long kernel_load_addr;unsigned long initrd_load_addr =0 , fdt_load_addr; -void *fdt; +void *fdt =NULL ;const void *slave_code;struct elfhdr ehdr;char *modified_cmdline =NULL ;
@@ -103,18 +104,12 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, } fdt_size =fdt_totalsize (initial_boot_params) *2 ; - fdt =kmalloc (fdt_size,GFP_KERNEL ); + fdt =of_alloc_and_init_fdt (fdt_size);if (!fdt) {pr_err ("Not enough memory for the device tree.\n" ); ret = -ENOMEM ; goto out; } - ret =fdt_open_into (initial_boot_params, fdt, fdt_size); -if (ret <0 ) { -pr_err ("Error setting up the new device tree.\n" ); - ret = -EINVAL ; - goto out; - } ret =setup_new_fdt_ppc64 (image, fdt, initrd_load_addr, initrd_len, cmdline);
@@ -131,6 +126,10 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, ret =kexec_add_buffer (&kbuf);if (ret) goto out; + +/* FDT will be freed in arch_kimage_file_post_load_cleanup */ + image->arch.fdt = fdt; + fdt_load_addr = kbuf.mem;pr_debug ("Loaded device tree at 0x%lx\n" , fdt_load_addr);
@@ -145,8 +144,15 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,kfree (modified_cmdline);kexec_free_elf_info (&elf_info); -/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */ -return ret ?ERR_PTR (ret) : fdt; +/* +* Once FDT buffer has been successfully passed to kexec_add_buffer(), +* the FDT buffer address is saved in image->arch.fdt. In that case, +* the memory cannot be freed here in case of any other error. +*/ +if (ret && !image->arch.fdt) +of_free_fdt (fdt); + +return ret ?ERR_PTR (ret) :NULL ; }const struct kexec_file_ops kexec_elf64_ops = {
arch/powerpc/kexec/file_load_64.c +3 -0
@@ -1113,5 +1113,8 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image) image->arch.elf_headers =NULL ; image->arch.elf_headers_sz =0 ; +of_free_fdt (image->arch.fdt); + image->arch.fdt =NULL ; +return kexec_image_post_load_cleanup_default (image); }
Signature · 7 lines
2.30.0 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Rob Herring
maintainerRe: [PATCH v16 11/12] powerpc: Use OF alloc and free for FDT
Quoted 4 lines · Lakshmi Ramasubramanian
of_alloc_and_init_fdt()andof_free_fdt()have been defined indrivers/of/kexec.cto allocate and free memory for FDT.
Quoted 8 lines
Use
of_alloc_and_init_fdt()andof_free_fdt()to allocate and initialize the FDT, and to free the FDT respectively.powerpc sets the FDT address in
image_loader_datafield in "struct kimage" and the memory is freed inkimage_file_post_load_cleanup(). This cleanup function useskfree()to free the memory. But sinceof_alloc_and_init_fdt()useskvmalloc()for allocation, the buffer needs to be freed usingkvfree().
You could just change the kexec core to call kvfree() instead.
Quoted 3 lines
Define "fdt" field in "struct
kimage_arch" for powerpc to store the address of FDT, and free the memory in powerpc specificarch_kimage_file_post_load_cleanup().
However, given all the other buffers have an explicit field in kimage or kimage_arch, changing powerpc is to match arm64 is better IMO.
Quoted 52 lines
Signed-off-by Lakshmi Ramasubramanian <[email protected]>Suggested-by Rob Herring <[email protected]>Suggested-by Thiago Jung Bauermann <[email protected]>--- arch/powerpc/include/asm/kexec.h | 2 ++ arch/powerpc/kexec/elf_64.c | 26 ++++++++++++++++---------- arch/powerpc/kexec/file_load_64.c | 3 +++ 3 files changed, 21 insertions(+), 10 deletions(-)arch/powerpc/include/asm/kexec.h +2
-0@@ -111,6 +111,8 @@ struct kimage_arch {unsigned long elf_headers_mem;unsigned long elf_headers_sz;void *elf_headers; + +void *fdt; }; char *setup_kdump_cmdline(struct kimage *image, char *cmdline,arch/powerpc/kexec/elf_64.c +3
-2@@ -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> @@ -32,7 +33,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,unsigned int fdt_size;unsigned long kernel_load_addr;unsigned long initrd_load_addr =0 , fdt_load_addr; -void *fdt; +void *fdt =NULL ;const void *slave_code;struct elfhdr ehdr;char *modified_cmdline =NULL ;@@ -103,18 +104,12 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, } fdt_size =fdt_totalsize (initial_boot_params) *2 ; - fdt =kmalloc (fdt_size,GFP_KERNEL ); + fdt =of_alloc_and_init_fdt (fdt_size);if (!fdt) {pr_err ("Not enough memory for the device tree.\n" ); ret = -ENOMEM ;
arch/powerpc/kexec/elf_64.c static void *elf64_load(struct kimage *image, char *kernel_buf, · line 114
Quoted 10 lines
arch/powerpc/kexec/elf_64.c +0
-6goto out; } - ret =fdt_open_into (initial_boot_params, fdt, fdt_size); -if (ret <0 ) { -pr_err ("Error setting up the new device tree.\n" ); - ret = -EINVAL ; - goto out; - } ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
The first thing this function does is call setup_new_fdt() which first calls of_kexec_setup_new_fdt(). (Note, I really don't understand the PPC code split. It looks like there's a 32-bit and 64-bit split, but 32-bit looks broken to me. Nothing ever calls setup_new_fdt() except setup_new_fdt_ppc64()). The arm64 version is calling of_alloc_and_init_fdt() and then of_kexec_setup_new_fdt() directly.
So we can just make of_alloc_and_init_fdt() also call of_kexec_setup_new_fdt() (really, just tweak of_kexec_setup_new_fdt do the alloc and copy). I don't think the architecture needs to pick the size either. It's doubtful that either one is that sensitive to the amount of extra space.
Quoted 15 lines
arch/powerpc/kexec/elf_64.c +4
-0initrd_len, cmdline);@@ -131,6 +126,10 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, ret =kexec_add_buffer (&kbuf);if (ret) goto out; + +/* FDT will be freed in arch_kimage_file_post_load_cleanup */ + image->arch.fdt = fdt; + fdt_load_addr = kbuf.mem;pr_debug ("Loaded device tree at 0x%lx\n" , fdt_load_addr);@@ -145,8 +144,15 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,kfree (modified_cmdline);kexec_free_elf_info (&elf_info);
arch/powerpc/kexec/elf_64.c static void *elf64_load(struct kimage *image, char *kernel_buf, · line 153
Quoted 10 lines
arch/powerpc/kexec/elf_64.c +7
-2-/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */ -return ret ?ERR_PTR (ret) : fdt; +/* +* Once FDT buffer has been successfully passed to kexec_add_buffer(), +* the FDT buffer address is saved in image->arch.fdt. In that case, +* the memory cannot be freed here in case of any other error. +*/ +if (ret && !image->arch.fdt) +of_free_fdt (fdt);
Just call kvfree() directly.
Quoted 21 lines
arch/powerpc/kexec/elf_64.c +2
-0+ +return ret ?ERR_PTR (ret) :NULL ; }const struct kexec_file_ops kexec_elf64_ops = {arch/powerpc/kexec/file_load_64.c +3
-1@@ -1113,5 +1113,8 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image) image->arch.elf_headers =NULL ; image->arch.elf_headers_sz =0 ; +of_free_fdt (image->arch.fdt); + image->arch.fdt =NULL ; +return kexec_image_post_load_cleanup_default (image); } --2.30.0
Re: [PATCH v16 11/12] powerpc: Use OF alloc and free for FDT
Quoted 18 lines · Rob Herring
Quoted 4 lines · Lakshmi Ramasubramanian
of_alloc_and_init_fdt()andof_free_fdt()have been defined indrivers/of/kexec.cto allocate and free memory for FDT.Quoted 8 lines
Use
of_alloc_and_init_fdt()andof_free_fdt()to allocate and initialize the FDT, and to free the FDT respectively.powerpc sets the FDT address in
image_loader_datafield in "struct kimage" and the memory is freed inkimage_file_post_load_cleanup(). This cleanup function useskfree()to free the memory. But sinceof_alloc_and_init_fdt()useskvmalloc()for allocation, the buffer needs to be freed usingkvfree().You could just change the kexec core to call
kvfree()instead.
Quoted 6 lines
Quoted 3 lines
Define "fdt" field in "struct
kimage_arch" for powerpc to store the address of FDT, and free the memory in powerpc specificarch_kimage_file_post_load_cleanup().However, given all the other buffers have an explicit field in kimage or
kimage_arch, changing powerpc is to match arm64 is better IMO.
Just to be clear:
I'll leave this as is - free FDT buffer in powerpc's arch_kimage_file_post_load_cleanup() to match arm64 behavior.
Will not change "kexec core" to call kvfree() - doing that change would require changing all architectures to use kvmalloc() for image_loader_data allocation.
Quoted 71 lines
Quoted 53 lines
Signed-off-by Lakshmi Ramasubramanian <[email protected]>Suggested-by Rob Herring <[email protected]>Suggested-by Thiago Jung Bauermann <[email protected]>--- arch/powerpc/include/asm/kexec.h | 2 ++ arch/powerpc/kexec/elf_64.c | 26 ++++++++++++++++---------- arch/powerpc/kexec/file_load_64.c | 3 +++ 3 files changed, 21 insertions(+), 10 deletions(-)arch/powerpc/include/asm/kexec.h +2
-0@@ -111,6 +111,8 @@ struct kimage_arch {unsigned long elf_headers_mem;unsigned long elf_headers_sz;void *elf_headers; + +void *fdt; }; char *setup_kdump_cmdline(struct kimage *image, char *cmdline,arch/powerpc/kexec/elf_64.c +3
-2@@ -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> @@ -32,7 +33,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,unsigned int fdt_size;unsigned long kernel_load_addr;unsigned long initrd_load_addr =0 , fdt_load_addr; -void *fdt; +void *fdt =NULL ;const void *slave_code;struct elfhdr ehdr;char *modified_cmdline =NULL ;@@ -103,18 +104,12 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, } fdt_size =fdt_totalsize (initial_boot_params) *2 ; - fdt =kmalloc (fdt_size,GFP_KERNEL ); + fdt =of_alloc_and_init_fdt (fdt_size);if (!fdt) {pr_err ("Not enough memory for the device tree.\n" ); ret = -ENOMEM ;arch/powerpc/kexec/elf_64.c static void *elf64_load(struct kimage *image, char *kernel_buf, · line 114
Quoted 10 lines
arch/powerpc/kexec/elf_64.c +0
-6goto out; } - ret =fdt_open_into (initial_boot_params, fdt, fdt_size); -if (ret <0 ) { -pr_err ("Error setting up the new device tree.\n" ); - ret = -EINVAL ; - goto out; - } ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,The first thing this function does is call
setup_new_fdt()which first callsof_kexec_setup_new_fdt(). (Note, I really don't understand the PPC code split. It looks like there's a 32-bit and 64-bit split, but 32-bit looks broken to me. Nothing ever callssetup_new_fdt()exceptsetup_new_fdt_ppc64()). The arm64 version is callingof_alloc_and_init_fdt()and thenof_kexec_setup_new_fdt()directly.
Quoted 3 lines
So we can just make
of_alloc_and_init_fdt()also callof_kexec_setup_new_fdt()(really, just tweakof_kexec_setup_new_fdtdo the alloc and copy).
ok - will move fdt allocation into of_kexec_setup_new_fdt().
I don't think the architecture needs to pick the
Quoted 2 lines
size either. It's doubtful that either one is that sensitive to the amount of extra space.
I am not clear about the above comment -
are you saying the architectures don't need to pass FDT size to the alloc function?
arm64 is adding command line string length and some extra space to the size computed from initial_boot_params for FDT Size:
buf_size = fdt_totalsize(initial_boot_params)
+ cmdline_len + DTB_EXTRA_SPACE;
powerpc is just using twice the size computed from initial_boot_params
fdt_size = fdt_totalsize(initial_boot_params) * 2;
I think it would be safe to let arm64 and powerpc pass the required FDT size, along with the other params to of_kexec_setup_new_fdt() - and in this function we allocate FDT and set it up.
And, for powerpc leave the remaining code in setup_new_fdt_ppc64().
Would that be ok?
Quoted 17 lines
Quoted 17 lines
arch/powerpc/kexec/elf_64.c +4
-0initrd_len, cmdline);@@ -131,6 +126,10 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, ret =kexec_add_buffer (&kbuf);if (ret) goto out; + +/* FDT will be freed in arch_kimage_file_post_load_cleanup */ + image->arch.fdt = fdt; + fdt_load_addr = kbuf.mem;pr_debug ("Loaded device tree at 0x%lx\n" , fdt_load_addr);@@ -145,8 +144,15 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,kfree (modified_cmdline);kexec_free_elf_info (&elf_info);
Quoted 11 lines
arch/powerpc/kexec/elf_64.c static void *elf64_load(struct kimage *image, char *kernel_buf, · line 153
Quoted 9 lines
arch/powerpc/kexec/elf_64.c +7
-2-/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */ -return ret ?ERR_PTR (ret) : fdt; +/* +* Once FDT buffer has been successfully passed to kexec_add_buffer(), +* the FDT buffer address is saved in image->arch.fdt. In that case, +* the memory cannot be freed here in case of any other error. +*/ +if (ret && !image->arch.fdt) +of_free_fdt (fdt);Just call
kvfree()directly.
Sure - will do.
-lakshmi
Quoted 22 lines
Quoted 22 lines
arch/powerpc/kexec/elf_64.c +2
-0+ +return ret ?ERR_PTR (ret) :NULL ; }const struct kexec_file_ops kexec_elf64_ops = {arch/powerpc/kexec/file_load_64.c +3
-1@@ -1113,5 +1113,8 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image) image->arch.elf_headers =NULL ; image->arch.elf_headers_sz =0 ; +of_free_fdt (image->arch.fdt); + image->arch.fdt =NULL ; +return kexec_image_post_load_cleanup_default (image); } --2.30.0
Rob Herring
maintainerRe: [PATCH v16 11/12] powerpc: Use OF alloc and free for FDT
Quoted 24 lines · Lakshmi Ramasubramanian
Quoted 22 lines · Rob Herring
Quoted 4 lines · Lakshmi Ramasubramanian
of_alloc_and_init_fdt()andof_free_fdt()have been defined indrivers/of/kexec.cto allocate and free memory for FDT.Quoted 8 lines
Use
of_alloc_and_init_fdt()andof_free_fdt()to allocate and initialize the FDT, and to free the FDT respectively.powerpc sets the FDT address in
image_loader_datafield in "struct kimage" and the memory is freed inkimage_file_post_load_cleanup(). This cleanup function useskfree()to free the memory. But sinceof_alloc_and_init_fdt()useskvmalloc()for allocation, the buffer needs to be freed usingkvfree().You could just change the kexec core to call
kvfree()instead.Quoted 3 lines
Define "fdt" field in "struct
kimage_arch" for powerpc to store the address of FDT, and free the memory in powerpc specificarch_kimage_file_post_load_cleanup().
Quoted 6 lines
Quoted 2 lines
However, given all the other buffers have an explicit field in kimage or
kimage_arch, changing powerpc is to match arm64 is better IMO.Just to be clear:
I'll leave this as is - free FDT buffer in powerpc'sarch_kimage_file_post_load_cleanup()to match arm64 behavior.
Yes.
Quoted 3 lines
Will not change "kexec core" to call
kvfree()- doing that change would require changing all architectures to usekvmalloc()forimage_loader_dataallocation.
Actually, no. AIUI, kvfree() can be used whether you used kvmalloc, vmalloc, or kmalloc for the alloc.
Quoted 88 lines
Quoted 70 lines
Quoted 52 lines
Signed-off-by Lakshmi Ramasubramanian <[email protected]>Suggested-by Rob Herring <[email protected]>Suggested-by Thiago Jung Bauermann <[email protected]>--- arch/powerpc/include/asm/kexec.h | 2 ++ arch/powerpc/kexec/elf_64.c | 26 ++++++++++++++++---------- arch/powerpc/kexec/file_load_64.c | 3 +++ 3 files changed, 21 insertions(+), 10 deletions(-)arch/powerpc/include/asm/kexec.h +2
-0@@ -111,6 +111,8 @@ struct kimage_arch {unsigned long elf_headers_mem;unsigned long elf_headers_sz;void *elf_headers; + +void *fdt; }; char *setup_kdump_cmdline(struct kimage *image, char *cmdline,arch/powerpc/kexec/elf_64.c +3
-2@@ -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> @@ -32,7 +33,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,unsigned int fdt_size;unsigned long kernel_load_addr;unsigned long initrd_load_addr =0 , fdt_load_addr; -void *fdt; +void *fdt =NULL ;const void *slave_code;struct elfhdr ehdr;char *modified_cmdline =NULL ;@@ -103,18 +104,12 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, } fdt_size =fdt_totalsize (initial_boot_params) *2 ; - fdt =kmalloc (fdt_size,GFP_KERNEL ); + fdt =of_alloc_and_init_fdt (fdt_size);if (!fdt) {pr_err ("Not enough memory for the device tree.\n" ); ret = -ENOMEM ;arch/powerpc/kexec/elf_64.c static void *elf64_load(struct kimage *image, char *kernel_buf, · line 114
Quoted 10 lines
arch/powerpc/kexec/elf_64.c +0
-6goto out; } - ret =fdt_open_into (initial_boot_params, fdt, fdt_size); -if (ret <0 ) { -pr_err ("Error setting up the new device tree.\n" ); - ret = -EINVAL ; - goto out; - } ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,The first thing this function does is call
setup_new_fdt()which first callsof_kexec_setup_new_fdt(). (Note, I really don't understand the PPC code split. It looks like there's a 32-bit and 64-bit split, but 32-bit looks broken to me. Nothing ever callssetup_new_fdt()exceptsetup_new_fdt_ppc64()). The arm64 version is callingof_alloc_and_init_fdt()and thenof_kexec_setup_new_fdt()directly.Quoted 3 lines
So we can just make
of_alloc_and_init_fdt()also callof_kexec_setup_new_fdt()(really, just tweakof_kexec_setup_new_fdtdo the alloc and copy).ok - will move fdt allocation into
of_kexec_setup_new_fdt().I don't think the architecture needs to pick the
Quoted 2 lines
size either. It's doubtful that either one is that sensitive to the amount of extra space.
I am not clear about the above comment -
are you saying the architectures don't need to pass FDT size to the alloc function?arm64 is adding command line string length and some extra space to the size computed from
initial_boot_paramsfor FDT Size:
buf_size=fdt_totalsize(initial_boot_params)
+cmdline_len+DTB_EXTRA_SPACE;
Quoted 7 lines
powerpc is just using twice the size computed from
initial_boot_paramsfdt_size = fdt_totalsize(initial_boot_params) * 2;I think it would be safe to let arm64 and powerpc pass the required FDT size, along with the other params to
of_kexec_setup_new_fdt()- and in this function we allocate FDT and set it up.
It's pretty clear that someone just picked something that 'should be enough'. The only thing I can guess for the difference is that arm DT's tend to be a bit larger. So doubling the size would be even more excessive. Either way, we're talking 10s kB to few 100kB. I'd go with DTB_EXTRA_SPACE and we can bump it up if someone has problems.
Also, I would like for 'initial_boot_params' to be private ultimately, so removing any references is helpful.
Quoted 1 line
And, for powerpc leave the remaining code in
setup_new_fdt_ppc64().
Right.
Rob
Re: [PATCH v16 11/12] powerpc: Use OF alloc and free for FDT
Quoted 26 lines · Rob Herring
Quoted 24 lines · Lakshmi Ramasubramanian
Quoted 22 lines · Rob Herring
Quoted 4 lines · Lakshmi Ramasubramanian
of_alloc_and_init_fdt()andof_free_fdt()have been defined indrivers/of/kexec.cto allocate and free memory for FDT.Quoted 8 lines
Use
of_alloc_and_init_fdt()andof_free_fdt()to allocate and initialize the FDT, and to free the FDT respectively.powerpc sets the FDT address in
image_loader_datafield in "struct kimage" and the memory is freed inkimage_file_post_load_cleanup(). This cleanup function useskfree()to free the memory. But sinceof_alloc_and_init_fdt()useskvmalloc()for allocation, the buffer needs to be freed usingkvfree().You could just change the kexec core to call
kvfree()instead.Quoted 3 lines
Define "fdt" field in "struct
kimage_arch" for powerpc to store the address of FDT, and free the memory in powerpc specificarch_kimage_file_post_load_cleanup().
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
However, given all the other buffers have an explicit field in kimage or
kimage_arch, changing powerpc is to match arm64 is better IMO.Just to be clear:
I'll leave this as is - free FDT buffer in powerpc'sarch_kimage_file_post_load_cleanup()to match arm64 behavior.Yes.
ok
Quoted 7 lines
Quoted 4 lines
Will not change "kexec core" to call
kvfree()- doing that change would require changing all architectures to usekvmalloc()forimage_loader_dataallocation.Actually, no. AIUI,
kvfree()can be used whether you used kvmalloc, vmalloc, or kmalloc for the alloc.
That is good information. Thanks.
Quoted 97 lines
Quoted 89 lines
Quoted 71 lines
Quoted 53 lines
Signed-off-by Lakshmi Ramasubramanian <[email protected]>Suggested-by Rob Herring <[email protected]>Suggested-by Thiago Jung Bauermann <[email protected]>--- arch/powerpc/include/asm/kexec.h | 2 ++ arch/powerpc/kexec/elf_64.c | 26 ++++++++++++++++---------- arch/powerpc/kexec/file_load_64.c | 3 +++ 3 files changed, 21 insertions(+), 10 deletions(-)arch/powerpc/include/asm/kexec.h +2
-0@@ -111,6 +111,8 @@ struct kimage_arch {unsigned long elf_headers_mem;unsigned long elf_headers_sz;void *elf_headers; + +void *fdt; }; char *setup_kdump_cmdline(struct kimage *image, char *cmdline,arch/powerpc/kexec/elf_64.c +3
-2@@ -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> @@ -32,7 +33,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,unsigned int fdt_size;unsigned long kernel_load_addr;unsigned long initrd_load_addr =0 , fdt_load_addr; -void *fdt; +void *fdt =NULL ;const void *slave_code;struct elfhdr ehdr;char *modified_cmdline =NULL ;@@ -103,18 +104,12 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, } fdt_size =fdt_totalsize (initial_boot_params) *2 ; - fdt =kmalloc (fdt_size,GFP_KERNEL ); + fdt =of_alloc_and_init_fdt (fdt_size);if (!fdt) {pr_err ("Not enough memory for the device tree.\n" ); ret = -ENOMEM ;arch/powerpc/kexec/elf_64.c static void *elf64_load(struct kimage *image, char *kernel_buf, · line 114
Quoted 10 lines
arch/powerpc/kexec/elf_64.c +0
-6goto out; } - ret =fdt_open_into (initial_boot_params, fdt, fdt_size); -if (ret <0 ) { -pr_err ("Error setting up the new device tree.\n" ); - ret = -EINVAL ; - goto out; - } ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,The first thing this function does is call
setup_new_fdt()which first callsof_kexec_setup_new_fdt(). (Note, I really don't understand the PPC code split. It looks like there's a 32-bit and 64-bit split, but 32-bit looks broken to me. Nothing ever callssetup_new_fdt()exceptsetup_new_fdt_ppc64()). The arm64 version is callingof_alloc_and_init_fdt()and thenof_kexec_setup_new_fdt()directly.Quoted 3 lines
So we can just make
of_alloc_and_init_fdt()also callof_kexec_setup_new_fdt()(really, just tweakof_kexec_setup_new_fdtdo the alloc and copy).ok - will move fdt allocation into
of_kexec_setup_new_fdt().I don't think the architecture needs to pick the
Quoted 2 lines
size either. It's doubtful that either one is that sensitive to the amount of extra space.
I am not clear about the above comment -
are you saying the architectures don't need to pass FDT size to the alloc function?arm64 is adding command line string length and some extra space to the size computed from
initial_boot_paramsfor FDT Size:
buf_size=fdt_totalsize(initial_boot_params)
+cmdline_len+DTB_EXTRA_SPACE;Quoted 7 lines
powerpc is just using twice the size computed from
initial_boot_paramsfdt_size = fdt_totalsize(initial_boot_params) * 2;I think it would be safe to let arm64 and powerpc pass the required FDT size, along with the other params to
of_kexec_setup_new_fdt()- and in this function we allocate FDT and set it up.
Quoted 5 lines
It's pretty clear that someone just picked something that 'should be enough'. The only thing I can guess for the difference is that arm DT's tend to be a bit larger. So doubling the size would be even more excessive. Either way, we're talking 10s kB to few 100kB. I'd go with
DTB_EXTRA_SPACEand we can bump it up if someone has problems.
ok - I'll use DTB_EXTRA_SPACE for the fdt allocation.
Quoted 3 lines
Also, I would like for '
initial_boot_params' to be private ultimately, so removing any references is helpful.
ok
Quoted 4 lines
Quoted 2 lines
And, for powerpc leave the remaining code in
setup_new_fdt_ppc64().Right.
ok
thanks,
-lakshmi
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Michael Ellerman
reviewerRe: [PATCH v16 11/12] powerpc: Use OF alloc and free for FDT
In reply to Rob HerringQuoted 2 lines · Rob Herring
On Thu, Feb 4, 2021 at 10:42 AM Lakshmi Ramasubramanian <[email protected]> wrote:
...
Quoted 39 lines
Quoted 39 lines
arch/powerpc/kexec/elf_64.c +3
-8@@ -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> @@ -32,7 +33,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,unsigned int fdt_size;unsigned long kernel_load_addr;unsigned long initrd_load_addr =0 , fdt_load_addr; -void *fdt; +void *fdt =NULL ;const void *slave_code;struct elfhdr ehdr;char *modified_cmdline =NULL ;@@ -103,18 +104,12 @@ static void *elf64_load(struct kimage *image, char *kernel_buf, } fdt_size =fdt_totalsize (initial_boot_params) *2 ; - fdt =kmalloc (fdt_size,GFP_KERNEL ); + fdt =of_alloc_and_init_fdt (fdt_size);if (!fdt) {pr_err ("Not enough memory for the device tree.\n" ); ret = -ENOMEM ; goto out; } - ret =fdt_open_into (initial_boot_params, fdt, fdt_size); -if (ret <0 ) { -pr_err ("Error setting up the new device tree.\n" ); - ret = -EINVAL ; - goto out; - }
Quoted 7 lines
arch/powerpc/kexec/elf_64.c static void *elf64_load(struct kimage *image, char *kernel_buf, · line 114
Quoted 1 line
arch/powerpc/kexec/elf_64.c +0
-0ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,The first thing this function does is call
setup_new_fdt()which first callsof_kexec_setup_new_fdt(). (Note, I really don't understand the PPC code split. It looks like there's a 32-bit and 64-bit split, but 32-bit looks broken to me. Nothing ever callssetup_new_fdt()exceptsetup_new_fdt_ppc64()).
I think that's because 32-bit doesn't support kexec_file_load().
cheers
PATCH v16 12/12 arm64: Enable passing IMA log to next kernel on kexec
In reply to Lakshmi RamasubramanianUpdate 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.
--- arch/arm64/Kconfig | 1 + 1 file changed, 1 insertion(+)
arch/arm64/Kconfig +1 -0
@@ -1093,6 +1093,7 @@ config KEXEC config KEXEC_FILE bool "kexec file based system call" select KEXEC_CORE + select HAVE_IMA_KEXEC if IMA help This is new version of kexec system call. This system call is file based and takes file descriptors as system call argument
Signature · 7 lines
2.30.0 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel