Re: [PATCH 0/5] target/riscv: Enhanced ISA extension checks

[Alistair Francis <alistair23@gmail.com>]

On Fri, May 13, 2022 at 7:46 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> Hello,
>
> This is another patchset for RISC-V ISA extension / feature handling.
>
> Aside from coding style fix / refactoring patch (PATCH 1 and 5), this
> patchset contains two changes:
>
>
>
> 1. "G" extension handling
>
> 1.A. "G" extension expansion (PATCH 3)
>
> On ISA version 20190608 or later, "G" expands to "IMAFD_Zicsr_Zifencei",
> not just "IMAFD" (this is because "Zicsr" and "Zifencei" extensions are
> splitted from "I").  Because both "Zicsr" and "Zifencei" are enabled by
> default, it should be safe to change "G" extension expansion.
>
> 1.B. Disable "G" by default (PATCH 2)
>
> This seems quite odd but I have a good reason.  While "G" is enabled by
> default, all "I", "M", "A", "F", "D", "Zicsr" and "Zifencei" are also
> enabled by default, making default "G" expansion useless.
>
> There's more.  If we want to change detailed configuration of a RV32/RV64
> CPU and want to disable some, for example, "F" and "D", we must also
> disable "G".  This is obviously bloat.
>
> This doesn't work:
>     -cpu rv64,f=off,d=off
>
> This does work (but bloat):
>     -cpu rv64,g=off,f=off,d=off
>
> Disabling "G" suppresses such problem and mostly harmless, too.
>
>
>
> 2. Floating point arithmetic consistency (PATCH 4)
>
> With -cpu option, we can configure details of RISC-V CPU emulated by QEMU.
> However, we can set some invalid combinations that would make FP arithmetic
> effectively unusable (and invalid on RISC-V ISA specification).
>
> -   F requires Zicsr
> -   Zfinx requires Zicsr
> -   Zfh/Zfhmin require F
> -   D requires F
> -   V requires D
>
> Reproducing this kind of problem requires manually disabling certain
> extensions (because all "Zicsr", "F" and "D" are enabled by default).  So,
> I consider that just making an error message (when unsatisfied) should be
> enough, not implying related extensions like "zk*" properties.
>
>
> Note that this checking only works on any, rv32 and rv64.  On other CPUs
> (for example, sifive-u54), it sets nonzero `misa' value on corresponding
> `set_misa' call (c.f. in rv64_sifive_u_cpu_init in target/riscv/cpu.c) and
> consistency checks are skipped (because nonzero `misa' value is set prior
> to RISC-V CPU realization process).
>
> I think we generally use generic "rv32" or "rv64" on heavy customizing so I
> don't think this is not a big problem.  Still, we could fix this later
> (e.g. by setting properties on CPU init function or by checking some
> consistency problems even if previously-set `misa' is nonzero).
>
>
>
>
> Tsukasa OI (5):
>   target/riscv: Fix "G" extension expansion typing
>   target/riscv: Disable "G" by default
>   target/riscv: Change "G" expansion
>   target/riscv: FP extension requirements
>   target/riscv: Move/refactor ISA extension checks

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  target/riscv/cpu.c | 62 +++++++++++++++++++++++++++++++++-------------
>  1 file changed, 45 insertions(+), 17 deletions(-)
>
>
> base-commit: 178bacb66d98d9ee7a702b9f2a4dfcd88b72a9ab
> --
> 2.34.1
>