Xen devel - Sep 2006 - [RFC][PATCH] Emulate instructions in vm86 mode

Hi folks

This patch fixes the emulation of instructions in vm86 mode. It fetches
them using cs and eip instead of only eip. This makes it at least
possible to use the i945GM vesa bios from the running system.

I''m not sure if this is the correct fix or if we should call the gpf
handler of the running system. At least the support in linux reports
traps back to the userspace caller which is not possible if emulated in
xen.

Signed-off-by: Bastian Blank <waldi@debian.org>

Bastian

-- 
Not one hundred percent efficient, of course ... but nothing ever is.
		-- Kirk, "Metamorphosis", stardate 3219.8



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

On 7/9/06 10:51, "Bastian Blank" <bastian@waldi.eu.org> wrote:
> This patch fixes the emulation of instructions in vm86 mode. It fetches
> them using cs and eip instead of only eip. This makes it at least
> possible to use the i945GM vesa bios from the running system.
> 
> I''m not sure if this is the correct fix or if we should call the
gpf
> handler of the running system. At least the support in linux reports
> traps back to the userspace caller which is not possible if emulated in
> xen.
There are also data operands of some instructions that may need fixing
(INS/OUTS). What happens if you just bail to the guest''s GPF handler if
in
vm86 mode? I think the libint10 library in userspace will emulate all these
instructions itself.

 -- Keir



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

On Thu, Sep 07, 2006 at 01:42:02PM +0100, Keir Fraser
wrote:
> > I''m not sure if this is the correct fix or if we should call
the gpf
> > handler of the running system. At least the support in linux reports
> > traps back to the userspace caller which is not possible if emulated
in
> > xen.
> There are also data operands of some instructions that may need fixing
> (INS/OUTS). What happens if you just bail to the guest''s GPF
handler if in
> vm86 mode?
I don''t know how to do that.

There is some code to pass it to the guest in do_general_protection, but
I don''t know why it fails than.

Bastian

-- 
You''re dead, Jim.
		-- McCoy, "Amok Time", stardate 3372.7


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

On 7/9/06 14:09, "Bastian Blank" <bastian@waldi.eu.org> wrote:
>> (INS/OUTS). What happens if you just bail to the guest''s GPF
handler if in
>> vm86 mode?
> 
> I don''t know how to do that.
Just add ''if (vm86_mode(regs)) goto fail'' to the top of
emulate_privileged_op(). This will cause Xen to fall back to propagating the
fault to the guest''s GPF handler.

 -- Keir
> There is some code to pass it to the guest in do_general_protection, but
> I don''t know why it fails than.


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

On Thu, Sep 07, 2006 at 02:24:33PM +0100, Keir Fraser
wrote:
> On 7/9/06 14:09, "Bastian Blank" <bastian@waldi.eu.org>
wrote:
> >> (INS/OUTS). What happens if you just bail to the guest''s
GPF handler if in
> >> vm86 mode?
> > I don''t know how to do that.
> Just add ''if (vm86_mode(regs)) goto fail'' to the top of
> emulate_privileged_op(). This will cause Xen to fall back to propagating
the
> fault to the guest''s GPF handler.
Seems to work partialy. The i810 driver can init the hardware but I get
weird artefacts. It seems that some of the memory which is used for the
cursor is overwriten by the real video output or so. Therefor I think
that one of the segment registers which the bios uses is overwriten
somewhere in this gpf handling.

Hmm, the trap bounce code clears ds and es.

Bastian

-- 
Schshschshchsch.
		-- The Gorn, "Arena", stardate 3046.2


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

On 7/9/06 14:59, "Bastian Blank" <bastian@waldi.eu.org> wrote:
> Seems to work partialy. The i810 driver can init the hardware but I get
> weird artefacts. It seems that some of the memory which is used for the
> cursor is overwriten by the real video output or so. Therefor I think
> that one of the segment registers which the bios uses is overwriten
> somewhere in this gpf handling.
> 
> Hmm, the trap bounce code clears ds and es.
The trap bounce code clears fs and gs too, just as native hardware does on
an exception while in vm86 mode. The real values are saved on the exception
stack frame (hopefully!).

 -- Keir



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

