Re: [edk2] apparent KVM problem with LRET in TianoCore S3 resume trampoline

[Laszlo Ersek <lersek@redhat.com>]

On 12/05/13 18:42, Paolo Bonzini wrote:
> Il 05/12/2013 17:12, Laszlo Ersek ha scritto:
>> Hi,
>>
>> I'm working on S3 suspend/resume in OVMF. The problem is that I'm getting an
>> unexpected guest reboot for code (LRET) that works on physical hardware. I
>> tried to trace the problem with ftrace, but I didn't get any mentions of
>> em_ret_far(). (Maybe I was looking in the wrong place.)
> 
> What does ftrace say anyway?

(pls. see in the next msg I sent)

> 
>> Please find the the assembly-language "trampoline" that is invoked (in 64-bit
>> mode) with the 16-bit real mode resume vector placed in "rcx" (EFIAPI calling
>> convention). The excerpt is from the edk2 tree,
>> "MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/S3Asm.S".
> 
> Can you send me a pointer to a git tree or, even better, an OVMF.fd file + instructions
> on how to trigger the problem?

http://people.redhat.com/~lersek/ovmf_s3_lret/

I use a stock F19 guest, with the default systemd target set to
multi-user. Then I login as root at the console, and issue "pm-suspend".
Once the guest is suspended, I type

virsh qemu-monitor-command ovmf.f19 --hmp system_wakeup

on the host.

At this point the guest starts spinning (visible eg. in the virt-manager
CPU usage chart), and the OVMF debug log (written to the qemu debug
console) is continuously growing. It always takes the same path: selects
the S3 boot path due to the 0xFE byte at 0xF in CMOS, progresses to the
trampoline, and resets.


> 
>>   - shared properties:
>>     Base=0x00000000
>>     Limit=0xFFFFF
>>     Type=0xB (C ER A     )
>>     S=0x1 (code/data)
>>     DPL=0x0
>>     Present=1
>>     Avail=0
>>     LimitGran=0x1 (4KB)
>>
>>   - different properties:
>>     0x0010: 0x00CF9B000000FFFF: 64-bitC=0 D/B=1  works
>>     0x0028: 0x008F9B000000FFFF: 64-bitC=0 D/B=0  reboots
>>     0x0038: 0x00AF9B000000FFFF: 64-bitC=1 D/B=0  works
>>
>> That is:
>> - if I let 64-bit mode execution enabled (64-bitC=1, desc 0x38), the lret
>>   works.
>> - If I switch to compat mode execution (64-bitC=0, desc 0x10), *and* keep the
>>   default addr/op size 32 bits, the lret still works.
> 
> Perhaps you could try switching to 32-bit mode first, then disable paging,
> then jump to 16-bit mode.  Like this (untested):

I had something like this in mind (I even mentioned it on edk2-devel
<http://thread.gmane.org/gmane.comp.bios.tianocore.devel/5297/focus=5331>),
but didn't know how to implement it. There are at least 6 factors in
play here:
- the L bit in the segment descriptor,
- the D/B bit in the segment descriptor,
- Paging,
- Protection Enable,
- IA-32e Mode Enable,
- PAE.

That's (almost) 6! orderings to test :)

So thanks a lot for suggesting a patch, I'll try to play with it.

> 
> diff --git a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/S3Asm.S b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/S3Asm.S
> index e59fd04..d1cac9d 100644
> --- a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/S3Asm.S
> +++ b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/S3Asm.S
> @@ -19,7 +19,7 @@ ASM_PFX(AsmTransferControl):
>      # rcx S3WakingVector    :DWORD
>      # rdx AcpiLowMemoryBase :DWORD
>      lea   _AsmTransferControl_al_0000(%rip), %eax 
> -    movq  $0x2800000000, %r8 
> +    movq  $0x1000000000, %r8 
>      orq   %r8, %rax
>      pushq %rax
>      shrd  $20, %ecx, %ebx
> @@ -28,24 +28,32 @@ ASM_PFX(AsmTransferControl):
>      movl  %ebx, jmp_addr(%rip) 
>      lret
>  _AsmTransferControl_al_0000:
> +    # Old SS should still be okay?
> +    addl  _AsmTransferControl_al_0001-_AsmTransferControl_al_0000, %eax
> +    pushl $0x28
> +    pushl %eax
> +    movq  %cr0, %rax
> +    movq  %cr4, %rbx
> +    andl  $0x7fffffff, %eax
> +    andb  $0xdf, %bl
> +    movq  %rax, %cr0             # sets EFER.LMA=0 too, so says Intel
> +    movl  $0x0c0000080, %ecx
> +    rdmsr
> +    andb  $0xfe, %ah             # set EFER.LME=0
> +    wrmsr
> +    movq  %rbx, %cr4             # only now set CR4.PAE=0
> +    lret
> +_AsmTransferControl_al_0001:
>      .byte    0x0b8, 0x30, 0      # mov ax, 30h as selector
>      movl  %eax, %ds
>      movl  %eax, %es
>      movl  %eax, %fs
>      movl  %eax, %gs
>      movl  %eax, %ss
> -    movq  %cr0, %rax
> -    movq  %cr4, %rbx
> -    .byte    0x66
> -    andl  $0x7ffffffe, %eax 
> -    andb  $0xdf, %bl 
> -    movq  %rax, %cr0
> -    .byte    0x66
> -    movl  $0x0c0000080, %ecx 
> -    rdmsr
> -    andb  $0xfe, %ah 
> -    wrmsr
> -    movq  %rbx, %cr4
> +    movl  %cr0, %rax        # Get control register 0
> +    .byte 0x66
> +    .byte 0x83,0xe0,0xfe    # and    eax, 0fffffffeh  ; Clear PE bit (bit #0)
> +    .byte 0xf,0x22,0xc0     # mov    cr0, eax         ; Activate real mode
> 
>> - If I switch to compat mode execution (64-bitC=0, desc 0x28), but also change
>>   the default addr/op size to 16-bits, then the lret reboots the guest in KVM
>>   (but works on physical hardware).
> 
> Did you try this on physical hardware, or just assumed that? :)

I was told on edk2-devel O:)

http://thread.gmane.org/gmane.comp.bios.tianocore.devel/5297/focus=5333

Thanks!
Laszlo

------------------------------------------------------------------------------
Sponsored by Intel(R) XDK 
Develop, test and display web and hybrid apps with a single code base.
Download it for free now!
http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk