Weird mem callback issue with high address locations under genplus-gx

[kaithar]

Summary

Frankly, I have no idea how or why this is broken beyond that it seems to have no obvious reason. But hey, it seems super reproducible, so there's that: OnMemoryWrite and on_bus_write callbacks registered for bus addresses around 0xFFF600-0xFFFFFF never fire.

• event.onmemorywrite(trapper, 0xfff370) is the highest address I've managed to find that works and is written to by the rom I'm testing with
• event.onmemorywrite(trapper, 0xfff600) never fires even as I'm watching the values changing in the hex editor
• event.onmemorywrite(trapper, 0xfff400) doesn't seem to fire but I say that with the caveat that I wrote to that via the hex editor because the rom doesn't use that memory area (to be exact, it seems to be allocated to the sound engine but that channel is dead code or something)
• event.onmemorywrite(trapper, 0xffff3) is the address I tested going the other way, doesn't fire... probably safe to assume it's all the way to 0xffffff with that.

I could thus assume that any address above 0xfff400 doesn't work, but I can't narrow the error bars further than F370-F600.
This is the upper kilobyte of the M68k's RAM, a bit frustrating for me because that's where a lot of the game engine values are.

I'm testing with Sonic 1, Rev 0 from 1991, hash 1BC674BE034E43C96B86487AC69D9293 ... in case you're wondering, 0xFFF600 is the game mode byte and changes every 30 seconds at most when the game is looping through the Sega/Title/Demo sequence ... I don't think it's specific to that game though, that'd be weird.

I did a cursory scan through trying to see if there was anything that would break the setting of the callback and abandoned the exercise when I'd gotten all the way to the gpgx cinterface without seeing anything that would logically lead to callbacks not working for only a range. If it were a higher or lower cutoff I might guess some kind of sign conversion shenanigans, but the addresses I tested suggest not. It shouldn't be a byte flip either, with those bit patterns.

To rule out some more possibles, I've tested this on Bizhawk 2.9 on Linux and Bizhawk 2.8 on Windows, obviously accounting for the expected difference that 2.8 doesn't have params for memory callbacks, and I'm seeing the same behaviour, so likely not a bug added during the big 2.9 memory changes or platform specific.

On 2.9 the RAM Watch and it's Breakpoints work correctly, and reading the bytes directly returns correct info, sooooo something magical that's specific to an interaction of EventsLuaLibrary.cs ? Yeah, I'm out of ideas.

Repro

function trapper (addr, val, flags)
    print("Trapped! "..addr)
end

event.onmemorywrite(trapper, 0xfff600)

Wait for something... you get a param error when it fires on 2.8, I'm getting the sound of silence on that address.

[CasualPokePlayer]

Setting a write breakpoint at 0xfff600 means writes specifically at 0xfff600, not any kind of mirrors to that address. Are you sure the game is specifically writing to that address and not say some mirror address?

[CasualPokePlayer]

Some testing and it seems the game writes to the 0xFFFFF600 mirror address. This is ultimately just standard mem hook behavior.

[kaithar]

Some testing and it seems the game writes to the 0xFFFFF600 mirror address. This is ultimately just standard mem hook behavior.

Sorry, I'm confused... it's standard behaviour that it simply doesn't work? As I said, 0xfff370 works perfectly while 0xfff600 doesn't. I can't see a reason why one would work and the other would not.

[YoshiRulz]

The address you see in the tracelog is the one for which hooks will trigger, not any mirrors. You can imagine EmuHawk having more "magic", but we're not in a position to improve that behaviour right now.

[kaithar]

To be clear, the Genesis m68k has a 24 bit address space, its RAM is mirrored across the address range 0xE00000-0xFFFFFF but the normal behaviour is the access 0xFF0000-0xFFFFFF.... so 0xFFF600, which is what I was checking against, is the correct hardware address... accessing 0xFFFFF600 is impossible because that address doesn't physically exist, it's being truncated down. The address registers may technically be 32 bits, but only the 24 bit address space and 16 bit external bus are used. The top byte of the address register is discarded and only the bottom 2 bytes get written to the pins.

Heck, I have the disassembly in front of me, so I can actually give you the exact instruction being used:

11FC 0000 F600      		move.b	#id_Sega,(v_gamemode).w

It's a single byte immediate write, the EA is encoded in the instruction as a 16 bit address, it would have to be (v_gamemode).l to encode 0xFFFFF600.

The writes to 0xFFF370 are harder to show since the sound driver is all relative addressing, but it should be a combination of these two instructions:

4DF9 00FF F000      		lea	(v_snddriver_ram&$FFFFFF).l,a6
4BEE 0370           		lea	v_spcsfx_psg3_track(a6),a5

though other parts are handled like one of these two instructions:

00FF F370           		dc.l (v_snddriver_ram+v_spcsfx_psg3_track)&$FFFFFF
08EE 0002 0370      		bset	#2,v_spcsfx_psg3_track+TrackPlaybackControl(a6)	; Set 'SFX is overriding' bit

Where the former is encoding an address literal in the ROM and the latter is how some of the relative addresses of the sound driver are referenced.

I can't follow what possible address magic would cause event.onmemorywrite(trapper, 0xfff370) to correctly trap writes while event.onmemorywrite(trapper, 0xfff600) doesn't. You could, perhaps, make the argument that the sign extended 32 bit internal representation, that can't actually leave the address register, is what's being trapped against, but in that case 0xFFF370 shouldn't work and it definitely does, while 0xFFF600 is provided as an immediate operand (so shouldn't really be going anywhere near an address register?) and so should never get extended because, as I said, the address bus is only 24 bits wide.

What am I missing here?

[YoshiRulz]

The write callback is simply passed the address which the core is using internally (biz_writecb here calls MemoryCallbackSystem.CallMemoryCallbacks which calls your Lua function, and at no point are heuristics applied):

BizHawk/waterbox/gpgx/core/m68k/m68kcpu.h

Lines 927 to 970 in b10f896

	INLINE void m68ki_write_8_fc(uint address, uint fc, uint value)
	{
	cpu_memory_map *temp;
	if (biz_writecb)
	biz_writecb(address);
	m68ki_set_fc(fc) /* auto-disable (see m68kcpu.h) */
	temp = &m68ki_cpu.memory_map[((address)>>16)&0xff];
	if (temp->write8) (*temp->write8)(ADDRESS_68K(address),value);
	else WRITE_BYTE(temp->base, (address) & 0xffff, value);
	}
	INLINE void m68ki_write_16_fc(uint address, uint fc, uint value)
	{
	cpu_memory_map *temp;
	if (biz_writecb)
	biz_writecb(address);
	m68ki_set_fc(fc) /* auto-disable (see m68kcpu.h) */
	m68ki_check_address_error(address, MODE_WRITE, fc); /* auto-disable (see m68kcpu.h) */
	temp = &m68ki_cpu.memory_map[((address)>>16)&0xff];
	if (temp->write16) (*temp->write16)(ADDRESS_68K(address),value);
	else *(uint16 *)(temp->base + ((address) & 0xffff)) = value;
	}
	INLINE void m68ki_write_32_fc(uint address, uint fc, uint value)
	{
	cpu_memory_map *temp;
	if (biz_writecb)
	biz_writecb(address);
	m68ki_set_fc(fc) /* auto-disable (see m68kcpu.h) */
	m68ki_check_address_error(address, MODE_WRITE, fc) /* auto-disable (see m68kcpu.h) */
	temp = &m68ki_cpu.memory_map[((address)>>16)&0xff];
	if (temp->write16) (*temp->write16)(ADDRESS_68K(address),value>>16);
	else *(uint16 *)(temp->base + ((address) & 0xffff)) = value >> 16;
	temp = &m68ki_cpu.memory_map[((address + 2)>>16)&0xff];
	if (temp->write16) (*temp->write16)(ADDRESS_68K(address+2),value&0xffff);
	else *(uint16 *)(temp->base + ((address + 2) & 0xffff)) = value;
	}

[kaithar]

Hmmm... are those involved in macro rewritten calls somewhere? I can't see anything that actually calls them. I think I can guess what's happening though.

1. The instruction translation has an address in a variable.
2. It calls the write function with the content of the variable.
3. The write function calls the cb with whatever address was sent in.
4. The write function applies bit math to address to simulate how the hardware done bank switching.
5. The write function actually sets the memory.

The callback being fired at step 3 is consistent with it triggering against the bus address, but the issue is that there's no sanity applied to the value in uint address thus my guess is that the two writes are behaving differently due to a difference in how address modes are being treated.

The IOP that is using an address register is applying bit masking to correctly emulate the address bus being 24 bits wide. Dunno precisely where in the sequence the value is being masked but it'll be before use. This instruction fires the callback handler correctly.

The IOP that is using an immediate address is storing the address pulled from the operand in local variable, it sign extends that value in that variable and then passes that to the write which passes it through unhindered to the callback. What it doesn't do is mask to 24 bits... the sign extension is either 32 bit (size of a data register) or 64 bit (from size of the type), so the bus write is address that's impossible on hardware. Likely this isn't showing issues anywhere else since the masking in step 4 doesn't care how big the address is, it's just the call back that's getting inconsistent behaviour due to different move instructions generating drastically different address sizes.

The only other place I could imagine this causing an issue is if there's some odd case where an immediate loaded address is compared against a value rather than being used to memory address, but I'd have to guess that all the comparison and test instructions apply a mask to both values (after operands have been resolved) to enforce the data size part of the opcode.
That'd give an edge case I can't guess about where an address register is long compared to a sign extended immediate value... the address register would be truncated to 24 bits by the address bus and the immediate would be truncated to 32 bits by the comp width, but I don't know if the addressing mode would mask both down to 24 or if the 32 bit comp would sign extend the address in the ALU. The answer's probably documented somewhere but I don't care enough to try looking for it heh.

If the intent is for bus callbacks to fire against addresses on the bus in a hardware accurate way, they should really be masked to the correct width for the architecture, thus 24 bit here.
If the intent is for the bus callbacks to be predictable the same likely also applies because right now, if is working the way it seems, you would have to know how the emulator is handling a specific opcode to be able to figure out how many F's you need on your address string... or trap every width to be inefficient.

That all said... doesn't the callback registration code apply a range sanity to make sure that address requested lies within the domain requested? I thought it generated an error when you tried to use an invalid address. If the trap is constrained to legal bus addresses then it's impossible to trap a 32 bit memory address write regardless.

[YoshiRulz]

I'll be honest, I didn't understand any of that.

doesn't the callback registration code apply a range sanity to make sure that address requested lies within the domain requested?

I'm not seeing one. But the sysbus is indeed 24 bits, and that's enforced by the peek/poke callbacks:

BizHawk/src/BizHawk.Emulation.Cores/Consoles/Sega/gpgx64/GPGX.IMemoryDomains.cs

Lines 82 to 94 in 3f92764

	var m68Bus = new MemoryDomainDelegate("M68K BUS", 0x1000000, MemoryDomain.Endian.Big,
	addr =>
	{
	var a = (uint)addr;
	if (a > 0xFFFFFF) throw new ArgumentOutOfRangeException(paramName: nameof(addr), a, message: "address out of range");
	return Core.gpgx_peek_m68k_bus(a);
	},
	(addr, val) =>
	{
	var a = (uint)addr;
	if (a > 0xFFFFFF) throw new ArgumentOutOfRangeException(paramName: nameof(addr), a, message: "address out of range");
	Core.gpgx_write_m68k_bus(a, val);
	}, 2);

[kaithar]

Could I ask this be reopened?

[YoshiRulz]

Oops.