Patchwork [v2,2/2] drivers: firmware: efi: install new fdt in configuration table

login
register
mail settings
Submitter Pankaj Bansal
Date Dec. 11, 2018, 9:04 a.m.
Message ID <20181211142929.25507-3-pankaj.bansal@nxp.com>
Download mbox | patch
Permalink /patch/677725/
State New
Headers show

Comments

Pankaj Bansal - Dec. 11, 2018, 9:04 a.m.
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 <ard.biesheuvel@linaro.org>
Cc: linux-efi@vger.kernel.org
Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

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(+)
Ard Biesheuvel - Dec. 11, 2018, 9:18 a.m.
On Tue, 11 Dec 2018 at 10:04, Pankaj Bansal <pankaj.bansal@nxp.com> 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 <ard.biesheuvel@linaro.org>
> Cc: linux-efi@vger.kernel.org
> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>

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.

> ---
>
> 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
>
Pankaj Bansal - Dec. 11, 2018, 9:23 a.m.
> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Tuesday, December 11, 2018 2:48 PM

> To: Pankaj Bansal <pankaj.bansal@nxp.com>

> Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm

> <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>; Varun Sethi

> <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>; Bhupesh Sharma

> <bhsharma@redhat.com>; linux-efi <linux-efi@vger.kernel.org>

> 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 <pankaj.bansal@nxp.com> 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 <ard.biesheuvel@linaro.org>

> > Cc: linux-efi@vger.kernel.org

> > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>

> 

> 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

> >
Ard Biesheuvel - Dec. 11, 2018, 9:24 a.m.
On Tue, 11 Dec 2018 at 10:23, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Tuesday, December 11, 2018 2:48 PM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm
> > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>; Varun Sethi
> > <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>; Bhupesh Sharma
> > <bhsharma@redhat.com>; linux-efi <linux-efi@vger.kernel.org>
> > 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 <pankaj.bansal@nxp.com> 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 <ard.biesheuvel@linaro.org>
> > > Cc: linux-efi@vger.kernel.org
> > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> >
> > 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.
>

That doesn't matter. The kernel will not use devices from the DT
before ExitBootServices() anyway.
Pankaj Bansal - Dec. 11, 2018, 9:27 a.m.
> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Tuesday, December 11, 2018 2:55 PM

> To: Pankaj Bansal <pankaj.bansal@nxp.com>

> Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm

> <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>; Varun Sethi

> <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>; Bhupesh Sharma

> <bhsharma@redhat.com>; linux-efi <linux-efi@vger.kernel.org>

> Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in

> configuration table

> 

> On Tue, 11 Dec 2018 at 10:23, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:

> >

> >

> >

> > > -----Original Message-----

> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > Sent: Tuesday, December 11, 2018 2:48 PM

> > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm

> > > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>;

> > > Varun Sethi <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;

> > > Bhupesh Sharma <bhsharma@redhat.com>; linux-efi

> > > <linux-efi@vger.kernel.org>

> > > 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 <pankaj.bansal@nxp.com>

> 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 <ard.biesheuvel@linaro.org>

> > > > Cc: linux-efi@vger.kernel.org

> > > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>

> > >

> > > 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.

> >

> 

> That doesn't matter. The kernel will not use devices from the DT before

> ExitBootServices() anyway.


But what is the indication uefi driver has to "relieve the device and restore it because now kernel need to use it"?
The kernel is just like any other UEFI application to uefi firmware. How will uefi firmware know when to relinquish control?
Ard Biesheuvel - Dec. 11, 2018, 9:29 a.m.
On Tue, 11 Dec 2018 at 10:27, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Tuesday, December 11, 2018 2:55 PM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm
> > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>; Varun Sethi
> > <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>; Bhupesh Sharma
> > <bhsharma@redhat.com>; linux-efi <linux-efi@vger.kernel.org>
> > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in
> > configuration table
> >
> > On Tue, 11 Dec 2018 at 10:23, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > Sent: Tuesday, December 11, 2018 2:48 PM
> > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm
> > > > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>;
> > > > Varun Sethi <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;
> > > > Bhupesh Sharma <bhsharma@redhat.com>; linux-efi
> > > > <linux-efi@vger.kernel.org>
> > > > 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 <pankaj.bansal@nxp.com>
> > 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 <ard.biesheuvel@linaro.org>
> > > > > Cc: linux-efi@vger.kernel.org
> > > > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > >
> > > > 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.
> > >
> >
> > That doesn't matter. The kernel will not use devices from the DT before
> > ExitBootServices() anyway.
>
> But what is the indication uefi driver has to "relieve the device and restore it because now kernel need to use it"?

That is ExitBootServices().

> The kernel is just like any other UEFI application to uefi firmware. How will uefi firmware know when to relinquish control?

At ExitBootServices() time. Any modification you make to the device
tree can be made beforehand, since the kernel does not even read the
device tree to discover devices until after the stub terminates.
Pankaj Bansal - Dec. 11, 2018, 10:08 a.m.
> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Tuesday, December 11, 2018 3:32 PM

> To: Pankaj Bansal <pankaj.bansal@nxp.com>

> Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in

> configuration table

> 

> On Tue, 11 Dec 2018 at 10:47, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:

> >

> >

> >

> > > -----Original Message-----

> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > Sent: Tuesday, December 11, 2018 3:10 PM

> > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt

> > > in configuration table

> > >

> > > On Tue, 11 Dec 2018 at 10:39, Pankaj Bansal <pankaj.bansal@nxp.com>

> wrote:

> > > >

> > > >

> > > >

> > > > > -----Original Message-----

> > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > > Sent: Tuesday, December 11, 2018 3:00 PM

> > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm

> > > > > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>;

> > > > > Varun Sethi <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;

> > > > > Bhupesh Sharma <bhsharma@redhat.com>; linux-efi

> > > > > <linux-efi@vger.kernel.org>

> > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new

> > > > > fdt in configuration table

> > > > >

> > > > > On Tue, 11 Dec 2018 at 10:27, Pankaj Bansal

> > > > > <pankaj.bansal@nxp.com>

> > > wrote:

> > > > > >

> > > > > >

> > > > > >

> > > > > > > -----Original Message-----

> > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > > > > Sent: Tuesday, December 11, 2018 2:55 PM

> > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm

> > > > > > > <leif.lindholm@linaro.org>; Grant Likely

> > > > > > > <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit

> > > > > > > Kumar <udit.kumar@nxp.com>; Bhupesh Sharma

> > > > > > > <bhsharma@redhat.com>; linux-efi <linux-efi@vger.kernel.org>

> > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install

> > > > > > > new fdt in configuration table

> > > > > > >

> > > > > > > On Tue, 11 Dec 2018 at 10:23, Pankaj Bansal

> > > > > > > <pankaj.bansal@nxp.com>

> > > > > wrote:

> > > > > > > >

> > > > > > > >

> > > > > > > >

> > > > > > > > > -----Original Message-----

> > > > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > > > > > > Sent: Tuesday, December 11, 2018 2:48 PM

> > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm

> > > > > > > > > <leif.lindholm@linaro.org>; Grant Likely

> > > > > > > > > <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>;

> > > > > > > > > Udit Kumar <udit.kumar@nxp.com>; Bhupesh Sharma

> > > > > > > > > <bhsharma@redhat.com>; linux-efi

> > > > > > > > > <linux-efi@vger.kernel.org>

> > > > > > > > > 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

> > > > > > > > > <pankaj.bansal@nxp.com>

> > > > > > > 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 <ard.biesheuvel@linaro.org>

> > > > > > > > > > Cc: linux-efi@vger.kernel.org

> > > > > > > > > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > > >

> > > > > > > > > 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.

> > > > > > > >

> > > > > > >

> > > > > > > That doesn't matter. The kernel will not use devices from

> > > > > > > the DT before

> > > > > > > ExitBootServices() anyway.

> > > > > >

> > > > > > But what is the indication uefi driver has to "relieve the

> > > > > > device and restore it

> > > > > because now kernel need to use it"?

> > > > >

> > > > > That is ExitBootServices().

> > > > >

> > > > > > The kernel is just like any other UEFI application to uefi

> > > > > > firmware. How will uefi

> > > > > firmware know when to relinquish control?

> > > > >

> > > > > At ExitBootServices() time. Any modification you make to the

> > > > > device tree can be made beforehand,

> > > >

> > > > When you say this, you mean before the ExitBootServices() is called ?

> > > > If yes, then when? Because before ExitBootServices(), the firmware

> > > > is using

> > > the devices and they are not ready  for kernel to use.

> > > > Before ExitBootServices, there is no other event that indicates

> > > > that now kernel

> > > is going to boot.

> > > > We fix the status of devices in device tree, only when firmware

> > > > has released

> > > the control and restored the devices for kernel to use.

> > > > If for some reason, the firmware is not able to do so, it disables

> > > > the device in

> > > device tree.

> > > >

> > > > > since the kernel does not even read the device tree to discover

> > > > > devices until after the stub terminates.

> > > >

> > > > Yes, but the stub has already copied the device tree into new

> > > > location. So, any fixups done on device tree in configuration

> > > > table would not

> > > reflect in device tree that kernel would use.

> > > >

> > > > This is why I am registering the new device tree in configuration

> > > > table. BUT only if it was generated from device tree already

> > > > present in configuration table

> > >

> > > The firmware should make changes to the DT before it supplies it to the OS.

> >

> > The point is that for uefi "supplying DT to OS" is not something that firmware

> can control.

> > Firmware is just running an efi application. It may be OS, but firmware doesn't

> know that.

> > So, firmware cannot release control of the devices beforehand.

> 

> The firmware does not have to release control of the devices before enabling

> them in the DT.


Actually I am not talking about all the devices that uefi firmware manages. I am talking about
some devices that are specific to NXP platforms. We have some devices in our platform, that
operate differently in uefi firmware than in kernel. So if we were to use these devices in kernel,
we need to release the firmware control over them and prep these for kernel.
If the devices are made ready for kernel to use, then we enable these in fdt otherwise disable these.

> 

> > If the efi application is not

> > OS, then uefi needs the control of devices to function.

> > If the efi application is OS, then it will signal ExitBootServices,

> > only then uefi knows that the Efi application is OS and uefi firmware needs to

> release the devices.

