target/riscv: raise access fault for Zicbom MMIO-like accesses

PATCH Applied ZhengXiang Qin 4 participants 6 messages Jun 21 – Nov 10 RISC-V

Share & tools

ZhengXiang Qin

author

2026-06-21 12:47 UTC #0 lore raw

PATCH v3 1/1 target/riscv: raise access fault for Zicbom MMIO-like accesses

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]>

Reviewed-by Philippe Mathieu-Daudé <[email protected]>

Signed-off-by ZhengXiang Qin <[email protected]>

---
Changes since v2:
- Add else before the success path for readability.
- Keep fault_addr as target_ulong since CPURISCVState::badaddr is target_ulong.
- Add Philippe's Reviewed-by tag.

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 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

target/riscv/op_helper.c +5 -1

index 81873014cb..50fd1b73ea 100644
@@ -218,6 +218,7 @@ static void check_zicbom_access(CPURISCVState *env,
     void *phost;
     int ret;
 
+    target_ulong fault_addr = address;
     /* Mask off low-bits to align-down to the cache-block. */
     address &= ~(cbomlen - 1);

@@ -235,7 +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_INVALID_MASK) {
+    if (ret & TLB_MMIO) {
+        env->badaddr = fault_addr;
+        riscv_raise_exception(env, RISCV_EXCP_STORE_AMO_ACCESS_FAULT, ra);
+    } else if (ret != TLB_INVALID_MASK) {
         /* Success: readable */
         return;
     }

Signature · 1 line

2.43.0

Alistair Francis

Reviewed-by on v3 1/1

2026-06-22 03:02 UTC #1 lore raw

Quoted 10 lines · ZhengXiang Qin

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.

Quoted 5 lines

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3501
Reviewed-by Daniel Henrique Barboza <[email protected]>
Reviewed-by Chao Liu <[email protected]>
Reviewed-by Philippe Mathieu-Daudé <[email protected]>
Signed-off-by ZhengXiang Qin <[email protected]>

Reviewed-by Alistair Francis <[email protected]>

Alistair

Quoted 41 lines

---
Changes since v2:
- Add else before the success path for readability.
- Keep fault_addr as target_ulong since CPURISCVState::badaddr is target_ulong.
- Add Philippe's Reviewed-by tag.

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 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

target/riscv/op_helper.c +5 -2

index 81873014cb..50fd1b73ea 100644
@@ -218,6 +218,7 @@ static void check_zicbom_access(CPURISCVState *env,
     void *phost;
     int ret;
 
+    target_ulong fault_addr = address;
     /* Mask off low-bits to align-down to the cache-block. */
     address &= ~(cbomlen - 1);

@@ -235,7 +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_INVALID_MASK) {
+    if (ret & TLB_MMIO) {
+        env->badaddr = fault_addr;
+        riscv_raise_exception(env, RISCV_EXCP_STORE_AMO_ACCESS_FAULT, ra);
+    } else if (ret != TLB_INVALID_MASK) {
         /* Success: readable */
         return;
     }
--

2.43.0

Alistair Francis

Applied

2026-06-22 03:03 UTC #2 lore raw

In reply to ZhengXiang Qin

Quoted 10 lines · ZhengXiang Qin

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.

Quoted 5 lines

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3501
Reviewed-by Daniel Henrique Barboza <[email protected]>
Reviewed-by Chao Liu <[email protected]>
Reviewed-by Philippe Mathieu-Daudé <[email protected]>
Signed-off-by ZhengXiang Qin <[email protected]>

Thanks!

Applied to riscv-to-apply.next

Alistair

Quoted 41 lines

---
Changes since v2:
- Add else before the success path for readability.
- Keep fault_addr as target_ulong since CPURISCVState::badaddr is target_ulong.
- Add Philippe's Reviewed-by tag.

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 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

target/riscv/op_helper.c +5 -2

index 81873014cb..50fd1b73ea 100644
@@ -218,6 +218,7 @@ static void check_zicbom_access(CPURISCVState *env,
     void *phost;
     int ret;
 
+    target_ulong fault_addr = address;
     /* Mask off low-bits to align-down to the cache-block. */
     address &= ~(cbomlen - 1);

@@ -235,7 +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_INVALID_MASK) {
+    if (ret & TLB_MMIO) {
+        env->badaddr = fault_addr;
+        riscv_raise_exception(env, RISCV_EXCP_STORE_AMO_ACCESS_FAULT, ra);
+    } else if (ret != TLB_INVALID_MASK) {
         /* Success: readable */
         return;
     }
--

2.43.0

Philippe Mathieu-Daudé

2026-06-22 13:39 UTC #3 lore raw

Hi Alistair,

Quoted 18 lines · Alistair Francis

Quoted 10 lines · ZhengXiang Qin
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.
Quoted 5 lines
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3501
Reviewed-by Daniel Henrique Barboza <[email protected]>
Reviewed-by Chao Liu <[email protected]>
Reviewed-by Philippe Mathieu-Daudé <[email protected]>
Signed-off-by ZhengXiang Qin <[email protected]>

Quoted 10 lines

Thanks!
Applied to riscv-to-apply.next
Alistair
Quoted 4 lines
---
Changes since v2:
- Add else before the success path for readability.
- Keep fault_addr as target_ulong since CPURISCVState::badaddr is target_ulong.

This is not true, the fields is uint64_t since this commit:

commit f2287c7020f36e2f13622d9635a968d6f6fb507d
Author: Anton Johansson <[email protected]>
Date: Wed May 20 14:53:43 2026 +0200

target/riscv: Fix size of badaddr and bins

Could you update the commit, or ask the contributor to repost, or clean on top, please?

Thanks,

Phil.

Quoted 37 lines

- Add Philippe's Reviewed-by tag.

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 | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)

target/riscv/op_helper.c +5 -2

index 81873014cb..50fd1b73ea 100644
@@ -218,6 +218,7 @@ static void check_zicbom_access(CPURISCVState *env,
      void *phost;
      int ret;
 
+    target_ulong fault_addr = address;
      /* Mask off low-bits to align-down to the cache-block. */
      address &= ~(cbomlen - 1);

@@ -235,7 +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_INVALID_MASK) {
+    if (ret & TLB_MMIO) {
+        env->badaddr = fault_addr;
+        riscv_raise_exception(env, RISCV_EXCP_STORE_AMO_ACCESS_FAULT, ra);
+    } else if (ret != TLB_INVALID_MASK) {
          /* Success: readable */
          return;
      }
--

2.43.0

Alistair Francis

2026-06-29 01:56 UTC #4 lore raw

Quoted 35 lines · Philippe Mathieu-Daudé

Hi Alistair,
Quoted 18 lines · Alistair Francis
Quoted 10 lines · ZhengXiang Qin
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.
Quoted 5 lines
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3501
Reviewed-by Daniel Henrique Barboza <[email protected]>
Reviewed-by Chao Liu <[email protected]>
Reviewed-by Philippe Mathieu-Daudé <[email protected]>
Signed-off-by ZhengXiang Qin <[email protected]>
Quoted 10 lines
Thanks!
Applied to riscv-to-apply.next
Alistair
Quoted 4 lines
---
Changes since v2:
- Add else before the success path for readability.
- Keep fault_addr as target_ulong since CPURISCVState::badaddr is target_ulong.
This is not true, the fields is uint64_t since this commit:

Quoted 8 lines

commit f2287c7020f36e2f13622d9635a968d6f6fb507d
Author: Anton Johansson <[email protected]>
Date: Wed May 20 14:53:43 2026 +0200
target/riscv: Fix size of badaddr and bins
Could you update the commit, or ask the contributor to repost, or clean on top, please?

Thanks for catching this. I have dropped the patch, please rebase on master and send a new version

Alistair

Daniel Henrique Barboza

2026-11-10 21:05 UTC #5 lore raw

Hi,

I'll send a v4. We have people asking for this fix in Gitlab.

Thanks,
Daniel

Quoted 50 lines · Alistair Francis

Quoted 35 lines · Philippe Mathieu-Daudé
Hi Alistair,
Quoted 18 lines · Alistair Francis
Quoted 10 lines · ZhengXiang Qin
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.
Quoted 5 lines
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3501
Reviewed-by Daniel Henrique Barboza <[email protected]>
Reviewed-by Chao Liu <[email protected]>
Reviewed-by Philippe Mathieu-Daudé <[email protected]>
Signed-off-by ZhengXiang Qin <[email protected]>
Quoted 10 lines
Thanks!
Applied to riscv-to-apply.next
Alistair
Quoted 4 lines
---
Changes since v2:
- Add else before the success path for readability.
- Keep fault_addr as target_ulong since CPURISCVState::badaddr is target_ulong.
This is not true, the fields is uint64_t since this commit:
Quoted 8 lines
commit f2287c7020f36e2f13622d9635a968d6f6fb507d
Author: Anton Johansson <[email protected]>
Date: Wed May 20 14:53:43 2026 +0200
target/riscv: Fix size of badaddr and bins
Could you update the commit, or ask the contributor to repost, or clean on top, please?
Thanks for catching this. I have dropped the patch, please rebase on master and send a new version
Alistair