Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bug: WS-AP3825i dtb is not actually padded by the build process, preventing reliable boot #9779

Closed
Hurricos opened this issue Apr 26, 2022 · 5 comments
Closed

bug: WS-AP3825i dtb is not actually padded by the build process, preventing reliable boot #9779

Hurricos opened this issue Apr 26, 2022 · 5 comments

Comments

@Hurricos
Copy link
Contributor

Hurricos commented Apr 26, 2022 •
edited

Despite our work in this thread during initial porting of the AP3825i, the image build process doesn't actually pad the device tree blob; see the linked thread for the implications of that. It appears that in the thread, I accepted "the image happens to boot this time [because the device tree blob is spaced in such a way that fdt resize adds a lot to get to the next 4k-byte page]" in place of "the image boots this time because the fdt is actually greatly padded".

Anyways, in practice, what happens is this:

Boot (PRI)-> interrupts off;
Boot (PRI)-> bootm start 0x2000000;
## Booting kernel from FIT Image at 02000000 ...
   Using 'config-1' configuration
   Trying 'kernel-1' kernel subimage
     Description:  POWERPC OpenWrt Linux-5.10.111
:
## Flattened Device Tree from FIT Image at 02000000
   Using 'config-1' configuration
   Trying 'fdt-1' FDT blob subimage
     Description:  POWERPC OpenWrt extreme-networks_ws-ap3825i device tree blob
:
     Data Size:    11524 Bytes = 11.3 KiB
:
   Booting using the fdt blob at 0x2681ffc
Boot (PRI)-> bootm loados;
   Uncompressing Kernel Image ... OK
Boot (PRI)-> fdt resize;
Boot (PRI)-> fdt boardsetup;
ft_fixup_l2cache: FDT_ERR_NOTFOUND
Boot (PRI)-> fdt chosen;
WARNING: could not set bootargs FDT_ERR_NOSPACE.
WARNING: could not set linux,stdout-path FDT_ERR_NOSPACE.
Boot (PRI)->

Note closely: the Data Size: 11524 Bytes = 11.3 KiB, despite that DTB_SIZE := 20480 is set in the AP3825i device definition.

This was discovered while trying to boot OpenWrt 22.03-rc1 through the serial console.

A temporary user workaround is to double each fdt operation in the bootcmd and add an fdt resize between the two copies; again, see the linked thread. However, I need to look much more closely at how OpenWrt device image formulas to work, as clearly this code does not pad the DTB.
@Hurricos
Copy link
Contributor Author

The build script which is responsible for building a FIT image consumes build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/image-ws-ap3825i.dtb:

/devel/openwrt/AP3825i/openwrt/scripts/mkits.sh -D extreme-networks_ws-ap3825i -o /devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/extreme-networks_ws-ap3825i-kernel.bin.its -k /devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/extreme-networks_ws-ap3825i-kernel.bin -C lzma   -d /devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/image-ws-ap3825i.dtb   -a 0x00000000 -e 0x00000000    -c "config-1" -A powerpc -v 5.15.31

This file is produced much earlier:

powerpc-openwrt-linux-musl-cpp -nostdinc -x assembler-with-cpp  -I/devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/linux-5.15.31/arch/powerpc/boot/dts -I/devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/linux-5.15.31/arch/powerpc/boot/dts/include -I/devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/linux-5.15.31/include/ -undef -D__DTS__  -o /devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/image-ws-ap3825i.dtb.tmp /devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/linux-5.15.31/arch/powerpc/boot/dts/ws-ap3825i.dts
:
/devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/linux-5.15.31/scripts/dtc/dtc -O dtb -i/devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/linux-5.15.31/arch/powerpc/boot/dts/ -Wno-unit_address_vs_reg -Wno-simple_bus_reg -Wno-unit_address_format -Wno-pci_bridge -Wno-pci_device_bus_num -Wno-pci_device_reg -Wno-avoid_unnecessary_addr_size -Wno-alias_paths -Wno-graph_child_address -Wno-graph_port -Wno-unique_unit_address   -o /devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/image-ws-ap3825i.dtb /devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/image-ws-ap3825i.dtb.tmp

The Build/dtb recipe does run later (though before mkits), but outputs elsewhere:

