[RFC PATCH 4/5] bootefi: Call the EVT_FT_FIXUP event handler

[Heinrich Schuchardt]

On 28.08.23 11:32, Sughosh Ganu wrote:
> On Sat, 26 Aug 2023 at 15:57, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> On 8/26/23 11:06, Sughosh Ganu wrote:
>>> The bootefi command passes the devicetree to the kernel through the
>>> EFI config table. Call the event handlers for fixing the devicetree
>>> before jumping into the kernel. This removes any devicetree nodes
>>> and/or properties that are specific only to U-Boot, and are not to be
>>> passed to the OS.
>>>
>>> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
>>> ---
>>>    cmd/bootefi.c | 18 ++++++++++++++++++
>>>    1 file changed, 18 insertions(+)
>>>
>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>> index f73d6eb0e2..c359a46ec4 100644
>>> --- a/cmd/bootefi.c
>>> +++ b/cmd/bootefi.c
>>> @@ -237,6 +237,23 @@ static void *get_config_table(const efi_guid_t *guid)
>>>        return NULL;
>>>    }
>>>
>>> +/**
>>> + * event_notify_dt_fixup() - call ft_fixup event
>>> + *
>>> + * @fdt:     address of the device tree to be passed to the kernel
>>> + *           through the configuration table
>>> + * Return:   None
>>> + */
>>> +static void event_notify_dt_fixup(void *fdt)
>>> +{
>>> +     int ret;
>>> +     struct event_ft_fixup fixup = {0};
>>> +
>>> +     fixup.tree.fdt = fdt;
>>> +     ret = event_notify(EVT_FT_FIXUP, &fixup, sizeof(fixup));
>>> +     if (ret)
>>> +             printf("Error: %d: FDT Fixup event failed\n", ret);
>>> +}
>>>    #endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
>>>
>>>    /**
>>> @@ -318,6 +335,7 @@ efi_status_t efi_install_fdt(void *fdt)
>>>        efi_carve_out_dt_rsv(fdt);
>>>
>>>        efi_try_purge_kaslr_seed(fdt);
>>> +     event_notify_dt_fixup(fdt);
>>
>> The event is already triggered in image_setup_libfdt(). Don't trigger it
>> twice.
>
> The reason I put an explicit event_notify call is because the
> image_setup_libfdt() call only calls the ft_fixup handlers if the
> livetree is not active. So the fixup handlers would not be called on
> platforms that enable livetree. Although I'm not sure if livetree
> should be disabled before the ft fixup has to happen, or platforms
> that need ft fixup should not enable OF_LIVE. Disabling the livetree
> is not happening now, so I am not sure how the fixup event should work
> on platforms which have OF_LIVE enabled.

We still must not call the same event twice. We further must ensure that
our fix-up is called last (e.g. by calling it zzz_something) and not
before adding VBE nodes.

Is there any need to use the event mechanism here?

@Simon:
None of the other function calls in image_setup_libfdt() depend on
of_live_active(). I think it would be preferable to move that check into
the listeners. If instead of of_live_active() you could use a
configuration setting for the decision, gcc could eliminate a lot of code.

We definitely need better descriptions in include/event.h and a man-page
generated from that include:

EVT_NONE
     undescribed
EVT_TEST
     undescribed
EVT_DM_POST_INIT_F
     undescribed
EVT_DM_POST_INIT_R
     undescribed
EVT_DM_PRE_PROBE
     Device is about to be probed
EVT_DM_POST_PROBE
     undescribed
EVT_DM_PRE_REMOVE
     undescribed
EVT_DM_POST_REMOVE
     undescribed
EVT_MISC_INIT_F
     undescribed
EVT_FPGA_LOAD
     undescribed
EVT_FT_FIXUP
     undescribed
EVT_MAIN_LOOP
     undescribed
EVT_COUNT
     undescribed

Best regards

Heinrich