1. Patchew
  2. QEMU
  3. View series

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

ZhengXiang Qin posted 1 patch 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/tencent._5FCA380CBF4A98C990C07BC04A9E748C438309@qq.com
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Chao Liu <chao.liu.zevorn@gmail.com>
target/riscv/op_helper.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
[PATCH v3] target/riscv: raise access fault for Zicbom MMIO-like accesses
Posted by ZhengXiang Qin 1 month ago 
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 <daniel.barboza@oss.qualcomm.com>
Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Signed-off-by: ZhengXiang Qin <qinzhengxiang@foxmail.com>
---
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(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 81873014cb..50fd1b73ea 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;
     /* 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
Re: [PATCH v3] target/riscv: raise access fault for Zicbom MMIO-like accesses
Posted by Alistair Francis 1 month ago 
On Sun, Jun 21, 2026 at 10:48 PM ZhengXiang Qin
<qinzhengxiang@foxmail.com> 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 <daniel.barboza@oss.qualcomm.com>
> Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> Signed-off-by: ZhengXiang Qin <qinzhengxiang@foxmail.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
> 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(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 81873014cb..50fd1b73ea 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;
>      /* 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
>
>
Re: [PATCH v3] target/riscv: raise access fault for Zicbom MMIO-like accesses
Posted by Philippe Mathieu-Daudé 1 month ago 
Hi Alistair,

On 22/6/26 05:03, Alistair Francis wrote:
> On Sun, Jun 21, 2026 at 10:48 PM ZhengXiang Qin
> <qinzhengxiang@foxmail.com> 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 <daniel.barboza@oss.qualcomm.com>
>> Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
>> Signed-off-by: ZhengXiang Qin <qinzhengxiang@foxmail.com>
> 
> Thanks!
> 
> Applied to riscv-to-apply.next
> 
> Alistair
> 
>> ---
>> 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 <anjo@rev.ng>
  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.

>> - 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(-)
>>
>> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
>> index 81873014cb..50fd1b73ea 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;
>>       /* 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
>>
>>
Re: [PATCH v3] target/riscv: raise access fault for Zicbom MMIO-like accesses
Posted by Alistair Francis 3 weeks, 6 days ago 
On Mon, Jun 22, 2026 at 11:39 PM Philippe Mathieu-Daudé
<philmd@oss.qualcomm.com> wrote:
>
> Hi Alistair,
>
> On 22/6/26 05:03, Alistair Francis wrote:
> > On Sun, Jun 21, 2026 at 10:48 PM ZhengXiang Qin
> > <qinzhengxiang@foxmail.com> 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 <daniel.barboza@oss.qualcomm.com>
> >> Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>
> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> >> Signed-off-by: ZhengXiang Qin <qinzhengxiang@foxmail.com>
> >
> > Thanks!
> >
> > Applied to riscv-to-apply.next
> >
> > Alistair
> >
> >> ---
> >> 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 <anjo@rev.ng>
>   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
Re: [PATCH v3] target/riscv: raise access fault for Zicbom MMIO-like accesses
Posted by Alistair Francis 1 month ago 
On Sun, Jun 21, 2026 at 10:48 PM ZhengXiang Qin
<qinzhengxiang@foxmail.com> 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 <daniel.barboza@oss.qualcomm.com>
> Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> Signed-off-by: ZhengXiang Qin <qinzhengxiang@foxmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> 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(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 81873014cb..50fd1b73ea 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;
>      /* 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
>
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.