Re: [PATCH] kexec/dt-ops.c: Fix check against 'fdt_add_subnode' return value

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks for the feedback.

On Fri, Nov 30, 2018 at 8:29 PM Vicente Bergas <vicencb@xxxxxxxxx> wrote:
>
> On Fri, Nov 30, 2018 at 6:41 AM Bhupesh Sharma <bhsharma@xxxxxxxxxx> wrote:
> >
> > Hi Vicenç,
> >
> > I have posted a revised and more comprehensive patchset for this issue
> > today (please see the same here:
> > <http://lists.infradead.org/pipermail/kexec/2018-November/021999.html>)
> >
> > I was able to exercise all the basic 'kexe load' and 'kdump' use cases
> > on arm64 after these changes and I can also get the use-cases where we
> > don't have a '/chosen' node in the .dtb to either PASS or FAIL with
> > correct error reporting. Here is a listing of the basic use cases and
> > their results:
> >
> > # kexec -d -l Image --append=debug --dtb rk3399-sapphire.dtb
> > ok
> >
> > # kexec -d -l Image --reuse-cmdline --initrd=/boot/initramfs-`uname -r`.img
> > ok
> >
> > # kexec -d -p Image --reuse-cmdline --initrd=/boot/initramfs-`uname -r`.img
> > ok
> >
> > # kexec -d -p Image --append=debug --dtb rk3399-sapphire.dtb
> > <..snip..>
> > setup_2nd_dtb: /chosen node not found - can't create dtb for dump
> > kernel: FDT_ERR_NOTFOUND
> > kexec: setup_2nd_dtb failed.
> > kexec: load failed.
> > Cannot load Image
> >
> > Can you please try and test this patchset on your setup and let me
> > know in case you land in any further issues?
>
> Hi Bhupesh,
> thank you, this is an improvement.
> Unfortunately there is a remaining issue with the initrd option and
> without the reuse-cmdline option:
> kexec -d -l /boot/Image --dtb /boot/dtbs/rockchip/rk3399-sapphire.dtb
> --initrd /boot/initramfs-linux.img
> ...
> get_cells_size: #address-cells:2 #size-cells:2
> cells_size_fitted: 0-0
> cells_size_fitted: 0-0
> setup_2nd_dtb: no kaslr-seed found
> initrd: base 1d36000, size 5b3d3dh (5979453)
> dtb_set_initrd: start 30629888, end 36609341, size 5979453 (5839 KiB)
> dtb_set_property: fdt_add_subnode failed: FDT_ERR_EXISTS
> dtb_delete_property: fdt_path_offset failed: FDT_ERR_NOTFOUND
> kexec: load failed.
> Cannot load /boot/Image
>
> In any case, do not worry and take your time, this is not a blocker issue.

I remember testing this use case as well (but perhaps not with the
rockchip dtb) and did not meet this issue.
Let me recheck with the rockchip dtb as well and will send a v2
patchset if a fix is needed.

But its good to know that the rest of the issues are fixed.

Thanks,
Bhupesh

>
> > Thanks,
> > Bhupesh
> >
> >
> > On Thu, Nov 29, 2018 at 3:30 AM Bhupesh Sharma <bhsharma@xxxxxxxxxx> wrote:
> > >
> > > Hi Vicenç,
> > >
> > > Sorry I got busy and just got some spare time to look at this issue. I
> > > am able to fix it partially, but I am still not completely happy with
> > > it, but mainly here is the issue for the 'kexec load' option - we are
> > > passing wrong 'nodeoffset' while calling 'fdt_add_subnode()', which
> > > should be 0 instead of FDT_ERR_NOTFOUND when the chosen node is not
> > > seen in the passed dtb :
> > >
> > > From 2433daf279744ae30fa9c74ec5b39507740d7163 Mon Sep 17 00:00:00 2001
> > > From: Bhupesh Sharma <bhsharma@xxxxxxxxxx>
> > > Date: Thu, 29 Nov 2018 00:41:06 +0530
> > > Subject: [PATCH] Add some debug logs
> > >
> > > Signed-off-by: Bhupesh Sharma <bhsharma@xxxxxxxxxx>
> > > ---
> > >  kexec/arch/arm64/kexec-arm64.c |  8 ++++++++
> > >  kexec/dt-ops.c                 |  3 ++-
> > >  kexec/libfdt/fdt_ro.c          | 14 +++++++-------
> > >  kexec/libfdt/fdt_rw.c          |  2 ++
> > >  4 files changed, 19 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> > > index 6c7d2512b1d4..e29afc30db43 100644
> > > --- a/kexec/arch/arm64/kexec-arm64.c
> > > +++ b/kexec/arch/arm64/kexec-arm64.c
> > > @@ -207,6 +207,9 @@ static int set_bootargs(struct dtb *dtb, const
> > > char *command_line)
> > >      if (!command_line || !command_line[0])
> > >          return 0;
> > >
> > > +    dbgprintf("%s: found %s\n", __func__, dtb->path);
> > > +    dump_fdt(dtb->buf);
> > > +
> > >      result = dtb_set_bootargs(&dtb->buf, &dtb->size, command_line);
> > >
> > >      if (result) {
> > > @@ -413,6 +416,11 @@ static int setup_2nd_dtb(struct dtb *dtb, char
> > > *command_line, int on_crash)
> > >      }
> > >
> > >      result = set_bootargs(dtb, command_line);
> > > +    if (result) {
> > > +        fprintf(stderr, "kexec: cannot set bootargs.\n");
> > > +        result = -EINVAL;
> > > +        goto on_error;
> > > +    }
> > >
> > >      /* determine #address-cells and #size-cells */
> > >      result = get_cells_size(dtb->buf, &address_cells, &size_cells);
> > > diff --git a/kexec/dt-ops.c b/kexec/dt-ops.c
> > > index a0276f1e48b3..79b0404bafc4 100644
> > > --- a/kexec/dt-ops.c
> > > +++ b/kexec/dt-ops.c
> > > @@ -86,13 +86,14 @@ int dtb_set_property(char **dtb, off_t *dtb_size,
> > > const char *node,
> > >      nodeoffset = fdt_path_offset(new_dtb, node);
> > >
> > >      if (nodeoffset == -FDT_ERR_NOTFOUND) {
> > > -        result = fdt_add_subnode(new_dtb, nodeoffset, node);
> > > +        result = fdt_add_subnode(new_dtb, 0, node);
> > >
> > >          if (result < 0) {
> > >              dbgprintf("%s: fdt_add_subnode failed: %s\n", __func__,
> > >                  fdt_strerror(result));
> > >              goto on_error;
> > >          }
> > > +        nodeoffset = result;
> > >      } else if (nodeoffset < 0) {
> > >          dbgprintf("%s: fdt_path_offset failed: %s\n", __func__,
> > >              fdt_strerror(nodeoffset));
> > > diff --git a/kexec/libfdt/fdt_ro.c b/kexec/libfdt/fdt_ro.c
> > > index 129b532bcc1a..0dce53083c96 100644
> > > --- a/kexec/libfdt/fdt_ro.c
> > > +++ b/kexec/libfdt/fdt_ro.c
> > > @@ -105,14 +105,14 @@ int fdt_subnode_offset_namelen(const void *fdt,
> > > int offset,
> > >      FDT_CHECK_HEADER(fdt);
> > >
> > >      for (depth = 0;
> > > -         offset >= 0;
> > > -         offset = fdt_next_node(fdt, offset, &depth)) {
> > > -        if (depth < 0)
> > > -            return -FDT_ERR_NOTFOUND;
> > > -        else if ((depth == 1)
> > > -             && _fdt_nodename_eq(fdt, offset, name, namelen))
> > > +         (offset >= 0) && (depth >= 0);
> > > +         offset = fdt_next_node(fdt, offset, &depth))
> > > +        if ((depth == 1)
> > > +            && _fdt_nodename_eq(fdt, offset, name, namelen))
> > >              return offset;
> > > -    }
> > > +
> > > +    if (depth < 0)
> > > +        return -FDT_ERR_NOTFOUND;
> > >
> > >      return offset; /* error */
> > >  }
> > > diff --git a/kexec/libfdt/fdt_rw.c b/kexec/libfdt/fdt_rw.c
> > > index 8e7ec4cb7bcd..2fac7c0731c0 100644
> > > --- a/kexec/libfdt/fdt_rw.c
> > > +++ b/kexec/libfdt/fdt_rw.c
> > > @@ -101,6 +101,8 @@ static int _fdt_splice(void *fdt, void
> > > *splicepoint, int oldlen, int newlen)
> > >
> > >      if (((p + oldlen) < p) || ((p + oldlen) > end))
> > >          return -FDT_ERR_BADOFFSET;
> > > +    if ((p < (char *)fdt) || ((end - oldlen + newlen) < (char *)fdt))
> > > +        return -FDT_ERR_BADOFFSET;
> > >      if ((end - oldlen + newlen) > ((char *)fdt + fdt_totalsize(fdt)))
> > >          return -FDT_ERR_NOSPACE;
> > >      memmove(p + newlen, p + oldlen, end - p - oldlen);
> > > --
> > > 2.7.4
> > >
> > > Here are some logs at my end after this fix (I used some earlier work
> > > I did for dump'ing dtb on arm64 boards to get the dtb logs:
> > > <https://www.spinics.net/lists/kexec/msg20951.html>):
> > >
> > > # kexec -d -l /root/Image --append 'debug' --dtb /root/rk3399-sapphire.dtb
> > > <..snip..>
> > >
> > > image_arm64_load: PE format:      yes
> > >  / {
> > >     compatible = "rockchip,rk3399-sapphire";
> > >     interrupt-parent = <0x00000001>;
> > >     #address-cells = <0x00000002>;
> > >     #size-cells = <0x00000002>;
> > >     model = "Sapphire-RK3399 Board";
> > >     /chosen {
> > >         bootargs = "debug";
> > >     };
> > >     aliases {
> > >
> > >  <..snip..>
> > >  };
> > > get_cells_size: #address-cells:2 #size-cells:2
> > > cells_size_fitted: 0-0
> > > cells_size_fitted: 0-0
> > > setup_2nd_dtb: no kaslr-seed found
> > > dtb:    base 81ef0000, size d0a1h (53409)
> > > sym: sha256_starts info: 12 other: 00 shndx: 1 value: ec0 size: 6c
> > > sym: sha256_starts value: 81f00ec0 addr: 81f00018
> > > machine_apply_elf_rel: CALL26 580006b394000000->580006b3940003aa
> > > sym: sha256_update info: 12 other: 00 shndx: 1 value: 5168 size: c
> > > sym: sha256_update value: 81f05168 addr: 81f00034
> > > machine_apply_elf_rel: CALL26 9100427394000000->910042739400144d
> > > sym: sha256_finish info: 12 other: 00 shndx: 1 value: 5174 size: 1cc
> > > sym: sha256_finish value: 81f05174 addr: 81f00050
> > > machine_apply_elf_rel: CALL26 aa1403e094000000->aa1403e094001449
> > > sym:     memcmp info: 12 other: 00 shndx: 1 value: 64c size: 34
> > > sym: memcmp value: 81f0064c addr: 81f00060
> > > machine_apply_elf_rel: CALL26 340003c094000000->340003c09400017b
> > > sym:     printf info: 12 other: 00 shndx: 1 value: 55c size: 90
> > > sym: printf value: 81f0055c addr: 81f00070
> > > machine_apply_elf_rel: CALL26 5800046094000000->580004609400013b
> > > sym:     printf info: 12 other: 00 shndx: 1 value: 55c size: 90
> > > sym: printf value: 81f0055c addr: 81f00078
> > > machine_apply_elf_rel: CALL26 5800047594000000->5800047594000139
> > > sym:     printf info: 12 other: 00 shndx: 1 value: 55c size: 90
> > > sym: printf value: 81f0055c addr: 81f00088
> > > machine_apply_elf_rel: CALL26 9100067394000000->9100067394000135
> > > sym:     printf info: 12 other: 00 shndx: 1 value: 55c size: 90
> > > sym: printf value: 81f0055c addr: 81f000a8
> > > machine_apply_elf_rel: CALL26 5800036094000000->580003609400012d
> > > sym:     printf info: 12 other: 00 shndx: 1 value: 55c size: 90
> > > sym: printf value: 81f0055c addr: 81f000b0
> > > machine_apply_elf_rel: CALL26 910402e194000000->910402e19400012b
> > > sym:     printf info: 12 other: 00 shndx: 1 value: 55c size: 90
> > > sym: printf value: 81f0055c addr: 81f000c0
> > > machine_apply_elf_rel: CALL26 9100067394000000->9100067394000127
> > > sym:     printf info: 12 other: 00 shndx: 1 value: 55c size: 90
> > > sym: printf value: 81f0055c addr: 81f000d4
> > > machine_apply_elf_rel: CALL26 5280002094000000->5280002094000122
> > > sym:      .data info: 03 other: 00 shndx: 4 value: 0 size: 0
> > > sym: .data value: 81f053b8 addr: 81f000f0
> > > machine_apply_elf_rel: ABS64 0000000000000000->0000000081f053b8
> > > sym: .rodata.str1.1 info: 03 other: 00 shndx: 3 value: 0 size: 0
> > > sym: .rodata.str1.1 value: 81f05348 addr: 81f000f8
> > > machine_apply_elf_rel: ABS64 0000000000000000->0000000081f05348
> > > sym: .rodata.str1.1 info: 03 other: 00 shndx: 3 value: 0 size: 0
> > > sym: .rodata.str1.1 value: 81f05368 addr: 81f00100
> > > machine_apply_elf_rel: ABS64 0000000000000000->0000000081f05368
> > > sym: .rodata.str1.1 info: 03 other: 00 shndx: 3 value: 0 size: 0
> > > sym: .rodata.str1.1 value: 81f05378 addr: 81f00108
> > > machine_apply_elf_rel: ABS64 0000000000000000->0000000081f05378
> > > sym: .rodata.str1.1 info: 03 other: 00 shndx: 3 value: 0 size: 0
> > > sym: .rodata.str1.1 value: 81f0537e addr: 81f00110
> > > machine_apply_elf_rel: ABS64 0000000000000000->0000000081f0537e
> > > sym: .rodata.str1.1 info: 03 other: 00 shndx: 3 value: 0 size: 0
> > > sym: .rodata.str1.1 value: 81f05380 addr: 81f00118
> > > machine_apply_elf_rel: ABS64 0000000000000000->0000000081f05380
> > > sym:     printf info: 12 other: 00 shndx: 1 value: 55c size: 90
> > > sym: printf value: 81f0055c addr: 81f0012c
> > > machine_apply_elf_rel: CALL26 9400000094000000->940000009400010c
> > > sym: setup_arch info: 12 other: 00 shndx: 1 value: eb8 size: 4
> > > sym: setup_arch value: 81f00eb8 addr: 81f00130
> > > machine_apply_elf_rel: CALL26 5800016094000000->5800016094000362
> > > sym: verify_sha256_digest info: 12 other: 00 shndx: 1 value: 0 size: f0
> > > sym: verify_sha256_digest value: 81f00000 addr: 81f00140
> > > machine_apply_elf_rel: CALL26 3400004094000000->3400004097ffffb0
> > > sym: post_verification_setup_arch info: 12 other: 00 shndx: 1 value: eb4 size: 4
> > > sym: post_verification_setup_arch value: 81f00eb4 addr: 81f00150
> > > machine_apply_elf_rel: JUMP26 d503201f14000000->d503201f14000359
> > > sym: .rodata.str1.1 info: 03 other: 00 shndx: 3 value: 0 size: 0
> > > sym: .rodata.str1.1 value: 81f05390 addr: 81f00158
> > > machine_apply_elf_rel: ABS64 0000000000000000->0000000081f05390
> > > sym:      .data info: 03 other: 00 shndx: 4 value: 0 size: 0
> > > sym: .data value: 81f053b8 addr: 81f00160
> > > machine_apply_elf_rel: ABS64 0000000000000000->0000000081f053b8
> > > sym:    putchar info: 12 other: 00 shndx: 1 value: eb0 size: 4
> > > sym: putchar value: 81f00eb0 addr: 81f001c4
> > > machine_apply_elf_rel: CALL26 f94037a194000000->f94037a19400033b
> > > sym:    putchar info: 12 other: 00 shndx: 1 value: eb0 size: 4
> > > sym: putchar value: 81f00eb0 addr: 81f00238
> > > machine_apply_elf_rel: CALL26 910006f794000000->910006f79400031e
> > > sym:    putchar info: 12 other: 00 shndx: 1 value: eb0 size: 4
> > > sym: putchar value: 81f00eb0 addr: 81f00490
> > > machine_apply_elf_rel: CALL26 9100073994000000->9100073994000288
> > > sym: .rodata.str1.1 info: 03 other: 00 shndx: 3 value: 0 size: 0
> > > sym: .rodata.str1.1 value: 81f053a2 addr: 81f004d0
> > > machine_apply_elf_rel: ABS64 0000000000000000->0000000081f053a2
> > > sym:   vsprintf info: 12 other: 00 shndx: 1 value: 168 size: 364
> > > sym: vsprintf value: 81f00168 addr: 81f00550
> > > machine_apply_elf_rel: CALL26 a8d07bfd94000000->a8d07bfd97ffff06
> > > sym:   vsprintf info: 12 other: 00 shndx: 1 value: 168 size: 364
> > > sym: vsprintf value: 81f00168 addr: 81f005e0
> > > machine_apply_elf_rel: CALL26 a8d17bfd94000000->a8d17bfd97fffee2
> > > sym:  purgatory info: 12 other: 00 shndx: 1 value: 120 size: 34
> > > sym: purgatory value: 81f00120 addr: 81f00688
> > > machine_apply_elf_rel: CALL26 5800001194000000->5800001197fffea6
> > > sym: arm64_kernel_entry info: 10 other: 00 shndx: 4 value: 128 size: 8
> > > sym: arm64_kernel_entry value: 81f054e0 addr: 81f0068c
> > > machine_apply_elf_rel: LD_PREL_LO19 5800000058000011->58000000580272b1
> > > sym: arm64_dtb_addr info: 10 other: 00 shndx: 4 value: 130 size: 8
> > > sym: arm64_dtb_addr value: 81f054e8 addr: 81f00690
> > > machine_apply_elf_rel: LD_PREL_LO19 aa1f03e158000000->aa1f03e1580272c0
> > > sym: sha256_process info: 12 other: 00 shndx: 1 value: f2c size: 4134
> > > sym: sha256_process value: 81f00f2c addr: 81f050cc
> > > machine_apply_elf_rel: CALL26 d101029494000000->d101029497ffef98
> > > sym:     memcpy info: 12 other: 00 shndx: 1 value: 62c size: 20
> > > sym: memcpy value: 81f0062c addr: 81f05128
> > > machine_apply_elf_rel: JUMP26 b4fffc5814000000->b4fffc5817ffed41
> > > sym:     memcpy info: 12 other: 00 shndx: 1 value: 62c size: 20
> > > sym: memcpy value: 81f0062c addr: 81f05140
> > > machine_apply_elf_rel: CALL26 aa1503e094000000->aa1503e097ffed3b
> > > sym: sha256_process info: 12 other: 00 shndx: 1 value: f2c size: 4134
> > > sym: sha256_process value: 81f00f2c addr: 81f0514c
> > > machine_apply_elf_rel: CALL26 cb1302d694000000->cb1302d697ffef78
> > > sym:      .data info: 03 other: 00 shndx: 4 value: 0 size: 0
> > > sym: .data value: 81f054f0 addr: 81f05340
> > > machine_apply_elf_rel: ABS64 0000000000000000->0000000081f054f0
> > > kexec_load: entry = 0x81f00680 flags = 0xb70000
> > > nr_segments = 3
> > > segment[0].buf   = 0xffff8bab0010
> > > segment[0].bufsz = 0x1999a00
> > > segment[0].mem   = 0x80480000
> > > segment[0].memsz = 0x1a70000
> > > segment[1].buf   = 0x16984e10
> > > segment[1].bufsz = 0xd0a1
> > > segment[1].mem   = 0x81ef0000
> > > segment[1].memsz = 0x10000
> > > segment[2].buf   = 0x16992230
> > > segment[2].bufsz = 0x5530
> > > segment[2].mem   = 0x81f00000
> > > segment[2].memsz = 0x10000
> > >
> > > I will try to stabilize this more both for 'kexec load' and 'kexec
> > > dump' use-cases and try to send a patch soon to fix this issue.
> > >
> > > Regards,
> > > Bhupesh
> > > On Fri, Nov 16, 2018 at 8:59 PM Vicente Bergas <vicencb@xxxxxxxxx> wrote:
> > > >
> > > > On Fri, Nov 16, 2018 at 1:04 PM Bhupesh Sharma <bhsharma@xxxxxxxxxx> wrote:
> > > > >
> > > > > Hi Vicenç,
> > > > >
> > > > > I tried the patch at my end and it works fine as per my checks.
> > > > > So, can I request you to provide some additional data which will help
> > > > > me understand your environment better. See inline:
> > > > >
> > > > > On Fri, Nov 16, 2018 at 12:08 AM Vicente Bergas <vicencb@xxxxxxxxx> wrote:
> > > > > >
> > > > > > On Thu, Nov 15, 2018 at 3:14 PM Simon Horman <horms@xxxxxxxxxxxx> wrote:
> > > > > > >
> > > > > > > On Thu, Nov 15, 2018 at 06:10:36AM -0800, Simon Horman wrote:
> > > > > > > > On Mon, Nov 12, 2018 at 11:24:07AM +0900, AKASHI Takahiro wrote:
> > > > > > > > > On Mon, Nov 12, 2018 at 02:00:16AM +0530, Bhupesh Sharma wrote:
> > > > > > > > > > Vicenç reported (via [1]) that currently executing kexec with
> > > > > > > > > > '--dtb' option and passing a .dtb which doesn't have a '/chosen' node
> > > > > > > > > > leads to the following error:
> > > > > > > > > >
> > > > > > > > > >   # kexec -d --dtb dtb_without_chosen_node.dtb --append 'cmdline' --load Image
> > > > > > > > > >
> > > > > > > > > >     dtb_set_property: fdt_add_subnode failed: <valid offset/length>
> > > > > > > > > >     kexec: Set device tree bootargs failed.
> > > > > > > > > >
> > > > > > > > > > This happens because currently we check the return value of
> > > > > > > > > > 'fdt_add_subnode()' function call in 'dt-ops.c' incorrectly:
> > > > > > > > > >
> > > > > > > > > >    result = fdt_add_subnode(new_dtb, nodeoffset, node);
> > > > > > > > > >    if (result) {
> > > > > > > > > >           dbgprintf("%s: fdt_add_subnode failed: %s\n", _func__,
> > > > > > > > > >                   fdt_strerror(result));
> > > > > > > > > >           goto on_error;
> > > > > > > > > >    }
> > > > > > > > > >
> > > > > > > > > > As we can see in 'fdt_rw.c', a positive return value from
> > > > > > > > > > 'fdt_add_subnode()' function doesn't indicate an error.
> > > > > > > > > >
> > > > > > > > > > We can see that the Linux kernel (see 'drivers/firmware/efi/libstub/fdt.c'
> > > > > > > > > > for example) also checks the 'fdt_add_subnode()' function against negative
> > > > > > > > > > return values for errors. See an example below from 'update_fdt()' function in
> > > > > > > > > > 'drivers/firmware/efi/libstub/fdt.c':
> > > > > > > > > >
> > > > > > > > > >    node = fdt_add_subnode(fdt, 0, "chosen");
> > > > > > > > > >    if (node < 0) {
> > > > > > > > > >           status = node;
> > > > > > > > > >           <..snip..>
> > > > > > > > > >           goto fdt_set_fail;
> > > > > > > > > >    }
> > > > > > > > > >
> > > > > > > > > > This patch fixes the same in 'kexec-tools'.
> > > > > > > > >
> > > > > > > > > Looks good.
> > > > > > > > > Thank you.
> > > > > > > > > -Takahiro Akashi
> > > > > > > >
> > > > > > > > Thanks, applied.
> > > > > > >
> > > > > > > Perhaps I was a little too hasty there, I have dropped the patch for now.
> > > > > > >
> > > > > > > Vicente, does this patch work for you?
> > > > > >
> > > > > > The patch looks fine, but the end result is still non-working:
> > > > > >   # kexec -d --load Image --append 'debug' --dtb no_chosen.dtb
> > > > >
> > > > > Can you share the .dts with which you see this issue and steps you use
> > > > > to create the .dtb file at your end?
> > > > >
> > > > > I normally use at my end the following command:
> > > > > dtc -o no-chosen.dtb -I dts -O dtb no-chosen.dts
> > > > >
> > > > > Thanks,
> > > > > Bhupesh
> > > >
> > > > Hi Bhupesh,
> > > > the issue is not with dtc, it is with kexec. The following script
> > > > reproduces the issue:
> > > > #!/bin/sh -ve
> > > >
> > > > uname -m
> > > > # aarch64
> > > >
> > > > git clone 'https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git'
> > > > kexec-tools-orig
> > > > cp -a kexec-tools-orig kexec-tools-test
> > > > git -C kexec-tools-orig checkout -b orig 'v2.0.18'
> > > > git -C kexec-tools-test checkout -b test 'v2.0.18'
> > > >
> > > > cat patch.diff
> > > > #--- a/kexec/dt-ops.c
> > > > #+++ b/kexec/dt-ops.c
> > > > #@@ -84,7 +84,7 @@ int dtb_set_property(char **dtb, off_t *dtb_size,
> > > > const char *node,
> > > > #     if (nodeoffset == -FDT_ERR_NOTFOUND) {
> > > > #         result = fdt_add_subnode(new_dtb, nodeoffset, node);
> > > > #
> > > > #-        if (result) {
> > > > #+        if (result < 0) {
> > > > #             dbgprintf("%s: fdt_add_subnode failed: %s\n", __func__,
> > > > #                 fdt_strerror(result));
> > > > #             goto on_error;
> > > > patch -d kexec-tools-test -p1 < patch.diff
> > > >
> > > > ( cd kexec-tools-orig ; ./bootstrap )
> > > > ( cd kexec-tools-test ; ./bootstrap )
> > > > ( cd kexec-tools-orig ; ./configure )
> > > > ( cd kexec-tools-test ; ./configure )
> > > > make -C kexec-tools-orig
> > > > make -C kexec-tools-test
> > > >
> > > > wget 'http://mirror.archlinuxarm.org/aarch64/core/linux-aarch64-4.19.2-1-aarch64.pkg.tar.xz'
> > > > bsdtar xf 'linux-aarch64-4.19.2-1-aarch64.pkg.tar.xz'
> > > >
> > > > sudo kexec-tools-orig/build/sbin/kexec -d --load boot/Image --append
> > > > 'debug' --dtb boot/dtbs/rockchip/rk3399-sapphire.dtb
> > > > # dtb_set_property: fdt_add_subnode failed: <valid offset/length>
> > > > # kexec: Set device tree bootargs failed.
> > > >
> > > > sudo kexec-tools-test/build/sbin/kexec -d --load boot/Image --append
> > > > 'debug' --dtb boot/dtbs/rockchip/rk3399-sapphire.dtb
> > > > # dtb_set_property: fdt_setprop failed: FDT_ERR_BADOFFSET
> > > > # kexec: Set device tree bootargs failed.
> > > >
> > > > Regards,
> > > >   Vicenç.
> > > >
> > > > > >   ...
> > > > > >   dtb_set_property: fdt_setprop failed: FDT_ERR_BADOFFSET
> > > > > >   kexec: Set device tree bootargs failed.
> > > > > >
> > > > > > Also tested this with the same result:
> > > > > > --- a/kexec/dt-ops.c
> > > > > > +++ b/kexec/dt-ops.c
> > > > > > @@ -84,11 +84,13 @@
> > > > > >         if (nodeoffset == -FDT_ERR_NOTFOUND) {
> > > > > >                 result = fdt_add_subnode(new_dtb, nodeoffset, node);
> > > > > >
> > > > > > -               if (result) {
> > > > > > +               if (result < 0) {
> > > > > >                         dbgprintf("%s: fdt_add_subnode failed: %s\n", __func__,
> > > > > >                                 fdt_strerror(result));
> > > > > >                         goto on_error;
> > > > > >                 }
> > > > > > +
> > > > > > +               nodeoffset = result;
> > > > > >         } else if (nodeoffset < 0) {
> > > > > >                 dbgprintf("%s: fdt_path_offset failed: %s\n", __func__,
> > > > > >                         fdt_strerror(nodeoffset));
> > > > > >
> > > > > > Regards,
> > > > > >   Vicenç.

_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux