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:[email protected]] On Behalf Of Wang,
> Jian J
> Sent: Thursday, November 23, 2017 1:04 PM
> To: Yao, Jiewen <[email protected]>; [email protected]
> Cc: Kinney, Michael D <[email protected]>; Laszlo Ersek
> <[email protected]>; Dong, Eric <[email protected]>
> 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 <[email protected]>; [email protected]
> > Cc: Dong, Eric <[email protected]>; Laszlo Ersek <[email protected]>;
> > Kinney, Michael D <[email protected]>
> > 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: [email protected]
> > > Cc: Dong, Eric <[email protected]>; Laszlo Ersek <[email protected]>;
> > Yao,
> > > Jiewen <[email protected]>; Kinney, Michael D
> > > <[email protected]>
> > > 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 <[email protected]>
> > > Cc: Laszlo Ersek <[email protected]>
> > > Cc: Jiewen Yao <[email protected]>
> > > Cc: Michael Kinney <[email protected]>
> > > Suggested-by: Ayellet Wolman <[email protected]>
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Jian J Wang <[email protected]>
> > > ---
> > > 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
> [email protected]
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
