Re: [edk2-devel] [PATCH v1 20/26] OvmfPkg/LoongArchVirt: Add NorFlashQemuLib

["Gerd Hoffmann" <kraxel@redhat.com>]

On Sat, Mar 16, 2024 at 06:19:00PM +0800, lixianglai wrote:
> Hi Gerd:
> > On Mon, Mar 11, 2024 at 02:39:24AM -0700, Chao Li wrote:
> >> Add NorFlashQemuLib for LoongArch, it is referenced from ArmVirtPkg.
> > What are the differences to the ArmVirtPkg version?
> In this lib we have assigned the following three pcd variables:
> PcdFlashNvStorageVariableBase
> PcdFlashNvStorageFtwWorkingBase
> PcdFlashNvStorageFtwSpareBase
> Instead of hardcoding these three variables in the VarStore.fdf.inc file as arm does,
> the benefit is that when the flash base address changes in the qemu implementation,
> there is no need to re-adapt and compile UEFI.

The flash memory layout (address + size) for the aarch64 virt machine
has never changed.  So while it sounds nice in theory to have that
option it could very well be that this will never ever needed in
practice.

Having sayed that I'd also note that I think it should also be possible
to switch the aarch64 builds to set the PCDs at runtime instead of
compile time.

> When I tried to implement the current patch scheme on aarch64,
> I found that the FaultTolerantWriteDxe driver loaded earlier than VirtNorFlashDxe.
> And It requires the PcdFlashNvStorageFtwWorkingSize and PcdFlashNvStorageFtwSpareSize variables for initialization,
> However the initialization of these two variables is completed in VirtNorFlashDxe,
> The fdf file specifies that VirtNorFlashDxe is loaded first and then FaultTolerantWriteDxe is loaded in loongarch64.
> So this is going to be a problem if we want to apply the current solution to aarch64 or risc-v.

There is a non-obvious twist:

VirtNorFlashDxe registers the gEdkiiNvVarStoreFormattedGuid protocol.

There is the
EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
library, which only purpose is to add a dependency to
gEdkiiNvVarStoreFormattedGuid to depex.

NvVarStoreFormattedLib.inf is used this way ...

  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
    <LibraryClasses>A
      [ ... ]
      NULL|EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
      [ ... ]
  }

... to make sure VariableRuntimeDxe is scheduled after VirtNorFlashDxe.

I think you can apply the same idea to FaultTolerantWriteDxe.

> I can't tell the implementation scheme of the current lib and existing
> lib implementation scheme which one is better, Could you give we some
> advice?

I'd suggest to merge your code as OvmfPkg/Library/FdtNorFlashQemuLib as
it is not really loongarch-specific.

If you want try switch aarch64 to use the same code that'll be great,
but sorting that out later is also fine with me.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116842): https://edk2.groups.io/g/devel/message/116842
Mute This Topic: https://groups.io/mt/104859896/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-