I can't find patch 1 in the series in my email so putting a few
comments here. I really hope this patch series can wait for PRs
so code comments can more easily be correlated with code change.
A few comments for now.
Variable is spelled incorrectly in commit message.
I would really prefer to not mix PCDs and Hobs. It is really
confusing what has to be turned on and set to what to get the
different behaviors. For a hob producer it is OK to take that
info from PCDs but lets not mix in the consumer code PCDs and hob
data. The HOB definition should not reference a PCD and a HOB
definition should not be focused on producer/consumer but on the
data.
The existence of a hob is also a good indicator that a feature is
used so you may not need to have "enable" PCDs anymore.
Please don't include other header files in public header files
(especially for super common headers like PiPei.h. i know over
the last few years this practice has become more common but it
just creates pain when debugging build errors. The trade off is
not worth it.
Hobs really shouldn't use UINTN sizes. Since hobs helps transfer
config state across phases where the size of UINTN may be
different this causes problems.
Hobs used for cross phase sharing shouldn't have pointers for the
reason.
Better and more complete comments on the different field members
would be helpful. I assume the "Buffer" fields are physical
addresses and the "Pages" are number of 4K pages. I would
suggest EFI_PHYSICAL_ADDRESS for addresses and UINT64 for
lengths.
More details on what the three cache's are would be helpful or at
least reference the feature of the cache they are supporting.
This hob definition file isn't perfect and I believe some of the
comment in the file header belong in different places, it is at
least a good template for a hob definition.
Thanks
Sean
This patch set defines a new VARIABLE_RUNTIME_CACHE_INFO HOB. The HOB is used to store the address and size of the buffer that will be used for variable runtime service when the PcdEnableVariableRuntimeCache is TRUE. In following patches, when PcdEnableVariableRuntimeCache is TRUE, VariablePei will install a callback of gEfiPeiMemoryDiscoveredPpiGuid to allocate the needed buffer for different type variable runtime cache and build the HOB. Then VariableSmmRuntimeDxe driver will consume gEdkiiVariableRuntimeCacheInfoHobGuid to initialize the variable runtime cache related content. The code to allocate and unblock the runtime cache buffer in VariableSmmRuntimeDxe is also removed in this patc set. PR for review: https://github.com/tianocore/edk2/pull/5607 Cc: Ray Ni <ray.ni@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Jiaxin Wu <jiaxin.wu@intel.com> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> Cc: Leif Lindholm <quic_llindhol@quicinc.com> Cc: Sami Mujawar <sami.mujawar@arm.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Andrew Fish <afish@apple.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Dun Tan (9): MdeModulePkg:Add new gEdkiiVariableRuntimeCacheInfoHobGuid ArmVirtPkg: Add MmUnblockMemoryLib in DSC EmulatorPkg: Add MmUnblockMemoryLib in DSC OvmfPkg: Add MmUnblockMemoryLib in DSC MdeModulePkg:Create gEdkiiVariableRuntimeCacheInfoHobGuid MdeModulePkg:Remove unnecessary global variable MdeModulePkg:Consume gEdkiiVariableRuntimeCacheInfoHobGuid MdeModulePkg: Refine InitVariableCache() MdeModulePkg:Add global variable mVariableRtCacheInfo ArmVirtPkg/ArmVirtCloudHv.dsc | 2 ++ EmulatorPkg/EmulatorPkg.dsc | 1 + MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ MdeModulePkg/MdeModulePkg.dec | 3 +++ MdeModulePkg/Universal/Variable/Pei/Variable.c | 298 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- MdeModulePkg/Universal/Variable/Pei/Variable.h | 3 +++ MdeModulePkg/Universal/Variable/Pei/VariablePei.inf | 8 +++++++- MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c | 293 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------- MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf | 5 +++-- OvmfPkg/OvmfPkgIa32X64.dsc | 2 +- 10 files changed, 506 insertions(+), 174 deletions(-) create mode 100644 MdeModulePkg/Include/Guid/VariableRuntimeCacheInfo.h