Re: [PATCH 1/1] target/riscv: Remove redundant insn length check for zama16b

[Richard Henderson]

On 7/23/24 15:29, LIU Zhiwei wrote:
> The more detailed information about its meaning is in priviledged 1.13 specification. More 
> exactly, in 3.6.4. Misaligned Atomicity Granule PMA.
>
> The specification said:
>
> "The misaligned atomicity granule PMA applies only to AMOs, loads and stores 
> defined in the base
> ISAs, and loads and stores of no more than MXLEN bits defined in the F, D, and 
> Q extensions. For an
> instruction in that set, if all accessed bytes lie within the same misaligned 
> atomicity granule, the
> instruction will not raise an exception for reasons of address alignment, and 
> the instruction will give
> rise to only one memory operation for the purposes of RVWMO—i.e., it will execute 
> atomically."
>
> That's the reason why I do not apply zama16b to compressed instructions.
Given the non-specificity of this paragraph, I think not specifically calling out 
compressed forms of the base ISA is simply a documentation error.  In general, the 
compressed ISA is supposed to be a smaller encoding of the exact same instruction as the 
standard ISA.

However!  It does explicitly say "no more than MXLEN bits", which means that an RV32/RV64 
check is appropriate for FLD/FSD, since MXLEN may be less than 64.

In addition, your change for AMOs is incomplete.  From the text:

  If a misaligned AMO accesses a region that does not specify a misaligned
  atomicity granule PMA, or if not all accessed bytes lie within the same
  misaligned atomicity granule, then an exception is raised.

The second clause corresponds exactly with the Arm FEAT_LSE2.
See check_lse2_align in target/arm/tcg/translate-a64.c.


r~


PS: The first clause is similar to Arm access to pages marked as Device memory, for which 
all misaligned accesses trap.  I didn't dig deep enough to see how PMAs are defined to 
suggest how that might be applied.