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, <seanedm...@linux.microsoft.com> wrote:
From: Dhananjay Phadke <dpha...@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 <dpha...@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