[BUG] Set device tree bootargs failed when DTB does not contain a chosen node.

[BUG] Set device tree bootargs failed when DTB does not contain a chosen node.

[Vicente Bergas]

Hello,
when executing
  kexec -d --dtb dtb_without_chosen_node.dtb --append 'cmdline' --load Image
it reports
  dtb_set_property: fdt_add_subnode failed: <valid offset/length>
  kexec: Set device tree bootargs failed.

It has been tested on the arm64 architecture with version v2.0.17 and
v2.0.18-rc1

Regards,
  Vicenç.

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

Re: [BUG] Set device tree bootargs failed when DTB does not contain a chosen node.

[Bhupesh Sharma]

Hi Vicenç,

On Fri, Oct 26, 2018 at 6:42 PM Vicente Bergas <vicencb@gmail.com> wrote:
>
> Hello,
> when executing
>   kexec -d --dtb dtb_without_chosen_node.dtb --append 'cmdline' --load Image
> it reports
>   dtb_set_property: fdt_add_subnode failed: <valid offset/length>
>   kexec: Set device tree bootargs failed.
>
> It has been tested on the arm64 architecture with version v2.0.17 and
> v2.0.18-rc1

Can you share some details on the underlying platform and kernel
version you are using?

Looking at the logs, I am assuming you are running 'kexec' on a arm64
platform (as it depends on creating a dtb to be passed to the 2nd
kernel).

My general advice is to avoid the --dtb option as it is known to cause
issues with kexec in the past (see [0] for details)
Ideally, if you don't use the --dtb option, then kexec will read the existing
kernel's dtb from /proc/device-tree, and that device tree will include
all the changes that the boot loader has done, including adding details
of the available memory in the system.

[0]. https://www.spinics.net/lists/arm-kernel/msg618236.html

Hope this helps.

Regards,
Bhupesh

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

Re: [BUG] Set device tree bootargs failed when DTB does not contain a chosen node.

[Vicente Bergas]

On Sat, Oct 27, 2018 at 12:55 AM Bhupesh Sharma <bhsharma@redhat.com> wrote:
>
> Hi Vicenç,
>
> On Fri, Oct 26, 2018 at 6:42 PM Vicente Bergas <vicencb@gmail.com> wrote:
> >
> > Hello,
> > when executing
> >   kexec -d --dtb dtb_without_chosen_node.dtb --append 'cmdline' --load Image
> > it reports
> >   dtb_set_property: fdt_add_subnode failed: <valid offset/length>
> >   kexec: Set device tree bootargs failed.
> >
> > It has been tested on the arm64 architecture with version v2.0.17 and
> > v2.0.18-rc1
>
> Can you share some details on the underlying platform and kernel
> version you are using?
>
> Looking at the logs, I am assuming you are running 'kexec' on a arm64
> platform (as it depends on creating a dtb to be passed to the 2nd
> kernel).
>
> My general advice is to avoid the --dtb option as it is known to cause
> issues with kexec in the past (see [0] for details)
> Ideally, if you don't use the --dtb option, then kexec will read the existing
> kernel's dtb from /proc/device-tree, and that device tree will include
> all the changes that the boot loader has done, including adding details
> of the available memory in the system.
>
> [0]. https://www.spinics.net/lists/arm-kernel/msg618236.html
>
> Hope this helps.
>
> Regards,
> Bhupesh

Hi Bhupesh, thanks for answering and yes, this helps!

As you say, when not using the --dtb option it works fine. But this
way is not applicable in my use case.

I am trying to use Linux as a bootloader. This bootloader will reside
in a difficult-to-write memory (SPI-NOR Flash) and, because of that,
will rarely be updated.
Over time, new kernel versions will provide updated DTB files, so,
the DT from both kernels will diverge over time.
This is the reason to use the --dtb option.

Thanks for the information you provided, now I see a bug and also a
missing feature.


Focusing on the bug, I have traced it down to here:
https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/dt-ops.c#n87
the result is tested for non-zero as error condition, but, as seen here
https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/libfdt/fdt_rw.c#n345
it looks like positive values are not errors.


