[PATCH] vmx: correct EIP value of task-state segment

[PATCH] vmx: correct EIP value of task-state segment

[Kouya Shimura]

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 144 bytes --]

Major OSes(Linux, windows, ...) don't seem to use task switching. 
So this bug is missed.

Signed-off-by: Kouya Shimura <kouya@jp.fujitsu.com>


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

diff -r e6c966b3a4d8 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c	Thu Jul 30 17:56:23 2009 +0100
+++ b/xen/arch/x86/hvm/vmx/vmx.c	Fri Jul 31 09:15:16 2009 +0900
@@ -2490,6 +2490,8 @@ asmlinkage void vmx_vmexit_handler(struc
         if ( (idtv_info & INTR_INFO_VALID_MASK) &&
              (idtv_info & INTR_INFO_DELIVER_CODE_MASK) )
             errcode = __vmread(IDT_VECTORING_ERROR_CODE);
+        inst_len = __get_instruction_length(); /* Safe: See SDM 3B 23.2.4 */
+        regs->eip += inst_len;
         hvm_task_switch((uint16_t)exit_qualification,
                         reasons[(exit_qualification >> 30) & 3],
                         errcode);

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] vmx: correct EIP value of task-state segment

[Keir Fraser]

On 31/07/2009 02:19, "Kouya Shimura" <kouya@jp.fujitsu.com> wrote:

> Major OSes(Linux, windows, ...) don't seem to use task switching.
> So this bug is missed.
> 
> Signed-off-by: Kouya Shimura <kouya@jp.fujitsu.com>

What about SVM, and what if the reason for the task switch is a hardware
interrupt or exception (the only case we care about for Windows at least)?

The reference manuals are sketchy enough you may need to test out some task
switches on SVM and VMX hardware to make sure you make patches that really
do the right thing.

 -- Keir

Re: [PATCH] vmx: correct EIP value of task-state segment

[Kouya Shimura]

Keir Fraser writes:
> What about SVM, and what if the reason for the task switch is a hardware
> interrupt or exception (the only case we care about for Windows at least)?

Unfortunately I have no SVM machine. 
The task switch is a software interrupt "int $2" in a proprietary OS.

> The reference manuals are sketchy enough you may need to test out some task
> switches on SVM and VMX hardware to make sure you make patches that really
> do the right thing.

Sorry, I don't have much time to test this. 
But the OS works fine with this patch. (of course, the OS is for native) 
I confirmed Windows(XP,2008Server) never use a task switch. 
So I think it's worth to get in the tree.

Thanks,
Kouya

Re: [PATCH] vmx: correct EIP value of task-state segment

[Keir Fraser]

On 31/07/2009 10:34, "Kouya Shimura" <kouya@jp.fujitsu.com> wrote:

>> The reference manuals are sketchy enough you may need to test out some task
>> switches on SVM and VMX hardware to make sure you make patches that really
>> do the right thing.
> 
> Sorry, I don't have much time to test this.
> But the OS works fine with this patch. (of course, the OS is for native)
> I confirmed Windows(XP,2008Server) never use a task switch.

They do on blue screen I believe. OTOH I suppose they don't come back from
it. I'll have to queue this up for testing myself.

Is the "int 2" in that popular proprietary OS "BS2000" perchance? ;-)

 -- Keir

> So I think it's worth to get in the tree.

Re: [PATCH] vmx: correct EIP value of task-state segment

[Juergen Gross]

Keir Fraser wrote:
> On 31/07/2009 10:34, "Kouya Shimura" <kouya@jp.fujitsu.com> wrote:
> 
>>> The reference manuals are sketchy enough you may need to test out some task
>>> switches on SVM and VMX hardware to make sure you make patches that really
>>> do the right thing.
>> Sorry, I don't have much time to test this.
>> But the OS works fine with this patch. (of course, the OS is for native)
>> I confirmed Windows(XP,2008Server) never use a task switch.
> 
> They do on blue screen I believe. OTOH I suppose they don't come back from
> it. I'll have to queue this up for testing myself.
> 
> Is the "int 2" in that popular proprietary OS "BS2000" perchance? ;-)

:-)

No, it isn't.
And we are not doing task switches.


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
TSP ES&S SWE OS6                       Telephone: +49 (0) 89 636 47950
Fujitsu Technolgy Solutions               e-mail: juergen.gross@ts.fujitsu.com
Otto-Hahn-Ring 6                        Internet: ts.fujitsu.com
D-81739 Muenchen                 Company details: ts.fujitsu.com/imprint.html

Re: [PATCH] vmx: correct EIP value of task-state segment

[Keir Fraser]

On 31/07/2009 13:18, "Juergen Gross" <juergen.gross@ts.fujitsu.com> wrote:

>> They do on blue screen I believe. OTOH I suppose they don't come back from
>> it. I'll have to queue this up for testing myself.
>> 
>> Is the "int 2" in that popular proprietary OS "BS2000" perchance? ;-)
> 
> :-)
> 
> No, it isn't.
> And we are not doing task switches.

Okay. Well, I don't disagree we should get this fixed. The patch is in my
todo queue now.

 -- Keir

Re: [PATCH] vmx: correct EIP value of task-state segment

[Keir Fraser]

Hi Kouya,

I applied an extended version of your patch as c/s 20097. It should do the
right thing for task switches triggered by ExtInt, NMI, or hardware
exception (i.e, not update EIP in those cases). It would be good if you
could take a look and also test.

It's worth noting that I did not fix the AMD SVM case as that is rather
trickier. This is because SVM does not provide the instruction length, so we
would have to decode it ourselves. And the instruction can be a fairly
arbitrary JMPF/CALLF variant, so we would have to smarten up the SVM
insn-len decoder considerably (to decode effective addresses, for example),
or go into x86_emulate() and have that properly emulate task switches.
Neither is an attractive work item. :-) If I had to pick one I'd probably go
for a smarter insn-len decoder, even though that's aesthetically perhaps
more 'hacky'. But someone who cares can go do the work.

 -- Keir

On 31/07/2009 02:19, "Kouya Shimura" <kouya@jp.fujitsu.com> wrote:

> Major OSes(Linux, windows, ...) don't seem to use task switching.
> So this bug is missed.
> 
> Signed-off-by: Kouya Shimura <kouya@jp.fujitsu.com>
> 

Re: [PATCH] vmx: correct EIP value of task-state segment

[Kouya Shimura]

Hi Keir,

Thanks for remembering. 
It looks good. And our proprietary OS works fine.
Thanks a lot.

Nonsupporting the AMD SVM case is no problem for us.
The OS is embedded to an Intel platform and
we have no plan to use an AMD platform.

Thanks,
Kouya

Keir Fraser writes:
> Hi Kouya,
> 
> I applied an extended version of your patch as c/s 20097. It should do the
> right thing for task switches triggered by ExtInt, NMI, or hardware
> exception (i.e, not update EIP in those cases). It would be good if you
> could take a look and also test.
> 
> It's worth noting that I did not fix the AMD SVM case as that is rather
> trickier. This is because SVM does not provide the instruction length, so we
> would have to decode it ourselves. And the instruction can be a fairly
> arbitrary JMPF/CALLF variant, so we would have to smarten up the SVM
> insn-len decoder considerably (to decode effective addresses, for example),
> or go into x86_emulate() and have that properly emulate task switches.
> Neither is an attractive work item. :-) If I had to pick one I'd probably go
> for a smarter insn-len decoder, even though that's aesthetically perhaps
> more 'hacky'. But someone who cares can go do the work.
> 
>  -- Keir
> 
> On 31/07/2009 02:19, "Kouya Shimura" <kouya@jp.fujitsu.com> wrote:
> 
> > Major OSes(Linux, windows, ...) don't seem to use task switching.
> > So this bug is missed.
> > 
> > Signed-off-by: Kouya Shimura <kouya@jp.fujitsu.com>
> > 
>