> -----Original Message----- > From: Ard Biesheuvel [mailto:[email protected]] > Sent: Tuesday, December 11, 2018 6:31 PM > To: Pankaj Bansal <[email protected]> > Cc: Bhupesh Sharma <[email protected]>; Mark Rutland > <[email protected]>; Leif Lindholm <[email protected]>; Grant > Likely <[email protected]>; Varun Sethi <[email protected]>; Udit Kumar > <[email protected]>; linux-efi <[email protected]> > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt in > configuration table > > On Tue, 11 Dec 2018 at 13:55, Pankaj Bansal <[email protected]> wrote: > > > > > > > > > -----Original Message----- > > > From: Ard Biesheuvel [mailto:[email protected]] > > > Sent: Tuesday, December 11, 2018 6:18 PM > > > To: Pankaj Bansal <[email protected]> > > > Cc: Bhupesh Sharma <[email protected]>; Mark Rutland > > > <[email protected]>; Leif Lindholm <[email protected]>; > > > Grant Likely <[email protected]>; Varun Sethi <[email protected]>; > > > Udit Kumar <[email protected]>; linux-efi > > > <[email protected]> > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new fdt > > > in configuration table > > > > > > On Tue, 11 Dec 2018 at 13:44, Pankaj Bansal <[email protected]> > wrote: > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Ard Biesheuvel [mailto:[email protected]] > > > > > Sent: Tuesday, December 11, 2018 6:02 PM > > > > > To: Pankaj Bansal <[email protected]> > > > > > Cc: Bhupesh Sharma <[email protected]>; Mark Rutland > > > > > <[email protected]>; Leif Lindholm > > > > > <[email protected]>; Grant Likely <[email protected]>; > > > > > Varun Sethi <[email protected]>; Udit Kumar <[email protected]>; > > > > > linux-efi <[email protected]> > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install new > > > > > fdt in configuration table > > > > > > > > > > On Tue, 11 Dec 2018 at 13:29, Pankaj Bansal > > > > > <[email protected]> > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: Ard Biesheuvel [mailto:[email protected]] > > > > > > > Sent: Tuesday, December 11, 2018 5:55 PM > > > > > > > To: Pankaj Bansal <[email protected]> > > > > > > > Cc: Bhupesh Sharma <[email protected]>; Mark Rutland > > > > > > > <[email protected]>; Leif Lindholm > > > > > > > <[email protected]>; Grant Likely > > > > > > > <[email protected]>; Varun Sethi <[email protected]>; Udit > > > > > > > Kumar <[email protected]>; linux-efi > > > > > > > <[email protected]> > > > > > > > Subject: Re: [PATCH v2 2/2] drivers: firmware: efi: install > > > > > > > new fdt in configuration table > > > > > > > > > > > > > > On Tue, 11 Dec 2018 at 13:22, Pankaj Bansal > > > > > > > <[email protected]> > > > > > wrote: > > > > > > > > > -----Original Message----- > > > > > > > > > From: Bhupesh Sharma [mailto:[email protected]] > > > > > > > > > Sent: Tuesday, December 11, 2018 4:25 PM > > > > > > > > > To: Pankaj Bansal <[email protected]> > > > > > > > > > Cc: Ard Biesheuvel <[email protected]>; Mark > > > > > > > > > Rutland <[email protected]>; Leif Lindholm > > > > > > > > > <[email protected]>; Grant Likely > > > > > > > > > <[email protected]>; Varun Sethi <[email protected]>; > > > > > > > > > Udit Kumar <[email protected]>; > > > > > > > > > [email protected] > > > > > > > > > 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.
