On 2023/4/18 15:36, Richard Henderson wrote:
On 4/18/23 09:18, Richard Henderson wrote:
- /*
- * 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;
I missed the original patch, but this is definitely wrong.
Clearing this bit locally (!) is correct because we want to inform
the caller of probe_access_* that the access is valid. We know that
it is valid because we have just queried tlb_fill (and thus for
riscv, PMP).
Clearing the bit locally does *not* cause the tlb entry to be cached
-- the INVALID bit is still set within the tlb entry. The next access
will again go through tlb_fill.
What is the original problem you are seeing? The commit message does
not say.
From
https://lore.kernel.org/qemu-devel/3ace9e9e-91cf-36e6-a18f-494fd44df...@iscas.ac.cn/
I see that it is a problem with execution.
Yeah. I found this problem in PMP check for instruction fetch.
By eye, it appears that get_page_addr_code_hostp needs adjustment, e.g.
(void)probe_access_internal(env, addr, 1, MMU_INST_FETCH,
cpu_mmu_index(env, true), false, &p,
&full, 0);
if (p == NULL) {
return -1;
}
+ if (full->lg_page_size < TARGET_PAGE_BITS) {
+ return -1;
+ }
if (hostp) {
*hostp = p;
}
It seems like we could do slightly better than this, perhaps by
single-stepping through such a page, but surely this edge case is so
uncommon as to not make it worthwhile to consider.
OK. I'll update and test it later.
Regards,
Weiwei Li
r~