On 08/10/18 06:19, Eric Dong wrote: > Current implementation copies GDT/IDT at SmmReadyToLock point > from ACPI NVS memory to Smram. Later at S3 resume phase, it restores > memory saved in Smram to ACPI NVS. Driver can directly use GDT/IDT > saved in Smram instead of restore the original ACPI NVS memory. > > This patch do this change. > > Test Done: > Do the OS boot and S3 resume test. > > Cc: Laszlo Ersek <[email protected]> > Cc: Ruiyu Ni <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong <[email protected]> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > index 0b8ef70359..764d8276d3 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > @@ -448,13 +448,6 @@ PrepareApStartupVector ( > CopyMem ((VOID *) (UINTN) &mExchangeInfo->GdtrProfile, (VOID *) (UINTN) > mAcpiCpuData.GdtrProfile, sizeof (IA32_DESCRIPTOR)); > CopyMem ((VOID *) (UINTN) &mExchangeInfo->IdtrProfile, (VOID *) (UINTN) > mAcpiCpuData.IdtrProfile, sizeof (IA32_DESCRIPTOR)); > > - // > - // Copy AP's GDT, IDT and Machine Check handler from SMRAM to ACPI NVS > memory > - // > - CopyMem ((VOID *) mExchangeInfo->GdtrProfile.Base, mGdtForAp, > mExchangeInfo->GdtrProfile.Limit + 1); > - CopyMem ((VOID *) mExchangeInfo->IdtrProfile.Base, mIdtForAp, > mExchangeInfo->IdtrProfile.Limit + 1); > - CopyMem ((VOID *)(UINTN) mAcpiCpuData.ApMachineCheckHandlerBase, > mMachineCheckHandlerForAp, mAcpiCpuData.ApMachineCheckHandlerSize); > - > mExchangeInfo->StackStart = (VOID *) (UINTN) mAcpiCpuData.StackAddress; > mExchangeInfo->StackSize = mAcpiCpuData.StackSize; > mExchangeInfo->BufferStart = (UINT32) StartupVector; > @@ -901,6 +894,10 @@ GetAcpiCpuData ( > CopyMem (mGdtForAp, (VOID *)Gdtr->Base, Gdtr->Limit + 1); > CopyMem (mIdtForAp, (VOID *)Idtr->Base, Idtr->Limit + 1); > CopyMem (mMachineCheckHandlerForAp, (VOID > *)(UINTN)mAcpiCpuData.ApMachineCheckHandlerBase, > mAcpiCpuData.ApMachineCheckHandlerSize); > + > + Gdtr->Base = (UINTN)mGdtForAp; > + Idtr->Base = (UINTN)mIdtForAp; > + mAcpiCpuData.ApMachineCheckHandlerBase = > (EFI_PHYSICAL_ADDRESS)(UINTN)mMachineCheckHandlerForAp; > } > > /** >
This patch looks good to me (I'm ready to give R-b), but I think we can simplify the code further. Can we replace the mGdtForAp, mIdtForAp and mMachineCheckHandlerForAp global variables, with GdtForAp, IdtForAp and MachineCheckHandlerForAp local variables, used only in the GetAcpiCpuData() function? If you prefer to do that as an incremental patch, that's fine too. Thanks, Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
