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.20; helo=mga02.intel.com; envelope-from=jian.j.wang@intel.com; receiver=edk2-devel@lists.01.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (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 E3DD42035BB16 for ; Wed, 22 Nov 2017 20:59:45 -0800 (PST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Nov 2017 21:04:02 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,440,1505804400"; d="scan'208";a="152353490" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga004.jf.intel.com with ESMTP; 22 Nov 2017 21:04:02 -0800 Received: from fmsmsx112.amr.corp.intel.com (10.18.116.6) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 22 Nov 2017 21:04:02 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX112.amr.corp.intel.com (10.18.116.6) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 22 Nov 2017 21:04:01 -0800 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.213]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.93]) with mapi id 14.03.0319.002; Thu, 23 Nov 2017 13:03:59 +0800 From: "Wang, Jian J" To: "Yao, Jiewen" , "edk2-devel@lists.01.org" CC: "Dong, Eric" , Laszlo Ersek , "Kinney, Michael D" Thread-Topic: [PATCH v2 8/8] UefiCpuPkg/CpuDxe: Initialize stack switch for MP Thread-Index: AQHTZBF6Ep2xc7bTZEufEYOpEI0ivaMhZzWg Date: Thu, 23 Nov 2017 05:03:59 +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: <74D8A39837DF1E4DA445A8C0B3885C503AA290B0@shsmsx102.ccr.corp.intel.com> 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 04:59:46 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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 switch fo= r MP >=20 > Hi > 1) Can we enable this feature in early DxeCore? >=20 Yes. Intead of calling InitializeExceptionStackSwitchHandlers () directly i= n DxeCore, InitializeCpuExceptionHandlers() calls InitializeExceptionStackSwitchHandle= rs(). I think it's reasonable to do this because InitializeExceptionStackSwitchHa= ndlers() is arch dependent. It'd be better not to call it in DxeCore. Another benefi= t is that=20 this can avoid backward compatibility issue introduced by new API, which ha= sn't=20 been implemented by cpu driver or lib of other archs. > Current DxeCore still calling InitializeCpuExceptionHandlers(). > But I hope InitializeExceptionStackSwitchHandlers() can be used here. >=20 > In order to handle buffer from different arch, the DxeIpl can help provid= e some > data in hob and pass to DxeCore. >=20 > 2) In addition, InitializeCpuExceptionHandlers () has VectorInfoList as p= arameter. > Do we also need that for InitializeExceptionStackSwitchHandlers()? >=20 I don't see the need. Do you have any use cases in mind? > Thank you > Yao Jiewen >=20 > > -----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 sta= ck > > > 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 that = BSP > > and AP have their own configuration. This patch adds code to ask BSP an= d 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.in= f > > 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 different= 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 req= uirement. > > + > > + This function is a wrapper of InitializeCpuExceptionStackSwitchHandl= ers. > > + It's mainly for AP purpose because of EFI_AP_PROCEDURE API requireme= nt. > > + > > +**/ > > +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 a= ssume > > + // 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 requ= irement. > > + > > + 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 (PcdCpuStackSwitchExceptionLis= t); > > + 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 ID= T. > > + // > > + 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 exceptio= ns > > + // because of its IST feature. IA32 needs one TSS for each excepti= on > > + // in addition to current task. Since AP is not supposed to alloca= te > > + // memory, we have to do it in BSP. To simplify the code, we alloc= ate > > + // memory for IA32 case to cover both IA32 and X64 exception stack > > + // 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.GdtTabl= e + > > + Gdtr.Limit + 1); > > + EssData.Tss =3D (IA32_TASK_STATE_SEGMENT *)((UINTN)EssData.GdtTabl= e + > > + 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 process= or. > > + // > > + 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