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 <[email protected]> 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 <[email protected]> wrote:
>
> 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?
--
Cheers,
Stephen Rothwell
On Thu, Feb 18, 2021 at 5:34 AM Stephen Rothwell <[email protected]> wrote:
>
> Hi Michael,
>
> On Thu, 18 Feb 2021 21:44:37 +1100 Michael Ellerman <[email protected]> wrote:
> >
> > 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 <[email protected]> wrote:
>
> On Thu, Feb 18, 2021 at 5:34 AM Stephen Rothwell <[email protected]> wrote:
> >
> > On Thu, 18 Feb 2021 21:44:37 +1100 Michael Ellerman <[email protected]> wrote:
> > >
> > > 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 <[email protected]> writes:
> On Thu, Feb 18, 2021 at 5:34 AM Stephen Rothwell <[email protected]> wrote:
>> On Thu, 18 Feb 2021 21:44:37 +1100 Michael Ellerman <[email protected]> wrote:
>> >
>> > 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