> >
Ard Biesheuvel - Dec. 11, 2018, 10:10 a.m.
On Tue, 11 Dec 2018 at 11:08, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Tuesday, December 11, 2018 3:32 PM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in
> > configuration table
> >
> > On Tue, 11 Dec 2018 at 10:47, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > Sent: Tuesday, December 11, 2018 3:10 PM
> > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt
> > > > in configuration table
> > > >
> > > > On Tue, 11 Dec 2018 at 10:39, Pankaj Bansal <pankaj.bansal@nxp.com>
> > wrote:
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > > > Sent: Tuesday, December 11, 2018 3:00 PM
> > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm
> > > > > > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>;
> > > > > > Varun Sethi <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;
> > > > > > Bhupesh Sharma <bhsharma@redhat.com>; linux-efi
> > > > > > <linux-efi@vger.kernel.org>
> > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new
> > > > > > fdt in configuration table
> > > > > >
> > > > > > On Tue, 11 Dec 2018 at 10:27, Pankaj Bansal
> > > > > > <pankaj.bansal@nxp.com>
> > > > wrote:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > > > > > Sent: Tuesday, December 11, 2018 2:55 PM
> > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm
> > > > > > > > <leif.lindholm@linaro.org>; Grant Likely
> > > > > > > > <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit
> > > > > > > > Kumar <udit.kumar@nxp.com>; Bhupesh Sharma
> > > > > > > > <bhsharma@redhat.com>; linux-efi <linux-efi@vger.kernel.org>
> > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install
> > > > > > > > new fdt in configuration table
> > > > > > > >
> > > > > > > > On Tue, 11 Dec 2018 at 10:23, Pankaj Bansal
> > > > > > > > <pankaj.bansal@nxp.com>
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > -----Original Message-----
> > > > > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > > > > > > > Sent: Tuesday, December 11, 2018 2:48 PM
> > > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm
> > > > > > > > > > <leif.lindholm@linaro.org>; Grant Likely
> > > > > > > > > > <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>;
> > > > > > > > > > Udit Kumar <udit.kumar@nxp.com>; Bhupesh Sharma
> > > > > > > > > > <bhsharma@redhat.com>; linux-efi
> > > > > > > > > > <linux-efi@vger.kernel.org>
> > > > > > > > > > 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
> > > > > > > > > > <pankaj.bansal@nxp.com>
> > > > > > > > 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 <ard.biesheuvel@linaro.org>
> > > > > > > > > > > Cc: linux-efi@vger.kernel.org
> > > > > > > > > > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > > > > >
> > > > > > > > > > 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.
> > > > > > > > >
> > > > > > > >
> > > > > > > > That doesn't matter. The kernel will not use devices from
> > > > > > > > the DT before
> > > > > > > > ExitBootServices() anyway.
> > > > > > >
> > > > > > > But what is the indication uefi driver has to "relieve the
> > > > > > > device and restore it
> > > > > > because now kernel need to use it"?
> > > > > >
> > > > > > That is ExitBootServices().
> > > > > >
> > > > > > > The kernel is just like any other UEFI application to uefi
> > > > > > > firmware. How will uefi
> > > > > > firmware know when to relinquish control?
> > > > > >
> > > > > > At ExitBootServices() time. Any modification you make to the
> > > > > > device tree can be made beforehand,
> > > > >
> > > > > When you say this, you mean before the ExitBootServices() is called ?
> > > > > If yes, then when? Because before ExitBootServices(), the firmware
> > > > > is using
> > > > the devices and they are not ready  for kernel to use.
> > > > > Before ExitBootServices, there is no other event that indicates
> > > > > that now kernel
> > > > is going to boot.
> > > > > We fix the status of devices in device tree, only when firmware
> > > > > has released
> > > > the control and restored the devices for kernel to use.
> > > > > If for some reason, the firmware is not able to do so, it disables
> > > > > the device in
> > > > device tree.
> > > > >
> > > > > > since the kernel does not even read the device tree to discover
> > > > > > devices until after the stub terminates.
> > > > >
> > > > > Yes, but the stub has already copied the device tree into new
> > > > > location. So, any fixups done on device tree in configuration
> > > > > table would not
> > > > reflect in device tree that kernel would use.
> > > > >
> > > > > This is why I am registering the new device tree in configuration
> > > > > table. BUT only if it was generated from device tree already
> > > > > present in configuration table
> > > >
> > > > The firmware should make changes to the DT before it supplies it to the OS.
> > >
> > > The point is that for uefi "supplying DT to OS" is not something that firmware
> > can control.
> > > Firmware is just running an efi application. It may be OS, but firmware doesn't
> > know that.
> > > So, firmware cannot release control of the devices beforehand.
> >
> > The firmware does not have to release control of the devices before enabling
> > them in the DT.
>
> Actually I am not talking about all the devices that uefi firmware manages. I am talking about
> some devices that are specific to NXP platforms. We have some devices in our platform, that
> operate differently in uefi firmware than in kernel. So if we were to use these devices in kernel,
> we need to release the firmware control over them and prep these for kernel.
> If the devices are made ready for kernel to use, then we enable these in fdt otherwise disable these.
>

i understand that.

But you fail to explain why those DT changes cannot be made while the
firmware is still using those devices. The kernel will not look at the
DT until after ExitBootServices() so why is it a problem to enable
them in the DT earlier?
Pankaj Bansal - Dec. 11, 2018, 10:13 a.m.
> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Tuesday, December 11, 2018 3:41 PM

> To: Pankaj Bansal <pankaj.bansal@nxp.com>

> Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm

> <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>; Varun Sethi

> <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>; Bhupesh Sharma

> <bhsharma@redhat.com>; linux-efi <linux-efi@vger.kernel.org>

> Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in

> configuration table

> 

> On Tue, 11 Dec 2018 at 11:08, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:

> >

> >

> >

> > > -----Original Message-----

> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > Sent: Tuesday, December 11, 2018 3:32 PM

> > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt

> > > in configuration table

> > >

> > > On Tue, 11 Dec 2018 at 10:47, Pankaj Bansal <pankaj.bansal@nxp.com>

> wrote:

> > > >

> > > >

> > > >

> > > > > -----Original Message-----

> > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > > Sent: Tuesday, December 11, 2018 3:10 PM

> > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new

> > > > > fdt in configuration table

> > > > >

> > > > > On Tue, 11 Dec 2018 at 10:39, Pankaj Bansal

> > > > > <pankaj.bansal@nxp.com>

> > > wrote:

> > > > > >

> > > > > >

> > > > > >

> > > > > > > -----Original Message-----

> > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > > > > Sent: Tuesday, December 11, 2018 3:00 PM

> > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm

> > > > > > > <leif.lindholm@linaro.org>; Grant Likely

> > > > > > > <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit

> > > > > > > Kumar <udit.kumar@nxp.com>; Bhupesh Sharma

> > > > > > > <bhsharma@redhat.com>; linux-efi <linux-efi@vger.kernel.org>

> > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install

> > > > > > > new fdt in configuration table

> > > > > > >

> > > > > > > On Tue, 11 Dec 2018 at 10:27, Pankaj Bansal

> > > > > > > <pankaj.bansal@nxp.com>

> > > > > wrote:

> > > > > > > >

> > > > > > > >

> > > > > > > >

> > > > > > > > > -----Original Message-----

> > > > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > > > > > > Sent: Tuesday, December 11, 2018 2:55 PM

> > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm

> > > > > > > > > <leif.lindholm@linaro.org>; Grant Likely

> > > > > > > > > <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>;

> > > > > > > > > Udit Kumar <udit.kumar@nxp.com>; Bhupesh Sharma

> > > > > > > > > <bhsharma@redhat.com>; linux-efi

> > > > > > > > > <linux-efi@vger.kernel.org>

> > > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi:

> > > > > > > > > install new fdt in configuration table

> > > > > > > > >

> > > > > > > > > On Tue, 11 Dec 2018 at 10:23, Pankaj Bansal

> > > > > > > > > <pankaj.bansal@nxp.com>

> > > > > > > wrote:

> > > > > > > > > >

> > > > > > > > > >

> > > > > > > > > >

> > > > > > > > > > > -----Original Message-----

> > > > > > > > > > > From: Ard Biesheuvel

> > > > > > > > > > > [mailto:ard.biesheuvel@linaro.org]

> > > > > > > > > > > Sent: Tuesday, December 11, 2018 2:48 PM

> > > > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif

> > > > > > > > > > > Lindholm <leif.lindholm@linaro.org>; Grant Likely

> > > > > > > > > > > <grant.likely@arm.com>; Varun Sethi

> > > > > > > > > > > <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;

> > > > > > > > > > > Bhupesh Sharma <bhsharma@redhat.com>; linux-efi

> > > > > > > > > > > <linux-efi@vger.kernel.org>

> > > > > > > > > > > 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

> > > > > > > > > > > <pankaj.bansal@nxp.com>

> > > > > > > > > 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 <ard.biesheuvel@linaro.org>

> > > > > > > > > > > > Cc: linux-efi@vger.kernel.org

> > > > > > > > > > > > Signed-off-by: Pankaj Bansal

> > > > > > > > > > > > <pankaj.bansal@nxp.com>

> > > > > > > > > > >

> > > > > > > > > > > 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.

> > > > > > > > > >

> > > > > > > > >

> > > > > > > > > That doesn't matter. The kernel will not use devices

> > > > > > > > > from the DT before

> > > > > > > > > ExitBootServices() anyway.

> > > > > > > >

> > > > > > > > But what is the indication uefi driver has to "relieve the

> > > > > > > > device and restore it

> > > > > > > because now kernel need to use it"?

> > > > > > >

> > > > > > > That is ExitBootServices().

> > > > > > >

> > > > > > > > The kernel is just like any other UEFI application to uefi

> > > > > > > > firmware. How will uefi

> > > > > > > firmware know when to relinquish control?

> > > > > > >

> > > > > > > At ExitBootServices() time. Any modification you make to the

> > > > > > > device tree can be made beforehand,

> > > > > >

> > > > > > When you say this, you mean before the ExitBootServices() is called ?

> > > > > > If yes, then when? Because before ExitBootServices(), the

> > > > > > firmware is using

> > > > > the devices and they are not ready  for kernel to use.

> > > > > > Before ExitBootServices, there is no other event that

> > > > > > indicates that now kernel

> > > > > is going to boot.

> > > > > > We fix the status of devices in device tree, only when

> > > > > > firmware has released

> > > > > the control and restored the devices for kernel to use.

> > > > > > If for some reason, the firmware is not able to do so, it

> > > > > > disables the device in

> > > > > device tree.

> > > > > >

> > > > > > > since the kernel does not even read the device tree to

> > > > > > > discover devices until after the stub terminates.

> > > > > >

> > > > > > Yes, but the stub has already copied the device tree into new

> > > > > > location. So, any fixups done on device tree in configuration

> > > > > > table would not

> > > > > reflect in device tree that kernel would use.

> > > > > >

> > > > > > This is why I am registering the new device tree in

> > > > > > configuration table. BUT only if it was generated from device

> > > > > > tree already present in configuration table

> > > > >

> > > > > The firmware should make changes to the DT before it supplies it to the

> OS.

> > > >

> > > > The point is that for uefi "supplying DT to OS" is not something

> > > > that firmware

> > > can control.

> > > > Firmware is just running an efi application. It may be OS, but

> > > > firmware doesn't

> > > know that.

> > > > So, firmware cannot release control of the devices beforehand.

> > >

> > > The firmware does not have to release control of the devices before

> > > enabling them in the DT.

> >

> > Actually I am not talking about all the devices that uefi firmware

> > manages. I am talking about some devices that are specific to NXP

> > platforms. We have some devices in our platform, that operate

> > differently in uefi firmware than in kernel. So if we were to use these devices in

> kernel, we need to release the firmware control over them and prep these for

> kernel.

> > If the devices are made ready for kernel to use, then we enable these in fdt

> otherwise disable these.

> >

> 

> i understand that.

> 

> But you fail to explain why those DT changes cannot be made while the

> firmware is still using those devices. The kernel will not look at the DT until after

> ExitBootServices() so why is it a problem to enable them in the DT earlier?


What if we were not able to prep these devices for kernel? How will we disable these devices?
Ard Biesheuvel - Dec. 11, 2018, 10:14 a.m.
On Tue, 11 Dec 2018 at 11:13, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Tuesday, December 11, 2018 3:41 PM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm
> > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>; Varun Sethi
> > <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>; Bhupesh Sharma
> > <bhsharma@redhat.com>; linux-efi <linux-efi@vger.kernel.org>
> > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in
> > configuration table
> >
> > On Tue, 11 Dec 2018 at 11:08, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > Sent: Tuesday, December 11, 2018 3:32 PM
> > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt
> > > > in configuration table
> > > >
> > > > On Tue, 11 Dec 2018 at 10:47, Pankaj Bansal <pankaj.bansal@nxp.com>
> > wrote:
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > > > Sent: Tuesday, December 11, 2018 3:10 PM
> > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new
> > > > > > fdt in configuration table
> > > > > >
> > > > > > On Tue, 11 Dec 2018 at 10:39, Pankaj Bansal
> > > > > > <pankaj.bansal@nxp.com>
> > > > wrote:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > > > > > Sent: Tuesday, December 11, 2018 3:00 PM
> > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm
> > > > > > > > <leif.lindholm@linaro.org>; Grant Likely
> > > > > > > > <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit
> > > > > > > > Kumar <udit.kumar@nxp.com>; Bhupesh Sharma
> > > > > > > > <bhsharma@redhat.com>; linux-efi <linux-efi@vger.kernel.org>
> > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install
> > > > > > > > new fdt in configuration table
> > > > > > > >
> > > > > > > > On Tue, 11 Dec 2018 at 10:27, Pankaj Bansal
> > > > > > > > <pankaj.bansal@nxp.com>
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > -----Original Message-----
> > > > > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > > > > > > > Sent: Tuesday, December 11, 2018 2:55 PM
> > > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm
> > > > > > > > > > <leif.lindholm@linaro.org>; Grant Likely
> > > > > > > > > > <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>;
> > > > > > > > > > Udit Kumar <udit.kumar@nxp.com>; Bhupesh Sharma
> > > > > > > > > > <bhsharma@redhat.com>; linux-efi
> > > > > > > > > > <linux-efi@vger.kernel.org>
> > > > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi:
> > > > > > > > > > install new fdt in configuration table
> > > > > > > > > >
> > > > > > > > > > On Tue, 11 Dec 2018 at 10:23, Pankaj Bansal
> > > > > > > > > > <pankaj.bansal@nxp.com>
> > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > > -----Original Message-----
> > > > > > > > > > > > From: Ard Biesheuvel
> > > > > > > > > > > > [mailto:ard.biesheuvel@linaro.org]
> > > > > > > > > > > > Sent: Tuesday, December 11, 2018 2:48 PM
> > > > > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif
> > > > > > > > > > > > Lindholm <leif.lindholm@linaro.org>; Grant Likely
> > > > > > > > > > > > <grant.likely@arm.com>; Varun Sethi
> > > > > > > > > > > > <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;
> > > > > > > > > > > > Bhupesh Sharma <bhsharma@redhat.com>; linux-efi
> > > > > > > > > > > > <linux-efi@vger.kernel.org>
> > > > > > > > > > > > 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
> > > > > > > > > > > > <pankaj.bansal@nxp.com>
> > > > > > > > > > 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 <ard.biesheuvel@linaro.org>
> > > > > > > > > > > > > Cc: linux-efi@vger.kernel.org
> > > > > > > > > > > > > Signed-off-by: Pankaj Bansal
> > > > > > > > > > > > > <pankaj.bansal@nxp.com>
> > > > > > > > > > > >
> > > > > > > > > > > > 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.
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > That doesn't matter. The kernel will not use devices
> > > > > > > > > > from the DT before
> > > > > > > > > > ExitBootServices() anyway.
> > > > > > > > >
> > > > > > > > > But what is the indication uefi driver has to "relieve the
> > > > > > > > > device and restore it
> > > > > > > > because now kernel need to use it"?
> > > > > > > >
> > > > > > > > That is ExitBootServices().
> > > > > > > >
> > > > > > > > > The kernel is just like any other UEFI application to uefi
> > > > > > > > > firmware. How will uefi
> > > > > > > > firmware know when to relinquish control?
> > > > > > > >
> > > > > > > > At ExitBootServices() time. Any modification you make to the
> > > > > > > > device tree can be made beforehand,
> > > > > > >
> > > > > > > When you say this, you mean before the ExitBootServices() is called ?
> > > > > > > If yes, then when? Because before ExitBootServices(), the
> > > > > > > firmware is using
> > > > > > the devices and they are not ready  for kernel to use.
> > > > > > > Before ExitBootServices, there is no other event that
> > > > > > > indicates that now kernel
> > > > > > is going to boot.
> > > > > > > We fix the status of devices in device tree, only when
> > > > > > > firmware has released
> > > > > > the control and restored the devices for kernel to use.
> > > > > > > If for some reason, the firmware is not able to do so, it
> > > > > > > disables the device in
> > > > > > device tree.
> > > > > > >
> > > > > > > > since the kernel does not even read the device tree to
> > > > > > > > discover devices until after the stub terminates.
> > > > > > >
> > > > > > > Yes, but the stub has already copied the device tree into new
> > > > > > > location. So, any fixups done on device tree in configuration
> > > > > > > table would not
> > > > > > reflect in device tree that kernel would use.
> > > > > > >
> > > > > > > This is why I am registering the new device tree in
> > > > > > > configuration table. BUT only if it was generated from device
> > > > > > > tree already present in configuration table
> > > > > >
> > > > > > The firmware should make changes to the DT before it supplies it to the
> > OS.
> > > > >
> > > > > The point is that for uefi "supplying DT to OS" is not something
> > > > > that firmware
> > > > can control.
> > > > > Firmware is just running an efi application. It may be OS, but
> > > > > firmware doesn't
> > > > know that.
> > > > > So, firmware cannot release control of the devices beforehand.
> > > >
> > > > The firmware does not have to release control of the devices before
> > > > enabling them in the DT.
> > >
> > > Actually I am not talking about all the devices that uefi firmware
> > > manages. I am talking about some devices that are specific to NXP
> > > platforms. We have some devices in our platform, that operate
> > > differently in uefi firmware than in kernel. So if we were to use these devices in
> > kernel, we need to release the firmware control over them and prep these for
> > kernel.
> > > If the devices are made ready for kernel to use, then we enable these in fdt
> > otherwise disable these.
> > >
> >
> > i understand that.
> >
> > But you fail to explain why those DT changes cannot be made while the
> > firmware is still using those devices. The kernel will not look at the DT until after
> > ExitBootServices() so why is it a problem to enable them in the DT earlier?
>
> What if we were not able to prep these devices for kernel? How will we disable these devices?

Ah.

That is a completely different matter.

So why would you be unable to prep a device for the kernel, and why
would the correct course of action be to remove it from the DT as if
it no longer exists?
Pankaj Bansal - Dec. 11, 2018, 10:24 a.m.
> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Tuesday, December 11, 2018 3:45 PM

> To: Pankaj Bansal <pankaj.bansal@nxp.com>

> Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm

> <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>; Varun Sethi

> <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>; Bhupesh Sharma

> <bhsharma@redhat.com>; linux-efi <linux-efi@vger.kernel.org>

> Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in

> configuration table

> 

> On Tue, 11 Dec 2018 at 11:13, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:

> >

> >

> >

> > > -----Original Message-----

> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > Sent: Tuesday, December 11, 2018 3:41 PM

> > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm

> > > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>;

> > > Varun Sethi <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;

> > > Bhupesh Sharma <bhsharma@redhat.com>; linux-efi

> > > <linux-efi@vger.kernel.org>

> > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt

> > > in configuration table

> > >

> > > On Tue, 11 Dec 2018 at 11:08, Pankaj Bansal <pankaj.bansal@nxp.com>

> wrote:

> > > >

> > > >

> > > >

> > > > > -----Original Message-----

> > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > > Sent: Tuesday, December 11, 2018 3:32 PM

> > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new

> > > > > fdt in configuration table

> > > > >

> > > > > On Tue, 11 Dec 2018 at 10:47, Pankaj Bansal

> > > > > <pankaj.bansal@nxp.com>

> > > wrote:

> > > > > >

> > > > > >

> > > > > >

> > > > > > > -----Original Message-----

> > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > > > > Sent: Tuesday, December 11, 2018 3:10 PM

> > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install

> > > > > > > new fdt in configuration table

> > > > > > >

> > > > > > > On Tue, 11 Dec 2018 at 10:39, Pankaj Bansal

> > > > > > > <pankaj.bansal@nxp.com>

> > > > > wrote:

> > > > > > > >

> > > > > > > >

> > > > > > > >

> > > > > > > > > -----Original Message-----

> > > > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > > > > > > Sent: Tuesday, December 11, 2018 3:00 PM

> > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm

> > > > > > > > > <leif.lindholm@linaro.org>; Grant Likely

> > > > > > > > > <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>;

> > > > > > > > > Udit Kumar <udit.kumar@nxp.com>; Bhupesh Sharma

> > > > > > > > > <bhsharma@redhat.com>; linux-efi

> > > > > > > > > <linux-efi@vger.kernel.org>

> > > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi:

> > > > > > > > > install new fdt in configuration table

> > > > > > > > >

> > > > > > > > > On Tue, 11 Dec 2018 at 10:27, Pankaj Bansal

> > > > > > > > > <pankaj.bansal@nxp.com>

> > > > > > > wrote:

> > > > > > > > > >

> > > > > > > > > >

> > > > > > > > > >

> > > > > > > > > > > -----Original Message-----

> > > > > > > > > > > From: Ard Biesheuvel

> > > > > > > > > > > [mailto:ard.biesheuvel@linaro.org]

> > > > > > > > > > > Sent: Tuesday, December 11, 2018 2:55 PM

> > > > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif

> > > > > > > > > > > Lindholm <leif.lindholm@linaro.org>; Grant Likely

> > > > > > > > > > > <grant.likely@arm.com>; Varun Sethi

> > > > > > > > > > > <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;

> > > > > > > > > > > Bhupesh Sharma <bhsharma@redhat.com>; linux-efi

> > > > > > > > > > > <linux-efi@vger.kernel.org>

> > > > > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi:

> > > > > > > > > > > install new fdt in configuration table

> > > > > > > > > > >

> > > > > > > > > > > On Tue, 11 Dec 2018 at 10:23, Pankaj Bansal

> > > > > > > > > > > <pankaj.bansal@nxp.com>

> > > > > > > > > wrote:

> > > > > > > > > > > >

> > > > > > > > > > > >

> > > > > > > > > > > >

> > > > > > > > > > > > > -----Original Message-----

> > > > > > > > > > > > > From: Ard Biesheuvel

> > > > > > > > > > > > > [mailto:ard.biesheuvel@linaro.org]

> > > > > > > > > > > > > Sent: Tuesday, December 11, 2018 2:48 PM

> > > > > > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif

> > > > > > > > > > > > > Lindholm <leif.lindholm@linaro.org>; Grant

> > > > > > > > > > > > > Likely <grant.likely@arm.com>; Varun Sethi

> > > > > > > > > > > > > <V.Sethi@nxp.com>; Udit Kumar

> > > > > > > > > > > > > <udit.kumar@nxp.com>; Bhupesh Sharma

> > > > > > > > > > > > > <bhsharma@redhat.com>; linux-efi

> > > > > > > > > > > > > <linux-efi@vger.kernel.org>

> > > > > > > > > > > > > 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

> > > > > > > > > > > > > <pankaj.bansal@nxp.com>

> > > > > > > > > > > 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 <ard.biesheuvel@linaro.org>

> > > > > > > > > > > > > > Cc: linux-efi@vger.kernel.org

> > > > > > > > > > > > > > Signed-off-by: Pankaj Bansal

> > > > > > > > > > > > > > <pankaj.bansal@nxp.com>

> > > > > > > > > > > > >

> > > > > > > > > > > > > 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.

> > > > > > > > > > > >

> > > > > > > > > > >

> > > > > > > > > > > That doesn't matter. The kernel will not use devices

> > > > > > > > > > > from the DT before

> > > > > > > > > > > ExitBootServices() anyway.

> > > > > > > > > >

> > > > > > > > > > But what is the indication uefi driver has to "relieve

> > > > > > > > > > the device and restore it

> > > > > > > > > because now kernel need to use it"?

> > > > > > > > >

> > > > > > > > > That is ExitBootServices().

> > > > > > > > >

> > > > > > > > > > The kernel is just like any other UEFI application to

> > > > > > > > > > uefi firmware. How will uefi

> > > > > > > > > firmware know when to relinquish control?

> > > > > > > > >

> > > > > > > > > At ExitBootServices() time. Any modification you make to

> > > > > > > > > the device tree can be made beforehand,

> > > > > > > >

> > > > > > > > When you say this, you mean before the ExitBootServices() is called

> ?

> > > > > > > > If yes, then when? Because before ExitBootServices(), the

> > > > > > > > firmware is using

> > > > > > > the devices and they are not ready  for kernel to use.

> > > > > > > > Before ExitBootServices, there is no other event that

> > > > > > > > indicates that now kernel

> > > > > > > is going to boot.

> > > > > > > > We fix the status of devices in device tree, only when

> > > > > > > > firmware has released

> > > > > > > the control and restored the devices for kernel to use.

> > > > > > > > If for some reason, the firmware is not able to do so, it

> > > > > > > > disables the device in

> > > > > > > device tree.

> > > > > > > >

> > > > > > > > > since the kernel does not even read the device tree to

> > > > > > > > > discover devices until after the stub terminates.

> > > > > > > >

> > > > > > > > Yes, but the stub has already copied the device tree into

> > > > > > > > new location. So, any fixups done on device tree in

> > > > > > > > configuration table would not

> > > > > > > reflect in device tree that kernel would use.

> > > > > > > >

> > > > > > > > This is why I am registering the new device tree in

> > > > > > > > configuration table. BUT only if it was generated from

> > > > > > > > device tree already present in configuration table

> > > > > > >

> > > > > > > The firmware should make changes to the DT before it

> > > > > > > supplies it to the

> > > OS.

> > > > > >

> > > > > > The point is that for uefi "supplying DT to OS" is not

> > > > > > something that firmware

> > > > > can control.

> > > > > > Firmware is just running an efi application. It may be OS, but

> > > > > > firmware doesn't

> > > > > know that.

> > > > > > So, firmware cannot release control of the devices beforehand.

> > > > >

> > > > > The firmware does not have to release control of the devices

> > > > > before enabling them in the DT.

> > > >

> > > > Actually I am not talking about all the devices that uefi firmware

> > > > manages. I am talking about some devices that are specific to NXP

> > > > platforms. We have some devices in our platform, that operate

> > > > differently in uefi firmware than in kernel. So if we were to use

> > > > these devices in

> > > kernel, we need to release the firmware control over them and prep

> > > these for kernel.

> > > > If the devices are made ready for kernel to use, then we enable

> > > > these in fdt

> > > otherwise disable these.

> > > >

> > >

> > > i understand that.

> > >

> > > But you fail to explain why those DT changes cannot be made while

> > > the firmware is still using those devices. The kernel will not look

> > > at the DT until after

> > > ExitBootServices() so why is it a problem to enable them in the DT earlier?

> >

> > What if we were not able to prep these devices for kernel? How will we disable

> these devices?

> 

> Ah.

> 

> That is a completely different matter.

> 

> So why would you be unable to prep a device for the kernel, and why would the

> correct course of action be to remove it from the DT as if it no longer exists?


Actually the preparation of these devices depends on certain parameter files that user supplies.
These parameters are not used by firmware but are used by kernel.
These parameters need to match to the boot time configuration of platform.
That is why these parameters are applied by boot firmware and not by kernel.
If there is a mismatch between the boot time configuration and parameters, then the parameters would not be applied.
In this case kernel is not able to use the devices.
Which is why we disable this particular devices, so that rest of the system can boot.
Otherwise the device driver in kernel misbehaves.
Bhupesh Sharma - Dec. 11, 2018, 10:55 a.m.
Hi Pankaj,

On Tue, Dec 11, 2018 at 3:54 PM Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Tuesday, December 11, 2018 3:45 PM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm
> > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>; Varun Sethi
> > <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>; Bhupesh Sharma
> > <bhsharma@redhat.com>; linux-efi <linux-efi@vger.kernel.org>
> > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in
> > configuration table
> >
> > On Tue, 11 Dec 2018 at 11:13, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > Sent: Tuesday, December 11, 2018 3:41 PM
> > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm
> > > > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>;
> > > > Varun Sethi <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;
> > > > Bhupesh Sharma <bhsharma@redhat.com>; linux-efi
> > > > <linux-efi@vger.kernel.org>
> > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt
> > > > in configuration table
> > > >
> > > > On Tue, 11 Dec 2018 at 11:08, Pankaj Bansal <pankaj.bansal@nxp.com>
> > wrote:
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > > > Sent: Tuesday, December 11, 2018 3:32 PM
> > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new
> > > > > > fdt in configuration table
> > > > > >
> > > > > > On Tue, 11 Dec 2018 at 10:47, Pankaj Bansal
> > > > > > <pankaj.bansal@nxp.com>
> > > > wrote:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > > > > > Sent: Tuesday, December 11, 2018 3:10 PM
> > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install
> > > > > > > > new fdt in configuration table
> > > > > > > >
> > > > > > > > On Tue, 11 Dec 2018 at 10:39, Pankaj Bansal
> > > > > > > > <pankaj.bansal@nxp.com>
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > -----Original Message-----
> > > > > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > > > > > > > Sent: Tuesday, December 11, 2018 3:00 PM
> > > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm
> > > > > > > > > > <leif.lindholm@linaro.org>; Grant Likely
> > > > > > > > > > <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>;
> > > > > > > > > > Udit Kumar <udit.kumar@nxp.com>; Bhupesh Sharma
> > > > > > > > > > <bhsharma@redhat.com>; linux-efi
> > > > > > > > > > <linux-efi@vger.kernel.org>
> > > > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi:
> > > > > > > > > > install new fdt in configuration table
> > > > > > > > > >
> > > > > > > > > > On Tue, 11 Dec 2018 at 10:27, Pankaj Bansal
> > > > > > > > > > <pankaj.bansal@nxp.com>
> > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > > -----Original Message-----
> > > > > > > > > > > > From: Ard Biesheuvel
> > > > > > > > > > > > [mailto:ard.biesheuvel@linaro.org]
> > > > > > > > > > > > Sent: Tuesday, December 11, 2018 2:55 PM
> > > > > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif
> > > > > > > > > > > > Lindholm <leif.lindholm@linaro.org>; Grant Likely
> > > > > > > > > > > > <grant.likely@arm.com>; Varun Sethi
> > > > > > > > > > > > <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;
> > > > > > > > > > > > Bhupesh Sharma <bhsharma@redhat.com>; linux-efi
> > > > > > > > > > > > <linux-efi@vger.kernel.org>
> > > > > > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi:
> > > > > > > > > > > > install new fdt in configuration table
> > > > > > > > > > > >
> > > > > > > > > > > > On Tue, 11 Dec 2018 at 10:23, Pankaj Bansal
> > > > > > > > > > > > <pankaj.bansal@nxp.com>
> > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > > -----Original Message-----
> > > > > > > > > > > > > > From: Ard Biesheuvel
> > > > > > > > > > > > > > [mailto:ard.biesheuvel@linaro.org]
> > > > > > > > > > > > > > Sent: Tuesday, December 11, 2018 2:48 PM
> > > > > > > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > > > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif
> > > > > > > > > > > > > > Lindholm <leif.lindholm@linaro.org>; Grant
> > > > > > > > > > > > > > Likely <grant.likely@arm.com>; Varun Sethi
> > > > > > > > > > > > > > <V.Sethi@nxp.com>; Udit Kumar
> > > > > > > > > > > > > > <udit.kumar@nxp.com>; Bhupesh Sharma
> > > > > > > > > > > > > > <bhsharma@redhat.com>; linux-efi
> > > > > > > > > > > > > > <linux-efi@vger.kernel.org>
> > > > > > > > > > > > > > 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
> > > > > > > > > > > > > > <pankaj.bansal@nxp.com>
> > > > > > > > > > > > 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 <ard.biesheuvel@linaro.org>
> > > > > > > > > > > > > > > Cc: linux-efi@vger.kernel.org
> > > > > > > > > > > > > > > Signed-off-by: Pankaj Bansal
> > > > > > > > > > > > > > > <pankaj.bansal@nxp.com>
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > 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.
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > That doesn't matter. The kernel will not use devices
> > > > > > > > > > > > from the DT before
> > > > > > > > > > > > ExitBootServices() anyway.
> > > > > > > > > > >
> > > > > > > > > > > But what is the indication uefi driver has to "relieve
> > > > > > > > > > > the device and restore it
> > > > > > > > > > because now kernel need to use it"?
> > > > > > > > > >
> > > > > > > > > > That is ExitBootServices().
> > > > > > > > > >
> > > > > > > > > > > The kernel is just like any other UEFI application to
> > > > > > > > > > > uefi firmware. How will uefi
> > > > > > > > > > firmware know when to relinquish control?
> > > > > > > > > >
> > > > > > > > > > At ExitBootServices() time. Any modification you make to
> > > > > > > > > > the device tree can be made beforehand,
> > > > > > > > >
> > > > > > > > > When you say this, you mean before the ExitBootServices() is called
> > ?
> > > > > > > > > If yes, then when? Because before ExitBootServices(), the
> > > > > > > > > firmware is using
> > > > > > > > the devices and they are not ready  for kernel to use.
> > > > > > > > > Before ExitBootServices, there is no other event that
> > > > > > > > > indicates that now kernel
> > > > > > > > is going to boot.
> > > > > > > > > We fix the status of devices in device tree, only when
> > > > > > > > > firmware has released
> > > > > > > > the control and restored the devices for kernel to use.
> > > > > > > > > If for some reason, the firmware is not able to do so, it
> > > > > > > > > disables the device in
> > > > > > > > device tree.
> > > > > > > > >
> > > > > > > > > > since the kernel does not even read the device tree to
> > > > > > > > > > discover devices until after the stub terminates.
> > > > > > > > >
> > > > > > > > > Yes, but the stub has already copied the device tree into
> > > > > > > > > new location. So, any fixups done on device tree in
> > > > > > > > > configuration table would not
> > > > > > > > reflect in device tree that kernel would use.
> > > > > > > > >
> > > > > > > > > This is why I am registering the new device tree in
> > > > > > > > > configuration table. BUT only if it was generated from
> > > > > > > > > device tree already present in configuration table
> > > > > > > >
> > > > > > > > The firmware should make changes to the DT before it
> > > > > > > > supplies it to the
> > > > OS.
> > > > > > >
> > > > > > > The point is that for uefi "supplying DT to OS" is not
> > > > > > > something that firmware
> > > > > > can control.
> > > > > > > Firmware is just running an efi application. It may be OS, but
> > > > > > > firmware doesn't
> > > > > > know that.
> > > > > > > So, firmware cannot release control of the devices beforehand.
> > > > > >
> > > > > > The firmware does not have to release control of the devices
> > > > > > before enabling them in the DT.
> > > > >
> > > > > Actually I am not talking about all the devices that uefi firmware
> > > > > manages. I am talking about some devices that are specific to NXP
> > > > > platforms. We have some devices in our platform, that operate
> > > > > differently in uefi firmware than in kernel. So if we were to use
> > > > > these devices in
> > > > kernel, we need to release the firmware control over them and prep
> > > > these for kernel.
> > > > > If the devices are made ready for kernel to use, then we enable
> > > > > these in fdt
> > > > otherwise disable these.
> > > > >
> > > >
> > > > i understand that.
> > > >
> > > > But you fail to explain why those DT changes cannot be made while
> > > > the firmware is still using those devices. The kernel will not look
> > > > at the DT until after
> > > > ExitBootServices() so why is it a problem to enable them in the DT earlier?
> > >
> > > What if we were not able to prep these devices for kernel? How will we disable
> > these devices?
> >
> > Ah.
> >
> > That is a completely different matter.
> >
> > So why would you be unable to prep a device for the kernel, and why would the
> > correct course of action be to remove it from the DT as if it no longer exists?
>
> Actually the preparation of these devices depends on certain parameter files that user supplies.

