From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=40.107.1.81; helo=eur02-he1-obe.outbound.protection.outlook.com; envelope-from=udit.kumar@nxp.com; receiver=edk2-devel@lists.01.org Received: from EUR02-HE1-obe.outbound.protection.outlook.com (mail-eopbgr10081.outbound.protection.outlook.com [40.107.1.81]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 22A33222A54C3 for ; Tue, 2 Jan 2018 21:04:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=s+6xqyfjsTCAbJRdpHrhHS2hmR93Dd/D9YgXMcaIKkU=; b=Ocz/+tjQUhbb0iOaxnpeKWMsbNeo/E4jAgX1GCbI8D+PMEWLmEnrDkvVREQFyGdrObzuYWv5a3YkuNZ2iECbgIOIJTd511uei9ga6ylc3zWAkxT0LtIGoV/bVVHYdL6TMhPCjV3osyL188wCjS6Kasj22EPyTS17wjGNeQpaPpc= Received: from AM6PR0402MB3334.eurprd04.prod.outlook.com (52.133.18.151) by AM2PR04MB0995.eurprd04.prod.outlook.com (10.162.35.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.386.5; Wed, 3 Jan 2018 05:09:06 +0000 Received: from AM6PR0402MB3334.eurprd04.prod.outlook.com ([fe80::a5fd:3b4c:2300:b2bf]) by AM6PR0402MB3334.eurprd04.prod.outlook.com ([fe80::a5fd:3b4c:2300:b2bf%13]) with mapi id 15.20.0366.009; Wed, 3 Jan 2018 05:09:05 +0000 From: Udit Kumar To: Ard Biesheuvel , "edk2-devel@lists.01.org" CC: "leif.lindholm@linaro.org" , "vladimir.olovyannikov@broadcom.com" , Meenakshi Aggarwal Thread-Topic: [PATCH] ArmPlatformPkg/MemoryInitPeiLib: mark primary FV region as boot services data Thread-Index: AQHTg+FyvLhS2YAGu0aOHa9AEiHHlqNhmTtQ Date: Wed, 3 Jan 2018 05:09:05 +0000 Message-ID: References: <20180102155034.13688-1-ard.biesheuvel@linaro.org> In-Reply-To: <20180102155034.13688-1-ard.biesheuvel@linaro.org> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=udit.kumar@nxp.com; x-originating-ip: [192.88.170.1] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM2PR04MB0995; 7:ZoOAgACKf1jaPIQN/d+E9Snll27YmbV2JIysyfLBOy8lUnaJA6JypUlKss2zN3/l1Q3lre1NuKpYv0643HdvlkoKsIGzSWDfFxHcfoowPRfwzb3S+L0ZIOvi4HynFYeA66WVRYXRZCzspQEyhppsBeH5nvneLCK93GjP3baz/0rKyXChpoX9h5Def0qOkdaprUpbbT5ju/yTvFN81dUY00ZYfNQTT2k7BQqaI47oqGnV50E2WmdgIwEBmm1MqCfN x-ms-exchange-antispam-srfa-diagnostics: SSOS;SSOR; x-forefront-antispam-report: SFV:SKI; SCL:-1; SFV:NSPM; SFS:(10009020)(346002)(39860400002)(376002)(366004)(39380400002)(396003)(52314003)(199004)(189003)(13464003)(3280700002)(54906003)(4326008)(25786009)(9686003)(575784001)(5250100002)(229853002)(110136005)(2501003)(86362001)(6246003)(53936002)(7696005)(14454004)(3660700001)(6116002)(99286004)(3846002)(6436002)(55016002)(316002)(81156014)(76176011)(97736004)(81166006)(8936002)(68736007)(59450400001)(305945005)(7736002)(74316002)(2900100001)(8676002)(478600001)(2906002)(2950100002)(66066001)(5660300001)(105586002)(102836004)(33656002)(53546011)(6506007)(106356001); DIR:OUT; SFP:1101; SCL:1; SRVR:AM2PR04MB0995; H:AM6PR0402MB3334.eurprd04.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; x-ms-office365-filtering-correlation-id: e256d2d7-0433-40ae-5d9f-08d552681cdd x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(5600026)(4604075)(3008032)(4534020)(4602075)(4627115)(201703031133081)(201702281549075)(48565401081)(2017052603307)(7153060); SRVR:AM2PR04MB0995; x-ms-traffictypediagnostic: AM2PR04MB0995: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(185117386973197)(162533806227266); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040470)(2401047)(5005006)(8121501046)(93006095)(93001095)(3002001)(3231023)(944501075)(10201501046)(6055026)(6041268)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123558120)(20161123564045)(20161123560045)(20161123562045)(6072148)(201708071742011); SRVR:AM2PR04MB0995; BCL:0; PCL:0; RULEID:(100000803101)(100110400095); SRVR:AM2PR04MB0995; x-forefront-prvs: 0541031FF6 received-spf: None (protection.outlook.com: nxp.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: 10xHN3zfCdJHmcgr+j44ZnIySrMGwPokEecIscSX4rbw9iJswIYAPaS2bxBHXPu57fr2cjiVx8zK8fmvBWt7gw== spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-Network-Message-Id: e256d2d7-0433-40ae-5d9f-08d552681cdd X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Jan 2018 05:09:05.6357 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM2PR04MB0995 Subject: Re: [PATCH] ArmPlatformPkg/MemoryInitPeiLib: mark primary FV region as boot services data X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Jan 2018 05:04:08 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks Ard,=20 This works for us as well=20 Few comments inline=20 Regards Udit > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Tuesday, January 02, 2018 9:21 PM > To: edk2-devel@lists.01.org > Cc: leif.lindholm@linaro.org; vladimir.olovyannikov@broadcom.com; Udit > Kumar ; Meenakshi Aggarwal > ; Ard Biesheuvel > > Subject: [PATCH] ArmPlatformPkg/MemoryInitPeiLib: mark primary FV region > as boot services data >=20 > Commit 8ae5fc182941 ("ArmPlatformPkg/MemoryInitPeiLib: don't reserve > primary FV in memory") deleted the code that removes the memory covering > the primary firmware volume from the memory map. The assumption was > that > this is no longer necessary now that we no longer expose compression and > PE/COFF loader library code from the PrePi module to DXE core. >=20 > However, the FV is still declared, and so code may attempt to access it > anyway, which may cause unexpected results depending on whether the > memory has been reused for other purposes in the mean time. >=20 > So reinstate the code that splits off the resource descriptor HOB that > describes the firmware device, but this time, don't mark the memory as > unusable, but create a memory allocation HOB that marks the region as > boot services data. >=20 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > --- > Vladimir, Udit, Meenakshi: please confirm whether this code works for you= . >=20 > ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 74 > ++++++++++++++++++++ > 1 file changed, 74 insertions(+) >=20 > diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > index d03214b5df66..d1b5c0be9497 100644 > --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > @@ -70,7 +70,11 @@ MemoryPeim ( > { > ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable; > EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttributes; > + UINT64 ResourceLength; > EFI_PEI_HOB_POINTERS NextHob; > + EFI_PHYSICAL_ADDRESS FdTop; > + EFI_PHYSICAL_ADDRESS SystemMemoryTop; > + EFI_PHYSICAL_ADDRESS ResourceTop; > BOOLEAN Found; >=20 > // Get Virtual Memory Map from the Platform Library > @@ -117,6 +121,76 @@ MemoryPeim ( > ); > } >=20 > + // > + // Reserve the memory space occupied by the firmware volume > + // > + > + SystemMemoryTop =3D (EFI_PHYSICAL_ADDRESS)PcdGet64 > (PcdSystemMemoryBase) + (EFI_PHYSICAL_ADDRESS)PcdGet64 > (PcdSystemMemorySize); > + FdTop =3D (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) + > (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize); > + > + // EDK2 does not have the concept of boot firmware copied into DRAM. T= o > avoid the DXE > + // core to overwrite this area we must create a memory allocation HOB = for > the region, > + // but this only works if we split off the underlying resource descrip= tor as > well. > + if ((PcdGet64 (PcdFdBaseAddress) >=3D PcdGet64 (PcdSystemMemoryBase)) > && (FdTop <=3D SystemMemoryTop)) { > + Found =3D FALSE; > + > + // Search for System Memory Hob that contains the firmware > + NextHob.Raw =3D GetHobList (); > + while ((NextHob.Raw =3D GetNextHob > (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) !=3D NULL) { > + if ((NextHob.ResourceDescriptor->ResourceType =3D=3D > EFI_RESOURCE_SYSTEM_MEMORY) && > + (PcdGet64 (PcdFdBaseAddress) >=3D NextHob.ResourceDescriptor- > >PhysicalStart) && > + (FdTop <=3D NextHob.ResourceDescriptor->PhysicalStart + > NextHob.ResourceDescriptor->ResourceLength)) > + { > + ResourceAttributes =3D NextHob.ResourceDescriptor->ResourceAttri= bute; > + ResourceLength =3D NextHob.ResourceDescriptor->ResourceLength; > + ResourceTop =3D NextHob.ResourceDescriptor->PhysicalStart + > ResourceLength; > + > + if (PcdGet64 (PcdFdBaseAddress) =3D=3D NextHob.ResourceDescripto= r- > >PhysicalStart) { > + if (SystemMemoryTop !=3D FdTop) { > + // Create the System Memory HOB for the firmware with the no= n- > present attribute Please correct comments, now this memory is present=20 > + BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, > + ResourceAttributes, > + PcdGet64 (PcdFdBaseAddress), > + PcdGet32 (PcdFdSize)); > + > + // Top of the FD is system memory available for UEFI > + NextHob.ResourceDescriptor->PhysicalStart +=3D PcdGet32(PcdF= dSize); > + NextHob.ResourceDescriptor->ResourceLength -=3D > PcdGet32(PcdFdSize); > + } > + } else { > + // Create the System Memory HOB for the firmware > + BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, > + ResourceAttributes, > + PcdGet64 (PcdFdBaseAddress), > + PcdGet32 (PcdFdSize)); Hob List is already created for PcdSystemMemoryBase and its size=20 Within this, we got Fd, then do we want to create another Hob here=20 > + > + // Update the HOB > + NextHob.ResourceDescriptor->ResourceLength =3D PcdGet64 > (PcdFdBaseAddress) - NextHob.ResourceDescriptor->PhysicalStart; > + > + // If there is some memory available on the top of the FD then= create > a HOB > + if (FdTop < NextHob.ResourceDescriptor->PhysicalStart + > ResourceLength) { > + // Create the System Memory HOB for the remaining region (to= p of > the FD) > + BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, > + ResourceAttributes, > + FdTop, > + ResourceTop - FdTop); > + } > + } > + > + // Mark the memory covering the Firmware Device as boot services > data > + BuildMemoryAllocationHob (PcdGet64 (PcdFdBaseAddress), > + PcdGet32 (PcdFdSize), > + EfiBootServicesData); IMO, only this call should be enough to protect FD area.=20 > + > + Found =3D TRUE; > + break; > + } > + NextHob.Raw =3D GET_NEXT_HOB (NextHob); > + } > + > + ASSERT(Found); > + } > + > // Build Memory Allocation Hob > InitMmu (MemoryTable); >=20 > -- > 2.11.0