nes6502 - issue with mirrored memory

[olduf]

First thing, amazing project. Very useful for emulator development.

The CPU instructions I coded passed all the tests successfully with a simple memory management class.

#pragma once

#include <cstdint>

class MMU
{
public:
  uint8_t getByte(uint16_t address) {
    return this->memory[address];
  }

  void setByte(uint16_t address, uint8_t value) {
    this->memory[address] = value;
  }

private:
  uint8_t memory[0xFFFF] = {};
};

With that done, I started implementing the memory mirroring of the NES CPU and that's when my test started failing.

In the NESDEV wiki CPU memory map, we see that the memory at 0x0000–0x07FF and 0x2000–0x2007 are each repeated multiple times.

This means that multiple address should always have the same value (if I understand everything correctly). So:

• 0x0001 should contain the same value as 0x0801, 0x1001 and 0x1801.
• 0x2004 should contain the same value as 0x200C, 0x2014, 0x201C, ..., 0x3FFC.

Examples of the issue:

• in 00 6f 9f, memory locations 308 and 4404 should have the same value (since 308 % 0x800 == 4404 % 0x800)
  • [308, 180]
  • [4404, 159]
• in 00 88 2e, memory locations 12176 and 13352 should have the same value (since 12176 % 8 == 13352 % 8)
  • [12176, 136]
  • [13352, 196]

It would be nice not have to provide a different memory management class implementation in order to pass the tests.

Not sure how the test case are generated, but I could try making the change.

And if I'm wrong about this, sorry 😅 .

[matthewoates]

Hi @olduf - thanks for the detailed report. I'm new to the emulation space but as far as I can tell this is a real issue. addresses 308 (0x134) and 4404 (0x1134) map to the same physical CPU RAM address.

[TomHarte]

Apologies for the long time with no input; I'm very wishy-washy on this.

On the one hand the tests in net are designed not to assume anything external to the CPU — including how addresses are decoded.

On the other, the Ricoh 2A03 (/'NES 6502'): (i) is used (almost) exclusively in the NES; and (ii) includes the APU and some other parts in the chip, so the "external to the CPU" test is fuzzier than it should be.

I think possibly a good compromise would be just to restrict all memory accesses in the NES-specific tests to the first 2kb of memory? Would that satisfy the problem adequately?

[matthewoates]

I think possibly a good compromise would be just to restrict all memory accesses in the NES-specific tests to the first 2kb of memory? Would that satisfy the problem adequately?

Now that I've finished my cycle-accurate CPU emulator of this chip for the NES I feel more confident weighing in here.

I think it could be an improvement to only use addresses that don't have special mapping behavior, but it invites a lot of complexity. I'm confused about why you're choosing the first 2KB (assuming this is 0x0000 - 0x1fff which is just 0x0000 - 0x07ff mirrored.

However if we do acknowledge the memory mapping, then that invites a lot of additional complexity where we need to support mappers.

I think the real issue is that the documentation could be more explicit (instead of implicit) about not choosing to respect memory mapping or any special cartridge mapping behavior, and I do like that this suite of tests allows me to properly isolate and unit test the CPU. Time and energy permitting, I'd like to find a suite of test roms and create some per-cycle truth data with a similar level of thoroughness found in this repo. (It's a fantastic repo!)

[TomHarte]

I'm confused about why you're choosing the first 2KB

The first 2kb runs to address 0x07ff; up to 0x1fff would be the first 8kb.

However if we do acknowledge the memory mapping, then that invites a lot of additional complexity where we need to support mappers.

I'm suggesting I just get the generator silently to drop any test that happens to generate an access outside of the range [0x0000, 0x07ff] and let it keep going until there are the desired number of test cases.

... though more explicit documentation might also just be the correct fix.

[TomHarte]

So I've gone with "more explicit documentation" as the immediate fix, as per #37 but if I can persuade myself that an appropriate test filter — such as the proposed first-2kb-only, but possibly something else — would render the issue moot then I'll return to the topic.