[PATCH 0/3] Populate kaslr seed with TPM
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(-)
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);
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
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
*/ int fdt_fixup_kaslr(void *fdt)
- @eret: 0 for success
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
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
*/ int fdt_fixup_kaslr(void *fdt)
- @eret: 0 for success
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
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
*/ int fdt_fixup_kaslr(void *fdt)
- @eret: 0 for success
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
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
int fdt_fixup_kaslr(void *fdt)
- @eret: 0 for success */
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?
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
int fdt_fixup_kaslr(void *fdt)
- @eret: 0 for success */
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
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
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
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
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
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
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"
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_V2helpThis enables support for using TPMs as entropy source for KASLR seedpopulated in kernel's device tree. Both TPMv1 and TPMv2 are supportedfor the low-level TPM interface, but only one TPM is supported ata time by the library.endmenu
menu "Android Verified Boot"
2.40.0
Regards, Simon
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
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; }
participants (5)
-
Chris Morgan -
Ilias Apalodimas -
Sean Edmond -
seanedmond＠linux.microsoft.com
-
Simon Glass