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
