Laszlo, Thanks for the very detailed and complete root cause on this issue. I agree that the change you suggest is functional, but I am going to recommend a different solution.
Unfortunately, there are several modules that set up a GDT and it is easier from an implementation perspective and for debugging if all the modules agree to use the same GDT layout. It would be even better if no modules make any assumptions about GDT layouts, but that is a bigger change that we can tackle later. The root cause of this specific issue is that the GDT that is set up by MdeModulePkg/Core/DxeIplPeim is not the same GDT that is set up by UefiCpuPkg /CpuDxe. And the UefiCpuPkg/PiSmmCpuDxeSmm module has some assumptions that the GDT is the one setup by MdeModulePkg/Core/DxeIplPeim. I recommend we update UefiCpuPkg/CpuDxe to use the same GDT layout as MdeModulePkg/Core/DxeIplPeim and then the changes you found worked for PiSmmCpuDxeSmm are not required. I will work on a patch for you to test. Mike >-----Original Message----- >From: edk2-devel [mailto:[email protected]] On Behalf Of >Laszlo Ersek >Sent: Thursday, October 29, 2015 9:19 AM >To: Yao, Jiewen; Kinney, Michael D; Fan, Jeff >Cc: Justen, Jordan L; edk2-devel-01 >Subject: Re: [edk2] about the SMM_S3_RESUME_SMM_64 branch in >S3Resume2Pei > >Hi Jiewen, > >On 10/29/15 03:28, Yao, Jiewen wrote: >> Thanks for the info. I think we might have to dig out why LONG_JUMP fail >here. >> Most our IA BIOS support Ia32X64 mode, so I am sure it should work. > >I think I have found the bug. > >(1) >I added a debug patch to OvmfPkg/QuarkPort/CpuS3DataDxe. I'm attaching it >for illustration. The debug patch dumps the following information: >- the location of the GDT descriptor (that gets loaded into the GDT register) >- the contents of the GDT descriptor (i.e., the base and limit of the global >descriptor table) >- the contents of the GDT entries (the individual segment descriptors) > >CpuS3DataDxe saves these in AcpiNVS. Later they are saved by >PiSmmCpuDxeSmm from AcpiNVS to SMRAM (during boot), and then restored >from SMRAM to AcpiNVS (during S3 Resume). > >The patch dumps these details when the preparation for PiSmmCpuDxeSmm, >in AcpiNVS, is ready. > >Here's the debug output: > >SaveCpuS3Data: 350: GDT descriptor at 0x7E3E2050 >SaveCpuS3Data: 353: GDT Base=0x7F706000 Limit=0x3F >SaveCpuS3Data: 363: Idx=0x00 Base=0x00000000 Limit=0x00000000 Type=0x0 >System=0x0 Dpl=0x0 Present=0x0 Software=0x0 LCodeSeg=0x0 DefaultSize=0x0 >SaveCpuS3Data: 363: Idx=0x08 Base=0x00000000 Limit=0xFFFFFFFF Type=0x3 >System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x0 DefaultSize=0x1 >SaveCpuS3Data: 363: Idx=0x10 Base=0x00000000 Limit=0xFFFFFFFF Type=0xB >System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x0 DefaultSize=0x1 >SaveCpuS3Data: 363: Idx=0x18 Base=0x00000000 Limit=0xFFFFFFFF Type=0x2 >System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x0 DefaultSize=0x1 >SaveCpuS3Data: 363: Idx=0x20 Base=0x00000000 Limit=0xFFFFFFFF Type=0xA >System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x0 DefaultSize=0x1 >SaveCpuS3Data: 363: Idx=0x28 Base=0x00000000 Limit=0xFFFFFFFF Type=0xB >System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x1 DefaultSize=0x0 >SaveCpuS3Data: 363: Idx=0x30 Base=0x00000000 Limit=0x00000000 Type=0x0 >System=0x0 Dpl=0x0 Present=0x0 Software=0x0 LCodeSeg=0x0 DefaultSize=0x0 >SaveCpuS3Data: 363: Idx=0x38 Base=0x00000000 Limit=0x00000000 Type=0x0 >System=0x0 Dpl=0x0 Present=0x0 Software=0x0 LCodeSeg=0x0 DefaultSize=0x0 > >(Importantly, the debug output is identical between the pure Ia32 build and >the Ia32X64 build, except the addresses in the first two lines -- the location >of >the GDT descriptor, and the location of the GDT itself.) > > >(2) Now locate the offset (= selector) of the *only* segment that is advertised >as "64-bit code". The LCodeSeg bit-field is 1 only for Idx=0x28. > > >(3) If you examine "UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.asm", you >can see: > >LONG_JUMP:: > > db 67h, 0EAh ; far jump > dd 0h ; 32-bit offset > dw 38h ; 16-bit selector > >The segment selector is 0x38 here, which not only lacks the LCodeSeg (= long >mode code) bit, but it is actually a null-descriptor. > >So this is the direct cause for the triple fault, but why does it work in the >pure >Ia32 case, and what is the *root* cause for the triple fault in the Ia32X64 >case? > >We can investigate some more: > > >(4) The DXE IPL PEIM actually sets up the 0x38 segment correctly. In the >HandOffToDxeCore() function >[MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c], branch >"PcdDxeIplSwitchToLongMode", the last statement is: > > // > // Go to Long Mode and transfer control to DxeCore. > // Interrupts will not get turned on until the CPU AP is loaded. > // Call x64 drivers passing in single argument, a pointer to the HOBs. > // > AsmEnablePaging64 ( > SYS_CODE64_SEL, > DxeCoreEntryPoint, > (EFI_PHYSICAL_ADDRESS)(UINTN)(HobList.Raw), > 0, > TopOfStack > ); > >where SYS_CODE64_SEL equals 0x38. Because the 64-bit DXE core starts >running, segment 0x38 is obviously correctly configured at this point. (See >also the "gGdtEntries" array in >"MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c" -- the entry at byte >offset 0x38 sets the LCodeSeg bit.) > > >(5) *However*, when the CpuDxe driver starts, the InitializeCpu() entry point >function does the following: > >InitializeCpu() [UefiCpuPkg/CpuDxe/CpuDxe.c] > > // > // Init GDT for DXE > // > InitGlobalDescriptorTable (); > > // > // Initialize all GDT entries > // > CopyMem (gdt, &GdtTemplate, sizeof (GdtTemplate)); > >And the GdtTemplate array [UefiCpuPkg/CpuDxe/CpuGdt.c] is what we can >see in the debug dump above! > >0x38 corresponds to "SPARE5_SEL" in CpuDxe, and the only long-mode code >segment is LINEAR_CODE64_SEL (0x28). > >In summary, the bug is that CpuDxe sets up a GDT that is incompatible with >"UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.*". > > >(6) I'm unsure how this could be fixed. *Although* both "gGdtEntries" from >the DXE IPL, and "GdtTemplate" from CpuDxe, have one 64-bit code segment >each, the segment descriptors are not identical. (For which reason I'm unsure >if we can simply point "UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.*" to >the segment that comes from CpuDxe, when the assembly had been clearly >written with the DXE IPL's segment in mind.) > >There is precisely one difference between them: the one from the DXE IPL has >type 0xa (according to the Intel SDM, that means "Execute/Read"), whereas >the one from CpuDxe has type 0xb ("Execute/Read, accessed"). > >I guess the "accessed" bit in the segment type is not a critical difference. >Some >confirmation would be nice. > > >(7) Side investigation for the pure Ia32 build, and for the protected mode >stage of the X64 MpFuncs: for jumping to PMODE_ENTRY, both >"Ia32/MpFuncs.asm" and "X64/MpFuncs.asm" use selector 0x20: > >FLAT32_JUMP:: > > db 66h, 67h, 0EAh ; far jump > dd 0h ; 32-bit offset > dw 20h ; 16-bit selector > >PMODE_ENTRY:: ; protected mode entry point > >Now, the segment identified by selector 0x20 in gGdtEntries (from the DXE >IPL) is: > >/* 0x20 */ {{0xffff, 0, 0, 0xa, 1, 0, 1, 0xf, 0, 0, 1, 1, 0}}, >//system code >segment descriptor > >whereas the segment identified by the same selector 0x20 in GdtTemplate >(from CpuDxe) is: > >SaveCpuS3Data: 363: Idx=0x20 Base=0x00000000 Limit=0xFFFFFFFF Type=0xA >System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x0 DefaultSize=0x1 > >which are *exactly* the same. > >This is the reason why the pure Ia32 build works fine, and also the reason >why the Ia32X64 build doesn't blow up in FLAT32_JUMP (only in LONG_JUMP). > > >(8) As far as testing goes, with the following patch in place: > >> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.S >b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.S >> index d7cbc8c..146b54b 100644 >> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.S >> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.S >> @@ -120,7 +120,7 @@ LONG_JUMP: >> >> .byte 0x67,0xEA # far jump >> .long 0x0 # 32-bit offset >> - .word 0x38 # 16-bit selector >> + .word 0x28 # 16-bit selector >> >> LongModeStart: >> > >the Ia32X64 guest *still* crashes, but it occurs somewhat later now: > > qemu-system-x86-25660 [000] 20922.542598: kvm_exit: reason >CR_ACCESS rip 0x97077 info 0 0 > qemu-system-x86-25660 [000] 20922.542598: kvm_cr: cr_write 0 = >0xe0000011 > qemu-system-x86-25660 [000] 20922.542601: kvm_entry: vcpu 1 > > qemu-system-x86-25660 [000] 20922.542602: kvm_exit: reason >TRIPLE_FAULT rip 0x97086 info 0 0 > qemu-system-x86-25660 [000] 20922.542603: kvm_userspace_exit: reason >KVM_EXIT_SHUTDOWN (8) > >(9) I *think* this then hits somewhere in here: > >LongModeStart:: > > mov ax, 30h > mov ds, ax > mov es, ax > mov ss, ax > >Looking again the segment descriptors at the top, the 0x30 selector is just as >invalid -- CpuDxe calls it SPARE4_SEL. > >Hm... let's see what it used to be in the DXE IPL PEIM... "gGdtEntries" in >"MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c": > >/* 0x30 */ {{0xffff, 0, 0, 0x2, 1, 0, 1, 0xf, 0, 0, 1, 1, 0}}, >//system data >segment descriptor > >From CpuDxe, it seems to match SYS_DATA_SEL == 0x18 precisely: > >SaveCpuS3Data: 363: Idx=0x18 Base=0x00000000 Limit=0xFFFFFFFF Type=0x2 >System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x0 DefaultSize=0x1 > >(10) YES! :) With the following cumulative patch: > >> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.S >b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.S >> index d7cbc8c..6ec6363 100644 >> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.S >> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.S >> @@ -120,11 +120,11 @@ LONG_JUMP: >> >> .byte 0x67,0xEA # far jump >> .long 0x0 # 32-bit offset >> - .word 0x38 # 16-bit selector >> + .word 0x28 # 16-bit selector >> >> LongModeStart: >> >> - movw $0x30,%ax >> + movw $0x18,%ax >> .byte 0x66 >> movw %ax,%ds >> .byte 0x66 > >S3 resume works in the Ia32X64 build!!! :) > >This is the final part of the log written during S3 resume: > >> SmmLockBoxPeiLib RestoreAllLockBoxInPlace - Exit (Success) >> S3NvsPageTableAddress - 7DFDE000 (1) >> SMM S3 Signature = 534D4D53 >> SMM S3 Stack Base = 7FF8A000 >> SMM S3 Stack Size = 8000 >> SMM S3 Resume Entry Point = 7FFB5617 >> SMM S3 CR0 = 80000033 >> SMM S3 CR3 = 7FF84000 >> SMM S3 CR4 = 668 >> SMM S3 Return CS = 10 >> SMM S3 Return Entry Point = 846C69 >> SMM S3 Return Context1 = 7F6FA000 >> SMM S3 Return Context2 = 7E039000 >> SMM S3 Return Stack Pointer = 81730C >> SMM S3 Smst = 7FFFDE00 >> SmmRestoreCpu() >> SMM S3 Return CS = 10 >> SMM S3 Return Entry Point = 846C69 >> SMM S3 Return Context1 = 7F6FA000 >> SMM S3 Return Context2 = 7E039000 >> SMM S3 Return Stack Pointer = 81730C >> Call AsmDisablePaging64() to return to S3 Resume in PEI Phase >> S3ResumeExecuteBootScript() >> Close all SMRAM regions before executing boot script >> Lock all SMRAM regions before executing boot script >> PeiS3ResumeState - 81D1A8 >> Enable X64 and transfer control to Standalone Boot Script Executor >> S3BootScriptExecute: >> TableHeader - 0x7E620000 >> TableHeader.Version - 0x0001 >> TableHeader.TableLength - 0x00000045 >> ExecuteBootScript - 7E62000D >> EFI_BOOT_SCRIPT_IO_READ_WRITE_OPCODE >> BootScriptExecuteIoReadWrite - 0x0000B030, 0x00000000FFFFFFFF, >0x0000000000000021 >> S3BootScriptWidthUint32 - 0x0000B030 >> S3BootScriptWidthUint32 - 0x0000B030 (0x00000021) >> ExecuteBootScript - 7E620024 >> EFI_BOOT_SCRIPT_PCI_CONFIG_READ_WRITE_OPCODE >> BootScriptExecutePciCfgReadWrite - 0x000780A0, 0x000000000000FFFF, >0x0000000000000010 >> S3BootScriptWidthUint16 - 0x000780A0 >> S3BootScriptWidthUint16 - 0x000780A0 (0xFFFF) >> ExecuteBootScript - 7E620037 >> EFI_BOOT_SCRIPT_INFORMATION_OPCODE >> BootScriptExecuteInformation - 0x7E62003E >> BootScriptInformation: DE AD BE EF >> ExecuteBootScript - 7E620042 >> S3_BOOT_SCRIPT_LIB_TERMINATE_OPCODE >> S3BootScriptDone - Success >> Call AsmDisablePaging64() to return to S3 Resume in PEI Phase >> Install PPI: 88C9D306-0900-4EB5-8260-3E2DBEDA1F89 >> Install PPI: 605EA650-C65C-42E1-BA80-91A52AB618C6 >> Transfer to 16bit OS waking vector - 991D0 > >Do you guys agree that the cumulative patch above is correct? If so, then I'll >extend the fix to cover the MpFuncs.asm file as well (not just MpFuncs.S), and >then submit it as a formal patch. > >... With this bug analyzed, we have no more *functional* bugs that would >*directly* block the SMM-for-OVMF series. Should we find other bugs, those >are now possible to solve incrementally. What remains is: >- reaching agreements in the pending debates >- addressing patch feedback and securing reviews. > >Thank you >Laszlo > > >> >> For "RSM from 64-bit mode ", I have already confirmed with IA32 SDM >owner that it is just typo. >> RSM should work in 64bit mode, so this patch is unnecessary, I believe. >> For debug purpose, you can apply it at first in your branch, just in case it >> is >emulator error. >> >> Have a good night! >> >> Thank you >> Yao Jiewen >> >> -----Original Message----- >> From: edk2-devel [mailto:[email protected]] On Behalf Of >Laszlo Ersek >> Sent: Thursday, October 29, 2015 9:50 AM >> To: Yao, Jiewen; Kinney, Michael D; Fan, Jeff >> Cc: edk2-devel-01 >> Subject: Re: [edk2] about the SMM_S3_RESUME_SMM_64 branch in >S3Resume2Pei >> >> On 10/29/15 02:32, Laszlo Ersek wrote: >>> On 10/29/15 01:31, Yao, Jiewen wrote: >>>> Hi Ersek >>>> I think S3ResumePei supports Ia32 and Ia32X64. It does not support X64. >So I believe Ia32X64 crash is a bug somewhere. >>>> >>>> Since you already run into SmmRestoreCpu(), would you please help to >check where is last instruction causing crash? >>> >>> Sure. The crash occurs on the following call path (starting with >SmmRestoreCpu()): >>> >>> SmmRestoreCpu() >[UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c] >>> EarlyInitializeCpu() [UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c] >>> SendInitSipiSipiAllExcludingSelf() >>> [UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c] >>> >>> I have a debug print right after the >>> SendInitSipiSipiAllExcludingSelf(), and the BSP starts printing it, >>> but before the message is completely printed, one of the APs (just >>> started) seems to crash the VM. >>> >>> It is perfectly possible that the ACPI_CPU_DATA structure that >>> OvmfPkg/QuarkPort/CpuS3DataDxe collects earlier causes this. Because, >>> before EarlyInitializeCpu() calls SendInitSipiSipiAllExcludingSelf(), >>> the startup vector is prepared from ACPI_CPU_DATA, in the >>> PrepareApStartupVector() function. No clue what goes wrong there. >>> >>> Let me check the KVM trace though... Okay, I massaged it a little bit >>> for easier readability. Here's when VCPU#0 runs >>> SendInitSipiSipiAllExcludingSelf(): >>> >>>> kvm_exit: reason APIC_ACCESS rip 0x7ffc9d7f info 1300 0 >>>> kvm_emulate_insn: 0:7ffc9d7f: 89 10 >>>> kvm_mmio: mmio write len 4 gpa 0xfee00300 val 0xc4697 >>>> kvm_apic: apic_write APIC_ICR = 0xc4697 >>>> kvm_apic_ipi: dst 0 vec 151 >>>> (SIPI|physical|assert|edge|all-but-self) >>>> kvm_apic_accept_irq: apicid 1 vec 151 (SIPI|edge) >>>> kvm_apic_accept_irq: apicid 2 vec 151 (SIPI|edge) >>>> kvm_apic_accept_irq: apicid 3 vec 151 (SIPI|edge) >>>> kvm_entry: vcpu 0 >>> >>>> kvm_exit: reason APIC_ACCESS rip 0x7ffc9d06 info 300 0 >>>> kvm_emulate_insn: 0:7ffc9d06: 8b 00 >>>> kvm_apic: apic_read APIC_ICR = 0xc4697 >>>> kvm_mmio: mmio read len 4 gpa 0xfee00300 val 0xc4697 >>>> kvm_entry: vcpu 0 >>> >>>> kvm_exit: reason APIC_ACCESS rip 0x7ffc9d7f info 1310 0 >>>> kvm_emulate_insn: 0:7ffc9d7f: 89 10 >>>> kvm_mmio: mmio write len 4 gpa 0xfee00310 val 0x0 >>>> kvm_apic: apic_write APIC_ICR2 = 0x0 >>>> kvm_entry: vcpu 0 >>> >>> And in response, this is how VCPU#1 behaves (I guess this matches >"UefiCpuPkg/PiSmmCpuDxeSmm/X64/MpFuncs.asm"?): >>> >>>> kvm_exit: reason CR_ACCESS rip 0x35 info 0 0 >>>> kvm_cr: cr_write 0 = 0x60000011 >>>> kvm_entry: vcpu 1 >>> >>>> kvm_exit: reason CR_ACCESS rip 0x9705b info 4 0 >>>> kvm_cr: cr_write 4 = 0x20 >>>> kvm_entry: vcpu 1 >>> >>>> kvm_exit: reason CR_ACCESS rip 0x9705e info 103 0 >>>> kvm_cr: cr_write 3 = 0x7ff83000 >>>> kvm_entry: vcpu 1 >>> >>>> kvm_exit: reason MSR_READ rip 0x97068 info 0 0 >>>> kvm_msr: msr_read c0000080 = 0x0 >>>> kvm_entry: vcpu 1 >>> >>>> kvm_exit: reason MSR_WRITE rip 0x9706e info 0 0 >>>> kvm_msr: msr_write c0000080 = 0x100 >>>> kvm_entry: vcpu 1 >>> >>>> kvm_exit: reason CR_ACCESS rip 0x97077 info 0 0 >>>> kvm_cr: cr_write 0 = 0xe0000011 >>>> kvm_entry: vcpu 1 >>> >>>> kvm_exit: reason TRIPLE_FAULT rip 0x9707a info 0 0 >>>> kvm_userspace_exit: reason KVM_EXIT_SHUTDOWN (8) >>> >>> From counting the bytes in "MpFuncs.asm", and correlating them with >>> the RIP values in the trace, I *think* that the triple fault happens >>> right here: >>> >>> LONG_JUMP:: >>> >>> db 67h, 0EAh ; far jump >>> dd 0h ; 32-bit offset >>> dw 38h ; 16-bit selector >>> >>> Do you want me to log some values from from PrepareApStartupVector()? >>> Like the offset that gets patched in here, or the GDT (to see that >>> 0x38 makes sense)... I might have to do that tomorrow ^W after getting >>> some sleep however, it's 02:32 AM here. >> >> FWIW I retested with TCG too (i.e., emulation, not hw virtualization), and >the VM crashes the same way. >> >> Does it matter that I have this patch in my build, temporarily: >> >> UefiCpuPkg: PiSmmCpuDxeSmm: do not execute RSM from 64-bit mode >> http://thread.gmane.org/gmane.comp.bios.edk2.devel/3020/focus=3023 >> >> (I don't think it matters at this stage in "MpFuncs.asm", but I should be >> clear >about my environment...) >> >> Thanks >> Laszlo >> >> >>>> -----Original Message----- >>>> From: Laszlo Ersek [mailto:[email protected]] >>>> Sent: Thursday, October 29, 2015 7:59 AM >>>> To: Yao, Jiewen; Kinney, Michael D; Fan, Jeff >>>> Cc: edk2-devel-01 >>>> Subject: Re: about the SMM_S3_RESUME_SMM_64 branch in >S3Resume2Pei >>>> >>>> On 10/28/15 23:41, Laszlo Ersek wrote: >>>>> On 10/28/15 23:26, Yao, Jiewen wrote: >>>>>> Right. It seems S3Resume2Pei does not consider X64 mode. I found at >least 3 functions need enhancement on mode transition: >>>>>> 1) S3RestoreConfig2() - S3Resume <-> SmmCpu (DXE mode); >>>>>> 2) S3ResumeExecuteBootScript() - S3Resume <-> BootScriptExecutor >>>>>> (DXE >>>>>> mode) >>>>>> 3) S3ResumeBootOs() - S3Resume -> OS WakingVector (OS decide). >>>>> >>>>> In practice at least, these problems appear specific to SMM / SMRAM >>>>> usage. When we use OVMF's custom (insecure) LockBoxLib instance, the >>>>> X64 build of S3Resume2Pei (actually, a fully X64 build of OVMF) >>>>> provides a working S3 feature, including Windows 7 and later guests, >>>>> and Linux guests. Even a minimal boot script is executed correctly >>>>> (it has just an INFO opcode). >>>>> >>>>> If I remember correctly, quite a few code paths are possible through >>>>> S3Resume2Pei. I don't exactly recall which one is taken in the above >>>>> case, but I thought I'd point out that it works very well in practice. >>>>> (The fact notwithstanding that the lockbox is not protected from the >>>>> runtime guest OS.) >>>>> >>>>> The pure Ia32 case works well both with and without OVMF's SMM >feature. >>>>> >>>>> I don't recall ever testing S3 with the Ia32X64 build; I plan to do >>>>> that soonish. >>>> >>>> Ia32X64 crashes (with SMM enabled) with the following messages leading >up to it: >>>> >>>> -------- >>>> SmmLockBoxPeiLib RestoreAllLockBoxInPlace - Exit (Success) >S3NvsPageTableAddress - 7DFDE000 (1) >>>> SMM S3 Signature = 534D4D53 >>>> SMM S3 Stack Base = 7FF8A000 >>>> SMM S3 Stack Size = 8000 >>>> SMM S3 Resume Entry Point = 7FFB5617 >>>> SMM S3 CR0 = 80000033 >>>> SMM S3 CR3 = 7FF84000 >>>> SMM S3 CR4 = 668 >>>> SMM S3 Return CS = 10 >>>> SMM S3 Return Entry Point = 846C69 >>>> SMM S3 Return Context1 = 7F6FA000 >>>> SMM S3 Return Context2 = 7E039000 >>>> SMM S3 Return Stack Pointer = 81730C >>>> SMM S3 Smst = 7FFFDE00 >>>> SmmRestoreCpu() >>>> <CRASH> >>>> -------- >>>> >>>> If I build without SMM, then Ia32X64 works fine as well. >>>> >>>> Summary: >>>> - without SMM: S3 works in all three of the Ia32, Ia32X64, and X64 >>>> OVMF builds >>>> - with SMM: Ia32 works, the other two crash. >>>> >>>> I guess this just confirms what you've already determined from the code. >>>> But, at least, it confirms it. :) >>>> >>>> Thank you all for looking into it! >>>> Laszlo >>>> >>>> >>>>> >>>>> Thanks, >>>>> Laszlo >>>>> >>>>>> Thank you >>>>>> Yao Jiewen >>>>>> >>>>>> -----Original Message----- >>>>>> From: Laszlo Ersek [mailto:[email protected]] >>>>>> Sent: Thursday, October 29, 2015 1:34 AM >>>>>> To: Kinney, Michael D; Fan, Jeff; Yao, Jiewen >>>>>> Cc: edk2-devel-01 >>>>>> Subject: Re: about the SMM_S3_RESUME_SMM_64 branch in >S3Resume2Pei >>>>>> >>>>>> On 10/28/15 17:54, Kinney, Michael D wrote: >>>>>>> Laszlo, >>>>>>> >>>>>>> I do not believe any X64 PEI testing has not been performed with this >module. We will investigate a fix. >>>>>> >>>>>> Thank you. >>>>>> >>>>>> In any case, in OVMF we might be able to use this module nonetheless, >with the OvmfPkgIa32X64.dsc build (== 32-bit PEI, 64-bit DXE). >>>>>> >>>>>> Thanks! >>>>>> Laszlo >>>>>> >>>>>>> >>>>>>> Thanks, >>>>>>> >>>>>>> Mike >>>>>>> >>>>>>>> -----Original Message----- >>>>>>>> From: Laszlo Ersek [mailto:[email protected]] >>>>>>>> Sent: Wednesday, October 28, 2015 8:57 AM >>>>>>>> To: Fan, Jeff; Yao, Jiewen >>>>>>>> Cc: edk2-devel-01; Kinney, Michael D >>>>>>>> Subject: about the SMM_S3_RESUME_SMM_64 branch in >S3Resume2Pei >>>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> I have a question about the following code in >>>>>>>> "UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c", function >>>>>>>> S3RestoreConfig2(): >>>>>>>> >>>>>>>>> if (SmmS3ResumeState->Signature == >SMM_S3_RESUME_SMM_64) { >>>>>>>>> // >>>>>>>>> // Switch to long mode to complete resume. >>>>>>>>> // >>>>>>>>> >>>>>>>>> InterruptStatus = SaveAndDisableInterrupts (); >>>>>>>>> // >>>>>>>>> // Need to make sure the GDT is loaded with values that >>>>>>>>> support long >>>>>>>> mode and real mode. >>>>>>>>> // >>>>>>>>> AsmWriteGdtr (&mGdt); >>>>>>>>> // >>>>>>>>> // update segment selectors per the new GDT. >>>>>>>>> // >>>>>>>>> AsmSetDataSelectors (DATA_SEGEMENT_SELECTOR); >>>>>>>>> // >>>>>>>>> // Restore interrupt state. >>>>>>>>> // >>>>>>>>> SetInterruptState (InterruptStatus); >>>>>>>>> >>>>>>>>> AsmWriteCr3 ((UINTN)SmmS3ResumeState->SmmS3Cr3); >>>>>>>>> >>>>>>>>> // >>>>>>>>> // Disable interrupt of Debug timer, since IDT table >>>>>>>>> cannot work in long >>>>>>>> mode. >>>>>>>>> // NOTE: On x64 platforms, because DisablePaging64() will >>>>>>>>> disable >>>>>>>> interrupts, >>>>>>>>> // the code in S3ResumeExecuteBootScript() cannot be >>>>>>>>> halted by soft >>>>>>>> debugger. >>>>>>>>> // >>>>>>>>> SaveAndSetDebugTimerInterrupt (FALSE); >>>>>>>>> >>>>>>>>> AsmEnablePaging64 ( >>>>>>>>> 0x38, >>>>>>>>> SmmS3ResumeState->SmmS3ResumeEntryPoint, >>>>>>>>> (UINT64)(UINTN)AcpiS3Context, >>>>>>>>> 0, >>>>>>>>> SmmS3ResumeState->SmmS3StackBase + SmmS3ResumeState- >>>>>>>>> SmmS3StackSize >>>>>>>>> ); >>>>>>>>> } >>>>>>>> >>>>>>>> At the end of this block, the AsmEnablePaging64() function is called. >>>>>>>> That call results in the following call tree, *if* the module was built >for X64: >>>>>>>> >>>>>>>> AsmEnablePaging64() >[MdePkg/Library/BaseLib/X86EnablePaging64.c] >>>>>>>> InternalX86EnablePaging64() [MdePkg/Library/BaseLib/X64/Non- >existing.c] >>>>>>>> ASSERT (FALSE) >>>>>>>> >>>>>>>> This is because the InternalX86EnablePaging64() functionality is >>>>>>>> unavailable in BaseLib on X64. >>>>>>>> >>>>>>>> My question: how is this branch in S3RestoreConfig2() supposed to >>>>>>>> work *at >>>>>>>> all* in an X64 PEI build? >>>>>>>> >>>>>>>> Thank you, >>>>>>>> Laszlo >>>>>> >>>>> >>>> >>> >> >> _______________________________________________ >> 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 >> _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
