Reimplement/fix PRX patches #9746

elad335 · 2021-02-08T18:50:47Z

What is a PRX file? PRX stands for "PPU Relocatable Executable". What "relocatable" means in this context you ask? it means that the data/code segments of the PRX file can be mapped at any given address as long as the address is free from another memory mapping. Because every single PS3 application has different memory layout (it can even change within different boots of the same application!) we need PRX patches to be able to exist at any address based on relative offsets from segments instead of hardcoded addresses. This is fixed by this pull request.

Enhancements

Make PRX patches relocatable as they should.
Due to the changed control for PRX patches depending of knowing the segments addresses, I made them display in kernel explorer.
Added patch validation checks for PRX segments bounds. (TODO: do it for PPU/OVL patches as well)

Bugfixes

Fix LE32, LE64 and BE64 patches on PPU LLVM.

What this allows?
This allows to patch SPRX files as for firmware's.

I ported rujkosto's Metal Gear Solid 4's hack build in cellSpurs to a patch file of the firmware instead.
Here is the entry (need to be manually added until pushed to main patches)

PRX-rbYkgZMgx8Prd7WUAA39oYAdWcNo-0:
  "cellSpurs urgent commands hack - Metal Gear Solid 4":
    Games:
      All:
        All: [ All ]
    Author: elad335
    Notes: [ "Workaround for freezes in Metal Gear Solid 4.\nThis patch applies to a firmware file for all games!\nUsage outside of Metal Gear Solid 4 is for debug purposes only.\nYou must have firmware version of exactly 4.87 in order to use it.\n\nKnown to affect:\nMetal Gear Solid 4" ]
    Patch Version: 1.0
    Patch:
      - [ be32, 0x182B8, 0x38000002 ]

elad335 · 2021-02-08T19:00:56Z

Just a note: the MGS4 patch only applies for firmware version 4.87 and may need to be updated with newer firmware versions.

elad335 · 2021-02-08T20:37:52Z

rpcs3/Emu/Cell/PPUModule.cpp

-		hash[4 + i * 2] = pal[prx->sha1[i] >> 4];
-		hash[5 + i * 2] = pal[prx->sha1[i] & 15];
-	}
+	const std::string hash = fmt::format("PRX-%s", fmt::base57(prx->sha1));


Killing two birds in one stone: old PRX patches are invalidated, and the hashes naming system is updated to use the newer LLVM's base 57 format as opposed to the old base 16.

MSuih · 2021-02-09T05:49:06Z

I don't really like the idea of having a patch that you need to toggle off and on on game-by-game basis, to me it seems like something that'd cause a lot of confusion for end users and equal amounts of annoyance to those who try to help them. We should find a way to limit this to only apply to MGS4 by default, maybe in another PR if it's out of scope for this one.

elad335 · 2021-02-09T05:54:24Z

Where do you see the patch in pr.. lmao.

MSuih · 2021-02-09T06:16:45Z

The patch itself isn't part of this pr, but since we're introducing patching system that allows firmware modifications we should consider the possible problems in such system and ways to avoid them. The very first implementation to this system already seems like a support nightmare, and I suspect that we'll see more patches like this in future.

This all is just my own opinion though, I'm just raising my concerns and not demanding changes. That's up to the people who have merge rights.

elad335 · 2021-02-09T06:18:44Z

It supports gameids.. I don't understand your concern.

MSuih · 2021-02-09T06:29:23Z

Oh right, you can have the patches in patch_<gameid>.yml files. I guess that counts as a good enough of a solution if we get people to download it as such file.

elad335 · 2021-02-09T06:35:05Z

No, not right, patches support gameids even with main online patches database. I don't understand why we need specific files for each gameid.

elad335 · 2021-02-09T06:38:26Z

I'm sorry if I talked agressively but you seemed to not understand how patches work at all.

MSuih · 2021-02-09T06:42:08Z

Seems like I've misunderstood how patches are applied then. Sorry for the confusion, and feel free to ignore my comments.

rpcs3/Emu/Cell/PPUModule.cpp

elad335 · 2021-02-09T08:26:41Z

There's a problem in theory with squeezing allocation: there is more chance for OOM because the memory layout may not allow it as it requires one bigger memory "hole" as opposed a few smaller holes which are easier to find. So this should be a function from patches side to handle PRX specially. I will work on it when I have time.

