GBA DMA: Fix invalid DMA reads (fixes #142)

CHANGES

@@ -23,6 +23,7 @@ Bugfixes:
  - GB Video: Only trigger STAT write IRQs when screen is on (fixes mgba.io/i/912)
  - GBA Cheats: Fix PARv3 slide codes (fixes mgba.io/i/919)
  - GBA Video: OBJWIN can change blend params after OBJ is drawn (fixes mgba.io/i/921)
+ - GBA DMA: Fix invalid DMA reads (fixes mgba.io/i/142)
 Misc:
  - GBA Timer: Use global cycles for timers
  - GBA: Extend oddly-sized ROMs to full address space (fixes mgba.io/i/722)

include/mgba/internal/gba/memory.h

@@ -106,6 +106,7 @@ struct GBAMemory {
 	struct GBADMA dma[4];
 	struct mTimingEvent dmaEvent;
 	int activeDMA;
+	uint32_t dmaTransferRegister;
 
 	bool mirroring;
 };

include/mgba/internal/gba/serialize.h

@@ -169,7 +169,8 @@ mLOG_DECLARE_CATEGORY(GBA_STATE);
  *   | bits 4 - 8: GB Player transmit position
  *   | bits 9 - 23: Reserved
  * 0x002C4 - 0x002C7: Game Boy Player next event
- * 0x002C8 - 0x002DF: Reserved (leave zero)
+ * 0x002C8 - 0x002CB: Current DMA transfer word
+ * 0x002CC - 0x002DF: Reserved (leave zero)
  * 0x002E0 - 0x002EF: Savedata state
  * | 0x002E0 - 0x002E0: Savedata type
  * | 0x002E1 - 0x002E1: Savedata command (see savedata.h)
@@ -293,7 +294,9 @@ struct GBASerializedState {
 		uint32_t gbpNextEvent;
 	} hw;
 
-	uint32_t reservedHardware[6];
+	uint32_t dmaTransferRegister;
+
+	uint32_t reservedHardware[5];
 
 	struct {
 		uint8_t type;

src/gba/dma.c

@@ -46,6 +46,8 @@ uint32_t GBADMAWriteSAD(struct GBA* gba, int dma, uint32_t address) {
 	address &= 0x0FFFFFFE;
 	if (_isValidDMASAD(dma, address)) {
 		memory->dma[dma].source = address;
+	} else {
+		memory->dma[dma].source = 0;
 	}
 	return memory->dma[dma].source;
 }
@@ -242,31 +244,44 @@ void GBADMAService(struct GBA* gba, int number, struct GBADMA* info) {
 	info->when += cycles;
 
 	gba->performingDMA = 1 | (number << 1);
-	uint32_t word;
 	if (width == 4) {
-		word = cpu->memory.load32(cpu, source, 0);
+		if (source) {
-		gba->bus = word;
+			memory->dmaTransferRegister = cpu->memory.load32(cpu, source, 0);
-		cpu->memory.store32(cpu, dest, word, 0);
+		}
+		gba->bus = memory->dmaTransferRegister;
+		cpu->memory.store32(cpu, dest, memory->dmaTransferRegister, 0);
+		memory->dmaTransferRegister &= 0xFFFF0000;
+		memory->dmaTransferRegister |= memory->dmaTransferRegister >> 16;
 	} else {
 		if (sourceRegion == REGION_CART2_EX && memory->savedata.type == SAVEDATA_EEPROM) {
-			word = GBASavedataReadEEPROM(&memory->savedata);
-			cpu->memory.store16(cpu, dest, word, 0);
-		} else if (destRegion == REGION_CART2_EX) {
 			if (memory->savedata.type == SAVEDATA_AUTODETECT) {
 				mLOG(GBA_MEM, INFO, "Detected EEPROM savegame");
 				GBASavedataInitEEPROM(&memory->savedata, gba->realisticTiming);
 			}
-			word = cpu->memory.load16(cpu, source, 0);
+			memory->dmaTransferRegister = GBASavedataReadEEPROM(&memory->savedata);
-			GBASavedataWriteEEPROM(&memory->savedata, word, wordsRemaining);
 		} else {
-			word = cpu->memory.load16(cpu, source, 0);
+			if (source) {
-			cpu->memory.store16(cpu, dest, word, 0);
+				memory->dmaTransferRegister = cpu->memory.load16(cpu, source, 0);
+			}
 		}
-		gba->bus = word | (word << 16);
+		if (destRegion == REGION_CART2_EX) {
+			if (memory->savedata.type == SAVEDATA_AUTODETECT) {
+				mLOG(GBA_MEM, INFO, "Detected EEPROM savegame");
+				GBASavedataInitEEPROM(&memory->savedata, gba->realisticTiming);
+			}
+			GBASavedataWriteEEPROM(&memory->savedata, memory->dmaTransferRegister, wordsRemaining);
+		} else {
+			cpu->memory.store16(cpu, dest, memory->dmaTransferRegister, 0);
+
+		}
+		memory->dmaTransferRegister |= memory->dmaTransferRegister << 16;
+		gba->bus = memory->dmaTransferRegister;
 	}
 	int sourceOffset = DMA_OFFSET[GBADMARegisterGetSrcControl(info->reg)] * width;
 	int destOffset = DMA_OFFSET[GBADMARegisterGetDestControl(info->reg)] * width;
-	source += sourceOffset;
+	if (source) {
+		source += sourceOffset;
+	}
 	dest += destOffset;
 	--wordsRemaining;
 	gba->performingDMA = 0;

src/gba/io.c

@@ -939,6 +939,8 @@ void GBAIOSerialize(struct GBA* gba, struct GBASerializedState* state) {
 		STORE_32(gba->memory.dma[i].when, 0, &state->dma[i].when);
 	}
 
+	state->dmaTransferRegister = gba->memory.dmaTransferRegister;
+
 	GBAHardwareSerialize(&gba->memory.hw, state);
 }
 
@@ -984,6 +986,7 @@ void GBAIODeserialize(struct GBA* gba, const struct GBASerializedState* state) {
 		}
 	}
 	GBAAudioWriteSOUNDCNT_X(&gba->audio, gba->memory.io[REG_SOUNDCNT_X >> 1]);
+	gba->memory.dmaTransferRegister = state->dmaTransferRegister;
 	GBADMAUpdate(gba);
 	GBAHardwareDeserialize(&gba->memory.hw, state);
 }