On Tue, 11 Dec 2018 at 13:55, Pankaj Bansal <pankaj.ban...@nxp.com> wrote: > > > > > -----Original Message----- > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > > Sent: Tuesday, December 11, 2018 6:18 PM > > To: Pankaj Bansal <pankaj.ban...@nxp.com> > > Cc: Bhupesh Sharma <bhsha...@redhat.com>; Mark Rutland > > <mark.rutl...@arm.com>; Leif Lindholm <leif.lindh...@linaro.org>; Grant > > Likely <grant.lik...@arm.com>; Varun Sethi <v.se...@nxp.com>; Udit Kumar > > <udit.ku...@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.ban...@nxp.com> wrote: > > > > > > > > > > > > > -----Original Message----- > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > > > > Sent: Tuesday, December 11, 2018 6:02 PM > > > > To: Pankaj Bansal <pankaj.ban...@nxp.com> > > > > Cc: Bhupesh Sharma <bhsha...@redhat.com>; Mark Rutland > > > > <mark.rutl...@arm.com>; Leif Lindholm <leif.lindh...@linaro.org>; > > > > Grant Likely <grant.lik...@arm.com>; Varun Sethi <v.se...@nxp.com>; > > > > Udit Kumar <udit.ku...@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.ban...@nxp.com> > > wrote: > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > > > > > > Sent: Tuesday, December 11, 2018 5:55 PM > > > > > > To: Pankaj Bansal <pankaj.ban...@nxp.com> > > > > > > Cc: Bhupesh Sharma <bhsha...@redhat.com>; Mark Rutland > > > > > > <mark.rutl...@arm.com>; Leif Lindholm > > > > > > <leif.lindh...@linaro.org>; Grant Likely <grant.lik...@arm.com>; > > > > > > Varun Sethi <v.se...@nxp.com>; Udit Kumar <udit.ku...@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.ban...@nxp.com> > > > > wrote: > > > > > > > > -----Original Message----- > > > > > > > > From: Bhupesh Sharma [mailto:bhsha...@redhat.com] > > > > > > > > Sent: Tuesday, December 11, 2018 4:25 PM > > > > > > > > To: Pankaj Bansal <pankaj.ban...@nxp.com> > > > > > > > > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>; Mark Rutland > > > > > > > > <mark.rutl...@arm.com>; Leif Lindholm > > > > > > > > <leif.lindh...@linaro.org>; Grant Likely > > > > > > > > <grant.lik...@arm.com>; Varun Sethi <v.se...@nxp.com>; Udit > > > > > > > > Kumar <udit.ku...@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.