Nekotekina · 2021-02-09T08:34:39Z

I doubt minimal patch support needs anything more than one segment base.
Possible recommendations:

Extend exec allocation range to 0x10000 as well in ppu_register_range, it's currently inconsistent.
Have multiple PRX hashes for each segment separately, add segment index to the end, since you are changing naming anyways.

elad335 · 2021-02-12T05:41:40Z

Added patch validation checks for PRX segments bounds. (TODO: do it for PPU/OVL patches as well)
Bugfixes
Fix non-BE32 patches on PPU LLVM.

elad335 · 2021-02-12T05:42:57Z

I improved further this, this is the format of PRX patches:
PRX-(hash)-(segment)

elad335 · 2021-02-12T05:53:52Z

Clear firmware PPU cache before testing the patch itself with pr.

elad335 · 2021-02-12T06:09:17Z

I updated the patch entry in pr description so update it if you wanna test.

Nekotekina · 2021-02-12T07:18:44Z

Utilities/bin_patch.cpp

 			break;
 		}
 		}

-		applied.push_back(resval);
+		// Possibly an executable instruction
+		applied.push_back(offset & -4);


Only be32 is practically used to signify executable instruction. Okay, also be64 can contain two. But other types are more likely to add garbage to the analyser rather than improve it. Especially floats.

I mean, other types push 0 to applied on purpose

Who said BE32 must not be garbage as well? Analyser should deal with anything you throw at it basically including garbage.

In fact, I consider changing the patch to be16 as it only meant to change op.uimm16 of ORI instruction.

Don't invert question like this. Basically if it works, don't break it.
If it's be16 it already contains some valid instruction, there is no need to inform analyser.

FTR I don't agree but there's no point in addressing in this pr.

Nekotekina · 2021-02-12T11:39:01Z

Utilities/bin_patch.cpp

@@ -607,14 +596,14 @@ static std::basic_string<u32> apply_modification(const patch_engine::patch_info&
 		}
 		}

+		// Possibly an executable instruction


Instead of aligning them down, seems better to not add unaligned patches to resval at all.

Nekotekina · 2021-02-12T11:40:35Z

Utilities/bin_patch.cpp

@@ -559,6 +546,7 @@ static std::basic_string<u32> apply_modification(const patch_engine::patch_info&
 		case patch_type::le32:
 		{
 			*reinterpret_cast<le_t<u32, 1>*>(ptr) = static_cast<u32>(p.value.long_value);
+			resval = offset;


Who uses le32 for instructions? It seems very inconvenient.

It's not about conveniency, it's about not introducing bugs which can be discovered by the user with some effort.

Changed anyways.

elad335 · 2021-02-12T13:46:39Z

Fixed typo in patch hash.

Nekotekina · 2021-02-12T13:54:45Z

Where? It's merged

elad335 · 2021-02-12T14:00:46Z

Just edited pr description.

elad335 force-pushed the patches branch from 6c15848 to 4c55fe4 Compare February 8, 2021 19:40

elad335 commented Feb 8, 2021

View reviewed changes

elad335 changed the title ~~Reimplement/fix PRX patches~~ [WIP] Reimplement/fix PRX patches Feb 8, 2021

elad335 changed the title ~~[WIP] Reimplement/fix PRX patches~~ Reimplement/fix PRX patches Feb 8, 2021

Nekotekina reviewed Feb 9, 2021

View reviewed changes

rpcs3/Emu/Cell/PPUModule.cpp Outdated Show resolved Hide resolved

elad335 marked this pull request as draft February 9, 2021 08:23

elad335 marked this pull request as ready for review February 12, 2021 05:38

elad335 force-pushed the patches branch from 4c55fe4 to 2fe5858 Compare February 12, 2021 05:41

elad335 force-pushed the patches branch 2 times, most recently from ffaaad6 to 2544487 Compare February 12, 2021 05:49

elad335 force-pushed the patches branch from 2544487 to f1559a2 Compare February 12, 2021 06:13

elad335 requested a review from Nekotekina February 12, 2021 06:13

Nekotekina reviewed Feb 12, 2021

View reviewed changes

elad335 force-pushed the patches branch 2 times, most recently from e7a7989 to ddc9c75 Compare February 12, 2021 08:37

Nekotekina reviewed Feb 12, 2021

View reviewed changes

elad335 added 2 commits February 12, 2021 14:07

Reimplement/fix PRX patches

44cfadf

kernel explorer: Display all PRX/OVL segments

7a29cc2

elad335 force-pushed the patches branch from ddc9c75 to 7a29cc2 Compare February 12, 2021 12:07

elad335 requested a review from Nekotekina February 12, 2021 12:17

Nekotekina approved these changes Feb 12, 2021

View reviewed changes

Nekotekina merged commit 5401cb7 into RPCS3:master Feb 12, 2021

elad335 deleted the patches branch March 16, 2021 14:50

Megamouse added the Patches Patches and Mods label Nov 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reimplement/fix PRX patches #9746

Reimplement/fix PRX patches #9746

elad335 commented Feb 8, 2021 •

edited

elad335 commented Feb 8, 2021 •

edited

elad335 Feb 8, 2021

MSuih commented Feb 9, 2021

elad335 commented Feb 9, 2021

MSuih commented Feb 9, 2021

elad335 commented Feb 9, 2021

MSuih commented Feb 9, 2021

elad335 commented Feb 9, 2021

elad335 commented Feb 9, 2021

MSuih commented Feb 9, 2021

elad335 commented Feb 9, 2021

Nekotekina commented Feb 9, 2021 •

edited

elad335 commented Feb 12, 2021

elad335 commented Feb 12, 2021

elad335 commented Feb 12, 2021

elad335 commented Feb 12, 2021

Nekotekina Feb 12, 2021

Nekotekina Feb 12, 2021 •

edited

elad335 Feb 12, 2021 •

edited

elad335 Feb 12, 2021 •

edited

Nekotekina Feb 12, 2021 •

edited

elad335 Feb 12, 2021

Nekotekina Feb 12, 2021

Nekotekina Feb 12, 2021

elad335 Feb 12, 2021

elad335 Feb 12, 2021

elad335 commented Feb 12, 2021

Nekotekina commented Feb 12, 2021

elad335 commented Feb 12, 2021

Reimplement/fix PRX patches #9746

Reimplement/fix PRX patches #9746

Conversation

elad335 commented Feb 8, 2021 • edited

Enhancements

Bugfixes

elad335 commented Feb 8, 2021 • edited

elad335 Feb 8, 2021

Choose a reason for hiding this comment

MSuih commented Feb 9, 2021

elad335 commented Feb 9, 2021

MSuih commented Feb 9, 2021

elad335 commented Feb 9, 2021

MSuih commented Feb 9, 2021

elad335 commented Feb 9, 2021

elad335 commented Feb 9, 2021

MSuih commented Feb 9, 2021

elad335 commented Feb 9, 2021

Nekotekina commented Feb 9, 2021 • edited

elad335 commented Feb 12, 2021

elad335 commented Feb 12, 2021

elad335 commented Feb 12, 2021

elad335 commented Feb 12, 2021

Nekotekina Feb 12, 2021

Choose a reason for hiding this comment

Nekotekina Feb 12, 2021 • edited

Choose a reason for hiding this comment

elad335 Feb 12, 2021 • edited

Choose a reason for hiding this comment

elad335 Feb 12, 2021 • edited

Choose a reason for hiding this comment

Nekotekina Feb 12, 2021 • edited

Choose a reason for hiding this comment

elad335 Feb 12, 2021

Choose a reason for hiding this comment

Nekotekina Feb 12, 2021

Choose a reason for hiding this comment

Nekotekina Feb 12, 2021

Choose a reason for hiding this comment

elad335 Feb 12, 2021

Choose a reason for hiding this comment

elad335 Feb 12, 2021

Choose a reason for hiding this comment

elad335 commented Feb 12, 2021

Nekotekina commented Feb 12, 2021

elad335 commented Feb 12, 2021

elad335 commented Feb 8, 2021 •

edited

elad335 commented Feb 8, 2021 •

edited

Nekotekina commented Feb 9, 2021 •

edited

Nekotekina Feb 12, 2021 •

edited

elad335 Feb 12, 2021 •

edited

elad335 Feb 12, 2021 •

edited

Nekotekina Feb 12, 2021 •

edited