From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.136; helo=mga12.intel.com; envelope-from=eric.dong@intel.com; receiver=edk2-devel@lists.01.org Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) (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 0A5CD211D56C4 for ; Wed, 20 Jun 2018 22:57:45 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Jun 2018 22:57:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,250,1526367600"; d="scan'208";a="59643482" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga002.fm.intel.com with ESMTP; 20 Jun 2018 22:57:45 -0700 Received: from fmsmsx156.amr.corp.intel.com (10.18.116.74) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 20 Jun 2018 22:57:45 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx156.amr.corp.intel.com (10.18.116.74) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 20 Jun 2018 22:57:44 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.223]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.82]) with mapi id 14.03.0319.002; Thu, 21 Jun 2018 13:57:43 +0800 From: "Dong, Eric" To: "Ni, Ruiyu" , "afish@apple.com" CC: edk2-devel , Fan Jeff , "Gao, Liming" Thread-Topic: [edk2] Is the PEI Core MP Safe? UefiCpuPkg seems to think so calling GetFirstGuidHob on the APs? Thread-Index: AQHUCLtqsM/Cqi6shEGJpJ4IMd6FcKRpgggAgAAFPwCAAALFgIAArcew Date: Thu, 21 Jun 2018 05:57:42 +0000 Message-ID: References: <31C6D3FB-91D3-491B-8059-4B6DEA11BDF9@apple.com> <734D49CCEBEEF84792F5B80ED585239D5BD3F73E@SHSMSX104.ccr.corp.intel.com> <1800C800-ED89-4C23-89CB-E8E5E1CCE672@apple.com> <734D49CCEBEEF84792F5B80ED585239D5BD3F788@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5BD3F788@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: Is the PEI Core MP Safe? UefiCpuPkg seems to think so calling GetFirstGuidHob on the APs? X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Jun 2018 05:57:46 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Ruiyu & Andrew, I have submit https://bugzilla.tianocore.org/show_bug.cgi?id=3D993 for this= issue and will follow up to fix it. Thanks, Eric -----Original Message----- From: Ni, Ruiyu=20 Sent: Thursday, June 21, 2018 11:35 AM To: afish@apple.com Cc: edk2-devel ; Fan Jeff ; Dong, Eric ; Gao, Liming Subject: RE: [edk2] Is the PEI Core MP Safe? UefiCpuPkg seems to think so c= alling GetFirstGuidHob on the APs? Andrew, Today's MpInitLib does depend on physical memory (the waking up vector sits= below 1MB). So the MpPei module can use RegisterForShadow() to make sure it is loaded i= n memory before running. Then the module global variable can be used. But when system is in S3 boot path, it seems PeiCore doesn't follow Registe= rForShadow() to load the PEIM in memory. Using IDT entry as the storage of global variable is a good idea. We will e= valuate. Thanks/Ray > -----Original Message----- > From: afish@apple.com > Sent: Thursday, June 21, 2018 11:25 AM > To: Ni, Ruiyu > Cc: edk2-devel > Subject: Re: [edk2] Is the PEI Core MP Safe? UefiCpuPkg seems to think=20 > so calling GetFirstGuidHob on the APs? >=20 >=20 >=20 > > On Jun 20, 2018, at 8:06 PM, Ni, Ruiyu wrote: > > > > Andrew, > > Good catch! It does break the rule that AP shouldn't call PEI services. > > But considering the specific case, it should be safe. > > Because: > > 1. HOB only stores a pointer to the buffer that contains all the MP > information. > > 2. BSP modifies the HOB by only appending data to the end. It may=20 > > modifies some HOB content. But MpInitLib implementation itself=20 > > doesn't > modify the pointer value stored in HOB. > > >=20 > Ray, >=20 > I think the should be safe is also making assumptions about the debug=20 > and a few other library instances that got tested against. It is not=20 > really an architectural guaranty against the library classes that=20 > could be linked. We don't use the default >=20 > It would be good to use an IDT entry, or optionally a global variable=20 > if memory is present, longer term. >=20 > Thanks, >=20 > Andrew Fish >=20 > > In PEI environment where global variable is read-only, it's hard to=20 > > not rely > on HOB. > > I guess that's the reason of today's tricky implementation. > > > > Thanks/Ray > > > >> -----Original Message----- > >> From: edk2-devel On Behalf Of=20 > >> Andrew Fish > >> Sent: Thursday, June 21, 2018 1:23 AM > >> To: edk2-devel > >> Subject: [edk2] Is the PEI Core MP Safe? UefiCpuPkg seems to think=20 > >> so calling GetFirstGuidHob on the APs? > >> > >> Is there some MP safe contract with the PEI Core implementation I=20 > >> don't know about? > >> > >> Looks like the APs are calling PeiServicesGetHobList() to figure=20 > >> out "who they are"? How does this work? Or am I missing something? > >> > >> > >> > https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/MpIn > >> i > >> tLib/MpLib.c#L1945 > >> > >> /** > >> This return the handle number for the calling processor. This=20 > >> service may be called from the BSP and APs. > >> @param[out] ProcessorNumber Pointer to the handle number of AP. > >> The range is from 0 to the total number = of > >> logical processors minus 1. The total nu= mber of > >> logical processors can be retrieved by > >> MpInitLibGetNumberOfProcessors(). > >> @retval EFI_SUCCESS The current processor handle number w= as > >> returned > >> in ProcessorNumber. > >> @retval EFI_INVALID_PARAMETER ProcessorNumber is NULL. > >> @retval EFI_NOT_READY MP Initialize Library is not initiali= zed. > >> **/ > >> EFI_STATUS > >> EFIAPI > >> MpInitLibWhoAmI ( > >> OUT UINTN *ProcessorNumber > >> ) > >> { > >> CPU_MP_DATA *CpuMpData; > >> > >> if (ProcessorNumber =3D=3D NULL) { > >> return EFI_INVALID_PARAMETER; > >> } > >> > >> CpuMpData =3D GetCpuMpData (); > >> > >> return GetProcessorNumber (CpuMpData, ProcessorNumber); } > >> > >> > >> > https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/MpIn > >> i > >> tLib/PeiMpLib.c#L34 > >> > >> /** > >> Get pointer to CPU MP Data structure. > >> @return The pointer to CPU MP Data structure. > >> **/ > >> CPU_MP_DATA * > >> GetCpuMpData ( > >> VOID > >> ) > >> { > >> CPU_MP_DATA *CpuMpData; > >> > >> CpuMpData =3D GetCpuMpDataFromGuidedHob (); ASSERT > (CpuMpData !=3D > >> NULL); return CpuMpData; } > >> > >> > https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/MpIn > >> i > >> tLib/MpLib.c#L2302 > >> > >> > >> /** > >> Get pointer to CPU MP Data structure from GUIDed HOB. > >> @return The pointer to CPU MP Data structure. > >> **/ > >> CPU_MP_DATA * > >> GetCpuMpDataFromGuidedHob ( > >> VOID > >> ) > >> { > >> EFI_HOB_GUID_TYPE *GuidHob; > >> VOID *DataInHob; > >> CPU_MP_DATA *CpuMpData; > >> > >> CpuMpData =3D NULL; > >> GuidHob =3D GetFirstGuidHob (&mCpuInitMpLibHobGuid); if (GuidHob != =3D > >> NULL) { > >> DataInHob =3D GET_GUID_HOB_DATA (GuidHob); > >> CpuMpData =3D (CPU_MP_DATA *) (*(UINTN *) DataInHob); } return=20 > >> CpuMpData; } > >> > >> Thanks, > >> > >> Andrew Fish > >> _______________________________________________ > >> edk2-devel mailing list > >> edk2-devel@lists.01.org > >> https://lists.01.org/mailman/listinfo/edk2-devel > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel