linux-next: manual merge of the devicetree tree with the powerpc tree

Hi all,

Today's linux-next merge of the devicetree tree got a conflict in:

  arch/powerpc/kexec/elf_64.c

between commit:

  2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump kernel")

from the powerpc tree and commit:

  130b2d59cec0 ("powerpc: Use common of_kexec_alloc_and_setup_fdt()")

from the devicetree tree.

I can't easily see how to resolve these, so for now I have just used
the latter' changes to this file.

I fixed it up and can carry the fix as necessary. This is now fixed as
far as linux-next is concerned, but any non trivial conflicts should be
mentioned to your upstream maintainer when your tree is submitted for
merging.  You may also want to consider cooperating with the maintainer
of the conflicting tree to minimise any particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

Stephen Rothwell [off-list ref] writes:

Hi all,

Today's linux-next merge of the devicetree tree got a conflict in:

  arch/powerpc/kexec/elf_64.c

between commit:

  2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump kernel")

from the powerpc tree and commit:

  130b2d59cec0 ("powerpc: Use common of_kexec_alloc_and_setup_fdt()")

from the devicetree tree.

I can't easily see how to resolve these, so for now I have just used
the latter' changes to this file.

I think it just needs this?

diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 87e34611f93d..0492ca6003f3 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,
-					   fdt_totalsize(initial_boot_params));
+					   kexec_fdt_totalsize_ppc64(image));
 	if (!fdt) {
 		pr_err("Error setting up the new device tree.\n");
 		ret = -EINVAL;

cheers

Hi Michael,

On Thu, 18 Feb 2021 21:44:37 +1100 Michael Ellerman [off-list ref] wrote:

I thought about that, but the last argument to
of_kexec_alloc_and_setup_fdt() is extra_fdt_size and the allocation
done is for this:

fdt_size = fdt_totalsize(initial_boot_params) +
                   (cmdline ? strlen(cmdline) : 0) +
                   FDT_EXTRA_SPACE +
                   extra_fdt_size;

and kexec_fdt_totalsize_ppc64() also includes
fdt_totalsize(initial_boot_params) so I was not sure.  Maybe
kexec_fdt_totalsize_ppc64() needs modification as well?

-- 
Cheers,
Stephen Rothwell

On Thu, Feb 18, 2021 at 5:34 AM Stephen Rothwell [off-list ref] wrote:

Hi Michael,

On Thu, 18 Feb 2021 21:44:37 +1100 Michael Ellerman [off-list ref] wrote:

quoted

I think it just needs this?

diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 87e34611f93d..0492ca6003f3 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,
-                                        fdt_totalsize(initial_boot_params));
+                                        kexec_fdt_totalsize_ppc64(image));
      if (!fdt) {
              pr_err("Error setting up the new device tree.\n");
              ret = -EINVAL;

I thought about that, but the last argument to
of_kexec_alloc_and_setup_fdt() is extra_fdt_size and the allocation
done is for this:

fdt_size = fdt_totalsize(initial_boot_params) +
                   (cmdline ? strlen(cmdline) : 0) +
                   FDT_EXTRA_SPACE +
                   extra_fdt_size;

and kexec_fdt_totalsize_ppc64() also includes
fdt_totalsize(initial_boot_params) so I was not sure.  Maybe
kexec_fdt_totalsize_ppc64() needs modification as well?

You're both right. Michael's fix is sufficient for the merge. The only
risk with a larger size is failing to allocate it, but we're talking
only 10s of KB. Historically until the commit causing the conflict,
PPC was just used 2x fdt_totalsize(initial_boot_params). You could
drop 'fdt_size = fdt_totalsize(initial_boot_params) + (2 *
COMMAND_LINE_SIZE);' from kexec_fdt_totalsize_ppc64() as well, but
then the function name is misleading.

Lakshmi can send a follow-up patch to fine tune the size and rename
kexec_fdt_totalsize_ppc64.

Rob

Hi all,

On Thu, 18 Feb 2021 07:52:52 -0600 Rob Herring [off-list ref] wrote:

On Thu, Feb 18, 2021 at 5:34 AM Stephen Rothwell [off-list ref] wrote:

quoted

On Thu, 18 Feb 2021 21:44:37 +1100 Michael Ellerman [off-list ref] wrote:  

quoted

I think it just needs this?

diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 87e34611f93d..0492ca6003f3 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,
-                                        fdt_totalsize(initial_boot_params));
+                                        kexec_fdt_totalsize_ppc64(image));
      if (!fdt) {
              pr_err("Error setting up the new device tree.\n");
              ret = -EINVAL;
 

I thought about that, but the last argument to
of_kexec_alloc_and_setup_fdt() is extra_fdt_size and the allocation
done is for this:

fdt_size = fdt_totalsize(initial_boot_params) +
                   (cmdline ? strlen(cmdline) : 0) +
                   FDT_EXTRA_SPACE +
                   extra_fdt_size;

and kexec_fdt_totalsize_ppc64() also includes
fdt_totalsize(initial_boot_params) so I was not sure.  Maybe
kexec_fdt_totalsize_ppc64() needs modification as well?  

You're both right. Michael's fix is sufficient for the merge. The only
risk with a larger size is failing to allocate it, but we're talking
only 10s of KB. Historically until the commit causing the conflict,
PPC was just used 2x fdt_totalsize(initial_boot_params). You could
drop 'fdt_size = fdt_totalsize(initial_boot_params) + (2 *
COMMAND_LINE_SIZE);' from kexec_fdt_totalsize_ppc64() as well, but
then the function name is misleading.

Lakshmi can send a follow-up patch to fine tune the size and rename
kexec_fdt_totalsize_ppc64.

OK, I have mode Michael's suggested change to my resolution from today.

-- 
Cheers,
Stephen Rothwell

Rob Herring [off-list ref] writes:

On Thu, Feb 18, 2021 at 5:34 AM Stephen Rothwell [off-list ref] wrote:

quoted

On Thu, 18 Feb 2021 21:44:37 +1100 Michael Ellerman [off-list ref] wrote:

quoted

I think it just needs this?

diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 87e34611f93d..0492ca6003f3 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,
-                                        fdt_totalsize(initial_boot_params));
+                                        kexec_fdt_totalsize_ppc64(image));
      if (!fdt) {
              pr_err("Error setting up the new device tree.\n");
              ret = -EINVAL;

I thought about that, but the last argument to
of_kexec_alloc_and_setup_fdt() is extra_fdt_size and the allocation
done is for this:

fdt_size = fdt_totalsize(initial_boot_params) +
                   (cmdline ? strlen(cmdline) : 0) +
                   FDT_EXTRA_SPACE +
                   extra_fdt_size;

and kexec_fdt_totalsize_ppc64() also includes
fdt_totalsize(initial_boot_params) so I was not sure.  Maybe
kexec_fdt_totalsize_ppc64() needs modification as well?

You're both right. Michael's fix is sufficient for the merge. The only
risk with a larger size is failing to allocate it, but we're talking
only 10s of KB. Historically until the commit causing the conflict,
PPC was just used 2x fdt_totalsize(initial_boot_params). You could
drop 'fdt_size = fdt_totalsize(initial_boot_params) + (2 *
COMMAND_LINE_SIZE);' from kexec_fdt_totalsize_ppc64() as well, but
then the function name is misleading.

Lakshmi can send a follow-up patch to fine tune the size and rename
kexec_fdt_totalsize_ppc64.

Sounds good.

cheers