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
