[PATCH 1/3] fdt: common API to populate kaslr seed
Simon Glass
sjg at chromium.org
Tue Aug 15 16:44:21 CEST 2023
Hi Sean,
On Mon, 14 Aug 2023 at 13:12, Sean Edmond
<seanedmond at 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 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?
> 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
More information about the U-Boot
mailing list