Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

FPU save-restore hooks for vm86/native/cpuemu #1886

Open
wants to merge 13 commits into
base: devel
Choose a base branch
from
Open

FPU save-restore hooks for vm86/native/cpuemu #1886

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
02bafaa
DPMI: native: only copy to/from vm86_fpu_state on demand.
bartoldeman Jan 23, 2023
81df683
DPMI/cpuemu: save/restore FPU state when entering/leaving backend
bartoldeman Jan 24, 2023
5d8bebb
DPMI/VM86: save/restore FPU state when entering/leaving backend
bartoldeman Jan 24, 2023
746e059
FPU: call pic_untrigger(13) from port handler.
bartoldeman Jan 25, 2023
5984456
FPU: eliminate global vm86_fpu_state.
bartoldeman Jan 25, 2023
71f8c9e
FPU: convert tag word properly to fsave format.
bartoldeman Jan 25, 2023
9f6d52d
dpmi: make temporary fpstate static
bartoldeman Jan 25, 2023
f587481
FPU: use [backend_]{get,set}_fpu_state.
bartoldeman Jan 25, 2023
99071c3
FPU: Keep SSE state in backends even if it doesn't change.
bartoldeman Jan 27, 2023
8c33f6d
FPU: Reduce size of emu_fpstate to 288 bytes.
bartoldeman Jan 27, 2023
f9c3281
DPMI native: cache contents of FPU state instead of pointer.
bartoldeman Jan 27, 2023
3e3d231
Add fnclex to bios.S int75 (IRQ13, coprocessor exception) handler.
bartoldeman Jan 28, 2023
7aa8cf2
FPU: bios: move int 2 and fnclex before port i/o
bartoldeman Jan 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/base/bios/x86/bios.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,12 @@ INT75_OFF:
outb %al, $0xa0
outb %al, $0x20
int $2 /* Bochs does this; RBIL says: redirected to INT 02 */
iret /* by the BIOS, for compatibility with the PC */
/* by the BIOS, for compatibility with the PC */
fnclex /* Clear FP exceptions just in case a hooked
stsp marked this conversation as resolved.
Show resolved Hide resolved
handler hasn't already done so, so we won't
get stuck (in real mode IGNNE would prevent
further exceptions but we don't emulate that) */
iret
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied, thanks!


/* ----------------------------------------------------------------- */
.globl INT08_OFF
Expand Down
2 changes: 0 additions & 2 deletions src/base/core/emu.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,6 @@ int main(int argc, char **argv, char * const *envp)
set_kvm_memory_regions();

cpu_reset();
if (config.cpu_vm == CPUVM_KVM)
kvm_enter(0);
can_leavedos = 1;

while (!fatalerr && !config.exitearly) {
Expand Down
121 changes: 98 additions & 23 deletions src/base/emu-i386/cpu.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "emudpmi.h"
#include "priv.h"
#include "kvm.h"
#include "dnative.h"

#ifdef X86_EMULATOR
#include "simx86/syncpu.h"
Expand Down Expand Up @@ -92,8 +93,6 @@ static unsigned int TRs[2] =
};
#endif

/* fpu_state needs to be paragraph aligned for fxrstor/fxsave */
emu_fpstate vm86_fpu_state __attribute__((aligned(16)));
fenv_t dosemu_fenv;
static void fpu_reset(void);

Expand Down Expand Up @@ -239,13 +238,56 @@ static void fpu_init(void)
}
#endif

void get_fpu_state(emu_fpstate *fpstate)
{
int cpu_vm = in_dpmi_pm() ? config.cpu_vm_dpmi : config.cpu_vm;
switch(cpu_vm) {
case CPUVM_KVM:
kvm_get_fpu_state(fpstate);
break;
case CPUVM_NATIVE:
native_dpmi_get_fpu_state(fpstate);
break;
case CPUVM_EMU:
e_get_fpu_state(fpstate);
break;
#ifdef __i386__
case CPUVM_VM86:
true_vm86_get_fpu_state(fpstate);
break;
#endif
}
}

void set_fpu_state(const emu_fpstate *fpstate)
{
int cpu_vm = in_dpmi_pm() ? config.cpu_vm_dpmi : config.cpu_vm;
switch(cpu_vm) {
case CPUVM_KVM:
kvm_set_fpu_state(fpstate);
break;
case CPUVM_NATIVE:
native_dpmi_set_fpu_state(fpstate);
break;
case CPUVM_EMU:
e_set_fpu_state(fpstate);
break;
#ifdef __i386__
case CPUVM_VM86:
true_vm86_set_fpu_state(fpstate);
break;
#endif
}
}

static void fpu_reset(void)
{
vm86_fpu_state.cwd = 0x0040;
vm86_fpu_state.swd = 0;
vm86_fpu_state.ftw = 0x5555; //bochs
if (config.cpu_vm == CPUVM_KVM || config.cpu_vm_dpmi == CPUVM_KVM)
kvm_update_fpu();
emu_fpstate vm86_fpu_state;

memset(&vm86_fpu_state, 0, sizeof vm86_fpu_state);
vm86_fpu_state.cwd = 0x0040; //bochs
vm86_fpu_state.ftw = 0xff; //all valid (-> 0x5555 in fsave tag)
set_fpu_state(&vm86_fpu_state);
}

static Bit8u fpu_io_read(ioport_t port)
Expand All @@ -257,8 +299,7 @@ static void fpu_io_write(ioport_t port, Bit8u val)
{
switch (port) {
case 0xf0:
/* not sure about this */
vm86_fpu_state.swd &= ~0x8000;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But wasn't this also from bochs?
Untrigger of an IRQ is usually happening
as a result of clearing some status bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No this wasn't from Bochs. I checked the Bochs source code, and Bochs does the equivalent pic_untrigger(13).
Resetting this bit is wrong, it's not related to IRQ13, rather it dates back to the time when the CPU and FPU were seperate and CPUs needed fwait which would wait until this (FPU busy) bit was cleared by the FPU.

Copy link
Member

@stsp stsp Jan 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK but do you want to say that
just untrigger the irq is good idea?
I mean, only 2 possibilities do exist:

  • IRQ is edge-delivered (not to confuse
    with edge-triggered), in which case
    my code was correct and doesn't need
    this patch.
  • IRQ is level-delivered (still edge-triggered),
    in which case it gets lowered only together
    with some state bit. Maybe that state bit is
    fully internal and is not exported to fxsave,
    but it should be there if you want to change
    the IRQ to level-delivery.

So in bochs is it level-delivered?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it just moves the untrigger from 3 other places in the code to the port handler. So the untrigger happens no matter what since IRQ13 invokes INT75, which does the port i/o, it just happens a bit later with this patch.

Or do you mean that we need something similar to port60_ready in src/base/dev/misc/8042.c.

There is some documentation here https://xem.github.io/minix86/manual/intel-x86-and-64-manual-vol1/o_7281d5ea06a5b67a-427.html
Bochs does this:
https://github.com/bochs-emu/Bochs/blob/master/bochs/iodev/extfpuirq.cc
but they don't handle IGNNE either.

This comment was marked as outdated.

Sign in to view

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall professional).

OK still no one uses dosemu there, and overall, what future do you see for that kind of freedos? Go to virtualbox? I can only think of the future of a 64bit clang-buildable gdb-debuggable freedos, not more, not less. :)

I do use dosemu2 with the latest FreeDOS kernels. I also have contributed to the kernel, SYS, boot loaders, and SHARE.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do use dosemu2 with the latest FreeDOS kernels.

Unless I am mistaken, you have your
own DOS kernel too. So your use of a
freedos kernel is probably not too frequent.
Though if you know why you use freedos
kernel and not fdpp, please speak up. :)
If there is anything better done in freedos,
fdpp can do that better yet. So tell me what
is it, and get it in fdpp after a while.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do use dosemu2 with the latest FreeDOS kernels.

Unless I am mistaken, you have your own DOS kernel too.

No, actually, the RxDOS kernel is not nearly usable generally. For dosemu2 the biggest missing piece is the lack of FS redirector support. I rarely use it and haven't progressed on it for years. My debugger(s) and TSRs are the only programs that I currently develop.

So your use of a freedos kernel is probably not too frequent.

Nearly all developing, testing, and building (including decompress-testing) of DOS software on our server and local Linux desktop box is done using a recent FreeDOS kernel, either in dosemu2 or qemu (or, rarely, Virtual Box 6.x). The DOS desktop box still boots MS-DOS 7.10 by default though, but I rarely use it. (The HP 95LX devices use their own MS-DOS 3.20, and I do not know how viable it is to run another kernel on that.)

Though if you know why you use freedos kernel and not fdpp, please speak up. :) If there is anything better done in freedos, fdpp can do that better yet. So tell me what is it, and get it in fdpp after a while.

Our kernel is portable among PC-compatible machines =) And it is easier for me to modify and debug, though obviously if you babysat me I could probably learn how to do the same or better for fdpp.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are only a few tricks needed
to know when debugging fdpp.
Namely, you need to add a local.mak
file into a build dir and write
EXTRA_DEBUG = 1 there, after which
you'll get an access to most freedos
variables with gdb.
If some variable is still missing, you
can get an info about it with a double
underscode, eg:

(gdb) p break_ena
No symbol "break_ena" in current context.
(gdb) p /x __break_ena
$2 = {sym = {<FarPtrBase<char>> = {ptr = {off = 0x337, seg = 0xfa96}}, 
    obj = std::shared_ptr<class ObjIf> (empty) = {get() = 0x0}, nonnull = 0x0}}
(gdb) p lowmem_base[(0xfa96<<4)+0x337]
$3 = 1 '\001'

So you look up the address with 2
underscores and then look up the
value via lowmem_base.
IIRC Andrew wanted to add a gdb
macros for this all, but that was on
a very early development stages so
I wasn't yet interested.
Andrew, maybe you still want to add
the gdb instrumentation for fdpp?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Bart.
I cherry-picked that patch after the
fnclex patches, as in that case it makes
sense. I also added the check if an FPU
irq can be injected, as we discussed
above.

pic_untrigger(13); /* done by default via int75 handler in bios.S */
break;
case 0xf1:
fpu_reset();
Expand Down Expand Up @@ -292,7 +333,6 @@ void cpu_setup(void)
io_dev.handler_name = "Math Coprocessor";
port_register_handler(io_dev, 0);

savefpstate(vm86_fpu_state);
#ifdef FE_NOMASK_ENV
feenableexcept(FE_DIVBYZERO | FE_OVERFLOW);
#endif
Expand Down Expand Up @@ -368,24 +408,59 @@ void cpu_setup(void)
#endif
}

#define FP_EXP_TAG_VALID 0
#define FP_EXP_TAG_ZERO 1
#define FP_EXP_TAG_SPECIAL 2
#define FP_EXP_TAG_EMPTY 3

static inline uint32_t twd_fxsr_to_i387(const struct emu_fpstate *fxsave)
{
const struct emu_fpxreg *st;
uint32_t tos = (fxsave->swd >> 11) & 7;
uint32_t twd = (unsigned long) fxsave->ftw;
uint32_t tag;
uint32_t ret = 0xffff0000u;
int i;

for (i = 0; i < 8; i++, twd >>= 1) {
if (twd & 0x1) {
st = &fxsave->st[(i - tos) & 7];

switch (st->exponent & 0x7fff) {
case 0x7fff:
tag = FP_EXP_TAG_SPECIAL;
break;
case 0x0000:
if (!st->significand[0] &&
!st->significand[1] &&
!st->significand[2] &&
!st->significand[3])
tag = FP_EXP_TAG_ZERO;
else
tag = FP_EXP_TAG_SPECIAL;
break;
default:
if (st->significand[3] & 0x8000)
tag = FP_EXP_TAG_VALID;
else
tag = FP_EXP_TAG_SPECIAL;
break;
}
} else {
tag = FP_EXP_TAG_EMPTY;
}
ret |= tag << (2 * i);
}
return ret;
}

void fxsave_to_fsave(const struct emu_fpxstate *fxsave, struct emu_fsave *fptr)
void fxsave_to_fsave(const struct emu_fpstate *fxsave, struct emu_fsave *fptr)
{
unsigned int tmp;
int i;

fptr->cw = fxsave->cwd | 0xffff0000u;
fptr->sw = fxsave->swd | 0xffff0000u;
/* Expand the valid bits */
tmp = fxsave->ftw; /* 00000000VVVVVVVV */
tmp = (tmp | (tmp << 4)) & 0x0f0f; /* 0000VVVV0000VVVV */
tmp = (tmp | (tmp << 2)) & 0x3333; /* 00VV00VV00VV00VV */
tmp = (tmp | (tmp << 1)) & 0x5555; /* 0V0V0V0V0V0V0V0V */
/* Transform each bit from 1 to 00 (valid) or 0 to 11 (empty).
The kernel will convert it back, so no need to worry about zero
and special states 01 and 10. */
tmp = ~(tmp | (tmp << 1));
fptr->tag = tmp | 0xffff0000u;
fptr->tag = twd_fxsr_to_i387(fxsave);
fptr->ipoff = fxsave->fip;
fptr->cssel = (uint16_t)fxsave->fcs | ((uint32_t)fxsave->fop << 16);
fptr->dataoff = fxsave->fdp;
Expand Down Expand Up @@ -414,7 +489,7 @@ static unsigned short twd_i387_to_fxsr(unsigned short twd)

/* NOTE: this function does NOT memset the "unused" fxsave fields.
* We preserve the previous fxsave context. */
void fsave_to_fxsave(const struct emu_fsave *fptr, struct emu_fpxstate *fxsave)
void fsave_to_fxsave(const struct emu_fsave *fptr, struct emu_fpstate *fxsave)

{
int i;
Expand Down
30 changes: 27 additions & 3 deletions src/base/emu-i386/do_vm86.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ int vm86_fault(unsigned trapno, unsigned err, dosaddr_t cr2)
return 0;

case 0x10: /* coprocessor error */
pic_untrigger(13);
pic_request(13); /* this is the 386 way of signalling this */
return 0;

Expand Down Expand Up @@ -441,12 +440,20 @@ static int handle_GP_hlt(void)
}

#ifdef __i386__
/* avoid converting to and from fxsave when staying in vm86 */
static struct emu_fsave true_vm86_fsave;
/* this needs to be paragraph aligned for fxrstor/fxsave */
static struct emu_fpxstate true_vm86_fxsave __attribute__((aligned(16)));

static int true_vm86(union vm86_union *x)
{
int ret;
uint32_t old_flags = REG(eflags);

loadfpstate(vm86_fpu_state);
if (config.cpufxsr)
loadfxsave(true_vm86_fxsave);
else
loadfsave(true_vm86_fsave);
again:
#if 0
ret = vm86(&x->vm86ps);
Expand All @@ -464,7 +471,10 @@ static int true_vm86(union vm86_union *x)
* TODO: check kernel version */
REG(eflags) |= (old_flags & VIP);

savefpstate(vm86_fpu_state);
if (config.cpufxsr)
savefxsave(true_vm86_fxsave);
else
savefsave(true_vm86_fsave);
/* there is no real need to save and restore the FPU state of the
emulator itself: savefpstate (fnsave) also resets the current FPU
state using fninit; fesetenv then restores trapping of division by
Expand All @@ -474,6 +484,20 @@ static int true_vm86(union vm86_union *x)
fesetenv(&dosemu_fenv);
return ret;
}

void true_vm86_set_fpu_state(const emu_fpstate *fpstate)
{
true_vm86_fxsave.emu_fpstate = *fpstate;
if (!config.cpufxsr)
fxsave_to_fsave(fpstate, &true_vm86_fsave);
}

void true_vm86_get_fpu_state(emu_fpstate *fpstate)
{
*fpstate = true_vm86_fxsave.emu_fpstate;
if (!config.cpufxsr)
fsave_to_fxsave(&true_vm86_fsave, fpstate);
}
#endif

static int do_vm86(union vm86_union *x)
Expand Down
14 changes: 4 additions & 10 deletions src/base/emu-i386/kvm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,33 +783,28 @@ static void set_ldt_seg(struct kvm_segment *seg, unsigned selector)
seg->unusable = !desc->present;
}

void kvm_update_fpu(void)
void kvm_set_fpu_state(const emu_fpstate *fpstate)
{
struct kvm_xsave fpu = {};
int ret;

memcpy(fpu.region, &vm86_fpu_state, sizeof(vm86_fpu_state));
memcpy(fpu.region, fpstate, sizeof(*fpstate));
ret = ioctl(vcpufd, KVM_SET_XSAVE, &fpu);
if (ret == -1) {
perror("KVM: KVM_SET_XSAVE");
leavedos_main(99);
}
}

void kvm_enter(int pm)
{
kvm_update_fpu();
}

void kvm_leave(int pm)
void kvm_get_fpu_state(emu_fpstate *fpstate)
{
struct kvm_xsave fpu;
int ret = ioctl(vcpufd, KVM_GET_XSAVE, &fpu);
if (ret == -1) {
perror("KVM: KVM_GET_XSAVE");
leavedos_main(99);
}
memcpy(&vm86_fpu_state, fpu.region, sizeof(vm86_fpu_state));
memcpy(fpstate, fpu.region, sizeof(*fpstate));
}

static int kvm_post_run(struct vm86_regs *regs, struct kvm_regs *kregs)
Expand Down Expand Up @@ -1166,7 +1161,6 @@ int kvm_dpmi(cpuctx_t *scp)
print_exception_info(scp);
#endif
dbug_printf("coprocessor exception, calling IRQ13\n");
pic_untrigger(13);
pic_request(13);
ret = DPMI_RET_DOSEMU;
} else if (_trapno == 0x0e &&
Expand Down
2 changes: 2 additions & 0 deletions src/base/emu-i386/simx86/codegen.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,8 @@ extern unsigned int (*CloseAndExec)(unsigned int PC, int mode);
void EndGen(void);
extern void fp87_set_rounding(void);
extern void fp87_save_except(void);
extern void fp87_save_fpstate(emu_fpstate *);
extern void fp87_load_fpstate(const emu_fpstate *);
//
extern unsigned char InterOps[];
extern char RmIsReg[];
Expand Down
28 changes: 24 additions & 4 deletions src/base/emu-i386/simx86/cpu-emu.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ static void Reg2Cpu (int mode)
trans_addr = LONG_CS + TheCPU.eip;

/* FPU state is loaded later on demand for JIT, not used for simulator */
TheCPU.fpstate = &vm86_fpu_state;
TheCPU.fpstate = &TheCPU._fpstate;
if (debug_level('e')>1) {
if (debug_level('e')==9) e_printf("Reg2Cpu< vm86=%08x dpm=%08x emu=%08x\n%s\n",
REG(eflags),get_FLAGS(TheCPU.eflags),TheCPU.eflags,
Expand Down Expand Up @@ -571,7 +571,7 @@ void Cpu2Reg (void)

if (TheCPU.fpstate == NULL) {
if (!CONFIG_CPUSIM)
savefpstate(vm86_fpu_state);
savefpstate(TheCPU._fpstate);
else
fp87_save_except();
fesetenv(&dosemu_fenv);
Expand All @@ -581,6 +581,26 @@ void Cpu2Reg (void)
REG(eflags),get_FLAGS(TheCPU.eflags),TheCPU.eflags);
}

void e_set_fpu_state(const emu_fpstate *fpstate)
{
TheCPU._fpstate.emu_fpstate = *fpstate;
if (CONFIG_CPUSIM)
fp87_load_fpstate(fpstate);
else {
// unmasked exception settings are emulated
TheCPU.fpuc = fpstate->cwd;
TheCPU._fpstate.emu_fpstate.cwd |= 0x3f;
}
}

void e_get_fpu_state(emu_fpstate *fpstate)
{
*fpstate = TheCPU._fpstate.emu_fpstate;
if (CONFIG_CPUSIM)
fp87_save_fpstate(fpstate);
else
fpstate->cwd = (fpstate->cwd & ~0x3f) | (TheCPU.fpuc & 0x3f);
}

/* ======================================================================= */

Expand Down Expand Up @@ -610,7 +630,7 @@ static void Scp2Cpu (cpuctx_t *scp)
TheCPU.cr2 = _cr2;
TheCPU.df_increments = (TheCPU.eflags&DF)?0xfcfeff:0x040201;

TheCPU.fpstate = &vm86_fpu_state;
TheCPU.fpstate = &TheCPU._fpstate;
}

/*
Expand Down Expand Up @@ -651,7 +671,7 @@ static void Cpu2Scp (cpuctx_t *scp, int trapno)
if (!TheCPU.err) _err = 0; //???
if (TheCPU.fpstate == NULL) {
if (!CONFIG_CPUSIM)
savefpstate(vm86_fpu_state);
savefpstate(TheCPU._fpstate);
else
fp87_save_except();
/* there is no real need to save and restore the FPU state of the
Expand Down
Loading