Stircrazy69 / pcsx2

Automatically exported from code.google.com/p/pcsx2
0 stars 0 forks source link

Coproccessor timer interrupt handling isn't working. #482

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
I was trying to figure out how to initiate a CPU timer handler using the
CP0 count/compare registers. I finally got it working on a real ps2, but
that was after trying various ways to do it on PCSX2 :P. I learned a lot
about exception/interrupt handling, anyway.

1) Did the game ever work correctly (i.e. not have this problem) on the
Official PCSX2 build or an earlier version of PCSX2 playground?
(If so, please specify the latest pcsx2-playground or Official revision
that last worked.)

No idea if it ever worked correctly. Homebrew related.

2) What steps will reproduce the problem?
 1. Set CP0's compare register to count + interval.
 2. Unmask IM7 in CP0 status register.
 3. PCSX2 doesn't clear the timer interrupt after setting compare register.

3) What exactly happens when you experience this issue (listing any console
errors or screen output you recieve)?

 timr intr: 7ec1d30, 7ec1d2a
 timr intr: 7ec1d48, 7ec1d2a
 timr intr: 7ec1d55, 7ec1d2a
 timr intr: 7ec1d61, 7ec1d2a
 timr intr: 7ec1d6e, 7ec1d2a

The 7ec1d2a is the initial count+tick_delta. The tick_delta was set to
tickrate/10 or 0.1 seconds.

4) What version of PCSX2 are you using? On what operating system?

Revision 1972 on Archlinux using legacy gui. The plugins are revision 2008.

5) Please provide any additional information below.

I'm also getting tlb miss exceptions on PCSX2 when trying to get the
address of the vector interrupt handlers. I'm not sure if it works on a
real PS2. I found out when trying to replace the timer vector interrupt
handler with my own, heh.

6) Attachments.
This is the code related to getting the address of a vector interrupt handler.

From ps2sdk/ee/kernel/getkernel.c AFL 2.0:
void *GetInterruptHandler(int intr_cause_no)
{
    u32 oldintr, oldop;
    u32 addr;
    u16 lo16, hi16;

    // make sure intr_cause_no is between 0 and 7
    if((intr_cause_no < 0) || (intr_cause_no > 7))
    {
        return(NULL);
    }

    // get address of the syscall "SetVInterruptHandler"
    addr = (u32) GetSyscallHandler(15);

    // suspend interrupts and enter "kernel" operating mode.
    oldintr = DIntr();
    oldop = ee_set_opmode(0);

    // harvest the address of the corresponding exception handler table.
    lo16 = ((vu32 *) addr)[0x10 / 4];
    hi16 = ((vu32 *) addr)[0x1C / 4];

    addr = ((u32 *) ((u32) (hi16 << 16) | lo16))[intr_cause_no];

    // return to the old operating mode and resume interrupts.
    ee_set_opmode(oldop);
    if(oldintr) { EIntr(); }

    return (void *) addr;
}

Original issue reported on code.google.com by ragnarok2040@gmail.com on 20 Nov 2009 at 3:06

GoogleCodeExporter commented 9 years ago
Thanks for the info.  Typically the CP0 TIMR register is intended for debugging 
tools 
only, and very few games use it at all (only a couple known cases, and they 
only use 
it at startup for simple incremental instruction tests, from which the results 
are 
subsequently ignored anyway  -- so any dummy value suffices).

So yeah, it only ever got a partial implementation.  Even when I fix this part 
of it, 
the timer will be woefully inaccurate in the recompilers until it gets a proper 
fix 
(partly being put off because it could incur some measurable speed hit if not 
implemented carefully).

For the record, I am interested in continuing support for homebrew stuff, and I 
welcome additional issues that help document and improve some of the areas of 
PS2 
emulation that might be little used by games.  But be warned, most of those 
features 
are currently poorly implemented or not implemented at all.

For example, PCSX2 currently *only* supports kernel mode.  So the code you 
posted 
above very likely won't behave as intended if you're using anything other than 
kernel 
mode in your samples.  PCSX2 also has limited support for the TLB.  Namely the 
TLBMiss exceptions are not yet dispatched properly, which makes it hard for 
apps to 
refill it. ;)

Again, no known PS2 game uses either User/Supervisor modes or TLBs.  So there 
hasn't 
been a lot of priority in trying to implement them.  But it *is* something I'd 
like 
to see get done anyway. ;)

Original comment by Jake.Stine on 21 Nov 2009 at 7:23

GoogleCodeExporter commented 9 years ago
Ok, thanks for the reply, :D. I was trying to figure out a way to implement 
ps2sdk's
libc time/unistd functions without hogging both EE timers. I think I'll go a new
direction and try a small inline asm loop for microsecond delays.

All ps2 games, I think, use user mode or a mix of user/kernel mode, but since 
kernel
mode's kseg0 and kseg1 are user mode addresses, it doesn't matter, heh.

It looks like PCSX2 currently quits out of the emulation and throws a real 
exception
for the emulated exception. Have you noticed if ps2 game developers took 
advantage of
that and forced an exception? As long as they knew what was supposed to happen, 
they
could bypass it using a handler and continue on a real ps2 but cause havoc on 
PCSX2.
I actually tested that by purposefully accessing a NULL pointer.

I'll see if I can't get exception handling working. I'm working on a dash that's
going to handle exceptions, where it displays the error, saves it to a file, 
then
restarts the dash thus allowing for applications to fail gracefully. It'd be a 
lot
easier debugging it on PCSX2 than on the PS2, simply for the fact that I don't 
have
to cross the room when ps2link freezes.

Original comment by ragnarok2040@gmail.com on 21 Nov 2009 at 4:37

GoogleCodeExporter commented 9 years ago
>> All ps2 games, I think, use user mode or a mix of user/kernel mode, but 
since 
kernel
mode's kseg0 and kseg1 are user mode addresses, it doesn't matter, heh.

I can double check when I get home, but I'm 99% sure all games use Kernel Mode. 

Until 0.9.6, PCSX2 didn't even correctly support exception vectoring in User 
mode, 
which offsets to a different exception vector and sets exception status flags a 
wee 
bit differently. The PSP on the other hand has a real multitasking kernel, and 
actually runs games in user mode afaik.

>> Have you noticed if ps2 game developers took advantage of that and forced an 
exception?

Heh, nah.  Believe me there would be far easier ways for a developer to break 
PCSX2's 
ability to emulate their game than utilizing a TLB Miss.  Although a full-on 
TLB Miss 
/ Refill might be one of the more long-term effective routes, since 
implementing 
proper and efficient TLB refilling is one of the more complicated aspects of 
MIPS cpu 
emulation.

(well complicated if you want it done efficiently in the scope of a recompiler 
-- the 
interpreter would be able to handle it without too much complication).

Notes on Exceptions:

 * I implemented the C++ Exception model initially as a pre-cursor to a more advanced 
design I have in mind for recompiler-supported Addressing exceptions (Address 
Error, 
TLB Miss, Bus Error).

 * For it to work, however, the recompiler needs significant modifications that would 
likely come with a rewrite.  Basically the recompiler needs to create a lookup 
table 
of all recompiled instructions that can throw exceptions, and in that table 
store: 
const and allocated register states.  It sounds kinda simple, but the current 
rec 
design makes nothing simple ;)

 * This is needed because the recompiler optimizes code in a way that doesn't 
typically retain per-instruction cpu states.  ie, Register caching.

 * At that point the C++ exception would restore the cpuRegs state based on the table 
lookup, execute cpuException (which vectors the VM to the MIPS exception 
handler) and 
then it would resume execution of the VM.

 * Finally, the COP0's TLB Refill code needs verified (it's largely untested) and the 
recompiler code caches need to be flushed for every TLB modification (again due 
to 
internal const-propagated optimizations that unmap const-style TLB lookups at 
compilation time).

 * I think I set the interpreters to use the C++ exceptions at the time, thinking a 
unified solution would be cleaner -- but really they shouldn't.  Interps should 
just 
call cpuException directly, and then just interpret the vectored instruction.  
(ie, a 
helluva lot simpler ;)

Original comment by Jake.Stine on 22 Nov 2009 at 10:16

GoogleCodeExporter commented 9 years ago
I noticed that the exception code was there, but it wasn't being called by 
anything.
There actually seems to be a couple of versions of the same code. I got 
ps2link's
exception handler working in about an hour after playing around a bit. It only 
works
for tlb miss exceptions, though. It even has the correct addr stored in EPC. 
This is
using the interpreter.

Error output:
vtlb miss : addr 0x0, mode 1 [write] //Store to NULL
cpuTlbMiss pc:100828, cycl:317420c, addr: 0, status=70030c11, code=c
MFC0 Breakpoint debug Registers code = 0
read/write allocate memory 4000 //ps2link getting temp stack space for handler

           EE Exception handler: TLB store exception

      Cause 0000000C  BadVAddr 00000000  Status 70030C13  EPC 00100824

zero: 00000000000000000000000000000000   t8: 0000000000000000FFFFFFFF800212C0
  at: 0000000000000000FFFFFFFF80010034   t9: 00000000000000000000000000019600
  v0: 00000000000000000000000000000004   s0: 00000000000000000000000000090290
  v1: 00000000000000000000000000000038   s1: 00000000000000000000000000000001
  a0: 0000000000000000000000000000000E   s2: 00000000000000000000000000000000
  a1: 000000000000000000000000001004D4   s3: 00000000000000000000000000000000
  a2: 0000000000000000FFFFFFFF80018F68   s4: 00000000000000000000000000090288
  a3: 0000000000000000FFFFFFFFFFFFFFFF   s5: 00000000000000000000000000000000
  t0: 00000000000000000000000000000001   s6: 00000000000000000000000000000000
  t1: 00000000000000000000000000000000   s7: 00000000000000000000000000000000
  t2: 00000000000000000000000000000040   k0: 00000000000000000000000000000000
  t3: 0000000000000000FFFFFFFFA00217C0   k1: 00000000000000000000000000000000
  t4: 00000000000000000000000020093D40   gp: 0000000000000000000000000010FA70
  t5: 0000000000000000FFFFFFFFA0000000   sp: 00000000000000000000000001FFED50
  t6: 00000000000000000000000000000004   fp: 00000000000000000000000000000000
  t7: 0000000000000000000000000000001F   ra: 00000000000000000000000000100820

Line 237 of vtlb.cpp
static __forceinline void vtlb_Miss(u32 addr,u32 mode)
{
    Console::Error( "vtlb miss : addr 0x%X, mode %d [%s]", params addr, mode,
_getModeStr(mode) );
    //verify(false);
    (mode == 0) ? cpuTlbMissR(addr,cpuRegs.branch) : cpuTlbMissW(addr,cpuRegs.branch);
    //throw R5900Exception::TLBMiss( addr, !!mode );
}

I tested the recompiler and this was all I got:
vtlb miss : addr 0x0, mode 1 [write]
cpuTlbMiss pc:100820, cycl:3171ba1, addr: 0, status=70030c11, code=c

Is there any way to enable printf output when the recompiler is enabled?

Original comment by ragnarok2040@gmail.com on 22 Nov 2009 at 12:58

GoogleCodeExporter commented 9 years ago
Actually, I'm not sure what's causing that read/write allocation.

Original comment by ragnarok2040@gmail.com on 22 Nov 2009 at 1:32

GoogleCodeExporter commented 9 years ago
It'll never work in the recompiler that way, so don't even bother.  The 
recompiler 
runs instructions in batches, so even if you set the PC, it won't be "realized" 
until 
the next branch instruction (which is the only time the recompiler can vector 
to 
exceptions).  This is one of the primary speedups of recompiled MIPS machine 
language.  We remove some 85-98% of the overhead of checking for 
exceptions/events 
and PC changes by only checking them after branching instructions.

So stick with the interps for now, because adding support to the recs for them 
will 
require significant code modifications.  Still, if we can have working TLB 
exceptions 
in the ints that's still a great thing, and will make implementing them into 
the recs 
later on a bit easier too. :)

Original comment by Jake.Stine on 23 Nov 2009 at 4:24

GoogleCodeExporter commented 9 years ago
>> The recompiler  runs instructions in batches, so even if you set the PC, it 
won't be "realized" 
until the next branch instruction (which is the only time the recompiler can 
vector to exceptions).

... To add to that: the recompiler will override the TLB Miss PC vector with 
it's own optimize PC 
address determined by the branching instruction.  That's why it never exception 
vectors or printfs 
(PS2 printf in fact works fine from the recompiler).  There's a way around that 
of course, but it 
would require recompiler code modifications, etc.

So yeah just stick with interps for now.  If you have code patches for 
improving TLB Miss and Refill 
exception handling, you can post them here.

Original comment by Jake.Stine on 23 Nov 2009 at 10:18

GoogleCodeExporter commented 9 years ago
Alright, heh, I think I understand what you have in mind, so I'll leave the
exceptions alone, :D. I'm slowly feeling my way to getting the CP0 timer 
interrupt
working. 

I have my timer interrupt handler working now, by fixing the test to see if an
exception was pending, and modifying MTC0() to test if Compare was being 
written to,
which clears the IP7 bit in the Status register. It's not the proper fix and I'm
getting weird behavior, though. I need to look at it a bit more :D.

Original comment by ragnarok2040@gmail.com on 24 Nov 2009 at 9:43

GoogleCodeExporter commented 9 years ago
It seems printf() that I was using to print out the count and tick status was
crashing on PCSX2 since it was being called every loop. After putting in a 
check to
print only if __ticks had changed, it started working fine. I had to delay it 
with
the timer :D.

The default interrupt exception handler for IP7 seems to set the count to 0, 
prior to
calling my interrupt handler which accounts for the weird behavior I was 
seeing. It
doesn't matter, since I just use whatever's in Count + interval to set the 
Compare
register.

timr intr: e10016, e1000d
MTC0 Breakpoint count Registers code = 0
MTC0 Breakpoint compare Registers code = e1000d
MTC0 Breakpoint compare Registers code = e1000d
ticks = 1884

I changed MFC0()'s case 9:
        case 9:
        {
            cpuRegs.CP0.n.Count += cpuRegs.cycle-s_iLastCOP0Cycle;
            s_iLastCOP0Cycle = cpuRegs.cycle;
            if( !_Rt_ ) break;
        }

I added this case for the Compare register in MTC0() in COP0.cpp:
        case 11: 
            cpuRegs.CP0.r[11] = cpuRegs.GPR.r[_Rt_].UL[0];
            cpuRegs.CP0.r[13] = cpuRegs.CP0.r[13] & ~0x8000; 
        break;

It clears the IP7 bit from the Cause register. If it's already clear, no harm 
done :D.

Here's the relevant code for recMTC0()
if( GPR_IS_CONST1(_Rt_) )
{
...
            case 11:
                MOV32MtoR(EAX, (uptr)&cpuRegs.CP0.r[ 13 ]);
                AND32ItoR(EAX, ~0x8000);
                MOV32RtoM((uptr)&cpuRegs.CP0.r[13],EAX);
                MOV32ItoM((uptr)&cpuRegs.CP0.r[_Rd_], g_cpuConstRegs[_Rt_].UL[0]);
            break;
...
else
{
...
            case 11:
                MOV32MtoR(EAX, (uptr)&cpuRegs.CP0.r[ 13 ]);
                AND32ItoR(EAX, ~0x8000);
                MOV32RtoM((uptr)&cpuRegs.CP0.r[13],EAX);
                _eeMoveGPRtoM((uptr)&cpuRegs.CP0.r[_Rd_], _Rt_);
            break;
...
}

I also changed the test _cpuTestTIMR() in R5900.c, as it was resulting in an 
infinite
loop of producing exceptions:
    if ( (cpuRegs.CP0.n.Status.val & 0x8000) &&
        cpuRegs.CP0.n.Count >= cpuRegs.CP0.n.Compare && !(cpuRegs.CP0.n.Cause & 0x8000))
    {
        //Console::Status("timr intr: %x, %x", params cpuRegs.CP0.n.Count,
cpuRegs.CP0.n.Compare);
        cpuException(0x8000, cpuRegs.branch); //IP7 & 0
    }

Now it checks if the IP7 bit is enabled in Cause, if it is, then the exception 
has
already been generated.

There seems to be quite a few redundant calls to _cpuTestTIMR() in R5900.c, but 
I
left that alone.

It's working fine for me in both the recompiler and interpreter. For some 
reason, I'm
getting a ~0.5 fps boost when I have the timer running using the recompiler,
24.0~24.7 vs. 23.5-24.0, which is kind of weird.

Original comment by ragnarok2040@gmail.com on 25 Nov 2009 at 4:52

GoogleCodeExporter commented 9 years ago
MFC0's current 'case 9' implementation was a hack fix specifically designed to 
fix a 
couple games that use the TIMR during boot up/post.  Removing the code and 
replacing 
it with what you have above may cause them to cease to boot.

Basically, what happens is that the recompiler is quite inexact in its 
instruction / 
cycle counting (as noted before, it runs instructions in batches), and because 
the 
EE's superscalar it's 'estimated' cycle count for instructions is often less 
than 1.  
A small enough register branch loop was possible to end up rounding toward 
zero, and 
causing a zero-cycle increment.  That led to the COP0 case 9 test spinning 
forever as 
the game waited for the TIMR counter to change, and it never did.

Now the better fix is to force recompiled blocks to always be at least one 
cycle in 
length (which I think is true now, since some months ago), but since we 
neglected to 
document which specific titles were broken, it's hard to test to be sure that 
fix is 
working as expected. >_<  (and that's my fault).

The reason for redundant calls to cpuTestTIMR is that it doesn't have a proper 
event 
scheduling mechanism embedded into the recompiler yet.  Instead it piggybacks 
it's 
test on the heels of some other events.  This is really bad design since it 
means 
short timr durations can be missed entirely in some cases, if one of those 
other 
events doesn't occur in that timeframe.

Original comment by Jake.Stine on 25 Nov 2009 at 9:06

GoogleCodeExporter commented 9 years ago
I see what you mean, cpuRegs.cycle-s_iLastCOP0Cycle runs through once, just 
fine, but
then s_iLastCOP0Cycle gets set to cpuRegs.cycle, and now that they're both 
equal, the
result is 0 on the next run through if cpuRegs.cycle hadn't been incremented 
yet. The
rest of the code should work fine without the case 9 change in MFC0(). I think I
overlooked the operator and was thinking it'd be alright for Count to be 0, and
didn't notice it was being incremented instead of being assigned a value.

I found the note about what happens in COP0.cpp above one of the performance 
counter
functions. Some games probably use a "udelay" implementation for setting the 
SMODE1
register, for around 2500 microseconds, and use MFC0 %0,Count instructions to 
time
the loop. The EE stops processing data if the pll isn't started correctly on 
older
revisions after modifying the SMODE1 register, for some reason. At least, 
that's what
I've heard.

Original comment by ragnarok2040@gmail.com on 26 Nov 2009 at 12:48

GoogleCodeExporter commented 9 years ago
I modified the code a bit to be more in line with the other interrupts. I used 
bit 7
of cpuRegs.interrupt, which seemed to be unused, and modified _cpuTestTIMR to 
set a
test every cycle. I tried increasing the interval between tests, but it just 
made my
interrupt handler get called more slowly. I also moved the call for 
_cpuTestTIMR() to
go alongside the other counters and commented out the other call and added a 
callback
handler for issuing the exception.

I compared it using a real ps2, and it seems to be fine, as the seconds 
generate at
the same rate. PCSX2 even displays the same behavior as the ps2 when I sleep 
the main
thread for 1/2 a second between loops.

I made a patch, and it probably won't apply cleanly, since it's based on 
revision
1972. Given the fact that the cpu timer interrupt is almost never used, you 
could
probably move the TESTINT(7,cpuTIMRInterrupt) call to where the others are for
optimization.

Original comment by ragnarok2040@gmail.com on 29 Nov 2009 at 3:19

Attachments:

GoogleCodeExporter commented 9 years ago
Long overdue that I accept this issue -- I missed the last post made to it, due 
to being so absorbed in other things, and UI work.  I should be able to get the 
patch applied pretty soon, I hope.

Original comment by Jake.Stine on 11 Jun 2010 at 1:48

GoogleCodeExporter commented 9 years ago
Patch for r4789.

Original comment by AndreyLegchilin on 3 Jul 2011 at 4:33

Attachments:

GoogleCodeExporter commented 9 years ago
I wasn't aware of this issue, I'll review the patch soon.

Original comment by sudonim1 on 2 Aug 2011 at 9:27

GoogleCodeExporter commented 9 years ago
The only application I have that makes use of the timer is the media player 
"SMS" in version 2.4.
This version works as well as before with this patch, meaning it runs videos 
for a short while and then hangs :p

Original comment by ramapcsx2 on 2 Aug 2011 at 5:42

GoogleCodeExporter commented 9 years ago
Rama is this still broken with SMS? I know you've been using it a lot.

Original comment by refraction on 5 Apr 2013 at 10:55