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.65; helo=mga03.intel.com; envelope-from=jian.j.wang@intel.com; receiver=edk2-devel@lists.01.org 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 383D42035BB16 for ; Wed, 22 Nov 2017 21:15:04 -0800 (PST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Nov 2017 21:19:19 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,440,1505804400"; d="scan'208";a="5442222" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga003.jf.intel.com with ESMTP; 22 Nov 2017 21:19:20 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 22 Nov 2017 21:19:20 -0800 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.213]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.218]) with mapi id 14.03.0319.002; Thu, 23 Nov 2017 13:19:18 +0800 From: "Wang, Jian J" To: "Wang, Jian J" , "Yao, Jiewen" , "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: AQHTZBF6Ep2xc7bTZEufEYOpEI0ivaMhZzWggAAFJkA= Date: Thu, 23 Nov 2017 05:19:17 +0000 Message-ID: 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: 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:15:04 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable About 1), the code is in [PATCH v2 7/8]. Following is part of it. @@ -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 mExternalInterruptHan= dlerTable; InitializeSpinLock (&mExceptionHandlerData.DisplayMessageSpinLock); - return InitializeCpuExceptionHandlersWorker (VectorInfo, &mExceptionHand= lerData); + Status =3D InitializeCpuExceptionHandlersWorker (VectorInfo, &mException= HandlerData); + 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.Lim= it + 1); + StackSwitchData.Tss =3D (IA32_TASK_STATE_SEGMENT *)(mNewGdt + Gdtr.Lim= it + 1 + + CPU_TSS_DESC_SIZE); + Status =3D InitializeCpuExceptionStackSwitchHandlers ( + &StackSwitchData + ); + } + return Status; } > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Wa= ng, > 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 sw= itch > for MP >=20 > Hi, >=20 > > -----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 switch = for MP > > > > Hi > > 1) Can we enable this feature in early DxeCore? > > > Yes. Intead of calling InitializeExceptionStackSwitchHandlers () directly= in > DxeCore, > InitializeCpuExceptionHandlers() calls InitializeExceptionStackSwitchHand= lers(). >=20 > I think it's reasonable to do this because InitializeExceptionStackSwitch= Handlers() > is arch dependent. It'd be better not to call it in DxeCore. Another bene= fit is that > this can avoid backward compatibility issue introduced by new API, which = hasn't > been implemented by cpu driver or lib of other archs. >=20 > > 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 prov= ide 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? >=20 > > 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 fo= r MP > > > > > > > v2: > > > > Add code to reserve resources and initialize AP exception with s= tack > > > > 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 tha= t BSP > > > and AP have their own configuration. This patch adds code to ask BSP = 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 differe= nt 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 InitializeCpuExceptionStackSwitchHan= dlers. > > > + 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 re= quirement. > > > + > > > + This function will allocate required resources for stack switch an= d 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 (PcdCpuStackSwitchExceptionLis= t); > > > + NewStackSize =3D FixedPcdGet32 (PcdCpuKnownGoodStackSize) * > > > ExceptionNumber; > > > + > > > + StackTop =3D AllocateRuntimeZeroPool (NewStackSize * > > > mNumberOfProcessors); > > > + ASSERT (StackTop !=3D NULL); > > > + StackTop +=3D NewStackSize * mNumberOfProcessors; > > > + > > > + EssData.Exceptions =3D FixedPcdGetPtr (PcdCpuStackSwitchExceptionL= ist); > > > + 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 not = IDT. > > > + // > > > + if (Index =3D=3D Bsp) { > > > + GetGdtr (&Gdtr); > > > + } else { > > > + // > > > + // AP might have different size of GDT from BSP. > > > + // > > > + MpInitLibStartupThisAP (GetGdtr, Index, NULL, 0, (VOID *)&Gdtr= , NULL); > > > + } > > > + > > > + // > > > + // X64 needs only one TSS of current task working for all except= ions > > > + // because of its IST feature. IA32 needs one TSS for each excep= tion > > > + // in addition to current task. Since AP is not supposed to allo= cate > > > + // memory, we have to do it in BSP. To simplify the code, we all= ocate > > > + // memory for IA32 case to cover both IA32 and X64 exception sta= ck > > > + // 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.GdtTa= ble + > > > + Gdtr.Limit + 1); > > > + EssData.Tss =3D (IA32_TASK_STATE_SEGMENT *)((UINTN)EssData.GdtTa= ble > + > > > + 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 proce= ssor. > > > + // > > > + 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 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel