RE: Uefi runtime property in device tree

2018-12-27 Thread Pankaj Bansal
Hello Ard,

Thanks for replying

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Wednesday, December 26, 2018 7:16 PM
> To: Pankaj Bansal 
> Cc: Varun Sethi ; Udit Kumar ; linux-
> efi ; edk2-de...@lists.01.org
> Subject: Re: Uefi runtime property in device tree
> 
> On Wed, 26 Dec 2018 at 11:15, Pankaj Bansal  wrote:
> >
> > Hello Ard et al.
> >
> > I have a query regarding the device tree usage in UEFI.
> > In our UEFI implementation for NXP SOCs, we are using device tree to detect
> Non discoverable platform devices.
> > Based on the device detected in device tree, a device instance is created 
> > and
> the device’s driver binds to that device’s handle (a DXE driver or an UEFI 
> driver).
> > if the device were to be used for runtime service, then we need to allocate 
> > the
> memory for that device instance from runtime pool and set its virtual address
> using EfiConvertPointer.
> > To facilitate this, I wish to add an optional property to the device node 
> > “uefi-
> runtime”.
> > If this property is present in device tree the UEFI firmware will allocate 
> > the
> data from runtime pool for this device.
> > Also firmware will disable/delete the node in device tree before passing 
> > onto
> OS, so that OS doesn’t use the device.
> >
> > I wish to know your thoughts on this. If this doesn’t seem the right way, I 
> > am
> happy to hear your suggestions.
> >
> 
> Hello Pankaj,
> 
> In general, you are free to do whatever you like in the internal 
> implementation
> of your firmware. You can invent your own DT properties, bindings, etc, or 
> even
> invent your own description language altogether as long as you ensure that the
> descriptions don't leak into places where they are visible to the OS.
> 
> However, I do wonder how generic this has to be. Since the DT and the firmware
> will be tightly coupled in any case, what is preventing you from defining the 
> RTC
> and NOR flash devices as DT device paths in the firmware source, and attaching
> to them directly rather than traversing the device tree looking for 
> uefi_runtime
> nodes.

The UEFI driver model that we are following, creates the device instances for 
all the devices
attached to a controller by parsing DT node and sub nodes of a controller on 
ConnectController call.
So we are not traversing the DT for runtime devices, rather we need to know if 
the device is runtime
or not when parsing the DT node.

Take the case of SPI NOR for example. When a SPI controller is connected, then 
instances for all the NOR flash devices
attached to controller are created. Only one of the NOR flash attached to SPI 
bus is going to be used for runtime service.

> 
> Note that *any* logic that operates on device trees that are provided by the 
> OS
> to the firmware will be rejected. It is the firmware's job to describe the 
> platform
> to the OS, not the other way around.

I have a question : if we want to use DT for DXE phase in firmware and to boot 
OS, should we use a common DT for firmware and OS or separate?

If DT HAS TO BE SAME, then how do we solve this conundrum ? 
https://elinux.org/Device_Tree_Linux#forward_and_backward_dts_compatibility
Current practice is that
•   new kernels work with old devicetrees
•   old kernels may or may not work with new devicetrees
This means that if a binding is modified in a non-compatible manner then the 
kernel implementation must still recognize the old binding until old 
devicetrees have been obsoleted and no longer exist.

i.e. a platform's DT is updated in linux 4.19 and I start to use it in UEFI. 
Then I try to boot linux 4.9 with it and it fails.

Regards,
Pankaj Bansal



Uefi runtime property in device tree

2018-12-26 Thread Pankaj Bansal
Hello Ard et al.

I have a query regarding the device tree usage in UEFI.
In our UEFI implementation for NXP SOCs, we are using device tree to detect Non 
discoverable platform devices.
Based on the device detected in device tree, a device instance is created and 
the device's driver binds to that device's handle (a DXE driver or an UEFI 
driver).
if the device were to be used for runtime service, then we need to allocate the 
memory for that device instance from runtime pool and set its virtual address 
using EfiConvertPointer.
To facilitate this, I wish to add an optional property to the device node 
"uefi-runtime".
If this property is present in device tree the UEFI firmware will allocate the 
data from runtime pool for this device.
Also firmware will disable/delete the node in device tree before passing onto 
OS, so that OS doesn't use the device.

I wish to know your thoughts on this. If this doesn't seem the right way, I am 
happy to hear your suggestions.

Thanks & Regards,
Pankaj Bansal



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

2018-12-11 Thread Pankaj Bansal


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, December 11, 2018 6:31 PM
> To: Pankaj Bansal 
> Cc: Bhupesh Sharma ; Mark Rutland
> ; Leif Lindholm ; Grant
> Likely ; Varun Sethi ; Udit Kumar
> ; linux-efi 
> 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  wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > Sent: Tuesday, December 11, 2018 6:18 PM
> > > To: Pankaj Bansal 
> > > Cc: Bhupesh Sharma ; Mark Rutland
> > > ; Leif Lindholm ;
> > > Grant Likely ; Varun Sethi ;
> > > Udit Kumar ; linux-efi
> > > 
> > > 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 
> wrote:
> > > >
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > Sent: Tuesday, December 11, 2018 6:02 PM
> > > > > To: Pankaj Bansal 
> > > > > Cc: Bhupesh Sharma ; Mark Rutland
> > > > > ; Leif Lindholm
> > > > > ; Grant Likely ;
> > > > > Varun Sethi ; Udit Kumar ;
> > > > > linux-efi 
> > > > > 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
> > > > > 
> > > wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -Original Message-
> > > > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > > > Sent: Tuesday, December 11, 2018 5:55 PM
> > > > > > > To: Pankaj Bansal 
> > > > > > > Cc: Bhupesh Sharma ; Mark Rutland
> > > > > > > ; Leif Lindholm
> > > > > > > ; Grant Likely
> > > > > > > ; Varun Sethi ; Udit
> > > > > > > Kumar ; linux-efi
> > > > > > > 
> > > > > > > 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
> > > > > > > 
> > > > > wrote:
> > > > > > > > > -Original Message-
> > > > > > > > > From: Bhupesh Sharma [mailto:bhsha...@redhat.com]
> > > > > > > > > Sent: Tuesday, December 11, 2018 4:25 PM
> > > > > > > > > To: Pankaj Bansal 
> > > > > > > > > Cc: Ard Biesheuvel ; Mark
> > > > > > > > > Rutland ; Leif Lindholm
> > > > > > > > > ; Grant Likely
> > > > > > > > > ; Varun Sethi ;
> > > > > > > > > Udit Kumar ;
> > > > > > > > > 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.
> &g

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

2018-12-11 Thread Pankaj Bansal


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, December 11, 2018 6:18 PM
> To: Pankaj Bansal 
> Cc: Bhupesh Sharma ; Mark Rutland
> ; Leif Lindholm ; Grant
> Likely ; Varun Sethi ; Udit Kumar
> ; linux-efi 
> 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  wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > Sent: Tuesday, December 11, 2018 6:02 PM
> > > To: Pankaj Bansal 
> > > Cc: Bhupesh Sharma ; Mark Rutland
> > > ; Leif Lindholm ;
> > > Grant Likely ; Varun Sethi ;
> > > Udit Kumar ; linux-efi
> > > 
> > > 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 
> wrote:
> > > >
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > Sent: Tuesday, December 11, 2018 5:55 PM
> > > > > To: Pankaj Bansal 
> > > > > Cc: Bhupesh Sharma ; Mark Rutland
> > > > > ; Leif Lindholm
> > > > > ; Grant Likely ;
> > > > > Varun Sethi ; Udit Kumar ;
> > > > > linux-efi 
> > > > > 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
> > > > > 
> > > wrote:
> > > > > > > -Original Message-
> > > > > > > From: Bhupesh Sharma [mailto:bhsha...@redhat.com]
> > > > > > > Sent: Tuesday, December 11, 2018 4:25 PM
> > > > > > > To: Pankaj Bansal 
> > > > > > > Cc: Ard Biesheuvel ; Mark Rutland
> > > > > > > ; Leif Lindholm
> > > > > > > ; Grant Likely
> > > > > > > ; Varun Sethi ; Udit
> > > > > > > Kumar ; 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.
> >
> 
>

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

2018-12-11 Thread Pankaj Bansal


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, December 11, 2018 6:02 PM
> To: Pankaj Bansal 
> Cc: Bhupesh Sharma ; Mark Rutland
> ; Leif Lindholm ; Grant
> Likely ; Varun Sethi ; Udit Kumar
> ; linux-efi 
> 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  wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > Sent: Tuesday, December 11, 2018 5:55 PM
> > > To: Pankaj Bansal 
> > > Cc: Bhupesh Sharma ; Mark Rutland
> > > ; Leif Lindholm ;
> > > Grant Likely ; Varun Sethi ;
> > > Udit Kumar ; linux-efi
> > > 
> > > 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 
> wrote:
> > > > > -Original Message-
> > > > > From: Bhupesh Sharma [mailto:bhsha...@redhat.com]
> > > > > Sent: Tuesday, December 11, 2018 4:25 PM
> > > > > To: Pankaj Bansal 
> > > > > Cc: Ard Biesheuvel ; Mark Rutland
> > > > > ; Leif Lindholm
> > > > > ; Grant Likely ;
> > > > > Varun Sethi ; Udit Kumar ;
> > > > > 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?


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

2018-12-11 Thread Pankaj Bansal


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, December 11, 2018 5:55 PM
> To: Pankaj Bansal 
> Cc: Bhupesh Sharma ; Mark Rutland
> ; Leif Lindholm ; Grant
> Likely ; Varun Sethi ; Udit Kumar
> ; linux-efi 
> 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  wrote:
> > > -Original Message-
> > > From: Bhupesh Sharma [mailto:bhsha...@redhat.com]
> > > Sent: Tuesday, December 11, 2018 4:25 PM
> > > To: Pankaj Bansal 
> > > Cc: Ard Biesheuvel ; Mark Rutland
> > > ; Leif Lindholm ;
> > > Grant Likely ; Varun Sethi ;
> > > Udit Kumar ; 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.



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

2018-12-11 Thread Pankaj Bansal


> -Original Message-
> From: Bhupesh Sharma [mailto:bhsha...@redhat.com]
> Sent: Tuesday, December 11, 2018 4:25 PM
> To: Pankaj Bansal 
> Cc: Ard Biesheuvel ; Mark Rutland
> ; Leif Lindholm ; Grant
> Likely ; Varun Sethi ; Udit Kumar
> ; 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 
> wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > Sent: Tuesday, December 11, 2018 3:45 PM
> > > To: Pankaj Bansal 
> > > Cc: Mark Rutland ; Leif Lindholm
> > > ; Grant Likely ;
> > > Varun Sethi ; Udit Kumar ;
> > > Bhupesh Sharma ; linux-efi
> > > 
> > > 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 
> wrote:
> > > >
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > Sent: Tuesday, December 11, 2018 3:41 PM
> > > > > To: Pankaj Bansal 
> > > > > Cc: Mark Rutland ; Leif Lindholm
> > > > > ; Grant Likely ;
> > > > > Varun Sethi ; Udit Kumar ;
> > > > > Bhupesh Sharma ; linux-efi
> > > > > 
> > > > > 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
> > > > > 
> > > wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -Original Message-
> > > > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > > > Sent: Tuesday, December 11, 2018 3:32 PM
> > > > > > > To: Pankaj Bansal 
> > > > > > > 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
> > > > > > > 
> > > > > wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > > -Original Message-
> > > > > > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > > > > > Sent: Tuesday, December 11, 2018 3:10 PM
> > > > > > > > > To: Pankaj Bansal 
> > > > > > > > > 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
> > > > > > > > > 
> > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > -Original Message-
> > > > > > > > > > > From: Ard Biesheuvel
> > > > > > > > > > > [mailto:ard.biesheu...@linaro.org]
> > > > > > > > > > > Sent: Tuesday, December 11, 2018 3:00 PM
> > > > > > > > > > > To: Pankaj Bansal 
> > > > > > > > > > > Cc: Mark Rutland ; Leif
> > > > > > > > > > > Lindholm ; Grant Likely
> > > > > > > > > > > ; Varun Sethi
> > > > > > > > > > > ; Udit Kumar ;
> > > > > > > > > > > Bhupesh Sharma ; linux-efi
> > > > > > > > > > > 
> > > > > > > > > > > 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
> > > > > > > > > > > 
> > > > > > > > > wrote:
> > > &g

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

2018-12-11 Thread Pankaj Bansal


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, December 11, 2018 3:45 PM
> To: Pankaj Bansal 
> Cc: Mark Rutland ; Leif Lindholm
> ; Grant Likely ; Varun Sethi
> ; Udit Kumar ; Bhupesh Sharma
> ; linux-efi 
> 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  wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > Sent: Tuesday, December 11, 2018 3:41 PM
> > > To: Pankaj Bansal 
> > > Cc: Mark Rutland ; Leif Lindholm
> > > ; Grant Likely ;
> > > Varun Sethi ; Udit Kumar ;
> > > Bhupesh Sharma ; linux-efi
> > > 
> > > 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 
> wrote:
> > > >
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > Sent: Tuesday, December 11, 2018 3:32 PM
> > > > > To: Pankaj Bansal 
> > > > > 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
> > > > > 
> > > wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -Original Message-
> > > > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > > > Sent: Tuesday, December 11, 2018 3:10 PM
> > > > > > > To: Pankaj Bansal 
> > > > > > > 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
> > > > > > > 
> > > > > wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > > -Original Message-
> > > > > > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > > > > > Sent: Tuesday, December 11, 2018 3:00 PM
> > > > > > > > > To: Pankaj Bansal 
> > > > > > > > > Cc: Mark Rutland ; Leif Lindholm
> > > > > > > > > ; Grant Likely
> > > > > > > > > ; Varun Sethi ;
> > > > > > > > > Udit Kumar ; Bhupesh Sharma
> > > > > > > > > ; linux-efi
> > > > > > > > > 
> > > > > > > > > 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
> > > > > > > > > 
> > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > -Original Message-
> > > > > > > > > > > From: Ard Biesheuvel
> > > > > > > > > > > [mailto:ard.biesheu...@linaro.org]
> > > > > > > > > > > Sent: Tuesday, December 11, 2018 2:55 PM
> > > > > > > > > > > To: Pankaj Bansal 
> > > > > > > > > > > Cc: Mark Rutland ; Leif
> > > > > > > > > > > Lindholm ; Grant Likely
> > > > > > > > > > > ; Varun Sethi
> > > > > > > > > > > ; Udit Kumar ;
> > > > > > > > > > > Bhupesh Sharma ; linux-efi
> > > > > > > > > > > 
> > > > > > > > > > > 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
> > > &g

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

2018-12-11 Thread Pankaj Bansal


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, December 11, 2018 3:41 PM
> To: Pankaj Bansal 
> Cc: Mark Rutland ; Leif Lindholm
> ; Grant Likely ; Varun Sethi
> ; Udit Kumar ; Bhupesh Sharma
> ; linux-efi 
> 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  wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > Sent: Tuesday, December 11, 2018 3:32 PM
> > > To: Pankaj Bansal 
> > > 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 
> wrote:
> > > >
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > Sent: Tuesday, December 11, 2018 3:10 PM
> > > > > To: Pankaj Bansal 
> > > > > 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
> > > > > 
> > > wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -Original Message-
> > > > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > > > Sent: Tuesday, December 11, 2018 3:00 PM
> > > > > > > To: Pankaj Bansal 
> > > > > > > Cc: Mark Rutland ; Leif Lindholm
> > > > > > > ; Grant Likely
> > > > > > > ; Varun Sethi ; Udit
> > > > > > > Kumar ; Bhupesh Sharma
> > > > > > > ; linux-efi 
> > > > > > > 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
> > > > > > > 
> > > > > wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > > -Original Message-
> > > > > > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > > > > > Sent: Tuesday, December 11, 2018 2:55 PM
> > > > > > > > > To: Pankaj Bansal 
> > > > > > > > > Cc: Mark Rutland ; Leif Lindholm
> > > > > > > > > ; Grant Likely
> > > > > > > > > ; Varun Sethi ;
> > > > > > > > > Udit Kumar ; Bhupesh Sharma
> > > > > > > > > ; linux-efi
> > > > > > > > > 
> > > > > > > > > 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
> > > > > > > > > 
> > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > -Original Message-
> > > > > > > > > > > From: Ard Biesheuvel
> > > > > > > > > > > [mailto:ard.biesheu...@linaro.org]
> > > > > > > > > > > Sent: Tuesday, December 11, 2018 2:48 PM
> > > > > > > > > > > To: Pankaj Bansal 
> > > > > > > > > > > Cc: Mark Rutland ; Leif
> > > > > > > > > > > Lindholm ; Grant Likely
> > > > > > > > > > > ; Varun Sethi
> > > > > > > > > > > ; Udit Kumar ;
> > > > > > > > > > > Bhupesh Sharma ; linux-efi
> > > > > > > > > > > 
> > > > > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi:
> > > > > > > > > > > install new fdt in configuration table
> > > > > > > > > > >
> > 

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

2018-12-11 Thread Pankaj Bansal


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, December 11, 2018 3:32 PM
> To: Pankaj Bansal 
> 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  wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > Sent: Tuesday, December 11, 2018 3:10 PM
> > > To: Pankaj Bansal 
> > > 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 
> wrote:
> > > >
> > > >
> > > >
> > > > > -----Original Message-
> > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > Sent: Tuesday, December 11, 2018 3:00 PM
> > > > > To: Pankaj Bansal 
> > > > > Cc: Mark Rutland ; Leif Lindholm
> > > > > ; Grant Likely ;
> > > > > Varun Sethi ; Udit Kumar ;
> > > > > Bhupesh Sharma ; linux-efi
> > > > > 
> > > > > 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
> > > > > 
> > > wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -Original Message-
> > > > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > > > Sent: Tuesday, December 11, 2018 2:55 PM
> > > > > > > To: Pankaj Bansal 
> > > > > > > Cc: Mark Rutland ; Leif Lindholm
> > > > > > > ; Grant Likely
> > > > > > > ; Varun Sethi ; Udit
> > > > > > > Kumar ; Bhupesh Sharma
> > > > > > > ; linux-efi 
> > > > > > > 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
> > > > > > > 
> > > > > wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > > -Original Message-
> > > > > > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > > > > > Sent: Tuesday, December 11, 2018 2:48 PM
> > > > > > > > > To: Pankaj Bansal 
> > > > > > > > > Cc: Mark Rutland ; Leif Lindholm
> > > > > > > > > ; Grant Likely
> > > > > > > > > ; Varun Sethi ;
> > > > > > > > > Udit Kumar ; Bhupesh Sharma
> > > > > > > > > ; linux-efi
> > > > > > > > > 
> > > > > > > > > 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
> > > > > > > > > 
> > > > > > > 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 
> > > > > > > > > > Cc: linux-efi@vger.kernel.org
> > > > > > > > > > Signed-off-by: Pankaj Bansal 
> > > > > > > > >
> > > > > > > > > I still think this is a bad idea. The firmware is
> > > > > > > > > closely tied to 

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

2018-12-11 Thread Pankaj Bansal


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, December 11, 2018 2:55 PM
> To: Pankaj Bansal 
> Cc: Mark Rutland ; Leif Lindholm
> ; Grant Likely ; Varun Sethi
> ; Udit Kumar ; Bhupesh Sharma
> ; linux-efi 
> 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  wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > Sent: Tuesday, December 11, 2018 2:48 PM
> > > To: Pankaj Bansal 
> > > Cc: Mark Rutland ; Leif Lindholm
> > > ; Grant Likely ;
> > > Varun Sethi ; Udit Kumar ;
> > > Bhupesh Sharma ; linux-efi
> > > 
> > > 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 
> 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 
> > > > Cc: linux-efi@vger.kernel.org
> > > > Signed-off-by: Pankaj Bansal 
> > >
> > > 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?


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

2018-12-11 Thread Pankaj Bansal


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, December 11, 2018 2:48 PM
> To: Pankaj Bansal 
> Cc: Mark Rutland ; Leif Lindholm
> ; Grant Likely ; Varun Sethi
> ; Udit Kumar ; Bhupesh Sharma
> ; linux-efi 
> 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  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 
> > Cc: linux-efi@vger.kernel.org
> > Signed-off-by: Pankaj Bansal 
> 
> 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
> >


[PATCH v2 1/2] drivers: firmware: efi: change sys_table to sys_table_arg

2018-12-11 Thread Pankaj Bansal
efi_call_early(f, ...) macro expands to
sys_table_arg->boottime->f(__VA_ARGS__).
Therefore, change sys_table to sys_table_arg so that efi_call_early
macro can be used.

Cc: Ard Biesheuvel 
Cc: linux-efi@vger.kernel.org
Signed-off-by: Pankaj Bansal 
---

Notes:
V2:
No change

 drivers/firmware/efi/libstub/fdt.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/firmware/efi/libstub/fdt.c 
b/drivers/firmware/efi/libstub/fdt.c
index 0dc7b4987cc2..45cded5b5d5c 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -252,7 +252,7 @@ static efi_status_t exit_boot_func(efi_system_table_t 
*sys_table_arg,
  * with the final memory map in it.
  */
 
-efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
+efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table_arg,
void *handle,
unsigned long *new_fdt_addr,
unsigned long max_addr,
@@ -283,20 +283,20 @@ efi_status_t 
allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
 * subsequent allocations adding entries, since they could not affect
 * the number of EFI_MEMORY_RUNTIME regions.
 */
-   status = efi_get_memory_map(sys_table, &map);
+   status = efi_get_memory_map(sys_table_arg, &map);
if (status != EFI_SUCCESS) {
-   pr_efi_err(sys_table, "Unable to retrieve UEFI memory map.\n");
+   pr_efi_err(sys_table_arg, "Unable to retrieve UEFI memory 
map.\n");
return status;
}
 
-   pr_efi(sys_table,
+   pr_efi(sys_table_arg,
   "Exiting boot services and installing virtual address map...\n");
 
map.map = &memory_map;
-   status = efi_high_alloc(sys_table, MAX_FDT_SIZE, EFI_FDT_ALIGN,
+   status = efi_high_alloc(sys_table_arg, MAX_FDT_SIZE, EFI_FDT_ALIGN,
new_fdt_addr, max_addr);
if (status != EFI_SUCCESS) {
-   pr_efi_err(sys_table,
+   pr_efi_err(sys_table_arg,
   "Unable to allocate memory for new device tree.\n");
goto fail;
}
@@ -305,30 +305,30 @@ efi_status_t 
allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
 * Now that we have done our final memory allocation (and free)
 * we can get the memory map key needed for exit_boot_services().
 */
-   status = efi_get_memory_map(sys_table, &map);
+   status = efi_get_memory_map(sys_table_arg, &map);
if (status != EFI_SUCCESS)
goto fail_free_new_fdt;
 
-   status = update_fdt(sys_table, (void *)fdt_addr, fdt_size,
+   status = update_fdt(sys_table_arg, (void *)fdt_addr, fdt_size,
(void *)*new_fdt_addr, MAX_FDT_SIZE, cmdline_ptr,
initrd_addr, initrd_size);
 
if (status != EFI_SUCCESS) {
-   pr_efi_err(sys_table, "Unable to construct new device tree.\n");
+   pr_efi_err(sys_table_arg, "Unable to construct 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;
-   status = efi_exit_boot_services(sys_table, handle, &map, &priv,
+   status = efi_exit_boot_services(sys_table_arg, handle, &map, &priv,
exit_boot_func);
 
if (status == EFI_SUCCESS) {
efi_set_virtual_address_map_t *svam;
 
/* Install the new virtual address map */
-   svam = sys_table->runtime->set_virtual_address_map;
+   svam = sys_table_arg->runtime->set_virtual_address_map;
status = svam(runtime_entry_count * desc_size, desc_size,
  desc_ver, runtime_map);
 
@@ -356,13 +356,13 @@ efi_status_t 
allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
return EFI_SUCCESS;
}
 
-   pr_efi_err(sys_table, "Exit boot services failed.\n");
+   pr_efi_err(sys_table_arg, "Exit boot services failed.\n");
 
 fail_free_new_fdt:
-   efi_free(sys_table, MAX_FDT_SIZE, *new_fdt_addr);
+   efi_free(sys_table_arg, MAX_FDT_SIZE, *new_fdt_addr);
 
 fail:
-   sys_table->boottime->free_pool(runtime_map);
+   sys_table_arg->boottime->free_pool(runtime_map);
return EFI_LOAD_ERROR;
 }
 
-- 
2.17.1



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

2018-12-11 Thread Pankaj Bansal
This patch series is the modification of patches that i had sent a while back:
https://www.spinics.net/lists/linux-efi/msg13701.html

The main reservation about these changes was that these patches made the fdt
supplied by "dtb=" kernel command line parameter available to boot firmware.
Now, for boot firmware this fdt is an *unknown*, which can cause errors when
boot firmware tries to use it.

I have alleviate these concerns by adding a check that the new fdt (allocated 
by efistub)
would be made available to boot firmware only if it has been generated from the 
fdt which
boot firmware has supplied (i.e. which is already present in configuration 
table)

This makes the new fdt *known* to boot firmware, which it can use.

Reason behind these changes is this:

we have some devices that can be used by uefi firmware as well as OS. When the 
uefi firmware
is using these devices, then they are in certain state.
before OS can use these devices, their state needs to be changed. after which 
uefi firmware
cannot use these devices.

We use exit_boot_services as indicator as when to change the state of these 
devices.
if the state of devices is successfully changed, their status is fixed in fdt 
as "okay" otherwise
these are "disabled" in fdt, so that OS has correct status of these devices.

Pankaj Bansal (2):
  drivers: firmware: efi: change sys_table to sys_table_arg
  drivers: firmware: efi: install new fdt in configuration table

 drivers/firmware/efi/libstub/fdt.c | 48 +-
 1 file changed, 34 insertions(+), 14 deletions(-)

-- 
2.17.1



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

2018-12-11 Thread Pankaj Bansal
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 
Cc: linux-efi@vger.kernel.org
Signed-off-by: Pankaj Bansal 
---

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



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

2018-04-29 Thread Pankaj Bansal


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Thursday, April 26, 2018 2:12 PM
> To: Grant Likely 
> Cc: Udit Kumar ; Leif Lindholm
> ; Pankaj Bansal ;
> mark.rutl...@arm.com; linux-efi@vger.kernel.org; Varun Sethi
> ; Wasim Khan ; n...@arm.com; Rod
> Dorris 
> Subject: Re: [PATCH 2/2] drivers: firmware: efi: install new fdt in 
> configuration
> table
> 
> On 26 April 2018 at 10:37, Grant Likely  wrote:
> > On 25/04/2018 15:48, Udit Kumar wrote:
> >>
> >>
> >>
> >>> -Original Message-
> >>> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> >>> Sent: Wednesday, April 25, 2018 4:20 PM
> >>> To: Grant Likely 
> >>> Cc: Udit Kumar ; Ard Biesheuvel
> >>> ; Pankaj Bansal ;
> >>> mark.rutl...@arm.com; linux-efi@vger.kernel.org; Varun Sethi
> >>> ; Wasim Khan ; n...@arm.com;
> Rod
> >>> Dorris 
> >>> Subject: Re: [PATCH 2/2] drivers: firmware: efi: install new fdt in
> >>> configuration table
> >>>
> >>> On Wed, Apr 25, 2018 at 11:04:13AM +0100, Grant Likely wrote:
> >>>>
> >>>> On 25/04/2018 07:49, Udit Kumar wrote:
> >>>>>>
> >>>>>> 2) Kernel provides/overrides DTB
> >>>>>> The kernel always has the option of loading it's own DTB, either
> >>>>>> because firmware doesn't provide one, or it needs a newer DTB.
> >>>>>> This is similar to CONFIG_ACPI_CUSTOM_DSDT in ACPI land. However,
> >>>>>> if the kernel is overriding the DTB, then firmware has no part of
> >>>>>> it, and it should not be allowed to modify the DTB at
> >>>>>> ExitBootServices()
> >>>
> >>> time.
> >>>>>
> >>>>>
> >>>>> For our platforms at least, this will not work unless firmware is
> >>>>> doing modifications.  I think this I likely true for Ard as well.
> >>>>> Firmware has a logic to modify default DTB for available devices,
> >>>>> reading board configuration programming required muxes, based upon
> >>>>> this removing/adding devices into DTB.
> >>>>> IMO, this will not be an optimal way to use untouched DTB provided
> >>>>> by
> >>>
> >>> kernel.
> >>>>
> >>>>
> >>>> Right, which is part of the reason for insisting on the firmare
> >>>> being responsible. If the kernel or Grub overrides the DTB, then it
> >>>> is an explicit
> >>>> *rejection* of anything firmware provides; presumable because
> >>>> something is broken and it has to be worked around.
> >>>
> >>>
> >>> So, the functionality in GRUB to replace a firmware-provided
> >>> devicetree needs to go. And dtb= loading should ideally be made
> >>> dependent on some sort of DEBUG config option.
> >>
> >>
> >> In my view , dtb= sort of option should go away. If we are booting
> >> some debug image in development environment then why not to re-flash
> >> DTB
> >
> >
> > I don't want to drop the prototyping use cases on the floor. ie.
> > Wiring up a development board to external hardware. The development
> > board is already supported, but the user is going to be experimenting
> > with DT changes as they go. Reflashing the DT is not a good option in
> > this case. It is more usefule to be able to load the DT modifications
> > at runtime before booting the kernel, because they will change every boot.
> >
> > I can see this scenario playing out for both full DT replacement and
> > DTBO scenarios.
> >
> 
> This applies mainly to prototyping under the OS, right?
> 
> One aspect we did not consider yet I think is the use of DT to describe the
> platform to ARM-TF and UEFI itself. I expect this to become more common
> going forward, and will make replacing just the DTB even trickier.

Should we keep the dtb for UEFI and OS separate or not? The bootloader is 
expected to 
control only those devices from which it expects to get the OS to boot. The dtb 
file
for bootloader will contain only these devices' nodes, while the dtb passed to 
OS will contain
list of all devices that are available on a platform.

> 
> So overriding the DTB like we allow for DSDT makes perfect sense, but as a
> development feature only. I still think the idea of just dropping an updated 
> DTB
> into an existing firmware build consisting of ARM-TF, UEFI etc (and perhaps 
> even
> OP-TEE?) is not something that is going to be feasible in general.
N�r��yb�X��ǧv�^�)޺{.n�+{�y^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�

FW: [PATCH 1/2] drivers: firmware: efi: change sys_table to sys_table_arg

2018-03-27 Thread Pankaj Bansal
Sorry I sent these patches again to mailing list.
I meant to send this series to internal members. Please ignore this patch 
series and don't respond on this thread.

> -Original Message-
> From: Pankaj Bansal
> Sent: Tuesday, March 27, 2018 4:52 PM
> To: linux-de...@linux.freescale.net
> Cc: Leo Li ; Varun Sethi ; Udit Kumar
> ; Wasim Khan ; Meenakshi
> Aggarwal ; Pankaj Bansal
> ; Ard Biesheuvel ; linux-
> e...@vger.kernel.org
> Subject: [PATCH 1/2] drivers: firmware: efi: change sys_table to sys_table_arg
> 
> efi_call_early(f, ...) macro expands to
> sys_table_arg->boottime->f(__VA_ARGS__).
> Therefore, change sys_table to sys_table_arg so that efi_call_early macro can
> be used.
> 
> Cc: Ard Biesheuvel 
> Cc: linux-efi@vger.kernel.org
> Signed-off-by: Pankaj Bansal 
> ---
>  drivers/firmware/efi/libstub/fdt.c | 28 ++--
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/firmware/efi/libstub/fdt.c
> b/drivers/firmware/efi/libstub/fdt.c
> index 8830fa6..177654e 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -248,7 +248,7 @@ static efi_status_t exit_boot_func(efi_system_table_t
> *sys_table_arg,
>   * with the final memory map in it.
>   */
> 
> -efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
> +efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t
> +*sys_table_arg,
>   void *handle,
>   unsigned long *new_fdt_addr,
>   unsigned long max_addr,
> @@ -279,20 +279,20 @@ efi_status_t
> allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
>* subsequent allocations adding entries, since they could not affect
>* the number of EFI_MEMORY_RUNTIME regions.
>*/
> - status = efi_get_memory_map(sys_table, &map);
> + status = efi_get_memory_map(sys_table_arg, &map);
>   if (status != EFI_SUCCESS) {
> - pr_efi_err(sys_table, "Unable to retrieve UEFI memory
> map.\n");
> + pr_efi_err(sys_table_arg, "Unable to retrieve UEFI memory
> map.\n");
>   return status;
>   }
> 
> - pr_efi(sys_table,
> + pr_efi(sys_table_arg,
>  "Exiting boot services and installing virtual address map...\n");
> 
>   map.map = &memory_map;
> - status = efi_high_alloc(sys_table, MAX_FDT_SIZE, EFI_FDT_ALIGN,
> + status = efi_high_alloc(sys_table_arg, MAX_FDT_SIZE, EFI_FDT_ALIGN,
>   new_fdt_addr, max_addr);
>   if (status != EFI_SUCCESS) {
> - pr_efi_err(sys_table,
> + pr_efi_err(sys_table_arg,
>  "Unable to allocate memory for new device tree.\n");
>   goto fail;
>   }
> @@ -301,30 +301,30 @@ efi_status_t
> allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
>* Now that we have done our final memory allocation (and free)
>* we can get the memory map key needed for exit_boot_services().
>*/
> - status = efi_get_memory_map(sys_table, &map);
> + status = efi_get_memory_map(sys_table_arg, &map);
>   if (status != EFI_SUCCESS)
>   goto fail_free_new_fdt;
> 
> - status = update_fdt(sys_table, (void *)fdt_addr, fdt_size,
> + status = update_fdt(sys_table_arg, (void *)fdt_addr, fdt_size,
>   (void *)*new_fdt_addr, MAX_FDT_SIZE, cmdline_ptr,
>   initrd_addr, initrd_size);
> 
>   if (status != EFI_SUCCESS) {
> - pr_efi_err(sys_table, "Unable to construct new device tree.\n");
> + pr_efi_err(sys_table_arg, "Unable to construct 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;
> - status = efi_exit_boot_services(sys_table, handle, &map, &priv,
> + status = efi_exit_boot_services(sys_table_arg, handle, &map, &priv,
>   exit_boot_func);
> 
>   if (status == EFI_SUCCESS) {
>   efi_set_virtual_address_map_t *svam;
> 
>   /* Install the new virtual address map */
> - svam = sys_table->runtime->set_virtual_address_map;
> + svam = sys_table_arg->runtime->set_virtual_address_map;
>   status = svam(runtime_entry_count * desc_size, desc_size,
&

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

2018-03-27 Thread Pankaj Bansal
Bootloader may need to fixup the device tree before OS can use it.
e.g. a UEFI/DXE driver that has initialized a controller can add
controller's clock frequency in controller node. This way OS need not to
call get/set clock for that controller.

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 
Cc: linux-efi@vger.kernel.org
Signed-off-by: Pankaj Bansal 
---
 drivers/firmware/efi/libstub/fdt.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/firmware/efi/libstub/fdt.c 
b/drivers/firmware/efi/libstub/fdt.c
index 177654e..df862e6 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -265,6 +265,7 @@ efi_status_t 
allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table_arg,
int runtime_entry_count = 0;
struct efi_boot_memmap map;
struct exit_boot_struct priv;
+   efi_guid_t fdt_guid = DEVICE_TREE_GUID;
 
map.map =   &runtime_map;
map.map_size =  &map_size;
@@ -314,6 +315,13 @@ efi_status_t 
allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table_arg,
goto fail_free_new_fdt;
}
 
+   status = efi_call_early(install_configuration_table, &fdt_guid,
+   (void *)*new_fdt_addr);
+   if (status != EFI_SUCCESS) {
+   pr_efi_err(sys_table_arg, "Unable to install new device 
tree.\n");
+   goto fail_free_new_fdt;
+   }
+
priv.runtime_map = runtime_map;
priv.runtime_entry_count = &runtime_entry_count;
priv.new_fdt_addr = (void *)*new_fdt_addr;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] drivers: firmware: efi: change sys_table to sys_table_arg

2018-03-27 Thread Pankaj Bansal
efi_call_early(f, ...) macro expands to
sys_table_arg->boottime->f(__VA_ARGS__).
Therefore, change sys_table to sys_table_arg so that efi_call_early
macro can be used.

Cc: Ard Biesheuvel 
Cc: linux-efi@vger.kernel.org
Signed-off-by: Pankaj Bansal 
---
 drivers/firmware/efi/libstub/fdt.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/firmware/efi/libstub/fdt.c 
b/drivers/firmware/efi/libstub/fdt.c
index 8830fa6..177654e 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -248,7 +248,7 @@ static efi_status_t exit_boot_func(efi_system_table_t 
*sys_table_arg,
  * with the final memory map in it.
  */
 
-efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
+efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table_arg,
void *handle,
unsigned long *new_fdt_addr,
unsigned long max_addr,
@@ -279,20 +279,20 @@ efi_status_t 
allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
 * subsequent allocations adding entries, since they could not affect
 * the number of EFI_MEMORY_RUNTIME regions.
 */
-   status = efi_get_memory_map(sys_table, &map);
+   status = efi_get_memory_map(sys_table_arg, &map);
if (status != EFI_SUCCESS) {
-   pr_efi_err(sys_table, "Unable to retrieve UEFI memory map.\n");
+   pr_efi_err(sys_table_arg, "Unable to retrieve UEFI memory 
map.\n");
return status;
}
 
-   pr_efi(sys_table,
+   pr_efi(sys_table_arg,
   "Exiting boot services and installing virtual address map...\n");
 
map.map = &memory_map;
-   status = efi_high_alloc(sys_table, MAX_FDT_SIZE, EFI_FDT_ALIGN,
+   status = efi_high_alloc(sys_table_arg, MAX_FDT_SIZE, EFI_FDT_ALIGN,
new_fdt_addr, max_addr);
if (status != EFI_SUCCESS) {
-   pr_efi_err(sys_table,
+   pr_efi_err(sys_table_arg,
   "Unable to allocate memory for new device tree.\n");
goto fail;
}
@@ -301,30 +301,30 @@ efi_status_t 
allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
 * Now that we have done our final memory allocation (and free)
 * we can get the memory map key needed for exit_boot_services().
 */
-   status = efi_get_memory_map(sys_table, &map);
+   status = efi_get_memory_map(sys_table_arg, &map);
if (status != EFI_SUCCESS)
goto fail_free_new_fdt;
 
-   status = update_fdt(sys_table, (void *)fdt_addr, fdt_size,
+   status = update_fdt(sys_table_arg, (void *)fdt_addr, fdt_size,
(void *)*new_fdt_addr, MAX_FDT_SIZE, cmdline_ptr,
initrd_addr, initrd_size);
 
if (status != EFI_SUCCESS) {
-   pr_efi_err(sys_table, "Unable to construct new device tree.\n");
+   pr_efi_err(sys_table_arg, "Unable to construct 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;
-   status = efi_exit_boot_services(sys_table, handle, &map, &priv,
+   status = efi_exit_boot_services(sys_table_arg, handle, &map, &priv,
exit_boot_func);
 
if (status == EFI_SUCCESS) {
efi_set_virtual_address_map_t *svam;
 
/* Install the new virtual address map */
-   svam = sys_table->runtime->set_virtual_address_map;
+   svam = sys_table_arg->runtime->set_virtual_address_map;
status = svam(runtime_entry_count * desc_size, desc_size,
  desc_ver, runtime_map);
 
@@ -352,13 +352,13 @@ efi_status_t 
allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
return EFI_SUCCESS;
}
 
-   pr_efi_err(sys_table, "Exit boot services failed.\n");
+   pr_efi_err(sys_table_arg, "Exit boot services failed.\n");
 
 fail_free_new_fdt:
-   efi_free(sys_table, MAX_FDT_SIZE, *new_fdt_addr);
+   efi_free(sys_table_arg, MAX_FDT_SIZE, *new_fdt_addr);
 
 fail:
-   sys_table->boottime->free_pool(runtime_map);
+   sys_table_arg->boottime->free_pool(runtime_map);
return EFI_LOAD_ERROR;
 }
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2018-03-26 Thread Pankaj Bansal
Hi Bhupesh,

> -Original Message-
> From: Bhupesh Sharma [mailto:bhsha...@redhat.com]
> Sent: Monday, March 26, 2018 2:15 PM
> To: Pankaj Bansal 
> Cc: Varun Sethi ; Wasim Khan ;
> Udit Kumar ; linux-efi@vger.kernel.org; Ard Biesheuvel
> 
> Subject: Re: [PATCH 2/2] drivers: firmware: efi: install new fdt in 
> configuration
> table
> 
> On 03/26/2018 11:13 AM, Pankaj Bansal wrote:
> > Hello Bhupesh/Ard,
> >
> > Thank you for reviewing my changes.
> >
> > > -Original Message-
> > > From: Bhupesh Sharma [mailto:bhsha...@redhat.com]
> > > Sent: Monday, March 26, 2018 10:51 AM
> > > To: Pankaj Bansal 
> > > Cc: Varun Sethi ; Wasim Khan ;
> > > Udit Kumar ; Ard Biesheuvel
> > > ; linux-efi@vger.kernel.org
> > > Subject: Re: [PATCH 2/2] drivers: firmware: efi: install new fdt in
> > > configuration table
> > >
> > > Hi Pankaj,
> > >
> > > On 03/25/2018 07:26 PM, Pankaj Bansal wrote:
> > >> Bootloader may need to fixup the device tree before OS can use it.
> > >> e.g. a UEFI/DXE driver that has initialized a controller can add
> > >> controller's clock frequency in controller node. This way OS need
> > >> not to call get/set clock for that controller.
> > > Its usually better to include a cover letter to provide a
> > > description of what the patchset is adding/fixing.
> >
> > Sure. Will follow this practice now onwards.
> >
> > > What I understand from looking at this patch is that you are
> > > providing the device tree to the kernel (via dtb= ? command) and
> > > trying to fix it via the bootloader - is this understanding correct?
> >
> > That is correct.
> >
> > >
> > > If yes, I am not sure why the bootloader would not pass the dtb in
> > > the first place to the kernel via the standard interface defined in
> > > <https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > www
> > >
> .kernel.org%2Fdoc%2FDocumentation%2Farm64%2Fbooting.txt&data=02%7C0
> > >
> 1%7Cpankaj.bansal%40nxp.com%7Ce917e5770a544261e9ee08d592d971a0%7C
> > >
> 686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636576384962510170&sd
> > > ata=GlBJchtRXnrX2IyoaKqe25k1ZDMmy0bH8ZileRFFBqo%3D&reserved=0>,
> i.e.:
> > >
> > > x0 = physical address of device tree blob (dtb) in system RAM.
> > >
> > > Normally bootloaders like UEFI/u-boot pass a fixed up dtb to the
> > > kernel, in such a case, the dtb would be fixed up by the bootloader
> > > itself and the kernel needs not worry about the same.
> >
> > Yes this is the default case. But we have seen such cases when the linux 
> > kernel
> fails to boot with the UEFI provided dtb.
> > e.g. say UEFI dtb binary is generated with linux v4.4. When we try to
> > boot linux v4.9 or v4.14 without updating dtb, then kernel boot fails. Maybe
> this is because device trees are not backward compatible as Ard pointed out.
> >
> > I see the boot behavior in other boot loader u-boot. In that bootloader we 
> > can
> provide the kernel, initrd, dtb triad.
> >
> > Usage:
> > bootm [addr [arg ...]]
> >  - boot application image stored in memory
> > passing arguments 'arg ...'; when booting a Linux kernel,
> > 'arg' can be the address of an initrd image
> > When booting a Linux kernel which requires a flat device-tree
> > a third argument is required which is the address of the
> > device-tree blob. To boot that kernel without an initrd image,
> > use a '-' for the second argument. If you do not pass a third
> > a bd_info struct will be passed instead
> >
> > The u-boot fixes up the dtb provided in bootm command before booting the
> kernel image.
> > This same behavior we want to achieve with UEFI boot. i.e. UEFI will 
> > provide a
> dtb image, but it can be overridden using dtb = .
> But there are several limitations to this approach. One of them is that dtb= 
> is
> ignored from kernel command line when UEFI Secure Boot is enabled.
> See
> <https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.
> bootlin.com%2Flinux%2Fv4.4.41%2Fsource%2Fdrivers%2Ffirmware%2Fefi%2Flib
> stub%2Farm-
> stub.c%23L235&data=02%7C01%7Cpankaj.bansal%40nxp.com%7C49f4c16493c
> b4cd66a7108d592f5d2b9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1
> %7C636576506854276136&sdata=HAjJP8mZYQ7UWMoQ1AwOfUQdAK2ztsbdks
> hZFRMxn4s%3D&reserved=0> for details.
> 

Right no

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

2018-03-25 Thread Pankaj Bansal
Hello Bhupesh/Ard,

Thank you for reviewing my changes.

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

Sure. Will follow this practice now onwards.

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

That is correct.

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

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

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

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

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

> 
> Regards,
> Bhupesh
> 
> > Therefore, install fdt used by OS in configuration tables and
> > associate it with device tree guid.
> >
> > UEFI/DXE drivers can fixup this device tree in their respective
> > ExitBootServices events.
> >
> > Cc: Ard Biesheuvel 
> > Cc: linux-efi@vger.kernel.org
> > Signed-off-by: Pankaj Bansal 
> > ---
> >   drivers/firmware/efi/libstub/fdt.c | 8 
> >   1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/firmware/efi/libstub/fdt.c
> > b/drivers/firmware/efi/libstub/fdt.c
> > index 177654e..df862e6 100644
> > --- a/drivers/firmware/efi/libstub/fdt.c
> > +++ b/drivers/firmware/efi/libstub/fdt.c
> > @@ -265,6 +265,7 @@ efi_status_t
> allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table_arg,
> >   int runtime_entry_count = 0;
> >   struct efi_boot_memmap map;
> >   struct exit_boot_struct priv;
> > +efi_guid_t fdt_guid = DEVICE_TREE_GUID;
> >
> >   map.map =&runtime_map;
> >   map.map_size =&map_size;
> > @@ -314,6 +315,13 @@ efi_status_t
> allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table_arg,
> >   goto fail_free_new_fdt;
> >   }
> >
> > +status = efi_call_early(install_configuration_table, &fdt_guid,
> > +(void *)*new_fdt_addr);
> > +if (status != EFI_SUCCESS) {
> > +pr_efi_err(sys_table_arg, "Unable to install new device tree.\n");
> > +goto fail_free_new_fdt;
> > +}
> > +
> >   priv.runtime_map = runtime_map;
> >   priv.runtime_entry_count = &runtime_entry_count;
> >   priv.new_fdt_addr = (void *)*new_fdt_addr;
> >



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

2018-03-25 Thread Pankaj Bansal
Bootloader may need to fixup the device tree before OS can use it.
e.g. a UEFI/DXE driver that has initialized a controller can add
controller's clock frequency in controller node. This way OS need not to
call get/set clock for that controller.

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 
Cc: linux-efi@vger.kernel.org
Signed-off-by: Pankaj Bansal 
---
 drivers/firmware/efi/libstub/fdt.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/firmware/efi/libstub/fdt.c 
b/drivers/firmware/efi/libstub/fdt.c
index 177654e..df862e6 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -265,6 +265,7 @@ efi_status_t 
allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table_arg,
int runtime_entry_count = 0;
struct efi_boot_memmap map;
struct exit_boot_struct priv;
+   efi_guid_t fdt_guid = DEVICE_TREE_GUID;
 
map.map =   &runtime_map;
map.map_size =  &map_size;
@@ -314,6 +315,13 @@ efi_status_t 
allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table_arg,
goto fail_free_new_fdt;
}
 
+   status = efi_call_early(install_configuration_table, &fdt_guid,
+   (void *)*new_fdt_addr);
+   if (status != EFI_SUCCESS) {
+   pr_efi_err(sys_table_arg, "Unable to install new device 
tree.\n");
+   goto fail_free_new_fdt;
+   }
+
priv.runtime_map = runtime_map;
priv.runtime_entry_count = &runtime_entry_count;
priv.new_fdt_addr = (void *)*new_fdt_addr;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] drivers: firmware: efi: change sys_table to sys_table_arg

2018-03-25 Thread Pankaj Bansal
efi_call_early(f, ...) macro expands to
sys_table_arg->boottime->f(__VA_ARGS__).
Therefore, change sys_table to sys_table_arg so that efi_call_early
macro can be used.

Cc: Ard Biesheuvel 
Cc: linux-efi@vger.kernel.org
Signed-off-by: Pankaj Bansal 
---
 drivers/firmware/efi/libstub/fdt.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/firmware/efi/libstub/fdt.c 
b/drivers/firmware/efi/libstub/fdt.c
index 8830fa6..177654e 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -248,7 +248,7 @@ static efi_status_t exit_boot_func(efi_system_table_t 
*sys_table_arg,
  * with the final memory map in it.
  */
 
-efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
+efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table_arg,
void *handle,
unsigned long *new_fdt_addr,
unsigned long max_addr,
@@ -279,20 +279,20 @@ efi_status_t 
allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
 * subsequent allocations adding entries, since they could not affect
 * the number of EFI_MEMORY_RUNTIME regions.
 */
-   status = efi_get_memory_map(sys_table, &map);
+   status = efi_get_memory_map(sys_table_arg, &map);
if (status != EFI_SUCCESS) {
-   pr_efi_err(sys_table, "Unable to retrieve UEFI memory map.\n");
+   pr_efi_err(sys_table_arg, "Unable to retrieve UEFI memory 
map.\n");
return status;
}
 
-   pr_efi(sys_table,
+   pr_efi(sys_table_arg,
   "Exiting boot services and installing virtual address map...\n");
 
map.map = &memory_map;
-   status = efi_high_alloc(sys_table, MAX_FDT_SIZE, EFI_FDT_ALIGN,
+   status = efi_high_alloc(sys_table_arg, MAX_FDT_SIZE, EFI_FDT_ALIGN,
new_fdt_addr, max_addr);
if (status != EFI_SUCCESS) {
-   pr_efi_err(sys_table,
+   pr_efi_err(sys_table_arg,
   "Unable to allocate memory for new device tree.\n");
goto fail;
}
@@ -301,30 +301,30 @@ efi_status_t 
allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
 * Now that we have done our final memory allocation (and free)
 * we can get the memory map key needed for exit_boot_services().
 */
-   status = efi_get_memory_map(sys_table, &map);
+   status = efi_get_memory_map(sys_table_arg, &map);
if (status != EFI_SUCCESS)
goto fail_free_new_fdt;
 
-   status = update_fdt(sys_table, (void *)fdt_addr, fdt_size,
+   status = update_fdt(sys_table_arg, (void *)fdt_addr, fdt_size,
(void *)*new_fdt_addr, MAX_FDT_SIZE, cmdline_ptr,
initrd_addr, initrd_size);
 
if (status != EFI_SUCCESS) {
-   pr_efi_err(sys_table, "Unable to construct new device tree.\n");
+   pr_efi_err(sys_table_arg, "Unable to construct 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;
-   status = efi_exit_boot_services(sys_table, handle, &map, &priv,
+   status = efi_exit_boot_services(sys_table_arg, handle, &map, &priv,
exit_boot_func);
 
if (status == EFI_SUCCESS) {
efi_set_virtual_address_map_t *svam;
 
/* Install the new virtual address map */
-   svam = sys_table->runtime->set_virtual_address_map;
+   svam = sys_table_arg->runtime->set_virtual_address_map;
status = svam(runtime_entry_count * desc_size, desc_size,
  desc_ver, runtime_map);
 
@@ -352,13 +352,13 @@ efi_status_t 
allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
return EFI_SUCCESS;
}
 
-   pr_efi_err(sys_table, "Exit boot services failed.\n");
+   pr_efi_err(sys_table_arg, "Exit boot services failed.\n");
 
 fail_free_new_fdt:
-   efi_free(sys_table, MAX_FDT_SIZE, *new_fdt_addr);
+   efi_free(sys_table_arg, MAX_FDT_SIZE, *new_fdt_addr);
 
 fail:
-   sys_table->boottime->free_pool(runtime_map);
+   sys_table_arg->boottime->free_pool(runtime_map);
return EFI_LOAD_ERROR;
 }
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html