Re: Boot failure for ArmVExpress-FVP-AArch64, CpuArch protocol does not appear to be registered

["Gao, Liming" <liming.gao@intel.com>]

Laszlo:
  On variable storage format are hex, could you submit one bugzillar for it? I have some idea to simplify its description. We can reuse FV section for Variable storage. Its FV section is like below. 

[FV.NVStorage]
FileSystemGuid = gEfiSystemNvDataFvGuid
VariableSignatureGuid = gEfiAuthenticatedVariableGuid
VariabeRegionSize = 0x40000
FtwWorkingSpaceSize = 0x40000
FtwSpareSpaceSize = 0x40000

Thanks
Liming
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
> Sent: Wednesday, April 11, 2018 8:26 PM
> To: Steve Capper <steve.capper@linaro.org>; edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Leif Lindholm <leif.lindholm@linaro.org>; Nariman Poushin
> <nariman.poushin@linaro.org>
> Subject: Re: [edk2] Boot failure for ArmVExpress-FVP-AArch64, CpuArch protocol does not appear to be registered
> 
> Hi Steve,
> 
> On 04/11/18 12:30, Steve Capper wrote:
> > Hello,
> > I am trying to debug a boot problem for a recent build of EDK2 in an
> > Arm FVP model.
> >
> > I built EDK2 from the following branches:
> > Edk2: a146c532c754106431b063fec9985a838afd82be
> > Edk2-platforms: e74f53df8b18e4aed0c9df0942ce0c30f78a68e2
> >
> > Toolchain: Debian Stretch latest (though I have also tried Linaro\x19s
> > GCC 5 latest).
> >
> > Build command:
> > GCC5_AARCH64_PREFIX=aarch64-linux-gnu- build -a AARCH64 -t GCC5 -b
> > DEBUG -p Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> >
> > The boot log can be found in the attached file. I *think* this is due
> > to the Cpu Arch protocol not actually being ready when
> > CoreConvertSpace is called by the SetMemoryAttributes from the
> > NorFlash code.
> >
> > I have found that this code path fails:
> >
> https://github.com/tianocore/edk2/blob/a146c532c754106431b063fec9985a838afd82be/MdeModulePkg/Core/Dxe/Gcd/Gcd.c#L893
> >
> > The only modification I have made is to edk2-platforms to get more
> > debug output (PcdDebugPrintErrorLevel).
> >
> > Unfortunately, I'm not sure how to fix this?
> 
> This is a bit of a problem.
> 
> The gDS->SetMemorySpaceAttributes() DXE service is allowed to fail if
> the CPU architectural protocol has not been installed yet. This is
> documented in the Platform Init spec (although riddled by a few typos --
> EFI_NOT_AVAILABLE_YET is documented under the preceding
> GetMemorySpaceDescriptor() section, and not in the correct
> SetMemorySpaceAttributes() section).
> 
> The usual solution for DXE drivers is to add an explicit dependency on
> gEfiCpuArchProtocolGuid to the DEPEX section in the INF file. You can
> see an example in commit f9a8be423cdd
> ("ArmVirtualizationPkg/PciHostBridgeDxe: MMIO aperture must not be
> uncached", 2015-02-23).
> 
> And that's where the issue is. If you check the [Depex] section in
> "ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf", you find
> 
> > [Depex]
> >   #
> >   # NorFlashDxe must be loaded before VariableRuntimeDxe in case empty flash needs populating with default values
> >   #
> >   BEFORE gVariableRuntimeDxeFileGuid
> 
> Why is BEFORE technically an issue here? Because, it cannot be combined
> with other depex opcodes. From the PI spec:
> 
> > If this opcode is present in a dependency expression, it must be the
> > first and only opcode in the expression. If is appears in any other
> > location in the dependency expression, then the dependency expression
> > is evaluated to FALSE.
> 
> So we can't simply combine the existent BEFORE depex with "AND
> gEfiCpuArchProtocolGuid", like you see in the commit f9a8be423cdd
> example above.
> 
> Now, more to the point, a "BEFORE" depex is *always* a hack. From the PI
> spec:
> 
> > This opcode tells the DXE Dispatcher that the DXE driver that is
> > associated with this dependency expression must be dispatched just
> > before the DXE driver with the file name specified by GUID. This means
> > that as soon as the dependency expression for the DXE driver specified
> > by GUID evaluates to TRUE, then this DXE driver must be placed in the
> > dispatch queue just before the DXE driver with the file name specified
> > by GUID.
> 
> Sometimes there's really no way around such a hack, to express an
> inter-module dependency, but even then, there is a better way to
> implement it. Namely,
> - a custom protocol GUID can be invented,
> - the pre-requisite module can install a NULL protocol interface with
>   that GUID,
> - and
>   - either the protocol GUID can be added directly to the [depex]
>     section of the dependent module (if the latter module is under the
>     platform's control),
>   - or the platform DSC file can hook a NULL-class library instance into
>     the dependent module, such that the lib instance makes the dependent
>     module *inherit* a [depex] on the protocol GUID.
> 
> In other words, if we want to order the dispatch of a
> platform-independent module (that lives in another Package), due to
> platform-specific circumstances, behind another module that our platform
> owns, then we can change our platform DSC to "imbue" the module with a
> dependency on a custom protocol GUID. We can then install that NULL
> protocol in our own module. Under this scheme the usual dependency
> resolution will order things the right way, and all the depex-es remain
> composable with other GUIDs.
> 
> OK -- so *should* we rewrite this BEFORE depex, like described above? I
> don't think so. In my opinion, the *idea* of this dependency is wrong.
> The BEFORE depex was introduced in commit 6acb379fbcf8
> ("ArmPlatformPkg/CTA9x4: Remove Variable Storage FD file from FDF",
> 2011-03-31), and the idea there was to replace the build-time
> pre-formatting of the flash variable store with runtime formatting. You
> might ask why this replacement needed an explicit ordering hint between
> drivers (i.e. why any DEPEXes had to be touched at all). Here's why:
> 
> - For *writing* non-volatile UEFI variables, a platform-independent
>   protocol dependency chain already exists. At the least abstract level,
>   we have the FVB (firmware volume block) protocol service, e.g.
>   NorFlashDxe. At the next level, we have the platform-independent FTW
>   (fault tolerant write) driver. Finally, at the top (the most abstract
>   level), we have the non-volatile variable write service / driver.
> 
> - However, for *reading* non-volatile UEFI variables, no such dependency
>   chain exists. The variable driver (the top level from the previous
>   bullet) will just go ahead and read various headers from the flash
>   chip. If those headers are not correctly formatted, the variable
>   driver blows up and the platform will not boot. To solve this issue,
>   platform FDF files usually pre-format a variable store template at
>   build-time, which will at once satisfy the "read-side startup" of the
>   variable driver.
> 
> Unfortunately, this requires FDF files to contain various ugly hexdumps,
> and people are tempted to replace those with runtime formatting of the
> varstore headers. In turn though, they have to employ hacks for ordering
> the variable driver behind their runtime formatting. Hence the BEFORE
> depex. :(
> 
> So, what can we do? Given that NorFlashDxe is used by several platforms,
> the ugly BEFORE depex should be removed. Once BEFORE is removed, you can
> simply add "gEfiCpuArchProtocolGuid", and then the current issue will be
> solved. But, what shall take the place of BEFORE?
> 
> Using ArmVirtQemu as an example platform, the answer is "nothing",
> because ArmVirtQemu already pre-formats the varstore template -- please
> see "ArmVirtPkg/VarStore.fdf.inc". For all other platforms that don't do
> this -- and I don't see
> "Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.fdf" doing it --,
> there are two choices:
> 
> - build a varstore template in *all* of the platform FDF files where the
>   platform DSC file includes the NorFlashDxe driver -- in this case, the
>   BEFORE needs no replacement, it can simply be dropped,
> 
> - or implement the custom NULL protocol trick that I described above.
>   Platforms that don't actually *depend* on this trick, such as
>   ArmVirtQemu, will simply ignore the new protocol instance in the
>   protocol database.
> 
> For the second option, there are several examples under OvmfPkg and
> ArmVirtPkg. One is:
> 
> - "MdeModulePkg/Include/Guid/PlatformHasAcpi.h" defines the custom
>   "gEdkiiPlatformHasAcpiGuid".
> 
> - "ArmVirtPkg/ArmVirtQemu.dsc" includes the
>   "ArmVirtPkg/PlatformHasAcpiDtDxe" platform driver, which installs a
>   NULL protocol interface with the above custom GUID under certain
>   circumstances.
> 
> - "EmbeddedPkg/Library/PlatformHasAcpiLib" is a library instance that
>   does nothing; it just has an empty constructor function, and a
>   dependency on the GUID.
> 
> - "ArmVirtPkg/ArmVirt.dsc.inc" hooks the library into
>   "MdeModulePkg/Universal/Acpi/AcpiTableDxe", imbuing the latter with a
>   dependency on the protocol GUID.
> 
> As a result, "MdeModulePkg/Universal/Acpi/AcpiTableDxe" will not be
> dispatched until (and, unless!) "ArmVirtPkg/PlatformHasAcpiDtDxe"
> installs the GUID. If the GUID is never installed, then
> "MdeModulePkg/Universal/Acpi/AcpiTableDxe" will never be dispatched.
> 
> Summary:
> (1) introduce a custom GUID for "NorFlashDxe has formatted the variable
>     store headers for the variable driver to read";
> (2) install the GUID in NorFlashDxe once it's done verifying and/or
>     formatting the headers;
> (3) introduce a custom library instance with an empty constructor
>     function, and a DEPEX on the GUID;
> (4) hook the library instance into
>     "MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf"
>     in all the platform DSC files where the platform build does not
>     pre-format a varstore template in the FDF file;
> (5) replace the BEFORE depex with gEfiCpuArchProtocolGuid.
> 
> Thanks,
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel