Jiewen, Yes. That is a longer term goal for the PiSmmCpuDxeSmm. Making the GDTs consistent helps debug and is a fix that works until we can add logic to PiSmmCpuiDxeSmm to inherit current GDT information and use it throughout this module.
Thanks, Mike >-----Original Message----- >From: Yao, Jiewen >Sent: Friday, October 30, 2015 7:18 AM >To: Laszlo Ersek; 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 > >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
