On 5/27/25 11:42 PM, Richard Henderson wrote:
On 5/27/25 21:45, Pierrick Bouvier wrote:
On 5/24/25 7:40 AM, Richard Henderson wrote:
When we moved TLB_MMIO and TLB_DISCARD_WRITE to TLB_SLOW_FLAGS_MASK,
we failed to update atomic_mmu_lookup to properly reconstruct flags.
Fixes: 24b5e0fdb543 ("include/exec: Move TLB_MMIO, TLB_DISCARD_WRITE to slow
flags")
Reported-by: Jonathan Cameron <jonathan.came...@huawei.com>
Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
---
accel/tcg/cputlb.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 5f6d7c601c..86d0deb08c 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
[...]
@@ -1882,13 +1886,12 @@ 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)) {
notdirty_write(cpu, addr, size, full, retaddr);
}
- if (unlikely(tlb_addr & TLB_FORCE_SLOW)) {
+ if (unlikely(tlb_addr & TLB_WATCHPOINT)) {
int wp_flags = 0;
if (full->slow_flags[MMU_DATA_STORE] & TLB_WATCHPOINT) {
@@ -1897,10 +1900,8 @@ static void *atomic_mmu_lookup(CPUState *cpu, vaddr addr,
MemOpIdx oi,
if (full->slow_flags[MMU_DATA_LOAD] & TLB_WATCHPOINT) {
wp_flags |= BP_MEM_READ;
}
- if (wp_flags) {
- cpu_check_watchpoint(cpu, addr, size,
- full->attrs, wp_flags, retaddr);
- }
+ cpu_check_watchpoint(cpu, addr, size,
+ full->attrs, wp_flags, retaddr);
}
return hostaddr;
The watchpoint part is an additional cleanup, (BP_MEM_READ or BP_MEM_WRITE
implies
TLB_WATCHPOINT is set). No problem to include it though, it might just be
confusing for
the reviewer.
The watchpoint cleanup is required, since I remove TLB_FORCE_SLOW from the
flags. I
suppose *that* isn't strictly necessary, but it's what we do elsewhere while
combining
"fast" and slow_flags.
Yes! I was just referring to the last part where you remove if
(wp_flags) but kept the whole diff chunk for convenience.
r~
