[PATCH 0/3] Populate kaslr seed with TPM

[PATCH 0/3] Populate kaslr seed with TPM

[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 TPM interface.  This can be enabled with CONFIG_KASLR_TPM_SEED.  

Dhananjay Phadke (2):
  fdt: common API to populate kaslr seed
  fdt: kaslr seed from tpm entropy

Sean Edmond (1):
  cmd: kaslrseed: Use common API to fixup FDT

 arch/arm/cpu/armv8/sec_firmware.c | 32 ++++-----------
 boot/image-fdt.c                  |  3 ++
 cmd/kaslrseed.c                   | 18 ++------
 common/fdt_support.c              | 68 +++++++++++++++++++++++++++++++
 include/fdt_support.h             |  4 ++
 lib/Kconfig                       |  9 ++++
 6 files changed, 94 insertions(+), 40 deletions(-)

-- 
2.40.0

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

[seanedmond]

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

fdt_fixup_kaslr_seed() will update given FDT 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>
---
 arch/arm/cpu/armv8/sec_firmware.c | 32 +++++++------------------------
 common/fdt_support.c              | 31 ++++++++++++++++++++++++++++++
 include/fdt_support.h             |  3 +++
 3 files changed, 41 insertions(+), 25 deletions(-)

diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
index c0e8726346..84ba49924e 100644
--- a/arch/arm/cpu/armv8/sec_firmware.c
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -411,46 +411,28 @@ 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];
+
 	/* 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) {
 		printf("WARNING: No random number to set kaslr-seed\n");
-		return 0;
-	}
-
-	err = fdt_check_header(fdt);
-	if (err < 0) {
-		printf("fdt_chosen: %s\n", fdt_strerror(err));
-		return 0;
+		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(fdt, rand, sizeof(rand));
 #endif
 
 	return ret;
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 5e49078f8c..35d4f26dbd 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -631,6 +631,37 @@ void fdt_fixup_ethernet(void *fdt)
 	}
 }
 
+/*
+ * fdt_fix_kaslr_seed - Add kalsr-seed node in Device tree
+ * @fdt:		Device tree
+ * @eret:		0 for success
+ */
+int fdt_fixup_kaslr_seed(void *fdt, const u8 *seed, int len)
+{
+	int nodeoffset;
+	int err;
+
+	err = fdt_check_header(fdt);
+	if (err < 0) {
+		printf("fdt_chosen: %s\n", fdt_strerror(err));
+		return err;
+	}
+
+	/* find or create "/chosen" node. */
+	nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen");
+	if (nodeoffset < 0)
+		return -ENOENT;
+
+	err = fdt_setprop(fdt, nodeoffset, "kaslr-seed", seed, len);
+	if (err < 0) {
+		printf("WARNING: can't set kaslr-seed %s.\n",
+		       fdt_strerror(err));
+		return err;
+	}
+
+	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/include/fdt_support.h b/include/fdt_support.h
index 2cd8366898..d74ef4e0a7 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -121,6 +121,9 @@ static inline int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[],
 #endif
 
 void fdt_fixup_ethernet(void *fdt);
+
+int fdt_fixup_kaslr_seed(void *fdt, 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.40.0

[PATCH 2/3] fdt: kaslr seed from tpm entropy

[seanedmond]

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

Add support for KASLR seed from TPM device. Invokes tpm_get_random()
API to read 8-bytes of random bytes for KASLR.

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/image-fdt.c      |  3 +++
 common/fdt_support.c  | 39 ++++++++++++++++++++++++++++++++++++++-
 include/fdt_support.h |  1 +
 lib/Kconfig           |  9 +++++++++
 4 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index f10200f647..127443963e 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -624,6 +624,9 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob,
 		goto err;
 	}
 
+	if (IS_ENABLED(CONFIG_KASLR_TPM_SEED))
+		fdt_tpm_kaslr_seed(blob);
+
 	fdt_ret = optee_copy_fdt_nodes(blob);
 	if (fdt_ret) {
 		printf("ERROR: transfer of optee nodes to new fdt failed: %s\n",
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 35d4f26dbd..1ac33355a0 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -13,6 +13,10 @@
 #include <mapmem.h>
 #include <net.h>
 #include <stdio_dev.h>
+#include <tpm-v1.h>
+#include <tpm-v2.h>
+#include <dm/device.h>
+#include <dm/uclass.h>
 #include <dm/ofnode.h>
 #include <linux/ctype.h>
 #include <linux/types.h>
@@ -632,7 +636,7 @@ void fdt_fixup_ethernet(void *fdt)
 }
 
 /*
- * fdt_fix_kaslr_seed - Add kalsr-seed node in Device tree
+ * fdt_fixup_kaslr_seed - Add kaslr-seed node in Device tree
  * @fdt:		Device tree
  * @eret:		0 for success
  */
@@ -662,6 +666,39 @@ int fdt_fixup_kaslr_seed(void *fdt, const u8 *seed, int len)
 	return 0;
 }
 
+/*
+ * fdt_add_tpm_kaslr_seed - Add kalsr-seed node in Device tree with random
+ *			    bytes from TPM device
+ * @fdt:		Device tree
+ * @eret:		0 for success
+ */
+int fdt_tpm_kaslr_seed(void *fdt)
+{
+	u8 rand[8] = {0};
+	struct udevice *dev;
+	int ret;
+
+	ret = uclass_get_device(UCLASS_TPM, 0, &dev);
+	if (ret) {
+		printf("ERROR: Failed to find TPM device\n");
+		return ret;
+	}
+
+	ret = tpm_get_random(dev, rand, sizeof(rand));
+	if (ret) {
+		printf("ERROR: TPM GetRandom failed, ret=%d\n", ret);
+		return ret;
+	}
+
+	ret = fdt_fixup_kaslr_seed(fdt, rand, sizeof(rand));
+	if (ret) {
+		printf("ERROR: failed to add kaslr-seed to fdt\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/include/fdt_support.h b/include/fdt_support.h
index d74ef4e0a7..9e50db1b96 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -123,6 +123,7 @@ static inline int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[],
 void fdt_fixup_ethernet(void *fdt);
 
 int fdt_fixup_kaslr_seed(void *fdt, const u8 *seed, int len);
+int fdt_tpm_kaslr_seed(void *fdt);
 
 int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
 			 const void *val, int len, int create);
diff --git a/lib/Kconfig b/lib/Kconfig
index 3926652db6..1530ef7c86 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -465,6 +465,15 @@ config VPL_TPM
 	  for the low-level TPM interface, but only one TPM is supported at
 	  a time by the TPM library.
 
+config KASLR_TPM_SEED
+	bool "Use TPM for KASLR random seed"
+	depends on TPM_V1 || TPM_V2
+	help
+	  This enables support for using TPMs as entropy source for KASLR seed
+	  populated in kernel's device tree. Both TPMv1 and TPMv2 are supported
+	  for the low-level TPM interface, but only one TPM is supported at
+	  a time by the library.
+
 endmenu
 
 menu "Android Verified Boot"
-- 
2.40.0

[PATCH 3/3] 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 | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/cmd/kaslrseed.c b/cmd/kaslrseed.c
index 8a1d8120cd..82117bc484 100644
--- a/cmd/kaslrseed.c
+++ b/cmd/kaslrseed.c
@@ -45,21 +45,9 @@ 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 = 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(working_fdt, buf, sizeof(buf));
+	if (ret) {
+		printf("ERROR: failed to add kaslr-seed to fdt\n");
 		return CMD_RET_FAILURE;
 	}
 
-- 
2.40.0

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

[Simon Glass]

Hi,

On Fri, 4 Aug 2023 at 17:34, <seanedmond@linux.microsoft.com> wrote:
>
> From: Dhananjay Phadke <dphadke@linux.microsoft.com>
>
> fdt_fixup_kaslr_seed() will update given FDT 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>
> ---
>  arch/arm/cpu/armv8/sec_firmware.c | 32 +++++++------------------------
>  common/fdt_support.c              | 31 ++++++++++++++++++++++++++++++
>  include/fdt_support.h             |  3 +++
>  3 files changed, 41 insertions(+), 25 deletions(-)

We need to find a way to use the ofnode API here.

>
> diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
> index c0e8726346..84ba49924e 100644
> --- a/arch/arm/cpu/armv8/sec_firmware.c
> +++ b/arch/arm/cpu/armv8/sec_firmware.c
> @@ -411,46 +411,28 @@ 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)

You could pass an oftree to this function, e.g. obtained with:

oftree_from_fdt(fdt)

>  {
> -       int nodeoffset;
> -       int err, ret = 0;
> -       u8 rand[8];
> +       int ret = 0;
>
>  #if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT)
> +       u8 rand[8];
> +
>         /* 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) {
>                 printf("WARNING: No random number to set kaslr-seed\n");
> -               return 0;
> -       }
> -
> -       err = fdt_check_header(fdt);
> -       if (err < 0) {
> -               printf("fdt_chosen: %s\n", fdt_strerror(err));
> -               return 0;
> +               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(fdt, rand, sizeof(rand));
>  #endif
>
>         return ret;
> diff --git a/common/fdt_support.c b/common/fdt_support.c
> index 5e49078f8c..35d4f26dbd 100644
> --- a/common/fdt_support.c
> +++ b/common/fdt_support.c
> @@ -631,6 +631,37 @@ void fdt_fixup_ethernet(void *fdt)
>         }
>  }
>
> +/*
> + * fdt_fix_kaslr_seed - Add kalsr-seed node in Device tree
> + * @fdt:               Device tree
> + * @eret:              0 for success
> + */
> +int fdt_fixup_kaslr_seed(void *fdt, const u8 *seed, int len)
> +{
> +       int nodeoffset;
> +       int err;
> +
> +       err = fdt_check_header(fdt);
> +       if (err < 0) {
> +               printf("fdt_chosen: %s\n", fdt_strerror(err));
> +               return err;
> +       }
> +
> +       /* find or create "/chosen" node. */
> +       nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen");
> +       if (nodeoffset < 0)
> +               return -ENOENT;
> +
> +       err = fdt_setprop(fdt, nodeoffset, "kaslr-seed", seed, len);
> +       if (err < 0) {
> +               printf("WARNING: can't set kaslr-seed %s.\n",
> +                      fdt_strerror(err));
> +               return err;
> +       }
> +
> +       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/include/fdt_support.h b/include/fdt_support.h
> index 2cd8366898..d74ef4e0a7 100644
> --- a/include/fdt_support.h
> +++ b/include/fdt_support.h
> @@ -121,6 +121,9 @@ static inline int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[],
>  #endif
>
>  void fdt_fixup_ethernet(void *fdt);
> +
> +int fdt_fixup_kaslr_seed(void *fdt, const u8 *seed, int len);

Please get in the habit of adding full comments to exported functions.

> +
>  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.40.0
>

Regards,
Simon

Re: [PATCH 2/3] fdt: kaslr seed from tpm entropy

[Simon Glass]

Hi,

On Fri, 4 Aug 2023 at 17:34, <seanedmond@linux.microsoft.com> wrote:
>
> From: Dhananjay Phadke <dphadke@linux.microsoft.com>
>
> Add support for KASLR seed from TPM device. Invokes tpm_get_random()
> API to read 8-bytes of random bytes for KASLR.
>
> 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/image-fdt.c      |  3 +++
>  common/fdt_support.c  | 39 ++++++++++++++++++++++++++++++++++++++-
>  include/fdt_support.h |  1 +
>  lib/Kconfig           |  9 +++++++++
>  4 files changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> index f10200f647..127443963e 100644
> --- a/boot/image-fdt.c
> +++ b/boot/image-fdt.c
> @@ -624,6 +624,9 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob,
>                 goto err;
>         }
>
> +       if (IS_ENABLED(CONFIG_KASLR_TPM_SEED))
> +               fdt_tpm_kaslr_seed(blob);

Error checking needed. Also please make your new function take an
oftree or ofnode

> +
>         fdt_ret = optee_copy_fdt_nodes(blob);
>         if (fdt_ret) {
>                 printf("ERROR: transfer of optee nodes to new fdt failed: %s\n",
> diff --git a/common/fdt_support.c b/common/fdt_support.c
> index 35d4f26dbd..1ac33355a0 100644
> --- a/common/fdt_support.c
> +++ b/common/fdt_support.c
> @@ -13,6 +13,10 @@
>  #include <mapmem.h>
>  #include <net.h>
>  #include <stdio_dev.h>
> +#include <tpm-v1.h>
> +#include <tpm-v2.h>
> +#include <dm/device.h>
> +#include <dm/uclass.h>
>  #include <dm/ofnode.h>
>  #include <linux/ctype.h>
>  #include <linux/types.h>
> @@ -632,7 +636,7 @@ void fdt_fixup_ethernet(void *fdt)
>  }
>
>  /*
> - * fdt_fix_kaslr_seed - Add kalsr-seed node in Device tree
> + * fdt_fixup_kaslr_seed - Add kaslr-seed node in Device tree
>   * @fdt:               Device tree
>   * @eret:              0 for success
>   */
> @@ -662,6 +666,39 @@ int fdt_fixup_kaslr_seed(void *fdt, const u8 *seed, int len)
>         return 0;
>  }
>
> +/*
> + * fdt_add_tpm_kaslr_seed - Add kalsr-seed node in Device tree with random
> + *                         bytes from TPM device
> + * @fdt:               Device tree
> + * @eret:              0 for success
> + */
> +int fdt_tpm_kaslr_seed(void *fdt)
> +{
> +       u8 rand[8] = {0};
> +       struct udevice *dev;
> +       int ret;
> +
> +       ret = uclass_get_device(UCLASS_TPM, 0, &dev);

uclass_first_device_err(UCLASS_TPM, &dev)

> +       if (ret) {
> +               printf("ERROR: Failed to find TPM device\n");
> +               return ret;
> +       }
> +
> +       ret = tpm_get_random(dev, rand, sizeof(rand));
> +       if (ret) {
> +               printf("ERROR: TPM GetRandom failed, ret=%d\n", ret);
> +               return ret;
> +       }
> +
> +       ret = fdt_fixup_kaslr_seed(fdt, rand, sizeof(rand));
> +       if (ret) {
> +               printf("ERROR: failed to add kaslr-seed to fdt\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/include/fdt_support.h b/include/fdt_support.h
> index d74ef4e0a7..9e50db1b96 100644
> --- a/include/fdt_support.h
> +++ b/include/fdt_support.h
> @@ -123,6 +123,7 @@ static inline int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[],
>  void fdt_fixup_ethernet(void *fdt);
>
>  int fdt_fixup_kaslr_seed(void *fdt, const u8 *seed, int len);
> +int fdt_tpm_kaslr_seed(void *fdt);
>
>  int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
>                          const void *val, int len, int create);
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 3926652db6..1530ef7c86 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -465,6 +465,15 @@ config VPL_TPM
>           for the low-level TPM interface, but only one TPM is supported at
>           a time by the TPM library.
>
> +config KASLR_TPM_SEED
> +       bool "Use TPM for KASLR random seed"
> +       depends on TPM_V1 || TPM_V2
> +       help
> +         This enables support for using TPMs as entropy source for KASLR seed
> +         populated in kernel's device tree. Both TPMv1 and TPMv2 are supported
> +         for the low-level TPM interface, but only one TPM is supported at
> +         a time by the library.
> +
>  endmenu
>
>  menu "Android Verified Boot"
> --
> 2.40.0

Regards,
Simon

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

[Sean Edmond]

On 2023-08-08 7:03 p.m., Simon Glass wrote:
> Hi,
>
> On Fri, 4 Aug 2023 at 17:34, <seanedmond@linux.microsoft.com> wrote:
>> From: Dhananjay Phadke <dphadke@linux.microsoft.com>
>>
>> fdt_fixup_kaslr_seed() will update given FDT 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>
>> ---
>>   arch/arm/cpu/armv8/sec_firmware.c | 32 +++++++------------------------
>>   common/fdt_support.c              | 31 ++++++++++++++++++++++++++++++
>>   include/fdt_support.h             |  3 +++
>>   3 files changed, 41 insertions(+), 25 deletions(-)
> We need to find a way to use the ofnode API here.
>
>> diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
>> index c0e8726346..84ba49924e 100644
>> --- a/arch/arm/cpu/armv8/sec_firmware.c
>> +++ b/arch/arm/cpu/armv8/sec_firmware.c
>> @@ -411,46 +411,28 @@ 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)
> You could pass an oftree to this function, e.g. obtained with:
>
> oftree_from_fdt(fdt)

The common API I added is fdt_fixup_kaslr_seed(), which was added to 
"common/fdt_support.c".

There are 3 callers:
sec_firmware_init()->fdt_fixup_kaslr_seed()
do_kaslr_seed()->fdt_fixup_kaslr_seed()
image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed()

I think the ask is to create a common API that uses the ofnode API.  So, 
instead of fdt_fixup_kaslr_seed() I can create 
ofnode_fixup_kaslr_seed()?  Where should it live?  Are you also wanting 
the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take oftree as 
input too?

>
>>   {
>> -       int nodeoffset;
>> -       int err, ret = 0;
>> -       u8 rand[8];
>> +       int ret = 0;
>>
>>   #if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT)
>> +       u8 rand[8];
>> +
>>          /* 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) {
>>                  printf("WARNING: No random number to set kaslr-seed\n");
>> -               return 0;
>> -       }
>> -
>> -       err = fdt_check_header(fdt);
>> -       if (err < 0) {
>> -               printf("fdt_chosen: %s\n", fdt_strerror(err));
>> -               return 0;
>> +               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(fdt, rand, sizeof(rand));
>>   #endif
>>
>>          return ret;
>> diff --git a/common/fdt_support.c b/common/fdt_support.c
>> index 5e49078f8c..35d4f26dbd 100644
>> --- a/common/fdt_support.c
>> +++ b/common/fdt_support.c
>> @@ -631,6 +631,37 @@ void fdt_fixup_ethernet(void *fdt)
>>          }
>>   }
>>
>> +/*
>> + * fdt_fix_kaslr_seed - Add kalsr-seed node in Device tree
>> + * @fdt:               Device tree
>> + * @eret:              0 for success
>> + */
>> +int fdt_fixup_kaslr_seed(void *fdt, const u8 *seed, int len)
>> +{
>> +       int nodeoffset;
>> +       int err;
>> +
>> +       err = fdt_check_header(fdt);
>> +       if (err < 0) {
>> +               printf("fdt_chosen: %s\n", fdt_strerror(err));
>> +               return err;
>> +       }
>> +
>> +       /* find or create "/chosen" node. */
>> +       nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen");
>> +       if (nodeoffset < 0)
>> +               return -ENOENT;
>> +
>> +       err = fdt_setprop(fdt, nodeoffset, "kaslr-seed", seed, len);
>> +       if (err < 0) {
>> +               printf("WARNING: can't set kaslr-seed %s.\n",
>> +                      fdt_strerror(err));
>> +               return err;
>> +       }
>> +
>> +       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/include/fdt_support.h b/include/fdt_support.h
>> index 2cd8366898..d74ef4e0a7 100644
>> --- a/include/fdt_support.h
>> +++ b/include/fdt_support.h
>> @@ -121,6 +121,9 @@ static inline int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[],
>>   #endif
>>
>>   void fdt_fixup_ethernet(void *fdt);
>> +
>> +int fdt_fixup_kaslr_seed(void *fdt, const u8 *seed, int len);
> Please get in the habit of adding full comments to exported functions.
>
>> +
>>   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.40.0
>>
> Regards,
> Simon
>
>

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

[Simon Glass]

Hi Sean,

On Wed, 9 Aug 2023 at 16:35, Sean Edmond <seanedmond@linux.microsoft.com> wrote:
>
>
> On 2023-08-08 7:03 p.m., Simon Glass wrote:
> > Hi,
> >
> > On Fri, 4 Aug 2023 at 17:34, <seanedmond@linux.microsoft.com> wrote:
> >> From: Dhananjay Phadke <dphadke@linux.microsoft.com>
> >>
> >> fdt_fixup_kaslr_seed() will update given FDT 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>
> >> ---
> >>   arch/arm/cpu/armv8/sec_firmware.c | 32 +++++++------------------------
> >>   common/fdt_support.c              | 31 ++++++++++++++++++++++++++++++
> >>   include/fdt_support.h             |  3 +++
> >>   3 files changed, 41 insertions(+), 25 deletions(-)
> > We need to find a way to use the ofnode API here.
> >
> >> diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
> >> index c0e8726346..84ba49924e 100644
> >> --- a/arch/arm/cpu/armv8/sec_firmware.c
> >> +++ b/arch/arm/cpu/armv8/sec_firmware.c
> >> @@ -411,46 +411,28 @@ 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)
> > You could pass an oftree to this function, e.g. obtained with:
> >
> > oftree_from_fdt(fdt)
>
> The common API I added is fdt_fixup_kaslr_seed(), which was added to
> "common/fdt_support.c".
>
> There are 3 callers:
> sec_firmware_init()->fdt_fixup_kaslr_seed()
> do_kaslr_seed()->fdt_fixup_kaslr_seed()
> image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed()
>
> I think the ask is to create a common API that uses the ofnode API.  So,
> instead of fdt_fixup_kaslr_seed() I can create
> ofnode_fixup_kaslr_seed()?  Where should it live?

If you like you could add common/ofnode_support.c ?

But it is OK to have it in the same file, I think.

> Are you also wanting
> the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take oftree as
> input too?

So far as you can go, yes. Also you may want to pass an ofnode (the
root node) so that things can deal with adding their stuff to any
node.

Regards,
Simon

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

[Chris Morgan]

On Wed, Aug 09, 2023 at 07:49:08PM -0600, Simon Glass wrote:
> Hi Sean,
> 
> On Wed, 9 Aug 2023 at 16:35, Sean Edmond <seanedmond@linux.microsoft.com> wrote:
> >
> >
> > On 2023-08-08 7:03 p.m., Simon Glass wrote:
> > > Hi,
> > >
> > > On Fri, 4 Aug 2023 at 17:34, <seanedmond@linux.microsoft.com> wrote:
> > >> From: Dhananjay Phadke <dphadke@linux.microsoft.com>
> > >>
> > >> fdt_fixup_kaslr_seed() will update given FDT 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>
> > >> ---
> > >>   arch/arm/cpu/armv8/sec_firmware.c | 32 +++++++------------------------
> > >>   common/fdt_support.c              | 31 ++++++++++++++++++++++++++++++
> > >>   include/fdt_support.h             |  3 +++
> > >>   3 files changed, 41 insertions(+), 25 deletions(-)
> > > We need to find a way to use the ofnode API here.
> > >
> > >> diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
> > >> index c0e8726346..84ba49924e 100644
> > >> --- a/arch/arm/cpu/armv8/sec_firmware.c
> > >> +++ b/arch/arm/cpu/armv8/sec_firmware.c
> > >> @@ -411,46 +411,28 @@ 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)
> > > You could pass an oftree to this function, e.g. obtained with:
> > >
> > > oftree_from_fdt(fdt)
> >
> > The common API I added is fdt_fixup_kaslr_seed(), which was added to
> > "common/fdt_support.c".
> >
> > There are 3 callers:
> > sec_firmware_init()->fdt_fixup_kaslr_seed()
> > do_kaslr_seed()->fdt_fixup_kaslr_seed()
> > image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed()
> >
> > I think the ask is to create a common API that uses the ofnode API.  So,
> > instead of fdt_fixup_kaslr_seed() I can create
> > ofnode_fixup_kaslr_seed()?  Where should it live?
> 
> If you like you could add common/ofnode_support.c ?
> 
> But it is OK to have it in the same file, I think.
> 
> > Are you also wanting
> > the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take oftree as
> > input too?
> 
> So far as you can go, yes. Also you may want to pass an ofnode (the
> root node) so that things can deal with adding their stuff to any
> node.
> 
> Regards,
> Simon

I'm almost wondering if now would be a good time to look at removing
the kaslrseed command and instead doing it in the fdt_support.c like
we do with rng-seed and like is being done here with the TPM seeding.

I'm not a security expert and please don't yell at me if I'm proposing
something stupid, but what if the kaslrseed command be reworked to
only seed with software based entropy via the rand() function?

Thank you,
Chris Morgan

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

[Sean Edmond]

On 2023-08-09 6:49 p.m., Simon Glass wrote:
> Hi Sean,
>
> On Wed, 9 Aug 2023 at 16:35, Sean Edmond <seanedmond@linux.microsoft.com> wrote:
>>
>> On 2023-08-08 7:03 p.m., Simon Glass wrote:
>>> Hi,
>>>
>>> On Fri, 4 Aug 2023 at 17:34, <seanedmond@linux.microsoft.com> wrote:
>>>> From: Dhananjay Phadke <dphadke@linux.microsoft.com>
>>>>
>>>> fdt_fixup_kaslr_seed() will update given FDT 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>
>>>> ---
>>>>    arch/arm/cpu/armv8/sec_firmware.c | 32 +++++++------------------------
>>>>    common/fdt_support.c              | 31 ++++++++++++++++++++++++++++++
>>>>    include/fdt_support.h             |  3 +++
>>>>    3 files changed, 41 insertions(+), 25 deletions(-)
>>> We need to find a way to use the ofnode API here.
>>>
>>>> diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
>>>> index c0e8726346..84ba49924e 100644
>>>> --- a/arch/arm/cpu/armv8/sec_firmware.c
>>>> +++ b/arch/arm/cpu/armv8/sec_firmware.c
>>>> @@ -411,46 +411,28 @@ 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)
>>> You could pass an oftree to this function, e.g. obtained with:
>>>
>>> oftree_from_fdt(fdt)
>> The common API I added is fdt_fixup_kaslr_seed(), which was added to
>> "common/fdt_support.c".
>>
>> There are 3 callers:
>> sec_firmware_init()->fdt_fixup_kaslr_seed()
>> do_kaslr_seed()->fdt_fixup_kaslr_seed()
>> image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed()
>>
>> I think the ask is to create a common API that uses the ofnode API.  So,
>> instead of fdt_fixup_kaslr_seed() I can create
>> ofnode_fixup_kaslr_seed()?  Where should it live?
> If you like you could add common/ofnode_support.c ?
>
> But it is OK to have it in the same file, I think.
>
>> Are you also wanting
>> the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take oftree as
>> input too?
> So far as you can go, yes. Also you may want to pass an ofnode (the
> root node) so that things can deal with adding their stuff to any
> node.
>
> Regards,
> Simon


I re-worked the API to use the ofnode API and tested it on our board.  I 
was required to explicitly enable CONFIG_OFNODE_MULTI_TREE in order for 
it to work.

I have concerns this will create a breaking change for users of the 
kaslr fdt touch up.  In our case, if CONFIG_OFNODE_MULTI_TREE isn't set, 
the control FDT gets touched up, not the kernel FDT as required.  
Everything runs to completion, but "/proc/device-tree/chosen/kaslr-seed" 
isn't present after boot.

Am I missing something?  Perhaps there's a way to modify the default 
value for CONFIG_OFNODE_MULTI_TREE to ensure this works out-of-the-box?

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

[Simon Glass]

Hi Sean,

On Fri, 11 Aug 2023 at 11:14, Sean Edmond <seanedmond@linux.microsoft.com>
wrote:
>
>
> On 2023-08-09 6:49 p.m., Simon Glass wrote:
> > Hi Sean,
> >
> > On Wed, 9 Aug 2023 at 16:35, Sean Edmond <seanedmond@linux.microsoft.com>
wrote:
> >>
> >> On 2023-08-08 7:03 p.m., Simon Glass wrote:
> >>> Hi,
> >>>
> >>> On Fri, 4 Aug 2023 at 17:34, <seanedmond@linux.microsoft.com> wrote:
> >>>> From: Dhananjay Phadke <dphadke@linux.microsoft.com>
> >>>>
> >>>> fdt_fixup_kaslr_seed() will update given FDT 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>
> >>>> ---
> >>>>    arch/arm/cpu/armv8/sec_firmware.c | 32
+++++++------------------------
> >>>>    common/fdt_support.c              | 31
++++++++++++++++++++++++++++++
> >>>>    include/fdt_support.h             |  3 +++
> >>>>    3 files changed, 41 insertions(+), 25 deletions(-)
> >>> We need to find a way to use the ofnode API here.
> >>>
> >>>> diff --git a/arch/arm/cpu/armv8/sec_firmware.c
b/arch/arm/cpu/armv8/sec_firmware.c
> >>>> index c0e8726346..84ba49924e 100644
> >>>> --- a/arch/arm/cpu/armv8/sec_firmware.c
> >>>> +++ b/arch/arm/cpu/armv8/sec_firmware.c
> >>>> @@ -411,46 +411,28 @@ 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)
> >>> You could pass an oftree to this function, e.g. obtained with:
> >>>
> >>> oftree_from_fdt(fdt)
> >> The common API I added is fdt_fixup_kaslr_seed(), which was added to
> >> "common/fdt_support.c".
> >>
> >> There are 3 callers:
> >> sec_firmware_init()->fdt_fixup_kaslr_seed()
> >> do_kaslr_seed()->fdt_fixup_kaslr_seed()
> >> image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed()
> >>
> >> I think the ask is to create a common API that uses the ofnode API.
So,
> >> instead of fdt_fixup_kaslr_seed() I can create
> >> ofnode_fixup_kaslr_seed()?  Where should it live?
> > If you like you could add common/ofnode_support.c ?
> >
> > But it is OK to have it in the same file, I think.
> >
> >> Are you also wanting
> >> the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take oftree as
> >> input too?
> > So far as you can go, yes. Also you may want to pass an ofnode (the
> > root node) so that things can deal with adding their stuff to any
> > node.
> >
> > Regards,
> > Simon
>
>
> I re-worked the API to use the ofnode API and tested it on our board.  I
> was required to explicitly enable CONFIG_OFNODE_MULTI_TREE in order for
> it to work.
>
> I have concerns this will create a breaking change for users of the
> kaslr fdt touch up.  In our case, if CONFIG_OFNODE_MULTI_TREE isn't set,
> the control FDT gets touched up, not the kernel FDT as required.
> Everything runs to completion, but "/proc/device-tree/chosen/kaslr-seed"
> isn't present after boot.
>
> Am I missing something?  Perhaps there's a way to modify the default
> value for CONFIG_OFNODE_MULTI_TREE to ensure this works out-of-the-box?
>

Yes, perhaps we should enable this when fixups are used? Is there a way to
tell?

Also, we should make it return an error when attempting to use a tree
without that option enabled. I would expect oftree_ensure() to provide that?

Regards,
Simon

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

[Sean Edmond]

On 2023-08-12 6:09 a.m., Simon Glass wrote:
> Hi Sean,
>
> On Fri, 11 Aug 2023 at 11:14, Sean Edmond <seanedmond@linux.microsoft.com>
> wrote:
>>
>> On 2023-08-09 6:49 p.m., Simon Glass wrote:
>>> Hi Sean,
>>>
>>> On Wed, 9 Aug 2023 at 16:35, Sean Edmond <seanedmond@linux.microsoft.com>
> wrote:
>>>> On 2023-08-08 7:03 p.m., Simon Glass wrote:
>>>>> Hi,
>>>>>
>>>>> On Fri, 4 Aug 2023 at 17:34, <seanedmond@linux.microsoft.com> wrote:
>>>>>> From: Dhananjay Phadke <dphadke@linux.microsoft.com>
>>>>>>
>>>>>> fdt_fixup_kaslr_seed() will update given FDT 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>
>>>>>> ---
>>>>>>     arch/arm/cpu/armv8/sec_firmware.c | 32
> +++++++------------------------
>>>>>>     common/fdt_support.c              | 31
> ++++++++++++++++++++++++++++++
>>>>>>     include/fdt_support.h             |  3 +++
>>>>>>     3 files changed, 41 insertions(+), 25 deletions(-)
>>>>> We need to find a way to use the ofnode API here.
>>>>>
>>>>>> diff --git a/arch/arm/cpu/armv8/sec_firmware.c
> b/arch/arm/cpu/armv8/sec_firmware.c
>>>>>> index c0e8726346..84ba49924e 100644
>>>>>> --- a/arch/arm/cpu/armv8/sec_firmware.c
>>>>>> +++ b/arch/arm/cpu/armv8/sec_firmware.c
>>>>>> @@ -411,46 +411,28 @@ 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)
>>>>> You could pass an oftree to this function, e.g. obtained with:
>>>>>
>>>>> oftree_from_fdt(fdt)
>>>> The common API I added is fdt_fixup_kaslr_seed(), which was added to
>>>> "common/fdt_support.c".
>>>>
>>>> There are 3 callers:
>>>> sec_firmware_init()->fdt_fixup_kaslr_seed()
>>>> do_kaslr_seed()->fdt_fixup_kaslr_seed()
>>>> image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed()
>>>>
>>>> I think the ask is to create a common API that uses the ofnode API.
> So,
>>>> instead of fdt_fixup_kaslr_seed() I can create
>>>> ofnode_fixup_kaslr_seed()?  Where should it live?
>>> If you like you could add common/ofnode_support.c ?
>>>
>>> But it is OK to have it in the same file, I think.
>>>
>>>> Are you also wanting
>>>> the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take oftree as
>>>> input too?
>>> So far as you can go, yes. Also you may want to pass an ofnode (the
>>> root node) so that things can deal with adding their stuff to any
>>> node.
>>>
>>> Regards,
>>> Simon
>>
>> I re-worked the API to use the ofnode API and tested it on our board.  I
>> was required to explicitly enable CONFIG_OFNODE_MULTI_TREE in order for
>> it to work.
>>
>> I have concerns this will create a breaking change for users of the
>> kaslr fdt touch up.  In our case, if CONFIG_OFNODE_MULTI_TREE isn't set,
>> the control FDT gets touched up, not the kernel FDT as required.
>> Everything runs to completion, but "/proc/device-tree/chosen/kaslr-seed"
>> isn't present after boot.
>>
>> Am I missing something?  Perhaps there's a way to modify the default
>> value for CONFIG_OFNODE_MULTI_TREE to ensure this works out-of-the-box?
>>
> Yes, perhaps we should enable this when fixups are used? Is there a way to
> tell?
I don't think there's a way to tell unfortunately.  Fixups are always 
called if OF_LIBFDT is enabled, and if an FDT is found during bootm.

I'm having trouble understanding the intention of the current default 
for OFNODE_MULTI_TREE:
     default y if EVENT && !DM_DEV_READ_INLINE && !DM_INLINE_OFNODE
Could we simplify to this?
         default y if !OF_LIVE
>
> Also, we should make it return an error when attempting to use a tree
> without that option enabled. I would expect oftree_ensure() to provide that?
I'll add a check.
>
> Regards,
> Simon
>
>

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

[Simon Glass]

Hi Sean,

On Mon, 14 Aug 2023 at 13:12, Sean Edmond
<seanedmond@linux.microsoft.com> wrote:
>
>
> On 2023-08-12 6:09 a.m., Simon Glass wrote:
> > Hi Sean,
> >
> > On Fri, 11 Aug 2023 at 11:14, Sean Edmond <seanedmond@linux.microsoft.com>
> > wrote:
> >>
> >> On 2023-08-09 6:49 p.m., Simon Glass wrote:
> >>> Hi Sean,
> >>>
> >>> On Wed, 9 Aug 2023 at 16:35, Sean Edmond <seanedmond@linux.microsoft.com>
> > wrote:
> >>>> On 2023-08-08 7:03 p.m., Simon Glass wrote:
> >>>>> Hi,
> >>>>>
> >>>>> On Fri, 4 Aug 2023 at 17:34, <seanedmond@linux.microsoft.com> wrote:
> >>>>>> From: Dhananjay Phadke <dphadke@linux.microsoft.com>
> >>>>>>
> >>>>>> fdt_fixup_kaslr_seed() will update given FDT 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>
> >>>>>> ---
> >>>>>>     arch/arm/cpu/armv8/sec_firmware.c | 32
> > +++++++------------------------
> >>>>>>     common/fdt_support.c              | 31
> > ++++++++++++++++++++++++++++++
> >>>>>>     include/fdt_support.h             |  3 +++
> >>>>>>     3 files changed, 41 insertions(+), 25 deletions(-)
> >>>>> We need to find a way to use the ofnode API here.
> >>>>>
> >>>>>> diff --git a/arch/arm/cpu/armv8/sec_firmware.c
> > b/arch/arm/cpu/armv8/sec_firmware.c
> >>>>>> index c0e8726346..84ba49924e 100644
> >>>>>> --- a/arch/arm/cpu/armv8/sec_firmware.c
> >>>>>> +++ b/arch/arm/cpu/armv8/sec_firmware.c
> >>>>>> @@ -411,46 +411,28 @@ 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)
> >>>>> You could pass an oftree to this function, e.g. obtained with:
> >>>>>
> >>>>> oftree_from_fdt(fdt)
> >>>> The common API I added is fdt_fixup_kaslr_seed(), which was added to
> >>>> "common/fdt_support.c".
> >>>>
> >>>> There are 3 callers:
> >>>> sec_firmware_init()->fdt_fixup_kaslr_seed()
> >>>> do_kaslr_seed()->fdt_fixup_kaslr_seed()
> >>>> image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed()
> >>>>
> >>>> I think the ask is to create a common API that uses the ofnode API.
> > So,
> >>>> instead of fdt_fixup_kaslr_seed() I can create
> >>>> ofnode_fixup_kaslr_seed()?  Where should it live?
> >>> If you like you could add common/ofnode_support.c ?
> >>>
> >>> But it is OK to have it in the same file, I think.
> >>>
> >>>> Are you also wanting
> >>>> the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take oftree as
> >>>> input too?
> >>> So far as you can go, yes. Also you may want to pass an ofnode (the
> >>> root node) so that things can deal with adding their stuff to any
> >>> node.
> >>>
> >>> Regards,
> >>> Simon
> >>
> >> I re-worked the API to use the ofnode API and tested it on our board.  I
> >> was required to explicitly enable CONFIG_OFNODE_MULTI_TREE in order for
> >> it to work.
> >>
> >> I have concerns this will create a breaking change for users of the
> >> kaslr fdt touch up.  In our case, if CONFIG_OFNODE_MULTI_TREE isn't set,
> >> the control FDT gets touched up, not the kernel FDT as required.
> >> Everything runs to completion, but "/proc/device-tree/chosen/kaslr-seed"
> >> isn't present after boot.
> >>
> >> Am I missing something?  Perhaps there's a way to modify the default
> >> value for CONFIG_OFNODE_MULTI_TREE to ensure this works out-of-the-box?
> >>
> > Yes, perhaps we should enable this when fixups are used? Is there a way to
> > tell?
> I don't think there's a way to tell unfortunately.  Fixups are always
> called if OF_LIBFDT is enabled, and if an FDT is found during bootm.
>
> I'm having trouble understanding the intention of the current default
> for OFNODE_MULTI_TREE:
>      default y if EVENT && !DM_DEV_READ_INLINE && !DM_INLINE_OFNODE
> Could we simplify to this?
>          default y if !OF_LIVE

I don't think it will build if inlining is used, but I can't remember...

The EVENT thing is because there is an of-fixup event, which was the
original thing using ofnode fixups.

I wonder what sort of size increment this will create with !OF_LlIVE ?

> >
> > Also, we should make it return an error when attempting to use a tree
> > without that option enabled. I would expect oftree_ensure() to provide that?
> I'll add a check.

OK thanks.

Regards,
Simon

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

[Sean Edmond]

On 2023-08-15 7:44 a.m., Simon Glass wrote:
> Hi Sean,
>
> On Mon, 14 Aug 2023 at 13:12, Sean Edmond
> <seanedmond@linux.microsoft.com> wrote:
>>
>> On 2023-08-12 6:09 a.m., Simon Glass wrote:
>>> Hi Sean,
>>>
>>> On Fri, 11 Aug 2023 at 11:14, Sean Edmond <seanedmond@linux.microsoft.com>
>>> wrote:
>>>> On 2023-08-09 6:49 p.m., Simon Glass wrote:
>>>>> Hi Sean,
>>>>>
>>>>> On Wed, 9 Aug 2023 at 16:35, Sean Edmond <seanedmond@linux.microsoft.com>
>>> wrote:
>>>>>> On 2023-08-08 7:03 p.m., Simon Glass wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Fri, 4 Aug 2023 at 17:34, <seanedmond@linux.microsoft.com> wrote:
>>>>>>>> From: Dhananjay Phadke <dphadke@linux.microsoft.com>
>>>>>>>>
>>>>>>>> fdt_fixup_kaslr_seed() will update given FDT 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>
>>>>>>>> ---
>>>>>>>>      arch/arm/cpu/armv8/sec_firmware.c | 32
>>> +++++++------------------------
>>>>>>>>      common/fdt_support.c              | 31
>>> ++++++++++++++++++++++++++++++
>>>>>>>>      include/fdt_support.h             |  3 +++
>>>>>>>>      3 files changed, 41 insertions(+), 25 deletions(-)
>>>>>>> We need to find a way to use the ofnode API here.
>>>>>>>
>>>>>>>> diff --git a/arch/arm/cpu/armv8/sec_firmware.c
>>> b/arch/arm/cpu/armv8/sec_firmware.c
>>>>>>>> index c0e8726346..84ba49924e 100644
>>>>>>>> --- a/arch/arm/cpu/armv8/sec_firmware.c
>>>>>>>> +++ b/arch/arm/cpu/armv8/sec_firmware.c
>>>>>>>> @@ -411,46 +411,28 @@ 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)
>>>>>>> You could pass an oftree to this function, e.g. obtained with:
>>>>>>>
>>>>>>> oftree_from_fdt(fdt)
>>>>>> The common API I added is fdt_fixup_kaslr_seed(), which was added to
>>>>>> "common/fdt_support.c".
>>>>>>
>>>>>> There are 3 callers:
>>>>>> sec_firmware_init()->fdt_fixup_kaslr_seed()
>>>>>> do_kaslr_seed()->fdt_fixup_kaslr_seed()
>>>>>> image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed()
>>>>>>
>>>>>> I think the ask is to create a common API that uses the ofnode API.
>>> So,
>>>>>> instead of fdt_fixup_kaslr_seed() I can create
>>>>>> ofnode_fixup_kaslr_seed()?  Where should it live?
>>>>> If you like you could add common/ofnode_support.c ?
>>>>>
>>>>> But it is OK to have it in the same file, I think.
>>>>>
>>>>>> Are you also wanting
>>>>>> the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take oftree as
>>>>>> input too?
>>>>> So far as you can go, yes. Also you may want to pass an ofnode (the
>>>>> root node) so that things can deal with adding their stuff to any
>>>>> node.
>>>>>
>>>>> Regards,
>>>>> Simon
>>>> I re-worked the API to use the ofnode API and tested it on our board.  I
>>>> was required to explicitly enable CONFIG_OFNODE_MULTI_TREE in order for
>>>> it to work.
>>>>
>>>> I have concerns this will create a breaking change for users of the
>>>> kaslr fdt touch up.  In our case, if CONFIG_OFNODE_MULTI_TREE isn't set,
>>>> the control FDT gets touched up, not the kernel FDT as required.
>>>> Everything runs to completion, but "/proc/device-tree/chosen/kaslr-seed"
>>>> isn't present after boot.
>>>>
>>>> Am I missing something?  Perhaps there's a way to modify the default
>>>> value for CONFIG_OFNODE_MULTI_TREE to ensure this works out-of-the-box?
>>>>
>>> Yes, perhaps we should enable this when fixups are used? Is there a way to
>>> tell?
>> I don't think there's a way to tell unfortunately.  Fixups are always
>> called if OF_LIBFDT is enabled, and if an FDT is found during bootm.
>>
>> I'm having trouble understanding the intention of the current default
>> for OFNODE_MULTI_TREE:
>>       default y if EVENT && !DM_DEV_READ_INLINE && !DM_INLINE_OFNODE
>> Could we simplify to this?
>>           default y if !OF_LIVE
> I don't think it will build if inlining is used, but I can't remember...
I wasn't able to break this by turning off DM_DEV_READ_INLINE, and 
enabling OFNODE_MULTI_TREE/SPL_DM_INLINE_OFNODE/TPL_DM_INLINE_OFNODE. 
Perhaps things have changes since this was created.
> The EVENT thing is because there is an of-fixup event, which was the
> original thing using ofnode fixups.
>
> I wonder what sort of size increment this will create with !OF_LIVE ?

With default (OFNODE_MULTI_TREE is not set):

textdatabssdechex filename

9380135536846752 1040133fdf05 u-boot/u-boot/u-boot


With !OF_LIVE (OFNODE_MULTI_TREE is set with OFNODE_MULTI_TREE_MAX=4):

seanedmond@ovlvm106:~/code/QEMU/qemu_uboot$ sizeu-boot/u-boot/u-boot

textdatabssdechex filename

9390165536846752 1041136fe2f0 u-boot/u-boot/u-boot

>
>>> Also, we should make it return an error when attempting to use a tree
>>> without that option enabled. I would expect oftree_ensure() to provide that?
>> I'll add a check.
> OK thanks.
>
> Regards,
> Simon
>
>

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

[Sean Edmond]

On 2023-08-15 10:46 a.m., Sean Edmond wrote:
>
> On 2023-08-15 7:44 a.m., Simon Glass wrote:
>> Hi Sean,
>>
>> On Mon, 14 Aug 2023 at 13:12, Sean Edmond
>> <seanedmond@linux.microsoft.com> wrote:
>>>
>>> On 2023-08-12 6:09 a.m., Simon Glass wrote:
>>>> Hi Sean,
>>>>
>>>> On Fri, 11 Aug 2023 at 11:14, Sean Edmond 
>>>> <seanedmond@linux.microsoft.com>
>>>> wrote:
>>>>> On 2023-08-09 6:49 p.m., Simon Glass wrote:
>>>>>> Hi Sean,
>>>>>>
>>>>>> On Wed, 9 Aug 2023 at 16:35, Sean Edmond 
>>>>>> <seanedmond@linux.microsoft.com>
>>>> wrote:
>>>>>>> On 2023-08-08 7:03 p.m., Simon Glass wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On Fri, 4 Aug 2023 at 17:34, <seanedmond@linux.microsoft.com> 
>>>>>>>> wrote:
>>>>>>>>> From: Dhananjay Phadke <dphadke@linux.microsoft.com>
>>>>>>>>>
>>>>>>>>> fdt_fixup_kaslr_seed() will update given FDT 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>
>>>>>>>>> ---
>>>>>>>>>      arch/arm/cpu/armv8/sec_firmware.c | 32
>>>> +++++++------------------------
>>>>>>>>> common/fdt_support.c              | 31
>>>> ++++++++++++++++++++++++++++++
>>>>>>>>> include/fdt_support.h             |  3 +++
>>>>>>>>>      3 files changed, 41 insertions(+), 25 deletions(-)
>>>>>>>> We need to find a way to use the ofnode API here.
>>>>>>>>
>>>>>>>>> diff --git a/arch/arm/cpu/armv8/sec_firmware.c
>>>> b/arch/arm/cpu/armv8/sec_firmware.c
>>>>>>>>> index c0e8726346..84ba49924e 100644
>>>>>>>>> --- a/arch/arm/cpu/armv8/sec_firmware.c
>>>>>>>>> +++ b/arch/arm/cpu/armv8/sec_firmware.c
>>>>>>>>> @@ -411,46 +411,28 @@ 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)
>>>>>>>> You could pass an oftree to this function, e.g. obtained with:
>>>>>>>>
>>>>>>>> oftree_from_fdt(fdt)
>>>>>>> The common API I added is fdt_fixup_kaslr_seed(), which was 
>>>>>>> added to
>>>>>>> "common/fdt_support.c".
>>>>>>>
>>>>>>> There are 3 callers:
>>>>>>> sec_firmware_init()->fdt_fixup_kaslr_seed()
>>>>>>> do_kaslr_seed()->fdt_fixup_kaslr_seed()
>>>>>>> image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed()
>>>>>>>
>>>>>>> I think the ask is to create a common API that uses the ofnode API.
>>>> So,
>>>>>>> instead of fdt_fixup_kaslr_seed() I can create
>>>>>>> ofnode_fixup_kaslr_seed()?  Where should it live?
>>>>>> If you like you could add common/ofnode_support.c ?
>>>>>>
>>>>>> But it is OK to have it in the same file, I think.
>>>>>>
>>>>>>> Are you also wanting
>>>>>>> the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take 
>>>>>>> oftree as
>>>>>>> input too?
>>>>>> So far as you can go, yes. Also you may want to pass an ofnode (the
>>>>>> root node) so that things can deal with adding their stuff to any
>>>>>> node.
>>>>>>
>>>>>> Regards,
>>>>>> Simon
>>>>> I re-worked the API to use the ofnode API and tested it on our 
>>>>> board.  I
>>>>> was required to explicitly enable CONFIG_OFNODE_MULTI_TREE in 
>>>>> order for
>>>>> it to work.
>>>>>
>>>>> I have concerns this will create a breaking change for users of the
>>>>> kaslr fdt touch up.  In our case, if CONFIG_OFNODE_MULTI_TREE 
>>>>> isn't set,
>>>>> the control FDT gets touched up, not the kernel FDT as required.
>>>>> Everything runs to completion, but 
>>>>> "/proc/device-tree/chosen/kaslr-seed"
>>>>> isn't present after boot.
>>>>>
>>>>> Am I missing something?  Perhaps there's a way to modify the default
>>>>> value for CONFIG_OFNODE_MULTI_TREE to ensure this works 
>>>>> out-of-the-box?
>>>>>
>>>> Yes, perhaps we should enable this when fixups are used? Is there a 
>>>> way to
>>>> tell?
>>> I don't think there's a way to tell unfortunately.  Fixups are always
>>> called if OF_LIBFDT is enabled, and if an FDT is found during bootm.
>>>
>>> I'm having trouble understanding the intention of the current default
>>> for OFNODE_MULTI_TREE:
>>>       default y if EVENT && !DM_DEV_READ_INLINE && !DM_INLINE_OFNODE
>>> Could we simplify to this?
>>>           default y if !OF_LIVE
>> I don't think it will build if inlining is used, but I can't remember...
> I wasn't able to break this by turning off DM_DEV_READ_INLINE, and 
> enabling OFNODE_MULTI_TREE/SPL_DM_INLINE_OFNODE/TPL_DM_INLINE_OFNODE. 
> Perhaps things have changes since this was created.
>> The EVENT thing is because there is an of-fixup event, which was the
>> original thing using ofnode fixups.
>>
>> I wonder what sort of size increment this will create with !OF_LIVE ?
>
> With default (OFNODE_MULTI_TREE is not set):
>
> textdatabssdechex filename
>
> 9380135536846752 1040133fdf05 u-boot/u-boot/u-boot
>
>
> With !OF_LIVE (OFNODE_MULTI_TREE is set with OFNODE_MULTI_TREE_MAX=4):
>
> seanedmond@ovlvm106:~/code/QEMU/qemu_uboot$ sizeu-boot/u-boot/u-boot
>
> textdatabssdechex filename
>
> 9390165536846752 1041136fe2f0 u-boot/u-boot/u-boot

Sorry about the formatting... let's try that again.

With default (OFNODE_MULTI_TREE is not set):

text: 938013
data: 55368
bss:46752
dec: 1040133
hex: fdf05

With !OF_LIVE (OFNODE_MULTI_TREE is set with OFNODE_MULTI_TREE_MAX=4):

text: 939016
data: 55368
bss: 46752
dec: 1041136
hex: fe2f0

>
>>
>>>> Also, we should make it return an error when attempting to use a tree
>>>> without that option enabled. I would expect oftree_ensure() to 
>>>> provide that?
>>> I'll add a check.
>> OK thanks.
>>
>> Regards,
>> Simon
>>
>>
>
>

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

[Simon Glass]

Hi Sean,

On Thu, 17 Aug 2023 at 10:03, Sean Edmond <seanedmond@linux.microsoft.com>
wrote:
>
>
> On 2023-08-15 10:46 a.m., Sean Edmond wrote:
>
>
> On 2023-08-15 7:44 a.m., Simon Glass wrote:
>
> Hi Sean,
>
> On Mon, 14 Aug 2023 at 13:12, Sean Edmond
> <seanedmond@linux.microsoft.com> wrote:
>
>
> On 2023-08-12 6:09 a.m., Simon Glass wrote:
>
> Hi Sean,
>
> On Fri, 11 Aug 2023 at 11:14, Sean Edmond <seanedmond@linux.microsoft.com>
> wrote:
>
> On 2023-08-09 6:49 p.m., Simon Glass wrote:
>
> Hi Sean,
>
> On Wed, 9 Aug 2023 at 16:35, Sean Edmond <seanedmond@linux.microsoft.com>
>
> wrote:
>
> On 2023-08-08 7:03 p.m., Simon Glass wrote:
>
> Hi,
>
> On Fri, 4 Aug 2023 at 17:34, <seanedmond@linux.microsoft.com> wrote:
>
> From: Dhananjay Phadke <dphadke@linux.microsoft.com>
>
> fdt_fixup_kaslr_seed() will update given FDT 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>
> ---
>      arch/arm/cpu/armv8/sec_firmware.c | 32
>
> +++++++------------------------
>
>      common/fdt_support.c              | 31
>
> ++++++++++++++++++++++++++++++
>
>      include/fdt_support.h             |  3 +++
>      3 files changed, 41 insertions(+), 25 deletions(-)
>
> We need to find a way to use the ofnode API here.
>
> diff --git a/arch/arm/cpu/armv8/sec_firmware.c
>
> b/arch/arm/cpu/armv8/sec_firmware.c
>
> index c0e8726346..84ba49924e 100644
> --- a/arch/arm/cpu/armv8/sec_firmware.c
> +++ b/arch/arm/cpu/armv8/sec_firmware.c
> @@ -411,46 +411,28 @@ 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)
>
> You could pass an oftree to this function, e.g. obtained with:
>
> oftree_from_fdt(fdt)
>
> The common API I added is fdt_fixup_kaslr_seed(), which was added to
> "common/fdt_support.c".
>
> There are 3 callers:
> sec_firmware_init()->fdt_fixup_kaslr_seed()
> do_kaslr_seed()->fdt_fixup_kaslr_seed()
> image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed()
>
> I think the ask is to create a common API that uses the ofnode API.
>
> So,
>
> instead of fdt_fixup_kaslr_seed() I can create
> ofnode_fixup_kaslr_seed()?  Where should it live?
>
> If you like you could add common/ofnode_support.c ?
>
> But it is OK to have it in the same file, I think.
>
> Are you also wanting
> the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take oftree as
> input too?
>
> So far as you can go, yes. Also you may want to pass an ofnode (the
> root node) so that things can deal with adding their stuff to any
> node.
>
> Regards,
> Simon
>
> I re-worked the API to use the ofnode API and tested it on our board.  I
> was required to explicitly enable CONFIG_OFNODE_MULTI_TREE in order for
> it to work.
>
> I have concerns this will create a breaking change for users of the
> kaslr fdt touch up.  In our case, if CONFIG_OFNODE_MULTI_TREE isn't set,
> the control FDT gets touched up, not the kernel FDT as required.
> Everything runs to completion, but "/proc/device-tree/chosen/kaslr-seed"
> isn't present after boot.
>
> Am I missing something?  Perhaps there's a way to modify the default
> value for CONFIG_OFNODE_MULTI_TREE to ensure this works out-of-the-box?
>
> Yes, perhaps we should enable this when fixups are used? Is there a way to
> tell?
>
> I don't think there's a way to tell unfortunately.  Fixups are always
> called if OF_LIBFDT is enabled, and if an FDT is found during bootm.
>
> I'm having trouble understanding the intention of the current default
> for OFNODE_MULTI_TREE:
>       default y if EVENT && !DM_DEV_READ_INLINE && !DM_INLINE_OFNODE
> Could we simplify to this?
>           default y if !OF_LIVE
>
> I don't think it will build if inlining is used, but I can't remember...
>
> I wasn't able to break this by turning off DM_DEV_READ_INLINE, and
enabling OFNODE_MULTI_TREE/SPL_DM_INLINE_OFNODE/TPL_DM_INLINE_OFNODE.
Perhaps things have changes since this was created.
>
> The EVENT thing is because there is an of-fixup event, which was the
> original thing using ofnode fixups.
>
> I wonder what sort of size increment this will create with !OF_LIVE ?
>
>
> With default (OFNODE_MULTI_TREE is not set):
>
> textdatabssdechex filename
>
> 9380135536846752 1040133fdf05 u-boot/u-boot/u-boot
>
>
> With !OF_LIVE (OFNODE_MULTI_TREE is set with OFNODE_MULTI_TREE_MAX=4):
>
> seanedmond@ovlvm106:~/code/QEMU/qemu_uboot$ sizeu-boot/u-boot/u-boot
>
> textdatabssdechex filename
>
> 9390165536846752 1041136fe2f0 u-boot/u-boot/u-boot
>
>
> Sorry about the formatting... let's try that again.
>
> With default (OFNODE_MULTI_TREE is not set):
>
>    text: 938013
>    data:  55368
>    bss: 46752
>    dec:  1040133
>    hex: fdf05
>
> With !OF_LIVE (OFNODE_MULTI_TREE is set with OFNODE_MULTI_TREE_MAX=4):
>
>    text: 939016
>    data: 55368
>    bss: 46752
>    dec: 1041136
>    hex: fe2f0
>
>
>
> Also, we should make it return an error when attempting to use a tree
> without that option enabled. I would expect oftree_ensure() to provide
that?
>
> I'll add a check.
>
> OK thanks.

Something odd about this email, but anyway, please consider whether (with
your changes) there is any point in still having the inline options. If
not, we should remove them in the same series, but need to explain the
code-size cost.

Regards,
Simon

Re: [PATCH 2/3] fdt: kaslr seed from tpm entropy

[Ilias Apalodimas]

Hi Sean,

On Fri, Aug 04, 2023 at 04:33:56PM -0700, seanedmond@linux.microsoft.com wrote:
> From: Dhananjay Phadke <dphadke@linux.microsoft.com>
>
> Add support for KASLR seed from TPM device. Invokes tpm_get_random()
> API to read 8-bytes of random bytes for KASLR.

Can you elaborate a bit more why you specifically need an RNG from the TPM?

>
> 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/image-fdt.c      |  3 +++
>  common/fdt_support.c  | 39 ++++++++++++++++++++++++++++++++++++++-
>  include/fdt_support.h |  1 +
>  lib/Kconfig           |  9 +++++++++
>  4 files changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> index f10200f647..127443963e 100644
> --- a/boot/image-fdt.c
> +++ b/boot/image-fdt.c
> @@ -624,6 +624,9 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob,
>  		goto err;
>  	}
>
> +	if (IS_ENABLED(CONFIG_KASLR_TPM_SEED))
> +		fdt_tpm_kaslr_seed(blob);

So, why can't we just add entropy from any available RNG? In Arm world we
could have TF-A, OP-TEE, an RNG hardware or a TPM capable of doing that (or
all of them).

Can't we just do
platform_get_rng_device(&dev);
dm_rng_read(....);

And even if we specifically need an RNG from a TPM, I think it's better to
find a way and teach platform_get_rng_device() to return a list of devices
in priority instead of hardcoding that.

[...]

Thanks
/Ilias