Re: kvm: grub breakage due to SS.RPL/DPL alignment

[Avi Kivity <avi@redhat.com>]

On 12/27/2010 04:37 PM, Jan Kiszka wrote:
> Hi,
>
> when interrupting a guest (grub in graphical mode) in this state
>
> EAX=00000011 EBX=0004bc88 ECX=0000000d EDX=000db51d
> ESI=000008ff EDI=002462da EBP=00000000 ESP=00001fbc
> EIP=000078b6 EFL=00000006 [-----P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
> ES =0000 00000000 0000ffff 00009300 DPL=0 DS16 [-WA]
> CS =2d32 0002d320 0000ffff 00009b00 DPL=0 CS16 [-RA]
> SS =45aa 00045aa0 0000ffff 00009300 DPL=0 DS16 [-WA]
> DS =2d32 0002d320 0000ffff 00009300 DPL=0 DS16 [-WA]
> FS =0000 00000000 0000ffff 00009300 DPL=0 DS16 [-WA]
> GS =0000 00000000 0000ffff 00009300 DPL=0 DS16 [-WA]
> LDT=0000 00000000 ffffffff 00000000
> TR =0048 00266009 00000067 00008b00 DPL=0 TSS32-busy
> GDT=     0002dd48 0000004f
> IDT=     0026607a 000007ff
> CR0=00000011 CR2=00000000 CR3=00000000 CR4=00000000
>
> the SS alignment to CPL corrupts the guest state on write back:
>
>      if (env->cr[0]&  CR0_PE_MASK) {
> 	/* force ss cpl to cs cpl */
> 	sregs.ss.selector = (sregs.ss.selector&  ~3) |
> 		(sregs.cs.selector&  3);
> 	sregs.ss.dpl = sregs.ss.selector&  3;
>      }
>
> Aligning SS.RPL to CPL is not problematic here (as it already is), but
> forcing SS.DPL to CPL is definitely wrong and causes an immediate guest
> reboot.
>
> Looking at commit 292a55081e5eee62db42209463cf385e7ff1d86d of qemu-kvm
> which introduced this workaround I wonder if it still applies and if it
> wasn't misplaced from day one anyway.

Hey, that commit was done before kvm supported real mode or mmio 
emulation; those were done by qemu.

It definitely doesn't fit today's kvm (where everything to do with a cpu 
is emulated in the kernel).

> If we really need this (when
> BTW?), doesn't it belong into the kernel, particularly into vmx.c as it
> addresses an _Intel_ quirk?
>

I don't think it's needed, see enter_pmode():

     vmcs_write16(GUEST_SS_SELECTOR, 0);
     vmcs_write32(GUEST_SS_AR_BYTES, 0x93);

     vmcs_write16(GUEST_CS_SELECTOR,
              vmcs_read16(GUEST_CS_SELECTOR) & ~SELECTOR_RPL_MASK);

so ss.rpl and cpl are set to compatible values during the transition.  
We don't even exit to qemu so the code isn't exercised.  I think it can 
be safely removed with no backward compatibility issues to any supported 
kernel.

-- 
error compiling committee.c: too many arguments to function