Now, focusing on the missing feature.
kexec-tools already has the (buggy) ability to add the "/chosen/bootargs" node.
I assume that it also adds or updates the "/chosen/linux,initrd-start"
and "/chosen/linux,initrd-end" nodes because it has the --initrd option.
Again, I assume it also adds/updates all "/memreserve" nodes with
the location where the DTB and initrd reside in memory.

So, just adding the "/memory" node will make it feature complete.
This node can be populated with data provided from the command line as
parameters or with data extracted from /proc/device-tree.

Adding "/ethernetX/local-mac-address" would be a bonus.


As a side node, can I suggest updating libfdt with latest upstream?
from:
https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/libfdt
to:
https://git.kernel.org/pub/scm/utils/dtc/dtc.git/tree/libfdt

Regards,
  Vicenç

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

Re: [BUG] Set device tree bootargs failed when DTB does not contain a chosen node.

[Bhupesh Sharma]

Hi Vicenç,

Sorry I was on holidays and came back to office just yesterday.

On Sat, Oct 27, 2018 at 3:56 PM Vicente Bergas <vicencb@gmail.com> wrote:
>
> On Sat, Oct 27, 2018 at 12:55 AM Bhupesh Sharma <bhsharma@redhat.com> wrote:
> >
> > Hi Vicenç,
> >
> > On Fri, Oct 26, 2018 at 6:42 PM Vicente Bergas <vicencb@gmail.com> wrote:
> > >
> > > Hello,
> > > when executing
> > >   kexec -d --dtb dtb_without_chosen_node.dtb --append 'cmdline' --load Image
> > > it reports
> > >   dtb_set_property: fdt_add_subnode failed: <valid offset/length>
> > >   kexec: Set device tree bootargs failed.
> > >
> > > It has been tested on the arm64 architecture with version v2.0.17 and
> > > v2.0.18-rc1
> >
> > Can you share some details on the underlying platform and kernel
> > version you are using?

Again, would request you to share the above details, which would help
me answer your queries in a better way.

> > Looking at the logs, I am assuming you are running 'kexec' on a arm64
> > platform (as it depends on creating a dtb to be passed to the 2nd
> > kernel).
> >
> > My general advice is to avoid the --dtb option as it is known to cause
> > issues with kexec in the past (see [0] for details)
> > Ideally, if you don't use the --dtb option, then kexec will read the existing
> > kernel's dtb from /proc/device-tree, and that device tree will include
> > all the changes that the boot loader has done, including adding details
> > of the available memory in the system.
> >
> > [0]. https://www.spinics.net/lists/arm-kernel/msg618236.html
> >
> > Hope this helps.
> >
> > Regards,
> > Bhupesh
>
> Hi Bhupesh, thanks for answering and yes, this helps!
>
> As you say, when not using the --dtb option it works fine. But this
> way is not applicable in my use case.
>
> I am trying to use Linux as a bootloader. This bootloader will reside
> in a difficult-to-write memory (SPI-NOR Flash) and, because of that,
> will rarely be updated.
> Over time, new kernel versions will provide updated DTB files, so,
> the DT from both kernels will diverge over time.
> This is the reason to use the --dtb option.
>
> Thanks for the information you provided, now I see a bug and also a
> missing feature.
>
>
> Focusing on the bug, I have traced it down to here:
> https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/dt-ops.c#n87
> the result is tested for non-zero as error condition, but, as seen here
> https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/libfdt/fdt_rw.c#n345
> it looks like positive values are not errors.

Hmmm.. looking hard at it, I am not sure if its indeed a bug, but more
on the same below ...

> Now, focusing on the missing feature.
> kexec-tools already has the (buggy) ability to add the "/chosen/bootargs" node.
> I assume that it also adds or updates the "/chosen/linux,initrd-start"
> and "/chosen/linux,initrd-end" nodes because it has the --initrd option.
> Again, I assume it also adds/updates all "/memreserve" nodes with
> the location where the DTB and initrd reside in memory.

> So, just adding the "/memory" node will make it feature complete.
> This node can be populated with data provided from the command line as
> parameters or with data extracted from /proc/device-tree.
>
> Adding "/ethernetX/local-mac-address" would be a bonus.

... Not really, as again this is very board specific. For e.g. I can
have a 'usb' (or even multiple Ethernet
cards on the board) as the transport media, which means that the
'kexec-tools' must be able to handle all of those
via the --dtb option and prepare the dtb to be sent to the 2nd kernel
accordingly.

I hope you understand my point here that we have a minimalistic
implementation intentionally as catering to all
dtb nodes in kexec-tools is probably equivalent to adding
functionality similar to how a bootloader (like u-boot) allows
dtb properties to be changed. And even if we add some basic skeleton
for the same, we cannot be sure that it will be suitable for all arm64
hardware available.

> As a side node, can I suggest updating libfdt with latest upstream?
> from:
> https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/libfdt
> to:
> https://git.kernel.org/pub/scm/utils/dtc/dtc.git/tree/libfdt

This is a good suggestion. I have this already on my to-do queue. I
will find some time to complete it over the weekend and share a patch
on the same probably by next week.

Thanks,
Bhupesh

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

Re: [BUG] Set device tree bootargs failed when DTB does not contain a chosen node.

[Vicente Bergas]

Hi Bhupesh,

On Fri, Nov 9, 2018 at 8:30 AM Bhupesh Sharma <bhsharma@redhat.com> wrote:
>
> Hi Vicenç,
>
> Sorry I was on holidays and came back to office just yesterday.
>
> On Sat, Oct 27, 2018 at 3:56 PM Vicente Bergas <vicencb@gmail.com> wrote:
> >
> > On Sat, Oct 27, 2018 at 12:55 AM Bhupesh Sharma <bhsharma@redhat.com> wrote:
> > >
> > > Hi Vicenç,
> > >
> > > On Fri, Oct 26, 2018 at 6:42 PM Vicente Bergas <vicencb@gmail.com> wrote:
> > > >
> > > > Hello,
> > > > when executing
> > > >   kexec -d --dtb dtb_without_chosen_node.dtb --append 'cmdline' --load Image
> > > > it reports
> > > >   dtb_set_property: fdt_add_subnode failed: <valid offset/length>
> > > >   kexec: Set device tree bootargs failed.
> > > >
> > > > It has been tested on the arm64 architecture with version v2.0.17 and
> > > > v2.0.18-rc1
> > >
> > > Can you share some details on the underlying platform and kernel
> > > version you are using?
>
> Again, would request you to share the above details, which would help
> me answer your queries in a better way.

It has been tested on the arm64 architecture and the kernel version
was 4.19, now moved to 4.20-rc.

> > > Looking at the logs, I am assuming you are running 'kexec' on a arm64
> > > platform (as it depends on creating a dtb to be passed to the 2nd
> > > kernel).
> > >
> > > My general advice is to avoid the --dtb option as it is known to cause
> > > issues with kexec in the past (see [0] for details)
> > > Ideally, if you don't use the --dtb option, then kexec will read the existing
> > > kernel's dtb from /proc/device-tree, and that device tree will include
> > > all the changes that the boot loader has done, including adding details
> > > of the available memory in the system.
> > >
> > > [0]. https://www.spinics.net/lists/arm-kernel/msg618236.html
> > >
> > > Hope this helps.
> > >
> > > Regards,
> > > Bhupesh
> >
> > Hi Bhupesh, thanks for answering and yes, this helps!
> >
> > As you say, when not using the --dtb option it works fine. But this
> > way is not applicable in my use case.
> >
> > I am trying to use Linux as a bootloader. This bootloader will reside
> > in a difficult-to-write memory (SPI-NOR Flash) and, because of that,
> > will rarely be updated.
> > Over time, new kernel versions will provide updated DTB files, so,
> > the DT from both kernels will diverge over time.
> > This is the reason to use the --dtb option.
> >
> > Thanks for the information you provided, now I see a bug and also a
> > missing feature.
> >
> >
> > Focusing on the bug, I have traced it down to here:
> > https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/dt-ops.c#n87
> > the result is tested for non-zero as error condition, but, as seen here
> > https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/libfdt/fdt_rw.c#n345
> > it looks like positive values are not errors.
>
> Hmmm.. looking hard at it, I am not sure if its indeed a bug, but more
> on the same below ...
>
> > Now, focusing on the missing feature.
> > kexec-tools already has the (buggy) ability to add the "/chosen/bootargs" node.
> > I assume that it also adds or updates the "/chosen/linux,initrd-start"
> > and "/chosen/linux,initrd-end" nodes because it has the --initrd option.
> > Again, I assume it also adds/updates all "/memreserve" nodes with
> > the location where the DTB and initrd reside in memory.
>
> > So, just adding the "/memory" node will make it feature complete.
> > This node can be populated with data provided from the command line as
> > parameters or with data extracted from /proc/device-tree.
> >
> > Adding "/ethernetX/local-mac-address" would be a bonus.
>
> ... Not really, as again this is very board specific. For e.g. I can
> have a 'usb' (or even multiple Ethernet
> cards on the board) as the transport media, which means that the
> 'kexec-tools' must be able to handle all of those
> via the --dtb option and prepare the dtb to be sent to the 2nd kernel
> accordingly.
>
> I hope you understand my point here that we have a minimalistic
> implementation intentionally as catering to all
> dtb nodes in kexec-tools is probably equivalent to adding
> functionality similar to how a bootloader (like u-boot) allows
> dtb properties to be changed. And even if we add some basic skeleton
> for the same, we cannot be sure that it will be suitable for all arm64
> hardware available.

Ok, I see the point, so, forget about new features. But I still think
the situation is improvable in one or other way for DTBs without
a chosen node:
 a.- In dt-ops.c:85 add the chosen node and proceed without error.
     The current code already adds it, but then it aborts.
 b.- Report an error message saying that this use case is not supported.
     Otherwise the current message looks like the program is broken.

> > As a side node, can I suggest updating libfdt with latest upstream?
> > from:
> > https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/libfdt
> > to:
> > https://git.kernel.org/pub/scm/utils/dtc/dtc.git/tree/libfdt
>
> This is a good suggestion. I have this already on my to-do queue. I
> will find some time to complete it over the weekend and share a patch
> on the same probably by next week.
>
> Thanks,
> Bhupesh

Regards,
  Vicenç.

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

Re: [BUG] Set device tree bootargs failed when DTB does not contain a chosen node.

[Bhupesh Sharma]

Hi Vicenç,

On Sat, Nov 10, 2018 at 5:17 PM Vicente Bergas <vicencb@gmail.com> wrote:
>
> Hi Bhupesh,
>
> On Fri, Nov 9, 2018 at 8:30 AM Bhupesh Sharma <bhsharma@redhat.com> wrote:
> >
> > Hi Vicenç,
> >
> > Sorry I was on holidays and came back to office just yesterday.
> >
> > On Sat, Oct 27, 2018 at 3:56 PM Vicente Bergas <vicencb@gmail.com> wrote:
> > >
> > > On Sat, Oct 27, 2018 at 12:55 AM Bhupesh Sharma <bhsharma@redhat.com> wrote:
> > > >
> > > > Hi Vicenç,
> > > >
> > > > On Fri, Oct 26, 2018 at 6:42 PM Vicente Bergas <vicencb@gmail.com> wrote:
> > > > >
> > > > > Hello,
> > > > > when executing
> > > > >   kexec -d --dtb dtb_without_chosen_node.dtb --append 'cmdline' --load Image
> > > > > it reports
> > > > >   dtb_set_property: fdt_add_subnode failed: <valid offset/length>
> > > > >   kexec: Set device tree bootargs failed.
> > > > >
> > > > > It has been tested on the arm64 architecture with version v2.0.17 and
> > > > > v2.0.18-rc1
> > > >
> > > > Can you share some details on the underlying platform and kernel
> > > > version you are using?
> >
> > Again, would request you to share the above details, which would help
> > me answer your queries in a better way.
>
> It has been tested on the arm64 architecture and the kernel version
> was 4.19, now moved to 4.20-rc.
>
> > > > Looking at the logs, I am assuming you are running 'kexec' on a arm64
> > > > platform (as it depends on creating a dtb to be passed to the 2nd
> > > > kernel).
> > > >
> > > > My general advice is to avoid the --dtb option as it is known to cause
> > > > issues with kexec in the past (see [0] for details)
> > > > Ideally, if you don't use the --dtb option, then kexec will read the existing
> > > > kernel's dtb from /proc/device-tree, and that device tree will include
> > > > all the changes that the boot loader has done, including adding details
> > > > of the available memory in the system.
> > > >
> > > > [0]. https://www.spinics.net/lists/arm-kernel/msg618236.html
> > > >
> > > > Hope this helps.
> > > >
> > > > Regards,
> > > > Bhupesh
> > >
> > > Hi Bhupesh, thanks for answering and yes, this helps!
> > >
> > > As you say, when not using the --dtb option it works fine. But this
> > > way is not applicable in my use case.
> > >
> > > I am trying to use Linux as a bootloader. This bootloader will reside
> > > in a difficult-to-write memory (SPI-NOR Flash) and, because of that,
> > > will rarely be updated.
> > > Over time, new kernel versions will provide updated DTB files, so,
> > > the DT from both kernels will diverge over time.
> > > This is the reason to use the --dtb option.
> > >
> > > Thanks for the information you provided, now I see a bug and also a
> > > missing feature.
> > >
> > >
> > > Focusing on the bug, I have traced it down to here:
> > > https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/dt-ops.c#n87
> > > the result is tested for non-zero as error condition, but, as seen here
> > > https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/libfdt/fdt_rw.c#n345
> > > it looks like positive values are not errors.
> >
> > Hmmm.. looking hard at it, I am not sure if its indeed a bug, but more
> > on the same below ...
> >
> > > Now, focusing on the missing feature.
> > > kexec-tools already has the (buggy) ability to add the "/chosen/bootargs" node.
> > > I assume that it also adds or updates the "/chosen/linux,initrd-start"
> > > and "/chosen/linux,initrd-end" nodes because it has the --initrd option.
> > > Again, I assume it also adds/updates all "/memreserve" nodes with
> > > the location where the DTB and initrd reside in memory.
> >
> > > So, just adding the "/memory" node will make it feature complete.
> > > This node can be populated with data provided from the command line as
> > > parameters or with data extracted from /proc/device-tree.
> > >
> > > Adding "/ethernetX/local-mac-address" would be a bonus.
> >
> > ... Not really, as again this is very board specific. For e.g. I can
> > have a 'usb' (or even multiple Ethernet
> > cards on the board) as the transport media, which means that the
> > 'kexec-tools' must be able to handle all of those
> > via the --dtb option and prepare the dtb to be sent to the 2nd kernel
> > accordingly.
> >
> > I hope you understand my point here that we have a minimalistic
> > implementation intentionally as catering to all
> > dtb nodes in kexec-tools is probably equivalent to adding
> > functionality similar to how a bootloader (like u-boot) allows
> > dtb properties to be changed. And even if we add some basic skeleton
> > for the same, we cannot be sure that it will be suitable for all arm64
> > hardware available.
>
> Ok, I see the point, so, forget about new features. But I still think
> the situation is improvable in one or other way for DTBs without
> a chosen node:
>  a.- In dt-ops.c:85 add the chosen node and proceed without error.
>      The current code already adds it, but then it aborts.

Fair enough. I have submitted a patch (see here:
<http://lists.infradead.org/pipermail/kexec/2018-November/021828.html>),
taking the approach in point (a). Can you please test the same on your
platform and share your test results?

Thanks,
Bhupesh

>  b.- Report an error message saying that this use case is not supported.
>      Otherwise the current message looks like the program is broken.
>
> > > As a side node, can I suggest updating libfdt with latest upstream?
> > > from:
> > > https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/libfdt
> > > to:
> > > https://git.kernel.org/pub/scm/utils/dtc/dtc.git/tree/libfdt
> >
> > This is a good suggestion. I have this already on my to-do queue. I
> > will find some time to complete it over the weekend and share a patch
> > on the same probably by next week.
> >
> > Thanks,
> > Bhupesh
>
> Regards,
>   Vicenç.

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