Yes, first, thanks the great analysis from Laszlo. Is that possible to eliminate the assumption by parsing current GDT entry?
Instead of hardcode 0x38 or 0x28, if PiSmmCpu inherits GDT from other module, should it parse GDT to get correct long mode segment? I do not object the idea to change DxeCpu driver. I'm just thinking if we have robust way to prevent error happening again... if DxeCpu driver is not written by us. Thank you Yao Jiewen -----Original Message----- From: edk2-devel [mailto:[email protected]] On Behalf Of Laszlo Ersek Sent: Friday, October 30, 2015 4:56 AM To: Kinney, Michael D; Yao, Jiewen; Fan, Jeff Cc: Justen, Jordan L; edk2-devel-01 Subject: Re: [edk2] about the SMM_S3_RESUME_SMM_64 branch in S3Resume2Pei On 10/29/15 18:30, Kinney, Michael D wrote: > 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. I agree this is the best solution. (I didn't know which one of CpuDxe and DxeIplPeim should prevail in dictating the GDT convention, so I didn't suggest any unification like this.) Thanks! Laszlo > > 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 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
