-
Notifications
You must be signed in to change notification settings - Fork 63
Implement direct mapped cache for instruction fetch #103
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| #include <stdio.h> | ||
| #include <string.h> | ||
|
|
||
| #include "common.h" | ||
| #include "device.h" | ||
|
|
@@ -180,11 +181,17 @@ static inline uint32_t read_rs2(const hart_t *vm, uint32_t insn) | |
| return vm->x_regs[decode_rs2(insn)]; | ||
| } | ||
|
|
||
| static inline void icache_invalidate_all(hart_t *vm) | ||
| { | ||
| memset(&vm->icache, 0, sizeof(vm->icache)); | ||
| } | ||
|
|
||
| /* virtual addressing */ | ||
|
|
||
| void mmu_invalidate(hart_t *vm) | ||
| { | ||
| vm->cache_fetch.n_pages = 0xFFFFFFFF; | ||
| vm->cache_fetch[0].n_pages = 0xFFFFFFFF; | ||
| vm->cache_fetch[1].n_pages = 0xFFFFFFFF; | ||
| /* Invalidate all 8 sets × 2 ways for load cache */ | ||
| for (int set = 0; set < 8; set++) { | ||
| for (int way = 0; way < 2; way++) | ||
|
|
@@ -197,6 +204,7 @@ void mmu_invalidate(hart_t *vm) | |
| vm->cache_store[set].ways[way].n_pages = 0xFFFFFFFF; | ||
| vm->cache_store[set].lru = 0; /* Reset LRU to way 0 */ | ||
| } | ||
| icache_invalidate_all(vm); | ||
| } | ||
|
|
||
| /* Invalidate MMU caches for a specific virtual address range. | ||
|
|
@@ -227,9 +235,11 @@ void mmu_invalidate_range(hart_t *vm, uint32_t start_addr, uint32_t size) | |
| uint32_t end_vpn = (uint32_t) end_addr >> RV_PAGE_SHIFT; | ||
|
|
||
| /* Cache invalidation for fetch cache */ | ||
| if (vm->cache_fetch.n_pages >= start_vpn && | ||
| vm->cache_fetch.n_pages <= end_vpn) | ||
| vm->cache_fetch.n_pages = 0xFFFFFFFF; | ||
| for (int i = 0; i < 2; i++) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mmu_invalidate_range now needs to flush the new instruction cache as well; otherwise pages whose mappings change keep returning stale instructions from vm->icache. Prompt for AI agents
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’m a bit confused about this. /* Cache invalidation for fetch cache */
for (int i = 0; i < 2; i++) {
if (vm->cache_fetch[i].n_pages >= start_vpn &&
vm->cache_fetch[i].n_pages <= end_vpn)
vm->cache_fetch[i].n_pages = 0xFFFFFFFF;
}There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @yy214123 You're right to be confused—the code snippet you showed only handles invalidating the page translation caches ( Right now, when you call To fix it properly, you need to add logic in /* Invalidate I-cache blocks in the address range */
uint32_t start_block_addr = (start_addr >> ICACHE_OFFSET_BITS) << ICACHE_OFFSET_BITS;
uint32_t end_block_addr = ((end_addr + ICACHE_BLOCK_SIZE - 1) >> ICACHE_OFFSET_BITS) << ICACHE_OFFSET_BITS;
for (uint32_t block_idx = 0; block_idx < ICACHE_BLOCKS; block_idx++) {
uint32_t block_base = (block_idx << ICACHE_OFFSET_BITS) | (vm->icache.i_block[block_idx].tag << (ICACHE_OFFSET_BITS + ICACHE_INDEX_BITS));
if (block_base >= start_block_addr && block_base <= end_block_addr && vm->icache.i_block[block_idx].valid) {
vm->icache.i_block[block_idx].valid = false;
}
}
/* Similar loop for victim cache - check if v_block[i].tag matches any address in range */
for (int i = 0; i < VCACHE_BLOCKS; i++) {
if (vm->icache.v_block[i].valid) {
uint32_t v_block_addr = vm->icache.v_block[i].tag << ICACHE_OFFSET_BITS;
if (v_block_addr >= start_block_addr && v_block_addr <= end_block_addr) {
vm->icache.v_block[i].valid = false;
}
}
}This is a bit more involved than the full |
||
| if (vm->cache_fetch[i].n_pages >= start_vpn && | ||
| vm->cache_fetch[i].n_pages <= end_vpn) | ||
| vm->cache_fetch[i].n_pages = 0xFFFFFFFF; | ||
| } | ||
|
|
||
| /* Invalidate load cache: 8 sets × 2 ways */ | ||
| for (int set = 0; set < 8; set++) { | ||
|
|
@@ -361,11 +371,47 @@ static void mmu_fence(hart_t *vm, uint32_t insn UNUSED) | |
|
|
||
| static void mmu_fetch(hart_t *vm, uint32_t addr, uint32_t *value) | ||
| { | ||
| /* cache hit */ | ||
| uint32_t idx = (addr >> ICACHE_OFFSET_BITS) & ICACHE_INDEX_MASK; | ||
| uint32_t tag = addr >> (ICACHE_OFFSET_BITS + ICACHE_INDEX_BITS); | ||
| icache_block_t *blk = &vm->icache.i_block[idx]; | ||
| uint32_t vpn = addr >> RV_PAGE_SHIFT; | ||
| if (unlikely(vpn != vm->cache_fetch.n_pages)) { | ||
| uint32_t index = __builtin_parity(vpn) & 0x1; | ||
|
|
||
| if (likely(blk->valid && blk->tag == tag)) { | ||
| #ifdef MMU_CACHE_STATS | ||
| vm->cache_fetch[index].hits++; | ||
| #endif | ||
| uint32_t ofs = addr & ICACHE_BLOCK_MASK; | ||
| *value = *(const uint32_t *) (blk->base + ofs); | ||
| return; | ||
| } | ||
|
|
||
| /* search the victim cache */ | ||
| uint32_t vcache_key = addr >> ICACHE_OFFSET_BITS; | ||
| for (int i = 0; i < VCACHE_BLOCKS; i++) { | ||
| victim_cache_block_t *vblk = &vm->icache.v_block[i]; | ||
|
|
||
| /* victim cache hit, swap blocks */ | ||
| if (vblk->valid && vblk->tag == vcache_key) { | ||
| icache_block_t tmp = *blk; | ||
| *blk = *vblk; | ||
| *vblk = tmp; | ||
| blk->tag = tag; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code looks suspicious to me. When you move the evicted I-cache block (tmp) back into the victim cache, you are setting the vblk->tag to tmp.tag, which is the 16-bit I-cache tag. Won't this corrupts the victim cache entry? The VC search logic requires a 24-bit tag ([ICache Tag | ICache Index]) to function. Because you're only storing the 16-bit tag, this VCache entry will never be hit again.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thank you for pointing that out. I’ve added the following expressions to ensure correctness : + vblk->tag = (tmp.tag << ICACHE_INDEX_BITS) | idx; |
||
| vblk->tag = (tmp.tag << ICACHE_INDEX_BITS) | idx; | ||
|
|
||
| uint32_t ofs = addr & ICACHE_BLOCK_MASK; | ||
| *value = *(const uint32_t *) (blk->base + ofs); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| #ifdef MMU_CACHE_STATS | ||
| vm->cache_fetch.misses++; | ||
| vm->cache_fetch[index].misses++; | ||
| #endif | ||
|
|
||
| /* cache miss, Continue using the original va->pa*/ | ||
| if (unlikely(vpn != vm->cache_fetch[index].n_pages)) { | ||
| mmu_translate(vm, &addr, (1 << 3), (1 << 6), false, RV_EXC_FETCH_FAULT, | ||
| RV_EXC_FETCH_PFAULT); | ||
| if (vm->error) | ||
|
|
@@ -374,15 +420,27 @@ static void mmu_fetch(hart_t *vm, uint32_t addr, uint32_t *value) | |
| vm->mem_fetch(vm, addr >> RV_PAGE_SHIFT, &page_addr); | ||
| if (vm->error) | ||
| return; | ||
| vm->cache_fetch.n_pages = vpn; | ||
| vm->cache_fetch.page_addr = page_addr; | ||
| vm->cache_fetch[index].n_pages = vpn; | ||
| vm->cache_fetch[index].page_addr = page_addr; | ||
| } | ||
| #ifdef MMU_CACHE_STATS | ||
| else { | ||
| vm->cache_fetch.hits++; | ||
|
|
||
| *value = | ||
| vm->cache_fetch[index].page_addr[(addr >> 2) & MASK(RV_PAGE_SHIFT - 2)]; | ||
|
|
||
| /* Move the current icache block into the victim cache before replacement */ | ||
| if (blk->valid) { | ||
| victim_cache_block_t *vblk = &vm->icache.v_block[vm->icache.v_next]; | ||
| *vblk = *blk; | ||
| vblk->tag = (blk->tag << ICACHE_INDEX_BITS) | idx; | ||
| vblk->valid = true; | ||
| vm->icache.v_next = (vm->icache.v_next + 1) % VCACHE_BLOCKS; | ||
| } | ||
| #endif | ||
| *value = vm->cache_fetch.page_addr[(addr >> 2) & MASK(RV_PAGE_SHIFT - 2)]; | ||
|
|
||
| /* fill into the icache */ | ||
| uint32_t block_off = (addr & RV_PAGE_MASK) & ~ICACHE_BLOCK_MASK; | ||
| blk->base = (const uint8_t *) vm->cache_fetch[index].page_addr + block_off; | ||
| blk->tag = tag; | ||
| blk->valid = true; | ||
| } | ||
|
|
||
| static void mmu_load(hart_t *vm, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cache_fetchonly has two entries (indices 0 and 1), so reading index 2 here is undefined behavior. Please sum entries 0 and 1 instead.Prompt for AI agents