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

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

 




> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@xxxxxxxxxx]
> Sent: Tuesday, December 11, 2018 6:31 PM
> To: Pankaj Bansal <pankaj.bansal@xxxxxxx>
> Cc: Bhupesh Sharma <bhsharma@xxxxxxxxxx>; Mark Rutland
> <mark.rutland@xxxxxxx>; Leif Lindholm <leif.lindholm@xxxxxxxxxx>; Grant
> Likely <grant.likely@xxxxxxx>; Varun Sethi <V.Sethi@xxxxxxx>; Udit Kumar
> <udit.kumar@xxxxxxx>; linux-efi <linux-efi@xxxxxxxxxxxxxxx>
> 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@xxxxxxx> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@xxxxxxxxxx]
> > > Sent: Tuesday, December 11, 2018 6:18 PM
> > > To: Pankaj Bansal <pankaj.bansal@xxxxxxx>
> > > Cc: Bhupesh Sharma <bhsharma@xxxxxxxxxx>; Mark Rutland
> > > <mark.rutland@xxxxxxx>; Leif Lindholm <leif.lindholm@xxxxxxxxxx>;
> > > Grant Likely <grant.likely@xxxxxxx>; Varun Sethi <V.Sethi@xxxxxxx>;
> > > Udit Kumar <udit.kumar@xxxxxxx>; linux-efi
> > > <linux-efi@xxxxxxxxxxxxxxx>
> > > 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@xxxxxxx>
> wrote:
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@xxxxxxxxxx]
> > > > > Sent: Tuesday, December 11, 2018 6:02 PM
> > > > > To: Pankaj Bansal <pankaj.bansal@xxxxxxx>
> > > > > Cc: Bhupesh Sharma <bhsharma@xxxxxxxxxx>; Mark Rutland
> > > > > <mark.rutland@xxxxxxx>; Leif Lindholm
> > > > > <leif.lindholm@xxxxxxxxxx>; Grant Likely <grant.likely@xxxxxxx>;
> > > > > Varun Sethi <V.Sethi@xxxxxxx>; Udit Kumar <udit.kumar@xxxxxxx>;
> > > > > linux-efi <linux-efi@xxxxxxxxxxxxxxx>
> > > > > 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@xxxxxxx>
> > > wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@xxxxxxxxxx]
> > > > > > > Sent: Tuesday, December 11, 2018 5:55 PM
> > > > > > > To: Pankaj Bansal <pankaj.bansal@xxxxxxx>
> > > > > > > Cc: Bhupesh Sharma <bhsharma@xxxxxxxxxx>; Mark Rutland
> > > > > > > <mark.rutland@xxxxxxx>; Leif Lindholm
> > > > > > > <leif.lindholm@xxxxxxxxxx>; Grant Likely
> > > > > > > <grant.likely@xxxxxxx>; Varun Sethi <V.Sethi@xxxxxxx>; Udit
> > > > > > > Kumar <udit.kumar@xxxxxxx>; linux-efi
> > > > > > > <linux-efi@xxxxxxxxxxxxxxx>
> > > > > > > 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@xxxxxxx>
> > > > > wrote:
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Bhupesh Sharma [mailto:bhsharma@xxxxxxxxxx]
> > > > > > > > > Sent: Tuesday, December 11, 2018 4:25 PM
> > > > > > > > > To: Pankaj Bansal <pankaj.bansal@xxxxxxx>
> > > > > > > > > Cc: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>; Mark
> > > > > > > > > Rutland <mark.rutland@xxxxxxx>; Leif Lindholm
> > > > > > > > > <leif.lindholm@xxxxxxxxxx>; Grant Likely
> > > > > > > > > <grant.likely@xxxxxxx>; Varun Sethi <V.Sethi@xxxxxxx>;
> > > > > > > > > Udit Kumar <udit.kumar@xxxxxxx>;
> > > > > > > > > linux-efi@xxxxxxxxxxxxxxx
> > > > > > > > > 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.




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

  Powered by Linux