Where are these parameters present during the boot time and do you
mean here that the default dtb passed during the boot time needs to be
modified by the kernel or boot firmware here?

> These parameters are not used by firmware but are used by kernel.
> These parameters need to match to the boot time configuration of platform.
> That is why these parameters are applied by boot firmware and not by kernel.
> If there is a mismatch between the boot time configuration and parameters, then the parameters would not be applied.
> In this case kernel is not able to use the devices.
> Which is why we disable this particular devices, so that rest of the system can boot.
> Otherwise the device driver in kernel misbehaves.

Can you please share an example, as the above description is not very
clear to me. May be you can include a dt property that you are trying
to fix via kernel and what happens in the kernel driver when it is not
in a expected state.

Also may be you can share why the boot firmware is not able to set a
correct state of the same.

Thanks,
Bhupesh
Pankaj Bansal - Dec. 11, 2018, 12:22 p.m.
> -----Original Message-----

> From: Bhupesh Sharma [mailto:bhsharma@redhat.com]

> Sent: Tuesday, December 11, 2018 4:25 PM

> To: Pankaj Bansal <pankaj.bansal@nxp.com>

> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Mark Rutland

> <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>; Grant

> Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit Kumar

> <udit.kumar@nxp.com>; linux-efi@vger.kernel.org

> Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in

> configuration table

> 

> Hi Pankaj,

> 

> On Tue, Dec 11, 2018 at 3:54 PM Pankaj Bansal <pankaj.bansal@nxp.com>

> wrote:

> >

> >

> >

> > > -----Original Message-----

> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > Sent: Tuesday, December 11, 2018 3:45 PM

> > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm

> > > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>;

> > > Varun Sethi <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;

> > > Bhupesh Sharma <bhsharma@redhat.com>; linux-efi

> > > <linux-efi@vger.kernel.org>

> > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt

> > > in configuration table

> > >

> > > On Tue, 11 Dec 2018 at 11:13, Pankaj Bansal <pankaj.bansal@nxp.com>

> wrote:

> > > >

> > > >

> > > >

> > > > > -----Original Message-----

> > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > > Sent: Tuesday, December 11, 2018 3:41 PM

> > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif Lindholm

> > > > > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>;

> > > > > Varun Sethi <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;

> > > > > Bhupesh Sharma <bhsharma@redhat.com>; linux-efi

> > > > > <linux-efi@vger.kernel.org>

> > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new

> > > > > fdt in configuration table

> > > > >

> > > > > On Tue, 11 Dec 2018 at 11:08, Pankaj Bansal

> > > > > <pankaj.bansal@nxp.com>

> > > wrote:

> > > > > >

> > > > > >

> > > > > >

> > > > > > > -----Original Message-----

> > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > > > > Sent: Tuesday, December 11, 2018 3:32 PM

> > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install

> > > > > > > new fdt in configuration table

> > > > > > >

> > > > > > > On Tue, 11 Dec 2018 at 10:47, Pankaj Bansal

> > > > > > > <pankaj.bansal@nxp.com>

> > > > > wrote:

> > > > > > > >

> > > > > > > >

> > > > > > > >

> > > > > > > > > -----Original Message-----

> > > > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > > > > > > Sent: Tuesday, December 11, 2018 3:10 PM

> > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi:

> > > > > > > > > install new fdt in configuration table

> > > > > > > > >

> > > > > > > > > On Tue, 11 Dec 2018 at 10:39, Pankaj Bansal

> > > > > > > > > <pankaj.bansal@nxp.com>

> > > > > > > wrote:

> > > > > > > > > >

> > > > > > > > > >

> > > > > > > > > >

> > > > > > > > > > > -----Original Message-----

> > > > > > > > > > > From: Ard Biesheuvel

> > > > > > > > > > > [mailto:ard.biesheuvel@linaro.org]

> > > > > > > > > > > Sent: Tuesday, December 11, 2018 3:00 PM

> > > > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif

> > > > > > > > > > > Lindholm <leif.lindholm@linaro.org>; Grant Likely

> > > > > > > > > > > <grant.likely@arm.com>; Varun Sethi

> > > > > > > > > > > <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;

> > > > > > > > > > > Bhupesh Sharma <bhsharma@redhat.com>; linux-efi

> > > > > > > > > > > <linux-efi@vger.kernel.org>

> > > > > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi:

> > > > > > > > > > > install new fdt in configuration table

> > > > > > > > > > >

> > > > > > > > > > > On Tue, 11 Dec 2018 at 10:27, Pankaj Bansal

> > > > > > > > > > > <pankaj.bansal@nxp.com>

> > > > > > > > > wrote:

> > > > > > > > > > > >

> > > > > > > > > > > >

> > > > > > > > > > > >

> > > > > > > > > > > > > -----Original Message-----

> > > > > > > > > > > > > From: Ard Biesheuvel

> > > > > > > > > > > > > [mailto:ard.biesheuvel@linaro.org]

> > > > > > > > > > > > > Sent: Tuesday, December 11, 2018 2:55 PM

> > > > > > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>; Leif

> > > > > > > > > > > > > Lindholm <leif.lindholm@linaro.org>; Grant

> > > > > > > > > > > > > Likely <grant.likely@arm.com>; Varun Sethi

> > > > > > > > > > > > > <V.Sethi@nxp.com>; Udit Kumar

> > > > > > > > > > > > > <udit.kumar@nxp.com>; Bhupesh Sharma

> > > > > > > > > > > > > <bhsharma@redhat.com>; linux-efi

> > > > > > > > > > > > > <linux-efi@vger.kernel.org>

> > > > > > > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi:

> > > > > > > > > > > > > install new fdt in configuration table

> > > > > > > > > > > > >

> > > > > > > > > > > > > On Tue, 11 Dec 2018 at 10:23, Pankaj Bansal

> > > > > > > > > > > > > <pankaj.bansal@nxp.com>

> > > > > > > > > > > wrote:

> > > > > > > > > > > > > >

> > > > > > > > > > > > > >

> > > > > > > > > > > > > >

> > > > > > > > > > > > > > > -----Original Message-----

> > > > > > > > > > > > > > > From: Ard Biesheuvel

> > > > > > > > > > > > > > > [mailto:ard.biesheuvel@linaro.org]

> > > > > > > > > > > > > > > Sent: Tuesday, December 11, 2018 2:48 PM

> > > > > > > > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > > > > > > > > > Cc: Mark Rutland <mark.rutland@arm.com>;

> > > > > > > > > > > > > > > Leif Lindholm <leif.lindholm@linaro.org>;

> > > > > > > > > > > > > > > Grant Likely <grant.likely@arm.com>; Varun

> > > > > > > > > > > > > > > Sethi <V.Sethi@nxp.com>; Udit Kumar

> > > > > > > > > > > > > > > <udit.kumar@nxp.com>; Bhupesh Sharma

> > > > > > > > > > > > > > > <bhsharma@redhat.com>; linux-efi

> > > > > > > > > > > > > > > <linux-efi@vger.kernel.org>

> > > > > > > > > > > > > > > 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

> > > > > > > > > > > > > > > <pankaj.bansal@nxp.com>

> > > > > > > > > > > > > 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

> > > > > > > > > > > > > > > > <ard.biesheuvel@linaro.org>

> > > > > > > > > > > > > > > > Cc: linux-efi@vger.kernel.org

> > > > > > > > > > > > > > > > Signed-off-by: Pankaj Bansal

> > > > > > > > > > > > > > > > <pankaj.bansal@nxp.com>

> > > > > > > > > > > > > > >

> > > > > > > > > > > > > > > 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.

> > > > > > > > > > > > > >

> > > > > > > > > > > > >

> > > > > > > > > > > > > That doesn't matter. The kernel will not use

> > > > > > > > > > > > > devices from the DT before

> > > > > > > > > > > > > ExitBootServices() anyway.

> > > > > > > > > > > >

> > > > > > > > > > > > But what is the indication uefi driver has to

> > > > > > > > > > > > "relieve the device and restore it

> > > > > > > > > > > because now kernel need to use it"?

> > > > > > > > > > >

> > > > > > > > > > > That is ExitBootServices().

> > > > > > > > > > >

> > > > > > > > > > > > The kernel is just like any other UEFI application

> > > > > > > > > > > > to uefi firmware. How will uefi

> > > > > > > > > > > firmware know when to relinquish control?

> > > > > > > > > > >

> > > > > > > > > > > At ExitBootServices() time. Any modification you

> > > > > > > > > > > make to the device tree can be made beforehand,

> > > > > > > > > >

> > > > > > > > > > When you say this, you mean before the

> > > > > > > > > > ExitBootServices() is called

> > > ?

> > > > > > > > > > If yes, then when? Because before ExitBootServices(),

> > > > > > > > > > the firmware is using

> > > > > > > > > the devices and they are not ready  for kernel to use.

> > > > > > > > > > Before ExitBootServices, there is no other event that

> > > > > > > > > > indicates that now kernel

> > > > > > > > > is going to boot.

> > > > > > > > > > We fix the status of devices in device tree, only when

> > > > > > > > > > firmware has released

> > > > > > > > > the control and restored the devices for kernel to use.

> > > > > > > > > > If for some reason, the firmware is not able to do so,

> > > > > > > > > > it disables the device in

> > > > > > > > > device tree.

> > > > > > > > > >

> > > > > > > > > > > since the kernel does not even read the device tree

> > > > > > > > > > > to discover devices until after the stub terminates.

> > > > > > > > > >

> > > > > > > > > > Yes, but the stub has already copied the device tree

> > > > > > > > > > into new location. So, any fixups done on device tree

> > > > > > > > > > in configuration table would not

> > > > > > > > > reflect in device tree that kernel would use.

> > > > > > > > > >

> > > > > > > > > > This is why I am registering the new device tree in

> > > > > > > > > > configuration table. BUT only if it was generated from

> > > > > > > > > > device tree already present in configuration table

> > > > > > > > >

> > > > > > > > > The firmware should make changes to the DT before it

> > > > > > > > > supplies it to the

> > > > > OS.

> > > > > > > >

> > > > > > > > The point is that for uefi "supplying DT to OS" is not

> > > > > > > > something that firmware

> > > > > > > can control.

> > > > > > > > Firmware is just running an efi application. It may be OS,

> > > > > > > > but firmware doesn't

> > > > > > > know that.

> > > > > > > > So, firmware cannot release control of the devices beforehand.

> > > > > > >

> > > > > > > The firmware does not have to release control of the devices

> > > > > > > before enabling them in the DT.

> > > > > >

> > > > > > Actually I am not talking about all the devices that uefi

> > > > > > firmware manages. I am talking about some devices that are

> > > > > > specific to NXP platforms. We have some devices in our

> > > > > > platform, that operate differently in uefi firmware than in

> > > > > > kernel. So if we were to use these devices in

> > > > > kernel, we need to release the firmware control over them and

> > > > > prep these for kernel.

> > > > > > If the devices are made ready for kernel to use, then we

> > > > > > enable these in fdt

> > > > > otherwise disable these.

> > > > > >

> > > > >

> > > > > i understand that.

> > > > >

> > > > > But you fail to explain why those DT changes cannot be made

> > > > > while the firmware is still using those devices. The kernel will

> > > > > not look at the DT until after

> > > > > ExitBootServices() so why is it a problem to enable them in the DT

> earlier?

> > > >

> > > > What if we were not able to prep these devices for kernel? How

> > > > will we disable

> > > these devices?

> > >

> > > Ah.

> > >

> > > That is a completely different matter.

> > >

> > > So why would you be unable to prep a device for the kernel, and why

> > > would the correct course of action be to remove it from the DT as if it no

> longer exists?

> >

> > Actually the preparation of these devices depends on certain parameter files

> that user supplies.

> 

> Where are these parameters present during the boot time and do you mean

> here that the default dtb passed during the boot time needs to be modified by

> the kernel or boot firmware here?


These parameters are present in Non-volatile flash memory.
Boot firmware needs to modify the dtb passed during boot time in exit_boot_services routine.

> 

> > These parameters are not used by firmware but are used by kernel.

> > These parameters need to match to the boot time configuration of platform.

> > That is why these parameters are applied by boot firmware and not by kernel.

> > If there is a mismatch between the boot time configuration and parameters,

> then the parameters would not be applied.

> > In this case kernel is not able to use the devices.

> > Which is why we disable this particular devices, so that rest of the system can

> boot.

> > Otherwise the device driver in kernel misbehaves.

> 

> Can you please share an example, as the above description is not very clear to

> me. May be you can include a dt property that you are trying to fix via kernel and

> what happens in the kernel driver when it is not in a expected state.


We already do. The "status = "okay";" or "status = "disabled";" is added to the device node in dts file.
Based on this the device structure is created or not created in kernel when booting.

> 

> Also may be you can share why the boot firmware is not able to set a correct

> state of the same.


The correct state of device would depend on the user supplied parameters and boot time configuration.
Boot firmware is able to set the "status" in fdt file in exit boot services.
BUT the point is that it will not get reflected in kernel, because efi stub has copied the fdt file already.

> 

> Thanks,

> Bhupesh
Ard Biesheuvel - Dec. 11, 2018, 12:25 p.m.
On Tue, 11 Dec 2018 at 13:22, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
> > -----Original Message-----
> > From: Bhupesh Sharma [mailto:bhsharma@redhat.com]
> > Sent: Tuesday, December 11, 2018 4:25 PM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Mark Rutland
> > <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>; Grant
> > Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit Kumar
> > <udit.kumar@nxp.com>; linux-efi@vger.kernel.org
> > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in
> > configuration table
> >
> > Hi Pankaj,
> >
..
> > Can you please share an example, as the above description is not very clear to
> > me. May be you can include a dt property that you are trying to fix via kernel and
> > what happens in the kernel driver when it is not in a expected state.
>
> We already do. The "status = "okay";" or "status = "disabled";" is added to the device node in dts file.
> Based on this the device structure is created or not created in kernel when booting.
>
> >
> > Also may be you can share why the boot firmware is not able to set a correct
> > state of the same.
>
> The correct state of device would depend on the user supplied parameters and boot time configuration.
> Boot firmware is able to set the "status" in fdt file in exit boot services.

But why not before? Why does it have to wait until ExitBootServices()
to do this?
Pankaj Bansal - Dec. 11, 2018, 12:29 p.m.
> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Tuesday, December 11, 2018 5:55 PM

> To: Pankaj Bansal <pankaj.bansal@nxp.com>

> Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland

> <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>; Grant

> Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit Kumar

> <udit.kumar@nxp.com>; linux-efi <linux-efi@vger.kernel.org>

> Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in

> configuration table

> 

> On Tue, 11 Dec 2018 at 13:22, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:

> > > -----Original Message-----

> > > From: Bhupesh Sharma [mailto:bhsharma@redhat.com]

> > > Sent: Tuesday, December 11, 2018 4:25 PM

> > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Mark Rutland

> > > <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>;

> > > Grant Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>;

> > > Udit Kumar <udit.kumar@nxp.com>; linux-efi@vger.kernel.org

> > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt

> > > in configuration table

> > >

> > > Hi Pankaj,

> > >

> ..

> > > Can you please share an example, as the above description is not

> > > very clear to me. May be you can include a dt property that you are

> > > trying to fix via kernel and what happens in the kernel driver when it is not in

> a expected state.

> >

> > We already do. The "status = "okay";" or "status = "disabled";" is added to the

> device node in dts file.

> > Based on this the device structure is created or not created in kernel when

> booting.

> >

> > >

> > > Also may be you can share why the boot firmware is not able to set a

> > > correct state of the same.

> >

> > The correct state of device would depend on the user supplied parameters and

> boot time configuration.

> > Boot firmware is able to set the "status" in fdt file in exit boot services.

> 

> But why not before? Why does it have to wait until ExitBootServices() to do this?


We attempt to apply the user supplied parameters in ExitBootServices.
If it fails, then the device state is un deterministic. If it passed, then device can be used in kernel.
Once there parameters are applied, regardless of they failed or passed, the boot firmware cannot use the device.
So we have no choice but to apply these parameters when we no longer wish to use the device in boot firmware.
Ard Biesheuvel - Dec. 11, 2018, 12:31 p.m.
On Tue, 11 Dec 2018 at 13:29, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Tuesday, December 11, 2018 5:55 PM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland
> > <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>; Grant
> > Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit Kumar
> > <udit.kumar@nxp.com>; linux-efi <linux-efi@vger.kernel.org>
> > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in
> > configuration table
> >
> > On Tue, 11 Dec 2018 at 13:22, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
> > > > -----Original Message-----
> > > > From: Bhupesh Sharma [mailto:bhsharma@redhat.com]
> > > > Sent: Tuesday, December 11, 2018 4:25 PM
> > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Mark Rutland
> > > > <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>;
> > > > Grant Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>;
> > > > Udit Kumar <udit.kumar@nxp.com>; linux-efi@vger.kernel.org
> > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt
> > > > in configuration table
> > > >
> > > > Hi Pankaj,
> > > >
> > ..
> > > > Can you please share an example, as the above description is not
> > > > very clear to me. May be you can include a dt property that you are
> > > > trying to fix via kernel and what happens in the kernel driver when it is not in
> > a expected state.
> > >
> > > We already do. The "status = "okay";" or "status = "disabled";" is added to the
> > device node in dts file.
> > > Based on this the device structure is created or not created in kernel when
> > booting.
> > >
> > > >
> > > > Also may be you can share why the boot firmware is not able to set a
> > > > correct state of the same.
> > >
> > > The correct state of device would depend on the user supplied parameters and
> > boot time configuration.
> > > Boot firmware is able to set the "status" in fdt file in exit boot services.
> >
> > But why not before? Why does it have to wait until ExitBootServices() to do this?
>
> We attempt to apply the user supplied parameters in ExitBootServices.

What does 'user supplied' mean? And why can't you apply them earlier?

> If it fails, then the device state is un deterministic. If it passed, then device can be used in kernel.
> Once there parameters are applied, regardless of they failed or passed, the boot firmware cannot use the device.
> So we have no choice but to apply these parameters when we no longer wish to use the device in boot firmware.
>

This is incorrect. Setting the DT status property does absolutely
nothing until long after ExitBootServices() completes. So if you want
to set the device status, you need to do it before invoking the
kernel.
Pankaj Bansal - Dec. 11, 2018, 12:44 p.m.
> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Tuesday, December 11, 2018 6:02 PM

> To: Pankaj Bansal <pankaj.bansal@nxp.com>

> Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland

> <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>; Grant

> Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit Kumar

> <udit.kumar@nxp.com>; linux-efi <linux-efi@vger.kernel.org>

> Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in

> configuration table

> 

> On Tue, 11 Dec 2018 at 13:29, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:

> >

> >

> >

> > > -----Original Message-----

> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > Sent: Tuesday, December 11, 2018 5:55 PM

> > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland

> > > <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>;

> > > Grant Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>;

> > > Udit Kumar <udit.kumar@nxp.com>; linux-efi

> > > <linux-efi@vger.kernel.org>

> > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt

> > > in configuration table

> > >

> > > On Tue, 11 Dec 2018 at 13:22, Pankaj Bansal <pankaj.bansal@nxp.com>

> wrote:

> > > > > -----Original Message-----

> > > > > From: Bhupesh Sharma [mailto:bhsharma@redhat.com]

> > > > > Sent: Tuesday, December 11, 2018 4:25 PM

> > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Mark Rutland

> > > > > <mark.rutland@arm.com>; Leif Lindholm

> > > > > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>;

> > > > > Varun Sethi <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;

> > > > > linux-efi@vger.kernel.org

> > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new

> > > > > fdt in configuration table

> > > > >

> > > > > Hi Pankaj,

> > > > >

> > > ..

> > > > > Can you please share an example, as the above description is not

> > > > > very clear to me. May be you can include a dt property that you

> > > > > are trying to fix via kernel and what happens in the kernel

> > > > > driver when it is not in

> > > a expected state.

> > > >

> > > > We already do. The "status = "okay";" or "status = "disabled";" is

> > > > added to the

> > > device node in dts file.

> > > > Based on this the device structure is created or not created in

> > > > kernel when

> > > booting.

> > > >

> > > > >

> > > > > Also may be you can share why the boot firmware is not able to

> > > > > set a correct state of the same.

> > > >

> > > > The correct state of device would depend on the user supplied

> > > > parameters and

> > > boot time configuration.

> > > > Boot firmware is able to set the "status" in fdt file in exit boot services.

> > >

> > > But why not before? Why does it have to wait until ExitBootServices() to do

> this?

> >

> > We attempt to apply the user supplied parameters in ExitBootServices.

> 

> What does 'user supplied' mean? And why can't you apply them earlier?


The parameters are not part of uefi firmware. There is separate binary file that
the uefi firmware copies from Nonvolatile flash memory and applied to device.
As I have already said, if we apply them earlier, the boot firmware would not be
able to use these devices. While we want to use these devices in uefi firmware.

> 

> > If it fails, then the device state is un deterministic. If it passed, then device can

> be used in kernel.

> > Once there parameters are applied, regardless of they failed or passed, the

> boot firmware cannot use the device.

> > So we have no choice but to apply these parameters when we no longer wish

> to use the device in boot firmware.

> >

> 

> This is incorrect. Setting the DT status property does absolutely nothing until

> long after ExitBootServices() completes. So if you want to set the device status,

> you need to do it before invoking the kernel.


As I have said before, how do we determine "we have invoked kernel or we have invoked any other efi application" ?
We can hit the scenario where
1. we fetched the efi images (from tftp or from fat partition etc)
2. we applied the parameters and modified the dts file.
3. we started the efi image.
4. The efi image was NOT kernel image but a efi driver. (say hello world)
5. we are back in uefi firmware, but now we can't use the device. !!! Big problem

How do I solve this?
Ard Biesheuvel - Dec. 11, 2018, 12:47 p.m.
On Tue, 11 Dec 2018 at 13:44, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Tuesday, December 11, 2018 6:02 PM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland
> > <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>; Grant
> > Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit Kumar
> > <udit.kumar@nxp.com>; linux-efi <linux-efi@vger.kernel.org>
> > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in
> > configuration table
> >
> > On Tue, 11 Dec 2018 at 13:29, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > Sent: Tuesday, December 11, 2018 5:55 PM
> > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland
> > > > <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>;
> > > > Grant Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>;
> > > > Udit Kumar <udit.kumar@nxp.com>; linux-efi
> > > > <linux-efi@vger.kernel.org>
> > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt
> > > > in configuration table
> > > >
> > > > On Tue, 11 Dec 2018 at 13:22, Pankaj Bansal <pankaj.bansal@nxp.com>
> > wrote:
> > > > > > -----Original Message-----
> > > > > > From: Bhupesh Sharma [mailto:bhsharma@redhat.com]
> > > > > > Sent: Tuesday, December 11, 2018 4:25 PM
> > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Mark Rutland
> > > > > > <mark.rutland@arm.com>; Leif Lindholm
> > > > > > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>;
> > > > > > Varun Sethi <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;
> > > > > > linux-efi@vger.kernel.org
> > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new
> > > > > > fdt in configuration table
> > > > > >
> > > > > > Hi Pankaj,
> > > > > >
> > > > ..
> > > > > > Can you please share an example, as the above description is not
> > > > > > very clear to me. May be you can include a dt property that you
> > > > > > are trying to fix via kernel and what happens in the kernel
> > > > > > driver when it is not in
> > > > a expected state.
> > > > >
> > > > > We already do. The "status = "okay";" or "status = "disabled";" is
> > > > > added to the
> > > > device node in dts file.
> > > > > Based on this the device structure is created or not created in
> > > > > kernel when
> > > > booting.
> > > > >
> > > > > >
> > > > > > Also may be you can share why the boot firmware is not able to
> > > > > > set a correct state of the same.
> > > > >
> > > > > The correct state of device would depend on the user supplied
> > > > > parameters and
> > > > boot time configuration.
> > > > > Boot firmware is able to set the "status" in fdt file in exit boot services.
> > > >
> > > > But why not before? Why does it have to wait until ExitBootServices() to do
> > this?
> > >
> > > We attempt to apply the user supplied parameters in ExitBootServices.
> >
> > What does 'user supplied' mean? And why can't you apply them earlier?
>
> The parameters are not part of uefi firmware. There is separate binary file that
> the uefi firmware copies from Nonvolatile flash memory and applied to device.
> As I have already said, if we apply them earlier, the boot firmware would not be
> able to use these devices. While we want to use these devices in uefi firmware.
>

How does updating the DT 'status' field prevent you from using the
device in UEFI?

> >
> > > If it fails, then the device state is un deterministic. If it passed, then device can
> > be used in kernel.
> > > Once there parameters are applied, regardless of they failed or passed, the
> > boot firmware cannot use the device.
> > > So we have no choice but to apply these parameters when we no longer wish
> > to use the device in boot firmware.
> > >
> >
> > This is incorrect. Setting the DT status property does absolutely nothing until
> > long after ExitBootServices() completes. So if you want to set the device status,
> > you need to do it before invoking the kernel.
>
> As I have said before, how do we determine "we have invoked kernel or we have invoked any other efi application" ?
> We can hit the scenario where
> 1. we fetched the efi images (from tftp or from fat partition etc)
> 2. we applied the parameters and modified the dts file.
> 3. we started the efi image.
> 4. The efi image was NOT kernel image but a efi driver. (say hello world)
> 5. we are back in uefi firmware, but now we can't use the device. !!! Big problem
>
> How do I solve this?

Why is it not possible to use the device now? Please describe
*exactly* how updating the DT status property results in the device no
longer being usable in the firmware.
Pankaj Bansal - Dec. 11, 2018, 12:55 p.m.
> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Tuesday, December 11, 2018 6:18 PM

> To: Pankaj Bansal <pankaj.bansal@nxp.com>

> Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland

> <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>; Grant

> Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit Kumar

> <udit.kumar@nxp.com>; linux-efi <linux-efi@vger.kernel.org>

> Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in

> configuration table

> 

> On Tue, 11 Dec 2018 at 13:44, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:

> >

> >

> >

> > > -----Original Message-----

> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > Sent: Tuesday, December 11, 2018 6:02 PM

> > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland

> > > <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>;

> > > Grant Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>;

> > > Udit Kumar <udit.kumar@nxp.com>; linux-efi

> > > <linux-efi@vger.kernel.org>

> > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt

> > > in configuration table

> > >

> > > On Tue, 11 Dec 2018 at 13:29, Pankaj Bansal <pankaj.bansal@nxp.com>

> wrote:

> > > >

> > > >

> > > >

> > > > > -----Original Message-----

> > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > > Sent: Tuesday, December 11, 2018 5:55 PM

> > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland

> > > > > <mark.rutland@arm.com>; Leif Lindholm

> > > > > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>;

> > > > > Varun Sethi <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;

> > > > > linux-efi <linux-efi@vger.kernel.org>

> > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new

> > > > > fdt in configuration table

> > > > >

> > > > > On Tue, 11 Dec 2018 at 13:22, Pankaj Bansal

> > > > > <pankaj.bansal@nxp.com>

> > > wrote:

> > > > > > > -----Original Message-----

> > > > > > > From: Bhupesh Sharma [mailto:bhsharma@redhat.com]

> > > > > > > Sent: Tuesday, December 11, 2018 4:25 PM

> > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Mark Rutland

> > > > > > > <mark.rutland@arm.com>; Leif Lindholm

> > > > > > > <leif.lindholm@linaro.org>; Grant Likely

> > > > > > > <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit

> > > > > > > Kumar <udit.kumar@nxp.com>; linux-efi@vger.kernel.org

> > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install

> > > > > > > new fdt in configuration table

> > > > > > >

> > > > > > > Hi Pankaj,

> > > > > > >

> > > > > ..

> > > > > > > Can you please share an example, as the above description is

> > > > > > > not very clear to me. May be you can include a dt property

> > > > > > > that you are trying to fix via kernel and what happens in

> > > > > > > the kernel driver when it is not in

> > > > > a expected state.

> > > > > >

> > > > > > We already do. The "status = "okay";" or "status =

> > > > > > "disabled";" is added to the

> > > > > device node in dts file.

> > > > > > Based on this the device structure is created or not created

> > > > > > in kernel when

> > > > > booting.

> > > > > >

> > > > > > >

> > > > > > > Also may be you can share why the boot firmware is not able

> > > > > > > to set a correct state of the same.

> > > > > >

> > > > > > The correct state of device would depend on the user supplied

> > > > > > parameters and

> > > > > boot time configuration.

> > > > > > Boot firmware is able to set the "status" in fdt file in exit boot services.

> > > > >

> > > > > But why not before? Why does it have to wait until

> > > > > ExitBootServices() to do

> > > this?

> > > >

> > > > We attempt to apply the user supplied parameters in ExitBootServices.

> > >

> > > What does 'user supplied' mean? And why can't you apply them earlier?

> >

> > The parameters are not part of uefi firmware. There is separate binary

> > file that the uefi firmware copies from Nonvolatile flash memory and applied

> to device.

> > As I have already said, if we apply them earlier, the boot firmware

> > would not be able to use these devices. While we want to use these devices in

> uefi firmware.

> >

> 

> How does updating the DT 'status' field prevent you from using the device in

> UEFI?


It's not about just changing the status in dts file. Once the device is prepped for kernel
(i.e. the user supplied parameters are applied to it), the device behaves in different manner.
It behaves in interrupt based mechanism, which we don't have in UEFI.

We can use only polling based mechanism in UEFI.

> 

> > >

> > > > If it fails, then the device state is un deterministic. If it

> > > > passed, then device can

> > > be used in kernel.

> > > > Once there parameters are applied, regardless of they failed or

> > > > passed, the

> > > boot firmware cannot use the device.

> > > > So we have no choice but to apply these parameters when we no

> > > > longer wish

> > > to use the device in boot firmware.

> > > >

> > >

> > > This is incorrect. Setting the DT status property does absolutely

> > > nothing until long after ExitBootServices() completes. So if you

> > > want to set the device status, you need to do it before invoking the kernel.

> >

> > As I have said before, how do we determine "we have invoked kernel or we

> have invoked any other efi application" ?

> > We can hit the scenario where

> > 1. we fetched the efi images (from tftp or from fat partition etc) 2.

> > we applied the parameters and modified the dts file.

> > 3. we started the efi image.

> > 4. The efi image was NOT kernel image but a efi driver. (say hello

> > world) 5. we are back in uefi firmware, but now we can't use the

> > device. !!! Big problem

> >

> > How do I solve this?

> 

> Why is it not possible to use the device now? Please describe

> *exactly* how updating the DT status property results in the device no longer

> being usable in the firmware.


It's not about just changing the status in dts file. Once the device is prepped for kernel
(i.e. the user supplied parameters are applied to it), the device behaves in different manner.
It behaves in interrupt based mechanism, which we don't have in UEFI.

We can use only polling based mechanism in UEFI.

I am not able to understand the reservation about this patch.
At least in earlier version, the apprehension was that if dtb supplied by kernel (using command line parameters)
Is supplied to firmware, it may break firmware, as firmware might not understand the bindings in it.
But that problem should not come with these changes.
Ard Biesheuvel - Dec. 11, 2018, 1:01 p.m.
On Tue, 11 Dec 2018 at 13:55, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Tuesday, December 11, 2018 6:18 PM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland
> > <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>; Grant
> > Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit Kumar
> > <udit.kumar@nxp.com>; linux-efi <linux-efi@vger.kernel.org>
> > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in
> > configuration table
> >
> > On Tue, 11 Dec 2018 at 13:44, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > Sent: Tuesday, December 11, 2018 6:02 PM
> > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland
> > > > <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>;
> > > > Grant Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>;
> > > > Udit Kumar <udit.kumar@nxp.com>; linux-efi
> > > > <linux-efi@vger.kernel.org>
> > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt
> > > > in configuration table
> > > >
> > > > On Tue, 11 Dec 2018 at 13:29, Pankaj Bansal <pankaj.bansal@nxp.com>
> > wrote:
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > > > Sent: Tuesday, December 11, 2018 5:55 PM
> > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland
> > > > > > <mark.rutland@arm.com>; Leif Lindholm
> > > > > > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>;
> > > > > > Varun Sethi <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;
> > > > > > linux-efi <linux-efi@vger.kernel.org>
> > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new
> > > > > > fdt in configuration table
> > > > > >
> > > > > > On Tue, 11 Dec 2018 at 13:22, Pankaj Bansal
> > > > > > <pankaj.bansal@nxp.com>
> > > > wrote:
> > > > > > > > -----Original Message-----
> > > > > > > > From: Bhupesh Sharma [mailto:bhsharma@redhat.com]
> > > > > > > > Sent: Tuesday, December 11, 2018 4:25 PM
> > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Mark Rutland
> > > > > > > > <mark.rutland@arm.com>; Leif Lindholm
> > > > > > > > <leif.lindholm@linaro.org>; Grant Likely
> > > > > > > > <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit
> > > > > > > > Kumar <udit.kumar@nxp.com>; linux-efi@vger.kernel.org
> > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install
> > > > > > > > new fdt in configuration table
> > > > > > > >
> > > > > > > > Hi Pankaj,
> > > > > > > >
> > > > > > ..
> > > > > > > > Can you please share an example, as the above description is
> > > > > > > > not very clear to me. May be you can include a dt property
> > > > > > > > that you are trying to fix via kernel and what happens in
> > > > > > > > the kernel driver when it is not in
> > > > > > a expected state.
> > > > > > >
> > > > > > > We already do. The "status = "okay";" or "status =
> > > > > > > "disabled";" is added to the
> > > > > > device node in dts file.
> > > > > > > Based on this the device structure is created or not created
> > > > > > > in kernel when
> > > > > > booting.
> > > > > > >
> > > > > > > >
> > > > > > > > Also may be you can share why the boot firmware is not able
> > > > > > > > to set a correct state of the same.
> > > > > > >
> > > > > > > The correct state of device would depend on the user supplied
> > > > > > > parameters and
> > > > > > boot time configuration.
> > > > > > > Boot firmware is able to set the "status" in fdt file in exit boot services.
> > > > > >
> > > > > > But why not before? Why does it have to wait until
> > > > > > ExitBootServices() to do
> > > > this?
> > > > >
> > > > > We attempt to apply the user supplied parameters in ExitBootServices.
> > > >
> > > > What does 'user supplied' mean? And why can't you apply them earlier?
> > >
> > > The parameters are not part of uefi firmware. There is separate binary
> > > file that the uefi firmware copies from Nonvolatile flash memory and applied
> > to device.
> > > As I have already said, if we apply them earlier, the boot firmware
> > > would not be able to use these devices. While we want to use these devices in
> > uefi firmware.
> > >
> >
> > How does updating the DT 'status' field prevent you from using the device in
> > UEFI?
>
> It's not about just changing the status in dts file. Once the device is prepped for kernel
> (i.e. the user supplied parameters are applied to it), the device behaves in different manner.
> It behaves in interrupt based mechanism, which we don't have in UEFI.
>
> We can use only polling based mechanism in UEFI.
>
> >
> > > >
> > > > > If it fails, then the device state is un deterministic. If it
> > > > > passed, then device can
> > > > be used in kernel.
> > > > > Once there parameters are applied, regardless of they failed or
> > > > > passed, the
> > > > boot firmware cannot use the device.
> > > > > So we have no choice but to apply these parameters when we no
> > > > > longer wish
> > > > to use the device in boot firmware.
> > > > >
> > > >
> > > > This is incorrect. Setting the DT status property does absolutely
> > > > nothing until long after ExitBootServices() completes. So if you
> > > > want to set the device status, you need to do it before invoking the kernel.
> > >
> > > As I have said before, how do we determine "we have invoked kernel or we
> > have invoked any other efi application" ?
> > > We can hit the scenario where
> > > 1. we fetched the efi images (from tftp or from fat partition etc) 2.
> > > we applied the parameters and modified the dts file.
> > > 3. we started the efi image.
> > > 4. The efi image was NOT kernel image but a efi driver. (say hello
> > > world) 5. we are back in uefi firmware, but now we can't use the
> > > device. !!! Big problem
> > >
> > > How do I solve this?
> >
> > Why is it not possible to use the device now? Please describe
> > *exactly* how updating the DT status property results in the device no longer
> > being usable in the firmware.
>
> It's not about just changing the status in dts file. Once the device is prepped for kernel
> (i.e. the user supplied parameters are applied to it), the device behaves in different manner.
> It behaves in interrupt based mechanism, which we don't have in UEFI.
>

OK, this sounds like an action that is appropriate in the context of
ExitBootServices()

> We can use only polling based mechanism in UEFI.
>
> I am not able to understand the reservation about this patch.
> At least in earlier version, the apprehension was that if dtb supplied by kernel (using command line parameters)
> Is supplied to firmware, it may break firmware, as firmware might not understand the bindings in it.
> But that problem should not come with these changes.
>

Passing back a device tree to the firmware is the problem. This
introduces dependencies by the firmware on actions taken by the
kernel, which limits our future ability to make changes to it, since
it might then break your platform.

So I am not going to apply this patch. If the device may end up in a
funny state, please update the OS driver to be robust against that,
which sounds like a good idea in any case if this is such a likely
occurrence.
Pankaj Bansal - Dec. 11, 2018, 1:04 p.m.
> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Tuesday, December 11, 2018 6:31 PM

> To: Pankaj Bansal <pankaj.bansal@nxp.com>

> Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland

> <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>; Grant

> Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit Kumar

> <udit.kumar@nxp.com>; linux-efi <linux-efi@vger.kernel.org>

> Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in

> configuration table

> 

> On Tue, 11 Dec 2018 at 13:55, Pankaj Bansal <pankaj.bansal@nxp.com> wrote:

> >

> >

> >

> > > -----Original Message-----

> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > Sent: Tuesday, December 11, 2018 6:18 PM

> > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland

> > > <mark.rutland@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>;

> > > Grant Likely <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>;

> > > Udit Kumar <udit.kumar@nxp.com>; linux-efi

> > > <linux-efi@vger.kernel.org>

> > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt

> > > in configuration table

> > >

> > > On Tue, 11 Dec 2018 at 13:44, Pankaj Bansal <pankaj.bansal@nxp.com>

> wrote:

> > > >

> > > >

> > > >

> > > > > -----Original Message-----

> > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > > Sent: Tuesday, December 11, 2018 6:02 PM

> > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland

> > > > > <mark.rutland@arm.com>; Leif Lindholm

> > > > > <leif.lindholm@linaro.org>; Grant Likely <grant.likely@arm.com>;

> > > > > Varun Sethi <V.Sethi@nxp.com>; Udit Kumar <udit.kumar@nxp.com>;

> > > > > linux-efi <linux-efi@vger.kernel.org>

> > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new

> > > > > fdt in configuration table

> > > > >

> > > > > On Tue, 11 Dec 2018 at 13:29, Pankaj Bansal

> > > > > <pankaj.bansal@nxp.com>

> > > wrote:

> > > > > >

> > > > > >

> > > > > >

> > > > > > > -----Original Message-----

> > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > > > > Sent: Tuesday, December 11, 2018 5:55 PM

> > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > Cc: Bhupesh Sharma <bhsharma@redhat.com>; Mark Rutland

> > > > > > > <mark.rutland@arm.com>; Leif Lindholm

> > > > > > > <leif.lindholm@linaro.org>; Grant Likely

> > > > > > > <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Udit

> > > > > > > Kumar <udit.kumar@nxp.com>; linux-efi

> > > > > > > <linux-efi@vger.kernel.org>

> > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install

> > > > > > > new fdt in configuration table

> > > > > > >

> > > > > > > On Tue, 11 Dec 2018 at 13:22, Pankaj Bansal

> > > > > > > <pankaj.bansal@nxp.com>

> > > > > wrote:

> > > > > > > > > -----Original Message-----

> > > > > > > > > From: Bhupesh Sharma [mailto:bhsharma@redhat.com]

> > > > > > > > > Sent: Tuesday, December 11, 2018 4:25 PM

> > > > > > > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>

> > > > > > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Mark

> > > > > > > > > Rutland <mark.rutland@arm.com>; Leif Lindholm

> > > > > > > > > <leif.lindholm@linaro.org>; Grant Likely

> > > > > > > > > <grant.likely@arm.com>; Varun Sethi <V.Sethi@nxp.com>;

> > > > > > > > > Udit Kumar <udit.kumar@nxp.com>;

> > > > > > > > > linux-efi@vger.kernel.org

> > > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi:

> > > > > > > > > install new fdt in configuration table

> > > > > > > > >

> > > > > > > > > Hi Pankaj,

> > > > > > > > >

> > > > > > > ..

> > > > > > > > > Can you please share an example, as the above

> > > > > > > > > description is not very clear to me. May be you can

> > > > > > > > > include a dt property that you are trying to fix via

> > > > > > > > > kernel and what happens in the kernel driver when it is

> > > > > > > > > not in

> > > > > > > a expected state.

> > > > > > > >

> > > > > > > > We already do. The "status = "okay";" or "status =

> > > > > > > > "disabled";" is added to the

> > > > > > > device node in dts file.

> > > > > > > > Based on this the device structure is created or not

> > > > > > > > created in kernel when

> > > > > > > booting.

> > > > > > > >

> > > > > > > > >

> > > > > > > > > Also may be you can share why the boot firmware is not

> > > > > > > > > able to set a correct state of the same.

> > > > > > > >

> > > > > > > > The correct state of device would depend on the user

> > > > > > > > supplied parameters and

> > > > > > > boot time configuration.

> > > > > > > > Boot firmware is able to set the "status" in fdt file in exit boot

> services.

> > > > > > >

> > > > > > > But why not before? Why does it have to wait until

> > > > > > > ExitBootServices() to do

> > > > > this?

> > > > > >

> > > > > > We attempt to apply the user supplied parameters in ExitBootServices.

> > > > >

> > > > > What does 'user supplied' mean? And why can't you apply them earlier?

> > > >

> > > > The parameters are not part of uefi firmware. There is separate

> > > > binary file that the uefi firmware copies from Nonvolatile flash

> > > > memory and applied

> > > to device.

> > > > As I have already said, if we apply them earlier, the boot

> > > > firmware would not be able to use these devices. While we want to

> > > > use these devices in

> > > uefi firmware.

> > > >

> > >

> > > How does updating the DT 'status' field prevent you from using the

> > > device in UEFI?

> >

> > It's not about just changing the status in dts file. Once the device

> > is prepped for kernel (i.e. the user supplied parameters are applied to it), the

> device behaves in different manner.

> > It behaves in interrupt based mechanism, which we don't have in UEFI.

> >

> > We can use only polling based mechanism in UEFI.

> >

> > >

> > > > >

> > > > > > If it fails, then the device state is un deterministic. If it

> > > > > > passed, then device can

> > > > > be used in kernel.

> > > > > > Once there parameters are applied, regardless of they failed

> > > > > > or passed, the

> > > > > boot firmware cannot use the device.

> > > > > > So we have no choice but to apply these parameters when we no

> > > > > > longer wish

> > > > > to use the device in boot firmware.

> > > > > >

> > > > >

> > > > > This is incorrect. Setting the DT status property does

> > > > > absolutely nothing until long after ExitBootServices()

> > > > > completes. So if you want to set the device status, you need to do it

> before invoking the kernel.

> > > >

> > > > As I have said before, how do we determine "we have invoked kernel

> > > > or we

> > > have invoked any other efi application" ?

> > > > We can hit the scenario where

> > > > 1. we fetched the efi images (from tftp or from fat partition etc) 2.

> > > > we applied the parameters and modified the dts file.

> > > > 3. we started the efi image.

> > > > 4. The efi image was NOT kernel image but a efi driver. (say hello

> > > > world) 5. we are back in uefi firmware, but now we can't use the

> > > > device. !!! Big problem

> > > >

> > > > How do I solve this?

> > >

> > > Why is it not possible to use the device now? Please describe

> > > *exactly* how updating the DT status property results in the device

> > > no longer being usable in the firmware.

> >

> > It's not about just changing the status in dts file. Once the device

> > is prepped for kernel (i.e. the user supplied parameters are applied to it), the

> device behaves in different manner.

> > It behaves in interrupt based mechanism, which we don't have in UEFI.

> >

> 

> OK, this sounds like an action that is appropriate in the context of

> ExitBootServices()

> 

> > We can use only polling based mechanism in UEFI.

> >

> > I am not able to understand the reservation about this patch.

> > At least in earlier version, the apprehension was that if dtb supplied

> > by kernel (using command line parameters) Is supplied to firmware, it may

> break firmware, as firmware might not understand the bindings in it.

> > But that problem should not come with these changes.

> >

> 

> Passing back a device tree to the firmware is the problem. This introduces

> dependencies by the firmware on actions taken by the kernel, which limits our

> future ability to make changes to it, since it might then break your platform.

> 


Then can I make changes to efi stub so that exit boot services is called before dtb is read from configuration table?
It will require quite a rejig of efi stub code.

> So I am not going to apply this patch. If the device may end up in a funny state,

> please update the OS driver to be robust against that, which sounds like a good

> idea in any case if this is such a likely occurrence.
Leif Lindholm - Dec. 11, 2018, 1:17 p.m.
On Tue, Dec 11, 2018 at 12:55:39PM +0000, Pankaj Bansal wrote:
> I am not able to understand the reservation about this patch.

The reservation is that from an outside perspective, this patch set
gives the impression that somewhere deep down the line, a fundamental
misunderstanding exists.

This impression may be completely incorrect, but from the information
presented here, we cannot understand what problem is being solved by
this set. This is why we keep coming with endless follow-up questions.

> At least in earlier version, the apprehension was that if dtb
> supplied by kernel (using command line parameters)
> Is supplied to firmware, it may break firmware, as firmware might
> not understand the bindings in it.

Then I am afraid there has been some miscommunication. I have re-read
the thread on the original patch set, and my interpretation on the
feedback is completely different. It is referring to creating multiple
layers of stability requirements between kernel and firmware when
dealing with command-line loaded device trees.

The firmware could _never_ be a consumer of a device tree loaded by
the kernel stub.

Best Regards,

Leif

Patch

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;