From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (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 B686E81F2B for ; Tue, 6 Dec 2016 21:28:26 -0800 (PST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP; 06 Dec 2016 21:28:26 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,310,1477983600"; d="scan'208";a="1068960548" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga001.jf.intel.com with ESMTP; 06 Dec 2016 21:28:23 -0800 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 6 Dec 2016 21:27:51 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx110.amr.corp.intel.com (10.18.116.10) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 6 Dec 2016 21:27:50 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.239]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.96]) with mapi id 14.03.0248.002; Wed, 7 Dec 2016 13:27:48 +0800 From: "Fan, Jeff" To: "Yao, Jiewen" , "edk2-devel@lists.01.org" CC: "Kinney, Michael D" , Laszlo Ersek Thread-Topic: [PATCH] MdeModulePkg/PiSmmCore: MemoryAttributeTable need keep non-PE record. Thread-Index: AQHSS6w8z5zDAs1xtkuNRmSUNMM786D7/azg Date: Wed, 7 Dec 2016 05:27:47 +0000 Message-ID: <542CF652F8836A4AB8DBFAAD40ED192A4A2F5B2C@shsmsx102.ccr.corp.intel.com> References: <1480580607-19928-1-git-send-email-jiewen.yao@intel.com> <1480580607-19928-2-git-send-email-jiewen.yao@intel.com> In-Reply-To: <1480580607-19928-2-git-send-email-jiewen.yao@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNDRmMmUyNjktZTRmMS00MDNiLWFhMDUtZjkwZDVhZmViNjlhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IlNQYTJTV0V1aEc2dnUwOHZmTFp4bmxQZUlhbU1tM3dmKzIyVUU4Y3lXZ3M9In0= x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] MdeModulePkg/PiSmmCore: MemoryAttributeTable need keep non-PE record. 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: Wed, 07 Dec 2016 05:28:26 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Jeff Fan -----Original Message----- From: Yao, Jiewen=20 Sent: Thursday, December 01, 2016 4:23 PM To: edk2-devel@lists.01.org Cc: Fan, Jeff; Kinney, Michael D; Laszlo Ersek Subject: [PATCH] MdeModulePkg/PiSmmCore: MemoryAttributeTable need keep non= -PE record. Current memory attribute table implementation will only mark PE code to be = EfiRuntimeServicesCode, and mark rest to be EfiRuntimeServicesData. However, there might be a case that a SMM code wants to allocate EfiRuntime= ServicesCode explicitly to let page table protect this region to be read on= ly. It is unsupported. This patch enhances the current solution so that MemoryAttributeTable does = not touch non PE image record. Only the PE image region is forced to be EfiRuntimeServicesCode for code an= d EfiRuntimeServicesData for data. Cc: Jeff Fan Cc: Michael D Kinney Cc: Laszlo Ersek Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jiewen Yao --- MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 91 +++++++++++------= --- 1 file changed, 51 insertions(+), 40 deletions(-) diff --git a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c b/MdeModul= ePkg/Core/PiSmmCore/MemoryAttributesTable.c index 3a5a2c8..a6ab830 100644 --- a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c +++ b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c @@ -268,15 +268,19 @@ EnforceMemoryMapAttribute ( MemoryMapEntry =3D MemoryMap; MemoryMapEnd =3D (EFI_MEMORY_DESCRIPTOR *) ((UINT8 *) MemoryMap + Memo= ryMapSize); while ((UINTN)MemoryMapEntry < (UINTN)MemoryMapEnd) { - switch (MemoryMapEntry->Type) { - case EfiRuntimeServicesCode: - MemoryMapEntry->Attribute |=3D EFI_MEMORY_RO; - break; - case EfiRuntimeServicesData: - MemoryMapEntry->Attribute |=3D EFI_MEMORY_XP; - break; + if (MemoryMapEntry->Attribute !=3D 0) { + // It is PE image, the attribute is already set. + } else { + switch (MemoryMapEntry->Type) { + case EfiRuntimeServicesCode: + MemoryMapEntry->Attribute =3D EFI_MEMORY_RO; + break; + case EfiRuntimeServicesData: + default: + MemoryMapEntry->Attribute |=3D EFI_MEMORY_XP; + break; + } } - MemoryMapEntry =3D NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorS= ize); } =20 @@ -358,6 +362,21 @@ SetNewRecord ( PhysicalEnd =3D TempRecord.PhysicalStart + EfiPagesToSize(TempRecord.Num= berOfPages); NewRecordCount =3D 0; =20 + // + // Always create a new entry for non-PE image record // if=20 + (ImageRecord->ImageBase > TempRecord.PhysicalStart) { + NewRecord->Type =3D TempRecord.Type; + NewRecord->PhysicalStart =3D TempRecord.PhysicalStart; + NewRecord->VirtualStart =3D 0; + NewRecord->NumberOfPages =3D EfiSizeToPages(ImageRecord->ImageBase - T= empRecord.PhysicalStart); + NewRecord->Attribute =3D TempRecord.Attribute; + NewRecord =3D NEXT_MEMORY_DESCRIPTOR (NewRecord, DescriptorSize); + NewRecordCount ++; + TempRecord.PhysicalStart =3D ImageRecord->ImageBase; + TempRecord.NumberOfPages =3D EfiSizeToPages(PhysicalEnd -=20 + TempRecord.PhysicalStart); } + ImageRecordCodeSectionList =3D &ImageRecord->CodeSegmentList; =20 ImageRecordCodeSectionLink =3D ImageRecordCodeSectionList->ForwardLink; @@ -452,14 +471,10 @@ GetMaxSplitRecordCount ( if (ImageRecord =3D=3D NULL) { break; } - SplitRecordCount +=3D (2 * ImageRecord->CodeSegmentCount + 1); + SplitRecordCount +=3D (2 * ImageRecord->CodeSegmentCount + 2); PhysicalStart =3D ImageRecord->ImageBase + ImageRecord->ImageSize; } while ((ImageRecord !=3D NULL) && (PhysicalStart < PhysicalEnd)); =20 - if (SplitRecordCount !=3D 0) { - SplitRecordCount--; - } - return SplitRecordCount; } =20 @@ -516,28 +531,16 @@ SplitRecord ( // // No more image covered by this range, stop // - if ((PhysicalEnd > PhysicalStart) && (ImageRecord !=3D NULL)) { + if (PhysicalEnd > PhysicalStart) { // - // If this is still address in this record, need record. + // Always create a new entry for non-PE image record // - NewRecord =3D PREVIOUS_MEMORY_DESCRIPTOR (NewRecord, DescriptorSiz= e); - if (NewRecord->Type =3D=3D EfiRuntimeServicesData) { - // - // Last record is DATA, just merge it. - // - NewRecord->NumberOfPages =3D EfiSizeToPages(PhysicalEnd - NewRec= ord->PhysicalStart); - } else { - // - // Last record is CODE, create a new DATA entry. - // - NewRecord =3D NEXT_MEMORY_DESCRIPTOR (NewRecord, DescriptorSize)= ; - NewRecord->Type =3D EfiRuntimeServicesData; - NewRecord->PhysicalStart =3D TempRecord.PhysicalStart; - NewRecord->VirtualStart =3D 0; - NewRecord->NumberOfPages =3D TempRecord.NumberOfPages; - NewRecord->Attribute =3D TempRecord.Attribute | EFI_MEMORY_X= P; - TotalNewRecordCount ++; - } + NewRecord->Type =3D TempRecord.Type; + NewRecord->PhysicalStart =3D TempRecord.PhysicalStart; + NewRecord->VirtualStart =3D 0; + NewRecord->NumberOfPages =3D TempRecord.NumberOfPages; + NewRecord->Attribute =3D TempRecord.Attribute; + TotalNewRecordCount ++; } break; } @@ -580,6 +583,8 @@ SplitRecord ( =3D=3D> +---------------+ | Record X | + +---------------+ + | Record RtCode | +---------------+ ---- | Record RtData | | +---------------+ | @@ -587,12 +592,16 @@ SplitRecord ( +---------------+ | | Record RtData | | +---------------+ ---- + | Record RtCode | + +---------------+ ---- | Record RtData | | +---------------+ | | Record RtCode | |-> PE/COFF2 +---------------+ | | Record RtData | | +---------------+ ---- + | Record RtCode | + +---------------+ | Record Y | +---------------+ =20 @@ -622,7 +631,7 @@ SplitTable ( UINTN TotalSplitRecordCount; UINTN AdditionalRecordCount; =20 - AdditionalRecordCount =3D (2 * mImagePropertiesPrivateData.CodeSegmentCo= untMax + 1) * mImagePropertiesPrivateData.ImageRecordCount; + AdditionalRecordCount =3D (2 *=20 + mImagePropertiesPrivateData.CodeSegmentCountMax + 2) *=20 + mImagePropertiesPrivateData.ImageRecordCount; =20 TotalSplitRecordCount =3D 0; // @@ -648,11 +657,13 @@ SplitTable ( // // Adjust IndexNew according to real split. // - CopyMem ( - ((UINT8 *)MemoryMap + (IndexNew + MaxSplitRecordCount - RealSplitRec= ordCount) * DescriptorSize), - ((UINT8 *)MemoryMap + IndexNew * DescriptorSize), - RealSplitRecordCount * DescriptorSize - ); + if (MaxSplitRecordCount !=3D RealSplitRecordCount) { + CopyMem ( + ((UINT8 *)MemoryMap + (IndexNew + MaxSplitRecordCount - RealSplitR= ecordCount) * DescriptorSize), + ((UINT8 *)MemoryMap + IndexNew * DescriptorSize), + (RealSplitRecordCount + 1) * DescriptorSize + ); + } IndexNew =3D IndexNew + MaxSplitRecordCount - RealSplitRecordCount; TotalSplitRecordCount +=3D RealSplitRecordCount; IndexNew --; @@ -744,7 +755,7 @@ SmmCoreGetMemoryMapMemoryAttributesTable ( return EFI_INVALID_PARAMETER; } =20 - AdditionalRecordCount =3D (2 * mImagePropertiesPrivateData.CodeSegmentCo= untMax + 1) * mImagePropertiesPrivateData.ImageRecordCount; + AdditionalRecordCount =3D (2 *=20 + mImagePropertiesPrivateData.CodeSegmentCountMax + 2) *=20 + mImagePropertiesPrivateData.ImageRecordCount; =20 OldMemoryMapSize =3D *MemoryMapSize; Status =3D SmmCoreGetMemoryMap (MemoryMapSize, MemoryMap, MapKey, Descri= ptorSize, DescriptorVersion); -- 2.7.4.windows.1