From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E344B81DC3 for ; Fri, 9 Dec 2016 02:20:35 -0800 (PST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga102.fm.intel.com with ESMTP; 09 Dec 2016 02:20:32 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,323,1477983600"; d="scan'208";a="40709959" Received: from shzintpr01.sh.intel.com (HELO [10.253.24.32]) ([10.239.4.80]) by fmsmga005.fm.intel.com with ESMTP; 09 Dec 2016 02:20:31 -0800 To: Liming Gao , edk2-devel@lists.01.org References: <1480574974-22276-1-git-send-email-liming.gao@intel.com> <1480574974-22276-2-git-send-email-liming.gao@intel.com> Cc: Jiewen Yao , star.zeng@intel.com From: "Zeng, Star" Message-ID: <228150f0-f389-b2bc-df5e-e97c42b6ab42@intel.com> Date: Fri, 9 Dec 2016 18:20:01 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1480574974-22276-2-git-send-email-liming.gao@intel.com> Subject: Re: [Patch 1/2] MdeModulePkg SmmIpl: Fill Smram range for SMM driver when LMFA enable X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 09 Dec 2016 10:20:36 -0000 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Liming, A small comment added inline. With that update, Reviewed-by: Star Zeng 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 > Cc: Jiewen Yao > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Liming Gao > --- > 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 >