>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 09/07/06 2:42 PM
>>>
>On 7/9/06 10:51, "Bastian Blank" <bastian@waldi.eu.org>
wrote:
>
>> This patch fixes the emulation of instructions in vm86 mode. It fetches
>> them using cs and eip instead of only eip. This makes it at least
>> possible to use the i945GM vesa bios from the running system.
>> 
>> I''m not sure if this is the correct fix or if we should call
the gpf
>> handler of the running system. At least the support in linux reports
>> traps back to the userspace caller which is not possible if emulated in
>> xen.
>
>There are also data operands of some instructions that may need fixing
>(INS/OUTS). What happens if you just bail to the guest''s GPF
handler if in
>vm86 mode? I think the libint10 library in userspace will emulate all these
>instructions itself.
If the vm86 code isn''t running with an i/o bitmap permitting access
(which
supposedly libint10 doesn''t do), all i/o related faults should alread
be
reflected back to the guest, shouldn''t they? If otoh there was
permission
granted from the kernel, then we shouldn''t assume that code is prepared
to deal with gp faults from respective port accesses.

However, the original patch would, if to be integrated, in my opinion need
quite a bit of additional work - it should honor non-zero segment bases
generally, and it should do proper limit checking for non-flat segments.

Jan

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

On Fri, Sep 08, 2006 at 06:00:25PM +0100, Jan Beulich
wrote:
> If the vm86 code isn''t running with an i/o bitmap permitting
access (which
> supposedly libint10 doesn''t do), all i/o related faults should
alread be
> reflected back to the guest, shouldn''t they? If otoh there was
permission
> granted from the kernel, then we shouldn''t assume that code is
prepared
> to deal with gp faults from respective port accesses.
Yes, but it does weird things, so I think there is something broken.
> However, the original patch would, if to be integrated, in my opinion need
> quite a bit of additional work - it should honor non-zero segment bases
> generally, and it should do proper limit checking for non-flat segments.
Isn''t this already done by copy_from_user?

Bastian

-- 
Ahead warp factor one, Mr. Sulu.


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

On Thu, Sep 07, 2006 at 06:17:58PM +0100, Keir Fraser
wrote:
> The trap bounce code clears fs and gs too, just as native hardware does on
> an exception while in vm86 mode. The real values are saved on the exception
> stack frame (hopefully!).
Hmm, the code write 0 for this values to the exception frame in VM86
mode, lets check if this is the problem.

Bastian

-- 
Punishment becomes ineffective after a certain point.  Men become insensitive.
		-- Eneg, "Patterns of Force", stardate 2534.7


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

On Fri, Sep 08, 2006 at 09:37:44PM +0200, Bastian Blank
wrote:
> On Thu, Sep 07, 2006 at 06:17:58PM +0100, Keir Fraser wrote:
> > The trap bounce code clears fs and gs too, just as native hardware
does on
> > an exception while in vm86 mode. The real values are saved on the
exception
> > stack frame (hopefully!).
> Hmm, the code write 0 for this values to the exception frame in VM86
> mode, lets check if this is the problem.
It seems that this is the problem.

The following patch always bounces traps in vm86 mode to the guest
kernel and don''t longer clears the segment register values in the
exception frame.

Signed-off-by: Bastian Blank <waldi@debian.org>

Bastian

-- 
The sooner our happiness together begins, the longer it will last.
		-- Miramanee, "The Paradise Syndrome", stardate 4842.6



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

On 8/9/06 21:58, "Bastian Blank" <bastian@waldi.eu.org> wrote:
>> Hmm, the code write 0 for this values to the exception frame in VM86
>> mode, lets check if this is the problem.
> 
> It seems that this is the problem.
> 
> The following patch always bounces traps in vm86 mode to the guest
> kernel and don''t longer clears the segment register values in the
> exception frame.
> 
> Signed-off-by: Bastian Blank <waldi@debian.org>
That is only on the ''failsafe'' stackframe path. It is okay to
write zero
values there because we already wrote the real values lower down on the
stack frame (at labels FLT7 to FLT10). In fact you probably shouldn''t
end up
taking the ''failsafe'' path anyway, so the code you remove
wouldn''t normally
be executed at all unless something in the guest is buggy.

 -- Keir



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

On Fri, Sep 08, 2006 at 10:11:11PM +0100, Keir Fraser
wrote:
> That is only on the ''failsafe'' stackframe path. It is
okay to write zero
> values there because we already wrote the real values lower down on the
> stack frame (at labels FLT7 to FLT10). In fact you probably
shouldn''t end up
> taking the ''failsafe'' path anyway, so the code you remove
wouldn''t normally
> be executed at all unless something in the guest is buggy.
Yep, saw that. So I have to recheck that.

Bastian

-- 
	"That unit is a woman."
	"A mass of conflicting impulses."
		-- Spock and Nomad, "The Changeling", stardate 3541.9


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

On Fri, Sep 08, 2006 at 11:28:25PM +0200, Bastian Blank
wrote:
> On Fri, Sep 08, 2006 at 10:11:11PM +0100, Keir Fraser wrote:
> > That is only on the ''failsafe'' stackframe path. It
is okay to write zero
> > values there because we already wrote the real values lower down on
the
> > stack frame (at labels FLT7 to FLT10). In fact you probably
shouldn''t end up
> > taking the ''failsafe'' path anyway, so the code you
remove wouldn''t normally
> > be executed at all unless something in the guest is buggy.
> 
> Yep, saw that. So I have to recheck that.
It works correctly with the patch but not without. Can''t say why.

Bastian

-- 
You!  What PLANET is this!
		-- McCoy, "The City on the Edge of Forever", stardate 3134.0


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

On 8/9/06 23:04, "Bastian Blank" <bastian@waldi.eu.org> wrote:
>>> That is only on the ''failsafe'' stackframe path.
It is okay to write zero
>>> values there because we already wrote the real values lower down on
the
>>> stack frame (at labels FLT7 to FLT10). In fact you probably
shouldn''t end up
>>> taking the ''failsafe'' path anyway, so the code
you remove wouldn''t normally
>>> be executed at all unless something in the guest is buggy.
>> 
>> Yep, saw that. So I have to recheck that.
> 
> It works correctly with the patch but not without. Can''t say why.
That doesn''t seem to make sense. It''s probably worthwhile
working out why
you are taking that ''failsafe'' path through
create_bounce_frame. If you run
a debug build of Xen, do you get lines of the form ''Pre-exception:
xxxxxx ->
xxxxxx'' in ''xm dmesg'' or on the Xen emergency
console?

 -- Keir



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

>>> Bastian Blank <bastian@waldi.eu.org> 08.09.06 21:33
>>>
>On Fri, Sep 08, 2006 at 06:00:25PM +0100, Jan Beulich wrote:
>> If the vm86 code isn''t running with an i/o bitmap permitting
access (which
>> supposedly libint10 doesn''t do), all i/o related faults should
alread be
>> reflected back to the guest, shouldn''t they? If otoh there was
permission
>> granted from the kernel, then we shouldn''t assume that code is
prepared
>> to deal with gp faults from respective port accesses.
>
>Yes, but it does weird things, so I think there is something broken.
It would probably be good to first exactly understand what exactly is
happening...
>> However, the original patch would, if to be integrated, in my opinion
need
>> quite a bit of additional work - it should honor non-zero segment bases
>> generally, and it should do proper limit checking for non-flat
segments.
>
>Isn''t this already done by copy_from_user?
How can it, if it isn''t being passed a segment limit (and generally,
shouldn''t)?

Jan

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

On Sat, Sep 09, 2006 at 01:15:28AM +0100, Keir Fraser
wrote:
> That doesn''t seem to make sense. It''s probably worthwhile
working out why
> you are taking that ''failsafe'' path through
create_bounce_frame. If you run
> a debug build of Xen, do you get lines of the form ''Pre-exception:
xxxxxx ->
> xxxxxx'' in ''xm dmesg'' or on the Xen emergency
console?
Nope. In fact I''m unable to say why this happens. Is it possible that
you at least merge the non-failsafe part of the patch (i.e. the if
vm86_mode). It makes most things working.

Bastian

-- 
A princess should not be afraid -- not with a brave knight to protect her.
		-- McCoy, "Shore Leave", stardate 3025.3


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

On 21/9/06 19:35, "Bastian Blank" <bastian@waldi.eu.org> wrote:
> On Sat, Sep 09, 2006 at 01:15:28AM +0100, Keir Fraser wrote:
>> That doesn''t seem to make sense. It''s probably
worthwhile working out why
>> you are taking that ''failsafe'' path through
create_bounce_frame. If you run
>> a debug build of Xen, do you get lines of the form
''Pre-exception: xxxxxx ->
>> xxxxxx'' in ''xm dmesg'' or on the Xen
emergency console?
> 
> Nope. In fact I''m unable to say why this happens. Is it possible
that
> you at least merge the non-failsafe part of the patch (i.e. the if
> vm86_mode). It makes most things working.
I already applied your original patch (which adds cs<<4 to eip if in vm86
mode). It was obviously the right thing to do.

 -- Keir



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

