[PATCH v4 0/5] Populate kaslr seed with RNG

[PATCH v4 0/5] Populate kaslr seed with RNG

[seanedmond]

From: Sean Edmond <seanedmond@microsoft.com>

This patch series creates a common API (fdt_fixup_kaslr_seed()) for 
populating the kaslr seed in the DTB.  Existing users (kaslrseed,
and ARMv8 sec firmware) have been updated to use this common API.

New functionality has been introduced to populate the kaslr using
the RNG.  This can be enabled with CONFIG_RNG_TPM_SEED.  

changes in v4:
- Fix compile issue when CONFIG_OF_CONTROL not set

changes in v3:
- Populate with RNG device instead of TPM device (this is a more generic solution)
- Use event spy to do the FDT fixup
- fix compile error for sandbox for !OFNODE_MULTI_TREE

changes in v2:
- fdt_fixup_kaslr_seed() uses the ofnode API
- Add root_ofnode_from_fdt() to get the root node from an FDT and
  perform error checking on the oftree
- add comments to exported functions
- Add error checking in image_setup_libfdt() for return from
  fdt_tpm_kaslr_seed()
- uclass_get_device() -> uclass_first_device_err()
- Change default config for OFNODE_MULTI_TREE (y if !OF_LIVE)

Dhananjay Phadke (2):
  fdt: common API to populate kaslr seed
  fdt: kaslr seed from RNG device

Sean Edmond (3):
  cmd: kaslrseed: Use common API to fixup FDT
  dm: core: Modify default for OFNODE_MULTI_TREE
  fdt: Fix compile error for !OFNODE_MULTI_TREE

 arch/arm/cpu/armv8/sec_firmware.c | 39 ++++++++--------------
 boot/fdt_support.c                | 55 +++++++++++++++++++++++++++++++
 cmd/kaslrseed.c                   | 22 +++++--------
 drivers/core/Kconfig              |  3 +-
 drivers/core/ofnode.c             | 29 ++++++++++++----
 include/dm/ofnode.h               | 12 +++++++
 include/fdt_support.h             |  9 +++++
 lib/Kconfig                       |  7 ++++
 8 files changed, 130 insertions(+), 46 deletions(-)

-- 
2.42.0

[PATCH v4 1/5] fdt: common API to populate kaslr seed

[seanedmond]

From: Dhananjay Phadke <dphadke@linux.microsoft.com>

fdt_fixup_kaslr_seed() will update given ofnode with random seed value.
Source for random seed can be TPM or RNG driver in u-boot or sec
firmware (ARM).

Signed-off-by: Dhananjay Phadke <dphadke@linux.microsoft.com>
Signed-off-by: Sean Edmond <senaedmond@microsoft.com>
---
 arch/arm/cpu/armv8/sec_firmware.c | 39 +++++++++++--------------------
 boot/fdt_support.c                | 19 +++++++++++++++
 drivers/core/ofnode.c             | 17 ++++++++++++++
 include/dm/ofnode.h               | 12 ++++++++++
 include/fdt_support.h             |  9 +++++++
 5 files changed, 71 insertions(+), 25 deletions(-)

diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
index c0e8726346f..5f04cd8aecd 100644
--- a/arch/arm/cpu/armv8/sec_firmware.c
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -411,46 +411,35 @@ int sec_firmware_init(const void *sec_firmware_img,
 /*
  * fdt_fix_kaslr - Add kalsr-seed node in Device tree
  * @fdt:		Device tree
- * @eret:		0 in case of error, 1 for success
+ * @eret:		0 for success
  */
 int fdt_fixup_kaslr(void *fdt)
 {
-	int nodeoffset;
-	int err, ret = 0;
-	u8 rand[8];
+	int ret = 0;
 
 #if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT)
+	u8 rand[8];
+	ofnode root;
+
 	/* Check if random seed generation is  supported */
 	if (sec_firmware_support_hwrng() == false) {
 		printf("WARNING: SEC firmware not running, no kaslr-seed\n");
-		return 0;
+		return -EOPNOTSUPP;
 	}
 
-	err = sec_firmware_get_random(rand, 8);
-	if (err < 0) {
+	ret = sec_firmware_get_random(rand, 8);
+	if (ret < 0) {
 		printf("WARNING: No random number to set kaslr-seed\n");
-		return 0;
+		return ret;
 	}
 
-	err = fdt_check_header(fdt);
-	if (err < 0) {
-		printf("fdt_chosen: %s\n", fdt_strerror(err));
-		return 0;
+	ret = root_ofnode_from_fdt(fdt, &root);
+	if (ret < 0) {
+		printf("WARNING: Unable to get root ofnode\n");
+		return ret;
 	}
 
-	/* find or create "/chosen" node. */
-	nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen");
-	if (nodeoffset < 0)
-		return 0;
-
-	err = fdt_setprop(fdt, nodeoffset, "kaslr-seed", rand,
-				  sizeof(rand));
-	if (err < 0) {
-		printf("WARNING: can't set kaslr-seed %s.\n",
-		       fdt_strerror(err));
-		return 0;
-	}
-	ret = 1;
+	ret = fdt_fixup_kaslr_seed(root, rand, sizeof(rand));
 #endif
 
 	return ret;
diff --git a/boot/fdt_support.c b/boot/fdt_support.c
index 5e49078f8c3..52be4375b46 100644
--- a/boot/fdt_support.c
+++ b/boot/fdt_support.c
@@ -631,6 +631,25 @@ void fdt_fixup_ethernet(void *fdt)
 	}
 }
 
+int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int len)
+{
+	ofnode chosen;
+	int ret;
+
+	/* find or create "/chosen" node. */
+	ret = ofnode_add_subnode(node, "chosen", &chosen);
+	if (ret && ret != -EEXIST)
+		return -ENOENT;
+
+	ret = ofnode_write_prop(chosen, "kaslr-seed", seed, len, true);
+	if (ret) {
+		printf("WARNING: can't set kaslr-seed\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 int fdt_record_loadable(void *blob, u32 index, const char *name,
 			uintptr_t load_addr, u32 size, uintptr_t entry_point,
 			const char *type, const char *os, const char *arch)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 29a42945102..55291f0202b 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -966,6 +966,23 @@ ofnode oftree_path(oftree tree, const char *path)
 	}
 }
 
+int root_ofnode_from_fdt(void *fdt, ofnode *root_node)
+{
+	oftree tree;
+	/* If OFNODE_MULTI_TREE is not set, and if fdt is not the control FDT,
+	 *  oftree_from_fdt() will return NULL
+	 */
+	tree = oftree_from_fdt(fdt);
+
+	if (!oftree_valid(tree)) {
+		printf("Cannot create oftree\n");
+		return -EINVAL;
+	}
+	*root_node = oftree_root(tree);
+
+	return 0;
+}
+
 const void *ofnode_read_chosen_prop(const char *propname, int *sizep)
 {
 	ofnode chosen_node;
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 19e97a90327..5759cac5b30 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -936,6 +936,18 @@ ofnode oftree_path(oftree tree, const char *path);
  */
 ofnode oftree_root(oftree tree);
 
+/**
+ * root_ofnode_from_fdt() - Gets the root ofnode given an FDT blob.
+ *                          Note, this will fail if OFNODE_MULTI_TREE
+ *                          is not set.
+ *
+ * @fdt: Device tree to use
+ * @root_node : Root ofnode
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int root_ofnode_from_fdt(void *fdt, ofnode *root_node);
+
 /**
  * ofnode_read_chosen_prop() - get the value of a chosen property
  *
diff --git a/include/fdt_support.h b/include/fdt_support.h
index 2cd83668982..d967118bedf 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -11,6 +11,7 @@
 	!defined(USE_HOSTCC)
 
 #include <asm/u-boot.h>
+#include <dm/ofnode.h>
 #include <linux/libfdt.h>
 #include <abuf.h>
 
@@ -121,6 +122,14 @@ static inline int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[],
 #endif
 
 void fdt_fixup_ethernet(void *fdt);
+
+/*
+ * fdt_fixup_kaslr_seed - Add kaslr-seed node in Device tree
+ * @node:		ofnode
+ * @eret:		0 for success
+ */
+int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int len);
+
 int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
 			 const void *val, int len, int create);
 void fdt_fixup_qe_firmware(void *fdt);
-- 
2.42.0

[PATCH v4 2/5] fdt: kaslr seed from RNG device

[seanedmond]

From: Dhananjay Phadke <dphadke@linux.microsoft.com>

Add support for KASLR seed from the RNG device. Invokes dm_rng_read()
API to read 8-bytes of random bytes.  Performs the FDT fixup using event
spy.  To enable use CONFIG_KASLR_RNG_SEED

Signed-off-by: Dhananjay Phadke <dphadke@linux.microsoft.com>
Signed-off-by: Drew Kluemke <ankluemk@microsoft.com>
Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
---
 boot/fdt_support.c | 36 ++++++++++++++++++++++++++++++++++++
 lib/Kconfig        |  7 +++++++
 2 files changed, 43 insertions(+)

diff --git a/boot/fdt_support.c b/boot/fdt_support.c
index 52be4375b46..09ce5828659 100644
--- a/boot/fdt_support.c
+++ b/boot/fdt_support.c
@@ -12,7 +12,10 @@
 #include <log.h>
 #include <mapmem.h>
 #include <net.h>
+#include <rng.h>
 #include <stdio_dev.h>
+#include <dm/device.h>
+#include <dm/uclass.h>
 #include <dm/ofnode.h>
 #include <linux/ctype.h>
 #include <linux/types.h>
@@ -650,6 +653,39 @@ int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int len)
 	return 0;
 }
 
+int fdt_rng_kaslr_seed(void *ctx, struct event *event)
+{
+	u8 rand[8] = {0};
+	struct udevice *dev;
+	int ret;
+	oftree tree = event->data.ft_fixup.tree;
+	ofnode root_node = oftree_root(tree);
+
+	ret = uclass_first_device_err(UCLASS_RNG, &dev);
+	if (ret) {
+		printf("ERROR: Failed to find RNG device\n");
+		return ret;
+	}
+
+	ret = dm_rng_read(dev, rand, sizeof(rand));
+	if (ret) {
+		printf("ERROR: RNG read failed, ret=%d\n", ret);
+		return ret;
+	}
+
+	ret = fdt_fixup_kaslr_seed(root_node, rand, sizeof(rand));
+	if (ret) {
+		printf("ERROR: failed to add kaslr-seed to fdt\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+#if defined(CONFIG_KASLR_RNG_SEED)
+EVENT_SPY(EVT_FT_FIXUP, fdt_rng_kaslr_seed);
+#endif
+
 int fdt_record_loadable(void *blob, u32 index, const char *name,
 			uintptr_t load_addr, u32 size, uintptr_t entry_point,
 			const char *type, const char *os, const char *arch)
diff --git a/lib/Kconfig b/lib/Kconfig
index 19649517a39..4f5dfc00d6f 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -477,6 +477,13 @@ config VPL_TPM
 	  for the low-level TPM interface, but only one TPM is supported at
 	  a time by the TPM library.
 
+config KASLR_RNG_SEED
+	bool "Use RNG driver for KASLR random seed"
+	depends on DM_RNG
+	help
+	  This enables support for using the RNG driver as entropy source for
+	  KASLR seed populated in kernel's device tree.
+
 endmenu
 
 menu "Android Verified Boot"
-- 
2.42.0

[PATCH v4 3/5] cmd: kaslrseed: Use common API to fixup FDT

[seanedmond]

From: Sean Edmond <seanedmond@microsoft.com>

Use the newly introduced common API fdt_fixup_kaslr_seed() in the
kaslrseed command.

Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
---
 cmd/kaslrseed.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/cmd/kaslrseed.c b/cmd/kaslrseed.c
index 9acb8e16386..6f423702e7c 100644
--- a/cmd/kaslrseed.c
+++ b/cmd/kaslrseed.c
@@ -19,7 +19,7 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag, int argc, char *const
 	size_t n = 0x8;
 	struct udevice *dev;
 	u64 *buf;
-	int nodeoffset;
+	ofnode root;
 	int ret = CMD_RET_SUCCESS;
 
 	if (uclass_get_device(UCLASS_RNG, 0, &dev) || !dev) {
@@ -45,21 +45,15 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag, int argc, char *const
 		return CMD_RET_FAILURE;
 	}
 
-	ret = fdt_check_header(working_fdt);
-	if (ret < 0) {
-		printf("fdt_chosen: %s\n", fdt_strerror(ret));
-		return CMD_RET_FAILURE;
-	}
-
-	nodeoffset = fdt_find_or_add_subnode(working_fdt, 0, "chosen");
-	if (nodeoffset < 0) {
-		printf("Reading chosen node failed\n");
-		return CMD_RET_FAILURE;
+	ret = root_ofnode_from_fdt(working_fdt, &root);
+	if (ret) {
+		printf("ERROR: Unable to get root ofnode\n");
+		goto CMD_RET_FAILURE;
 	}
 
-	ret = fdt_setprop(working_fdt, nodeoffset, "kaslr-seed", buf, sizeof(buf));
-	if (ret < 0) {
-		printf("Unable to set kaslr-seed on chosen node: %s\n", fdt_strerror(ret));
+	ret = fdt_fixup_kaslr_seed(root, buf, sizeof(buf));
+	if (ret) {
+		printf("ERROR: failed to add kaslr-seed to fdt\n");
 		return CMD_RET_FAILURE;
 	}
 
-- 
2.42.0

[PATCH v4 4/5] dm: core: Modify default for OFNODE_MULTI_TREE

[seanedmond]

From: Sean Edmond <seanedmond@microsoft.com>

There is a preference to use the "ofnode" API for FDT fixups
moving forward.  The FDT fixup will usually be for the kernel FDT.  To
fixup the kernel FDT with the ofnode API, it's required to set the
OFNODE_MULTI_TREE option.

To ensure existing users of kaslr fdt fixup are not impacted, Let's modify
the default value for OFNODE_MULTI_TREE to ensure it's always set if
!OF_LIVE.  This will cause a 1007 byte increase in the code size.

Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
---
 drivers/core/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index fe5c41d57ec..f8f4d429eba 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -424,7 +424,8 @@ config DM_DEV_READ_INLINE
 
 config OFNODE_MULTI_TREE
 	bool "Allow the ofnode interface to access any tree"
-	default y if EVENT && !DM_DEV_READ_INLINE && !DM_INLINE_OFNODE
+	depends on OF_CONTROL
+	default y if !OF_LIVE
 	help
 	  Normally U-Boot makes use of its control FDT, the one used to bind
 	  devices and provide options. In some cases, U-Boot must also process
-- 
2.42.0

[PATCH v4 5/5] fdt: Fix compile error for !OFNODE_MULTI_TREE

[seanedmond]

From: Sean Edmond <seanedmond@microsoft.com>

Required to fix the following compile error when building sandbox:
/tmp/cci9ibby.ltrans21.ltrans.o: In function `do_cedit_load':
<artificial>:(.text+0x601d): undefined reference to `oftree_dispose'

Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
---
 drivers/core/ofnode.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 55291f0202b..c3d326831fc 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -135,12 +135,6 @@ int oftree_new(oftree *treep)
 	return 0;
 }
 
-void oftree_dispose(oftree tree)
-{
-	if (of_live_active())
-		of_live_free(tree.np);
-}
-
 void *ofnode_lookup_fdt(ofnode node)
 {
 	if (gd->flags & GD_FLG_RELOC) {
@@ -243,6 +237,12 @@ int oftree_new(oftree *treep)
 
 #endif /* OFNODE_MULTI_TREE */
 
+void oftree_dispose(oftree tree)
+{
+	if (of_live_active())
+		of_live_free(tree.np);
+}
+
 int oftree_to_fdt(oftree tree, struct abuf *buf)
 {
 	int ret;
-- 
2.42.0

Re: [PATCH v4 5/5] fdt: Fix compile error for !OFNODE_MULTI_TREE

[Simon Glass]

On Fri, 3 Nov 2023 at 12:38, <seanedmond@linux.microsoft.com> wrote:
>
> From: Sean Edmond <seanedmond@microsoft.com>
>
> Required to fix the following compile error when building sandbox:
> /tmp/cci9ibby.ltrans21.ltrans.o: In function `do_cedit_load':
> <artificial>:(.text+0x601d): undefined reference to `oftree_dispose'
>
> Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
> ---
>  drivers/core/ofnode.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v4 4/5] dm: core: Modify default for OFNODE_MULTI_TREE

[Simon Glass]

On Fri, 3 Nov 2023 at 12:38, <seanedmond@linux.microsoft.com> wrote:
>
> From: Sean Edmond <seanedmond@microsoft.com>
>
> There is a preference to use the "ofnode" API for FDT fixups
> moving forward.  The FDT fixup will usually be for the kernel FDT.  To
> fixup the kernel FDT with the ofnode API, it's required to set the
> OFNODE_MULTI_TREE option.
>
> To ensure existing users of kaslr fdt fixup are not impacted, Let's modify
> the default value for OFNODE_MULTI_TREE to ensure it's always set if
> !OF_LIVE.  This will cause a 1007 byte increase in the code size.
>
> Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
> ---
>  drivers/core/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v4 0/5] Populate kaslr seed with RNG

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2067 bytes --]

On Fri, Nov 03, 2023 at 11:38:39AM -0700, seanedmond@linux.microsoft.com wrote:

> From: Sean Edmond <seanedmond@microsoft.com>
> 
> This patch series creates a common API (fdt_fixup_kaslr_seed()) for 
> populating the kaslr seed in the DTB.  Existing users (kaslrseed,
> and ARMv8 sec firmware) have been updated to use this common API.
> 
> New functionality has been introduced to populate the kaslr using
> the RNG.  This can be enabled with CONFIG_RNG_TPM_SEED.  

Please run this series through CI:
https://docs.u-boot.org/en/latest/develop/ci_testing.html
With the whole series applied I get:
========================================== FAILURES ===========================================
_______________________________ test_ut[ut_bootstd_cedit_base] ________________________________
test/py/tests/test_ut.py:503: in test_ut
    assert output.endswith('Failures: 0')
E   assert False
E    +  where False = <built-in method endswith of str object at 0x7f8536092ef0>('Failures: 0')
E    +    where <built-in method endswith of str object at 0x7f8536092ef0> = 'Test: cedit_base: cedit.c\r\r\nCannot create oftree\r\r\ntest/boot/cedit.c:27, cedit_base(): 0 == run_command("cedit load hostfs - cedit.dtb", 0): Expected 0x0 (0), got 0x1 (1)\r\r\nTest cedit_base failed 1 times\r\r\nFailures: 1'.endswith
------------------------------------ Captured stdout call -------------------------------------
=> ut bootstd cedit_base
Test: cedit_base: cedit.c
Cannot create oftree
test/boot/cedit.c:27, cedit_base(): 0 == run_command("cedit load hostfs - cedit.dtb", 0): Expected 0x0 (0), got 0x1 (1)
Test cedit_base failed 1 times
Failures: 1
=>
=================================== short test summary info ===================================
FAILED test/py/tests/test_ut.py::test_ut[ut_bootstd_cedit_base] - assert False

And the problem is in one of these two commits (one does not build by
itself leading to this bisect outcome):
dm: core: Modify default for OFNODE_MULTI_TREE
fdt: Fix compile error for !OFNODE_MULTI_TREE

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v4 4/5] dm: core: Modify default for OFNODE_MULTI_TREE

[Simon Glass]

Hi Sean,

On Fri, 3 Nov 2023 at 13:38, Simon Glass <sjg@chromium.org> wrote:
>
> On Fri, 3 Nov 2023 at 12:38, <seanedmond@linux.microsoft.com> wrote:
> >
> > From: Sean Edmond <seanedmond@microsoft.com>
> >
> > There is a preference to use the "ofnode" API for FDT fixups
> > moving forward.  The FDT fixup will usually be for the kernel FDT.  To
> > fixup the kernel FDT with the ofnode API, it's required to set the
> > OFNODE_MULTI_TREE option.
> >
> > To ensure existing users of kaslr fdt fixup are not impacted, Let's modify
> > the default value for OFNODE_MULTI_TREE to ensure it's always set if
> > !OF_LIVE.  This will cause a 1007 byte increase in the code size.
> >
> > Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
> > ---
> >  drivers/core/Kconfig | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
>
> Reviewed-by: Simon Glass <sjg@chromium.org>

This actually disables the option for sandbox. You may as well enabled
it by default always, since if OF_LIVE is active we are already adding
a lot of code.

That explains the cedit error that Tom mentioned.

Regards,
Simon

Re: [PATCH v4 1/5] fdt: common API to populate kaslr seed

[Simon Glass]

Hi Sean,

On Fri, 3 Nov 2023 at 12:39, <seanedmond@linux.microsoft.com> wrote:
>
> From: Dhananjay Phadke <dphadke@linux.microsoft.com>
>
> fdt_fixup_kaslr_seed() will update given ofnode with random seed value.
> Source for random seed can be TPM or RNG driver in u-boot or sec
> firmware (ARM).
>
> Signed-off-by: Dhananjay Phadke <dphadke@linux.microsoft.com>
> Signed-off-by: Sean Edmond <senaedmond@microsoft.com>
> ---
>  arch/arm/cpu/armv8/sec_firmware.c | 39 +++++++++++--------------------
>  boot/fdt_support.c                | 19 +++++++++++++++
>  drivers/core/ofnode.c             | 17 ++++++++++++++
>  include/dm/ofnode.h               | 12 ++++++++++
>  include/fdt_support.h             |  9 +++++++
>  5 files changed, 71 insertions(+), 25 deletions(-)
>
> diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
> index c0e8726346f..5f04cd8aecd 100644
> --- a/arch/arm/cpu/armv8/sec_firmware.c
> +++ b/arch/arm/cpu/armv8/sec_firmware.c
> @@ -411,46 +411,35 @@ int sec_firmware_init(const void *sec_firmware_img,
>  /*
>   * fdt_fix_kaslr - Add kalsr-seed node in Device tree
>   * @fdt:               Device tree
> - * @eret:              0 in case of error, 1 for success
> + * @eret:              0 for success
>   */
>  int fdt_fixup_kaslr(void *fdt)
>  {
> -       int nodeoffset;
> -       int err, ret = 0;
> -       u8 rand[8];
> +       int ret = 0;
>
>  #if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT)
> +       u8 rand[8];
> +       ofnode root;
> +
>         /* Check if random seed generation is  supported */
>         if (sec_firmware_support_hwrng() == false) {
>                 printf("WARNING: SEC firmware not running, no kaslr-seed\n");
> -               return 0;
> +               return -EOPNOTSUPP;
>         }
>
> -       err = sec_firmware_get_random(rand, 8);
> -       if (err < 0) {
> +       ret = sec_firmware_get_random(rand, 8);
> +       if (ret < 0) {
>                 printf("WARNING: No random number to set kaslr-seed\n");
> -               return 0;
> +               return ret;
>         }
>
> -       err = fdt_check_header(fdt);
> -       if (err < 0) {
> -               printf("fdt_chosen: %s\n", fdt_strerror(err));
> -               return 0;
> +       ret = root_ofnode_from_fdt(fdt, &root);
> +       if (ret < 0) {
> +               printf("WARNING: Unable to get root ofnode\n");
> +               return ret;
>         }
>
> -       /* find or create "/chosen" node. */
> -       nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen");
> -       if (nodeoffset < 0)
> -               return 0;
> -
> -       err = fdt_setprop(fdt, nodeoffset, "kaslr-seed", rand,
> -                                 sizeof(rand));
> -       if (err < 0) {
> -               printf("WARNING: can't set kaslr-seed %s.\n",
> -                      fdt_strerror(err));
> -               return 0;
> -       }
> -       ret = 1;
> +       ret = fdt_fixup_kaslr_seed(root, rand, sizeof(rand));
>  #endif
>
>         return ret;
> diff --git a/boot/fdt_support.c b/boot/fdt_support.c
> index 5e49078f8c3..52be4375b46 100644
> --- a/boot/fdt_support.c
> +++ b/boot/fdt_support.c
> @@ -631,6 +631,25 @@ void fdt_fixup_ethernet(void *fdt)
>         }
>  }
>
> +int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int len)
> +{
> +       ofnode chosen;
> +       int ret;
> +
> +       /* find or create "/chosen" node. */
> +       ret = ofnode_add_subnode(node, "chosen", &chosen);
> +       if (ret && ret != -EEXIST)
> +               return -ENOENT;
> +
> +       ret = ofnode_write_prop(chosen, "kaslr-seed", seed, len, true);
> +       if (ret) {
> +               printf("WARNING: can't set kaslr-seed\n");
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
>  int fdt_record_loadable(void *blob, u32 index, const char *name,
>                         uintptr_t load_addr, u32 size, uintptr_t entry_point,
>                         const char *type, const char *os, const char *arch)
> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> index 29a42945102..55291f0202b 100644
> --- a/drivers/core/ofnode.c
> +++ b/drivers/core/ofnode.c
> @@ -966,6 +966,23 @@ ofnode oftree_path(oftree tree, const char *path)
>         }
>  }
>
> +int root_ofnode_from_fdt(void *fdt, ofnode *root_node)
> +{
> +       oftree tree;
> +       /* If OFNODE_MULTI_TREE is not set, and if fdt is not the control FDT,
> +        *  oftree_from_fdt() will return NULL
> +        */
> +       tree = oftree_from_fdt(fdt);
> +
> +       if (!oftree_valid(tree)) {
> +               printf("Cannot create oftree\n");
> +               return -EINVAL;
> +       }
> +       *root_node = oftree_root(tree);
> +
> +       return 0;
> +}
> +
>  const void *ofnode_read_chosen_prop(const char *propname, int *sizep)
>  {
>         ofnode chosen_node;
> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
> index 19e97a90327..5759cac5b30 100644
> --- a/include/dm/ofnode.h
> +++ b/include/dm/ofnode.h
> @@ -936,6 +936,18 @@ ofnode oftree_path(oftree tree, const char *path);
>   */
>  ofnode oftree_root(oftree tree);
>
> +/**
> + * root_ofnode_from_fdt() - Gets the root ofnode given an FDT blob.
> + *                          Note, this will fail if OFNODE_MULTI_TREE
> + *                          is not set.
> + *
> + * @fdt: Device tree to use
> + * @root_node : Root ofnode
> + *
> + * Return: 0 if OK, -ve on error
> + */
> +int root_ofnode_from_fdt(void *fdt, ofnode *root_node);
> +
>  /**
>   * ofnode_read_chosen_prop() - get the value of a chosen property
>   *
> diff --git a/include/fdt_support.h b/include/fdt_support.h
> index 2cd83668982..d967118bedf 100644
> --- a/include/fdt_support.h
> +++ b/include/fdt_support.h
> @@ -11,6 +11,7 @@
>         !defined(USE_HOSTCC)
>
>  #include <asm/u-boot.h>
> +#include <dm/ofnode.h>

I believe you only need dm/ofnode-decl.h here. That avoids pulling in
ofnode.h which ends up with of.h which includes asm/global_data.h
which causes a warning for qemu_arm64 for me.

Perhaps that last include could be dropped? But in any case, we should
use ofnode-decl.h in preference to ofnode.h if possible

>  #include <linux/libfdt.h>
>  #include <abuf.h>
>
> @@ -121,6 +122,14 @@ static inline int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[],
>  #endif
>
>  void fdt_fixup_ethernet(void *fdt);
> +
> +/*
> + * fdt_fixup_kaslr_seed - Add kaslr-seed node in Device tree
> + * @node:              ofnode
> + * @eret:              0 for success
> + */
> +int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int len);
> +
>  int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
>                          const void *val, int len, int create);
>  void fdt_fixup_qe_firmware(void *fdt);
> --
> 2.42.0
>

Regards,
Simon

Re: [PATCH v4 3/5] cmd: kaslrseed: Use common API to fixup FDT

[Simon Glass]

Hi Sean,

On Fri, 3 Nov 2023 at 12:38, <seanedmond@linux.microsoft.com> wrote:
>
> From: Sean Edmond <seanedmond@microsoft.com>
>
> Use the newly introduced common API fdt_fixup_kaslr_seed() in the
> kaslrseed command.
>
> Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
> ---
>  cmd/kaslrseed.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/cmd/kaslrseed.c b/cmd/kaslrseed.c
> index 9acb8e16386..6f423702e7c 100644
> --- a/cmd/kaslrseed.c
> +++ b/cmd/kaslrseed.c
> @@ -19,7 +19,7 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag, int argc, char *const
>         size_t n = 0x8;
>         struct udevice *dev;
>         u64 *buf;
> -       int nodeoffset;
> +       ofnode root;
>         int ret = CMD_RET_SUCCESS;
>
>         if (uclass_get_device(UCLASS_RNG, 0, &dev) || !dev) {
> @@ -45,21 +45,15 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag, int argc, char *const
>                 return CMD_RET_FAILURE;
>         }
>
> -       ret = fdt_check_header(working_fdt);
> -       if (ret < 0) {
> -               printf("fdt_chosen: %s\n", fdt_strerror(ret));
> -               return CMD_RET_FAILURE;
> -       }
> -
> -       nodeoffset = fdt_find_or_add_subnode(working_fdt, 0, "chosen");
> -       if (nodeoffset < 0) {
> -               printf("Reading chosen node failed\n");
> -               return CMD_RET_FAILURE;
> +       ret = root_ofnode_from_fdt(working_fdt, &root);
> +       if (ret) {
> +               printf("ERROR: Unable to get root ofnode\n");
> +               goto CMD_RET_FAILURE;
>         }
>
> -       ret = fdt_setprop(working_fdt, nodeoffset, "kaslr-seed", buf, sizeof(buf));

It looks like this buf needs a cast to u8 *

> -       if (ret < 0) {
> -               printf("Unable to set kaslr-seed on chosen node: %s\n", fdt_strerror(ret));
> +       ret = fdt_fixup_kaslr_seed(root, buf, sizeof(buf));
> +       if (ret) {
> +               printf("ERROR: failed to add kaslr-seed to fdt\n");
>                 return CMD_RET_FAILURE;
>         }
>
> --
> 2.42.0
>

Regards,
Simon