RE: [PATCH 2/2] drivers: firmware: efi: install new fdt in configuration table

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Bhupesh/Ard,

Thank you for reviewing my changes.

> -----Original Message-----
> From: Bhupesh Sharma [mailto:bhsharma@xxxxxxxxxx]
> Sent: Monday, March 26, 2018 10:51 AM
> To: Pankaj Bansal <pankaj.bansal@xxxxxxx>
> Cc: Varun Sethi <V.Sethi@xxxxxxx>; Wasim Khan <wasim.khan@xxxxxxx>;
> Udit Kumar <udit.kumar@xxxxxxx>; Ard Biesheuvel
> <ard.biesheuvel@xxxxxxxxxx>; linux-efi@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 2/2] drivers: firmware: efi: install new fdt in configuration
> table
> 
> Hi Pankaj,
> 
> On 03/25/2018 07:26 PM, Pankaj Bansal wrote:
> > Bootloader may need to fixup the device tree before OS can use it.
> > e.g. a UEFI/DXE driver that has initialized a controller can add
> > controller's clock frequency in controller node. This way OS need not
> > to call get/set clock for that controller.
> Its usually better to include a cover letter to provide a description of what the
> patchset is adding/fixing.

Sure. Will follow this practice now onwards.

> What I understand from looking at this patch is that you are providing the device
> tree to the kernel (via dtb= ? command) and trying to fix it via the bootloader - is
> this understanding correct?

That is correct.

> 
> If yes, I am not sure why the bootloader would not pass the dtb in the first place
> to the kernel via the standard interface defined in
> <https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww
> .kernel.org%2Fdoc%2FDocumentation%2Farm64%2Fbooting.txt&data=02%7C0
> 1%7Cpankaj.bansal%40nxp.com%7Ce917e5770a544261e9ee08d592d971a0%7C
> 686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636576384962510170&sd
> ata=GlBJchtRXnrX2IyoaKqe25k1ZDMmy0bH8ZileRFFBqo%3D&reserved=0>, i.e.:
> 
> x0 = physical address of device tree blob (dtb) in system RAM.
> 
> Normally bootloaders like UEFI/u-boot pass a fixed up dtb to the kernel, in such
> a case, the dtb would be fixed up by the bootloader itself and the kernel needs
> not worry about the same.

Yes this is the default case. But we have seen such cases when the linux kernel fails to boot with the UEFI provided dtb.
e.g. say UEFI dtb binary is generated with linux v4.4. When we try to boot linux v4.9 or v4.14 without updating dtb, then kernel
boot fails. Maybe this is because device trees are not backward compatible as Ard pointed out.

I see the boot behavior in other boot loader u-boot. In that bootloader we can provide the kernel, initrd, dtb triad.

Usage:
bootm [addr [arg ...]]
    - boot application image stored in memory
   passing arguments 'arg ...'; when booting a Linux kernel,
   'arg' can be the address of an initrd image
   When booting a Linux kernel which requires a flat device-tree
   a third argument is required which is the address of the
   device-tree blob. To boot that kernel without an initrd image,
   use a '-' for the second argument. If you do not pass a third
   a bd_info struct will be passed instead

The u-boot fixes up the dtb provided in bootm command before booting the kernel image.
This same behavior we want to achieve with UEFI boot. i.e. UEFI will provide a dtb image, but it can be overridden using dtb = <path/to/dtb>.

> 
> Regards,
> Bhupesh
> 
> > 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 <ard.biesheuvel@xxxxxxxxxx>
> > Cc: linux-efi@xxxxxxxxxxxxxxx
> > Signed-off-by: Pankaj Bansal <pankaj.bansal@xxxxxxx>
> > ---
> >   drivers/firmware/efi/libstub/fdt.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/firmware/efi/libstub/fdt.c
> > b/drivers/firmware/efi/libstub/fdt.c
> > index 177654e..df862e6 100644
> > --- a/drivers/firmware/efi/libstub/fdt.c
> > +++ b/drivers/firmware/efi/libstub/fdt.c
> > @@ -265,6 +265,7 @@ 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;
> >
> >       map.map =    &runtime_map;
> >       map.map_size =    &map_size;
> > @@ -314,6 +315,13 @@ efi_status_t
> allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table_arg,
> >           goto fail_free_new_fdt;
> >       }
> >
> > +    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;
> >

��.n��������+%������w��{.n�����{����*jg��������ݢj����G�������j:+v���w�m������w�������h�����٥




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux