hrydgard / ppsspp

A PSP emulator for Android, Windows, Mac and Linux, written in C++. Want to contribute? Join us on Discord at https://discord.gg/5NJB6dD or just send pull requests / issues. For discussion use the forums at forums.ppsspp.org.
https://www.ppsspp.org
Other
11.22k stars 2.17k forks source link

Tony Hawk's Underground 2 Remix - randomly crash ingame (previously also gfx issues) #3854

Open Superearth958 opened 11 years ago

Superearth958 commented 11 years ago

It never worked well.

Intros stopped working since v0.8.1-997. Game randomy crashes ingame.

Graphic issue in menu :

And ingame :

I tested it again with v0.9.1-1073 it's still the same.

Log : http://pastebin.com/rqwabL2H

unknownbrackets commented 9 years ago

Recently fixed an invalidation bug in jit, did that improve anything here?

-[Unknown]

daniel229 commented 9 years ago

Still hanging.

ppmeis commented 9 years ago

Confirmed it still hanging, even earlier than before.

Ozzypozzy commented 9 years ago

What's happening with THUG 2 Remix? It works fairly well if you let the levels load without dynarec on.

ppmeis commented 9 years ago

Tested with latest build. Crashing with fastmem on, black screen after a while with fastmen off.

sum2012 commented 9 years ago

Can we learn from jpcsp 's code of Icache ? (as it run no problem )

sum2012 commented 9 years ago

Yeah,I have some idea

sum2012 commented 9 years ago

Hmm,I need more time to test

sum2012 commented 9 years ago

The jpcsp's Icache code is in https://github.com/jpcsp/jpcsp/blob/master/src/jpcsp/Allegrex/Instructions.java#L155

LunaMoo commented 7 years ago

I worked around this issue by using cwcheat which doesn't do anything, but reads the code which game modifies(to refresh JIT), the game modifies lots of vhdp.q instructions(does so using sb) tonyhawk it replaced some more code initially which was directly causing those crashes, but I think only because earlier messup with those broken vfpu instructions as it stopped affecting anything else after I invalidated jit there every frame.

unknownbrackets commented 7 years ago

The sb - is it in a function that only gets called to update those instructions? Or just some memcpy? Does it get called once per frame? Does it call any cache instructions after doing so (in the function or its caller)?

It's not a great solution, but we could use a replacement hook on some function to clear the jit block it modifies (invalidating all jit may cause crashes in a replacement, though.) Not sure if it can be that simple.

Since there's no branching to the modified instructions our typical self-modifying check isn't triggering.

-[Unknown]

LunaMoo commented 7 years ago

The function that modifies the code is called once per frame, but there are 4 sb's in two loops: tony hawk modify The second jump(after both loops) goes to a function with cache instructions: tony hawk cache

Edit: In case they're useful, hashes: 50f7971181eab792:248 = sbFunction f7a202f7aaa3eae4:60 = cacheFunction

hrydgard commented 7 years ago

To recap what I remember, the function being modified is used for frustum culling, it effectively changes the sign of a few plane tests to adjust for where the camera is looking (which is a silly way to go about it, but whatever).

I seem to remember something about the function being in scratch memory (0x4000), maybe it's copied there after being modified?

The cache instructions afterwards correctly causes it to be invalidated and recompiled, so the culling works, but when the JIT cache later fills up and gets cleared a few hundred frames later, that's when things go south.

Can you check if the function being invalidated by the cache instructions is in the original location or moved to 0x4000-ish? If we patch the function with the "jit marker" but it then gets copied, that could explain things breaking, maybe...

LunaMoo commented 7 years ago

0x4000 is empty, scratchpad starts from 0x10000 by memory viewer and seems to finish at 0x14000, seems there are only some values there, floats and some ints, mostly it's filled with 0's and doesn't have those instructions. Unless I missunderstand the scratchpad thing as I think "cache" has same address + 0x40000000.

I'm confused now as I'm not sure how to check if they get invalidated as they appear to change under both 0x088655D8 and 0x488655D8 in disassembly same way. I definitely saw invalid emuhack with those opcodes in RAM earlier at least once that was after sb, but before cache instruction, so I guess it would be caching broken instructions, but I didn't checked the cache then and now I can't notice any invalid emuhack. Made sure that I don't have any hacks and game is crashing as it should, tried with and without breakpoints, no luck, maybe they're just too spammy and I got lucky earlier as the invalid emuhack I remember seeing was only in one of those instructions at once.

hrydgard commented 7 years ago

Sorry, I meant 0x10000 not 0x4000. Seems I might be mis-remembering or confusing with another game.

0x4xxxxxxxx is an "uncached" mirror of 0x0xxxxxxxxx. On the real system, writes/reads to 0x4xxxxxxxx bypass the cache entirely, and is thus suitable for writing data to be consumed by the GPU because then you don't need to invalidate the data cache lines. In PPSSPP we simply ignore this and emulate all memory as coherent ("uncached" from the point of view of the code, but in reality cached).

To check what is being validated, I meant to check the contents of the register used as a parameter for the cache instruction. That points to the cacheline being invalidated. The CPU needs to invalidate cachelines after writing to code, so that writes it did through the data cache appear when fetching instructions - the instruction cache could hide such changes.

LunaMoo commented 7 years ago

Cache instruction parameter points to ram(0x088655D8 and following), so it would be invalidated after being modified. ~ This reminds me the problem cheats had before and solution which was invalidating before reading/writing with them, so I guess if it invalidates after writing portion of the opcode breaking it with invalid emuhack, it's too late and it just stays broken?

In the scratchpad what I thought was floating points, might just be cache instructions(disassembly behaved weirdly when starting from 0x4000, so I only checked in memory viewer before;p), but those don't trigger breakpoints.

unknownbrackets commented 7 years ago

Invalidation should be checking that the instruction matches the emuhack before writing anything.

If an emuhack is being copied, that could definitely be an issue. We may be able to detect this if we detect a memcpy - we could check if the jit contains the source range, or more simply just invalidate the source range first?

Need some way to make sure it's not invalidating the caller. Maybe we can make the CALL to the replacement directly return to the dispatcher?

-[Unknown]

Catarax commented 6 years ago

With a cheat code from LunaMoo, you can now play without any crash. Tested on EUR version also, refresh rate to 1.

_C1 JIT refresh
//To be safe best set cheat refresh rate to or close to single digit value,
//this cheat doesn't do anything aside from checking some code which game modifies on it's own
//read code type refreshes JIT which avoids crashes just like that:]
_L 0xE0000000 0x00065690
_L 0xE0000000 0x00065600
_L 0xE0000000 0x00065604
_L 0xE0000000 0x00065608
_L 0xE0000000 0x0006560C
_L 0xE0000000 0x00065610
_L 0xE0000000 0x00065614
_L 0xE0000000 0x00065618
_L 0xE0000000 0x0006561C
_L 0xE0000000 0x00065620
_L 0xE0000000 0x00065624
_L 0xE0000000 0x00065628
_L 0xE0000000 0x0006562C
_L 0xE0000000 0x00065630
_L 0xE0000000 0x00065634
_L 0xE0000000 0x00065638
_L 0xE0000000 0x0006563C
_L 0xE0000000 0x00065640
_L 0xE0000000 0x00065644

https://forums.ppsspp.org/showthread.php?tid=1475&pid=126855#pid126855

unknownbrackets commented 6 years ago

Do we know what PSP function writes to that memory?

Worst case we could use a replacement hook to cause it to automatically refresh jit.

-[Unknown]

LunaMoo commented 6 years ago

Few posts above there is a hash(50f7971181eab792:248) and a screenshot of the function which is replacing the opcodes partially.

hrydgard commented 5 years ago

The real solution for this is to, like Dolphin, stop overwriting game code with emuhacks (jit entry opcodes) and instead write them to a separate memory space or some kind of hierarchical lookup table. Unfortunately this will increase RAM consumption a bit although that's becoming less and less of an issue, and also won't catch when code gets overwritten (although games are really supposed to invalidate the instruction cache in that case, and they really have to for things to work reliably on hardware).

Thinking about hardcoding these cheats in the emulator for now ... because the UX of having to use cheats just to make a game run properly is just horrible. Alternatively make a mode where we simply interpret instead of JIT in specially marked regions. Not sure what's best...

hrydgard commented 5 years ago

Proper fix won't happen in 1.8.0, merged the ugly workaround.

unknownbrackets commented 4 years ago

If we create a hook for 50f7971181eab792:248, and invalidate those same addresses at both the start and the end of that function, but disable the "auto-cheat"... does it work? Or does that not work, but the cheat does?

That there are sb instructions with offsets to 0x01/0x11/0x15 makes me worry that it is indeed disrupting emuhacks, so the proper fix is indeed moving away from them (for replacements too, really.)

We could at least detect this scenario if we had a "shadow" version of RAM where we've written the emuhacks again. If the shadow emuhack doesn't match the read emuhack, then we know it's been modified. In theory, we could even reconstruct the intended opcode via the difference (at least for sb/sh modifications), but that's just doubling down on emuhacks.

I haven't looked at Dolphin, but in theory we could still support self-modifying code as we do now with a shadow buffer (not many pages would even commit) + a check against the original first instruction (maybe just stored at the emuhack address, with actual code at that + 4). It'd add some overhead, but not that much...

-[Unknown]

hrydgard commented 4 years ago

Yeah, it's breaking emuhacks. I might try that hook idea though later.

I agree that the real path forward is getting rid of the emuhacks. We can afford a shadow RAM, I've only been concerned about address space on 32-bit devices (which conveniently Dolphin doesn't support).

unknownbrackets commented 4 years ago

Well, at least we would not need VRAM. I guess the only worry is if you can execute code in the scratchpad - maybe I'll test later on a PSP. It'd make things easier for address space if you can't.

-[Unknown]

hrydgard commented 4 years ago

You can absolutely execute code in the scratchpad, unfortunately

Hm to be honest I'm not actually that sure. But I seem to recall seeing it...

Probably worth testing indeed.

unknownbrackets commented 4 years ago

PC can indeed be the scratchpad AND VRAM. It runs fine. In fact, scratchpad is a bit faster:

Cached RAM: r=42, t=5799us
Uncached RAM: r=42, t=197767us
Cached VRAM: r=42, t=5925us
Uncached VRAM: r=42, t=154839us
Cached SRAM: r=42, t=5797us
Uncached SRAM: r=42, t=71891us

These timings are fairly consistent (at least to the millisecond) when running the code 10k times with about 106 ops each. We support all those cases currently, although our timings are a bit faster than cached in all cases (~5226us.)

Somehow Cached VRAM is slightly slower (5243us) even in PPSSPP... which makes me question my test, but I can't find anything wrong with it. Uncached VRAM is ironically faster.

Anyway, I guess that means we can't ignore either. Some enterprising developer may have realized that VRAM and scratchpad are faster for their one-off uncached dynamic code.

-[Unknown]

hrydgard commented 4 years ago

VRAM is a surprise! Hah.

I am sure I have never seen code executing from VRAM, at least...

sum2012 commented 2 years ago

v1.13.1-745-ga42807ea6 still need that hack @benderscruffy don't keep tap me.Some issue like this already hard hack solution in ppsspp. If have better solution pr, @hrydgard or @unknownbrackets would tap us.