On 15/8/26 22:37, Richard Henderson wrote:
Signed-off-by: Richard Henderson <[email protected]>
---
include/accel/tcg/probe.h | 24 +++++++++++++++++-------
accel/tcg/cputlb.c | 15 ++++++++-------
accel/tcg/user-exec.c | 7 +++++--
semihosting/uaccess.c | 10 ++++------
target/arm/tcg/sve_helper.c | 5 ++++-
target/riscv/tcg/op_helper.c | 4 ++--
target/riscv/tcg/vector_helper.c | 21 ++++++++++++---------
target/s390x/tcg/mem_helper.c | 4 ++--
8 files changed, 54 insertions(+), 36 deletions(-)
diff --git a/include/accel/tcg/probe.h b/include/accel/tcg/probe.h
index e3068a79de..06faf1269d 100644
--- a/include/accel/tcg/probe.h
+++ b/include/accel/tcg/probe.h
@@ -49,23 +49,33 @@ static inline void *probe_read(CPUArchState *env, vaddr
addr, int size,
/**
* probe_access_flags:
* @env: CPUArchState
- * @addr: guest virtual address to look up
- * @size: size of the access
+ * @addr: virtual address
+ * @first: beginning of virtual address range
+ * @last: end of virtual address range
* @access_type: read, write or execute permission
* @mmu_idx: MMU index to use for lookup
* @nonfault: suppress the fault
* @phost: return value for host address
* @retaddr: return address for unwinding
*
- * Similar to probe_access, loosely returning the TLB_FLAGS_MASK for
- * the page, and storing the host address for RAM in @phost.
+ * Probe an access for [@first, @last], where @addr is somewhere
+ * in that range. Normally @addr == @first, but some targets have
+ * accesses which are forcibly aligned after an initial fault for an
+ * inaccessible page (e.g. Arm DC_ZVA, where [@first, @last] will be
+ * the bounds of the cacheline containing @addr).
+ *
+ * If the access does not satisfy @access_type:
+ * - if @nonfault is false, raise an exception at @addr
+ * - otherwise return TLB_INVALID_MASK.
+ *
+ * Otherwise, return the TLB_FLAGS_MASK for the page, and set @phost:
+ * - host address for @addr, if direct host accesses are allowed,
+ * - otherwise NULL.
This If/Otherwise block sounds like implementation detail leaking into
documentation API contract, but I don't mind.
*
- * If @nonfault is set, do not raise an exception but return TLB_INVALID_MASK.
* Do not handle watchpoints, but include TLB_WATCHPOINT in the returned
flags.
* Do handle clean pages, so exclude TLB_NOTDIRY from the returned flags.
- * For simplicity, all "mmio-like" flags are folded to TLB_MMIO.
*/
-int probe_access_flags(CPUArchState *env, vaddr addr, int size,
+int probe_access_flags(CPUArchState *env, vaddr addr, vaddr first, vaddr last,
MMUAccessType access_type, int mmu_idx,
bool nonfault, void **phost, uintptr_t retaddr);
diff --git a/target/riscv/tcg/vector_helper.c b/target/riscv/tcg/vector_helper.c
index b7b3805a6d..87f2196bc7 100644
--- a/target/riscv/tcg/vector_helper.c
+++ b/target/riscv/tcg/vector_helper.c
@@ -704,7 +704,7 @@ vext_ldff(void *vd, target_ulong base, CPURISCVState *env,
uint32_t esz = 1 << log2_esz;
uint32_t msize = nf * esz;
uint32_t vma = vext_vma(desc);
- target_ulong addr, last, last_in_page, page_split, elems;
+ target_ulong addr, last, last_in_page, page_split, elems, adj, adj_last;
MemOpIdx oi = vext_oi(desc, log2_esz);
int mmu_index = get_mmuidx(oi);
bool first_active;
@@ -759,10 +759,10 @@ vext_ldff(void *vd, target_ulong base, CPURISCVState *env,
* Test whether the first page is accessible.
* If the first element is active, it must succeed.
*/
- flags = probe_access_flags(env, adjust_addr(env, addr),
- MIN(last, last_in_page) - addr + 1,
- MMU_DATA_LOAD, mmu_index, !first_active,
- &host, ra);
+ adj = adjust_addr(env, addr);
+ adj_last = MIN(last, last_in_page) - (addr - adj);
+ flags = probe_access_flags(env, adj, adj, adj_last, MMU_DATA_LOAD,
+ mmu_index, !first_active, &host, ra);
Something in this function (on the pre-existing code) bugged me,
confirmed by your cover letter (I haven't looked at the RISCV
preliminary series).
/* Get number of complete elements in the first page. */
elems = MIN(page_split / msize, vl - i);
@@ -853,8 +853,10 @@ vext_ldff(void *vd, target_ulong base, CPURISCVState *env,
* We have not yet advanced addr to the next page.
*/
target_ulong next_page = addr + page_split;
- flags |= probe_access_flags(env, adjust_addr(env, next_page),
- last - next_page + 1, MMU_DATA_LOAD,
+
+ adj = adjust_addr(env, next_page);
+ adj_last = last - (next_page - adj);
+ flags |= probe_access_flags(env, adj, adj, adj_last, MMU_DATA_LOAD,
mmu_index, true, &host, ra);
/* Stop if invalid (unmapped) or mmio (transaction may fail). */
@@ -873,8 +875,9 @@ vext_ldff(void *vd, target_ulong base, CPURISCVState *env,
host += addr - next_page;
}
} else {
- flags = probe_access_flags(env, adjust_addr(env, addr),
- last - addr + 1, MMU_DATA_LOAD,
+ adj = adjust_addr(env, addr);
+ adj_last = last - (addr - adj);
+ flags = probe_access_flags(env, adj, adj, adj_last, MMU_DATA_LOAD,
mmu_index, true, &host, ra);
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
