Re: [edk2] 'fastboot boot' TPL
On 02/28/18 09:19, Ard Biesheuvel wrote: > On 28 February 2018 at 08:15, Michael Zimmermann >wrote: >>> I agree. Did you run into any issues due to this? >> Surprisingly no. I was just trying to understand the fastboot implementation >> before I use it on my platform >> and was surprised that this works at all. I can imagine that's because this >> is supposed to load linux's efistub which probably doesn't do anything but >> calling SetVirtualMemoryMap and ExitBootServices. > > The ARM/Linux EFI stub does quite a bit more than that, actually. It > uses the various memory allocation and protocol handling services, to > carve out an allocation for the kernel, initrd and device tree, and to > access the command line, the EFI_RNG_PROTOCOL (if available) and to > interrogate the protocol database for GOP instances. > >> I can imagine that more complex loaders like the one used for Windows >> wouldn't work this way. >> > > No, and this is indeed something that should be fixed. Any clue as to how? - Change the type of the event "ReceiveEvent" from EVT_NOTIFY_SIGNAL to zero (0) - remove the DataReady() function - continue passing "ReceiveEvent" to mTransport->Start() - in the WaitForEvent() call near the end of FastbootAppEntryPoint(), also wait for "ReceiveEvent" - whenever "ReceiveEvent" fires, implement the same logic as seen originally in DataReady(), but now near the end of FastbootAppEntryPoint(). The idea -- already used with "mFinishedEvent" BTW -- is that an event can have type 0, which means it can be signaled and waited for without ever queueing a notification function (at either signaling or waiting). So mTransport should continue signaling ReceiveEvent whenever data is ready. But, readiness should be observed in the "main loop", and the data should be read, parsed and acted upon in that context as well, not in a callback. Thanks, Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] 'fastboot boot' TPL
Well since the fastboot implementation already is an application (and not a driver) all we need to do is to use WaitEvent instead of a notify callback. Once that's fixed I'd add ASSERTs on the current tpl to the relevant API functions so you know immediately when you try to violate the spec. On Feb 28, 2018 9:19 AM, "Ard Biesheuvel"wrote: > On 28 February 2018 at 08:15, Michael Zimmermann > wrote: > >> I agree. Did you run into any issues due to this? > > Surprisingly no. I was just trying to understand the fastboot > implementation > > before I use it on my platform > > and was surprised that this works at all. I can imagine that's because > this > > is supposed to load linux's efistub which probably doesn't do anything > but > > calling SetVirtualMemoryMap and ExitBootServices. > > The ARM/Linux EFI stub does quite a bit more than that, actually. It > uses the various memory allocation and protocol handling services, to > carve out an allocation for the kernel, initrd and device tree, and to > access the command line, the EFI_RNG_PROTOCOL (if available) and to > interrogate the protocol database for GOP instances. > > > I can imagine that more complex loaders like the one used for Windows > > wouldn't work this way. > > > > No, and this is indeed something that should be fixed. Any clue as to how? > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] 'fastboot boot' TPL
On 28 February 2018 at 08:15, Michael Zimmermannwrote: >> I agree. Did you run into any issues due to this? > Surprisingly no. I was just trying to understand the fastboot implementation > before I use it on my platform > and was surprised that this works at all. I can imagine that's because this > is supposed to load linux's efistub which probably doesn't do anything but > calling SetVirtualMemoryMap and ExitBootServices. The ARM/Linux EFI stub does quite a bit more than that, actually. It uses the various memory allocation and protocol handling services, to carve out an allocation for the kernel, initrd and device tree, and to access the command line, the EFI_RNG_PROTOCOL (if available) and to interrogate the protocol database for GOP instances. > I can imagine that more complex loaders like the one used for Windows > wouldn't work this way. > No, and this is indeed something that should be fixed. Any clue as to how? ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] 'fastboot boot' TPL
> I agree. Did you run into any issues due to this? Surprisingly no. I was just trying to understand the fastboot implementation before I use it on my platform and was surprised that this works at all. I can imagine that's because this is supposed to load linux's efistub which probably doesn't do anything but calling SetVirtualMemoryMap and ExitBootServices. I can imagine that more complex loaders like the one used for Windows wouldn't work this way. On Wed, Feb 28, 2018 at 9:10 AM, Ard Biesheuvelwrote: > On 28 February 2018 at 07:56, Michael Zimmermann > wrote: > > I feel like both of you misunderstood my intention. > > As I said in my initial mail, I'm not arguing the spec - I know that > > StartImage must be called from TPL_APPLICATION. > > > > I'm just discussing a bug inside EmbeddedPkg/Application/AndroidFastboot > - > > because they actually do call StartImage from TPL_CALLBACK. > > I think the confusion is a result of the fact that you never mentioned > that particular module/file until now. > > > As I proposed in my initial mail, we should not only fix that, but also > add > > tpl ASSERTs inside several BootServices to prevent this from happening in > > future. > > > > I agree. Did you run into any issues due to this? > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] 'fastboot boot' TPL
On 28 February 2018 at 07:56, Michael Zimmermannwrote: > I feel like both of you misunderstood my intention. > As I said in my initial mail, I'm not arguing the spec - I know that > StartImage must be called from TPL_APPLICATION. > > I'm just discussing a bug inside EmbeddedPkg/Application/AndroidFastboot - > because they actually do call StartImage from TPL_CALLBACK. I think the confusion is a result of the fact that you never mentioned that particular module/file until now. > As I proposed in my initial mail, we should not only fix that, but also add > tpl ASSERTs inside several BootServices to prevent this from happening in > future. > I agree. Did you run into any issues due to this? ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] 'fastboot boot' TPL
I feel like both of you misunderstood my intention. As I said in my initial mail, I'm not arguing the spec - I know that StartImage must be called from TPL_APPLICATION. I'm just discussing a bug inside EmbeddedPkg/Application/AndroidFastboot - because they actually do call StartImage from TPL_CALLBACK. As I proposed in my initial mail, we should not only fix that, but also add tpl ASSERTs inside several BootServices to prevent this from happening in future. Thanks Michael On Wed, Feb 28, 2018 at 8:42 AM, Andrew Fishwrote: > Violating the spec is undefined behavior. If it works that is bad luck, or > good luck depending on your point of view. > > Sent from my iPhone > > > On Feb 27, 2018, at 11:33 PM, Michael Zimmermann < > sigmaepsilo...@gmail.com> wrote: > > > > Are you sure? > > > > If you look at this file: > > https://github.com/tianocore/edk2/blob/master/EmbeddedPkg/ > Application/AndroidFastboot/AndroidFastbootApp.c > > > > The DataReady Event is a TPL_CALLBACK event. > > From there the call chain goes as follows: > > AcceptCmd -> HandleBoot -> BootAndroidBootImg -> StartEfiApplication -> > > "gBS->StartImage" > > > > Thanks > > Michael > > > >> On Wed, Feb 28, 2018 at 8:29 AM, Ni, Ruiyu wrote: > >> > >>> On 2/28/2018 2:06 PM, Michael Zimmermann wrote: > >>> > >>> From looking at the code it seems to me that StartImage is called from > >>> TPL_CALLBACK. > >>> According to the Spec StartImage can only be called from >>> > >>> If the current code actually works it means that there are at least 3 > >>> problems that should be addressed: > >>> - call StartImage from TPL_APPLICATION > >>> - ASSERT the tpl in LoadImage and StartImage > >>> - ASSERT the tpl in ExitBootServices > >>> > >>> Thanks > >>> Michael > >>> ___ > >>> edk2-devel mailing list > >>> edk2-devel@lists.01.org > >>> https://lists.01.org/mailman/listinfo/edk2-devel > >>> > >>> NO, LoadImage and StartImage are called at TPL_APPLICATION. > >> > >> -- > >> Thanks, > >> Ray > >> > > ___ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] 'fastboot boot' TPL
Violating the spec is undefined behavior. If it works that is bad luck, or good luck depending on your point of view. Sent from my iPhone > On Feb 27, 2018, at 11:33 PM, Michael Zimmermann> wrote: > > Are you sure? > > If you look at this file: > https://github.com/tianocore/edk2/blob/master/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c > > The DataReady Event is a TPL_CALLBACK event. > From there the call chain goes as follows: > AcceptCmd -> HandleBoot -> BootAndroidBootImg -> StartEfiApplication -> > "gBS->StartImage" > > Thanks > Michael > >> On Wed, Feb 28, 2018 at 8:29 AM, Ni, Ruiyu wrote: >> >>> On 2/28/2018 2:06 PM, Michael Zimmermann wrote: >>> >>> From looking at the code it seems to me that StartImage is called from >>> TPL_CALLBACK. >>> According to the Spec StartImage can only be called from >> >>> If the current code actually works it means that there are at least 3 >>> problems that should be addressed: >>> - call StartImage from TPL_APPLICATION >>> - ASSERT the tpl in LoadImage and StartImage >>> - ASSERT the tpl in ExitBootServices >>> >>> Thanks >>> Michael >>> ___ >>> edk2-devel mailing list >>> edk2-devel@lists.01.org >>> https://lists.01.org/mailman/listinfo/edk2-devel >>> >>> NO, LoadImage and StartImage are called at TPL_APPLICATION. >> >> -- >> Thanks, >> Ray >> > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] 'fastboot boot' TPL
Are you sure? If you look at this file: https://github.com/tianocore/edk2/blob/master/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c The DataReady Event is a TPL_CALLBACK event. >From there the call chain goes as follows: AcceptCmd -> HandleBoot -> BootAndroidBootImg -> StartEfiApplication -> "gBS->StartImage" Thanks Michael On Wed, Feb 28, 2018 at 8:29 AM, Ni, Ruiyuwrote: > On 2/28/2018 2:06 PM, Michael Zimmermann wrote: > >> From looking at the code it seems to me that StartImage is called from >> TPL_CALLBACK. >> According to the Spec StartImage can only be called from > >> If the current code actually works it means that there are at least 3 >> problems that should be addressed: >> - call StartImage from TPL_APPLICATION >> - ASSERT the tpl in LoadImage and StartImage >> - ASSERT the tpl in ExitBootServices >> >> Thanks >> Michael >> ___ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel >> >> NO, LoadImage and StartImage are called at TPL_APPLICATION. > > -- > Thanks, > Ray > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] 'fastboot boot' TPL
>From looking at the code it seems to me that StartImage is called from TPL_CALLBACK. According to the Spec StartImage can only be called from https://lists.01.org/mailman/listinfo/edk2-devel