Merge pull request #22328 from hrydgard/lle-firmware-module-unload

LLE firmware module unload

Core/CmdLine.cpp

@@ -232,6 +232,7 @@ static const CommandLineParam g_autoParams[] = {
 	{POFF(autoSaveLoadSymbols), CmdParamType::Bool, "auto-save-load-symbols", '\0', "Auto save/load per-module and per-game symbol files (see bAutoSaveLoadSymbols)", CmdLineMode::Both},
 	{POFF(bootVSH), CmdParamType::Bool, "vsh", '\0', "Boot the VSH (requires files dumped from a PSP in the flash0 directory)"},
 	{POFF(disableHLE), CmdParamType::Int, "disable-hle", '\0', "Bitmask of libraries to run the real firmware module for instead of our HLE", CmdLineMode::Both},
+	{POFF(forceHLE), CmdParamType::Int, "force-hle", '\0', "Bitmask of libraries to run our HLE for, even where the real module is the default", CmdLineMode::Both},
 	{POFF(memReadAction), CmdParamType::Enum, "memread", '\0', "Set the action for memory read exceptions", CmdLineMode::Both, g_ExceptionActionValues, ARRAY_SIZE(g_ExceptionActionValues)},
 	{POFF(memWriteAction), CmdParamType::Enum, "memwrite", '\0', "Set the action for memory write exceptions", CmdLineMode::Both, g_ExceptionActionValues, ARRAY_SIZE(g_ExceptionActionValues)},
 	{POFF(breakAction), CmdParamType::Enum, "break", '\0', "Set the action for break exceptions", CmdLineMode::Both, g_ExceptionActionValues, ARRAY_SIZE(g_ExceptionActionValues)},
@@ -565,6 +566,11 @@ void CommandLineOptions::ApplyToConfig() const {
 		g_Config.DoNotSaveSetting(&g_Config.iDisableHLE);
 	}
 
+	if (forceHLE.has_value()) {
+		g_Config.iForceEnableHLE = forceHLE.value();
+		g_Config.DoNotSaveSetting(&g_Config.iForceEnableHLE);
+	}
+
 	if (logLevel.has_value()) {
 		g_logManager.SetAllLogLevels(logLevel.value());
 	}

Core/CmdLine.h

@@ -96,6 +96,10 @@ struct CommandLineOptions {
 	// libraries. Needs a firmware dump under the NAND directory.
 	std::optional<int> disableHLE;
 
+	// The opposite: put our HLE back for libraries that now run the real module by default. The
+	// way to compare the two without editing a config, and the way out if the real one breaks a game.
+	std::optional<int> forceHLE;
+
 	// Headless: install the game update in a .pkg (given as the boot filename) into this
 	// directory, then exit without booting anything. The directory is the game folder itself -
 	// the app puts that under PSP/GAME/<DISC_ID>, but here the caller picks. See

Core/HLE/sceAudiocodec.cpp

@@ -229,8 +229,12 @@ static int __AudioCodecInitCommon(u32 ctxPtr, int codec, bool mono) {
 		return hleLogError(Log::ME, SCE_KERNEL_ERROR_OUT_OF_RANGE, "Invalid codec");
 	}
 
+	// Re-initialising a context that still has a decoder is normal, not a report-worthy
+	// surprise: mpeg.prx sizes the allocation through a scratch context of its own and only ever
+	// releases that one, so the context it actually decodes through still holds a decoder when the
+	// next movie starts. Once per video, on every game running the real module.
 	if (removeDecoder(ctxPtr)) {
-		WARN_LOG_REPORT(Log::HLE, "sceAudiocodecInit(%08x, %d): replacing existing context", ctxPtr, codec);
+		INFO_LOG(Log::HLE, "sceAudiocodecInit(%08x, %d): replacing existing context", ctxPtr, codec);
 	}
 
 	// Initialize the codec memory.
@@ -523,11 +527,17 @@ static int sceAudiocodecGetEDRAM(u32 ctxPtr, int codec) {
 	return hleLogInfo(Log::ME, 0, "edram address set to %08x", ctx->edramAddr);
 }
 
-static int sceAudiocodecReleaseEDRAM(u32 ctxPtr, int id) {
-	if (removeDecoder(ctxPtr)){
+// One parameter, not two: the real module (audiocodec_260.prx, 080007c0) reads only a0 and sets
+// up a1-a3 itself, so a second argument here just logs whatever was left in the register.
+//
+// Releasing the EDRAM without ever having made a decoder is normal - mpeg.prx calls
+// CheckNeedMem/GetEDRAM to size the allocation and only creates a decoder if the stream turns out
+// to need one, so there is often nothing here to drop.
+static int sceAudiocodecReleaseEDRAM(u32 ctxPtr) {
+	if (removeDecoder(ctxPtr)) {
 		return hleLogInfo(Log::ME, 0);
 	}
-	return hleLogWarning(Log::ME, 0, "failed to remove decoder");
+	return hleLogDebug(Log::ME, 0, "no decoder for this context");
 }
 
 static int sceAudiocodecGetOutputBytes(u32 ctxPtr, int codec, u32 outBytesAddr) {
@@ -607,7 +617,7 @@ const HLEFunction sceAudiocodec[] = {
 	{0X5B37EB1D, &WrapI_UI<sceAudiocodecInit>,           "sceAudiocodecInit",         'i', "xx"},
 	{0X8ACA11D5, &WrapI_UI<sceAudiocodecGetInfo>,        "sceAudiocodecGetInfo",      'i', "xx"},
 	{0X3A20A200, &WrapI_UI<sceAudiocodecGetEDRAM>,       "sceAudiocodecGetEDRAM",     'i', "xx"},
-	{0X29681260, &WrapI_UI<sceAudiocodecReleaseEDRAM>,   "sceAudiocodecReleaseEDRAM", 'i', "xx"},
+	{0X29681260, &WrapI_U<sceAudiocodecReleaseEDRAM>,    "sceAudiocodecReleaseEDRAM", 'i', "x"},
 	{0X9D3F790C, &WrapI_UI<sceAudiocodecCheckNeedMem>,   "sceAudiocodecCheckNeedMem", 'i', "xx"},
 	{0X59176A0F, &WrapI_UIU<sceAudiocodecGetOutputBytes>, "sceAudiocodecGetOutputBytes", 'i', "xxp" },  // params are context, codec, outptr
 	{0X3DD7EE1A, &WrapI_UI<sceAudiocodecInitMono>,       "sceAudiocodecInitMono",     'i', "xx"},  // Used by sceAtrac for MOut* functions.

Core/HLE/sceKernelModule.cpp

@@ -2026,6 +2026,17 @@ static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 load
 	return module;
 }
 
+bool KernelUnloadModuleByID(SceUID moduleId) {
+	u32 error;
+	PSPModule *module = kernelObjects.Get<PSPModule>(moduleId, error);
+	if (!module) {
+		return false;
+	}
+	module->Cleanup();
+	kernelObjects.Destroy<PSPModule>(moduleId);
+	return true;
+}
+
 bool KernelModuleIsLoaded(std::string_view name) {
 	u32 error;
 	for (SceUID moduleId : loadedModules) {

Core/HLE/sceKernelModule.h

@@ -256,6 +256,12 @@ SceUID KernelLoadModule(const std::string &filename, std::string *error_string,
 // Whether a real (non-HLE-stub) module calling itself this is loaded. Lets a caller tell whether a
 // library is already provided before bringing in another copy of it.
 bool KernelModuleIsLoaded(std::string_view name);
+
+// Take back out a module we brought in ourselves (the firmware swap in sceUtility). Frees the
+// memory block, which is the point: a game that unloads its video libraries before a level load
+// expects that space back, and one 33KB module left sitting in the middle of it is enough to
+// break a contiguous allocation. Returns false if the id is not a live module.
+bool KernelUnloadModuleByID(SceUID moduleId);
 int __KernelStartModule(SceUID moduleId, u32 argsize, u32 argAddr, u32 returnValueAddr, SceKernelSMOption *smoption, bool *needsWait);
 u32 __KernelStopUnloadSelfModuleWithOrWithoutStatus(u32 exitCode, u32 argSize, u32 argp, u32 statusAddr, u32 optionAddr, bool WithStatus);
 u32 sceKernelFindModuleByUID(u32 uid);

Core/HLE/sceUtility.cpp

@@ -95,12 +95,17 @@ static const int mp4ModuleDeps[] = {0x0300, 0};
 // So the module-list check below is a guard rather than the normal path, and it costs nothing:
 // asking the list rather than remembering what we loaded means this needs no state of its own. It
 // is right after a savestate load, across games, and if a game unloads a library and asks again.
+// The firmware modules we swapped in for a library whose HLE is disabled, keyed by the utility
+// module whose load brought them in. Remembered because the unload has to take out what we put
+// in and nothing else: a game that ships its own copy loads it itself, and we must not free that.
+static std::map<int, std::vector<SceUID>> swappedFirmwareModules;
+
 struct FirmwareModule {
 	const char *path;        // in the firmware
 	const char *moduleName;  // what the module calls itself once loaded
 };
 
-static void LoadFirmwareModules(const char *library, const FirmwareModule *modules, size_t count) {
+static void LoadFirmwareModules(int utilityModule, const char *library, const FirmwareModule *modules, size_t count) {
 	for (size_t i = 0; i < count; i++) {
 		if (KernelModuleIsLoaded(modules[i].moduleName)) {
 			DEBUG_LOG(Log::sceUtility, "%s is already loaded - not loading %s on top of it",
@@ -133,13 +138,33 @@ static void LoadFirmwareModules(const char *library, const FirmwareModule *modul
 			ERROR_LOG(Log::sceUtility, "Failed to start %s (%08x)", modules[i].path, result);
 			return;
 		}
+		swappedFirmwareModules[utilityModule].push_back(id);
 		INFO_LOG(Log::sceUtility, "Loaded the real %s", modules[i].path);
 	}
 }
 
+// The other half: give the memory back when the game says it is done with the library. Reverse
+// order, since a later module may import from an earlier one.
+static void UnloadFirmwareModules(int utilityModule) {
+	auto it = swappedFirmwareModules.find(utilityModule);
+	if (it == swappedFirmwareModules.end()) {
+		return;
+	}
+	for (auto id = it->second.rbegin(); id != it->second.rend(); ++id) {
+		if (KernelUnloadModuleByID(*id)) {
+			INFO_LOG(Log::sceUtility, "Unloaded the real module %d we had swapped in", *id);
+		}
+	}
+	swappedFirmwareModules.erase(it);
+}
+
 // mpeg.prx needs sceVideocodec, sceMpegbase and sceAudiocodec from us, all of which we implement,
 // so the module itself is the only thing that has to come from somewhere real.
 static void NotifyLoadStatusMpegBase(int state, u32 loadAddr, u32 totalSize) {
+	if (state == -1) {
+		UnloadFirmwareModules(0x303);
+		return;
+	}
 	// The effective flags, not the raw setting: those also account for the compat flags, for a
 	// firmware dump that isn't there, and for the boundary a savestate restored - resolving
 	// imports one way and loading modules the other is how a game ends up with neither.
@@ -149,18 +174,22 @@ static void NotifyLoadStatusMpegBase(int state, u32 loadAddr, u32 totalSize) {
 	static const FirmwareModule modules[] = {
 		{ "flash0:/kd/mpeg.prx", "sceMpeg_library" },
 	};
-	LoadFirmwareModules("sceMpeg", modules, ARRAY_SIZE(modules));
+	LoadFirmwareModules(0x303, "sceMpeg", modules, ARRAY_SIZE(modules));
 }
 
 // libmp3.prx imports nothing but the kernel and sceAudiocodec, which we have.
 static void NotifyLoadStatusMp3(int state, u32 loadAddr, u32 totalSize) {
+	if (state == -1) {
+		UnloadFirmwareModules(0x304);
+		return;
+	}
 	if (state != 1 || !(GetEffectiveDisableHLEFlags() & DisableHLEFlags::sceMp3)) {
 		return;
 	}
 	static const FirmwareModule modules[] = {
 		{ "flash0:/kd/libmp3.prx", "sceMp3_Library" },
 	};
-	LoadFirmwareModules("sceMp3", modules, ARRAY_SIZE(modules));
+	LoadFirmwareModules(0x304, "sceMp3", modules, ARRAY_SIZE(modules));
 }
 
 static void NotifyLoadStatusAvcodec(int state, u32 loadAddr, u32 totalSize) {
@@ -171,6 +200,10 @@ static void NotifyLoadStatusAvcodec(int state, u32 loadAddr, u32 totalSize) {
 // functions from sceAudiocodec (Init and Decode) plus ordinary kernel calls, and mp4msv.prx - the
 // 41 functions libmp4 leans on - imports nothing at all.
 static void NotifyLoadStatusMp4(int state, u32 loadAddr, u32 totalSize) {
+	if (state == -1) {
+		UnloadFirmwareModules(0x308);
+		return;
+	}
 	if (state != 1) {
 		return;
 	}
@@ -194,10 +227,14 @@ static void NotifyLoadStatusMp4(int state, u32 loadAddr, u32 totalSize) {
 		{ "flash0:/kd/mp4msv.prx", "mp4msv_module" },
 		{ "flash0:/kd/libmp4.prx", "sceMp4_library" },
 	};
-	LoadFirmwareModules("sceMp4", modules, ARRAY_SIZE(modules));
+	LoadFirmwareModules(0x308, "sceMp4", modules, ARRAY_SIZE(modules));
 }
 
 static void NotifyLoadStatusAtrac(int state, u32 loadAddr, u32 totalSize) {
+	if (state == -1) {
+		UnloadFirmwareModules(0x302);
+		return;
+	}
 	if (state == 1) {
 		// The effective flags, for the same reason the loads above use them.
 		if (GetEffectiveDisableHLEFlags() & DisableHLEFlags::sceAtrac) {
@@ -208,7 +245,7 @@ static void NotifyLoadStatusAtrac(int state, u32 loadAddr, u32 totalSize) {
 			static const FirmwareModule modules[] = {
 				{ "flash0:/kd/libatrac3plus.prx", "sceATRAC3plus_Library" },
 			};
-			LoadFirmwareModules("sceAtrac", modules, ARRAY_SIZE(modules));
+			LoadFirmwareModules(0x302, "sceAtrac", modules, ARRAY_SIZE(modules));
 			return;
 		}
 
@@ -449,13 +486,14 @@ void __UtilityInit() {
 	DeactivateDialog();
 	SavedataParam::Init();
 	currentlyLoadedModules.clear();
+	swappedFirmwareModules.clear();
 	volatileUnlockEvent = CoreTiming::RegisterEvent("UtilityVolatileUnlock", UtilityVolatileUnlock);
 
 	ResetSecondsSinceLastGameSave();
 }
 
 void __UtilityDoState(PointerWrap &p) {
-	auto s = p.Section("sceUtility", 1, 6);
+	auto s = p.Section("sceUtility", 1, 7);
 	if (!s) {
 		return;
 	}
@@ -479,6 +517,14 @@ void __UtilityDoState(PointerWrap &p) {
 		}
 	}
 
+	if (s >= 7) {
+		Do(p, swappedFirmwareModules);
+	} else if (p.mode == p.MODE_READ) {
+		// An older state has no record of what we swapped in, so the unload notification will
+		// leave those modules loaded rather than risk freeing something the game owns.
+		swappedFirmwareModules.clear();
+	}
+
 	if (s >= 3) {
 		Do(p, volatileUnlockEvent);
 	} else {
@@ -743,6 +789,17 @@ static int UnloadModuleInternal(u32 module, bool av);
 
 // Same as sceUtilityLoadModule, just limited in categories.
 // It seems this just loads module 0x300 + module & 0xFF..
+// Loading a module that is already loaded is a normal answer, not a fault: a game asks for the
+// libraries it wants without tracking whether something else already brought them in, and just
+// ignores this (Tekken 6 loads av_avcodec three times and never unloads it). Everything else that
+// comes back from here is worth an error.
+static int LogModuleLoadResult(int result) {
+	if (result == SCE_ERROR_MODULE_ALREADY_LOADED || result == SCE_ERROR_AV_MODULE_ALREADY_LOADED) {
+		return hleLogDebug(Log::sceUtility, result, "already loaded");
+	}
+	return hleLogDebugOrError(Log::sceUtility, result);
+}
+
 static u32 sceUtilityLoadAvModule(u32 module) {
 	if (module > 7) {
 		ERROR_LOG_REPORT(Log::sceUtility, "sceUtilityLoadAvModule(%i): invalid module id", module);
@@ -750,7 +807,7 @@ static u32 sceUtilityLoadAvModule(u32 module) {
 	}
 
 	int result = LoadModuleInternal(0x300 | module, true);
-	return hleDelayResult(hleLogDebugOrError(Log::sceUtility, result), "utility av module loaded", 25000);
+	return hleDelayResult(LogModuleLoadResult(result), "utility av module loaded", 25000);
 }
 
 static u32 sceUtilityUnloadAvModule(u32 module) {
@@ -767,9 +824,9 @@ static u32 sceUtilityLoadModule(u32 module) {
 	int result = LoadModuleInternal(module, false);
 	// TODO: Each module has its own timing, technically, but this is a low-end.
 	if (module == 0x3FF) {
-		return hleDelayResult(hleLogDebugOrError(Log::sceUtility, result), "utility module loaded", 130);
+		return hleDelayResult(LogModuleLoadResult(result), "utility module loaded", 130);
 	} else {
-		return hleDelayResult(hleLogDebugOrError(Log::sceUtility, result), "utility module loaded", 25000);
+		return hleDelayResult(LogModuleLoadResult(result), "utility module loaded", 25000);
 	}
 }
 

docs/debugging.md

@@ -178,6 +178,13 @@ it looked in (usually enough to spot that it's the one beside the exe), and exit
 machine with no firmware would fail. So when a measurement depends on the real module, pass the flag
 explicitly even though it's on by default, and the run will tell you if it didn't get it.
 
+`--force-hle=` is the other direction, and the one to reach for when deciding whether something is
+our fault: it puts our HLE back for libraries that now run the real module by default, so the same
+repro can be run both ways and the logs diffed. That is how the leftover warnings in Tekken 6 were
+sorted - three appeared identically with `--force-hle=16`, which made them the game's own, and the
+fourth only under the real module, which made it ours. Neither `--nand=` pointing somewhere empty
+nor `--appendconfig` does this job: the firmware gets found anyway and the setting is per-game.
+
 ## Debugging and breakpoint considerations
 
 It might be worth trying the interpreter - all types of breakpoints are the most reliable with this CPU backend.

frametests.py

@@ -28,13 +28,19 @@ import re
 import shlex
 import shutil
 import subprocess
+import platform
 import sys
 import time
 from pathlib import Path
 
 # test.py-style candidate paths for the headless binary, relative to the
 # current working directory, in preference order.
-HEADLESS_CANDIDATES = [
+# The machine's own architecture comes first, the same way test.py picks: an x64 build runs on
+# Windows-on-ARM too, under emulation, so looking for it first quietly tests the emulated build.
+HEADLESS_CANDIDATES = ([
+    "Windows/ARM64/Debug/PPSSPPHeadless.exe",
+    "Windows/ARM64/Release/PPSSPPHeadless.exe",
+] if platform.machine().lower() in ("arm64", "aarch64") else []) + [
     "Windows/x64/Debug/PPSSPPHeadless.exe",
     "Windows/Debug/PPSSPPHeadless.exe",
     "Windows/x64/Release/PPSSPPHeadless.exe",

headless/Headless.cpp

@@ -454,12 +454,24 @@ static bool RunAutoTest(GraphicsContext *graphicsContext, CoreParameter &corePar
 		if (coreState == CORE_NEXTFRAME) {
 			// INFO_LOG(Log::System, "(frame)");
 			coreState = CORE_RUNNING_CPU;
-			// Close and reopen the host frame, which is what the app does once per displayed
-			// frame. All the GPU's per-frame work hangs off BeginHostFrame - the texture cache's
+			// Close and reopen the frame, which is what the app does once per displayed frame.
+			// All the GPU's per-frame work hangs off BeginHostFrame - the texture cache's
 			// StartFrame and the framebuffer manager's DecimateFBOs - so with a single host frame
 			// spanning the whole run, none of it ever ran here, and a long test decayed nothing.
+			//
+			// The draw context's frame has to turn over too, and for the same reason one level up:
+			// Vulkan's push buffers are recycled by BeginFrame, so one frame spanning the run means
+			// nothing is ever reused and every allocation takes a fresh 8MB block - about 13MB a
+			// second, which runs a long test out of device memory. Draw frame outside, host frame
+			// inside, the way the app nests them.
 			if (gpu) {
 				gpu->EndHostFrame();
+			}
+			if (draw) {
+				draw->EndFrame();
+				draw->BeginFrame(Draw::DebugFlags::NONE);
+			}
+			if (gpu) {
 				gpu->BeginHostFrame(g_Config.GetDisplayLayoutConfig(DeviceOrientation::Landscape));
 			}
 		}