powerpc-openwrt-linux-musl-cpp -nostdinc -x assembler-with-cpp  -I/devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/linux-5.15.31/arch/powerpc/boot/dts -I/devel/openwrt/AP3825i/openwrt/build_dir/target-
powerpc_8540_musl/linux-mpc85xx_p1020/linux-5.15.31/arch/powerpc/boot/dts/include -I/devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/linux-5.15.31/include/ -undef -D__DTS__  -o /devel/openwrt/AP3825i/o
penwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/extreme-networks_ws-ap3825i-kernel.bin.dtb.tmp /devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/linux-5.15.31/arch/powerpc/boot/dts/ws-ap38
25i.dts
/devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/linux-5.15.31/scripts/dtc/dtc -O dtb -i/devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/linux-5.15.31/arch/powerpc/
boot/dts/ -Wno-unit_address_vs_reg -Wno-simple_bus_reg -Wno-unit_address_format -Wno-pci_bridge -Wno-pci_device_bus_num -Wno-pci_device_reg -Wno-avoid_unnecessary_addr_size -Wno-alias_paths -Wno-graph_child_address -Wno-graph_port -Wno-un
ique_unit_address --space 20480  -o /devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-mpc85xx_p1020/extreme-networks_ws-ap3825i-kernel.bin.dtb /devel/openwrt/AP3825i/openwrt/build_dir/target-powerpc_8540_musl/linux-m
pc85xx_p1020/extreme-networks_ws-ap3825i-kernel.bin.dtb.tmp

So basically, I just need to find a way to make sure the output is redirected accordingly. Tough to do. Groan.

@Hurricos
Copy link
Contributor Author

Hurricos commented Apr 26, 2022 •
edited

OK, I've fixed it.

This diff:

diff --git a/target/linux/mpc85xx/image/Makefile b/target/linux/mpc85xx/image/Makefile
index e1d1bc6948..fb931760a7 100644
--- a/target/linux/mpc85xx/image/Makefile
+++ b/target/linux/mpc85xx/image/Makefile
@@ -6,7 +6,7 @@ include $(INCLUDE_DIR)/image.mk
 DEVICE_VARS += DTB_SIZE
 
 define Build/dtb
-       $(call Image/BuildDTB,$(DTS_DIR)/$(DEVICE_DTS).dts,$@.dtb,,--space $(DTB_SIZE))
+       $(call Image/BuildDTB,$(DTS_DIR)/$(DEVICE_DTS).dts,$(dir $@)/image-$(DEVICE_DTS).dtb,,--space $(DTB_SIZE))
 endef
 
 define Device/Default

... turns this build output:

FIT description: POWERPC OpenWrt FIT (Flattened Image Tree)
Created:         Fri Apr  1 19:58:03 2022
 Image 0 (kernel-1)
  Description:  POWERPC OpenWrt Linux-5.15.31
  Created:      Fri Apr  1 19:58:03 2022
  Type:         Kernel Image
  Compression:  lzma compressed
  Data Size:    3077153 Bytes = 3005.03 KiB = 2.93 MiB
  Architecture: PowerPC
  OS:           Linux
  Load Address: 0x00000000
  Entry Point:  0x00000000
  Hash algo:    crc32
  Hash value:   d515981d
  Hash algo:    sha1
  Hash value:   1748cb39c9c095f016e7d33e43bef4585f9c3cbc
 Image 1 (fdt-1)
  Description:  POWERPC OpenWrt extreme-networks_ws-ap3825i device tree blob
  Created:      Fri Apr  1 19:58:03 2022
  Type:         Flat Device Tree
  Compression:  uncompressed
  Data Size:    11524 Bytes = 11.25 KiB = 0.01 MiB
  Architecture: PowerPC
  Hash algo:    crc32
  Hash value:   6386dc28
  Hash algo:    sha1
  Hash value:   67a9afe125d50ce068e900e090845fec3836ac7d
 Default Configuration: 'config-1'
 Configuration 0 (config-1)
  Description:  OpenWrt extreme-networks_ws-ap3825i
  Kernel:       kernel-1
  FDT:          fdt-1

... into this build output:

FIT description: POWERPC OpenWrt FIT (Flattened Image Tree)
Created:         Fri Apr  1 19:58:03 2022
 Image 0 (kernel-1)
  Description:  POWERPC OpenWrt Linux-5.15.31
  Created:      Fri Apr  1 19:58:03 2022
  Type:         Kernel Image
  Compression:  lzma compressed
  Data Size:    3077153 Bytes = 3005.03 KiB = 2.93 MiB
  Architecture: PowerPC
  OS:           Linux
  Load Address: 0x00000000
  Entry Point:  0x00000000
  Hash algo:    crc32
  Hash value:   d515981d
  Hash algo:    sha1
  Hash value:   1748cb39c9c095f016e7d33e43bef4585f9c3cbc
 Image 1 (fdt-1)
  Description:  POWERPC OpenWrt extreme-networks_ws-ap3825i device tree blob
  Created:      Fri Apr  1 19:58:03 2022
  Type:         Flat Device Tree
  Compression:  uncompressed
  Data Size:    20480 Bytes = 20.00 KiB = 0.02 MiB
  Architecture: PowerPC
  Hash algo:    crc32
  Hash value:   366df45c
  Hash algo:    sha1
  Hash value:   93ce4ed0430a7f246c0077ba7999e3d8868eead6
 Default Configuration: 'config-1'
 Configuration 0 (config-1)
  Description:  OpenWrt extreme-networks_ws-ap3825i
  Kernel:       kernel-1
  FDT:          fdt-1

(edited to compare two sets of kernel-only outputs rather than one initramfs and one rootfsless-kernel; and then edited twice more because I'm having a hard time pasting into Github today.)

@Hurricos
Copy link
Contributor Author

Hurricos commented Apr 26, 2022 •
edited

Once concern I have about this diff:

diff --git a/target/linux/mpc85xx/image/Makefile b/target/linux/mpc85xx/image/Makefile
index e1d1bc6948..fb931760a7 100644
--- a/target/linux/mpc85xx/image/Makefile
+++ b/target/linux/mpc85xx/image/Makefile
@@ -6,7 +6,7 @@ include $(INCLUDE_DIR)/image.mk
 DEVICE_VARS += DTB_SIZE
 
 define Build/dtb
-       $(call Image/BuildDTB,$(DTS_DIR)/$(DEVICE_DTS).dts,$@.dtb,,--space $(DTB_SIZE))
+       $(call Image/BuildDTB,$(DTS_DIR)/$(DEVICE_DTS).dts,$(dir $@)/image-$(DEVICE_DTS).dtb,,--space $(DTB_SIZE))
 endef
 
 define Device/Default

... is that it looks like it impacts anywhere in the mpc85xx target where we construct an image using the dtb step. Rather than having our later redefinition overwrite the image-$(DEVICE_DTS).dtb file, I see two alternatives:

  1. Have the ws-ap3825i use a different dtb file:
modified   target/linux/mpc85xx/image/Makefile
@@ -6,7 +6,7 @@ include $(INCLUDE_DIR)/image.mk
 DEVICE_VARS += DTB_SIZE

 define Build/dtb
-    $(call Image/BuildDTB,$(DTS_DIR)/$(DEVICE_DTS).dts,$(dir $@)/image-$(DEVICE_DTS).dtb,,--space $(DTB_SIZE))
+    $(call Image/BuildDTB,$(DTS_DIR)/$(DEVICE_DTS).dts,$(dir $@)/padded-image-$(DEVICE_DTS).dtb,,--space $(DTB_SIZE))
 endef

 define Device/Default
modified   target/linux/mpc85xx/image/p1020.mk
@@ -74,7 +74,7 @@ define Device/extreme-networks_ws-ap3825i
   DEVICE_PACKAGES := kmod-ath10k-ct ath10k-firmware-qca988x-ct
   BLOCKSIZE := 128k
   DTB_SIZE := 20480
-  KERNEL = kernel-bin | lzma | dtb | fit lzma $(KDIR)/image-$$(DEVICE_DTS).dtb
+  KERNEL = kernel-bin | lzma | dtb | fit lzma $(KDIR)/padded-image-$$(DEVICE_DTS).dtb
   IMAGES := sysupgrade.bin
   IMAGE/sysupgrade.bin := append-kernel | append-rootfs | pad-rootfs | append-metadata
 endef
  1. Explicitly change the build rule from dtb to padded-dtb:
modified   target/linux/mpc85xx/image/Makefile
@@ -5,8 +5,8 @@ include $(INCLUDE_DIR)/image.mk

DEVICE_VARS += DTB_SIZE

-define Build/dtb
-    $(call Image/BuildDTB,$(DTS_DIR)/$(DEVICE_DTS).dts,$(dir $@)/image-$(DEVICE_DTS).dtb,,--space $(DTB_SIZE))
+define Build/padded-dtb
+    $(call Image/BuildDTB,$(DTS_DIR)/$(DEVICE_DTS).dts,$(dir $@)/padded-image-$(DEVICE_DTS).dtb,,--space $(DTB_SIZE))
endef

define Device/Default
modified   target/linux/mpc85xx/image/p1020.mk
@@ -74,7 +74,7 @@ define Device/extreme-networks_ws-ap3825i
  DEVICE_PACKAGES := kmod-ath10k-ct ath10k-firmware-qca988x-ct
  BLOCKSIZE := 128k
  DTB_SIZE := 20480
-  KERNEL = kernel-bin | lzma | dtb | fit lzma $(KDIR)/image-$$(DEVICE_DTS).dtb
+  KERNEL = kernel-bin | lzma | padded-dtb | fit lzma $(KDIR)/padded-image-$$(DEVICE_DTS).dtb
  IMAGES := sysupgrade.bin
  IMAGE/sysupgrade.bin := append-kernel | append-rootfs | pad-rootfs | append-metadata
endef

I honestly prefer the latter as it seems safer, but I first need to test:

Checklist

  • Is my assertion correct that the Build/dtb definition override will break other, non-DTB_SIZE boards?
    • It is not correct: none of the other recipes actually run dtb.
  • Does the second patch actually work, or does it misunderstand the expected input / output of the dtb build definition?
    • It works.

Hurricos added a commit to Hurricos/openwrt that referenced this issue Apr 27, 2022
@Hurricos
mpc85xx: Fix output location of padded dtb
da8ae7d 
In commit 7e61482 ("mpc85xx: add support for Extreme Networks
WS-AP3825i"), we borrowed a recipe convention from apm821xx for device
tree blob padding. Unfortunately, in the apm821xx target, the image
recipes name the device tree blob differently, meaning that in
mpc85xx, the padded dtb is never consumed.

Change the definition of `Build/dtb` so that it outputs the padded dtb
to the correct location for it to be consumed.

Also, rename the recipe to `Build/pad-dtb`, so it is clear we
are building and padding the device tree blob.

This change fixes Github issue openwrt#9779 [1].

[1]: openwrt#9779

Fixes: 7e61482 ("mpc85xx: add support for Extreme Networks
WS-AP3825i")

Signed-off-by: Martin Kennedy <hurricos@gmail.com>
@Hurricos Hurricos mentioned this issue Apr 27, 2022
Closed
jow- pushed a commit that referenced this issue Apr 30, 2022
@Hurricos @hauke
mpc85xx: Fix output location of padded dtb
1d06277 
In commit 7e61482 ("mpc85xx: add support for Extreme Networks
WS-AP3825i"), we borrowed a recipe convention from apm821xx for device
tree blob padding. Unfortunately, in the apm821xx target, the image
recipes name the device tree blob differently, meaning that in
mpc85xx, the padded dtb is never consumed.

Change the definition of `Build/dtb` so that it outputs the padded dtb
to the correct location for it to be consumed.

Also, rename the recipe to `Build/pad-dtb`, so it is clear we
are building and padding the device tree blob.

This change fixes Github issue #9779 [1].

[1]: #9779

Fixes: 7e61482 ("mpc85xx: add support for Extreme Networks WS-AP3825i")
Signed-off-by: Martin Kennedy <hurricos@gmail.com>
@hauke
Copy link
Member

hauke commented Apr 30, 2022

The problem was fixed in 1d06277,

@hauke hauke closed this as completed Apr 30, 2022
jow- pushed a commit to lede-project/source that referenced this issue May 1, 2022
@Hurricos @hauke
mpc85xx: Fix output location of padded dtb
a142d96 
In commit 7e61482 ("mpc85xx: add support for Extreme Networks
WS-AP3825i"), we borrowed a recipe convention from apm821xx for device
tree blob padding. Unfortunately, in the apm821xx target, the image
recipes name the device tree blob differently, meaning that in
mpc85xx, the padded dtb is never consumed.

Change the definition of `Build/dtb` so that it outputs the padded dtb
to the correct location for it to be consumed.

Also, rename the recipe to `Build/pad-dtb`, so it is clear we
are building and padding the device tree blob.

This change fixes Github issue #9779 [1].

[1]: openwrt/openwrt#9779

Fixes: 7e61482 ("mpc85xx: add support for Extreme Networks WS-AP3825i")
Signed-off-by: Martin Kennedy <hurricos@gmail.com>
(cherry picked from commit 1d06277)
@Hurricos
Copy link
Contributor Author

Hurricos commented May 1, 2022

Thank you!

Vladdrako pushed a commit to Vladdrako/openwrt that referenced this issue May 2, 2022
@Hurricos @Vladdrako
mpc85xx: Fix output location of padded dtb
6b15ab0 
In commit 7e61482 ("mpc85xx: add support for Extreme Networks
WS-AP3825i"), we borrowed a recipe convention from apm821xx for device
tree blob padding. Unfortunately, in the apm821xx target, the image
recipes name the device tree blob differently, meaning that in
mpc85xx, the padded dtb is never consumed.

Change the definition of `Build/dtb` so that it outputs the padded dtb
to the correct location for it to be consumed.

Also, rename the recipe to `Build/pad-dtb`, so it is clear we
are building and padding the device tree blob.

This change fixes Github issue openwrt#9779 [1].

[1]: openwrt#9779

Fixes: 7e61482 ("mpc85xx: add support for Extreme Networks WS-AP3825i")
Signed-off-by: Martin Kennedy <hurricos@gmail.com>
@robimarko robimarko mentioned this issue Apr 23, 2024
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@hauke @Hurricos