Re: [PATCH v3] target/riscv: Enable Zicbo[m,z,p] instructions

[Richard Henderson <richard.henderson@linaro.org>]

On 2/11/22 03:48, Philipp Tomsich wrote:
>     -lq       ............   ..... 010 ..... 0001111 @i
>     +{
>     +  [
>     +    # *** RV32 Zicbom Standard Extension ***
>     +    cbo_clean  0000000 00001 ..... 010 00000 0001111 @sfence_vm
>     +    cbo_flush  0000000 00010 ..... 010 00000 0001111 @sfence_vm
>     +    cbo_inval  0000000 00000 ..... 010 00000 0001111 @sfence_vm
>     +
>     +    # *** RV32 Zicboz Standard Extension ***
>     +    cbo_zero   0000000 00100 ..... 010 00000 0001111 @sfence_vm
>     +  ]
>     +
>     +  # *** RVI128 lq ***
>     +  lq       ............   ..... 010 ..... 0001111 @i
>     +}

...
>     +#define REQUIRE_ZICBOM(ctx) do {               \
>     +    if (!RISCV_CPU(ctx->cs)->cfg.ext_icbom) {  \
>     +        return false;                          \
>     +    }                                          \
>     +} while (0)
> 
> 
> The exception semantics seem to be broken here: if Zicbom is not implemented, but the 
> requirements for lq (i.e. rv128) are satisfied, then this needs to be passed on to lq: "lq 
> zero, 0(rs1)" is still expected to raise exceptions based on the permissions for the 
> address at 0(rs1).
> 
> There are multiple ways to do this, including:
> 1) perform a tail-call to trans_lq, in case Zicbom is not enabled (instead of just 
> returning false);
> 2) use the table-based dispatch (added for XVentanaCondOps) and hook a Zicbom 
> disptacher before the RVI dispatcher: if Zicbom then falls through, the RVI dispatcher 
> would drop into trans_lq;

No, returning false will cause the next pattern in the { } group to be matched.  No need 
for other workarounds.


r~