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

[Sean Edmond <seanedmond@linux.microsoft.com>]

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?