Hi Laszlo, 2018-04-16 21:41 GMT+02:00 Laszlo Ersek <ler...@redhat.com>: > On 04/16/18 07:40, Ard Biesheuvel wrote: >> (+ Laszlo) >> >> On 16 April 2018 at 07:09, Marcin Wojtas <m...@semihalf.com> wrote: >>> Recent changes in the EDK2 mainline resulted in breaking >>> of compilation and booting of Armada platforms. >>> This patch adjust the MvFvbDxe driver by: >>> >>> * installation of gEdkiiNvVarStoreFormattedGuid in order to signal >>> NvVarStoreFormattedLib to the generic variable runtime driver >>> >> >> Hello Marcin, >> >> Installing this GUID is only necessary if you update your platform >> .DSC to make the generic variable runtime driver depend on it by >> adding a NULL library class resolution using NvVarStoreFormattedLib. >> So I think this patch is correct, but you'll need an additional change >> to make it work as expected. (Otherwise, the variable runtime driver >> could still be dispatched early and invoked for read access before the >> variable store is reformatted) > > I agree. > > I'd also like to point out another frequent pitfall in this patch: > > > While gBS->InstallProtocolInterface() takes a *pointer* to a handle > (because it can *create* a handle, if the handle is NULL on input, and > the first protocol interface is installed on it), > gBS->UninstallProtocolInterface() takes the handle *itself*. If the last > protocol interface is uninstalled from the handle, then the handle is > destroyed, but gBS->UninstallProtocolInterface() does not attempt to > NULL the handle itself. So, here you should pass "gImageHandle", not > "&gImageHandle". >
Right, I'll correct it. > There's also a bit of whitespace mangling here that's not compatible > with either multiline function call style that we like in edk2, but > perhaps edk2-platforms treats that more laxly. > We had some discussions last year - I followed the coding standards: Function ( Argument1, Argument2, Argument3 ); But was requested to place Argument1 to the function line and the last bracket to Argument3 line: Function (Argument1, Argument2, Argument3); Afair, there were some attempts to modify coding standards at the time, but I see the original version persisted. In fact I can do whatever line-breaking necessary: Ard - what style do you prefer in future patches? Best regards, Marcin > Thanks > Laszlo > >>> +ErrorInstallNvVarStoreFormatted: >>> gBS->UninstallMultipleProtocolInterfaces (&mFvbDevice->Handle, >>> &gEfiDevicePathProtocolGuid, >>> &gEfiFirmwareVolumeBlockProtocolGuid, >>> diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf >>> b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf >>> index 117fe8b..fd3f2f7 100644 >>> --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf >>> +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf >>> @@ -63,6 +63,7 @@ >>> UefiRuntimeServicesTableLib >>> >>> [Guids] >>> + gEdkiiNvVarStoreFormattedGuid >>> gEfiAuthenticatedVariableGuid >>> gEfiEventVirtualAddressChangeGuid >>> gEfiSystemNvDataFvGuid >>> @@ -84,8 +85,4 @@ >>> gMarvellTokenSpaceGuid.PcdSpiMemoryBase >>> >>> [Depex] >>> - # >>> - # MvFvbDxe must be loaded before VariableRuntimeDxe in case empty >>> - # flash needs populating with default values. >>> - # >>> - BEFORE gVariableRuntimeDxeFileGuid >>> + gEfiCpuArchProtocolGuid >>> -- >>> 2.7.4 >>> >> _______________________________________________ >> 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
