Fix to keep the FIT DTB's own bootargs unless LINUX_BOOTARGS is explicitly configured

docs/Targets.md

@@ -1290,7 +1290,9 @@ power-on.
   `0x01000000`, which is uncached and coherent for all harts. Cacheable L2-scratchpad memory
   must not be used for cross-hart signalling (stores can be lost on dirty-line eviction).
 
-**Device-tree fixups** applied to the loaded dtb (`hal/mpfs250.c`): bootargs/root device,
+**Device-tree fixups** applied to the loaded dtb (`hal/mpfs250.c`): bootargs/root device
+(the DTB's own `/chosen/bootargs` win unless `LINUX_BOOTARGS`/`LINUX_BOOTARGS_ROOT` is set;
+see "Linux kernel command line (bootargs)" in `docs/compile.md`),
 MAC addresses from the device serial number, and all five MSS watchdog nodes are disabled.
 
 **Watchdog policy:** the MSS watchdogs always count and reset the chip on timeout (they cannot
@@ -4448,7 +4450,10 @@ images {
 that wolfBoot decompresses straight to the kernel load address at boot.
 See the [Versal "Booting Linux via FIT image"](#versal-gen-1-vmk180)
 section for a full walkthrough - the flow is identical apart from the
-load addresses and the `bl31`/`fsbl` versus `bl31`/`plm` boot chain. Set
+load addresses and the `bl31`/`fsbl` versus `bl31`/`plm` boot chain,
+including the bootargs handling (the FIT DTB's own `/chosen/bootargs`
+win unless `LINUX_BOOTARGS`/`LINUX_BOOTARGS_ROOT` is set; see
+"Linux kernel command line (bootargs)" in `docs/compile.md`). Set
 `GZIP=0` in
 `.config` if you want to keep using an uncompressed `Image` plus
 `compression = "none"`.
@@ -5129,21 +5134,17 @@ A stock PetaLinux `image.ub` carries a `ramdisk` sub-image that `bootm` passes t
 
 `WOLFBOOT_LOAD_RAMDISK_ADDRESS` defaults to 0, which uses the ramdisk in place inside the staged FIT. Set it to a DDR address clear of the kernel, DTB and staging area if the payload needs a fixed location.
 
-**DTB Fixup for Root Filesystem**
+**Kernel Command Line (bootargs)**
 
-wolfBoot automatically modifies the device tree to set the kernel command line (`bootargs`). The default configuration mounts the root filesystem from SD card partition 2:
-
-```
-earlycon root=/dev/mmcblk0p2 rootwait
-```
-
-To customize the root device, add to your config:
+If the FIT's DTB carries `/chosen/bootargs`, wolfBoot keeps them by default - an image boots with the arguments its kernel was validated with. Setting `LINUX_BOOTARGS` or `LINUX_BOOTARGS_ROOT` in the config replaces the DTB's value (the replaced value is logged); `CFLAGS_EXTRA+=-DLINUX_BOOTARGS_OVERRIDE=0` demotes an explicit `LINUX_BOOTARGS` to a fallback used only when the DTB has none.
 
 ```makefile
-# Mount root from SD card partition 4
+# Replace the image's bootargs, mounting root from SD card partition 4
 CFLAGS_EXTRA+=-DLINUX_BOOTARGS_ROOT=\"/dev/mmcblk0p4\"
 ```
 
+On Versal the PS UART console is `ttyAMA0` (PL011); when supplying your own bootargs prefer an explicit `earlycon=pl011,mmio32,0xFF000000,115200n8 console=ttyAMA0,115200`.
+
 **Automated Testing**
 
 ```sh

docs/compile.md

@@ -292,6 +292,16 @@ downgrades, compile with `ALLOW_DOWNGRADE=1`.
 Warning: this option will disable version checking before the updates, thus exposing the system to potential
 forced downgrade attacks.
 
+### Linux kernel command line (bootargs)
+
+On the FIT Linux-boot targets whose DTB comes from the signed FIT (Versal, ZynqMP, PolarFire SoC), a DTB that already carries a non-empty `/chosen/bootargs` keeps it by default - the image boots with the arguments its kernel was validated with, and wolfBoot logs `FDT: using DTB bootargs: ...`. Three macros control this:
+
+- `LINUX_BOOTARGS` - the full command line wolfBoot injects. Setting it (via `CFLAGS_EXTRA+=-DLINUX_BOOTARGS='"..."'`) replaces the DTB's value; the replaced value is logged.
+- `LINUX_BOOTARGS_ROOT` - shorthand that only swaps the `root=` device in the HAL's default command line; setting it also selects replace semantics.
+- `LINUX_BOOTARGS_OVERRIDE` - explicit polarity control: `1` forces replacement, `0` demotes `LINUX_BOOTARGS` to a fallback used only when the DTB carries no bootargs. Unset, it defaults to `1` when either macro above is configured and `0` otherwise.
+
+The Raspberry Pi CM4 firmware-DTB path is excluded and always injects wolfBoot's command line, because that DTB is firmware-provided and unverified. Note for raw-DTB targets: an unauthenticated raw DTB's bootargs are honored under the default too, so either sign the DTB (`sign --dts`, see the next section) or set an explicit `LINUX_BOOTARGS` when the DTB partition is not trusted.
+
 ### Require an authenticated device tree (raw-DTB targets)
 
 On non-FIT MMU targets that load a raw device tree from flash, wolfBoot authenticates the DTB against the `HDR_DEVICE_TREE_DIGEST` TLV bound to the signed kernel (`sign --dts <board.dtb>`, see `docs/Signing.md`). A DTB carrying the digest is always verified; a raw DTB with no digest only warns and boots by default. Compile with `WOLFBOOT_REQUIRE_SIGNED_DTB=1` to make a missing digest a hard failure (fail-closed) once every raw-DTB payload is signed with `--dts`.

hal/mpfs250.c

@@ -585,6 +585,8 @@ int mpfs_read_serial_number(uint8_t *serial)
 }
 
 /* Linux kernel command line arguments */
+/* Must stay below the fdt.h include: the LINUX_BOOTARGS_OVERRIDE default
+ * there keys off build-supplied macros only, not this fallback. */
 #ifndef LINUX_BOOTARGS
 #ifndef LINUX_BOOTARGS_ROOT
 /* wolfBoot SD layout (tools/scripts/program-sdcard.sh): p1=boot FIT,
@@ -637,16 +639,11 @@ static int mpfs_dts_fixup_inplace(void* dts_addr, uint32_t capacity)
         return ret;
     }
 
-    /* Find /chosen node */
-    off = fdt_subnode_offset(&ctx, 0, "chosen");
-    if (off < 0) {
-        /* Create /chosen node if it doesn't exist */
-        off = fdt_add_subnode(&ctx, 0, "chosen");
-    }
-
-    if (off >= 0) {
-        /* Set bootargs property */
-        fdt_fixup_str(&ctx, off, "chosen", "bootargs", LINUX_BOOTARGS);
+    /* Later fixups (watchdog disable, serial number) must run even if the
+     * bootargs fixup fails, so log and continue rather than return. */
+    ret = fdt_fixup_bootargs(&ctx, LINUX_BOOTARGS, LINUX_BOOTARGS_OVERRIDE);
+    if (ret < 0) {
+        wolfBoot_printf("FDT: Failed to set bootargs (%d)\n", ret);
     }
 
 #if defined(MPFS_DDR_INIT) && defined(WOLFBOOT_MMODE_SMODE_BOOT)

hal/versal.c

@@ -64,6 +64,8 @@
  */
 
 /* Linux kernel command line arguments */
+/* Must stay below the fdt.h include: the LINUX_BOOTARGS_OVERRIDE default
+ * there keys off build-supplied macros only, not this fallback. */
 #ifndef LINUX_BOOTARGS
 #ifndef LINUX_BOOTARGS_ROOT
 /* Default Versal SD layout: rootfs on partition 2. Configurations that use
@@ -1290,7 +1292,7 @@ void* hal_get_dts_update_address(void)
 int hal_dts_fixup(void* dts_addr, uint32_t capacity)
 {
     fdt_ctx ctx;
-    int off, ret;
+    int ret;
 
     /* Validate the blob against the window it actually occupies. */
     ret = fdt_open(&ctx, dts_addr, capacity);
@@ -1310,22 +1312,7 @@ int hal_dts_fixup(void* dts_addr, uint32_t capacity)
         return ret;
     }
 
-    /* Find /chosen node; create it only if genuinely missing. Any other
-     * negative return (malformed FDT, etc.) is surfaced directly rather
-     * than masked by a follow-on fdt_add_subnode() failure. */
-    off = fdt_subnode_offset(&ctx, 0, "chosen");
-    if (off == -FDT_ERR_NOTFOUND) {
-        off = fdt_add_subnode(&ctx, 0, "chosen");
-    }
-
-    if (off < 0) {
-        wolfBoot_printf("FDT: Failed to find/create chosen node (%d)\n", off);
-        return off;
-    }
-
-    /* Set bootargs property - overrides the PetaLinux default root= with
-     * the wolfBoot partition layout. */
-    ret = fdt_fixup_str(&ctx, off, "chosen", "bootargs", LINUX_BOOTARGS);
+    ret = fdt_fixup_bootargs(&ctx, LINUX_BOOTARGS, LINUX_BOOTARGS_OVERRIDE);
     if (ret < 0) {
         wolfBoot_printf("FDT: Failed to set bootargs (%d)\n", ret);
         return ret;

hal/zynq.c

@@ -64,6 +64,8 @@
  * Note: console=ttyPS0 is ZynqMP-specific (PS UART0). Versal's default
  * (hal/versal.c) omits the console= token because Versal relies on
  * earlycon alone plus a DT-declared stdout-path. */
+/* Must stay below the fdt.h include: the LINUX_BOOTARGS_OVERRIDE default
+ * there keys off build-supplied macros only, not this fallback. */
 #ifndef LINUX_BOOTARGS
 #ifndef LINUX_BOOTARGS_ROOT
 #define LINUX_BOOTARGS_ROOT "/dev/mmcblk0p4"
@@ -2632,7 +2634,7 @@ void* hal_get_dts_address(void)
 int hal_dts_fixup(void* dts_addr, uint32_t capacity)
 {
     fdt_ctx ctx;
-    int off, ret;
+    int ret;
 
     /* Validate the blob against the window it actually occupies. Every
      * bound the parser applies below comes from `capacity`, not from the
@@ -2657,21 +2659,7 @@ int hal_dts_fixup(void* dts_addr, uint32_t capacity)
         return ret;
     }
 
-    /* Find /chosen node; create it only if genuinely missing. Any other
-     * negative return (malformed FDT, etc.) is surfaced directly rather
-     * than masked by a follow-on fdt_add_subnode() failure. */
-    off = fdt_subnode_offset(&ctx, 0, "chosen");
-    if (off == -FDT_ERR_NOTFOUND) {
-        off = fdt_add_subnode(&ctx, 0, "chosen");
-    }
-    if (off < 0) {
-        wolfBoot_printf("FDT: Failed to find/create chosen node (%d)\n", off);
-        return off;
-    }
-
-    /* Set bootargs property - overrides PetaLinux default root= with
-     * the wolfBoot partition layout. */
-    ret = fdt_fixup_str(&ctx, off, "chosen", "bootargs", LINUX_BOOTARGS);
+    ret = fdt_fixup_bootargs(&ctx, LINUX_BOOTARGS, LINUX_BOOTARGS_OVERRIDE);
     if (ret < 0) {
         wolfBoot_printf("FDT: Failed to set bootargs (%d)\n", ret);
         return ret;

include/fdt.h

@@ -314,6 +314,23 @@ int fdt_del_node(fdt_ctx* ctx, int nodeoffset);
 int fdt_add_mem_rsv(fdt_ctx* ctx, uint64_t address, uint64_t size);
 
 /* Logging wrappers around fdt_setprop() used by the HAL dts fixups. */
+/* LINUX_BOOTARGS_OVERRIDE: nonzero replaces the DTB's own bootargs, zero
+ * keeps them when present (LINUX_BOOTARGS then only fills a missing value).
+ * Defaults to replace when the build supplies LINUX_BOOTARGS or
+ * LINUX_BOOTARGS_ROOT, and to keep when only the HAL fallback exists. */
+#ifndef LINUX_BOOTARGS_OVERRIDE
+    #if defined(LINUX_BOOTARGS) || defined(LINUX_BOOTARGS_ROOT)
+        #define LINUX_BOOTARGS_OVERRIDE 1
+    #else
+        #define LINUX_BOOTARGS_OVERRIDE 0
+    #endif
+#endif
+
+/* Set /chosen bootargs; force 0 keeps an existing non-empty DTB value.
+ * Evaluated here (before any HAL fallback LINUX_BOOTARGS definition) so
+ * the default reflects only build-supplied macros. */
+int fdt_fixup_bootargs(fdt_ctx* ctx, const char* args, int force);
+
 int fdt_fixup_str(fdt_ctx* ctx, int off, const char* node, const char* name,
     const char* str);
 int fdt_fixup_val(fdt_ctx* ctx, int off, const char* node, const char* name,

src/fdt.c

@@ -1287,6 +1287,42 @@ int fdt_fixup_val64(fdt_ctx* ctx, int off, const char* node, const char* name,
     return fdt_setprop(ctx, off, name, &be, (int)sizeof(be));
 }
 
+/* Set /chosen bootargs. With force nonzero the DTB's existing value is
+ * replaced by `args` (and logged); with force 0 an existing non-empty
+ * value wins, so an image keeps the arguments its kernel was validated
+ * with unless the build explicitly overrides them. */
+int fdt_fixup_bootargs(fdt_ctx* ctx, const char* args, int force)
+{
+    const char* old_args;
+    int off, old_len = 0;
+
+    if (!fdt_ctx_ok(ctx) || args == NULL) {
+        return -FDT_ERR_BADARG;
+    }
+    off = fdt_subnode_offset(ctx, 0, "chosen");
+    if (off == -FDT_ERR_NOTFOUND) {
+        off = fdt_add_subnode(ctx, 0, "chosen");
+    }
+    if (off < 0) {
+        wolfBoot_printf("FDT: Failed to find/create chosen node (%d)\n", off);
+        return off;
+    }
+    /* Treat the existing value as present only when it is a non-empty,
+     * NUL-terminated string: property bytes are opaque, and %s printing or
+     * keeping an unterminated value would be wrong either way. Malformed
+     * or empty bootargs are simply replaced. */
+    old_args = (const char*)fdt_getprop(ctx, off, "bootargs", &old_len);
+    if (old_args != NULL && old_len > 1 && old_args[0] != '\0' &&
+            memchr(old_args, '\0', (size_t)old_len) != NULL) {
+        if (!force) {
+            wolfBoot_printf("FDT: using DTB bootargs: %s\n", old_args);
+            return 0;
+        }
+        wolfBoot_printf("FDT: replacing DTB bootargs: %s\n", old_args);
+    }
+    return fdt_fixup_str(ctx, off, "chosen", "bootargs", args);
+}
+
 int fdt_fixup_initrd(fdt_ctx* ctx, uint64_t start, uint64_t size)
 {
     int off, ret;

tools/unit-tests/unit-fdt.c

@@ -801,6 +801,63 @@ START_TEST(test_fdt_fixup_initrd)
 }
 END_TEST
 
+/* fdt_fixup_bootargs(): DTB-provided bootargs win unless force is set;
+ * a missing property is always filled in. */
+START_TEST(test_fdt_fixup_bootargs_keep_and_force)
+{
+    static uint8_t buf[0x800];
+    fdt_ctx ctx;
+    const char *val;
+    int off, len;
+
+    (void)build_compat_fdt(buf, sizeof(buf), (const uint8_t *)"abc\0", 4);
+    ck_assert_int_eq(fdt_open(&ctx, buf, (uint32_t)sizeof(buf)), 0);
+
+    /* absent: set regardless of force */
+    ck_assert_int_eq(fdt_fixup_bootargs(&ctx, "one=1", 0), 0);
+    off = fdt_subnode_offset(&ctx, 0, "chosen");
+    ck_assert_int_gt(off, 0);
+    val = (const char *)fdt_getprop(&ctx, off, "bootargs", &len);
+    ck_assert_str_eq(val, "one=1");
+
+    /* present + force 0: existing value kept */
+    ck_assert_int_eq(fdt_fixup_bootargs(&ctx, "two=2", 0), 0);
+    val = (const char *)fdt_getprop(&ctx, off, "bootargs", &len);
+    ck_assert_str_eq(val, "one=1");
+
+    /* present + force 1: replaced */
+    ck_assert_int_eq(fdt_fixup_bootargs(&ctx, "two=2", 1), 0);
+    off = fdt_subnode_offset(&ctx, 0, "chosen");
+    val = (const char *)fdt_getprop(&ctx, off, "bootargs", &len);
+    ck_assert_str_eq(val, "two=2");
+
+    /* empty (lone NUL) value counts as missing: filled even with force 0 */
+    ck_assert_int_eq(fdt_fixup_str(&ctx, off, "chosen", "bootargs", ""), 0);
+    ck_assert_int_eq(fdt_fixup_bootargs(&ctx, "three=3", 0), 0);
+    off = fdt_subnode_offset(&ctx, 0, "chosen");
+    val = (const char *)fdt_getprop(&ctx, off, "bootargs", &len);
+    ck_assert_str_eq(val, "three=3");
+
+    /* NULL args are rejected */
+    ck_assert_int_eq(fdt_fixup_bootargs(&ctx, NULL, 0), -FDT_ERR_BADARG);
+
+    /* a value with no NUL inside its declared length is malformed and is
+     * replaced even with force 0 */
+    ck_assert_int_eq(fdt_setprop(&ctx, off, "bootargs", "abc", 3), 0);
+    ck_assert_int_eq(fdt_fixup_bootargs(&ctx, "four=4", 0), 0);
+    off = fdt_subnode_offset(&ctx, 0, "chosen");
+    val = (const char *)fdt_getprop(&ctx, off, "bootargs", &len);
+    ck_assert_str_eq(val, "four=4");
+
+    /* an empty string padded to length 2 is still empty: replaced */
+    ck_assert_int_eq(fdt_setprop(&ctx, off, "bootargs", "\0", 2), 0);
+    ck_assert_int_eq(fdt_fixup_bootargs(&ctx, "five=5", 0), 0);
+    off = fdt_subnode_offset(&ctx, 0, "chosen");
+    val = (const char *)fdt_getprop(&ctx, off, "bootargs", &len);
+    ck_assert_str_eq(val, "five=5");
+}
+END_TEST
+
 /* A start+size that wraps must be rejected: linux,initrd-end would
  * otherwise precede linux,initrd-start. */
 START_TEST(test_fdt_fixup_initrd_rejects_wrapped_end)
@@ -948,6 +1005,7 @@ static Suite *fdt_suite(void)
     tcase_add_test(tc, test_fdt_add_subnode_bounded_by_capacity);
     tcase_add_test(tc, test_fdt_setprop_resizes_existing_property);
     tcase_add_test(tc, test_fdt_fixup_initrd);
+    tcase_add_test(tc, test_fdt_fixup_bootargs_keep_and_force);
     tcase_add_test(tc, test_fdt_fixup_initrd_rejects_wrapped_end);
     tcase_add_test(tc, test_fdt_peek_size_header_only);
     tcase_add_test(tc, test_fit_find_images_rejects_unterminated_image_name);