diff mbox series

[v2] powerpc/kexec_file: Restore FDT size estimation for kdump kernel

Message ID 20210220005204.1417200-1-bauerman@linux.ibm.com (mailing list archive)
State Handled Elsewhere
Headers show
Series [v2] powerpc/kexec_file: Restore FDT size estimation for kdump kernel | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch warning Failed to apply on branch powerpc/merge (626a6c3d2e20da80aaa710104f34ea6037b28b33)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch powerpc/next (6895c5ba7bdcc55eacad03cf309ab23be63b9cac)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch linus/master (92bf22614b21a2706f4993b278017e437f7785b3)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch powerpc/fixes (24321ac668e452a4942598533d267805f291fdc9)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch linux-next (1e0d27fce010b0a4a9e595506b6ede75934c31be)
snowpatch_ozlabs/apply_patch fail Failed to apply to any branch

Commit Message

Thiago Jung Bauermann Feb. 20, 2021, 12:52 a.m. UTC
Commit 2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump
kernel") fixed how elf64_load() estimates the FDT size needed by the
crashdump kernel.

At the same time, commit 130b2d59cec0 ("powerpc: Use common
of_kexec_alloc_and_setup_fdt()") changed the same code to use the generic
function of_kexec_alloc_and_setup_fdt() to calculate the FDT size. That
change made the code overestimate it a bit by counting twice the space
required for the kernel command line and /chosen properties.

Therefore change kexec_fdt_totalsize_ppc64() to calculate just the extra
space needed by the kdump kernel, and change the function name so that it
better reflects what the function is now doing.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 arch/powerpc/include/asm/kexec.h  |  2 +-
 arch/powerpc/kexec/elf_64.c       |  2 +-
 arch/powerpc/kexec/file_load_64.c | 26 ++++++++------------------
 3 files changed, 10 insertions(+), 20 deletions(-)

Applies on top of next-20210219.

Changes since v1:

- Adjusted comment describing kexec_extra_fdt_size_ppc64() as suggested
  by Lakshmi.

Comments

Hari Bathini Feb. 20, 2021, 5:13 p.m. UTC | #1
On 20/02/21 6:22 am, Thiago Jung Bauermann wrote:
> Commit 2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump
> kernel") fixed how elf64_load() estimates the FDT size needed by the
> crashdump kernel.
> 
> At the same time, commit 130b2d59cec0 ("powerpc: Use common
> of_kexec_alloc_and_setup_fdt()") changed the same code to use the generic
> function of_kexec_alloc_and_setup_fdt() to calculate the FDT size. That
> change made the code overestimate it a bit by counting twice the space
> required for the kernel command line and /chosen properties.
> 
> Therefore change kexec_fdt_totalsize_ppc64() to calculate just the extra
> space needed by the kdump kernel, and change the function name so that it
> better reflects what the function is now doing.

Thanks for fixing this, Thiago.

Reviewed-by: Hari Bathini <hbathini@linux.ibm.com>

> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
>   arch/powerpc/include/asm/kexec.h  |  2 +-
>   arch/powerpc/kexec/elf_64.c       |  2 +-
>   arch/powerpc/kexec/file_load_64.c | 26 ++++++++------------------
>   3 files changed, 10 insertions(+), 20 deletions(-)
> 
> Applies on top of next-20210219.
> 
> Changes since v1:
> 
> - Adjusted comment describing kexec_extra_fdt_size_ppc64() as suggested
>    by Lakshmi.
> 
> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
> index baab158e215c..5a11cc8d2350 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.h
> @@ -128,7 +128,7 @@ int load_crashdump_segments_ppc64(struct kimage *image,
>   int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
>   			  const void *fdt, unsigned long kernel_load_addr,
>   			  unsigned long fdt_load_addr);
> -unsigned int kexec_fdt_totalsize_ppc64(struct kimage *image);
> +unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image);
>   int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
>   			unsigned long initrd_load_addr,
>   			unsigned long initrd_len, const char *cmdline);
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index 0492ca6003f3..5a569bb51349 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -104,7 +104,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>   
>   	fdt = of_kexec_alloc_and_setup_fdt(image, initrd_load_addr,
>   					   initrd_len, cmdline,
> -					   kexec_fdt_totalsize_ppc64(image));
> +					   kexec_extra_fdt_size_ppc64(image));
>   	if (!fdt) {
>   		pr_err("Error setting up the new device tree.\n");
>   		ret = -EINVAL;
> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
> index 3609de30a170..297f73795a1f 100644
> --- a/arch/powerpc/kexec/file_load_64.c
> +++ b/arch/powerpc/kexec/file_load_64.c
> @@ -927,37 +927,27 @@ int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
>   }
>   
>   /**
> - * kexec_fdt_totalsize_ppc64 - Return the estimated size needed to setup FDT
> - *                             for kexec/kdump kernel.
> - * @image:                     kexec image being loaded.
> + * kexec_extra_fdt_size_ppc64 - Return the estimated additional size needed to
> + *                              setup FDT for kexec/kdump kernel.
> + * @image:                      kexec image being loaded.
>    *
> - * Returns the estimated size needed for kexec/kdump kernel FDT.
> + * Returns the estimated extra size needed for kexec/kdump kernel FDT.
>    */
> -unsigned int kexec_fdt_totalsize_ppc64(struct kimage *image)
> +unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image)
>   {
> -	unsigned int fdt_size;
>   	u64 usm_entries;
>   
> -	/*
> -	 * The below estimate more than accounts for a typical kexec case where
> -	 * the additional space is to accommodate things like kexec cmdline,
> -	 * chosen node with properties for initrd start & end addresses and
> -	 * a property to indicate kexec boot..
> -	 */
> -	fdt_size = fdt_totalsize(initial_boot_params) + (2 * COMMAND_LINE_SIZE);
>   	if (image->type != KEXEC_TYPE_CRASH)
> -		return fdt_size;
> +		return 0;
>   
>   	/*
> -	 * For kdump kernel, also account for linux,usable-memory and
> +	 * For kdump kernel, account for linux,usable-memory and
>   	 * linux,drconf-usable-memory properties. Get an approximate on the
>   	 * number of usable memory entries and use for FDT size estimation.
>   	 */
>   	usm_entries = ((memblock_end_of_DRAM() / drmem_lmb_size()) +
>   		       (2 * (resource_size(&crashk_res) / drmem_lmb_size())));
> -	fdt_size += (unsigned int)(usm_entries * sizeof(u64));
> -
> -	return fdt_size;
> +	return (unsigned int)(usm_entries * sizeof(u64));
>   }
>   
>   /**
>
Rob Herring (Arm) March 10, 2021, 2:08 a.m. UTC | #2
On Fri, Feb 19, 2021 at 6:52 PM Thiago Jung Bauermann
<bauerman@linux.ibm.com> wrote:
>
> Commit 2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump
> kernel") fixed how elf64_load() estimates the FDT size needed by the
> crashdump kernel.
>
> At the same time, commit 130b2d59cec0 ("powerpc: Use common
> of_kexec_alloc_and_setup_fdt()") changed the same code to use the generic
> function of_kexec_alloc_and_setup_fdt() to calculate the FDT size. That
> change made the code overestimate it a bit by counting twice the space
> required for the kernel command line and /chosen properties.
>
> Therefore change kexec_fdt_totalsize_ppc64() to calculate just the extra
> space needed by the kdump kernel, and change the function name so that it
> better reflects what the function is now doing.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
>  arch/powerpc/include/asm/kexec.h  |  2 +-
>  arch/powerpc/kexec/elf_64.c       |  2 +-
>  arch/powerpc/kexec/file_load_64.c | 26 ++++++++------------------
>  3 files changed, 10 insertions(+), 20 deletions(-)

I ended up delaying the referenced series til 5.13, but have applied
it now. Can I get an ack from the powerpc maintainers on this one?
I'll fixup the commit log to make sense given the commit id's aren't
valid.

Rob
Lakshmi Ramasubramanian March 10, 2021, 2:31 a.m. UTC | #3
On 3/9/21 6:08 PM, Rob Herring wrote:

Hi Rob,

> On Fri, Feb 19, 2021 at 6:52 PM Thiago Jung Bauermann
> <bauerman@linux.ibm.com> wrote:
>>
>> Commit 2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump
>> kernel") fixed how elf64_load() estimates the FDT size needed by the
>> crashdump kernel.
>>
>> At the same time, commit 130b2d59cec0 ("powerpc: Use common
>> of_kexec_alloc_and_setup_fdt()") changed the same code to use the generic
>> function of_kexec_alloc_and_setup_fdt() to calculate the FDT size. That
>> change made the code overestimate it a bit by counting twice the space
>> required for the kernel command line and /chosen properties.
>>
>> Therefore change kexec_fdt_totalsize_ppc64() to calculate just the extra
>> space needed by the kdump kernel, and change the function name so that it
>> better reflects what the function is now doing.
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>> ---
>>   arch/powerpc/include/asm/kexec.h  |  2 +-
>>   arch/powerpc/kexec/elf_64.c       |  2 +-
>>   arch/powerpc/kexec/file_load_64.c | 26 ++++++++------------------
>>   3 files changed, 10 insertions(+), 20 deletions(-)
> 
> I ended up delaying the referenced series til 5.13, but have applied
> it now. Can I get an ack from the powerpc maintainers on this one?
> I'll fixup the commit log to make sense given the commit id's aren't
> valid.

I checked the change applied in linux-next branch and also Device Tree's 
for-next branch - it looks like v1 of Thiago's patch has been applied. 
Could you please pick up the v2 patch?

thanks,
  -lakshmi
Rob Herring (Arm) March 10, 2021, 3:09 a.m. UTC | #4
On Tue, Mar 9, 2021 at 7:31 PM Lakshmi Ramasubramanian
<nramas@linux.microsoft.com> wrote:
>
> On 3/9/21 6:08 PM, Rob Herring wrote:
>
> Hi Rob,
>
> > On Fri, Feb 19, 2021 at 6:52 PM Thiago Jung Bauermann
> > <bauerman@linux.ibm.com> wrote:
> >>
> >> Commit 2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump
> >> kernel") fixed how elf64_load() estimates the FDT size needed by the
> >> crashdump kernel.
> >>
> >> At the same time, commit 130b2d59cec0 ("powerpc: Use common
> >> of_kexec_alloc_and_setup_fdt()") changed the same code to use the generic
> >> function of_kexec_alloc_and_setup_fdt() to calculate the FDT size. That
> >> change made the code overestimate it a bit by counting twice the space
> >> required for the kernel command line and /chosen properties.
> >>
> >> Therefore change kexec_fdt_totalsize_ppc64() to calculate just the extra
> >> space needed by the kdump kernel, and change the function name so that it
> >> better reflects what the function is now doing.
> >>
> >> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> >> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> >> ---
> >>   arch/powerpc/include/asm/kexec.h  |  2 +-
> >>   arch/powerpc/kexec/elf_64.c       |  2 +-
> >>   arch/powerpc/kexec/file_load_64.c | 26 ++++++++------------------
> >>   3 files changed, 10 insertions(+), 20 deletions(-)
> >
> > I ended up delaying the referenced series til 5.13, but have applied
> > it now. Can I get an ack from the powerpc maintainers on this one?
> > I'll fixup the commit log to make sense given the commit id's aren't
> > valid.
>
> I checked the change applied in linux-next branch and also Device Tree's
> for-next branch - it looks like v1 of Thiago's patch has been applied.
> Could you please pick up the v2 patch?

Huh? This patch (v2) hasn't been applied to any tree AFAICT.

Rob
Michael Ellerman March 10, 2021, 5:03 a.m. UTC | #5
Rob Herring <robh@kernel.org> writes:
> On Fri, Feb 19, 2021 at 6:52 PM Thiago Jung Bauermann
> <bauerman@linux.ibm.com> wrote:
>>
>> Commit 2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump
>> kernel") fixed how elf64_load() estimates the FDT size needed by the
>> crashdump kernel.
>>
>> At the same time, commit 130b2d59cec0 ("powerpc: Use common
>> of_kexec_alloc_and_setup_fdt()") changed the same code to use the generic
>> function of_kexec_alloc_and_setup_fdt() to calculate the FDT size. That
>> change made the code overestimate it a bit by counting twice the space
>> required for the kernel command line and /chosen properties.
>>
>> Therefore change kexec_fdt_totalsize_ppc64() to calculate just the extra
>> space needed by the kdump kernel, and change the function name so that it
>> better reflects what the function is now doing.
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>> ---
>>  arch/powerpc/include/asm/kexec.h  |  2 +-
>>  arch/powerpc/kexec/elf_64.c       |  2 +-
>>  arch/powerpc/kexec/file_load_64.c | 26 ++++++++------------------
>>  3 files changed, 10 insertions(+), 20 deletions(-)
>
> I ended up delaying the referenced series til 5.13, but have applied
> it now. Can I get an ack from the powerpc maintainers on this one?
> I'll fixup the commit log to make sense given the commit id's aren't
> valid.

Thanks for handling it.

Acked-by: Michael Ellerman <mpe@ellerman.id.au>


cheers
Rob Herring (Arm) March 11, 2021, 4:53 p.m. UTC | #6
On Fri, 19 Feb 2021 21:52:04 -0300, Thiago Jung Bauermann wrote:
> Commit 2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump
> kernel") fixed how elf64_load() estimates the FDT size needed by the
> crashdump kernel.
> 
> At the same time, commit 130b2d59cec0 ("powerpc: Use common
> of_kexec_alloc_and_setup_fdt()") changed the same code to use the generic
> function of_kexec_alloc_and_setup_fdt() to calculate the FDT size. That
> change made the code overestimate it a bit by counting twice the space
> required for the kernel command line and /chosen properties.
> 
> Therefore change kexec_fdt_totalsize_ppc64() to calculate just the extra
> space needed by the kdump kernel, and change the function name so that it
> better reflects what the function is now doing.
> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
>  arch/powerpc/include/asm/kexec.h  |  2 +-
>  arch/powerpc/kexec/elf_64.c       |  2 +-
>  arch/powerpc/kexec/file_load_64.c | 26 ++++++++------------------
>  3 files changed, 10 insertions(+), 20 deletions(-)
> 
> Applies on top of next-20210219.
> 
> Changes since v1:
> 
> - Adjusted comment describing kexec_extra_fdt_size_ppc64() as suggested
>   by Lakshmi.
> 

Applied, thanks!
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index baab158e215c..5a11cc8d2350 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -128,7 +128,7 @@  int load_crashdump_segments_ppc64(struct kimage *image,
 int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
 			  const void *fdt, unsigned long kernel_load_addr,
 			  unsigned long fdt_load_addr);
-unsigned int kexec_fdt_totalsize_ppc64(struct kimage *image);
+unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image);
 int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
 			unsigned long initrd_load_addr,
 			unsigned long initrd_len, const char *cmdline);
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 0492ca6003f3..5a569bb51349 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -104,7 +104,7 @@  static void *elf64_load(struct kimage *image, char *kernel_buf,
 
 	fdt = of_kexec_alloc_and_setup_fdt(image, initrd_load_addr,
 					   initrd_len, cmdline,
-					   kexec_fdt_totalsize_ppc64(image));
+					   kexec_extra_fdt_size_ppc64(image));
 	if (!fdt) {
 		pr_err("Error setting up the new device tree.\n");
 		ret = -EINVAL;
diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index 3609de30a170..297f73795a1f 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -927,37 +927,27 @@  int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
 }
 
 /**
- * kexec_fdt_totalsize_ppc64 - Return the estimated size needed to setup FDT
- *                             for kexec/kdump kernel.
- * @image:                     kexec image being loaded.
+ * kexec_extra_fdt_size_ppc64 - Return the estimated additional size needed to
+ *                              setup FDT for kexec/kdump kernel.
+ * @image:                      kexec image being loaded.
  *
- * Returns the estimated size needed for kexec/kdump kernel FDT.
+ * Returns the estimated extra size needed for kexec/kdump kernel FDT.
  */
