On Wed, Oct 03, 2018 at 09:55:02AM +0200, Fabrice Gasnier wrote:
> This adds a test case to demonstrate some issue seen when applying
> overlays using 'fdtoverlay'. It fails with FDT_ERR_NOSPACE:
> - with long target path
> - symbols in order to use these nodes in possible subsequent overlay.
>
> This is seen with this patch, by running:
> $ make check # Reports a failed test
> $ ./fdtoverlay -i tests/overlay_base.test.dtb -o out.dtb \
> tests/overlay_overlay_long_path.fdoverlay.test.dtb
> Failed to apply tests/overlay_overlay_long_path.fdoverlay.test.dtb (-3)
>
> This overlay fails to apply, because dtb size is close to modulo 1024
> bytes chunk: utilfdt_read() -> utilfdt_read_err() -> bufsize = 1024.
>
> As there is not much extra space in the blob to resolve symbols (long
> target path), it fails with FDT_ERR_NOSPACE. In fdtoverlay, size is :
> /* grow the blob to worst case */
> blob_len = fdt_totalsize(blob) + total_len;
>
> I can see assumption is made that result should be lower than:
> - base fdt size + overlay size. Is there a simple way to find to know
> what the final size is?
> I'm not sure what the correct fix might be, for such (worst) case?
> Similar issue is also seen in u-boot/common/image-fit.c that implements
> similar approach (e.g. base fdt size + overlay size).
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@xxxxxx>
Sorry I've taken so long to look at this. I finally had a look here
and reworked fdtoverlay not to hit this bug. I've applied a fix, as
well as your testcase to the master branch.
> ---
> tests/overlay_base.dts | 4 ++++
> tests/overlay_overlay_long_path.dts | 32 ++++++++++++++++++++++++++++++++
> tests/run_tests.sh | 8 ++++++++
> 3 files changed, 44 insertions(+)
> create mode 100644 tests/overlay_overlay_long_path.dts
>
> diff --git a/tests/overlay_base.dts b/tests/overlay_base.dts
> index 2603adb..a5e55b2 100644
> --- a/tests/overlay_base.dts
> +++ b/tests/overlay_base.dts
> @@ -14,6 +14,10 @@
>
> subtest: sub-test-node {
> sub-test-property;
> +
> + subtest_with_long_path: sub-test-node-with-very-long-target-path {
> + long-test-path-property;
> + };
> };
> };
> };
> diff --git a/tests/overlay_overlay_long_path.dts b/tests/overlay_overlay_long_path.dts
> new file mode 100644
> index 0000000..4f0d40b
> --- /dev/null
> +++ b/tests/overlay_overlay_long_path.dts
> @@ -0,0 +1,32 @@
> +/dts-v1/;
> +/plugin/;
> +
> +&subtest_with_long_path {
> + lpath_0: test-0 {
> + prop = "lpath";
> + };
> +
> + lpath_1: test-1 {
> + prop = "lpath";
> + };
> +
> + lpath_2: test-2 {
> + prop = "lpath";
> + };
> +
> + lpath_3: test-3 {
> + prop = "lpath";
> + };
> +
> + lpath_4: test-4 {
> + prop = "lpath";
> + };
> +
> + lpath_5: test-5 {
> + prop = "lpath";
> + };
> +
> + lpath_6: test-6 {
> + prop = "lpath";
> + };
> +};
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 7348c9c..d1bba5d 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -894,6 +894,14 @@ fdtoverlay_tests() {
>
> # test that baz correctly inserted the property
> run_fdtoverlay_test baz "/foonode/barnode/baznode" "baz-property" "-ts" ${stacked_basedtb} ${stacked_targetdtb} ${stacked_bardtb} ${stacked_bazdtb}
> +
> + overlay_long_path=overlay_overlay_long_path.dts
> + overlay_long_pathdtb=overlay_overlay_long_path.fdoverlay.test.dtb
> + target_long_pathdtb=overlay_overlay_long_path_target.fdoverlay.test.dtb
> + run_dtc_test -@ -I dts -O dtb -o $overlay_long_pathdtb $overlay_long_path
> +
> + # test that fdtoverlay manages to apply overlays with long target path
> + run_fdtoverlay_test lpath "/test-node/sub-test-node/sub-test-node-with-very-long-target-path/test-0" "prop" "-ts" ${basedtb} ${target_long_pathdtb} ${overlay_long_pathdtb}
> }
>
> pylibfdt_tests () {
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
