On Wed, 7 May 2008, Bakul Shah wrote:
> On Wed, 07 May 2008 15:54:56 +1000 Bruce Evans
<brde optusnet.com.au> wrote:
>> On Tue, 6 May 2008, Bakul Shah wrote:
>>> See for instance
>>>
>>> http://docs.freebsd.org/cgi/getmsg.cg
i?fetch=100953+0+archive/2007/freebsd-
>> emulation/20070415.freebsd-emulation
>>>
>>> This seems to have not caused any problem in
practice. And
>>> any way taking out the message doesn't change
the essential
>>> behavior (the invariant is still broken) but it
can speed up
>>> your emulation considerably.
>>
>> I should have changed it to a panic long ago. That
would give the correct
>> number of messages (1)  .
>
> Too late now for you to go fundamentalist
>
>> i386 still doesn't even print a message (perhaps it
never did). The
>> bug would probably never have existed in any
FreeBSD version of kqemu if
>> i386 had had enough invariant checking.
>
> It does (in isa/npx.c) and I've disabled it!
Not too late to go fundamentalist for that . I wrote
it correctly.
It paniced, but it was broken to a printf in rev.1.131 when
I wasn't
watching closely enough (though the log claims that I
reviewed the
patch, ISTR looking mainly at the parts in machdep.c).
The message in npx.c is actually about violation of an even
more
fundamental invariant -- the invariant that owning the FPU
includes
having the TS flag clear so that DNA traps cannot occur.
The bug in
kqemu seems to be mismanagement of the TS flag related to
this. I
forget if it is the host or the target TS flag that seems to
be mismanaged.
For the target, it would take a bug in the virtualization of
the TS flag
to break this invariant (assuming no related bugs in the
target kernel).
The message in amd64/machdep.c is about violation of the
invariant
that the kernel cannot cause DNA traps. Spurious DNA traps
in the
target kernel might be caused either by violation of the
previous
invariant (then we might get a spurious DNA trap as part of
non-broken
target kernel FPU handling, after we have properly acquired
ownership
of the FPU). Non-spurious but wrong DNA traps in the host
kernel might
be caused by not properly acquiring ownership of the FPU
before using
it. This bug would be easier to implement, but I now
remember more
of the previous discussion and doubt that it is the bug in
kqemu. It
was said that kqemu never uses the FPU directly (not even
for
fxsave/fxrstor?)
The second bug was implemented 3 years ago in the i386 linux
emulator
in rev.1.136 of linux_sysvec.c, but the bug is not detected
because
there is not even a printf about it in the i386 trap.c :-(
and it is
not a bug at the level of npx.c. The implementation is
simply to hack
on the FPU directly to set the control word. This is also a
layering
violation, but the API intended to be used for this
initialization
(npxinit()) is no longer used for this and no longer works
for this.
Direct hacking on the FPU works almost correctly here. It
mainly wastes
time by defeating the lazy reinitialization of the FPU on
exec.
> I seem to recall it is not just qemu but also some
ndis
> drivers that trigger this fpudna/npxdna message?
I think that is from just hacking on the FPU directly.
Windows drivers
could easily have a bug like that.
> Didn't
> someone (Attilio?) has ported dragonfly code to allow
FPU
> register use in kernel mode? Whatever happened to it?
I objected to it, and Attilio found better things to work
on. Parts
of it give a cleaner implementation of allowing controlled
use of the
FPU in the kernel, but even it didn't allow arbitrary use
(as might be
necessary for bad Windows drivers), and there are no known
useful uses
for the FPU/SSE in the kernel except on old systems:
- Copying through SSE2 can be up to twice as fast as copying
through
32-bit integer registers, but not-so-old systems in
64-bit mode can
use 64-bit integer registers. Also, it is hard to find a
real
application where the speedups possible from copying
through SSE2
affects more than the noise, since it takes very large
data for
copying running at multi-GB/S to take very long, but very
large data
is too large to cache and copying through SSE2 makes no
difference
in the uncached case.)
- some multimedia instructions might be useful in the
kernel. But it is
hard to find a useful use that executes enough of them to
justify a FPU
context switch just to use them.
Bruce
_______________________________________________
freebsd-emulationfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-emu
lation
To unsubscribe, send any mail to
"freebsd-emulation-unsubscribefreebsd.org"
|
On Thu, May 08, 2008 at 09:59:57PM +1000, Bruce Evans
wrote:
> On Wed, 7 May 2008, Bakul Shah wrote:
>
>> On Wed, 07 May 2008 15:54:56 +1000 Bruce Evans
<brdeoptusnet.com.au>
>> wrote:
>>> On Tue, 6 May 2008, Bakul Shah wrote:
>>>> See for instance
>>>>
>>>> http://docs.freebsd.org/cgi/getmsg.cg
i?fetch=100953+0+archive/2007/freebsd-
>>> emulation/20070415.freebsd-emulation
>>>>
>>>> This seems to have not caused any problem
in practice. And
>>>> any way taking out the message doesn't
change the essential
>>>> behavior (the invariant is still broken)
but it can speed up
>>>> your emulation considerably.
>>>
>>> I should have changed it to a panic long ago.
That would give the
>>> correct
>>> number of messages (1) .
>>
>> Too late now for you to go fundamentalist
>>
>>> i386 still doesn't even print a message
(perhaps it never did). The
>>> bug would probably never have existed in any
FreeBSD version of kqemu if
>>> i386 had had enough invariant checking.
>>
>> It does (in isa/npx.c) and I've disabled it!
>
> Not too late to go fundamentalist for that . I wrote
it correctly.
> It paniced, but it was broken to a printf in rev.1.131
when I wasn't
> watching closely enough (though the log claims that I
reviewed the
> patch, ISTR looking mainly at the parts in machdep.c).
>
> The message in npx.c is actually about violation of an
even more
> fundamental invariant -- the invariant that owning the
FPU includes
> having the TS flag clear so that DNA traps cannot
occur. The bug in
> kqemu seems to be mismanagement of the TS flag related
to this. I
> forget if it is the host or the target TS flag that
seems to be mismanaged.
> For the target, it would take a bug in the
virtualization of the TS flag
> to break this invariant (assuming no related bugs in
the target kernel).
>
Well the `fpcurthread == curthread' bug has been fixed quite
a while
ago already, or do you mean another one?
> The message in amd64/machdep.c is about violation of
the invariant
> that the kernel cannot cause DNA traps. Spurious DNA
traps in the
> target kernel might be caused either by violation of
the previous
> invariant (then we might get a spurious DNA trap as
part of non-broken
> target kernel FPU handling, after we have properly
acquired ownership
> of the FPU). Non-spurious but wrong DNA traps in the
host kernel might
> be caused by not properly acquiring ownership of the
FPU before using
> it. This bug would be easier to implement, but I now
remember more
> of the previous discussion and doubt that it is the bug
in kqemu. It
> was said that kqemu never uses the FPU directly (not
even for
> fxsave/fxrstor?)
>
Okay I _think_ I know a little more about this now...
kqemu itself
doesn't use the fpu, but the guest code it runs can, and in
that case the
DNA trap is just used for (host) lazy fpu context switching
like as if the
code was running in userland regularly. And I just tested
the following
patch that should get rid of the message by calling
fpudna/npxdna directly
(files/patch-fpucontext is the interesting part
Index: Makefile
============================================================
=======
RCS file: /home/pcvs/ports/emulators/kqemu-kmod/Makefile,v
retrieving revision 1.23
diff -u -p -r1.23 Makefile
--- Makefile 1 May 2008 13:29:16 -0000 1.23
+++ Makefile 9 May 2008 19:53:08 -0000
-7,7
+7,7
PORTNAME= kqemu
PORTVERSION= 1.3.0.p11
-PORTREVISION= 4
+PORTREVISION= 5
CATEGORIES= emulators kld
MASTER_SITES= http://fabrice.b
ellard.free.fr/qemu/
http://qemu.org/
Index: files/patch-tssworkaround
============================================================
=======
RCS file:
/home/pcvs/ports/emulators/kqemu-kmod/files/patch-tssworkaro
und,v
retrieving revision 1.1
diff -u -p -r1.1 patch-tssworkaround
--- files/patch-tssworkaround 1 May 2008 13:29:16 -0000 1.1
+++ files/patch-tssworkaround 9 May 2008 19:34:12 -0000
-1,8
+1,8
Index: kqemu-freebsd.c
- -33,6 +33,11
-
- #include <machine/vmparam.h>
- #include <machine/stdarg.h>
+ -38,6 +38,11
+ #else
+ #include <machine/npx.h>
+ #endif
+#ifdef __x86_64__
+#include <sys/pcpu.h>
+#include <machine/segments.h>
-11,13
+11,13 Index: kqemu-freebsd.c
#include "kqemu-kernel.h"
- -234,6 +239,19
+ -248,6 +253,19
va_end(ap);
}
+#ifdef __x86_64__
+/* called with interrupts disabled */
-+void CDECL kqemu_tss_workaround(void)
++void CDECL kqemu_tss_fixup(void)
+{
+ int gsel_tss = GSEL(GPROC0_SEL, SEL_KPL);
+
-49,7
+49,7 Index: common/kernel.c
+#ifdef __FreeBSD__
+#ifdef __x86_64__
+ spin_lock(&g->lock);
-+ kqemu_tss_workaround();
++ kqemu_tss_fixup();
+ spin_unlock(&g->lock);
+#endif
+#endif
-63,7
+63,7 Index: kqemu-kernel.h
+#ifdef __FreeBSD__
+#ifdef __x86_64__
-+void CDECL kqemu_tss_workaround(void);
++void CDECL kqemu_tss_fixup(void);
+#endif
+#endif
+
Index: files/patch-fpucontext
-0,0
+1,76
+Index: common/kernel.c
+ -1240,6 +1240,9
+ case MON_REQ_EXCEPTION:
+ exec_exception(s->arg0);
+ break;
++ case MON_REQ_LOADFPUCONTEXT:
++ kqemu_loadfpucontext(s->arg0);
++ break;
+ default:
+ kqemu_log("invalid mon request:
%dn", s->mon_req);
+ break;
+Index: common/kqemu_int.h
+ -523,6 +523,7
+ MON_REQ_LOCK_USER_PAGE,
+ MON_REQ_UNLOCK_USER_PAGE,
+ MON_REQ_EXCEPTION,
++ MON_REQ_LOADFPUCONTEXT,
+ } MonitorRequest;
+
+ #define INTERRUPT_ENTRY_SIZE 16
+Index: common/monitor.c
+ -1995,8 +1995,13
+ raise_exception_err(s, EXCP07_PREX, 0);
+ } else {
+ /* the host needs to restore the FPU state for us
*/
++#ifndef __FreeBSD__
+ s->mon_req = MON_REQ_EXCEPTION;
+ s->arg0 = 0x07;
++#else
++ s->mon_req = MON_REQ_LOADFPUCONTEXT;
++ s->arg0 = (unsigned long)s->cpu_state.cpl;
++#endif
+ monitor2kernel1(s);
+ }
+ }
+Index: kqemu-freebsd.c
+ -33,6 +33,11
+
+ #include <machine/vmparam.h>
+ #include <machine/stdarg.h>
++#ifdef __x86_64__
++#include <machine/fpu.h>
++#else
++#include <machine/npx.h>
++#endif
+
+ #include "kqemu-kernel.h"
+
+ -172,6 +177,15
+ {
+ }
+
++void CDECL kqemu_loadfpucontext(unsigned long cpl)
++{
++#ifdef __x86_64__
++ fpudna();
++#else
++ npxdna();
++#endif
++}
++
+ #if __FreeBSD_version < 500000
+ static int
+ curpriority_cmp(struct proc *p)
+Index: kqemu-kernel.h
+ -40,6 +40,10
+ void * CDECL kqemu_io_map(unsigned long page_index,
unsigned int size);
+ void CDECL kqemu_io_unmap(void *ptr, unsigned int size);
+
++#ifdef __FreeBSD__
++void CDECL kqemu_loadfpucontext(unsigned long cpl);
++#endif
++
+ int CDECL kqemu_schedule(void);
+
+ void CDECL kqemu_log(const char *fmt, ...);
_______________________________________________
freebsd-emulationfreebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-emu
lation
To unsubscribe, send any mail to
"freebsd-emulation-unsubscribefreebsd.org"
|