-unsigned int kexec_fdt_totalsize_ppc64(struct kimage *image)
+unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image)
 {
-	unsigned int fdt_size;
 	u64 usm_entries;
 
-	/*
-	 * The below estimate more than accounts for a typical kexec case where
-	 * the additional space is to accommodate things like kexec cmdline,
-	 * chosen node with properties for initrd start & end addresses and
-	 * a property to indicate kexec boot..
-	 */
-	fdt_size = fdt_totalsize(initial_boot_params) + (2 * COMMAND_LINE_SIZE);
 	if (image->type != KEXEC_TYPE_CRASH)
-		return fdt_size;
+		return 0;
 
 	/*
-	 * For kdump kernel, also account for linux,usable-memory and
+	 * For kdump kernel, account for linux,usable-memory and
 	 * linux,drconf-usable-memory properties. Get an approximate on the
 	 * number of usable memory entries and use for FDT size estimation.
 	 */
 	usm_entries = ((memblock_end_of_DRAM() / drmem_lmb_size()) +
 		       (2 * (resource_size(&crashk_res) / drmem_lmb_size())));
-	fdt_size += (unsigned int)(usm_entries * sizeof(u64));
-
-	return fdt_size;
+	return (unsigned int)(usm_entries * sizeof(u64));
 }
 
 /**