[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 at 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 at 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 at linux.microsoft.com> wrote:
> >>>> From: Dhananjay Phadke <dphadke at 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 at 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