On 10/9/24 08:08, Richard Henderson wrote:
The INVALID bit should only be auto-cleared when we have
just called tlb_fill, not along the victim_tlb_hit path.

In atomic_mmu_lookup, rename tlb_addr to flags, as that
is what we're actually carrying around.

Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
---
  accel/tcg/cputlb.c | 33 ++++++++++++++++++++++-----------
  1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 6773874f2d..fd8da8586f 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1657,7 +1657,7 @@ static bool mmu_lookup1(CPUState *cpu, MMULookupPageData 
*data, MemOp memop,
      uint64_t tlb_addr = tlb_read_idx(entry, access_type);
      bool maybe_resized = false;
      CPUTLBEntryFull *full;
-    int flags;
+    int flags = TLB_FLAGS_MASK & ~TLB_FORCE_SLOW;
/* If the TLB entry is for a different page, reload and try again. */
      if (!tlb_hit(tlb_addr, addr)) {
@@ -1668,8 +1668,14 @@ static bool mmu_lookup1(CPUState *cpu, MMULookupPageData 
*data, MemOp memop,
              maybe_resized = true;
              index = tlb_index(cpu, mmu_idx, addr);
              entry = tlb_entry(cpu, mmu_idx, addr);
+            /*
+             * With PAGE_WRITE_INV, we set TLB_INVALID_MASK immediately,
+             * to force the next access through tlb_fill.  We've just
+             * called tlb_fill, so we know that this entry *is* valid.
+             */
+            flags &= ~TLB_INVALID_MASK;
          }
-        tlb_addr = tlb_read_idx(entry, access_type) & ~TLB_INVALID_MASK;
+        tlb_addr = tlb_read_idx(entry, access_type);
      }
full = &cpu->neg.tlb.d[mmu_idx].fulltlb[index];
@@ -1819,10 +1825,10 @@ static void *atomic_mmu_lookup(CPUState *cpu, vaddr 
addr, MemOpIdx oi,
      MemOp mop = get_memop(oi);
      uintptr_t index;
      CPUTLBEntry *tlbe;
-    vaddr tlb_addr;
      void *hostaddr;
      CPUTLBEntryFull *full;
      bool did_tlb_fill = false;
+    int flags;
tcg_debug_assert(mmu_idx < NB_MMU_MODES); @@ -1833,8 +1839,8 @@ static void *atomic_mmu_lookup(CPUState *cpu, vaddr addr, MemOpIdx oi,
      tlbe = tlb_entry(cpu, mmu_idx, addr);
/* Check TLB entry and enforce page permissions. */
-    tlb_addr = tlb_addr_write(tlbe);
-    if (!tlb_hit(tlb_addr, addr)) {
+    flags = TLB_FLAGS_MASK;
+    if (!tlb_hit(tlb_addr_write(tlbe), addr)) {
          if (!victim_tlb_hit(cpu, mmu_idx, index, MMU_DATA_STORE,
                              addr & TARGET_PAGE_MASK)) {
              tlb_fill_align(cpu, addr, MMU_DATA_STORE, mmu_idx,
@@ -1842,8 +1848,13 @@ static void *atomic_mmu_lookup(CPUState *cpu, vaddr 
addr, MemOpIdx oi,
              did_tlb_fill = true;
              index = tlb_index(cpu, mmu_idx, addr);
              tlbe = tlb_entry(cpu, mmu_idx, addr);
+            /*
+             * With PAGE_WRITE_INV, we set TLB_INVALID_MASK immediately,
+             * to force the next access through tlb_fill.  We've just
+             * called tlb_fill, so we know that this entry *is* valid.
+             */
+            flags &= ~TLB_INVALID_MASK;
          }
-        tlb_addr = tlb_addr_write(tlbe) & ~TLB_INVALID_MASK;
      }
/*
@@ -1879,11 +1890,11 @@ static void *atomic_mmu_lookup(CPUState *cpu, vaddr 
addr, MemOpIdx oi,
          goto stop_the_world;
      }
- /* Collect tlb flags for read. */
-    tlb_addr |= tlbe->addr_read;
+    /* Collect tlb flags for read and write. */
+    flags &= tlbe->addr_read | tlb_addr_write(tlbe);
/* Notice an IO access or a needs-MMU-lookup access */
-    if (unlikely(tlb_addr & (TLB_MMIO | TLB_DISCARD_WRITE))) {
+    if (unlikely(flags & (TLB_MMIO | TLB_DISCARD_WRITE))) {
          /* There's really nothing that can be done to
             support this apart from stop-the-world.  */
          goto stop_the_world;
@@ -1892,11 +1903,11 @@ static void *atomic_mmu_lookup(CPUState *cpu, vaddr 
addr, MemOpIdx oi,
      hostaddr = (void *)((uintptr_t)addr + tlbe->addend);
      full = &cpu->neg.tlb.d[mmu_idx].fulltlb[index];
- if (unlikely(tlb_addr & TLB_NOTDIRTY)) {
+    if (unlikely(flags & TLB_NOTDIRTY)) {
          notdirty_write(cpu, addr, size, full, retaddr);
      }
- if (unlikely(tlb_addr & TLB_FORCE_SLOW)) {
+    if (unlikely(flags & TLB_FORCE_SLOW)) {
          int wp_flags = 0;
if (full->slow_flags[MMU_DATA_STORE] & TLB_WATCHPOINT) {

Reviewed-by: Pierrick Bouvier <pierrick.bouv...@linaro.org>

Reply via email to