On March 1, 2022 9:10 PM, Gerd Hoffmann wrote: > On Mon, Feb 28, 2022 at 03:20:51PM +0800, Min Xu wrote: > > Below functions are introduced in PlatformInitLib: > > - PlatformGetFirstNonAddress > > - PlatformAddressWidthInitialization > > - PlatformGetSystemMemorySizeBelow4gb > > - PlatformQemuUc32BaseInitialization > > - PlatformInitializeRamRegions > > > > They correspond to the below functions in OvmfPkg/PlatformPei: > > - GetFirstNonAddress > > - AddressWidthInitialization > > - GetSystemMemorySizeBelow4gb > > - QemuUc32BaseInitialization > > - InitializeRamRegions > > > > After that OvmfPkg/PlatformPei is refactored with this library. > > > > Note: PlatformInitLib will not determine whether SMM or S3 is > > supported or not. Instead the caller of these functions should input > > SMM / S3 support as the IN parameter by themselves. This is to reduce > > the complexity of PlatformInitLib. Another reason is that some PCDs > > cannot be declared as FixedAtBuild while PlatformInitLib is designed > > to be used in both SEC and PEI phase. > > Hmm. Unlike patches 17+18 which are pure code motion (except the > function renaming but that doesn't change the workflow) this patch mixes > code changes and code moving which makes it hard to review. > > It should be splitted into one (or more) patches changing the functions as > needed (and keeping the code in PlatformPei), and one patch moving things > over to PlatformInitLib without functional changes. Ok. Looks like #21 & #22 in tdvf_wave2.v6? https://github.com/mxu9/edk2/commit/ef0615ca5665b2058e4352a322dfa74d258f9f31 https://github.com/mxu9/edk2/commit/25f356a0bf7ee347be30e270aeffe6cbd8e0b464 > > > +PlatformGetFirstNonAddress ( > > + OUT UINT64 *Pci64Base, > > + OUT UINT64 *Pci64Size, > > + IN UINT64 DefaultPciMmio64Size > > + ) > > > +VOID > > +QemuInitializeRam ( > > + UINT32 Uc32Base, > > + UINT16 HostBridgeDevId, > > + EFI_BOOT_MODE BootMode, > > + BOOLEAN SmmSmramRequire, > > + UINT32 LowerMemorySize, > > + UINT16 Q35TsegMbytes > > + ) > > > +VOID > > +EFIAPI > > +PlatformInitializeRamRegions ( > > + IN UINT32 Uc32Base, > > + IN UINT16 HostBridgeDevId, > > + IN BOOLEAN SmmSmramRequire, > > + IN EFI_BOOT_MODE BootMode, > > + IN BOOLEAN S3Supported, > > + IN UINT32 LowerMemorySize, > > + IN UINT16 Q35TsegMbytes > > + ) > > I think we should add all those parameters to the PLATFORM_INFO struct, > then simply pass around a pointer to that struct instead of having tons of > parameters for each function. Ok. It will be updated in the next version. > > Due to this patch needing an update anyway it might be easier to do it right > away instead of an incremental cleanup after merging this series. > > > @@ -85,7 +87,7 @@ MemMapInitialization ( > > return; > > } > > > > - TopOfLowRam = GetSystemMemorySizeBelow4gb (); > > + TopOfLowRam = PlatformGetSystemMemorySizeBelow4gb (); This is in function MemMapInitialization(). > > PciExBarBase = 0; > > if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) { > > // > > @@ -736,6 +738,11 @@ InitializePlatform ( > > Q35SmramAtDefaultSmbaseInitialization (); > > } > > > > + // > > + // Fetch the lower memory size (Below 4G) // mLowerMemorySize = > > + PlatformGetSystemMemorySizeBelow4gb (); This is in function InitializePlatform(). > > Can't you just use TopOfLowRam here? TopOfLowRam is a local variable in function MemMapInitialization(). It cannot be used in function InitialziePlatform().
Thanks Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87178): https://edk2.groups.io/g/devel/message/87178 Mute This Topic: https://groups.io/mt/89446168/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
