On 18/6/26 16:06, ZhengXiang Qin wrote:
check_zicbom_access() currently treats any probe_access_flags() result
other than TLB_INVALID_MASK as a successful access. However, a result
with TLB_MMIO means that the target is MMIO-like and should not be
treated as a normal cache block management target.
Raise a store/AMO access fault for Zicbom accesses to MMIO-like regions
so that cbo.clean, cbo.flush and cbo.inval do not silently succeed on
non-cacheable/MMIO-like memory.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3501
Reviewed-by: Daniel Henrique Barboza <[email protected]>
Reviewed-by: Chao Liu <[email protected]>
Signed-off-by: ZhengXiang Qin <[email protected]>
---
Changes since v1:
- Use a bit test for TLB_MMIO since probe_access_flags() returns a bitmask.
- Keep Reviewed-by tags from v1.
target/riscv/op_helper.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 81873014cb..f23b060585 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -218,6 +218,7 @@ static void check_zicbom_access(CPURISCVState *env,
void *phost;
int ret;
+ target_ulong fault_addr = address;
CPURISCVState::badaddr is of type uint64_t, can we use that here instead
of target_ulong?
/* Mask off low-bits to align-down to the cache-block. */
address &= ~(cbomlen - 1);
@@ -235,6 +236,10 @@ static void check_zicbom_access(CPURISCVState *env,
*/
ret = probe_access_flags(env, address, cbomlen, MMU_DATA_LOAD,
mmu_idx, true, &phost, ra);
+ if (ret & TLB_MMIO) {
+ env->badaddr = fault_addr;
+ riscv_raise_exception(env, RISCV_EXCP_STORE_AMO_ACCESS_FAULT, ra);
+ }
Maybe adding 'else' make this code path more readable.
if (ret != TLB_INVALID_MASK) {
/* Success: readable */
return;
Feel free to ignore these nitpicking comments.
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
