Re: [platforms PATCH 1/2] Marvell/Drivers: MvFvbDxe: Adjust to new dependencies

[Marcin Wojtas <mw@semihalf.com>]

Hi Ard,

2018-04-16 7:40 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> (+ Laszlo)
>
> On 16 April 2018 at 07:09, Marcin Wojtas <mw@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 added NULL class lib hook for VariableRuntimeDxe and it still works.
I'll send third patch on top of already submitted (the branch is
updated as well).

Thanks,
Marcin


>
>>  * making explicit dependency to ArmPkg/Drivers/CpuDxe drivers in order
>>    to enable successful calling of gDS->SetMemorySpaceAttributes
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> ---
>>  Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c   | 21 ++++++++++++++++++++
>>  Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf |  7 ++-----
>>  2 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c
>> index 252ef67..6e583a3 100644
>> --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c
>> +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c
>> @@ -26,6 +26,7 @@
>>  #include <Library/UefiLib.h>
>>  #include <Library/UefiRuntimeLib.h>
>>
>> +#include <Guid/NvVarStoreFormatted.h>
>>  #include <Guid/SystemNvDataGuid.h>
>>  #include <Guid/VariableFormat.h>
>>
>> @@ -1076,6 +1077,21 @@ MvFvbEntryPoint (
>>    }
>>
>>    //
>> +  // The driver implementing the variable read service can now be dispatched;
>> +  // the varstore headers are in place.
>> +  //
>> +  Status = gBS->InstallProtocolInterface (&gImageHandle,
>> +                  &gEdkiiNvVarStoreFormattedGuid,
>> +                  EFI_NATIVE_INTERFACE,
>> +                  NULL);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR,
>> +      "%a: Failed to install gEdkiiNvVarStoreFormattedGuid\n",
>> +      __FUNCTION__));
>> +    goto ErrorInstallNvVarStoreFormatted;
>> +  }
>> +
>> +  //
>>    // Declare the Non-Volatile storage as EFI_MEMORY_RUNTIME
>>    //
>>    RuntimeMmioRegionSize = mFvbDevice->FvbSize;
>> @@ -1126,6 +1142,11 @@ ErrorSetMemAttr:
>>    gDS->RemoveMemorySpace (RegionBaseAddress, RuntimeMmioRegionSize);
>>
>>  ErrorAddSpace:
>> +  gBS->UninstallProtocolInterface (&gImageHandle,
>> +                  &gEdkiiNvVarStoreFormattedGuid,
>> +                  NULL);
>> +
>> +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
>>