Require interrupt handlers to be registered.

This allows it to be save stated sanely.

Core/HLE/sceKernel.cpp

@@ -78,18 +78,18 @@ void __KernelInit()
 	}
 
 	SaveState::Init();
+	__InterruptsInit();
 	__KernelMemoryInit();
 	__KernelThreadingInit();
-	__KernelMutexInit();
-	__KernelSemaInit();
 	__KernelAlarmInit();
 	__KernelEventFlagInit();
 	__KernelMbxInit();
+	__KernelMutexInit();
+	__KernelSemaInit();
 	__IoInit();
 	__AudioInit();
 	__SasInit();
 	__DisplayInit();
-	__InterruptsInit();
 	__GeInit();
 	__PowerInit();
 	__UtilityInit();
@@ -126,10 +126,10 @@ void __KernelShutdown()
 	__DisplayShutdown();
 	__AudioShutdown();
 	__IoShutdown();
-	__InterruptsShutdown();
 	__KernelMutexShutdown();
 	__KernelThreadingShutdown();
 	__KernelMemoryShutdown();
+	__InterruptsShutdown();
 
 	CoreTiming::ClearPendingEvents();
 	CoreTiming::UnregisterAllEvents();
@@ -144,14 +144,17 @@ void __KernelDoState(PointerWrap &p)
 	p.DoMarker("KernelObjects");
 
 	// TODO: Put these in the correct order...
+	__InterruptsDoState(p);
+	__KernelMemoryDoState(p);
+	// TODO: __KernelThreadingDoState(p);
 	__KernelAlarmDoState(p);
 	__KernelEventFlagDoState(p);
 	__KernelMbxDoState(p);
-	__KernelMemoryDoState(p);
 	__KernelModuleDoState(p);
 	__KernelMutexDoState(p);
 	__KernelSemaDoState(p);
-	// TODO: more modules
+	// TODO: non-kernel modules
+	// TODO: PPGe
 }
 
 bool __KernelIsRunning() {
@@ -328,7 +331,9 @@ void KernelObjectPool::DoState(PointerWrap &p)
 	if (_maxCount != maxCount)
 		ERROR_LOG(HLE, "Unable to load state: different kernel object storage.");
 
-	// Assumption: on load, we've already cleared.
+	if (p.mode == p.MODE_READ)
+		kernelObjects.Clear();
+
 	p.DoArray(occupied, maxCount);
 	for (int i = 0; i < maxCount; ++i)
 	{

Core/HLE/sceKernelAlarm.cpp

@@ -52,7 +52,12 @@ void __KernelScheduleAlarm(Alarm *alarm, u64 ticks);
 class AlarmIntrHandler : public SubIntrHandler
 {
 public:
-	AlarmIntrHandler(Alarm *alarm)
+	static SubIntrHandler *Create()
+	{
+		return new AlarmIntrHandler();
+	}
+
+	void setAlarm(Alarm *alarm)
 	{
 		alarmID = alarm->GetUID();
 		handlerAddress = alarm->alm.handlerPtr;
@@ -87,10 +92,17 @@ public:
 
 			// Delete the alarm if it's not rescheduled.
 			kernelObjects.Destroy<Alarm>(alarmID);
-			__ReleaseSubInterruptHandler(PSP_SYSTIMER0_INTR, alarmID);
+			__ReleaseSubIntrHandler(PSP_SYSTIMER0_INTR, alarmID);
 		}
 	}
 
+	virtual void DoState(PointerWrap &p)
+	{
+		SubIntrHandler::DoState(p);
+		p.Do(alarmID);
+		p.DoMarker("AlarmIntrHandler");
+	}
+
 	SceUID alarmID;
 };
 
@@ -109,6 +121,7 @@ void __KernelTriggerAlarm(u64 userdata, int cyclesLate)
 void __KernelAlarmInit()
 {
 	alarmTimer = CoreTiming::RegisterEvent("Alarm", __KernelTriggerAlarm);
+	__RegisterSubIntrCreator(PSP_SYSTIMER0_INTR, AlarmIntrHandler::Create);
 }
 
 void __KernelAlarmDoState(PointerWrap &p)
@@ -142,10 +155,15 @@ SceUID __KernelSetAlarm(u64 ticks, u32 handlerPtr, u32 commonPtr)
 	alarm->alm.handlerPtr = handlerPtr;
 	alarm->alm.commonPtr = commonPtr;
 
-	u32 error = __RegisterSubInterruptHandler(PSP_SYSTIMER0_INTR, uid, new AlarmIntrHandler(alarm));
+	u32 error;
+	AlarmIntrHandler *handler = (AlarmIntrHandler *) __RegisterSubIntrHandler(PSP_SYSTIMER0_INTR, uid, error);
 	if (error != 0)
+	{
+		kernelObjects.Destroy<Alarm>(uid);
 		return error;
+	}
 
+	handler->setAlarm(alarm);
 	__KernelScheduleAlarm(alarm, ticks);
 	return uid;
 }
@@ -174,7 +192,7 @@ int sceKernelCancelAlarm(SceUID uid)
 	DEBUG_LOG(HLE, "sceKernelCancelAlarm(%08x)", uid);
 
 	CoreTiming::UnscheduleEvent(alarmTimer, uid);
-	__ReleaseSubInterruptHandler(PSP_SYSTIMER0_INTR, uid);
+	__ReleaseSubIntrHandler(PSP_SYSTIMER0_INTR, uid);
 
 	return kernelObjects.Destroy<Alarm>(uid);
 }

Core/HLE/sceKernelInterrupt.cpp

@@ -28,11 +28,6 @@
 #include "sceKernelInterrupt.h"
 #include "sceKernelMutex.h"
 
-struct Interrupt
-{
-	PSPInterrupt intno;
-};
-
 void __DisableInterrupts();
 void __EnableInterrupts();
 bool __InterruptsEnabled();
@@ -92,9 +87,21 @@ void sceKernelCpuResumeIntrWithSync(u32 enable)
 
 class IntrHandler {
 public:
-	void add(int subIntrNum, SubIntrHandler *handler)
+	IntrHandler()
+		: subIntrCreator(NULL)
 	{
+	}
+
+	SubIntrHandler *add(int subIntrNum)
+	{
+		SubIntrHandler *handler;
+		if (subIntrCreator != NULL)
+			handler = subIntrCreator();
+		else
+			handler = new SubIntrHandler();
+
 		subIntrHandlers[subIntrNum] = handler;
+		return handler;
 	}
 	void remove(int subIntrNum)
 	{
@@ -113,8 +120,7 @@ public:
 		if (has(subIntrNum))
 			return subIntrHandlers[subIntrNum];
 		else
-			return 0;
-		// what to do, what to do...
+			return NULL;
 	}
 	void clear()
 	{
@@ -146,7 +152,49 @@ public:
 		}
 	}
 
+	void setCreator(SubIntrCreator creator)
+	{
+		subIntrCreator = creator;
+	}
+
+	void DoState(PointerWrap &p)
+	{
+		// We assume that the same creator has already been registered.
+		bool hasCreator = subIntrCreator != NULL;
+		p.Do(hasCreator);
+		if (hasCreator != (subIntrCreator != NULL))
+		{
+			ERROR_LOG(HLE, "Savestate failure: incompatible sub interrupt handler.");
+			return;
+		}
+
+		size_t n = subIntrHandlers.size();
+		p.Do(n);
+
+		if (p.mode == p.MODE_READ)
+		{
+			clear();
+			for (size_t i = 0; i < n; ++i)
+			{
+				int subIntrNum;
+				p.Do(subIntrNum);
+				SubIntrHandler *handler = add(subIntrNum);
+				handler->DoState(p);
+			}
+		}
+		else
+		{
+			std::map<int, SubIntrHandler *>::iterator it, end;
+			for (it = subIntrHandlers.begin(), end = subIntrHandlers.end(); it != end; ++it)
+			{
+				p.Do(it->first);
+				it->second->DoState(p);
+			}
+		}
+	}
+
 private:
+	SubIntrCreator subIntrCreator;
 	std::map<int, SubIntrHandler *> subIntrHandlers;
 };
 
@@ -157,6 +205,13 @@ public:
 	void restore();
 	void clear();
 
+	void DoState(PointerWrap &p)
+	{
+		p.Do(insideInterrupt);
+		p.Do(savedCpu);
+		p.DoMarker("InterruptState");
+	}
+
 	bool insideInterrupt;
 	ThreadContext savedCpu;
 //	Action afterInterruptAction;
@@ -181,6 +236,25 @@ void __InterruptsInit()
 	intState.clear();
 }
 
+void __InterruptsDoState(PointerWrap &p)
+{
+	int numInterrupts = PSP_NUMBER_INTERRUPTS;
+	p.Do(numInterrupts);
+	if (numInterrupts != PSP_NUMBER_INTERRUPTS)
+	{
+		ERROR_LOG(HLE, "Savestate failure: wrong number of interrupts, can't load.");
+		return;
+	}
+
+	intState.DoState(p);
+	for (int i = 0; i < PSP_NUMBER_INTERRUPTS; ++i)
+		intrHandlers[i].DoState(p);
+	p.Do(pendingInterrupts, PendingInterrupt(0, 0));
+	p.Do(interruptsEnabled);
+	p.Do(inInterrupt);
+	p.DoMarker("sceKernelInterrupt");
+}
+
 void __InterruptsShutdown()
 {
 	for (int i = 0; i < PSP_NUMBER_INTERRUPTS; ++i)
@@ -238,25 +312,16 @@ void SubIntrHandler::queueUp()
 {
 	if (!enabled)
 		return;
-	PendingInterrupt pend;
-	pend.handler = this;
-	pend.hasArg = false;
-	pend.intr = intrNumber;
-	pend.subintr = number;
-	pendingInterrupts.push_back(pend);
+
+	pendingInterrupts.push_back(PendingInterrupt(intrNumber, number));
 };
 
 void SubIntrHandler::queueUpWithArg(int arg)
 {
 	if (!enabled)
 		return;
-	PendingInterrupt pend;
-	pend.handler = this;
-	pend.arg = arg;
-	pend.hasArg = true;
-	pend.intr = intrNumber;
-	pend.subintr = number;
-	pendingInterrupts.push_back(pend);
+
+	pendingInterrupts.push_back(PendingInterrupt(intrNumber, number, arg));
 }
 
 void SubIntrHandler::copyArgsToCPU(const PendingInterrupt &pend)
@@ -278,6 +343,7 @@ bool __RunOnePendingInterrupt()
 		return false;
 	}
 	// Can easily prioritize between different kinds of interrupts if necessary.
+retry:
 	if (pendingInterrupts.size())
 	{
 		// If we came from CoreTiming::Advance(), we might've come from a waiting thread's callback.
@@ -285,8 +351,16 @@ bool __RunOnePendingInterrupt()
 		__KernelSwitchOffThread("interrupt");
 
 		PendingInterrupt pend = pendingInterrupts.front();
+		SubIntrHandler *handler = intrHandlers[pend.intr].get(pend.subintr);
+		if (handler == NULL)
+		{
+			WARN_LOG(HLE, "Ignoring interrupt, already been released.");
+			pendingInterrupts.pop_front();
+			goto retry;
+		}
+
 		intState.save();
-		pend.handler->copyArgsToCPU(pend);
+		handler->copyArgsToCPU(pend);
 
 		currentMIPS->r[MIPS_REG_RA] = __KernelInterruptReturnAddress();
 		inInterrupt = true;
@@ -340,7 +414,12 @@ void __KernelReturnFromInterrupt()
 	// This is what we just ran.
 	PendingInterrupt pend = pendingInterrupts.front();
 	pendingInterrupts.pop_front();
-	pend.handler->handleResult(currentMIPS->r[MIPS_REG_V0]);
+
+	SubIntrHandler *handler = intrHandlers[pend.intr].get(pend.subintr);
+	if (handler != NULL)
+		handler->handleResult(currentMIPS->r[MIPS_REG_V0]);
+	else
+		ERROR_LOG(HLE, "Interrupt released itself?  Should not happen.");
 
 	// Restore context after running the interrupt.
 	intState.restore();
@@ -351,15 +430,21 @@ void __KernelReturnFromInterrupt()
 		__KernelReSchedule("return from interrupt");
 }
 
-u32 __RegisterSubInterruptHandler(u32 intrNumber, u32 subIntrNumber, SubIntrHandler *subIntrHandler)
+void __RegisterSubIntrCreator(u32 intrNumber, SubIntrCreator creator)
 {
-	subIntrHandler->number = subIntrNumber;
-	subIntrHandler->intrNumber = intrNumber;
-	intrHandlers[intrNumber].add(subIntrNumber, subIntrHandler);
-	return 0;
+	intrHandlers[intrNumber].setCreator(creator);
 }
 
-u32 __ReleaseSubInterruptHandler(u32 intrNumber, u32 subIntrNumber)
+SubIntrHandler *__RegisterSubIntrHandler(u32 intrNumber, u32 subIntrNumber, u32 &error)
+{
+	SubIntrHandler *subIntrHandler = intrHandlers[intrNumber].add(subIntrNumber);
+	subIntrHandler->number = subIntrNumber;
+	subIntrHandler->intrNumber = intrNumber;
+	error = 0;
+	return subIntrHandler;
+}
+
+u32 __ReleaseSubIntrHandler(u32 intrNumber, u32 subIntrNumber)
 {
 	if (!intrHandlers[intrNumber].has(subIntrNumber))
 		return -1;
@@ -383,11 +468,15 @@ u32 sceKernelRegisterSubIntrHandler(u32 intrNumber, u32 subIntrNumber, u32 handl
 	if (intrNumber >= PSP_NUMBER_INTERRUPTS)
 		return -1;
 
-	SubIntrHandler *subIntrHandler = new SubIntrHandler();
-	subIntrHandler->enabled = false;
-	subIntrHandler->handlerAddress = handler;
-	subIntrHandler->handlerArg = handlerArg;
-	return __RegisterSubInterruptHandler(intrNumber, subIntrNumber, subIntrHandler);
+	u32 error;
+	SubIntrHandler *subIntrHandler = __RegisterSubIntrHandler(intrNumber, subIntrNumber, error);
+	if (subIntrHandler)
+	{
+		subIntrHandler->enabled = false;
+		subIntrHandler->handlerAddress = handler;
+		subIntrHandler->handlerArg = handlerArg;
+	}
+	return error;
 }
 
 u32 sceKernelReleaseSubIntrHandler(u32 intrNumber, u32 subIntrNumber)
@@ -397,7 +486,7 @@ u32 sceKernelReleaseSubIntrHandler(u32 intrNumber, u32 subIntrNumber)
 	if (intrNumber >= PSP_NUMBER_INTERRUPTS)
 		return -1;
 
-	return __ReleaseSubInterruptHandler(intrNumber, subIntrNumber);
+	return __ReleaseSubIntrHandler(intrNumber, subIntrNumber);
 }
 
 u32 sceKernelEnableSubIntr(u32 intrNumber, u32 subIntrNumber)

Core/HLE/sceKernelInterrupt.h

@@ -17,6 +17,8 @@
 
 #pragma once
 
+#include "../../Common/ChunkFile.h"
+
 enum PSPInterrupt {
 	PSP_GPIO_INTR      =  4,
 	PSP_ATA_INTR       =  5,
@@ -63,36 +65,38 @@ enum PSPInterruptTriggerType {
 	PSP_INTR_ONLY_IF_ENABLED = 0x2,
 };
 
-class AllegrexInterruptHandler;
-
 struct PendingInterrupt {
-	AllegrexInterruptHandler *handler;
+	PendingInterrupt(int intr_, int subintr_)
+		: intr(intr_), subintr(subintr_), hasArg(false) {}
+	PendingInterrupt(int intr_, int subintr_, int arg_)
+		: intr(intr_), subintr(subintr_), hasArg(true), arg(arg_) {}
+
 	int arg;
 	bool hasArg;
 	int intr;
 	int subintr;
 };
 
-
-class AllegrexInterruptHandler
-{
-public:
-	virtual ~AllegrexInterruptHandler() {}
-	virtual void copyArgsToCPU(const PendingInterrupt &pend) = 0;
-	virtual void queueUp() = 0;
-	virtual void queueUpWithArg(int arg) = 0;
-	virtual void handleResult(int result) = 0;
-};
-
-class SubIntrHandler : public AllegrexInterruptHandler
+class SubIntrHandler
 {
 public:
 	SubIntrHandler() {}
+	virtual ~SubIntrHandler() {}
 	virtual void queueUp();
 	virtual void queueUpWithArg(int arg);
 	virtual void copyArgsToCPU(const PendingInterrupt &pend);
 	virtual void handleResult(int result) {}
 
+	virtual void DoState(PointerWrap &p)
+	{
+		p.Do(enabled);
+		p.Do(intrNumber);
+		p.Do(number);
+		p.Do(handlerAddress);
+		p.Do(handlerArg);
+		p.DoMarker("SubIntrHandler");
+	}
+
 	bool enabled;
 	int intrNumber;
 	int number;
@@ -100,16 +104,20 @@ public:
 	u32 handlerArg;
 };
 
+typedef SubIntrHandler *(*SubIntrCreator)();
+
 bool __IsInInterrupt();
 void __InterruptsInit();
+void __InterruptsDoState(PointerWrap &p);
 void __InterruptsShutdown();
 void __TriggerInterrupt(int type, PSPInterrupt intno, int subInterrupts = -1);
 void __TriggerInterruptWithArg(int type, PSPInterrupt intno, int subintr, int arg);  // For GE "callbacks"
 bool __RunOnePendingInterrupt();
 void __KernelReturnFromInterrupt();
 
-u32 __RegisterSubInterruptHandler(u32 intrNumber, u32 subIntrNumber, SubIntrHandler *subIntrHandler);
-u32 __ReleaseSubInterruptHandler(u32 intrNumber, u32 subIntrNumber);
+void __RegisterSubIntrCreator(u32 intrNumber, SubIntrCreator creator);
+SubIntrHandler *__RegisterSubIntrHandler(u32 intrNumber, u32 subIntrNumber, u32 &error);
+u32 __ReleaseSubIntrHandler(u32 intrNumber, u32 subIntrNumber);
 
 u32 sceKernelRegisterSubIntrHandler(u32 intrNumber, u32 subIntrNumber, u32 handler, u32 handlerArg);
 u32 sceKernelReleaseSubIntrHandler(u32 intrNumber, u32 subIntrNumber);