Re: [PATCH v16 11/12] powerpc: Use OF alloc and free for FDT
Rob Herring writes:
> On Thu, Feb 4, 2021 at 10:42 AM Lakshmi Ramasubramanian
> wrote:
...
>> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>> index d0e459bb2f05..51d2d8eb6c1b 100644
>> --- a/arch/powerpc/kexec/elf_64.c
>> +++ b/arch/powerpc/kexec/elf_64.c
>> @@ -19,6 +19,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -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,
>
> 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()).
I think that's because 32-bit doesn't support kexec_file_load().
cheers
Re: [PATCH v16 11/12] powerpc: Use OF alloc and free for FDT
On 2/4/21 3:36 PM, Rob Herring wrote:
On Thu, Feb 4, 2021 at 5:23 PM Lakshmi Ramasubramanian
wrote:
On 2/4/21 11:26 AM, Rob Herring wrote:
On Thu, Feb 4, 2021 at 10:42 AM Lakshmi Ramasubramanian
wrote:
of_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().
You could just change the kexec core to call kvfree() instead.
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().
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.
Yes.
ok
Will not change "kexec core" to call kvfree() - doing that change would
require changing all architectures to use kvmalloc() for
image_loader_data allocation.
Actually, no. AIUI, kvfree() can be used whether you used kvmalloc,
vmalloc, or kmalloc for the alloc.
That is good information. Thanks.
Signed-off-by: Lakshmi Ramasubramanian
Suggested-by: Rob Herring
Suggested-by: Thiago Jung Bauermann
---
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(-)
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 2c0be93d239a..d7d13cac4d31 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -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,
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index d0e459bb2f05..51d2d8eb6c1b 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -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,
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).
ok - will move fdt allocation into of_kexec_setup_new_fdt().
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.
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
Re: [PATCH v16 11/12] powerpc: Use OF alloc and free for FDT
On Thu, Feb 4, 2021 at 5:23 PM Lakshmi Ramasubramanian
wrote:
>
> On 2/4/21 11:26 AM, Rob Herring wrote:
> > On Thu, Feb 4, 2021 at 10:42 AM Lakshmi Ramasubramanian
> > wrote:
> >>
> >> of_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().
> >
> > You could just change the kexec core to call kvfree() instead.
>
> >
> >> 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().
> >
> > 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.
Yes.
> Will not change "kexec core" to call kvfree() - doing that change would
> require changing all architectures to use kvmalloc() for
> image_loader_data allocation.
Actually, no. AIUI, kvfree() can be used whether you used kvmalloc,
vmalloc, or kmalloc for the alloc.
> >> Signed-off-by: Lakshmi Ramasubramanian
> >> Suggested-by: Rob Herring
> >> Suggested-by: Thiago Jung Bauermann
> >> ---
> >> 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(-)
> >>
> >> diff --git a/arch/powerpc/include/asm/kexec.h
> >> b/arch/powerpc/include/asm/kexec.h
> >> index 2c0be93d239a..d7d13cac4d31 100644
> >> --- a/arch/powerpc/include/asm/kexec.h
> >> +++ b/arch/powerpc/include/asm/kexec.h
> >> @@ -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,
> >> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> >> index d0e459bb2f05..51d2d8eb6c1b 100644
> >> --- a/arch/powerpc/kexec/elf_64.c
> >> +++ b/arch/powerpc/kexec/elf_64.c
> >> @@ -19,6 +19,7 @@
> >> #include
> >> #include
> >> #include
> >> +#include
> >> #include
> >> #include
> >> #include
> >> @@ -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,
> >
> > 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).
> ok - will move fdt allocation into of_kexec_setup_new_fdt().
>
> 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.
> 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
Re: [PATCH v16 11/12] powerpc: Use OF alloc and free for FDT
On 2/4/21 11:26 AM, Rob Herring wrote:
On Thu, Feb 4, 2021 at 10:42 AM Lakshmi Ramasubramanian
wrote:
of_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().
You could just change the kexec core to call kvfree() instead.
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().
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.
Signed-off-by: Lakshmi Ramasubramanian
Suggested-by: Rob Herring
Suggested-by: Thiago Jung Bauermann
---
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(-)
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 2c0be93d239a..d7d13cac4d31 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -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,
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index d0e459bb2f05..51d2d8eb6c1b 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -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,
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).
ok - will move fdt allocation into of_kexec_setup_new_fdt().
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.
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?
Re: [PATCH v16 11/12] powerpc: Use OF alloc and free for FDT
On Thu, Feb 4, 2021 at 10:42 AM Lakshmi Ramasubramanian
wrote:
>
> of_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().
You could just change the kexec core to call kvfree() instead.
> 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().
However, given all the other buffers have an explicit field in kimage
or kimage_arch, changing powerpc is to match arm64 is better IMO.
> Signed-off-by: Lakshmi Ramasubramanian
> Suggested-by: Rob Herring
> Suggested-by: Thiago Jung Bauermann
> ---
> 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(-)
>
> diff --git a/arch/powerpc/include/asm/kexec.h
> b/arch/powerpc/include/asm/kexec.h
> index 2c0be93d239a..d7d13cac4d31 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.h
> @@ -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,
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index d0e459bb2f05..51d2d8eb6c1b 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -19,6 +19,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -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,
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.
> initrd_len, cmdline);
> @@ -131,6 +126,10 @@ static void *elf64_load(struct kimage *image, char
> *kernel_buf,
> ret = kexec_add_buffer();
> 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(_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);
Just call kvfree() directly.
> +
> +
[PATCH v16 11/12] powerpc: Use OF alloc and free for FDT
of_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().
Signed-off-by: Lakshmi Ramasubramanian
Suggested-by: Rob Herring
Suggested-by: Thiago Jung Bauermann
---
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(-)
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 2c0be93d239a..d7d13cac4d31 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -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,
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index d0e459bb2f05..51d2d8eb6c1b 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -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();
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(_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 = {
diff --git a/arch/powerpc/kexec/file_load_64.c
b/arch/powerpc/kexec/file_load_64.c
index 3cab318aa3b9..d9d5b5569a6d 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -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
