[RFC PATCH] emulator: Fix task switch into/out of VM86

[RFC PATCH] emulator: Fix task switch into/out of VM86

[Kevin Wolf]

Not sure if it actually makes sense to send a patch with this as I have
more problems to describe than lines that actually fix something - but
well, this hack at least makes some hardware task switches into/out of
VM86 work for me that failed before.

So what I really intend is to get information on what I need to do to
get this right (alternatively consider this a bug report and do the fix
yourself):

* I have disabled privilege checks for task gates (I need to switch out
  of VM86/ring 3 back into ring 0). This is wrong, they still need to be
  applied for software interrupts, but not for exceptions or IRQs (note
  that the check was wrong anyway, it should check the task gate DPL,
  not the TSS one).

  How do I find out what kind of interrupt caused the task switch? (Hm,
  I guess vmx->idt_vectoring_info could do it for Intel and I would have
  to pass this down into the emulator; for AMD see below)

* May the emulator access the vcpu or should this go through a new
  x86_emulate_ops.set_rflags function pointer?

* This works with VMX, but with SVM I have an additional problem: When
  trying to exit VM86 (usually by an exception) through a task gate in
  the IDT, the code runs into the reason = TASK_SWITCH_CALL path. I
  searched a bit in the documentation, but didn't find any obvious way
  to fix this.

* Yes, I've yet to write a nice testcase for kvm-unittests

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 arch/x86/kvm/emulate.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 05a562b..fdd3cca 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1227,6 +1227,7 @@ static int load_segment_descriptor(struct x86_emulate_ctxt *ctxt,
 		seg_desc.type = 3;
 		seg_desc.p = 1;
 		seg_desc.s = 1;
+		seg_desc.dpl = 3;
 		goto load;
 	}
 
@@ -2246,6 +2247,9 @@ static void save_state_to_tss32(struct x86_emulate_ctxt *ctxt,
 	tss->ldt_selector = get_segment_selector(ctxt, VCPU_SREG_LDTR);
 }
 
+#define emul_to_vcpu(ctxt) \
+	container_of(ctxt, struct kvm_vcpu, arch.emulate_ctxt)
+
 static int load_state_from_tss32(struct x86_emulate_ctxt *ctxt,
 				 struct tss_segment_32 *tss)
 {
@@ -2254,7 +2258,14 @@ static int load_state_from_tss32(struct x86_emulate_ctxt *ctxt,
 	if (ctxt->ops->set_cr(ctxt, 3, tss->cr3))
 		return emulate_gp(ctxt, 0);
 	ctxt->_eip = tss->eip;
+
 	ctxt->eflags = tss->eflags | 2;
+	if (ctxt->eflags & 0x20000)
+		ctxt->mode = X86EMUL_MODE_VM86;
+	else
+		ctxt->mode = X86EMUL_MODE_PROT32;
+	kvm_set_rflags(emul_to_vcpu(ctxt), ctxt->eflags);
+
 	ctxt->regs[VCPU_REGS_RAX] = tss->eax;
 	ctxt->regs[VCPU_REGS_RCX] = tss->ecx;
 	ctxt->regs[VCPU_REGS_RDX] = tss->edx;
@@ -2372,7 +2383,7 @@ static int emulator_do_task_switch(struct x86_emulate_ctxt *ctxt,
 
 	/* FIXME: check that next_tss_desc is tss */
 
-	if (reason != TASK_SWITCH_IRET) {
+	if (reason != TASK_SWITCH_IRET && reason != TASK_SWITCH_GATE) {
 		if ((tss_selector & 3) > next_tss_desc.dpl ||
 		    ops->cpl(ctxt) > next_tss_desc.dpl)
 			return emulate_gp(ctxt, 0);
-- 
1.7.6.5

Re: [RFC PATCH] emulator: Fix task switch into/out of VM86

[Takuya Yoshikawa]

(2012/01/10 5:10), Kevin Wolf wrote:
> * May the emulator access the vcpu or should this go through a new
>    x86_emulate_ops.set_rflags function pointer?

I think the latter is better.
Accessing the vcpu from the emulator looks like a layer violation.

	Takuya

Re: [RFC PATCH] emulator: Fix task switch into/out of VM86

[Takuya Yoshikawa]

(2012/01/10 5:10), Kevin Wolf wrote:

> @@ -2254,7 +2258,14 @@ static int load_state_from_tss32(struct x86_emulate_ctxt *ctxt,
>   	if (ctxt->ops->set_cr(ctxt, 3, tss->cr3))
>   		return emulate_gp(ctxt, 0);
>   	ctxt->_eip = tss->eip;
> +
>   	ctxt->eflags = tss->eflags | 2;

(Though not directly related to this RFC ...)

What is this 2 for?
Do we need to set a reserved bit?

> +	if (ctxt->eflags & 0x20000)

You can use a macro to indicate the flag.

1. from arch/x86/kvm/emulate.c:

/* EFLAGS bit definitions. */
#define EFLG_ID (1<<21)
#define EFLG_VIP (1<<20)
#define EFLG_VIF (1<<19)
#define EFLG_AC (1<<18)
#define EFLG_VM (1<<17)
#define EFLG_RF (1<<16)
...

#define EFLG_RESERVED_ZEROS_MASK 0xffc0802a
#define EFLG_RESERVED_ONE_MASK 2

2. from arch/x86/include/asm/processor-flags.h

/*
 * EFLAGS bits
 */
#define X86_EFLAGS_CF   0x00000001 /* Carry Flag */
#define X86_EFLAGS_PF   0x00000004 /* Parity Flag */
...
#define X86_EFLAGS_VM   0x00020000 /* Virtual Mode */
#define X86_EFLAGS_AC   0x00040000 /* Alignment Check */
#define X86_EFLAGS_VIF  0x00080000 /* Virtual Interrupt Flag */
#define X86_EFLAGS_VIP  0x00100000 /* Virtual Interrupt Pending */
#define X86_EFLAGS_ID   0x00200000 /* CPUID detection flag */


Two possibilities, not nice, but both are used in emulate.c.

	Takuya

Re: [RFC PATCH] emulator: Fix task switch into/out of VM86

[Gleb Natapov]

On Mon, Jan 09, 2012 at 09:10:10PM +0100, Kevin Wolf wrote:
> * This works with VMX, but with SVM I have an additional problem: When
>   trying to exit VM86 (usually by an exception) through a task gate in
>   the IDT, the code runs into the reason = TASK_SWITCH_CALL path. I
>   searched a bit in the documentation, but didn't find any obvious way
>   to fix this.
> 
Hmm, so exit_int_info is invalid during task switch exit even though
task switch was caused by an exception. I wonder is this the case when
vcpu is not in vm86 mode too? For vm86 we can change:

        else
                reason = TASK_SWITCH_CALL;

to
	else if (vcpu in vm86 mode)
		reason = TASK_SWITCH_GATE;
	else
		reason = TASK_SWITCH_CALL;

IIRC you can't change tasks by call in vm86 mode.

--
			Gleb.

Re: [RFC PATCH] emulator: Fix task switch into/out of VM86

[Kevin Wolf]

Am 10.01.2012 10:01, schrieb Gleb Natapov:
> On Mon, Jan 09, 2012 at 09:10:10PM +0100, Kevin Wolf wrote:
>> * This works with VMX, but with SVM I have an additional problem: When
>>   trying to exit VM86 (usually by an exception) through a task gate in
>>   the IDT, the code runs into the reason = TASK_SWITCH_CALL path. I
>>   searched a bit in the documentation, but didn't find any obvious way
>>   to fix this.
>>
> Hmm, so exit_int_info is invalid during task switch exit even though
> task switch was caused by an exception. I wonder is this the case when
> vcpu is not in vm86 mode too?

No idea, I would have to try it out.

> For vm86 we can change:
> 
>         else
>                 reason = TASK_SWITCH_CALL;
> 
> to
> 	else if (vcpu in vm86 mode)
> 		reason = TASK_SWITCH_GATE;
> 	else
> 		reason = TASK_SWITCH_CALL;
> 
> IIRC you can't change tasks by call in vm86 mode.

Didn't check it in the manual, but you'll have a hard time accessing a
protected mode segment in VM86, so I guess you're right. And in the VM86
branch we can probably fake the rest of the interrupt information so
that we can pass the checks in the emulator (basically saying "not a
software interrupt" should be enough).

Kevin

Re: [RFC PATCH] emulator: Fix task switch into/out of VM86

[Gleb Natapov]

On Tue, Jan 10, 2012 at 10:28:06AM +0100, Kevin Wolf wrote:
> Am 10.01.2012 10:01, schrieb Gleb Natapov:
> > On Mon, Jan 09, 2012 at 09:10:10PM +0100, Kevin Wolf wrote:
> >> * This works with VMX, but with SVM I have an additional problem: When
> >>   trying to exit VM86 (usually by an exception) through a task gate in
> >>   the IDT, the code runs into the reason = TASK_SWITCH_CALL path. I
> >>   searched a bit in the documentation, but didn't find any obvious way
> >>   to fix this.
> >>
> > Hmm, so exit_int_info is invalid during task switch exit even though
> > task switch was caused by an exception. I wonder is this the case when
> > vcpu is not in vm86 mode too?
> 
> No idea, I would have to try it out.
> 
IIRC (again) 32 bit Linux kernel configures double fault as a task gate,
so causing #DF on 32 bit kernel guest should be enough to test that.
 
--
			Gleb.

Re: [RFC PATCH] emulator: Fix task switch into/out of VM86

[Kevin Wolf]

Am 10.01.2012 10:28, schrieb Kevin Wolf:
> Am 10.01.2012 10:01, schrieb Gleb Natapov:
>> On Mon, Jan 09, 2012 at 09:10:10PM +0100, Kevin Wolf wrote:
>>> * This works with VMX, but with SVM I have an additional problem: When
>>>   trying to exit VM86 (usually by an exception) through a task gate in
>>>   the IDT, the code runs into the reason = TASK_SWITCH_CALL path. I
>>>   searched a bit in the documentation, but didn't find any obvious way
>>>   to fix this.
>>>
>> Hmm, so exit_int_info is invalid during task switch exit even though
>> task switch was caused by an exception. I wonder is this the case when
>> vcpu is not in vm86 mode too?
> 
> No idea, I would have to try it out.

Did that now, and it looks like exit_int_info is always 0 during the
task switch intercept for a task gate in the IDT. So special-casing VM86
won't help, I'm afraid.

Kevin

Re: [RFC PATCH] emulator: Fix task switch into/out of VM86

[Gleb Natapov]

On Tue, Jan 10, 2012 at 12:25:18PM +0100, Kevin Wolf wrote:
> Am 10.01.2012 10:28, schrieb Kevin Wolf:
> > Am 10.01.2012 10:01, schrieb Gleb Natapov:
> >> On Mon, Jan 09, 2012 at 09:10:10PM +0100, Kevin Wolf wrote:
> >>> * This works with VMX, but with SVM I have an additional problem: When
> >>>   trying to exit VM86 (usually by an exception) through a task gate in
> >>>   the IDT, the code runs into the reason = TASK_SWITCH_CALL path. I
> >>>   searched a bit in the documentation, but didn't find any obvious way
> >>>   to fix this.
> >>>
> >> Hmm, so exit_int_info is invalid during task switch exit even though
> >> task switch was caused by an exception. I wonder is this the case when
> >> vcpu is not in vm86 mode too?
> > 
> > No idea, I would have to try it out.
> 
> Did that now, and it looks like exit_int_info is always 0 during the
> task switch intercept for a task gate in the IDT. So special-casing VM86
> won't help, I'm afraid.
> 
Joerg, do you know how can we check that task switch was cause by an
exception triggering task gate if exit_int_info is not provided during
intercept (short of decoding instruction that caused intercept to see if
it's a call to a gate, which is racy) ?

--
			Gleb.

Re: [RFC PATCH] emulator: Fix task switch into/out of VM86

[Joerg Roedel]

On Tue, Jan 10, 2012 at 01:30:47PM +0200, Gleb Natapov wrote:
> On Tue, Jan 10, 2012 at 12:25:18PM +0100, Kevin Wolf wrote:
> > Did that now, and it looks like exit_int_info is always 0 during the
> > task switch intercept for a task gate in the IDT. So special-casing VM86
> > won't help, I'm afraid.
> > 
> Joerg, do you know how can we check that task switch was cause by an
> exception triggering task gate if exit_int_info is not provided during
> intercept (short of decoding instruction that caused intercept to see if
> it's a call to a gate, which is racy) ?

Hmm, havn't found a solution yet. But I will check further and let you
know if I find out something.


	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [RFC PATCH] emulator: Fix task switch into/out of VM86

[Kevin Wolf]

Am 10.01.2012 18:51, schrieb Joerg Roedel:
> On Tue, Jan 10, 2012 at 01:30:47PM +0200, Gleb Natapov wrote:
>> On Tue, Jan 10, 2012 at 12:25:18PM +0100, Kevin Wolf wrote:
>>> Did that now, and it looks like exit_int_info is always 0 during the
>>> task switch intercept for a task gate in the IDT. So special-casing VM86
>>> won't help, I'm afraid.
>>>
>> Joerg, do you know how can we check that task switch was cause by an
>> exception triggering task gate if exit_int_info is not provided during
>> intercept (short of decoding instruction that caused intercept to see if
>> it's a call to a gate, which is racy) ?
> 
> Hmm, havn't found a solution yet. But I will check further and let you
> know if I find out something.

Jörg, any news on this?

Kevin

Re: [RFC PATCH] emulator: Fix task switch into/out of VM86

[Joerg Roedel]

On Mon, Jan 16, 2012 at 04:37:33PM +0100, Kevin Wolf wrote:
> Am 10.01.2012 18:51, schrieb Joerg Roedel:
> >> Joerg, do you know how can we check that task switch was cause by an
> >> exception triggering task gate if exit_int_info is not provided during
> >> intercept (short of decoding instruction that caused intercept to see if
> >> it's a call to a gate, which is racy) ?
> > 
> > Hmm, havn't found a solution yet. But I will check further and let you
> > know if I find out something.
> 
> Jörg, any news on this?

Not yet. We are currently trying to figure out which priviledge checks
the software has to do and which ones are done by the hardware before a
task-switch intercept is thrown.


	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632