From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.24; helo=mga09.intel.com; envelope-from=jiewen.yao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (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 6427C2035BB16 for ; Wed, 22 Nov 2017 21:34:58 -0800 (PST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Nov 2017 21:39:14 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,440,1505804400"; d="scan'208";a="6045171" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga001.fm.intel.com with ESMTP; 22 Nov 2017 21:39:13 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 22 Nov 2017 21:39:13 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.175]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.213]) with mapi id 14.03.0319.002; Thu, 23 Nov 2017 13:39:11 +0800 From: "Yao, Jiewen" To: "Wang, Jian J" , "edk2-devel@lists.01.org" CC: "Kinney, Michael D" , Laszlo Ersek , "Dong, Eric" Thread-Topic: [PATCH v2 8/8] UefiCpuPkg/CpuDxe: Initialize stack switch for MP Thread-Index: AQHTY25hymynXsgZ1US/s9YNH+VjZqMhWwzw//+I7ICAAARGgIAAi4Kg Date: Thu, 23 Nov 2017 05:39:10 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503AA292AF@shsmsx102.ccr.corp.intel.com> References: <20171122084548.6564-1-jian.j.wang@intel.com> <20171122084548.6564-9-jian.j.wang@intel.com> <74D8A39837DF1E4DA445A8C0B3885C503AA290B0@shsmsx102.ccr.corp.intel.com> In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTcxNzA1NjUtZTBiYi00YTAyLWIxY2EtYzMxMTU3N2ZhYjhjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJDUFd1UkZmNFVDdU9GRkpPSEY3ZUlBdmZudGVQeTVLUVBNdVJ0dHlWa3FHaGFtcHNONE1XQ2VYQTlCUFBzaHN6In0= x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v2 8/8] UefiCpuPkg/CpuDxe: Initialize stack switch for MP X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Nov 2017 05:34:58 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Got it. I like this idea. It is better to hide it from DxeCore. Thank you Yao Jiewen > -----Original Message----- > From: Wang, Jian J > Sent: Thursday, November 23, 2017 1:19 PM > To: Wang, Jian J ; Yao, Jiewen ; > edk2-devel@lists.01.org > Cc: Kinney, Michael D ; Laszlo Ersek > ; Dong, Eric > Subject: RE: [PATCH v2 8/8] UefiCpuPkg/CpuDxe: Initialize stack switch fo= r MP >=20 > About 1), the code is in [PATCH v2 7/8]. Following is part of it. >=20 > @@ -63,10 +67,34 @@ InitializeCpuExceptionHandlers ( > IN EFI_VECTOR_HANDOFF_INFO *VectorInfo OPTIONAL > ) > { > + EFI_STATUS Status; > + EXCEPTION_STACK_SWITCH_DATA StackSwitchData; > + IA32_DESCRIPTOR Idtr; > + IA32_DESCRIPTOR Gdtr; > + > mExceptionHandlerData.ReservedVectors =3D > mReservedVectorsData; > mExceptionHandlerData.ExternalInterruptHandler =3D > mExternalInterruptHandlerTable; > InitializeSpinLock (&mExceptionHandlerData.DisplayMessageSpinLock); > - return InitializeCpuExceptionHandlersWorker (VectorInfo, > &mExceptionHandlerData); > + Status =3D InitializeCpuExceptionHandlersWorker (VectorInfo, > &mExceptionHandlerData); > + if (!EFI_ERROR (Status) && PcdGetBool (PcdCpuStackGuard)) { > + AsmReadIdtr (&Idtr); > + AsmReadGdtr (&Gdtr); > + > + StackSwitchData.StackTop =3D (UINTN)mNewStack; > + StackSwitchData.StackSize =3D CPU_KNOWN_GOOD_STACK_SIZE; > + StackSwitchData.Exceptions =3D CPU_STACK_SWITCH_EXCEPTION_LIST; > + StackSwitchData.ExceptionNumber =3D > CPU_STACK_SWITCH_EXCEPTION_NUMBER; > + StackSwitchData.IdtTable =3D (IA32_IDT_GATE_DESCRIPTOR *)Idtr.Base; > + StackSwitchData.GdtTable =3D (IA32_SEGMENT_DESCRIPTOR *)mNewGdt; > + StackSwitchData.GdtSize =3D sizeof (mNewGdt); > + StackSwitchData.TssDesc =3D (IA32_TSS_DESCRIPTOR *)(mNewGdt + > Gdtr.Limit + 1); > + StackSwitchData.Tss =3D (IA32_TASK_STATE_SEGMENT *)(mNewGdt + > Gdtr.Limit + 1 + > + > CPU_TSS_DESC_SIZE); > + Status =3D InitializeCpuExceptionStackSwitchHandlers ( > + &StackSwitchData > + ); > + } > + return Status; > } >=20 > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Wang, > > Jian J > > Sent: Thursday, November 23, 2017 1:04 PM > > To: Yao, Jiewen ; edk2-devel@lists.01.org > > Cc: Kinney, Michael D ; Laszlo Ersek > > ; Dong, Eric > > Subject: Re: [edk2] [PATCH v2 8/8] UefiCpuPkg/CpuDxe: Initialize stack = switch > > for MP > > > > Hi, > > > > > -----Original Message----- > > > From: Yao, Jiewen > > > Sent: Thursday, November 23, 2017 12:14 PM > > > To: Wang, Jian J ; edk2-devel@lists.01.org > > > Cc: Dong, Eric ; Laszlo Ersek ; > > > Kinney, Michael D > > > Subject: RE: [PATCH v2 8/8] UefiCpuPkg/CpuDxe: Initialize stack switc= h for MP > > > > > > Hi > > > 1) Can we enable this feature in early DxeCore? > > > > > Yes. Intead of calling InitializeExceptionStackSwitchHandlers () direct= ly in > > DxeCore, > > InitializeCpuExceptionHandlers() calls InitializeExceptionStackSwitchHa= ndlers(). > > > > I think it's reasonable to do this because > InitializeExceptionStackSwitchHandlers() > > is arch dependent. It'd be better not to call it in DxeCore. Another be= nefit is > that > > this can avoid backward compatibility issue introduced by new API, whic= h hasn't > > been implemented by cpu driver or lib of other archs. > > > > > Current DxeCore still calling InitializeCpuExceptionHandlers(). > > > But I hope InitializeExceptionStackSwitchHandlers() can be used here. > > > > > > In order to handle buffer from different arch, the DxeIpl can help pr= ovide > some > > > data in hob and pass to DxeCore. > > > > > > 2) In addition, InitializeCpuExceptionHandlers () has VectorInfoList = as > > parameter. > > > Do we also need that for InitializeExceptionStackSwitchHandlers()? > > > > > I don't see the need. Do you have any use cases in mind? > > > > > Thank you > > > Yao Jiewen > > > > > > > -----Original Message----- > > > > From: Wang, Jian J > > > > Sent: Wednesday, November 22, 2017 4:46 PM > > > > To: edk2-devel@lists.01.org > > > > Cc: Dong, Eric ; Laszlo Ersek ; > > > Yao, > > > > Jiewen ; Kinney, Michael D > > > > > > > > Subject: [PATCH v2 8/8] UefiCpuPkg/CpuDxe: Initialize stack switch = for MP > > > > > > > > > v2: > > > > > Add code to reserve resources and initialize AP exception with= stack > > > > > switch besides BSP, if PcdCpuStackGuard is enabled. > > > > > > > > In current MP implementation, BSP and AP shares the same exception > > > > configuration. Stack switch required by Stack Guard feature needs t= hat BSP > > > > and AP have their own configuration. This patch adds code to ask BS= P and > AP > > > > to do exception handler initialization separately. > > > > > > > > Cc: Eric Dong > > > > Cc: Laszlo Ersek > > > > Cc: Jiewen Yao > > > > Cc: Michael Kinney > > > > Suggested-by: Ayellet Wolman > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > Signed-off-by: Jian J Wang > > > > --- > > > > UefiCpuPkg/CpuDxe/CpuDxe.inf | 3 + > > > > UefiCpuPkg/CpuDxe/CpuMp.c | 168 > > > > +++++++++++++++++++++++++++++++++++++++++++ > > > > UefiCpuPkg/CpuDxe/CpuMp.h | 12 ++++ > > > > 3 files changed, 183 insertions(+) > > > > > > > > diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.inf > > b/UefiCpuPkg/CpuDxe/CpuDxe.inf > > > > index 3e8d196739..02f86b774c 100644 > > > > --- a/UefiCpuPkg/CpuDxe/CpuDxe.inf > > > > +++ b/UefiCpuPkg/CpuDxe/CpuDxe.inf > > > > @@ -81,6 +81,9 @@ > > > > > > > > [Pcd] > > > > > > > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask > > > > ## CONSUMES > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard > > > > ## CONSUMES > > > > + gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList > > > > ## CONSUMES > > > > + gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize > > > > ## CONSUMES > > > > > > > > [Depex] > > > > TRUE > > > > diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c > > > > index b3c0178d07..6b2ceacb39 100644 > > > > --- a/UefiCpuPkg/CpuDxe/CpuMp.c > > > > +++ b/UefiCpuPkg/CpuDxe/CpuMp.c > > > > @@ -601,6 +601,169 @@ CollectBistDataFromHob ( > > > > } > > > > } > > > > > > > > +/** > > > > + Get GDT register content. > > > > + > > > > + This function is mainly for AP purpose because AP may have diffe= rent > GDT > > > > + table than BSP. > > > > + > > > > +**/ > > > > +VOID > > > > +EFIAPI > > > > +GetGdtr ( > > > > + IN OUT VOID *Buffer > > > > + ) > > > > +{ > > > > + AsmReadGdtr ((IA32_DESCRIPTOR *)Buffer); > > > > +} > > > > + > > > > +/** > > > > + Initializes CPU exceptions handlers for the sake of stack switch > > requirement. > > > > + > > > > + This function is a wrapper of InitializeCpuExceptionStackSwitchH= andlers. > > > > + It's mainly for AP purpose because of EFI_AP_PROCEDURE API > > requirement. > > > > + > > > > +**/ > > > > +VOID > > > > +EFIAPI > > > > +InitializeExceptionStackSwitchHandlers ( > > > > + IN OUT VOID *Buffer > > > > + ) > > > > +{ > > > > + EXCEPTION_STACK_SWITCH_DATA *EssData; > > > > + IA32_DESCRIPTOR Idtr; > > > > + EFI_STATUS Status; > > > > + > > > > + EssData =3D Buffer; > > > > + // > > > > + // We don't plan to replace IDT table with a new one, and we don= 't > > assume > > > > + // the AP's IDT is the same as BSP's IDT either. > > > > + // > > > > + AsmReadIdtr (&Idtr); > > > > + EssData->IdtTable =3D (IA32_IDT_GATE_DESCRIPTOR *)Idtr.Base; > > > > + Status =3D InitializeCpuExceptionStackSwitchHandlers (EssData); > > > > + ASSERT_EFI_ERROR (Status); > > > > +} > > > > + > > > > +/** > > > > + Initializes MP exceptions handlers for the sake of stack switch > requirement. > > > > + > > > > + This function will allocate required resources for stack switch = and pass > > > > + them through EXCEPTION_STACK_SWITCH_DATA to each logic > processor. > > > > + > > > > +**/ > > > > +VOID > > > > +InitializeMpExceptionStackSwitchHandlers ( > > > > + VOID > > > > + ) > > > > +{ > > > > + UINTN Index; > > > > + UINTN Bsp; > > > > + UINTN ExceptionNumber; > > > > + UINTN NewGdtSize; > > > > + UINTN NewStackSize; > > > > + IA32_DESCRIPTOR Gdtr; > > > > + EXCEPTION_STACK_SWITCH_DATA EssData; > > > > + UINT8 *GdtBuffer; > > > > + UINT8 *StackTop; > > > > + > > > > + if (!PcdGetBool (PcdCpuStackGuard)) { > > > > + return; > > > > + } > > > > + > > > > + ExceptionNumber =3D FixedPcdGetSize > (PcdCpuStackSwitchExceptionList); > > > > + NewStackSize =3D FixedPcdGet32 (PcdCpuKnownGoodStackSize) * > > > > ExceptionNumber; > > > > + > > > > + StackTop =3D AllocateRuntimeZeroPool (NewStackSize * > > > > mNumberOfProcessors); > > > > + ASSERT (StackTop !=3D NULL); > > > > + StackTop +=3D NewStackSize * mNumberOfProcessors; > > > > + > > > > + EssData.Exceptions =3D FixedPcdGetPtr > (PcdCpuStackSwitchExceptionList); > > > > + EssData.ExceptionNumber =3D ExceptionNumber; > > > > + EssData.StackSize =3D FixedPcdGet32 (PcdCpuKnownGoodStackSize); > > > > + > > > > + MpInitLibWhoAmI (&Bsp); > > > > + for (Index =3D 0; Index < mNumberOfProcessors; ++Index) { > > > > + // > > > > + // To support stack switch, we need to re-construct GDT but no= t IDT. > > > > + // > > > > + if (Index =3D=3D Bsp) { > > > > + GetGdtr (&Gdtr); > > > > + } else { > > > > + // > > > > + // AP might have different size of GDT from BSP. > > > > + // > > > > + MpInitLibStartupThisAP (GetGdtr, Index, NULL, 0, (VOID *)&Gd= tr, > NULL); > > > > + } > > > > + > > > > + // > > > > + // X64 needs only one TSS of current task working for all exce= ptions > > > > + // because of its IST feature. IA32 needs one TSS for each exc= eption > > > > + // in addition to current task. Since AP is not supposed to al= locate > > > > + // memory, we have to do it in BSP. To simplify the code, we a= llocate > > > > + // memory for IA32 case to cover both IA32 and X64 exception s= tack > > > > + // switch. > > > > + // > > > > + // Layout of memory to allocate for each processor: > > > > + // -------------------------------- > > > > + // | Alignment | (just in case) > > > > + // -------------------------------- > > > > + // | | > > > > + // | Original GDT | > > > > + // | | > > > > + // -------------------------------- > > > > + // | Current task descriptor | > > > > + // -------------------------------- > > > > + // | | > > > > + // | Exception task descriptors | X ExceptionNumber > > > > + // | | > > > > + // -------------------------------- > > > > + // | Current task-state segment | > > > > + // -------------------------------- > > > > + // | | > > > > + // | Exception task-state segment | X ExceptionNumber > > > > + // | | > > > > + // -------------------------------- > > > > + // > > > > + NewGdtSize =3D sizeof (IA32_TSS_DESCRIPTOR) + > > > > + (Gdtr.Limit + 1) + > > > > + sizeof (IA32_TSS_DESCRIPTOR) * (ExceptionNumber + > 1) + > > > > + sizeof (IA32_TASK_STATE_SEGMENT) * > (ExceptionNumber + > > > > 1); > > > > + GdtBuffer =3D AllocateRuntimeZeroPool (NewGdtSize); > > > > + ASSERT (GdtBuffer !=3D NULL); > > > > + > > > > + EssData.GdtTable =3D ALIGN_POINTER(GdtBuffer, sizeof > > > > (IA32_TSS_DESCRIPTOR)); > > > > + NewGdtSize -=3D ((UINT8 *)EssData.GdtTable - GdtBuffer); > > > > + EssData.GdtSize =3D NewGdtSize; > > > > + > > > > + EssData.TssDesc =3D (IA32_TSS_DESCRIPTOR > *)((UINTN)EssData.GdtTable + > > > > + Gdtr.Limit + 1); > > > > + EssData.Tss =3D (IA32_TASK_STATE_SEGMENT > *)((UINTN)EssData.GdtTable > > + > > > > + Gdtr.Limit + 1 + > > > > + sizeof > > > > (IA32_TSS_DESCRIPTOR) * > > > > + (ExceptionNumber + > 1)); > > > > + > > > > + EssData.StackTop =3D (UINTN)StackTop; > > > > + DEBUG ((DEBUG_INFO, "Exception stack top[%d]: 0x%lX\n", Index, > > > > + (UINT64)(UINTN)StackTop)); > > > > + > > > > + if (Index =3D=3D Bsp) { > > > > + InitializeExceptionStackSwitchHandlers (&EssData); > > > > + } else { > > > > + MpInitLibStartupThisAP ( > > > > + InitializeExceptionStackSwitchHandlers, > > > > + Index, > > > > + NULL, > > > > + 0, > > > > + (VOID *)&EssData, > > > > + NULL > > > > + ); > > > > + } > > > > + > > > > + StackTop -=3D NewStackSize; > > > > + } > > > > +} > > > > + > > > > /** > > > > Initialize Multi-processor support. > > > > > > > > @@ -624,6 +787,11 @@ InitializeMpSupport ( > > > > mNumberOfProcessors =3D NumberOfProcessors; > > > > DEBUG ((DEBUG_INFO, "Detect CPU count: %d\n", > > mNumberOfProcessors)); > > > > > > > > + // > > > > + // Initialize exception stack switch handlers for each logic pro= cessor. > > > > + // > > > > + InitializeMpExceptionStackSwitchHandlers (); > > > > + > > > > // > > > > // Update CPU healthy information from Guided HOB > > > > // > > > > diff --git a/UefiCpuPkg/CpuDxe/CpuMp.h b/UefiCpuPkg/CpuDxe/CpuMp.h > > > > index d530149d7e..86d54a95e9 100644 > > > > --- a/UefiCpuPkg/CpuDxe/CpuMp.h > > > > +++ b/UefiCpuPkg/CpuDxe/CpuMp.h > > > > @@ -15,6 +15,18 @@ > > > > #ifndef _CPU_MP_H_ > > > > #define _CPU_MP_H_ > > > > > > > > +typedef struct { > > > > + UINTN StackTop; > > > > + UINTN StackSize; > > > > + UINT8 *Exceptions; > > > > + UINTN ExceptionNumber; > > > > + IA32_IDT_GATE_DESCRIPTOR *IdtTable; > > > > + IA32_SEGMENT_DESCRIPTOR *GdtTable; > > > > + UINTN GdtSize; > > > > + IA32_TSS_DESCRIPTOR *TssDesc; > > > > + IA32_TASK_STATE_SEGMENT *Tss; > > > > +} EXCEPTION_STACK_SWITCH_DATA; > > > > + > > > > /** > > > > Initialize Multi-processor support. > > > > > > > > -- > > > > 2.14.1.windows.1 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel