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

Mon, 22 Jun 2026 06:38:41 -0700 

On 21/6/26 14:47, GavinQin wrote:

Thanks Philippe.

In my tree CPURISCVState::badaddr is target_ulong,


But this list is for public mainstream QEMU... Where your patch
is going to land, and many developers are going to use it...

This field 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

You should base your patches on the latest QEMU /master branch.


so I'll keep fault_addr as
target_ulong to match it. I'll add the else for readability in v3 and keep your
Reviewed-by tag.

Thanks,
ZhengXiang




GavinQin
[email protected]






          原始邮件
发件人：Philippe Mathieu-Daudé <[email protected]&gt; 
发件时间：2026年6月18日 22:36
收件人：ZhengXiang Qin <[email protected]&gt;, qemu-devel 
<[email protected]&gt;
抄送：qemu-riscv <[email protected]&gt;, Daniel Henrique Barboza <[email protected]&gt;, Chao Liu 
<[email protected]&gt;, Palmer Dabbelt <[email protected]&gt;, Alistair Francis 
<[email protected]&gt;, Weiwei Li <[email protected]&gt;, Liu Zhiwei 
<[email protected]&gt;
主题：Re: [PATCH v2] target/riscv: raise access fault for Zicbom MMIO-like accesses



        On&nbsp;18/6/26&nbsp;16:06,&nbsp;ZhengXiang&nbsp;Qin&nbsp;wrote:
&gt;&nbsp;check_zicbom_access()&nbsp;currently&nbsp;treats&nbsp;any&nbsp;probe_access_flags()&nbsp;result
&gt;&nbsp;other&nbsp;than&nbsp;TLB_INVALID_MASK&nbsp;as&nbsp;a&nbsp;successful&nbsp;access.&nbsp;However,&nbsp;a&nbsp;result
&gt;&nbsp;with&nbsp;TLB_MMIO&nbsp;means&nbsp;that&nbsp;the&nbsp;target&nbsp;is&nbsp;MMIO-like&nbsp;and&nbsp;should&nbsp;not&nbsp;be
&gt;&nbsp;treated&nbsp;as&nbsp;a&nbsp;normal&nbsp;cache&nbsp;block&nbsp;management&nbsp;target.
&gt;&nbsp;
&gt;&nbsp;Raise&nbsp;a&nbsp;store/AMO&nbsp;access&nbsp;fault&nbsp;for&nbsp;Zicbom&nbsp;accesses&nbsp;to&nbsp;MMIO-like&nbsp;regions
&gt;&nbsp;so&nbsp;that&nbsp;cbo.clean,&nbsp;cbo.flush&nbsp;and&nbsp;cbo.inval&nbsp;do&nbsp;not&nbsp;silently&nbsp;succeed&nbsp;on
&gt;&nbsp;non-cacheable/MMIO-like&nbsp;memory.
&gt;&nbsp;
&gt;&nbsp;Resolves:&nbsp;https://gitlab.com/qemu-project/qemu/-/issues/3501
&gt;&nbsp;Reviewed-by:&nbsp;Daniel&nbsp;Henrique&nbsp;Barboza&nbsp;<[email protected]&gt;
&gt;&nbsp;Reviewed-by:&nbsp;Chao&nbsp;Liu&nbsp;<[email protected]&gt;
&gt;&nbsp;Signed-off-by:&nbsp;ZhengXiang&nbsp;Qin&nbsp;<[email protected]&gt;
&gt;&nbsp;---
&gt;&nbsp;Changes&nbsp;since&nbsp;v1:
&gt;&nbsp;-&nbsp;Use&nbsp;a&nbsp;bit&nbsp;test&nbsp;for&nbsp;TLB_MMIO&nbsp;since&nbsp;probe_access_flags()&nbsp;returns&nbsp;a&nbsp;bitmask.
&gt;&nbsp;-&nbsp;Keep&nbsp;Reviewed-by&nbsp;tags&nbsp;from&nbsp;v1.
&gt;&nbsp;
&gt;&nbsp;&nbsp;&nbsp;target/riscv/op_helper.c&nbsp;|&nbsp;5&nbsp;+++++
&gt;&nbsp;&nbsp;&nbsp;1&nbsp;file&nbsp;changed,&nbsp;5&nbsp;insertions(+)
&gt;&nbsp;
&gt;&nbsp;diff&nbsp;--git&nbsp;a/target/riscv/op_helper.c&nbsp;b/target/riscv/op_helper.c
&gt;&nbsp;index&nbsp;81873014cb..f23b060585&nbsp;100644
&gt;&nbsp;---&nbsp;a/target/riscv/op_helper.c
&gt;&nbsp;+++&nbsp;b/target/riscv/op_helper.c
&gt;&nbsp;@@&nbsp;-218,6&nbsp;+218,7&nbsp;@@&nbsp;static&nbsp;void&nbsp;check_zicbom_access(CPURISCVState&nbsp;*env,
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;void&nbsp;*phost;
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;int&nbsp;ret;
&gt;&nbsp;&nbsp;&nbsp;
&gt;&nbsp;+&nbsp;&nbsp;&nbsp;&nbsp;target_ulong&nbsp;fault_addr&nbsp;=&nbsp;address;

CPURISCVState::badaddr&nbsp;is&nbsp;of&nbsp;type&nbsp;uint64_t,&nbsp;can&nbsp;we&nbsp;use&nbsp;that&nbsp;here&nbsp;instead
of&nbsp;target_ulong?

&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/*&nbsp;Mask&nbsp;off&nbsp;low-bits&nbsp;to&nbsp;align-down&nbsp;to&nbsp;the&nbsp;cache-block.&nbsp;*/
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;address&nbsp;&amp;=&nbsp;~(cbomlen&nbsp;-&nbsp;1);
&gt;&nbsp;&nbsp;&nbsp;
&gt;&nbsp;@@&nbsp;-235,6&nbsp;+236,10&nbsp;@@&nbsp;static&nbsp;void&nbsp;check_zicbom_access(CPURISCVState&nbsp;*env,
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;*/
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;ret&nbsp;=&nbsp;probe_access_flags(env,&nbsp;address,&nbsp;cbomlen,&nbsp;MMU_DATA_LOAD,
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;mmu_idx,&nbsp;true,&nbsp;&amp;phost,&nbsp;ra);
&gt;&nbsp;+&nbsp;&nbsp;&nbsp;&nbsp;if&nbsp;(ret&nbsp;&amp;&nbsp;TLB_MMIO)&nbsp;{
&gt;&nbsp;+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;env-&gt;badaddr&nbsp;=&nbsp;fault_addr;
&gt;&nbsp;+&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;riscv_raise_exception(env,&nbsp;RISCV_EXCP_STORE_AMO_ACCESS_FAULT,&nbsp;ra);
&gt;&nbsp;+&nbsp;&nbsp;&nbsp;&nbsp;}

Maybe&nbsp;adding&nbsp;'else'&nbsp;make&nbsp;this&nbsp;code&nbsp;path&nbsp;more&nbsp;readable.

&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if&nbsp;(ret&nbsp;!=&nbsp;TLB_INVALID_MASK)&nbsp;{
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/*&nbsp;Success:&nbsp;readable&nbsp;*/
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;return;

Feel&nbsp;free&nbsp;to&nbsp;ignore&nbsp;these&nbsp;nitpicking&nbsp;comments.

Reviewed-by:&nbsp;Philippe&nbsp;Mathieu-Daudé&nbsp;<[email protected]&gt;

Reply via email to