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 = mReservedVectorsData; mExceptionHandlerData.ExternalInterruptHandler = mExternalInterruptHandlerTable; InitializeSpinLock (&mExceptionHandlerData.DisplayMessageSpinLock); - return InitializeCpuExceptionHandlersWorker (VectorInfo, &mExceptionHandlerData); + Status = InitializeCpuExceptionHandlersWorker (VectorInfo, &mExceptionHandlerData); + if (!EFI_ERROR (Status) && PcdGetBool (PcdCpuStackGuard)) { + AsmReadIdtr (&Idtr); + AsmReadGdtr (&Gdtr); + + StackSwitchData.StackTop = (UINTN)mNewStack; + StackSwitchData.StackSize = CPU_KNOWN_GOOD_STACK_SIZE; + StackSwitchData.Exceptions = CPU_STACK_SWITCH_EXCEPTION_LIST; + StackSwitchData.ExceptionNumber = CPU_STACK_SWITCH_EXCEPTION_NUMBER; + StackSwitchData.IdtTable = (IA32_IDT_GATE_DESCRIPTOR *)Idtr.Base; + StackSwitchData.GdtTable = (IA32_SEGMENT_DESCRIPTOR *)mNewGdt; + StackSwitchData.GdtSize = sizeof (mNewGdt); + StackSwitchData.TssDesc = (IA32_TSS_DESCRIPTOR *)(mNewGdt + Gdtr.Limit + 1); + StackSwitchData.Tss = (IA32_TASK_STATE_SEGMENT *)(mNewGdt + Gdtr.Limit + 1 + + CPU_TSS_DESC_SIZE); + Status = InitializeCpuExceptionStackSwitchHandlers ( + &StackSwitchData + ); + } + return Status; }
> -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang, > Jian J > Sent: Thursday, November 23, 2017 1:04 PM > To: Yao, Jiewen <jiewen....@intel.com>; edk2-devel@lists.01.org > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Laszlo Ersek > <ler...@redhat.com>; Dong, Eric <eric.d...@intel.com> > 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 <jian.j.w...@intel.com>; edk2-devel@lists.01.org > > Cc: Dong, Eric <eric.d...@intel.com>; Laszlo Ersek <ler...@redhat.com>; > > Kinney, Michael D <michael.d.kin...@intel.com> > > 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 > InitializeExceptionStackSwitchHandlers(). > > I think it's reasonable to do this because > InitializeExceptionStackSwitchHandlers() > is arch dependent. It'd be better not to call it in DxeCore. Another benefit > 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. > > > 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 provide > > 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 <eric.d...@intel.com>; Laszlo Ersek <ler...@redhat.com>; > > Yao, > > > Jiewen <jiewen....@intel.com>; Kinney, Michael D > > > <michael.d.kin...@intel.com> > > > 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 that 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 <eric.d...@intel.com> > > > Cc: Laszlo Ersek <ler...@redhat.com> > > > Cc: Jiewen Yao <jiewen....@intel.com> > > > Cc: Michael Kinney <michael.d.kin...@intel.com> > > > Suggested-by: Ayellet Wolman <ayellet.wol...@intel.com> > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Jian J Wang <jian.j.w...@intel.com> > > > --- > > > 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 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 > requirement. > > > + > > > + This function is a wrapper of > > > InitializeCpuExceptionStackSwitchHandlers. > > > + 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 = 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 = (IA32_IDT_GATE_DESCRIPTOR *)Idtr.Base; > > > + Status = 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 = FixedPcdGetSize (PcdCpuStackSwitchExceptionList); > > > + NewStackSize = FixedPcdGet32 (PcdCpuKnownGoodStackSize) * > > > ExceptionNumber; > > > + > > > + StackTop = AllocateRuntimeZeroPool (NewStackSize * > > > mNumberOfProcessors); > > > + ASSERT (StackTop != NULL); > > > + StackTop += NewStackSize * mNumberOfProcessors; > > > + > > > + EssData.Exceptions = FixedPcdGetPtr (PcdCpuStackSwitchExceptionList); > > > + EssData.ExceptionNumber = ExceptionNumber; > > > + EssData.StackSize = FixedPcdGet32 (PcdCpuKnownGoodStackSize); > > > + > > > + MpInitLibWhoAmI (&Bsp); > > > + for (Index = 0; Index < mNumberOfProcessors; ++Index) { > > > + // > > > + // To support stack switch, we need to re-construct GDT but not IDT. > > > + // > > > + if (Index == 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 exceptions > > > + // because of its IST feature. IA32 needs one TSS for each exception > > > + // in addition to current task. Since AP is not supposed to allocate > > > + // memory, we have to do it in BSP. To simplify the code, we allocate > > > + // 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 = sizeof (IA32_TSS_DESCRIPTOR) + > > > + (Gdtr.Limit + 1) + > > > + sizeof (IA32_TSS_DESCRIPTOR) * (ExceptionNumber + 1) + > > > + sizeof (IA32_TASK_STATE_SEGMENT) * (ExceptionNumber + > > > 1); > > > + GdtBuffer = AllocateRuntimeZeroPool (NewGdtSize); > > > + ASSERT (GdtBuffer != NULL); > > > + > > > + EssData.GdtTable = ALIGN_POINTER(GdtBuffer, sizeof > > > (IA32_TSS_DESCRIPTOR)); > > > + NewGdtSize -= ((UINT8 *)EssData.GdtTable - GdtBuffer); > > > + EssData.GdtSize = NewGdtSize; > > > + > > > + EssData.TssDesc = (IA32_TSS_DESCRIPTOR *)((UINTN)EssData.GdtTable + > > > + Gdtr.Limit + 1); > > > + EssData.Tss = (IA32_TASK_STATE_SEGMENT *)((UINTN)EssData.GdtTable > + > > > + Gdtr.Limit + 1 + > > > + sizeof > > > (IA32_TSS_DESCRIPTOR) * > > > + (ExceptionNumber + 1)); > > > + > > > + EssData.StackTop = (UINTN)StackTop; > > > + DEBUG ((DEBUG_INFO, "Exception stack top[%d]: 0x%lX\n", Index, > > > + (UINT64)(UINTN)StackTop)); > > > + > > > + if (Index == Bsp) { > > > + InitializeExceptionStackSwitchHandlers (&EssData); > > > + } else { > > > + MpInitLibStartupThisAP ( > > > + InitializeExceptionStackSwitchHandlers, > > > + Index, > > > + NULL, > > > + 0, > > > + (VOID *)&EssData, > > > + NULL > > > + ); > > > + } > > > + > > > + StackTop -= NewStackSize; > > > + } > > > +} > > > + > > > /** > > > Initialize Multi-processor support. > > > > > > @@ -624,6 +787,11 @@ InitializeMpSupport ( > > > mNumberOfProcessors = NumberOfProcessors; > > > DEBUG ((DEBUG_INFO, "Detect CPU count: %d\n", > mNumberOfProcessors)); > > > > > > + // > > > + // Initialize exception stack switch handlers for each logic processor. > > > + // > > > + 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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel