Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CP15 Cache Emulation Work #1955

Open
wants to merge 43 commits into
base: master
Choose a base branch
from

Conversation

DesperateProgrammer
Copy link
Contributor

The instruction and data cache register were not yet implemented in MelonDS and would act different against the hardware by just reading/writing them.

They have no impact on the - not yet implemented - data cache nor on the instruction cache until their implementation progresses.

src/ARM.h Outdated
@@ -322,6 +322,7 @@ class ARMv5 : public ARM
u32 RNGSeed;

u32 DTCMSetting, ITCMSetting;
u32 DCacheLockDown, ICacheLockDown;
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget to add these to the savestate (and to increment the savestate version).

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you. Could you please increment the major version number in Savestate.h as well? It's our only way to detect savestate format changes at runtime; if it's not updated, then loading old states may crash (or worse, silently break the emulated console).

@DesperateProgrammer DesperateProgrammer changed the title CP15 Cache Lockdown Registers CP15 Cache Emulation Work Jan 19, 2024
@DesperateProgrammer DesperateProgrammer marked this pull request as draft January 19, 2024 07:21
@DesperateProgrammer
Copy link
Contributor Author

I'll add some more changes to the CP15 Caching Instructions and Handling here.

@DesperateProgrammer
Copy link
Contributor Author

The last commit was a bit bigger / combined, as adding DCache required other things to not break some applications.

Instruction cache is working except:

  • Prefetch via CP15 (todo)
  • CacheLockdown has no effect yet
  • Timings on cache-fill are not correct yet
    Data cache is working except:
  • Timings on cache fill are not correct yet
  • Timings on Cache hit are not correct yet
  • CacheLockdown has no effect yet
  • WriteBack is not implemented yet (it writes through and invalidates the cache line atm)

CP15 Registers Operations:

  • Cache Clean has no effect yet (needs write back mechanism)
  • Cache Clean & Invalidate will now also invalidate the cache (was missing)
  • Cache Debug Index Register was added
  • ICache/DCache Tag Registers were added
  • ICache/DCache Data Registers are not done yet
  • Privileged/User Checks are missing

@DesperateProgrammer
Copy link
Contributor Author

Update on the status

Instruction cache is working except:

  • Prefetch via CP15 (todo)
    
  • Timings on cache-fill are not correct yet
    

Data cache is working except:

  • Timings on cache fill are not correct yet
    
  • WriteBack is not implemented yet (it writes through)
    

CP15 Registers Operations:

  • Cache Clean has no effect yet (needs write back mechanism)
    
  • Cache Clean & Invalidate will now also invalidate the cache (was missing)
    
  • Cache Debug Index Register is working
    
  • ICache/DCache Tag Registers are working
    
  • ICache/DCache Data Registers are working
    
  • Privileged/User Checks are missing
    

JIT:

  • Caches are now disabled when JIT is enabled and no longer breaks JIT.

DesperateProgrammer and others added 17 commits January 31, 2024 12:20
Split the cp15 constants into CP15_Constants.h instead from MemConstants.
Fixed Cache Debug registers were accessible, when op1 != 3 in MCR/MRC instructions
Added BIST Test Status register and its cache linefill disable bits
Fixed a bug causing overlapping protection regions priority not taken into account, when access permission or cachability bits were changed only on the least priority overlap
Added const properties to the CP15Write/Read functions
Replaced mogic values with named constants
Added const specifier to some argument and subsequent calls
Updated documenting comments for the DCacheClear* methods
…ed constant

Encapsuled the cache features in #if to disable the features via compile flags
@DesperateProgrammer
Copy link
Contributor Author

This turned out to be quite some rework for the CP15.
The Cache is now working including the write-back mode. Cache Debug registers, BIST Cache Test register
CP15 Registers and Functions received some documenting comments.
Many CP15 related calls received the const-specifier for its arguments when they are not changed in the call.

@DesperateProgrammer DesperateProgrammer marked this pull request as ready for review February 6, 2024 10:18
src/CP15.cpp Outdated

id <<= 2;
if (ICacheTags[id+0] == tag)
for (int set=0;set<ICACHE_SETS;set++)
Copy link
Member

Choose a reason for hiding this comment

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

can you add spaces after the semicolons in all the for loops?

src/CP15.cpp Outdated
// Do the same as above but instead of using if-else
// utilize the && and || operators to skip parts of the operations
// With the order of comparison we can put the most likely path
// checked first
Copy link
Member

Choose a reason for hiding this comment

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

I'm not really sure whether this abusing shortcircuit evaluation really gives better code. If you want to steer the compiler using likely and unlikely markers could be used for the ifs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed this and added the [[(un)likely]] attributes

for (int i = 0; i < DCACHE_LINELENGTH; i+=sizeof(u32))
{
if (tag+i < ITCMSize)
{
Copy link
Member

Choose a reason for hiding this comment

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

oh so you can actually put ITCM and DTCM into cache?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not entirely sure (I think ) I had a block diagram showing the access passing the write buffer before the TCMs but can't seem to find that diagram. If that is the case, the TCM access will go through the cache when enabled.
However my decision to place the read from TCMs here was to keep the access flow within the line fills in sync to the DataReadXX functions.

src/CP15.cpp Outdated
{
if (addr & (DCACHE_LINELENGTH / 2))
{
DCacheTags[id+set] |= CACHE_FLAG_DIRTY_UPPERHALF ;
Copy link
Member

Choose a reason for hiding this comment

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

please remove the spaces before semicolon

@@ -306,7 +340,7 @@ void ARMv5::UpdateRegionTimings(u32 addrstart, u32 addrend)
MemTimings[i][0] = bustimings[2] << NDS.ARM9ClockShift;
}

if (pu & 0x10)
if (pu & CP15_MAP_DCACHEABLE)
{
MemTimings[i][1] = kDataCacheTiming;
Copy link
Member

Choose a reason for hiding this comment

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

these values will only ever be used when JIT is enabled, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now, yes. They are used in ARMv5::CodeRead32, when the JIT is not enabled.

Removed premature optimization and replaced them with [[(un)likely]]
@KostaSaizo7
Copy link

Fixes #380

@KostaSaizo7
Copy link

Unfortunately #359 is still occuring

@Jaklyy
Copy link
Contributor

Jaklyy commented May 5, 2024

seems like disabling the pu via the control reg causes melonds to hang under this pr?
at least reproduceable under this test rom:
zboxtesttest.zip

@DesperateProgrammer
Copy link
Contributor Author

Thanks for the report and sample Jakly,

I'll check that as soon i got some time.

CodeCycles = (NDS.ARM9MemTimings[addr >> 14][2] + (NDS.ARM9MemTimings[addr >> 14][3] * 7)) << NDS.ARM9ClockShift;
CurICacheLine = ptr;
// first N32 remaining S32
CodeCycles = (NDS.ARM9MemTimings[tag >> 14][2] + (NDS.ARM9MemTimings[tag >> 14][3] * ((DCACHE_LINELENGTH / 4) - 1))) << NDS.ARM9ClockShift;
Copy link
Contributor

Choose a reason for hiding this comment

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

minor error: DCACHE_LINELENGTH should be ICACHE_LINELENGTH

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.

5 participants