Skip to content

expose physical RAM banks via memory descriptors for RetroAchievements - #173

Open
wescopeland wants to merge 1 commit into
libretro:masterfrom
wescopeland:ra-memory-map
Open

expose physical RAM banks via memory descriptors for RetroAchievements#173
wescopeland wants to merge 1 commit into
libretro:masterfrom
wescopeland:ra-memory-map

Conversation

@wescopeland

Copy link
Copy Markdown

Fixes the memory-map requirement from #135.

The memory descriptors from #73 copy pointers from memory_map_read[] at load time. Paging rewrites those entries. On any 128K machine, the descriptors for $C000-$FFFF become stale after the first write to port 0x7FFD. RetroAchievements caches descriptor pointers once at load, so it reads the wrong memory. This was flagged by @leiradel in the #73 review but never fixed.

This PR replaces the CPU-view descriptors with 8 fixed descriptors, one per physical RAM bank, in the order the RetroAchievements memory map defines.

The pointers go into the static RAM[][] array, so they never move and never go stale. Paging does not change any address. The layout is the same for every machine model, and 16K/48K just leave the upper banks empty.

retro_get_memory_data(SYSTEM_RAM) remains NULL on purpose, as the raw array is in bank order 0-7, and the rcheevos fallback expects canonical order, so exposing it would give silently wrong addresses.

This was tested with a strict libretro host on 48K, 128K, +2, +2A, +3, and 16K models. I tested the bank identity of every descriptor, pointer stability across reset and reload, and pokes at $C000 landing in the currently-paged bank. I also tested against rcheevos' real rc_libretro_memory_init. All 8 regions map with direct pointers, and known addresses read correctly.

Comment thread src/libretro.c
desc[i].ptr = memory_map_read[i].page;
desc[i].start = 0x4000 + (size_t)i * 0x4000;
desc[i].len = 0x4000;
desc[i].ptr = RAM[bank_order[i]];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong, start will be outside the Z80 address space when i is 3 or greater. I think RAM[0] should be declared only once at 0xc000 with size 128 Kb, but I'm not 100% sure that's how you declare banked memory. Maybe other retro_memory_descriptor must also be initialized.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The design is correct.

The >64K starts are intentional, as start isn't limited to the Z80 bus, and 128K of banks can't fit in 64K.

The layout must match the memory map defined in rcheevos. rcheevos resolves each region at exactly these start values, and addresses must not move when a game writes 0x7FFD. BizHawk already implements this same layout.

One 128K descriptor at $C000 pointing at RAM[0] gives the wrong order. The map's tail is banks 0, 1, 3, 4, 6, 7 (5 and 2 are pinned at $4000/$8000). Banks 3/4/6/7 would land at wrong addresses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants