> -----Original Message-----
> From: Ard Biesheuvel [mailto:[email protected]]
> Sent: Tuesday, December 11, 2018 2:48 PM
> To: Pankaj Bansal <[email protected]>
> Cc: Mark Rutland <[email protected]>; Leif Lindholm
> <[email protected]>; Grant Likely <[email protected]>; Varun Sethi
> <[email protected]>; Udit Kumar <[email protected]>; Bhupesh Sharma
> <[email protected]>; linux-efi <[email protected]>
> Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in
> configuration table
>
> On Tue, 11 Dec 2018 at 10:04, Pankaj Bansal <[email protected]> wrote:
> >
> > Bootloader may need to fixup the device tree before OS can use it.
> >
> > Therefore, install fdt used by OS in configuration tables and
> > associate it with device tree guid.
> >
> > UEFI/DXE drivers can fixup this device tree in their respective
> > ExitBootServices events.
> >
> > Cc: Ard Biesheuvel <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Pankaj Bansal <[email protected]>
>
> I still think this is a bad idea. The firmware is closely tied to the
> platform, so it
> should provide the DT instead of the kernel.
It is. It's just that firmware is responsible to fix the status of devices
before kernel
can use those. In efi stub, the fdt is copied into new_fdt before exit boot
services.
We need to fix the status of devices as part of exit boot services. We cannot
do it
before, because firmware is using these device and they are not ready for
kernel to use yet.
>
> > ---
> >
> > Notes:
> > V2:
> > - Add a check while installing fdt in configuration table, that
> > new_fdt would only be installed in configuration table if it
> > has been generated using fdt already present in configuration table.
> >
> > drivers/firmware/efi/libstub/fdt.c | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/drivers/firmware/efi/libstub/fdt.c
> > b/drivers/firmware/efi/libstub/fdt.c
> > index 45cded5b5d5c..dc228c05d0cd 100644
> > --- a/drivers/firmware/efi/libstub/fdt.c
> > +++ b/drivers/firmware/efi/libstub/fdt.c
> > @@ -269,6 +269,9 @@ efi_status_t
> allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table_arg,
> > int runtime_entry_count = 0;
> > struct efi_boot_memmap map;
> > struct exit_boot_struct priv;
> > + efi_guid_t fdt_guid = DEVICE_TREE_GUID;
> > + unsigned long fdt_config_table_addr = 0;
> > + unsigned long fdt_config_table_size = 0;
> >
> > map.map = &runtime_map;
> > map.map_size = &map_size;
> > @@ -318,6 +321,23 @@ efi_status_t
> allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table_arg,
> > goto fail_free_new_fdt;
> > }
> >
> > + /* Determine if the fdt_addr is obtained from uefi configuration
> > + * table or not? if yes, then install the new_fdt_addr in
> > configuration
> > + * table in its place, as the UEFI firmware may need to modify it
> > + * as part of exit_boot_services routine
> > + */
> > + fdt_config_table_addr = (uintptr_t)get_fdt(sys_table_arg,
> > + &fdt_config_table_size);
> > + if ((fdt_config_table_size == fdt_size) &&
> > + (fdt_config_table_addr == fdt_addr)) {
> > + status = efi_call_early(install_configuration_table,
> > &fdt_guid,
> > + (void *)*new_fdt_addr);
> > + if (status != EFI_SUCCESS) {
> > + pr_efi_err(sys_table_arg, "Unable to install new
> > device tree.\n");
> > + goto fail_free_new_fdt;
> > + }
> > + }
> > +
> > priv.runtime_map = runtime_map;
> > priv.runtime_entry_count = &runtime_entry_count;
> > priv.new_fdt_addr = (void *)*new_fdt_addr;
> > --
> > 2.17.1
> >
