[PATCH] gfxboot VMX workaround v2

[PATCH] gfxboot VMX workaround v2

[Alexander Graf]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

Hi,

this is an improved version of the patch I sent several weeks ago to
this list. Functionally nothing changed; it still hacks into gfxboot and
patches it to work on Intel CPUs on the fly. The big difference is that
this version is cleaned up and should work with every future CPU available.

Please do _not_ apply this patch. I send it to the list only for
interested people, who would like to have a working version of KVM for
their systems right now. It is neither a proper fix nor the right
approach to deal with this issue. It is merely a hack that works for me
and maybe for others too.

Alex



[-- Attachment #2: gfxboot.patch --]
[-- Type: text/x-patch, Size: 4609 bytes --]

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 6249810..ae96d99 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1161,6 +1161,8 @@ static void fix_pmode_dataseg(int seg, struct kvm_save_segment *save)
 static void enter_pmode(struct kvm_vcpu *vcpu)
 {
 	unsigned long flags;
+	unsigned long rip;
+	u8 opcodes[2];
 
 	vcpu->arch.rmode.active = 0;
 
@@ -1183,12 +1185,40 @@ static void enter_pmode(struct kvm_vcpu *vcpu)
 	fix_pmode_dataseg(VCPU_SREG_GS, &vcpu->arch.rmode.gs);
 	fix_pmode_dataseg(VCPU_SREG_FS, &vcpu->arch.rmode.fs);
 
+	/* Save real mode SS */
+	vcpu->arch.backup_ss = vmcs_read16(GUEST_SS_SELECTOR);
+
 	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);
 	vmcs_write32(GUEST_CS_AR_BYTES, 0x9b);
+
+	/* VMX checks for SS.CPL = CS.CPL on VM entry, if we are in
+	 * protected mode. This fails on the transistion from real mode
+	 * to protected mode, as just after that, SS still contains the
+	 * real mode segment, which does not know anything about CPLs.
+	 * 
+	 * As far as I know only gfxboot exploits this feature, by using
+	 * the old real mode SS value to find a new SS selector in protected
+	 * mode. This happens using a mov %ss, %eax instruction, which we
+	 * can patch to an ud2 instruction and emulate later on, giving eax
+	 * the real SS value, that existed before the protected mode
+	 * switch. */
+	rip = vcpu->arch.rip + vmcs_readl(GUEST_CS_BASE) + 14;
+	emulator_read_std(rip, (void *)opcodes, 2, vcpu);
+
+	if ( opcodes[0] ==  0x8c && opcodes[1] == 0xd0 ) {
+		vcpu_printf(vcpu, "%s: patching mov SS\n", __FUNCTION__);
+		opcodes[0] = 0x0f;
+		opcodes[1] = 0x0b;
+		vcpu->arch.backup_ss_rip = rip;
+		if (emulator_write_emulated(rip, opcodes,
+		    2, vcpu) != X86EMUL_CONTINUE)
+			vcpu_printf(vcpu, "%s: unable to patch mov SS\n",
+				__FUNCTION__);
+	}
 }
 
 static gva_t rmode_tss_base(struct kvm *kvm)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c7ad235..f4e28da 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2075,13 +2075,14 @@ int emulate_instruction(struct kvm_vcpu *vcpu,
 
 		r = x86_decode_insn(&vcpu->arch.emulate_ctxt, &emulate_ops);
 
-		/* Reject the instructions other than VMCALL/VMMCALL when
+		/* Reject the instructions other than VMCALL/VMMCALL/UD2 when
 		 * try to emulate invalid opcode */
 		c = &vcpu->arch.emulate_ctxt.decode;
 		if ((emulation_type & EMULTYPE_TRAP_UD) &&
-		    (!(c->twobyte && c->b == 0x01 &&
+		    ((!(c->twobyte && c->b == 0x01 &&
 		      (c->modrm_reg == 0 || c->modrm_reg == 3) &&
-		       c->modrm_mod == 3 && c->modrm_rm == 1)))
+		       c->modrm_mod == 3 && c->modrm_rm == 1)) &&
+		       c->b != 0x0b))
 			return EMULATE_FAIL;
 
 		++vcpu->stat.insn_emulation;
diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c
index f59ed93..1a3df0d 100644
--- a/arch/x86/kvm/x86_emulate.c
+++ b/arch/x86/kvm/x86_emulate.c
@@ -181,7 +181,7 @@ static u16 opcode_table[256] = {
 static u16 twobyte_table[256] = {
 	/* 0x00 - 0x0F */
 	0, Group | GroupDual | Group7, 0, 0, 0, 0, ImplicitOps, 0,
-	ImplicitOps, ImplicitOps, 0, 0, 0, ImplicitOps | ModRM, 0, 0,
+	ImplicitOps, ImplicitOps, 0, ImplicitOps, 0, ImplicitOps | ModRM, 0, 0,
 	/* 0x10 - 0x1F */
 	0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps | ModRM, 0, 0, 0, 0, 0, 0, 0,
 	/* 0x20 - 0x2F */
@@ -1774,6 +1774,19 @@ twobyte_insn:
 	case 0x18:		/* Grp16 (prefetch/nop) */
 		c->dst.type = OP_NONE;
 		break;
+	case 0x0b: /* UD2 (used to patch mov %ss, %eax) */
+		/* This opcode is declared invalid, according to the Intel
+		 * specification and exploited here to circumvent a
+		 * VMX restriction. For more information, why this is
+		 * needed, please see vmx.c:enter_pmode.
+		 */
+		if (ctxt->vcpu->arch.backup_ss_rip == ctxt->vcpu->arch.rip + ctxt->cs_base) {
+			c->dst.type = OP_NONE;
+			c->regs[VCPU_REGS_RAX] = ctxt->vcpu->arch.backup_ss;
+		} else {
+			goto cannot_emulate;
+		}
+		break;
 	case 0x20: /* mov cr, reg */
 		if (c->modrm_mod != 3)
 			goto cannot_emulate;
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index 781fc87..ea5078a 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -216,6 +216,10 @@ struct kvm_vcpu_arch {
 	unsigned long regs[NR_VCPU_REGS]; /* for rsp: vcpu_load_rsp_rip() */
 	unsigned long rip;      /* needs vcpu_load_rsp_rip() */
 
+	/* temporaries for gfxboot patching */
+	u16 backup_ss;
+	unsigned long backup_ss_rip;
+
 	unsigned long cr0;
 	unsigned long cr2;
 	unsigned long cr3;

[-- Attachment #3: Type: text/plain, Size: 325 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

[-- Attachment #4: Type: text/plain, Size: 158 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [PATCH] gfxboot VMX workaround v2

[Anthony Liguori]

Alexander Graf wrote:
> Hi,
>
> this is an improved version of the patch I sent several weeks ago to
> this list. Functionally nothing changed; it still hacks into gfxboot and
> patches it to work on Intel CPUs on the fly. The big difference is that
> this version is cleaned up and should work with every future CPU available.
>
> Please do _not_ apply this patch. I send it to the list only for
> interested people, who would like to have a working version of KVM for
> their systems right now. It is neither a proper fix nor the right
> approach to deal with this issue. It is merely a hack that works for me
> and maybe for others too.
>   

Perhaps a viable way to fix this upstream would be to catch the vmentry 
failure, look to see if SS.CPL != CS.CPL, and if so, invoke 
x86_emulate() in a loop until SS.CPL == CS.CPL.

There are very few instructions in gfxboot that would need to be added 
to x86_emulate (if they aren't already there).

Regards,

Anthony Liguori

> Alex
>
>
>   
> ------------------------------------------------------------------------
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
> Register now and save $200. Hurry, offer ends at 11:59 p.m., 
> Monday, April 7! Use priority code J8TLD2. 
> http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
> ------------------------------------------------------------------------
>
> _______________________________________________
> kvm-devel mailing list
> kvm-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/kvm-devel


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Alexander Graf]

On Apr 7, 2008, at 6:05 PM, Anthony Liguori wrote:

> Alexander Graf wrote:
>> Hi,
>>
>> this is an improved version of the patch I sent several weeks ago to
>> this list. Functionally nothing changed; it still hacks into  
>> gfxboot and
>> patches it to work on Intel CPUs on the fly. The big difference is  
>> that
>> this version is cleaned up and should work with every future CPU  
>> available.
>>
>> Please do _not_ apply this patch. I send it to the list only for
>> interested people, who would like to have a working version of KVM  
>> for
>> their systems right now. It is neither a proper fix nor the right
>> approach to deal with this issue. It is merely a hack that works  
>> for me
>> and maybe for others too.
>>
>
> Perhaps a viable way to fix this upstream would be to catch the  
> vmentry failure, look to see if SS.CPL != CS.CPL, and if so, invoke  
> x86_emulate() in a loop until SS.CPL == CS.CPL.
>
> There are very few instructions in gfxboot that would need to be  
> added to x86_emulate (if they aren't already there).

In a previous thread Avi already explained a quite reasonable way to  
approach this problem, which I believe is a really good approach. He  
wanted to x86_emulate until the environment is "VMX friendly" again,  
thus resolving big real mode problems as well.

I personally agree that the real approach is way superior to my patch.  
I just won't have the time to do it in the near future and not being  
able to boot intuitively hurts KVM users unnecessarily ;-).

Regards,

Alex

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Anthony Liguori]

[-- Attachment #1: Type: text/plain, Size: 2243 bytes --]

Alexander Graf wrote:
>
> On Apr 7, 2008, at 6:05 PM, Anthony Liguori wrote:
>
>> Alexander Graf wrote:
>>> Hi,
>>>
>>> this is an improved version of the patch I sent several weeks ago to
>>> this list. Functionally nothing changed; it still hacks into gfxboot 
>>> and
>>> patches it to work on Intel CPUs on the fly. The big difference is that
>>> this version is cleaned up and should work with every future CPU 
>>> available.
>>>
>>> Please do _not_ apply this patch. I send it to the list only for
>>> interested people, who would like to have a working version of KVM for
>>> their systems right now. It is neither a proper fix nor the right
>>> approach to deal with this issue. It is merely a hack that works for me
>>> and maybe for others too.
>>>
>>
>> Perhaps a viable way to fix this upstream would be to catch the 
>> vmentry failure, look to see if SS.CPL != CS.CPL, and if so, invoke 
>> x86_emulate() in a loop until SS.CPL == CS.CPL.
>>
>> There are very few instructions in gfxboot that would need to be 
>> added to x86_emulate (if they aren't already there).
>
> In a previous thread Avi already explained a quite reasonable way to 
> approach this problem, which I believe is a really good approach. He 
> wanted to x86_emulate until the environment is "VMX friendly" again, 
> thus resolving big real mode problems as well.

I've got a slightly lamer approach than what Avi probably wants.  I lost 
interest in updating x86_emulate once I realized how far xen's copy has 
gotten.  To get GFXBOOT 3.3.28 working just requires adding far jmp to 
x86_emulate.  The sequence should look like:

        jmp pm_seg.prog_c32:switch_to_pm_20
switch_to_pm_20:

        bits 32

        mov ax,pm_seg.prog_d16
        mov ds,ax
        mov eax,ss

Which means we'll get 3 vmentry failures.  The two moves should already 
be supported by x86_emulate but I haven't confirmed.  It's not a 
complete solution to our real mode woes but I think it's a reasonable 
first step.

Regards,

Anthony Liguori
> I personally agree that the real approach is way superior to my patch. 
> I just won't have the time to do it in the near future and not being 
> able to boot intuitively hurts KVM users unnecessarily ;-).
>
> Regards,
>
> Alex


[-- Attachment #2: x86_emulate.patch --]
[-- Type: application/mbox, Size: 2796 bytes --]

[-- Attachment #3: Type: text/plain, Size: 325 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

[-- Attachment #4: Type: text/plain, Size: 158 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [PATCH] gfxboot VMX workaround v2

[Alexander Graf]

On Apr 7, 2008, at 6:51 PM, Anthony Liguori wrote:

> Alexander Graf wrote:
>>
>> On Apr 7, 2008, at 6:05 PM, Anthony Liguori wrote:
>>
>>> Alexander Graf wrote:
>>>> Hi,
>>>>
>>>> this is an improved version of the patch I sent several weeks ago  
>>>> to
>>>> this list. Functionally nothing changed; it still hacks into  
>>>> gfxboot and
>>>> patches it to work on Intel CPUs on the fly. The big difference  
>>>> is that
>>>> this version is cleaned up and should work with every future CPU  
>>>> available.
>>>>
>>>> Please do _not_ apply this patch. I send it to the list only for
>>>> interested people, who would like to have a working version of  
>>>> KVM for
>>>> their systems right now. It is neither a proper fix nor the right
>>>> approach to deal with this issue. It is merely a hack that works  
>>>> for me
>>>> and maybe for others too.
>>>>
>>>
>>> Perhaps a viable way to fix this upstream would be to catch the  
>>> vmentry failure, look to see if SS.CPL != CS.CPL, and if so,  
>>> invoke x86_emulate() in a loop until SS.CPL == CS.CPL.
>>>
>>> There are very few instructions in gfxboot that would need to be  
>>> added to x86_emulate (if they aren't already there).
>>
>> In a previous thread Avi already explained a quite reasonable way  
>> to approach this problem, which I believe is a really good  
>> approach. He wanted to x86_emulate until the environment is "VMX  
>> friendly" again, thus resolving big real mode problems as well.
>
> I've got a slightly lamer approach than what Avi probably wants.  I  
> lost interest in updating x86_emulate once I realized how far xen's  
> copy has gotten.  To get GFXBOOT 3.3.28 working just requires adding  
> far jmp to x86_emulate.  The sequence should look like:
>
>       jmp pm_seg.prog_c32:switch_to_pm_20
> switch_to_pm_20:
>
>       bits 32
>
>       mov ax,pm_seg.prog_d16
>       mov ds,ax
>       mov eax,ss
>
> Which means we'll get 3 vmentry failures.  The two moves should  
> already be supported by x86_emulate but I haven't confirmed.  It's  
> not a complete solution to our real mode woes but I think it's a  
> reasonable first step.

Right, this was exactly the approach I wanted to go at first. I just  
backed off it as I saw how much ljmp actually does, as normal x86 CPUs  
switch to PM not on cr0 setting, but on the ljmp after that.

So is that simple implementation you have written actually doing the  
right thing?

Regards,

Alex

>
>
> Regards,
>
> Anthony Liguori
>> I personally agree that the real approach is way superior to my  
>> patch. I just won't have the time to do it in the near future and  
>> not being able to boot intuitively hurts KVM users unnecessarily ;-).
>>
>> Regards,
>>
>> Alex
>
> <x86_emulate.patch>


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Anthony Liguori]

Alexander Graf wrote:
>
> On Apr 7, 2008, at 6:51 PM, Anthony Liguori wrote:
>
>> Alexander Graf wrote:
>>>
>>> On Apr 7, 2008, at 6:05 PM, Anthony Liguori wrote:
>>>
>>>> Alexander Graf wrote:
>>>>> Hi,
>>>>>
>>>>> this is an improved version of the patch I sent several weeks ago to
>>>>> this list. Functionally nothing changed; it still hacks into 
>>>>> gfxboot and
>>>>> patches it to work on Intel CPUs on the fly. The big difference is 
>>>>> that
>>>>> this version is cleaned up and should work with every future CPU 
>>>>> available.
>>>>>
>>>>> Please do _not_ apply this patch. I send it to the list only for
>>>>> interested people, who would like to have a working version of KVM 
>>>>> for
>>>>> their systems right now. It is neither a proper fix nor the right
>>>>> approach to deal with this issue. It is merely a hack that works 
>>>>> for me
>>>>> and maybe for others too.
>>>>>
>>>>
>>>> Perhaps a viable way to fix this upstream would be to catch the 
>>>> vmentry failure, look to see if SS.CPL != CS.CPL, and if so, invoke 
>>>> x86_emulate() in a loop until SS.CPL == CS.CPL.
>>>>
>>>> There are very few instructions in gfxboot that would need to be 
>>>> added to x86_emulate (if they aren't already there).
>>>
>>> In a previous thread Avi already explained a quite reasonable way to 
>>> approach this problem, which I believe is a really good approach. He 
>>> wanted to x86_emulate until the environment is "VMX friendly" again, 
>>> thus resolving big real mode problems as well.
>>
>> I've got a slightly lamer approach than what Avi probably wants.  I 
>> lost interest in updating x86_emulate once I realized how far xen's 
>> copy has gotten.  To get GFXBOOT 3.3.28 working just requires adding 
>> far jmp to x86_emulate.  The sequence should look like:
>>
>>       jmp pm_seg.prog_c32:switch_to_pm_20
>> switch_to_pm_20:
>>
>>       bits 32
>>
>>       mov ax,pm_seg.prog_d16
>>       mov ds,ax
>>       mov eax,ss
>>
>> Which means we'll get 3 vmentry failures.  The two moves should 
>> already be supported by x86_emulate but I haven't confirmed.  It's 
>> not a complete solution to our real mode woes but I think it's a 
>> reasonable first step.
>
> Right, this was exactly the approach I wanted to go at first. I just 
> backed off it as I saw how much ljmp actually does, as normal x86 CPUs 
> switch to PM not on cr0 setting, but on the ljmp after that.
>
> So is that simple implementation you have written actually doing the 
> right thing?

Yes, but it won't compile as KVM does not have a proper load_seg() 
function like the Xen's x86_emulate.  I started copying stuff in but ran 
against some additional ops (like ->read_segment).  I don't think it 
will be very hard to implement but it exceeded the 45 seconds I was 
willing to spend looking at it :-)

Regards,

Anthony Liguori

> Regards,
>
> Alex
>
>>
>>
>> Regards,
>>
>> Anthony Liguori
>>> I personally agree that the real approach is way superior to my 
>>> patch. I just won't have the time to do it in the near future and 
>>> not being able to boot intuitively hurts KVM users unnecessarily ;-).
>>>
>>> Regards,
>>>
>>> Alex
>>
>> <x86_emulate.patch>
>


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Avi Kivity]

Anthony Liguori wrote:
> Yes, but it won't compile as KVM does not have a proper load_seg() 
> function like the Xen's x86_emulate.

Izik added something for task switching (see load_segment_descriptor).

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Guillaume Thouvenin]

On Mon, 07 Apr 2008 11:05:06 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> Perhaps a viable way to fix this upstream would be to catch the vmentry 
> failure, look to see if SS.CPL != CS.CPL, and if so, invoke 
> x86_emulate() in a loop until SS.CPL == CS.CPL.

I tried this solution some time ago but unfortunately x86_emulate()
failed. I suspected a problem with guest EIP that could different
between the vmentry catch and the emulation. I will rebase my patch and
post them on the mailing list.

Regards,
Guillaume

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Anthony Liguori]

Guillaume Thouvenin wrote:
> On Mon, 07 Apr 2008 11:05:06 -0500
> Anthony Liguori <anthony@codemonkey.ws> wrote:
>
>   
>> Perhaps a viable way to fix this upstream would be to catch the vmentry 
>> failure, look to see if SS.CPL != CS.CPL, and if so, invoke 
>> x86_emulate() in a loop until SS.CPL == CS.CPL.
>>     
>
> I tried this solution some time ago but unfortunately x86_emulate()
> failed. I suspected a problem with guest EIP that could different
> between the vmentry catch and the emulation. I will rebase my patch and
> post them on the mailing list.
>   

x86 emulate is missing support for jmp far which is used to switch into 
protected mode.  It just needs to be added.

Regards,

Anthony Liguori

> Regards,
> Guillaume
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Guillaume Thouvenin]

On Tue, 08 Apr 2008 07:14:13 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> Guillaume Thouvenin wrote:
> > On Mon, 07 Apr 2008 11:05:06 -0500
> > Anthony Liguori <anthony@codemonkey.ws> wrote:
> >
> >   
> >> Perhaps a viable way to fix this upstream would be to catch the vmentry 
> >> failure, look to see if SS.CPL != CS.CPL, and if so, invoke 
> >> x86_emulate() in a loop until SS.CPL == CS.CPL.
> >>     
> >
> > I tried this solution some time ago but unfortunately x86_emulate()
> > failed. I suspected a problem with guest EIP that could different
> > between the vmentry catch and the emulation. I will rebase my patch and
> > post them on the mailing list.
> >   
> 
> x86 emulate is missing support for jmp far which is used to switch into 
> protected mode.  It just needs to be added.

Ok I see. I understand now why you said in a previous email that KVM
needs to have a proper load_seg() function like the Xen's x86_emulate.
This function is used to load the segment in a far jmp. I will look how
it is done in Xen and I will try to copy the stuff like you did.

Regards,
Guillaume

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Avi Kivity]

Guillaume Thouvenin wrote:
>>>   
>>>       
>> x86 emulate is missing support for jmp far which is used to switch into 
>> protected mode.  It just needs to be added.
>>     
>
> Ok I see. I understand now why you said in a previous email that KVM
> needs to have a proper load_seg() function like the Xen's x86_emulate.
> This function is used to load the segment in a far jmp. I will look how
> it is done in Xen and I will try to copy the stuff like you did.
>
>   

kvm now has a load_segment_descriptor() function which might help.


-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Guillaume Thouvenin]

On Mon, 07 Apr 2008 11:05:06 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> Perhaps a viable way to fix this upstream would be to catch the vmentry 
> failure, look to see if SS.CPL != CS.CPL, and if so, invoke 
> x86_emulate() in a loop until SS.CPL == CS.CPL.
> 
> There are very few instructions in gfxboot that would need to be added 
> to x86_emulate (if they aren't already there).

So to see if I'm on the good way here is an attempt to implement the
solution. It doesn't work yet.

I'm trying to:
  - Disable the code that modifies SS value in order to detect VM entry
failure
  - Add the handler that catches the VM entry failure
  - Emulate the instruction until we recover a friendly VMX state
     => add the jmp far (opcode 0xea) instruction in the emulation.

With the patch, the VM entry failure is caught but the jmp far
instruction seems to fail. I've got the following dmesg:

...
handle_vmentry_failure: invalid guest state
handle_vmentry_failure: start emulation
handle_vmentry_failure: emulation failed
...


Regards,
Guillaume

---

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 8e5d664..a56bd83 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1178,13 +1178,16 @@ static void enter_pmode(struct kvm_vcpu *vcpu)
 
 	update_exception_bitmap(vcpu);
 
+	fix_pmode_dataseg(VCPU_SREG_SS, &vcpu->arch.rmode.ss);
 	fix_pmode_dataseg(VCPU_SREG_ES, &vcpu->arch.rmode.es);
 	fix_pmode_dataseg(VCPU_SREG_DS, &vcpu->arch.rmode.ds);
 	fix_pmode_dataseg(VCPU_SREG_GS, &vcpu->arch.rmode.gs);
 	fix_pmode_dataseg(VCPU_SREG_FS, &vcpu->arch.rmode.fs);
 
+#if 0
 	vmcs_write16(GUEST_SS_SELECTOR, 0);
 	vmcs_write32(GUEST_SS_AR_BYTES, 0x93);
+#endif
 
 	vmcs_write16(GUEST_CS_SELECTOR,
 		     vmcs_read16(GUEST_CS_SELECTOR) & ~SELECTOR_RPL_MASK);
@@ -1952,6 +1955,33 @@ static int handle_rmode_exception(struct kvm_vcpu *vcpu,
 	return 0;
 }
 
+static int handle_vmentry_failure(u32 exit_reason, struct kvm_vcpu *vcpu)
+{
+	unsigned int basic_exit_reason = (uint16_t) exit_reason;
+	unsigned int cs_rpl = vmcs_read16(GUEST_CS_SELECTOR) & SELECTOR_RPL_MASK;
+	unsigned int ss_rpl = vmcs_read16(GUEST_SS_SELECTOR) & SELECTOR_RPL_MASK;
+
+	switch (basic_exit_reason) {
+		case EXIT_REASON_INVALID_GUEST_STATE:
+			printk(KERN_INFO "%s: invalid guest state\n", __FUNCTION__);
+			printk(KERN_INFO "%s: start emulation \n", __FUNCTION__);
+			while (cs_rpl != ss_rpl) {
+				if (emulate_instruction(vcpu, NULL, 0, 0, 0) == EMULATE_FAIL) {
+					printk(KERN_INFO "%s: emulation failed\n", __FUNCTION__);
+					return 0;
+				}
+				cs_rpl = vmcs_read16(GUEST_CS_SELECTOR) & SELECTOR_RPL_MASK;
+				ss_rpl = vmcs_read16(GUEST_SS_SELECTOR) & SELECTOR_RPL_MASK;
+			}
+			printk(KERN_INFO "%s: VMX friendly state recovered\n", __FUNCTION__);
+			break;
+		default:
+			printk(KERN_INFO "VM-entry failure due to unkown reason\n");
+			return 0;
+	}
+	return 1;
+}
+
 static int handle_exception(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -2364,6 +2394,9 @@ static int kvm_handle_exit(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
 	KVMTRACE_3D(VMEXIT, vcpu, exit_reason, (u32)vmcs_readl(GUEST_RIP),
 		    (u32)((u64)vmcs_readl(GUEST_RIP) >> 32), entryexit);
 
+	if (unlikely(exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY))
+		return handle_vmentry_failure(exit_reason, vcpu);
+
 	if (unlikely(vmx->fail)) {
 		kvm_run->exit_reason = KVM_EXIT_FAIL_ENTRY;
 		kvm_run->fail_entry.hardware_entry_failure_reason
diff --git a/arch/x86/kvm/vmx.h b/arch/x86/kvm/vmx.h
index 5dff460..200c0f8 100644
--- a/arch/x86/kvm/vmx.h
+++ b/arch/x86/kvm/vmx.h
@@ -223,7 +223,10 @@ enum vmcs_field {
 #define EXIT_REASON_IO_INSTRUCTION      30
 #define EXIT_REASON_MSR_READ            31
 #define EXIT_REASON_MSR_WRITE           32
+#define EXIT_REASON_INVALID_GUEST_STATE 33
+#define EXIT_REASON_MSR_LOADING         34
 #define EXIT_REASON_MWAIT_INSTRUCTION   36
+#define EXIT_REASON_MACHINE_CHECK       41
 #define EXIT_REASON_TPR_BELOW_THRESHOLD 43
 #define EXIT_REASON_APIC_ACCESS         44
 #define EXIT_REASON_WBINVD		54
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0ce5563..b38065d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3267,7 +3267,7 @@ static int load_segment_descriptor_to_kvm_desct(struct kvm_vcpu *vcpu,
 	return 0;
 }
 
-static int load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector,
+int load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector,
 				   int type_bits, int seg)
 {
 	struct kvm_segment kvm_seg;
diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c
index 2ca0838..4bc3c80 100644
--- a/arch/x86/kvm/x86_emulate.c
+++ b/arch/x86/kvm/x86_emulate.c
@@ -168,7 +168,7 @@ static u16 opcode_table[256] = {
 	/* 0xE0 - 0xE7 */
 	0, 0, 0, 0, 0, 0, 0, 0,
 	/* 0xE8 - 0xEF */
-	ImplicitOps | Stack, SrcImm|ImplicitOps, 0, SrcImmByte|ImplicitOps,
+	ImplicitOps | Stack, SrcImm|ImplicitOps, ImplicitOps, SrcImmByte|ImplicitOps,
 	0, 0, 0, 0,
 	/* 0xF0 - 0xF7 */
 	0, 0, 0, 0,
@@ -1156,6 +1156,9 @@ static inline int emulate_grp45(struct x86_emulate_ctxt *ctxt,
 	case 4: /* jmp abs */
 		c->eip = c->src.val;
 		break;
+	case 5: /* jmp far */
+		printk(KERN_INFO "Jmp far need to be implemented\n");
+		break;
 	case 6:	/* push */
 		emulate_push(ctxt);
 		break;
@@ -1657,6 +1660,24 @@ special_insn:
 		break;
 	}
 	case 0xe9: /* jmp rel */
+		jmp_rel(c, c->src.val);
+		c->dst.type = OP_NONE; /* Disable writeback. */
+		break;
+	case 0xea: /* jmp far */ {
+		struct kvm_segment kvm_seg;
+		int ret;
+
+		kvm_x86_ops->get_segment(ctxt->vcpu, &kvm_seg, VCPU_SREG_CS);
+
+
+		ret = load_segment_descriptor(ctxt->vcpu, kvm_seg.selector, 9, VCPU_SREG_CS);
+		if (ret < 0){
+			printk(KERN_INFO "%s: Failed to load CS descriptor\n", __FUNCTION__);
+			return ret;
+		}
+
+		break;
+	}
 	case 0xeb: /* jmp rel short */
 		jmp_rel(c, c->src.val);
 		c->dst.type = OP_NONE; /* Disable writeback. */
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index 31aa7d6..cdb5572 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -270,7 +270,7 @@ struct kvm_vcpu_arch {
 			unsigned long base;
 			u32 limit;
 			u32 ar;
-		} tr, es, ds, fs, gs;
+		} tr, es, ds, fs, gs, ss;
 	} rmode;
 	int halt_request; /* real mode on Intel only */
 
@@ -487,6 +487,8 @@ int emulator_get_dr(struct x86_emulate_ctxt *ctxt, int dr,
 int emulator_set_dr(struct x86_emulate_ctxt *ctxt, int dr,
 		    unsigned long value);
 
+int load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector,
+				   int type_bits, int seg);
 int kvm_task_switch(struct kvm_vcpu *vcpu, u16 tss_selector, int reason);
 
 void kvm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0);

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Avi Kivity]

Guillaume Thouvenin wrote:
> On Mon, 07 Apr 2008 11:05:06 -0500
> Anthony Liguori <anthony@codemonkey.ws> wrote:
>
>   
>> Perhaps a viable way to fix this upstream would be to catch the vmentry 
>> failure, look to see if SS.CPL != CS.CPL, and if so, invoke 
>> x86_emulate() in a loop until SS.CPL == CS.CPL.
>>
>> There are very few instructions in gfxboot that would need to be added 
>> to x86_emulate (if they aren't already there).
>>     
>
> So to see if I'm on the good way here is an attempt to implement the
> solution. It doesn't work yet.
>
> I'm trying to:
>   - Disable the code that modifies SS value in order to detect VM entry
> failure
>   - Add the handler that catches the VM entry failure
>   - Emulate the instruction until we recover a friendly VMX state
>      => add the jmp far (opcode 0xea) instruction in the emulation.
>
> With the patch, the VM entry failure is caught but the jmp far
> instruction seems to fail. I've got the following dmesg:
>
> ...
> handle_vmentry_failure: invalid guest state
> handle_vmentry_failure: start emulation
> handle_vmentry_failure: emulation failed
>   

What instruction failed, exactly?

> ...
>
>
> Regards,
> Guillaume
>
> ---
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 8e5d664..a56bd83 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1178,13 +1178,16 @@ static void enter_pmode(struct kvm_vcpu *vcpu)
>  
>  	update_exception_bitmap(vcpu);
>  
> +	fix_pmode_dataseg(VCPU_SREG_SS, &vcpu->arch.rmode.ss);
>  	fix_pmode_dataseg(VCPU_SREG_ES, &vcpu->arch.rmode.es);
>  	fix_pmode_dataseg(VCPU_SREG_DS, &vcpu->arch.rmode.ds);
>  	fix_pmode_dataseg(VCPU_SREG_GS, &vcpu->arch.rmode.gs);
>  	fix_pmode_dataseg(VCPU_SREG_FS, &vcpu->arch.rmode.fs);
>   

You need to drop the fixes rather than add more.

>  
> +#if 0
>  	vmcs_write16(GUEST_SS_SELECTOR, 0);
>  	vmcs_write32(GUEST_SS_AR_BYTES, 0x93);
> +#endif
>  
>  	vmcs_write16(GUEST_CS_SELECTOR,
>  		     vmcs_read16(GUEST_CS_SELECTOR) & ~SELECTOR_RPL_MASK);
> @@ -1952,6 +1955,33 @@ static int handle_rmode_exception(struct kvm_vcpu *vcpu,
>  	return 0;
>  }
>  
> +static int handle_vmentry_failure(u32 exit_reason, struct kvm_vcpu *vcpu)
> +{
> +	unsigned int basic_exit_reason = (uint16_t) exit_reason;
> +	unsigned int cs_rpl = vmcs_read16(GUEST_CS_SELECTOR) & SELECTOR_RPL_MASK;
> +	unsigned int ss_rpl = vmcs_read16(GUEST_SS_SELECTOR) & SELECTOR_RPL_MASK;
> +
> +	switch (basic_exit_reason) {
> +		case EXIT_REASON_INVALID_GUEST_STATE:
> +			printk(KERN_INFO "%s: invalid guest state\n", __FUNCTION__);
> +			printk(KERN_INFO "%s: start emulation \n", __FUNCTION__);
> +			while (cs_rpl != ss_rpl) {
> +				if (emulate_instruction(vcpu, NULL, 0, 0, 0) == EMULATE_FAIL) {
> +					printk(KERN_INFO "%s: emulation failed\n", __FUNCTION__);
> +					return 0;
> +				}
> +				cs_rpl = vmcs_read16(GUEST_CS_SELECTOR) & SELECTOR_RPL_MASK;
> +				ss_rpl = vmcs_read16(GUEST_SS_SELECTOR) & SELECTOR_RPL_MASK;
> +			}
> +			printk(KERN_INFO "%s: VMX friendly state recovered\n", __FUNCTION__);
> +			break;
> +		default:
> +			printk(KERN_INFO "VM-entry failure due to unkown reason\n");
> +			return 0;
> +	}
> +	return 1;
> +}
> +
>  static int handle_exception(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>  {
>  	struct vcpu_vmx *vmx = to_vmx(vcpu);
> @@ -2364,6 +2394,9 @@ static int kvm_handle_exit(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
>  	KVMTRACE_3D(VMEXIT, vcpu, exit_reason, (u32)vmcs_readl(GUEST_RIP),
>  		    (u32)((u64)vmcs_readl(GUEST_RIP) >> 32), entryexit);
>  
> +	if (unlikely(exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY))
> +		return handle_vmentry_failure(exit_reason, vcpu);
> +
>  	if (unlikely(vmx->fail)) {
>  		kvm_run->exit_reason = KVM_EXIT_FAIL_ENTRY;
>  		kvm_run->fail_entry.hardware_entry_failure_reason
> diff --git a/arch/x86/kvm/vmx.h b/arch/x86/kvm/vmx.h
> index 5dff460..200c0f8 100644
> --- a/arch/x86/kvm/vmx.h
> +++ b/arch/x86/kvm/vmx.h
> @@ -223,7 +223,10 @@ enum vmcs_field {
>  #define EXIT_REASON_IO_INSTRUCTION      30
>  #define EXIT_REASON_MSR_READ            31
>  #define EXIT_REASON_MSR_WRITE           32
> +#define EXIT_REASON_INVALID_GUEST_STATE 33
> +#define EXIT_REASON_MSR_LOADING         34
>  #define EXIT_REASON_MWAIT_INSTRUCTION   36
> +#define EXIT_REASON_MACHINE_CHECK       41
>  #define EXIT_REASON_TPR_BELOW_THRESHOLD 43
>  #define EXIT_REASON_APIC_ACCESS         44
>  #define EXIT_REASON_WBINVD		54
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 0ce5563..b38065d 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3267,7 +3267,7 @@ static int load_segment_descriptor_to_kvm_desct(struct kvm_vcpu *vcpu,
>  	return 0;
>  }
>  
> -static int load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector,
> +int load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector,
>  				   int type_bits, int seg)
>  {
>  	struct kvm_segment kvm_seg;
> diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c
> index 2ca0838..4bc3c80 100644
> --- a/arch/x86/kvm/x86_emulate.c
> +++ b/arch/x86/kvm/x86_emulate.c
> @@ -168,7 +168,7 @@ static u16 opcode_table[256] = {
>  	/* 0xE0 - 0xE7 */
>  	0, 0, 0, 0, 0, 0, 0, 0,
>  	/* 0xE8 - 0xEF */
> -	ImplicitOps | Stack, SrcImm|ImplicitOps, 0, SrcImmByte|ImplicitOps,
> +	ImplicitOps | Stack, SrcImm|ImplicitOps, ImplicitOps, SrcImmByte|ImplicitOps,
>  	0, 0, 0, 0,
>  	/* 0xF0 - 0xF7 */
>  	0, 0, 0, 0,
> @@ -1156,6 +1156,9 @@ static inline int emulate_grp45(struct x86_emulate_ctxt *ctxt,
>  	case 4: /* jmp abs */
>  		c->eip = c->src.val;
>  		break;
> +	case 5: /* jmp far */
> +		printk(KERN_INFO "Jmp far need to be implemented\n");
> +		break;
>  	case 6:	/* push */
>  		emulate_push(ctxt);
>  		break;
> @@ -1657,6 +1660,24 @@ special_insn:
>  		break;
>  	}
>  	case 0xe9: /* jmp rel */
> +		jmp_rel(c, c->src.val);
> +		c->dst.type = OP_NONE; /* Disable writeback. */
> +		break;
> +	case 0xea: /* jmp far */ {
> +		struct kvm_segment kvm_seg;
> +		int ret;
> +
> +		kvm_x86_ops->get_segment(ctxt->vcpu, &kvm_seg, VCPU_SREG_CS);
> +
> +
> +		ret = load_segment_descriptor(ctxt->vcpu, kvm_seg.selector, 9, VCPU_SREG_CS);
> +		if (ret < 0){
> +			printk(KERN_INFO "%s: Failed to load CS descriptor\n", __FUNCTION__);
> +			return ret;
> +		}
> +
>   

You need to load rip as well.

I suggest you print the instruction to be emulated and the register 
state before and after, and compare with the expected state.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Guillaume Thouvenin]

On Tue, 15 Apr 2008 16:06:43 +0300
Avi Kivity <avi@qumranet.com> wrote:

> > ...
> > handle_vmentry_failure: invalid guest state
> > handle_vmentry_failure: start emulation
> > handle_vmentry_failure: emulation failed
> >   
> 
> What instruction failed, exactly?
> 

I added the code do dump the instruction and it seems that it's the
emulation of 0xe6 (== out imm8, al) that failed. I made modifications
to emulate it (see below) and now I have another problem in kvm
userspace with the following message (and the emulation doesn't work):

enterprise:~ $ kvm_run: Operation not permitted
enterprise:~ $ kvm_run returned -1
 
> You need to load rip as well.

Ooops, yes. So jump far emulation is now like:

+       case 0xea: /* jmp far */ {
+               struct kvm_segment kvm_seg;
+               long int eip;
+               int ret;
+
+               kvm_x86_ops->get_segment(ctxt->vcpu, &kvm_seg, VCPU_SREG_CS); 
+
+               ret = load_segment_descriptor(ctxt->vcpu, kvm_seg.selector, 9, VCPU_SREG_CS);
+               if (ret < 0){
+                       printk(KERN_INFO "%s: Failed to load CS descriptor\n", __FUNCTION__);
+                       goto cannot_emulate;
+               }
+
+               switch (c->op_bytes) {
+               case 2:
+                       eip = insn_fetch(s16, 2, c->eip);
+                       break;
+               case 4:
+                       eip = insn_fetch(s32, 4, c->eip);
+                       break;
+               default:
+                       DPRINTF("jmp far: Invalid op_bytes\n");
+                       goto cannot_emulate;
+               }
+               printk(KERN_INFO "eip == 0x%lx\n", eip);
+               c->eip = eip;
+               break;
+       }

It seems that the jump to cs:eip works and now I have the following error:

[18535.446917] handle_vmentry_failure: invalid guest state
[18535.449519] handle_vmentry_failure: start emulation
[18535.457519] eip == 0x6e18
[18535.467685] handle_vmentry_failure: emulation of 0xe6 failed

For the emulation of 0xe6 I used the following one that I found in
nitin's tree:

+       case 0xe6: /* out imm8, al */
+       case 0xe7: /* out imm8, ax/eax */ {
+               struct kvm_io_device *pio_dev;
+               
+               pio_dev = vcpu_find_pio_dev(ctxt->vcpu, c->src.val);
+               kvm_iodevice_write(pio_dev, c->src.val,
+                               (c->d & ByteOp) ? 1 : c->op_bytes,
+                               &c->regs[VCPU_REGS_RAX]);
+               }
+               break;

I will look closer where is the problem and as you suggested, I will
display the instruction to be emulated and the register state before
and after, and compare with the expected state.


Thanks for your help,
Regards,
Guillaume

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Guillaume Thouvenin]

On Fri, 18 Apr 2008 14:18:16 +0200
Guillaume Thouvenin <guillaume.thouvenin@ext.bull.net> wrote:

> I added the code do dump the instruction and it seems that it's the
> emulation of 0xe6 (== out imm8, al) that failed. I made modifications
> to emulate it (see below) and now I have another problem in kvm
> userspace with the following message (and the emulation doesn't work):
> 
> enterprise:~ $ kvm_run: Operation not permitted
> enterprise:~ $ kvm_run returned -1

Ok for this one it seems to be a wrong value in the opcode_table[]. Now
it generates an oops. I'm investigating... 

Regards,
Guillaume

---

Apr 18 14:48:53 enterprise kernel: [22321.010006] handle_vmentry_failure: invalid guest state
Apr 18 14:48:53 enterprise kernel: [22321.011953] handle_vmentry_failure: start emulation
Apr 18 14:48:53 enterprise kernel: [22321.015875] c->op_bytes == 2
Apr 18 14:48:53 enterprise kernel: [22321.019862] eip == 0x6e18

Message from syslogd@enterprise at Fri Apr 18 14:48:54 2008 ...
enterprise kernel: [22321.027850] Oops: 0000 [2] SMP

Message from syslogd@enterprise at Fri Apr 18 14:48:54 2008 ...
enterprise kernel: [22321.027850] Code: 75 58 48 8b 7d 00 e8 64 4f ff ff f6 85 98 00 00 00 01 ba 01 00 00 00 75 04 0f b6 55 4c 48 8b 75 58 48 8d 8d a0 00 00 00 48 89 c7 <ff> 50 08 e9 f1 07 00 00 8a 45 4c 3c 02 74 0a 3c 04 0f 85 73 13

Message from syslogd@enterprise at Fri Apr 18 14:48:54 2008 ...
enterprise kernel: [22321.027850] CR2: 0000000000000008
Apr 18 14:48:54 enterprise kernel: [22321.027850] PGD 36f1a8067 PUD 327c17067 PMD 0
Apr 18 14:48:54 enterprise kernel: [22321.027850] CPU 1
Apr 18 14:48:54 enterprise kernel: [22321.027850] Modules linked in: kvm_intel kvm aic94xx libsas scsi_transport_sas [last unloaded: kvm]
Apr 18 14:48:54 enterprise kernel: [22321.027850] Pid: 7814, comm: qemu-system-x86 Tainted: G      D  2.6.25 #207
Apr 18 14:48:54 enterprise kernel: [22321.027850] RIP: 0010:[<ffffffff88043933>]  [<ffffffff88043933>] :kvm:x86_emulate_insn+0x2d97/0x414c
Apr 18 14:48:54 enterprise kernel: [22321.027850] RSP: 0018:ffff81033005fb68  EFLAGS: 00010202
Apr 18 14:48:54 enterprise kernel: [22321.027850] RAX: 0000000000000000 RBX: ffff810344cf9440 RCX: ffff810344cf9498
Apr 18 14:48:54 enterprise kernel: [22321.027850] RDX: 0000000000000001 RSI: 000000000000007a RDI: 0000000000000000
Apr 18 14:48:54 enterprise kernel: [22321.027850] RBP: ffff810344cf93f8 R08: 0000000000000000 R09: 0000000000000000
Apr 18 14:48:54 enterprise kernel: [22321.027850] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
Apr 18 14:48:54 enterprise kernel: [22321.027850] R13: ffffffff88051e50 R14: ffff810344cf9498 R15: 0000000000007ad6
Apr 18 14:48:54 enterprise kernel: [22321.027850] FS:  000000004108b950(0000) GS:ffff810397c250c0(0000) knlGS:0000000000000000
Apr 18 14:48:54 enterprise kernel: [22321.027850] CS:  0010 DS: 002b ES: 002b CR0: 0000000080050033
Apr 18 14:48:54 enterprise kernel: [22321.027850] CR2: 0000000000000008 CR3: 00000003301b2000 CR4: 00000000000026e0
Apr 18 14:48:54 enterprise kernel: [22321.027850] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
Apr 18 14:48:54 enterprise kernel: [22321.027850] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Apr 18 14:48:54 enterprise kernel: [22321.027850] Process qemu-system-x86 (pid: 7814, threadinfo ffff81033005e000, task ffff810396023080)
Apr 18 14:48:54 enterprise kernel: [22321.027850] Stack:  ffff81033005fb04 0000000000000088 ffff810344cf9438 ffff810344cf9440
Apr 18 14:48:54 enterprise kernel: [22321.027850]  0000000000040040 0000000000055e1c 0000000000055e1c ffff810344cf9498
Apr 18 14:48:54 enterprise kernel: [22321.027850]  0000000000800009 ffffffff8805087a 0000000000000000 ffff810344cf80c0
Apr 18 14:48:54 enterprise kernel: [22321.027850] Call Trace:
Apr 18 14:48:54 enterprise kernel: [22321.027850]  [<ffffffff88038d91>] ? :kvm:emulate_instruction+0x1e5/0x2b9
Apr 18 14:48:54 enterprise kernel: [22321.027850]  [<ffffffff88057cd1>] ? :kvm_intel:kvm_handle_exit+0xea/0x1e8
Apr 18 14:48:54 enterprise kernel: [22321.027850]  [<ffffffff88057a96>] ? :kvm_intel:vmx_intr_assist+0x68/0x1b9
Apr 18 14:48:54 enterprise kernel: [22321.027850]  [<ffffffff80563398>] ? __down_read+0x12/0xa1
Apr 18 14:48:54 enterprise kernel: [22321.027850]  [<ffffffff8803b940>] ? :kvm:kvm_arch_vcpu_ioctl_run+0x4ae/0x631
Apr 18 14:48:54 enterprise kernel: [22321.027850]  [<ffffffff80291ec9>] ? touch_atime+0xae/0xed
Apr 18 14:48:54 enterprise kernel: [22321.027850]  [<ffffffff8803672e>] ? :kvm:kvm_vcpu_ioctl+0xf3/0x3a1
Apr 18 14:48:54 enterprise kernel: [22321.027850]  [<ffffffff802802c0>] ? do_sync_read+0xd1/0x118
Apr 18 14:48:54 enterprise kernel: [22321.027850]  [<ffffffff880363b1>] ? :kvm:kvm_vm_ioctl+0x1ab/0x1c3
Apr 18 14:48:54 enterprise kernel: [22321.027850]  [<ffffffff8028ae49>] ? vfs_ioctl+0x21/0x6b
Apr 18 14:48:54 enterprise kernel: [22321.027850]  [<ffffffff8028b0e6>] ? do_vfs_ioctl+0x253/0x264
Apr 18 14:48:54 enterprise kernel: [22321.027850]  [<ffffffff80280aa0>] ? vfs_read+0x11e/0x132
Apr 18 14:48:54 enterprise kernel: [22321.027850]  [<ffffffff8028b133>] ? sys_ioctl+0x3c/0x5d
Apr 18 14:48:54 enterprise kernel: [22321.027850]  [<ffffffff8020b08a>] ? system_call_after_swapgs+0x8a/0x8f
Apr 18 14:48:54 enterprise kernel: [22321.027850]
Apr 18 14:48:54 enterprise kernel: [22321.027850]
Apr 18 14:48:54 enterprise kernel: [22321.027850]  RSP <ffff81033005fb68>
Apr 18 14:48:54 enterprise kernel: [22321.028054] ---[ end trace 153cd0388d4ba429 ]---
q

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Anthony Liguori]

Guillaume Thouvenin wrote:
> On Tue, 15 Apr 2008 16:06:43 +0300
> Avi Kivity <avi@qumranet.com> wrote:
>
>   
>>> ...
>>> handle_vmentry_failure: invalid guest state
>>> handle_vmentry_failure: start emulation
>>> handle_vmentry_failure: emulation failed
>>>   
>>>       
>> What instruction failed, exactly?
>>
>>     
>
> I added the code do dump the instruction and it seems that it's the
> emulation of 0xe6 (== out imm8, al) that failed. I made modifications
> to emulate it (see below) and now I have another problem in kvm
> userspace with the following message (and the emulation doesn't work):
>
> enterprise:~ $ kvm_run: Operation not permitted
> enterprise:~ $ kvm_run returned -1
>  
>   
>> You need to load rip as well.
>>     
>
> Ooops, yes. So jump far emulation is now like:
>
> +       case 0xea: /* jmp far */ {
> +               struct kvm_segment kvm_seg;
> +               long int eip;
> +               int ret;
> +
> +               kvm_x86_ops->get_segment(ctxt->vcpu, &kvm_seg, VCPU_SREG_CS); 
> +
> +               ret = load_segment_descriptor(ctxt->vcpu, kvm_seg.selector, 9, VCPU_SREG_CS);
> +               if (ret < 0){
> +                       printk(KERN_INFO "%s: Failed to load CS descriptor\n", __FUNCTION__);
> +                       goto cannot_emulate;
> +               }
> +
> +               switch (c->op_bytes) {
> +               case 2:
> +                       eip = insn_fetch(s16, 2, c->eip);
> +                       break;
> +               case 4:
> +                       eip = insn_fetch(s32, 4, c->eip);
> +                       break;
> +               default:
> +                       DPRINTF("jmp far: Invalid op_bytes\n");
> +                       goto cannot_emulate;
> +               }
> +               printk(KERN_INFO "eip == 0x%lx\n", eip);
> +               c->eip = eip;
> +               break;
> +       }
>
> It seems that the jump to cs:eip works and now I have the following error:
>
> [18535.446917] handle_vmentry_failure: invalid guest state
> [18535.449519] handle_vmentry_failure: start emulation
> [18535.457519] eip == 0x6e18
> [18535.467685] handle_vmentry_failure: emulation of 0xe6 failed
>
> For the emulation of 0xe6 I used the following one that I found in
> nitin's tree:
>   

This doesn't seem right.  You should have been able to break out of the 
emulator long before encountering an out instruction.  The next 
instruction you encounter should be a mov instruction.  Are you sure 
you're updating eip correctly?

Regards,

Anthony Liguori

> +       case 0xe6: /* out imm8, al */
> +       case 0xe7: /* out imm8, ax/eax */ {
> +               struct kvm_io_device *pio_dev;
> +               
> +               pio_dev = vcpu_find_pio_dev(ctxt->vcpu, c->src.val);
> +               kvm_iodevice_write(pio_dev, c->src.val,
> +                               (c->d & ByteOp) ? 1 : c->op_bytes,
> +                               &c->regs[VCPU_REGS_RAX]);
> +               }
> +               break;
>
> I will look closer where is the problem and as you suggested, I will
> display the instruction to be emulated and the register state before
> and after, and compare with the expected state.
>
>
> Thanks for your help,
> Regards,
> Guillaume
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
> Don't miss this year's exciting event. There's still time to save $100. 
> Use priority code J8TL2D2. 
> http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
> _______________________________________________
> kvm-devel mailing list
> kvm-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/kvm-devel
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Guillaume Thouvenin]

On Fri, 18 Apr 2008 08:23:07 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

 
> This doesn't seem right.  You should have been able to break out of the 
> emulator long before encountering an out instruction.  The next 
> instruction you encounter should be a mov instruction.  Are you sure 
> you're updating eip correctly?

I think that eip is updated correctly but you're right, I think that
the condition to stop emulation is not well implemented. I emulate a
lot of mov instructions and I remain blocked in the emulation loop
until I reach the "out" instruction. The loop is the following:

  [...]
  cs_rpl = vmcs_read16(GUEST_CS_SELECTOR) & SELECTOR_RPL_MASK;
  ss_rpl = vmcs_read16(GUEST_SS_SELECTOR) & SELECTOR_RPL_MASK;

  while (cs_rpl != ss_rpl) {
      if (emulate_instruction(vcpu, NULL, 0,0, 0) == EMULATE_FAIL) {
          printk(KERN_INFO "%s: emulation of 0x%x failed\n",
                           __FUNCTION__,
                           vcpu->arch.emulate_ctxt.decode.b);
          return -1;
       }
       cs_rpl = vmcs_read16(GUEST_CS_SELECTOR) & SELECTOR_RPL_MASK;
       ss_rpl = vmcs_read16(GUEST_SS_SELECTOR) & SELECTOR_RPL_MASK;
  }
  printk(KERN_INFO "%s: VMX friendly state recovered\n", __FUNCTION__);
  // I never reach this point

Maybe CS and SS selector are not well updated. I will add trace to see
their values before and after the emulation.


Regards,
Guillaume

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Anthony Liguori]

Guillaume Thouvenin wrote:
> On Fri, 18 Apr 2008 08:23:07 -0500
> Anthony Liguori <anthony@codemonkey.ws> wrote:
>
>  
>   
>> This doesn't seem right.  You should have been able to break out of the 
>> emulator long before encountering an out instruction.  The next 
>> instruction you encounter should be a mov instruction.  Are you sure 
>> you're updating eip correctly?
>>     
>
> I think that eip is updated correctly but you're right, I think that
> the condition to stop emulation is not well implemented. I emulate a
> lot of mov instructions and I remain blocked in the emulation loop
> until I reach the "out" instruction. The loop is the following:
>
>   [...]
>   cs_rpl = vmcs_read16(GUEST_CS_SELECTOR) & SELECTOR_RPL_MASK;
>   ss_rpl = vmcs_read16(GUEST_SS_SELECTOR) & SELECTOR_RPL_MASK;
>
>   while (cs_rpl != ss_rpl) {
>       if (emulate_instruction(vcpu, NULL, 0,0, 0) == EMULATE_FAIL) {
>           printk(KERN_INFO "%s: emulation of 0x%x failed\n",
>                            __FUNCTION__,
>                            vcpu->arch.emulate_ctxt.decode.b);
>           return -1;
>        }
>        cs_rpl = vmcs_read16(GUEST_CS_SELECTOR) & SELECTOR_RPL_MASK;
>        ss_rpl = vmcs_read16(GUEST_SS_SELECTOR) & SELECTOR_RPL_MASK;
>   }
>   printk(KERN_INFO "%s: VMX friendly state recovered\n", __FUNCTION__);
>   // I never reach this point
>
> Maybe CS and SS selector are not well updated. I will add trace to see
> their values before and after the emulation.
>   

I'd prefer you not do an emulate_instruction loop at all.  Just emulate 
one instruction on vmentry failure and let VT tell you what instructions 
you need to emulate.

It's only four instructions so I don't think the performance is going to 
matter.  Take a look at the patch I posted previously.

Regards,

Anthony Liguori

> Regards,
> Guillaume
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Avi Kivity]

Anthony Liguori wrote:
>
> I'd prefer you not do an emulate_instruction loop at all.  Just 
> emulate one instruction on vmentry failure and let VT tell you what 
> instructions you need to emulate.
>
> It's only four instructions so I don't think the performance is going 
> to matter.  Take a look at the patch I posted previously.

Once we remove the other VT realmode hacks, we may need more 
instructions emulated.  Consider for example changing to real mode 
without reloading fs and gs; this will cause all real mode code to be 
emulated.

However, there's no need to do everything at once; the loop can 
certainly be added later when we have a proven need for it.


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [PATCH] gfxboot VMX workaround v2

[Guillaume Thouvenin]

On Fri, 18 Apr 2008 10:25:15 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> I'd prefer you not do an emulate_instruction loop at all.  Just emulate 
> one instruction on vmentry failure and let VT tell you what instructions 
> you need to emulate.
> 
> It's only four instructions so I don't think the performance is going to 
> matter.  Take a look at the patch I posted previously.

you were right, I not updated eip correctly. It is fixed now with the
following code:


  case 0xea: /* jmp (far, absolute) */ {
            struct kvm_segment kvm_seg;
            uint16_t eip;
            uint16_t sel;
            int ret;

            eip = insn_fetch(u16, 2, c->eip);
            sel = insn_fetch(u16, 2, c->eip);
            kvm_x86_ops->get_segment(ctxt->vcpu, &kvm_seg, VCPU_SREG_CS);
            kvm_seg.selector = sel;
            ret = load_segment_descriptor(ctxt->vcpu, kvm_seg.selector, 9,
                                          VCPU_SREG_CS);
            if (ret < 0 ) {
                   printk(KERN_INFO "%s: Failed to load CS selector\n",
                                    __FUNCTION__);
                   goto cannot_emulate;  
            }

            c->eip = eip;
            break;


I print the instruction to be emulated and it seems ok. I have the following outputs:

[24203.663324] vmentry_failure: emulation at (46e53) rip 6e13: ea 18 6e 18
[24203.664668] vmentry_failure: emulation at (46e58) rip 6e18: 66 b8 20 00
[24203.668650] vmentry_failure: emulation failed (vmentry failure) rip 6e18 66 b8 20 00

So the emulation that failed is "mov $0x20, %ax". It needs to be
emulated. As you said Anthony it's only four instructions that need to
be emulated, shouldn't be a big issue. 


Best regards,
Guillaume

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone