Agree. Thanks! > -----Original Message----- > From: Zeng, Star > Sent: Friday, December 09, 2016 6:20 PM > To: Gao, Liming <[email protected]>; [email protected] > Cc: Yao, Jiewen <[email protected]>; Zeng, Star <[email protected]> > Subject: Re: [edk2] [Patch 1/2] MdeModulePkg SmmIpl: Fill Smram range for > SMM driver when LMFA enable > > Liming, > > A small comment added inline. > With that update, Reviewed-by: Star Zeng <[email protected]> to this > patch series. > > On 2016/12/1 14:49, Liming Gao wrote: > > Allocate the additional Smram range to describe the reserved smram for > > SMM core and driver when LMFA feature is enabled. > > > > Cc: Star Zeng <[email protected]> > > Cc: Jiewen Yao <[email protected]> > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Liming Gao <[email protected]> > > --- > > MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 39 > +++++++++++++++++++++++++++++----- > > 1 file changed, 34 insertions(+), 5 deletions(-) > > > > diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c > b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c > > index 18bec84..fd7027d 100644 > > --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c > > +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c > > @@ -973,6 +973,10 @@ ExecuteSmmCoreFromSmram ( > > // Since the memory range to load SMM CORE will be cut out in SMM > core, so no need to allocate and free this range > > // > > PageCount = 0; > > + // > > + // Reserved Smram Region for SmmCore is not used, and remove it > from SmramRangeCount. > > + // > > + gSmmCorePrivate->SmramRangeCount --; > > } else { > > DEBUG ((EFI_D_INFO, "LOADING MODULE FIXED ERROR: Loading > module at fixed address at address failed\n")); > > // > > @@ -1299,6 +1303,7 @@ GetFullSmramRanges ( > > UINTN Index2; > > EFI_SMRAM_DESCRIPTOR *FullSmramRanges; > > UINTN TempSmramRangeCount; > > + UINTN AdditionSmramRangeCount; > > EFI_SMRAM_DESCRIPTOR *TempSmramRanges; > > UINTN SmramRangeCount; > > EFI_SMRAM_DESCRIPTOR *SmramRanges; > > @@ -1332,12 +1337,23 @@ GetFullSmramRanges ( > > } > > } > > > > + // > > + // Reserve one entry for SMM Core in the full SMRAM ranges. > > You have moved comment "Reserve one entry for SMM Core in the full > SMRAM > ranges." to here. > > > + // > > + AdditionSmramRangeCount = 1; > > + if (PcdGet64(PcdLoadModuleAtFixAddressEnable) != 0) { > > + // > > + // Reserve two entries for all SMM drivers and SMM Core in the full > SMRAM ranges. > > + // > > + AdditionSmramRangeCount = 2; > > + } > > + > > if (SmramReservedCount == 0) { > > // > > // No reserved SMRAM entry from SMM Configuration Protocol. > > // Reserve one entry for SMM Core in the full SMRAM ranges. > > Since you have move comment "Reserve one entry for SMM Core in the full > SMRAM ranges." to the code above, how about remove it in this comments? > > Thanks, > Star > > > // > > - *FullSmramRangeCount = SmramRangeCount + 1; > > + *FullSmramRangeCount = SmramRangeCount + > AdditionSmramRangeCount; > > Size = (*FullSmramRangeCount) * sizeof (EFI_SMRAM_DESCRIPTOR); > > FullSmramRanges = (EFI_SMRAM_DESCRIPTOR *) AllocateZeroPool > (Size); > > ASSERT (FullSmramRanges != NULL); > > @@ -1449,10 +1465,9 @@ GetFullSmramRanges ( > > ASSERT (TempSmramRangeCount <= MaxCount); > > > > // > > - // Sort the entries, > > - // and reserve one entry for SMM Core in the full SMRAM ranges. > > + // Sort the entries > > // > > - FullSmramRanges = AllocateZeroPool ((TempSmramRangeCount + 1) * > sizeof (EFI_SMRAM_DESCRIPTOR)); > > + FullSmramRanges = AllocateZeroPool ((TempSmramRangeCount + > AdditionSmramRangeCount) * sizeof (EFI_SMRAM_DESCRIPTOR)); > > ASSERT (FullSmramRanges != NULL); > > *FullSmramRangeCount = 0; > > do { > > @@ -1472,7 +1487,7 @@ GetFullSmramRanges ( > > TempSmramRanges[Index].PhysicalSize = 0; > > } while (*FullSmramRangeCount < TempSmramRangeCount); > > ASSERT (*FullSmramRangeCount == TempSmramRangeCount); > > - *FullSmramRangeCount += 1; > > + *FullSmramRangeCount += AdditionSmramRangeCount; > > > > FreePool (SmramRanges); > > FreePool (SmramReservedRanges); > > @@ -1510,6 +1525,7 @@ SmmIplEntry ( > > EFI_LOAD_FIXED_ADDRESS_CONFIGURATION_TABLE > *LMFAConfigurationTable; > > EFI_CPU_ARCH_PROTOCOL *CpuArch; > > EFI_STATUS SetAttrStatus; > > + EFI_SMRAM_DESCRIPTOR *SmramRangeSmmDriver; > > > > // > > // Fill in the image handle of the SMM IPL so the SMM Core can use this > > as > the > > @@ -1619,6 +1635,19 @@ SmmIplEntry ( > > // > > DEBUG ((EFI_D_INFO, "LOADING MODULE FIXED INFO: TSEG BASE > is %x. \n", LMFAConfigurationTable->SmramBase)); > > } > > + > > + // > > + // Fill the Smram range for all SMM code > > + // > > + SmramRangeSmmDriver = &gSmmCorePrivate- > >SmramRanges[gSmmCorePrivate->SmramRangeCount - 2]; > > + SmramRangeSmmDriver->CpuStart = mCurrentSmramRange- > >CpuStart; > > + SmramRangeSmmDriver->PhysicalStart = mCurrentSmramRange- > >PhysicalStart; > > + SmramRangeSmmDriver->RegionState = mCurrentSmramRange- > >RegionState | EFI_ALLOCATED; > > + SmramRangeSmmDriver->PhysicalSize = SmmCodeSize; > > + > > + mCurrentSmramRange->PhysicalSize -= SmmCodeSize; > > + mCurrentSmramRange->CpuStart = mCurrentSmramRange- > >CpuStart + SmmCodeSize; > > + mCurrentSmramRange->PhysicalStart = mCurrentSmramRange- > >PhysicalStart + SmmCodeSize; > > } > > // > > // Load SMM Core into SMRAM and execute it from SMRAM > >
_______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