On Thu, Sep 21, 2006 at 07:44:25PM +0100, Keir Fraser
wrote:
> I already applied your original patch (which adds cs<<4 to eip if in
vm86
> mode). It was obviously the right thing to do.
Ah, thanks.

Bastian

-- 
... The prejudices people feel about each other disappear when they get
to know each other.
		-- Kirk, "Elaan of Troyius", stardate 4372.5


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

* Keir Fraser (Keir.Fraser@cl.cam.ac.uk) wrote:
> On 21/9/06 19:35, "Bastian Blank" <bastian@waldi.eu.org>
wrote:
> 
> > On Sat, Sep 09, 2006 at 01:15:28AM +0100, Keir Fraser wrote:
> >> That doesn''t seem to make sense. It''s probably
worthwhile working out why
> >> you are taking that ''failsafe'' path through
create_bounce_frame. If you run
> >> a debug build of Xen, do you get lines of the form
''Pre-exception: xxxxxx ->
> >> xxxxxx'' in ''xm dmesg'' or on the Xen
emergency console?
> > 
> > Nope. In fact I''m unable to say why this happens. Is it
possible that
> > you at least merge the non-failsafe part of the patch (i.e. the if
> > vm86_mode). It makes most things working.
> 
> I already applied your original patch (which adds cs<<4 to eip if in
vm86
> mode). It was obviously the right thing to do.
This is breaking machines (with at least 845G) when running video
bios.  Appears it can''t set the video memory size appropriately,
and therefore X exits since it doesn''t have any modes that will fit
in the default 320k.  Both backing the change out and kicking back to
userspace using if(vm86_mode(regs)) goto fail;  allow X to run again.
I''ve tested various combos of patched and unpatched hv on 845, 945,
and 965 to find that backing out the patch breaks X on 945.  Bastian,
any further ideas on the failsafe path?

thanks,
-chris

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

>>> Chris Wright <chrisw@sous-sol.org> 02.11.06 12:53
>>>
>* Keir Fraser (Keir.Fraser@cl.cam.ac.uk) wrote:
>> On 21/9/06 19:35, "Bastian Blank"
<bastian@waldi.eu.org> wrote:
>> 
>> > On Sat, Sep 09, 2006 at 01:15:28AM +0100, Keir Fraser wrote:
>> >> That doesn''t seem to make sense. It''s
probably worthwhile working out why
>> >> you are taking that ''failsafe'' path through
create_bounce_frame. If you run
>> >> a debug build of Xen, do you get lines of the form
''Pre-exception: xxxxxx ->
>> >> xxxxxx'' in ''xm dmesg'' or on the Xen
emergency console?
>> > 
>> > Nope. In fact I''m unable to say why this happens. Is it
possible that
>> > you at least merge the non-failsafe part of the patch (i.e. the if
>> > vm86_mode). It makes most things working.
>> 
>> I already applied your original patch (which adds cs<<4 to eip if
in vm86
>> mode). It was obviously the right thing to do.
>
>This is breaking machines (with at least 845G) when running video
>bios.  Appears it can''t set the video memory size appropriately,
>and therefore X exits since it doesn''t have any modes that will fit
>in the default 320k.  Both backing the change out and kicking back to
>userspace using if(vm86_mode(regs)) goto fail;  allow X to run again.
>I''ve tested various combos of patched and unpatched hv on 845, 945,
>and 965 to find that backing out the patch breaks X on 945.  Bastian,
>any further ideas on the failsafe path?
Hmm, that would then perhaps also cause problems with the enhanced
emulation code I have pending in my 32on64 queue, unless the problem
is due to masked problems in the old code (ie when not adding the CS
base for vm86 mode emulation). Any chance you dug into why exactly
the more correct (but still incomplete) code doesn''t work anymore
(namely, what instruction(s) caused problems)?

Jan



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

* Jan Beulich (jbeulich@novell.com) wrote:
> Hmm, that would then perhaps also cause problems with the enhanced
> emulation code I have pending in my 32on64 queue, unless the problem
> is due to masked problems in the old code (ie when not adding the CS
> base for vm86 mode emulation). Any chance you dug into why exactly
> the more correct (but still incomplete) code doesn''t work anymore
> (namely, what instruction(s) caused problems)?
I haven''t figured out yet where the underlying problem is.  I had a
working
theory until i tested on the 945, so I still have some digging to do.

thanks,
-chris

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