Implement Ziccid (I$ coherence) correctly and cleanly

riscv/execute.cc

@@ -223,15 +223,6 @@ void processor_t::step(size_t n)
     }
   }
 
-  if (extension_enabled(EXT_ZICCID)) {
-    // Ziccid requires stores eventually become visible to instruction fetch,
-    // so periodically flush the I$
-    if (ziccid_flush_count-- == 0) {
-      ziccid_flush_count += ZICCID_FLUSH_PERIOD;
-      _mmu->flush_icache();
-    }
-  }
-
   while (n > 0) {
     size_t instret = 0;
     reg_t pc = state.pc;

riscv/mmu.cc

@@ -384,6 +384,32 @@ void mmu_t::store_slow_path(reg_t original_addr, reg_t len, const uint8_t* bytes
   }
 }
 
+void mmu_t::flush_icache_vpn(reg_t vpn)
+{
+  for (size_t i = 0; i < ICACHE_ENTRIES; i++)
+    if ((icache[i].tag >> PGSHIFT) == vpn)
+      icache[i].tag = -1;
+}
+
+void mmu_t::flush_stlb_paddr(reg_t paddr)
+{
+  for (size_t i = 0; i < TLB_ENTRIES; i++)
+    if (tlb_store[i].data.target_addr == paddr)
+      tlb_store[i].tag = -1;
+}
+
+void mmu_t::flush_itlb_paddr(reg_t paddr)
+{
+  for (size_t i = 0; i < TLB_ENTRIES; i++) {
+    if (tlb_insn[i].data.target_addr == paddr) {
+      // Maintain inclusion of I$
+      flush_icache_vpn(tlb_insn[i].tag & ~TLB_FLAGS);
+
+      tlb_insn[i].tag = -1;
+    }
+  }
+}
+
 tlb_entry_t mmu_t::refill_tlb(reg_t vaddr, reg_t paddr, char* host_addr, access_type type)
 {
   reg_t idx = (vaddr >> PGSHIFT) % TLB_ENTRIES;
@@ -402,14 +428,31 @@ tlb_entry_t mmu_t::refill_tlb(reg_t vaddr, reg_t paddr, char* host_addr, access_
 
   switch (type) {
     case FETCH:
+      if (proc && proc->extension_enabled(EXT_ZICCID)) {
+        // Maintain inclusion of I$
+        if (tlb_insn[idx].tag != (reg_t)-1)
+          flush_icache_vpn(tlb_insn[idx].tag & ~TLB_FLAGS);
+
+        // Maintain exclusion with all store TLBs
+        for (auto [_, p2] : sim->get_harts())
+          p2->mmu->flush_stlb_paddr(entry.target_addr);
+      }
+
       tlb_insn[idx].data = entry;
       tlb_insn[idx].tag = expected_tag | (check_triggers_fetch ? TLB_CHECK_TRIGGERS : 0) | trace_flag | mmio_flag;
       break;
     case LOAD:
       tlb_load[idx].data = entry;
       tlb_load[idx].tag = expected_tag | (check_triggers_load ? TLB_CHECK_TRIGGERS : 0) | trace_flag | mmio_flag;
       break;
+
     case STORE:
+      if (proc && proc->extension_enabled(EXT_ZICCID)) {
+        // Maintain exclusion with all instruction TLBs
+        for (auto [_, p2] : sim->get_harts())
+          p2->mmu->flush_itlb_paddr(entry.target_addr);
+      }
+
       tlb_store[idx].data = entry;
       tlb_store[idx].tag = expected_tag | (check_triggers_store ? TLB_CHECK_TRIGGERS : 0) | trace_flag | mmio_flag;
       break;

riscv/mmu.h

@@ -291,7 +291,7 @@ class mmu_t
     return have_reservation;
   }
 
-  static const reg_t ICACHE_ENTRIES = 1024;
+  static const reg_t ICACHE_ENTRIES = 8;
 
   inline size_t icache_index(reg_t addr)
   {
@@ -310,22 +310,26 @@ class mmu_t
 
   inline icache_entry_t* refill_icache(reg_t addr, icache_entry_t* entry)
   {
-    insn_bits_t insn = fetch_insn_parcel(addr);
+    insn_bits_t insn;
+    unsigned length;
 
-    int length = insn_length(insn);
+    if (auto [tlb_hit, host_addr, paddr] = access_tlb(tlb_insn, addr);
+        tlb_hit && ((addr % PGSIZE) + sizeof(insn_bits_t) <= PGSIZE)) {
+      insn = from_le(*(insn_parcel_t*)host_addr);
+      length = insn_length(insn);
 
-    if (likely(length == 4)) {
-      insn |= (insn_bits_t)fetch_insn_parcel(addr + 2) << 16;
-    } else if (length == 2) {
-      // entire instruction already fetched
-    } else if (length == 6) {
-      insn |= (insn_bits_t)fetch_insn_parcel(addr + 2) << 16;
-      insn |= (insn_bits_t)fetch_insn_parcel(addr + 4) << 32;
+      for (unsigned pos = sizeof(insn_parcel_t); unlikely(pos < length); pos += sizeof(insn_parcel_t)) {
+        insn |= (insn_bits_t)from_le(*(insn_parcel_t*)(host_addr + pos)) << (8 * pos);
+        length = insn_length(insn);
+      }
     } else {
-      static_assert(sizeof(insn_bits_t) == 8, "insn_bits_t must be uint64_t");
-      insn |= (insn_bits_t)fetch_insn_parcel(addr + 2) << 16;
-      insn |= (insn_bits_t)fetch_insn_parcel(addr + 4) << 32;
-      insn |= (insn_bits_t)fetch_insn_parcel(addr + 6) << 48;
+      insn = fetch_insn_parcel(addr);
+      length = insn_length(insn);
+
+      for (unsigned pos = sizeof(insn_parcel_t); unlikely(pos < length); pos += sizeof(insn_parcel_t)) {
+        insn |= fetch_insn_parcel(addr + pos) << (8 * pos);
+        length = insn_length(insn);
+      }
     }
 
     insn_fetch_t fetch = {proc->decode_insn(insn), insn};
@@ -372,7 +376,10 @@ class mmu_t
   }
 
   void flush_tlb();
+  void flush_itlb_paddr(reg_t paddr);
+  void flush_stlb_paddr(reg_t paddr);
   void flush_icache();
+  void flush_icache_vpn(reg_t vpn);
 
   void register_memtracer(memtracer_t*);
 

riscv/processor.h

@@ -414,9 +414,6 @@ class processor_t : public abstract_device_t
   static const size_t OPCODE_CACHE_SIZE = 4095;
   opcode_cache_entry_t opcode_cache[OPCODE_CACHE_SIZE];
 
-  unsigned ziccid_flush_count = 0;
-  static const unsigned ZICCID_FLUSH_PERIOD = 10;
-
   void take_pending_interrupt() { take_interrupt(state.mip->read() & state.mie->read()); }
   void take_interrupt(reg_t mask); // take first enabled interrupt in mask
   void take_trap(trap_t& t, reg_t epc); // take an exception